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

Authentication Class

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

Problem

I've wrote this class in PHP for my future projects:

```
user = $user;
$this->pass = $pass;
}

/**
* Auth::hash()
* Pass a user/password combination through the crypt algorithm and return the result.
*
* @return string hash Returns the complete hashed string.
*/
private function hash() {
$this->hash = crypt($this->pass, '$2a$10$'.sha1($this->user));
return $this->hash;
}

/**
* Auth::getUname()
* Return the username
*
* @return string The username
*/
public function getUname() {
return $this->user;
}

/**
* Auth::getHash()
* Check if there's a hash, if not, make one, then return the hash.
*
* @return string Hashed password
*/
public function getHash() {
if (empty($this->hash)) {
$this->hash();
}
return $this->hash;
}

public function __toString() {
return $this->getHash();
}

/**
* Auth::databaseConnect()
* Establish connection to database and store the connection on self::$db.
*
* @param PDO $pdo an optional PDO object to have an existing connection instead of a new one
* @return void
*/
public static function databaseConnect($pdo = null) {
#The class accepts an external
if (is_object(self::$db)) {
#Should the connection already been established, an exception will be thrown.
#If you want to start the connection yourself, make sure that this function is called before any other class functions.
throw new Exception('Could not complete operation. Connection was already established.');
}
if (is_object($pdo)) {
if (!($pdo instanceof PDO)) {

Solution

public function __construct($user = "", $pass = "") {
    if (empty($user) || empty($pass)) {


Don't use empty when the variable is guaranteed to exist. It just makes it harder to catch typos, since it suppresses PHP's error mechanism when there's no need to. Since you also require arguments for $user and $pass, don't give them default values:

public function __construct($user, $pass) {
    if (!$user || !$pass) {


Also, hardcoding the database credentials as constants and the database schema is probably not good. That's something you'll want to configure dynamically without modifying your code. Just pass a PDO instance into the class, since you'll likely want to use that connection in more than just the Auth class. Don't let the Auth class instantiate its own connection or manage the database schema, that's not its job.

Code Snippets

public function __construct($user = "", $pass = "") {
    if (empty($user) || empty($pass)) {
public function __construct($user, $pass) {
    if (!$user || !$pass) {

Context

StackExchange Code Review Q#8059, answer score: 2

Revisions (0)

No revisions yet.