Validating a login form

Agreaves

Below is my login script, but I need to validate the username and password before logging in the user.

<?php
session_start();

if(isset($_POST['login']))
{	
$username = $_POST['username'];
$password = $_POST['password'];
include("db_con.php");
$conn = mysqli_connect($host, $user, $pass, $dbase);
$sql = "SELECT username,password FROM members WHERE username= '$username' AND password=md5('$password')";
$result = mysqli_query($conn,$sql) or die ('Could not run select query');

if ($result > 0)
{
$sql = "UPDATE members SET logindate= NOW() WHERE username= '$username'";
$result2 = mysqli_query($conn,$sql) or die ('Could not run update query');
$message = "Logged in as $username";
}
else
{
$message = 'Please check the username and password you entered';
}
}
?>

NogDog

Looks like a reasonable start* (I didn't go over it in detail). Is it not working as expected, generating an error, or do you want to know what to do next, or what?

Except you need to escape user input before using it in your DB query (see [man]mysql_real_escape_string/man)

Agreaves

I have changed completely because it wasn't working as I expected but here is where im at right now, this is working up to a point the way I want it so far. I need help setting up another message if the user failed to put in the incorrect username or password and also to run a update query the update the login date and time.

<?php
session_start();

if(isset($_POST['login']))
{	
$username = $_POST['username'];
$password = $_POST['password'];
require_once('stripFormSlashes.php');
include("db_con.php");
$conn = mysqli_connect($host, $user, $pass, $dbase);
$sql = "SELECT firstname,lastname  FROM members WHERE username= '$username' AND password= '$password'";
$result = mysqli_query($conn,$sql) or die ('Could not run select query');

while($row = mysqli_fetch_array($result))
{
$message = 'Logged in as '.$row['firstname'];
}
}
?>

Agreaves

Also I tried that function to escape the user input but it wasn't working so if you can help me with that also please.

NogDog

Agreaves;10981661 wrote:
Also I tried that function to escape the user input but it wasn't working so if you can help me with that also please.

You'll need to use the mysqli_real_escape_string() function, not the one I referenced (which is for the older MySQL extension). Also, you need to do the mysqli_connect() before you call the escaping function.

NogDog

You can look at the query result object to see if any rows were returned, and use that in an if/else block to decide if login was successful:

if(mysqli_num_rows($result) == 0) {
   // invalid login, output error or whatever you want to do
}
else {
   // valid login, go ahead and fetch results and whatever else needs to be done on success
}

Agreaves

I just remembered that I already included a stripFormSlashes file in the script, so that should take care of the magic quotes. This is where the script is at right now, ill also try that if statement you suggested and see how it works out.

<?php
session_start();

if(isset($_POST['login']))
{	
require_once('stripFormSlashes.php');
include("db_con.php");
$conn = mysqli_connect($host, $user, $pass, $dbase);
$username = $_POST['username'];
$password = sha1($_POST['password']);
$sql = "SELECT firstname,lastname  FROM members WHERE username= '$username' AND password='$password'";
$result = mysqli_query($conn,$sql) or die ('Could not run select query');
}
?>

Agreaves

OK, here is the final script, this is currently working the way I want it, so now you can assist me with fine tuning it. Basically what it does is runs a query in the database and validate the user, log them in and updates the login time and date.

<?php
session_start();

if(isset($_POST['login']))
{	
require_once('stripFormSlashes.php');
include("db_con.php");
$conn = mysqli_connect($host, $user, $pass, $dbase);
$username = $_POST['username'];
$password = sha1($_POST['password']);
$sql = "SELECT firstname,lastname  FROM members WHERE username= '$username' AND password='$password'";
$result = mysqli_query($conn,$sql) or die ('Could not run select query');
$row = mysqli_fetch_array($result);

if(mysqli_num_rows($result) == 0)
{
$message = 'Please make sure you have entered the correct username and password';
} else
{
mysqli_query($conn,"UPDATE members SET logindate= NOW() WHERE password= '$password'") or die('Could not run update query');
$message = 'Logged in as '.$row['firstname'];	
}
}
?>

NogDog

Since I don't know what the "strip form slashes" file does, first thing I'd do is test it by trying a username and/or password that includes a single quote (e.g. "O'Brien"), to make sure it is properly escaping the user inputs.

SQL Injection Example 😉

Agreaves

Here is what the stripFromSlashes script contains.

<?php
function stripFormSlashes($arr){
	if (!is_array($arr)){
		return stripslashes($arr);
	} else{
		return array_map('stripFormSlashes', $arr);
	}
}

if(get_magic_quotes_gpc()){
	$_POST = stripFormSlashes($_POST);
	$_GET = stripFormSlashes($_GET);
}
?>

Bjom

Further improvements can be had from getting rid of "or die" and from using prepared statements

Agreaves

Whats the difference with the or die as opposed to the or trigger error? Dont they display the error message the same way?. I only had the or die there for trouble shooting though, i was planning to remove it after im done troubleshooting the entire script. The only problem im having now is including the mysqli_real_escape_string.

NogDog

Your stripFormSlashes() function is dealing with the "damage" that PHP's magic_quotes_gpc configuration setting can do if enabled, which is all well and good. But that does nothing to prevent SQL injection attacks/errors, which is why you need to use mysqli_real_escape_string() (or prepared statements, instead) to avoid such problems.

NogDog

Agreaves;10981683 wrote:
Whats the difference with the or die as opposed to the or trigger error? Dont they display the error message the same way?. I only had the or die there for trouble shooting though, i was planning to remove it after im done troubleshooting the entire script. The only problem im having now is including the mysqli_real_escape_string.

Functionally they are pretty much the same, with perhaps some minor differences (die()/exit() is a PHP language construct, and can be used to return an integer status code instead of displaying a message, while trigger_error()/user_error() is a PHP function). As to why one would be preferable over the other in this case, I don't know -- hopefully Bjorn will educate us?

Agreaves

Can you help me construct the script using the mysqli_real_escape_string? I have tried many ways and it still doesnt work

NogDog

Just change this:

$username = $_POST['username'];
$password = sha1($_POST['password']);

To this:

$username = mysqli_real_escape_string($_POST['username']);
$password = mysqli_real_escape_string(sha1($_POST['password']));

(The one for the $password probably isn't really necessary, since sha1() does not return any characters that MySQL should need escaped, but it's a good habit t get into. 🙂 )

Agreaves

I tried that already and got a error message that mysqli_real_escape_string needs another parameter.

bradgrafelman

So... did you try passing in that second parameter as the error message states?

Agreaves

I never even understood the message so I just left it alone.

bradgrafelman

You didn't understand:

Warning: mysqli_real_escape_string() expects exactly 2 parameters, 1 given

and then go to the PHP manual page for [man]mysqli_real_escape_string/man to find out what these two parameters are supposed to be?

If that's the case, then perhaps you should find a very basic tutorial on PHP (and one on common sense, too, if you can find one) instead of relying on other people's brains rather than your own.