Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 29 additions & 15 deletions lib/auth/auth_with_roles.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down Expand Up @@ -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)
}

Expand Down Expand Up @@ -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)
}

Expand Down Expand Up @@ -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)
}

Expand Down Expand Up @@ -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)
}

Expand Down Expand Up @@ -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)
}

Expand All @@ -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)
}

Expand Down Expand Up @@ -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)
}

Expand Down Expand Up @@ -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)
}

Expand Down Expand Up @@ -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)
}

Expand Down Expand Up @@ -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)
}

Expand Down Expand Up @@ -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)
}

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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)
}

Expand Down Expand Up @@ -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)
}

Expand Down
53 changes: 52 additions & 1 deletion lib/auth/auth_with_roles_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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)
})
}
}
6 changes: 4 additions & 2 deletions lib/auth/machineid/machineidv1/bot_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
}

Expand Down
3 changes: 2 additions & 1 deletion lib/auth/trust/trustv1/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
4 changes: 2 additions & 2 deletions lib/auth/trust/trustv1/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
9 changes: 6 additions & 3 deletions lib/auth/users/usersv1/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down Expand Up @@ -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)
}

Expand Down Expand Up @@ -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)
}

Expand Down
8 changes: 4 additions & 4 deletions lib/auth/users/usersv1/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,8 @@ func (a fakeAuthorizer) Authorize(ctx context.Context) (*authz.Context, error) {
},
},
},
Identity: identity,
AdminActionAuthorized: true,
Identity: identity,
AdminActionAuthState: authz.AdminActionAuthNotRequired,
}, nil
}

Expand Down Expand Up @@ -103,7 +103,7 @@ func (a fakeAuthorizer) Authorize(ctx context.Context) (*authz.Context, error) {
Username: "alice",
},
},
AdminActionAuthorized: true,
AdminActionAuthState: authz.AdminActionAuthNotRequired,
}, nil
}

Expand Down Expand Up @@ -888,7 +888,7 @@ func TestRBAC(t *testing.T) {
Groups: []string{"dev"},
},
},
AdminActionAuthorized: true,
AdminActionAuthState: authz.AdminActionAuthNotRequired,
}}))
require.NoError(t, err, "creating test service")

Expand Down
Loading