From fe351b308faa0366b83b671a2865828b8aa3257a Mon Sep 17 00:00:00 2001 From: Ieva Date: Fri, 6 May 2022 09:35:37 +0100 Subject: [PATCH] backport 48171 (#48775) --- pkg/api/common_test.go | 27 +++++++------------ pkg/services/accesscontrol/accesscontrol.go | 6 +++++ .../ossaccesscontrol/ossaccesscontrol.go | 2 +- .../dashboards/manager/dashboard_service.go | 2 +- .../dashboard_service_integration_test.go | 16 ++++++++--- .../dashboards/manager/folder_service.go | 2 +- .../dashboards/manager/folder_service_test.go | 2 ++ .../libraryelements/libraryelements_test.go | 21 ++++++++++----- .../librarypanels/librarypanels_test.go | 5 +++- .../serviceaccounts/database/database.go | 3 +-- pkg/services/sqlstore/annotation.go | 3 +-- pkg/services/sqlstore/dashboard.go | 4 +-- pkg/services/sqlstore/org_users.go | 5 ++-- pkg/services/sqlstore/team.go | 7 +++-- 14 files changed, 59 insertions(+), 46 deletions(-) diff --git a/pkg/api/common_test.go b/pkg/api/common_test.go index e3bd0a7f023..a725057e06c 100644 --- a/pkg/api/common_test.go +++ b/pkg/api/common_test.go @@ -228,15 +228,13 @@ func (s *fakeRenderService) Init() error { } func setupAccessControlScenarioContext(t *testing.T, cfg *setting.Cfg, url string, permissions []*accesscontrol.Permission) (*scenarioContext, *HTTPServer) { - features := featuremgmt.WithFeatures(featuremgmt.FlagAccesscontrol) - cfg.IsFeatureToggleEnabled = features.IsEnabled cfg.Quota.Enabled = false store := sqlstore.InitTestDB(t) hs := &HTTPServer{ Cfg: cfg, Live: newTestLive(t, store), - Features: features, + Features: featuremgmt.WithFeatures(), QuotaService: "a.QuotaService{Cfg: cfg}, RouteRegister: routing.NewRouteRegister(), AccessControl: accesscontrolmock.New().WithPermissions(permissions), @@ -330,39 +328,32 @@ func setupSimpleHTTPServer(features *featuremgmt.FeatureManager) *HTTPServer { } func setupHTTPServer(t *testing.T, useFakeAccessControl bool, enableAccessControl bool) accessControlScenarioContext { - // Use a new conf - features := featuremgmt.WithFeatures("accesscontrol", enableAccessControl) - cfg := setting.NewCfg() - cfg.IsFeatureToggleEnabled = features.IsEnabled - - return setupHTTPServerWithCfg(t, useFakeAccessControl, enableAccessControl, cfg) + return setupHTTPServerWithCfg(t, useFakeAccessControl, enableAccessControl, setting.NewCfg()) } func setupHTTPServerWithMockDb(t *testing.T, useFakeAccessControl bool, enableAccessControl bool) accessControlScenarioContext { // Use a new conf - features := featuremgmt.WithFeatures("accesscontrol", enableAccessControl) cfg := setting.NewCfg() - cfg.IsFeatureToggleEnabled = features.IsEnabled - db := sqlstore.InitTestDB(t) - db.Cfg = cfg + db.Cfg = setting.NewCfg() return setupHTTPServerWithCfgDb(t, useFakeAccessControl, enableAccessControl, cfg, db, mockstore.NewSQLStoreMock()) } func setupHTTPServerWithCfg(t *testing.T, useFakeAccessControl, enableAccessControl bool, cfg *setting.Cfg) accessControlScenarioContext { - var featureFlags []string - if enableAccessControl { - featureFlags = append(featureFlags, featuremgmt.FlagAccesscontrol) + var db *sqlstore.SQLStore + if useFakeAccessControl && enableAccessControl { + db = sqlstore.InitTestDB(t, sqlstore.InitTestDBOpt{FeatureFlags: []string{featuremgmt.FlagAccesscontrol}}) + } else { + db = sqlstore.InitTestDB(t, sqlstore.InitTestDBOpt{}) } - db := sqlstore.InitTestDB(t, sqlstore.InitTestDBOpt{FeatureFlags: featureFlags}) return setupHTTPServerWithCfgDb(t, useFakeAccessControl, enableAccessControl, cfg, db, db) } func setupHTTPServerWithCfgDb(t *testing.T, useFakeAccessControl, enableAccessControl bool, cfg *setting.Cfg, db *sqlstore.SQLStore, store sqlstore.Store) accessControlScenarioContext { t.Helper() - features := featuremgmt.WithFeatures("accesscontrol", enableAccessControl) + features := featuremgmt.WithFeatures(featuremgmt.FlagAccesscontrol, enableAccessControl) cfg.IsFeatureToggleEnabled = features.IsEnabled var acmock *accesscontrolmock.Mock diff --git a/pkg/services/accesscontrol/accesscontrol.go b/pkg/services/accesscontrol/accesscontrol.go index 161d9178cdb..32c048e9310 100644 --- a/pkg/services/accesscontrol/accesscontrol.go +++ b/pkg/services/accesscontrol/accesscontrol.go @@ -6,6 +6,8 @@ import ( "strings" "github.com/grafana/grafana/pkg/models" + "github.com/grafana/grafana/pkg/services/featuremgmt" + "github.com/grafana/grafana/pkg/setting" ) type Options struct { @@ -219,3 +221,7 @@ func extractPrefixes(prefix string) (string, string, bool) { attributePrefix := rootPrefix + parts[1] + ":" return rootPrefix, attributePrefix, true } + +func IsDisabled(cfg *setting.Cfg) bool { + return !cfg.IsFeatureToggleEnabled(featuremgmt.FlagAccesscontrol) +} diff --git a/pkg/services/accesscontrol/ossaccesscontrol/ossaccesscontrol.go b/pkg/services/accesscontrol/ossaccesscontrol/ossaccesscontrol.go index 36ed825e626..26b104e2dbe 100644 --- a/pkg/services/accesscontrol/ossaccesscontrol/ossaccesscontrol.go +++ b/pkg/services/accesscontrol/ossaccesscontrol/ossaccesscontrol.go @@ -197,7 +197,7 @@ func (ac *OSSAccessControlService) GetUserBuiltInRoles(user *models.SignedInUser builtInRoles := []string{string(user.OrgRole)} // With built-in role simplifying, inheritance is performed upon role registration. - if !ac.features.IsEnabled(featuremgmt.FlagAccesscontrolBuiltins) { + if ac.IsDisabled() { for _, br := range user.OrgRole.Children() { builtInRoles = append(builtInRoles, string(br)) } diff --git a/pkg/services/dashboards/manager/dashboard_service.go b/pkg/services/dashboards/manager/dashboard_service.go index d23bf6fce8a..9c288620e16 100644 --- a/pkg/services/dashboards/manager/dashboard_service.go +++ b/pkg/services/dashboards/manager/dashboard_service.go @@ -447,7 +447,7 @@ func (dr *DashboardServiceImpl) GetDashboardsByPluginID(ctx context.Context, que func (dr *DashboardServiceImpl) setDefaultPermissions(ctx context.Context, dto *m.SaveDashboardDTO, dash *models.Dashboard, provisioned bool) error { inFolder := dash.FolderId > 0 - if dr.features.IsEnabled(featuremgmt.FlagAccesscontrol) { + if !accesscontrol.IsDisabled(dr.cfg) { var permissions []accesscontrol.SetResourcePermissionCommand if !provisioned { permissions = append(permissions, accesscontrol.SetResourcePermissionCommand{ diff --git a/pkg/services/dashboards/manager/dashboard_service_integration_test.go b/pkg/services/dashboards/manager/dashboard_service_integration_test.go index 2bb0d4d571c..99a9d6cb97a 100644 --- a/pkg/services/dashboards/manager/dashboard_service_integration_test.go +++ b/pkg/services/dashboards/manager/dashboard_service_integration_test.go @@ -858,8 +858,10 @@ func callSaveWithResult(t *testing.T, cmd models.SaveDashboardCommand, sqlStore dto := toSaveDashboardDto(cmd) dashboardStore := database.ProvideDashboardStore(sqlStore) + cfg := setting.NewCfg() + cfg.IsFeatureToggleEnabled = featuremgmt.WithFeatures().IsEnabled service := ProvideDashboardService( - setting.NewCfg(), dashboardStore, &dummyDashAlertExtractor{}, + cfg, dashboardStore, &dummyDashAlertExtractor{}, featuremgmt.WithFeatures(), accesscontrolmock.NewPermissionsServicesMock(), ) res, err := service.SaveDashboard(context.Background(), &dto, false) @@ -871,8 +873,10 @@ func callSaveWithResult(t *testing.T, cmd models.SaveDashboardCommand, sqlStore func callSaveWithError(cmd models.SaveDashboardCommand, sqlStore *sqlstore.SQLStore) error { dto := toSaveDashboardDto(cmd) dashboardStore := database.ProvideDashboardStore(sqlStore) + cfg := setting.NewCfg() + cfg.IsFeatureToggleEnabled = featuremgmt.WithFeatures().IsEnabled service := ProvideDashboardService( - setting.NewCfg(), dashboardStore, &dummyDashAlertExtractor{}, + cfg, dashboardStore, &dummyDashAlertExtractor{}, featuremgmt.WithFeatures(), accesscontrolmock.NewPermissionsServicesMock(), ) _, err := service.SaveDashboard(context.Background(), &dto, false) @@ -902,8 +906,10 @@ func saveTestDashboard(t *testing.T, title string, orgID, folderID int64, sqlSto } dashboardStore := database.ProvideDashboardStore(sqlStore) + cfg := setting.NewCfg() + cfg.IsFeatureToggleEnabled = featuremgmt.WithFeatures().IsEnabled service := ProvideDashboardService( - setting.NewCfg(), dashboardStore, &dummyDashAlertExtractor{}, + cfg, dashboardStore, &dummyDashAlertExtractor{}, featuremgmt.WithFeatures(), accesscontrolmock.NewPermissionsServicesMock(), ) res, err := service.SaveDashboard(context.Background(), &dto, false) @@ -934,8 +940,10 @@ func saveTestFolder(t *testing.T, title string, orgID int64, sqlStore *sqlstore. } dashboardStore := database.ProvideDashboardStore(sqlStore) + cfg := setting.NewCfg() + cfg.IsFeatureToggleEnabled = featuremgmt.WithFeatures().IsEnabled service := ProvideDashboardService( - setting.NewCfg(), dashboardStore, &dummyDashAlertExtractor{}, + cfg, dashboardStore, &dummyDashAlertExtractor{}, featuremgmt.WithFeatures(), accesscontrolmock.NewPermissionsServicesMock(), ) res, err := service.SaveDashboard(context.Background(), &dto, false) diff --git a/pkg/services/dashboards/manager/folder_service.go b/pkg/services/dashboards/manager/folder_service.go index 77b31cf69ad..49641e26377 100644 --- a/pkg/services/dashboards/manager/folder_service.go +++ b/pkg/services/dashboards/manager/folder_service.go @@ -171,7 +171,7 @@ func (f *FolderServiceImpl) CreateFolder(ctx context.Context, user *models.Signe } var permissionErr error - if f.features.IsEnabled(featuremgmt.FlagAccesscontrol) { + if !accesscontrol.IsDisabled(f.cfg) { _, permissionErr = f.permissions.SetPermissions(ctx, orgID, folder.Uid, []accesscontrol.SetResourcePermissionCommand{ {UserID: userID, Permission: models.PERMISSION_ADMIN.String()}, {BuiltinRole: string(models.ROLE_EDITOR), Permission: models.PERMISSION_EDIT.String()}, diff --git a/pkg/services/dashboards/manager/folder_service_test.go b/pkg/services/dashboards/manager/folder_service_test.go index 69243876a5a..1cc03e0cb4a 100644 --- a/pkg/services/dashboards/manager/folder_service_test.go +++ b/pkg/services/dashboards/manager/folder_service_test.go @@ -31,6 +31,7 @@ func TestProvideFolderService(t *testing.T) { store := &dashboards.FakeDashboardStore{} cfg := setting.NewCfg() features := featuremgmt.WithFeatures() + cfg.IsFeatureToggleEnabled = features.IsEnabled permissionsServices := acmock.NewPermissionsServicesMock() dashboardService := ProvideDashboardService(cfg, store, nil, features, permissionsServices) ac := acmock.New() @@ -49,6 +50,7 @@ func TestFolderService(t *testing.T) { store := &dashboards.FakeDashboardStore{} cfg := setting.NewCfg() features := featuremgmt.WithFeatures() + cfg.IsFeatureToggleEnabled = features.IsEnabled permissionsServices := acmock.NewPermissionsServicesMock() dashboardService := ProvideDashboardService(cfg, store, nil, features, permissionsServices) mockStore := mockstore.NewSQLStoreMock() diff --git a/pkg/services/libraryelements/libraryelements_test.go b/pkg/services/libraryelements/libraryelements_test.go index a4bc6ed6cfa..8021c262aa0 100644 --- a/pkg/services/libraryelements/libraryelements_test.go +++ b/pkg/services/libraryelements/libraryelements_test.go @@ -202,9 +202,12 @@ func createDashboard(t *testing.T, sqlStore *sqlstore.SQLStore, user models.Sign dashboardStore := database.ProvideDashboardStore(sqlStore) dashAlertExtractor := alerting.ProvideDashAlertExtractorService(nil, nil, nil) + features := featuremgmt.WithFeatures() + cfg := setting.NewCfg() + cfg.IsFeatureToggleEnabled = features.IsEnabled service := dashboardservice.ProvideDashboardService( - setting.NewCfg(), dashboardStore, dashAlertExtractor, - featuremgmt.WithFeatures(), acmock.NewPermissionsServicesMock(), + cfg, dashboardStore, dashAlertExtractor, + features, acmock.NewPermissionsServicesMock(), ) dashboard, err := service.SaveDashboard(context.Background(), dashItem, true) require.NoError(t, err) @@ -218,6 +221,7 @@ func createFolderWithACL(t *testing.T, sqlStore *sqlstore.SQLStore, title string cfg := setting.NewCfg() features := featuremgmt.WithFeatures() + cfg.IsFeatureToggleEnabled = features.IsEnabled permissionsServices := acmock.NewPermissionsServicesMock() dashboardStore := database.ProvideDashboardStore(sqlStore) @@ -317,17 +321,20 @@ func testScenario(t *testing.T, desc string, fn func(t *testing.T, sc scenarioCo sqlStore := sqlstore.InitTestDB(t) guardian.InitLegacyGuardian(sqlStore) dashboardStore := database.ProvideDashboardStore(sqlStore) + features := featuremgmt.WithFeatures() + cfg := setting.NewCfg() + cfg.IsFeatureToggleEnabled = features.IsEnabled dashboardService := dashboardservice.ProvideDashboardService( - setting.NewCfg(), dashboardStore, nil, - featuremgmt.WithFeatures(), acmock.NewPermissionsServicesMock(), + cfg, dashboardStore, nil, + features, acmock.NewPermissionsServicesMock(), ) ac := acmock.New() service := LibraryElementService{ - Cfg: setting.NewCfg(), + Cfg: cfg, SQLStore: sqlStore, folderService: dashboardservice.ProvideFolderService( - setting.NewCfg(), dashboardService, dashboardStore, nil, - featuremgmt.WithFeatures(), acmock.NewPermissionsServicesMock(), ac, nil, + cfg, dashboardService, dashboardStore, nil, + features, acmock.NewPermissionsServicesMock(), ac, nil, ), } diff --git a/pkg/services/librarypanels/librarypanels_test.go b/pkg/services/librarypanels/librarypanels_test.go index 26f28a798c2..9196b857525 100644 --- a/pkg/services/librarypanels/librarypanels_test.go +++ b/pkg/services/librarypanels/librarypanels_test.go @@ -1423,8 +1423,10 @@ func createDashboard(t *testing.T, sqlStore *sqlstore.SQLStore, user *models.Sig dashboardStore := database.ProvideDashboardStore(sqlStore) dashAlertService := alerting.ProvideDashAlertExtractorService(nil, nil, nil) + cfg := setting.NewCfg() + cfg.IsFeatureToggleEnabled = featuremgmt.WithFeatures().IsEnabled service := dashboardservice.ProvideDashboardService( - setting.NewCfg(), dashboardStore, dashAlertService, + cfg, dashboardStore, dashAlertService, featuremgmt.WithFeatures(), acmock.NewPermissionsServicesMock(), ) dashboard, err := service.SaveDashboard(context.Background(), dashItem, true) @@ -1438,6 +1440,7 @@ func createFolderWithACL(t *testing.T, sqlStore *sqlstore.SQLStore, title string t.Helper() cfg := setting.NewCfg() + cfg.IsFeatureToggleEnabled = featuremgmt.WithFeatures().IsEnabled features := featuremgmt.WithFeatures() permissionsServices := acmock.NewPermissionsServicesMock() dashboardStore := database.ProvideDashboardStore(sqlStore) diff --git a/pkg/services/serviceaccounts/database/database.go b/pkg/services/serviceaccounts/database/database.go index 21557bd556f..08b05167a40 100644 --- a/pkg/services/serviceaccounts/database/database.go +++ b/pkg/services/serviceaccounts/database/database.go @@ -11,7 +11,6 @@ import ( "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/services/accesscontrol" - "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/serviceaccounts" "github.com/grafana/grafana/pkg/services/sqlstore" "xorm.io/xorm" @@ -313,7 +312,7 @@ func (s *ServiceAccountsStoreImpl) SearchOrgServiceAccounts( s.sqlStore.Dialect.Quote("user"), s.sqlStore.Dialect.BooleanStr(true))) - if s.sqlStore.Cfg.IsFeatureToggleEnabled(featuremgmt.FlagAccesscontrol) { + if !accesscontrol.IsDisabled(s.sqlStore.Cfg) { acFilter, err := accesscontrol.Filter(signedInUser, "org_user.user_id", "serviceaccounts:id:", serviceaccounts.ActionRead) if err != nil { return err diff --git a/pkg/services/sqlstore/annotation.go b/pkg/services/sqlstore/annotation.go index 83f3e0f319f..8c5228359d7 100644 --- a/pkg/services/sqlstore/annotation.go +++ b/pkg/services/sqlstore/annotation.go @@ -11,7 +11,6 @@ import ( "github.com/grafana/grafana/pkg/models" ac "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/annotations" - "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/sqlstore/permissions" "github.com/grafana/grafana/pkg/services/sqlstore/searchstore" ) @@ -229,7 +228,7 @@ func (r *SQLAnnotationRepo) Find(ctx context.Context, query *annotations.ItemQue } } - if r.sql.Cfg.IsFeatureToggleEnabled(featuremgmt.FlagAccesscontrol) { + if !ac.IsDisabled(r.sql.Cfg) { acFilter, acArgs, err := getAccessControlFilter(query.SignedInUser) if err != nil { return err diff --git a/pkg/services/sqlstore/dashboard.go b/pkg/services/sqlstore/dashboard.go index 4017e8d8872..4f961ee2c92 100644 --- a/pkg/services/sqlstore/dashboard.go +++ b/pkg/services/sqlstore/dashboard.go @@ -7,7 +7,7 @@ import ( "github.com/prometheus/client_golang/prometheus" "github.com/grafana/grafana/pkg/models" - "github.com/grafana/grafana/pkg/services/featuremgmt" + "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/sqlstore/permissions" "github.com/grafana/grafana/pkg/services/sqlstore/searchstore" "github.com/grafana/grafana/pkg/util" @@ -74,7 +74,7 @@ func (ss *SQLStore) FindDashboards(ctx context.Context, query *models.FindPersis }, } - if ss.Cfg.IsFeatureToggleEnabled(featuremgmt.FlagAccesscontrol) { + if !accesscontrol.IsDisabled(ss.Cfg) { // if access control is enabled, overwrite the filters so far filters = []interface{}{ permissions.NewAccessControlDashboardPermissionFilter(query.SignedInUser, query.Permission, query.Type), diff --git a/pkg/services/sqlstore/org_users.go b/pkg/services/sqlstore/org_users.go index 60d7e5ca8f7..b1216704a27 100644 --- a/pkg/services/sqlstore/org_users.go +++ b/pkg/services/sqlstore/org_users.go @@ -8,7 +8,6 @@ import ( "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/services/accesscontrol" - "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/util" ) @@ -110,7 +109,7 @@ func (ss *SQLStore) GetOrgUsers(ctx context.Context, query *models.GetOrgUsersQu whereConditions = append(whereConditions, fmt.Sprintf("%s.is_service_account = ?", dialect.Quote("user"))) whereParams = append(whereParams, dialect.BooleanStr(false)) - if ss.Cfg.IsFeatureToggleEnabled(featuremgmt.FlagAccesscontrol) && query.User != nil { + if !accesscontrol.IsDisabled(ss.Cfg) && query.User != nil { acFilter, err := accesscontrol.Filter(query.User, "org_user.user_id", "users:id:", accesscontrol.ActionOrgUsersRead) if err != nil { return err @@ -175,7 +174,7 @@ func (ss *SQLStore) SearchOrgUsers(ctx context.Context, query *models.SearchOrgU whereConditions = append(whereConditions, fmt.Sprintf("%s.is_service_account = %s", ss.Dialect.Quote("user"), ss.Dialect.BooleanStr(false))) - if ss.Cfg.IsFeatureToggleEnabled(featuremgmt.FlagAccesscontrol) { + if !accesscontrol.IsDisabled(ss.Cfg) { acFilter, err := accesscontrol.Filter(query.User, "org_user.user_id", "users:id:", accesscontrol.ActionOrgUsersRead) if err != nil { return err diff --git a/pkg/services/sqlstore/team.go b/pkg/services/sqlstore/team.go index 6e266fed697..5c5beb73315 100644 --- a/pkg/services/sqlstore/team.go +++ b/pkg/services/sqlstore/team.go @@ -9,7 +9,6 @@ import ( "github.com/grafana/grafana/pkg/models" ac "github.com/grafana/grafana/pkg/services/accesscontrol" - "github.com/grafana/grafana/pkg/services/featuremgmt" ) type TeamStore interface { @@ -214,7 +213,7 @@ func (ss *SQLStore) SearchTeams(ctx context.Context, query *models.SearchTeamsQu acFilter ac.SQLFilter err error ) - if ss.Cfg.IsFeatureToggleEnabled(featuremgmt.FlagAccesscontrol) { + if !ac.IsDisabled(ss.Cfg) { acFilter, err = ac.Filter(query.SignedInUser, "team.id", "teams:id:", ac.ActionTeamsRead) if err != nil { return err @@ -259,7 +258,7 @@ func (ss *SQLStore) SearchTeams(ctx context.Context, query *models.SearchTeamsQu } // Only count teams user can see - if ss.Cfg.IsFeatureToggleEnabled(featuremgmt.FlagAccesscontrol) { + if !ac.IsDisabled(ss.Cfg) { countSess.Where(acFilter.Where, acFilter.Args...) } @@ -513,7 +512,7 @@ func (ss *SQLStore) GetTeamMembers(ctx context.Context, query *models.GetTeamMem // With accesscontrol we filter out users based on the SignedInUser's permissions // Note we assume that checking SignedInUser is allowed to see team members for this team has already been performed // If the signed in user is not set no member will be returned - if ss.Cfg.IsFeatureToggleEnabled(featuremgmt.FlagAccesscontrol) { + if !ac.IsDisabled(ss.Cfg) { sqlID := fmt.Sprintf("%s.%s", x.Dialect().Quote("user"), x.Dialect().Quote("id")) *acFilter, err = ac.Filter(query.SignedInUser, sqlID, "users:id:", ac.ActionOrgUsersRead) if err != nil {