From 007c2c813144831a7799255b314b41e1fdb6deca Mon Sep 17 00:00:00 2001 From: Gabriel MABILLE Date: Tue, 10 Oct 2023 09:20:52 +0200 Subject: [PATCH] AuthN: Extract from OAuthServer service account management code (#76128) * Extract code to manage service accounts * Add test with client credentials grants * Fix test with the changed interface * Wire * Fix HandleTokenRequest * Add tests to extsvcaccounts * Rename Retrieve function * Document the interface --- pkg/server/wire.go | 3 + .../extsvcauth/extsvcaccounts/models.go | 21 ++ .../extsvcauth/extsvcaccounts/service.go | 126 +++++++++++ .../extsvcauth/extsvcaccounts/service_test.go | 211 ++++++++++++++++++ .../extsvcauth/extsvcmocks/extsvcaccmock.go | 79 +++++++ pkg/services/extsvcauth/models.go | 26 +++ .../extsvcauth/oauthserver/oasimpl/service.go | 123 ++-------- .../oauthserver/oasimpl/service_test.go | 144 +++--------- .../oauthserver/oasimpl/token_test.go | 14 +- 9 files changed, 524 insertions(+), 223 deletions(-) create mode 100644 pkg/services/extsvcauth/extsvcaccounts/models.go create mode 100644 pkg/services/extsvcauth/extsvcaccounts/service.go create mode 100644 pkg/services/extsvcauth/extsvcaccounts/service_test.go create mode 100644 pkg/services/extsvcauth/extsvcmocks/extsvcaccmock.go diff --git a/pkg/server/wire.go b/pkg/server/wire.go index 930647c0079..44755d416b4 100644 --- a/pkg/server/wire.go +++ b/pkg/server/wire.go @@ -64,6 +64,7 @@ import ( "github.com/grafana/grafana/pkg/services/encryption" encryptionservice "github.com/grafana/grafana/pkg/services/encryption/service" "github.com/grafana/grafana/pkg/services/extsvcauth" + "github.com/grafana/grafana/pkg/services/extsvcauth/extsvcaccounts" "github.com/grafana/grafana/pkg/services/extsvcauth/oauthserver" "github.com/grafana/grafana/pkg/services/extsvcauth/oauthserver/oasimpl" extsvcreg "github.com/grafana/grafana/pkg/services/extsvcauth/registry" @@ -359,6 +360,8 @@ var wireBasicSet = wire.NewSet( authnimpl.ProvideIdentitySynchronizer, authnimpl.ProvideAuthnService, supportbundlesimpl.ProvideService, + extsvcaccounts.ProvideExtSvcAccountsService, + wire.Bind(new(extsvcauth.ExtSvcAccountsService), new(*extsvcaccounts.ExtSvcAccountsService)), oasimpl.ProvideService, wire.Bind(new(oauthserver.OAuth2Server), new(*oasimpl.OAuth2ServiceImpl)), extsvcreg.ProvideExtSvcRegistry, diff --git a/pkg/services/extsvcauth/extsvcaccounts/models.go b/pkg/services/extsvcauth/extsvcaccounts/models.go new file mode 100644 index 00000000000..536d6432f32 --- /dev/null +++ b/pkg/services/extsvcauth/extsvcaccounts/models.go @@ -0,0 +1,21 @@ +package extsvcaccounts + +import ( + "github.com/grafana/grafana/pkg/models/roletype" + ac "github.com/grafana/grafana/pkg/services/accesscontrol" +) + +type saveExtSvcAccountCmd struct { + ExtSvcSlug string + OrgID int64 + Permissions []ac.Permission + SaID int64 +} + +func newRole(r roletype.RoleType) *roletype.RoleType { + return &r +} + +func newBool(b bool) *bool { + return &b +} diff --git a/pkg/services/extsvcauth/extsvcaccounts/service.go b/pkg/services/extsvcauth/extsvcaccounts/service.go new file mode 100644 index 00000000000..6bc60ace2c5 --- /dev/null +++ b/pkg/services/extsvcauth/extsvcaccounts/service.go @@ -0,0 +1,126 @@ +package extsvcaccounts + +import ( + "context" + "errors" + + "github.com/grafana/grafana/pkg/infra/log" + "github.com/grafana/grafana/pkg/models/roletype" + ac "github.com/grafana/grafana/pkg/services/accesscontrol" + "github.com/grafana/grafana/pkg/services/extsvcauth" + sa "github.com/grafana/grafana/pkg/services/serviceaccounts" +) + +type ExtSvcAccountsService struct { + acSvc ac.Service + logger log.Logger + saSvc sa.Service +} + +func ProvideExtSvcAccountsService(acSvc ac.Service, saSvc sa.Service) *ExtSvcAccountsService { + return &ExtSvcAccountsService{ + acSvc: acSvc, + logger: log.New("serviceauth.extsvcaccounts"), + saSvc: saSvc, + } +} + +// RetrieveExtSvcAccount fetches an external service account by ID +func (esa *ExtSvcAccountsService) RetrieveExtSvcAccount(ctx context.Context, orgID, saID int64) (*extsvcauth.ExtSvcAccount, error) { + sa, err := esa.saSvc.RetrieveServiceAccount(ctx, orgID, saID) + if err != nil { + return nil, err + } + return &extsvcauth.ExtSvcAccount{ + ID: sa.Id, + Login: sa.Login, + Name: sa.Name, + OrgID: sa.OrgId, + IsDisabled: sa.IsDisabled, + Role: roletype.RoleType(sa.Role), + }, nil +} + +// ManageExtSvcAccount creates, updates or deletes the service account associated with an external service +func (esa *ExtSvcAccountsService) ManageExtSvcAccount(ctx context.Context, cmd *extsvcauth.ManageExtSvcAccountCmd) (int64, error) { + if cmd == nil { + esa.logger.Warn("Received no input") + return 0, nil + } + + saID, errRetrieve := esa.saSvc.RetrieveServiceAccountIdByName(ctx, cmd.OrgID, cmd.ExtSvcSlug) + if errRetrieve != nil && !errors.Is(errRetrieve, sa.ErrServiceAccountNotFound) { + return 0, errRetrieve + } + + if !cmd.Enabled || len(cmd.Permissions) == 0 { + if saID > 0 { + if err := esa.deleteExtSvcAccount(ctx, cmd.OrgID, cmd.ExtSvcSlug, saID); err != nil { + esa.logger.Error("Error occurred while deleting service account", + "service", cmd.ExtSvcSlug, + "saID", saID, + "error", err.Error()) + return 0, err + } + } + esa.logger.Info("Skipping service account creation", + "service", cmd.ExtSvcSlug, + "enabled", cmd.Enabled, + "permission count", len(cmd.Permissions), + "saID", saID) + return 0, nil + } + + saID, errSave := esa.saveExtSvcAccount(ctx, &saveExtSvcAccountCmd{ + ExtSvcSlug: cmd.ExtSvcSlug, + OrgID: cmd.OrgID, + Permissions: cmd.Permissions, + SaID: saID, + }) + if errSave != nil { + esa.logger.Error("Could not save service account", "service", cmd.ExtSvcSlug, "error", errSave.Error()) + return 0, errSave + } + + return saID, nil +} + +// saveExtSvcAccount creates or updates the service account associated with an external service +func (esa *ExtSvcAccountsService) saveExtSvcAccount(ctx context.Context, cmd *saveExtSvcAccountCmd) (int64, error) { + if cmd.SaID <= 0 { + // Create a service account + esa.logger.Debug("Create service account", "service", cmd.ExtSvcSlug, "orgID", cmd.OrgID) + sa, err := esa.saSvc.CreateServiceAccount(ctx, cmd.OrgID, &sa.CreateServiceAccountForm{ + Name: cmd.ExtSvcSlug, + Role: newRole(roletype.RoleNone), + IsDisabled: newBool(false), + }) + if err != nil { + return 0, err + } + cmd.SaID = sa.Id + } + + // update the service account's permissions + esa.logger.Debug("Update role permissions", "service", cmd.ExtSvcSlug, "saID", cmd.SaID) + if err := esa.acSvc.SaveExternalServiceRole(ctx, ac.SaveExternalServiceRoleCommand{ + OrgID: ac.GlobalOrgID, + Global: true, + ExternalServiceID: cmd.ExtSvcSlug, + ServiceAccountID: cmd.SaID, + Permissions: cmd.Permissions, + }); err != nil { + return 0, err + } + + return cmd.SaID, nil +} + +// deleteExtSvcAccount deletes a service account by ID and removes its associated role +func (esa *ExtSvcAccountsService) deleteExtSvcAccount(ctx context.Context, orgID int64, slug string, saID int64) error { + esa.logger.Info("Delete service account", "service", slug, "saID", saID) + if err := esa.saSvc.DeleteServiceAccount(ctx, orgID, saID); err != nil { + return err + } + return esa.acSvc.DeleteExternalServiceRole(ctx, slug) +} diff --git a/pkg/services/extsvcauth/extsvcaccounts/service_test.go b/pkg/services/extsvcauth/extsvcaccounts/service_test.go new file mode 100644 index 00000000000..0cb27a55729 --- /dev/null +++ b/pkg/services/extsvcauth/extsvcaccounts/service_test.go @@ -0,0 +1,211 @@ +package extsvcaccounts + +import ( + "context" + "testing" + + "github.com/grafana/grafana/pkg/infra/localcache" + "github.com/grafana/grafana/pkg/infra/log" + "github.com/grafana/grafana/pkg/models/roletype" + ac "github.com/grafana/grafana/pkg/services/accesscontrol" + "github.com/grafana/grafana/pkg/services/accesscontrol/acimpl" + "github.com/grafana/grafana/pkg/services/accesscontrol/actest" + "github.com/grafana/grafana/pkg/services/extsvcauth" + "github.com/grafana/grafana/pkg/services/featuremgmt" + sa "github.com/grafana/grafana/pkg/services/serviceaccounts" + "github.com/grafana/grafana/pkg/services/serviceaccounts/tests" + "github.com/grafana/grafana/pkg/setting" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" +) + +type TestEnv struct { + S *ExtSvcAccountsService + AcStore *actest.MockStore + SaSvc *tests.MockServiceAccountService +} + +func setupTestEnv(t *testing.T) *TestEnv { + t.Helper() + + cfg := setting.NewCfg() + fmgt := featuremgmt.WithFeatures(featuremgmt.FlagExternalServiceAuth) + + env := &TestEnv{ + AcStore: &actest.MockStore{}, + SaSvc: &tests.MockServiceAccountService{}, + } + env.S = &ExtSvcAccountsService{ + acSvc: acimpl.ProvideOSSService(cfg, env.AcStore, localcache.New(0, 0), fmgt), + logger: log.New("extsvcaccounts.test"), + saSvc: env.SaSvc, + } + return env +} + +func TestExtSvcAccountsService_ManageExtSvcAccount(t *testing.T) { + extSvcSlug := "grafana-test-app" + extSvcOrgID := int64(20) + extSvcAccID := int64(10) + extSvcPerms := []ac.Permission{{Action: ac.ActionUsersRead, Scope: ac.ScopeUsersAll}} + extSvcAccount := &sa.ServiceAccountDTO{ + Id: extSvcAccID, + Name: extSvcSlug, + Login: extSvcSlug, + OrgId: extSvcOrgID, + IsDisabled: false, + Role: string(roletype.RoleNone), + } + + tests := []struct { + name string + init func(env *TestEnv) + cmd extsvcauth.ManageExtSvcAccountCmd + checks func(t *testing.T, env *TestEnv) + want int64 + wantErr bool + }{ + { + name: "should remove service account when disabled", + init: func(env *TestEnv) { + // A previous service account was attached to this slug + env.SaSvc.On("RetrieveServiceAccountIdByName", mock.Anything, mock.Anything, mock.Anything).Return(extSvcAccID, nil) + env.SaSvc.On("DeleteServiceAccount", mock.Anything, mock.Anything, mock.Anything).Return(nil) + env.AcStore.On("DeleteExternalServiceRole", mock.Anything, mock.Anything).Return(nil) + }, + cmd: extsvcauth.ManageExtSvcAccountCmd{ + ExtSvcSlug: extSvcSlug, + Enabled: false, + OrgID: extSvcOrgID, + Permissions: extSvcPerms, + }, + checks: func(t *testing.T, env *TestEnv) { + env.SaSvc.AssertCalled(t, "RetrieveServiceAccountIdByName", mock.Anything, + mock.MatchedBy(func(orgID int64) bool { return orgID == extSvcOrgID }), + mock.MatchedBy(func(slug string) bool { return slug == extSvcSlug })) + env.SaSvc.AssertCalled(t, "DeleteServiceAccount", mock.Anything, + mock.MatchedBy(func(orgID int64) bool { return orgID == extSvcOrgID }), + mock.MatchedBy(func(saID int64) bool { return saID == extSvcAccID })) + env.AcStore.AssertCalled(t, "DeleteExternalServiceRole", mock.Anything, + mock.MatchedBy(func(slug string) bool { return slug == extSvcSlug })) + }, + want: 0, + wantErr: false, + }, + { + name: "should remove service account when no permission", + init: func(env *TestEnv) { + // A previous service account was attached to this slug + env.SaSvc.On("RetrieveServiceAccountIdByName", mock.Anything, mock.Anything, mock.Anything).Return(extSvcAccID, nil) + env.SaSvc.On("DeleteServiceAccount", mock.Anything, mock.Anything, mock.Anything).Return(nil) + env.AcStore.On("DeleteExternalServiceRole", mock.Anything, mock.Anything).Return(nil) + }, + cmd: extsvcauth.ManageExtSvcAccountCmd{ + ExtSvcSlug: extSvcSlug, + Enabled: true, + OrgID: extSvcOrgID, + Permissions: []ac.Permission{}, + }, + checks: func(t *testing.T, env *TestEnv) { + env.SaSvc.AssertCalled(t, "RetrieveServiceAccountIdByName", mock.Anything, + mock.MatchedBy(func(orgID int64) bool { return orgID == extSvcOrgID }), + mock.MatchedBy(func(slug string) bool { return slug == extSvcSlug })) + env.SaSvc.AssertCalled(t, "DeleteServiceAccount", mock.Anything, + mock.MatchedBy(func(orgID int64) bool { return orgID == extSvcOrgID }), + mock.MatchedBy(func(saID int64) bool { return saID == extSvcAccID })) + env.AcStore.AssertCalled(t, "DeleteExternalServiceRole", mock.Anything, + mock.MatchedBy(func(slug string) bool { return slug == extSvcSlug })) + }, + want: 0, + wantErr: false, + }, + { + name: "should create new service account", + init: func(env *TestEnv) { + // No previous service account was attached to this slug + env.SaSvc.On("RetrieveServiceAccountIdByName", mock.Anything, mock.Anything, mock.Anything). + Return(int64(0), sa.ErrServiceAccountNotFound.Errorf("mock")) + env.SaSvc.On("CreateServiceAccount", mock.Anything, mock.Anything, mock.Anything). + Return(extSvcAccount, nil) + env.AcStore.On("SaveExternalServiceRole", mock.Anything, mock.Anything).Return(nil) + }, + cmd: extsvcauth.ManageExtSvcAccountCmd{ + ExtSvcSlug: extSvcSlug, + Enabled: true, + OrgID: extSvcOrgID, + Permissions: extSvcPerms, + }, + checks: func(t *testing.T, env *TestEnv) { + env.SaSvc.AssertCalled(t, "RetrieveServiceAccountIdByName", mock.Anything, + mock.MatchedBy(func(orgID int64) bool { return orgID == extSvcOrgID }), + mock.MatchedBy(func(slug string) bool { return slug == extSvcSlug })) + env.SaSvc.AssertCalled(t, "CreateServiceAccount", mock.Anything, + mock.MatchedBy(func(orgID int64) bool { return orgID == extSvcOrgID }), + mock.MatchedBy(func(cmd *sa.CreateServiceAccountForm) bool { + return cmd.Name == extSvcSlug && *cmd.Role == roletype.RoleNone + }), + ) + env.AcStore.AssertCalled(t, "SaveExternalServiceRole", mock.Anything, + mock.MatchedBy(func(cmd ac.SaveExternalServiceRoleCommand) bool { + return cmd.ServiceAccountID == extSvcAccount.Id && cmd.ExternalServiceID == extSvcSlug && + cmd.OrgID == int64(ac.GlobalOrgID) && len(cmd.Permissions) == 1 && + cmd.Permissions[0] == extSvcPerms[0] + })) + }, + want: extSvcAccID, + wantErr: false, + }, + { + name: "should update service account", + init: func(env *TestEnv) { + // A previous service account was attached to this slug + env.SaSvc.On("RetrieveServiceAccountIdByName", mock.Anything, mock.Anything, mock.Anything). + Return(int64(11), nil) + env.AcStore.On("SaveExternalServiceRole", mock.Anything, mock.Anything).Return(nil) + }, + cmd: extsvcauth.ManageExtSvcAccountCmd{ + ExtSvcSlug: extSvcSlug, + Enabled: true, + OrgID: extSvcOrgID, + Permissions: extSvcPerms, + }, + checks: func(t *testing.T, env *TestEnv) { + env.SaSvc.AssertCalled(t, "RetrieveServiceAccountIdByName", mock.Anything, + mock.MatchedBy(func(orgID int64) bool { return orgID == extSvcOrgID }), + mock.MatchedBy(func(slug string) bool { return slug == extSvcSlug })) + env.AcStore.AssertCalled(t, "SaveExternalServiceRole", mock.Anything, + mock.MatchedBy(func(cmd ac.SaveExternalServiceRoleCommand) bool { + return cmd.ServiceAccountID == int64(11) && cmd.ExternalServiceID == extSvcSlug && + cmd.OrgID == int64(ac.GlobalOrgID) && len(cmd.Permissions) == 1 && + cmd.Permissions[0] == extSvcPerms[0] + })) + }, + want: 11, + wantErr: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx := context.Background() + + env := setupTestEnv(t) + if tt.init != nil { + tt.init(env) + } + + got, err := env.S.ManageExtSvcAccount(ctx, &tt.cmd) + + if tt.wantErr { + require.Error(t, err) + return + } + require.NoError(t, err) + + if tt.checks != nil { + tt.checks(t, env) + } + + require.Equal(t, tt.want, got) + }) + } +} diff --git a/pkg/services/extsvcauth/extsvcmocks/extsvcaccmock.go b/pkg/services/extsvcauth/extsvcmocks/extsvcaccmock.go new file mode 100644 index 00000000000..b16fff68577 --- /dev/null +++ b/pkg/services/extsvcauth/extsvcmocks/extsvcaccmock.go @@ -0,0 +1,79 @@ +// Code generated by mockery v2.35.2. DO NOT EDIT. + +package extsvcmocks + +import ( + context "context" + + extsvcauth "github.com/grafana/grafana/pkg/services/extsvcauth" + mock "github.com/stretchr/testify/mock" +) + +// MockExtSvcAccountsService is an autogenerated mock type for the ExtSvcAccountsService type +type MockExtSvcAccountsService struct { + mock.Mock +} + +// ManageExtSvcAccount provides a mock function with given fields: ctx, cmd +func (_m *MockExtSvcAccountsService) ManageExtSvcAccount(ctx context.Context, cmd *extsvcauth.ManageExtSvcAccountCmd) (int64, error) { + ret := _m.Called(ctx, cmd) + + var r0 int64 + var r1 error + if rf, ok := ret.Get(0).(func(context.Context, *extsvcauth.ManageExtSvcAccountCmd) (int64, error)); ok { + return rf(ctx, cmd) + } + if rf, ok := ret.Get(0).(func(context.Context, *extsvcauth.ManageExtSvcAccountCmd) int64); ok { + r0 = rf(ctx, cmd) + } else { + r0 = ret.Get(0).(int64) + } + + if rf, ok := ret.Get(1).(func(context.Context, *extsvcauth.ManageExtSvcAccountCmd) error); ok { + r1 = rf(ctx, cmd) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// RetrieveExtSvcAccount provides a mock function with given fields: ctx, orgID, saID +func (_m *MockExtSvcAccountsService) RetrieveExtSvcAccount(ctx context.Context, orgID int64, saID int64) (*extsvcauth.ExtSvcAccount, error) { + ret := _m.Called(ctx, orgID, saID) + + var r0 *extsvcauth.ExtSvcAccount + var r1 error + if rf, ok := ret.Get(0).(func(context.Context, int64, int64) (*extsvcauth.ExtSvcAccount, error)); ok { + return rf(ctx, orgID, saID) + } + if rf, ok := ret.Get(0).(func(context.Context, int64, int64) *extsvcauth.ExtSvcAccount); ok { + r0 = rf(ctx, orgID, saID) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*extsvcauth.ExtSvcAccount) + } + } + + if rf, ok := ret.Get(1).(func(context.Context, int64, int64) error); ok { + r1 = rf(ctx, orgID, saID) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// NewMockExtSvcAccountsService creates a new instance of MockExtSvcAccountsService. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +// The first argument is typically a *testing.T value. +func NewMockExtSvcAccountsService(t interface { + mock.TestingT + Cleanup(func()) +}) *MockExtSvcAccountsService { + mock := &MockExtSvcAccountsService{} + mock.Mock.Test(t) + + t.Cleanup(func() { mock.AssertExpectations(t) }) + + return mock +} diff --git a/pkg/services/extsvcauth/models.go b/pkg/services/extsvcauth/models.go index efa9b4b8649..cb82ed08ef7 100644 --- a/pkg/services/extsvcauth/models.go +++ b/pkg/services/extsvcauth/models.go @@ -3,6 +3,7 @@ package extsvcauth import ( "context" + "github.com/grafana/grafana/pkg/models/roletype" "github.com/grafana/grafana/pkg/services/accesscontrol" ) @@ -19,6 +20,31 @@ type ExternalServiceRegistry interface { SaveExternalService(ctx context.Context, cmd *ExternalServiceRegistration) (*ExternalService, error) } +//go:generate mockery --name ExtSvcAccountsService --structname MockExtSvcAccountsService --output extsvcmocks --outpkg extsvcmocks --filename extsvcaccmock.go +type ExtSvcAccountsService interface { + // ManageExtSvcAccount creates, updates or deletes the service account associated with an external service + ManageExtSvcAccount(ctx context.Context, cmd *ManageExtSvcAccountCmd) (int64, error) + // RetrieveExtSvcAccount fetches an external service account by ID + RetrieveExtSvcAccount(ctx context.Context, orgID, saID int64) (*ExtSvcAccount, error) +} + +// ExtSvcAccount represents the service account associated to an external service +type ExtSvcAccount struct { + ID int64 + Login string + Name string + OrgID int64 + IsDisabled bool + Role roletype.RoleType +} + +type ManageExtSvcAccountCmd struct { + ExtSvcSlug string + Enabled bool // disabled: the service account and its permissions will be deleted + OrgID int64 + Permissions []accesscontrol.Permission +} + type SelfCfg struct { // Enabled allows the service to request access tokens for itself Enabled bool diff --git a/pkg/services/extsvcauth/oauthserver/oasimpl/service.go b/pkg/services/extsvcauth/oauthserver/oasimpl/service.go index 25e53cbeaa4..a0d4312057c 100644 --- a/pkg/services/extsvcauth/oauthserver/oasimpl/service.go +++ b/pkg/services/extsvcauth/oauthserver/oasimpl/service.go @@ -26,7 +26,6 @@ import ( "github.com/grafana/grafana/pkg/infra/localcache" "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/infra/slugify" - "github.com/grafana/grafana/pkg/models/roletype" ac "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/extsvcauth" "github.com/grafana/grafana/pkg/services/extsvcauth/oauthserver" @@ -34,8 +33,6 @@ import ( "github.com/grafana/grafana/pkg/services/extsvcauth/oauthserver/store" "github.com/grafana/grafana/pkg/services/extsvcauth/oauthserver/utils" "github.com/grafana/grafana/pkg/services/featuremgmt" - "github.com/grafana/grafana/pkg/services/org" - "github.com/grafana/grafana/pkg/services/serviceaccounts" "github.com/grafana/grafana/pkg/services/signingkeys" "github.com/grafana/grafana/pkg/services/team" "github.com/grafana/grafana/pkg/services/user" @@ -57,14 +54,14 @@ type OAuth2ServiceImpl struct { logger log.Logger accessControl ac.AccessControl acService ac.Service - saService serviceaccounts.Service + saService extsvcauth.ExtSvcAccountsService userService user.Service teamService team.Service publicKey any } func ProvideService(router routing.RouteRegister, db db.DB, cfg *setting.Cfg, - svcAccSvc serviceaccounts.Service, accessControl ac.AccessControl, acSvc ac.Service, userSvc user.Service, + extSvcAccSvc extsvcauth.ExtSvcAccountsService, accessControl ac.AccessControl, acSvc ac.Service, userSvc user.Service, teamSvc team.Service, keySvc signingkeys.Service, fmgmt *featuremgmt.FeatureManager) (*OAuth2ServiceImpl, error) { if !fmgmt.IsEnabled(featuremgmt.FlagExternalServiceAuth) { return nil, nil @@ -86,7 +83,7 @@ func ProvideService(router routing.RouteRegister, db db.DB, cfg *setting.Cfg, sqlstore: store.NewStore(db), logger: log.New("oauthserver"), userService: userSvc, - saService: svcAccSvc, + saService: extSvcAccSvc, teamService: teamSvc, } @@ -152,15 +149,15 @@ func (s *OAuth2ServiceImpl) GetExternalService(ctx context.Context, id string) ( // Retrieve self permissions and generate a signed in user s.logger.Debug("GetExternalService: fetch permissions", "client id", id) - sa, err := s.saService.RetrieveServiceAccount(ctx, oauthserver.TmpOrgID, client.ServiceAccountID) + sa, err := s.saService.RetrieveExtSvcAccount(ctx, oauthserver.TmpOrgID, client.ServiceAccountID) if err != nil { s.logger.Error("GetExternalService: error fetching service account", "id", id, "error", err) return nil, err } client.SignedInUser = &user.SignedInUser{ - UserID: sa.Id, + UserID: sa.ID, OrgID: oauthserver.TmpOrgID, - OrgRole: org.RoleType(sa.Role), // Need this to compute the permissions in OSS + OrgRole: sa.Role, // Need this to compute the permissions in OSS Login: sa.Login, Name: sa.Name, Permissions: map[int64]map[string][]string{}, @@ -225,13 +222,6 @@ func (s *OAuth2ServiceImpl) SaveExternalService(ctx context.Context, registratio return nil, errGenCred } - s.logger.Debug("Save service account") - saID, errSaveServiceAccount := s.saveServiceAccount(ctx, client.Name, client.ServiceAccountID, client.SelfPermissions) - if errSaveServiceAccount != nil { - return nil, errSaveServiceAccount - } - client.ServiceAccountID = saID - grantTypes := s.computeGrantTypes(registration.Self.Enabled, registration.Impersonation.Enabled) client.GrantTypes = strings.Join(grantTypes, ",") @@ -254,6 +244,18 @@ func (s *OAuth2ServiceImpl) SaveExternalService(ctx context.Context, registratio } client.Secret = string(hashedSecret) + s.logger.Debug("Save service account") + saID, errSaveServiceAccount := s.saService.ManageExtSvcAccount(ctx, &extsvcauth.ManageExtSvcAccountCmd{ + ExtSvcSlug: slugify.Slugify(client.Name), + Enabled: registration.Self.Enabled, + OrgID: oauthserver.TmpOrgID, + Permissions: client.SelfPermissions, + }) + if errSaveServiceAccount != nil { + return nil, errSaveServiceAccount + } + client.ServiceAccountID = saID + err = s.sqlstore.SaveExternalService(ctx, client) if err != nil { s.logger.Error("Error saving external service", "client", client.LogID(), "error", err) @@ -380,95 +382,6 @@ func (s *OAuth2ServiceImpl) handleKeyOptions(ctx context.Context, keyOption *ext return nil, fmt.Errorf("at least one key option must be specified") } -// saveServiceAccount creates a service account if the service account ID is NoServiceAccountID, otherwise it updates the service account's permissions -func (s *OAuth2ServiceImpl) saveServiceAccount(ctx context.Context, extSvcName string, saID int64, permissions []ac.Permission) (int64, error) { - if saID == oauthserver.NoServiceAccountID { - // Create a service account - s.logger.Debug("Create service account", "external service name", extSvcName) - return s.createServiceAccount(ctx, extSvcName, permissions) - } - - // check if the service account exists - s.logger.Debug("Update service account", "external service name", extSvcName) - sa, err := s.saService.RetrieveServiceAccount(ctx, oauthserver.TmpOrgID, saID) - if err != nil { - s.logger.Error("Error retrieving service account", "external service name", extSvcName, "error", err) - return oauthserver.NoServiceAccountID, err - } - - // update the service account's permissions - if len(permissions) > 0 { - s.logger.Debug("Update role permissions", "external service name", extSvcName, "saID", saID) - if err := s.acService.SaveExternalServiceRole(ctx, ac.SaveExternalServiceRoleCommand{ - OrgID: ac.GlobalOrgID, - Global: true, - ExternalServiceID: extSvcName, - ServiceAccountID: sa.Id, - Permissions: permissions, - }); err != nil { - return oauthserver.NoServiceAccountID, err - } - return saID, nil - } - - // remove the service account - errDelete := s.deleteServiceAccount(ctx, extSvcName, sa.Id) - return oauthserver.NoServiceAccountID, errDelete -} - -// deleteServiceAccount deletes a service account by ID and removes its associated role -func (s *OAuth2ServiceImpl) deleteServiceAccount(ctx context.Context, extSvcName string, saID int64) error { - s.logger.Debug("Delete service account", "external service name", extSvcName, "saID", saID) - if err := s.saService.DeleteServiceAccount(ctx, oauthserver.TmpOrgID, saID); err != nil { - return err - } - return s.acService.DeleteExternalServiceRole(ctx, extSvcName) -} - -// createServiceAccount creates a service account with the given permissions and returns the ID of the service account -// When no permission is given, the account isn't created and NoServiceAccountID is returned -// This first design does not use a single transaction for the whole service account creation process => database consistency is not guaranteed. -// Consider changing this in the future. -func (s *OAuth2ServiceImpl) createServiceAccount(ctx context.Context, extSvcName string, permissions []ac.Permission) (int64, error) { - if len(permissions) == 0 { - // No permission, no service account - s.logger.Debug("No permission, no service account", "external service name", extSvcName) - return oauthserver.NoServiceAccountID, nil - } - - newRole := func(r roletype.RoleType) *roletype.RoleType { - return &r - } - newBool := func(b bool) *bool { - return &b - } - - slug := slugify.Slugify(extSvcName) - - s.logger.Debug("Generate service account", "external service name", extSvcName, "orgID", oauthserver.TmpOrgID, "name", slug) - sa, err := s.saService.CreateServiceAccount(ctx, oauthserver.TmpOrgID, &serviceaccounts.CreateServiceAccountForm{ - Name: slug, - Role: newRole(roletype.RoleNone), - IsDisabled: newBool(false), - }) - if err != nil { - return oauthserver.NoServiceAccountID, err - } - - s.logger.Debug("Create tailored role for service account", "external service name", extSvcName, "name", slug, "service_account_id", sa.Id, "permissions", permissions) - if err := s.acService.SaveExternalServiceRole(ctx, ac.SaveExternalServiceRoleCommand{ - OrgID: ac.GlobalOrgID, - Global: true, - ExternalServiceID: slug, - ServiceAccountID: sa.Id, - Permissions: permissions, - }); err != nil { - return oauthserver.NoServiceAccountID, err - } - - return sa.Id, nil -} - // handleRegistrationPermissions parses the registration form to retrieve requested permissions and adds default // permissions when impersonation is requested func (*OAuth2ServiceImpl) handleRegistrationPermissions(registration *extsvcauth.ExternalServiceRegistration) ([]ac.Permission, []ac.Permission) { diff --git a/pkg/services/extsvcauth/oauthserver/oasimpl/service_test.go b/pkg/services/extsvcauth/oauthserver/oasimpl/service_test.go index 6f57f581cbb..1188a6f800d 100644 --- a/pkg/services/extsvcauth/oauthserver/oasimpl/service_test.go +++ b/pkg/services/extsvcauth/oauthserver/oasimpl/service_test.go @@ -18,16 +18,15 @@ import ( "github.com/grafana/grafana/pkg/infra/localcache" "github.com/grafana/grafana/pkg/infra/log" - "github.com/grafana/grafana/pkg/models/roletype" ac "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/accesscontrol/acimpl" "github.com/grafana/grafana/pkg/services/accesscontrol/actest" "github.com/grafana/grafana/pkg/services/extsvcauth" + "github.com/grafana/grafana/pkg/services/extsvcauth/extsvcmocks" "github.com/grafana/grafana/pkg/services/extsvcauth/oauthserver" "github.com/grafana/grafana/pkg/services/extsvcauth/oauthserver/oastest" "github.com/grafana/grafana/pkg/services/featuremgmt" sa "github.com/grafana/grafana/pkg/services/serviceaccounts" - satests "github.com/grafana/grafana/pkg/services/serviceaccounts/tests" "github.com/grafana/grafana/pkg/services/signingkeys/signingkeystest" "github.com/grafana/grafana/pkg/services/team/teamtest" "github.com/grafana/grafana/pkg/services/user" @@ -52,7 +51,7 @@ type TestEnv struct { OAuthStore *oastest.MockStore UserService *usertest.FakeUserService TeamService *teamtest.FakeService - SAService *satests.MockServiceAccountService + SAService *extsvcmocks.MockExtSvcAccountsService } func setupTestEnv(t *testing.T) *TestEnv { @@ -77,7 +76,7 @@ func setupTestEnv(t *testing.T) *TestEnv { OAuthStore: &oastest.MockStore{}, UserService: usertest.NewUserServiceFake(), TeamService: teamtest.NewFakeService(), - SAService: &satests.MockServiceAccountService{}, + SAService: extsvcmocks.NewMockExtSvcAccountsService(t), } env.S = &OAuth2ServiceImpl{ cache: localcache.New(cacheExpirationTime, cacheCleanupInterval), @@ -106,22 +105,6 @@ func setupTestEnv(t *testing.T) *TestEnv { func TestOAuth2ServiceImpl_SaveExternalService(t *testing.T) { const serviceName = "my-ext-service" - sa1 := sa.ServiceAccountDTO{Id: 1, Name: serviceName, Login: serviceName, OrgId: oauthserver.TmpOrgID, IsDisabled: false, Role: "None"} - sa1Profile := sa.ServiceAccountProfileDTO{Id: 1, Name: serviceName, Login: serviceName, OrgId: oauthserver.TmpOrgID, IsDisabled: false, Role: "None"} - prevSaID := int64(3) - // Using a function to prevent modifying the same object in the tests - client1 := func() *oauthserver.OAuthExternalService { - return &oauthserver.OAuthExternalService{ - Name: serviceName, - ClientID: "RANDOMID", - Secret: "RANDOMSECRET", - GrantTypes: "client_credentials", - PublicPem: []byte("-----BEGIN PUBLIC KEY-----"), - ServiceAccountID: prevSaID, - SelfPermissions: []ac.Permission{{Action: "users:impersonate", Scope: "users:*"}}, - } - } - tests := []struct { name string init func(*TestEnv) @@ -135,6 +118,9 @@ func TestOAuth2ServiceImpl_SaveExternalService(t *testing.T) { // No client at the beginning env.OAuthStore.On("GetExternalServiceByName", mock.Anything, mock.Anything).Return(nil, oauthserver.ErrClientNotFound(serviceName)) env.OAuthStore.On("SaveExternalService", mock.Anything, mock.Anything).Return(nil) + + // Return a service account ID + env.SAService.On("ManageExtSvcAccount", mock.Anything, mock.Anything).Return(int64(0), nil) }, cmd: &extsvcauth.ExternalServiceRegistration{ Name: serviceName, @@ -152,99 +138,38 @@ func TestOAuth2ServiceImpl_SaveExternalService(t *testing.T) { }, }, { - name: "should create a service account", + name: "should allow client credentials grant with correct permissions", init: func(env *TestEnv) { // No client at the beginning env.OAuthStore.On("GetExternalServiceByName", mock.Anything, mock.Anything).Return(nil, oauthserver.ErrClientNotFound(serviceName)) env.OAuthStore.On("SaveExternalService", mock.Anything, mock.Anything).Return(nil) - // Service account and permission creation - env.SAService.On("CreateServiceAccount", mock.Anything, mock.Anything, mock.Anything).Return(&sa1, nil) - env.AcStore.On("SaveExternalServiceRole", mock.Anything, mock.Anything).Return(nil) + + // Return a service account ID + env.SAService.On("ManageExtSvcAccount", mock.Anything, mock.Anything).Return(int64(10), nil) }, cmd: &extsvcauth.ExternalServiceRegistration{ - Name: serviceName, - OAuthProviderCfg: &extsvcauth.OAuthProviderCfg{Key: &extsvcauth.KeyOption{Generate: true}}, + Name: serviceName, Self: extsvcauth.SelfCfg{ Enabled: true, - Permissions: []ac.Permission{{Action: "users:read", Scope: "users:*"}}, + Permissions: []ac.Permission{{Action: ac.ActionUsersRead, Scope: ac.ScopeUsersAll}}, }, - }, - mockChecks: func(t *testing.T, env *TestEnv) { - // Check that the client has a service account and the correct grant type - env.OAuthStore.AssertCalled(t, "SaveExternalService", mock.Anything, mock.MatchedBy(func(client *oauthserver.OAuthExternalService) bool { - return client.Name == serviceName && - client.GrantTypes == "client_credentials" && client.ServiceAccountID == sa1.Id - })) - // Check that the service account is created in the correct org with the correct role - env.SAService.AssertCalled(t, "CreateServiceAccount", mock.Anything, - mock.MatchedBy(func(orgID int64) bool { return orgID == oauthserver.TmpOrgID }), - mock.MatchedBy(func(cmd *sa.CreateServiceAccountForm) bool { - return cmd.Name == serviceName && *cmd.Role == roletype.RoleNone - }), - ) - }, - }, - { - name: "should delete the service account", - init: func(env *TestEnv) { - // Existing client (with a service account hence a role) - env.OAuthStore.On("GetExternalServiceByName", mock.Anything, mock.Anything).Return(client1(), nil) - env.OAuthStore.On("SaveExternalService", mock.Anything, mock.Anything).Return(nil) - env.SAService.On("RetrieveServiceAccount", mock.Anything, mock.Anything, mock.Anything).Return(&sa1Profile, nil) - // No permission anymore will trigger deletion of the service account and its role - env.SAService.On("DeleteServiceAccount", mock.Anything, mock.Anything, mock.Anything).Return(nil) - env.AcStore.On("DeleteExternalServiceRole", mock.Anything, mock.Anything).Return(nil) - }, - cmd: &extsvcauth.ExternalServiceRegistration{ - Name: serviceName, OAuthProviderCfg: &extsvcauth.OAuthProviderCfg{Key: &extsvcauth.KeyOption{Generate: true}}, - Self: extsvcauth.SelfCfg{ - Enabled: false, - }, }, mockChecks: func(t *testing.T, env *TestEnv) { - // Check that the service has no service account anymore - env.OAuthStore.AssertCalled(t, "SaveExternalService", mock.Anything, mock.MatchedBy(func(client *oauthserver.OAuthExternalService) bool { - return client.Name == serviceName && client.ServiceAccountID == oauthserver.NoServiceAccountID + env.OAuthStore.AssertCalled(t, "GetExternalServiceByName", mock.Anything, mock.MatchedBy(func(name string) bool { + return name == serviceName })) - // Check that the service account is retrieved with the correct ID - env.SAService.AssertCalled(t, "RetrieveServiceAccount", mock.Anything, - mock.MatchedBy(func(orgID int64) bool { return orgID == oauthserver.TmpOrgID }), - mock.MatchedBy(func(saID int64) bool { return saID == prevSaID })) - // Check that the service account is deleted in the correct org - env.SAService.AssertCalled(t, "DeleteServiceAccount", mock.Anything, - mock.MatchedBy(func(orgID int64) bool { return orgID == oauthserver.TmpOrgID }), - mock.MatchedBy(func(saID int64) bool { return saID == sa1.Id })) - // Check that the associated role is deleted - env.AcStore.AssertCalled(t, "DeleteExternalServiceRole", mock.Anything, - mock.MatchedBy(func(extSvcName string) bool { return extSvcName == serviceName })) - }, - }, - { - name: "should update the service account", - init: func(env *TestEnv) { - // Existing client (with a service account hence a role) - env.OAuthStore.On("GetExternalServiceByName", mock.Anything, mock.Anything).Return(client1(), nil) - env.OAuthStore.On("SaveExternalService", mock.Anything, mock.Anything).Return(nil) - env.SAService.On("RetrieveServiceAccount", mock.Anything, mock.Anything, mock.Anything).Return(&sa1Profile, nil) - // Update the service account permissions - env.AcStore.On("SaveExternalServiceRole", mock.Anything, mock.Anything).Return(nil) - }, - cmd: &extsvcauth.ExternalServiceRegistration{ - Name: serviceName, - OAuthProviderCfg: &extsvcauth.OAuthProviderCfg{Key: &extsvcauth.KeyOption{Generate: true}}, - Self: extsvcauth.SelfCfg{ - Enabled: true, - Permissions: []ac.Permission{{Action: "dashboards:create", Scope: "folders:uid:general"}}, - }, - }, - mockChecks: func(t *testing.T, env *TestEnv) { - // Ensure new permissions are in place - env.AcStore.AssertCalled(t, "SaveExternalServiceRole", mock.Anything, - mock.MatchedBy(func(cmd ac.SaveExternalServiceRoleCommand) bool { - return cmd.ServiceAccountID == sa1.Id && cmd.ExternalServiceID == client1().Name && - cmd.OrgID == int64(ac.GlobalOrgID) && len(cmd.Permissions) == 1 && - cmd.Permissions[0] == ac.Permission{Action: "dashboards:create", Scope: "folders:uid:general"} + env.OAuthStore.AssertCalled(t, "SaveExternalService", mock.Anything, mock.MatchedBy(func(client *oauthserver.OAuthExternalService) bool { + return client.Name == serviceName && len(client.ClientID) > 0 && len(client.Secret) > 0 && + client.GrantTypes == string(fosite.GrantTypeClientCredentials) && + len(client.PublicPem) > 0 && client.ServiceAccountID == 10 && + len(client.ImpersonatePermissions) == 0 && + len(client.SelfPermissions) > 0 + })) + // Check that despite no credential_grants the service account still has a permission to impersonate users + env.SAService.AssertCalled(t, "ManageExtSvcAccount", mock.Anything, + mock.MatchedBy(func(cmd *extsvcauth.ManageExtSvcAccountCmd) bool { + return len(cmd.Permissions) == 1 && cmd.Permissions[0] == ac.Permission{Action: ac.ActionUsersRead, Scope: ac.ScopeUsersAll} })) }, }, @@ -255,8 +180,7 @@ func TestOAuth2ServiceImpl_SaveExternalService(t *testing.T) { env.OAuthStore.On("GetExternalServiceByName", mock.Anything, mock.Anything).Return(nil, oauthserver.ErrClientNotFound(serviceName)) env.OAuthStore.On("SaveExternalService", mock.Anything, mock.Anything).Return(nil) // The service account needs to be created with a permission to impersonate users - env.SAService.On("CreateServiceAccount", mock.Anything, mock.Anything, mock.Anything).Return(&sa1, nil) - env.AcStore.On("SaveExternalServiceRole", mock.Anything, mock.Anything).Return(nil) + env.SAService.On("ManageExtSvcAccount", mock.Anything, mock.Anything).Return(int64(10), nil) }, cmd: &extsvcauth.ExternalServiceRegistration{ Name: serviceName, @@ -277,8 +201,8 @@ func TestOAuth2ServiceImpl_SaveExternalService(t *testing.T) { slices.Contains(impPerm, ac.Permission{Action: ac.ActionTeamsRead, Scope: oauthserver.ScopeTeamsSelf}) })) // Check that despite no credential_grants the service account still has a permission to impersonate users - env.AcStore.AssertCalled(t, "SaveExternalServiceRole", mock.Anything, - mock.MatchedBy(func(cmd ac.SaveExternalServiceRoleCommand) bool { + env.SAService.AssertCalled(t, "ManageExtSvcAccount", mock.Anything, + mock.MatchedBy(func(cmd *extsvcauth.ManageExtSvcAccountCmd) bool { return len(cmd.Permissions) == 1 && cmd.Permissions[0] == ac.Permission{Action: ac.ActionUsersImpersonate, Scope: ac.ScopeUsersAll} })) }, @@ -396,11 +320,11 @@ func TestOAuth2ServiceImpl_GetExternalService(t *testing.T) { name: "should return error when the service account was not found", init: func(env *TestEnv) { env.OAuthStore.On("GetExternalService", mock.Anything, mock.Anything).Return(dummyClient(), nil) - env.SAService.On("RetrieveServiceAccount", mock.Anything, int64(1), int64(1)).Return(&sa.ServiceAccountProfileDTO{}, sa.ErrServiceAccountNotFound) + env.SAService.On("RetrieveExtSvcAccount", mock.Anything, int64(1), int64(1)).Return(&extsvcauth.ExtSvcAccount{}, sa.ErrServiceAccountNotFound) }, mockChecks: func(t *testing.T, env *TestEnv) { env.OAuthStore.AssertCalled(t, "GetExternalService", mock.Anything, mock.Anything) - env.SAService.AssertCalled(t, "RetrieveServiceAccount", mock.Anything, 1, 1) + env.SAService.AssertCalled(t, "RetrieveExtSvcAccount", mock.Anything, 1, 1) }, wantErr: true, }, @@ -408,12 +332,12 @@ func TestOAuth2ServiceImpl_GetExternalService(t *testing.T) { name: "should return error when the service account has no permissions", init: func(env *TestEnv) { env.OAuthStore.On("GetExternalService", mock.Anything, mock.Anything).Return(dummyClient(), nil) - env.SAService.On("RetrieveServiceAccount", mock.Anything, int64(1), int64(1)).Return(&sa.ServiceAccountProfileDTO{}, nil) + env.SAService.On("RetrieveExtSvcAccount", mock.Anything, int64(1), int64(1)).Return(&extsvcauth.ExtSvcAccount{}, nil) env.AcStore.On("GetUserPermissions", mock.Anything, mock.Anything).Return(nil, fmt.Errorf("some error")) }, mockChecks: func(t *testing.T, env *TestEnv) { env.OAuthStore.AssertCalled(t, "GetExternalService", mock.Anything, mock.Anything) - env.SAService.AssertCalled(t, "RetrieveServiceAccount", mock.Anything, 1, 1) + env.SAService.AssertCalled(t, "RetrieveExtSvcAccount", mock.Anything, 1, 1) }, wantErr: true, }, @@ -421,12 +345,12 @@ func TestOAuth2ServiceImpl_GetExternalService(t *testing.T) { name: "should return correctly", init: func(env *TestEnv) { env.OAuthStore.On("GetExternalService", mock.Anything, mock.Anything).Return(dummyClient(), nil) - env.SAService.On("RetrieveServiceAccount", mock.Anything, int64(1), int64(1)).Return(&sa.ServiceAccountProfileDTO{Id: 1}, nil) + env.SAService.On("RetrieveExtSvcAccount", mock.Anything, int64(1), int64(1)).Return(&extsvcauth.ExtSvcAccount{ID: 1}, nil) env.AcStore.On("GetUserPermissions", mock.Anything, mock.Anything).Return([]ac.Permission{{Action: ac.ActionUsersImpersonate, Scope: ac.ScopeUsersAll}}, nil) }, mockChecks: func(t *testing.T, env *TestEnv) { env.OAuthStore.AssertCalled(t, "GetExternalService", mock.Anything, mock.Anything) - env.SAService.AssertCalled(t, "RetrieveServiceAccount", mock.Anything, int64(1), int64(1)) + env.SAService.AssertCalled(t, "RetrieveExtSvcAccount", mock.Anything, int64(1), int64(1)) }, wantPerm: []ac.Permission{{Action: "users:impersonate", Scope: "users:*"}}, }, diff --git a/pkg/services/extsvcauth/oauthserver/oasimpl/token_test.go b/pkg/services/extsvcauth/oauthserver/oasimpl/token_test.go index f8f4eb25cce..5d0b68d4944 100644 --- a/pkg/services/extsvcauth/oauthserver/oasimpl/token_test.go +++ b/pkg/services/extsvcauth/oauthserver/oasimpl/token_test.go @@ -22,8 +22,8 @@ import ( "github.com/grafana/grafana/pkg/models/roletype" ac "github.com/grafana/grafana/pkg/services/accesscontrol" + "github.com/grafana/grafana/pkg/services/extsvcauth" "github.com/grafana/grafana/pkg/services/extsvcauth/oauthserver" - "github.com/grafana/grafana/pkg/services/serviceaccounts" "github.com/grafana/grafana/pkg/services/team" "github.com/grafana/grafana/pkg/services/user" ) @@ -703,15 +703,13 @@ func setupHandleTokenRequestEnv(t *testing.T, env *TestEnv, opt func(*oauthserve opt(client1) } - sa1 := &serviceaccounts.ServiceAccountProfileDTO{ - Id: client1.ServiceAccountID, + sa1 := &extsvcauth.ExtSvcAccount{ + ID: client1.ServiceAccountID, Name: client1.Name, Login: client1.Name, - OrgId: oauthserver.TmpOrgID, + OrgID: oauthserver.TmpOrgID, IsDisabled: false, - Created: now, - Updated: now, - Role: "Viewer", + Role: roletype.RoleNone, } user56 := &user.User{ @@ -735,7 +733,7 @@ func setupHandleTokenRequestEnv(t *testing.T, env *TestEnv, opt func(*oauthserve // To retrieve the Client, its publicKey and its permissions env.OAuthStore.On("GetExternalService", mock.Anything, client1.ClientID).Return(client1, nil) env.OAuthStore.On("GetExternalServicePublicKey", mock.Anything, client1.ClientID).Return(&jose.JSONWebKey{Key: Client1Key.Public(), Algorithm: "RS256"}, nil) - env.SAService.On("RetrieveServiceAccount", mock.Anything, oauthserver.TmpOrgID, client1.ServiceAccountID).Return(sa1, nil) + env.SAService.On("RetrieveExtSvcAccount", mock.Anything, oauthserver.TmpOrgID, client1.ServiceAccountID).Return(sa1, nil) env.AcStore.On("GetUserPermissions", mock.Anything, mock.Anything).Return(client1.SelfPermissions, nil) // To retrieve the user to impersonate, its permissions and its teams env.AcStore.On("SearchUsersPermissions", mock.Anything, mock.Anything, mock.Anything).Return(map[int64][]ac.Permission{