Skip to content

session_security breaks "End Session" on the current session when using user_sessions #65

Open

Description

@sdann

The middleware

session_security.middleware.SessionSecurityMiddleware

Provides a session last_activity update on each http request, to decrease chance of session logout.

When using the user_sessions middleware alone, clicking "End Session" will behave the same way as "Logout". Unfortunately, when combined with the session_security middleware, clicking "End Session" on the current session has no effect.

With some pdb tracing, I've figured out the following rough series of events:

  1. request:session_security updates the last_activity in the session
  2. request:user_sessions deletes the session object
  3. response:user_sessions detects the session was modified in step 1) and re-saves the session to the backing DB

The user is redirected to the same Session List page, with their current session still active.

I've opened an issue in session_security and have a proposed fix for that code base, which will add a configuration change that disables the last_activity update on the session_delete view.

yourlabs/django-session-security#89

Tracking here to update user_sessions documentation if/when my proposed fix is accepted.

Activity

  1. Bouke commented on Jan 5, 2017

    @Bouke
    Collaborator

    That's quite a debug session in order to find that! I've made a minor change in 317e337 to logout a user when he's trying to delete his current session. This should set the session_key to None (request.session.flush()), which prevents the session being re-written to db.

    However this might also point to another issue. What if a user ends all other sessions, and the above happens? Will there still be a new session object written to db in step 3? If so, that could be quite major issue.

  2. sdann commented on Jan 6, 2017

    @sdann
    Author

    @Bouke Yeah, it took a while stepping through code. Your change looks like it should fix the issue.

    In my testing, there's no conflict between the two middleware modules when deleting other sessions, because the session_security module only updates the last_activity on the current session. So deleting other sessions works as expected.

  3. Bouke commented on Jan 17, 2017

    @Bouke
    Collaborator

    I'm thinking along these lines;

    1. Session A clicks "end all other sessions"
    2. Session B clicks "heavy page" (a page that requires a while to process)
    3. Server starts processing both A and B (multi-threaded / -process server)
    4. Server completes request for A, removing all sessions except A.
    5. Server completes request B, however the session was modified, so this triggers the middleware to update the session in the database
    6. Will it realise that there is no session and accept no further requests? Or will it store the session as a new record in the database, effectively by-passing the forced log-out?

    Maybe this could be reproduced by artificially introducing a delay in a view (time.sleep(10)) and would also require a multi-threaded server.

  4. sdann commented on Jan 19, 2017

    @sdann
    Author

    Yeah, that sounds like a possible race in this scenario. The race exists even deleting a single session, if there's another ongoing request for that session, and session_security middleware is in use.

    You'd need to mark each session as "dying" in the DB, so as to fail the save() on request B. Though, in practice this won't occur often, and is detectable by the user refreshing the /account/sessions/ page.

  5. Lionqueen94 commented on Dec 6, 2018

    @Lionqueen94

    Hi there, this is an old bug, but I noticed this behaviour today.
    When ending all other sessions and then your current session, this error is returned:

    TypeError at /account/sessions/svxqrcy1ofr0yiyyoa8hukee6elmu6pt/delete/
    argument of type 'NoneType' is not iterable
    

    I also use the combination of session_security and user_sessions.
    Somehow the logout is not triggered but the session gets removed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions