diff --git a/pkg/services/extsvcauth/extsvcaccounts/models.go b/pkg/services/extsvcauth/extsvcaccounts/models.go index bfef6e1ab5c..9211ae6cc2a 100644 --- a/pkg/services/extsvcauth/extsvcaccounts/models.go +++ b/pkg/services/extsvcauth/extsvcaccounts/models.go @@ -3,15 +3,23 @@ package extsvcaccounts import ( "github.com/grafana/grafana/pkg/models/roletype" ac "github.com/grafana/grafana/pkg/services/accesscontrol" + "github.com/grafana/grafana/pkg/util/errutil" ) const ( - extsvcPrefix = "extsvc-" + ExtSvcPrefix = "extsvc-" kvStoreType = "extsvc-token" // #nosec G101 - this is not a hardcoded secret tokenNamePrefix = "extsvc-token" ) +var ( + ErrCannotBeDeleted = errutil.BadRequest("extsvcaccounts.ErrCannotBeDeleted", errutil.WithPublicMessage("external service account cannot be deleted")) + ErrInvalidName = errutil.BadRequest("extsvcaccounts.ErrInvalidName", errutil.WithPublicMessage("only external service account names can be prefixed with 'extsvc-'")) + ErrCannotBeUpdated = errutil.BadRequest("extsvcaccounts.ErrCannotBeUpdated", errutil.WithPublicMessage("external service account cannot be updated")) + ErrCannotCreateToken = errutil.BadRequest("extsvcaccounts.ErrCannotCreateToken", errutil.WithPublicMessage("cannot add external service account token")) +) + // Credentials represents the credentials associated to an external service type Credentials struct { Secret string diff --git a/pkg/services/extsvcauth/extsvcaccounts/service.go b/pkg/services/extsvcauth/extsvcaccounts/service.go index 1611d2d4503..e8bb45cb535 100644 --- a/pkg/services/extsvcauth/extsvcaccounts/service.go +++ b/pkg/services/extsvcauth/extsvcaccounts/service.go @@ -14,6 +14,7 @@ import ( "github.com/grafana/grafana/pkg/services/secrets" "github.com/grafana/grafana/pkg/services/secrets/kvstore" sa "github.com/grafana/grafana/pkg/services/serviceaccounts" + "github.com/grafana/grafana/pkg/services/serviceaccounts/manager" ) type ExtSvcAccountsService struct { @@ -23,7 +24,7 @@ type ExtSvcAccountsService struct { skvStore kvstore.SecretsKVStore } -func ProvideExtSvcAccountsService(acSvc ac.Service, saSvc sa.Service, db db.DB, secretsSvc secrets.Service) *ExtSvcAccountsService { +func ProvideExtSvcAccountsService(acSvc ac.Service, saSvc *manager.ServiceAccountsService, db db.DB, secretsSvc secrets.Service) *ExtSvcAccountsService { logger := log.New("serviceauth.extsvcaccounts") return &ExtSvcAccountsService{ acSvc: acSvc, @@ -96,7 +97,7 @@ func (esa *ExtSvcAccountsService) ManageExtSvcAccount(ctx context.Context, cmd * return 0, nil } - saID, errRetrieve := esa.saSvc.RetrieveServiceAccountIdByName(ctx, cmd.OrgID, extsvcPrefix+cmd.ExtSvcSlug) + saID, errRetrieve := esa.saSvc.RetrieveServiceAccountIdByName(ctx, cmd.OrgID, ExtSvcPrefix+cmd.ExtSvcSlug) if errRetrieve != nil && !errors.Is(errRetrieve, sa.ErrServiceAccountNotFound) { return 0, errRetrieve } @@ -139,7 +140,7 @@ func (esa *ExtSvcAccountsService) saveExtSvcAccount(ctx context.Context, cmd *sa // 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: extsvcPrefix + cmd.ExtSvcSlug, + Name: ExtSvcPrefix + cmd.ExtSvcSlug, Role: newRole(roletype.RoleNone), IsDisabled: newBool(false), }) diff --git a/pkg/services/extsvcauth/extsvcaccounts/service_test.go b/pkg/services/extsvcauth/extsvcaccounts/service_test.go index e6befbde733..3a5abbfe879 100644 --- a/pkg/services/extsvcauth/extsvcaccounts/service_test.go +++ b/pkg/services/extsvcauth/extsvcaccounts/service_test.go @@ -87,7 +87,7 @@ func TestExtSvcAccountsService_ManageExtSvcAccount(t *testing.T) { 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 == extsvcPrefix+extSvcSlug })) + mock.MatchedBy(func(slug string) bool { return slug == ExtSvcPrefix+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 })) @@ -114,7 +114,7 @@ func TestExtSvcAccountsService_ManageExtSvcAccount(t *testing.T) { 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 == extsvcPrefix+extSvcSlug })) + mock.MatchedBy(func(slug string) bool { return slug == ExtSvcPrefix+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 })) @@ -143,11 +143,11 @@ func TestExtSvcAccountsService_ManageExtSvcAccount(t *testing.T) { 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 == extsvcPrefix+extSvcSlug })) + mock.MatchedBy(func(slug string) bool { return slug == ExtSvcPrefix+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 == extsvcPrefix+extSvcSlug && *cmd.Role == roletype.RoleNone + return cmd.Name == ExtSvcPrefix+extSvcSlug && *cmd.Role == roletype.RoleNone }), ) env.AcStore.AssertCalled(t, "SaveExternalServiceRole", mock.Anything, @@ -177,7 +177,7 @@ func TestExtSvcAccountsService_ManageExtSvcAccount(t *testing.T) { 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 == extsvcPrefix+extSvcSlug })) + mock.MatchedBy(func(slug string) bool { return slug == ExtSvcPrefix+extSvcSlug })) env.AcStore.AssertCalled(t, "SaveExternalServiceRole", mock.Anything, mock.MatchedBy(func(cmd ac.SaveExternalServiceRoleCommand) bool { return cmd.ServiceAccountID == int64(11) && cmd.ExternalServiceID == extSvcSlug && @@ -257,7 +257,7 @@ func TestExtSvcAccountsService_SaveExternalService(t *testing.T) { checks: func(t *testing.T, env *TestEnv) { env.SaSvc.AssertCalled(t, "RetrieveServiceAccountIdByName", mock.Anything, mock.MatchedBy(func(orgID int64) bool { return orgID == tmpOrgID }), - mock.MatchedBy(func(slug string) bool { return slug == extsvcPrefix+extSvcSlug })) + mock.MatchedBy(func(slug string) bool { return slug == ExtSvcPrefix+extSvcSlug })) env.SaSvc.AssertCalled(t, "DeleteServiceAccount", mock.Anything, mock.MatchedBy(func(orgID int64) bool { return orgID == tmpOrgID }), mock.MatchedBy(func(saID int64) bool { return saID == extSvcAccID })) @@ -287,7 +287,7 @@ func TestExtSvcAccountsService_SaveExternalService(t *testing.T) { checks: func(t *testing.T, env *TestEnv) { env.SaSvc.AssertCalled(t, "RetrieveServiceAccountIdByName", mock.Anything, mock.MatchedBy(func(orgID int64) bool { return orgID == tmpOrgID }), - mock.MatchedBy(func(slug string) bool { return slug == extsvcPrefix+extSvcSlug })) + mock.MatchedBy(func(slug string) bool { return slug == ExtSvcPrefix+extSvcSlug })) env.SaSvc.AssertCalled(t, "DeleteServiceAccount", mock.Anything, mock.MatchedBy(func(orgID int64) bool { return orgID == tmpOrgID }), mock.MatchedBy(func(saID int64) bool { return saID == extSvcAccID })) @@ -319,11 +319,11 @@ func TestExtSvcAccountsService_SaveExternalService(t *testing.T) { checks: func(t *testing.T, env *TestEnv) { env.SaSvc.AssertCalled(t, "RetrieveServiceAccountIdByName", mock.Anything, mock.MatchedBy(func(orgID int64) bool { return orgID == tmpOrgID }), - mock.MatchedBy(func(slug string) bool { return slug == extsvcPrefix+extSvcSlug })) + mock.MatchedBy(func(slug string) bool { return slug == ExtSvcPrefix+extSvcSlug })) env.SaSvc.AssertCalled(t, "CreateServiceAccount", mock.Anything, mock.MatchedBy(func(orgID int64) bool { return orgID == tmpOrgID }), mock.MatchedBy(func(cmd *sa.CreateServiceAccountForm) bool { - return cmd.Name == extsvcPrefix+extSvcSlug && *cmd.Role == roletype.RoleNone + return cmd.Name == ExtSvcPrefix+extSvcSlug && *cmd.Role == roletype.RoleNone }), ) env.AcStore.AssertCalled(t, "SaveExternalServiceRole", mock.Anything, @@ -360,7 +360,7 @@ func TestExtSvcAccountsService_SaveExternalService(t *testing.T) { checks: func(t *testing.T, env *TestEnv) { env.SaSvc.AssertCalled(t, "RetrieveServiceAccountIdByName", mock.Anything, mock.MatchedBy(func(orgID int64) bool { return orgID == tmpOrgID }), - mock.MatchedBy(func(slug string) bool { return slug == extsvcPrefix+extSvcSlug })) + mock.MatchedBy(func(slug string) bool { return slug == ExtSvcPrefix+extSvcSlug })) env.AcStore.AssertCalled(t, "SaveExternalServiceRole", mock.Anything, mock.MatchedBy(func(cmd ac.SaveExternalServiceRoleCommand) bool { return cmd.ServiceAccountID == int64(11) && cmd.ExternalServiceID == extSvcSlug && diff --git a/pkg/services/serviceaccounts/database/store.go b/pkg/services/serviceaccounts/database/store.go index d3be8c981de..292719cedc5 100644 --- a/pkg/services/serviceaccounts/database/store.go +++ b/pkg/services/serviceaccounts/database/store.go @@ -44,7 +44,7 @@ func ProvideServiceAccountsStore(cfg *setting.Cfg, store db.DB, apiKeyService ap // CreateServiceAccount creates service account func (s *ServiceAccountsStoreImpl) CreateServiceAccount(ctx context.Context, orgId int64, saForm *serviceaccounts.CreateServiceAccountForm) (*serviceaccounts.ServiceAccountDTO, error) { - generatedLogin := "sa-" + strings.ToLower(saForm.Name) + generatedLogin := serviceaccounts.ServiceAccountPrefix + strings.ToLower(saForm.Name) generatedLogin = strings.ReplaceAll(generatedLogin, " ", "-") isDisabled := false role := org.RoleViewer diff --git a/pkg/services/serviceaccounts/models.go b/pkg/services/serviceaccounts/models.go index a01f2d0554a..a115628a148 100644 --- a/pkg/services/serviceaccounts/models.go +++ b/pkg/services/serviceaccounts/models.go @@ -14,6 +14,10 @@ var ( ScopeID = accesscontrol.Scope("serviceaccounts", "id", accesscontrol.Parameter(":serviceAccountId")) ) +const ( + ServiceAccountPrefix = "sa-" +) + const ( ActionRead = "serviceaccounts:read" ActionWrite = "serviceaccounts:write" @@ -146,7 +150,10 @@ type ServiceAccountProfileDTO struct { // example: Editor Role string `json:"role" xorm:"role"` // example: [] - Teams []string `json:"teams" xorm:"-"` + Teams []string `json:"teams" xorm:"-"` + // example: false + IsExternal bool `json:"isExternal,omitempty" xorm:"-"` + Tokens int64 `json:"tokens,omitempty"` AccessControl map[string]bool `json:"accessControl,omitempty" xorm:"-"` } diff --git a/pkg/services/serviceaccounts/proxy/service.go b/pkg/services/serviceaccounts/proxy/service.go new file mode 100644 index 00000000000..46785252a98 --- /dev/null +++ b/pkg/services/serviceaccounts/proxy/service.go @@ -0,0 +1,109 @@ +package proxy + +import ( + "context" + "strings" + + "github.com/grafana/grafana/pkg/infra/log" + "github.com/grafana/grafana/pkg/services/apikey" + "github.com/grafana/grafana/pkg/services/extsvcauth/extsvcaccounts" + "github.com/grafana/grafana/pkg/services/serviceaccounts" + "github.com/grafana/grafana/pkg/services/serviceaccounts/manager" +) + +// ServiceAccountsProxy is a proxy for the serviceaccounts.Service interface +// that is used to add validations to service accounts and protects external +// service accounts from being modified by users. + +type ServiceAccountsProxy struct { + log log.Logger + proxiedService serviceaccounts.Service +} + +func ProvideServiceAccountsProxy( + proxiedService *manager.ServiceAccountsService, +) (*ServiceAccountsProxy, error) { + s := &ServiceAccountsProxy{ + log: log.New("serviceaccounts.proxy"), + proxiedService: proxiedService, + } + return s, nil +} + +var _ serviceaccounts.Service = (*ServiceAccountsProxy)(nil) + +func (s *ServiceAccountsProxy) AddServiceAccountToken(ctx context.Context, serviceAccountID int64, cmd *serviceaccounts.AddServiceAccountTokenCommand) (*apikey.APIKey, error) { + sa, err := s.proxiedService.RetrieveServiceAccount(ctx, cmd.OrgId, serviceAccountID) + if err != nil { + return nil, err + } + + if isExternalServiceAccount(sa.Login) { + s.log.Error("unable to create tokens for external service accounts", "serviceAccountID", serviceAccountID) + return nil, extsvcaccounts.ErrCannotCreateToken + } + + return s.proxiedService.AddServiceAccountToken(ctx, serviceAccountID, cmd) +} + +func (s *ServiceAccountsProxy) CreateServiceAccount(ctx context.Context, orgID int64, saForm *serviceaccounts.CreateServiceAccountForm) (*serviceaccounts.ServiceAccountDTO, error) { + if !isNameValid(saForm.Name) { + s.log.Error("Unable to create service account with a protected name", "name", saForm.Name) + return nil, extsvcaccounts.ErrInvalidName + } + return s.proxiedService.CreateServiceAccount(ctx, orgID, saForm) +} + +func (s *ServiceAccountsProxy) DeleteServiceAccount(ctx context.Context, orgID, serviceAccountID int64) error { + sa, err := s.proxiedService.RetrieveServiceAccount(ctx, orgID, serviceAccountID) + if err != nil { + return err + } + + if isExternalServiceAccount(sa.Login) { + s.log.Error("unable to delete external service accounts", "serviceAccountID", serviceAccountID) + return extsvcaccounts.ErrCannotBeDeleted + } + + return s.proxiedService.DeleteServiceAccount(ctx, orgID, serviceAccountID) +} + +func (s *ServiceAccountsProxy) RetrieveServiceAccount(ctx context.Context, orgID, serviceAccountID int64) (*serviceaccounts.ServiceAccountProfileDTO, error) { + sa, err := s.proxiedService.RetrieveServiceAccount(ctx, orgID, serviceAccountID) + if err != nil { + return nil, err + } + + sa.IsExternal = isExternalServiceAccount(sa.Login) + + return sa, nil +} + +func (s *ServiceAccountsProxy) RetrieveServiceAccountIdByName(ctx context.Context, orgID int64, name string) (int64, error) { + return s.proxiedService.RetrieveServiceAccountIdByName(ctx, orgID, name) +} + +func (s *ServiceAccountsProxy) UpdateServiceAccount(ctx context.Context, orgID, serviceAccountID int64, saForm *serviceaccounts.UpdateServiceAccountForm) (*serviceaccounts.ServiceAccountProfileDTO, error) { + if !isNameValid(*saForm.Name) { + s.log.Error("Invalid service account name", "name", *saForm.Name) + return nil, extsvcaccounts.ErrInvalidName + } + sa, err := s.proxiedService.RetrieveServiceAccount(ctx, orgID, serviceAccountID) + if err != nil { + return nil, err + } + if isExternalServiceAccount(sa.Login) { + s.log.Error("unable to update external service accounts", "serviceAccountID", serviceAccountID) + return nil, extsvcaccounts.ErrCannotBeUpdated + } + + return s.proxiedService.UpdateServiceAccount(ctx, orgID, serviceAccountID, saForm) +} + +func isNameValid(name string) bool { + return !strings.HasPrefix(name, extsvcaccounts.ExtSvcPrefix) +} + +func isExternalServiceAccount(login string) bool { + return strings.HasPrefix(login, serviceaccounts.ServiceAccountPrefix+extsvcaccounts.ExtSvcPrefix) +} diff --git a/pkg/services/serviceaccounts/proxy/service_test.go b/pkg/services/serviceaccounts/proxy/service_test.go new file mode 100644 index 00000000000..78893503a5a --- /dev/null +++ b/pkg/services/serviceaccounts/proxy/service_test.go @@ -0,0 +1,259 @@ +package proxy + +import ( + "context" + "testing" + + "github.com/grafana/grafana/pkg/infra/log" + "github.com/grafana/grafana/pkg/services/apikey" + "github.com/grafana/grafana/pkg/services/extsvcauth/extsvcaccounts" + "github.com/grafana/grafana/pkg/services/serviceaccounts" + "github.com/stretchr/testify/assert" +) + +type FakeServiceAccountsService struct { + ExpectedServiceAccountProfileDTO *serviceaccounts.ServiceAccountProfileDTO +} + +var _ serviceaccounts.Service = (*FakeServiceAccountsService)(nil) + +func newServiceAccountServiceFake() *FakeServiceAccountsService { + return &FakeServiceAccountsService{} +} + +func (f *FakeServiceAccountsService) CreateServiceAccount(ctx context.Context, orgID int64, saForm *serviceaccounts.CreateServiceAccountForm) (*serviceaccounts.ServiceAccountDTO, error) { + return nil, nil +} + +func (f *FakeServiceAccountsService) DeleteServiceAccount(ctx context.Context, orgID, serviceAccountID int64) error { + return nil +} + +func (f *FakeServiceAccountsService) RetrieveServiceAccount(ctx context.Context, orgID, serviceAccountID int64) (*serviceaccounts.ServiceAccountProfileDTO, error) { + return f.ExpectedServiceAccountProfileDTO, nil +} + +func (f *FakeServiceAccountsService) RetrieveServiceAccountIdByName(ctx context.Context, orgID int64, name string) (int64, error) { + return 0, nil +} + +func (f *FakeServiceAccountsService) UpdateServiceAccount(ctx context.Context, orgID, serviceAccountID int64, + saForm *serviceaccounts.UpdateServiceAccountForm) (*serviceaccounts.ServiceAccountProfileDTO, error) { + return nil, nil +} + +func (f *FakeServiceAccountsService) AddServiceAccountToken(ctx context.Context, serviceAccountID int64, + cmd *serviceaccounts.AddServiceAccountTokenCommand) (*apikey.APIKey, error) { + return nil, nil +} + +func TestProvideServiceAccount_DeleteServiceAccount(t *testing.T) { + testOrgId := int64(1) + testServiceAccountId := int64(1) + serviceMock := newServiceAccountServiceFake() + svc := ServiceAccountsProxy{ + log.New("test"), + serviceMock, + } + + t.Run("should create service account", func(t *testing.T) { + testCases := []struct { + description string + form serviceaccounts.CreateServiceAccountForm + expectedError error + }{ + { + description: "should create service account and not return error", + form: serviceaccounts.CreateServiceAccountForm{ + Name: "my-service-account", + }, + expectedError: nil, + }, + { + description: "should not allow to create a service account with extsvc prefix", + form: serviceaccounts.CreateServiceAccountForm{ + Name: "extsvc-my-service-account", + }, + expectedError: extsvcaccounts.ErrInvalidName, + }, + } + + for _, tc := range testCases { + t.Run(tc.description, func(t *testing.T) { + tc := tc + _, err := svc.CreateServiceAccount(context.Background(), testOrgId, &tc.form) + assert.Equal(t, err, tc.expectedError, tc.description) + }) + } + }) + + t.Run("should delete service account", func(t *testing.T) { + testCases := []struct { + description string + expectedError error + expectedServiceAccount *serviceaccounts.ServiceAccountProfileDTO + }{ + { + description: "should allow to delete a service account", + expectedError: nil, + expectedServiceAccount: &serviceaccounts.ServiceAccountProfileDTO{ + Login: "my-service-account", + }, + }, + { + description: "should not allow to delete a service account with sa-extsvc prefix", + expectedError: extsvcaccounts.ErrCannotBeDeleted, + expectedServiceAccount: &serviceaccounts.ServiceAccountProfileDTO{ + Login: "sa-extsvc-my-service-account", + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.description, func(t *testing.T) { + serviceMock.ExpectedServiceAccountProfileDTO = tc.expectedServiceAccount + err := svc.DeleteServiceAccount(context.Background(), testOrgId, testServiceAccountId) + assert.Equal(t, err, tc.expectedError, tc.description) + }) + } + }) + + t.Run("should retrieve service account with IsExternal field", func(t *testing.T) { + testCases := []struct { + description string + expectedServiceAccount *serviceaccounts.ServiceAccountProfileDTO + expectedIsExternal bool + }{ + { + description: "should not mark as external", + expectedServiceAccount: &serviceaccounts.ServiceAccountProfileDTO{ + Login: "my-service-account", + }, + expectedIsExternal: false, + }, + { + description: "should mark as external", + expectedServiceAccount: &serviceaccounts.ServiceAccountProfileDTO{ + Login: "sa-extsvc-my-service-account", + }, + expectedIsExternal: true, + }, + } + + for _, tc := range testCases { + t.Run(tc.description, func(t *testing.T) { + serviceMock.ExpectedServiceAccountProfileDTO = tc.expectedServiceAccount + sa, err := svc.RetrieveServiceAccount(context.Background(), testOrgId, testServiceAccountId) + assert.NoError(t, err, tc.description) + assert.Equal(t, tc.expectedIsExternal, sa.IsExternal, tc.description) + }) + } + }) + + t.Run("should update service account", func(t *testing.T) { + nameWithoutProtectedPrefix := "my-updated-service-account" + nameWithProtectedPrefix := "extsvc-my-updated-service-account" + testCases := []struct { + description string + form serviceaccounts.UpdateServiceAccountForm + expectedServiceAccount *serviceaccounts.ServiceAccountProfileDTO + expectedError error + }{ + { + description: "should update a non-external service account with a valid name", + form: serviceaccounts.UpdateServiceAccountForm{ + Name: &nameWithoutProtectedPrefix, + }, + expectedServiceAccount: &serviceaccounts.ServiceAccountProfileDTO{ + Login: "my-service-account", + }, + expectedError: nil, + }, + { + description: "should not allow to update a non-external service account with extsvc prefix", + form: serviceaccounts.UpdateServiceAccountForm{ + Name: &nameWithProtectedPrefix, + }, + expectedServiceAccount: &serviceaccounts.ServiceAccountProfileDTO{ + Login: "my-service-account", + }, + expectedError: extsvcaccounts.ErrInvalidName, + }, + { + description: "should not allow to update an external service account with a valid name", + form: serviceaccounts.UpdateServiceAccountForm{ + Name: &nameWithoutProtectedPrefix, + }, + expectedServiceAccount: &serviceaccounts.ServiceAccountProfileDTO{ + Login: "sa-extsvc-my-service-account", + }, + expectedError: extsvcaccounts.ErrCannotBeUpdated, + }, + { + description: "should not allow to update an external service account with a extsvc prefix", + form: serviceaccounts.UpdateServiceAccountForm{ + Name: &nameWithProtectedPrefix, + }, + expectedServiceAccount: &serviceaccounts.ServiceAccountProfileDTO{ + Login: "sa-extsvc-my-service-account", + }, + expectedError: extsvcaccounts.ErrInvalidName, + }, + } + + for _, tc := range testCases { + t.Run(tc.description, func(t *testing.T) { + tc := tc + serviceMock.ExpectedServiceAccountProfileDTO = tc.expectedServiceAccount + _, err := svc.UpdateServiceAccount(context.Background(), testOrgId, testServiceAccountId, &tc.form) + assert.Equal(t, tc.expectedError, err, tc.description) + }) + } + }) + + t.Run("should add service account tokens", func(t *testing.T) { + testCases := []struct { + description string + cmd serviceaccounts.AddServiceAccountTokenCommand + expectedServiceAccount *serviceaccounts.ServiceAccountProfileDTO + expectedError error + }{ + { + description: "should allow to create a service account token", + cmd: serviceaccounts.AddServiceAccountTokenCommand{ + OrgId: testOrgId, + }, + expectedServiceAccount: &serviceaccounts.ServiceAccountProfileDTO{ + Login: "my-service-account", + }, + expectedError: nil, + }, + { + description: "should not allow to create a service account token", + cmd: serviceaccounts.AddServiceAccountTokenCommand{ + OrgId: testOrgId, + }, + expectedServiceAccount: &serviceaccounts.ServiceAccountProfileDTO{ + Login: "sa-extsvc-my-service-account", + }, + expectedError: extsvcaccounts.ErrCannotCreateToken, + }, + } + + for _, tc := range testCases { + t.Run(tc.description, func(t *testing.T) { + tc := tc + serviceMock.ExpectedServiceAccountProfileDTO = tc.expectedServiceAccount + _, err := svc.AddServiceAccountToken(context.Background(), testServiceAccountId, &tc.cmd) + assert.Equal(t, tc.expectedError, err, tc.description) + }) + } + }) + + t.Run("should identify service account logins for being external or not", func(t *testing.T) { + assert.False(t, isExternalServiceAccount("my-service-account")) + assert.False(t, isExternalServiceAccount("sa-my-service-account")) + assert.False(t, isExternalServiceAccount("extsvc-my-service-account")) + assert.True(t, isExternalServiceAccount("sa-extsvc-my-service-account")) + }) +}