kobotoolbox / kpi

kpi is the (frontend) server for KoboToolbox. It includes an API for users to access data and manage their forms, question library, sharing settings, create reports, and export data.
https://www.kobotoolbox.org
GNU Affero General Public License v3.0
131 stars 176 forks source link

Log out user after updating password #2258

Closed tinok closed 4 years ago

tinok commented 5 years ago

User story

  1. As a survey admin, I want to make sure anyone else using my account is logged out as soon as I change my account password in order to protect collected data.
  2. I would also like to sign out all other users even if I don't change my password (similar to Gmail feature)

UX flow

  1. Log in to User1 on computer 1 and computer 2 (or different browsers)
  2. Go to profile, modify password
  3. User on computer 2 should be redirected to login page (as soon as next request is made).

Relevant settings file: https://github.com/kobotoolbox/kpi/blob/7b5a1e708d5436d6d6348eb9066ae223507fc4ef/kobo/settings/base.py#L112-L114

Background

If a user changes their password of the account (while being logged in), anyone logged in with the same account continues being able to use the account.

jnm commented 5 years ago

@magicznyleszek, I don't think we need to log out the current session (despite the issue title). I think the point is to log out other sessions, which will have to happen on the backend. In practice, maybe it will be more practical to log out all sessions, but I think the goal is to deal with this situation:

  1. I gave you my password for some reason—maybe I needed to grant you temporary access to my account;
  2. I decide you don't need access to my account anymore and change my password;
  3. I think that you're now locked out of my account;
  4. In reality, any browser session where you've logged into my account still has full access, and there's nothing I can do about it except wait for you to log out.

IMO PR #2264 isn't necessary (sorry!)

magicznyleszek commented 5 years ago

@jnm oh, ok - I've closed the PR :)

tinok commented 5 years ago

Just an update on this feature idea - we are hoping to build this by Q4 2019.

jnm commented 4 years ago

@noliveleger, were you uncommenting this SessionAuthenticationMiddleware in #2460? I thought I saw that change somewhere but can't find it in the diff now.

noliveleger commented 4 years ago

@jnm I never uncommented it. I removed other Middleware classes.

noliveleger commented 4 years ago

Changed in Django 1.10: Session verification is enabled and mandatory in Django 1.10 (there’s no way to disable it) regardless of whether or not SessionAuthenticationMiddleware is enabled. In older versions, this protection only applies if django.contrib.auth.middleware.SessionAuthenticationMiddleware is enabled in MIDDLEWARE.

tinok commented 4 years ago

@noliveleger Can this issue be closed now?

noliveleger commented 4 years ago

@tinok, no, until it's not reviewed.

jnm commented 4 years ago

2595 remains, but we can close this.