From e4f3d9d453ae30f217700766490494be88cbce80 Mon Sep 17 00:00:00 2001 From: Ieva Date: Thu, 24 Nov 2022 14:38:55 +0000 Subject: [PATCH] resolve merge conflicts --- pkg/api/common_test.go | 20 ++++++----- pkg/api/dashboard.go | 8 ++++- pkg/api/dashboard_test.go | 3 ++ pkg/api/datasources.go | 6 ++++ pkg/api/datasources_test.go | 10 ++++-- pkg/api/folder.go | 6 ++++ pkg/api/folder_test.go | 10 +++--- pkg/api/team.go | 6 ++++ pkg/services/accesscontrol/accesscontrol.go | 2 ++ pkg/services/accesscontrol/acimpl/service.go | 8 +++++ pkg/services/accesscontrol/actest/fake.go | 2 ++ pkg/services/accesscontrol/mock/mock.go | 10 ++++++ pkg/services/serviceaccounts/api/api.go | 35 +++++++++++-------- pkg/services/serviceaccounts/api/api_test.go | 4 ++- .../serviceaccounts/manager/service.go | 2 +- 15 files changed, 100 insertions(+), 32 deletions(-) diff --git a/pkg/api/common_test.go b/pkg/api/common_test.go index f35c115865de..030458f6dab8 100644 --- a/pkg/api/common_test.go +++ b/pkg/api/common_test.go @@ -24,6 +24,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 setupAccessControlScenarioContext(t *testing.T, cfg *setting.Cfg, url strin store := sqlstore.InitTestDB(t) hs := &HTTPServer{ - Cfg: cfg, - Live: newTestLive(t, store), - License: &licensing.OSSLicensingService{}, - Features: featuremgmt.WithFeatures(), - QuotaService: "aimpl.Service{Cfg: cfg}, - 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: "aimpl.Service{Cfg: cfg}, + 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 40d865e3a990..9999d765c56d 100644 --- a/pkg/api/dashboard.go +++ b/pkg/api/dashboard.go @@ -461,7 +461,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) } } @@ -469,6 +469,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 57f57c79a824..514a857d7e98 100644 --- a/pkg/api/dashboard_test.go +++ b/pkg/api/dashboard_test.go @@ -20,6 +20,7 @@ import ( "github.com/grafana/grafana/pkg/framework/coremodel/registry" "github.com/grafana/grafana/pkg/infra/usagestats" "github.com/grafana/grafana/pkg/models" + "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" @@ -1034,6 +1035,7 @@ func postDashboardScenario(t *testing.T, desc string, url string, routePattern s folderService: folderService, Features: featuremgmt.WithFeatures(), Coremodels: registry.NewBase(), + accesscontrolService: actest.FakeService{}, } sc := setupScenarioContext(t, url) @@ -1106,6 +1108,7 @@ func restoreDashboardVersionScenario(t *testing.T, desc string, url string, rout Features: featuremgmt.WithFeatures(), dashboardVersionService: fakeDashboardVersionService, Coremodels: registry.NewBase(), + accesscontrolService: actest.FakeService{}, } sc := setupScenarioContext(t, url) diff --git a/pkg/api/datasources.go b/pkg/api/datasources.go index 2ad595a1ba92..45458f36a26f 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 cb27e6059e4a..e8cac89b952a 100644 --- a/pkg/api/datasources_test.go +++ b/pkg/api/datasources_test.go @@ -18,6 +18,8 @@ import ( "github.com/grafana/grafana/pkg/components/simplejson" "github.com/grafana/grafana/pkg/models" 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" @@ -111,7 +113,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") @@ -223,7 +227,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 0e5e1cf67465..f52cb193e071 100644 --- a/pkg/api/folder.go +++ b/pkg/api/folder.go @@ -123,6 +123,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) return response.JSON(http.StatusOK, hs.toFolderDto(c, g, folder)) } diff --git a/pkg/api/folder_test.go b/pkg/api/folder_test.go index 7bd1a30fec01..9613ca0360e4 100644 --- a/pkg/api/folder_test.go +++ b/pkg/api/folder_test.go @@ -16,6 +16,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" @@ -247,10 +248,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 01d33d69fa15..d85416e512bb 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/services/accesscontrol/accesscontrol.go b/pkg/services/accesscontrol/accesscontrol.go index 5bba73ea4fde..ecbdf55692d5 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 196df90a5461..04a95963639c 100644 --- a/pkg/services/accesscontrol/acimpl/service.go +++ b/pkg/services/accesscontrol/acimpl/service.go @@ -138,6 +138,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 66fffd38079c..d62f7a9424bb 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 a21a7b1bbbec..f8c11a5e5047 100644 --- a/pkg/services/accesscontrol/mock/mock.go +++ b/pkg/services/accesscontrol/mock/mock.go @@ -18,6 +18,7 @@ type fullAccessControl interface { type Calls struct { Evaluate []interface{} GetUserPermissions []interface{} + ClearUserPermissionCache []interface{} IsDisabled []interface{} DeclareFixedRoles []interface{} GetUserBuiltInRoles []interface{} @@ -40,6 +41,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 GetUserBuiltInRolesFunc func(user *user.SignedInUser) []string @@ -133,6 +135,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 745209c09ed3..549e5001d6c0 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 eae1918f8555..b104898a7e78 100644 --- a/pkg/services/serviceaccounts/api/api_test.go +++ b/pkg/services/serviceaccounts/api/api_test.go @@ -16,6 +16,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" @@ -284,8 +285,9 @@ func setupTestServer(t *testing.T, svc *tests.ServiceAccountMock, teamSvc := teamimpl.ProvideService(sqlStore, cfg) saPermissionService, err := ossaccesscontrol.ProvideServiceAccountPermissions(cfg, routing.NewRouteRegister(), sqlStore, acmock, &licensing.OSSLicensingService{}, saStore, acmock, teamSvc) 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 131c0998d126..0a1e0683014b 100644 --- a/pkg/services/serviceaccounts/manager/service.go +++ b/pkg/services/serviceaccounts/manager/service.go @@ -45,7 +45,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() return s, nil