RBAC: Fix resolver issue on wildcard resulting in wrong status code for endpoints (#54208)

* RBAC: Test evaluation before attaching mutator

* RBAC: Return error if no resolver is found for scope

* RBAC: Sync changes to evaluation in mock

* RBAC: Check for resolver not found error and just fail the evaluation in that case

pkg/services/accesscontrol/errors.go

@@ -6,4 +6,5 @@ var (
 	ErrFixedRolePrefixMissing = errors.New("fixed role should be prefixed with '" + FixedRolePrefix + "'")
 	ErrInvalidBuiltinRole     = errors.New("built-in role is not valid")
 	ErrInvalidScope           = errors.New("invalid scope")
+	ErrResolverNotFound       = errors.New("no resolver found")
 )

pkg/services/accesscontrol/mock/mock.go

@@ -2,6 +2,7 @@ package mock
 
 import (
 	"context"
+	"errors"
 
 	"github.com/grafana/grafana/pkg/infra/log"
 	"github.com/grafana/grafana/pkg/services/accesscontrol"
@@ -105,11 +106,18 @@ func (m *Mock) Evaluate(ctx context.Context, usr *user.SignedInUser, evaluator a
 		permissions = accesscontrol.GroupScopesByAction(userPermissions)
 	}
 
-	attributeMutator := m.scopeResolvers.GetScopeAttributeMutator(usr.OrgID)
+	if evaluator.Evaluate(permissions) {
-	resolvedEvaluator, err := evaluator.MutateScopes(ctx, attributeMutator)
+		return true, nil
+	}
+
+	resolvedEvaluator, err := evaluator.MutateScopes(ctx, m.scopeResolvers.GetScopeAttributeMutator(usr.OrgID))
 	if err != nil {
+		if errors.Is(err, accesscontrol.ErrResolverNotFound) {
+			return false, nil
+		}
 		return false, err
 	}
+
 	return resolvedEvaluator.Evaluate(permissions), nil
 }
 

pkg/services/accesscontrol/ossaccesscontrol/accesscontrol.go

@@ -2,6 +2,7 @@ package ossaccesscontrol
 
 import (
 	"context"
+	"errors"
 
 	"github.com/prometheus/client_golang/prometheus"
 
@@ -45,10 +46,19 @@ func (a *AccessControl) Evaluate(ctx context.Context, user *user.SignedInUser, e
 		user.Permissions[user.OrgID] = accesscontrol.GroupScopesByAction(permissions)
 	}
 
+	// Test evaluation without scope resolver first, this will prevent 403 for wildcard scopes when resource does not exist
+	if evaluator.Evaluate(user.Permissions[user.OrgID]) {
+		return true, nil
+	}
+
 	resolvedEvaluator, err := evaluator.MutateScopes(ctx, a.resolvers.GetScopeAttributeMutator(user.OrgID))
 	if err != nil {
+		if errors.Is(err, accesscontrol.ErrResolverNotFound) {
+			return false, nil
+		}
 		return false, err
 	}
+
 	return resolvedEvaluator.Evaluate(user.Permissions[user.OrgID]), nil
 }
 

pkg/services/accesscontrol/resolvers.go

@@ -69,7 +69,7 @@ func (s *Resolvers) GetScopeAttributeMutator(orgID int64) ScopeAttributeMutator
 			s.log.Debug("resolved scope", "scope", scope, "resolved_scopes", scopes)
 			return scopes, nil
 		}
-		return []string{scope}, nil
+		return nil, ErrResolverNotFound
 	}
 }
 

pkg/services/accesscontrol/resolvers_test.go

@@ -89,24 +89,34 @@ func TestResolvers_AttributeScope(t *testing.T) {
 			wantEvaluator: accesscontrol.EvalPermission("datasources:read", accesscontrol.Scope("datasources", "id", "5")),
 			wantCalls:     1,
 		},
+		{
+			name:          "should return error if no resolver is found for scope",
+			orgID:         1,
+			evaluator:     accesscontrol.EvalPermission("dashboards:read", "dashboards:id:1"),
+			wantEvaluator: nil,
+			wantCalls:     0,
+			wantErr:       accesscontrol.ErrResolverNotFound,
+		},
 	}
 	for _, tt := range tests {
-		resolvers := accesscontrol.NewResolvers(log.NewNopLogger())
+		t.Run(tt.name, func(t *testing.T) {
+			resolvers := accesscontrol.NewResolvers(log.NewNopLogger())
 
-		// Reset calls counter
+			// Reset calls counter
-		calls = 0
+			calls = 0
-		// Register a resolution method
+			// Register a resolution method
-		resolvers.AddScopeAttributeResolver("datasources:name:", fakeDataSourceResolver)
+			resolvers.AddScopeAttributeResolver("datasources:name:", fakeDataSourceResolver)
 
-		// Test
+			// Test
-		mutate := resolvers.GetScopeAttributeMutator(tt.orgID)
+			mutate := resolvers.GetScopeAttributeMutator(tt.orgID)
-		resolvedEvaluator, err := tt.evaluator.MutateScopes(context.Background(), mutate)
+			resolvedEvaluator, err := tt.evaluator.MutateScopes(context.Background(), mutate)
-		if tt.wantErr != nil {
+			if tt.wantErr != nil {
-			assert.ErrorAs(t, err, &tt.wantErr, "expected an error during the resolution of the scope")
+				assert.ErrorAs(t, err, &tt.wantErr, "expected an error during the resolution of the scope")
-			return
+				return
-		}
+			}
-		assert.NoError(t, err)
+			assert.NoError(t, err)
-		assert.EqualValues(t, tt.wantEvaluator, resolvedEvaluator, "permission did not match expected resolution")
+			assert.EqualValues(t, tt.wantEvaluator, resolvedEvaluator, "permission did not match expected resolution")
-		assert.Equal(t, tt.wantCalls, calls, "cache has not been used")
+			assert.Equal(t, tt.wantCalls, calls, "cache has not been used")
+		})
 	}
 }