Need PHP help in form registration

Question

The problem is, the username and password created in my database in any case, how to prevent this? I'm novice so I don't know much about PHP.

if (isset($_POST['username']) && isset($_POST['password']))
 {
  $username = $_POST["username"];
  $password = $_POST["password"];
  RegisterErrors($username , $password);

  $query = "INSERT INTO users (Username,Password) VALUES ('{$username}','{$password}')";
  $query_result = mysqli_query($connection , $query);
  echo "added";

}

Here is the function to validate form registration :

function RegisterErrors($username , $password)
{

  if (null == $_POST["username"] || null == $_POST["password"]) 
   {
    echo "username or password cant be empty";
    return false;

   }
   if (empty($_POST["username"]) || empty($_POST["password"])) 
    {
     echo "username or password cant be empty";
     return false;

    }
  if (strlen($_POST["username"]) > 14)
   {
     echo "username must be less than 14 character";
     return false;

   }
  if (strlen($_POST["password"]) < 6)
   {
      echo "password must be at least 6 character";
      return false;

   }
}

Please don't add doubts and thank yous, we know you're thankful ;p instead, add a full description of whats wrong. — Lawrence Cherone, Mar 02 '18 at 23:26
Don't store unhashed passwords. See `password_hash` and `password_verify`. — Progrock, Mar 02 '18 at 23:43
If you want the function to return true, just do it. Or is there any condition to return true which you didn't tell us yet? — Nico Haase, Mar 02 '18 at 23:46
This code has SQL injection vulnerabilities in it - **it is not safe to put live**. — halfer, Mar 04 '18 at 21:06

Lawrence Cherone · Answer 1 · 2018-03-03T00:00:17.960

Instead, you should check validate/check errors and store them in an array, then simply check that array is empty.

This not only allows you to do checking in one go but allows you to display specific errors back to the user in the form where you want, also you should be using prepared queries to prevent SQL injections or problems with escaping quotes etc.

<?php

function RegisterErrors($username, $password) {
    $errors = [];

    // validate username
    if (empty($username)) {
        $errors['username'] = "username cant be empty";
    } elseif (strlen($username) > 14) {
        $errors['username'] = "username must be less than 14 character";
    }

    // validate password
    if (empty($password)) {
        $errors['password'] = "password cant be empty";
    } elseif (strlen($password) < 6) {
        $errors['password'] = "password must be at least 6 character";
    }

    return $errors;
}

if (isset($_POST['username']) && isset($_POST['username'])) {
    $username = $_POST["username"];
    $password = $_POST["password"];

    $errors = RegisterErrors($username, $password);

    if (empty($errors)) {
        $stmt = $connection->prepare('INSERT INTO users (Username,Password) VALUES (?, ?)');
        $stmt->bind_param('ss', $username, password_hash($password, PASSWORD_DEFAULT));
        $stmt->execute();

        echo "added";
    }
}
?>

Errors will be an array which you can place the elements in the correct place in the form.

<?= (!empty($errors['username']) ? $errors['username'] : null) ?>

Edit: as I've added password_hash() you would need to change your signin code to SELECT by username, then check then $_POST'ed password with password_verify ($_POST['password'], $dbresult['password']).

More info about that here: Best way to store password in database

thanks but this is kind of difficult for me as novice,and thanks for explaining,i think i learnt something new. — Aqil, Mar 02 '18 at 23:52
The questioner didn't ask how to he do standard code man? He asked for a solution. — Mithu CN, Mar 02 '18 at 23:53
@MithuCN ,i appreciate you and rtfm for your answers and tips,and thanks for down vote,but why?is there any problem with my question?! — Aqil, Mar 02 '18 at 23:59
@rtfm: don't take downvotes personally. If you recycle a serious security problem from the question in your answer then you may get feedback about that, sometimes in the shape of a downvote. The only time you should take downvotes personally is when they are accompanied by a hostile remark. In all other cases, any assertion that the downvote is a "toxic" behaviour is unsubstantiated, and it can be psychologically helpful for you to understand that someone is merely giving you a useful communication. — halfer, Mar 04 '18 at 21:08

score 0 · Accepted Answer · edited Mar 04 '18 at 21:10

0

You don't use the returned value from the error checking function.

So, at the bottom of RegisterErrors before the last } add return true; and then:

if (isset($_POST['username']) && isset($_POST['username']))
 {
  $username = $_POST["username"];
  $password = $_POST["password"];
  $test=RegisterErrors($username , $password);//get the value returned, true\false; dont need to parse variables when your using the globals.

if($test){// if true, passed the tests then insert
  $query = "INSERT INTO users (Username,Password) VALUES ('{$username}','{$password}')";
  $query_result = mysqli_query($connection , $query);
  echo "added";
}
}

edited Mar 04 '18 at 21:10

halfer

19,471
17
87
173

answered Mar 02 '18 at 23:25

Hi,thanks for answer,but i tried this and didn't work – Aqil Mar 02 '18 at 23:26
now its worked,thanks for help brother. but the two argument for RegisterErrors() was needed too. – Aqil Mar 02 '18 at 23:35
great, please note storing raw text passwords is considered terrible practice, and your open to sql injection attacks – Mar 02 '18 at 23:38
do you mean to use hashed_password?i'm just practicing – Aqil Mar 02 '18 at 23:41
i dont think practising doing things wrong is ever helpful personally – Mar 02 '18 at 23:44

Ken Sawyerr · Answer 3 · 2018-03-02T23:49:56.403

0

if (isset($_POST['username']) && isset($_POST['password']))
 {
  $username = $_POST["username"];
  $password = $_POST["password"];
  if(RegisterErrors($username , $password)){
    $query = "INSERT INTO users (Username,Password) VALUES ('{$username}','{$password}')";
  $query_result = mysqli_query($connection , $query);
  echo "added";
  }
}



function RegisterErrors($username , $password)
{

  if (null == $username || null == $password) 
   {
    echo "username or password cant be empty";
    return false;

   }
   if (empty($username) || empty($password)) 
    {
     echo "username or password cant be empty";
     return false;

    }
  if (strlen($username) > 14)
   {
     echo "username must be less than 14 character";
     return false;

   }
  if (strlen($password) < 6)
   {
      echo "password must be at least 6 character";
      return false;

   }


    return true;

}

edited Mar 02 '18 at 23:49

answered Mar 02 '18 at 23:36

Ken Sawyerr

234
1
7

thanks for answer,can you explain why you use elseif instead if? – Aqil Mar 02 '18 at 23:43
Sorry for the confusion caused, elseif wasn't necessary. You'll be fine with just returning true at the end. Notice I modified the first line of your code because you were checking for $_POST['username'] twice. Its not going to generate an error but I think you wanted to check for username and password, not username and username. – Ken Sawyerr Mar 02 '18 at 23:52
Oop! , you right,my mistake. last time i just copied and didn't see that – Aqil Mar 03 '18 at 00:13
The problem with this and the OP's original approach for validation/error feedback. Is that if say you have two errors with your inputs, say an invalid username and password: you only get feedback for one error at a time. Which could mean multiple form posting/going backwards and/or refilling, which isn't user friendly. – Progrock Mar 03 '18 at 11:52

Need PHP help in form registration

3 Answers3