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

Destroy existing sessions when activating 2FA. #578

Merged
merged 4 commits into from
Sep 4, 2023

Conversation

dd32
Copy link
Member

@dd32 dd32 commented Jun 7, 2023

What?

Upon a new 2FA provider being set, existing sessions for the user are destroyed.

Fixes #577

Why?

It's considered best-practice to terminate existing sessions when "enhancing" an authenticated session.

How?

For the current user, the active session is kept, other sessions are destroyed upon 2FA being activated.
For other users, all their sessions are terminated.

Testing Instructions

See #577

Screenshots or screencast

Changelog Entry

Security - When 2FA is activated for a user, destroy any existing sessions.

@dd32 dd32 added this to the 0.9.0 milestone Jun 7, 2023
@dd32 dd32 self-assigned this Jun 7, 2023
@dd32 dd32 modified the milestones: 0.9.0, 0.8.2 Jun 7, 2023
Copy link
Member

@iandunn iandunn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like disabling a provider should be covered too, if others remain active. LMK if you had a reason for not doing that though.

class-two-factor-core.php Outdated Show resolved Hide resolved
class-two-factor-core.php Outdated Show resolved Hide resolved
dd32 and others added 3 commits July 14, 2023 15:10
Sessions are destroyed when:
 - No providers -> Some providers
 - Some providers -> Less providers
Copy link
Member

@iandunn iandunn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good 👍🏻

@jeffpaul
Copy link
Member

jeffpaul commented Aug 7, 2023

@dd32 seems as this may be good to merge and unblock the 0.8.2 release, yeah?

@iandunn iandunn merged commit 237cc2c into master Sep 4, 2023
2 checks passed
@iandunn iandunn mentioned this pull request Sep 4, 2023
14 tasks
@iandunn iandunn modified the milestones: 0.8.2, 0.9.0 Sep 4, 2023
@kasparsd kasparsd deleted the fix/577-destroy-sessions-on-enable branch April 23, 2024 09:06
@kasparsd kasparsd mentioned this pull request Apr 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Previously created sessions continue being valid after initial 2FA activation
3 participants