cgm225 Posted April 28, 2008 Share Posted April 28, 2008 I have a user authentication class that I am implementing as shown below. I wanted to get some constructive criticism of the class and how I am implementing it. Everything works, but would you do anything differently? Authentication class:: <?php interface Authentication { public function doLogin(); public function checkPermission($permission); } class MysqliAuthentication implements Authentication { /* Variable declaration */ private $username; private $password; private $connection; /* Sets username and raw password for authentication, as well as the * mysqli connection object WITH database selected */ public function __construct($username, $password, mysqli $connection) { $this->username = $username; $this->password = md5($password); $this->connection = $connection; } /* Checks provided username and password against a MySQL database table and * returns true if login is successful, followed with setting of username * and password session variables. */ public function doLogin() { $query = "SELECT COUNT(*) FROM users WHERE username = ? AND password = ?"; $statement = $this->connection->prepare($query); $statement->bind_param('ss', $this->username, $this->password); $statement->execute(); $statement->bind_result($count); $statement->fetch(); if ($count == 1) { return $this->setSession($this->username, $this->password); } else { return false; } } /* Checks if a username has a given permission found within a MySQL database * table, returning true if the username has the given permission. */ public function checkPermission($permission) { $query = "SELECT COUNT(*) FROM permissions WHERE username = ? AND permission_for = ?"; $statement = $this->connection->prepare($query); $statement->bind_param('ss', $this->username, $permission); $statement->execute(); $statement->bind_result($count); $statement->fetch(); return $count == 1; } /* Sets the provided username and password to session variables */ private function setSession($username, $password) { $_SESSION['username'] = $username; $_SESSION['password'] = $password; return true; } } ?> Implementation:: <?php /* Checks for existing login */ $login = (empty($_SESSION['username']) AND empty($_SESSION['password'])) ? false : true; /* Sets username, password, and referring URL */ $username = isset($_POST['username']) ? $_POST['username'] : null; $password = isset($_POST['password']) ? $_POST['password'] : null; $referrer = isset($_POST['referrer']) ? $_POST['referrer'] : null; /* Establishes a MySQLi connection WITH database selected */ $mysqli = new mysqli(MYSQL_SERVER, MYSQL_SERVER_USERNAME, MYSQL_SERVER_PASSWORD, 'example_authentication'); /* Instantiates a new MysqliAuthentication class, a class required within * the index file */ $authentication = new MysqliAuthentication($username, $password, $mysqli); /* If the username and password are verified and the username is logged in, or * if the username was already logged in, the following "refers" the user back * to the page he or she came from. If the login process fails, then the login * form is provided again. */ if (($authentication->doLogin()) || ($login)) { if (($referrer == "http://www.example.com/login") or ($referrer == "http://www.example.com/logout") or (empty($referrer))) { $this->setVar('loginMessage', 'successful'); } elseif (($referrer == "http://www.example.com/") or ($referrer == "http://example.com/")) { header("Location:http://www.example.com"); } else { header("Location:$referrer"); } } elseif (isset($username) OR isset($password)) { $this->setVar('loginMessage', 'Login to Example.com FAILED, please try again'); } else { $this->setVar('loginMessage', 'Login to Example.com'); } /* Closing MySQLi object */ $mysqli->close(); ?> Quote Link to comment Share on other sites More sharing options...
dptr1988 Posted April 28, 2008 Share Posted April 28, 2008 It looks good to me. Very clean and organized. I didn't see any security related issues. Although I don't see the point in storing the password in the $_SESSION variable. The password should only be needed when you do the actual login. Quote Link to comment Share on other sites More sharing options...
Recommended Posts
Join the conversation
You can post now and register later. If you have an account, sign in now to post with your account.