HiveBrain v1.2.0
Get Started
← Back to all entries
patternphpMinor

How would a senior PHP developer design this Login class?

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
thishowphpdesignloginwouldseniordeveloperclass

Problem

How can I design this class and its implementation so that it looks like it was developed by a seasoned senior level PHP developer that follows all of the best practices? I'm trying to design this class so that it can be reused in other applications. I also have some other questions that are located inside of the code.

Please overlook commenting on the following 2 things:

  • Docblock commenting



  • Unit testing



Login Class

```
pdo = $pdo;
$this->error = $error;
$this->username = $loginCredentials['username'];
$this->password = $loginCredentials['password'];

}

public function authenticate()
{
/**
* Question 3.
* In each class that uses a PDO connection object,
* is it proper to do all these database interaction details
* in each method that requires database interaction?
*/
$query = "SELECT * FROM admin WHERE user = :user AND password = :pass";

//Prepare statement
$stmt = $this->pdo->prepare($query);

//Set fetch mode
$stmt->setFetchMode(PDO::FETCH_ASSOC);

//Set params to bind
$params = array(
':user'=>$this->username,
':pass'=>sha1($this->password)
);

$stmt->execute($params);

//If the user is found, log them in.
if ($stmt->rowCount()) {
$this->loginUser();
return true;
} else {
/**
* Is this the best way to set errors?
*/
$this->error->set('Invalid Username and or Password combination');
}
}

/**
* Question 4.
* Is this the proper way to display errors back to the user?
* Once a use submits the login form, the client code calls
* authenticate(). If it returns false, it runs this method.
*/
public function getErrors() {
return $this->error->getErrors();
}

private function loginUser()
{
$_SESSION['

Solution

Red flag

$query = "SELECT * FROM admin WHERE user = :user AND password = :pass";


Ok, so you've heard that you should hash passwords before storing them, because it turns out that pass is a SHA hash. But you don't seem to have heard of salt. The password database is going to be wide open to a rainbow table attack.

I would recommend that you use a framework which has a good password storage mechanism (if there is one). If not, read the OWASP Password Storage Cheat Sheet very carefully before you rewrite this code.

Code Snippets

$query = "SELECT * FROM admin WHERE user = :user AND password = :pass";

Context

StackExchange Code Review Q#18068, answer score: 4

Revisions (0)

No revisions yet.