3

Following is my php code which should be unset all session after one hour if user inactivity. But after few minutes inactivity it's unset all session variable. It's should one hour Inactivity.

function timeout(){ 
    if(session_id() == '') {
        session_start();
    }
    global $loginUrl;
    $host =  $_SERVER['HTTP_HOST'];
    $sessinTime = $_SESSION['timeout'] = time();
    $session = isset($_SESSION['timeout']) ? $_SESSION['timeout'] : $sessinTime;
    if($session + (60 * 60 * 2) < time()) {
        unset($_SESSION['front_username']);
        unset($_SESSION['front_password']);     
        unset($_SESSION['user_id']);    
        unset($_SESSION['timeout']);    
        echo "<div class='error'>Session is expired. <a href='$loginUrl'>Login again.</a></div>";           
        //echo "<div class='error'>Session is expired. <a href='https://gc1-4.com/acc1/login.php'>Login again.</a></div>";          
        exit(); 
    }
    $_SESSION['timeout'] = time();
}

When user log in I register time() in session variable. Here is the code :

$_SESSION['front_username'] = $username;        
$_SESSION['front_password'] = $password2;       
$_SESSION['user_id'] = $userid;
$_SESSION['timeout'] = time();

Can you tell me that my code right or wrong ? How Can I fix it ? Thanks.

Shibbir
  • 1,963
  • 2
  • 25
  • 48
  • Use `session_destroy()` to clear all session data, or `$_SESSION['front_username'] = null` to clear specific key – fiction Dec 30 '14 at 11:19
  • @fiction problem is not about `unset`. The problem is it's `unset` the `session` variable before one hour inactivity. It's MUST be `unset` the `session` variable If user is passed lazy/inactivity Time. – Shibbir Dec 30 '14 at 11:21
  • session_destroy is way to harsh. What if OP needs to to track other stuff besides user login.. unsetting the specific keys is good practise – DarkBee Dec 30 '14 at 11:22
  • @DarkBee I disagree. How is session_destroy() "harsh"? What if you forget to unset a specific variable? You're leaving a huge window open there for bad stuff to happen by trying to remember each and every session variable you set and unsetting them all manually. – Mike Dec 30 '14 at 11:24
  • @Mike you are making dangerous assumptions about the OP's application. He could very well need other session data to persist, therefore until we know more unsetting specific variables is fine. We cannot judge whether `session_destroy` is too harsh with the knowledge we have now, so we should consider it to be. – Niels Keurentjes Dec 30 '14 at 11:27
  • Ok, Suppose I'm log in in same browser To Admin and Front Panel. So If I log out from front panel using `session_destroy()` then all `session` will be destroy. So that In same I'm loggin out from Admin panel. what should I now for that purpose ? – Shibbir Dec 30 '14 at 11:29
  • See this http://stackoverflow.com/a/1270960/4390089 – Lord Skeletor Dec 30 '14 at 11:36
  • @NielsKeurentjes I see your point and I guess it would work that way, however it does seem weird that OP would want only some of the session data to time out after inactivity and other data not to. If that were the case, I would likely go with a session cookie to store the user's log in stuff and a regular cookie to store everything else. – Mike Dec 30 '14 at 11:39

2 Answers2

1

You just made a small assignment error :

$sessinTime = $_SESSION['timeout'] = time();

This line will reset the initial time and should become :

$sessinTime = $_SESSION['timeout'];
DarkBee
  • 16,592
  • 6
  • 46
  • 58
  • Do you want to mean I should use this line when User log in `$sessinTime = $_SESSION['timeout'] = time();` ? – Shibbir Dec 30 '14 at 11:23
  • @creativeartbd You should know what that line actually *does* and use it accordingly. – Mike Dec 30 '14 at 11:26
  • When user logs in u need to store curren time in that session and in your function `timeout()` you should use the line that I posted. Otherwise u just set the time into the session again – DarkBee Dec 30 '14 at 11:26
  • 1
    @creativeartbd if it still times out, try looking at doing `ini_set('session.gc_maxlifetime' 60*60*2);` when starting the session. 2 hours is probably longer than most PHP installations are configured by default, so you may need to override it. I'm pretty sure the default on Ubuntu is 24 minutes. – Mike Dec 30 '14 at 11:43
1
$sessinTime = $_SESSION['timeout'] = time();
$session = isset($_SESSION['timeout']) ? $_SESSION['timeout'] : $sessinTime;
if($session + (60 * 60 * 2) < time()) {

So uhm:

  1. You assign the current system time to $_SESSION['timeout']
  2. Therefore the ternary in the second line always returns the 'true' part, assigning the current time to $session. Even if it were false it would assign the same value from the previous line, as $sessinTime and $_SESSION['timeout'] are copied into eachother there.
  3. Therefore the condition in the third line is always false, and your code there can never run - it's never two hours later than right now

As it is now, the code is never reset. You're probably seeing something else, like a short session cookie lifetime, or rogue code elsewhere destroying the session.

Also, this part is constructed rather dangerously:

if(session_id() == '') {
    session_start();
}

The check is unneeded and probably breaks things horribly. Just call session_start() always.

Niels Keurentjes
  • 41,402
  • 9
  • 98
  • 136
  • Oh, thanks for that. So I should use this `$sessinTime = $_SESSION['timeout'];` – Shibbir Dec 30 '14 at 11:26
  • Just don't stack assignments unless you know what that actually does. Just changing that line actually won't fix anything, the line after it will still have no logic. – Niels Keurentjes Dec 30 '14 at 11:28