From de50f39c12c21617cee3399258c020bf0ff4a869 Mon Sep 17 00:00:00 2001 From: Karl Persson Date: Mon, 2 May 2022 09:29:30 +0200 Subject: [PATCH] Access Control: Refactor scope resolvers with support to resolve into several scopes (#48202) * Refactor Scope resolver to support resolving into several scopes * Change permission evaluator to match at least one of passed scopes --- pkg/api/annotations.go | 20 +-- pkg/api/annotations_test.go | 7 +- pkg/api/http_server.go | 2 +- pkg/services/accesscontrol/accesscontrol.go | 4 +- pkg/services/accesscontrol/evaluator.go | 52 +++--- pkg/services/accesscontrol/evaluator_test.go | 8 +- pkg/services/accesscontrol/mock/mock.go | 26 ++- .../ossaccesscontrol/ossaccesscontrol.go | 32 ++-- .../ossaccesscontrol/ossaccesscontrol_test.go | 22 +-- pkg/services/accesscontrol/resolvers.go | 140 +++++++++++++++ pkg/services/accesscontrol/resolvers_test.go | 159 ++++++++++++++++++ pkg/services/accesscontrol/scope.go | 114 ------------- pkg/services/accesscontrol/scope_test.go | 155 +---------------- pkg/services/dashboards/accesscontrol.go | 36 ++-- pkg/services/dashboards/accesscontrol_test.go | 58 ++++--- .../dashboards/manager/folder_service.go | 4 +- .../datasources/service/datasource_service.go | 38 ++--- .../service/datasource_service_test.go | 10 +- 18 files changed, 453 insertions(+), 434 deletions(-) create mode 100644 pkg/services/accesscontrol/resolvers.go create mode 100644 pkg/services/accesscontrol/resolvers_test.go diff --git a/pkg/api/annotations.go b/pkg/api/annotations.go index 7aa1a2a3a2e..762d64e1e29 100644 --- a/pkg/api/annotations.go +++ b/pkg/api/annotations.go @@ -401,20 +401,21 @@ func (hs *HTTPServer) GetAnnotationTags(c *models.ReqContext) response.Response return response.JSON(http.StatusOK, annotations.GetAnnotationTagsResponse{Result: result}) } -// AnnotationTypeScopeResolver provides an AttributeScopeResolver able to +// AnnotationTypeScopeResolver provides an ScopeAttributeResolver able to // resolve annotation types. Scope "annotations:id:" will be translated to "annotations:type:, // where is the type of annotation with id . -func AnnotationTypeScopeResolver() (string, accesscontrol.AttributeScopeResolveFunc) { - annotationTypeResolver := func(ctx context.Context, orgID int64, initialScope string) (string, error) { +func AnnotationTypeScopeResolver() (string, accesscontrol.ScopeAttributeResolver) { + prefix := accesscontrol.ScopeAnnotationsProvider.GetResourceScope("") + return prefix, accesscontrol.ScopeAttributeResolverFunc(func(ctx context.Context, orgID int64, initialScope string) ([]string, error) { scopeParts := strings.Split(initialScope, ":") if scopeParts[0] != accesscontrol.ScopeAnnotationsRoot || len(scopeParts) != 3 { - return "", accesscontrol.ErrInvalidScope + return nil, accesscontrol.ErrInvalidScope } annotationIdStr := scopeParts[2] annotationId, err := strconv.Atoi(annotationIdStr) if err != nil { - return "", accesscontrol.ErrInvalidScope + return nil, accesscontrol.ErrInvalidScope } // tempUser is used to resolve annotation type. @@ -431,16 +432,15 @@ func AnnotationTypeScopeResolver() (string, accesscontrol.AttributeScopeResolveF annotation, resp := findAnnotationByID(ctx, annotations.GetRepository(), int64(annotationId), tempUser) if resp != nil { - return "", errors.New("could not resolve annotation type") + return nil, errors.New("could not resolve annotation type") } if annotation.GetType() == annotations.Organization { - return accesscontrol.ScopeAnnotationsTypeOrganization, nil + return []string{accesscontrol.ScopeAnnotationsTypeOrganization}, nil } else { - return accesscontrol.ScopeAnnotationsTypeDashboard, nil + return []string{accesscontrol.ScopeAnnotationsTypeDashboard}, nil } - } - return accesscontrol.ScopeAnnotationsProvider.GetResourceScope(""), annotationTypeResolver + }) } func (hs *HTTPServer) canCreateAnnotation(c *models.ReqContext, dashboardId int64) (bool, error) { diff --git a/pkg/api/annotations_test.go b/pkg/api/annotations_test.go index 2b0aff9d083..603972b06f4 100644 --- a/pkg/api/annotations_test.go +++ b/pkg/api/annotations_test.go @@ -722,7 +722,7 @@ func TestAPI_Annotations_AccessControl(t *testing.T) { t.Run(tt.name, func(t *testing.T) { setUpRBACGuardian(t) sc.acmock. - RegisterAttributeScopeResolver(AnnotationTypeScopeResolver()) + RegisterScopeAttributeResolver(AnnotationTypeScopeResolver()) setAccessControlPermissions(sc.acmock, tt.args.permissions, sc.initCtx.OrgId) r := callAPI(sc.server, tt.args.method, tt.args.url, tt.args.body, t) @@ -780,13 +780,14 @@ func TestService_AnnotationTypeScopeResolver(t *testing.T) { for _, tc := range testCases { t.Run(tc.desc, func(t *testing.T) { - resolved, err := resolver(context.Background(), 1, tc.given) + resolved, err := resolver.Resolve(context.Background(), 1, tc.given) if tc.wantErr != nil { require.Error(t, err) require.Equal(t, tc.wantErr, err) } else { require.NoError(t, err) - require.Equal(t, tc.want, resolved) + require.Len(t, resolved, 1) + require.Equal(t, tc.want, resolved[0]) } }) } diff --git a/pkg/api/http_server.go b/pkg/api/http_server.go index 5bceecfddf2..fb4a2438ff2 100644 --- a/pkg/api/http_server.go +++ b/pkg/api/http_server.go @@ -265,7 +265,7 @@ func ProvideHTTPServer(opts ServerOptions, cfg *setting.Cfg, routeRegister routi hs.registerRoutes() // Register access control scope resolver for annotations - hs.AccessControl.RegisterAttributeScopeResolver(AnnotationTypeScopeResolver()) + hs.AccessControl.RegisterScopeAttributeResolver(AnnotationTypeScopeResolver()) if err := hs.declareFixedRoles(); err != nil { return nil, err diff --git a/pkg/services/accesscontrol/accesscontrol.go b/pkg/services/accesscontrol/accesscontrol.go index f23795a44cd..faab1529485 100644 --- a/pkg/services/accesscontrol/accesscontrol.go +++ b/pkg/services/accesscontrol/accesscontrol.go @@ -32,9 +32,9 @@ type AccessControl interface { // assignments to organization roles ("Viewer", "Editor", "Admin") or "Grafana Admin" DeclareFixedRoles(...RoleRegistration) error - // RegisterAttributeScopeResolver allows the caller to register a scope resolver for a + // RegisterScopeAttributeResolver allows the caller to register a scope resolver for a // specific scope prefix (ex: datasources:name:) - RegisterAttributeScopeResolver(scopePrefix string, resolver AttributeScopeResolveFunc) + RegisterScopeAttributeResolver(scopePrefix string, resolver ScopeAttributeResolver) } type PermissionsProvider interface { diff --git a/pkg/services/accesscontrol/evaluator.go b/pkg/services/accesscontrol/evaluator.go index 52be2d80549..ac80a364a7b 100644 --- a/pkg/services/accesscontrol/evaluator.go +++ b/pkg/services/accesscontrol/evaluator.go @@ -14,7 +14,7 @@ type Evaluator interface { // Evaluate permissions that are grouped by action Evaluate(permissions map[string][]string) (bool, error) // MutateScopes executes a sequence of ScopeModifier functions on all embedded scopes of an evaluator and returns a new Evaluator - MutateScopes(context.Context, ...ScopeMutator) (Evaluator, error) + MutateScopes(ctx context.Context, mutate ScopeAttributeMutator) (Evaluator, error) // String returns a string representation of permission required by the evaluator fmt.Stringer fmt.GoStringer @@ -22,7 +22,7 @@ type Evaluator interface { var _ Evaluator = new(permissionEvaluator) -// EvalPermission returns an evaluator that will require all scopes in combination with action to match +// EvalPermission returns an evaluator that will require at least one of passed scopes to match func EvalPermission(action string, scopes ...string) Evaluator { return permissionEvaluator{Action: action, Scopes: scopes} } @@ -43,29 +43,19 @@ func (p permissionEvaluator) Evaluate(permissions map[string][]string) (bool, er } for _, target := range p.Scopes { - var err error - var matches bool - for _, scope := range userScopes { - matches, err = match(scope, target) - if err != nil { - return false, err + if match(scope, target) { + return true, nil } - if matches { - break - } - } - if !matches { - return false, nil } } - return true, nil + return false, nil } -func match(scope, target string) (bool, error) { +func match(scope, target string) bool { if scope == "" { - return false, nil + return false } if !ValidateScope(scope) { @@ -74,7 +64,7 @@ func match(scope, target string) (bool, error) { "scope", scope, "reason", "scopes should not contain meta-characters like * or ?, except in the last position", ) - return false, nil + return false } prefix, last := scope[:len(scope)-1], scope[len(scope)-1] @@ -82,29 +72,25 @@ func match(scope, target string) (bool, error) { if last == '*' { if strings.HasPrefix(target, prefix) { logger.Debug("matched scope", "user scope", scope, "target scope", target) - return true, nil + return true } } - return scope == target, nil + return scope == target } -func (p permissionEvaluator) MutateScopes(ctx context.Context, modifiers ...ScopeMutator) (Evaluator, error) { - var err error +func (p permissionEvaluator) MutateScopes(ctx context.Context, mutate ScopeAttributeMutator) (Evaluator, error) { if p.Scopes == nil { return EvalPermission(p.Action), nil } scopes := make([]string, 0, len(p.Scopes)) for _, scope := range p.Scopes { - modified := scope - for _, modifier := range modifiers { - modified, err = modifier(ctx, modified) - if err != nil { - return nil, err - } + mutated, err := mutate(ctx, scope) + if err != nil { + return nil, err } - scopes = append(scopes, modified) + scopes = append(scopes, mutated...) } return EvalPermission(p.Action, scopes...), nil } @@ -137,10 +123,10 @@ func (a allEvaluator) Evaluate(permissions map[string][]string) (bool, error) { return true, nil } -func (a allEvaluator) MutateScopes(ctx context.Context, modifiers ...ScopeMutator) (Evaluator, error) { +func (a allEvaluator) MutateScopes(ctx context.Context, mutate ScopeAttributeMutator) (Evaluator, error) { var modified []Evaluator for _, e := range a.allOf { - i, err := e.MutateScopes(ctx, modifiers...) + i, err := e.MutateScopes(ctx, mutate) if err != nil { return nil, err } @@ -191,10 +177,10 @@ func (a anyEvaluator) Evaluate(permissions map[string][]string) (bool, error) { return false, nil } -func (a anyEvaluator) MutateScopes(ctx context.Context, modifiers ...ScopeMutator) (Evaluator, error) { +func (a anyEvaluator) MutateScopes(ctx context.Context, mutate ScopeAttributeMutator) (Evaluator, error) { var modified []Evaluator for _, e := range a.anyOf { - i, err := e.MutateScopes(ctx, modifiers...) + i, err := e.MutateScopes(ctx, mutate) if err != nil { return nil, err } diff --git a/pkg/services/accesscontrol/evaluator_test.go b/pkg/services/accesscontrol/evaluator_test.go index 230a4c9b823..82df04ffe9b 100644 --- a/pkg/services/accesscontrol/evaluator_test.go +++ b/pkg/services/accesscontrol/evaluator_test.go @@ -25,11 +25,11 @@ func TestPermission_Evaluate(t *testing.T) { }, }, { - desc: "should evaluate to true when allEvaluator required scopes matches", + desc: "should evaluate to true when at least one scope matches", expected: true, evaluator: EvalPermission("reports:read", "reports:1", "reports:2"), permissions: map[string][]string{ - "reports:read": {"reports:1", "reports:2"}, + "reports:read": {"reports:2"}, }, }, { @@ -41,11 +41,11 @@ func TestPermission_Evaluate(t *testing.T) { }, }, { - desc: "should evaluate to false when only one of required scopes exists", + desc: "should evaluate to false when no scopes matches", expected: false, evaluator: EvalPermission("reports:read", "reports:1", "reports:2"), permissions: map[string][]string{ - "reports:read": {"reports:1"}, + "reports:read": {"reports:9", "reports:10"}, }, }, } diff --git a/pkg/services/accesscontrol/mock/mock.go b/pkg/services/accesscontrol/mock/mock.go index 943ce5718fd..9e5b42f79ec 100644 --- a/pkg/services/accesscontrol/mock/mock.go +++ b/pkg/services/accesscontrol/mock/mock.go @@ -45,9 +45,9 @@ type Mock struct { DeclareFixedRolesFunc func(...accesscontrol.RoleRegistration) error GetUserBuiltInRolesFunc func(user *models.SignedInUser) []string RegisterFixedRolesFunc func() error - RegisterAttributeScopeResolverFunc func(string, accesscontrol.AttributeScopeResolveFunc) + RegisterScopeAttributeResolverFunc func(string, accesscontrol.ScopeAttributeResolver) - scopeResolver accesscontrol.ScopeResolver + scopeResolvers accesscontrol.ScopeResolvers } // Ensure the mock stays in line with the interface @@ -55,11 +55,11 @@ var _ fullAccessControl = New() func New() *Mock { mock := &Mock{ - Calls: Calls{}, - disabled: false, - permissions: []*accesscontrol.Permission{}, - builtInRoles: []string{}, - scopeResolver: accesscontrol.NewScopeResolver(), + Calls: Calls{}, + disabled: false, + permissions: []*accesscontrol.Permission{}, + builtInRoles: []string{}, + scopeResolvers: accesscontrol.NewScopeResolvers(), } return mock @@ -98,7 +98,7 @@ func (m *Mock) Evaluate(ctx context.Context, user *models.SignedInUser, evaluato return false, err } - attributeMutator := m.scopeResolver.GetResolveAttributeScopeMutator(user.OrgId) + attributeMutator := m.scopeResolvers.GetScopeAttributeMutator(user.OrgId) resolvedEvaluator, err := evaluator.MutateScopes(ctx, attributeMutator) if err != nil { return false, err @@ -178,13 +178,11 @@ func (m *Mock) RegisterFixedRoles(ctx context.Context) error { return nil } -// RegisterAttributeScopeResolver allows the caller to register a scope resolver for a -// specific scope prefix (ex: datasources:name:) -func (m *Mock) RegisterAttributeScopeResolver(scopePrefix string, resolver accesscontrol.AttributeScopeResolveFunc) { - m.scopeResolver.AddAttributeResolver(scopePrefix, resolver) +func (m *Mock) RegisterScopeAttributeResolver(scopePrefix string, resolver accesscontrol.ScopeAttributeResolver) { + m.scopeResolvers.AddScopeAttributeResolver(scopePrefix, resolver) m.Calls.RegisterAttributeScopeResolver = append(m.Calls.RegisterAttributeScopeResolver, []struct{}{}) // Use override if provided - if m.RegisterAttributeScopeResolverFunc != nil { - m.RegisterAttributeScopeResolverFunc(scopePrefix, resolver) + if m.RegisterScopeAttributeResolverFunc != nil { + m.RegisterScopeAttributeResolverFunc(scopePrefix, resolver) } } diff --git a/pkg/services/accesscontrol/ossaccesscontrol/ossaccesscontrol.go b/pkg/services/accesscontrol/ossaccesscontrol/ossaccesscontrol.go index a8dd1378bdd..50df46e21f3 100644 --- a/pkg/services/accesscontrol/ossaccesscontrol/ossaccesscontrol.go +++ b/pkg/services/accesscontrol/ossaccesscontrol/ossaccesscontrol.go @@ -33,11 +33,11 @@ func ProvideService(features featuremgmt.FeatureToggles, func ProvideOSSAccessControl(features featuremgmt.FeatureToggles, provider accesscontrol.PermissionsProvider) *OSSAccessControlService { s := &OSSAccessControlService{ - features: features, - provider: provider, - log: log.New("accesscontrol"), - scopeResolver: accesscontrol.NewScopeResolver(), - roles: accesscontrol.BuildMacroRoleDefinitions(), + features: features, + provider: provider, + log: log.New("accesscontrol"), + scopeResolvers: accesscontrol.NewScopeResolvers(), + roles: accesscontrol.BuildMacroRoleDefinitions(), } return s @@ -45,12 +45,12 @@ func ProvideOSSAccessControl(features featuremgmt.FeatureToggles, provider acces // OSSAccessControlService is the service implementing role based access control. type OSSAccessControlService struct { - log log.Logger - features featuremgmt.FeatureToggles - scopeResolver accesscontrol.ScopeResolver - provider accesscontrol.PermissionsProvider - registrations accesscontrol.RegistrationList - roles map[string]*accesscontrol.RoleDTO + log log.Logger + features featuremgmt.FeatureToggles + scopeResolvers accesscontrol.ScopeResolvers + provider accesscontrol.PermissionsProvider + registrations accesscontrol.RegistrationList + roles map[string]*accesscontrol.RoleDTO } func (ac *OSSAccessControlService) IsDisabled() bool { @@ -92,7 +92,7 @@ func (ac *OSSAccessControlService) Evaluate(ctx context.Context, user *models.Si user.Permissions[user.OrgId] = accesscontrol.GroupScopesByAction(permissions) } - attributeMutator := ac.scopeResolver.GetResolveAttributeScopeMutator(user.OrgId) + attributeMutator := ac.scopeResolvers.GetScopeAttributeMutator(user.OrgId) resolvedEvaluator, err := evaluator.MutateScopes(ctx, attributeMutator) if err != nil { return false, err @@ -124,7 +124,7 @@ func (ac *OSSAccessControlService) GetUserPermissions(ctx context.Context, user permissions = append(permissions, dbPermissions...) resolved := make([]*accesscontrol.Permission, 0, len(permissions)) - keywordMutator := ac.scopeResolver.GetResolveKeywordScopeMutator(user) + keywordMutator := ac.scopeResolvers.GetScopeKeywordMutator(user) for _, p := range permissions { // if the permission has a keyword in its scope it will be resolved p.Scope, err = keywordMutator(ctx, p.Scope) @@ -217,8 +217,8 @@ func (ac *OSSAccessControlService) DeclareFixedRoles(registrations ...accesscont return nil } -// RegisterAttributeScopeResolver allows the caller to register scope resolvers for a +// RegisterScopeAttributeResolver allows the caller to register scope resolvers for a // specific scope prefix (ex: datasources:name:) -func (ac *OSSAccessControlService) RegisterAttributeScopeResolver(scopePrefix string, resolver accesscontrol.AttributeScopeResolveFunc) { - ac.scopeResolver.AddAttributeResolver(scopePrefix, resolver) +func (ac *OSSAccessControlService) RegisterScopeAttributeResolver(scopePrefix string, resolver accesscontrol.ScopeAttributeResolver) { + ac.scopeResolvers.AddScopeAttributeResolver(scopePrefix, resolver) } diff --git a/pkg/services/accesscontrol/ossaccesscontrol/ossaccesscontrol_test.go b/pkg/services/accesscontrol/ossaccesscontrol/ossaccesscontrol_test.go index 95177744710..3fb25138b60 100644 --- a/pkg/services/accesscontrol/ossaccesscontrol/ossaccesscontrol_test.go +++ b/pkg/services/accesscontrol/ossaccesscontrol/ossaccesscontrol_test.go @@ -20,12 +20,12 @@ func setupTestEnv(t testing.TB) *OSSAccessControlService { t.Helper() ac := &OSSAccessControlService{ - features: featuremgmt.WithFeatures(featuremgmt.FlagAccesscontrol), - log: log.New("accesscontrol"), - registrations: accesscontrol.RegistrationList{}, - scopeResolver: accesscontrol.NewScopeResolver(), - provider: database.ProvideService(sqlstore.InitTestDB(t)), - roles: accesscontrol.BuildMacroRoleDefinitions(), + features: featuremgmt.WithFeatures(featuremgmt.FlagAccesscontrol), + log: log.New("accesscontrol"), + registrations: accesscontrol.RegistrationList{}, + scopeResolvers: accesscontrol.NewScopeResolvers(), + provider: database.ProvideService(sqlstore.InitTestDB(t)), + roles: accesscontrol.BuildMacroRoleDefinitions(), } require.NoError(t, ac.RegisterFixedRoles(context.Background())) return ac @@ -439,12 +439,12 @@ func TestOSSAccessControlService_Evaluate(t *testing.T) { }, Grants: []string{"Viewer"}, } - userLoginScopeSolver := func(ctx context.Context, orgID int64, initialScope string) (string, error) { + userLoginScopeSolver := accesscontrol.ScopeAttributeResolverFunc(func(ctx context.Context, orgID int64, initialScope string) ([]string, error) { if initialScope == "users:login:testUser" { - return "users:id:2", nil + return []string{"users:id:2"}, nil } - return initialScope, nil - } + return []string{initialScope}, nil + }) tests := []struct { name string @@ -475,7 +475,7 @@ func TestOSSAccessControlService_Evaluate(t *testing.T) { t.Run(tt.name, func(t *testing.T) { // Setup ac := setupTestEnv(t) - ac.RegisterAttributeScopeResolver("users:login:", userLoginScopeSolver) + ac.RegisterScopeAttributeResolver("users:login:", userLoginScopeSolver) registration.Role.Permissions = []accesscontrol.Permission{tt.rawPerm} err := ac.DeclareFixedRoles(registration) diff --git a/pkg/services/accesscontrol/resolvers.go b/pkg/services/accesscontrol/resolvers.go new file mode 100644 index 00000000000..b59fc840969 --- /dev/null +++ b/pkg/services/accesscontrol/resolvers.go @@ -0,0 +1,140 @@ +package accesscontrol + +import ( + "bytes" + "context" + "fmt" + "text/template" + "time" + + "github.com/grafana/grafana/pkg/infra/localcache" + "github.com/grafana/grafana/pkg/infra/log" + "github.com/grafana/grafana/pkg/models" +) + +const ( + ttl = 30 * time.Second + cleanInterval = 2 * time.Minute +) + +func NewScopeResolvers() ScopeResolvers { + return ScopeResolvers{ + keywordResolvers: map[string]ScopeKeywordResolver{ + "users:self": userSelfResolver, + }, + attributeResolvers: map[string]ScopeAttributeResolver{}, + cache: localcache.New(ttl, cleanInterval), + log: log.New("accesscontrol.resolver"), + } +} + +type ScopeResolvers struct { + log log.Logger + cache *localcache.CacheService + keywordResolvers map[string]ScopeKeywordResolver + attributeResolvers map[string]ScopeAttributeResolver +} + +func (s *ScopeResolvers) GetScopeAttributeMutator(orgID int64) ScopeAttributeMutator { + return func(ctx context.Context, scope string) ([]string, error) { + key := getScopeCacheKey(orgID, scope) + // Check cache before computing the scope + if cachedScope, ok := s.cache.Get(key); ok { + scopes := cachedScope.([]string) + s.log.Debug("used cache to resolve '%v' to '%v'", scope, scopes) + return scopes, nil + } + + prefix := ScopePrefix(scope) + if resolver, ok := s.attributeResolvers[prefix]; ok { + scopes, err := resolver.Resolve(ctx, orgID, scope) + if err != nil { + return nil, fmt.Errorf("could not resolve %v: %w", scope, err) + } + // Cache result + s.cache.Set(key, scopes, ttl) + s.log.Debug("resolved '%v' to '%v'", scope, scopes) + return scopes, nil + } + return []string{scope}, nil + } +} + +func (s *ScopeResolvers) GetScopeKeywordMutator(user *models.SignedInUser) ScopeKeywordMutator { + return func(ctx context.Context, scope string) (string, error) { + if resolver, ok := s.keywordResolvers[scope]; ok { + scopes, err := resolver.Resolve(ctx, user) + if err != nil { + return "", fmt.Errorf("could not resolve %v: %w", scope, err) + } + s.log.Debug("resolved '%v' to '%v'", scope, scopes) + return scopes, nil + } + // By default, the scope remains unchanged + return scope, nil + } +} + +func (s *ScopeResolvers) AddScopeKeywordResolver(keyword string, resolver ScopeKeywordResolver) { + s.log.Debug("adding scope keyword resolver for '%v'", keyword) + s.keywordResolvers[keyword] = resolver +} + +func (s *ScopeResolvers) AddScopeAttributeResolver(prefix string, resolver ScopeAttributeResolver) { + s.log.Debug("adding scope attribute resolver for '%v'", prefix) + s.attributeResolvers[prefix] = resolver +} + +// ScopeAttributeResolver is used to resolve attributes in scopes to one or more scopes that are +// evaluated by logical or. E.g. "dashboards:id:1" -> "dashboards:uid:test-dashboard" or "folder:uid:test-folder" +type ScopeAttributeResolver interface { + Resolve(ctx context.Context, orgID int64, scope string) ([]string, error) +} + +// ScopeAttributeResolverFunc is an adapter to allow functions to implement ScopeAttributeResolver interface +type ScopeAttributeResolverFunc func(ctx context.Context, orgID int64, scope string) ([]string, error) + +func (f ScopeAttributeResolverFunc) Resolve(ctx context.Context, orgID int64, scope string) ([]string, error) { + return f(ctx, orgID, scope) +} + +type ScopeAttributeMutator func(context.Context, string) ([]string, error) + +// ScopeKeywordResolver is used to resolve keywords in scopes e.g. "users:self" -> "user:id:1". +// These type of resolvers is used when fetching stored permissions +type ScopeKeywordResolver interface { + Resolve(ctx context.Context, user *models.SignedInUser) (string, error) +} + +// ScopeKeywordResolverFunc is an adapter to allow functions to implement ScopeKeywordResolver interface +type ScopeKeywordResolverFunc func(ctx context.Context, user *models.SignedInUser) (string, error) + +func (f ScopeKeywordResolverFunc) Resolve(ctx context.Context, user *models.SignedInUser) (string, error) { + return f(ctx, user) +} + +type ScopeKeywordMutator func(context.Context, string) (string, error) + +// getScopeCacheKey creates an identifier to fetch and store resolution of scopes in the cache +func getScopeCacheKey(orgID int64, scope string) string { + return fmt.Sprintf("%s-%v", scope, orgID) +} + +//ScopeInjector inject request params into the templated scopes. e.g. "settings:" + eval.Parameters(":id") +func ScopeInjector(params ScopeParams) ScopeAttributeMutator { + return func(_ context.Context, scope string) ([]string, error) { + tmpl, err := template.New("scope").Parse(scope) + if err != nil { + return nil, err + } + var buf bytes.Buffer + if err = tmpl.Execute(&buf, params); err != nil { + return nil, err + } + return []string{buf.String()}, nil + } +} + +var userSelfResolver = ScopeKeywordResolverFunc(func(ctx context.Context, user *models.SignedInUser) (string, error) { + return Scope("users", "id", fmt.Sprintf("%v", user.UserId)), nil +}) diff --git a/pkg/services/accesscontrol/resolvers_test.go b/pkg/services/accesscontrol/resolvers_test.go new file mode 100644 index 00000000000..d7f88be7e07 --- /dev/null +++ b/pkg/services/accesscontrol/resolvers_test.go @@ -0,0 +1,159 @@ +package accesscontrol + +import ( + "context" + "testing" + + "github.com/grafana/grafana/pkg/models" + "github.com/stretchr/testify/assert" +) + +func TestResolveKeywordScope(t *testing.T) { + tests := []struct { + name string + user *models.SignedInUser + permission Permission + want Permission + wantErr bool + }{ + { + name: "no scope", + user: testUser, + permission: Permission{Action: "users:read"}, + want: Permission{Action: "users:read"}, + wantErr: false, + }, + { + name: "user if resolution", + user: testUser, + permission: Permission{Action: "users:read", Scope: "users:self"}, + want: Permission{Action: "users:read", Scope: "users:id:2"}, + wantErr: false, + }, + } + for _, tt := range tests { + var err error + t.Run(tt.name, func(t *testing.T) { + resolvers := NewScopeResolvers() + scopeModifier := resolvers.GetScopeKeywordMutator(tt.user) + tt.permission.Scope, err = scopeModifier(context.TODO(), tt.permission.Scope) + if tt.wantErr { + assert.Error(t, err, "expected an error during the resolution of the scope") + return + } + assert.NoError(t, err) + assert.EqualValues(t, tt.want, tt.permission, "permission did not match expected resolution") + }) + } +} + +var testUser = &models.SignedInUser{ + UserId: 2, + OrgId: 3, + OrgName: "TestOrg", + OrgRole: models.ROLE_VIEWER, + Login: "testUser", + Name: "Test User", + Email: "testuser@example.org", +} + +func TestResolveAttributeScope(t *testing.T) { + // Calls allow us to see how many times the fakeDataSourceResolution has been called + calls := 0 + fakeDataSourceResolver := ScopeAttributeResolverFunc(func(ctx context.Context, orgID int64, initialScope string) ([]string, error) { + calls++ + if initialScope == "datasources:name:testds" { + return []string{Scope("datasources", "id", "1")}, nil + } else if initialScope == "datasources:name:testds2" { + return []string{Scope("datasources", "id", "2")}, nil + } else if initialScope == "datasources:name:test:ds4" { + return []string{Scope("datasources", "id", "4")}, nil + } else if initialScope == "datasources:name:testds5*" { + return []string{Scope("datasources", "id", "5")}, nil + } else { + return nil, models.ErrDataSourceNotFound + } + }) + + tests := []struct { + name string + orgID int64 + evaluator Evaluator + wantEvaluator Evaluator + wantCalls int + wantErr error + }{ + { + name: "should work with scope less permissions", + evaluator: EvalPermission("datasources:read"), + wantEvaluator: EvalPermission("datasources:read"), + wantCalls: 0, + }, + { + name: "should handle an error", + orgID: 1, + evaluator: EvalPermission("datasources:read", Scope("datasources", "name", "testds3")), + wantErr: models.ErrDataSourceNotFound, + wantCalls: 1, + }, + { + name: "should resolve a scope", + orgID: 1, + evaluator: EvalPermission("datasources:read", Scope("datasources", "name", "testds")), + wantEvaluator: EvalPermission("datasources:read", Scope("datasources", "id", "1")), + wantCalls: 1, + }, + { + name: "should resolve nested scopes with cache", + orgID: 1, + evaluator: EvalAll( + EvalPermission("datasources:read", Scope("datasources", "name", "testds")), + EvalAny( + EvalPermission("datasources:read", Scope("datasources", "name", "testds")), + EvalPermission("datasources:read", Scope("datasources", "name", "testds2")), + ), + ), + wantEvaluator: EvalAll( + EvalPermission("datasources:read", Scope("datasources", "id", "1")), + EvalAny( + EvalPermission("datasources:read", Scope("datasources", "id", "1")), + EvalPermission("datasources:read", Scope("datasources", "id", "2")), + ), + ), + wantCalls: 2, + }, + { + name: "should resolve name with colon", + orgID: 1, + evaluator: EvalPermission("datasources:read", Scope("datasources", "name", "test:ds4")), + wantEvaluator: EvalPermission("datasources:read", Scope("datasources", "id", "4")), + wantCalls: 1, + }, + { + name: "should resolve names with '*'", + orgID: 1, + evaluator: EvalPermission("datasources:read", Scope("datasources", "name", "testds5*")), + wantEvaluator: EvalPermission("datasources:read", Scope("datasources", "id", "5")), + wantCalls: 1, + }, + } + for _, tt := range tests { + resolvers := NewScopeResolvers() + + // Reset calls counter + calls = 0 + // Register a resolution method + resolvers.AddScopeAttributeResolver("datasources:name:", fakeDataSourceResolver) + + // Test + mutate := resolvers.GetScopeAttributeMutator(tt.orgID) + resolvedEvaluator, err := tt.evaluator.MutateScopes(context.Background(), mutate) + if tt.wantErr != nil { + assert.ErrorAs(t, err, &tt.wantErr, "expected an error during the resolution of the scope") + return + } + assert.NoError(t, err) + assert.EqualValues(t, tt.wantEvaluator, resolvedEvaluator, "permission did not match expected resolution") + assert.Equal(t, tt.wantCalls, calls, "cache has not been used") + } +} diff --git a/pkg/services/accesscontrol/scope.go b/pkg/services/accesscontrol/scope.go index 3d58e60e0f7..a9ec8cbae60 100644 --- a/pkg/services/accesscontrol/scope.go +++ b/pkg/services/accesscontrol/scope.go @@ -1,21 +1,11 @@ package accesscontrol import ( - "bytes" - "context" "fmt" - "html/template" "strings" - "time" - - "github.com/grafana/grafana/pkg/infra/localcache" - "github.com/grafana/grafana/pkg/infra/log" - "github.com/grafana/grafana/pkg/models" ) const ( - ttl = 30 * time.Second - cleanInterval = 2 * time.Minute maxPrefixParts = 2 ) @@ -68,95 +58,6 @@ func Field(key string) string { return fmt.Sprintf(`{{ .%s }}`, key) } -// ScopeMutator alters a Scope to return a new modified Scope -type ScopeMutator func(context.Context, string) (string, error) - -type KeywordScopeResolveFunc func(*models.SignedInUser) (string, error) - -// ScopeResolver is used to resolve scope keywords such as `self` or `current` into `id` based scopes and scope attributes such as `name` or `uid` into `id` based scopes. -type ScopeResolver struct { - keywordResolvers map[string]KeywordScopeResolveFunc - attributeResolvers map[string]AttributeScopeResolveFunc - cache *localcache.CacheService - log log.Logger -} - -func NewScopeResolver() ScopeResolver { - return ScopeResolver{ - keywordResolvers: map[string]KeywordScopeResolveFunc{ - "users:self": resolveUserSelf, - }, - attributeResolvers: map[string]AttributeScopeResolveFunc{}, - cache: localcache.New(ttl, cleanInterval), - log: log.New("accesscontrol.scoperesolution"), - } -} - -func (s *ScopeResolver) AddKeywordResolver(keyword string, fn KeywordScopeResolveFunc) { - s.log.Debug("adding keyword resolution for '%v'", keyword) - s.keywordResolvers[keyword] = fn -} - -func (s *ScopeResolver) AddAttributeResolver(prefix string, fn AttributeScopeResolveFunc) { - s.log.Debug("adding attribute resolution for '%v'", prefix) - s.attributeResolvers[prefix] = fn -} - -func resolveUserSelf(u *models.SignedInUser) (string, error) { - return Scope("users", "id", fmt.Sprintf("%v", u.UserId)), nil -} - -// GetResolveKeywordScopeMutator returns a function to resolve scope with keywords such as `self` or `current` into `id` based scopes -func (s *ScopeResolver) GetResolveKeywordScopeMutator(user *models.SignedInUser) ScopeMutator { - return func(_ context.Context, scope string) (string, error) { - var err error - // By default the scope remains unchanged - resolvedScope := scope - if fn, ok := s.keywordResolvers[scope]; ok { - resolvedScope, err = fn(user) - if err != nil { - return "", fmt.Errorf("could not resolve %v: %w", scope, err) - } - s.log.Debug("resolved '%v' to '%v'", scope, resolvedScope) - } - return resolvedScope, nil - } -} - -type AttributeScopeResolveFunc func(ctx context.Context, orgID int64, initialScope string) (string, error) - -// getCacheKey creates an identifier to fetch and store resolution of scopes in the cache -func getCacheKey(orgID int64, scope string) string { - return fmt.Sprintf("%s-%v", scope, orgID) -} - -// GetResolveAttributeScopeMutator returns a function to resolve scopes with attributes such as `name` or `uid` into `id` based scopes -func (s *ScopeResolver) GetResolveAttributeScopeMutator(orgID int64) ScopeMutator { - return func(ctx context.Context, scope string) (string, error) { - // Check cache before computing the scope - if cachedScope, ok := s.cache.Get(getCacheKey(orgID, scope)); ok { - resolvedScope := cachedScope.(string) - s.log.Debug("used cache to resolve '%v' to '%v'", scope, resolvedScope) - return resolvedScope, nil - } - - var err error - // By default the scope remains unchanged - resolvedScope := scope - prefix := ScopePrefix(scope) - if fn, ok := s.attributeResolvers[prefix]; ok { - resolvedScope, err = fn(ctx, orgID, scope) - if err != nil { - return "", fmt.Errorf("could not resolve %v: %w", scope, err) - } - // Cache result - s.cache.Set(getCacheKey(orgID, scope), resolvedScope, ttl) - s.log.Debug("resolved '%v' to '%v'", scope, resolvedScope) - } - return resolvedScope, nil - } -} - // ScopePrefix returns the prefix associated to a given scope // we assume prefixes are all in the form :: // ex: "datasources:name:test" returns "datasources:name:" @@ -169,21 +70,6 @@ func ScopePrefix(scope string) string { return strings.Join(parts, ":") } -//Inject params into the evaluator's templated scopes. e.g. "settings:" + eval.Parameters(":id") -func ScopeInjector(params ScopeParams) ScopeMutator { - return func(_ context.Context, scope string) (string, error) { - tmpl, err := template.New("scope").Parse(scope) - if err != nil { - return "", err - } - var buf bytes.Buffer - if err = tmpl.Execute(&buf, params); err != nil { - return "", err - } - return buf.String(), nil - } -} - // ScopeProvider provides methods that construct scopes type ScopeProvider interface { GetResourceScope(resourceID string) string diff --git a/pkg/services/accesscontrol/scope_test.go b/pkg/services/accesscontrol/scope_test.go index 666d3c73862..f0ccae5f63b 100644 --- a/pkg/services/accesscontrol/scope_test.go +++ b/pkg/services/accesscontrol/scope_test.go @@ -1,165 +1,12 @@ package accesscontrol import ( - "context" "testing" - "github.com/grafana/grafana/pkg/models" "github.com/stretchr/testify/assert" ) -var testUser = &models.SignedInUser{ - UserId: 2, - OrgId: 3, - OrgName: "TestOrg", - OrgRole: models.ROLE_VIEWER, - Login: "testUser", - Name: "Test User", - Email: "testuser@example.org", -} - -func TestResolveKeywordedScope(t *testing.T) { - tests := []struct { - name string - user *models.SignedInUser - permission Permission - want Permission - wantErr bool - }{ - { - name: "no scope", - user: testUser, - permission: Permission{Action: "users:read"}, - want: Permission{Action: "users:read"}, - wantErr: false, - }, - { - name: "user if resolution", - user: testUser, - permission: Permission{Action: "users:read", Scope: "users:self"}, - want: Permission{Action: "users:read", Scope: "users:id:2"}, - wantErr: false, - }, - } - for _, tt := range tests { - var err error - t.Run(tt.name, func(t *testing.T) { - resolver := NewScopeResolver() - scopeModifier := resolver.GetResolveKeywordScopeMutator(tt.user) - tt.permission.Scope, err = scopeModifier(context.TODO(), tt.permission.Scope) - if tt.wantErr { - assert.Error(t, err, "expected an error during the resolution of the scope") - return - } - assert.NoError(t, err) - assert.EqualValues(t, tt.want, tt.permission, "permission did not match expected resolution") - }) - } -} - -func TestScopeResolver_ResolveAttribute(t *testing.T) { - // Calls allow us to see how many times the fakeDataSourceResolution has been called - calls := 0 - fakeDataSourceResolution := func(ctx context.Context, orgID int64, initialScope string) (string, error) { - calls++ - if initialScope == "datasources:name:testds" { - return Scope("datasources", "id", "1"), nil - } else if initialScope == "datasources:name:testds2" { - return Scope("datasources", "id", "2"), nil - } else if initialScope == "datasources:name:test:ds4" { - return Scope("datasources", "id", "4"), nil - } else if initialScope == "datasources:name:testds5*" { - return Scope("datasources", "id", "5"), nil - } else { - return "", models.ErrDataSourceNotFound - } - } - - tests := []struct { - name string - orgID int64 - evaluator Evaluator - wantEvaluator Evaluator - wantCalls int - wantErr error - }{ - { - name: "should work with scope less permissions", - evaluator: EvalPermission("datasources:read"), - wantEvaluator: EvalPermission("datasources:read"), - wantCalls: 0, - }, - { - name: "should handle an error", - orgID: 1, - evaluator: EvalPermission("datasources:read", Scope("datasources", "name", "testds3")), - wantErr: models.ErrDataSourceNotFound, - wantCalls: 1, - }, - { - name: "should resolve a scope", - orgID: 1, - evaluator: EvalPermission("datasources:read", Scope("datasources", "name", "testds")), - wantEvaluator: EvalPermission("datasources:read", Scope("datasources", "id", "1")), - wantCalls: 1, - }, - { - name: "should resolve nested scopes with cache", - orgID: 1, - evaluator: EvalAll( - EvalPermission("datasources:read", Scope("datasources", "name", "testds")), - EvalAny( - EvalPermission("datasources:read", Scope("datasources", "name", "testds")), - EvalPermission("datasources:read", Scope("datasources", "name", "testds2")), - ), - ), - wantEvaluator: EvalAll( - EvalPermission("datasources:read", Scope("datasources", "id", "1")), - EvalAny( - EvalPermission("datasources:read", Scope("datasources", "id", "1")), - EvalPermission("datasources:read", Scope("datasources", "id", "2")), - ), - ), - wantCalls: 2, - }, - { - name: "should resolve name with colon", - orgID: 1, - evaluator: EvalPermission("datasources:read", Scope("datasources", "name", "test:ds4")), - wantEvaluator: EvalPermission("datasources:read", Scope("datasources", "id", "4")), - wantCalls: 1, - }, - { - name: "should resolve names with '*'", - orgID: 1, - evaluator: EvalPermission("datasources:read", Scope("datasources", "name", "testds5*")), - wantEvaluator: EvalPermission("datasources:read", Scope("datasources", "id", "5")), - wantCalls: 1, - }, - } - for _, tt := range tests { - resolver := NewScopeResolver() - - // Reset calls counter - calls = 0 - // Register a resolution method - resolver.AddAttributeResolver("datasources:name:", fakeDataSourceResolution) - - // Test - scopeModifier := resolver.GetResolveAttributeScopeMutator(tt.orgID) - resolvedEvaluator, err := tt.evaluator.MutateScopes(context.TODO(), scopeModifier) - if tt.wantErr != nil { - assert.ErrorAs(t, err, &tt.wantErr, "expected an error during the resolution of the scope") - return - } - assert.NoError(t, err) - assert.EqualValues(t, tt.wantEvaluator, resolvedEvaluator, "permission did not match expected resolution") - - assert.Equal(t, tt.wantCalls, calls, "cache has not been used") - } -} - -func Test_scopePrefix(t *testing.T) { +func Test_ScopePrefix(t *testing.T) { tests := []struct { name string scope string diff --git a/pkg/services/dashboards/accesscontrol.go b/pkg/services/dashboards/accesscontrol.go index d362fc270f9..aa6f1db6971 100644 --- a/pkg/services/dashboards/accesscontrol.go +++ b/pkg/services/dashboards/accesscontrol.go @@ -28,49 +28,47 @@ var ( ScopeFoldersProvider = ac.NewScopeProvider(ScopeFoldersRoot) ) -// NewNameScopeResolver provides an AttributeScopeResolver that is able to convert a scope prefixed with "folders:name:" into an uid based scope. -func NewNameScopeResolver(db Store) (string, ac.AttributeScopeResolveFunc) { +// NewFolderNameScopeResolver provides an ScopeAttributeResolver that is able to convert a scope prefixed with "folders:name:" into an uid based scope. +func NewFolderNameScopeResolver(db Store) (string, ac.ScopeAttributeResolver) { prefix := ScopeFoldersProvider.GetResourceScopeName("") - resolver := func(ctx context.Context, orgID int64, scope string) (string, error) { + return prefix, ac.ScopeAttributeResolverFunc(func(ctx context.Context, orgID int64, scope string) ([]string, error) { if !strings.HasPrefix(scope, prefix) { - return "", ac.ErrInvalidScope + return nil, ac.ErrInvalidScope } nsName := scope[len(prefix):] if len(nsName) == 0 { - return "", ac.ErrInvalidScope + return nil, ac.ErrInvalidScope } folder, err := db.GetFolderByTitle(ctx, orgID, nsName) if err != nil { - return "", err + return nil, err } - return ScopeFoldersProvider.GetResourceScopeUID(folder.Uid), nil - } - return prefix, resolver + return []string{ScopeFoldersProvider.GetResourceScopeUID(folder.Uid)}, nil + }) } -// NewIDScopeResolver provides an AttributeScopeResolver that is able to convert a scope prefixed with "folders:id:" into an uid based scope. -func NewIDScopeResolver(db Store) (string, ac.AttributeScopeResolveFunc) { +// NewFolderIDScopeResolver provides an ScopeAttributeResolver that is able to convert a scope prefixed with "folders:id:" into an uid based scope. +func NewFolderIDScopeResolver(db Store) (string, ac.ScopeAttributeResolver) { prefix := ScopeFoldersProvider.GetResourceScope("") - resolver := func(ctx context.Context, orgID int64, scope string) (string, error) { + return prefix, ac.ScopeAttributeResolverFunc(func(ctx context.Context, orgID int64, scope string) ([]string, error) { if !strings.HasPrefix(scope, prefix) { - return "", ac.ErrInvalidScope + return nil, ac.ErrInvalidScope } id, err := strconv.ParseInt(scope[len(prefix):], 10, 64) if err != nil { - return "", ac.ErrInvalidScope + return nil, ac.ErrInvalidScope } if id == 0 { - return ScopeFoldersProvider.GetResourceScopeUID(ac.GeneralFolderUID), nil + return []string{ScopeFoldersProvider.GetResourceScopeUID(ac.GeneralFolderUID)}, nil } folder, err := db.GetFolderByID(ctx, orgID, id) if err != nil { - return "", err + return nil, err } - return ScopeFoldersProvider.GetResourceScopeUID(folder.Uid), nil - } - return prefix, resolver + return []string{ScopeFoldersProvider.GetResourceScopeUID(folder.Uid)}, nil + }) } diff --git a/pkg/services/dashboards/accesscontrol_test.go b/pkg/services/dashboards/accesscontrol_test.go index 04816092bed..db5743ddedb 100644 --- a/pkg/services/dashboards/accesscontrol_test.go +++ b/pkg/services/dashboards/accesscontrol_test.go @@ -15,16 +15,16 @@ import ( "github.com/grafana/grafana/pkg/util" ) -func TestNewNameScopeResolver(t *testing.T) { +func TestNewFolderNameScopeResolver(t *testing.T) { t.Run("prefix should be expected", func(t *testing.T) { - prefix, _ := NewNameScopeResolver(&FakeDashboardStore{}) + prefix, _ := NewFolderNameScopeResolver(&FakeDashboardStore{}) require.Equal(t, "folders:name:", prefix) }) t.Run("resolver should convert to uid scope", func(t *testing.T) { dashboardStore := &FakeDashboardStore{} - _, resolver := NewNameScopeResolver(dashboardStore) + _, resolver := NewFolderNameScopeResolver(dashboardStore) orgId := rand.Int63() title := "Very complex :title with: and /" + util.GenerateShortUID() @@ -36,53 +36,54 @@ func TestNewNameScopeResolver(t *testing.T) { scope := "folders:name:" + title - resolvedScope, err := resolver(context.Background(), orgId, scope) + resolvedScopes, err := resolver.Resolve(context.Background(), orgId, scope) require.NoError(t, err) + require.Len(t, resolvedScopes, 1) - require.Equal(t, fmt.Sprintf("folders:uid:%v", db.Uid), resolvedScope) + require.Equal(t, fmt.Sprintf("folders:uid:%v", db.Uid), resolvedScopes[0]) dashboardStore.AssertCalled(t, "GetFolderByTitle", mock.Anything, orgId, title) }) t.Run("resolver should fail if input scope is not expected", func(t *testing.T) { dashboardStore := &FakeDashboardStore{} - _, resolver := NewNameScopeResolver(dashboardStore) + _, resolver := NewFolderNameScopeResolver(dashboardStore) - _, err := resolver(context.Background(), rand.Int63(), "folders:id:123") + _, err := resolver.Resolve(context.Background(), rand.Int63(), "folders:id:123") require.ErrorIs(t, err, ac.ErrInvalidScope) }) t.Run("resolver should fail if resource of input scope is empty", func(t *testing.T) { dashboardStore := &FakeDashboardStore{} - _, resolver := NewNameScopeResolver(dashboardStore) + _, resolver := NewFolderNameScopeResolver(dashboardStore) - _, err := resolver(context.Background(), rand.Int63(), "folders:name:") + _, err := resolver.Resolve(context.Background(), rand.Int63(), "folders:name:") require.ErrorIs(t, err, ac.ErrInvalidScope) }) t.Run("returns 'not found' if folder does not exist", func(t *testing.T) { dashboardStore := &FakeDashboardStore{} - _, resolver := NewNameScopeResolver(dashboardStore) + _, resolver := NewFolderNameScopeResolver(dashboardStore) orgId := rand.Int63() dashboardStore.On("GetFolderByTitle", mock.Anything, mock.Anything, mock.Anything).Return(nil, models.ErrDashboardNotFound).Once() scope := "folders:name:" + util.GenerateShortUID() - resolvedScope, err := resolver(context.Background(), orgId, scope) + resolvedScopes, err := resolver.Resolve(context.Background(), orgId, scope) require.ErrorIs(t, err, models.ErrDashboardNotFound) - require.Empty(t, resolvedScope) + require.Nil(t, resolvedScopes) }) } -func TestNewIDScopeResolver(t *testing.T) { +func TestNewFolderIDScopeResolver(t *testing.T) { t.Run("prefix should be expected", func(t *testing.T) { - prefix, _ := NewIDScopeResolver(&FakeDashboardStore{}) + prefix, _ := NewFolderIDScopeResolver(&FakeDashboardStore{}) require.Equal(t, "folders:id:", prefix) }) t.Run("resolver should convert to uid scope", func(t *testing.T) { dashboardStore := &FakeDashboardStore{} - _, resolver := NewIDScopeResolver(dashboardStore) + _, resolver := NewFolderIDScopeResolver(dashboardStore) orgId := rand.Int63() uid := util.GenerateShortUID() @@ -92,18 +93,18 @@ func TestNewIDScopeResolver(t *testing.T) { scope := "folders:id:" + strconv.FormatInt(db.Id, 10) - resolvedScope, err := resolver(context.Background(), orgId, scope) + resolvedScopes, err := resolver.Resolve(context.Background(), orgId, scope) require.NoError(t, err) - - require.Equal(t, fmt.Sprintf("folders:uid:%v", db.Uid), resolvedScope) + require.Len(t, resolvedScopes, 1) + require.Equal(t, fmt.Sprintf("folders:uid:%v", db.Uid), resolvedScopes[0]) dashboardStore.AssertCalled(t, "GetFolderByID", mock.Anything, orgId, db.Id) }) t.Run("resolver should fail if input scope is not expected", func(t *testing.T) { dashboardStore := &FakeDashboardStore{} - _, resolver := NewIDScopeResolver(dashboardStore) + _, resolver := NewFolderIDScopeResolver(dashboardStore) - _, err := resolver(context.Background(), rand.Int63(), "folders:uid:123") + _, err := resolver.Resolve(context.Background(), rand.Int63(), "folders:uid:123") require.ErrorIs(t, err, ac.ErrInvalidScope) }) @@ -112,33 +113,34 @@ func TestNewIDScopeResolver(t *testing.T) { dashboardStore = &FakeDashboardStore{} orgId = rand.Int63() scope = "folders:id:0" - _, resolver = NewIDScopeResolver(dashboardStore) + _, resolver = NewFolderIDScopeResolver(dashboardStore) ) - resolvedScope, err := resolver(context.Background(), orgId, scope) + resolved, err := resolver.Resolve(context.Background(), orgId, scope) require.NoError(t, err) - require.Equal(t, "folders:uid:general", resolvedScope) + require.Len(t, resolved, 1) + require.Equal(t, "folders:uid:general", resolved[0]) }) t.Run("resolver should fail if resource of input scope is empty", func(t *testing.T) { dashboardStore := &FakeDashboardStore{} - _, resolver := NewIDScopeResolver(dashboardStore) + _, resolver := NewFolderIDScopeResolver(dashboardStore) - _, err := resolver(context.Background(), rand.Int63(), "folders:id:") + _, err := resolver.Resolve(context.Background(), rand.Int63(), "folders:id:") require.ErrorIs(t, err, ac.ErrInvalidScope) }) t.Run("returns 'not found' if folder does not exist", func(t *testing.T) { dashboardStore := &FakeDashboardStore{} - _, resolver := NewIDScopeResolver(dashboardStore) + _, resolver := NewFolderIDScopeResolver(dashboardStore) orgId := rand.Int63() dashboardStore.On("GetFolderByID", mock.Anything, mock.Anything, mock.Anything).Return(nil, models.ErrDashboardNotFound).Once() scope := "folders:id:10" - resolvedScope, err := resolver(context.Background(), orgId, scope) + resolvedScopes, err := resolver.Resolve(context.Background(), orgId, scope) require.ErrorIs(t, err, models.ErrDashboardNotFound) - require.Empty(t, resolvedScope) + require.Nil(t, resolvedScopes) }) } diff --git a/pkg/services/dashboards/manager/folder_service.go b/pkg/services/dashboards/manager/folder_service.go index 77b31cf69ad..d7c926133a4 100644 --- a/pkg/services/dashboards/manager/folder_service.go +++ b/pkg/services/dashboards/manager/folder_service.go @@ -32,8 +32,8 @@ func ProvideFolderService( searchService *search.SearchService, features featuremgmt.FeatureToggles, permissionsServices accesscontrol.PermissionsServices, ac accesscontrol.AccessControl, sqlStore sqlstore.Store, ) *FolderServiceImpl { - ac.RegisterAttributeScopeResolver(dashboards.NewNameScopeResolver(dashboardStore)) - ac.RegisterAttributeScopeResolver(dashboards.NewIDScopeResolver(dashboardStore)) + ac.RegisterScopeAttributeResolver(dashboards.NewFolderNameScopeResolver(dashboardStore)) + ac.RegisterScopeAttributeResolver(dashboards.NewFolderIDScopeResolver(dashboardStore)) return &FolderServiceImpl{ cfg: cfg, diff --git a/pkg/services/datasources/service/datasource_service.go b/pkg/services/datasources/service/datasource_service.go index 7f74a8bafa0..24c9ff6b47c 100644 --- a/pkg/services/datasources/service/datasource_service.go +++ b/pkg/services/datasources/service/datasource_service.go @@ -68,8 +68,8 @@ func ProvideService( ac: ac, } - ac.RegisterAttributeScopeResolver(NewNameScopeResolver(store)) - ac.RegisterAttributeScopeResolver(NewIDScopeResolver(store)) + ac.RegisterScopeAttributeResolver(NewNameScopeResolver(store)) + ac.RegisterScopeAttributeResolver(NewIDScopeResolver(store)) return s } @@ -82,55 +82,55 @@ type DataSourceRetriever interface { const secretType = "datasource" -// NewNameScopeResolver provides an AttributeScopeResolver able to +// NewNameScopeResolver provides an ScopeAttributeResolver able to // translate a scope prefixed with "datasources:name:" into an uid based scope. -func NewNameScopeResolver(db DataSourceRetriever) (string, accesscontrol.AttributeScopeResolveFunc) { +func NewNameScopeResolver(db DataSourceRetriever) (string, accesscontrol.ScopeAttributeResolver) { prefix := datasources.ScopeProvider.GetResourceScopeName("") - return prefix, func(ctx context.Context, orgID int64, initialScope string) (string, error) { + return prefix, accesscontrol.ScopeAttributeResolverFunc(func(ctx context.Context, orgID int64, initialScope string) ([]string, error) { if !strings.HasPrefix(initialScope, prefix) { - return "", accesscontrol.ErrInvalidScope + return nil, accesscontrol.ErrInvalidScope } dsName := initialScope[len(prefix):] if dsName == "" { - return "", accesscontrol.ErrInvalidScope + return nil, accesscontrol.ErrInvalidScope } query := models.GetDataSourceQuery{Name: dsName, OrgId: orgID} if err := db.GetDataSource(ctx, &query); err != nil { - return "", err + return nil, err } - return datasources.ScopeProvider.GetResourceScopeUID(query.Result.Uid), nil - } + return []string{datasources.ScopeProvider.GetResourceScopeUID(query.Result.Uid)}, nil + }) } -// NewIDScopeResolver provides an AttributeScopeResolver able to +// NewIDScopeResolver provides an ScopeAttributeResolver able to // translate a scope prefixed with "datasources:id:" into an uid based scope. -func NewIDScopeResolver(db DataSourceRetriever) (string, accesscontrol.AttributeScopeResolveFunc) { +func NewIDScopeResolver(db DataSourceRetriever) (string, accesscontrol.ScopeAttributeResolver) { prefix := datasources.ScopeProvider.GetResourceScope("") - return prefix, func(ctx context.Context, orgID int64, initialScope string) (string, error) { + return prefix, accesscontrol.ScopeAttributeResolverFunc(func(ctx context.Context, orgID int64, initialScope string) ([]string, error) { if !strings.HasPrefix(initialScope, prefix) { - return "", accesscontrol.ErrInvalidScope + return nil, accesscontrol.ErrInvalidScope } id := initialScope[len(prefix):] if id == "" { - return "", accesscontrol.ErrInvalidScope + return nil, accesscontrol.ErrInvalidScope } dsID, err := strconv.ParseInt(id, 10, 64) if err != nil { - return "", accesscontrol.ErrInvalidScope + return nil, accesscontrol.ErrInvalidScope } query := models.GetDataSourceQuery{Id: dsID, OrgId: orgID} if err := db.GetDataSource(ctx, &query); err != nil { - return "", err + return nil, err } - return datasources.ScopeProvider.GetResourceScopeUID(query.Result.Uid), nil - } + return []string{datasources.ScopeProvider.GetResourceScopeUID(query.Result.Uid)}, nil + }) } func (s *Service) GetDataSource(ctx context.Context, query *models.GetDataSourceQuery) error { diff --git a/pkg/services/datasources/service/datasource_service_test.go b/pkg/services/datasources/service/datasource_service_test.go index e250315fa73..120c6e54c7e 100644 --- a/pkg/services/datasources/service/datasource_service_test.go +++ b/pkg/services/datasources/service/datasource_service_test.go @@ -109,13 +109,14 @@ func TestService_NameScopeResolver(t *testing.T) { for _, tc := range testCases { t.Run(tc.desc, func(t *testing.T) { - resolved, err := resolver(context.Background(), 1, tc.given) + resolved, err := resolver.Resolve(context.Background(), 1, tc.given) if tc.wantErr != nil { require.Error(t, err) require.Equal(t, tc.wantErr, err) } else { require.NoError(t, err) - require.Equal(t, tc.want, resolved) + require.Len(t, resolved, 1) + require.Equal(t, tc.want, resolved[0]) } }) } @@ -164,13 +165,14 @@ func TestService_IDScopeResolver(t *testing.T) { for _, tc := range testCases { t.Run(tc.desc, func(t *testing.T) { - resolved, err := resolver(context.Background(), 1, tc.given) + resolved, err := resolver.Resolve(context.Background(), 1, tc.given) if tc.wantErr != nil { require.Error(t, err) require.Equal(t, tc.wantErr, err) } else { require.NoError(t, err) - require.Equal(t, tc.want, resolved) + require.Len(t, resolved, 1) + require.Equal(t, tc.want, resolved[0]) } }) }