Convert mysql login script to PDO

PHP
1

I am trying to convert an old mysql login function to PDO but it s not working as I had hoped. This is what I have so far:


[B]Connection:[/B]

    try
      {
      $conn = new PDO("mysql:host=$dbhost;dbname=$dbname",$dbuser,$dbpass);
      }
      catch (PDOException $e)
      {
      echo 'Unable to connect to the database server.';
	  exit();
      }
	
	  // define admin site path
      define('DIRADMIN','http://www.website..com/admin/');

      define('SITETITLE','Controlpanel');
	
	  //define include checker
      define('included', 1);
	
	  include('authenticate.php');



Authenticate:

  function login($user, $pass){
    $user       = $_POST['username'];
    $pass       = $_POST['password'];
	
	$pass = md5($pass);
	
	$login = $conn->prepare("SELECT id, fname, lname, username, password FROM users WHERE username = '$user' AND password = '$pass' ");
	$login->execute();
	$result = $login->rowCount();
	$user_data = $login->fetch(PDO::FETCH_ASSOC);
	
	if($result > 0){
	  $_SESSION['authorized'] = true;
	  $_SESSION['user'] = $user_data['fname'];
	  header('Location: '.DIRADMIN);
	  exit();
		
	}else{
	   $_SESSION['error'] = 'Verkeerde gebruikers naam en/of wachtwoord!';
	}		
  }

  // Authentication
  function logged_in() {
	if($_SESSION['authorized'] == true) {
		return true;
	} else {
		return false;
	}	
  }

  function login_required() {
	if(logged_in()) {	
		return true;
	} else {
		header('Location: '.DIRADMIN.'login.php');
		exit();
	}	
  }

  function logout(){
	unset($_SESSION['authorized']);
	header('Location: '.DIRADMIN.'login.php');
	exit();
  }

and on the login page I have the following:


  require('includes/db_connect.php');
  if(logged_in()) {
    header('Location: '.DIRADMIN);
  }

But nothing is happening. I don’t get an error, but It is not doing as I had hoped for. Does anybody see what I am doing wrong?

Thank you in advance!

2

For testing purpose I have changed the enire structure a bit:


    try
      {
      $conn = new PDO("mysql:host=$dbhost;dbname=$dbname",$dbuser,$dbpass);
      }
      catch (PDOException $e)
      {
      echo 'Unable to connect to the database server.';
	  exit();
      }
	

	  $sth = $conn->prepare("SELECT * FROM users WHERE username = '$username' AND password = '$password'");
	  $sth->execute();

	
	  while ($row = $sth->fetch()) {
		session_start();
		$_SESSION['userName'] ="$row[username]";
	  }

Strange thing is (or maybe not strange at all) when I echo the SESSION:


echo $_SESSION['userName'];

It shows me the username. So I presume the validtion is working. When on the other hand I redirect to index.php:

header('Location: index.php');

nothing is happening? What could be wrong?

3

What would you expect to happen? Indeed if I look at the code so far it’s not doing anything, because no code is executed, unless you’ve not posted all your code?

Also, your code is wide open to SQL Injection. When using prepared statements, please do it properly:


$login = $conn->prepare("SELECT id, fname, lname, username, password FROM users WHERE username = ? AND password = ?"); 
$login->execute(array($user, $pass));

That way the query is first sent to the server, and then the variables are sent, so there can never be any confusion as to what belongs to the query and what belongs to the data, making SQL Injection impossible.

Besides, if you only need fname from the result, it would be better to SELECT only that field, and not add a bunch of other fields you don’t need :slight_smile:

4

Hi donboe,

Are you remembering to call [fphp]session_start[/fphp] at the beginning of every page that needs access to the session?

There are a couple of other things I noticed about your code:-

You’re trying to use a prepared statement here, but you’re not getting the benefits of it because you’re including your values directly in the SQL string. What you need to do is use placeholders in the query, and then pass the values to the execute() method:


$sth = $conn->prepare("SELECT * FROM users WHERE username = :username AND password = :password"); 
$sth->execute(array(':username' => $username, ':password' => $password));

Doing this properly will protect your code against SQL injection attacks.

Also, it’s really not advisable to use md5 to hash your passwords. If you’re using PHP 5.5 or higher, you can use the built-in password hashing functions which will be much more secure. If you’re using an earlier version (PHP 5.3.7 or higher), there is a 3rd-party [URL=“https://github.com/ircmaxell/password_compat”]library that provides compatible functions.

Edit: Rémon beat me to it :slight_smile:

5

@ ScallioXTX & @fretburner

Thank you both for the very useful tips. I have adjusted the the code with your suggestions(I didn’t change the md5 hash as yet, but will do that later):


    try
      {
      $conn = new PDO("mysql:host=$dbhost;dbname=$dbname",$dbuser,$dbpass);
      }
      catch (PDOException $e)
      {
      echo 'Unable to connect to the database server.';
      exit();
      }


      $sth = $conn->prepare("SELECT id, fname, lname, username, password FROM users WHERE username = ? AND password = ?");
      $sth->execute(array($username, $password));


      while ($row = $sth->fetch()) {
        session_start();
        $_SESSION['userName'] ="$row[username]";
	    header('Location: http://www.website.com/test2.php');
      }

I have added session_start(); to test2.php. But still I am not redirected to test2.php.?

6 
      $sth = $conn->prepare("SELECT id, fname, lname, username, password FROM users WHERE username=:username AND password=:password"); 
      $sth->execute(array(':username' => $username, :password => $password)); 
 
     $row = $sth->fetch(PDO::FETCH_ASSOC)
 
       if ($row)) {
        session_start();
        $_SESSION['userName'] ="$row['username']";
        header('Location: test2.php');  // I'm assuming the file is in the root directory?
        exit();
      }
7
Off Topic:

Please don’t do this. Even though it works (albeit with a E_NOTICE because ‘username’ is not quoted), it is very bad practice and I wouldn’t get in the habit of doing it this way if I were you. Instead, use


$_SESSION['userName'] = $row['username'];
8

Works great thank you guys a lot :tup: