PHP session not resetting on login following a timeout

Question

I have the following code at the top every page:

FROM "PROCESS-LOGIN.PHP"

session_start();


$login_query
    = ("select * from members where email = '$email' and  password = '$password'");
$login_result = mysqli_query($link, $login_query);
$hits = mysqli_affected_rows($link);

if ($hits != 1)
{
    echo "Invalid username and password combination ";
};

while ($db_result = mysqli_fetch_assoc($login_result))
{
    ;
    $id = $db_result['id'];
    $membertype = $db_result['memberType'];

    $_SESSION['id'] = $id;

FROM "SESSION.PHP"

session_start();
if ( isset($_SESSION['id'])) {
    $userID = $_SESSION['id'];

    $session_query = ("select * from members where id = '$userID'");
    $session_result = mysqli_query($link, $session_query);

    while ($db_result = mysqli_fetch_assoc($session_result)) {

        $email = $db_result['email'];
        $membertype = $db_result['memberType'];

        // various other things set from the DB

    };

    if ($membertype == 1){
        $home = "/view/cook-dashboard.php";
    } else if ($membertype == 2) {
        $home = "/view/customer-dashboard.php";
    } else if ($membertype == 3) {
        $home = "/view/admin/dashboard.php";
    };


} else {$membertype = "xxx"; $userID = 0;};

At the top of certain pages I check for $membertype, here is an example of that code:

if ($membertype != 99){
    die("You're session has timed out. Please <a href='/view/login.php'>login again</a>");
}

If I log in to the web app I am taken to the page with the above check and all is fine. If I come back several hours later then I get the "You're session has timed out" message. If I output the variables it shows $membertype as "xxx" - which is what is set if $_SESSION['id'] is not set.

So far, to me, this all makes sense.

What's puzzling me is that if I log in then $membertype is still "xxx" and that page still fails.

Answer 1

There is a host of issues wrong with your code. It may work (or not) but falls short of best practise and so errors such as this one will crop up and be hard to stamp out.

1) What does your error log tell you?

find out here where your log is stored and use it with error_log to output custom messages.

2) Stop using isset()

Frankly, isset() is a terrible function because it's return values are not always logical for those not used to PHP's quirks. Don't use it. I feel empty() gives a more accurate result when judging the value of a variable.

Even better, be as specific as possible. If $_SESSION['id'] is going to be a numeric value you can simply do this:

if((int)$_SESSION['id'] > 0 ){
    //stuff happens when a positive value of id is found.
} 

Explanation:
The (int) casts the value to integer and then this is compared as to if it is above zero. Therefore false or null values are never false positives as they become 0. Whereas isset would return true for false values.

3) Clean up your excess ; .

You do not need them after } in any instance.

4) You're checking passwords WRONG.

You should be using password_hash(). You should be checking the database for the username, SELECTing the corresponding [hashed] password and then checking if this value matches the hash of the POSTed input with password_verify(). Why use password_hash?

Fix this. No excuses. This NEEDS to be fixed..

5) Stop wrapping your string variables in brackets.

It's not needed and can cause logic issues with dealing with functions (which should have brackets) issues as well as confuse reading the code for others.

6) When checking the same value multiple times use switch statements.

This is more efficient than a bunch of if statements as you're checking once and then seeing what the result matches.

switch ($membertype){
    case 1:
        $home = "/view/cook-dashboard.php";
        break;
    case 2:
        $home = "/view/customer-dashboard.php";
        break;
    case 3:
        $home = "/view/admin/dashboard.php";   
        break;
    default:
        $membertype = "xxx"; 
        $userID = 0;
   }

7) Stop creating wasted variables.

You have created the $userID variable but it's just the same as the $_SESSION['ID'] value, so what's the point of it? It add's a neatness possibly to your style of coding but your else value only ever resets the variable never the session.

Surely if you reach the else then you need to set $_SESSION['ID'] = 0; as well?

Excess and repetative variables are wasted space, processing power and memory.

8) Look into using Prepared Statements with MySQLi

There are a lot of almost decent tutorials on the net about using Prepared Statements. Explore it. There are also a lot of questions on Stack Overflow about how to use Prepared Statements.

9) Stop using SELECT *

Specifically pick the columns you need. Saves energy, saves time.

10) Check your database!

• Is your column name correct? Names are case sensitive.
• Does other data from that same row get returned ok?
• Is there any data in that specific field to return?

And finally......

11) Your issue needs to be explored.

try this:

while ($db_result = mysqli_fetch_assoc($login_result))
    {
    error_log("While loop run. Login Data found:\n");
    error_log(print_r($_db_result,true));

    $id = $db_result['id'];
    $membertype = $db_result['memberType'];
    ....

This will output telling you exactly if the column memberType has a value for the chosen row.

---

From the code you've shown I suspect that a) the excess ; are causing logic hiccups. or b) that you're database data is not being output as expected.

---

Disclaimer: There are a couple of other more minor errors and symptomatic issues but I have to drawn the line somewhere.... Also many of the best practises I have shared do have specific situations where they may not apply.