md5/login script trouble

TobyRT

Hi,

I'm trying to set up a user login system based on the notes I found here:

http://www.phpbuilder.com/board/showthread.php?t=10306819&highlight=login+script

But it doesn't appear to be working properly. I've tried various different ways around it, but I can't seem to make it login properly using the md5($pass) query. It keeps telling me that the username or password is incorrect (even though when I look in the db, it is!)

I'd be grateful if you could help me on this!

<?php 
require_once('sypphp/includes/common.php'); 
require_once('sypphp/sypcms/DbConnector3.php');

session_start(); 

//--- LOG OUT button clicked - destroy session     

if (isset($_REQUEST['log_out'])) {   

      $_SESSION = array(); 
      session_destroy; 
} 

//--- LOGIN CHECK - has the user filled in username and password 
//                    or is the user already logged in? 
$user = isset($_POST['user']) ? $_POST['user'] : $_SESSION['user']; 
$pass = isset($_POST['pass']) ? $_POST['pass'] : $_SESSION['pass']; 

//--- NOT LOGGED IN - display login page 
if(!isset($user)) { 

?> 

</head> 
<body> 
    <script language="javascript"> 
        window.location = "loginform.php" 
    </script> 
</body> 
</html> 

<? 
    exit; 
} 

//--- CHECK LOGIN CREDENTIALS 
$connector = new DbConnector(); 

$md5pass=md5($pass); 
echo 'username = ' . $user . ' password = ' . $pass ; 

// build safe query 
$logincheck = sprintf("SELECT firstname, surname FROM mem_details WHERE firstname=%s AND password=%s", quote_smart($user), quote_smart($md5pass)); 

// run safe query WHEN I TRY TO RUN THIS, IT DOESN'T WORK
$loginresult = $connector->query($logincheck) or die ("Error in Query: $query.".mysql_error()); 

//THIS QUERY DOES WORK
$loginresult = $connector->query("SELECT firstname, surname FROM mem_details WHERE firstname='$user' AND password='$pass'") or die ("Error in Query: $query.".mysql_error());
//(THIS DOESN'T WORK IF I SAY "password='$md5pass'")

//I won't be using num_rows, this is just part of my debugging - see bottom for actual usage
if (mysql_num_rows($loginresult) > 0)
{
while ($row = mysql_fetch_array($loginresult))
	{
	echo '<br>logged in as '.$row['firstname'].' '.$row['surname'];
	}
}
else
{
echo '<br>No results returned for this username';
}
// test results 

/* THIS IS THE BIT I'LL BE USING TO OPEN THE SESSION ETC.
$num= mysql_num_rows($loginresult); 
if($num == 1) 
{ 
    echo"User logged in"; 
    $_SESSION['user']=$user; 
    $_SESSION['pass']=$pass; 
    $_SESSION['auth']=true; 
} 
else 
{ 
    echo"The Username or Password you have entered is invalid. Please go back and try again or procede to our <a href=\"memberadmin\\registration.php\">registration</a> page."; 
    $_SESSION['auth']=false; 
} 
*/

TobyRT

I've just realised that one of the problems may have been that I didn't originally enter the password as an MD5 hash in the first place (oops!)

I've now done that - but it's still telling me it doesn't exist!

TobyRT

Anybody?

Please?

bpat1434

Your initial query is wrong. You need to quote around the "%s"..... I'd echo out the query and look at it. It should have this look:

SELECT firstname, surname FROM mem_details WHERE username='somename' AND password='someMD5Hash'

If it doesn't look like that, there's your problem. I'd say, you do something like:

$logincheck = sprintf("SELECT firstname, surname FROM mem_details WHERE firstname='%s' AND password='%s'", mysql_real_escape_string($user), mysql_real_escape_string($md5pass));

Now, note some differences:
You:

$logincheck = sprintf("SELECT firstname, surname FROM mem_details WHERE firstname=%s AND password=%s", quote_smart($user), quote_smart($md5pass));

Me:

$logincheck = sprintf("SELECT firstname, surname FROM mem_details WHERE firstname='%s' AND password='%s'", 
mysql_real_escape_string($user), mysql_real_escape_string($md5pass));

PHP Manual:

// Query
$query = sprintf("SELECT * FROM users WHERE user='%s' AND password='%s'",
           mysql_real_escape_string($user),
           mysql_real_escape_string($password));

Notice the single quotes around the strings? Kinda need those.....

TobyRT

Thanks for that - I put the quotes in, and at first it didn't work. I checked the hash more thoroughly and realised that I only had a varchar of 25 in my SQL db, so it was getting truncated - hence the perpetual mismatch.

This does bring me onto another issue, however...

Using mysql_real_escape_string () in the code works. if I use the quote_smart() as suggested in the post mentioned above, it doesn't.

I can't see any syntax errors, so any idea why this is happening? Is it to do with the addition of the single quotes that the mysql_real_escape_string() puts around the $value?

function quote_smart($value) 
{ 
   // Stripslashes 
   if (get_magic_quotes_gpc()) { 
       $value = stripslashes($value); 
   } 
   // Quote if not integer 
   if (!is_numeric($value)) { 
       $value = "'" . mysql_real_escape_string($value) . "'"; 
   } 
   return $value; 
}

bpat1434

Do you have magic_quotes on?

TobyRT

After all this...

The problem was not the code at all, it seems...

The reason it fell over, as I mentioned above, was that my varchar field was only 25char long and therefore truncating the md5hash.

using mysql_real_escape_string() was working with sprintf() because I'd wrapped the %s in quotes - as you recommended.

The reason quote_smart() wasn't working was that because the %s was in quotes, as was the value being output by quote_smart, it was coming out as ' ' %s ' ' instead of just '%s'

I hate Mondays.

Next challenge - since I'm now storing an md5hash in the db, how do I translate it back to real money so the user can change their password?

bpat1434

you can't translate back.....

To change the password, you just get them to enter their current, hash that. Check against DB. If it's okay, then you take the new password (and confirmation) and compare them. IF they're the same, hash them and update that record with the new hash.

TobyRT

OK - fair enough - but what if they've forgotten their password and need it resetting?

I don't want to do manual resets - would I then have to store the password twice (once with the hash, once without) so they can enter their email and membership number, if they are correct, a plain-english version of the pw is emailed to them?

It strikes me that doing so kinda defeats the object of the hashing anyway...

...or does it, since they're not sending the pw over the internet - only their email & memnum?

(A thousand thanks for all your help on this btw - I've been working on this site for so damn long, this is one of the final things i need to do before i hand over!)

bpat1434

Nope, to reset the password, you generate a random string, and hash that. Then send them the random string to the email address they registered with (stored in the database). Then, you set a flag in the database. If that flag is set ('temp') they have to supply a new password. Otherwise, they can proceed.

TobyRT

I get the logic - but I've NO idea how to implement it!

Could you point me in the right direction please?

bpat1434

Not sure where you could look, but Google should turn up some things....

TobyRT

OK - I got my lil test page working (yay!)

I even managed to tweak it so I did an include once it was logged in (double yay!)

The problem is, when I try and apply it to one of my pages (took a while just to get this far!) it permanently seems to think I'm logged in!

This raises two questions:

1) Why does it always think I'm logged in
2) How do I create the link/button to send the request to logout? It doesn't appear to work either 🙁

Here's me code...

<?php
require_once('../sypphp/includes/common.php');
require_once('../sypphp/sypcms/DbConnector3.php');

session_start();

//--- LOG OUT button clicked - destroy session
if (isset($_REQUEST['log_out'])) {
      $_SESSION = array();
      session_destroy;
}

//--- LOGIN CHECK - has the user filled in username and password
//                    or is the user already logged in?
$user = isset($_POST['user']) ? $_POST['user'] : $_SESSION['user'];
$pass = isset($_POST['pass']) ? $_POST['pass'] : $_SESSION['pass'];

//--- NOT LOGGED IN - display login page
if(!isset($user))
{
$loggedin=false;
}

//--- CHECK LOGIN CREDENTIALS
$connector = new DbConnector();

$md5pass=md5($pass);

// build safe query
$logincheck = sprintf('SELECT firstname, surname FROM mem_details WHERE firstname=%s AND password=%s',  quote_smart($user), quote_smart($md5pass));

// run safe query
$loginresult = $connector->query($logincheck) or die ("<br>Error in Query: $query.".mysql_error());

$num= mysql_num_rows($loginresult);
if($num == 1)
{
    //echo"User logged in";
    $_SESSION['user']=$user;
    $_SESSION['pass']=$pass;
    $_SESSION['auth']=true;

}
else
{
        $_SESSION['auth']=false;
		//mysql_close($loginresult);
}

//<html gubbins goes here>

if ($_SESSION['auth']=true) {
 echo 'logged in';

//include("jobstest.inc");
}
else
{
echo 'Bugger off';
}

 ?>
//<more html gubbins here>

bpat1434

exit; the script after you're done logging them out....

someurl.com/somepage.php?log_out=somevalue

that should get you to log out (not 100% sure though)

TobyRT

Thanks bpat - I'll have a look at that.

For some reason though, it still thinks I'm logged in when I'm not.

This is is the last bit of php before my html headers:

$num= mysql_num_rows($loginresult); 
if($num == 1) 
{ 
    echo"User logged in"; 
    $_SESSION['user']=$user; 
    $_SESSION['pass']=$pass; 
    $_SESSION['auth']=true; 

} 
else 
{ 
        $_SESSION['auth']=false; 
        echo 'you\'re not logged in';
}

That bit is working and giving the correct message

But this bit (after the headers and in the table cell where i want the content displayed) is always saying 'logged in', even if the code above says I'm not. It seems to be ignoring the if() statement completely - why?! 🙁

if ($_SESSION['auth']=true) { 
echo 'logged in'; 

//include("jobstest.inc"); 
} 
else 
{ 
echo 'Bugger off'; 
}

Roger_Ramjet

Yes Toby, that will be my code that you have copied. Sorry :queasy:

Still had my VB head on when I wrote that = not ==

Here is what it should be

if ($_SESSION['auth'] == true) {
echo 'logged in';

//include("jobstest.inc");
}
else
{
echo 'Bugger off';
}

In VB = is either an assignment operator or a comparison operator depending on context. In php it is only an assignment operator, so you were setting $_SESSION['auth'] equal to TRUE, not testing it. I was always doing that. As I discovered last night, nowdays I can't get my VBA head on. Had to keep looking things up in help just to remember how to write , or rather terminate a WHILE loop, I'd forgot all about WEND. :glare: And I'm in the habit of expecting dynamic type casting so I was getting overflow errors and all sorts. Don't you just love it eh 😃

TobyRT

Don't worry about it Roger - thanks for that - to be honest I can't beleive I missed the == bit myself, when I've used it all over my sodding site!!

I'm just going to nod and smile and pretend I know what you're talking about with VBA though 🙂

One more thing - could you advise on how I create the 'logoff' request? I'm assuming $_REQUEST is something you get from a form action? What did you have in mind when you wrote that bit?

Thanks a million for all your help!

bpat1434

bpat1434 wrote:
someurl.com/somepage.php?log_out=somevalue

As long as you set either a POST or GET variable with name "log_out" with anything but a false value (0, empty string, FALSE, NULL) you will be logged out. So you can either make it a link, or a small form to submit.

TobyRT

Hiya,

Thanks for all your help so far - just one small hurdle to surmount now (which sods law dictates will require an entire rewrite!)

Users can log in.
Users can log out.
Users can reset their password so a new one is emailed to them, then when they log in they're asked to change it.
If users enter an incorrect username/password, it says "your details are wrong"

The problem is that if, say, they're on the front page and enter incorrect information, they get the message, then think, actually I'll read this unrestricted article first, they go to another page, use their back button, then get told they need to 'resubmit' the information (which they incorrectly entered initially)

How can I stop this happening?

Here's my code so far:

The code before any of the html:

<?php
require_once('sypphp/includes/common.php');
require_once('sypphp/sypcms/DbConnector3.php');

session_start();

//--- LOG OUT button clicked - destroy session
if ($_GET['action'] == 'logout'){

  $_SESSION = array();
  session_destroy;
      $loggedin=false;
      echo '<script language="javascript">
    window.location = "news.php"
</script>';

}


//--- LOGIN CHECK - has the user filled in username and password
//                    or is the user already logged in?
$user = isset($_POST['user']) ? $_POST['user'] : $_SESSION['user'];
$pass = isset($_POST['pass']) ? $_POST['pass'] : $_SESSION['pass'];

//--- NOT LOGGED IN - display login page
if(!isset($user))
{
$loggedin=false;

}

//--- CHECK LOGIN CREDENTIALS
$connector = new DbConnector();

$md5pass=md5($pass);

// build safe query
$logincheck = sprintf('SELECT firstname, surname, reset FROM mem_details WHERE email=%s AND password=%s',  quote_smart($user), quote_smart($md5pass));

// run safe query
$loginresult = $connector->query($logincheck) or die ("<br>Error in Query: $query.".mysql_error());

$num= mysql_num_rows($loginresult);
if($num == 1)
{
while ($row = mysql_fetch_array($loginresult)){
$firstname=$row['firstname'];
$reset=$row['reset'];
}

//echo"User logged in";
$_SESSION['user']=$user;
$_SESSION['pass']=$pass;
$_SESSION['auth']=true;
    $loggedin=true;


    if ($reset==1)
    {
    echo '<script language="javascript">
    window.location = "changepw.php"
</script>';
    }

}
else
{
        $_SESSION['auth']=false;
        }
?>

The code in the include file which changes according to login status:

<?php

if (isset($user) && $loggedin == false || $loggedin == false)
{
?>
<form action="<?php echo $_SERVER['PHP_SELF'] ?>" method="post" name="loginform" id="loginform">
  <table width="54%"  border="0" cellspacing="0" cellpadding="1">
      <tr>
        <td colspan="2" align="center" valign="middle"><font size="1"><a href="resetpw.php">Get/Reset Password</a> :: <a href="<?php echo $_SERVER['PHP_SELF']?>?action=logout">Logout</a></font></td>
        <td width="26%" align="center">&nbsp;</td>
      </tr>
      <tr>
        <td width="33%" align="center"><input type="text" name="user" size="10" onFocus="if(this.value=='email')this.value='';">
        </td>
        <td width="33%" align="center"><input type="password" name="pass" size="10" onFocus="if(this.value=='password')this.value='';"></td>
        <td align="center"><input type="submit" name="Submit" value="Login" /></td>
      </tr>
      <tr>
        <td align="center" valign="top"><font size="1">email<br>
        </font></td>
        <td align="center" valign="top"><font size="1">password<br>
        </font></td>
        <td align="center">&nbsp;</td>
      </tr>
	  <?php 
	  if (isset($user) && $loggedin == false)
	  {
	  $logintry=true;
	  $_SESSION = array();
  session_destroy;
      $loggedin=false;
	  }	  

	  if ($logintry == true)
	  {
	  echo '<tr align="left" valign="top">
        <td colspan="3">Sorry, either your username or password was incorrect. Please try again, or click above to reset your password</td></tr>';
	  }
	  ?>
    </table>
    <?
}
else
{
echo 'Welcome '.$firstname.', you are now logged in!<br>';
echo '<a href="changepw.php">Change password</a> :: <a href="'.$_SERVER['PHP_SELF'] .'?action=logout">Logout</a>';
}
?>

Can someone point me in the right direction please?

bpat1434

Are you talking about the pop-up that says "Resubmit Posted data'?

I dont' think there's a work around for it. You could try setting the cache to a previous date so that they always have to re-login when they hit the back button, but I've seen it be hit or miss with this. I haven't seen it work personally....