diff --git a/pkg/services/accesscontrol/accesscontrol.go b/pkg/services/accesscontrol/accesscontrol.go index 41de06d0ac8..fc05a021ee7 100644 --- a/pkg/services/accesscontrol/accesscontrol.go +++ b/pkg/services/accesscontrol/accesscontrol.go @@ -40,6 +40,8 @@ type Service interface { DeclareFixedRoles(registrations ...RoleRegistration) error // SaveExternalServiceRole creates or updates an external service's role and assigns it to a given service account id. SaveExternalServiceRole(ctx context.Context, cmd SaveExternalServiceRoleCommand) error + // DeleteExternalServiceRole removes an external service's role and its assignment. + DeleteExternalServiceRole(ctx context.Context, externalServiceID string) error //IsDisabled returns if access control is enabled or not IsDisabled() bool } diff --git a/pkg/services/accesscontrol/acimpl/service.go b/pkg/services/accesscontrol/acimpl/service.go index 479a5941a9a..55ca349fb56 100644 --- a/pkg/services/accesscontrol/acimpl/service.go +++ b/pkg/services/accesscontrol/acimpl/service.go @@ -14,6 +14,7 @@ import ( "github.com/grafana/grafana/pkg/infra/localcache" "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/infra/metrics" + "github.com/grafana/grafana/pkg/infra/slugify" "github.com/grafana/grafana/pkg/plugins" "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/accesscontrol/api" @@ -63,6 +64,7 @@ type store interface { GetUsersBasicRoles(ctx context.Context, userFilter []int64, orgID int64) (map[int64][]string, error) DeleteUserPermissions(ctx context.Context, orgID, userID int64) error SaveExternalServiceRole(ctx context.Context, cmd accesscontrol.SaveExternalServiceRoleCommand) error + DeleteExternalServiceRole(ctx context.Context, externalServiceID string) error } // Service is the service implementing role based access control. @@ -416,7 +418,7 @@ func (s *Service) SaveExternalServiceRole(ctx context.Context, cmd accesscontrol } if !s.features.IsEnabled(featuremgmt.FlagExternalServiceAuth) { - s.log.Debug("registering external service role is behind a feature flag, enable it to use this feature.") + s.log.Debug("registering an external service role is behind a feature flag, enable it to use this feature.") return nil } @@ -426,3 +428,19 @@ func (s *Service) SaveExternalServiceRole(ctx context.Context, cmd accesscontrol return s.store.SaveExternalServiceRole(ctx, cmd) } + +func (s *Service) DeleteExternalServiceRole(ctx context.Context, externalServiceID string) error { + // If accesscontrol is disabled no need to delete the external service role + if accesscontrol.IsDisabled(s.cfg) { + return nil + } + + if !s.features.IsEnabled(featuremgmt.FlagExternalServiceAuth) { + s.log.Debug("deleting an external service role is behind a feature flag, enable it to use this feature.") + return nil + } + + slug := slugify.Slugify(externalServiceID) + + return s.store.DeleteExternalServiceRole(ctx, slug) +} diff --git a/pkg/services/accesscontrol/acimpl/service_test.go b/pkg/services/accesscontrol/acimpl/service_test.go index 6da1b4bbcef..3085fed5835 100644 --- a/pkg/services/accesscontrol/acimpl/service_test.go +++ b/pkg/services/accesscontrol/acimpl/service_test.go @@ -852,3 +852,55 @@ func TestService_SaveExternalServiceRole(t *testing.T) { }) } } + +func TestService_DeleteExternalServiceRole(t *testing.T) { + tests := []struct { + name string + initCmd *accesscontrol.SaveExternalServiceRoleCommand + externalServiceID string + wantErr bool + }{ + { + name: "handles deleting role that doesn't exist", + externalServiceID: "App 1", + wantErr: false, + }, + { + name: "handles deleting role that exists", + initCmd: &accesscontrol.SaveExternalServiceRoleCommand{ + Global: true, + ServiceAccountID: 2, + ExternalServiceID: "App 1", + Permissions: []accesscontrol.Permission{{Action: "users:read", Scope: "users:id:1"}}, + }, + externalServiceID: "App 1", + wantErr: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx := context.Background() + ac := setupTestEnv(t) + ac.features = featuremgmt.WithFeatures(featuremgmt.FlagExternalServiceAuth) + + if tt.initCmd != nil { + err := ac.SaveExternalServiceRole(ctx, *tt.initCmd) + require.NoError(t, err) + } + + err := ac.DeleteExternalServiceRole(ctx, tt.externalServiceID) + if tt.wantErr { + require.Error(t, err) + return + } + require.NoError(t, err) + + if tt.initCmd != nil { + // Check that the permissions and assignment are removed correctly + perms, errGetPerms := ac.getUserPermissions(ctx, &user.SignedInUser{OrgID: tt.initCmd.OrgID, UserID: 2}, accesscontrol.Options{}) + require.NoError(t, errGetPerms) + assert.Empty(t, perms) + } + }) + } +} diff --git a/pkg/services/accesscontrol/actest/fake.go b/pkg/services/accesscontrol/actest/fake.go index f25f001a398..e0e5dc44e89 100644 --- a/pkg/services/accesscontrol/actest/fake.go +++ b/pkg/services/accesscontrol/actest/fake.go @@ -57,6 +57,10 @@ func (f FakeService) SaveExternalServiceRole(ctx context.Context, cmd accesscont return f.ExpectedErr } +func (f FakeService) DeleteExternalServiceRole(ctx context.Context, externalServiceID string) error { + return f.ExpectedErr +} + var _ accesscontrol.AccessControl = new(FakeAccessControl) type FakeAccessControl struct { @@ -103,6 +107,10 @@ func (f FakeStore) SaveExternalServiceRole(ctx context.Context, cmd accesscontro return f.ExpectedErr } +func (f FakeStore) DeleteExternalServiceRole(ctx context.Context, externalServiceID string) error { + return f.ExpectedErr +} + var _ accesscontrol.PermissionsService = new(FakePermissionsService) type FakePermissionsService struct { diff --git a/pkg/services/accesscontrol/database/externalservices.go b/pkg/services/accesscontrol/database/externalservices.go index bf724cd1572..ea9d1af1807 100644 --- a/pkg/services/accesscontrol/database/externalservices.go +++ b/pkg/services/accesscontrol/database/externalservices.go @@ -10,6 +10,52 @@ import ( "github.com/grafana/grafana/pkg/services/accesscontrol" ) +func extServiceRoleUID(externalServiceID string) string { + uid := fmt.Sprintf("%s%s_permissions", accesscontrol.ExternalServiceRoleUIDPrefix, externalServiceID) + return uid +} + +func extServiceRoleName(externalServiceID string) string { + name := fmt.Sprintf("%s%s:permissions", accesscontrol.ExternalServiceRolePrefix, externalServiceID) + return name +} + +func (s *AccessControlStore) DeleteExternalServiceRole(ctx context.Context, externalServiceID string) error { + uid := extServiceRoleUID(externalServiceID) + + return s.sql.WithDbSession(ctx, func(sess *db.Session) error { + stored, errGet := getRoleByUID(ctx, sess, uid) + if errGet != nil { + // Role not found, nothing to do + if errors.Is(errGet, accesscontrol.ErrRoleNotFound) { + return nil + } + return errGet + } + + // Delete the assignments + _, errDel := sess.Exec("DELETE FROM user_role WHERE role_id = ?", stored.ID) + if errDel != nil { + return errDel + } + // Shouldn't happen but just in case delete any team assignments + _, errDel = sess.Exec("DELETE FROM team_role WHERE role_id = ?", stored.ID) + if errDel != nil { + return errDel + } + + // Delete the permissions + _, errDel = sess.Exec("DELETE FROM permission WHERE role_id = ?", stored.ID) + if errDel != nil { + return errDel + } + + // Delete the role + _, errDel = sess.Exec("DELETE FROM role WHERE id = ?", stored.ID) + return errDel + }) +} + func (s *AccessControlStore) SaveExternalServiceRole(ctx context.Context, cmd accesscontrol.SaveExternalServiceRoleCommand) error { role := genExternalServiceRole(cmd) assignment := genExternalServiceAssignment(cmd) @@ -39,8 +85,8 @@ func genExternalServiceRole(cmd accesscontrol.SaveExternalServiceRoleCommand) ac role := accesscontrol.Role{ OrgID: cmd.OrgID, Version: 1, - Name: fmt.Sprintf("%s%s:permissions", accesscontrol.ExternalServiceRolePrefix, cmd.ExternalServiceID), - UID: fmt.Sprintf("%s%s_permissions", accesscontrol.ExternalServiceRoleUIDPrefix, cmd.ExternalServiceID), + Name: extServiceRoleName(cmd.ExternalServiceID), + UID: extServiceRoleUID(cmd.ExternalServiceID), DisplayName: fmt.Sprintf("External Service %s Permissions", cmd.ExternalServiceID), Description: fmt.Sprintf("External Service %s permissions", cmd.ExternalServiceID), Group: "External Service", diff --git a/pkg/services/accesscontrol/database/externalservices_test.go b/pkg/services/accesscontrol/database/externalservices_test.go index fbf926ea784..6f90350b76a 100644 --- a/pkg/services/accesscontrol/database/externalservices_test.go +++ b/pkg/services/accesscontrol/database/externalservices_test.go @@ -2,6 +2,7 @@ package database import ( "context" + "errors" "fmt" "testing" @@ -189,3 +190,113 @@ func TestAccessControlStore_SaveExternalServiceRole(t *testing.T) { }) } } + +func TestAccessControlStore_DeleteExternalServiceRole(t *testing.T) { + extID := "app1" + tests := []struct { + name string + init func(t *testing.T, ctx context.Context, s *AccessControlStore) + id string + wantErr bool + }{ + { + name: "delete no role", + id: extID, + wantErr: false, + }, + { + name: "delete local role", + init: func(t *testing.T, ctx context.Context, s *AccessControlStore) { + errSave := s.SaveExternalServiceRole(ctx, accesscontrol.SaveExternalServiceRoleCommand{ + OrgID: 2, + ExternalServiceID: extID, + ServiceAccountID: 3, + Permissions: []accesscontrol.Permission{ + {Action: "users:read", Scope: "users:id:1"}, + {Action: "users:write", Scope: "users:id:1"}, + }, + }) + require.NoError(t, errSave) + }, + id: extID, + wantErr: false, + }, + { + name: "delete global role", + init: func(t *testing.T, ctx context.Context, s *AccessControlStore) { + errSave := s.SaveExternalServiceRole(ctx, accesscontrol.SaveExternalServiceRoleCommand{ + Global: true, + ExternalServiceID: extID, + ServiceAccountID: 3, + Permissions: []accesscontrol.Permission{ + {Action: "users:read", Scope: "users:id:1"}, + {Action: "users:write", Scope: "users:id:1"}, + }, + }) + require.NoError(t, errSave) + }, + id: extID, + wantErr: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx := context.Background() + s := &AccessControlStore{ + sql: db.InitTestDB(t), + } + if tt.init != nil { + tt.init(t, ctx, s) + } + roleID := int64(-1) + err := s.sql.WithDbSession(ctx, func(sess *db.Session) error { + role, err := getRoleByUID(ctx, sess, extServiceRoleUID(tt.id)) + if err != nil && !errors.Is(err, accesscontrol.ErrRoleNotFound) { + return err + } + if role != nil { + roleID = role.ID + } + return nil + }) + require.NoError(t, err) + err = s.DeleteExternalServiceRole(ctx, tt.id) + if tt.wantErr { + require.Error(t, err) + return + } + require.NoError(t, err) + + // Only check removal if the role existed before + if roleID == -1 { + return + } + + // Assignments should be deleted + _ = s.sql.WithDbSession(ctx, func(sess *db.Session) error { + var assignment accesscontrol.UserRole + count, err := sess.Where("role_id = ?", roleID).Count(&assignment) + require.NoError(t, err) + require.Zero(t, count) + return nil + }) + + // Permissions should be deleted + _ = s.sql.WithDbSession(ctx, func(sess *db.Session) error { + var permission accesscontrol.Permission + count, err := sess.Where("role_id = ?", roleID).Count(&permission) + require.NoError(t, err) + require.Zero(t, count) + return nil + }) + + // Role should be deleted + _ = s.sql.WithDbSession(ctx, func(sess *db.Session) error { + storedRole, err := getRoleByUID(ctx, sess, extServiceRoleUID(tt.id)) + require.ErrorIs(t, err, accesscontrol.ErrRoleNotFound) + require.Nil(t, storedRole) + return nil + }) + }) + } +} diff --git a/pkg/services/accesscontrol/mock/mock.go b/pkg/services/accesscontrol/mock/mock.go index 4b7bf250ccb..7e9a9546d2a 100644 --- a/pkg/services/accesscontrol/mock/mock.go +++ b/pkg/services/accesscontrol/mock/mock.go @@ -31,6 +31,7 @@ type Calls struct { SearchUsersPermissions []interface{} SearchUserPermissions []interface{} SaveExternalServiceRole []interface{} + DeleteExternalServiceRole []interface{} } type Mock struct { @@ -58,6 +59,7 @@ type Mock struct { SearchUsersPermissionsFunc func(context.Context, *user.SignedInUser, int64, accesscontrol.SearchOptions) (map[int64][]accesscontrol.Permission, error) SearchUserPermissionsFunc func(ctx context.Context, orgID int64, searchOptions accesscontrol.SearchOptions) ([]accesscontrol.Permission, error) SaveExternalServiceRoleFunc func(ctx context.Context, cmd accesscontrol.SaveExternalServiceRoleCommand) error + DeleteExternalServiceRoleFunc func(ctx context.Context, externalServiceID string) error scopeResolvers accesscontrol.Resolvers } @@ -246,3 +248,12 @@ func (m *Mock) SaveExternalServiceRole(ctx context.Context, cmd accesscontrol.Sa } return nil } + +func (m *Mock) DeleteExternalServiceRole(ctx context.Context, externalServiceID string) error { + m.Calls.DeleteExternalServiceRole = append(m.Calls.DeleteExternalServiceRole, []interface{}{ctx, externalServiceID}) + // Use override if provided + if m.DeleteExternalServiceRoleFunc != nil { + return m.DeleteExternalServiceRoleFunc(ctx, externalServiceID) + } + return nil +}