PHP login and session establishment

Question

I would like to ask your advice on my simple code to login and registration sessions.

In the User class, login function:

public function login($username, $password){   
    $this->db->query("SELECT * FROM users WHERE username = :username AND status = :status LIMIT 1");
    $this->db->bind(':username', $username);
    $this->db->bind(':status', 1);
    $row = $this->db->single();
    $count = $this->db->rowCount();
        if ($count > 0) {
            if (password_verify($password, $row['password'])) {
                $_SESSION['session'] = [
                    'id' => $row['id'],
                    'username' => $row['username'],
                    'email' => $row['email'],
                    ];
                return true;
            } else {
                return false;
            }
        }
}

check logged:

public function isLoggedIn() {
    if (isset($_SESSION['session'])) {
    return true;
    }
}

And login page with check logged:

if ($user->isLoggedIn()) {
    header('Location: index.php');
    exit();
}

if(isset($_POST['login'])){

    //Retrieve the field values from our login form.
    $username = !empty($_POST['username']) ? trim($_POST['username']) : null;
    $password = !empty($_POST['password']) ? trim($_POST['password']) : null;

    if($user->login($username, $password)) {
        header('Location: index.php');
        exit();
    } else {
        $message[] = 'We found problems with the login, please try again.';
    }
}

Answer 1

The could looks like it would work properly, and I think it's clearly written and easy to understand.

When you check that a user is logged in, you should check for something in the session that clearly states they are logged in. For instance you could check that $_SESSION['user']['loggedIn'] === true. It's risky to simply test for isset($_SESSION['session']) as another programmer, or even you in six months time, could write code to set that when the user is not logged in. Perhaps they will want to store a message in the session, like "sorry your password didn't match try again".

I would replace $_SESSION['session'] with $_SESSION['User']. You might then want to only access $_SESSION['User'] from the User class of your application. Other parts of the application may want to store other things in the session - messages to display on next request, or a basket of goods, or whatever the app deals with.

I assume you have called session_start() before the code we see runs.

Using password_verify() is good. Far too many apps implement their own insecure password algorithms.

I don't see the 'new' keyword, which is good - that means you may be doing dependency injection by having the constructor of User take a db as a parameter. If that's true you can easily write a test for the login function that uses a mock db.