PHP User management class

Question

Here is my code for a class that is responsible for managing some common users task like registration, login and logout. It implements also a sub class that can be used to check the session of a logged in user. I've tested it and works fine, but I will improve it as soon as I can. Any suggestion on security improvements are appreciated.

<?php
require_once 'config.php';
Interface UserInterface{
 public function createUser(array $args);
 public function loginUser(array $args);
 public function logoutUser();
}
class User Implements UserInterface{
 private $db;
 private $stmt;
 private $email;
 private $username;
 private $password;
 private $id;
 private $sessioncode;
 private $args;
 /*
 * @param $db must be a PDO instance
 *
 */
 public function __construct(\PDO $db){
 $this->db = $db;
 }
 /*
 * @param $args must be a key/value array
 *
 */
 public function createUser(array $args){
 if($this->checkEmail($args['email'])){
 #header("HTTP/1.1 400 Email already exsist");
 #http_response_code(400);
 echo 'Email address already exsist.';
 }
 elseif($this->checkUsername($args['username'])){
 #header("HTTP/1.1 400 Username already exsist");
 #http_response_code(400);
 echo 'Username already exsist.';
 }
 else {
 $this->password = password_hash($args['password'], PASSWORD_BCRYPT);
 $stmt = $this->db->prepare('INSERT INTO _users (email, username, password) VALUES (?, ?, ? )');
 if($stmt->execute(array($args['email'],$args['username'],$this->password))){
 echo 'Account successful created';
 }
 }
 }
 /*
 * @param $args must be a key/value array
 *
 */
 public function loginUser(array $args){
 $stmt = $this->db->prepare('SELECT id,username,password FROM _users WHERE username = ?');
 $stmt->execute(array($args['username']));
 $result = $stmt->fetch(PDO::FETCH_OBJ);
 if(count($result) > 0 && password_verify($args['password'], $result->password)){
 UserSessionHelper::setSession($result->username, $result->id);
 #header('HTTP/1.1 200');
 echo 'Logged in';
 }
 else {
 echo 'Wrong username or password';
 #header('HTTP/1.1 400');
 }
 }
 /*
 * This method wehn called will logout an user
 *
 */
 public function logoutUser(){
 UserSessionHelper::unsetSession();
 header('HTTP/1.1 200');
 #header('Location: ');
 #echo 'Logged out';
 }
 /*
 * @param $email is a key part of the $args array;
 * This method will check if a given email is already registered.
 */
 private function checkEmail($email){
 $stmt = $this->db->prepare('SELECT email FROM _users WHERE email = ?');
 $stmt->execute(array($email));
 $result = $stmt->fetch(PDO::FETCH_OBJ);
 if(count($result) > 0){
 return true;
 }
 }
 /*
 * @param $username is a key part of the $args array;
 * This method will check if a given username is already registered.
 */
 private function checkUsername($username){
 $stmt = $this->db->prepare('SELECT username FROM _users WHERE username = ?');
 $stmt->execute(array($username));
 $result = $stmt->fetch(PDO::FETCH_OBJ);
 if(count($result) > 0){
 return true;
 }
 }
}
interface UserSessionHelperInterface{
 public static function unsetSession();
 public static function setSession(string $username, int $user_id);
 public static function validateSessionID(string $session_id, string $session_hash);
}
class UserSessionHelper implements UserSessionHelperInterface{
 private $session_hash;
 private $username;
 private $user_id;
 /*
 * @params $username must be a string, $user_id must be an integer.
 * This method will register the $_SESSION variables when an user login.
 */
 public static function setSession(string $username,int $user_id){
 $_SESSION['session_'] = self::sessionHash();
 $_SESSION['id_'] = $user_id;
 $_SESSION['username_'] = $username;
 return true;
 }
 /*
 * @param
 * This method will remove all $_SESSION data wehn an user logout.
 */
 public static function unsetSession(){
 session_destroy();
 session_unset();
 }
 /*
 * @params $session_id must be a valid string, $session_hash must be a valid string.
 * This method will check for valid session credentials when an user is logged in.
 */
 public static function validateSessionID(string $session_id,string $session_hash){
 $computed_session_hash = hash('sha384', $session_id);
 if(!preg_match('/^[-,a-zA-Z0-9]{1,128}$/', $session_id) > 0){
 #return header('HTTP/1.1 403');
 }
 elseif(!hash_equals($computed_session_hash, $session_hash)){
 #return header('HTTP/1.1 403');
 }
 else{
 return true;
 }
 }
 /*
 * This method is responsable to hash the regenerated session id, then return it
 *
 */
 private function sessionHash(){
 session_regenerate_id();
 $session_hash = hash('sha384', session_id());
 return $session_hash;
 }
}
?>

• KIKO Software: Would validateSessionID() not get $session_id and $session_hash from session_id() and $_SESSION['session_']? Why have them as method arguments, if this is the case?

Answer 1

• For session management, you might want to take a look at SessionHandlerInterface. Generally, your class can handle it all, but further on - you can set custom session data storage, like Memcache or Redis.

• It seems like you're using php7(param typehints), but no return types. Consider doing that too. For instance, UserSessionHelper::sessionHash can be declared as function sessionHash() : string

• Class User sounds like a model class, while it's actually kind of a repository/manager. Consider re-naming.

• It is considered a good practice to separate the resource handler logic and business logic, e.g. queries for DB and the user management. What I would do, is move the DB logic to a separate class, UserStore, for instance,

   class UserStore {
   public function isEmailUnique(string $email) : bool {
   // check in DB
   }
   public function isEmailUnique(string $username) : bool {
   // check in DB
   }
   public function create($email, $username, $password): UserModel {
   }
   }
  

  and inject it to User class. So that the UserManager class could looks something like this:

  class UserManager {
   /**
   * @var UserStore
   */
   protected $store;
   public function __construct(UserStore $store) {
   $this-store = $store;
   }
   /**
   * @param array $args
   * @return UserModel
   */
   public function createUser(array $args): UserModel {
   if (!$this->store->isEmailUnique($args['email'])) {
   throw new \LogicException('Email address already exsist');
   }
   if (!$this->store->isUsernameUnique($args['username'])) {
   throw new \LogicException('Username is already taken.');
   }
   return $this->store->create(
   $args['email'], 
   $args['username'], 
   password_hash($password, PASSWORD_BCRYPT)
   );
   }
   }
  

Hope that helps.