From dff7403b29c5cfbd9d57b4b8fead0ebd738f17ea Mon Sep 17 00:00:00 2001 From: linoman <2051016+linoman@users.noreply.github.com> Date: Wed, 25 Oct 2023 16:44:05 +0200 Subject: [PATCH] auth: implement feature flag for service account proxy (#77129) * add FlagExternalServiceAccounts to proxy service * add FlagExternalServiceAccounts value to tests --------- Co-authored-by: Gabriel MABILLE --- pkg/services/serviceaccounts/proxy/service.go | 103 ++++++++++-------- .../serviceaccounts/proxy/service_test.go | 1 + 2 files changed, 59 insertions(+), 45 deletions(-) diff --git a/pkg/services/serviceaccounts/proxy/service.go b/pkg/services/serviceaccounts/proxy/service.go index 6d77ec33692..312fa8c2990 100644 --- a/pkg/services/serviceaccounts/proxy/service.go +++ b/pkg/services/serviceaccounts/proxy/service.go @@ -2,11 +2,11 @@ package proxy import ( "context" - "fmt" "strings" "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/services/apikey" + "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/serviceaccounts" "github.com/grafana/grafana/pkg/services/serviceaccounts/extsvcaccounts" "github.com/grafana/grafana/pkg/services/serviceaccounts/manager" @@ -19,14 +19,17 @@ import ( type ServiceAccountsProxy struct { log log.Logger proxiedService serviceaccounts.Service + isProxyEnabled bool } func ProvideServiceAccountsProxy( + features *featuremgmt.FeatureManager, proxiedService *manager.ServiceAccountsService, ) (*ServiceAccountsProxy, error) { s := &ServiceAccountsProxy{ log: log.New("serviceaccounts.proxy"), proxiedService: proxiedService, + isProxyEnabled: features.IsEnabled(featuremgmt.FlagExternalServiceAccounts) || features.IsEnabled(featuremgmt.FlagExternalServiceAuth), } return s, nil } @@ -34,55 +37,59 @@ func ProvideServiceAccountsProxy( 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 s.isProxyEnabled { + 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 + 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 + if s.isProxyEnabled { + 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 s.isProxyEnabled { + 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 + 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) DeleteServiceAccountToken(ctx context.Context, orgID int64, serviceAccountID int64, tokenID int64) error { - sa, err := s.proxiedService.RetrieveServiceAccount(ctx, 0, serviceAccountID) - if err != nil { - return err + if s.isProxyEnabled { + sa, err := s.proxiedService.RetrieveServiceAccount(ctx, orgID, serviceAccountID) + if err != nil { + return err + } + + if isExternalServiceAccount(sa.Login) { + s.log.Error("unable to delete tokens for external service accounts", "serviceAccountID", serviceAccountID) + return extsvcaccounts.ErrCannotDeleteToken + } } - - fmt.Println("sa.Login: ", sa.Login) - - if isExternalServiceAccount(sa.Login) { - s.log.Error("unable to delete tokens for external service accounts", "serviceAccountID", serviceAccountID) - return extsvcaccounts.ErrCannotDeleteToken - } - - return s.proxiedService.DeleteServiceAccountToken(ctx, sa.OrgId, serviceAccountID, tokenID) + return s.proxiedService.DeleteServiceAccountToken(ctx, orgID, serviceAccountID, tokenID) } func (s *ServiceAccountsProxy) ListTokens(ctx context.Context, query *serviceaccounts.GetSATokensQuery) ([]apikey.APIKey, error) { @@ -103,7 +110,9 @@ func (s *ServiceAccountsProxy) RetrieveServiceAccount(ctx context.Context, orgID return nil, err } - sa.IsExternal = isExternalServiceAccount(sa.Login) + if s.isProxyEnabled { + sa.IsExternal = isExternalServiceAccount(sa.Login) + } return sa, nil } @@ -113,17 +122,19 @@ func (s *ServiceAccountsProxy) RetrieveServiceAccountIdByName(ctx context.Contex } 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 + if s.isProxyEnabled { + 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) @@ -135,8 +146,10 @@ func (s *ServiceAccountsProxy) SearchOrgServiceAccounts(ctx context.Context, que return nil, err } - for i := range sa.ServiceAccounts { - sa.ServiceAccounts[i].IsExternal = isExternalServiceAccount(sa.ServiceAccounts[i].Login) + if s.isProxyEnabled { + for i := range sa.ServiceAccounts { + sa.ServiceAccounts[i].IsExternal = isExternalServiceAccount(sa.ServiceAccounts[i].Login) + } } return sa, nil } diff --git a/pkg/services/serviceaccounts/proxy/service_test.go b/pkg/services/serviceaccounts/proxy/service_test.go index 1701d9fbc0d..db0325a2172 100644 --- a/pkg/services/serviceaccounts/proxy/service_test.go +++ b/pkg/services/serviceaccounts/proxy/service_test.go @@ -23,6 +23,7 @@ func TestProvideServiceAccount_crudServiceAccount(t *testing.T) { svc := ServiceAccountsProxy{ log.New("test"), serviceMock, + true, } t.Run("should create service account", func(t *testing.T) {