Forget password page not working...

[man5]

I am trying to create a forget password option.  For some reason, it's not working. Please check the code below to see what is wrong with it.

      if(isset($_POST['submit'])) {
				
		if(!empty($_POST['email'])) {	
		
			$email = escape($_POST['email']);
			
			$getData = DB::getInstance()->query("SELECT * FROM users WHERE email = {$email}");
			
			if($getData->count()) {
				foreach($getData->results() as $row) {
					$username	=	escape($row->username);
					$full_name	=	escape($row->full_name);
					$password	=	escape($row->password);
					$db_email	=	escape($row->email);
				}
			    } else {
				echo 'No data returned!';
			    }

 			if($email == $db_email) {
				
				$new_password = substr(md5(rand(999, 999999)), 0, ;
				
				$to = $db_email;
				$subject = "Password Reset";
				$body = "Hello " . $full_name . ",\n\nYour new password is: " . $new_password . "\n\n-Favecave";
				
				$updatePass = DB::getInstance()->query("UPDATE users SET password = {$new_password} WHERE email= {$email}");
				mail($to,$subject,$body);
				
				
			} else {
				echo 'Email not found';
			}	
		} else {
			echo 'Please enter field';
		}	
	}
?>
<form action="" method="post">
	<span>Enter your email address:<span><br>
	<input type="email" name="email" size="40"><br>
	<input type="submit" value="Recover">
</form>

[Psycho]

Well, why don't you tell us what IS happening instead of just stating it doesn't work. You have no debugging code to even help you know what vales are or are not present - which would help.

 

But, there is no input type of 'email' - so that may be your problem.

 

<input type="email" name="email" size="40"><br>

[man5]

That's the weird thing. Nothing happens. No errors show up. It doesn't process.

 

As for the input type. It is actually available in HTML5.

[Psycho]

Well, the flow of your logic has problems. It is always best to check for errors rather than the lack of errors - otherwise the errors appear at the end of the code separated by the check for them. For example, the "Please enter field" error at the end of the code is based upon the condition at the very beginning. Better to do something like this

f(empty($_POST['email']))
{
    echo 'Please enter field';
}
else
{
   //Continue with success condition

Also, you have a duplication of error conditions. You do this condition after running the query

if($getData->count()) {

Which, based on the error is to see if there was a matching record. But, later you do another check

if($email == $db_email) {

If there was a record in the result set then that means the email matched, so why the second check. Plus, you are comparing the escaped value of $email to the DB return value of $db_email - which would not match if anything was actually escaped. but, I don't know what the function escape() does. You are using the same function on the a value returned from the query for use in the email. Different outputs/usages require different types of escaping. I assume you are using a function you built. Don't. Use the functions that already exist. Plus, you should UPDATE the password before you send the email that it was updated.

 

Ok, so having said all of the above, I am guessing your query is failing. I see that you are not enclosing the email in the query within quotes! Here is a complete rewrite of the code in a more logical flow with some additional error handling - not to mention COMMENTS. Use them! They will help you (and us when you ask for help).

<?php
 
$message = '';
 
//Check if the form was submitted
if(isset($_POST['email']))
{
    //Trim the post value and verify it is not empty
    $postEmail = trim($_POST['email']);
    if(empty($postEmail))
    {
        //Email was empty
        $message = "Please enter email<br>\n";
    }
    else
    {
        //Run select query using email
        $emailSQL = escape($postEmail);
        $getData = DB::getInstance()->query("SELECT email, full_name FROM users WHERE email='{$emailSQL}'");
        //Check if query passed
        if(!$getData)
        {
            //Query failed
            $message = "Select query failed<br>\n";
        }
        elseif(!$getData->count())
        {
            //No records returned
            $message = "Email not found<br>\n";
        }
        else
        {
            //Record returned, attempt to set temp password
            $full_name = htmlentities($row->full_name);
            $email     = $row->email;
            $new_password = substr(md5(rand(999, 999999)), 0, ;
            $updatePass = DB::getInstance()->query("UPDATE users SET password = {$new_password} WHERE email='{$emailSQL}'");
            //Check if query passed
            if(!$updatePass)
            {
                //Query failed
                $message = "Update query failed<br>\n";
            }
            else
            {
                //Send confirmation email
                $subject = "Password Reset";
                $body = "Hello {$full_name},\n\nYour new password is: {$new_password}\n\n-Favecave";
                if(!mail($to, $subject, $body))
                {
                    $message = "Error sending email.<br>\n";
                }
                else
                {
                    $message = "An email has been sent with a new temp password.<br>\n";
                }
            }
        } 
}
}
?>

<?php echo $error; ?>
<form action="" method="post">
<span>Enter your email address:<span><br>
<input type="email" name="email" size="40"><br>
<input type="submit" value="Recover">
</form>

Edited March 23, 2014 by Psycho

[man5]

That's great.  It makes it clear now. The only thing you are missing in the above code is foreach statement to get the variables.  I did that and it's good now. It does email the new pass code.

 

Now on to a next issue.  I am unable to login with the new pass code.  Also, it doesn't update the password in the database. I am assuming that is done after I login with the new pass code?

 

I would like to mention that the original password created from registration is done with hash and salt.  In the DB, there are two columns;

Password column
a04eff113e34a39fab1410c22d0b4061ffe300dbd3d7244a8d

Salt column
–°Ò'{èUQð¾~îþhâLY§Ò÷×ÍB9ù‡\

[man5]

Update.

 

I got it to update the database with the new password(the email matching was incorrect).

 

Now when it updatest he database with the new password, it inputs the same string as as the one emailed. For eg. 0a6564e8.  It should be showing the encrypted version of the password in the database.

The passwords in the DB are encrypted with hash and salt functions, which are this.

public static function make($string, $salt = '')  {
     return hash('sha256', $string . $salt);
}
	
public static function salt($length)  {
      return mcrypt_create_iv($length);
}

If I am correct with all of the above. Then I have to get the following variable right.

$salt = Hash::salt(32);
$new_password = substr(Hash::make(rand(999, 999999)), 0, 8, $salt));

Edited March 23, 2014 by man5

[man5]

Update again.

 

So i got it working. I get the substring code sent to my  email, while hashed and salted password is updated in the DB. Here is the code.

$email_password = substr(Hash::make(rand(999, 999999),$salt), 0, ;
			
$new_password = Hash::make(rand(999, 999999),$salt);

$updatePass = DB::getInstance()->query("UPDATE users SET password = '{$new_password}', salt = '{$salt}' WHERE email='{$email}'");

Now the only problem left is that it won't log me in with the new code. Granted I can't login with the old pass, which is good. 

[man5]

It'll probably help to see what my login function looks like.

public function login($email = null, $password = null, $remember = false) {
		
		if(!$email && !$password && $this->exists()) {
			Session::put($this->_sessionName, $this->data()->id);
		} else {
			$user = $this->find($email);
			
			if($user) {
				if($this->data()->password === Hash::make($password, $this->data()->salt)) {
					Session::put($this->_sessionName, $this->data()->id);
					
					if($remember) {
						$hash = Hash::unique();
						$hashCheck = $this->_db->get('users_session', array('user_id', '=', $this->data()->id));
						
						if(!$hashCheck->count()) {
							$this->_db->insert('users_session', array(
								'user_id' => $this->data()->id,
								'hash' => $hash
							));
						} else {
							$hash = $hashCheck->first()->hash;
						}
						
						Cookie::put($this->_cookieName, $hash, Config::get('remember/cookie_expiry'));
					}
					
					return true;
				}
			}
		}
		return false;
	}

my login.php page
 

if(Input::exists()) {
	if(Token::check(Input::get('token'))) {
	
		$validate = new Validate();
		$validation = $validate->check($_POST, array(
			'email' => array('required' => true),
			'password' => array('required' => true)
		));
		
		if ($validation->passed()) {
			$user = new User();
			
			$remember = (Input::get('remember') === 'on') ? true : false;
			
			$login = $user->login(Input::get('email'), Input::get('password'), $remember);
			
			if($login) {
				Redirect::to('index.php');
			} else {
				echo '<p>Sorry, login failed.</p>';
			}
			
		} else {
			foreach($validation->errors() as $error) {
				echo $error, '<br>';
			}
		}
	}
}

?>


<form action="" method="post">
	<div class="field">
		<label for="email">Email</label>
		<input type="text" name="email" autocomplete="off">
	</div>
	<div class="field">
		<label for="password">Password</label>
		<input type="password" name="password" autocomplete="off">
	</div>
	<div class="field">
		<label for="remember">
			<input type="checkbox" name="remember" id="remember" autocomplete="off">Remember me
		</label>
	</div>
	<input type="hidden" name="token" value="<?php echo Token::generate(); ?>">
	<input type="submit" value="Log in">
	<div class="forgot_password">
		<span><a href="reset_password.php">Forgot password?</a></span><br>
	</div>
</form>

Edited March 23, 2014 by man5

[Psycho]

 

That's great.  It makes it clear now. The only thing you are missing in the above code is foreach statement to get the variables.  I did that and it's good now. It does email the new pass code.

 

No, you don't need a foreach() for that scenario. The emails should be unique in the database, so you should only get 0 or 1 results. You just need to make a single call to get that one record.

[man5]

No, you don't need a foreach() for that scenario. The emails should be unique in the database, so you should only get 0 or 1 results. You just need to make a single call to get that one record.

Yes you are correct that the emails should be unique.  However we need to get the $full_name variable to send in email.  So we require the use of foreach to get that from DB.

 

Below is the updated code.  I've been at it all day and still can't fix it. As far as I can tell, the emailed password does not match with the stored password when i try to log in, despite the fact they are the same.

<?php require_once 'core/init.php'; ?>
	
<!DOCTYPE HTML>
<html lang="en">
<head>
</head>	
<body><?php	
 

//Check if the form was submitted
if(isset($_POST['email'])) {
    //Trim the post value and verify it is not empty
    $postEmail = Input::get('email');
	
    if(empty($postEmail)) {
        //Email was empty
        echo 'please enter email';
    }
	
    else {
        //Run select query using email
        $emailSQL = escape($postEmail);
		
        $getData = DB::getInstance()->query("SELECT * FROM users WHERE email='{$emailSQL}'");
        //Check if query passed
       if(!$getData) {
            //Query failed
             echo 'select query failed';
        }
        
		elseif(!$getData->count()) {
            //No records returned
             echo 'email not found';
        }
       
	   else {
			foreach($getData->results() as $row) {
				$full_name = $row->full_name;
			}	
		
            $salt = Hash::salt(32);
			$new_password = Hash::make(rand(999, 999999),$salt);
			
			$temp_pass	= substr($new_password, 0, ; 

			$updatePass = DB::getInstance()->query("UPDATE users SET password = '{$new_password}', salt = '{$salt}' WHERE email='{$postEmail}'");
            
			//Check if query passed
            if(!$updatePass) {
                //Query failed
                 echo 'updated query failed';
            }
			
            else {
                //Send confirmation email
				$to = $postEmail;
                $subject = "Password Reset";
                $body = "Hello {$full_name},\n\nYour new password is: {$temp_pass}\n\n-mycomp";
                
				if(!mail($to, $subject, $body)) {
                     echo 'error sending email';
                }
                
				else {
                     echo 'an email has been sent';
                }
            }
        } 
	}
}


?>
	<form action="" method="post">
	<span>Enter your email address:<span><br>
	<input type="text" name="email" size="40"><br>
	<input type="submit" value="Recover">
	</form>
</body>
</html>

Edited March 24, 2014 by man5

[mac_gyver]

during the log in process, exactly at which step in that code is the data and/or code doing what you expect and exactly at what step is the data and/or code failing and indicating the login is not successful? since we don't have your full code or your database, YOU must pin down the problem to the specific statement in the code that is giving an indicate that the log in failed, and at that point, what exactly is the data vs what the data should be.

public function login($email = null, $password = null, $remember = false) {

also, are you seriously writing code like the above? the $email and $password parameters are required for the function to work. if you are calling this function without supplying any of those call time parameters, there's no point in even calling it. making all the parameters optional, as coded, is opening you up to writing code that won't throw errors when used incorrectly.

[mac_gyver]

you need to rethink your password reset logic. your current code will allow anyone who learns, sees, or just tries a bunch of email addresses to go through and trash existing user's passwords, because you are directly updating the password in the users table when the request to reset a password is made.

 

the request to reset a password should be stored in a separate table and should only update in the actual users table when the new password that was sent in the email gets used.

 

as to why your last posted code isn't working. the new password is whatever is produced by rand(999, 999999). the new password is NOT in $new_password or any part of $new_password. $new_password is the salted/hashed value. you would need to store the rand(999, 999999) value in a variable in order to be able to send it through an email and to use that same value in the Hash::make( ... ,$salt); statement.

Edited March 24, 2014 by mac_gyver

[man5]

Yes I came to that conclusion as well(after 2 days of sweat and tears).  So I decided to stop beating the dying horse.

 

I have finally found a solution. Works beautifully.

 

Instead of trying to change the user's password mysql, I decided to give the user an option to update their own password.

 

Thanks for all your help guys. I needed it to find the solution.