Prevent deleting last passwordless device with no password. (#47592)

gravitational · Oct 16, 2024 · 98c11b6 · 98c11b6
1 parent 871dc08
commit 98c11b6
Show file tree

Hide file tree

Showing 2 changed files with 25 additions and 23 deletions.
diff --git a/lib/auth/auth.go b/lib/auth/auth.go
@@ -3888,11 +3888,11 @@ func (a *Server) deleteMFADeviceSafely(ctx context.Context, user, deviceName str
 	// Check whether the device to delete is the last passwordless device,
 	// 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
 	// is not set and passwordless is disabled. Prevent them from deleting
 	// their last passkey to prevent them from being locked out further,
 	// in the case of passwordless being re-enabled.
-	if isPasskey(deviceToDelete) && remainingPasskeys == 0 && readOnlyAuthPref.GetAllowPasswordless() {
+	if isPasskey(deviceToDelete) && remainingPasskeys == 0 {
 		u, err := a.Services.GetUser(ctx, user, false /* withSecrets */)
 		if err != nil {
 			return nil, trace.Wrap(err)

diff --git a/lib/auth/grpcserver_test.go b/lib/auth/grpcserver_test.go
@@ -464,27 +464,6 @@ func TestDeletingLastPasswordlessDevice(t *testing.T) {
 				)
 			},
 		},
-		{
-			// TODO(Joerger): the user may already be locked out from login if a password
-			// is not set and passwordless is disabled. Prevent them from deleting
-			// their last passkey to prevent them from being locked out further,
-			// in the case of passwordless being re-enabled.
-			name: "OK other MFAs, but no password set, passwordless is off",
-			setup: func(t *testing.T, _ string, userClient *authclient.Client, pwdlessDev *TestDevice) {
-				// Register a non-passwordless device without adding a password.
-				_, err := RegisterTestDevice(ctx, userClient, "another-dev", proto.DeviceType_DEVICE_TYPE_TOTP, pwdlessDev, WithTestDeviceClock(clock))
-				require.NoError(t, err, "RegisterTestDevice")
-
-				authPref, err := authServer.GetAuthPreference(ctx)
-				require.NoError(t, err, "GetAuthPreference")
-
-				// Turn off passwordless authentication.
-				authPref.SetAllowPasswordless(false)
-				_, err = authServer.UpsertAuthPreference(ctx, authPref)
-				require.NoError(t, err, "UpsertAuthPreference")
-			},
-			checkErr: require.NoError,
-		},
 		{
 			name: "OK extra passwordless device",
 			setup: func(t *testing.T, username string, userClient *authclient.Client, pwdlessDev *TestDevice) {
@@ -560,6 +539,29 @@ func TestDeletingLastPasswordlessDevice(t *testing.T) {
 				)
 			},
 		},
+		{
+			name: "NOK other MFAs, but no password set, passwordless is off",
+			setup: func(t *testing.T, _ string, userClient *authclient.Client, pwdlessDev *TestDevice) {
+				// Register a non-passwordless device without adding a password.
+				_, err := RegisterTestDevice(ctx, userClient, "another-dev", proto.DeviceType_DEVICE_TYPE_TOTP, pwdlessDev, WithTestDeviceClock(clock))
+				require.NoError(t, err, "RegisterTestDevice")
+
+				authPref, err := authServer.GetAuthPreference(ctx)
+				require.NoError(t, err, "GetAuthPreference")
+
+				// Turn off passwordless authentication.
+				authPref.SetAllowPasswordless(false)
+				_, err = authServer.UpsertAuthPreference(ctx, authPref)
+				require.NoError(t, err, "UpsertAuthPreference")
+			},
+			checkErr: func(t require.TestingT, err error, _ ...any) {
+				require.ErrorContains(t,
+					err,
+					"cannot delete last passwordless credential for user",
+					"Unexpected error deleting last passwordless device",
+				)
+			},
+		},
 	}
 
 	for i, test := range tests {