Users management php class

Question

After a little bit of troubleshoots, I've finished this simple users registration and login class. It uses PDO to connect to a mysql database, I've tried also to manage the session when an user do the login or logout. The main objective of this code form me was to make an exercise for using correctly the PDO and related functions, and to start improving my coding skills to manage account creations and login using PHP OOP.

<?php
reqiure_once 'config.php';
class User{

private $db = null;
private $email;    
private $username;   
private $password;
private $uid;
private $sessioncode;        

public function __construct(\PDO $db){

    $this->db = $db;
}    

public function createUser($email,$username,$password){

    if($this->checkEmail($email)){
        echo 'Email address already exsist.';
    } elseif($this->checkUsername($username)){
        echo 'Username already exsist.';
    } else {
        $this->password = password_hash($password,PASSWORD_BCRYPT);
        $stmt = $this->db->prepare('INSERT INTO users (email,username,password) VALUES (?, ?, ? )');      
        if($stmt->execute(array($email,$username,$this->password))){
        echo 'Account successful created';     
        }       
    } 
}    

public function loginUser($username,$password){

    $stmt = $this->db->prepare('SELECT id,username,password FROM users WHERE username = ?');
        $stmt->execute(array($username));
            if($stmt->rowCount() > 0){
                $result = $stmt->fetch(PDO::FETCH_OBJ);
                if(password_verify($password, $result->password)){
                $this->setSession($result->username,$result->id);

            } else {
                echo 'wrong password';
                }
            } else {
                echo 'Username or email does not exist in the database.';
            }     
}    

public function logoutUser($uid){

    $this->unsetSession($uid);    
}

private function checkEmail($email){

    $stmt = $this->db->prepare('SELECT email FROM users WHERE email = ?');
        $stmt->execute(array($email));
        if($stmt->rowCount() > 0){
        return true;
        } else {
        return false;    
        }
}    

private function checkUsername($username){

    $stmt = $this->db->prepare('SELECT username FROM users WHERE username = ?');
        $stmt->execute(array($username));
        if($stmt->rowCount() > 0){
        return true;
        } else {
        return false;    
        }        
}    

private function setSession($username,$uid){

    $this->sessioncode = bin2hex(rand());
    $_SESSION['id'] = $uid;    
    $_SESSION['username'] = $username;
    $_SESSION['session_code'] = $this->sessioncode;
        $stmt = $this->db->prepare('UPDATE users SET session_code = ? WHERE id = ?');
        if($stmt->execute(array($this->sessioncode,$uid))){
        header('Location: u/dashboard.php');    
    }   
}    

private function unsetSession($uid){

    $this->uid = $uid;
    $this->sessioncode = '';
    $stmt = $this->db->prepare('UPDATE users SET session_code = ? WHERE id = ?');
        if($stmt->execute(array($this->sessioncode,$this->uid))){
        session_unset();
        session_destroy();
        header('Location: ../index.php');    
    }    
}

}
?>

Answer 1

Whole user management system is very hard to do properly even with structural code. When you get a grasp of entire logic & security nuances OOP itself takes it to the next level, because this logic will be hard to encapsulate in objects of separate concerns - session, cookies, http request and response, domain models, view models, factories, routing... potential abstraction leaks are everywhere. That said, let's take a look at your class...

From OOP perspective

Output:

This class is not concerned about output - you don't know its format (json/html) nor channel/destination (http/console/log file) - you will validate passed values against your data model, but nothing more. For example when registering new user, the response should be the same user data (withut password - not needed anymore) with user id or class specific validation errors attached like (bool) usernameConflict, (bool) emailConflict - that's it. The rest is up to clients of this class (they might not want to reveal users' emails in this indirect way)

Database:

You should separate yourself from database - it's not relevant to your application where data comes from. Use port & adapter technique - interface to query methods (taking only required parameters) and its PDO implementation.

Session:

Session at this point is a global dependency and hidden structural dependency (there needs to be session_start() somewhere). Remove it from this class and return what it needs instead. It should be something that provides user id & not only that. There will be other parties interested in authentication result, so return user data without password hash and other "secrets".

Handling hashes:

This is the only concern of your class: get credentials for concrete user (defined by username, email, cookie token id) and its hash signature (password hash, persistent cookie token, ...), compare result & return data useful to others - beyond this class hashes should be no use to anyone. You won't have methods covering entire login and logout procedure - just a part of it.

and side-effects...

There will be also some side-effect logic in this class like possible password rehashing on algorithm's change (after succesful match), removing cookie tokens (id+hash) on mismatch, hashing token (id+token is generated outside and passed as response cookie)... etc.

Some security concerns:

• After succesful login session id should be regenerated (see: session fixation attack)
• After failed login/registration - don't inform that same email is in database (mentioned in "output" section). Registering user with existing email might be handled by sending some (merge/restore) options in activation email.
• Session code doesn't protect you from anything - it all happens inside application environment (only session id goes outside). Holding user's id for authentication (in case user is deactivated/removed) is enough
• Make sure that httpOnly and Secure flags are set for session-type cookies (https might be hard to setup for localhost env, but it's neccessary on remote)
• All forms should be signed/marked with CSRF tokens - new session id = generate new token is most convenient approach (every form with new token doensn't add much to protection, but causes problems with multiple open tabs, back button...)
• More...

Answer 2

Early Returns

I prefer early returns for example

if(!$stmt->rowCount()){
    echo 'Username or email does not exist in the database.';
    return false;
}

$result = $stmt->fetch(PDO::FETCH_OBJ);

if(password_verify($password, $result->password) !== true){
    echo 'wrong password';
    return false;
}

$this->setSession($result->username,$result->id);

Session Ids

Its against best practise (as i understand it) to create your own session id you can very easily get the current sessions id by calling session_id()

\PDO Is Not Injectable (without custom definitions)

I like dependency injection (especially auto wiring) so in the future if you were to use auto wiring you would have to add a definition some where for the \PDO object so it might be better to put that pdo object in a class so you can get it in the constructor

public function __construct(database $database)
{
    $this->db = $database->dbObject; 
    // Or 
    $this->db = $database->getSomeObjectOrThrowSomeException()
}

but thats just me

The last thing id say which yourcommonsense has already pointed out dont echo in the model