Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
sdann opened this issue Jan 4, 2017 · 6 comments

Comments

@sdann
Copy link
Contributor

sdann commented Jan 4, 2017

The middleware

user_sessions.middleware.SessionMiddleware

Provides a listing of a user's sessions across various clients. The user can then choose to "End Session" on any of their existing sessions, including their current session they are logged in as.

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.

@sdann
Copy link
Contributor Author

sdann commented Jan 4, 2017

As a fix, I propose enhancing the SESSION_SECURITY_PASSIVE_URLS parameter to understand URLs with parameterized arguments, instead of just static strings.

The URLs in this instance look like this:

http://127.0.0.1:8000/account/sessions/5srnkbv9rczmud7v7zp0oi8t76b6fgwm/delete/

Where the 3rd entry in the path is the session ID. PR coming soon.

sdann pushed a commit to sdann/django-session-security that referenced this issue Jan 4, 2017
… session when using user_sessions

Add a new parameter, SESSION_SECURITY_PASSIVE_URL_NAMES, which takes a
list of URL names to skip when performing session activity updates.

This provides an easy method for parameterized URLs to be skipped. The
existing SESSION_SECURITY_PASSIVE_URLS parameter continues to work
with static path names.

For example, if you have the following URL from the user_sessions app:

/account/sessions/<session_id>/delete/

You can skip the activity tracker by adding the following to
settings.py:

SESSION_SECURITY_PASSIVE_URL_NAMES = ['session_delete']

This currently only handles direct URL names and can be updated in the
future to handled fully namespaced and instanced URL names.
@sdann
Copy link
Contributor Author

sdann commented Jan 13, 2017

The original issue for which I submitted this patch has been fixed in the user_sessions module.

However, I think this is still the better way to do PASSIVE URLS (DRY).

@jpic
Copy link
Member

jpic commented Jan 14, 2017

Hi ! Thanks a lot for your contribution ! Would this work for you ?

from django.core.urlresolvers import reverse_lazy as reverse
SESSION_SECURITY_PASSIVE_URLS = [reverse('session_delete')]

@sdann
Copy link
Contributor Author

sdann commented Jan 19, 2017

Hey @jpic

I don't think reverse_lazy will work here, because the URL is parameterized with the session token.

http://127.0.0.1:8000/account/sessions/5srnkbv9rczmud7v7zp0oi8t76b6fgwm/delete/

There's no way for us to pass this dynamic portion of the URL (5srnkbv9rczmud7v7zp0oi8t76b6fgwm) into the static SESSION_SECURITY_PASSIVE_URLS parameter in the existing code.

A quick attempt returns a NoReverseMatch exception.

@jpic
Copy link
Member

jpic commented Jan 19, 2017

Hi @sdann

Sounds fair, I think we'd like to merge this, care to add a unit tests or two and a bit of doc for the new setting ?

Thanks

@sdann
Copy link
Contributor Author

sdann commented Jan 31, 2017

Indeed. Was on vacation last week. I'll try to add some testing and docs to the PR by end of the week.

sdann pushed a commit to sdann/django-session-security that referenced this issue Feb 7, 2017
Fix yourlabs#89: session_security breaks "End Session"
sdann pushed a commit to sdann/django-session-security that referenced this issue Feb 7, 2017
Fix yourlabs#89: session_security breaks "End Session"
@jpic jpic closed this as completed in 7345638 Feb 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants