PHP & MYSQL login system

The system almost works, but I get the error message "The specified email address or password was incorrect. I double checked the database and triple check it and the user does exist. What else could be going wrong?

<?php

function userIsLoggedIn()
{
  if (isset($_POST['action']) and $_POST['action'] == 'login')
  {
    if (!isset($_POST['email']) or $_POST['email'] == '' or
      !isset($_POST['password']) or $_POST['password'] == '')
    {
      $GLOBALS['loginError'] = 'Please fill in both fields';
      return FALSE;
    }

    $password = md5($_POST['password'] . 'ijdb');

    if (databaseContainsAuthor($_POST['email'], $password))
    {
      session_start();
      $_SESSION['loggedIn'] = TRUE;
      $_SESSION['email'] = $_POST['email'];
      $_SESSION['password'] = $password;
      return TRUE;
    }
    else
    {
      session_start();
      unset($_SESSION['loggedIn']);
      unset($_SESSION['email']);
      unset($_SESSION['password']);
      $GLOBALS['loginError'] =
          'The specified email address or password was incorrect.';
      return FALSE;
    }
  }

  if (isset($_POST['action']) and $_POST['action'] == 'logout')
  {
    session_start();
    unset($_SESSION['loggedIn']);
    unset($_SESSION['email']);
    unset($_SESSION['password']);
    header('Location: ' . $_POST['goto']);
    exit();
  }

  session_start();
  if (isset($_SESSION['loggedIn']))
  {
    return databaseContainsAuthor($_SESSION['email'], 
      $_SESSION['password']);
  }
}

function databaseContainsAuthor($email, $password)
{
  include 'db.inc.php';

  try
  {
  $sql = 'SELECT COUNT(*) FROM users
        WHERE email = :email AND password = :password';
    $s = $pdo->prepare($sql);
    $s->bindValue(':email', $email);
    $s->bindValue(':password', $password);
    $s->execute();
  }
  catch (PDOException $e)
  {
    $error = 'Error searching for users.';
    include 'error.html.php';
    exit();
  }

  $row = $s->fetch();

  if ($row[0] > 0)
  {
    return TRUE;
  }
  else
  {
    return FALSE;
  }
}

What happens if you change

$row = $s->fetch();

to

$count = $s->fetchColumn();

and then check for the value of $count? What value do you get in $row[0] or $count?

I get the same error. How can a echo or print the value of $count?

echo $count, or var_dump it.

I var_dump it and I get NULL in both instances. I know it’s connecting to the database because that is not generating an error. But it’s acting like the data is not there.

There are a lot of things that are wrong. The logic is wrong and the code is wrong. First off in a login system, you SHOULD NEVER compare the email and password in the SQL statement. This is the main problem. Your snippet is looking to see if the email and password combination are correct which is not right. What you should do is look for just the email and then specify the password column. Then compare the posted password with the password you just specified. If the password are the same, you can proceed. If they are not, do whatever you want.

Second most important thing that is wrong with your snippet. You SHOULD NEVER modify client posted password regardless if they are using SQL based syntax as their password. As you have did right here.

If you are using prepared statements and you are using them correctly, then you don’t need to worry about the password as it will become just a string and not part of the SQL statement.

Third thing that is wrong with your snippet. You are using md5 which is NOT a password algorithm. You should be using password_hash to encrypt the passwords and using password_verify to compare the 2 passwords.

Fourth thing that is wrong with your snippet. You don’t need that much session_start(). Just put one at the very top of the page afer <?php and you should be good.

Fifth thing that is wrong with your snippet. Don’t use if(isset($_POST['submit'])), if(isset($_POST['action'])), if(isset($_POST['btn'])), .etc as they are very flawed. These are amateur hacks you can find in tutorials from the 90’s. The correct and right way is using if($_SERVER['REQUEST_METHOD'] == 'POST') as it was designed specifically to find whether the request was made via POST or GET.

You have to set error reporting at max and start debugging your code.

In short, debugging stands verifying each step’s result. You already started with it, but you have to go further. What is the query result? What are email and password values used in the query? What a query looks like with these values substituted? Does this query return any result if run against a database?

It sounds boring but you have to realize that

  • Debugging is the only way to find the problem. It’s no use to stare in the code.
  • Debugging takes considerable part of the programmer’s worktime, you can’t avoid it.
2 Likes

The OP is actually better off making use of PHP’s functions for password hashing instead of relying on MySQL’s md5 function as md5 has been rainbow tabled to death, making it effectively useless for password hashing

OK, so there’s a starting point - change your function to query on the email address (which presumably is checked as unique when creating an account), then compare the password you get back. For example:

function databaseContainsAuthor($email, $password)
{
  include 'db.inc.php';

  try
  {
  $sql = 'SELECT password FROM users
        WHERE email = :email';
    $s = $pdo->prepare($sql);
    $s->bindValue(':email', $email);
    $s->execute();
  }
  catch (PDOException $e)
  {
    $error = 'Error searching for users.';
    include 'error.html.php';
    exit();
}

  $row = $s->fetch();

  if ($row[0] == $password)
  {
    return TRUE;
  }
  else
  {
    return FALSE;
  }
}

I didn’t comment on whether or not the password encoding is good or bad (as I don’t have the experience to do so), and I am assuming that when you store the password, you run through the same steps that you do in your code, i.e. appending your four-digit string and then taking the MD5. If this doesn’t help, you can at least compare $row[0] against $password in the above query and see whether the encoded password stored in your database is different to the encoded password you passed into the function. I’m not sure about the return from your query being NULL - if the problem was not matching the email and password in any rows, surely that would return a zero count rather than a NULL. So that suggests your query has a problem, as Colshrapnel said, turn on error reporting and see what that tells you. And trace the variable values throughout to see that things are as you expect.

Not using the built in password functions is bad - the built in functions are designed to improve security as prior hashes become to easy to break - as MD5 has been for several years now.

Thanks a lot guys, I was running out of time and found a better solution that hashes the password and it indeed uses the built in PHP password function. This code that I posted came from a book that was published here at Sitepoint (PHP & MYSQL novice to ninja). I am very new to PHP and this is really the first project that I had to wright from scratch. I got it working now. I appreciate everybody’s feedback. It was invaluable to get a better more secure solution.

Is there a reason the project has to be written from scratch. Many of the modern frameworks including Laravel and Symfony have very flexible and robust authentication system not to mention a boat load of other optional features including testing framework integration.

It is a really simple system. I will look into the frameworks you are talking about. Thanks

Yup, that’s why I had linked the hash and verify page in my post above.

Was that snippet you posted from the novice e-book?

Yes

This topic was automatically closed 91 days after the last reply. New replies are no longer allowed.