If a user leave's a session open on a machine that is compromised or has the session sniffed in some way they might change their password (its common advice if your account is compromised). To make that password change more effective, we should log out all sessions when that happens.

CommentFileSizeAuthor
#1 2294061_paranoia_remove_other_sessions.patch1.29 KBgreggles

Comments

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented
Status: Active » Needs review
StatusFileSize
new 2294061_paranoia_remove_other_sessions.patch1.29 KB

OK, here's a patch that needs review.

pere orga’s picture

pere orga
Primary language Catalan
commented

I like that idea.

Wouldn't be even better if the current session is logged out as well? For the cases the session id has been stolen.

scor’s picture

scor commented

@Pere Orga I believe core will regenerate a session ID after you change your password (needs to be verified).

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented

@Pere - I thought about that too. It seems like a ux annoyance to be logged out when you change your password and since you *just* set your password it's clear that this session should be able to login again with the password that was just set, so I didn't try to do that. I think scor is right that regenerating the session but keeping the user logged in achieves the same goal. I just confirmed that's how core works :)

  • greggles committed 9cd7c3d on 7.x-1.x 
    Issue #2294061 by greggles: Remove all sessions for a uid if password is...
greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented
Status: Needs review » Fixed

Thanks for the feedback, scor/pere. Now committed :)

Status: Fixed » Closed (fixed)

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

znerol’s picture

znerol commented

Coming here from #2508637: Changing password should invalidate all other sessions. This patch seems to replicate behavior which is already implemented in D7, see user.module around line 544. I've tested this on a fresh D7 install (sans Paranoia) and changing the password in one browser kills all sessions for the same user in other browsers.

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented
Title: Remove all sessions for a uid if password is changed » Remove code to Remove all sessions for a uid if password is changed
Status: Closed (fixed) » Active

Wow, I could swear I tested this. Maybe it has to do with Bakery cookies?

@Pere/@Scor - did either of you test it out?

nedjo’s picture

nedjo
he/him/his
Primary language English
commented

In Drupal 8 this appears to be covered in User::postSave().

drumm’s picture

drumm
he/him
Location NY, US
commented

Yes, bakery cookies used to over-regenerate sessions on Drupal.org.