diff --git a/lib/auth/auth_with_roles.go b/lib/auth/auth_with_roles.go index e0b16dc3026e7..58d18e66c3489 100644 --- a/lib/auth/auth_with_roles.go +++ b/lib/auth/auth_with_roles.go @@ -2099,7 +2099,8 @@ func (a *ServerWithRoles) UpsertToken(ctx context.Context, token types.Provision return trace.Wrap(err) } - if err := authz.AuthorizeAdminAction(ctx, &a.context); err != nil { + // Support reused MFA for bulk tctl create requests. + if err := authz.AuthorizeAdminActionAllowReusedMFA(ctx, &a.context); err != nil { return trace.Wrap(err) } @@ -3152,7 +3153,8 @@ func (a *ServerWithRoles) CreateResetPasswordToken(ctx context.Context, req Crea return nil, trace.AccessDenied("access denied") } - if err := authz.AuthorizeAdminAction(ctx, &a.context); err != nil { + // Allow reused MFA responses to allow creating a reset token after creating a user. + if err := authz.AuthorizeAdminActionAllowReusedMFA(ctx, &a.context); err != nil { return nil, trace.Wrap(err) } @@ -3298,7 +3300,8 @@ func (a *ServerWithRoles) UpsertOIDCConnector(ctx context.Context, connector typ return nil, trace.AccessDenied("OIDC is only available in Teleport Enterprise") } - if err := authz.AuthorizeAdminAction(ctx, &a.context); err != nil { + // Support reused MFA for bulk tctl create requests. + if err := authz.AuthorizeAdminActionAllowReusedMFA(ctx, &a.context); err != nil { return nil, trace.Wrap(err) } @@ -3338,7 +3341,8 @@ func (a *ServerWithRoles) CreateOIDCConnector(ctx context.Context, connector typ return nil, trace.AccessDenied("OIDC is only available in Teleport Enterprise") } - if err := authz.AuthorizeAdminAction(ctx, &a.context); err != nil { + // Support reused MFA for bulk tctl create requests. + if err := authz.AuthorizeAdminActionAllowReusedMFA(ctx, &a.context); err != nil { return nil, trace.Wrap(err) } @@ -3453,7 +3457,8 @@ func (a *ServerWithRoles) UpsertSAMLConnector(ctx context.Context, connector typ return nil, trace.Wrap(err) } - if err := authz.AuthorizeAdminAction(ctx, &a.context); err != nil { + // Support reused MFA for bulk tctl create requests. + if err := authz.AuthorizeAdminActionAllowReusedMFA(ctx, &a.context); err != nil { return nil, trace.Wrap(err) } @@ -3654,7 +3659,8 @@ func (a *ServerWithRoles) UpsertGithubConnector(ctx context.Context, connector t return nil, trace.Wrap(err) } - if err := authz.AuthorizeAdminAction(ctx, &a.context); err != nil { + // Support reused MFA for bulk tctl create requests. + if err := authz.AuthorizeAdminActionAllowReusedMFA(ctx, &a.context); err != nil { return nil, trace.Wrap(err) } @@ -3672,7 +3678,8 @@ func (a *ServerWithRoles) CreateGithubConnector(ctx context.Context, connector t return nil, trace.Wrap(err) } - if err := authz.AuthorizeAdminAction(ctx, &a.context); err != nil { + // Support reused MFA for bulk tctl create requests. + if err := authz.AuthorizeAdminActionAllowReusedMFA(ctx, &a.context); err != nil { return nil, trace.Wrap(err) } @@ -4107,7 +4114,8 @@ func (a *ServerWithRoles) UpsertRole(ctx context.Context, role types.Role) (type return nil, trace.Wrap(err) } - if err := authz.AuthorizeAdminAction(ctx, &a.context); err != nil { + // Support reused MFA for bulk tctl create requests. + if err := authz.AuthorizeAdminActionAllowReusedMFA(ctx, &a.context); err != nil { return nil, trace.Wrap(err) } @@ -4322,7 +4330,8 @@ func (a *ServerWithRoles) SetAuthPreference(ctx context.Context, newAuthPref typ return trace.Wrap(err) } - if err := authz.AuthorizeAdminAction(ctx, &a.context); err != nil { + // Support reused MFA for bulk tctl create requests. + if err := authz.AuthorizeAdminActionAllowReusedMFA(ctx, &a.context); err != nil { return trace.Wrap(err) } @@ -4394,7 +4403,8 @@ func (a *ServerWithRoles) SetClusterNetworkingConfig(ctx context.Context, newNet } } - if err := authz.AuthorizeAdminAction(ctx, &a.context); err != nil { + // Support reused MFA for bulk tctl create requests. + if err := authz.AuthorizeAdminActionAllowReusedMFA(ctx, &a.context); err != nil { return trace.Wrap(err) } @@ -4456,7 +4466,8 @@ func (a *ServerWithRoles) SetSessionRecordingConfig(ctx context.Context, newRecC } } - if err := authz.AuthorizeAdminAction(ctx, &a.context); err != nil { + // Support reused MFA for bulk tctl create requests. + if err := authz.AuthorizeAdminActionAllowReusedMFA(ctx, &a.context); err != nil { return trace.Wrap(err) } @@ -5391,7 +5402,8 @@ func (a *ServerWithRoles) SetNetworkRestrictions(ctx context.Context, nr types.N return trace.Wrap(err) } - if err := authz.AuthorizeAdminAction(ctx, &a.context); err != nil { + // Support reused MFA for bulk tctl create requests. + if err := authz.AuthorizeAdminActionAllowReusedMFA(ctx, &a.context); err != nil { return trace.Wrap(err) } @@ -5455,7 +5467,7 @@ func (a *ServerWithRoles) IsMFARequired(ctx context.Context, req *proto.IsMFAReq // Check if MFA is required for admin actions. We don't currently have // a reason to check the name of the admin action in question. if _, ok := req.Target.(*proto.IsMFARequiredRequest_AdminAction); ok { - if a.context.AdminActionAuthorized { + if a.context.AdminActionAuthState == authz.AdminActionAuthUnauthorized { return &proto.IsMFARequiredResponse{ Required: true, MFARequired: proto.MFARequired_MFA_REQUIRED_YES, @@ -6463,7 +6475,8 @@ func (a *ServerWithRoles) CreateSAMLIdPServiceProvider(ctx context.Context, sp t return trace.Wrap(err) } - if err = authz.AuthorizeAdminAction(ctx, &a.context); err != nil { + // Support reused MFA for bulk tctl create requests. + if err = authz.AuthorizeAdminActionAllowReusedMFA(ctx, &a.context); err != nil { return trace.Wrap(err) } @@ -6501,7 +6514,8 @@ func (a *ServerWithRoles) UpdateSAMLIdPServiceProvider(ctx context.Context, sp t return trace.Wrap(err) } - if err := authz.AuthorizeAdminAction(ctx, &a.context); err != nil { + // Support reused MFA for bulk tctl create requests. + if err := authz.AuthorizeAdminActionAllowReusedMFA(ctx, &a.context); err != nil { return trace.Wrap(err) } diff --git a/lib/auth/auth_with_roles_test.go b/lib/auth/auth_with_roles_test.go index 594e1f48f8af3..4a8c608cf6dc4 100644 --- a/lib/auth/auth_with_roles_test.go +++ b/lib/auth/auth_with_roles_test.go @@ -1824,7 +1824,7 @@ func serverWithAllowRules(t *testing.T, srv *TestAuthServer, allowRules []types. localUser := authz.LocalUser{Username: username, Identity: tlsca.Identity{Username: username}} authContext, err := authz.ContextForLocalUser(ctx, localUser, srv.AuthServer.Services, srv.ClusterName, true /* disableDeviceAuthz */) require.NoError(t, err) - authContext.AdminActionAuthorized = true + authContext.AdminActionAuthState = authz.AdminActionAuthMFAVerified return &ServerWithRoles{ authServer: srv.AuthServer, @@ -6874,3 +6874,54 @@ func inlineEventually(t *testing.T, cond func() bool, waitFor time.Duration, tic } } } + +func TestIsMFARequired_AdminAction(t *testing.T) { + for _, tt := range []struct { + name string + adminActionAuthState authz.AdminActionAuthState + expectResp *proto.IsMFARequiredResponse + }{ + { + name: "unauthorized", + adminActionAuthState: authz.AdminActionAuthUnauthorized, + expectResp: &proto.IsMFARequiredResponse{ + Required: true, + MFARequired: proto.MFARequired_MFA_REQUIRED_YES, + }, + }, { + name: "not required", + adminActionAuthState: authz.AdminActionAuthNotRequired, + expectResp: &proto.IsMFARequiredResponse{ + Required: false, + MFARequired: proto.MFARequired_MFA_REQUIRED_NO, + }, + }, { + name: "mfa verified", + adminActionAuthState: authz.AdminActionAuthMFAVerified, + expectResp: &proto.IsMFARequiredResponse{ + Required: false, + MFARequired: proto.MFARequired_MFA_REQUIRED_NO, + }, + }, { + name: "mfa verified with reuse", + adminActionAuthState: authz.AdminActionAuthMFAVerifiedWithReuse, + expectResp: &proto.IsMFARequiredResponse{ + Required: false, + MFARequired: proto.MFARequired_MFA_REQUIRED_NO, + }, + }, + } { + t.Run(tt.name, func(t *testing.T) { + server := ServerWithRoles{ + context: authz.Context{ + AdminActionAuthState: tt.adminActionAuthState, + }, + } + resp, err := server.IsMFARequired(context.Background(), &proto.IsMFARequiredRequest{ + Target: &proto.IsMFARequiredRequest_AdminAction{}, + }) + require.NoError(t, err) + require.Equal(t, tt.expectResp, resp) + }) + } +} diff --git a/lib/auth/machineid/machineidv1/bot_service.go b/lib/auth/machineid/machineidv1/bot_service.go index 9e6a4f1b6088e..1a0e6003d7ee6 100644 --- a/lib/auth/machineid/machineidv1/bot_service.go +++ b/lib/auth/machineid/machineidv1/bot_service.go @@ -263,7 +263,8 @@ func (bs *BotService) createBotAuthz(ctx context.Context) (*authz.Context, error } bs.logger.Warn("CreateBot authz fell back to legacy resource/verbs. Explicitly grant access to the Bot resource. From V16.0.0, this will fail!") } - if err := authz.AuthorizeAdminAction(ctx, authCtx); err != nil { + // Support reused MFA for bulk tctl create requests. + if err := authz.AuthorizeAdminActionAllowReusedMFA(ctx, authCtx); err != nil { return nil, trace.Wrap(err) } return authCtx, nil @@ -382,7 +383,8 @@ func (bs *BotService) UpsertBot(ctx context.Context, req *pb.UpsertBotRequest) ( if err != nil { return nil, trace.Wrap(err) } - if err := authz.AuthorizeAdminAction(ctx, authCtx); err != nil { + // Support reused MFA for bulk tctl create requests. + if err := authz.AuthorizeAdminActionAllowReusedMFA(ctx, authCtx); err != nil { return nil, trace.Wrap(err) } diff --git a/lib/auth/trust/trustv1/service.go b/lib/auth/trust/trustv1/service.go index 2d901cd1207ae..1a57190c7d812 100644 --- a/lib/auth/trust/trustv1/service.go +++ b/lib/auth/trust/trustv1/service.go @@ -198,7 +198,8 @@ func (s *Service) UpsertCertAuthority(ctx context.Context, req *trustpb.UpsertCe return nil, trace.Wrap(err) } - if err := authz.AuthorizeAdminAction(ctx, authzCtx); err != nil { + // Support reused MFA for bulk tctl create requests. + if err := authz.AuthorizeAdminActionAllowReusedMFA(ctx, authzCtx); err != nil { return nil, trace.Wrap(err) } diff --git a/lib/auth/trust/trustv1/service_test.go b/lib/auth/trust/trustv1/service_test.go index 57196a4cc09b1..e313722a2c64f 100644 --- a/lib/auth/trust/trustv1/service_test.go +++ b/lib/auth/trust/trustv1/service_test.go @@ -73,8 +73,8 @@ func (f *fakeAuthorizer) Authorize(ctx context.Context) (*authz.Context, error) } return &authz.Context{ - Checker: f.checker, - AdminActionAuthorized: true, + Checker: f.checker, + AdminActionAuthState: authz.AdminActionAuthMFAVerified, }, nil } diff --git a/lib/auth/users/usersv1/service.go b/lib/auth/users/usersv1/service.go index 3cf41c29a630e..4b2d3fa287fc5 100644 --- a/lib/auth/users/usersv1/service.go +++ b/lib/auth/users/usersv1/service.go @@ -218,7 +218,8 @@ func (s *Service) CreateUser(ctx context.Context, req *userspb.CreateUserRequest return nil, trace.Wrap(err) } - if err := authz.AuthorizeAdminAction(ctx, authzCtx); err != nil { + // Support reused MFA for bulk tctl create requests and chained invite commands (CreateResetPasswordToken). + if err := authz.AuthorizeAdminActionAllowReusedMFA(ctx, authzCtx); err != nil { return nil, trace.Wrap(err) } @@ -285,7 +286,8 @@ func (s *Service) UpdateUser(ctx context.Context, req *userspb.UpdateUserRequest return nil, trace.Wrap(err) } - if err := authz.AuthorizeAdminAction(ctx, authzCtx); err != nil { + // Allow reused MFA responses to allow Updating a user after get (WebUI). + if err := authz.AuthorizeAdminActionAllowReusedMFA(ctx, authzCtx); err != nil { return nil, trace.Wrap(err) } @@ -367,7 +369,8 @@ func (s *Service) UpsertUser(ctx context.Context, req *userspb.UpsertUserRequest return nil, trace.Wrap(err) } - if err := authz.AuthorizeAdminAction(ctx, authzCtx); err != nil { + // Support reused MFA for bulk tctl create requests. + if err := authz.AuthorizeAdminActionAllowReusedMFA(ctx, authzCtx); err != nil { return nil, trace.Wrap(err) } diff --git a/lib/auth/users/usersv1/service_test.go b/lib/auth/users/usersv1/service_test.go index cb44a6c0a067a..e600c3478a638 100644 --- a/lib/auth/users/usersv1/service_test.go +++ b/lib/auth/users/usersv1/service_test.go @@ -73,8 +73,8 @@ func (a fakeAuthorizer) Authorize(ctx context.Context) (*authz.Context, error) { }, }, }, - Identity: identity, - AdminActionAuthorized: true, + Identity: identity, + AdminActionAuthState: authz.AdminActionAuthNotRequired, }, nil } @@ -103,7 +103,7 @@ func (a fakeAuthorizer) Authorize(ctx context.Context) (*authz.Context, error) { Username: "alice", }, }, - AdminActionAuthorized: true, + AdminActionAuthState: authz.AdminActionAuthNotRequired, }, nil } @@ -888,7 +888,7 @@ func TestRBAC(t *testing.T) { Groups: []string{"dev"}, }, }, - AdminActionAuthorized: true, + AdminActionAuthState: authz.AdminActionAuthNotRequired, }})) require.NoError(t, err, "creating test service") diff --git a/lib/authz/permissions.go b/lib/authz/permissions.go index d77351a46bb61..2722d88f30e59 100644 --- a/lib/authz/permissions.go +++ b/lib/authz/permissions.go @@ -211,9 +211,30 @@ type Context struct { // AdminActionVerified is whether this auth request is verified for admin actions. This // either means that the request was MFA verified through the context or Hardware Key support, // or the identity does not require admin MFA (built in roles, bot impersonated user, etc). + // TODO(Joerger): Deprecated in favor of AdminActionAuthState, remove once e is no longer dependent. AdminActionAuthorized bool + + // AdminActionAuthState is the state of admin action authorization for this auth context. + AdminActionAuthState AdminActionAuthState } +// AdminActionAuthState is an admin action authorization state. +type AdminActionAuthState int + +const ( + // AdminActionAuthUnauthorized admin action is not authorized. + AdminActionAuthUnauthorized AdminActionAuthState = iota + // AdminActionAuthNotRequired admin action authorization is not authorized. + // This state is used for non-user cases, like internal service roles or Machine ID. + AdminActionAuthNotRequired + // AdminActionAuthMFAVerified admin action is authorized with MFA verification. + AdminActionAuthMFAVerified + // AdminActionAuthMFAVerifiedWithReuse admin action is authorized with MFA verification. + // The MFA challenged used for verification allows reuse, which may be denied by some + // admin actions. + AdminActionAuthMFAVerifiedWithReuse +) + // GetUserMetadata returns information about the authenticated identity // to be included in audit events. func (c *Context) GetUserMetadata() apievents.UserMetadata { @@ -394,40 +415,44 @@ func (a *authorizer) fromUser(ctx context.Context, userI interface{}) (*Context, // checkAdminActionVerification checks if this auth request is verified for admin actions. func (a *authorizer) checkAdminActionVerification(ctx context.Context, authContext *Context) error { - err := a.authorizeAdminAction(ctx, authContext) + required, err := a.isAdminActionAuthorizationRequired(ctx, authContext) if err != nil { - if trace.IsNotFound(err) { - // missing MFA verification should be a noop. - return nil - } return trace.Wrap(err) } - authContext.AdminActionAuthorized = true + if !required { + authContext.AdminActionAuthState = AdminActionAuthNotRequired + return nil + } + + if err := a.authorizeAdminAction(ctx, authContext); err != nil { + return trace.Wrap(err) + } + return nil } -func (a *authorizer) authorizeAdminAction(ctx context.Context, authContext *Context) error { +func (a *authorizer) isAdminActionAuthorizationRequired(ctx context.Context, authContext *Context) (bool, error) { // Builtin roles do not require MFA to perform admin actions. switch authContext.Identity.(type) { case BuiltinRole, RemoteBuiltinRole: - return nil + return false, nil } authpref, err := a.accessPoint.GetAuthPreference(ctx) if err != nil { - return trace.Wrap(err) + return false, trace.Wrap(err) } // Admin actions do not require MFA when Webauthn is not enabled. if authpref.GetPreferredLocalMFA() != constants.SecondFactorWebauthn { - return nil + return false, nil } // Skip MFA check if the user is a Bot. if user, err := a.accessPoint.GetUser(ctx, authContext.Identity.GetIdentity().Username, false); err == nil && user.IsBot() { a.logger.Debugf("Skipping admin action MFA check for bot identity: %v", authContext.Identity.GetIdentity()) - return nil + return false, nil } // Skip MFA if the identity is being impersonated by the Bot or Admin built in role. @@ -435,7 +460,7 @@ func (a *authorizer) authorizeAdminAction(ctx context.Context, authContext *Cont impersonatorUser, err := a.accessPoint.GetUser(ctx, impersonator, false) if err == nil && impersonatorUser.IsBot() { a.logger.Debugf("Skipping admin action MFA check for bot-impersonated identity: %v", authContext.Identity.GetIdentity()) - return nil + return false, nil } // If we don't find a user matching the impersonator, it may be the admin role impersonating. @@ -445,20 +470,29 @@ func (a *authorizer) authorizeAdminAction(ctx context.Context, authContext *Cont if hostFQDNParts[1] == a.clusterName { if _, err := uuid.Parse(hostFQDNParts[0]); err == nil { a.logger.Debugf("Skipping admin action MFA check for admin-impersonated identity: %v", authContext.Identity.GetIdentity()) - return nil + return false, nil } } } } + return true, nil +} + +func (a *authorizer) authorizeAdminAction(ctx context.Context, authContext *Context) error { // Certain hardware-key based private key policies require MFA for each request. if authContext.Identity.GetIdentity().PrivateKeyPolicy.MFAVerified() { + authContext.AdminActionAuthState = AdminActionAuthMFAVerified return nil } // MFA is required to be passed through the request context. mfaResp, err := mfa.CredentialsFromContext(ctx) if err != nil { + if trace.IsNotFound(err) { + // missing MFA verification should be a noop. + return nil + } return trace.Wrap(err) } @@ -467,11 +501,16 @@ func (a *authorizer) authorizeAdminAction(ctx context.Context, authContext *Cont } requiredExt := &mfav1.ChallengeExtensions{Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_ADMIN_ACTION} - _, err = a.mfaAuthenticator.ValidateMFAAuthResponse(ctx, mfaResp, authContext.User.GetName(), requiredExt) + mfaData, err := a.mfaAuthenticator.ValidateMFAAuthResponse(ctx, mfaResp, authContext.User.GetName(), requiredExt) if err != nil { return trace.Wrap(err) } + authContext.AdminActionAuthState = AdminActionAuthMFAVerified + if mfaData.AllowReuse == mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_YES { + authContext.AdminActionAuthState = AdminActionAuthMFAVerifiedWithReuse + } + return nil } @@ -1142,8 +1181,8 @@ func ContextForBuiltinRole(r BuiltinRole, recConfig types.SessionRecordingConfig Checker: checker, Identity: r, UnmappedIdentity: r, - disableDeviceRoleMode: true, // Builtin roles skip device trust. - AdminActionAuthorized: true, // builtin roles skip mfa for admin actions. + disableDeviceRoleMode: true, // Builtin roles skip device trust. + AdminActionAuthState: AdminActionAuthNotRequired, // builtin roles skip mfa for admin actions. }, nil } @@ -1367,10 +1406,24 @@ func AuthorizeContextWithVerbs(ctx context.Context, log logrus.FieldLogger, auth // AuthorizeAdminAction will ensure that the user is authorized to perform admin actions. func AuthorizeAdminAction(ctx context.Context, authCtx *Context) error { - if !authCtx.AdminActionAuthorized { - return trace.Wrap(&mfa.ErrAdminActionMFARequired) + // TODO(Joerger): AdminActionAuthorized is deprecated in favor of AdminActionAuthState, remove once e is no longer dependent. + if authCtx.AdminActionAuthorized { + return nil } - return nil + switch authCtx.AdminActionAuthState { + case AdminActionAuthMFAVerified, AdminActionAuthNotRequired: + return nil + } + return trace.Wrap(&mfa.ErrAdminActionMFARequired) +} + +// AuthorizeAdminActionAllowReusedMFA will ensure that the user is authorized to perform +// admin actions. Additionally, MFA challenges that allow reuse will be accepted. +func AuthorizeAdminActionAllowReusedMFA(ctx context.Context, authCtx *Context) error { + if authCtx.AdminActionAuthState == AdminActionAuthMFAVerifiedWithReuse { + return nil + } + return AuthorizeAdminAction(ctx, authCtx) } // LocalUser is a local user diff --git a/lib/authz/permissions_test.go b/lib/authz/permissions_test.go index 73779ed3417b1..8b0fe400f6a37 100644 --- a/lib/authz/permissions_test.go +++ b/lib/authz/permissions_test.go @@ -511,12 +511,37 @@ func TestAuthorizer_AuthorizeAdminAction(t *testing.T) { }) require.NoError(t, err, "NewAuthorizer failed") + validMFA := &proto.MFAAuthenticateResponse{ + Response: &proto.MFAAuthenticateResponse_TOTP{ + TOTP: &proto.TOTPResponse{ + Code: validTOTPCode, + }, + }, + } + + validMFAWithReuse := &proto.MFAAuthenticateResponse{ + Response: &proto.MFAAuthenticateResponse_TOTP{ + TOTP: &proto.TOTPResponse{ + Code: validReusableTOTPCode, + }, + }, + } + + invalidMFA := &proto.MFAAuthenticateResponse{ + Response: &proto.MFAAuthenticateResponse_TOTP{ + TOTP: &proto.TOTPResponse{ + Code: "invalid", + }, + }, + } + for _, tt := range []struct { name string user IdentityGetter - withTOTPInContext string + withMFA *proto.MFAAuthenticateResponse + allowedReusedMFA bool contextGetter func() context.Context - wantErr string + wantErrContains string wantAdminActionAuthorized bool }{ { @@ -527,10 +552,9 @@ func TestAuthorizer_AuthorizeAdminAction(t *testing.T) { Username: localUser.GetName(), }, }, - wantErr: "", wantAdminActionAuthorized: false, }, { - name: "NOK local user with mfa verified cert", + name: "NOK local user mfa verified cert", user: LocalUser{ Username: localUser.GetName(), Identity: tlsca.Identity{ @@ -538,7 +562,6 @@ func TestAuthorizer_AuthorizeAdminAction(t *testing.T) { MFAVerified: "mfa-verified-test", }, }, - wantErr: "", wantAdminActionAuthorized: false, }, { // edge case for the admin role check. @@ -549,32 +572,51 @@ func TestAuthorizer_AuthorizeAdminAction(t *testing.T) { Username: userWithHostName.GetName(), }, }, - wantErr: "", wantAdminActionAuthorized: false, }, { - name: "NOK local user with invalid mfa from context", + name: "NOK local user invalid mfa", + user: LocalUser{ + Username: localUser.GetName(), + Identity: tlsca.Identity{ + Username: localUser.GetName(), + }, + }, + withMFA: invalidMFA, + wantErrContains: "invalid MFA", + wantAdminActionAuthorized: true, + }, { + name: "NOK local user reused mfa with reuse not allowed", + user: LocalUser{ + Username: localUser.GetName(), + Identity: tlsca.Identity{ + Username: localUser.GetName(), + }, + }, + withMFA: validMFAWithReuse, + wantAdminActionAuthorized: false, + }, { + name: "OK local user valid mfa", user: LocalUser{ Username: localUser.GetName(), Identity: tlsca.Identity{ Username: localUser.GetName(), }, }, - withTOTPInContext: "invalid", - wantErr: "invalid MFA", + withMFA: validMFA, wantAdminActionAuthorized: true, }, { - name: "OK local user with valid mfa from context", + name: "OK local user reused mfa with reuse allowed", user: LocalUser{ Username: localUser.GetName(), Identity: tlsca.Identity{ Username: localUser.GetName(), }, }, - withTOTPInContext: validTOTPCode, - wantErr: "", + withMFA: validMFAWithReuse, + allowedReusedMFA: true, wantAdminActionAuthorized: true, }, { - name: "OK local user with mfa verified private key policy", + name: "OK local user mfa verified private key policy", user: LocalUser{ Username: localUser.GetName(), Identity: tlsca.Identity{ @@ -582,7 +624,6 @@ func TestAuthorizer_AuthorizeAdminAction(t *testing.T) { PrivateKeyPolicy: keys.PrivateKeyPolicyHardwareKeyTouch, }, }, - wantErr: "", wantAdminActionAuthorized: true, }, { name: "OK admin", @@ -590,7 +631,6 @@ func TestAuthorizer_AuthorizeAdminAction(t *testing.T) { Role: types.RoleAdmin, Username: hostFQDN(uuid.NewString(), clusterName), }, - wantErr: "", wantAdminActionAuthorized: true, }, { name: "OK bot", @@ -600,7 +640,6 @@ func TestAuthorizer_AuthorizeAdminAction(t *testing.T) { Username: bot.GetName(), }, }, - wantErr: "", wantAdminActionAuthorized: true, }, { name: "OK admin impersonating local user", @@ -611,7 +650,6 @@ func TestAuthorizer_AuthorizeAdminAction(t *testing.T) { Impersonator: hostFQDN(uuid.NewString(), clusterName), }, }, - wantErr: "", wantAdminActionAuthorized: true, }, { name: "OK bot impersonating local user", @@ -622,21 +660,13 @@ func TestAuthorizer_AuthorizeAdminAction(t *testing.T) { Impersonator: bot.GetName(), }, }, - wantErr: "", wantAdminActionAuthorized: true, }, } { t.Run(tt.name, func(t *testing.T) { ctx := context.Background() - if tt.withTOTPInContext != "" { - mfaResp := &proto.MFAAuthenticateResponse{ - Response: &proto.MFAAuthenticateResponse_TOTP{ - TOTP: &proto.TOTPResponse{ - Code: tt.withTOTPInContext, - }, - }, - } - encodedMFAResp, err := mfa.EncodeMFAChallengeResponseCredentials(mfaResp) + if tt.withMFA != nil { + encodedMFAResp, err := mfa.EncodeMFAChallengeResponseCredentials(tt.withMFA) require.NoError(t, err) md := metadata.MD(map[string][]string{ mfa.ResponseMetadataKey: {encodedMFAResp}, @@ -645,13 +675,19 @@ func TestAuthorizer_AuthorizeAdminAction(t *testing.T) { } userCtx := context.WithValue(ctx, contextUser, tt.user) authCtx, err := authorizer.Authorize(userCtx) - if tt.wantErr != "" { - require.ErrorContains(t, err, tt.wantErr, "Expected matching Authorize error") + if tt.wantErrContains != "" { + require.ErrorContains(t, err, tt.wantErrContains, "Expected matching Authorize error") return } require.NoError(t, err) - authAdminActionErr := AuthorizeAdminAction(ctx, authCtx) + var authAdminActionErr error + if tt.allowedReusedMFA { + authAdminActionErr = AuthorizeAdminActionAllowReusedMFA(ctx, authCtx) + } else { + authAdminActionErr = AuthorizeAdminAction(ctx, authCtx) + } + if tt.wantAdminActionAuthorized { require.NoError(t, authAdminActionErr) } else {