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

Allow deleting last passkey in certain circumstances #41727

Merged
merged 4 commits into from
May 20, 2024

Conversation

bl-nero
Copy link
Contributor

@bl-nero bl-nero commented May 17, 2024

This PR allows deleting the last passkey if the following conditions are true:

  • The user is known to have a password set.
  • The user has at least 1 additional MFA method, and this method can be used to log in on this cluster.

Changelog: Allowed deleting user's last passkey if they have a password and another MFA set up.

Closes #38433

Tested manually:

  • Attempt to delete with password enabled, no other MFA
  • Attempt to delete with other MFA, but no password
  • Attempt to delete with other MFA, unknown password state
  • Delete with password and YubiKey
  • Delete with password and OTP

Copy link
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!

We couldn't do better at the time, but we certainly can now. I got a couple of complaints over this one, so it's definitely good to see it relaxed.

Could you add the appropriate backport labels, please?

lib/auth/auth.go Outdated Show resolved Hide resolved
lib/auth/auth.go Show resolved Hide resolved
authPref, err = authServer.UpsertAuthPreference(ctx, authPref)
assert.NoError(t, err, "Resetting AuthPreference")
}()
func TestDeletingLastPasswordlessDevice(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
func TestDeletingLastPasswordlessDevice(t *testing.T) {
func TestDeletingLastPasswordlessDevice(t *testing.T) {
t.Parallel()

lib/auth/grpcserver_test.go Show resolved Hide resolved
lib/auth/grpcserver_test.go Outdated Show resolved Hide resolved
@bl-nero
Copy link
Contributor Author

bl-nero commented May 20, 2024

Could you add the appropriate backport labels, please?

Added v15; that's as early as we can go here, since it depends on #40911.

@codingllama
Copy link
Contributor

Could you add the appropriate backport labels, please?

Added v15; that's as early as we can go here, since it depends on #40911.

Sounds good, thanks!

@bl-nero
Copy link
Contributor Author

bl-nero commented May 20, 2024

Retested, will merge once tests pass

@bl-nero bl-nero enabled auto-merge May 20, 2024 15:32
@bl-nero bl-nero added this pull request to the merge queue May 20, 2024
Merged via the queue into master with commit b32ec15 May 20, 2024
37 checks passed
@bl-nero bl-nero deleted the bl-nero/allow-deleting-last-passkey branch May 20, 2024 16:40
@public-teleport-github-review-bot

@bl-nero See the table below for backport results.

Branch Result
branch/v15 Create PR

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.

Don't prevent deleting last passwordless key if we are certain that the user has a password
3 participants