Update several admin actions to allow reusable MFA challenges (#49095)

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

* Add update to when to extend reuse section of RFD 155.
This commit is contained in:
Brian Joerger
2024-11-19 18:56:32 +00:00
committed by GitHub
parent 46e2fb26f1
commit cf1dee4a29
21 changed files with 233 additions and 126 deletions
+22 -22
View File
@@ -2135,7 +2135,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)
}
@@ -2736,7 +2736,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)
}
@@ -3501,7 +3501,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)
}
@@ -3623,7 +3623,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)
}
@@ -3681,7 +3681,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)
}
@@ -3810,7 +3810,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)
}
@@ -3884,7 +3884,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)
}
@@ -3929,7 +3929,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)
}
@@ -4238,7 +4238,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)
}
@@ -4270,7 +4270,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)
}
@@ -4388,7 +4388,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)
}
@@ -4586,7 +4586,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)
}
@@ -4729,7 +4729,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)
}
@@ -4827,7 +4827,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)
}
@@ -4929,7 +4929,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)
}
@@ -4952,7 +4952,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)
}
@@ -5722,7 +5722,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)
}
@@ -6997,7 +6997,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)
}
@@ -7147,7 +7147,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)
}
@@ -7164,7 +7164,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)
}
@@ -7186,7 +7186,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)
}
@@ -7208,7 +7208,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)
}
+3 -3
View File
@@ -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)
}
@@ -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)
}
@@ -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)
}
@@ -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.
{
@@ -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.
{
@@ -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},
},
}
@@ -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)
}
@@ -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)
}
@@ -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()
+4 -6
View File
@@ -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)
}
@@ -186,7 +186,6 @@ func (s *Service) GetCrownJewel(ctx context.Context, req *crownjewelv1.GetCrownJ
}
return rsp, nil
}
// UpdateCrownJewel updates crown jewel resource.
@@ -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)
}
@@ -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)
}
@@ -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) {
@@ -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)
}
@@ -47,24 +47,40 @@ func TestServiceAccess(t *testing.T) {
allowedStates []authz.AdminActionAuthState
}{
{
name: "CreateCrownJewel",
allowedStates: []authz.AdminActionAuthState{authz.AdminActionAuthNotRequired, authz.AdminActionAuthMFAVerified},
allowedVerbs: []string{types.VerbCreate},
name: "CreateCrownJewel",
allowedStates: []authz.AdminActionAuthState{
authz.AdminActionAuthNotRequired,
authz.AdminActionAuthMFAVerified,
authz.AdminActionAuthMFAVerifiedWithReuse,
},
allowedVerbs: []string{types.VerbCreate},
},
{
name: "UpdateCrownJewel",
allowedStates: []authz.AdminActionAuthState{authz.AdminActionAuthNotRequired, authz.AdminActionAuthMFAVerified},
allowedVerbs: []string{types.VerbUpdate},
name: "UpdateCrownJewel",
allowedStates: []authz.AdminActionAuthState{
authz.AdminActionAuthNotRequired,
authz.AdminActionAuthMFAVerified,
authz.AdminActionAuthMFAVerifiedWithReuse,
},
allowedVerbs: []string{types.VerbUpdate},
},
{
name: "DeleteCrownJewel",
allowedStates: []authz.AdminActionAuthState{authz.AdminActionAuthNotRequired, authz.AdminActionAuthMFAVerified},
allowedVerbs: []string{types.VerbDelete},
name: "DeleteCrownJewel",
allowedStates: []authz.AdminActionAuthState{
authz.AdminActionAuthNotRequired,
authz.AdminActionAuthMFAVerified,
authz.AdminActionAuthMFAVerifiedWithReuse,
},
allowedVerbs: []string{types.VerbDelete},
},
{
name: "UpsertCrownJewel",
allowedStates: []authz.AdminActionAuthState{authz.AdminActionAuthNotRequired, authz.AdminActionAuthMFAVerified},
allowedVerbs: []string{types.VerbCreate, types.VerbUpdate},
name: "UpsertCrownJewel",
allowedStates: []authz.AdminActionAuthState{
authz.AdminActionAuthNotRequired,
authz.AdminActionAuthMFAVerified,
authz.AdminActionAuthMFAVerifiedWithReuse,
},
allowedVerbs: []string{types.VerbCreate, types.VerbUpdate},
},
{
name: "ListCrownJewels",
+1 -2
View File
@@ -87,7 +87,7 @@ func (rs *DatabaseObjectService) authorize(ctx context.Context, adminAction bool
}
if adminAction {
err = authCtx.AuthorizeAdminAction()
err = authCtx.AuthorizeAdminActionAllowReusedMFA()
if err != nil {
return trace.Wrap(err)
}
@@ -153,7 +153,6 @@ func (rs *DatabaseObjectService) CreateDatabaseObject(
return nil, trace.Wrap(err)
}
return out, nil
}
// UpsertDatabaseObject creates a new DatabaseObject or forcefully updates an existing DatabaseObject.
+28 -12
View File
@@ -82,24 +82,40 @@ func TestServiceAccess(t *testing.T) {
allowedStates []authz.AdminActionAuthState
}{
{
name: "UpsertDatabaseObject",
allowedStates: []authz.AdminActionAuthState{authz.AdminActionAuthNotRequired, authz.AdminActionAuthMFAVerified},
allowedVerbs: []string{types.VerbUpdate, types.VerbCreate},
name: "UpsertDatabaseObject",
allowedStates: []authz.AdminActionAuthState{
authz.AdminActionAuthNotRequired,
authz.AdminActionAuthMFAVerified,
authz.AdminActionAuthMFAVerifiedWithReuse,
},
allowedVerbs: []string{types.VerbUpdate, types.VerbCreate},
},
{
name: "CreateDatabaseObject",
allowedStates: []authz.AdminActionAuthState{authz.AdminActionAuthNotRequired, authz.AdminActionAuthMFAVerified},
allowedVerbs: []string{types.VerbCreate},
name: "CreateDatabaseObject",
allowedStates: []authz.AdminActionAuthState{
authz.AdminActionAuthNotRequired,
authz.AdminActionAuthMFAVerified,
authz.AdminActionAuthMFAVerifiedWithReuse,
},
allowedVerbs: []string{types.VerbCreate},
},
{
name: "UpdateDatabaseObject",
allowedStates: []authz.AdminActionAuthState{authz.AdminActionAuthNotRequired, authz.AdminActionAuthMFAVerified},
allowedVerbs: []string{types.VerbUpdate},
name: "UpdateDatabaseObject",
allowedStates: []authz.AdminActionAuthState{
authz.AdminActionAuthNotRequired,
authz.AdminActionAuthMFAVerified,
authz.AdminActionAuthMFAVerifiedWithReuse,
},
allowedVerbs: []string{types.VerbUpdate},
},
{
name: "DeleteDatabaseObject",
allowedStates: []authz.AdminActionAuthState{authz.AdminActionAuthNotRequired, authz.AdminActionAuthMFAVerified},
allowedVerbs: []string{types.VerbDelete},
name: "DeleteDatabaseObject",
allowedStates: []authz.AdminActionAuthState{
authz.AdminActionAuthNotRequired,
authz.AdminActionAuthMFAVerified,
authz.AdminActionAuthMFAVerifiedWithReuse,
},
allowedVerbs: []string{types.VerbDelete},
},
{
name: "GetDatabaseObject",
@@ -87,7 +87,7 @@ func (rs *DatabaseObjectImportRuleService) authorize(ctx context.Context, adminA
}
if adminAction {
err = authCtx.AuthorizeAdminAction()
err = authCtx.AuthorizeAdminActionAllowReusedMFA()
if err != nil {
return trace.Wrap(err)
}
@@ -153,7 +153,6 @@ func (rs *DatabaseObjectImportRuleService) CreateDatabaseObjectImportRule(
return nil, trace.Wrap(err)
}
return out, nil
}
// UpsertDatabaseObjectImportRule creates a new DatabaseObjectImportRule or forcefully updates an existing DatabaseObjectImportRule.
@@ -82,24 +82,40 @@ func TestServiceAccess(t *testing.T) {
allowedStates []authz.AdminActionAuthState
}{
{
name: "UpsertDatabaseObjectImportRule",
allowedStates: []authz.AdminActionAuthState{authz.AdminActionAuthNotRequired, authz.AdminActionAuthMFAVerified},
allowedVerbs: []string{types.VerbUpdate, types.VerbCreate},
name: "UpsertDatabaseObjectImportRule",
allowedStates: []authz.AdminActionAuthState{
authz.AdminActionAuthNotRequired,
authz.AdminActionAuthMFAVerified,
authz.AdminActionAuthMFAVerifiedWithReuse,
},
allowedVerbs: []string{types.VerbUpdate, types.VerbCreate},
},
{
name: "CreateDatabaseObjectImportRule",
allowedStates: []authz.AdminActionAuthState{authz.AdminActionAuthNotRequired, authz.AdminActionAuthMFAVerified},
allowedVerbs: []string{types.VerbCreate},
name: "CreateDatabaseObjectImportRule",
allowedStates: []authz.AdminActionAuthState{
authz.AdminActionAuthNotRequired,
authz.AdminActionAuthMFAVerified,
authz.AdminActionAuthMFAVerifiedWithReuse,
},
allowedVerbs: []string{types.VerbCreate},
},
{
name: "UpdateDatabaseObjectImportRule",
allowedStates: []authz.AdminActionAuthState{authz.AdminActionAuthNotRequired, authz.AdminActionAuthMFAVerified},
allowedVerbs: []string{types.VerbUpdate},
name: "UpdateDatabaseObjectImportRule",
allowedStates: []authz.AdminActionAuthState{
authz.AdminActionAuthNotRequired,
authz.AdminActionAuthMFAVerified,
authz.AdminActionAuthMFAVerifiedWithReuse,
},
allowedVerbs: []string{types.VerbUpdate},
},
{
name: "DeleteDatabaseObjectImportRule",
allowedStates: []authz.AdminActionAuthState{authz.AdminActionAuthNotRequired, authz.AdminActionAuthMFAVerified},
allowedVerbs: []string{types.VerbDelete},
name: "DeleteDatabaseObjectImportRule",
allowedStates: []authz.AdminActionAuthState{
authz.AdminActionAuthNotRequired,
authz.AdminActionAuthMFAVerified,
authz.AdminActionAuthMFAVerifiedWithReuse,
},
allowedVerbs: []string{types.VerbDelete},
},
{
name: "GetDatabaseObjectImportRule",
@@ -159,7 +159,7 @@ func (s *Service) CreateDynamicWindowsDesktop(ctx context.Context, req *dynamicw
if err != nil {
return nil, trace.Wrap(err)
}
if err := auth.AuthorizeAdminAction(); err != nil {
if err := auth.AuthorizeAdminActionAllowReusedMFA(); err != nil {
return nil, trace.Wrap(err)
}
if err := auth.CheckAccessToKind(types.KindDynamicWindowsDesktop, types.VerbCreate); err != nil {
@@ -191,7 +191,7 @@ func (s *Service) UpdateDynamicWindowsDesktop(ctx context.Context, req *dynamicw
if err != nil {
return nil, trace.Wrap(err)
}
if err := auth.AuthorizeAdminAction(); err != nil {
if err := auth.AuthorizeAdminActionAllowReusedMFA(); err != nil {
return nil, trace.Wrap(err)
}
if err := auth.CheckAccessToKind(types.KindDynamicWindowsDesktop, types.VerbUpdate); err != nil {
@@ -226,7 +226,7 @@ func (s *Service) UpsertDynamicWindowsDesktop(ctx context.Context, req *dynamicw
if err != nil {
return nil, trace.Wrap(err)
}
if err := auth.AuthorizeAdminAction(); err != nil {
if err := auth.AuthorizeAdminActionAllowReusedMFA(); err != nil {
return nil, trace.Wrap(err)
}
if err := auth.CheckAccessToKind(types.KindDynamicWindowsDesktop, types.VerbCreate, types.VerbUpdate); err != nil {
@@ -263,7 +263,7 @@ func (s *Service) DeleteDynamicWindowsDesktop(ctx context.Context, req *dynamicw
if err != nil {
return nil, trace.Wrap(err)
}
if err := auth.AuthorizeAdminAction(); err != nil {
if err := auth.AuthorizeAdminActionAllowReusedMFA(); err != nil {
return nil, trace.Wrap(err)
}
if err := auth.CheckAccessToKind(types.KindDynamicWindowsDesktop, types.VerbDelete); err != nil {
@@ -83,24 +83,40 @@ func TestServiceAccess(t *testing.T) {
allowedStates []authz.AdminActionAuthState
}{
{
name: "CreateDynamicWindowsDesktop",
allowedStates: []authz.AdminActionAuthState{authz.AdminActionAuthNotRequired, authz.AdminActionAuthMFAVerified},
allowedVerbs: []string{types.VerbCreate},
name: "CreateDynamicWindowsDesktop",
allowedStates: []authz.AdminActionAuthState{
authz.AdminActionAuthNotRequired,
authz.AdminActionAuthMFAVerified,
authz.AdminActionAuthMFAVerifiedWithReuse,
},
allowedVerbs: []string{types.VerbCreate},
},
{
name: "UpdateDynamicWindowsDesktop",
allowedStates: []authz.AdminActionAuthState{authz.AdminActionAuthNotRequired, authz.AdminActionAuthMFAVerified},
allowedVerbs: []string{types.VerbUpdate},
name: "UpdateDynamicWindowsDesktop",
allowedStates: []authz.AdminActionAuthState{
authz.AdminActionAuthNotRequired,
authz.AdminActionAuthMFAVerified,
authz.AdminActionAuthMFAVerifiedWithReuse,
},
allowedVerbs: []string{types.VerbUpdate},
},
{
name: "UpsertDynamicWindowsDesktop",
allowedStates: []authz.AdminActionAuthState{authz.AdminActionAuthNotRequired, authz.AdminActionAuthMFAVerified},
allowedVerbs: []string{types.VerbCreate, types.VerbUpdate},
name: "UpsertDynamicWindowsDesktop",
allowedStates: []authz.AdminActionAuthState{
authz.AdminActionAuthNotRequired,
authz.AdminActionAuthMFAVerified,
authz.AdminActionAuthMFAVerifiedWithReuse,
},
allowedVerbs: []string{types.VerbCreate, types.VerbUpdate},
},
{
name: "DeleteDynamicWindowsDesktop",
allowedStates: []authz.AdminActionAuthState{authz.AdminActionAuthNotRequired, authz.AdminActionAuthMFAVerified},
allowedVerbs: []string{types.VerbDelete},
name: "DeleteDynamicWindowsDesktop",
allowedStates: []authz.AdminActionAuthState{
authz.AdminActionAuthNotRequired,
authz.AdminActionAuthMFAVerified,
authz.AdminActionAuthMFAVerifiedWithReuse,
},
allowedVerbs: []string{types.VerbDelete},
},
{
name: "ListDynamicWindowsDesktops",
@@ -102,7 +102,7 @@ func (b *BotInstanceService) DeleteBotInstance(ctx context.Context, req *pb.Dele
return nil, trace.Wrap(err)
}
if err := authCtx.AuthorizeAdminAction(); err != nil {
if err := authCtx.AuthorizeAdminActionAllowReusedMFA(); err != nil {
return nil, trace.Wrap(err)
}
@@ -70,9 +70,13 @@ func TestBotInstanceServiceAccess(t *testing.T) {
allowedVerbs: []string{types.VerbRead, types.VerbList},
},
{
name: "DeleteBotInstance",
allowedStates: []authz.AdminActionAuthState{authz.AdminActionAuthNotRequired, authz.AdminActionAuthMFAVerified},
allowedVerbs: []string{types.VerbDelete},
name: "DeleteBotInstance",
allowedStates: []authz.AdminActionAuthState{
authz.AdminActionAuthNotRequired,
authz.AdminActionAuthMFAVerified,
authz.AdminActionAuthMFAVerifiedWithReuse,
},
allowedVerbs: []string{types.VerbDelete},
},
{
name: "SubmitHeartbeat",
+2 -2
View File
@@ -222,7 +222,7 @@ func (s *Service) UpdateRemoteCluster(
if err := authCtx.CheckAccessToKind(types.KindRemoteCluster, types.VerbUpdate); err != nil {
return nil, trace.Wrap(err)
}
if err := authCtx.AuthorizeAdminAction(); err != nil {
if err := authCtx.AuthorizeAdminActionAllowReusedMFA(); err != nil {
return nil, trace.Wrap(err)
}
@@ -295,7 +295,7 @@ func (s *Service) DeleteRemoteCluster(
); err != nil {
return nil, trace.Wrap(err)
}
if err := authCtx.AuthorizeAdminAction(); err != nil {
if err := authCtx.AuthorizeAdminActionAllowReusedMFA(); err != nil {
return nil, trace.Wrap(err)
}
@@ -91,7 +91,7 @@ func (s *Service) ListStaticHostUsers(ctx context.Context, req *userprovisioning
if err := authCtx.CheckAccessToKind(types.KindStaticHostUser, types.VerbList, types.VerbRead); err != nil {
return nil, trace.Wrap(err)
}
if err := authCtx.AuthorizeAdminAction(); err != nil {
if err := authCtx.AuthorizeAdminActionAllowReusedMFA(); err != nil {
return nil, trace.Wrap(err)
}
@@ -117,7 +117,7 @@ func (s *Service) GetStaticHostUser(ctx context.Context, req *userprovisioningpb
if err := authCtx.CheckAccessToKind(types.KindStaticHostUser, types.VerbRead); err != nil {
return nil, trace.Wrap(err)
}
if err := authCtx.AuthorizeAdminAction(); err != nil {
if err := authCtx.AuthorizeAdminActionAllowReusedMFA(); err != nil {
return nil, trace.Wrap(err)
}
@@ -147,7 +147,7 @@ func (s *Service) CreateStaticHostUser(ctx context.Context, req *userprovisionin
if err := authCtx.CheckAccessToKind(types.KindStaticHostUser, types.VerbCreate); err != nil {
return nil, trace.Wrap(err)
}
if err := authCtx.AuthorizeAdminAction(); err != nil {
if err := authCtx.AuthorizeAdminActionAllowReusedMFA(); err != nil {
return nil, trace.Wrap(err)
}
@@ -180,7 +180,7 @@ func (s *Service) UpdateStaticHostUser(ctx context.Context, req *userprovisionin
if err := authCtx.CheckAccessToKind(types.KindStaticHostUser, types.VerbUpdate); err != nil {
return nil, trace.Wrap(err)
}
if err := authCtx.AuthorizeAdminAction(); err != nil {
if err := authCtx.AuthorizeAdminActionAllowReusedMFA(); err != nil {
return nil, trace.Wrap(err)
}
@@ -213,7 +213,7 @@ func (s *Service) UpsertStaticHostUser(ctx context.Context, req *userprovisionin
if err := authCtx.CheckAccessToKind(types.KindStaticHostUser, types.VerbCreate, types.VerbUpdate); err != nil {
return nil, trace.Wrap(err)
}
if err := authCtx.AuthorizeAdminAction(); err != nil {
if err := authCtx.AuthorizeAdminActionAllowReusedMFA(); err != nil {
return nil, trace.Wrap(err)
}
@@ -249,7 +249,7 @@ func (s *Service) DeleteStaticHostUser(ctx context.Context, req *userprovisionin
if err := authCtx.CheckAccessToKind(types.KindStaticHostUser, types.VerbDelete); err != nil {
return nil, trace.Wrap(err)
}
if err := authCtx.AuthorizeAdminAction(); err != nil {
if err := authCtx.AuthorizeAdminActionAllowReusedMFA(); err != nil {
return nil, trace.Wrap(err)
}
+1 -1
View File
@@ -472,7 +472,7 @@ func (s *Service) DeleteUser(ctx context.Context, req *userspb.DeleteUserRequest
return nil, trace.Wrap(err)
}
if err := authCtx.AuthorizeAdminAction(); err != nil {
if err := authCtx.AuthorizeAdminActionAllowReusedMFA(); err != nil {
return nil, trace.Wrap(err)
}
+3 -3
View File
@@ -77,7 +77,7 @@ func (s *Service) CreateVnetConfig(ctx context.Context, req *vnet.CreateVnetConf
return nil, trace.Wrap(err)
}
if err := authCtx.AuthorizeAdminAction(); err != nil {
if err := authCtx.AuthorizeAdminActionAllowReusedMFA(); err != nil {
return nil, trace.Wrap(err)
}
@@ -96,7 +96,7 @@ func (s *Service) UpdateVnetConfig(ctx context.Context, req *vnet.UpdateVnetConf
return nil, trace.Wrap(err)
}
if err := authCtx.AuthorizeAdminAction(); err != nil {
if err := authCtx.AuthorizeAdminActionAllowReusedMFA(); err != nil {
return nil, trace.Wrap(err)
}
@@ -135,7 +135,7 @@ func (s *Service) DeleteVnetConfig(ctx context.Context, _ *vnet.DeleteVnetConfig
return nil, trace.Wrap(err)
}
if err := authCtx.AuthorizeAdminAction(); err != nil {
if err := authCtx.AuthorizeAdminActionAllowReusedMFA(); err != nil {
return nil, trace.Wrap(err)
}
@@ -60,18 +60,26 @@ func TestServiceAccess(t *testing.T) {
}
testCases := []testCase{
{
name: "CreateVnetConfig",
allowedStates: []authz.AdminActionAuthState{authz.AdminActionAuthNotRequired, authz.AdminActionAuthMFAVerified},
allowedVerbs: []string{types.VerbCreate},
name: "CreateVnetConfig",
allowedStates: []authz.AdminActionAuthState{
authz.AdminActionAuthNotRequired,
authz.AdminActionAuthMFAVerified,
authz.AdminActionAuthMFAVerifiedWithReuse,
},
allowedVerbs: []string{types.VerbCreate},
action: func(service *Service) error {
_, err := service.CreateVnetConfig(ctx, &vnet.CreateVnetConfigRequest{VnetConfig: vnetConfig})
return trace.Wrap(err)
},
},
{
name: "UpdateVnetConfig",
allowedStates: []authz.AdminActionAuthState{authz.AdminActionAuthNotRequired, authz.AdminActionAuthMFAVerified},
allowedVerbs: []string{types.VerbUpdate},
name: "UpdateVnetConfig",
allowedStates: []authz.AdminActionAuthState{
authz.AdminActionAuthNotRequired,
authz.AdminActionAuthMFAVerified,
authz.AdminActionAuthMFAVerifiedWithReuse,
},
allowedVerbs: []string{types.VerbUpdate},
action: func(service *Service) error {
if _, err := service.storage.CreateVnetConfig(ctx, vnetConfig); err != nil {
return trace.Wrap(err, "creating vnet_config as pre-req for Update test")
@@ -81,9 +89,13 @@ func TestServiceAccess(t *testing.T) {
},
},
{
name: "DeleteVnetConfig",
allowedStates: []authz.AdminActionAuthState{authz.AdminActionAuthNotRequired, authz.AdminActionAuthMFAVerified},
allowedVerbs: []string{types.VerbDelete},
name: "DeleteVnetConfig",
allowedStates: []authz.AdminActionAuthState{
authz.AdminActionAuthNotRequired,
authz.AdminActionAuthMFAVerified,
authz.AdminActionAuthMFAVerifiedWithReuse,
},
allowedVerbs: []string{types.VerbDelete},
action: func(service *Service) error {
if _, err := service.storage.CreateVnetConfig(ctx, vnetConfig); err != nil {
return trace.Wrap(err, "creating vnet_config as pre-req for Delete test")
+14 -1
View File
@@ -1507,6 +1507,13 @@ func (c *Context) CheckAccessToRule(ruleCtx *services.Context, kind string, verb
}
// AuthorizeAdminAction will ensure that the user is authorized to perform admin actions.
// MFA challenges that allow reuse will not be accepted.
//
// In the majority of cases, allowing reuse is ok and can result in better UX. Forbidding
// reuse should be reserved for critical actions (e.g. CA rotation, cert generation) and
// other actions that are not expected to be performed in bulk (e.g. access request reviews).
//
// See https://github.com/gravitational/teleport/blob/master/rfd/0155-scoped-webauthn-credentials.md#when-to-extend-reuse
func (c *Context) AuthorizeAdminAction() error {
switch c.AdminActionAuthState {
case AdminActionAuthMFAVerified, AdminActionAuthNotRequired:
@@ -1516,7 +1523,13 @@ func (c *Context) AuthorizeAdminAction() error {
}
// AuthorizeAdminActionAllowReusedMFA will ensure that the user is authorized to perform
// admin actions. Additionally, MFA challenges that allow reuse will be accepted.
// admin actions. MFA challenges that allow reuse will be accepted.
//
// In the majority of cases, allowing reuse is ok and can result in better UX. Forbidding
// reuse should be reserved for critical actions (e.g. CA rotation, cert generation) and
// other actions that are not expected to be performed in bulk (e.g. access request reviews).
//
// See https://github.com/gravitational/teleport/blob/master/rfd/0155-scoped-webauthn-credentials.md#when-to-extend-reuse
func (c *Context) AuthorizeAdminActionAllowReusedMFA() error {
if c.AdminActionAuthState == AdminActionAuthMFAVerifiedWithReuse {
return nil
+6
View File
@@ -245,6 +245,12 @@ endpoints:
- `http createWebSession`
- `http deleteWebSession`
Update: Minimizing which admin actions allow reuse has caused several issues
in new bulk admin actions, most notably the new Discover flows. Other than the
critical admin action endpoints listed above, most now allow reuse. It is
instead left up to the client to be reasonable, only requesting a reusable MFA
challenge in preparation for a bulk admin action.
#### Expiration
Webauthn challenges are always set to expire after 5 minutes. However, as we've