Is this method to login secure?

[oz11]

Function to check if user is remembered..

function isRemembered($pdo) {
    if(isset($_COOKIE["remember"])){
        return true;
    } elseif(!isset($_COOKIE["remember"])){
        return false;
    }
}

Function to check user's login cookie hash to the hash in the db

function tokenLoggedIn($pdo) {

    //$user_id= $_COOKIE["token_userid"];
    $_COOKIE["token"];

    $stmt = $pdo->prepare("SELECT user_id FROM users WHERE cookie_token=?");
    $stmt->execute([md5($_COOKIE['token'])]); 
    $info0 = $stmt->fetch();
    
    $stmt = $pdo->prepare("SELECT cookie_token FROM users WHERE user_id=?");
    $stmt->execute([$info0['user_id']]); 
    $info = $stmt->fetch();
    

    //echo "<br>";
    //echo "<hr>";
    $str1 =  $info['cookie_token'];
    //echo "<br>";
    $str2 = md5($_COOKIE["token"]);
    //echo "<hr><br>";

    if($str1 != $str2){
        return false;
        //header("location: login.php");
    }elseif($str1 == $str2)
    {
        return true;
    } 

}

 

 

Protected page:

if (isRemembered($pdo) == 0)
{
    if (!isset($_SESSION['loggedin']) OR $_SESSION['loggedin'] != true) {
    
        header("location: login.php");
        exit;
    } 
    echo "good. not remembered but still passes as logged in via sessions";
}
elseif (isRemembered($pdo) == 1) {
    if (tokenLoggedIn($pdo) == false){
        header("location: login.php");
        echo "bad. remembered but not allowed";
        exit;
    } elseif(tokenLoggedIn($pdo) == true) {
        echo "good. remembered and (passes denied/allowed) so logged in";
    }
}

 

Just not sure if using "isRemembered" function could be a security vulnerability... and i don't know any other way to manage this code above ^. The idea is that when you press "remember me", the user can either (not remembered) to the session information check and if remembered this is skipped and goes to the token check. How else can i go about this securely? 

ps. This skipping allow for the session bit to not be necessary if the user has the cookie. Because session data expires, and when it expires then the cookie wont work.. so this is kinda a work around.

 

Tnx. 

Edited September 2, 2022 by oz11
thanks again. Just an extra message at the end

[kicken]

Your code could be simplified.  Your cookie token is essentially a long random password so you should treat it as such and use the password functions to hash and verify it.

Your isRemembered function can be simplified to a single statement. You also have the $pdo parameter which is unused and should be removed.

function isRemembered() {
	return isset($_COOKIE["remember"]);
}

Any time you find yourself writing code like if (<some condition>){ return true; } else { return false; } you can simplify that to just return <some condition>.

If you update your code to use the password functions, you'll need to re-factor your tokenLoggedIn function and your token storage format.  With proper hashing, you can't just look up the record in the DB by hashing the input like you are currently doing.  The reason is that each hash includes some random data called a Salt which will make the input different from what you stored.  This means you need to store either the username or the user ID into your cookie as well.  This could be either a separate cookie, or just as a part of the token cookie.  I'll assume storing the ID and token separated by a ; in my example rewrite below.  Once gain, your final if/return can be simplified.

function tokenLoggedIn($pdo) {
	//Extract user id and token from the cookie.
	[$user_id, $token] = explode(';', $_COOKIE['token'], 2);

	//Lookup the stored hash
	$stmt = $pdo->prepare("SELECT cookie_token FROM users WHERE user_id=?");
	$stmt->execute([$user_id]); 
	$info = $stmt->fetch();

	//Compare the token in the cookie against the stored hash.
	return password_verify($token, $info['cookie_token']);
}

For your final bit of code that puts it all together, I'd suggest ordering the code as this:

1. Check if the user has a valid logged in session
2. If not, check for the token cookie and try to log them in using it
3. If the cookie is missing/invalid, send them to the login page.

This way you only need to spend the time checking and validating the cookie once to start the session.  After that it's a simple matter of checking the session exists and is valid.

Having both $_COOKIE['remember'] and $_COOKIE['token'] is unnecessary.  You only need one which would hold your token.  Since your isRemembered() function has been reduced to a single line, it's also unnecessary, you can just inline that check.  You'd then end up with something like this:

//If no valid session exists
if (($_SESSION['loggedin']??false) !== true){
	if (isset($_COOKIE['token']) && tokenLoggedIn($pdo)){
		//Cookie is valid, setup the session
		$_SESSION['loggedin'] = true;
	} else {
		//No or invalid cookie, send to login page.
		header("location: login.php");
		exit;
	} 
}

For a little extra security, I would consider generating a new cookie token when the previous one has been successfully used to start a session.  This would help guard against the cookie being leaked and used in the future by a third party. You could also limit each token to only be valid for a specific length of time (a few days or a week for example) for the same reason.