This is similar to #2792305: DigiD doesn't force autologout when browser was closed for a long time.

In this case, when a DigiD user closes a tab then restores it after dvg_digid_autologout_timeout seconds, but still within dvg_digid_autologout_max_session_time, the session will be prolonged with dvg_digid_autologout_timeout seconds, instead of closed.

Attached minimal patch adds a check in init. We cannot use _dvg_digid_autologout_get_user_timeout() for this check as this returns a future timeout which would incorrectly log a user out in the following scenario:

// timeout = 15 minutes, L is last refresh, Q is current request, M is when max session duration is reached:
L--------Q--------M
  12 min   10 min

Note that the inactivity message is not correct on exceeding max session time, but that's for another issue.

Comments

Heine created an issue. See original summary.

askibinski’s picture

Assigned: Unassigned » ralphvdhoudt

Sounds logical. I'll ask Ralph to take a look at this one, he's got more experience with the logout module.

ralphvdhoudt’s picture

Assigned: ralphvdhoudt » Unassigned
Status: Needs review » Needs work

@Heine

I'm trying to reproduce it. I changed the dvg_digid_autologout_timeout to 1 minute and dvg_digid_autologout_max_session_time to 90 minutes. When I close the browser and return after 2 minutes but still within the 90 minutes the session is closed correctly. Can you explain the bug a little bit more

heine’s picture

- Are you using RC3?
- Did you set dvg_digid_autlogout_max_session_time to 5400 ?
- I'm just closing tabs, not the browser, because our cookie_lifetime is set to 0.

When I use the values from #3, I can still reproduce. I also see no way how the code would force a logout after expiry as I cannot find a check on idle time since $_SESSION['autologout_last'].

function dvg_digid_autologout_init() {
  global $user;

  if (empty($user->uid)) {
    if (!empty($_GET['dvg_digid_autologout_timeout']) && $_GET['dvg_digid_autologout_timeout'] == 1 && empty($_POST)) {
      _dvg_digid_autologout_inactivity_message();
    }
    return;
  }

  // Check if JS should be included on this request.
  if (_dvg_digid_autologout_prevent()) {
    return;
  }

  // Check if anything wants to be refresh only. This URL would
  // include the javascript but will keep the login alive whilst
  // that page is opened.
  $refresh_only = _dvg_digid_autologout_refresh_only();

  $now = REQUEST_TIME;
  $timeout = _dvg_digid_autologout_get_user_timeout();
  $redirect_url = variable_get('dvg_digid_autologout_redirect_url', 'user/login');
  $redirect_query = drupal_get_destination() + array('dvg_digid_autologout_timeout' => 1);

  $settings = array(
    'timeout' => $refresh_only ? ($timeout * 500) : ($timeout * 1000),
    'can_refresh' => _dvg_digid_autologout_can_refresh(),
    'redirect_url' => url($redirect_url, array('query' => $redirect_query)),
    'refresh_only' => $refresh_only,
  );

  drupal_add_library('system', 'drupal.ajax');
  drupal_add_js(array('autologout' => $settings), 'setting');
  drupal_add_js(drupal_get_path('module', 'dvg_digid_autologout') . "/dvg_digid_autologout.js");

  // We need a backup plan if JS is disabled.
  if (!$refresh_only && isset($_SESSION['autologout_last'])) {
    // If there is a negative timeout, log them out.
    if ($timeout < 0) {
      _dvg_digid_autologout_logout();
      _dvg_digid_autologout_inactivity_message();
    }
    else {
      $_SESSION['autologout_last'] = $now;
    }
  }
  else {
    $_SESSION['autologout_last'] = $now;
  }
}
function _dvg_digid_autologout_get_user_timeout($uid = NULL) {

  if (is_null($uid)) {
    // If $uid is not provided, use the logged in user.
    global $user;
  }
  else {
    $user = user_load($uid);
  }

  if (_dvg_digid_logged_in($user)) {
    // Only DigiD user gets timeout.
    $timeout = variable_get('dvg_digid_autologout_timeout', 900);
    $max_timeout = variable_get('dvg_digid_autologout_max_session_time', '7200');
    if ($max_timeout > 0) {
      $session_max_end = $_SESSION['autologout_start'] + $max_timeout;
      if ($session_max_end >= REQUEST_TIME) {
        $time_remaining = $session_max_end - REQUEST_TIME;
        // If the next interval exceeds the max session time,
        // use the remainder as timeout.
        if ($timeout > $time_remaining) {
          return $time_remaining;
        }
      }
      else {
        return -1;
      }
    }
    return $timeout;
  }
  else {
    return 0;
  }

}
ralphvdhoudt’s picture

Status: Needs work » Fixed

Reviewed & committed

  • ralphvdhoudt committed c5efc93 on 7.x-1.x authored by Heine
    Issue #2803907 by Heine: DigiD autologout doesn't force logout when a...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.