PHP session_set_saver_handler with session timeout

Question

I have developed a class that utilises the session_set_saver_handler function so I can store sessions within my DB. The class works just as I would like.

However, my only concern is the way I have approached the session timeout.

Current the _read() function code looks like:

/**
     * Read session function
     * @access public
     * @return the 'data' record providinf the PDO statement executed correctly. Otherwise, return false.
     */ 
    public function _read($id) 
    {

        $timeout = time() - $this->accessTime;
        $locked  = false; 

        $this->database->query('SELECT updatedTime, data FROM sessions WHERE session = :id AND locked = :locked');
        $this->database->bind(':id', $id); 
        $this->database->bind(':locked', $locked); 

        if($this->database->execute())
        {
            if($this->database->rowCount() > 0)
            {
                $row = $this->database->singleResult();

                if($row['updatedTime'] < $timeout)
                {

                    //Set the location of the user. 
                    $url = "http://" . $_SERVER['SERVER_NAME'] . $_SERVER['REQUEST_URI']; 

                    if($url != $this->redirectUrl)
                    {
                        header('Location: ' . $this->redirectUrl);
                        return; 
                    }   

                    return ''; 
                }

                return $row['data']; 
            }
        }

        return '';
    }

When I originally create the script I hard coded the redirect URL. The problem was logout.php (the file the user is redirected to) contains the session class. Meaning I have a constant loop. So I approached it by implementing the following:

if($row['updatedTime'] < $timeout)
                {

                    //Set the location of the user. 
                    $url = "http://" . $_SERVER['SERVER_NAME'] . $_SERVER['REQUEST_URI']; 

                    if($url != $this->redirectUrl)
                    {
                        header('Location: ' . $this->redirectUrl);
                        return; 
                    }   

                    return ''; 
                }

This seems to me more of a 'hack' than intelligent code.

Did I approach this correct (probably not)? If not, what would be a more intelligent work around?

Answer 1

Since the code for logout.php is not provided, I'll just make assumptions. Your code is pretty clean but I think it could be improved a bit.

Miscellaneous

Both locked and timeout are defined but only used once. Since they are local variables, I consider that they could be removed and just replaced by their values.

Still concerning the timeout, I prefer working with proper date and time types provided by MySQL rather than working with timestamps stored as integers. The main reason is that there are a lot of functions at your disposition, like for example NOW() and TIMESTAMPDIFF(), which could be used to rewrite your query easily

SELECT TIMESTAMPDIFF(MINUTE, NOW(), updatedTime ) AS minutesSinceLastActivity, data FROM sessions WHERE session = :id AND locked = :locked 

Once you have that, your condition is pretty simple : if($row['minutesSinceLastActivity'] > $this->accessTime) . Which leads me to another point, the naming of accessTime is pretty bad, I would define it as something like minutesInactivityBeforeLogout.

One last thing that I'm a bit unsure about (I know it works for C, don't remember for PHP), you could regroup the two conditions if($this->database->execute()) and if($this->database->rowCount() > 0) into one with the and operator. Since you'll use an and, if the execution of the query fails, execution the rest of the code in the if condition is unnecessary since it cannot change the result of the condition. Once again, this is something I know works in C, not quite sure about PHP.

How it works

You said you had a constant redirection loop with a previous version of your code. The main reason I could fin for that is that if you detect a timeout, you don't do anything on your MySQL table, which means you'll always end up in the same condition and doing always the same thing.

That's why you should keep your structure and query as it is (you could put the timeout condition in the query, but doing so, you would be unable to differentiate the case where the timeout is reached from the case where there is no record for the current id). The only thing that you have to do is for example to delete the record if the timeout condition is reached.

Answer 2

Comparing URLs is a fragile mechanism for deciding whether to redirect the user. It will break if there are URL-remapping rules in the server configuration, or if HTTPS is in use.

I'd rather set and test a global variable such as $is_login_page = TRUE; to suppress the redirect than compare URLs — even though global variables are evil in general.