From e2737f195bcd970f782d99c7aa1cc36a947d0b80 Mon Sep 17 00:00:00 2001 From: Ieva Date: Fri, 21 Mar 2025 10:32:27 +0000 Subject: [PATCH] RBAC: Remove dashboard guardians pt 2 (#102556) * remove NewByDashboard guardian * remove unused authorizer * more cleanup * simplify canAdmin evaluation --- pkg/api/dashboard.go | 4 +- pkg/registry/apis/dashboard/authorizer.go | 91 ----------------------- pkg/registry/apis/dashboard/sub_dto.go | 34 +++------ pkg/services/guardian/guardian.go | 14 ---- pkg/services/guardian/provider.go | 4 - pkg/services/live/features/dashboard.go | 25 +++---- pkg/services/live/live.go | 1 + 7 files changed, 22 insertions(+), 151 deletions(-) delete mode 100644 pkg/registry/apis/dashboard/authorizer.go diff --git a/pkg/api/dashboard.go b/pkg/api/dashboard.go index 704d736f371..17d37830645 100644 --- a/pkg/api/dashboard.go +++ b/pkg/api/dashboard.go @@ -151,9 +151,7 @@ func (hs *HTTPServer) GetDashboard(c *contextmodel.ReqContext) response.Response } deleteEvaluator := accesscontrol.EvalPermission(dashboards.ActionDashboardsDelete, dashScope) canDelete, _ := hs.AccessControl.Evaluate(ctx, c.SignedInUser, deleteEvaluator) - adminEvaluator := accesscontrol.EvalAll( - accesscontrol.EvalPermission(dashboards.ActionDashboardsPermissionsRead, dashScope), - accesscontrol.EvalPermission(dashboards.ActionDashboardsPermissionsWrite, dashScope)) + adminEvaluator := accesscontrol.EvalPermission(dashboards.ActionDashboardsPermissionsWrite, dashScope) canAdmin, _ := hs.AccessControl.Evaluate(ctx, c.SignedInUser, adminEvaluator) isStarred, err := hs.isDashboardStarredByUser(c, dash.ID) diff --git a/pkg/registry/apis/dashboard/authorizer.go b/pkg/registry/apis/dashboard/authorizer.go deleted file mode 100644 index a6e316b1584..00000000000 --- a/pkg/registry/apis/dashboard/authorizer.go +++ /dev/null @@ -1,91 +0,0 @@ -package dashboard - -import ( - "context" - - "k8s.io/apiserver/pkg/authorization/authorizer" - - claims "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/dashboards" - "github.com/grafana/grafana/pkg/services/guardian" -) - -func GetAuthorizer(dashboardService dashboards.DashboardService, l log.Logger) authorizer.Authorizer { - return authorizer.AuthorizerFunc( - func(ctx context.Context, attr authorizer.Attributes) (authorized authorizer.Decision, reason string, err error) { - // Use the standard authorizer - if !attr.IsResourceRequest() { - return authorizer.DecisionNoOpinion, "", nil - } - - user, err := identity.GetRequester(ctx) - if err != nil { - return authorizer.DecisionDeny, "", err - } - - // Allow search and list requests - if attr.GetResource() == "search" || attr.GetName() == "" { - return authorizer.DecisionNoOpinion, "", nil - } - - ns := attr.GetNamespace() - if ns == "" { - return authorizer.DecisionDeny, "expected namespace", nil - } - - info, err := claims.ParseNamespace(attr.GetNamespace()) - if err != nil { - return authorizer.DecisionDeny, "error reading org from namespace", err - } - - // expensive path to lookup permissions for a single dashboard - dto, err := dashboardService.GetDashboard(ctx, &dashboards.GetDashboardQuery{ - UID: attr.GetName(), - OrgID: info.OrgID, - }) - if err != nil { - return authorizer.DecisionDeny, "error loading dashboard", err - } - - ok := false - guardian, err := guardian.NewByDashboard(ctx, dto, info.OrgID, user) - if err != nil { - return authorizer.DecisionDeny, "", err - } - - switch attr.GetVerb() { - case "get": - ok, err = guardian.CanView() - if !ok || err != nil { - return authorizer.DecisionDeny, "can not view dashboard", err - } - case "create": - fallthrough - case "post": - ok, err = guardian.CanSave() // vs Edit? - if !ok || err != nil { - return authorizer.DecisionDeny, "can not save dashboard", err - } - case "update": - fallthrough - case "patch": - fallthrough - case "put": - ok, err = guardian.CanEdit() // vs Save - if !ok || err != nil { - return authorizer.DecisionDeny, "can not edit dashboard", err - } - case "delete": - ok, err = guardian.CanDelete() - if !ok || err != nil { - return authorizer.DecisionDeny, "can not delete dashboard", err - } - default: - l.Info("unknown verb", "verb", attr.GetVerb()) - return authorizer.DecisionNoOpinion, "unsupported verb", nil // Unknown verb - } - return authorizer.DecisionAllow, "", nil - }) -} diff --git a/pkg/registry/apis/dashboard/sub_dto.go b/pkg/registry/apis/dashboard/sub_dto.go index 5768ce906d5..32f0b787ee8 100644 --- a/pkg/registry/apis/dashboard/sub_dto.go +++ b/pkg/registry/apis/dashboard/sub_dto.go @@ -19,7 +19,6 @@ import ( "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/apiserver/endpoints/request" "github.com/grafana/grafana/pkg/services/dashboards" - "github.com/grafana/grafana/pkg/services/guardian" "github.com/grafana/grafana/pkg/storage/unified/apistore" "github.com/grafana/grafana/pkg/storage/unified/resource" ) @@ -87,7 +86,7 @@ func (r *DTOConnector) ProducesObject(verb string) interface{} { } func (r *DTOConnector) Connect(ctx context.Context, name string, opts runtime.Object, responder rest.Responder) (http.Handler, error) { - info, err := request.NamespaceInfoFrom(ctx, true) + _, err := request.NamespaceInfoFrom(ctx, true) if err != nil { return nil, err } @@ -128,33 +127,22 @@ func (r *DTOConnector) Connect(ctx context.Context, name string, opts runtime.Ob return } - // Calculate access information -- needed to help smooth transition from /api/dashboard format - dto := &dashboards.Dashboard{ - UID: name, - OrgID: info.OrgID, - ID: obj.GetDeprecatedInternalID(), // nolint:staticcheck - } - manager, ok := obj.GetManagerProperties() - if ok && manager.Kind == utils.ManagerKindPlugin { - dto.PluginID = manager.Identity - } - - guardian, err := guardian.NewByDashboard(ctx, dto, info.OrgID, user) - if err != nil { - responder.Error(err) - return - } - canView, err := guardian.CanView() + dashScope := dashboards.ScopeDashboardsProvider.GetResourceScopeUID(name) + evaluator := accesscontrol.EvalPermission(dashboards.ActionDashboardsRead, dashScope) + canView, err := r.accessControl.Evaluate(ctx, user, evaluator) if err != nil || !canView { responder.Error(fmt.Errorf("not allowed to view")) return } access := &dashboard.DashboardAccess{} - access.CanEdit, _ = guardian.CanEdit() - access.CanSave, _ = guardian.CanSave() - access.CanAdmin, _ = guardian.CanAdmin() - access.CanDelete, _ = guardian.CanDelete() + writeEvaluator := accesscontrol.EvalPermission(dashboards.ActionDashboardsWrite, dashScope) + access.CanSave, _ = r.accessControl.Evaluate(ctx, user, writeEvaluator) + access.CanEdit = access.CanSave + adminEvaluator := accesscontrol.EvalPermission(dashboards.ActionDashboardsPermissionsWrite, dashScope) + access.CanAdmin, _ = r.accessControl.Evaluate(ctx, user, adminEvaluator) + deleteEvaluator := accesscontrol.EvalPermission(dashboards.ActionDashboardsDelete, dashScope) + access.CanDelete, _ = r.accessControl.Evaluate(ctx, user, deleteEvaluator) access.CanStar = user.IsIdentityType(claims.TypeUser) access.AnnotationsPermissions = &dashboard.AnnotationPermission{} diff --git a/pkg/services/guardian/guardian.go b/pkg/services/guardian/guardian.go index 7100b7ffacc..58ebe2aa878 100644 --- a/pkg/services/guardian/guardian.go +++ b/pkg/services/guardian/guardian.go @@ -7,7 +7,6 @@ import ( "github.com/grafana/grafana/pkg/apimachinery/errutil" "github.com/grafana/grafana/pkg/apimachinery/identity" "github.com/grafana/grafana/pkg/infra/metrics" - "github.com/grafana/grafana/pkg/services/dashboards" "github.com/grafana/grafana/pkg/services/folder" ) @@ -34,12 +33,6 @@ var New = func(ctx context.Context, dashId int64, orgId int64, user identity.Req panic("no guardian factory implementation provided") } -// NewByDashboard factory for creating a new dashboard guardian instance -// When using access control this function is replaced on startup and the AccessControlDashboardGuardian is returned -var NewByDashboard = func(ctx context.Context, dash *dashboards.Dashboard, orgId int64, user identity.Requester) (DashboardGuardian, error) { - panic("no guardian factory implementation provided") -} - // NewByFolderUID factory for creating a new folder guardian instance // When using access control this function is replaced on startup and the AccessControlDashboardGuardian is returned var NewByFolderUID = func(ctx context.Context, folderUID string, orgId int64, user identity.Requester) (DashboardGuardian, error) { @@ -108,13 +101,6 @@ func MockDashboardGuardian(mock *FakeDashboardGuardian) { mock.User = user return mock, nil } - NewByDashboard = func(_ context.Context, dash *dashboards.Dashboard, orgId int64, user identity.Requester) (DashboardGuardian, error) { - mock.OrgID = orgId - mock.DashUID = dash.UID - mock.DashID = dash.ID - mock.User = user - return mock, nil - } NewByFolderUID = func(_ context.Context, folderUID string, orgId int64, user identity.Requester) (DashboardGuardian, error) { mock.OrgID = orgId diff --git a/pkg/services/guardian/provider.go b/pkg/services/guardian/provider.go index 7e3902f1e5a..7be96481b51 100644 --- a/pkg/services/guardian/provider.go +++ b/pkg/services/guardian/provider.go @@ -31,10 +31,6 @@ func InitAccessControlGuardian( return NewAccessControlDashboardGuardian(ctx, cfg, dashId, user, ac, dashboardService, folderService, logger) } - NewByDashboard = func(ctx context.Context, dash *dashboards.Dashboard, orgId int64, user identity.Requester) (DashboardGuardian, error) { - return NewAccessControlDashboardGuardianByDashboard(ctx, cfg, dash, user, ac, dashboardService, folderService, logger) - } - NewByFolderUID = func(ctx context.Context, folderUID string, orgId int64, user identity.Requester) (DashboardGuardian, error) { return NewAccessControlFolderGuardianByUID(ctx, cfg, folderUID, user, ac, dashboardService, folderService) } diff --git a/pkg/services/live/features/dashboard.go b/pkg/services/live/features/dashboard.go index 2390f409fb4..286dad7c49c 100644 --- a/pkg/services/live/features/dashboard.go +++ b/pkg/services/live/features/dashboard.go @@ -10,8 +10,8 @@ import ( "github.com/grafana/grafana/pkg/apimachinery/identity" "github.com/grafana/grafana/pkg/infra/db" + "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/dashboards" - "github.com/grafana/grafana/pkg/services/guardian" "github.com/grafana/grafana/pkg/services/live/model" ) @@ -63,6 +63,7 @@ type DashboardHandler struct { ClientCount model.ChannelClientCount Store db.DB DashboardService dashboards.DashboardService + AccessControl accesscontrol.AccessControl } // GetHandlerForPath called on init @@ -77,20 +78,17 @@ func (h *DashboardHandler) OnSubscribe(ctx context.Context, user identity.Reques // make sure can view this dashboard if len(parts) == 2 && parts[0] == "uid" { query := dashboards.GetDashboardQuery{UID: parts[1], OrgID: user.GetOrgID()} - queryResult, err := h.DashboardService.GetDashboard(ctx, &query) + _, err := h.DashboardService.GetDashboard(ctx, &query) if err != nil { logger.Error("Error getting dashboard", "query", query, "error", err) return model.SubscribeReply{}, backend.SubscribeStreamStatusNotFound, nil } - dash := queryResult - guard, err := guardian.NewByDashboard(ctx, dash, user.GetOrgID(), user) - if err != nil { + evaluator := accesscontrol.EvalPermission(dashboards.ActionDashboardsRead, dashboards.ScopeDashboardsProvider.GetResourceScopeUID(parts[1])) + canView, err := h.AccessControl.Evaluate(ctx, user, evaluator) + if err != nil || !canView { return model.SubscribeReply{}, backend.SubscribeStreamStatusPermissionDenied, err } - if canView, err := guard.CanView(); err != nil || !canView { - return model.SubscribeReply{}, backend.SubscribeStreamStatusPermissionDenied, nil - } return model.SubscribeReply{ Presence: true, @@ -119,19 +117,14 @@ func (h *DashboardHandler) OnPublish(ctx context.Context, requester identity.Req return model.PublishReply{}, backend.PublishStreamStatusNotFound, fmt.Errorf("ignore???") } query := dashboards.GetDashboardQuery{UID: parts[1], OrgID: requester.GetOrgID()} - queryResult, err := h.DashboardService.GetDashboard(ctx, &query) + _, err = h.DashboardService.GetDashboard(ctx, &query) if err != nil { logger.Error("Unknown dashboard", "query", query) return model.PublishReply{}, backend.PublishStreamStatusNotFound, nil } - guard, err := guardian.NewByDashboard(ctx, queryResult, requester.GetOrgID(), requester) - if err != nil { - logger.Error("Failed to create guardian", "err", err) - return model.PublishReply{}, backend.PublishStreamStatusNotFound, fmt.Errorf("internal error") - } - - canEdit, err := guard.CanEdit() + evaluator := accesscontrol.EvalPermission(dashboards.ActionDashboardsWrite, dashboards.ScopeDashboardsProvider.GetResourceScopeUID(parts[1])) + canEdit, err := h.AccessControl.Evaluate(ctx, requester, evaluator) if err != nil { return model.PublishReply{}, backend.PublishStreamStatusNotFound, fmt.Errorf("internal error") } diff --git a/pkg/services/live/live.go b/pkg/services/live/live.go index 35c2be26d69..64efaaa18b6 100644 --- a/pkg/services/live/live.go +++ b/pkg/services/live/live.go @@ -184,6 +184,7 @@ func ProvideService(plugCtxProvider *plugincontext.Provider, cfg *setting.Cfg, r ClientCount: g.ClientCount, Store: sqlStore, DashboardService: dashboardService, + AccessControl: accessControl, } g.storage = database.NewStorage(g.SQLStore, g.CacheService) g.GrafanaScope.Dashboards = dash