diff --git a/api/types/authentication.go b/api/types/authentication.go index a23df475cc252..1df48aa74ccc0 100644 --- a/api/types/authentication.go +++ b/api/types/authentication.go @@ -71,9 +71,8 @@ type AuthPreference interface { // SetType sets the type of authentication: local, saml, or oidc. SetType(string) - // GetSecondFactor gets the type of second factor. - GetSecondFactor() constants.SecondFactorType // SetSecondFactor sets the type of second factor. + // Deprecated: only used in tests to set the deprecated off/optional values. SetSecondFactor(constants.SecondFactorType) // GetSecondFactors gets a list of supported second factors. GetSecondFactors() []SecondFactorType @@ -321,16 +320,6 @@ func (c *AuthPreferenceV2) SetType(s string) { c.Spec.Type = s } -// GetSecondFactor returns the type of second factor. -func (c *AuthPreferenceV2) GetSecondFactor() constants.SecondFactorType { - // SecondFactors takes priority if set. - if len(c.Spec.SecondFactors) > 0 { - return legacySecondFactorFromSecondFactors(c.Spec.SecondFactors) - } - - return c.Spec.SecondFactor -} - // SetSecondFactor sets the type of second factor. func (c *AuthPreferenceV2) SetSecondFactor(s constants.SecondFactorType) { c.Spec.SecondFactor = s @@ -863,7 +852,7 @@ func (c *AuthPreferenceV2) CheckAndSetDefaults() error { // String represents a human readable version of authentication settings. func (c *AuthPreferenceV2) String() string { - return fmt.Sprintf("AuthPreference(Type=%q,SecondFactor=%q)", c.Spec.Type, c.GetSecondFactor()) + return fmt.Sprintf("AuthPreference(Type=%q,SecondFactors=%q)", c.Spec.Type, c.GetSecondFactors()) } // Clone returns a copy of the AuthPreference resource. diff --git a/api/types/authentication_authpreference_test.go b/api/types/authentication_authpreference_test.go index a1e9b3e9b9261..b6efaae1946b1 100644 --- a/api/types/authentication_authpreference_test.go +++ b/api/types/authentication_authpreference_test.go @@ -153,18 +153,13 @@ func TestAuthPreferenceV2_CheckAndSetDefaults_secondFactor(t *testing.T) { }, }, assertFn: func(t *testing.T, got *types.AuthPreferenceV2) { - // Did we derive the WebAuthn config from U2F? - if sf := got.GetSecondFactor(); sf == constants.SecondFactorWebauthn || - sf == constants.SecondFactorOn || - sf == constants.SecondFactorOptional { - wantWeb := &types.Webauthn{ - RPID: "example.com", - AttestationAllowedCAs: []string{yubicoU2FCA}, - } - gotWeb, err := got.GetWebauthn() - require.NoError(t, err, "webauthn config not found") - require.Empty(t, cmp.Diff(wantWeb, gotWeb)) + wantWeb := &types.Webauthn{ + RPID: "example.com", + AttestationAllowedCAs: []string{yubicoU2FCA}, } + gotWeb, err := got.GetWebauthn() + require.NoError(t, err, "webauthn config not found") + require.Empty(t, cmp.Diff(wantWeb, gotWeb)) }, }, { diff --git a/api/types/second_factor.go b/api/types/second_factor.go index d876a35493c83..46c969bb4fd4a 100644 --- a/api/types/second_factor.go +++ b/api/types/second_factor.go @@ -25,30 +25,13 @@ import ( "github.com/gravitational/teleport/api/constants" ) -// legacySecondFactorFromSecondFactors returns a suitable legacy second factor for the given list of second factors. -func legacySecondFactorFromSecondFactors(secondFactors []SecondFactorType) constants.SecondFactorType { - hasOTP := slices.Contains(secondFactors, SecondFactorType_SECOND_FACTOR_TYPE_OTP) - hasWebAuthn := slices.Contains(secondFactors, SecondFactorType_SECOND_FACTOR_TYPE_WEBAUTHN) - - switch { - case hasOTP && hasWebAuthn: - return constants.SecondFactorOn - case hasWebAuthn: - return constants.SecondFactorWebauthn - case hasOTP: - return constants.SecondFactorOTP - default: - return constants.SecondFactorOff - } -} - // secondFactorsFromLegacySecondFactor returns the list of SecondFactorTypes supported by the given second factor type. func secondFactorsFromLegacySecondFactor(sf constants.SecondFactorType) []SecondFactorType { switch sf { case constants.SecondFactorOff: return nil case constants.SecondFactorOptional, constants.SecondFactorOn: - return []SecondFactorType{SecondFactorType_SECOND_FACTOR_TYPE_WEBAUTHN, SecondFactorType_SECOND_FACTOR_TYPE_OTP} + return []SecondFactorType{SecondFactorType_SECOND_FACTOR_TYPE_OTP, SecondFactorType_SECOND_FACTOR_TYPE_WEBAUTHN} case constants.SecondFactorOTP: return []SecondFactorType{SecondFactorType_SECOND_FACTOR_TYPE_OTP} case constants.SecondFactorWebauthn: @@ -58,9 +41,32 @@ func secondFactorsFromLegacySecondFactor(sf constants.SecondFactorType) []Second } } +// LegacySecondFactorFromSecondFactors returns a suitable legacy second factor for the given list of second factors. +func LegacySecondFactorFromSecondFactors(secondFactors []SecondFactorType) constants.SecondFactorType { + hasOTP := slices.Contains(secondFactors, SecondFactorType_SECOND_FACTOR_TYPE_OTP) + hasWebAuthn := slices.Contains(secondFactors, SecondFactorType_SECOND_FACTOR_TYPE_WEBAUTHN) + hasSSO := slices.Contains(secondFactors, SecondFactorType_SECOND_FACTOR_TYPE_SSO) + + switch { + case hasOTP && hasWebAuthn: + return constants.SecondFactorOn + case hasWebAuthn: + return constants.SecondFactorWebauthn + case hasOTP: + return constants.SecondFactorOTP + case hasSSO: + // In the WebUI, we can treat exclusive SSO MFA as disabled. In practice this means + // things like the "add MFA device" button is disabled, but SSO MFA prompts will still work. + // TODO(Joerger): Ensure that SSO MFA flows work in the WebUI with this change, once implemented. + return constants.SecondFactorOff + default: + return constants.SecondFactorOff + } +} + // MarshalJSON marshals SecondFactorType to string. func (s *SecondFactorType) MarshalYAML() (interface{}, error) { - val, err := s.encode() + val, err := s.Encode() if err != nil { return nil, trace.Wrap(err) } @@ -81,7 +87,7 @@ func (s *SecondFactorType) UnmarshalYAML(unmarshal func(interface{}) error) erro // MarshalJSON marshals SecondFactorType to string. func (s *SecondFactorType) MarshalJSON() ([]byte, error) { - val, err := s.encode() + val, err := s.Encode() if err != nil { return nil, trace.Wrap(err) } @@ -110,7 +116,8 @@ const ( secondFactorTypeSSOString = "sso" ) -func (s *SecondFactorType) encode() (string, error) { +// Encode encodes the SecondFactorType in string form. +func (s *SecondFactorType) Encode() (string, error) { switch *s { case SecondFactorType_SECOND_FACTOR_TYPE_UNSPECIFIED: return "", nil diff --git a/api/types/second_factor_test.go b/api/types/second_factor_test.go index 17ed27dc8b35d..63aa9867e6085 100644 --- a/api/types/second_factor_test.go +++ b/api/types/second_factor_test.go @@ -42,7 +42,7 @@ func TestEncodeDecodeSecondFactorType(t *testing.T) { } { t.Run(tt.secondFactorType.String(), func(t *testing.T) { t.Run("encode", func(t *testing.T) { - encoded, err := tt.secondFactorType.encode() + encoded, err := tt.secondFactorType.Encode() assert.NoError(t, err) assert.Equal(t, tt.encoded, encoded) }) @@ -57,7 +57,7 @@ func TestEncodeDecodeSecondFactorType(t *testing.T) { } } -func TestLegacySecondFactorsFromLegacySecondFactor(t *testing.T) { +func TestSecondFactorsFromLegacySecondFactor(t *testing.T) { for _, tt := range []struct { sf constants.SecondFactorType sfs []SecondFactorType @@ -72,11 +72,11 @@ func TestLegacySecondFactorsFromLegacySecondFactor(t *testing.T) { }, { sf: constants.SecondFactorOptional, - sfs: []SecondFactorType{SecondFactorType_SECOND_FACTOR_TYPE_WEBAUTHN, SecondFactorType_SECOND_FACTOR_TYPE_OTP}, + sfs: []SecondFactorType{SecondFactorType_SECOND_FACTOR_TYPE_OTP, SecondFactorType_SECOND_FACTOR_TYPE_WEBAUTHN}, }, { sf: constants.SecondFactorOn, - sfs: []SecondFactorType{SecondFactorType_SECOND_FACTOR_TYPE_WEBAUTHN, SecondFactorType_SECOND_FACTOR_TYPE_OTP}, + sfs: []SecondFactorType{SecondFactorType_SECOND_FACTOR_TYPE_OTP, SecondFactorType_SECOND_FACTOR_TYPE_WEBAUTHN}, }, { sf: constants.SecondFactorOTP, @@ -92,3 +92,43 @@ func TestLegacySecondFactorsFromLegacySecondFactor(t *testing.T) { }) } } + +func TestLegacySecondFactorFromSecondFactors(t *testing.T) { + for _, tt := range []struct { + sfs []SecondFactorType + sf constants.SecondFactorType + }{ + { + sfs: nil, + sf: constants.SecondFactorOff, + }, + { + sfs: []SecondFactorType{}, + sf: constants.SecondFactorOff, + }, + { + sfs: []SecondFactorType{SecondFactorType_SECOND_FACTOR_TYPE_OTP}, + sf: constants.SecondFactorOTP, + }, + { + sfs: []SecondFactorType{SecondFactorType_SECOND_FACTOR_TYPE_WEBAUTHN}, + sf: constants.SecondFactorWebauthn, + }, + { + sfs: []SecondFactorType{SecondFactorType_SECOND_FACTOR_TYPE_SSO}, + sf: constants.SecondFactorOff, + }, + { + sfs: []SecondFactorType{ + SecondFactorType_SECOND_FACTOR_TYPE_OTP, + SecondFactorType_SECOND_FACTOR_TYPE_WEBAUTHN, + SecondFactorType_SECOND_FACTOR_TYPE_SSO, + }, + sf: constants.SecondFactorOn, + }, + } { + t.Run(string(tt.sfs), func(t *testing.T) { + assert.Equal(t, tt.sf, LegacySecondFactorFromSecondFactors(tt.sfs)) + }) + } +} diff --git a/integration/integration_test.go b/integration/integration_test.go index 0c8b62d839a22..cc579bdf6be7d 100644 --- a/integration/integration_test.go +++ b/integration/integration_test.go @@ -8823,7 +8823,7 @@ func testModeratedSessions(t *testing.T, suite *integrationTestSuite) { // Enable web service. cfg := suite.defaultServiceConfig() cfg.Auth.Enabled = true - cfg.Auth.Preference.SetSecondFactor("on") + cfg.Auth.Preference.SetSecondFactors(types.SecondFactorType_SECOND_FACTOR_TYPE_WEBAUTHN) cfg.Auth.Preference.(*types.AuthPreferenceV2).Spec.RequireMFAType = types.RequireMFAType_SESSION cfg.Auth.Preference.SetWebauthn(&types.Webauthn{RPID: "127.0.0.1"}) cfg.Proxy.DisableWebService = false diff --git a/lib/auth/auth.go b/lib/auth/auth.go index 1345301a7642b..ea5fb1d5954ae 100644 --- a/lib/auth/auth.go +++ b/lib/auth/auth.go @@ -3826,7 +3826,7 @@ func (a *Server) DeleteMFADeviceSync(ctx context.Context, req *proto.DeleteMFADe // - Last resident key credential in a passwordless-capable cluster (avoids // passwordless users from locking themselves out). func (a *Server) deleteMFADeviceSafely(ctx context.Context, user, deviceName string) (*types.MFADevice, error) { - devs, err := a.Services.GetMFADevices(ctx, user, true) + mfaDevices, err := a.Services.GetMFADevices(ctx, user, true) if err != nil { return nil, trace.Wrap(err) } @@ -3836,114 +3836,71 @@ 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 - var deviceToDelete *types.MFADevice - var numResidentKeys int - - isResidentKey := func(d *types.MFADevice) bool { + isPasskey := func(d *types.MFADevice) bool { return d.GetWebauthn() != nil && d.GetWebauthn().ResidentKey } + var deviceToDelete *types.MFADevice + remainingDevices := make(map[types.SecondFactorType]int) + var remainingPasskeys int + // Find the device to delete and count devices. - for _, d := range devs { + for _, d := range mfaDevices { // Match device by name or ID. if d.GetName() == deviceName || d.Id == deviceName { deviceToDelete = d + switch d.Device.(type) { + case *types.MFADevice_Totp, *types.MFADevice_U2F, *types.MFADevice_Webauthn: + default: + return nil, trace.NotImplemented("cannot delete device of type %T", d.Device) + } + continue } - 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: + switch d.Device.(type) { + case *types.MFADevice_Totp: + remainingDevices[types.SecondFactorType_SECOND_FACTOR_TYPE_OTP]++ + case *types.MFADevice_U2F, *types.MFADevice_Webauthn: + remainingDevices[types.SecondFactorType_SECOND_FACTOR_TYPE_WEBAUTHN]++ + default: log.Warnf("Ignoring unknown device with type %T in deletion.", d.Device) continue } - sfToCount[sf]++ - knownDevices++ - - if isResidentKey(d) { - numResidentKeys++ + if isPasskey(d) { + remainingPasskeys++ } } if deviceToDelete == nil { return nil, trace.NotFound("MFA device %q does not exist", deviceName) } - // Prevent users from deleting their last device for clusters that require second factors. - const minDevices = 1 - switch sf := readOnlyAuthPref.GetSecondFactor(); sf { - case constants.SecondFactorOff, constants.SecondFactorOptional: // MFA is not required, allow deletion - case constants.SecondFactorOn: - 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 { - return nil, trace.BadParameter( - "cannot delete the last %s device for this user; add a replacement device first to avoid getting locked out", sf) - } - default: - return nil, trace.BadParameter("unexpected second factor type: %s", sf) + var remainingAllowedDevices int + for _, sf := range readOnlyAuthPref.GetSecondFactors() { + remainingAllowedDevices += remainingDevices[sf] } - // canDeleteLastPasskey figures out whether the user can safely delete their - // credential without locking themselves out in case if it's the last passkey. - // It checks whether the credential to delete is a last passkey and whether - // the user has other valid local credentials. - canDeleteLastPasskey := func() (bool, error) { - if !readOnlyAuthPref.GetAllowPasswordless() || numResidentKeys > 1 || !isResidentKey(deviceToDelete) { - return true, nil - } + // Prevent users from deleting their last allowed device for clusters that require second factors. + if readOnlyAuthPref.IsSecondFactorEnforced() && remainingAllowedDevices == 0 { + return nil, trace.BadParameter("cannot delete the last MFA device for this user; add a replacement device first to avoid getting locked out") + } - // Deleting the last passkey is OK if the user has a password set and an - // additional MFA device, otherwise they would be locked out. + // 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 + // 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() { u, err := a.Services.GetUser(ctx, user, false /* withSecrets */) if err != nil { - return false, trace.Wrap(err) - } - - // SSO users can always login through their SSO provider. - if u.GetUserType() == types.UserTypeSSO { - return true, nil - } - - if u.GetPasswordState() != types.PasswordState_PASSWORD_STATE_SET { - return false, nil - } - - // Minimum number of WebAuthn devices includes the passkey that we attempt - // to delete, hence 2. - if sfToCount[constants.SecondFactorWebauthn] >= 2 { - return true, nil + return nil, trace.Wrap(err) } - // Whether we take TOTPs into consideration or not depends on whether it's - // enabled. - switch sf := readOnlyAuthPref.GetSecondFactor(); sf { - case constants.SecondFactorOTP, constants.SecondFactorOn, constants.SecondFactorOptional: - if sfToCount[constants.SecondFactorOTP] >= 1 { - return true, nil - } + if u.GetUserType() != types.UserTypeSSO && u.GetPasswordState() != types.PasswordState_PASSWORD_STATE_SET { + return nil, trace.BadParameter("cannot delete last passwordless credential for user") } - - return false, nil - } - - can, err := canDeleteLastPasskey() - if err != nil { - return nil, trace.Wrap(err) - } - if !can { - return nil, trace.BadParameter("cannot delete last passwordless credential for user") } if err := a.DeleteMFADevice(ctx, user, deviceToDelete.Id); err != nil { @@ -4048,7 +4005,7 @@ func (a *Server) verifyMFARespAndAddDevice(ctx context.Context, req *newMFADevic return nil, trace.Wrap(err) } - if cap.GetSecondFactor() == constants.SecondFactorOff { + if !cap.IsSecondFactorEnabled() { return nil, trace.BadParameter("second factor disabled by cluster configuration") } diff --git a/lib/auth/auth_test.go b/lib/auth/auth_test.go index 1774fb1df52c5..b1248f330397f 100644 --- a/lib/auth/auth_test.go +++ b/lib/auth/auth_test.go @@ -880,7 +880,7 @@ func TestAuthenticateUser_mfaDeviceLocked(t *testing.T) { // Configure auth preferences. authPref, err := authServer.GetAuthPreference(ctx) require.NoError(t, err, "GetAuthPreference") - authPref.SetSecondFactor(constants.SecondFactorOptional) // good enough + authPref.SetSecondFactors(types.SecondFactorType_SECOND_FACTOR_TYPE_WEBAUTHN) authPref.SetWebauthn(&types.Webauthn{ RPID: "localhost", }) diff --git a/lib/auth/auth_with_roles.go b/lib/auth/auth_with_roles.go index 162fe4081bf31..fb07cfd46981c 100644 --- a/lib/auth/auth_with_roles.go +++ b/lib/auth/auth_with_roles.go @@ -4571,9 +4571,6 @@ func (a *ServerWithRoles) SetAuthPreference(ctx context.Context, newAuthPref typ msg = err.Error() } - oldSecondFactor := storedAuthPref.GetSecondFactor() - newSecondFactor := newAuthPref.GetSecondFactor() - if auditErr := a.authServer.emitter.EmitAuditEvent(ctx, &apievents.AuthPreferenceUpdate{ Metadata: apievents.Metadata{ Type: events.AuthPreferenceUpdateEvent, @@ -4586,7 +4583,7 @@ func (a *ServerWithRoles) SetAuthPreference(ctx context.Context, newAuthPref typ Error: msg, UserMessage: msg, }, - AdminActionsMFA: clusterconfigv1.GetAdminActionsMFAStatus(oldSecondFactor, newSecondFactor), + AdminActionsMFA: clusterconfigv1.GetAdminActionsMFAStatus(storedAuthPref, newAuthPref), }); auditErr != nil { log.WithError(auditErr).Warn("Failed to emit auth preference update event event.") } @@ -4624,9 +4621,6 @@ func (a *ServerWithRoles) ResetAuthPreference(ctx context.Context) error { msg = err.Error() } - oldSecondFactor := storedAuthPref.GetSecondFactor() - newSecondFactor := defaultAuthPref.GetSecondFactor() - if auditErr := a.authServer.emitter.EmitAuditEvent(ctx, &apievents.AuthPreferenceUpdate{ Metadata: apievents.Metadata{ Type: events.AuthPreferenceUpdateEvent, @@ -4639,7 +4633,7 @@ func (a *ServerWithRoles) ResetAuthPreference(ctx context.Context) error { Error: msg, UserMessage: msg, }, - AdminActionsMFA: clusterconfigv1.GetAdminActionsMFAStatus(oldSecondFactor, newSecondFactor), + AdminActionsMFA: clusterconfigv1.GetAdminActionsMFAStatus(storedAuthPref, defaultAuthPref), }); auditErr != nil { log.WithError(auditErr).Warn("Failed to emit auth preference update event event.") } diff --git a/lib/auth/clusterconfig/clusterconfigv1/service.go b/lib/auth/clusterconfig/clusterconfigv1/service.go index db86db07fb46e..fcecf3d0700dc 100644 --- a/lib/auth/clusterconfig/clusterconfigv1/service.go +++ b/lib/auth/clusterconfig/clusterconfigv1/service.go @@ -22,7 +22,6 @@ import ( "github.com/gravitational/trace" - "github.com/gravitational/teleport/api/constants" clusterconfigpb "github.com/gravitational/teleport/api/gen/proto/go/teleport/clusterconfig/v1" "github.com/gravitational/teleport/api/types" "github.com/gravitational/teleport/api/types/clusterconfig" @@ -257,9 +256,6 @@ func (s *Service) UpdateAuthPreference(ctx context.Context, req *clusterconfigpb updated, err := s.backend.UpdateAuthPreference(ctx, req.AuthPreference) - oldSecondFactor := original.GetSecondFactor() - newSecondFactor := req.AuthPreference.GetSecondFactor() - if auditErr := s.emitter.EmitAuditEvent(ctx, &apievents.AuthPreferenceUpdate{ Metadata: apievents.Metadata{ Type: events.AuthPreferenceUpdateEvent, @@ -268,7 +264,7 @@ func (s *Service) UpdateAuthPreference(ctx context.Context, req *clusterconfigpb UserMetadata: authzCtx.GetUserMetadata(), ConnectionMetadata: authz.ConnectionMetadata(ctx), Status: eventStatus(err), - AdminActionsMFA: GetAdminActionsMFAStatus(oldSecondFactor, newSecondFactor), + AdminActionsMFA: GetAdminActionsMFAStatus(original, req.AuthPreference), }); auditErr != nil { slog.WarnContext(ctx, "Failed to emit auth preference update event event.", "error", auditErr) } @@ -324,9 +320,6 @@ func (s *Service) UpsertAuthPreference(ctx context.Context, req *clusterconfigpb updated, err := s.backend.UpsertAuthPreference(ctx, req.AuthPreference) - oldSecondFactor := original.GetSecondFactor() - newSecondFactor := req.AuthPreference.GetSecondFactor() - if auditErr := s.emitter.EmitAuditEvent(ctx, &apievents.AuthPreferenceUpdate{ Metadata: apievents.Metadata{ Type: events.AuthPreferenceUpdateEvent, @@ -335,7 +328,7 @@ func (s *Service) UpsertAuthPreference(ctx context.Context, req *clusterconfigpb UserMetadata: authzCtx.GetUserMetadata(), ConnectionMetadata: authz.ConnectionMetadata(ctx), Status: eventStatus(err), - AdminActionsMFA: GetAdminActionsMFAStatus(oldSecondFactor, newSecondFactor), + AdminActionsMFA: GetAdminActionsMFAStatus(original, req.AuthPreference), }); auditErr != nil { slog.WarnContext(ctx, "Failed to emit auth preference update event event.", "error", auditErr) } @@ -370,7 +363,6 @@ func (s *Service) ResetAuthPreference(ctx context.Context, _ *clusterconfigpb.Re defaultPreference := types.DefaultAuthPreference() defaultPreference.SetDefaultSignatureAlgorithmSuite(s.signatureAlgorithmSuiteParams) - newSecondFactor := defaultPreference.GetSecondFactor() const iterationLimit = 3 // Attempt a few iterations in case the conditional update fails // due to spurious networking conditions. @@ -379,7 +371,6 @@ func (s *Service) ResetAuthPreference(ctx context.Context, _ *clusterconfigpb.Re if err != nil { return nil, trace.Wrap(err) } - oldSecondFactor := pref.GetSecondFactor() if pref.Origin() == types.OriginConfigFile { return nil, trace.BadParameter("auth preference has been defined via the config file and cannot be reset back to defaults dynamically.") @@ -400,7 +391,7 @@ func (s *Service) ResetAuthPreference(ctx context.Context, _ *clusterconfigpb.Re UserMetadata: authzCtx.GetUserMetadata(), ConnectionMetadata: authz.ConnectionMetadata(ctx), Status: eventStatus(err), - AdminActionsMFA: GetAdminActionsMFAStatus(oldSecondFactor, newSecondFactor), + AdminActionsMFA: GetAdminActionsMFAStatus(pref, defaultPreference), }); auditErr != nil { slog.WarnContext(ctx, "Failed to emit auth preference update event event.", "error", auditErr) } @@ -423,14 +414,13 @@ func (s *Service) ResetAuthPreference(ctx context.Context, _ *clusterconfigpb.Re // GetAdminActionsMFAStatus returns whether MFA for admin actions was // altered when the auth preferences were updated. -func GetAdminActionsMFAStatus(oldSecondFactor, newSecondFactor constants.SecondFactorType) apievents.AdminActionsMFAStatus { - if oldSecondFactor == newSecondFactor { +func GetAdminActionsMFAStatus(oldPref, newPref types.AuthPreference) apievents.AdminActionsMFAStatus { + if oldPref.IsAdminActionMFAEnforced() == newPref.IsAdminActionMFAEnforced() { return apievents.AdminActionsMFAStatus_ADMIN_ACTIONS_MFA_STATUS_UNCHANGED } - if newSecondFactor == constants.SecondFactorWebauthn { + if newPref.IsAdminActionMFAEnforced() { return apievents.AdminActionsMFAStatus_ADMIN_ACTIONS_MFA_STATUS_ENABLED } - // oldSecondFactor == constants.SecondFactorWebauthn return apievents.AdminActionsMFAStatus_ADMIN_ACTIONS_MFA_STATUS_DISABLED } diff --git a/lib/auth/grpcserver_test.go b/lib/auth/grpcserver_test.go index e09497b651b9d..6e85e400f4dd1 100644 --- a/lib/auth/grpcserver_test.go +++ b/lib/auth/grpcserver_test.go @@ -88,8 +88,11 @@ func TestMFADeviceManagement(t *testing.T) { // Enable MFA support. authPref, err := types.NewAuthPreference(types.AuthPreferenceSpecV2{ - Type: constants.Local, - SecondFactor: constants.SecondFactorOptional, + Type: constants.Local, + SecondFactors: []types.SecondFactorType{ + types.SecondFactorType_SECOND_FACTOR_TYPE_OTP, + types.SecondFactorType_SECOND_FACTOR_TYPE_WEBAUTHN, + }, Webauthn: &types.Webauthn{ RPID: "localhost", }, @@ -291,13 +294,26 @@ func TestMFADeviceManagement(t *testing.T) { }) } - // Register a 2nd passwordless device, so we can test removal of the - // 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()) + // Register an extra device to test allow deletion of other devices and test that + // the last device cannot be deleted. + const lastDeviceName = "lastDevice" + lastDevice, err := RegisterTestDevice(ctx, userClient, lastDeviceName, proto.DeviceType_DEVICE_TYPE_WEBAUTHN, devs.WebDev) require.NoError(t, err, "RegisterTestDevice failed") + // Also add a password so we can testing add last non-passkey MFA device. Testing the + // deletion of the last passkey is handled in TestDeletingLastPasswordlessDevice below. + err = authServer.UpsertPassword(user.GetName(), []byte("living on the edge")) + require.NoError(t, err, "UpsertPassword") + + // Since this device won't be deleted, we can use it to solve webauthn + // challenges throughout the tests below. + lastDeviceWebAuthnHandler := func(t *testing.T, challenge *proto.MFAAuthenticateChallenge) *proto.MFAAuthenticateResponse { + require.NotNil(t, challenge.WebauthnChallenge, "nil Webauthn challenge") + mfaResp, err := lastDevice.SolveAuthn(challenge) + require.NoError(t, err, "SolveAuthn") + return mfaResp + } + // Check that all new devices are registered. resp, err = userClient.GetMFADevices(ctx, &proto.GetMFADevicesRequest{}) require.NoError(t, err) @@ -308,7 +324,7 @@ func TestMFADeviceManagement(t *testing.T) { deviceIDs[dev.GetName()] = dev.Id } sort.Strings(deviceNames) - require.Equal(t, []string{pwdlessDevName, pwdless2DevName, devs.TOTPName, devs.WebName, webDev2Name}, deviceNames) + require.Equal(t, []string{lastDeviceName, pwdlessDevName, devs.TOTPName, devs.WebName, webDev2Name}, deviceNames) // Delete several of the MFA devices. deleteTests := []struct { @@ -319,7 +335,7 @@ func TestMFADeviceManagement(t *testing.T) { desc: "fail to delete an unknown device", opts: mfaDeleteTestOpts{ deviceName: "unknown-dev", - authHandler: devs.totpAuthHandler, + authHandler: lastDeviceWebAuthnHandler, checkErr: require.Error, }, }, @@ -371,55 +387,46 @@ func TestMFADeviceManagement(t *testing.T) { desc: "delete TOTP device by name", opts: mfaDeleteTestOpts{ deviceName: devs.TOTPName, - authHandler: devs.totpAuthHandler, + authHandler: lastDeviceWebAuthnHandler, checkErr: require.NoError, }, }, { - desc: "delete pwdless device by name", + desc: "delete webauthn device by name", opts: mfaDeleteTestOpts{ - deviceName: pwdlessDevName, - authHandler: devs.webAuthHandler, + deviceName: devs.WebName, + authHandler: lastDeviceWebAuthnHandler, checkErr: require.NoError, }, }, { - desc: "delete last passwordless device fails", + desc: "delete passwordless device by name", opts: mfaDeleteTestOpts{ - deviceName: pwdless2DevName, - authHandler: devs.webAuthHandler, - checkErr: func(t require.TestingT, err error, _ ...interface{}) { - require.ErrorContains(t, - err, - "last passwordless credential", - "Unexpected error deleting last passwordless device", - ) - }, + deviceName: pwdlessDevName, + authHandler: lastDeviceWebAuthnHandler, + checkErr: require.NoError, }, }, { - desc: "delete webauthn device by name", + desc: "delete webauthn device by ID", opts: mfaDeleteTestOpts{ - deviceName: devs.WebName, - authHandler: devs.webAuthHandler, + deviceName: deviceIDs[webDev2Name], + authHandler: lastDeviceWebAuthnHandler, checkErr: require.NoError, }, }, { - desc: "delete webauthn device by ID", + desc: "fail to delete last device", opts: mfaDeleteTestOpts{ - deviceName: deviceIDs[webDev2Name], - authHandler: func(t *testing.T, challenge *proto.MFAAuthenticateChallenge) *proto.MFAAuthenticateResponse { - resp, err := webKey2.SignAssertion( - webOrigin, wantypes.CredentialAssertionFromProto(challenge.WebauthnChallenge)) - require.NoError(t, err) - return &proto.MFAAuthenticateResponse{ - Response: &proto.MFAAuthenticateResponse_Webauthn{ - Webauthn: wantypes.CredentialAssertionResponseToProto(resp), - }, - } + deviceName: lastDeviceName, + authHandler: lastDeviceWebAuthnHandler, + checkErr: func(t require.TestingT, err error, _ ...any) { + require.ErrorContains(t, + err, + "cannot delete the last MFA device for this user", + "Unexpected error deleting last MFA device", + ) }, - checkErr: require.NoError, }, }, } @@ -429,10 +436,10 @@ func TestMFADeviceManagement(t *testing.T) { }) } - // Check no remaining devices, apart from the additional passwordless device that we can't delete. + // Check no remaining devices, apart from the additional device that we can't delete. resp, err = userClient.GetMFADevices(ctx, &proto.GetMFADevicesRequest{}) require.NoError(t, err) - require.Equal(t, "pwdless2", resp.Devices[0].GetName()) + require.Equal(t, lastDeviceName, resp.Devices[0].GetName()) } func TestDeletingLastPasswordlessDevice(t *testing.T) { @@ -447,19 +454,27 @@ func TestDeletingLastPasswordlessDevice(t *testing.T) { checkErr require.ErrorAssertionFunc }{ { - name: "fails", + name: "NOK no other MFA device", setup: func(*testing.T, string, *authclient.Client, *TestDevice) {}, checkErr: func(t require.TestingT, err error, _ ...any) { require.ErrorContains(t, err, - "last passwordless credential", + "cannot delete the last MFA device for this user", "Unexpected error deleting last passwordless device", ) }, }, { - name: "succeeds when passwordless is off", - setup: func(t *testing.T, _ string, _ *authclient.Client, _ *TestDevice) { + // 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") @@ -471,7 +486,15 @@ func TestDeletingLastPasswordlessDevice(t *testing.T) { checkErr: require.NoError, }, { - name: "succeeds when there is a password and other WebAuthn MFAs", + name: "OK extra passwordless device", + setup: func(t *testing.T, username string, userClient *authclient.Client, pwdlessDev *TestDevice) { + _, err := RegisterTestDevice(ctx, userClient, "another-passkey", proto.DeviceType_DEVICE_TYPE_WEBAUTHN, pwdlessDev, WithPasswordless()) + require.NoError(t, err, "RegisterTestDevice failed") + }, + checkErr: require.NoError, + }, + { + name: "OK password set with other WebAuthn device", setup: func(t *testing.T, username string, userClient *authclient.Client, pwdlessDev *TestDevice) { err := authServer.UpsertPassword(username, []byte("living on the edge")) require.NoError(t, err, "UpsertPassword") @@ -482,7 +505,7 @@ func TestDeletingLastPasswordlessDevice(t *testing.T) { checkErr: require.NoError, }, { - name: "succeeds when there is a password and TOTP MFA", + name: "OK password set with other TOTP device", setup: func(t *testing.T, username string, userClient *authclient.Client, pwdlessDev *TestDevice) { err := authServer.UpsertPassword(username, []byte("living on the edge")) require.NoError(t, err, "UpsertPassword") @@ -493,7 +516,7 @@ func TestDeletingLastPasswordlessDevice(t *testing.T) { checkErr: require.NoError, }, { - name: "succeeds when the user is an SSO user", + name: "OK SSO user with other device", setup: func(t *testing.T, username string, userClient *authclient.Client, pwdlessDev *TestDevice) { user, err := authServer.GetUser(ctx, username, false) require.NoError(t, err, "GetUser") @@ -503,27 +526,39 @@ func TestDeletingLastPasswordlessDevice(t *testing.T) { _, err = authServer.UpsertUser(ctx, user) require.NoError(t, err, "UpsertUser") _, err = RegisterTestDevice( - ctx, userClient, "another-dev", proto.DeviceType_DEVICE_TYPE_TOTP, pwdlessDev, WithTestDeviceClock(clock)) + ctx, userClient, "another-dev", proto.DeviceType_DEVICE_TYPE_WEBAUTHN, pwdlessDev) require.NoError(t, err, "RegisterTestDevice") }, checkErr: require.NoError, }, { - name: "fails even if there is password, but no other MFAs", + name: "NOK password set but no other MFAs", setup: func(t *testing.T, username string, userClient *authclient.Client, pwdlessDev *TestDevice) { err := authServer.UpsertPassword(username, []byte("living on the edge")) require.NoError(t, err, "UpsertPassword") }, - checkErr: require.Error, + checkErr: func(t require.TestingT, err error, _ ...any) { + require.ErrorContains(t, + err, + "cannot delete the last MFA device for this user", + "Unexpected error deleting last passwordless device", + ) + }, }, { - name: "fails even if there is another MFA, but no password", + name: "NOK other MFAs, but no password set", setup: func(t *testing.T, username string, userClient *authclient.Client, pwdlessDev *TestDevice) { _, err := RegisterTestDevice( ctx, userClient, "another-dev", proto.DeviceType_DEVICE_TYPE_TOTP, pwdlessDev, WithTestDeviceClock(clock)) require.NoError(t, err, "RegisterTestDevice") }, - checkErr: require.Error, + 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", + ) + }, }, } @@ -531,8 +566,11 @@ func TestDeletingLastPasswordlessDevice(t *testing.T) { t.Run(test.name, func(t *testing.T) { // Enable MFA support. authPref, err := types.NewAuthPreference(types.AuthPreferenceSpecV2{ - Type: constants.Local, - SecondFactor: constants.SecondFactorOptional, + Type: constants.Local, + SecondFactors: []types.SecondFactorType{ + types.SecondFactorType_SECOND_FACTOR_TYPE_OTP, + types.SecondFactorType_SECOND_FACTOR_TYPE_WEBAUTHN, + }, Webauthn: &types.Webauthn{ RPID: "localhost", }, @@ -944,7 +982,10 @@ func TestGenerateUserCerts_deviceAuthz(t *testing.T) { const rpID = "localhost" const origin = "https://" + rpID + ":3080" // matches RPID. updateAuthPref(t, func(authPref types.AuthPreference) { - authPref.SetSecondFactor(constants.SecondFactorOptional) + authPref.SetSecondFactors( + types.SecondFactorType_SECOND_FACTOR_TYPE_OTP, + types.SecondFactorType_SECOND_FACTOR_TYPE_WEBAUTHN, + ) authPref.SetWebauthn(&types.Webauthn{ RPID: "localhost", }) @@ -1153,7 +1194,10 @@ func TestRegisterFirstDevice_deviceAuthz(t *testing.T) { // Enable webauthn updateAuthPref(t, func(authPref types.AuthPreference) { - authPref.SetSecondFactor(constants.SecondFactorOptional) + authPref.SetSecondFactors( + types.SecondFactorType_SECOND_FACTOR_TYPE_OTP, + types.SecondFactorType_SECOND_FACTOR_TYPE_WEBAUTHN, + ) authPref.SetWebauthn(&types.Webauthn{ RPID: "localhost", }) diff --git a/lib/auth/methods.go b/lib/auth/methods.go index 7c3ee63a278ee..0e7682370490f 100644 --- a/lib/auth/methods.go +++ b/lib/auth/methods.go @@ -22,11 +22,11 @@ import ( "bytes" "context" "errors" + "log/slog" "net" "time" "github.com/gravitational/trace" - "github.com/sirupsen/logrus" "github.com/gravitational/teleport/api/client/proto" "github.com/gravitational/teleport/api/constants" @@ -303,7 +303,10 @@ func (a *Server) authenticateUserInternal( if req.HeadlessAuthenticationID != "" { mfaDev, err = a.authenticateHeadless(ctx, req) if err != nil { - log.Debugf("Headless Authentication for user %q failed while waiting for approval: %v", user, err) + slog.DebugContext(ctx, "Headless authenticate failed while waiting for approval", + "user", user, + "error", err, + ) return nil, "", trace.Wrap(err) } return mfaDev, user, nil @@ -327,10 +330,10 @@ func (a *Server) authenticateUserInternal( case err != nil: return nil, "", trace.Wrap(err) case u.GetUserType() != types.UserTypeLocal: - log.WithFields(logrus.Fields{ - "user": user, - "user_type": u.GetUserType(), - }).Warn("Non-local user attempted local authentication") + slog.WarnContext(ctx, "Non-local user attempted local authentication", + "user", user, + "user_type", u.GetUserType(), + ) return nil, "", trace.Wrap(errSSOUserLocalAuth) } @@ -378,16 +381,22 @@ func (a *Server) authenticateUserInternal( }) switch { case err != nil: - log.Debugf("User %v failed to authenticate: %v.", user, err) + slog.DebugContext(ctx, "User failed to authenticate.", + "user", user, + "error", err, + ) if fieldErr := getErrorByTraceField(err); fieldErr != nil { return nil, "", trace.Wrap(fieldErr) } return nil, "", trace.Wrap(authErr) case mfaDev == nil: - log.Debugf( - "MFA authentication returned nil device (Webauthn = %v, TOTP = %v, Headless = %v): %v.", - req.Webauthn != nil, req.OTP != nil, req.HeadlessAuthenticationID != "", err) + slog.DebugContext(ctx, "MFA authentication returned nil device.", + "webauthn", req.Webauthn != nil, + "totp", req.OTP != nil, + "headless", req.HeadlessAuthenticationID != "", + "error", err, + ) return nil, "", trace.Wrap(authErr) default: return mfaDev, user, nil @@ -406,10 +415,14 @@ func (a *Server) authenticateUserInternal( // When using password only make sure that auth preference does not require // second factor, otherwise users could bypass it. - switch authPreference.GetSecondFactor() { - case constants.SecondFactorOff: - // No 2FA required, check password only. - case constants.SecondFactorOptional: + switch { + case authPreference.IsSecondFactorEnforced(): + // Some form of MFA is required but none provided. Either client is + // buggy (didn't send MFA response) or someone is trying to bypass + // MFA. + slog.WarnContext(ctx, "MFA bypass attempt, access denied.", "user", user) + return nil, "", trace.AccessDenied("missing second factor") + case authPreference.IsSecondFactorEnabled(): // 2FA is optional. Make sure that a user does not have MFA devices // registered. devs, err := a.Services.GetMFADevices(ctx, user, false /* withSecrets */) @@ -417,15 +430,11 @@ func (a *Server) authenticateUserInternal( return nil, "", trace.Wrap(err) } if len(devs) != 0 { - log.Warningf("MFA bypass attempt by user %q, access denied.", user) + slog.WarnContext(ctx, "MFA bypass attempt, access denied.", "user", user) return nil, "", trace.AccessDenied("missing second factor authentication") } default: - // Some form of MFA is required but none provided. Either client is - // buggy (didn't send MFA response) or someone is trying to bypass - // MFA. - log.Warningf("MFA bypass attempt by user %q, access denied.", user) - return nil, "", trace.AccessDenied("missing second factor") + // No 2FA required, check password only. } if err = a.WithUserLock(ctx, user, func() error { return a.checkPasswordWOToken(ctx, user, req.Pass.Password) @@ -435,7 +444,10 @@ func (a *Server) authenticateUserInternal( } // provide obscure message on purpose, while logging the real // error server side - log.Debugf("User %v failed to authenticate: %v.", user, err) + slog.DebugContext(ctx, "User failed to authenticate.", + "user", user, + "error", err, + ) return nil, "", trace.Wrap(authclient.InvalidUserPassError) } return nil, user, nil diff --git a/lib/auth/password.go b/lib/auth/password.go index ce340820fd080..f2045ddd2ef0c 100644 --- a/lib/auth/password.go +++ b/lib/auth/password.go @@ -31,7 +31,6 @@ import ( "github.com/gravitational/teleport" "github.com/gravitational/teleport/api/client/proto" - "github.com/gravitational/teleport/api/constants" mfav1 "github.com/gravitational/teleport/api/gen/proto/go/teleport/mfa/v1" "github.com/gravitational/teleport/api/types" apievents "github.com/gravitational/teleport/api/types/events" @@ -386,10 +385,10 @@ func (a *Server) changeUserSecondFactor(ctx context.Context, req *proto.ChangeUs return trace.Wrap(err) } - switch sf := cap.GetSecondFactor(); { - case sf == constants.SecondFactorOff: + switch { + case !cap.IsSecondFactorEnabled(): return nil - case req.GetNewMFARegisterResponse() == nil && sf == constants.SecondFactorOptional: + case req.GetNewMFARegisterResponse() == nil && !cap.IsSecondFactorEnforced(): // Optional second factor does not enforce users to add a MFA device. // No need to check if a user already has registered devices since we expect // users to have no devices at this point. diff --git a/lib/auth/tls_test.go b/lib/auth/tls_test.go index bd0a3d8fd5e5f..cc85616a315ec 100644 --- a/lib/auth/tls_test.go +++ b/lib/auth/tls_test.go @@ -1401,7 +1401,7 @@ func TestAuthPreferenceSettings(t *testing.T) { require.NoError(t, err) require.Equal(t, "local", gotAP.GetType()) - require.Equal(t, constants.SecondFactorOTP, gotAP.GetSecondFactor()) + require.Equal(t, []types.SecondFactorType{types.SecondFactorType_SECOND_FACTOR_TYPE_OTP}, gotAP.GetSecondFactors()) require.True(t, gotAP.GetDisconnectExpiredCert()) require.Empty(t, cmp.Diff(upsertedAP, gotAP, cmpopts.IgnoreFields(types.Metadata{}, "Revision"))) } diff --git a/lib/auth/usertoken.go b/lib/auth/usertoken.go index e874c5d44d1f0..5959f223cb491 100644 --- a/lib/auth/usertoken.go +++ b/lib/auth/usertoken.go @@ -30,7 +30,6 @@ import ( "github.com/pquerna/otp/totp" "github.com/gravitational/teleport/api/client/proto" - "github.com/gravitational/teleport/api/constants" mfav1 "github.com/gravitational/teleport/api/gen/proto/go/teleport/mfa/v1" "github.com/gravitational/teleport/api/types" apievents "github.com/gravitational/teleport/api/types/events" @@ -385,7 +384,7 @@ func (a *Server) CreatePrivilegeToken(ctx context.Context, req *proto.CreatePriv // For a user to add a device, second factor must be enabled. // A nil request will be interpreted as a user who has second factor enabled // but does not have any MFA registered, as can be the case with second factor optional. - if authPref.GetSecondFactor() == constants.SecondFactorOff { + if !authPref.IsSecondFactorEnabled() { return nil, trace.AccessDenied("second factor must be enabled") } diff --git a/lib/services/readonly/readonly.go b/lib/services/readonly/readonly.go index 47e60ec9c4daf..db65197a4338a 100644 --- a/lib/services/readonly/readonly.go +++ b/lib/services/readonly/readonly.go @@ -36,11 +36,13 @@ import ( // to ensure that we do not modify the underlying AuthPreference as it may be shared across // multiple goroutines. type AuthPreference interface { - GetSecondFactor() constants.SecondFactorType GetDisconnectExpiredCert() bool GetLockingMode() constants.LockingMode GetDeviceTrust() *types.DeviceTrust GetPrivateKeyPolicy() keys.PrivateKeyPolicy + GetSecondFactors() []types.SecondFactorType + IsSecondFactorEnforced() bool + IsSecondFactorTOTPAllowed() bool IsAdminActionMFAEnforced() bool GetRequireMFAType() types.RequireMFAType IsSAMLIdPEnabled() bool diff --git a/lib/web/apiserver.go b/lib/web/apiserver.go index 8087b5e216194..8404d66d6ce2f 100644 --- a/lib/web/apiserver.go +++ b/lib/web/apiserver.go @@ -1235,7 +1235,7 @@ func (h *Handler) AccessGraphAddr() utils.NetAddr { func localSettings(cap types.AuthPreference) (webclient.AuthenticationSettings, error) { as := webclient.AuthenticationSettings{ Type: constants.Local, - SecondFactor: cap.GetSecondFactor(), + SecondFactor: types.LegacySecondFactorFromSecondFactors(cap.GetSecondFactors()), PreferredLocalMFA: cap.GetPreferredLocalMFA(), AllowPasswordless: cap.GetAllowPasswordless(), AllowHeadless: cap.GetAllowHeadless(), @@ -1280,7 +1280,7 @@ func oidcSettings(connector types.OIDCConnector, cap types.AuthPreference) webcl Display: connector.GetDisplay(), }, // Local fallback / MFA. - SecondFactor: cap.GetSecondFactor(), + SecondFactor: types.LegacySecondFactorFromSecondFactors(cap.GetSecondFactors()), PreferredLocalMFA: cap.GetPreferredLocalMFA(), PrivateKeyPolicy: cap.GetPrivateKeyPolicy(), PIVSlot: cap.GetPIVSlot(), @@ -1298,7 +1298,7 @@ func samlSettings(connector types.SAMLConnector, cap types.AuthPreference) webcl SingleLogoutEnabled: connector.GetSingleLogoutURL() != "", }, // Local fallback / MFA. - SecondFactor: cap.GetSecondFactor(), + SecondFactor: types.LegacySecondFactorFromSecondFactors(cap.GetSecondFactors()), PreferredLocalMFA: cap.GetPreferredLocalMFA(), PrivateKeyPolicy: cap.GetPrivateKeyPolicy(), PIVSlot: cap.GetPIVSlot(), @@ -1315,7 +1315,7 @@ func githubSettings(connector types.GithubConnector, cap types.AuthPreference) w Display: connector.GetDisplay(), }, // Local fallback / MFA. - SecondFactor: cap.GetSecondFactor(), + SecondFactor: types.LegacySecondFactorFromSecondFactors(cap.GetSecondFactors()), PreferredLocalMFA: cap.GetPreferredLocalMFA(), PrivateKeyPolicy: cap.GetPrivateKeyPolicy(), PIVSlot: cap.GetPIVSlot(), @@ -1717,7 +1717,7 @@ func (h *Handler) getWebConfig(w http.ResponseWriter, r *http.Request, p httprou authSettings = webclient.WebConfigAuthSettings{ Providers: authProviders, - SecondFactor: cap.GetSecondFactor(), + SecondFactor: types.LegacySecondFactorFromSecondFactors(cap.GetSecondFactors()), LocalAuthEnabled: cap.GetAllowLocalAuth(), AllowPasswordless: cap.GetAllowPasswordless(), AuthType: authType, @@ -2337,19 +2337,15 @@ func (h *Handler) createWebSession(w http.ResponseWriter, r *http.Request, p htt clientMeta := clientMetaFromReq(r) var webSession types.WebSession - switch cap.GetSecondFactor() { - case constants.SecondFactorOff: + switch { + case !cap.IsSecondFactorEnforced(): + webSession, err = h.auth.AuthWithoutOTP(r.Context(), req.User, req.Pass, clientMeta) + case req.SecondFactorToken == "" && !cap.IsSecondFactorEnforced(): webSession, err = h.auth.AuthWithoutOTP(r.Context(), req.User, req.Pass, clientMeta) - case constants.SecondFactorOTP, constants.SecondFactorOn: + case cap.IsSecondFactorTOTPAllowed(): webSession, err = h.auth.AuthWithOTP(r.Context(), req.User, req.Pass, req.SecondFactorToken, clientMeta) - case constants.SecondFactorOptional: - if req.SecondFactorToken == "" { - webSession, err = h.auth.AuthWithoutOTP(r.Context(), req.User, req.Pass, clientMeta) - } else { - webSession, err = h.auth.AuthWithOTP(r.Context(), req.User, req.Pass, req.SecondFactorToken, clientMeta) - } default: - return nil, trace.AccessDenied("unknown second factor type: %q", cap.GetSecondFactor()) + return nil, trace.AccessDenied("direct login with password+otp not supported by this cluster") } if err != nil { h.log.WithError(err).Warnf("Access attempt denied for user %q.", req.User) @@ -4159,18 +4155,18 @@ func (h *Handler) createSSHCert(w http.ResponseWriter, r *http.Request, p httpro return loginResp, nil } - switch cap.GetSecondFactor() { - case constants.SecondFactorOff: + switch { + case !cap.IsSecondFactorEnforced(): authSSHUserReq.AuthenticateUserRequest.Pass = &authclient.PassCreds{ Password: []byte(req.Password), } - case constants.SecondFactorOTP, constants.SecondFactorOn, constants.SecondFactorOptional: + case cap.IsSecondFactorTOTPAllowed(): authSSHUserReq.AuthenticateUserRequest.OTP = &authclient.OTPCreds{ Password: []byte(req.Password), Token: req.OTPToken, } default: - return nil, trace.AccessDenied("unsupported second factor type: %q", cap.GetSecondFactor()) + return nil, trace.AccessDenied("direct login with password+otp not supported by this cluster") } loginResp, err := authClient.AuthenticateSSHUser(r.Context(), authSSHUserReq) diff --git a/lib/web/apiserver_ping_test.go b/lib/web/apiserver_ping_test.go index 5a54fd2407555..e14017d0ff135 100644 --- a/lib/web/apiserver_ping_test.go +++ b/lib/web/apiserver_ping_test.go @@ -67,7 +67,38 @@ func TestPing(t *testing.T) { }, assertResp: func(cap types.AuthPreference, resp *webclient.PingResponse) { assert.Equal(t, cap.GetType(), resp.Auth.Type) - assert.Equal(t, cap.GetSecondFactor(), resp.Auth.SecondFactor) + assert.Equal(t, constants.SecondFactorOn, resp.Auth.SecondFactor) + assert.NotEmpty(t, cap.GetPreferredLocalMFA(), "preferred local MFA empty") + assert.NotNil(t, resp.Auth.Local, "Auth.Local expected") + + u2f, _ := cap.GetU2F() + require.NotNil(t, resp.Auth.U2F) + assert.Equal(t, u2f.AppID, resp.Auth.U2F.AppID) + + webCfg, _ := cap.GetWebauthn() + require.NotNil(t, resp.Auth.Webauthn) + assert.Equal(t, webCfg.RPID, resp.Auth.Webauthn.RPID) + + assert.Equal(t, types.SignatureAlgorithmSuite_SIGNATURE_ALGORITHM_SUITE_UNSPECIFIED, resp.Auth.SignatureAlgorithmSuite) + }, + }, + { + name: "OK local auth SecondFactors", + spec: &types.AuthPreferenceSpecV2{ + Type: constants.Local, + SecondFactors: []types.SecondFactorType{ + types.SecondFactorType_SECOND_FACTOR_TYPE_WEBAUTHN, + }, + U2F: &types.U2F{ + AppID: "https://example.com", + }, + Webauthn: &types.Webauthn{ + RPID: "example.com", + }, + }, + assertResp: func(cap types.AuthPreference, resp *webclient.PingResponse) { + assert.Equal(t, cap.GetType(), resp.Auth.Type) + assert.Equal(t, constants.SecondFactorWebauthn, resp.Auth.SecondFactor) assert.NotEmpty(t, cap.GetPreferredLocalMFA(), "preferred local MFA empty") assert.NotNil(t, resp.Auth.Local, "Auth.Local expected") diff --git a/lib/web/apiserver_test.go b/lib/web/apiserver_test.go index 06b42f394e652..ad16a9c658dc8 100644 --- a/lib/web/apiserver_test.go +++ b/lib/web/apiserver_test.go @@ -4612,7 +4612,7 @@ func TestGetWebConfig_WithEntitlements(t *testing.T) { const MOTD = "Welcome to cluster, your activity will be recorded." ap, err := types.NewAuthPreference(types.AuthPreferenceSpecV2{ Type: constants.Local, - SecondFactor: constants.SecondFactorOptional, + SecondFactor: constants.SecondFactorOn, ConnectorName: constants.PasswordlessConnector, Webauthn: &types.Webauthn{ RPID: "localhost", @@ -4642,7 +4642,7 @@ func TestGetWebConfig_WithEntitlements(t *testing.T) { expectedCfg := webclient.WebConfig{ Auth: webclient.WebConfigAuthSettings{ - SecondFactor: constants.SecondFactorOptional, + SecondFactor: constants.SecondFactorOn, Providers: []webclient.WebConfigAuthProvider{{ Name: "test-github", Type: constants.Github, diff --git a/tool/tctl/common/collection.go b/tool/tctl/common/collection.go index d74bd31ceba2f..e3ae81f763945 100644 --- a/tool/tctl/common/collection.go +++ b/tool/tctl/common/collection.go @@ -568,8 +568,17 @@ func (c *authPrefCollection) resources() (r []types.Resource) { } func (c *authPrefCollection) writeText(w io.Writer, verbose bool) error { - t := asciitable.MakeTable([]string{"Type", "Second Factor"}) - t.AddRow([]string{c.authPref.GetType(), string(c.authPref.GetSecondFactor())}) + var secondFactorStrings []string + for _, sf := range c.authPref.GetSecondFactors() { + sfString, err := sf.Encode() + if err != nil { + return trace.Wrap(err) + } + secondFactorStrings = append(secondFactorStrings, sfString) + } + + t := asciitable.MakeTable([]string{"Type", "Second Factors"}) + t.AddRow([]string{c.authPref.GetType(), strings.Join(secondFactorStrings, ", ")}) _, err := t.AsBuffer().WriteTo(w) return trace.Wrap(err) } diff --git a/tool/tctl/common/edit_command_test.go b/tool/tctl/common/edit_command_test.go index c587080d33d86..42f9dee0043ed 100644 --- a/tool/tctl/common/edit_command_test.go +++ b/tool/tctl/common/edit_command_test.go @@ -30,7 +30,6 @@ import ( "github.com/stretchr/testify/require" "google.golang.org/protobuf/testing/protocmp" - "github.com/gravitational/teleport/api/constants" autoupdatev1pb "github.com/gravitational/teleport/api/gen/proto/go/teleport/autoupdate/v1" headerv1 "github.com/gravitational/teleport/api/gen/proto/go/teleport/header/v1" labelv1 "github.com/gravitational/teleport/api/gen/proto/go/teleport/label/v1" @@ -283,7 +282,7 @@ func testEditAuthPreference(t *testing.T, clt *authclient.Client) { } expected.SetRevision(initial.GetRevision()) - expected.SetSecondFactor(constants.SecondFactorOff) + expected.SetSecondFactors(types.SecondFactorType_SECOND_FACTOR_TYPE_OTP, types.SecondFactorType_SECOND_FACTOR_TYPE_SSO) collection := &authPrefCollection{authPref: expected} return trace.NewAggregate(writeYAML(collection, f), f.Close()) @@ -296,7 +295,7 @@ func testEditAuthPreference(t *testing.T, clt *authclient.Client) { actual, err := clt.GetAuthPreference(ctx) require.NoError(t, err, "retrieving cap after edit") - assert.NotEqual(t, initial.GetSecondFactor(), actual.GetSecondFactor(), "second factor should have been modified by edit") + assert.NotEqual(t, initial.GetSecondFactors(), actual.GetSecondFactors(), "second factors should have been modified by edit") require.Empty(t, cmp.Diff(expected, actual, cmpopts.IgnoreFields(types.Metadata{}, "Revision", "Labels"))) assert.Equal(t, types.OriginDynamic, actual.Origin()) diff --git a/tool/tctl/common/resource_command_test.go b/tool/tctl/common/resource_command_test.go index 6ec25abb1507d..1eb4b6ee34bfd 100644 --- a/tool/tctl/common/resource_command_test.go +++ b/tool/tctl/common/resource_command_test.go @@ -1832,7 +1832,7 @@ func testCreateAuthPreference(t *testing.T, clt *authclient.Client) { metadata: name: cluster-auth-preference spec: - second_factor: off + second_factors: [otp, sso] type: local version: v2 ` @@ -1849,16 +1849,18 @@ version: v2 cap = mustDecodeJSON[[]*types.AuthPreferenceV2](t, buf) require.Len(t, cap, 1) - var expected types.AuthPreferenceV2 - require.NoError(t, yaml.Unmarshal([]byte(capYAML), &expected)) + expectInitialSecondFactors := []types.SecondFactorType{types.SecondFactorType_SECOND_FACTOR_TYPE_OTP} // second factors defaults to [otp] + require.Equal(t, expectInitialSecondFactors, initial.GetSecondFactors()) - require.NotEqual(t, constants.SecondFactorOff, initial.GetSecondFactor()) - require.Equal(t, constants.SecondFactorOff, expected.GetSecondFactor()) + var revised types.AuthPreferenceV2 + require.NoError(t, yaml.Unmarshal([]byte(capYAML), &revised)) + expectRevisedSecondFactors := []types.SecondFactorType{types.SecondFactorType_SECOND_FACTOR_TYPE_OTP, types.SecondFactorType_SECOND_FACTOR_TYPE_SSO} + require.Equal(t, expectRevisedSecondFactors, revised.GetSecondFactors()) // Explicitly change the revision and try creating the cap with and without // the force flag. - expected.SetRevision(uuid.NewString()) - raw, err := services.MarshalAuthPreference(&expected, services.PreserveRevision()) + revised.SetRevision(uuid.NewString()) + raw, err := services.MarshalAuthPreference(&revised, services.PreserveRevision()) require.NoError(t, err) require.NoError(t, os.WriteFile(capYAMLPath, raw, 0644))