Dashboard/Folder permission fix session (#47174)

* Fix inherited scopes for dashboard to use folder uid

* Add inherited evaluators

* Slight modification of the commments

* Add test for inheritance

* Nit.

* extract shared function from tests

* Nit. Extra line

* Remove unused comment

Co-authored-by: Karl Persson <kalle.persson@grafana.com>
Co-authored-by: gamab <gabi.mabs@gmail.com>

pkg/services/accesscontrol/ossaccesscontrol/permissions_services.go

@@ -169,13 +169,18 @@ func ProvideDashboardPermissions(
 
 			return nil
 		},
+		InheritedScopePrefixes: []string{"folders:uid:"},
 		InheritedScopesSolver: func(ctx context.Context, orgID int64, resourceID string) ([]string, error) {
 			dashboard, err := getDashboard(ctx, orgID, resourceID)
 			if err != nil {
 				return nil, err
 			}
 			if dashboard.FolderId > 0 {
-				return []string{dashboards.ScopeFoldersProvider.GetResourceScopeUID(dashboard.Uid)}, nil
+				query := &models.GetDashboardQuery{Id: dashboard.FolderId, OrgId: orgID}
+				if err := sql.GetDashboard(ctx, query); err != nil {
+					return nil, err
+				}
+				return []string{dashboards.ScopeFoldersProvider.GetResourceScopeUID(query.Result.Uid)}, nil
 			}
 			return []string{}, nil
 		},

pkg/services/accesscontrol/resourcepermissions/api.go

@@ -30,18 +30,45 @@ func newApi(ac accesscontrol.AccessControl, router routing.RouteRegister, manage
 	return &api{ac, router, manager, permissions}
 }
 
+func (a *api) getEvaluators(actionRead, actionWrite, scope string) (read, write accesscontrol.Evaluator) {
+	if a.service.options.InheritedScopesSolver == nil || a.service.options.InheritedScopePrefixes == nil {
+		read = accesscontrol.EvalPermission(actionRead, scope)
+		write = accesscontrol.EvalPermission(actionWrite, scope)
+	} else {
+		// Add inherited scopes to the evaluators protecting the endpoint.
+		// Scopes in the request context parameters are to be added by solveInheritedScopes.
+		// If a user got actionRead on any of the inherited scopes, they will be granted access to the endpoint.
+		// Ex: a user inherits dashboards:read from the containing folder (folders:uid:BCeknZL7k)
+		inheritedRead := []accesscontrol.Evaluator{accesscontrol.EvalPermission(actionRead, scope)}
+		inheritedWrite := []accesscontrol.Evaluator{accesscontrol.EvalPermission(actionWrite, scope)}
+		for _, scopePrefix := range a.service.options.InheritedScopePrefixes {
+			inheritedRead = append(inheritedRead,
+				accesscontrol.EvalPermission(actionRead, accesscontrol.Parameter(scopePrefix)))
+			inheritedWrite = append(inheritedWrite,
+				accesscontrol.EvalPermission(actionWrite, accesscontrol.Parameter(scopePrefix)))
+		}
+
+		read = accesscontrol.EvalAny(inheritedRead...)
+		write = accesscontrol.EvalAny(inheritedWrite...)
+	}
+	return
+}
+
 func (a *api) registerEndpoints() {
 	auth := middleware.Middleware(a.ac)
 	uidSolver := solveUID(a.service.options.UidSolver)
+	inheritanceSolver := solveInheritedScopes(a.service.options.InheritedScopesSolver)
 	disable := middleware.Disable(a.ac.IsDisabled())
 	a.router.Group(fmt.Sprintf("/api/access-control/%s", a.service.options.Resource), func(r routing.RouteRegister) {
+		actionRead := fmt.Sprintf("%s.permissions:read", a.service.options.Resource)
+		actionWrite := fmt.Sprintf("%s.permissions:write", a.service.options.Resource)
 		scope := accesscontrol.Scope(a.service.options.Resource, a.service.options.ResourceAttribute, accesscontrol.Parameter(":resourceID"))
-		actionWrite, actionRead := fmt.Sprintf("%s.permissions:write", a.service.options.Resource), fmt.Sprintf("%s.permissions:read", a.service.options.Resource)
+		readEvaluator, writeEvaluator := a.getEvaluators(actionRead, actionWrite, scope)
 		r.Get("/description", auth(disable, accesscontrol.EvalPermission(actionRead)), routing.Wrap(a.getDescription))
-		r.Get("/:resourceID", uidSolver, auth(disable, accesscontrol.EvalPermission(actionRead, scope)), routing.Wrap(a.getPermissions))
-		r.Post("/:resourceID/users/:userID", uidSolver, auth(disable, accesscontrol.EvalPermission(actionWrite, scope)), routing.Wrap(a.setUserPermission))
-		r.Post("/:resourceID/teams/:teamID", uidSolver, auth(disable, accesscontrol.EvalPermission(actionWrite, scope)), routing.Wrap(a.setTeamPermission))
-		r.Post("/:resourceID/builtInRoles/:builtInRole", uidSolver, auth(disable, accesscontrol.EvalPermission(actionWrite, scope)), routing.Wrap(a.setBuiltinRolePermission))
+		r.Get("/:resourceID", inheritanceSolver, uidSolver, auth(disable, readEvaluator), routing.Wrap(a.getPermissions))
+		r.Post("/:resourceID/users/:userID", inheritanceSolver, uidSolver, auth(disable, writeEvaluator), routing.Wrap(a.setUserPermission))
+		r.Post("/:resourceID/teams/:teamID", inheritanceSolver, uidSolver, auth(disable, writeEvaluator), routing.Wrap(a.setTeamPermission))
+		r.Post("/:resourceID/builtInRoles/:builtInRole", inheritanceSolver, uidSolver, auth(disable, writeEvaluator), routing.Wrap(a.setBuiltinRolePermission))
 	})
 }
 

pkg/services/accesscontrol/resourcepermissions/api_test.go

@@ -167,16 +167,7 @@ func TestApi_getPermissions(t *testing.T) {
 			assert.Equal(t, tt.expectedStatus, recorder.Code)
 
 			if tt.expectedStatus == http.StatusOK {
-				assert.Len(t, permissions, 3, "expected three assignments: user, team, builtin")
-				for _, p := range permissions {
-					if p.UserID != 0 {
-						assert.Equal(t, "View", p.Permission)
-					} else if p.TeamID != 0 {
-						assert.Equal(t, "Edit", p.Permission)
-					} else {
-						assert.Equal(t, "Edit", p.Permission)
-					}
-				}
+				checkSeededPermissions(t, permissions)
 			}
 		})
 	}
@@ -468,18 +459,7 @@ func TestApi_UidSolver(t *testing.T) {
 			assert.Equal(t, tt.expectedStatus, recorder.Code)
 
 			if tt.expectedStatus == http.StatusOK {
-				assert.Len(t, permissions, 3, "expected three assignments: user, team, builtin")
-				for _, p := range permissions {
-					if p.UserID != 0 {
-						assert.Equal(t, "View", p.Permission)
-					} else if p.TeamID != 0 {
-						assert.Equal(t, "Edit", p.Permission)
-					} else {
-						assert.Equal(t, "Edit", p.Permission)
-					}
-				}
-			} else {
-				assert.Equal(t, tt.expectedStatus, recorder.Code)
+				checkSeededPermissions(t, permissions)
 			}
 		})
 	}
@@ -490,6 +470,66 @@ func withSolver(options Options, solver UidSolver) Options {
 	return options
 }
 
+type inheritSolverTestCase struct {
+	desc           string
+	resourceID     string
+	expectedStatus int
+}
+
+func TestApi_InheritSolver(t *testing.T) {
+	tests := []inheritSolverTestCase{
+		{
+			desc:           "expect parents permission to apply",
+			resourceID:     "resourceID",
+			expectedStatus: http.StatusOK,
+		},
+		{
+			desc:           "expect direct permissions to apply (no inheritance)",
+			resourceID:     "orphanedID",
+			expectedStatus: http.StatusOK,
+		},
+		{
+			desc:           "expect 404 when resource is not found",
+			resourceID:     "notfound",
+			expectedStatus: http.StatusNotFound,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.desc, func(t *testing.T) {
+			userPermissions := []*accesscontrol.Permission{
+				{Action: "dashboards.permissions:read", Scope: "parents:id:parentID"},      // Inherited permission
+				{Action: "dashboards.permissions:read", Scope: "dashboards:id:orphanedID"}, // Direct permission
+				{Action: accesscontrol.ActionTeamsRead, Scope: accesscontrol.ScopeTeamsAll},
+				{Action: accesscontrol.ActionOrgUsersRead, Scope: accesscontrol.ScopeUsersAll},
+			}
+			// Add the inheritance solver "resourceID -> [parentID]" "orphanedID -> []"
+			service, sql := setupTestEnvironment(t, userPermissions,
+				withInheritance(testOptions, testInheritedScopeSolver, testInheritedScopePrefixes),
+			)
+			server := setupTestServer(t, &models.SignedInUser{OrgId: 1, Permissions: map[int64]map[string][]string{
+				1: accesscontrol.GroupScopesByAction(userPermissions),
+			}}, service)
+
+			// Seed permissions for users/teams/built-in roles specific to the test case resourceID
+			seedPermissions(t, tt.resourceID, sql, service)
+
+			permissions, recorder := getPermission(t, server, testOptions.Resource, tt.resourceID)
+			require.Equal(t, tt.expectedStatus, recorder.Code)
+
+			if tt.expectedStatus == http.StatusOK {
+				checkSeededPermissions(t, permissions)
+			}
+		})
+	}
+}
+
+func withInheritance(options Options, solver InheritedScopesSolver, inheritedPrefixes []string) Options {
+	options.InheritedScopesSolver = solver
+	options.InheritedScopePrefixes = inheritedPrefixes
+	return options
+}
+
 func setupTestServer(t *testing.T, user *models.SignedInUser, service *Service) *web.Mux {
 	server := web.New()
 	server.UseMiddleware(web.Renderer(path.Join(setting.StaticRootPath, "views"), "[[", "]]"))
@@ -530,6 +570,17 @@ var testOptions = Options{
 	},
 }
 
+var testInheritedScopePrefixes = []string{"parents:id:"}
+var testInheritedScopeSolver = func(ctx context.Context, orgID int64, id string) ([]string, error) {
+	if id == "resourceID" { // Has parent
+		return []string{"parents:id:parentID"}, nil
+	}
+	if id == "orphanedID" { // Exists but with no parent
+		return nil, nil
+	}
+	return nil, errors.New("not found")
+}
+
 var testSolver = func(ctx context.Context, orgID int64, uid string) (int64, error) {
 	if uid == "resourceUID" {
 		return 1, nil
@@ -561,6 +612,19 @@ func setPermission(t *testing.T, server *web.Mux, resource, resourceID, permissi
 	return recorder
 }
 
+func checkSeededPermissions(t *testing.T, permissions []resourcePermissionDTO) {
+	assert.Len(t, permissions, 3, "expected three assignments: user, team, builtin")
+	for _, p := range permissions {
+		if p.UserID != 0 {
+			assert.Equal(t, "View", p.Permission)
+		} else if p.TeamID != 0 {
+			assert.Equal(t, "Edit", p.Permission)
+		} else {
+			assert.Equal(t, "Edit", p.Permission)
+		}
+	}
+}
+
 func seedPermissions(t *testing.T, resourceID string, sql *sqlstore.SQLStore, service *Service) {
 	t.Helper()
 	// seed team 1 with "Edit" permission on dashboard 1

pkg/services/accesscontrol/resourcepermissions/middleware.go

@@ -5,6 +5,7 @@ import (
 	"strconv"
 
 	"github.com/grafana/grafana/pkg/models"
+	ac "github.com/grafana/grafana/pkg/services/accesscontrol"
 	"github.com/grafana/grafana/pkg/util"
 	"github.com/grafana/grafana/pkg/web"
 )
@@ -23,3 +24,22 @@ func solveUID(solve UidSolver) web.Handler {
 		}
 	}
 }
+
+// solveInheritedScopes will add the inherited scopes to the context param by prefix
+// Ex: params["folders:uid:"] = "folders:uid:BCeknZL7k"
+func solveInheritedScopes(solve InheritedScopesSolver) web.Handler {
+	return func(c *models.ReqContext) {
+		if solve != nil && util.IsValidShortUID(web.Params(c.Req)[":resourceID"]) {
+			params := web.Params(c.Req)
+			scopes, err := solve(c.Req.Context(), c.OrgId, params[":resourceID"])
+			if err != nil {
+				c.JsonApiErr(http.StatusNotFound, "Resource not found", err)
+				return
+			}
+			for _, scope := range scopes {
+				params[ac.ScopePrefix(scope)] = scope
+			}
+			web.SetURLParams(c.Req, params)
+		}
+	}
+}

pkg/services/accesscontrol/resourcepermissions/options.go

@@ -42,4 +42,6 @@ type Options struct {
 	UidSolver UidSolver
 	// InheritedScopesSolver if configured can generate additional scopes that will be used when fetching permissions for a resource
 	InheritedScopesSolver InheritedScopesSolver
+	// InheritedScopePrefixes if configured are used to create evaluators with the scopes returned by InheritedScopesSolver
+	InheritedScopePrefixes []string
 }

pkg/services/accesscontrol/scope.go

@@ -143,7 +143,7 @@ func (s *ScopeResolver) GetResolveAttributeScopeMutator(orgID int64) ScopeMutato
 		var err error
 		// By default the scope remains unchanged
 		resolvedScope := scope
-		prefix := scopePrefix(scope)
+		prefix := ScopePrefix(scope)
 		if fn, ok := s.attributeResolvers[prefix]; ok {
 			resolvedScope, err = fn(ctx, orgID, scope)
 			if err != nil {
@@ -157,10 +157,10 @@ func (s *ScopeResolver) GetResolveAttributeScopeMutator(orgID int64) ScopeMutato
 	}
 }
 
-// scopePrefix returns the prefix associated to a given scope
+// ScopePrefix returns the prefix associated to a given scope
 // we assume prefixes are all in the form <resource>:<attribute>:<value>
 // ex: "datasources:name:test" returns "datasources:name:"
-func scopePrefix(scope string) string {
+func ScopePrefix(scope string) string {
 	parts := strings.Split(scope, ":")
 	// We assume prefixes don't have more than maxPrefixParts parts
 	if len(parts) > maxPrefixParts {

pkg/services/accesscontrol/scope_test.go

@@ -198,7 +198,7 @@ func Test_scopePrefix(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			prefix := scopePrefix(tt.scope)
+			prefix := ScopePrefix(tt.scope)
 
 			assert.Equal(t, tt.want, prefix)
 		})