Not validating user login correctly!!!!

PHP programming forum. Ask questions or help people concerning PHP code. Don't understand a function? Need help implementing a class? Don't understand a class? Here is where to ask. Remember to do your homework!

Moderator: General Moderators

sleeper24
Forum Newbie
Posts: 8
Joined: Wed Aug 05, 2009 1:13 am

Not validating user login correctly!!!!

Post by sleeper24 »

Hi there,

I'm building my first test site and I'm having a little trouble validating user login. I've google for solutions but my code looks correct and similar to those I've found online. I've created a test username and password in my database to test for validation. Even when I input a valid username and password in the form login, it doesn't recognize the user and redirect to a page where valid users should go. Any help is greatly appreciated.

Here is my form code:

Code: Select all

 
<form action="login.php" method="POST">
    <table>
        <tbody>
            <tr><td>Username: </td> <td><input type="text" name="username" size="25"></td></tr>
            <tr><td>Password: </td> <td><input type="password" name="password" size="25"></td></tr>
        </tbody>
    </table><br>
    <input type="submit" value="Login">
</form>
 
Here is my login.php which is my login validation PHP script:

Code: Select all

 
<?php
    session_start();
    
    if ( isset($_POST['username']) && isset($_POST['password']) )
    {   
        $host = 'localhost';        // Host name
        $username = 'root';         // Mysql username
        $password = 'root';         // Mysql password
        $db_name = 'practiceset';   // Database name
 
        // Connect to server and select databse.
        $db_conn = mysql_connect($host, $username, $password)
            or die("Cannot connect to Database.");
        mysql_select_db($db_name)
            or die("Cannot select Database.");
 
        // username and password sent from form
        $myusername = $_POST['username'];
        $mypassword = $_POST['password'];
        $mypassword = md5($mypassword);
 
        // To protect MySQL injection (more detail about MySQL injection)
        $myusername = stripslashes($myusername);
        $mypassword = stripslashes($mypassword);
        $myusername = mysql_real_escape_string($myusername);
        $mypassword = mysql_real_escape_string($mypassword);
 
        $result = mysql_query("SELECT * 
                               FROM user 
                               WHERE username='$myusername' AND password='$mypassword'");
    
        // Mysql_num_row is counting table row
        $count = mysql_num_rows($result);
    
        // If result matched $myusername and $mypassword, table row must be 1 row
        if($count == 1)
        {
            // Register $myusername, $mypassword and redirect to file "login_success.php"
            $row = mysql_fetch_assoc($result);
            $_SESSION['userid'] = $row['id'];
            session_register('username');
            session_register('password');
            header('Location: generaljournal.php');
        }
        else 
        {
            $_SESSION['login_error'] = '1';
            $header = 'Location: ./';
            header($header);
        }
    
        mysql_close($db_conn);
    }
?>
 
User avatar
bala_1225
Forum Commoner
Posts: 29
Joined: Tue Jul 28, 2009 3:20 am
Location: chennai,india

Re: Not validating user login correctly!!!!

Post by bala_1225 »

hi try this one it will help u...

<?
session_start();
include("config.php"); // database details

extract($_POST);
if($loginStage==1)
{
//from txt field
$txtusername = $_POST['txtuser'];
$txtpassword = $_POST['txtpass'];

$sql_admindetail="select * from user where user_name='$txtusername' AND password='$txtpassword'";
if (!mysql_query($sql_admindetail,$con))
{
die('Error: ' . mysql_error());
}
$row = mysql_fetch_array(mysql_query($sql_admindetail,$con));
//from data base
$checkuser=$row['user_name'];
$checkpass=$row['password'];

if(($checkuser==$txtusername) && ($checkpass==$txtpassword))
{
session_register('username');

$_SESSION['username'] = $txtusername;

echo "<script type='text/javascript'>window.top.location='index.php';</script>";//Login success....
}
else
{
$error_msg = "Login Failed - Try again !!";
session_unregister('username');

session_unset();
session_destroy();

}
}
?>

<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1" />
<title>Getting2delight</title>
<script type="text/javascript">
function loginValidation()
{
if(document.login_form.txtuser.value=="")
{
document.getElementById("login_error").innerHTML="User Name should not be Empty !";
document.login_form.txtuser.focus();
return false;
}
else if(document.login_form.txtpass.value=="")
{
document.getElementById("login_error").innerHTML="Password should not be Empty !";
document.login_form.txtpass.focus();
return false;
}
return true;
}

function clearErrorDisplay(getValue)//hide the error message when the mouse event occur in the txt field
{
if(getValue.length > 0 )
{
document.getElementById("login_error").innerHTML='&nbsp;';
return true;
}
}

/* Checking for accept only Alpha Numeric i.e User Name*/

var splchar={'special':/[\W]/g}
function chkAplhanum(getValue,spl)
{
getValue.value = getValue.value.replace(splchar[spl],'');
}


</script>
</head>

<body>
<form name="login_form" action="login.php" method="post" onSubmit="return loginValidation()">
<table width="360" border="0" cellspacing="0" cellpadding="0">
<tr bgcolor="#F6F6F6">
<td colspan="2" id="login_error">&nbsp;<?=$error_msg?></td>
</tr>
<tr bgcolor="#F6F6F6">
<td width="100" height="30" class="pad-both"><b>User Name</b></td>
<td width="257"><input type="text" name="txtuser" title="Only Alphanumeric" onKeyUp="chkAplhanum(this,'special');clearErrorDisplay(this.value);" /></td>
</tr>
<tr bgcolor="#F6F6F6">
<td height="30" class="pad-both"><b>Password</b></td>
<td><input type="password" name="txtpass" title="Only Alphanumeric" onKeyUp="chkAplhanum(this,'special');clearErrorDisplay(this.value);" /></td>
</tr>
<tr bgcolor="#F6F6F6">
<td>&nbsp;</td>
<td align="right" class="pad-both"><input type="submit" value="Login >" /></td>
</tr>
</table>
<input type="hidden" name="loginStage" value="1" />
</form>

</body>
</html>
Last edited by bala_1225 on Wed Aug 05, 2009 2:17 am, edited 1 time in total.
dejvos
Forum Contributor
Posts: 122
Joined: Tue Mar 10, 2009 8:40 am

Re: Not validating user login correctly!!!!

Post by dejvos »

I don't know which version of PHP do you use but function session_register is depracted. I have bad experiences with this function I had to inicialize $_SESSION['xxx'] ordinary way

Code: Select all

 
$_SESSION['xxx'] = 'value';
 
You have inicialized variable $_SESSION['password'] only by session_register (line 43). The last thing:
Do you thimk that saveing your password in to the $_SESSION is a good idea? The $_SESSION variables are saved on your local machine!
User avatar
bala_1225
Forum Commoner
Posts: 29
Joined: Tue Jul 28, 2009 3:20 am
Location: chennai,india

Re: Not validating user login correctly!!!!

Post by bala_1225 »

hi no need to inicialized the password session variable $_SESSION['password']
sleeper24
Forum Newbie
Posts: 8
Joined: Wed Aug 05, 2009 1:13 am

Re: Not validating user login correctly!!!!

Post by sleeper24 »

Thanks guys, I'll give it a try and let you all know if there is a success or not.
sleeper24
Forum Newbie
Posts: 8
Joined: Wed Aug 05, 2009 1:13 am

Re: Not validating user login correctly!!!!

Post by sleeper24 »

By the way, I'm using PHP 5.
User avatar
shiznatix
DevNet Master
Posts: 2745
Joined: Tue Dec 28, 2004 5:57 pm
Location: Tallinn, Estonia
Contact:

Re: Not validating user login correctly!!!!

Post by shiznatix »

Ok whoa, please please do not use bala_1225's code. First, sleeper24, ill go over what problems you have with your code then will go over what bala_1225 is doing wrong then give some hints to find out whats wrong with your code to get it working.

sleeper24:
All of your code is beautiful, well structured and taking proper precautions to check for variables and escape them. First problem:

Code: Select all

$myusername = stripslashes($myusername);
$mypassword = stripslashes($mypassword);
$myusername = mysql_real_escape_string($myusername);
$mypassword = mysql_real_escape_string($mypassword);
You don't need to stripslashes() and then escape_string(). Just use mysql_real_escape_string(), it will do everything you want, and will do it better than stripslashes().
Next:

Code: Select all

session_register('username');
session_register('password');
header('Location: generaljournal.php');
bala was right, you don't need session_register here, just do like $_SESSION['username'] = 'wjatever'; and that will be all that is needed. Next, after a header('Location') you should always die() or exit; on the next line. What happens is that the script does not die when you redirect, it will redirect after the rest of the code is executed. Take for example (pesudo-code)

Code: Select all

 
if (log_in_failed)
{
     header to login page
}
 
sign user is because they did not have a failed login
 
if you do not die after the "header to login page" then it will still log the user in as an invalid user. you have to die(); after the header to kill the script and make it redirect the user immediately (same for your second header() call).

Now, lessons to learn from bala:

Never, ever, ever, use the extract() function on $_POST or $_GET or anything user-supplied. I can throw in whatever i want into post and get, you can't trust the user, ever. Never ever ever trust the user.

Next, why use extract if you are going to set $txtusername to a $_POST variable anyway? it's redundant. Also, there is no check to see if $_POST['txtuser'] or 'txtpass' even exist, it is just assumed. Never, ever, assume anything.

Next, there is no escaping on the user supplied $_POST variables before they are thrown into an sql query. This is the number 1 biggest mistake you can make when coding. Look up "SQL injection"

Next, you don't need to do things like this:

Code: Select all

if (!mysql_query($sql_admindetail,$con))
{
die('Error: ' . mysql_error());
}
just use the mysql_query(...) or die(...). Its much cleaner and easier to read.

Next, session_register() again, don't use it.

Next, he is using javascript to redirect to a new page, without even using the die(); command after echoing it. This is well, just terrible practice. Use the header('locatlion');die(); way instead.

Next, do everything you can to not put PHP and HTML into the same page. Yes, for templates to echo variables its fine but don't put all the logic and HTML on the same file. Do something like:
login.php -> has the HTML for the login form, is a PHP page incase you have to echo out a varaible or 2 in the HTML but no logic or anything crazy going on
process-login.php -> has the sql queries and checks to log the user in or throw an error if there is invalid data or whatnot
That way, everything is separate and easier to manage / update

-----------------

Ok, now for why your code is failing. The keywords "username" and "password" are reserved keywords for sql, much like "select" and "from" are. You have this like:

Code: Select all

$result = mysql_query("SELECT *
FROM user
WHERE username='$myusername' AND password='$mypassword'");
First, you should always do some sort of error caching on queries. like this: mysql_query(...) or die(mysql_error()); You can also use trigger_error() or whatever other custom error message to silently send you an email if a query fails instead of spitting it out to the user. At first though die(mysql_error()) is the easiest. When you add that "or die()" part in you will find an error in the query. The last hint is the escape character for sql keywords is ` (so you can use them yourself without them being processed as sql)

Hope this helps. Any questions just post them
User avatar
bala_1225
Forum Commoner
Posts: 29
Joined: Tue Jul 28, 2009 3:20 am
Location: chennai,india

Re: Not validating user login correctly!!!!

Post by bala_1225 »

Hi shiznatix, thanks for your suggestion........about PHP....
sleeper24
Forum Newbie
Posts: 8
Joined: Wed Aug 05, 2009 1:13 am

Re: Not validating user login correctly!!!!

Post by sleeper24 »

Thank you so much shiznatix. I understand now that you have said so. I didn't know about die before. I've been learning php for only 2 months. I'll keep this in mind. Again think you so much. Thanks to all of you.

Shiznatix, Can just use die() at the end of:

Code: Select all

 
     header($header);
 
Like this:

Code: Select all

 
     header($header);
     die();
 
Shiznatix, so you are saying that my query below, 'username' and 'password' would have have to be changed to something else like 'uname' and 'pword'. Anything other than 'username' and 'password'.

Code: Select all

 
     $result = mysql_query("SELECT * 
     FROM user 
     WHERE username='$myusername' AND password='$mypassword'")
 
User avatar
shiznatix
DevNet Master
Posts: 2745
Joined: Tue Dec 28, 2004 5:57 pm
Location: Tallinn, Estonia
Contact:

Re: Not validating user login correctly!!!!

Post by shiznatix »

sleeper24 wrote:Thank you so much shiznatix.
no problem
sleeper24 wrote: Shiznatix, Can just use die() at the end of:

Code: Select all

 
     header($header);
 
Like this:

Code: Select all

 
     header($header);
     die();
 
exactly!
sleeper24 wrote: Shiznatix, so you are saying that my query below, 'username' and 'password' would have have to be changed to something else like 'uname' and 'pword'. Anything other than 'username' and 'password'.

Code: Select all

 
     $result = mysql_query("SELECT * 
     FROM user 
     WHERE username='$myusername' AND password='$mypassword'")
 
That is one way of doing it but the better way is to just escape the field names. You have to understand that SQL has keywords that are parsed as SQL statements. SELECT is one of them, for example. Another 2 are USERNAME and PASSWORD (caps does not matter). If you have fields that have the same name as a SQL keyword you have to escape them. This is good practice even if they are not keywords. You do it like this:

Code: Select all

 
//remember, use mysql_real_escape_string() on $myusername and $mypassword!
$sql = '
     SELECT
          *
     FROM
          `user`
     WHERE
          `username` = "'.$myusername.'"
     AND
          `password` = "'.$mypassword.'"
';
 
$query = mysql_query($sql) or die(mysql_error());//notice the "or die(mysql_error())". Use this always, makes debugging oh so much easier, trust me
 
See how I escaped everything that was not a keyword and was not in quotes? This is the best way to do it.
peterjwest
Forum Commoner
Posts: 63
Joined: Tue Aug 04, 2009 1:06 pm

Re: Not validating user login correctly!!!!

Post by peterjwest »

What happens is that the script does not die when you redirect, it will redirect after the rest of the code is executed.
As far as I know the server does redirect straight away however it continues to execute the other script until it is terminated. Remember that servers handle lots of requests in parallel. So while the script has redirected the user, it is still running the other script however no-one sees the output. But this also means that any database or session interactions still occur, so you should think carefully about whether you want your script to die() or not after a redirect.
sleeper24
Forum Newbie
Posts: 8
Joined: Wed Aug 05, 2009 1:13 am

Re: Not validating user login correctly!!!!

Post by sleeper24 »

Shiznatix & all,

For some reason it is still not working correctly. I can't seem to figure out what is wrong even with the changes you've suggest to.
Below is the form, to login.php and then redirects valid user to generaljournal.php.

Form:

Code: Select all

 
       <form name="form_login" action="login.php" method="POST">
        <table>
            <tbody>
                <tr><td>Username: </td> <td><input name="username" type="text" size="25"></td></tr>
                <tr><td>Password: </td> <td><input name="password" type="password" size="25"></td></tr>
            </tbody>
        </table><br>
        <input name="submit" type="submit" value="Login">
    </form>
 
login.php

Code: Select all

 
<?php  
    session_start();
    
    if (isset($_POST['username']) && isset($_POST['password']) && $_POST['username'] != "" && $_POST['password'])
    {   
        $db_host = 'localhost';     // Host name 
        $db_user = 'root';          // Mysql username
        $db_pass = 'root';          // Mysql password
        $db_name = 'practiceset';   // Database name
 
        // Connect to server and select databse.
        $db_conn = mysql_connect($db_host, $db_user, $db_pass)
            or die("Cannot connect to Database.");
        mysql_select_db($db_name)
            or die("Cannot select Database.");
 
        // username and password sent from form
        $username = $_POST['username'];
        $password = $_POST['password'];
        $password = md5($password);     // Hash out the password
 
        // To protect MySQL injection (more detail about MySQL injection)
        $username = mysql_real_escape_string($username);
        $password = mysql_real_escape_string($password);
 
        $query = "SELECT *
                  FROM `user` 
                  WHERE `username`='".$username."' AND `password`='".$password."'";
        
        $result = mysql_query($query)
            or die(mysql_error($result));
    
        // Mysql_num_rows is counting table row
        $count = mysql_num_rows($query);
        mysql_close($db_conn);  // Close connection to DB
    
        // If result matched $myusername and $mypassword, table row must be 1 row
        if($count == 1)
        {   
            // Register $myusername, $mypassword and redirect to file "generaljournal.php"
            $_SESSION['loggedin'] = true;
            $_SESSION['username'] = $username;
            header('Location: generaljournal.php');
            die();
        }
        else 
        {
            //echo 'Login failed';
            $_SESSION['login_error'] = '1';
            header('Location: index.php');
        }
    }
?>
 
Redirect page from login.php for valid users.

Code: Select all

 
<?php
    session_start();
    require('global.php');
    
    if($_SESSION['loggedin'] != true)
    {
        header('Locaiton: ./');
        die();
    }
?>
 
<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN" "http://www.w3.org/TR/html4/loose.dtd">   
 
<html>
    <head>
        <title>General Journal</title>
    </head>
    <body>
                You've successfully logged in.  WELCOME!
        </body>
</html>
 
peterjwest
Forum Commoner
Posts: 63
Joined: Tue Aug 04, 2009 1:06 pm

Re: Not validating user login correctly!!!!

Post by peterjwest »

I can't see any problem with it. The php should run no problem, but make sure you have error reporting on anyway (for development). There's probably a problem with MySQL or the redirect. I would suggest getting the mysql results and printing them:

Code: Select all

 
while ($row = mysql_fetch_assoc($result)) {
   print_r($row);
}
 
Test this with various inputs to see how your database responds.

Oh and you did run the md5 hash on your password before you saved it to the database, right?
sleeper24
Forum Newbie
Posts: 8
Joined: Wed Aug 05, 2009 1:13 am

Re: Not validating user login correctly!!!!

Post by sleeper24 »

I did md5 the password before saving it on the database. I'll start printing out results to see what is wrong.

Thanks peterjwest.
sleeper24
Forum Newbie
Posts: 8
Joined: Wed Aug 05, 2009 1:13 am

Re: Not validating user login correctly!!!!

Post by sleeper24 »

Hi all,

I figured out the problem why it wasn't redirecting if the user was a valid user. I queried the wrong variable for mysql_num_rows().

My original code:

Code: Select all

 
          $query = "SELECT *
                   FROM `user`
                   WHERE `username`='".$username."' AND `password`='".$password."'";
        
         $result = mysql_query($query)    
             or die(mysql_error($result));
    
         // Mysql_num_rows is counting table row
         $count = mysql_num_rows($query);
      
         mysql_close($db_conn); // Close connection to DB
    
        // If result matched $myusername and $mypassword, table row must be 1 row
    if($count == 1)
    { code goes here }  
 

Code: Select all

 
     $count = mysql_num_rows($result);   // NEEDED TO BE $result INSTEAD OF $query.
 
So thanks all for all the suggestion you guys have given me. Don't make the same mistake I did which took me days to find it. One variable can be a big problem throwing you crazy like hell.

Thanks all,
Post Reply