[release-12.0.1] K8s: Dashboards: Add fine grained access control checks to /apis (#104419)

K8s: Dashboards: Add fine grained access control checks to /apis (#104347)


---------

Co-authored-by: Ieva <ieva.vasiljeva@grafana.com>
Co-authored-by: Gabriel MABILLE <gabriel.mabille@grafana.com>
Co-authored-by: Marco de Abreu <marco.deabreu@grafana.com>
Co-authored-by: Georges Chaudy <chaudyg@gmail.com>

pkg/apimachinery/identity/context.go

@@ -58,7 +58,7 @@ func newInternalIdentity(name string, namespace string, orgID int64) Requester {
 // This is useful for background tasks that has to communicate with unfied storage. It also returns a Requester with
 // static permissions so it can be used in legacy code paths.
 func WithServiceIdentity(ctx context.Context, orgID int64) (context.Context, Requester) {
-	r := newInternalIdentity(serviceName, "", orgID)
+	r := newInternalIdentity(serviceName, "*", orgID)
 	return WithRequester(ctx, r), r
 }
 

pkg/registry/apis/dashboard/authorizer.go

@@ -0,0 +1,79 @@
+package dashboard
+
+import (
+	"context"
+
+	"k8s.io/apiserver/pkg/authorization/authorizer"
+
+	"github.com/grafana/authlib/types"
+	"github.com/grafana/grafana/pkg/apimachinery/identity"
+	"github.com/grafana/grafana/pkg/infra/log"
+	"github.com/grafana/grafana/pkg/services/accesscontrol"
+	"github.com/grafana/grafana/pkg/services/dashboards"
+)
+
+func GetAuthorizer(ac accesscontrol.AccessControl, l log.Logger) authorizer.Authorizer {
+	return authorizer.AuthorizerFunc(
+		func(ctx context.Context, attr authorizer.Attributes) (authorized authorizer.Decision, reason string, err error) {
+			// Note that we will return Allow more than expected.
+			// This is because we do NOT want to hit the RoleAuthorizer that would be evaluated afterwards.
+
+			if !attr.IsResourceRequest() {
+				return authorizer.DecisionDeny, "unexpected non-resource request", nil
+			}
+
+			user, err := identity.GetRequester(ctx)
+			if err != nil {
+				return authorizer.DecisionDeny, "error getting requester", err
+			}
+
+			ns := attr.GetNamespace()
+			if ns == "" {
+				return authorizer.DecisionDeny, "expected namespace", nil
+			}
+
+			info, err := types.ParseNamespace(attr.GetNamespace())
+			if err != nil {
+				return authorizer.DecisionDeny, "error reading org from namespace", err
+			}
+
+			// Validate organization access before we possibly step out here.
+			if user.GetOrgID() != info.OrgID {
+				return authorizer.DecisionDeny, "org mismatch", dashboards.ErrUserIsNotSignedInToOrg
+			}
+
+			switch attr.GetVerb() {
+			case "list", "search":
+				// Detailed read permissions are handled by authz, this just checks whether the user can ready *any* dashboard
+				ok, err := ac.Evaluate(ctx, user, accesscontrol.EvalPermission(dashboards.ActionDashboardsRead))
+				if !ok || err != nil {
+					return authorizer.DecisionDeny, "can not read any dashboards", err
+				}
+			case "create":
+				// Detailed create permissions are handled by authz, this just checks whether the user can create *any* dashboard
+				ok, err := ac.Evaluate(ctx, user, accesscontrol.EvalPermission(dashboards.ActionDashboardsCreate))
+				if !ok || err != nil {
+					return authorizer.DecisionDeny, "can not create any dashboards", err
+				}
+			case "get":
+				ok, err := ac.Evaluate(ctx, user, accesscontrol.EvalPermission(dashboards.ActionDashboardsRead, dashboards.ScopeDashboardsProvider.GetResourceScopeUID(attr.GetName())))
+				if !ok || err != nil {
+					return authorizer.DecisionDeny, "can not view dashboard", err
+				}
+			case "update", "patch":
+				ok, err := ac.Evaluate(ctx, user, accesscontrol.EvalPermission(dashboards.ActionDashboardsWrite, dashboards.ScopeDashboardsProvider.GetResourceScopeUID(attr.GetName())))
+				if !ok || err != nil {
+					return authorizer.DecisionDeny, "can not edit dashboard", err
+				}
+			case "delete":
+				ok, err := ac.Evaluate(ctx, user, accesscontrol.EvalPermission(dashboards.ActionDashboardsDelete, dashboards.ScopeDashboardsProvider.GetResourceScopeUID(attr.GetName())))
+				if !ok || err != nil {
+					return authorizer.DecisionDeny, "can not delete dashboard", err
+				}
+			default:
+				l.Info("unknown verb", "verb", attr.GetVerb())
+				return authorizer.DecisionDeny, "unsupported verb", nil // Unknown verb
+			}
+			return authorizer.DecisionAllow, "", nil
+		})
+}

pkg/registry/apis/dashboard/legacy/sql_dashboards.go

@@ -122,11 +122,6 @@ func (a *dashboardSqlAccess) getRows(ctx context.Context, sql *legacysql.LegacyD
 		rows:    rows,
 		a:       a,
 		history: query.GetHistory,
-		// This looks up rules from the permissions on a user
-		canReadDashboard: func(scopes ...string) bool {
-			return true // ???
-		},
-		// accesscontrol.Checker(user, dashboards.ActionDashboardsRead),
 	}, err
 }
 
@@ -138,8 +133,6 @@ type rowsWrapper struct {
 	history bool
 	count   int
 
-	canReadDashboard func(scopes ...string) bool
-
 	// Current
 	row *dashboardRow
 	err error
@@ -180,17 +173,6 @@ func (r *rowsWrapper) Next() bool {
 		}
 
 		if r.row != nil {
-			d := r.row
-
-			// Access control checker
-			scopes := []string{dashboards.ScopeDashboardsProvider.GetResourceScopeUID(d.Dash.Name)}
-			if d.FolderUID != "" { // Copied from searchV2... not sure the logic is right
-				scopes = append(scopes, dashboards.ScopeFoldersProvider.GetResourceScopeUID(d.FolderUID))
-			}
-			if !r.canReadDashboard(scopes...) {
-				continue
-			}
-
 			// returns the first visible dashboard
 			return true
 		}

pkg/registry/apis/dashboard/legacy/storage.go

@@ -211,6 +211,12 @@ func (a *dashboardSqlAccess) ReadResource(ctx context.Context, req *resource.Rea
 			Code: http.StatusNotFound,
 		}
 	} else {
+		meta, err := utils.MetaAccessor(dash)
+		if err != nil {
+			rsp.Error = resource.AsErrorResult(err)
+		}
+		rsp.Folder = meta.GetFolder()
+
 		rsp.Value, err = json.Marshal(dash)
 		if err != nil {
 			rsp.Error = resource.AsErrorResult(err)

pkg/registry/apis/dashboard/legacy_storage.go

@@ -10,6 +10,8 @@ import (
 	"k8s.io/apiserver/pkg/registry/generic/registry"
 	"k8s.io/apiserver/pkg/registry/rest"
 
+	"github.com/grafana/authlib/types"
+
 	"github.com/grafana/grafana/pkg/apimachinery/utils"
 	grafanaregistry "github.com/grafana/grafana/pkg/apiserver/registry/generic"
 	grafanarest "github.com/grafana/grafana/pkg/apiserver/rest"
@@ -25,10 +27,11 @@ type DashboardStorage struct {
 	DashboardService dashboards.DashboardService
 }
 
-func (s *DashboardStorage) NewStore(dash utils.ResourceInfo, scheme *runtime.Scheme, defaultOptsGetter generic.RESTOptionsGetter, reg prometheus.Registerer, permissions dashboards.PermissionsRegistrationService) (grafanarest.Storage, error) {
+func (s *DashboardStorage) NewStore(dash utils.ResourceInfo, scheme *runtime.Scheme, defaultOptsGetter generic.RESTOptionsGetter, reg prometheus.Registerer, permissions dashboards.PermissionsRegistrationService, ac types.AccessClient) (grafanarest.Storage, error) {
 	server, err := resource.NewResourceServer(resource.ResourceServerOptions{
 		Backend:      s.Access,
 		Reg:          reg,
+		AccessClient: ac,
 	})
 	if err != nil {
 		return nil, err

pkg/registry/apis/dashboard/register.go

@@ -12,6 +12,7 @@ import (
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apiserver/pkg/admission"
+	"k8s.io/apiserver/pkg/authorization/authorizer"
 	"k8s.io/apiserver/pkg/registry/rest"
 	genericapiserver "k8s.io/apiserver/pkg/server"
 	"k8s.io/kube-openapi/pkg/common"
@@ -71,6 +72,7 @@ type DashboardsAPIBuilder struct {
 	features         featuremgmt.FeatureToggles
 
 	accessControl                accesscontrol.AccessControl
+	accessClient                 claims.AccessClient
 	legacy                       *DashboardStorage
 	unified                      resource.ResourceClient
 	dashboardProvisioningService dashboards.DashboardProvisioningService
@@ -97,6 +99,7 @@ func RegisterAPIService(
 	provisioningDashboardService dashboards.DashboardProvisioningService,
 	dashboardPermissions dashboards.PermissionsRegistrationService,
 	accessControl accesscontrol.AccessControl,
+	accessClient claims.AccessClient,
 	provisioning provisioning.ProvisioningService,
 	dashStore dashboards.Store,
 	reg prometheus.Registerer,
@@ -121,6 +124,7 @@ func RegisterAPIService(
 		dashboardPermissions:         dashboardPermissions,
 		features:                     features,
 		accessControl:                accessControl,
+		accessClient:                 accessClient,
 		unified:                      unified,
 		dashboardProvisioningService: provisioningDashboardService,
 		search:                       NewSearchHandler(tracing, dual, legacyDashboardSearcher, unified, features),
@@ -328,7 +332,16 @@ func (b *DashboardsAPIBuilder) validateUpdate(ctx context.Context, a admission.A
 	}
 
 	// Validate folder existence if specified and changed
-	if !a.IsDryRun() && newAccessor.GetFolder() != "" && newAccessor.GetFolder() != oldAccessor.GetFolder() {
+	if !a.IsDryRun() && newAccessor.GetFolder() != oldAccessor.GetFolder() {
+		id, err := identity.GetRequester(ctx)
+		if err != nil {
+			return fmt.Errorf("error getting requester: %w", err)
+		}
+
+		if err := b.verifyFolderAccessPermissions(ctx, id, newAccessor.GetFolder()); err != nil {
+			return err
+		}
+
 		if err := b.validateFolderExists(ctx, newAccessor.GetFolder(), nsInfo.OrgID); err != nil {
 			return apierrors.NewNotFound(folders.FolderResourceInfo.GroupResource(), newAccessor.GetFolder())
 		}
@@ -472,7 +485,7 @@ func (b *DashboardsAPIBuilder) storageForVersion(
 	storage := map[string]rest.Storage{}
 	apiGroupInfo.VersionedResourcesStorageMap[dashboards.GroupVersion().Version] = storage
 
-	legacyStore, err := b.legacy.NewStore(dashboards, opts.Scheme, opts.OptsGetter, b.reg, b.dashboardPermissions)
+	legacyStore, err := b.legacy.NewStore(dashboards, opts.Scheme, opts.OptsGetter, b.reg, b.dashboardPermissions, b.accessClient)
 	if err != nil {
 		return err
 	}
@@ -535,3 +548,24 @@ func (b *DashboardsAPIBuilder) GetAPIRoutes(gv schema.GroupVersion) *builder.API
 	defs := b.GetOpenAPIDefinitions()(func(path string) spec.Ref { return spec.Ref{} })
 	return b.search.GetAPIRoutes(defs)
 }
+
+func (b *DashboardsAPIBuilder) GetAuthorizer() authorizer.Authorizer {
+	return GetAuthorizer(b.accessControl, b.log)
+}
+
+func (b *DashboardsAPIBuilder) verifyFolderAccessPermissions(ctx context.Context, user identity.Requester, folderIds ...string) error {
+	scopes := []string{}
+	for _, folderId := range folderIds {
+		scopes = append(scopes, dashboards.ScopeFoldersProvider.GetResourceScopeUID(folderId))
+	}
+	ok, err := b.accessControl.Evaluate(ctx, user, accesscontrol.EvalPermission(dashboards.ActionFoldersWrite, scopes...))
+	if err != nil {
+		return err
+	}
+
+	if !ok {
+		return dashboards.ErrFolderAccessDenied
+	}
+
+	return nil
+}

pkg/services/authz/rbac/service.go

@@ -18,6 +18,7 @@ import (
 	authzv1 "github.com/grafana/authlib/authz/proto/v1"
 	"github.com/grafana/authlib/cache"
 	"github.com/grafana/authlib/types"
+	"github.com/grafana/grafana/pkg/apimachinery/utils"
 
 	"github.com/grafana/grafana/pkg/infra/log"
 	"github.com/grafana/grafana/pkg/infra/tracing"
@@ -553,10 +554,14 @@ func (s *Service) checkPermission(ctx context.Context, scopeMap map[string]bool,
 	ctxLogger := s.logger.FromContext(ctx)
 
 	// Only check action if the request doesn't specify scope
-	if req.Name == "" {
+	if req.Name == "" && req.Verb != utils.VerbCreate {
 		return len(scopeMap) > 0, nil
 	}
 
+	if req.Verb == utils.VerbCreate && req.ParentFolder == "" {
+		req.ParentFolder = accesscontrol.GeneralFolderUID
+	}
+
 	// Wildcard grant, no further checks needed
 	if scopeMap["*"] {
 		return true, nil
@@ -568,7 +573,7 @@ func (s *Service) checkPermission(ctx context.Context, scopeMap map[string]bool,
 		return false, status.Error(codes.NotFound, "unsupported resource")
 	}
 
-	if scopeMap[t.scope(req.Name)] {
+	if req.Name != "" && scopeMap[t.scope(req.Name)] {
 		return true, nil
 	}
 

pkg/services/authz/rbac/service_test.go

@@ -17,6 +17,7 @@ import (
 	"github.com/grafana/authlib/cache"
 	"github.com/grafana/authlib/types"
 
+	"github.com/grafana/grafana/pkg/apimachinery/utils"
 	"github.com/grafana/grafana/pkg/infra/log"
 	"github.com/grafana/grafana/pkg/infra/tracing"
 	"github.com/grafana/grafana/pkg/registry/apis/iam/legacy"
@@ -127,19 +128,39 @@ func TestService_checkPermission(t *testing.T) {
 			expected: true,
 		},
 		{
-			name: "should return true if no resource is specified",
+			name: "should check general folder scope for root level resource creation",
 			permissions: []accesscontrol.Permission{
 				{
-					Action: "folders:create",
+					Action:     "dashboards:create",
+					Scope:      "folders:uid:general",
+					Kind:       "folders",
+					Attribute:  "uid",
+					Identifier: "general",
 				},
 			},
 			check: CheckRequest{
-				Action:   "folders:create",
+				Action:   "dashboards:create",
-				Group:    "folder.grafana.app",
+				Group:    "dashboard.grafana.app",
-				Resource: "folders",
+				Resource: "dashboards",
+				Verb:     utils.VerbCreate,
 			},
 			expected: true,
 		},
+		{
+			name: "should fail if user doesn't have general folder scope for root level resource creation",
+			permissions: []accesscontrol.Permission{
+				{
+					Action: "dashboards:create",
+				},
+			},
+			check: CheckRequest{
+				Action:   "dashboards:create",
+				Group:    "dashboard.grafana.app",
+				Resource: "dashboards",
+				Verb:     utils.VerbCreate,
+			},
+			expected: false,
+		},
 		{
 			name:        "should return false if user has no permissions on resource",
 			permissions: []accesscontrol.Permission{},
@@ -174,6 +195,72 @@ func TestService_checkPermission(t *testing.T) {
 			},
 			expected: true,
 		},
+		{
+			name: "should allow creating a nested resource",
+			permissions: []accesscontrol.Permission{
+				{
+					Action:     "dashboards:create",
+					Scope:      "folders:uid:parent",
+					Kind:       "folders",
+					Attribute:  "uid",
+					Identifier: "parent",
+				},
+			},
+			folders: []store.Folder{{UID: "parent"}},
+			check: CheckRequest{
+				Action:       "dashboards:create",
+				Group:        "dashboard.grafana.app",
+				Resource:     "dashboards",
+				Name:         "",
+				ParentFolder: "parent",
+				Verb:         utils.VerbCreate,
+			},
+			expected: true,
+		},
+		{
+			name: "should deny creating a nested resource",
+			permissions: []accesscontrol.Permission{
+				{
+					Action:     "dashboards:create",
+					Scope:      "folders:uid:parent",
+					Kind:       "folders",
+					Attribute:  "uid",
+					Identifier: "parent",
+				},
+			},
+			folders: []store.Folder{{UID: "parent"}},
+			check: CheckRequest{
+				Action:       "dashboards:create",
+				Group:        "dashboard.grafana.app",
+				Resource:     "dashboards",
+				Name:         "",
+				ParentFolder: "other_parent",
+				Verb:         utils.VerbCreate,
+			},
+			expected: false,
+		},
+		{
+			name: "should allow if it's an any check",
+			permissions: []accesscontrol.Permission{
+				{
+					Action:     "dashboards:read",
+					Scope:      "folders:uid:parent",
+					Kind:       "folders",
+					Attribute:  "uid",
+					Identifier: "parent",
+				},
+			},
+			folders: []store.Folder{{UID: "parent"}},
+			check: CheckRequest{
+				Action:       "dashboards:read",
+				Group:        "dashboard.grafana.app",
+				Resource:     "dashboards",
+				Name:         "",
+				ParentFolder: "",
+				Verb:         utils.VerbList,
+			},
+			expected: true,
+		},
 	}
 
 	for _, tc := range testCases {

scripts/grafana-server/custom.ini

@@ -10,6 +10,9 @@ grafanaAPIServer=true
 queryLibrary=true
 queryService=true
 
+[environment]
+stack_id = 12345
+
 [plugins]
 allow_loading_unsigned_plugins=grafana-extensionstest-app,grafana-extensionexample1-app,grafana-extensionexample2-app,grafana-extensionexample3-app,grafana-e2etest-datasource