-1

Afternoon.

I am trying to create a password reset page using php. Upon clicking the reset button I get my password reset successful message but no changes have been made to my database.

Any help would be appreciated.

<?php
  session_start();
  $_SESSION['message'] = '';
  $mysqli = new mysqli("localhost", "User", "password", "DarrenOBrien");

  if ($_SESSION['loggedin']) {
    if ( $_SERVER['REQUEST_METHOD'] == 'POST' ) {

      $email=$_SESSION('email');
      $result = $mysqli->query("SELECT * FROM accounts WHERE userEmail='$email'") or die($mysqli->error);

      $user = $result->fetch_assoc();
        if (password_verify($_POST['oldpassword'], $user['userPassword'])) {
          if (($_POST['newpassword'] == $_POST['confirmnewpassword'])) {
            $newpass=password_hash($_POST['confirmnewpassword'], PASSWORD_BCRYPT);
            $sql = "UPDATE accounts SET userPassword='$newpass' WHERE userEmail='$email'";
            $_SESSION['message'] = 'Password reset successful';
          }
          else {
            $_SESSION['message'] = 'Passwords do not match. Please try again.';
          }
        }
        else {
          $_SESSION['message'] = 'Old password does not match password in records. Please try again.';
        }



    }
  }
  else {
    header('location: register.php');
  }

?>


<head>
  <meta charset="utf-8">
  <meta http-equiv="X-UA-Compatible" content="IE=edge">
  <meta name="viewport" content="width=device-width, initial-scale=1">
  <title>Login</title>
  <link rel="stylesheet" href="css/bootstrap.min.css">
  <link rel="stylesheet" href="css/styles.css">
</head>

<body>
  <!--Navbar-->
   <nav class="navbar navbar-inverse">
     <div class="container">
       <div class="navbar-header">
         <button type="button" class="navbar-toggle collapsed" data-toggle="collapse" data-target="#navbar" aria-expanded="false" aria-controls="navbar">
           <span class="sr-only">Toggle navigation</span>
           <span class="icon-bar"></span>
           <span class="icon-bar"></span>
           <span class="icon-bar"></span>
         </button>
         <a class="navbar-brand" href="welcome.php">PHP Project </a>
       </div>
       <div id="navbar" class="collapse navbar-collapse">
         <ul class="nav navbar-nav">
           <li><a href="welcome.php">Home</a></li>
           <li class="active"><a href="profile.php">Profile</a></li>
           <li><a href="products.php">Products</a></li>
         </ul>
         <a href="logout.php" class="navbar-brand pull-right">Logout</a>
       </div>
     </div>
   </nav>
   <!--End of Navbar-->


   <div class="container-fluid" id="profile">
     <form action="reset.php" method="post" enctype="multipart/form-data" autocomplete="off">
      <div class="alert-error"><?= $_SESSION['message'] ?></div>

       <div class="form-group">
         <label for="oldpass">Old Password:</label>
         <input type="password" class="form-control" id="oldpass" placeholder="Password" name="oldpassword" autocomplete="new-password" minlength="4" required />
       </div>

       <div class="form-group">
         <label for="newpass">New Password:</label>
         <input type="password" class="form-control" id="newpass" placeholder="Password" name="newpassword" autocomplete="new-password" minlength="4" required />
       </div>

       <div class="form-group">
         <label for="confirmnewpass">Confirm New Password:</label>
         <input type="password" class="form-control" id="confirmnewpass" placeholder="Password" name="confirmnewpassword" autocomplete="new-password" minlength="4" required />
       </div>

       <input type="submit" value="Reset Password" name="reset" class="btn btn-block btn-primary" id="resetbtn"/>
     </form>
   </div>

<!-- Required bootstrap scripts -->
  <script src="js/jquery-3.2.1.min.js"></script>
  <script src="js/bootstrap.min.js"></script>
<!-- End of required bootstrap scripts -->
</body>
  • 2
    What is this `$_SESSION('email')`? Turn on your [PHP errors](https://stackoverflow.com/questions/1053424/how-do-i-get-php-errors-to-display) if you have not done so. – Mikey Jun 12 '17 at 16:23
  • It's a session variable containing the email address of the user. – Darren O'BRIEN Jun 12 '17 at 16:25
  • 2
    Shouldn't it be `$_SESSION['email']`? – Mikey Jun 12 '17 at 16:26
  • 1
    .... which would be a syntax / parse error. Error reporting is always someone's true friend. – Funk Forty Niner Jun 12 '17 at 16:26
  • 1
    And the query isn't being executed. And you should also bind your variables instead of injecting them directly into the querystring. – Qirel Jun 12 '17 at 16:27
  • 1
    @Fred-ii- No it is my best friend. He helps me everyday :P – Mikey Jun 12 '17 at 16:28
  • Why isn't the query being executed? – Darren O'BRIEN Jun 12 '17 at 16:28
  • Also, good catch @Mikey. I've been fiddling around with it for a while trying to fix my problem and I guess I entered the session variable wrong. – Darren O'BRIEN Jun 12 '17 at 16:29
  • Because you don't run the query, `$mysqli->query($sql)`. You just create the querystring, but that doesn't magically talk to the database without you making it do so ;-) – Qirel Jun 12 '17 at 16:29
  • Perfect. Thank you. Though I'm also interested in binding variables? Where could I learn more about that? – Darren O'BRIEN Jun 12 '17 at 16:31
  • @Mikey I do? hehe, always a pleasure ;-) – Funk Forty Niner Jun 12 '17 at 16:32
  • [Have a look at `mysqli_stmt::bind_param()`](http://php.net/manual/en/mysqli-stmt.bind-param.php), @Darren. And you'll need [`mysqli_stmt::bind_result()`](http://php.net/manual/en/mysqli-stmt.bind-result.php) when selecting. – Qirel Jun 12 '17 at 16:32
  • *"and I guess I entered the session variable wrong"* - @DarrenO'BRIEN are you saying that your `$_SESSION('email')` is actually `$_SESSION['email']`? which is what it should be, an array `[]` and not a function `()`. – Funk Forty Niner Jun 12 '17 at 16:41
  • @DarrenO'BRIEN I updated my answer with two complete examples of preparing statements and binding variables using `mysqli` library (procedural style). It should be interesting for you from the workflow point of view. The object-oriented style would then be probably a parallel to it. Good luck! Notice the exception handling codes. You can further insert `mysqli_error*` functions in other needed places. All these functions should be used only for development, not on production. –  Jun 12 '17 at 16:50

3 Answers3

0

use

 $reuslt = $mysqli->query("UPDATE accounts SET userPassword='$newpass' 
    WHERE userEmail='$email'");
    if ($result) {
     $_SESSION['message'] = 'Password reset successful';
    } else {
     $_SESSION['message'] = 'Passwords do not match. Please try again.';
    }

Because without query you just prepared the sql format and didn't send it to the database.

GhusiMushi
  • 33
  • 3
0

I would like to direct your eyes to this piece of code here

if (($_POST['newpassword'] == $_POST['confirmnewpassword'])) {
        $newpass=password_hash($_POST['confirmnewpassword'], PASSWORD_BCRYPT);
        $sql = "UPDATE accounts SET userPassword='$newpass' WHERE 
        userEmail='$email'";
        $_SESSION['message'] = 'Password reset successful';
      }

Here your $sql variable holds an sql statement, that is, a plain text string that currently does nothing, you have to execute it, much like you executed the select query above

if ($mysqli->query($sql) === TRUE) {
    $_SESSION['message'] = 'Password reset successful'; 
} else {
    $_SESSION['message'] = "Error updating record: " . $mysqli->error;
}

As taken from w3Schools

Also if that's the whole extent of your endpoint, you should remember to close the connection, calling the close method of your mysqli class instance

Last but not least, I would strongly recommend that you do not use the class name (mysqli) as your instance name ($mysqli), just for the sake of good practice

EDIT:

The comments received are right indeed, my answer is quite poor at this point, so let's take into account a few things

You should use prepared statements instead of throwing variables directly at the sql query, someone that's clever enough could use that to inject sql statements to your database

Please correct me if I'm wrong but this could be a lot safer this way:

//Email select query part
$email= $mysqli->real_escape_string($_SESSION['email']);
$stmt = $mysqli->prepare("SELECT * FROM accounts WHERE userEmail=(?)")
if (!$stmt->bind_param("s", mysqli->$email)) {
    echo "Binding parameters failed: (" . $stmt->errno . ") " . $stmt->error;
    //handle error code, disrupt execution...
}

if (!$stmt->execute()) {
    echo "Execute failed: (" . $stmt->errno . ") " . $stmt->error;
    //handle error code, disrupt execution...
}


//Update part
$newpass=password_hash(
    $mysqli->real_escape_string($_POST['confirmnewpassword']),
    PASSWORD_BCRYPT);
$stmt = mysqli->prepare("UPDATE accounts SET userPassword=(?) WHERE 
userEmail=(?)");
if (!$stmt->bind_param("ss", $newpass,$email)) {
    echo "Binding parameters failed: (" . $stmt->errno . ") " . $stmt->error;
    //handle error code, disrupt execution...
}

if (!$stmt->execute()) {
    echo "Execute failed: (" . $stmt->errno . ") " . $stmt->error;
    //handle error code, disrupt execution...
}
$_SESSION['message'] = 'Password reset successful';

Now I'm sure this can be refactored in much more efficient ways, but I hope I helped OP see what's up with his code

graren
  • 74
  • 5
  • However correct, it's just not a complete answer in regards to `$_SESSION('email')`. It's also prone to sql injection. – Funk Forty Niner Jun 12 '17 at 16:35
  • 1
    Since OP is using mysqli, you could have added prepared statements – Rotimi Jun 12 '17 at 16:45
  • *"Please correct me if I'm wrong"* - The session is array you included `$_SESSION('email')` is still considered a function `()` rather than an array `[]`. Consult the manual on superglobals http://php.net/manual/en/language.variables.superglobals.php and the brackets in `userEmail=(?)` are not necessary. – Funk Forty Niner Jun 13 '17 at 10:10
0

You forgot to run the password updating query: So, insert this:

$updated = $mysqli->query($sql) or die($mysqli->error);

after this:

$sql = "UPDATE accounts SET userPassword='$newpass' WHERE userEmail='$email'";

EDIT 1 - How to prepare and run queries using mysqli library:


Option 1: Using mysqli_stmt_get_result() + mysqli_fetch_array():

<?php

/*
 * Run prepared db queries.
 * 
 * Uses:
 *      - mysqli_prepare()
 *      - mysqli_stmt_bind_param()
 *      - mysqli_stmt_execute()
 *      - mysqli_stmt_get_result()
 *      - mysqli_fetch_array()
 */

try {
    $username = 'Hello';
    $password = 'World';

    //---------------------------------------------------------
    // Connect to db.
    //---------------------------------------------------------
    $conn = mysqli_connect('<host>', '<user>', '<pass>', '<db>');
    if (!$conn) {
        throw new Exception('Connect error: ' . mysqli_connect_errno() . ' - ' . mysqli_connect_error());
    }

    //---------------------------------------------------------
    // Sql statement.
    //---------------------------------------------------------
    $query = "SELECT * FROM users WHERE username = ? AND password = ?";

    //---------------------------------------------------------
    // Prepare sql statement.
    //---------------------------------------------------------
    $stmt = mysqli_prepare($conn, $query);
    if (!$stmt) {
        throw new Exception('The sql statement can not be prepared!');
    }

    //---------------------------------------------------------
    // Bind variables to the prepared statement as parameters.
    //---------------------------------------------------------
    $bound = mysqli_stmt_bind_param($stmt, 'ss', $username, $password);
    if (!$bound) {
        throw new Exception('The variables could not be bound to the prepared statement!');
    }

    //---------------------------------------------------------
    // Execute the prepared statement.
    //---------------------------------------------------------
    $executed = mysqli_stmt_execute($stmt);
    if (!$executed) {
        throw new Exception('The prepared statement could not be executed!');
    }

    //---------------------------------------------------------
    // Get the result set from the prepared statement.
    //---------------------------------------------------------
    $result = mysqli_stmt_get_result($stmt);
    if (!$result) {
        throw new Exception(mysqli_error($conn));
    }

    //---------------------------------------------------------
    // Get the number of rows in statements result set.
    //---------------------------------------------------------
    $rows = mysqli_num_rows($result);

    if ($rows > 0) {
        //---------------------------------------------------------
        // Read the result set.
        //---------------------------------------------------------
        $row = mysqli_fetch_array($result, MYSQLI_ASSOC);
        if (!isset($row)) {
            echo 'No records returned!';
            exit();
        }

        echo 'Login successful: ' . $row['username'] . '/' . $row['password'];
    } else {
        echo 'Invalid username/password. Please check and retry login.';
    }

    //-----------------------------------------------------------
    // Frees stored result memory for the given statement handle.
    //-----------------------------------------------------------
    mysqli_stmt_free_result($stmt);

    //---------------------------------------------------------
    // Close db connection.
    //---------------------------------------------------------
    $closed = mysqli_close($conn);
    if (!$closed) {
        throw new Exception('The database connection can not be closed!');
    }
} catch (Exception $exception) {
    echo '<pre>' . print_r($exception, true) . '</pre>';
    exit();
}


Option 2: Using mysqli_stmt_store_result() + mysqli_stmt_bind_result() + mysqli_stmt_fetch():

<?php

/*
 * Run prepared db queries.
 * 
 * Uses:
 *      - mysqli_prepare()
 *      - mysqli_stmt_bind_param()
 *      - mysqli_stmt_execute()
 *      - mysqli_stmt_store_result()
 *      - mysqli_stmt_bind_result()
 *      - mysqli_stmt_fetch()
 */

try {
    $username = 'Hello';
    $password = 'World';

    //---------------------------------------------------------
    // Connect to db.
    //---------------------------------------------------------
    $conn = mysqli_connect('<host>', '<user>', '<pass>', '<db>');
    if (!$conn) {
        throw new Exception('Connect error: ' . mysqli_connect_errno() . ' - ' . mysqli_connect_error());
    }

    //---------------------------------------------------------
    // Sql statement.
    //---------------------------------------------------------
    $query = "SELECT * FROM users WHERE username = ? AND password = ?";

    //---------------------------------------------------------
    // Prepare sql statement.
    //---------------------------------------------------------
    $stmt = mysqli_prepare($conn, $query);
    if (!$stmt) {
        throw new Exception('The sql statement can not be prepared!');
    }

    //---------------------------------------------------------
    // Bind variables to the prepared statement as parameters.
    //---------------------------------------------------------
    $bound = mysqli_stmt_bind_param($stmt, 'ss', $username, $password);
    if (!$bound) {
        throw new Exception('The variables could not be bound to the prepared statement!');
    }

    //---------------------------------------------------------
    // Execute the prepared statement.
    //---------------------------------------------------------
    $executed = mysqli_stmt_execute($stmt);
    if (!$executed) {
        throw new Exception('The prepared statement could not be executed!');
    }

    //---------------------------------------------------------
    // Transfer the result set from the prepared statement.
    //---------------------------------------------------------
    $stored = mysqli_stmt_store_result($stmt);
    if (!$stored) {
        throw new Exception('The result set from the prepared statement could not be transfered!');
    }

    //---------------------------------------------------------
    // Get the number of rows in statements' result set.
    //---------------------------------------------------------
    $rows = mysqli_stmt_num_rows($stmt);

    if ($rows > 0) {
        //---------------------------------------------------------
        // Bind result set columns to corresponding variables.
        //---------------------------------------------------------
        $bound = mysqli_stmt_bind_result($stmt, $resId, $resUsername, $resPassword);
        if (!$bound) {
            throw new Exception('The result set columns could not be bound to the variables');
        }

        //--------------------------------------------------------------------
        // Fetch results from the prepared statement into the bound variables.
        //--------------------------------------------------------------------
        while (mysqli_stmt_fetch($stmt)) {
            echo 'Successfully returned data:<br/><br/>';
            echo 'ID: ' . $resId . '<br/>';
            echo 'Username: ' . $resUsername . '<br/>';
            echo 'Password: ' . $resPassword . '<br/>';
        }
    } else {
        echo 'Invalid username/password. Please check and retry login!';
    }

    //-----------------------------------------------------------
    // Free stored result memory for the given statement handle.
    //-----------------------------------------------------------
    mysqli_stmt_free_result($stmt);

    //---------------------------------------------------------
    // Close db connection.
    //---------------------------------------------------------
    $closed = mysqli_close($conn);
    if (!$closed) {
        throw new Exception('The database connection can not be closed!');
    }
} catch (Exception $exception) {
    echo '<pre>' . print_r($exception, true) . '</pre>';
    exit();
}

Nota bene: Trying to use mysqli_stmt_store_result() together with mysqli_stmt_get_result() will lead to errors.