diff --git a/lib/auth/auth.go b/lib/auth/auth.go index 21cd351b1172..fb9077460d61 100644 --- a/lib/auth/auth.go +++ b/lib/auth/auth.go @@ -3829,13 +3829,7 @@ func (a *Server) deleteMFADeviceSafely(ctx context.Context, user, deviceName str return nil, trace.Wrap(err) } - kindToSF := map[string]constants.SecondFactorType{ - fmt.Sprintf("%T", &types.MFADevice_Totp{}): constants.SecondFactorOTP, - fmt.Sprintf("%T", &types.MFADevice_U2F{}): constants.SecondFactorWebauthn, - fmt.Sprintf("%T", &types.MFADevice_Webauthn{}): constants.SecondFactorWebauthn, - } - sfToCount := make(map[constants.SecondFactorType]int) - var knownDevices int + knownDevices := make(map[constants.SecondFactorType]int) var deviceToDelete *types.MFADevice var numResidentKeys int @@ -3850,18 +3844,26 @@ func (a *Server) deleteMFADeviceSafely(ctx context.Context, user, deviceName str deviceToDelete = d } - sf, ok := kindToSF[fmt.Sprintf("%T", d.Device)] - switch { - case !ok && d == deviceToDelete: - return nil, trace.NotImplemented("cannot delete device of type %T", d.Device) - case !ok: + var sfType constants.SecondFactorType + switch d.Device.(type) { + case *types.MFADevice_Totp: + sfType = constants.SecondFactorOTP + case *types.MFADevice_U2F, *types.MFADevice_Webauthn: + sfType = constants.SecondFactorWebauthn + case *types.MFADevice_Sso: + // TODO(Joerger): sso will not be supported in the current `second_factor` option. + // It will be supported in the new `second_factors` option instead, which will not + // use the old constants.SecondFactorType type. This will be handled in a separate PR. + sfType = "sso" + if d == deviceToDelete { + return nil, trace.BadParameter("cannot delete ephemeral SSO MFA device") + } + default: log.Warnf("Ignoring unknown device with type %T in deletion.", d.Device) continue } - sfToCount[sf]++ - knownDevices++ - + knownDevices[sfType]++ if isResidentKey(d) { numResidentKeys++ } @@ -3875,12 +3877,12 @@ func (a *Server) deleteMFADeviceSafely(ctx context.Context, user, deviceName str switch sf := readOnlyAuthPref.GetSecondFactor(); sf { case constants.SecondFactorOff, constants.SecondFactorOptional: // MFA is not required, allow deletion case constants.SecondFactorOn: - if knownDevices <= minDevices { + if len(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 knownDevices[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) } @@ -3909,7 +3911,7 @@ func (a *Server) deleteMFADeviceSafely(ctx context.Context, user, deviceName str // Minimum number of WebAuthn devices includes the passkey that we attempt // to delete, hence 2. - if sfToCount[constants.SecondFactorWebauthn] >= 2 { + if knownDevices[constants.SecondFactorWebauthn] >= 2 { return true, nil } @@ -3917,7 +3919,7 @@ func (a *Server) deleteMFADeviceSafely(ctx context.Context, user, deviceName str // enabled. switch sf := readOnlyAuthPref.GetSecondFactor(); sf { case constants.SecondFactorOTP, constants.SecondFactorOn, constants.SecondFactorOptional: - if sfToCount[constants.SecondFactorOTP] >= 1 { + if knownDevices[constants.SecondFactorOTP] >= 1 { return true, nil } } diff --git a/lib/auth/grpcserver_test.go b/lib/auth/grpcserver_test.go index 36ec662c2588..27c754fc6f42 100644 --- a/lib/auth/grpcserver_test.go +++ b/lib/auth/grpcserver_test.go @@ -295,7 +295,7 @@ func TestMFADeviceManagement(t *testing.T) { // 2nd-to-last resident credential. // This is already tested above so we just use RegisterTestDevice here. const pwdless2DevName = "pwdless2" - _, err = RegisterTestDevice(ctx, userClient, pwdless2DevName, proto.DeviceType_DEVICE_TYPE_WEBAUTHN, devs.WebDev, WithPasswordless()) + pwdlessDev, err := RegisterTestDevice(ctx, userClient, pwdless2DevName, proto.DeviceType_DEVICE_TYPE_WEBAUTHN, devs.WebDev, WithPasswordless()) require.NoError(t, err, "RegisterTestDevice failed") // Check that all new devices are registered. @@ -433,6 +433,51 @@ func TestMFADeviceManagement(t *testing.T) { resp, err = userClient.GetMFADevices(ctx, &proto.GetMFADevicesRequest{}) require.NoError(t, err) require.Equal(t, "pwdless2", resp.Devices[0].GetName()) + + // Change the user to an SSO user with an MFA enabled auth connector. + samlConnector, err := types.NewSAMLConnector("saml", types.SAMLConnectorSpecV2{ + AssertionConsumerService: "http://localhost:65535/acs", // not called + Issuer: "test", + SSO: "https://localhost:65535/sso", // not called + AttributesToRoles: []types.AttributeMapping{ + // not used. can be any name, value but role must exist + {Name: "groups", Value: "admin", Roles: user.GetRoles()}, + }, + MFASettings: &types.SAMLConnectorMFASettings{ + Enabled: true, + }, + }) + require.NoError(t, err) + _, err = authServer.UpsertSAMLConnector(ctx, samlConnector) + require.NoError(t, err) + user.SetCreatedBy(types.CreatedBy{ + Time: authServer.clock.Now(), + Connector: &types.ConnectorRef{ + ID: "saml", + Type: "saml", + }, + }) + _, err = authServer.UpsertUser(ctx, user) + require.NoError(t, err) + + // Ephemeral sso device should show up in the list now. It can't be deleted. + resp, err = userClient.GetMFADevices(ctx, &proto.GetMFADevicesRequest{}) + require.NoError(t, err) + require.Len(t, resp.Devices, 2) + + testDeleteMFADevice(ctx, t, userClient, mfaDeleteTestOpts{ + deviceName: "saml", + authHandler: func(t *testing.T, challenge *proto.MFAAuthenticateChallenge) *proto.MFAAuthenticateResponse { + require.NotNil(t, challenge.WebauthnChallenge, "nil Webauthn challenge") + mfaResp, err := pwdlessDev.SolveAuthn(challenge) + require.NoError(t, err, "SolveAuthn") + return mfaResp + }, + checkErr: func(t require.TestingT, err error, _ ...interface{}) { + require.ErrorAs(t, err, new(*trace.BadParameterError)) + require.ErrorContains(t, err, "cannot delete ephemeral SSO MFA device") + }}, + ) } func TestDeletingLastPasswordlessDevice(t *testing.T) {