password reset not updating users table

123frank · July 31, 2012

Hello,

I have written a password reset script for my site and I am having some trouble so I come here seeking help.

What I have so far is:

A user forgets their password and enters a username.

User email is found, a token and a time is set in the user table.

Email containing a link with token attached is sent to the user's email address.

When link is clicked, token is checked for existence and expiry.

If no good or expired user is told to try again.

If all is well a new password form is shown.

This works perfectly to this point but herein lies my problem.

When user enters new password and clicks Submit they should then be given the opportunity to Login but

the form simply appears again only blank.

The users table is not updated with the new password.

The first part of the script uses $_GET to check the token but for the password form I am trying to use $_POST and am very confused.

I include the code below. Hopefully most of it makes sense

<?php
require_once ('functions.inc.php');
include("header.php");
require_once ('config.inc.php');
require ("passhash.php");
?>
<body> 
<div data-role="page" id="resetpassword">
<div data-role="header">
	<h1>Reset Password</h1>
</div>
<div data-role="content">
<?php
$page_title = 'ResetPassword';
if ($_SERVER['REQUEST_METHOD'] == 'GET') { 
$tok = ($_REQUEST['token']);
$q1 = "SELECT * FROM users WHERE token = '$tok'";
$r1 = mysqli_query($dbconn,$q1)or die("Error: ".mysqli_error($dbconn));
$row = mysqli_fetch_array($r1);	
$tk = $row['token'];
//echo $tk;
$user = $row['uname'];	
//echo $user;
$then = $row['request_time'];
//echo $then;
$now = time();
//echo $now;
$expired = ($now - $then);
//echo $expired;
$num_rows = mysqli_num_rows($r1);		
//echo $num_rows;
if ($num_rows !== 1 || $expired > 900){	
echo "An error has prevented a password change.<br />Most likely the link has expired.<br />Please try again.";	
?>
</div>
<div data-role="footer" class="ui-bar">
<a href="../index.php" data-role="button">Try Again</a>	
</div><!-- /footer -->
</div>
<?php
exit();		
}		


	if ($_SERVER['REQUEST_METHOD'] == 'POST'){  
	$user = ($_POST['username']);
	$trimmed = array_map('trim', $_POST);
	if (preg_match ('/^[[:alnum:]]{8,}$/', ($trimmed['password']))) {
		$p = mysqli_real_escape_string($dbconn, ($trimmed['password']));			
		} else {
   			echo '<p>Please enter a valid password!</p>';
		}
		// hash the password
		$pass_hash = PassHash::hash($p);
		if ($pass_hash) {
		$q2 = "UPDATE users SET upass = '$pass_hash' WHERE uid = '$user'";
  			$r2 = mysqli_query($dbconn,$q2)or die("Error: ".mysqli_error($dbconn));
  			$row = mysqli_fetch_array($r2);	
  			$num_rows = mysqli_num_rows($r2);
  			if (num_rows == 1) {
  			echo "<p>Password Changed!</p>";
  			}
		} 				
?>
</div><!-- /content -->
<div data-role="footer" class="ui-bar">
<a href="../index.php" data-role="button">Login</a>	
</div><!-- /footer -->
</div><!-- /page -->
<?php
}

?>
<p>Choose a new password.<br />
Letters and numbers only.<br />
Minimum of 8 characters.</p>
<form id="passwordreset" method="post" action="<?php echo $_SERVER['PHP_SELF'];?>?token=<?php echo $tk;?>" data-ajax="false"> 
<label for="username" class="ui-hidden-accessible">Username:</label> 
<input type="hidden" name="username" id="username" value="" placeholder="Username"/>
<label for="password" class="ui-hidden-accessible">Password:</label> 
<input type="password" name="password" id="password" value="" placeholder="Password"/>
<button type="submit" name="submit" value="submit" data-inline="true"/>Submit</button>  
		</form>
</div><!-- /content -->
<div data-role="footer" class="ui-bar">
</div><!-- /footer -->
</div><!-- /page -->
<?php
}
?>
</body>
</html>

cpd · July 31, 2012

I once read an article linked from these forums detaliing why $_SERVER['PHP_SELF'] isn't good to use - someone may link. You can remove that section of PHP code entirely and just have "?token=...". It'll use the current page and append the query string.

Your prone to SQL Injection as well because you don't ensure the token is valid anywhere inside the massive 'GET' if; there may be more I haven't checked it with a fine toothcombe.

Have you attempted to put any debugging echos as it may be something simple like your preg_match not working as you think it is...

123frank · July 31, 2012

Thanks for looking and replying.

I will try getting rid of the 'PHP_SELF' bit.

At the beginning of the $_GET, I do check if the token is the same as the one stored in the db.

It is originally set in the previous page which I haven't included here.

Christian F. · August 1, 2012

Here's a nice method for ensuring that $_SERVER['PHP_SELF'] doesn't contain anything but the path to the current file:

// Hack for certain host configs.
if (isset ($_SERVER['ORIG_PATH_INFO']) && $_SERVER['ORIG_PATH_INFO'] != $_SERVER['PHP_SELF']) {
$_SERVER['PATH_INFO'] = $_SERVER['ORIG_PATH_INFO'];
}

// Security measure, to avoid XSS exploit.
if (!empty ($_SERVER['PATH_INFO']) && strrpos ($_SERVER['PHP_SELF'], $_SERVER['PATH_INFO'])) {
$_SERVER['PHP_SELF'] = substr ($_SERVER['PHP_SELF'], 0, -(strlen ($_SERVER['PATH_INFO'])));
}

Reason I post it is while it's quite unnecessary to use the full path in the form action, most of the time, there are instances where you'll want to use it.

Then to your issue:

Mixing HTML and PHP with each other is generally not a recommended approach. Not only because it'll make it a lot harder to manipulate content and headers as you like, but also because it makes things hard to read and thus maintain. This is a case of the latter, as your mixed mode scripting hid a problem with matching blocks.

In short: The curly bracket that you might think closes "GET" test doesn't, and thus you only try to test for the POST method if the GET method has been identified.

I strongly recommend that you move all of the PHP code to the top of the page, before you send out any information to the server. This will make things a lot easier for you, and allow you to write more powerful code without bending over backwards in the attempt.

cpd · August 1, 2012

Mixing HTML and PHP with each other is generally not a recommended approach.

I'm not convinced that's a good thing to say at all. To produce dynamic pages you must mix your DOM with PHP, or alternatively use a template engine which in turn mixes some other form of code with the template. Either way I wouldn't recommend not mixing the two.

floridaflatlander · August 1, 2012

Have you attempted to put any debugging echos ...

123frank · August 1, 2012

Well thank you for the suggestions. If I do not mix the php and html I can't achieve the page layout I want.

What has me stumped is the form processing after the new password is entered. The form simply re-appears blank.

There should simply be a message telling the user "Password Changed" with a link in the footer to send the user to the login page.

If I try to refresh the page when the blank form re-appears, the warning from the browser that 'information has been entered' shows up.

So something is happening, just not a database update.

I have tried echoing.

Everything in the top part -- the $_GET method -- echoes back

Nothing echoes in the $_POST part.

Christian F. · August 1, 2012

I'm not convinced that's a good thing to say at all. To produce dynamic pages you must mix your DOM with PHP, or alternatively use a template engine which in turn mixes some other form of code with the template. Either way I wouldn't recommend not mixing the two.

I see I was a bit too inaccurate in my statement, which I apologize for. What I meant to say was that you should never mix PHP processing and HTML output, and do all of the processing before you send a single byte to the browser.

Template engines is one way to do this, yes, and perhaps the most flexible solution for the least amount of work. Though, not a necessity. Using only "echo" (or similar functions) after the doctype has been sent, and only then, is another way of accomplishing this.

As a quick example:

<?php
// Do all of the checks, processing and generation of output here.
// Resulting HTML should be saved into variables, for use below.

$page = validate_page ($_POST['page']);
$menu = gen_menu ($page);

$dailyQuote = htmlspecialcahars ($db->get_quote ());

?>
<doctype>
<html>
<body>
    <div id="menu">
<?php echo $menu; ?>
    </div>

    <div id="content">
<?php echo $content ?>
    </div>

    <footer>The quote of today is: <quote><?php echo $dailyQuote ?></quote></footer>
</body>
</html>

So something is happening, just not a database update.

I have tried echoing.

Everything in the top part -- the $_GET method -- echoes back

Nothing echoes in the $_POST part.

I know, and I told you why in my previous post. I recommend re-reading it.

Reason I commented on the separation of processing and output is because if you'd done so, you wouldn't have had this issue. Well... You might have had it, but it would have been quite obvious in that case.

123frank · August 2, 2012

Okay I have put my php at the top but the query in the $_POST is still failing.

The part that is failing is the WHERE clause. I just do not know how to get a variable to put in that column.

Is there a way to carry a variable from the query in the $_GET method forward into the $_POST method?

Here is my updated code:

<?php
include("header.php");
?>
<body> 
<div data-role="page" id="resetpassword">
<div data-role="header">
<h1>Reset Password</h1>
</div>	
<div data-role="content">
<?php
if ($_SERVER['REQUEST_METHOD'] == 'GET') { 
	$tok = ($_REQUEST['token']);
	require_once ('config.inc.php');
	$q1 = "SELECT * FROM users WHERE token = '$tok'";
	$r1 = mysqli_query($dbconn,$q1)or die("Error: ".mysqli_error($dbconn));
	printf("Number of affected rows (SELECT): %d\n", mysqli_affected_rows($dbconn));
	$row = mysqli_fetch_array($r1);	
	$tk = $row['token'];
	//echo $tk;
	$uid = $row['uid'];	
	//echo $uid;
	$then = $row['request_time'];
	//echo $then;
	$now = time();
	//echo $now;
	$expired = ($now - $then);
	//echo $expired;
	$num_rows = mysqli_num_rows($r1);		
	//echo $num_rows;
	if ($num_rows !== 1 || $expired > 900){		
	echo "An error has prevented a password change.<br />Most likely the link has expired.";	
	exit();	
	}	
	}	
if ($_SERVER['REQUEST_METHOD'] == 'POST'){  
	require_once ('config.inc.php');
	$trimmed = array_map('trim', $_POST);
	$uid = mysqli_real_escape_string($dbconn, ($trimmed['uid']));
	if (preg_match ('/^[[:alnum:]]{8,}$/', ($trimmed['password']))) {
	$p = mysqli_real_escape_string($dbconn, ($trimmed['password']));			
	// hash the password
	require ("passhash.php");
	$pass_hash = PassHash::hash($p);
	//echo $pass_hash;
	if ($pass_hash) {
	$q2 = "UPDATE users SET upass='$pass_hash' WHERE uid='$uid'";
	$r2 = mysqli_query($dbconn, $q2)or die("Error: " . mysqli_error($dbconn));
	printf("Affected rows (UPDATE): %d\n", mysqli_affected_rows($dbconn));
	$affected_rows = mysqli_affected_rows($dbconn);
	if ($affected_rows == 1) {
	echo "<p>Password Changed!</p>";
	}
	}
} else {
	echo "Please enter a valid password type.";
	}
} 
	?>		
<p>Choose a new password.<br />
Letters and numbers only.<br />
Minimum of 8 characters.</p>
<form id="passwordreset" method="post" action="?token=<?php echo $tk;?>" data-ajax="false"> 
<label for="password" class="ui-hidden-accessible">Password:</label> 
<input type="password" name="password" id="password" value="" placeholder="Password"/>
<label for="uid" class="ui-hidden-accessible">UserId:</label> 
<input type="hidden" name="uid" id="uid" value="" placeholder="UserId"/>
<button type="submit" name="submit" value="submit" data-inline="true"/>Submit</button>  
</form>		  		
</div><!-- /content -->
<div data-role="footer" class="ui-bar">
</div><!-- /footer -->
</div><!-- /page -->		
</body>
</html>

Sign In

password reset not updating users table

Recommended Posts

123frank

Link to comment

Share on other sites

cpd

Link to comment

Share on other sites

123frank

Link to comment

Share on other sites

Christian F.

Link to comment

Share on other sites

cpd

Link to comment

Share on other sites

floridaflatlander

Link to comment

Share on other sites

123frank

Link to comment

Share on other sites

Christian F.

Link to comment

Share on other sites

123frank

Link to comment

Share on other sites

Join the conversation

Browse

Activity

Important Information