Handling user sessions on website using $_COOKIE and $_SESSION

Question

So I am building a website. It will be only used inside our small company and it's not made for public use.

What I want to achieve is the ability for user to maintain session after sign in for a prolonged period of time while also maintaining some sense of security and common sense.

My website building experience is 2-3 weeks of building things with HTML/PHP/Bootstrap/JS where I mostly just lurk around and do research.

So far, from what I have read, tried and found I was able to get a solution working, yet what I would like is for someone to take a look and tell me what is good and what is bad with my approach and how can I improve this.

This is the file I created for working with Cookies and Sessions. This file will be included using require() on each and every page that can be accessed by web-site navigation (that means this won't be used on any 'back-end' files because that makes no sense I think).

I will explain my thought process below in the comments to code.

<?php
// Firstly, we check if any session is set, because if the session is set, 
// then the user can still navigate the website and we don't need to do anything
if (!isset($_SESSION['id'])) {

    // The session is not set.
    // Check if Cookie with pre-defined name that I thought of for my web-site exist
    if (isset($_COOKIE['info'])) {

        // Okay. We have no session but we have a cookie. Let us start a session then.
        
        // Getting hold of database handler.
        require_once(__DIR__ . '/../dbh/dbh.php');

        // Preparing statement for accessing DB.
        $stmt = $connection->prepare("SELECT * FROM `users` WHERE `username`=?");

        // Information inside my Cookie is stored in an JSON encoded string, so we decode it.
        $info = json_decode($_COOKIE['info'], true);

        // Assign extracted information to variable for ease of use
        $username = $info['username'];
        $password = $info['password'];
        
        // Bind parameters and execute the statement we prepared
        $stmt->bind_param('s', $username);
        $stmt->execute();
        
        // Get results of statement execution
        $result = $stmt->get_result()->fetch_assoc();

        // The result may be `NULL` if the username we tried to find does not exist. 
        // So if the username does not exist, then we can't start the session.
        if ($result !== NULL) {
            // The username exists so we can proceed further.

            // Assign `username` and `password` we got from query to variables for ease of use
            $db_username = $result['username'];
            $db_password = $result['password'];

            // Here we do an extra step of verifying user`s credentials just to be 100% sure
            if (strcmp($username, $db_username) == 0 && strcmp($password, $db_password) == 0) {
                // Credentials are identical, so we can safely start a session for this user.

                session_start();
                $_SESSION['id'] = $result['id'];
                $_SESSION['username'] = $result['username'];
                $_SESSION['password'] = $result['password'];

                // JSON encoding user's credentials for storing in a cookie.
                $info = array(
                    'username' => $username,
                    'password' => $password
                );
                
                // We are going to set a refreshed cookie for to make sure 
                // that this cookie expires only when the user doesn't 
                // visit the website for a month.
                setcookie('info', json_encode($info), time() + 2592000);
            }
        }

        // This is an interesting part. 
        // This `if` statement was created to avoid endless loop when user initially enters the website. 
        // Here we check if the page is `index.php` and if it is, then we don't need to redirect user.
        if ($_SERVER['REQUEST_URI'] !== '/' && $_SERVER['REQUEST_URI'] !== '/index.php') {
            header("Location: http://" . $_SERVER['HTTP_HOST'] . "/index.php");
            exit();
        }
    }

    // Same thing as above, just in two different `if` statements.
    if ($_SERVER['REQUEST_URI'] !== '/' && $_SERVER['REQUEST_URI'] !== '/index.php') {
        header("Location: http://" . $_SERVER['HTTP_HOST'] . "/index.php");
        exit();
    }
}

Answer 1

IF logic

All encompassing ifs

Right off the bat you have two levels of nested if statements before we get to the main body of the code...

if (!isset($_SESSION['id'])) {
    if (isset($_COOKIE['info'])) {
        // Main code...
    }
    // Additional code...
}

...which is unnecessary; you could just as easily merge them into one statement:

if (!isset($_SESSION['id']) && isset($_COOKIE['info'])) {
    // Code here...
}

Repeated ifs

The first of the duplicated if statements isn't needed. A simplified version of your code would look like the following...

if (true) {
    if (true) {
        // Do "task"
    }

    // Do "task"
}

...notice that if the first condition is true then the task is carried out regardless of the state of the second condition. Which means that you don't need to repeat the code in the nested if statement because when that code block ends the task is completed anyway!

The only reason that it isn't completed twice in your case is because you exit the script manually after it runs the first time.

String comparison

There's nothing wrong with using strcmp to compare two strings; however it is not needed in this scenario. You don't care whether one string is greater or lesser than the other you simply want to know if they are identical. The === comparison operator does exactly that: the values are either identical (true; equal and of same type) or they aren't (false). For example:

if ($username === $db_username && $password === $db_password) {
    // Username and Password are correct...
}

Additionally, you're using a loose comparison: it's better practice to use a strict comparison as your default choice as you have done later on in the script. Especially here when the output you're looking for is an integer.

Whilst it may be true in this circumstance that the only false value output is (int) 0 reusing the same logic somewhere else could lead to unexpected behaviour.

Lastly, you compare both the username and the password. You don't need to do that. You've already compared the username in your query; if it didn't match then the record would never have been returned in the first place so the check is redundant.

Checking the $result

Again, technically, there's nothing wrong here. And you are using the strict comparison operators mentioned earlier. But in this case, there is no need: the result of fetch_assoc will either be a mysqli result set or null. Which is equivalent to true or false so a strict comparison to null isn't required.

Instead you can simply use:

if ($result) {

After all, if any false equivalent value was returned you wouldn't want to proceed. If you did then, at minimum, you would gain a bunch of Notices in your error log from trying to access non-existent array indexes.

Sessions

To put it bluntly, your first if statement never succeeds! You know this (I think) because you have implemented a work around with the duplicated if statements at the bottom of the code.

If you think about it: you'd never end up in a loop because after the first time the code is called the $_SESSION["id"] is set and therefore the opening if statement will evaluate to false on subsequent page loads.

So then the question is, why does it loop?

The answer is simple: the $_SESSION super global doesn't exist until after session_start(); is called. Up until that point...

$_SESSION === null;

// And...

isset($_SESSION) === false;

...no matter how many indexes you've assigned to it.

So in your if statement isset(...) evaluates to false and is then flipped with the preceding ! to true and therefore the code bock is executed.

The solution then is to make sure session_start(); is at the very top of your file.

<?php

session_start();

if (!isset($_SESSION["id"])) {
    // Do something...
}

Getting this right means you can do away with both of the duplicated if statements as now you won't have that endless loop.

Cookie management

You must delete the cookie if it doesn't match. You don't want random expired, useless, cookies sitting around. They may cause unexpected behaviour.

Cookies should not contain the users password (especially not in plain text form). We don't really want user details like (e.g. username and email address) stored in cookies either. In the event that a cookie is hijacked it would make using that information much easier.

Leaving both the username and password in the cookie means that if it were stolen then the attacker would have access to the account until the user changes their password worse still the attacker could change the password and lock out the real user!

Misc.

Variable names

Names like $connection aren't very clear to a reader unfamiliar with your code (connection to what?). Much better to use a name with clear meaning such as $mysqli (because it's a mysqli object) or $mysqliConnection.

Storing credentials in $_SESSION

You don't need to store anything apart from the id. Anything else can be looked up in the database based on the id because it is (or should be) a PRIMARY KEY.

Database and SQL

Password storage

You must not store a users password in plain text form. There are so many reasons why it's hard to know where to begin, but here are two of the key reasons:

1. If your database is leaked those passwords are accessible to the world
2. Anyone with access to the database can see ALL your users' passwords
   • Inevitably a percentage of your users will reuse the same (or similar) passwords elsewhere which means anyone with access to the DB can now access other accounts (i.e. other websites) belonging to your users!

PHP has in built functions to hash and check passwords:

// Hash the password; on account creation

$password     = "mySuperStrongPassword";                    // User enters
$passwordHash = password_hash($password, PASSWORD_DEFAULT); // DB stores

// Check the password against the DBs hash; on login

if (password_verify($password, $passwordHash)) {
    // Succesfully matched the password!
}

SQL in prepare

For short queries it's okay to put them directly into the prepare method. It's good practice though to keep them in a separate variable - especially for longer and more complex ones. Keeping them separate means you have more flexibility in formatting the query without confusing the flow of the page.

Table names

It's good practice to not user plurals as database table names. There's no major flaw here but it makes your code nicer/easier to read because usually we're working on one individual record at a time:

// In SQL queries...

SELECT user.email FROM user WHERE user.id = 123;
SELECT users.email FROM users WHERE users.id = 123;

// In PHP..

while ( $user = $query->fetchObject() ) {
    echo $user->email;
}

SELECT *

There's no harm in using the * operator to SELECT all records - unless you're selecting a large number of rows, the table consists of a large number of columns, or the columns pack large volumes of data.

For example, let's assume you're selecting all employees (e.g. 50 records) and each record has a blob for a profile picture. All of a sudden that's a much larger chunk of memory compared to the email address fields we were actually after!

So, it's much better to be explicit about what fields you want returned when you can be. In your case we're only interested in the password and id columns:

SELECT id, password FROM `users` WHERE `username` = ?

---

Persistent logon

DB Structure

If you only wanted a user to be active on one device at a time then you could simply add a field (or two) to the users table.

In most circumstances though it's preferred that users can logon on multiple devices; so we need to implement a one to many relationship in the database with the addition of a new session table. Which, on a simplistic level, will look something like:

CREATE TABLE session (
    id int AUTO_INCREMENT PRIMARY KEY,
    user_id int,
    cookie_id varchar(250),
    hash varchar(250),
    expires datetime
);

Code Example

Normal logon:

<?php

session_start();

require_once(__DIR__ . '/../dbh/dbh.php');

$username   = $_POST["username"]   ?? null;
$password   = $_POST["password"]   ?? null;
$persistent = $_POST["persistent"] ?? null;

if ($username && $password) {

    $logonSQL   = "SELECT id, password FROM user WHERE username = ?";
    $logonQuery = $mysqli->prepare($logonSQL);
    $logonQuery->bind_param("s", $username);
    $logonQuery->execute();

    $user = $logonQuery->get_result()->fetch_assoc();

    if ($user) {
        if (password_verify($password, $user["password"])) {
            // Logged on
            $_SESSION["id"] = $user["id"];

            // Create cookie, if persistent is checked
            if ($persistent) {
                // Create a session cookie
                $cookieId = bin2hex(random_bytes(50));
                $hash     = bin2hex(random_bytes(50));
                $dbHash   = password_hash($hash, PASSWORD_DEFAULT);
                $expires  = date("Y-m-d H:i:s", strtotime("+30 days"));

                $sessionSQL = "
                    INSERT INTO session
                        (user_id, cookie_id, hash, expires)
                    VALUES
                        (?, ?, ?, ?)
                    ";
                $sessionQuery = $mysqli->prepare($sql);
                $sessionQuery->bind_param("isss", $user["id"], $cookieId, $dbHash, $expires);
                $sessionQuery->execute();

                setcookie("rememberme", $cookieId.$hash, strtotime("+30 days"));
            }
        } else {
            // Wrong credentials...
        }
    }
}

Logon with cookie:

<?php

session_start();

if (!isset($_SESSION["id"]) && isset($_COOKIE["rememberme"])) {
    list($cookieId, $hash) = str_split($_COOKIE["rememberme"], 100);

    $sessionSQL   = "
        SELECT id, user_id, hash, expires
        FROM session
        WHERE cookieId = ?
        ";
    $sessionQuery = $mysqli->prepare($sessionSQL);
    $sessionQuery->bind_param("s", $cookieId);
    $sessionQuery->execute();

    $session = $sessionQuery->get_result->fetch_assoc();

    if (
        password_verify($hash, $session["hash"]) &&
        $session["expires"] > date("Y-m-d H:i:s")
    ) {
        // Successful login with cookies

        $_SESSION["id"] = $session["user_id"];

        $hash     = bin2hex(random_bytes(50));
        $dbHash   = password_hash($hash, PASSWORD_DEFAULT);
        $expires  = date("Y-m-d H:i:s", strtotime("+30 days"));

        /**
         * Regenerating the hash
         *
         * You don't have to do this _every_ time but frequently enough
         * that the hash doesn't become _stale_.
         *
         * Regenerating the hash means that even if your cookie is
         * hijacked the next time the _real_ user logs on the session
         * will be scrubbed and the _attacker_ won't be able to log on
         * with the stolen cookie again.
         *
         */

        setcookie("rememberme", $cookieId.$hash, strtotime("+30 days"));

        $updateSQL = "
            UPDATE session
            SET 
                expires = ?,
                hash    = ?
            WHERE id = ?
            ";
        $updateQuery = $mysqli->prepare($updateSQL);
        $updateQuery->bind_param("ssi", $expires, $hash, $session["id"]);
    } else {
        // Session expired or logon credentials incorrect...
        // Delete cookie and session record
        
        setcookie("rememberme", "", 1);
        
        unset($_SESSION["id"]);

        $deleteSQL = "DELETE FROM session WHERE id = ?";
        $deleteQuery = $mysqli->prepare($deleteSQL)
        $deleteQuery->bind_param("i", $session["id"]);
        $deleteQuery->execute();
    }
}

Log off:

if ($_GET["logout"] ?? null) {
    if (isset($_COOKIE["rememberme"])) {
        $cookieId = substr($_COOKIE["rememberme"], 0, 100);

        $deleteSQL = "DELETE FROM session WHERE cookie_id = ?";
        $deleteQuery = $mysqli->prepare($deleteSQL)
        $deleteQuery->bind_param("s", $cookieId);
        $deleteQuery->execute();

        setcookie("rememberme", "", 1);
    }

    unset($_SESSION["id"]);

    header("location: /safeplace.html");
}

Notes

• The above code is untested, there's likely a few typos etc.
• The code also doesn't check to ensure that the cookie_id is unique in the DB
  • Very unlikely to happen on a small site
  • Not a security problem, even if it did, because the worst case scenario is a session gets removed
    • Unless the 1 in 64^100 occurs and the hash/cookie_id are the same
  • Perhaps something to look at to ensure a consistent user experience
• Implementing this code you'd probably want to logout any active user before trying to logon with new credentials
  • Save any bugs
• You may also want to factor in things like garbage collection in the session table to remove expired sessions
• It's common practice that a remember me session doesn't have total access to the system
  • For example, you wouldn't let someone change their password without challenging them for the old password first!
  • This helps prevent problems if/when cookies are stolen