PHP MySQL Check for Key then implement if not used

Question

I am trying to check my database for a key that has already been put in. If the key exists then I need it to check to make sure the username field hasn't been filled. If it has then it needs to throw an error so that it doesn't update and overwrite the information already stored in the database.

Everything works. The update functions etc. the only part that does not work is the checking if the key exists and if the username portion is filled(not sure exactly how to do that) before updating the database.

Thanks,

Cameron Andrews

Code:

// If the Register form has been submitted

$err = array();

if(strlen($_POST['username'])<4 || strlen($_POST['username'])>32){
    $err[]='Your username must be between 3 and 32 characters!';
}

if(preg_match('/[^a-z0-9 _]+/i',$_POST['username'])){
    $err[]='Your username contains invalid characters!';
}

if(!checkEmail($_POST['email'])){
    $err[]='Your email is not valid!';
}

 $resultN = mysql_query("SELECT * FROM Users WHERE key='".$_POST['kgen']."'");
 while($row = mysql_fetch_array($resultN))//for the results that are returned set the local variables
      {
        if($_POST['kgen'] == $row['key']){
            if($_POST['username'] == $row['usr']){
                $err[]='Username already in use';
            }
        }else if($_POST['kgen'] == ""){
            $err[]='Invalid Key Code!';

        }else{
            $err[]='Error occured please try again';
        }
     }

if(!count($err)){
    // If there are no errors
    $_POST['email'] = mysql_real_escape_string($_POST['email']);
    $_POST['username'] = mysql_real_escape_string($_POST['username']);
    $_POST['pass'] = mysql_real_escape_string($_POST['pass']);
    // Escape the input data
    $theName = $_POST['name'];
    $theUser = $_POST['username'];
    $thePass = $_POST['pass'];
    $theEmail = $_POST['email'];
    $theType = "member";
    $theRegIP = $_SERVER['REMOTE_ADDR'];
    $theDate = "NOW()";
    $theKey = $_POST['kgen'];

        // If everything is OK register
        mysql_query("UPDATE cad.Users SET name = '$theName', usr = '$theUser', pass = '$thePass', email = '$theEmail', type = '$theType', regIP = '$theRegIP', dt = '$theDate' WHERE Users.key = '$theKey'");

Please can you say what incorrect outputs you get, and what inputs are needed to receive those? — M1ke, Apr 07 '15 at 17:06
Why do you need `if ($_POST['kgen'] == $row['key'])`? That's already guaranteed by the `WHERE` clause in the query. — Barmar, Apr 07 '15 at 17:08
Can there be multiple rows with the same `key`? If not, why do you need a `while` loop? — Barmar, Apr 07 '15 at 17:09
What if the username is already in use with a different key? You never check for that. — Barmar, Apr 07 '15 at 17:10
wow that was quick guys, i tried it without a while loop last night and no matter what is going on there its like it bugs out or something. I do check for the key in the clause but i need it to catch the error to be able to tell the user something. i also need to make sure the key is unique so that the database isnt overwritten with new information if a user trys to register twice by accident. — Cameron Andrews, Apr 07 '15 at 17:10
see i need to check all of that. before the update occurs. but im not sure how to go about doing exactly that. this is the most advanced PHP i have gotten myself into so far. — Cameron Andrews, Apr 07 '15 at 17:14

score 0 · Answer 1 · edited May 23 '17 at 10:24

First of all, you should see this. At second, i see, you have strange logic. Try to simplify it. :) As for me, i think it should looks like this:

<?php

if (strlen($_POST['username']) < 4 || strlen($_POST['username']) > 32) {

    throw new RuntimeException('Your username must be between 3 and 32 characters!');

} elseif (preg_match('/[^a-z0-9 _]+/i', $_POST['username'])) {

    throw new RuntimeException('Your username contains invalid characters!');

} elseif(!checkEmail($_POST['email'])) {

    throw new RuntimeException('Your email is not valid!');

} elseif (empty($_POST['kgen'])) {

    throw new RuntimeException('Invalid Key Code!');

}

$resultN = mysql_query("SELECT * FROM Users WHERE `key`='{$_POST['kgen']}' AND `usr`='{$_POST['username']}';");
$user = mysql_fetch_array($resultN);

if (!empty($user)) {

    throw new RuntimeException('Username already in use');

}
// if all is fine - update

You can use exceptions for checking error. Benefit - you don't go to next check, if failed prev. You also have ability to show user exception message or reason(better use custom exception for this). Negative - you can't get list of errors.

Mike Brant · Accepted Answer · 2015-04-07T20:06:44.747

Here is how I would approach it:

I would use mysqli or PDO instead of deprecated mysql functions.

I would rewrite all the queries to use prepared statements instead of concatenating your query string together - you have significant SQL injection vulnerability now.

But, since I am not going to rewrite your entire section of code for you, the rest of my approach will be described based on your current mysql/concatenated-query-string approach.

I would put a unique index on name field, but allow NULL value on the field.

I would simply run an update query rather than trying to run an unnecessary select plus an update.

UPDATE cad.Users
SET
    name = '$theName',
    usr = '$theUser',
    pass = '$thePass',
    email = '$theEmail',
    type = '$theType',
    regIP = '$theRegIP',
    dt = NOW() /* don't pass 'NOW()' in single quotes as you are currently doing */
WHERE
    Users.key = '$theKey'
    AND User.name IS NULL;

If you get an error here you should look at error messaging to determine if update failed due to a unique constraint violation (user tried to enter a name that was already used in another record associated with a different key), or some other unexpected reason.

Assuming there was no error, I would then call mysql_affected_rows() (or appropriate equivalent in mysqli or PDO). If the return value is 1, an update was made. If the return value is 0, then no update was made because you did not have any rows that satisfied the WHERE condition.

If you get 0 affected rows, you can re-query the database if you really want to determine if the cause was no matching key or an existing user name.

SELECT name FROM Users WHERE key = '$theKey';

If you get no rows in the result it is because the key is missing, otherwise it is because the name was not a NULL value.

The net is that in the happy path use case, you only make a single query against the database rather than two, with two queries only being necessary if you want to determine the reason no update occurred for those cases. Your current approach always requires 2 queries.

PHP MySQL Check for Key then implement if not used

2 Answers2