Trouble with "Change Password" option (PHP/MySQL)

[KyleScalise]

Hi,

I'm a 16 year old web designer and aspiring developer. I've learned the ins and outs of making a website look amazing, but I'm only now starting to try to figure out how the heck it's all possible. One of the things I run into with every site I make is a user account system (because everyone needs to sign up for everything nowadays). I'm trying to learn little-by-little how to make a simple yet fully-featured account system.

In this specific case, I'm working on a very simple login system that only myself and a few friends will use. For that reason, I'm not concerned with security, injections, all that stuff. Actually, the login system was built (by me) without a MySQL connection, using PHP arrays to authenticate the few of us. Now, however, I want to give them a chance to change their password whenever they want via an easy interface. Therefore, I knew I'd have to start integrating MySQL. The ideal goal is to allow them to login with either the password set in the PHP code or the updated one in the database.

My code is posted below (along with what page each code is in). Currently, a user types in their old password and both new passwords and it directs to ?error=1. Nothing can be found in the PHP error_log and no change is made to the password in the database. Any help getting this to work would be appreciated.

account.php (PHP)

<?php
    session_start();
    if(!$_SESSION['username'])
    {
        header("Location: index.php");
    }

    $db_host = *WITHHELD*
    $db_user = *WITHHELD*
    $db_pass = *WITHHELD*
    $db_name = *WITHHELD*

    $mysql = mysql_connect($db_host, $db_user, $db_pass, $db_name);

    if(isset($_POST['submitpw'])) {
    if($_POST['currentpw'] == $_SESSION['password'] && $_POST['newpw'] == $_POST['confirmpw']) {
    $username = $_SESSION['username'];
    $securepw = md5($_POST['newpw']);
    $query = "UPDATE users SET password='$securepw' WHERE username='$username'";
    if (mysql_query($query, $mysql)) {
    header("Location: account.php?success=1");
    } else {
    header("Location: account.php?error=1");
    }
    }
    }
    ?>

account.php (HTML Excerpt)

<h3>Change Password</h3>
    <form class="form-inline" action="<?php echo $_SERVER['PHP_SELF']; ?>" method="post">
    <fieldset>
    <div class="form-group">
        <input id="currentpw" name="currentpw" type="password" placeholder="Current Password" class="form-control input-md" required="">
    </div>
    <div class="form-group">
        <input id="newpw" name="newpw" type="password" placeholder="New Password" class="form-control input-md" required="">
    </div>
    <div class="form-group">
        <input id="confirmpw" name="confirmpw" type="password" placeholder="Confirm Password" class="form-control input-md" required="">
    </div>
    <div class="form-group">
        <button id="submitpw" name="submitpw" class="btn btn-primary">Submit</button>
    </div>
    </fieldset>
    </form>
    <?php
    if($_GET['success'] == "1") {
    echo "<div class='alert alert-success' style='margin-top:20px;'><i class='fa fa-check-circle'></i> Your password has been updated successfully.</div>";
    }
    elseif($_GET['error'] == "1") {
    echo "<div class='alert alert-danger' style='margin-top:20px;'><i class='fa fa-times-circle'></i> We are unable to process your request at this time.</div>";
    }
    ?>

login.php (I realize nobody will be able to login after changing their password. I haven't gotten there yet. I'll be able to do that on my own.)

<?php
    session_start();

    $userinfo = array(
                    'michaela'=>'*WITHHELD*',
                    'Kyle'=>'*WITHHELD*',
                    'josh'=>'*WITHHELD*'
                    );

    if(isset($_POST['username'])) {
        if($userinfo[$_POST['username']] == $_POST['password']) {
            $_SESSION['username'] = $_POST['username'];
            $_SESSION['password'] = $_POST['password'];
            header('Location: downloads.php');
        }else {
            header('Location: login-failed.php');
        }
    }
    ?>

Please let me know what I have wrong and how I can fix it to allow my users to change their passwords. (Once again, for the purposes of this project, I don't care about "good practice", mainly because I'll be integrating this project into another, and at that time, I'll be cleaning things up.

Thanks for any help.

Kind regards,
Kyle

[Santanu.Das]

From my ponit of view, the SQL Statement iswritten wrongly.

$query = "UPDATE users SET password='$securepw' WHERE username='$username'";

As per your statement $securepw and $username behave like a text not a variable.
I suppose your statement should be

$query = "UPDATE users SET password='" + $securepw + "' WHERE username='" + $username + "'";

Use parameterized sql statement to prevent malicious injections.

[KyleScalise]

Hi,

Thank you for your reply. I've implemented your suggested change, but the result is the same as it was before. It directs to ?error=1 and does not complete the change in the database.

[rch1231]

Hello,

Well I know this is the MySQL code to change a users password:

UPDATE mysql.user SET Password=PASSWORD('XXXXXXX') WHERE User='XXXX';

SO your code should look something like:

$query = "UPDATE users SET Password=PASSWORD('$newpw') WHERE User='$username'";

Hope that helps.

[imti321]

I think problem lies here have you defined all the variables
$query = "UPDATE users SET password='$securepw' WHERE username='$username'";
Where is $username and $Securepw defined

[diafol]

I'm not concerned with security, injections, all that stuff

You should be. It's a false economy not to be. While you may only have a limited number of users, that means nothing to the malicious user. A malicious user could totally wreck your system and glean passwords and emails from the DB which may be useful even if the passwords are hashed - md5 is pitifully inadequate.

You are responsible for the security of your users' data. They may use the same password for everything (unwise I know), so a successful attack on your db could open them up to a life of pain. And it would be your fault. Sorry, that sounds strong - not meant to be - just never take security lightly - even on a scratch site.

[diafol]

You are using deprecated code (mysql_* functions) - use mysqli or PDO with prepared statements. There are literally thousands of online examples and tutorials on these.

[KyleScalise]

Thanks everyone for your replies. It seems the issue still isn't resolved, but I'll respond to each of you individually.

@rch1231 - If I recall, the code you mention is to change the password of a MySQL user (as in a user that can access the database itself). I'm trying to change the password of a user in a PHP account system, so instead of Password and User, it would use the column names I've defined in the database, which I've done in my code. Thanks anyway.

@imti321 - That may be the problem, but I wouldn't have a clue how to go about fixing it. Both of the variables are defined right before they are called in the $query statement. The $username variable draws from the $_SESSION and the $securepw draws from the $_POST, so I don't see how anything could not be defined, but I may very well be missing something. (I'm very new at this.)

@diafol - Thanks for your concern, but as I mentioned in the OP, the website I'm creating now will be soon be integrated into a bigger project that will feature updated and more secure practices. I only mentioned my lack of concern in the OP because I was hoping to just fix whatever's causing my code to not work without having to hear all about how bad my security is. I honestly do care deeply about my users' data. As far as the outdated usage is concerned, I used the old code because I wasn't sure if my server configuration supported mysqli or PDO. While working on the new site last night, I found that mysqli wasn't enabled, so I fixed that. PDO was enabled, but I'd honestly never heard of it before checking out how to connect to a database on w3schools. Thanks again.

[Taywin]

A suggestion for your account.php (HTML) file for future improvement. You should check whether the new password and confirm password are the same before submitting the form. This could simply be verified using Javascript on onsubmit form event.

I am guessing your index.html has a login form to call account.php. From what I see so far, it will never go into your PHP part because you start the script with if(!$_SESSION['username']). Why? Because you would never have that value set even though you passed in a correct argument to the page unless you have it set somewhere before the page is redirected to account.php.

Now, what do you want to deal with database part? Currently, you hard code the user and password in your script. As a result, there is no way to update them regardless unless you update your script. What you need to do is to store the user and password somewhere in your system. It could be in database (mysql - prefer) or a physical file (not prefer). I don't know how much knowledge and preparation you have in database yet, so I can't give you an answer. I don't want to confuse you because it is a separated part of the script.

[KyleScalise]

Hi Taywin,

Thanks for your response. I was about to write that I had already checked to make sure the new and confirm passwords were the same, but I read more carefully and noticed that you mentioned it should be done before rather than after submission. Good point. I'll definitely consider that in my updated script.

My index.php page does indeed have a login form. The if SESSION statement you mention assures that the user is logged in before accessing the account page, since if they are, the session variable will already have been set; otherwise they're taken to the index page to login.

I do currently have the account info hard coded, so what I'm looking to do is allow my three users to login using the hard coded data, change a copy of their password that is currently stored in my MySQL database, and then login using that from then on. Since I already placed copies of the info in the database, my next step after getting this to work will be to remove the hard coded data lines and start verifying through the database.

Thanks again!

~Kyle

[Taywin]

Sounds good. :)

So what you need to do now is to set up rules for username and password. You need to know what to expect before you deal with the script. For username, you may set to allow only letters, number, hyphen, and underscore. There must not be any other characters. For password, you may allow letters, number, hyphen, underscore, and some special character (i.e. !@#$%^&\*+).

Once you have defined your rules, then you would need to update your login.php. Remember that I asked you to set up a set of rules first? Those rules will be used to sanitize user input. In other words, both username and password must meet the rules, or the authentication will fail. This is how to prevent SQL injection (not allow certain character that can be used to work around in SQL).

After you sanitize the user input, use the username to look into the database. The password should never be used in SQL call but rather is used to compare with the database. If the condition is satisfied, then you could update the password in the database; otherwise, reject it or do nothing.

I guess I don't need to show you any snippet here because you seem to know what you are doing. Hope this help.