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

Changing security config should cancel existing "remember me" tokens #52

Open
chillu opened this issue Nov 28, 2018 · 7 comments
Open

Comments

@chillu
Copy link

chillu commented Nov 28, 2018

Acceptance Criteria

  • All "remember me" tokens are expired when a password is changed or reset (retaining the current browser session)
  • All "remember me" tokens are expired when a second factor is added or removed (retaining the current browser session)
  • If the system is configured for multiple "remember me" tokens, they're all expired

Notes

  • Excludes clearing current PHP session or other active PHP sessions for the same user (since we can't read/write session data across sessions in PHP). Unless I'm missing something here?

From Olivier:

Changing security configuration (like adding/removing 2nd factor or changing/resetting password) should result in cancelling all existing sessions. rational: users change their password because they think it might have been compromised.

@chillu chillu changed the title Changing security config should cancel existing sessions Changing security config should cancel existing Nov 28, 2018
@chillu chillu changed the title Changing security config should cancel existing Changing security config should cancel existing "remember me" tokens Nov 28, 2018
@Firesphere
Copy link
Owner

Should this be in core as well?

@ScopeyNZ
Copy link
Collaborator

ScopeyNZ commented Dec 3, 2018

I think it probably should. I noted on Slack but this isn't necessarily a common flow. If there's the ability to log yourself out on other devices (a button labelled "log me out elsewhere" or words to that effect) then services don't always log you out of all your devices when changing your password.

Might need to have a little discussion about what suits SilverStripe best...

@robbieaverill
Copy link
Collaborator

@chillu are you happy for us to move this to a framework issue?

@robbieaverill
Copy link
Collaborator

Friendly bump @chillu

@robbieaverill
Copy link
Collaborator

@Firesphere
Copy link
Owner

I believe some work still needs to be done here. Without any extension hook or way to hook in to the reset method, the framework doesn't explicitly know the MFA has been reset, thus no reset will happen?

@Firesphere Firesphere reopened this Jan 7, 2019
@robbieaverill
Copy link
Collaborator

Yeah assuming silverstripe/silverstripe-framework#8694 gets implemented, it'd probably have an extension hook in it somewhere. Fair enough for reopening, let's re-target this issue as "ensure MFA changes are included in resetting 'remember me' cookies assuming silverstripe/silverstripe-framework#8694 is implemented" or something to that end

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

No branches or pull requests

4 participants