diff --git a/pkg/api/common_test.go b/pkg/api/common_test.go index e6f0c159401..3d363982b36 100644 --- a/pkg/api/common_test.go +++ b/pkg/api/common_test.go @@ -25,6 +25,7 @@ import ( "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/accesscontrol/acimpl" + "github.com/grafana/grafana/pkg/services/accesscontrol/actest" accesscontrolmock "github.com/grafana/grafana/pkg/services/accesscontrol/mock" "github.com/grafana/grafana/pkg/services/accesscontrol/ossaccesscontrol" "github.com/grafana/grafana/pkg/services/annotations/annotationstest" @@ -250,15 +251,16 @@ func (s *fakeRenderService) Init() error { func setupAccessControlScenarioContext(t *testing.T, cfg *setting.Cfg, url string, permissions []accesscontrol.Permission) (*scenarioContext, *HTTPServer) { store := sqlstore.InitTestDB(t) hs := &HTTPServer{ - Cfg: cfg, - Live: newTestLive(t, store), - License: &licensing.OSSLicensingService{}, - Features: featuremgmt.WithFeatures(), - QuotaService: quotatest.New(false, nil), - RouteRegister: routing.NewRouteRegister(), - AccessControl: accesscontrolmock.New().WithPermissions(permissions), - searchUsersService: searchusers.ProvideUsersService(filters.ProvideOSSSearchUserFilter(), usertest.NewUserServiceFake()), - ldapGroups: ldap.ProvideGroupsService(), + Cfg: cfg, + Live: newTestLive(t, store), + License: &licensing.OSSLicensingService{}, + Features: featuremgmt.WithFeatures(), + QuotaService: quotatest.New(false, nil), + RouteRegister: routing.NewRouteRegister(), + AccessControl: accesscontrolmock.New().WithPermissions(permissions), + searchUsersService: searchusers.ProvideUsersService(filters.ProvideOSSSearchUserFilter(), usertest.NewUserServiceFake()), + ldapGroups: ldap.ProvideGroupsService(), + accesscontrolService: actest.FakeService{}, } sc := setupScenarioContext(t, url) diff --git a/pkg/api/dashboard.go b/pkg/api/dashboard.go index ffb0f0098bc..7e8d5ca263f 100644 --- a/pkg/api/dashboard.go +++ b/pkg/api/dashboard.go @@ -470,7 +470,7 @@ func (hs *HTTPServer) postDashboard(c *models.ReqContext, cmd models.SaveDashboa } if liveerr != nil { - hs.log.Warn("unable to broadcast save event", "uid", dashboard.Uid, "error", err) + hs.log.Warn("unable to broadcast save event", "uid", dashboard.Uid, "error", liveerr) } } @@ -478,6 +478,12 @@ func (hs *HTTPServer) postDashboard(c *models.ReqContext, cmd models.SaveDashboa return apierrors.ToDashboardErrorResponse(ctx, hs.pluginStore, err) } + // Clear permission cache for the user who's created the dashboard, so that new permissions are fetched for their next call + // Required for cases when caller wants to immediately interact with the newly created object + if newDashboard && !hs.accesscontrolService.IsDisabled() { + hs.accesscontrolService.ClearUserPermissionCache(c.SignedInUser) + } + // connect library panels for this dashboard after the dashboard is stored and has an ID err = hs.LibraryPanelService.ConnectLibraryPanelsForDashboard(ctx, c.SignedInUser, dashboard) if err != nil { diff --git a/pkg/api/dashboard_test.go b/pkg/api/dashboard_test.go index 42bdd60a957..980642c1dfd 100644 --- a/pkg/api/dashboard_test.go +++ b/pkg/api/dashboard_test.go @@ -22,6 +22,7 @@ import ( "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/plugins" "github.com/grafana/grafana/pkg/registry/corekind" + "github.com/grafana/grafana/pkg/services/accesscontrol/actest" accesscontrolmock "github.com/grafana/grafana/pkg/services/accesscontrol/mock" "github.com/grafana/grafana/pkg/services/alerting" "github.com/grafana/grafana/pkg/services/annotations/annotationstest" @@ -1093,6 +1094,7 @@ func postDashboardScenario(t *testing.T, desc string, url string, routePattern s folderService: folderService, Features: featuremgmt.WithFeatures(), Kinds: corekind.NewBase(nil), + accesscontrolService: actest.FakeService{}, } sc := setupScenarioContext(t, url) @@ -1201,6 +1203,7 @@ func restoreDashboardVersionScenario(t *testing.T, desc string, url string, rout Features: featuremgmt.WithFeatures(), dashboardVersionService: fakeDashboardVersionService, Kinds: corekind.NewBase(nil), + accesscontrolService: actest.FakeService{}, } sc := setupScenarioContext(t, url) diff --git a/pkg/api/datasources.go b/pkg/api/datasources.go index 2ad595a1ba9..45458f36a26 100644 --- a/pkg/api/datasources.go +++ b/pkg/api/datasources.go @@ -396,6 +396,12 @@ func (hs *HTTPServer) AddDataSource(c *models.ReqContext) response.Response { return response.Error(500, "Failed to add datasource", err) } + // Clear permission cache for the user who's created the data source, so that new permissions are fetched for their next call + // Required for cases when caller wants to immediately interact with the newly created object + if !hs.AccessControl.IsDisabled() { + hs.accesscontrolService.ClearUserPermissionCache(c.SignedInUser) + } + ds := hs.convertModelToDtos(c.Req.Context(), cmd.Result) return response.JSON(http.StatusOK, util.DynMap{ "message": "Datasource added", diff --git a/pkg/api/datasources_test.go b/pkg/api/datasources_test.go index 7ef272919f9..ccfd15be3a1 100644 --- a/pkg/api/datasources_test.go +++ b/pkg/api/datasources_test.go @@ -19,6 +19,8 @@ import ( "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/plugins" 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/datasources" "github.com/grafana/grafana/pkg/services/datasources/permissions" "github.com/grafana/grafana/pkg/services/org" @@ -112,7 +114,9 @@ func TestAddDataSource_URLWithoutProtocol(t *testing.T) { DataSourcesService: &dataSourcesServiceMock{ expectedDatasource: &datasources.DataSource{}, }, - Cfg: setting.NewCfg(), + Cfg: setting.NewCfg(), + AccessControl: acimpl.ProvideAccessControl(setting.NewCfg()), + accesscontrolService: actest.FakeService{}, } sc := setupScenarioContext(t, "/api/datasources") @@ -224,7 +228,9 @@ func TestUpdateDataSource_URLWithoutProtocol(t *testing.T) { DataSourcesService: &dataSourcesServiceMock{ expectedDatasource: &datasources.DataSource{}, }, - Cfg: setting.NewCfg(), + Cfg: setting.NewCfg(), + AccessControl: acimpl.ProvideAccessControl(setting.NewCfg()), + accesscontrolService: actest.FakeService{}, } sc := setupScenarioContext(t, "/api/datasources/1234") diff --git a/pkg/api/folder.go b/pkg/api/folder.go index 4373bb2d54f..f2a8096741d 100644 --- a/pkg/api/folder.go +++ b/pkg/api/folder.go @@ -128,6 +128,12 @@ func (hs *HTTPServer) CreateFolder(c *models.ReqContext) response.Response { return apierrors.ToFolderErrorResponse(err) } + // Clear permission cache for the user who's created the folder, so that new permissions are fetched for their next call + // Required for cases when caller wants to immediately interact with the newly created object + if !hs.AccessControl.IsDisabled() { + hs.accesscontrolService.ClearUserPermissionCache(c.SignedInUser) + } + g := guardian.New(c.Req.Context(), folder.ID, c.OrgID, c.SignedInUser) // TODO set ParentUID if nested folders are enabled return response.JSON(http.StatusOK, hs.newToFolderDto(c, g, folder)) diff --git a/pkg/api/folder_test.go b/pkg/api/folder_test.go index 5c6ebabf06c..777fe6a281e 100644 --- a/pkg/api/folder_test.go +++ b/pkg/api/folder_test.go @@ -15,6 +15,7 @@ import ( "github.com/grafana/grafana/pkg/api/routing" "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/services/accesscontrol" + "github.com/grafana/grafana/pkg/services/accesscontrol/actest" acmock "github.com/grafana/grafana/pkg/services/accesscontrol/mock" "github.com/grafana/grafana/pkg/services/dashboards" "github.com/grafana/grafana/pkg/services/featuremgmt" @@ -242,10 +243,11 @@ func createFolderScenario(t *testing.T, desc string, url string, routePattern st store := mockstore.NewSQLStoreMock() guardian.InitLegacyGuardian(store, dashSvc, teamSvc) hs := HTTPServer{ - AccessControl: acmock.New(), - folderService: folderService, - Cfg: setting.NewCfg(), - Features: featuremgmt.WithFeatures(), + AccessControl: acmock.New(), + folderService: folderService, + Cfg: setting.NewCfg(), + Features: featuremgmt.WithFeatures(), + accesscontrolService: actest.FakeService{}, } sc := setupScenarioContext(t, url) diff --git a/pkg/api/team.go b/pkg/api/team.go index 01d33d69fa1..d85416e512b 100644 --- a/pkg/api/team.go +++ b/pkg/api/team.go @@ -41,6 +41,12 @@ func (hs *HTTPServer) CreateTeam(c *models.ReqContext) response.Response { return response.Error(500, "Failed to create Team", err) } + // Clear permission cache for the user who's created the team, so that new permissions are fetched for their next call + // Required for cases when caller wants to immediately interact with the newly created object + if !hs.AccessControl.IsDisabled() { + hs.accesscontrolService.ClearUserPermissionCache(c.SignedInUser) + } + if accessControlEnabled || (c.OrgRole == org.RoleEditor && hs.Cfg.EditorsCanAdmin) { // if the request is authenticated using API tokens // the SignedInUser is an empty struct therefore diff --git a/pkg/api/team_test.go b/pkg/api/team_test.go index af010b06ab0..3d5cb0e4d1f 100644 --- a/pkg/api/team_test.go +++ b/pkg/api/team_test.go @@ -15,6 +15,8 @@ import ( "github.com/grafana/grafana/pkg/infra/log/logtest" "github.com/grafana/grafana/pkg/models" "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/org" pref "github.com/grafana/grafana/pkg/services/preference" "github.com/grafana/grafana/pkg/services/preference/preftest" @@ -213,6 +215,8 @@ func TestTeamAPIEndpoint_CreateTeam_RBAC(t *testing.T) { server := SetupAPITestServer(t, func(hs *HTTPServer) { hs.Cfg = setting.NewCfg() hs.teamService = teamtest.NewFakeService() + hs.AccessControl = acimpl.ProvideAccessControl(setting.NewCfg()) + hs.accesscontrolService = actest.FakeService{} }) input := strings.NewReader(fmt.Sprintf(teamCmd, 1)) diff --git a/pkg/services/accesscontrol/accesscontrol.go b/pkg/services/accesscontrol/accesscontrol.go index 5bba73ea4fd..ecbdf55692d 100644 --- a/pkg/services/accesscontrol/accesscontrol.go +++ b/pkg/services/accesscontrol/accesscontrol.go @@ -26,6 +26,8 @@ type Service interface { registry.ProvidesUsageStats // GetUserPermissions returns user permissions with only action and scope fields set. GetUserPermissions(ctx context.Context, user *user.SignedInUser, options Options) ([]Permission, error) + // ClearUserPermissionCache removes the permission cache entry for the given user + ClearUserPermissionCache(user *user.SignedInUser) // DeleteUserPermissions removes all permissions user has in org and all permission to that user // If orgID is set to 0 remove permissions from all orgs DeleteUserPermissions(ctx context.Context, orgID, userID int64) error diff --git a/pkg/services/accesscontrol/acimpl/service.go b/pkg/services/accesscontrol/acimpl/service.go index 9bab820f8eb..b5ac20d5ad1 100644 --- a/pkg/services/accesscontrol/acimpl/service.go +++ b/pkg/services/accesscontrol/acimpl/service.go @@ -147,6 +147,14 @@ func (s *Service) getCachedUserPermissions(ctx context.Context, user *user.Signe return permissions, nil } +func (s *Service) ClearUserPermissionCache(user *user.SignedInUser) { + key, err := permissionCacheKey(user) + if err != nil { + return + } + s.cache.Delete(key) +} + func (s *Service) DeleteUserPermissions(ctx context.Context, orgID int64, userID int64) error { return s.store.DeleteUserPermissions(ctx, orgID, userID) } diff --git a/pkg/services/accesscontrol/actest/fake.go b/pkg/services/accesscontrol/actest/fake.go index 66fffd38079..d62f7a9424b 100644 --- a/pkg/services/accesscontrol/actest/fake.go +++ b/pkg/services/accesscontrol/actest/fake.go @@ -24,6 +24,8 @@ func (f FakeService) GetUserPermissions(ctx context.Context, user *user.SignedIn return f.ExpectedPermissions, f.ExpectedErr } +func (f FakeService) ClearUserPermissionCache(user *user.SignedInUser) {} + func (f FakeService) DeleteUserPermissions(ctx context.Context, orgID, userID int64) error { return f.ExpectedErr } diff --git a/pkg/services/accesscontrol/mock/mock.go b/pkg/services/accesscontrol/mock/mock.go index a06beb9df3b..663902e95ca 100644 --- a/pkg/services/accesscontrol/mock/mock.go +++ b/pkg/services/accesscontrol/mock/mock.go @@ -20,6 +20,7 @@ type fullAccessControl interface { type Calls struct { Evaluate []interface{} GetUserPermissions []interface{} + ClearUserPermissionCache []interface{} IsDisabled []interface{} DeclareFixedRoles []interface{} DeclarePluginRoles []interface{} @@ -43,6 +44,7 @@ type Mock struct { // Override functions EvaluateFunc func(context.Context, *user.SignedInUser, accesscontrol.Evaluator) (bool, error) GetUserPermissionsFunc func(context.Context, *user.SignedInUser, accesscontrol.Options) ([]accesscontrol.Permission, error) + ClearUserPermissionCacheFunc func(*user.SignedInUser) IsDisabledFunc func() bool DeclareFixedRolesFunc func(...accesscontrol.RoleRegistration) error DeclarePluginRolesFunc func(context.Context, string, string, []plugins.RoleRegistration) error @@ -138,6 +140,14 @@ func (m *Mock) GetUserPermissions(ctx context.Context, user *user.SignedInUser, return m.permissions, nil } +func (m *Mock) ClearUserPermissionCache(user *user.SignedInUser) { + m.Calls.ClearUserPermissionCache = append(m.Calls.ClearUserPermissionCache, []interface{}{user}) + // Use override if provided + if m.ClearUserPermissionCacheFunc != nil { + m.ClearUserPermissionCacheFunc(user) + } +} + // Middleware checks if service disabled or not to switch to fallback authorization. // This mock return m.disabled unless an override is provided. func (m *Mock) IsDisabled() bool { diff --git a/pkg/services/serviceaccounts/api/api.go b/pkg/services/serviceaccounts/api/api.go index 745209c09ed..549e5001d6c 100644 --- a/pkg/services/serviceaccounts/api/api.go +++ b/pkg/services/serviceaccounts/api/api.go @@ -21,31 +21,34 @@ import ( ) type ServiceAccountsAPI struct { - cfg *setting.Cfg - service serviceaccounts.Service - accesscontrol accesscontrol.AccessControl - RouterRegister routing.RouteRegister - store serviceaccounts.Store - log log.Logger - permissionService accesscontrol.ServiceAccountPermissionsService + cfg *setting.Cfg + service serviceaccounts.Service + accesscontrol accesscontrol.AccessControl + accesscontrolService accesscontrol.Service + RouterRegister routing.RouteRegister + store serviceaccounts.Store + log log.Logger + permissionService accesscontrol.ServiceAccountPermissionsService } func NewServiceAccountsAPI( cfg *setting.Cfg, service serviceaccounts.Service, accesscontrol accesscontrol.AccessControl, + accesscontrolService accesscontrol.Service, routerRegister routing.RouteRegister, store serviceaccounts.Store, permissionService accesscontrol.ServiceAccountPermissionsService, ) *ServiceAccountsAPI { return &ServiceAccountsAPI{ - cfg: cfg, - service: service, - accesscontrol: accesscontrol, - RouterRegister: routerRegister, - store: store, - log: log.New("serviceaccounts.api"), - permissionService: permissionService, + cfg: cfg, + service: service, + accesscontrol: accesscontrol, + accesscontrolService: accesscontrolService, + RouterRegister: routerRegister, + store: store, + log: log.New("serviceaccounts.api"), + permissionService: permissionService, } } @@ -127,6 +130,10 @@ func (api *ServiceAccountsAPI) CreateServiceAccount(c *models.ReqContext) respon return response.Error(http.StatusInternalServerError, "Failed to set permissions for service account creator", err) } } + + // Clear permission cache for the user who's created the service account, so that new permissions are fetched for their next call + // Required for cases when caller wants to immediately interact with the newly created object + api.accesscontrolService.ClearUserPermissionCache(c.SignedInUser) } return response.JSON(http.StatusCreated, serviceAccount) diff --git a/pkg/services/serviceaccounts/api/api_test.go b/pkg/services/serviceaccounts/api/api_test.go index 8abeeb43789..49cc8a63329 100644 --- a/pkg/services/serviceaccounts/api/api_test.go +++ b/pkg/services/serviceaccounts/api/api_test.go @@ -20,6 +20,7 @@ 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/accesscontrol/actest" accesscontrolmock "github.com/grafana/grafana/pkg/services/accesscontrol/mock" "github.com/grafana/grafana/pkg/services/accesscontrol/ossaccesscontrol" "github.com/grafana/grafana/pkg/services/apikey/apikeyimpl" @@ -296,8 +297,9 @@ func setupTestServer(t *testing.T, svc *tests.ServiceAccountMock, saPermissionService, err := ossaccesscontrol.ProvideServiceAccountPermissions( cfg, routing.NewRouteRegister(), sqlStore, acmock, &licensing.OSSLicensingService{}, saStore, acmock, teamSvc, userSvc) require.NoError(t, err) + acService := actest.FakeService{} - a := NewServiceAccountsAPI(cfg, svc, acmock, routerRegister, saStore, saPermissionService) + a := NewServiceAccountsAPI(cfg, svc, acmock, acService, routerRegister, saStore, saPermissionService) a.RegisterAPIEndpoints() a.cfg.ApiKeyMaxSecondsToLive = -1 // disable api key expiration diff --git a/pkg/services/serviceaccounts/manager/service.go b/pkg/services/serviceaccounts/manager/service.go index 1301bc90659..4365fcd52af 100644 --- a/pkg/services/serviceaccounts/manager/service.go +++ b/pkg/services/serviceaccounts/manager/service.go @@ -51,7 +51,7 @@ func ProvideServiceAccountsService( usageStats.RegisterMetricsFunc(s.getUsageMetrics) - serviceaccountsAPI := api.NewServiceAccountsAPI(cfg, s, ac, routeRegister, s.store, permissionService) + serviceaccountsAPI := api.NewServiceAccountsAPI(cfg, s, ac, accesscontrolService, routeRegister, s.store, permissionService) serviceaccountsAPI.RegisterAPIEndpoints() s.secretScanEnabled = cfg.SectionWithEnvOverrides("secretscan").Key("enabled").MustBool(false)