patternphpMinor
How would a senior PHP developer design this Login class?
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:
```
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['
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
Ok, so you've heard that you should hash passwords before storing them, because it turns out that
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.
$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.