Skip to content

Commit

Permalink
[v17] Update several admin actions to allow reusable MFA challenges (#…
Browse files Browse the repository at this point in the history
…49236)

* Allow reused MFA for more admin action endpoints; Add comment to AuthorizeAdminAction."

* Add update to when to extend reuse section of RFD 155.
  • Loading branch information
Joerger authored Nov 20, 2024
1 parent dbfa596 commit a9e1ff5
Show file tree
Hide file tree
Showing 21 changed files with 233 additions and 126 deletions.
44 changes: 22 additions & 22 deletions lib/auth/auth_with_roles.go
Original file line number Diff line number Diff line change
Expand Up @@ -2141,7 +2141,7 @@ func (a *ServerWithRoles) DeleteToken(ctx context.Context, token string) error {
return trace.Wrap(err)
}

if err := a.context.AuthorizeAdminAction(); err != nil {
if err := a.context.AuthorizeAdminActionAllowReusedMFA(); err != nil {
return trace.Wrap(err)
}

Expand Down Expand Up @@ -2742,7 +2742,7 @@ func (a *ServerWithRoles) DeleteAccessRequest(ctx context.Context, name string)
return trace.Wrap(err)
}

if err := a.context.AuthorizeAdminAction(); err != nil {
if err := a.context.AuthorizeAdminActionAllowReusedMFA(); err != nil {
return trace.Wrap(err)
}

Expand Down Expand Up @@ -3507,7 +3507,7 @@ func (a *ServerWithRoles) UpdateOIDCConnector(ctx context.Context, connector typ
return nil, trace.AccessDenied("OIDC is only available in Teleport Enterprise")
}

if err := a.context.AuthorizeAdminAction(); err != nil {
if err := a.context.AuthorizeAdminActionAllowReusedMFA(); err != nil {
return nil, trace.Wrap(err)
}

Expand Down Expand Up @@ -3629,7 +3629,7 @@ func (a *ServerWithRoles) DeleteOIDCConnector(ctx context.Context, connectorID s
return trace.Wrap(err)
}

if err := a.context.AuthorizeAdminAction(); err != nil {
if err := a.context.AuthorizeAdminActionAllowReusedMFA(); err != nil {
return trace.Wrap(err)
}

Expand Down Expand Up @@ -3687,7 +3687,7 @@ func (a *ServerWithRoles) UpdateSAMLConnector(ctx context.Context, connector typ
return nil, trace.Wrap(err)
}

if err := a.context.AuthorizeAdminAction(); err != nil {
if err := a.context.AuthorizeAdminActionAllowReusedMFA(); err != nil {
return nil, trace.Wrap(err)
}

Expand Down Expand Up @@ -3816,7 +3816,7 @@ func (a *ServerWithRoles) DeleteSAMLConnector(ctx context.Context, connectorID s
return trace.Wrap(err)
}

if err := a.context.AuthorizeAdminAction(); err != nil {
if err := a.context.AuthorizeAdminActionAllowReusedMFA(); err != nil {
return trace.Wrap(err)
}

Expand Down Expand Up @@ -3890,7 +3890,7 @@ func (a *ServerWithRoles) UpdateGithubConnector(ctx context.Context, connector t
return nil, trace.Wrap(err)
}

if err := a.context.AuthorizeAdminAction(); err != nil {
if err := a.context.AuthorizeAdminActionAllowReusedMFA(); err != nil {
return nil, trace.Wrap(err)
}

Expand Down Expand Up @@ -3935,7 +3935,7 @@ func (a *ServerWithRoles) DeleteGithubConnector(ctx context.Context, connectorID
return trace.Wrap(err)
}

if err := a.context.AuthorizeAdminAction(); err != nil {
if err := a.context.AuthorizeAdminActionAllowReusedMFA(); err != nil {
return trace.Wrap(err)
}

Expand Down Expand Up @@ -4244,7 +4244,7 @@ func (a *ServerWithRoles) CreateRole(ctx context.Context, role types.Role) (type
return nil, trace.Wrap(err)
}

if err := a.context.AuthorizeAdminAction(); err != nil {
if err := a.context.AuthorizeAdminActionAllowReusedMFA(); err != nil {
return nil, trace.Wrap(err)
}

Expand Down Expand Up @@ -4276,7 +4276,7 @@ func (a *ServerWithRoles) UpdateRole(ctx context.Context, role types.Role) (type
return nil, trace.Wrap(err)
}

if err := a.context.AuthorizeAdminAction(); err != nil {
if err := a.context.AuthorizeAdminActionAllowReusedMFA(); err != nil {
return nil, trace.Wrap(err)
}

Expand Down Expand Up @@ -4394,7 +4394,7 @@ func (a *ServerWithRoles) DeleteRole(ctx context.Context, name string) error {
return trace.Wrap(err)
}

if err := a.context.AuthorizeAdminAction(); err != nil {
if err := a.context.AuthorizeAdminActionAllowReusedMFA(); err != nil {
return trace.Wrap(err)
}

Expand Down Expand Up @@ -4592,7 +4592,7 @@ func (a *ServerWithRoles) ResetAuthPreference(ctx context.Context) error {
return trace.Wrap(err)
}

if err := a.context.AuthorizeAdminAction(); err != nil {
if err := a.context.AuthorizeAdminActionAllowReusedMFA(); err != nil {
return trace.Wrap(err)
}

Expand Down Expand Up @@ -4735,7 +4735,7 @@ func (a *ServerWithRoles) ResetClusterNetworkingConfig(ctx context.Context) erro
return trace.Wrap(err)
}

if err := a.context.AuthorizeAdminAction(); err != nil {
if err := a.context.AuthorizeAdminActionAllowReusedMFA(); err != nil {
return trace.Wrap(err)
}

Expand Down Expand Up @@ -4833,7 +4833,7 @@ func (a *ServerWithRoles) ResetSessionRecordingConfig(ctx context.Context) error
}
}

if err := a.context.AuthorizeAdminAction(); err != nil {
if err := a.context.AuthorizeAdminActionAllowReusedMFA(); err != nil {
return trace.Wrap(err)
}

Expand Down Expand Up @@ -4935,7 +4935,7 @@ func (a *ServerWithRoles) UpsertTrustedCluster(ctx context.Context, tc types.Tru
return nil, trace.Wrap(err)
}

if err := a.context.AuthorizeAdminAction(); err != nil {
if err := a.context.AuthorizeAdminActionAllowReusedMFA(); err != nil {
return nil, trace.Wrap(err)
}

Expand All @@ -4958,7 +4958,7 @@ func (a *ServerWithRoles) DeleteTrustedCluster(ctx context.Context, name string)
return trace.Wrap(err)
}

if err := a.context.AuthorizeAdminAction(); err != nil {
if err := a.context.AuthorizeAdminActionAllowReusedMFA(); err != nil {
return trace.Wrap(err)
}

Expand Down Expand Up @@ -5728,7 +5728,7 @@ func (a *ServerWithRoles) DeleteNetworkRestrictions(ctx context.Context) error {
return trace.Wrap(err)
}

if err := a.context.AuthorizeAdminAction(); err != nil {
if err := a.context.AuthorizeAdminActionAllowReusedMFA(); err != nil {
return trace.Wrap(err)
}

Expand Down Expand Up @@ -6954,7 +6954,7 @@ func (a *ServerWithRoles) DeleteSAMLIdPServiceProvider(ctx context.Context, name
return trace.Wrap(err)
}

if err := a.context.AuthorizeAdminAction(); err != nil {
if err := a.context.AuthorizeAdminActionAllowReusedMFA(); err != nil {
return trace.Wrap(err)
}

Expand Down Expand Up @@ -7083,7 +7083,7 @@ func (a *ServerWithRoles) CreateUserGroup(ctx context.Context, userGroup types.U
return trace.Wrap(err)
}

if err := a.context.AuthorizeAdminAction(); err != nil {
if err := a.context.AuthorizeAdminActionAllowReusedMFA(); err != nil {
return trace.Wrap(err)
}

Expand All @@ -7100,7 +7100,7 @@ func (a *ServerWithRoles) UpdateUserGroup(ctx context.Context, userGroup types.U
return trace.Wrap(err)
}

if err := a.context.AuthorizeAdminAction(); err != nil {
if err := a.context.AuthorizeAdminActionAllowReusedMFA(); err != nil {
return trace.Wrap(err)
}

Expand All @@ -7122,7 +7122,7 @@ func (a *ServerWithRoles) DeleteUserGroup(ctx context.Context, name string) erro
return trace.Wrap(err)
}

if err := a.context.AuthorizeAdminAction(); err != nil {
if err := a.context.AuthorizeAdminActionAllowReusedMFA(); err != nil {
return trace.Wrap(err)
}

Expand All @@ -7144,7 +7144,7 @@ func (a *ServerWithRoles) DeleteAllUserGroups(ctx context.Context) error {
return trace.Wrap(err)
}

if err := a.context.AuthorizeAdminAction(); err != nil {
if err := a.context.AuthorizeAdminActionAllowReusedMFA(); err != nil {
return trace.Wrap(err)
}

Expand Down
6 changes: 3 additions & 3 deletions lib/auth/autoupdate/autoupdatev1/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ func (s *Service) DeleteAutoUpdateConfig(ctx context.Context, req *autoupdate.De
return nil, trace.Wrap(err)
}

if err := authCtx.AuthorizeAdminAction(); err != nil {
if err := authCtx.AuthorizeAdminActionAllowReusedMFA(); err != nil {
return nil, trace.Wrap(err)
}

Expand Down Expand Up @@ -419,7 +419,7 @@ func (s *Service) DeleteAutoUpdateVersion(ctx context.Context, req *autoupdate.D
return nil, trace.Wrap(err)
}

if err := authCtx.AuthorizeAdminAction(); err != nil {
if err := authCtx.AuthorizeAdminActionAllowReusedMFA(); err != nil {
return nil, trace.Wrap(err)
}

Expand Down Expand Up @@ -571,7 +571,7 @@ func (s *Service) DeleteAutoUpdateAgentRollout(ctx context.Context, req *autoupd
return nil, trace.Wrap(err)
}

if err := authCtx.AuthorizeAdminAction(); err != nil {
if err := authCtx.AuthorizeAdminActionAllowReusedMFA(); err != nil {
return nil, trace.Wrap(err)
}

Expand Down
32 changes: 22 additions & 10 deletions lib/auth/autoupdate/autoupdatev1/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,9 +125,13 @@ func TestServiceAccess(t *testing.T) {
allowedVerbs: []string{types.VerbRead},
},
{
name: "DeleteAutoUpdateConfig",
allowedStates: []authz.AdminActionAuthState{authz.AdminActionAuthNotRequired, authz.AdminActionAuthMFAVerified},
allowedVerbs: []string{types.VerbDelete},
name: "DeleteAutoUpdateConfig",
allowedStates: []authz.AdminActionAuthState{
authz.AdminActionAuthNotRequired,
authz.AdminActionAuthMFAVerified,
authz.AdminActionAuthMFAVerifiedWithReuse,
},
allowedVerbs: []string{types.VerbDelete},
},
// AutoUpdate version check.
{
Expand Down Expand Up @@ -168,9 +172,13 @@ func TestServiceAccess(t *testing.T) {
allowedVerbs: []string{types.VerbRead},
},
{
name: "DeleteAutoUpdateVersion",
allowedStates: []authz.AdminActionAuthState{authz.AdminActionAuthNotRequired, authz.AdminActionAuthMFAVerified},
allowedVerbs: []string{types.VerbDelete},
name: "DeleteAutoUpdateVersion",
allowedStates: []authz.AdminActionAuthState{
authz.AdminActionAuthNotRequired,
authz.AdminActionAuthMFAVerified,
authz.AdminActionAuthMFAVerifiedWithReuse,
},
allowedVerbs: []string{types.VerbDelete},
},
// AutoUpdate agent rollout check.
{
Expand Down Expand Up @@ -214,10 +222,14 @@ func TestServiceAccess(t *testing.T) {
builtinRole: &authz.BuiltinRole{Role: types.RoleAuth},
},
{
name: "DeleteAutoUpdateAgentRollout",
allowedStates: []authz.AdminActionAuthState{authz.AdminActionAuthNotRequired, authz.AdminActionAuthMFAVerified},
allowedVerbs: []string{types.VerbDelete},
builtinRole: &authz.BuiltinRole{Role: types.RoleAuth},
name: "DeleteAutoUpdateAgentRollout",
allowedStates: []authz.AdminActionAuthState{
authz.AdminActionAuthNotRequired,
authz.AdminActionAuthMFAVerified,
authz.AdminActionAuthMFAVerifiedWithReuse,
},
allowedVerbs: []string{types.VerbDelete},
builtinRole: &authz.BuiltinRole{Role: types.RoleAuth},
},
}

Expand Down
6 changes: 3 additions & 3 deletions lib/auth/clusterconfig/clusterconfigv1/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -357,7 +357,7 @@ func (s *Service) ResetAuthPreference(ctx context.Context, _ *clusterconfigpb.Re
return nil, trace.Wrap(err)
}

if err := authzCtx.AuthorizeAdminAction(); err != nil {
if err := authzCtx.AuthorizeAdminActionAllowReusedMFA(); err != nil {
return nil, trace.Wrap(err)
}

Expand Down Expand Up @@ -627,7 +627,7 @@ func (s *Service) ResetClusterNetworkingConfig(ctx context.Context, _ *clusterco
}
}

if err := authzCtx.AuthorizeAdminAction(); err != nil {
if err := authzCtx.AuthorizeAdminActionAllowReusedMFA(); err != nil {
return nil, trace.Wrap(err)
}

Expand Down Expand Up @@ -881,7 +881,7 @@ func (s *Service) ResetSessionRecordingConfig(ctx context.Context, _ *clustercon
}
}

if err := authzCtx.AuthorizeAdminAction(); err != nil {
if err := authzCtx.AuthorizeAdminActionAllowReusedMFA(); err != nil {
return nil, trace.Wrap(err)
}
defaultConfig := types.DefaultSessionRecordingConfig()
Expand Down
10 changes: 4 additions & 6 deletions lib/auth/crownjewel/crownjewelv1/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ func (s *Service) CreateCrownJewel(ctx context.Context, req *crownjewelv1.Create
return nil, trace.Wrap(err)
}

if err := authCtx.AuthorizeAdminAction(); err != nil {
if err := authCtx.AuthorizeAdminActionAllowReusedMFA(); err != nil {
return nil, trace.Wrap(err)
}

Expand Down Expand Up @@ -186,7 +186,6 @@ func (s *Service) GetCrownJewel(ctx context.Context, req *crownjewelv1.GetCrownJ
}

return rsp, nil

}

// UpdateCrownJewel updates crown jewel resource.
Expand All @@ -200,7 +199,7 @@ func (s *Service) UpdateCrownJewel(ctx context.Context, req *crownjewelv1.Update
return nil, trace.Wrap(err)
}

if err := authCtx.AuthorizeAdminAction(); err != nil {
if err := authCtx.AuthorizeAdminActionAllowReusedMFA(); err != nil {
return nil, trace.Wrap(err)
}

Expand Down Expand Up @@ -252,7 +251,7 @@ func (s *Service) UpsertCrownJewel(ctx context.Context, req *crownjewelv1.Upsert
return nil, trace.Wrap(err)
}

if err := authCtx.AuthorizeAdminAction(); err != nil {
if err := authCtx.AuthorizeAdminActionAllowReusedMFA(); err != nil {
return nil, trace.Wrap(err)
}

Expand All @@ -274,7 +273,6 @@ func (s *Service) UpsertCrownJewel(ctx context.Context, req *crownjewelv1.Upsert
}

return rsp, nil

}

func (s *Service) emitUpsertAuditEvent(ctx context.Context, old, new *crownjewelv1.CrownJewel, authCtx *authz.Context, err error) {
Expand All @@ -296,7 +294,7 @@ func (s *Service) DeleteCrownJewel(ctx context.Context, req *crownjewelv1.Delete
return nil, trace.Wrap(err)
}

if err := authCtx.AuthorizeAdminAction(); err != nil {
if err := authCtx.AuthorizeAdminActionAllowReusedMFA(); err != nil {
return nil, trace.Wrap(err)
}

Expand Down
Loading

0 comments on commit a9e1ff5

Please sign in to comment.