diff --git a/lib/auth/auth.go b/lib/auth/auth.go index 3e6237b773f2a..b48189ba022ed 100644 --- a/lib/auth/auth.go +++ b/lib/auth/auth.go @@ -2768,16 +2768,16 @@ func (a *Server) deleteMFADeviceSafely(ctx context.Context, user, deviceName str } // Prevent users from deleting their last device for clusters that require second factors. - const minDevices = 2 + const minDevices = 1 switch sf := authPref.GetSecondFactor(); sf { case constants.SecondFactorOff, constants.SecondFactorOptional: // MFA is not required, allow deletion case constants.SecondFactorOn: - if knownDevices < minDevices { + if knownDevices <= minDevices { return nil, trace.BadParameter( "cannot delete the last MFA device for this user; add a replacement device first to avoid getting locked out") } case constants.SecondFactorOTP, constants.SecondFactorWebauthn: - if sfToCount[sf] < minDevices { + if sfToCount[sf] <= minDevices { return nil, trace.BadParameter( "cannot delete the last %s device for this user; add a replacement device first to avoid getting locked out", sf) } diff --git a/lib/auth/auth_test.go b/lib/auth/auth_test.go index 7bdf091b22788..b09c275c69885 100644 --- a/lib/auth/auth_test.go +++ b/lib/auth/auth_test.go @@ -2091,13 +2091,16 @@ func TestNewWebSession(t *testing.T) { func TestDeleteMFADeviceSync(t *testing.T) { t.Parallel() - srv := newTestTLSServer(t) - ctx := context.Background() + + testServer := newTestTLSServer(t) + authServer := testServer.Auth() mockEmitter := &eventstest.MockEmitter{} - srv.Auth().emitter = mockEmitter + authServer.emitter = mockEmitter + + ctx := context.Background() username := "llama@goteleport.com" - _, _, err := CreateUserAndRole(srv.Auth(), username, []string{username}, nil) + _, _, err := CreateUserAndRole(authServer, username, []string{username}, nil /* allowRules */) require.NoError(t, err) authPreference, err := types.NewAuthPreference(types.AuthPreferenceSpecV2{ @@ -2108,73 +2111,80 @@ func TestDeleteMFADeviceSync(t *testing.T) { }, }) require.NoError(t, err) - err = srv.Auth().SetAuthPreference(ctx, authPreference) + err = authServer.SetAuthPreference(ctx, authPreference) require.NoError(t, err) - clt, err := srv.NewClient(TestUser(username)) + clt, err := testServer.NewClient(TestUser(username)) require.NoError(t, err) // Insert dummy devices. webDev1, err := RegisterTestDevice(ctx, clt, "web-1", proto.DeviceType_DEVICE_TYPE_WEBAUTHN, nil /* authenticator */) require.NoError(t, err) - webDev2, err := RegisterTestDevice(ctx, clt, "web-2", proto.DeviceType_DEVICE_TYPE_WEBAUTHN, webDev1) + _, err = RegisterTestDevice(ctx, clt, "web-2", proto.DeviceType_DEVICE_TYPE_WEBAUTHN, webDev1) require.NoError(t, err) - totpDev1, err := RegisterTestDevice(ctx, clt, "otp-1", proto.DeviceType_DEVICE_TYPE_TOTP, webDev1, WithTestDeviceClock(srv.Clock())) + totpDev1, err := RegisterTestDevice(ctx, clt, "otp-1", proto.DeviceType_DEVICE_TYPE_TOTP, webDev1, WithTestDeviceClock(testServer.Clock())) require.NoError(t, err) - totpDev2, err := RegisterTestDevice(ctx, clt, "otp-2", proto.DeviceType_DEVICE_TYPE_TOTP, webDev1, WithTestDeviceClock(srv.Clock())) + _, err = RegisterTestDevice(ctx, clt, "otp-2", proto.DeviceType_DEVICE_TYPE_TOTP, webDev1, WithTestDeviceClock(testServer.Clock())) require.NoError(t, err) tests := []struct { name string - deviceToDelete string tokenReq CreateUserTokenRequest + deviceToDelete string }{ { - name: "recovery approved token", - deviceToDelete: webDev1.MFA.GetName(), + name: "recovery approved token", tokenReq: CreateUserTokenRequest{ Name: username, TTL: 5 * time.Minute, Type: UserTokenTypeRecoveryApproved, }, + deviceToDelete: webDev1.MFA.GetName(), }, { - name: "privilege token", - deviceToDelete: totpDev1.MFA.GetName(), + name: "privilege token", tokenReq: CreateUserTokenRequest{ Name: username, TTL: 5 * time.Minute, Type: UserTokenTypePrivilege, }, + deviceToDelete: totpDev1.MFA.GetName(), }, } - - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - token, err := srv.Auth().newUserToken(tc.tokenReq) + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + token, err := authServer.newUserToken(test.tokenReq) require.NoError(t, err) - _, err = srv.Auth().CreateUserToken(ctx, token) + _, err = authServer.CreateUserToken(ctx, token) require.NoError(t, err) - // Delete the TOTP device. - err = srv.Auth().DeleteMFADeviceSync(ctx, &proto.DeleteMFADeviceSyncRequest{ + // Delete the device. + mockEmitter.Reset() + err = authServer.DeleteMFADeviceSync(ctx, &proto.DeleteMFADeviceSyncRequest{ TokenID: token.GetName(), - DeviceName: tc.deviceToDelete, + DeviceName: test.deviceToDelete, }) - require.NoError(t, err) + require.NoError(t, err, "DeleteMFADeviceSync failed") + + // Verify device deletion. + devs, err := authServer.Services.GetMFADevices(ctx, username, false /* withSecrets */) + require.NoError(t, err, "GetMFADevices failed") + for _, dev := range devs { + if dev.GetName() == test.deviceToDelete { + t.Errorf("DeleteMFADeviceSync(%q): device not deleted", test.deviceToDelete) + return + } + } + + // Verify deletion event. + event := mockEmitter.LastEvent() + assert.Equal(t, events.MFADeviceDeleteEvent, event.GetType(), "event.Type") + assert.Equal(t, events.MFADeviceDeleteEventCode, event.GetCode(), "event.Code") + require.IsType(t, &apievents.MFADeviceDelete{}, event, "underlying event type") + deleteEvent := event.(*apievents.MFADeviceDelete) // asserted above + assert.Equal(t, username, deleteEvent.User, "event.User") }) } - - // Check it's been deleted. - devs, err := srv.Auth().Services.GetMFADevices(ctx, username, false) - require.NoError(t, err) - compareDevices(t, false /* ignoreUpdateAndCounter */, devs, webDev2.MFA, totpDev2.MFA) - - // Test last events emitted. - event := mockEmitter.LastEvent() - require.Equal(t, events.MFADeviceDeleteEvent, event.GetType()) - require.Equal(t, events.MFADeviceDeleteEventCode, event.GetCode()) - require.Equal(t, event.(*apievents.MFADeviceDelete).UserMetadata.User, username) } func TestDeleteMFADeviceSync_WithErrors(t *testing.T) {