Skip to content

Prevent deleting last passwordless device with no password#47592

Merged
Joerger merged 1 commit intomasterfrom
joerger/prevent-transitory-passwordless-lockout
Oct 16, 2024
Merged

Prevent deleting last passwordless device with no password#47592
Joerger merged 1 commit intomasterfrom
joerger/prevent-transitory-passwordless-lockout

Conversation

@Joerger
Copy link
Copy Markdown
Contributor

@Joerger Joerger commented Oct 15, 2024

Prevent users from deleting their last passwordless device if they have no other means of logging in, even if passwordless is currently disabled. This way, if passwordless is re-enabled, the user will be able to log in.

I found this small logic edge case while creating #47426 and figured it may as well be covered.

Honestly, after giving this more thought and seeing the change on its merit, this may be a distinction without a difference. In practice the user will probably need to be reset anyways, except for the hard to imagine edge case:

  1. admin temporarily disabled passwordless login
  2. the user has an active login session
  3. the user deletes their passkey in, due to confusion or otherwise
  4. admin re-enables passworldess, user is now locked out, which could have been avoided

Changelog: Update logic around deleting last passkey to prevent edge case lockouts.

Copy link
Copy Markdown
Contributor

@codingllama codingllama left a comment

Choose a reason for hiding this comment

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

Thanks for the small, focused PR.

Comment thread lib/auth/auth.go
// and whether deleting it would lockout the user from login.
//
// TODO(Joerger): the user may already be locked out from login if a password
// Note: the user may already be locked out from login if a password
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

As you said, it's a pretty weird edge case, but I think I'm OK with the if essentially saying "is last passkey && has no password".

@codingllama
Copy link
Copy Markdown
Contributor

Note: depends on #47426. Please wait for the base PR to be merged before merging this one (naturally).

Base automatically changed from joerger/use-second-factors to master October 16, 2024 19:40
@Joerger Joerger force-pushed the joerger/prevent-transitory-passwordless-lockout branch from 3c36234 to 4baf9f6 Compare October 16, 2024 21:54
@Joerger Joerger enabled auto-merge October 16, 2024 21:54
@Joerger Joerger added this pull request to the merge queue Oct 16, 2024
Merged via the queue into master with commit 98c11b6 Oct 16, 2024
@Joerger Joerger deleted the joerger/prevent-transitory-passwordless-lockout branch October 16, 2024 22:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants