0

I'm using password_verify with database when I login it always say password incorrect and in my database I'm using char 255 for password.

function login(){
    global $db, $username, $errors;

    // grap form values
    $username = e($_POST['username']);
    $password = e($_POST['password']);

     $userrecord1 = mysqli_query($db, "SELECT * FROM users WHERE username='$_POST[username]' LIMIT 1");

    if (count($userrecord1) == 1 ) {
        $urow1 = mysqli_fetch_array($userrecord1);
        $hash = $urow1["password"];
    }


    // attempt login if no errors on form
    if (count($errors) == 0) {

    $passuser = password_verify($password, $hash);

        $query = "SELECT * FROM users WHERE (username='$username' OR email='$username') AND password='$passuser' LIMIT 1";
        $results = mysqli_query($db, $query);

        if (mysqli_num_rows($results) == 1) { // user found
            // check if user is admin or user
            $logged_in_user = mysqli_fetch_assoc($results);
            if ($logged_in_user['user_type'] == 'admin') {

                $_SESSION['user'] = $logged_in_user;
                $_SESSION['success']  = "Welcome admin";
                header('location: /admin/home');          
            }else{
                $_SESSION['user'] = $logged_in_user;
                $_SESSION['success']  = "Welcome user";

                header('location: /home/index');
            }
        }else {
            array_push($errors, "Wrong username/password combination");
        }
    }
}
xmaster
  • 1,042
  • 7
  • 20
raf gres
  • 1
  • 3
  • 1
    Once you have done your first select and retrieved the users password, you only need to test the result of `password_verify()` - if this is true then the user password is correct. The second SQL makes no sense. – Nigel Ren May 08 '19 at 06:49
  • 1
    Your code is open for [SQL injection](https://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php) – xmaster May 08 '19 at 06:51
  • [How to do it properly](https://phpdelusions.net/mysqli/password_hash) – Your Common Sense May 08 '19 at 06:52

1 Answers1

0

There's a few things I'd like to note,

  1. Usage of the global keyword is not recommended - you should instead pass values as arguments to the function instead.
  2. You are wide open to SQL injection - use a prepared statement
  3. Once you have verified the password by password_verify(), the password is correct and the user has verified his login.
  4. if (count($userrecord1) == 1 ) { doesn't make much sense, as you haven't fetched any records yet.
  5. Your second select is moot (see point 3), and invalid as selecting from a hashed password will not yield the result.
  6. Use exit; after header("Location: .."); calls
function login(){
    global $db, $username, $errors;

    // grap form values
    $username = e($_POST['username']);
    $password = e($_POST['password']);

    $stmt = $db->prepare("SELECT password, user_type FROM users WHERE username=? LIMIT 1");
    $stmt->bind_param("s", $username);
    $stmt->execute();
    $stmt->bind_result($dbPassword, $userType);
    if ($stmt->fetch() && password_verify($password, $dbPassword)) {
        if ($userType == 'admin') {
            $_SESSION['user'] = $username;
            $_SESSION['success']  = "Welcome admin";
            header('location: /admin/home');     
            exit;     
        } else {
            $_SESSION['user'] = $username;
            $_SESSION['success']  = "Welcome user";
            header('location: /home/index');
            exit;
        }
    } else {
        $errors[] = "Wrong username/password combination";
    }
}
Qirel
  • 25,449
  • 7
  • 45
  • 62