From ce6b775ac286edce6dd891a461966f7b1b429835 Mon Sep 17 00:00:00 2001 From: joerger Date: Thu, 9 Nov 2023 17:46:19 -0800 Subject: [PATCH 1/2] Authorize role endpoints as admin actions. --- lib/auth/auth_with_roles.go | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/lib/auth/auth_with_roles.go b/lib/auth/auth_with_roles.go index 084ae6ab396fb..00b9ccf5635c9 100644 --- a/lib/auth/auth_with_roles.go +++ b/lib/auth/auth_with_roles.go @@ -3887,6 +3887,10 @@ func (a *ServerWithRoles) CreateRole(ctx context.Context, role types.Role) (type return nil, trace.Wrap(err) } + if err := authz.AuthorizeAdminAction(ctx, &a.context); err != nil { + return nil, trace.Wrap(err) + } + if err := a.validateRole(ctx, role); err != nil { return nil, trace.Wrap(err) } @@ -3901,6 +3905,10 @@ func (a *ServerWithRoles) UpdateRole(ctx context.Context, role types.Role) (type return nil, trace.Wrap(err) } + if err := authz.AuthorizeAdminAction(ctx, &a.context); err != nil { + return nil, trace.Wrap(err) + } + if err := a.validateRole(ctx, role); err != nil { return nil, trace.Wrap(err) } @@ -3915,6 +3923,10 @@ 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 { + return nil, trace.Wrap(err) + } + if err := a.validateRole(ctx, role); err != nil { return nil, trace.Wrap(err) } @@ -4062,6 +4074,11 @@ func (a *ServerWithRoles) DeleteRole(ctx context.Context, name string) error { if err := a.action(apidefaults.Namespace, types.KindRole, types.VerbDelete); err != nil { return trace.Wrap(err) } + + if err := authz.AuthorizeAdminAction(ctx, &a.context); err != nil { + return trace.Wrap(err) + } + // DELETE IN (7.0) // It's OK to delete this code alongside migrateOSS code in auth. // It prevents 6.0 from migrating resources multiple times From 49f969f86cebd238686714ac8219f5973c9c3c08 Mon Sep 17 00:00:00 2001 From: joerger Date: Wed, 6 Dec 2023 10:43:53 -0800 Subject: [PATCH 2/2] Add tctl test. --- tool/tctl/common/admin_action_test.go | 67 +++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/tool/tctl/common/admin_action_test.go b/tool/tctl/common/admin_action_test.go index 8604ed88ceada..c2e1a9bc171a7 100644 --- a/tool/tctl/common/admin_action_test.go +++ b/tool/tctl/common/admin_action_test.go @@ -25,6 +25,7 @@ import ( "path/filepath" "strings" "testing" + "time" "github.com/gravitational/trace" "github.com/stretchr/testify/require" @@ -52,6 +53,7 @@ func TestAdminActionMFA(t *testing.T) { s := newAdminActionTestSuite(t) t.Run("Users", s.testAdminActionMFA_Users) + t.Run("Roles", s.testAdminActionMFA_Roles) } func (s *adminActionTestSuite) testAdminActionMFA_Users(t *testing.T) { @@ -100,6 +102,39 @@ func (s *adminActionTestSuite) testAdminActionMFA_Users(t *testing.T) { }) } +func (s *adminActionTestSuite) testAdminActionMFA_Roles(t *testing.T) { + ctx := context.Background() + + role, err := types.NewRole("telerole", types.RoleSpecV6{}) + require.NoError(t, err) + + createRole := func() error { + _, err := s.authServer.CreateRole(ctx, role) + return trace.Wrap(err) + } + + getRole := func() (types.Resource, error) { + return s.authServer.GetRole(ctx, role.GetName()) + } + + deleteRole := func() error { + return s.authServer.DeleteRole(ctx, role.GetName()) + } + + s.testAdminActionMFA_ResourceCommand(t, ctx, resourceCommandTestCase{ + resource: role, + resourceCreate: createRole, + resourceDelete: deleteRole, + }) + + s.testAdminActionMFA_EditCommand(t, ctx, editCommandTestCase{ + resourceRef: getResourceRef(role), + resourceCreate: createRole, + resourceGet: getRole, + resourceDelete: deleteRole, + }) +} + type resourceCommandTestCase struct { resource types.Resource resourceCreate func() error @@ -140,6 +175,38 @@ func (s *adminActionTestSuite) testAdminActionMFA_ResourceCommand(t *testing.T, }) } +type editCommandTestCase struct { + resourceRef string + resourceCreate func() error + resourceGet func() (types.Resource, error) + resourceDelete func() error +} + +func (s *adminActionTestSuite) testAdminActionMFA_EditCommand(t *testing.T, ctx context.Context, tc editCommandTestCase) { + t.Run("tctl edit", func(t *testing.T) { + s.runTestCase(t, ctx, adminActionTestCase{ + command: fmt.Sprintf("edit %v", tc.resourceRef), + setup: tc.resourceCreate, + cliCommand: &tctl.EditCommand{ + Editor: func(filename string) error { + // Get the latest version of the resource with the correct revision ID. + resource, err := tc.resourceGet() + require.NoError(t, err) + + // Update the expiry so that the edit goes through. + resource.SetExpiry(time.Now()) + + f, err := os.Create(filename) + require.NoError(t, err) + require.NoError(t, utils.WriteYAML(f, resource)) + return nil + }, + }, + cleanup: tc.resourceDelete, + }) + }) +} + type adminActionTestSuite struct { authServer *auth.Server // userClientWithMFA supports MFA prompt for admin actions.