From 5dd98a0fd5ee0e17ba5da94645259723d6c95f92 Mon Sep 17 00:00:00 2001 From: Karl Persson Date: Thu, 28 Mar 2024 10:08:07 +0100 Subject: [PATCH] RBAC: handle partially resolved scopes (#85323) * RBAC: handle partially resolved scopes Co-authored-by: Gabriel MABILLE --- pkg/services/accesscontrol/evaluator.go | 33 ++++++++++++++ pkg/services/accesscontrol/evaluator_test.go | 46 ++++++++++++++++++++ 2 files changed, 79 insertions(+) diff --git a/pkg/services/accesscontrol/evaluator.go b/pkg/services/accesscontrol/evaluator.go index c8421ebd17b..4b082e267d4 100644 --- a/pkg/services/accesscontrol/evaluator.go +++ b/pkg/services/accesscontrol/evaluator.go @@ -2,6 +2,7 @@ package accesscontrol import ( "context" + "errors" "fmt" "strings" @@ -84,14 +85,25 @@ func (p permissionEvaluator) MutateScopes(ctx context.Context, mutate ScopeAttri return EvalPermission(p.Action), nil } + resolved := false scopes := make([]string, 0, len(p.Scopes)) for _, scope := range p.Scopes { mutated, err := mutate(ctx, scope) if err != nil { + if errors.Is(err, ErrResolverNotFound) { + scopes = append(scopes, mutated...) + continue + } return nil, err } + resolved = true scopes = append(scopes, mutated...) } + + if !resolved { + return nil, ErrResolverNotFound + } + return EvalPermission(p.Action, scopes...), nil } @@ -124,14 +136,24 @@ func (a allEvaluator) Evaluate(permissions map[string][]string) bool { } func (a allEvaluator) MutateScopes(ctx context.Context, mutate ScopeAttributeMutator) (Evaluator, error) { + resolved := false modified := make([]Evaluator, 0, len(a.allOf)) for _, e := range a.allOf { i, err := e.MutateScopes(ctx, mutate) if err != nil { + if errors.Is(err, ErrResolverNotFound) { + modified = append(modified, e) + continue + } return nil, err } + resolved = true modified = append(modified, i) } + + if !resolved { + return nil, ErrResolverNotFound + } return EvalAll(modified...), nil } @@ -174,14 +196,25 @@ func (a anyEvaluator) Evaluate(permissions map[string][]string) bool { } func (a anyEvaluator) MutateScopes(ctx context.Context, mutate ScopeAttributeMutator) (Evaluator, error) { + resolved := false modified := make([]Evaluator, 0, len(a.anyOf)) for _, e := range a.anyOf { i, err := e.MutateScopes(ctx, mutate) if err != nil { + if errors.Is(err, ErrResolverNotFound) { + modified = append(modified, e) + continue + } return nil, err } + resolved = true modified = append(modified, i) } + + if !resolved { + return nil, ErrResolverNotFound + } + return EvalAny(modified...), nil } diff --git a/pkg/services/accesscontrol/evaluator_test.go b/pkg/services/accesscontrol/evaluator_test.go index 8d36eba492d..2cd9a21af4c 100644 --- a/pkg/services/accesscontrol/evaluator_test.go +++ b/pkg/services/accesscontrol/evaluator_test.go @@ -410,3 +410,49 @@ func TestEval(t *testing.T) { }) } } + +func TestEval_MutateScopes(t *testing.T) { + t.Run("should return error if none of the scopes was a resolved", func(t *testing.T) { + eval := EvalAll( + EvalPermission("action:1", "scope:uid:1"), + EvalPermission("action:2", "scope:id:1"), + ) + + calls := 0 + _, err := eval.MutateScopes(context.Background(), func(ctx context.Context, s string) ([]string, error) { + calls += 1 + return nil, ErrResolverNotFound + }) + + assert.Equal(t, 2, calls) + assert.ErrorIs(t, err, ErrResolverNotFound) + }) + + t.Run("should return if at least one scope was resolved", func(t *testing.T) { + eval := EvalAll( + EvalPermission("action:1", "scope:uid:1"), + EvalPermission("action:2", "scope:id:1"), + ) + + calls := 0 + resolved := 0 + eval, err := eval.MutateScopes(context.Background(), func(ctx context.Context, s string) ([]string, error) { + calls += 1 + if s == "scope:id:1" { + resolved += 1 + return []string{"scope:uid:2"}, nil + } + return nil, ErrResolverNotFound + }) + + assert.NoError(t, err) + assert.Equal(t, 2, calls) + assert.Equal(t, 1, resolved) + + hasAccess := eval.Evaluate(map[string][]string{ + "action:1": {"scope:uid:1"}, + "action:2": {"scope:uid:2"}, + }) + assert.True(t, hasAccess) + }) +}