IntenseWebs/grafana
3
0
Fork 0
mirror of https://github.com/grafana/grafana.git synced 2025-01-09 15:43:23 -06:00
Code Issues Packages Projects Releases Wiki Activity

AuthN: Extract from OAuthServer service account management code (#76128)

Browse Source 
* 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
This commit is contained in:
Gabriel MABILLE 2023-10-10 09:20:52 +02:00 committed by GitHub
parent 1355660313
commit 007c2c8131
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 524 additions and 223 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
pkg/server/wire.go
View File

@ -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,

21
pkg/services/extsvcauth/extsvcaccounts/models.go Normal file
View File

@ -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
}

126
pkg/services/extsvcauth/extsvcaccounts/service.go Normal file
View File

@ -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)
}

211
pkg/services/extsvcauth/extsvcaccounts/service_test.go Normal file
View File

@ -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)
})
}
}

79
pkg/services/extsvcauth/extsvcmocks/extsvcaccmock.go Normal file
View File

@ -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
}

26
pkg/services/extsvcauth/models.go
View File

@ -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

123
pkg/services/extsvcauth/oauthserver/oasimpl/service.go
View File

@ -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) {

144
pkg/services/extsvcauth/oauthserver/oasimpl/service_test.go
View File

@ -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:*"}},
},

14
pkg/services/extsvcauth/oauthserver/oasimpl/token_test.go
View File

@ -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{