Hi,

Don't know why but i always seem to mess my, mysql querys up somewhere, usually silly things like quotes etc.

The code below, i have a problem. Basically when user gets an email they click the link and they are taken to a page where they can change there password,

Problem i am having, again i think this is a query problem, i have tried many many ways with no luck.

When i enter a new password the password does not get updated in the database.

Basically that the only problem i am having, if someone could take a look and tell me where i am going wrong again.

here is my code, thanks as always
genieuk

<?php @include("top.inc"); ?>
<?php include("db_connect.php"); // Database Connection ?>

<?php
if (isset($_POST['submitted'])) { // Handle the form.

$pass1 = $_POST['pass1'];
$pass2 = $_POST['pass2'];

// Validation enter password
if ( empty ( $_POST['pass1'] ) ) {
	echo "<span class=\"error\"><p>You must type in a password</p></span>";
	$err++;
}

// Validation Confirm Password
if ( empty ( $_POST['pass2'] ) ) {
	echo "<span class=\"error\"><p>You must confirm your password</p></span>";
	$err++;
}

// Validation passwords do not match
if ( $_POST['pass1'] != $_POST['pass2'] ) {
	echo "<span class=\"error\"><p>Your passwords do not match</p></span>";
	$err++;
}

// If all is ok 
if ($err == 0) {

$queryString = $_GET['key'];

$query = "SELECT * FROM userinformation"; 

$result = mysql_query($query) or die(mysql_error());

// Ecnrypt new password
$pass1 = sha1( $pass1 ) ;

  while($row = mysql_fetch_array($result)){

    if ($queryString == $row["activationkey"]){
			
		$sql = "UPDATE userinformation SET password = '$pass1' WHERE activationkey = '$queryString'";
		
		echo "<span class=\"error\"><p>Your password has been successfully updated</p>
			 <p>You will automatically be redirected in 5 seconds</p></span>";
		 
		 redirect( "login.php" , "5" );
		
    } 
	}  
}

}

?>

<h1>Please enter a new password</h1>

<form action="verifypasswordreset.php" method="post">

New Password 
<p><input type="password" name="pass1" maxlength="20"  /></p>

Confirm New Password
<p><input type="password" name="pass2" maxlength="20"  /></p>

<input type="submit" name="submit" value="Update My Password"  />
<input type="hidden" name="submitted" value="TRUE" />

<?php @include("footer.inc"); ?>

You are not even running the update query. Its just a string. You need to put it into the mysql_query function.

If you run into problems like this again, take a break and then come back and look at the code again. You will see silly mistakes like that.

You also need to rethink the activation key stuff. You are looping through all of the users just to compare a key. This should be done by the sql statement itself.

hi,

thanks, sometimes i dont know, ur rite take a break and come back.

the silly mistakes sometimes it makes me laugh.

Also when you say

You also need to rethink the activation key stuff. You are looping through all of the users just to compare a key. This should be done by the sql statement itself.

what do you mean exactly? not sure how else i could do it, and ur idea sounds good.

Thank you,
genieuk

Do your rows in the userinformation table have id's? In my projects I base all of my updates on ids. Since you are using an activation key, you need to compare that along with an id so its harder to crack.

Are you requiring your users to login before they update their password? (I am trying to figure out why you need the activation key stuff anyway)

Hi, yes users does have unique IDs, they auto increment when they register.

Also as i use sha1 password encryption users cannot retrieve there passwords, so on my login page i have a link where users can reset there password.

Basically they click the reset password link then enter there email address in the text box and hit submit.

Once they submit a new activation key is generated on the account, they then have to click on the link which will also contain the unqiue activation key and enter in a new password, once they enter a new password it then checks the DB for that activation key, when found it will then update there password, as the activation key is unique to that account and cannot be used more than once.

If user wants to change password they can after the login without any activation etc. This is password reset so if they forget password and cannot login they can reset there password via a unique id.

I use $activationkey = mt_rand() . mt_rand() . mt_rand() . mt_rand() . mt_rand();

so basically an mt_rand() is generated over 5 times. Which is what i use to auto generate the unique keys.

Thank you
genieuk

Ok, so its used for resetting forgotten passwords. Is the link with the activation code sent to the user by email?

I usually generate a new one and send it to them via email. Then I have a prompt telling them to change their password after they login.

Here is a better way of checking the activation key. Its easier on the database.

<?php

include 'top.inc';
include 'db_connect.php';

if ( isset( $_POST['submitted'] ) ) {
	$err = 0;
	$pass1 = mysql_real_escape_string( $_POST['pass1'] );
	$pass2 = mysql_real_escape_string( $_POST['pass2'] );
	if ( empty( $pass1 ) ) {
		echo "<span class=\"error\"><p>You must type in a password</p></span>";
		$err++;
	}
	if ( empty( $pass2 ) ) {
		echo "<span class=\"error\"><p>You must confirm your password</p></span>";
		$err++;
	}
	if ( $pass1 !== $pass2 ) {
		echo "<span class=\"error\"><p>Your passwords do not match</p></span>";
		$err++;
	}
	if ( $err == 0 ) {
		$key = mysql_real_escape_string( $_GET['key'] ); //to prevent sql injection
		$query = mysql_query( "SELECT COUNT(*) FROM `userinformation` WHERE `activationkey` = '{$key}' LIMIT 1" ) or die(mysql_error());
		list( $total ) = mysql_fetch_row( $query );
		if ( $total == 0 ) {
			echo "<span class=\"error\"><p>Activation Key is invalid</p></span>";
		}
		else {
			$pass1 = sha1( $pass1 );
			$query = mysql_query( "UPDATE `userinformation` SET `password` = '{$pass1}' WHERE `activationkey` = '{$key}'" );
			echo "<span class=\"error\"><p>Your password has been successfully updated</p><p>You will automatically be redirected in 5 seconds</p></span>";
			redirect( "login.php",5 );
		}
	}
}

echo <<<HTML
<h1>Please enter a new password</h1>
<form action="verifypasswordreset.php" method="post">
New Password 
<p><input type="password" name="pass1" maxlength="20"  /></p>
Confirm New Password
<p><input type="password" name="pass2" maxlength="20"  /></p>
<input type="submit" name="submit" value="Update My Password"  />
<input type="hidden" name="submitted" value="TRUE" />
</form>
HTML;

include 'footer.inc';

?>

Hi,

Thanks Kkeith29 for your time and effort.

Sadly it still do not work.

But what i have done is removed the activation for password reset and now have made it so it generates a new password and sends it to user whilst encrypting it as it is sent to DB.

I found it much easier and much less problematic. It has taken me all day and i got no where the other way. Within a matter of minutes of doing what you said you do on your site, i had it working in literally minutes without problems.

Thanks for everything, very much appreciate your time, patience and effort.

May i ask, when you said you force your users to have to change there password on login after doing a password reset.

May i ask how you yourself do this.?

I mean do you have a field in DB where you get the login page to firstly check whether status of the particular field is set to let say changepassword and if it finds that field it forces them to change password? ,

I do something similar see on activation after change of email via a unique key. Where if the status verify is found on there account after changing email, in DB it wont log them in until they re-verify there email address by the unique link in email.

Interested to see how you go about it.

Thanks as always,
genieuk

I put a field in the database for it, like you said. I have some code that check for it and redirects to the change password area.

I put a field in the database for it, like you said. I have some code that check for it and redirects to the change password area.

Thanks kkeith, yeah thought that how you may have done it.

I will do the same.

Thanks for everything.

genieuk

This article has been dead for over six months. Start a new discussion instead.