Access Control: Clear user's permission cache after resource creation (#59101)

* refresh user's permission cache after resource creation

* clear the cache instead of reloading the permissions

* don't error if can't clear cache

* fix tests

* fix tests again

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)

pkg/api/dashboard.go

@@ -471,7 +471,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)
 		}
 	}
 
@@ -479,6 +479,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 {

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)

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",

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")

pkg/api/folder.go

@@ -129,6 +129,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))

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)

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

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))

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

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)
 }

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
 }

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 {

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)

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

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)