From 5caf97be40341e506c742fef8ea69297e65330b5 Mon Sep 17 00:00:00 2001 From: Karl Persson Date: Wed, 25 May 2022 20:40:41 +0200 Subject: [PATCH] AccessControl: Replace IsEnterprise checks with license checks (#49572) --- pkg/api/api.go | 4 +- pkg/api/common_test.go | 7 ++- pkg/api/org_users.go | 9 ++-- pkg/models/org_user.go | 2 + .../ossaccesscontrol/permissions_services.go | 12 ++--- .../accesscontrol/resourcepermissions/api.go | 2 +- .../resourcepermissions/service.go | 15 ++++-- .../resourcepermissions/service_test.go | 9 +++- .../guardian/accesscontrol_guardian_test.go | 8 ++- pkg/services/licensing/licensingtest/fake.go | 52 +++++++++++++++++++ pkg/services/sqlstore/org_users.go | 3 +- 11 files changed, 98 insertions(+), 25 deletions(-) create mode 100644 pkg/services/licensing/licensingtest/fake.go diff --git a/pkg/api/api.go b/pkg/api/api.go index d017b7d8465..5f691bbaf08 100644 --- a/pkg/api/api.go +++ b/pkg/api/api.go @@ -250,15 +250,15 @@ func (hs *HTTPServer) registerRoutes() { // current org without requirement of user to be org admin apiRoute.Group("/org", func(orgRoute routing.RouteRegister) { lookupEvaluator := func() ac.Evaluator { - if hs.Cfg.IsEnterprise { + if hs.License.FeatureEnabled("accesscontrol.enforcement") { return ac.EvalPermission(ac.ActionOrgUsersRead) } // For oss we allow users with access to update permissions on either folders, teams or dashboards to perform the lookup return ac.EvalAny( ac.EvalPermission(ac.ActionOrgUsersRead), ac.EvalPermission(ac.ActionTeamsPermissionsWrite), - ac.EvalPermission(dashboards.ActionDashboardsPermissionsWrite), ac.EvalPermission(dashboards.ActionFoldersPermissionsWrite), + ac.EvalPermission(dashboards.ActionDashboardsPermissionsWrite), ) } orgRoute.Get("/users/lookup", authorize(reqOrgAdminFolderAdminOrTeamAdmin, lookupEvaluator()), routing.Wrap(hs.GetOrgUsersForCurrentOrgLookup)) diff --git a/pkg/api/common_test.go b/pkg/api/common_test.go index 2df657da44a..0cdc01c4496 100644 --- a/pkg/api/common_test.go +++ b/pkg/api/common_test.go @@ -238,6 +238,7 @@ func setupAccessControlScenarioContext(t *testing.T, cfg *setting.Cfg, url strin hs := &HTTPServer{ Cfg: cfg, Live: newTestLive(t, store), + License: &licensing.OSSLicensingService{}, Features: featuremgmt.WithFeatures(), QuotaService: "a.QuotaService{Cfg: cfg}, RouteRegister: routing.NewRouteRegister(), @@ -327,6 +328,7 @@ func setupSimpleHTTPServer(features *featuremgmt.FeatureManager) *HTTPServer { return &HTTPServer{ Cfg: cfg, Features: features, + License: &licensing.OSSLicensingService{}, AccessControl: accesscontrolmock.New().WithDisabled(), } } @@ -390,7 +392,7 @@ func setupHTTPServerWithCfgDb(t *testing.T, useFakeAccessControl, enableAccessCo acmock = acmock.WithDisabled() } hs.AccessControl = acmock - teamPermissionService, err := ossaccesscontrol.ProvideTeamPermissions(cfg, routeRegister, db, acmock, database.ProvideService(db)) + teamPermissionService, err := ossaccesscontrol.ProvideTeamPermissions(cfg, routeRegister, db, acmock, database.ProvideService(db), hs.License) require.NoError(t, err) hs.teamPermissionsService = teamPermissionService } else { @@ -402,7 +404,7 @@ func setupHTTPServerWithCfgDb(t *testing.T, useFakeAccessControl, enableAccessCo require.NoError(t, err) err = ac.RegisterFixedRoles(context.Background()) require.NoError(t, err) - teamPermissionService, err := ossaccesscontrol.ProvideTeamPermissions(cfg, routeRegister, db, ac, database.ProvideService(db)) + teamPermissionService, err := ossaccesscontrol.ProvideTeamPermissions(cfg, routeRegister, db, ac, database.ProvideService(db), hs.License) require.NoError(t, err) hs.teamPermissionsService = teamPermissionService } @@ -463,6 +465,7 @@ func SetupAPITestServer(t *testing.T, opts ...APITestServerOption) *webtest.Serv hs := &HTTPServer{ RouteRegister: routing.NewRouteRegister(), Cfg: setting.NewCfg(), + License: &licensing.OSSLicensingService{}, AccessControl: accesscontrolmock.New().WithDisabled(), Features: featuremgmt.WithFeatures(), searchUsersService: &searchusers.OSSService{}, diff --git a/pkg/api/org_users.go b/pkg/api/org_users.go index b10aae80c45..7d07bc086a2 100644 --- a/pkg/api/org_users.go +++ b/pkg/api/org_users.go @@ -89,10 +89,11 @@ func (hs *HTTPServer) GetOrgUsersForCurrentOrg(c *models.ReqContext) response.Re // GET /api/org/users/lookup func (hs *HTTPServer) GetOrgUsersForCurrentOrgLookup(c *models.ReqContext) response.Response { orgUsers, err := hs.getOrgUsersHelper(c, &models.GetOrgUsersQuery{ - OrgId: c.OrgId, - Query: c.Query("query"), - Limit: c.QueryInt("limit"), - User: c.SignedInUser, + OrgId: c.OrgId, + Query: c.Query("query"), + Limit: c.QueryInt("limit"), + User: c.SignedInUser, + DontEnforceAccessControl: !hs.License.FeatureEnabled("accesscontrol.enforcement"), }, c.SignedInUser) if err != nil { diff --git a/pkg/models/org_user.go b/pkg/models/org_user.go index e08a9352ee3..0b91f064956 100644 --- a/pkg/models/org_user.go +++ b/pkg/models/org_user.go @@ -119,6 +119,8 @@ type GetOrgUsersQuery struct { OrgId int64 Query string Limit int + // Flag used to allow oss edition to query users without access control + DontEnforceAccessControl bool User *SignedInUser Result []*OrgUserDTO diff --git a/pkg/services/accesscontrol/ossaccesscontrol/permissions_services.go b/pkg/services/accesscontrol/ossaccesscontrol/permissions_services.go index 65f4621bb7f..e9b704cf548 100644 --- a/pkg/services/accesscontrol/ossaccesscontrol/permissions_services.go +++ b/pkg/services/accesscontrol/ossaccesscontrol/permissions_services.go @@ -35,7 +35,7 @@ var ( func ProvideTeamPermissions( cfg *setting.Cfg, router routing.RouteRegister, sql *sqlstore.SQLStore, - ac accesscontrol.AccessControl, store resourcepermissions.Store, + ac accesscontrol.AccessControl, store resourcepermissions.Store, license models.Licensing, ) (*TeamPermissionsService, error) { options := resourcepermissions.Options{ Resource: "teams", @@ -91,7 +91,7 @@ func ProvideTeamPermissions( }, } - srv, err := resourcepermissions.New(options, cfg, router, ac, store, sql) + srv, err := resourcepermissions.New(options, cfg, router, license, ac, store, sql) if err != nil { return nil, err } @@ -109,7 +109,7 @@ var DashboardAdminActions = append(DashboardEditActions, []string{dashboards.Act func ProvideDashboardPermissions( cfg *setting.Cfg, router routing.RouteRegister, sql *sqlstore.SQLStore, ac accesscontrol.AccessControl, store resourcepermissions.Store, - dashboardStore dashboards.Store, + license models.Licensing, dashboardStore dashboards.Store, ) (*DashboardPermissionsService, error) { getDashboard := func(ctx context.Context, orgID int64, resourceID string) (*models.Dashboard, error) { query := &models.GetDashboardQuery{Uid: resourceID, OrgId: orgID} @@ -164,7 +164,7 @@ func ProvideDashboardPermissions( RoleGroup: "Dashboards", } - srv, err := resourcepermissions.New(options, cfg, router, ac, store, sql) + srv, err := resourcepermissions.New(options, cfg, router, license, ac, store, sql) if err != nil { return nil, err } @@ -182,7 +182,7 @@ var FolderAdminActions = append(FolderEditActions, []string{dashboards.ActionFol func ProvideFolderPermissions( cfg *setting.Cfg, router routing.RouteRegister, sql *sqlstore.SQLStore, accesscontrol accesscontrol.AccessControl, store resourcepermissions.Store, - dashboardStore dashboards.Store, + license models.Licensing, dashboardStore dashboards.Store, ) (*FolderPermissionsService, error) { options := resourcepermissions.Options{ Resource: "folders", @@ -213,7 +213,7 @@ func ProvideFolderPermissions( WriterRoleName: "Folder permission writer", RoleGroup: "Folders", } - srv, err := resourcepermissions.New(options, cfg, router, accesscontrol, store, sql) + srv, err := resourcepermissions.New(options, cfg, router, license, accesscontrol, store, sql) if err != nil { return nil, err } diff --git a/pkg/services/accesscontrol/resourcepermissions/api.go b/pkg/services/accesscontrol/resourcepermissions/api.go index 53fbbf55399..c89ff795af7 100644 --- a/pkg/services/accesscontrol/resourcepermissions/api.go +++ b/pkg/services/accesscontrol/resourcepermissions/api.go @@ -118,7 +118,7 @@ func (a *api) getPermissions(c *models.ReqContext) response.Response { return response.Error(http.StatusInternalServerError, "failed to get permissions", err) } - if a.service.options.Assignments.BuiltInRoles && !a.service.cfg.IsEnterprise { + if a.service.options.Assignments.BuiltInRoles && !a.service.license.FeatureEnabled("accesscontrol.enforcement") { permissions = append(permissions, accesscontrol.ResourcePermission{ Actions: a.service.actions, Scope: "*", diff --git a/pkg/services/accesscontrol/resourcepermissions/service.go b/pkg/services/accesscontrol/resourcepermissions/service.go index 90291f16a26..5eb0b1ece9e 100644 --- a/pkg/services/accesscontrol/resourcepermissions/service.go +++ b/pkg/services/accesscontrol/resourcepermissions/service.go @@ -46,7 +46,10 @@ type Store interface { GetResourcePermissions(ctx context.Context, orgID int64, query types.GetResourcePermissionsQuery) ([]accesscontrol.ResourcePermission, error) } -func New(options Options, cfg *setting.Cfg, router routing.RouteRegister, ac accesscontrol.AccessControl, store Store, sqlStore *sqlstore.SQLStore) (*Service, error) { +func New( + options Options, cfg *setting.Cfg, router routing.RouteRegister, license models.Licensing, + ac accesscontrol.AccessControl, store Store, sqlStore *sqlstore.SQLStore, +) (*Service, error) { var permissions []string actionSet := make(map[string]struct{}) for permission, actions := range options.PermissionsToActions { @@ -71,6 +74,7 @@ func New(options Options, cfg *setting.Cfg, router routing.RouteRegister, ac acc cfg: cfg, store: store, options: options, + license: license, permissions: permissions, actions: actions, sqlStore: sqlStore, @@ -89,10 +93,11 @@ func New(options Options, cfg *setting.Cfg, router routing.RouteRegister, ac acc // Service is used to create access control sub system including api / and service for managed resource permission type Service struct { - cfg *setting.Cfg - ac accesscontrol.AccessControl - store Store - api *api + cfg *setting.Cfg + ac accesscontrol.AccessControl + store Store + api *api + license models.Licensing options Options permissions []string diff --git a/pkg/services/accesscontrol/resourcepermissions/service_test.go b/pkg/services/accesscontrol/resourcepermissions/service_test.go index 622ef8ad617..f5fc1cb0fcb 100644 --- a/pkg/services/accesscontrol/resourcepermissions/service_test.go +++ b/pkg/services/accesscontrol/resourcepermissions/service_test.go @@ -12,6 +12,7 @@ import ( "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/accesscontrol/database" accesscontrolmock "github.com/grafana/grafana/pkg/services/accesscontrol/mock" + "github.com/grafana/grafana/pkg/services/licensing/licensingtest" "github.com/grafana/grafana/pkg/services/sqlstore" "github.com/grafana/grafana/pkg/setting" ) @@ -221,8 +222,12 @@ func setupTestEnvironment(t *testing.T, permissions []*accesscontrol.Permission, sql := sqlstore.InitTestDB(t) store := database.ProvideService(sql) cfg := setting.NewCfg() - cfg.IsEnterprise = true - service, err := New(ops, cfg, routing.NewRouteRegister(), accesscontrolmock.New().WithPermissions(permissions), store, sql) + license := licensingtest.NewFakeLicensing() + license.On("FeatureEnabled", "accesscontrol.enforcement").Return(true).Maybe() + service, err := New( + ops, cfg, routing.NewRouteRegister(), license, + accesscontrolmock.New().WithPermissions(permissions), store, sql, + ) require.NoError(t, err) return service, sql diff --git a/pkg/services/guardian/accesscontrol_guardian_test.go b/pkg/services/guardian/accesscontrol_guardian_test.go index 00ec21bd7e9..f1c540b76d6 100644 --- a/pkg/services/guardian/accesscontrol_guardian_test.go +++ b/pkg/services/guardian/accesscontrol_guardian_test.go @@ -17,6 +17,7 @@ import ( "github.com/grafana/grafana/pkg/services/accesscontrol/ossaccesscontrol" "github.com/grafana/grafana/pkg/services/dashboards" dashdb "github.com/grafana/grafana/pkg/services/dashboards/database" + "github.com/grafana/grafana/pkg/services/licensing/licensingtest" "github.com/grafana/grafana/pkg/services/sqlstore" "github.com/grafana/grafana/pkg/setting" ) @@ -594,11 +595,14 @@ func setupAccessControlGuardianTest(t *testing.T, uid string, permissions []*acc }) require.NoError(t, err) ac := accesscontrolmock.New().WithPermissions(permissions) + license := licensingtest.NewFakeLicensing() + license.On("FeatureEnabled", "accesscontrol.enforcement").Return(true).Maybe() + folderPermissions, err := ossaccesscontrol.ProvideFolderPermissions( - setting.NewCfg(), routing.NewRouteRegister(), store, ac, database.ProvideService(store), &dashboards.FakeDashboardStore{}) + setting.NewCfg(), routing.NewRouteRegister(), store, ac, database.ProvideService(store), license, &dashboards.FakeDashboardStore{}) require.NoError(t, err) dashboardPermissions, err := ossaccesscontrol.ProvideDashboardPermissions( - setting.NewCfg(), routing.NewRouteRegister(), store, ac, database.ProvideService(store), &dashboards.FakeDashboardStore{}) + setting.NewCfg(), routing.NewRouteRegister(), store, ac, database.ProvideService(store), license, &dashboards.FakeDashboardStore{}) require.NoError(t, err) if dashboardSvc == nil { dashboardSvc = &dashboards.FakeDashboardService{} diff --git a/pkg/services/licensing/licensingtest/fake.go b/pkg/services/licensing/licensingtest/fake.go new file mode 100644 index 00000000000..f5d8454ece1 --- /dev/null +++ b/pkg/services/licensing/licensingtest/fake.go @@ -0,0 +1,52 @@ +package licensingtest + +import ( + "github.com/stretchr/testify/mock" + + "github.com/grafana/grafana/pkg/models" +) + +var _ models.Licensing = new(FakeLicensing) + +func NewFakeLicensing() *FakeLicensing { + return &FakeLicensing{&mock.Mock{}} +} + +type FakeLicensing struct { + *mock.Mock +} + +func (f *FakeLicensing) Expiry() int64 { + mockedArgs := f.Called() + return mockedArgs.Get(0).(int64) +} + +func (f *FakeLicensing) Edition() string { + mockedArgs := f.Called() + return mockedArgs.Get(0).(string) +} + +func (f *FakeLicensing) ContentDeliveryPrefix() string { + mockedArgs := f.Called() + return mockedArgs.Get(0).(string) +} + +func (f *FakeLicensing) LicenseURL(showAdminLicensingPage bool) string { + mockedArgs := f.Called(showAdminLicensingPage) + return mockedArgs.Get(0).(string) +} + +func (f *FakeLicensing) StateInfo() string { + mockedArgs := f.Called() + return mockedArgs.Get(0).(string) +} + +func (f *FakeLicensing) EnabledFeatures() map[string]bool { + mockedArgs := f.Called() + return mockedArgs.Get(0).(map[string]bool) +} + +func (f *FakeLicensing) FeatureEnabled(feature string) bool { + mockedArgs := f.Called(feature) + return mockedArgs.Get(0).(bool) +} diff --git a/pkg/services/sqlstore/org_users.go b/pkg/services/sqlstore/org_users.go index 65eb4a67c57..1afea95b156 100644 --- a/pkg/services/sqlstore/org_users.go +++ b/pkg/services/sqlstore/org_users.go @@ -112,7 +112,8 @@ func (ss *SQLStore) GetOrgUsers(ctx context.Context, query *models.GetOrgUsersQu if query.User == nil { ss.log.Warn("Query user not set for filtering.") } - if ss.Cfg.IsEnterprise && !accesscontrol.IsDisabled(ss.Cfg) { + + if !query.DontEnforceAccessControl && !accesscontrol.IsDisabled(ss.Cfg) { acFilter, err := accesscontrol.Filter(query.User, "org_user.user_id", "users:id:", accesscontrol.ActionOrgUsersRead) if err != nil { return err