Change password.not working

here is my database table I need to change the admin password, where do i go wrong??

Here my code…

//session already set in config file
ini_set('display_errors', '1');
ini_set('display_startup_errors', '1');
error_reporting(E_ALL);

include_once 'classes/config.php';

$user = $_SESSION['username'];

if(isset($_POST['Submit'])) {

  $oldpassword = md5($_POST['oldpass']);
  $newpassword = md5($_POST['newpass']);
  $confirmpassword = md5($_POST['confirmpassword']);

  $sql = $dbh -> prepare ("SELECT password FROM users WHERE username = '$user'");

  $row = $sql->fetch();

  $oldpassworddb = $row['password'];

  if ($oldpassword==$oldpassworddb) {

   //success
    //cnage password

  if ($newpassword==$confirmpassword) {
      //success
    //cnage password

  if (strlen($_POST['newpassword']) <= 6) {

    echo "New Password too short";

   }else{

  $sql = $dbh -> prepare ("UPDATE users SET password='$newpassword' WHERE username='$user'");

  session_destroy();

  die ("Your pass has benn changed");
    

   }
  }
}

     
}

?>```

Using md5 for a password.
Using prepared statements and then hardcoding the strings into it.
strlen of an md5 will never be shorter than 6 characters; it’s a 128 bit hash, even if you hash the empty string.
Verify your inputs by echoing them out for the moment.
Define “go wrong”? What happens? or doesn’t happen?

1 Like

I am doing this for practice moment…

Understood …

Let me do that

Am I on the right track???

Well you’re not changing cpassword, but your SQL is correct for updating a field in a table.

Is it possible that when you change the password the cpassword will also change?? because cpassword is confirm password on signup form…

I have tried another approach… kindly check on it…

//session already set in config file
ini_set('display_errors', '1');
ini_set('display_startup_errors', '1');
error_reporting(E_ALL);

include_once 'classes/config.php';

  $username = $_SESSION['login'];

if(isset($_POST['Submit'])) {

  $oldpassword = md5($_POST['oldpass']);
  $newpassword = md5($_POST['newpass']);
 

  $sql = $dbh -> prepare ("SELECT password FROM users WHERE password='$oldpassword' && username = '$username'");

  $num = $sql->fetch();

  if ($num > 0 ) {

   $sql = $dbh -> prepare ("UPDATE users SET password='$newpassword' WHERE username='$username'");

   echo "Password Changed Successfully !!";


  }else{

   echo "Old Password dont match !!"; 
}

     
}

?>

and what happens when you run it?

Notice: Undefined index: login

so what’s missing from the top of your file?

I thought it was SESSION but its already started on the config file…

You’re calling session_destroy at the end of your code. Have you rerun the code that would add login to the session?

<\nothing happens??>

Should make this a require that way if this is the issue you will see as PHP will throw error if you use require_once

That’s done in the form as a check for the user to ensure they’ve typed the password correctly. In my (admittedly very limited) experience the confirmation password isn’t usually stored. Why would you want to store it, if it’s always the same as the real password?

In this bit:

$sql = $dbh -> prepare ("SELECT password FROM users WHERE password='$oldpassword' && username = '$username'");

$num = $sql->fetch();

you prepare the query, again with a password concatenated into the string rather that actually taking advantage of prepared statements to get around that, and deal with quotes, but you never actually execute it. Same a few lines further down.

What’s the point of practising with an outdated method of storing passwords?

This

  }else{

   echo "Old Password dont match !!"; 
}

is a confusing error message, as it could also be triggered by the username not existing in the table, once you’ve added code to execute the query. In general, though, you should just say something like “there was a problem with the credentials you supplied, please try again” so as not to be specific about which part of the user-supplied information was invalid.

thanks for advice I appreciate…

Changed that Thanks!!

Yes!! i saw that thanks…Just wanted to desperately work I think.

Proper approach??