mysql query to update db with hashed passwords

Question

I'm building a registration and log in form and I would like to hash my user passwords on registration and my main problem at this point is how to write the query to update the passwords when the user is signing his email and pass for the 1st time and I want to add mysqli_insert_id() to the query to follow the unique id's for each user.

so I have database named testdb with users inside.

my code is perfectly working until the moment when you have to query the password and update it.

First I'm hashing my passwords

$password = $_POST['password'];

$hashed_password = password_hash($password, PASSWORD_BCRYPT);

$query = "UPDATE `users` SET `password` = '$hashed_password' WHERE id = "

As you can see I have problem with my query which should update passwords in my DB.

I have this code written until this point so I need help to proceed my UPDATE query

if (array_key_exists("submit", $_POST) ) {

    // connect to our db
    $link = mysqli_connect("localhost", "root", "", "secretdi");
    // check for connection
    if ( mysqli_connect_error() ) {
        die("Database Connection Error");
    }

    $error = "";

    if ( !$_POST['email'] ) {
        $error .= "An email address is required<br>";
    }

    if ( !$_POST['password'] ) {
        $error .= "A password is required<br>";
    }

    if ( $error != "" ) {
        $error = "<p>There were error(s) in your form:</p>".$error;
    } else {

        $query = "SELECT id FROM `users` WHERE `email` = '".mysqli_real_escape_string($link, $_POST['email'])."' LIMIT 1";

        $results = mysqli_query($link, $query);

        if ( mysqli_num_rows($results) > 0 ) {
            $error = "That email address is taken.";
        } else { 

            $query = "INSERT INTO `users` (`email`, `password`) VALUES('".mysqli_real_escape_string($link, $_POST['email'])."','".mysqli_real_escape_string($link, $_POST['password'])."') ";

            if (!mysqli_query($link,$query)) {
                $error = "<p>Could not sign you up - please try again later</p>";
            } else {

                $password = $_POST['password'];

                $hashed_password = password_hash($password, PASSWORD_BCRYPT);


                $query = "UPDATE `users` SET `password` = '$hashed_password' WHERE id = "


                echo "Sign up successful";
            }

        }

    }

Wait, you want to hash password at log in?! Why not at registration? — Vini, Apr 11 '17 at 11:05
yea I want to hash them on registration I was about to edit my question — Learno, Apr 11 '17 at 11:05
you hash passwords at registration and verify the hashpassword with input password on login. I think you are confused — Rotimi, Apr 11 '17 at 11:05
What's the problem? What's not working? Your query is obviously not finished as you're not completing the query after the = symbol. What do you expect to happen here? Also, please use prepared statements. — Jonast92, Apr 11 '17 at 11:08
I don't get the logic... If a new user is registering, you should have an INSERT, not UPDATE — OldPadawan, Apr 11 '17 at 11:08
why are you updating the passwords? Is this a password reset module? — Rotimi, Apr 11 '17 at 11:08
yes i'm confused about how i should end the query using mysqli_insert_id() also inside it — Learno, Apr 11 '17 at 11:09
if you want to update, just use the current user's id you get from the query. and this really does not make any sense. Your logic looks really off — Rotimi, Apr 11 '17 at 11:10
You're trying to insert a plain text password in your insert statement and if it's successful then you try to update it with a hashed password. Don't do this at all. Simply store the hashed password from the beginning. Don't store it in plain-text and then update it. — Jonast92, Apr 11 '17 at 11:11
weird logic, weird code... ^^ anyway -> [LINK to DOC last_insert_id](https://www.w3schools.com/php/func_mysqli_insert_id.asp) — OldPadawan, Apr 11 '17 at 11:14
yes thank you all i am in a learning process so thats why i need some extra help. — Learno, Apr 11 '17 at 11:18
To back up what @MasivuyeCokile is saying, make sure you ***[don't escape passwords](http://stackoverflow.com/q/36628418/1011527)*** or use any other cleansing mechanism on them before hashing. Doing so *changes* the password and causes unnecessary additional coding. — Jay Blanchard, Apr 11 '17 at 12:25

Jonast92 · Accepted Answer · 2017-04-11T11:27:26.613

1

You should never, under any circumstances, store the password in plain-text.

This means that when you register the user you should hash the password right away and store it in the hashed format when you perform the insert statement. Your current logic stores the password in plain-text, and if the registration is successful then you attempt to update the password to become the hashed one. This means that if anything fails you might end up with having only the plain-text password in your database, and either way it means that at a certain point it does exist there in plain-text at some given point for any user which we don't want.

Besides, the update statement serves no purpose other than wasting a few extra lines of code.

Something like this should do:

if ( mysqli_num_rows($results) > 0 ) {
    $error = "That email address is taken.";
} else { 
    $password = $_POST['password'];
    $hashed_password = password_hash($password, PASSWORD_BCRYPT);

    $query = "INSERT INTO `users` (`email`, `password`) VALUES('".mysqli_real_escape_string($link, $_POST['email'])."','".$hashed_password."') ";

    if (!mysqli_query($link,$query)) {
        $error = "<p>Could not sign you up - please try again later</p>";
    } else {
        echo "Sign up successful";
    }
}

If you really want to stick to your plan, however, then you can select the user id for the user you just created (e.g. by using the email identifier which is hopefully unique) and use it as such to identify which user to update. Please don't do that.

Note: I recommend taking a look at mysqli's prepared statements to ensure a higher level of security instead of escaping individual variables.

edited Apr 11 '17 at 11:27

answered Apr 11 '17 at 11:16

Jonast92

4,964
1
18
32

You should never, under any circumstances escape passwords – Masivuye Cokile Apr 11 '17 at 11:18
@MasivuyeCokile it isn't, it's correct. If you think it's wrong, you're entitled to explain why instead of just stating it's wrong, since no one knows who you are or why anyone should trust you. The only thing missing in this answer is using PDO and prepared statements, but mechanism behind password hashing is completely correct. – Mjh Apr 11 '17 at 11:20
@MasivuyeCokile Can you elaborate? – Jonast92 Apr 11 '17 at 11:21
yea i saw the escape on password but what if there is some special character in the password like ``''' what we are going to do then when we are not using real_escape – Learno Apr 11 '17 at 11:21
@Learno maybe it'd be a good idea to write the solution using PDO and prepared statements (or mysqli and prepared statements)? The mechanism explained is correct, but there are always nitpickers who whine about every detail, missing the big picture. – Mjh Apr 11 '17 at 11:22
You should never escape, trim or use any other cleansing mechanism on passwords you'll be hashing with PHP's password_hash() for a number of reasons, the single largest of which is because doing additional cleansing to the password requires unnecessary additional code. – Masivuye Cokile Apr 11 '17 at 11:23
I'm not going to re-write the code using better storing procedures or hashing algorithms. I'm simply guiding op to a more logical solution to his problem. I took OP's code and edited to a level that he understands and works for him. I'm not encouraging bad practices. I think your down vote is simply unfair. – Jonast92 Apr 11 '17 at 11:24
@Learno I also suggest you to use `PASSWORD_DEFAULT` instead of `PASSWORD_BCRYPT` since the first one is automatically replaced if a new, stronger, algorithm is added to PHP. Read: [PHP Manual - password_hash()](http://php.net/manual/en/function.password-hash.php) – Brigo Apr 11 '17 at 11:26
@MasivuyeCokile Your job as a SO participant is not to write out fully functional or perfect solutions, I want to get paid for that. If we can't help people with their logical problems without getting gunned down for quick-fixing OPs code without going into too much details and re-writing everything we can't answer anything. You want me to cut out the code altogether and make it harder for OP to understand? All I want is for OP to understand the logic he's not understanding. How safe his code is up to him but I've at least given him a place to start, that's all I should have to do. – Jonast92 Apr 11 '17 at 11:30
No man @Jonast92 I'm not saying re write the code All I'm trying to say it's not a good idea to insert password like this `mysqli_real_escape_string($link, $hashed_password)` at the end this will comeback to bit the OP. You could save the `$hashed_password` without escaping it, OP does not know that but m pretty sure u do know that is not a good idea to do what m saying point the OP to the correct direction as u did with the logic – Masivuye Cokile Apr 11 '17 at 11:32
@MasivuyeCokile actually, it will not. It's not the best practice, you got stuck on a detail - OP and anyone else at this point, should know how to deal with the database. Code here highlights the **method of using password_hash and updating the database**. Not everyone have to be spoon-fed every single detail. It's incredible how much you're missing the whole point. You could have written your own answer instead of going through such a debate here. – Mjh Apr 11 '17 at 11:34
*but what if there is some special character in the password like ``''' what we are going to do then when we are not using real_escape* @Learno tht is where prepared statement comes in – Masivuye Cokile Apr 11 '17 at 11:35
1

@Mjh we are also he to help improve answers as well, there's no need to re write a new answer when there's an answer already that needs improvement I thought I could just contribute to that answer. m out cheers – Masivuye Cokile Apr 11 '17 at 11:37
Make sure you ***[don't escape passwords](http://stackoverflow.com/q/36628418/1011527)*** or use any other cleansing mechanism on them before hashing. Doing so *changes* the password and causes unnecessary additional coding. – Jay Blanchard Apr 11 '17 at 12:25
Let's not teach/propagate sloppy and dangerous coding practices. If you post an answer without prepared statements [you may want to consider this before posting](http://meta.stackoverflow.com/q/344703/). Additionally [a more valuable answer comes from showing the OP the right method](https://meta.stackoverflow.com/a/290789/1011527). – Jay Blanchard Apr 11 '17 at 12:26
You need to tell the OP about the length of hashed passwords so they understand the database will be setup correctly to handle the length. If they don't and the length is wrong `password_verify()` will never work. – Jay Blanchard Apr 11 '17 at 12:29
BTW, a lot of what @MasivuyeCokile is saying here is *correct*. There has been a lot of debate lately (links have been provided). These details are important to new coders. Remember what it was like when you first began to code. – Jay Blanchard Apr 11 '17 at 12:31
@Mjh sometimes folks new to coding need to be spoon fed details in order to learn how to write safe and secure code. Think about all of the concepts here: prepared statements, escaping strings, not escaping strings, database setup - heck that is a lot for every new user. You were new to coding once too and telling someone they didn't contribute is not constructive at all. – Jay Blanchard Apr 11 '17 at 12:40
1

I'm sorry about the confusion I made when I didn't remove the escaping from the password in my answer, I edited that part as that was obviously "wrong". – Jonast92 Apr 11 '17 at 12:55

mysql query to update db with hashed passwords

1 Answers1