-1

I have my MySQL database connected to my php code (it works just fine, as I use it at a few other places), and I have a registration/login page. Registration works fine as well, I can see all the details in my database. However, I can't login.

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

        if (empty($username)) { array_push($errors, "Username is required"); }
        if (empty($password)) { array_push($errors, "Password is required"); }
        if (empty($errors)) {
            $password = md5($password); 

            $stmt = $conn->prepare("SELECT * FROM users WHERE username='$username' and password='$password' LIMIT 1");
                        $stmt->execute();

            $result = $stmt->get_result();
            if (mysqli_num_rows($result) > 0) {

                $reg_user_id = mysqli_fetch_assoc($result)['id'];

                $_SESSION['user'] = getUserById($reg_user_id);

                if ( in_array($_SESSION['user']['role'], ["Admin", "Author"])) {
                    $_SESSION['message'] = "You are now logged in";

                    header('location: ' . BASE_URL . '/admin/dashboard.php');
                    exit(0);
                } else {
                    $_SESSION['message'] = "You are now logged in";

                    header('location: index.php');
                    exit(0);
                }
            } else {
                array_push($errors, 'Wrong credentials');
            }
        }
    }

My code always goes to the else branch ("Wrong credentials"). Looks like it never gets my user row in the first place. I am quite stuck on this one.

Shadow
  • 33,525
  • 10
  • 51
  • 64
  • From what you write the `username` is not posted. Try to see if you a typo in your front end form on the field username. Additionally add this line `var_dump($_POST['username'])` below this one `$username = $_POST['username'];` to see if username is empty. –  Sep 19 '20 at 09:37
  • Note that LIMIT without ORDER BY is fairly meaningless – Strawberry Sep 19 '20 at 09:38
  • @Strawberry `LIMIT` is useless since the username is theoretically unique – Simone Rossaini Sep 19 '20 at 09:39
  • @SimoneRossaini Fair point - so perhaps one of the few instances where LIMIT without ORDER BY is in fact (potentially) useful – Strawberry Sep 19 '20 at 11:12
  • **Warning:** You are wide open to [SQL Injections](https://stackoverflow.com/a/60496/1839439) and should use parameterized **prepared statements** instead of manually building your queries. They are provided by [PDO](https://php.net/manual/pdo.prepared-statements.php) or by [MySQLi](https://php.net/manual/mysqli.quickstart.prepared-statements.php). Never trust any kind of input! Even when your queries are executed only by trusted users, [you are still in risk of corrupting your data](http://bobby-tables.com/). [Escaping is not enough!](https://stackoverflow.com/q/5741187) – Dharman Sep 19 '20 at 12:47
  • **Never store passwords in clear text or using MD5/SHA1!** Only store password hashes created using PHP's [`password_hash()`](https://php.net/manual/en/function.password-hash.php), which you can then verify using [`password_verify()`](https://php.net/manual/en/function.password-verify.php). Take a look at this post: [How to use password_hash](https://stackoverflow.com/q/30279321/1839439) and learn more about [bcrypt & password hashing in PHP](https://stackoverflow.com/a/6337021/1839439) – Dharman Sep 19 '20 at 12:47

1 Answers1

1

Try to change that (sql inject prevent) :

$stmt = $conn->prepare("SELECT * FROM users WHERE username='$username' and password='$password' LIMIT 1");
$result = $stmt->get_result();
if (mysqli_num_rows($result) > 0) {

to

$stmt = $conn->prepare("SELECT * FROM users WHERE username=? and password=? ");
$stmt->bind_param('ss',$username,$password);
$stmt->execute();
$result = $stmt->get_result();
if ($result->num_rows > 0) {

Another questions:

  1. Have you try your query into mysql panel?
  2. Are you sure all variable from POST is ok?

Why you don't use password_verify? (official php)

So like :

if (password_verify($_POST['password'], $user['password'])) {

}

Select $user just with username.

Complete:

$stmt = $conn->prepare("SELECT * FROM users WHERE username=?");
$stmt->bind_param('s',$username);
$stmt->execute();
$result = $stmt->get_result();
if ($result->num_rows > 0) {
 while($user = $result->fetch_array()){

  if (password_verify($_POST['password'], $user['password'])) {
  //start all session var
  }

}
}
Simone Rossaini
  • 8,115
  • 1
  • 13
  • 34
  • Where does `$user['password']` come from? – Dharman Sep 19 '20 at 12:47
  • I accepted your answer, as it led me to refactor my code and to go over it again step by step... My issue was kind of foolish in the end. I encrypted the password, but at one time (not sure why, testing purposes perhaps)I changed my password manually in the database to the non-crypted version. And of course when I tried to log in, it couldn't find my encrypted one. – Balázs Kozma Sep 19 '20 at 20:07
  • Use `foreach` instead. You don't need to check for `num_rows`. Besides I would expect there to be only 1 row. – Dharman Sep 19 '20 at 21:01
  • check if user exist is not important why? – Simone Rossaini Sep 19 '20 at 21:01
  • You can simply do `$user = $result->fetch_assoc(); if($user && password_verify($_POST['password'], $user['password']))` – Dharman Sep 19 '20 at 21:10
  • if($user) will return true or false if exist or not? – Simone Rossaini Sep 21 '20 at 05:58