Using SESSIONS in Change Password Script

[rocky48]

I am having difficulty finding out what is wrong with these scripts.

When a user logs on a session is started in the logon script:

Login.php:

<?php # - login.php
// This is the login page for the site.
session_start();

require_once ('includes/config.inc.php'); 
$page_title = 'Login';
include ('includes/header1.html');


if (isset($_POST['submitted'])) {
	require_once('includes/Connect_login.php');
	
	
	// Validate the email address:
	if (!empty($_POST['email'])) {
		$e = mysqli_real_escape_string ($dbc, $_POST['email']);
	} else {
		$e = FALSE;
		echo '<p class="error">You forgot to enter your email address!</p>';
	}
	
	// Validate the password:
	if (!empty($_POST['pass'])) {
		$p = mysqli_real_escape_string ($dbc, $_POST['pass']);
	} else {
		$p = FALSE;
		echo '<p class="error">You forgot to enter your password!</p>';
	}
	
	// Validate the BMFA No:
	if (!empty($_POST['BMFA'])) {
		$b = mysqli_real_escape_string ($dbc, $_POST['BMFA']);
	} else {
		$b = FALSE;
		echo '<p class="error">You forgot to enter your BMFA number!</p>';
	}
	
	if ($e && $p && $b) { // If everything's OK.
	
		// Query the database:
		$q = "SELECT user_id, first_name, user_level, BMFA_No FROM users WHERE (email='$e' AND pass=SHA1('$p') AND BMFA_No= ('$b')) AND active IS NULL";		
		$r = mysqli_query ($dbc, $q) or trigger_error("Query: $q\n<br />MySQL Error: " . mysqli_error($dbc));
		
		if (@mysqli_num_rows($r) == 1) { // A match was made.

			// Register the values & redirect:
			$_SESSION = mysqli_fetch_array ($r, MYSQLI_ASSOC);
			$_SESSION['loggedin']=$_POST['user_id'];
			echo $_POST['user_id'];
			mysqli_free_result($r);
			mysqli_close($dbc);
							
			$url = BASE_URL . 'mempage.php'; // Define the URL:
			ob_end_clean(); // Delete the buffer.
			header("Location: $url");
			exit(); // Quit the script.
				
		} else { // No match was made.
			echo '<p class="error">Either the email address and password entered do not match those on file or you have not yet activated your account.</p>';
			
		}
		
	} else { // If everything wasn't OK.
		echo '<p class="error">Please try again.</p>';
	}
	
	mysqli_close($dbc);

} // End of SUBMIT conditional.

?>

<h1>Login</h1>
<p>Your browser must allow cookies in order to log in.</p>

<form action="login.php" method="post">
	<fieldset>
	<p><b>Email Address:</b> <input type="text" name="email" size="20" maxlength="40" /></p>
	<p><b>Password:</b> <input type="password" name="pass" size="20" maxlength="20" /></p>
	<p><b>BMFA No:</b> <input type="text" name="BMFA" size="20" maxlength="20" /></p>
	<div align="center"><input type="submit" name="submit" value="Login" /></div>
	<input type="hidden" name="submitted" value="TRUE" />
	</fieldset>
</form>
<h2>Forgot your password?  Click on this link:<a href="forgot_password.php"target="_blank">Forgot Password</a></h2>
<?php // Include the HTML footer.
include ('includes/footer1.html');
?>

I am using output buffering, so that I can include files and only send to the server when the form is submitted.

Would this screw up the sessions?

I'm not sure if the way I have set the session is correct on line 48 as there is a session which is saving the array of the MYSQL query.

I am using a book for this example, so I don't understand  how the first session statement is saving the array, if it is not named?

 

The script allows me to login OK and on the page it redirects to I have put a button that allows the user to change their password.

This runs the change_password.php script.

<?php #  Change_password.php
// This page allows a logged-in user to change their password.
session_start();
ob_start;
require_once ('includes/config.inc.php'); 
$page_title = 'Change Your Password';
include ('includes/header1.html');

// If no first_name session variable exists, redirect the user:
if (!isset($_SESSION['email'])) {
	
	$url = BASE_URL . 'index.php'; // Define the URL.
	ob_end_clean(); // Delete the buffer.
	header("Location: $url");
	exit(); // Quit the script.
}
print_r($_POST);
if (isset($_POST['submitted'])) {
	require_once('includes/Connect_login.php');
			
	// Check for a new password and match against the confirmed password:
	$p = FALSE;
	if (preg_match ('/^(\w){4,20}$/', $_POST['password1']) ) {
		if ($_POST['password1'] == $_POST['password2']) {
			$p = mysqli_real_escape_string ($dbc, $_POST['password1']);
		} else {
			echo '<p class="error">Your password did not match the confirmed password!</p>';
		}
	} else {
		echo '<p class="error">Please enter a valid password!</p>';
	}
	
	if ($p) { // If everything's OK.

		// Make the query.
		$q = "UPDATE users SET pass=SHA1('$p') WHERE user_id={$_SESSION=$_POST['user_id']";	
		$r = mysqli_query ($dbc, $q) or trigger_error("Query: $q\n<br />MySQL Error: " . mysqli_error($dbc));
		if (mysqli_affected_rows($dbc) == 1) { // If it ran OK.
		
			// Send an email, if desired.
			echo '<h3>Your password has been changed.</h3>';
			mysqli_close($dbc); // Close the database connection.
			include ('includes/footer.html'); // Include the HTML footer.
			exit();
			
		} else { // If it did not run OK.
		
			echo '<p class="error">Your password was not changed. Make sure your new password is different than the current password. Contact the system administrator if you think an error occurred.</p>'; 

		}

	} else { // Failed the validation test.
		echo '<p class="error">Please try again.</p>';		
	}
	
	mysqli_close($dbc); // Close the database connection.

} // End of the main Submit conditional.

?>

<h1>Change Your Password</h1>
<form action="change_password.php" method="post">
	<fieldset>
	<p><b>New Password:</b> <input type="password" name="password1" size="20" maxlength="20" /> <small>Use only letters, numbers, and the underscore. Must be between 4 and 20 characters long.</small></p>
	<p><b>Confirm New Password:</b> <input type="password" name="password2" size="20" maxlength="20" /></p>
	</fieldset>
	<div align="center"><input type="submit" name="submit" value="Change My Password" /></div>
	<input type="hidden" name="submitted" value="TRUE" />
</form>

<?php
include ('includes/footer1.html');
?>

When this runs all I get is a blank page and not the message that the password has been changed.

As there are no error messages I am not sure where the script is wrong.

 

Can someone tell me where I am going wrong?

 

[NotionCommotion]

The following resets all $_SESSION indexes.  This is probably not what you want, and should do $_SESSION['someIndex']=....

$_SESSION = mysqli_fetch_array ($r, MYSQLI_ASSOC);

Then, when this happens, $_SESSION['email'] is not set because of the above line, and it exists.

if (!isset($_SESSION['email'])) {

[mac_gyver]

your change_password.php script contains a php syntax error on line 36.

 

you need to set php's error_reporting to E_ALL and display_errors to ON in the php.ini on your development system to get php to help you. if you try to set these settings in your code, it won't help find php syntax errors in the same file because your code never runs when there is a php syntax error and the code trying to set the settings is never executed.

 

next, as NotionCommotion posted above, you don't have a session variable named email, so the logic testing that session variable will always be false. when you log in a user, the only thing you should store in a session variable is the user id from the row in the user table. to get any of the other user data, you would use that user id to query for it when you need it.

 

and, as you have probably seen in forum posts, you need to use php's password_hash() and password_verify() functions for your password hashing and you need to remove any @ error suppressors in your code.

[rocky48]

Firstly, I was at first trying to use the email address as the session data to use throughout the session, but soon realized tat was not a good idea.

So my scripts were not totally changed.

Here are the changes I have made.

Login script:

			// Register the values & redirect:
			//$_SESSION = mysqli_fetch_array ($r, MYSQLI_ASSOC);
			$_SESSION['loggedin']=$_POST['user_id'];
	

As you can see I have only commented out the superfluous line for now.

Now to the Change_password script:

		$q = "UPDATE users SET pass=SHA1('$p') WHERE user_id={$_SESSION['loggedin']::$_POST['user_id']}";	
		$r = mysqli_query ($dbc, $q) or trigger_error("Query: $q\n<br />MySQL Error: " . mysqli_error($dbc));

When i run this on my localhost, I get the following error:

 

( ! ) Parse error: syntax error, unexpected '=', expecting :: (T_PAAMAYIM_NEKUDOTAYIM) in C:\wamp64\www\MFC1066\change_password.php on line 36

According to the web query that strange sounding error message is to do with double colons.

It is inferring that it should be :: and not =, but when I changed it to the double colons it returned to my home page and did not process the html code on lines 62 onward.  The ob_start is in the header1.html script!

Any ideas to why this is happening?

Edited February 23, 2017 by rocky48

[rocky48]

Just realized why it returns to home page!

Its due to this line:

if (!isset($_SESSION['loggedin'])) {
	
	$url = BASE_URL . 'index.php'; // Define the URL.
	ob_end_clean(); // Delete the buffer.
	header("Location: $url");
	exit(); // Quit the script.
}

So I guess it is not recognizing the session.

Should I have the session start on this script?

[rocky48]

Having some more thoughts on this!

Do I need the line:
 

$_SESSION = mysqli_fetch_array ($r, MYSQLI_ASSOC);
			

Does this capture all of the data in the SELECT query?

So if I then have the line:

$_SESSION['loggedin']='user_id';

Should this not pass the user_id as a named session, so when I try to use it in the change_password script it will identify the user?

Is my thinking correct or am I totally wrong?

[Barand]

Should I have the session start on this script?

You need session_start() at beginning of every script that needs to reference $_SESSION.

Edited February 23, 2017 by Barand

[rocky48]

I eventually found that ob_start should go BEFORE Session_start, so it now recognizes the session.

However, I now have another problem!

The query is not working on line 436 of change_password.php.

All I get is the echo of the array when the line is like so:

$q = "UPDATE users SET pass=SHA1('$p') WHERE user_id={$_SESSION=$_POST['user_id']";    

I tried changing it but keep getting errors.

I tried this:

$q = "UPDATE users SET pass=SHA1('$p') WHERE user_id={$_SESSION['loggedin']::'user_id'}";

As this field is not one that has been posted, how do you retrieve it from the record?

[rocky48]

I take it back!

The SESSION is still not being recognized!

I am at my wits end trying to solve this problem.

 

Please can anyone help?

[Jacques1]

The scripts are riddled with problems. Even if you spend another week trying to fix the obvious bugs, it will still be bad code, so it's not really worth the trouble.

 

I would rewrite the code from scratch, this time using modern practices.

• Learn how to use mysqli properly, especially how to perform prepared statements. Or even better: Switch to PDO.
• Stop doing those output buffering gymnastics. Write proper code that works without any band-aid.
• Structure your code in a meaningful way (business logic at the top, output at the bottom). Also avoid useless actions like closing the database connection right before the script ends. PHP does this automatically.
• SHA-1 cannot protect your passwords. Today, any cell phone has enough computing power to perform a brute-force attack. Use a proper algorithm like bcrypt.
• The session ID must be regenerated after the log-in procedure. Otherwise the application will be wide open to session fixation attacks.
• Write valid HTML.

A template:

<?php

// "require" statements go here

session_start();

// collect errors, then display them at the bottom
$input_errors = [];

if ($_SERVER['REQUEST_METHOD'] == 'POST')
{
    if (!isset($_POST['email']) || trim($_POST['email']) == '')    // empty() is usually a bad idea, because even strings like "0" are considered "empty"
    {
        $input_errors['email'] = 'You forgot to enter your e-mail address!';
    }

    // etc.

    if (!$input_errors)
    {
        // fetch the user data with a prepared statement (this example uses PDO)
        $user_stmt = $database_connection->prepare('
            SELECT ...
        ');
        $user_stmt->execute([
            'email' => $_POST['email'],
        ]);
        $user = $user_stmt->fetch();

        if ($user && password_verify($_POST['pass'], $user['pass']))
        {
            session_regenerate_id(true);
            $_SESSION['user_id'] = $user['user_id'];

            redirect(BASE_URL.'mempage.php');    // write your own function to avoid repeating the same boilerplate code over and over again
        }
        else
        {
            $input_errors['pass'] = 'Wrong e-mail address or password!';
        }
    }
}

?>
<!DOCTYPE html>
<html lang="en">
    <head>
        <meta charset="utf-8">
        <title>Title</title>
    </head>
    <body>

    </body>
</html>

As you can see, this is much more straightforward and compact than what you currently have.