First, thanks for the great module!

I am currently using the session_limit module to prevent a certain amount of sessions on a particular role/account. In my testing I have the account limited to 2 sessions and prevents any more until those are ended. I am relying on the automated logout module to end the sessions after a particular amount of time (1 min for testing). This works perfectly as long as the browser remains open, the timeout occurs as expected and the session is ended.

The problem comes when the user closes the browser before the timeout happens, in which case the session is now stuck and cannot be ended. The account quickly becomes locked without any access possible. Is there a way, or is automated logout supposed to, log-out the session even after the browser is closed?

Any ideas about this would be greatly appreciated. Thanks again!
- Riki

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Liam McDermott’s picture

Category: support » feature
Status: Active » Needs review
FileSize
2.27 KB

Here's a first attempt at a patch. While it doesn't delete the session when they close the browser window, it does clean up sessions that should have expired when cron is run.

One thing to bear in mind is cron is using a different $_SESSION to the user, I would have liked to have loaded up the session and checked the autologout_last, but had to use timestamp from {sessions} instead, because:

1. There's no simple way of unserialising session data;
2. Switching between sessions using session_id() is ... tweaky;
3. Even if either of the above worked it can still be horribly broken if the user has the Suhosin hardened PHP patch applied and session data encrypted.

Liam McDermott’s picture

That last patch had a bug, I used the $result variable twice in two nested loops. Here's a better one.

RikiB’s picture

Status: Needs review » Reviewed & tested by the community

#2 works perfectly. Thanks Liam!

RikiB’s picture

Status: Reviewed & tested by the community » Needs work

After more tests on multiple computers, the sessions are still not being completely removed during cron. Another approach may be needed.

Liam McDermott’s picture

Status: Needs work » Needs review
FileSize
2.33 KB

Here's a new patch with the cron problem fixed.

johnennew’s picture

Version: 6.x-4.x-dev » 7.x-4.x-dev
Status: Needs review » Needs work

I've committed the patch in #5 to 6.x-4.x branch as it looks like it's working great.

Changing to 7.x-4.x and needs work so this can be added to the 7.x-4.x branch if needed.

johnennew’s picture

Status: Needs work » Needs review
FileSize
1.24 KB

I have a (possibly) cleaner solution to this problem and attach a patch for 7.x-4.x branch.

The current solution is to clean stale sessions up via cron which can be quite costly from cron and also means that you have to wait for cron to have worked before the session_limit module knows a user has a free session.

Instead, this patch cleans stale sessions at login. My initial testing shows this to be a valid solution but I'm going to write some automated tests to prove it in a cople of different situations.

I'd be interested in other's feedback on this approach.

johnennew’s picture

Priority: Major » Normal
FileSize
6.72 KB

Same patch with tests showing stale sessions cleaned up at login.

johnennew’s picture

Status: Needs review » Fixed

I like this approach so am committing to development 7.x-4.x branch. Will leave 6.x as it is for the time being.

Status: Fixed » Closed (fixed)

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

Anonymous’s picture

Issue summary: View changes

changed the wording a little

  • Commit 69a3dec on 7.x-4.x, 8.x-1.x by ceng:
    Issue #1315708 by ceng: Clean up stale sessions at login to improve...