PDO Login Error

[Developer]

Register is working fine, but I can't seem to get the login working. I keep getting "Invalid Username/Password". Can anyone find the problem?

This code is on loginclass.php

	 public function __construct( $data = array() ) {
		   if( isset( $data['username'] ) ) $this->username = stripslashes( strip_tags( $data['username'] ) );
		  if( isset( $data['password'] ) ) $this->password = stripslashes( strip_tags( $data['password'] ) );
}

 public function storeFormValues( $params ) {
		  //store the parameters
		  $this->__construct( $params );
}

 public function userLogin() {
			   //success variable will be used to return if the login was successful or not.
			   $success = false;
			  try{
				 //create our pdo object
				 $con = new PDO('mysql:host=localhost;dbname=dbname;charset=utf8', 'username', 'password');
				 //set how pdo will handle errors
				 $con->setAttribute( PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION );
				 //this would be our query.
				 $sql = "SELECT * FROM users WHERE username = :username AND password = :password LIMIT 1";

				  //prepare the statements
				 $stmt = $con->prepare( $sql );
				 //give value to named parameter :username
				 $stmt->bindValue( "username", $this->username, PDO::PARAM_STR );
				 //give value to named parameter :password
				 $stmt->bindValue( "password", hash("sha256", $this->password . $this->salt), PDO::PARAM_STR );
				 $stmt->execute();

				 $valid = $stmt->fetchColumn();

				if( $valid ) {
					  $success = true;
				 }

				 $con = null;
				 return $success;
			 }catch (PDOException $e) {
				  echo $e->getMessage();
				  return $success;
			 }
}

 

Here is the welcome/invalid message code. This is on a separate page called login.php

<?php 
} else {
$usr = new Users;
$usr->storeFormValues( $_POST );

if( $usr->userLogin() ) {
	echo "Welcome";	
} else {
	echo "Incorrect Username/Password";	
}
}
?>

Edited April 11, 2014 by Developer

[gamble]

Instead of the $stmt->fetchColumn() try just $stmt->fetch()

[Developer]

Still not working :/.

Edit:

Found the error, the collation was wrong.

I just had to replace the uft8 with latin1

Edited April 11, 2014 by Developer

[HazardBoy]

You can pass the prams in constructor no need to detour.

 

   $usr = new Users( $_POST );

 

Glad you solved it.

[Guest]

You can pass the prams in constructor no need to detour.

 

   $usr = new Users( $_POST );

 

Glad you solved it.

Not everytime you initialize Users may you want $_POST fields :)

[john.]

1. Remove stripslashes( strip_tags( , they are redundant when using prepared statements.

2. Use a better hashing method, like bcrypt.

3. No need to use bindValue, these methods are mostly used when doing stored procedures.

$stmt-execute(array($this->username, $this->password)); is fine.

4. Move the creating of PDO instance outside, it does not belong there (SRP) and should rather be injected throughout the constructor so it can be tested.

5. Remove the try {} catch block, you're not actually doing anything useful with what you catch, should rather let some global exception handler outside the class take care of it.

6. Optimize your query by removing * and just set id, you're actually not fetching anything or returning the PDOStatement object for that matters.

 

<?php
class Login {

   protected $db;

   protected $username;
   protected $password;

   public function __construct(PDO $db) {
       $this->db = $db;
   }

   public function setCredentials(array $credentials) {
       $this->username = $credentials['username'];
       $this->password = $credentials['password'];
   }

   public function login() {

       $success = false;

       $stmt = $this->db->prepare("SELECT `id` FROM `users` WHERE `username?  = ? AND `password` = ?");
       $stmt->execute(array($this->username, $this->password));

       if ($stmt->fetch()) {
           $success = true;
       }

       return $success;
   }
}

 

I rework the code a bit, still not a reusable it can be. What if you want to use email as credential in your next project? Going in the class and changing is not very reusable, neither object-oriented approach :-)

What if you want to use a NoSQL db? etc.

[HazardBoy]

Not every time you initialize Users may you want $_POST fields :)

Then you don't pass the params or pass a different array on init.

The "detour" I see there makes the data verification to be called twice. Once on instance and once on the method call.

I should have explained why before. Sorry/