Access control: Update evaluator to authorize when at least one of the scopes is a match (#33393)

pkg/services/accesscontrol/evaluator/evaluator.go

@@ -9,18 +9,24 @@ import (
 	"github.com/grafana/grafana/pkg/services/accesscontrol"
 )
 
-// Evaluate evaluates access to the given resource, using provided AccessControl instance
+// Evaluate evaluates access to the given resource, using provided AccessControl instance.
-func Evaluate(ctx context.Context, ac accesscontrol.AccessControl, user *models.SignedInUser, permission string, scope ...string) (bool, error) {
+// Scopes are evaluated with an `OR` relationship.
-	res, err := ac.GetUserPermissions(ctx, user)
+func Evaluate(ctx context.Context, ac accesscontrol.AccessControl, user *models.SignedInUser, action string, scope ...string) (bool, error) {
+	userPermissions, err := ac.GetUserPermissions(ctx, user)
 	if err != nil {
 		return false, err
 	}
 
-	ok, dbScopes := extractPermission(res, permission)
+	ok, dbScopes := extractScopes(userPermissions, action)
 	if !ok {
 		return false, nil
 	}
-	for _, s := range scope {
+
+	return evaluateScope(dbScopes, scope...)
+}
+
+func evaluateScope(dbScopes map[string]struct{}, targetScopes ...string) (bool, error) {
+	for _, s := range targetScopes {
 		var match bool
 		for dbScope := range dbScopes {
 			rule, err := glob.Compile(dbScope, ':', '/')
@@ -30,19 +36,15 @@ func Evaluate(ctx context.Context, ac accesscontrol.AccessControl, user *models.
 
 			match = rule.Match(s)
 			if match {
-				break
+				return true, nil
 			}
 		}
-
-		if !match {
-			return false, nil
-		}
 	}
 
-	return true, nil
+	return false, nil
 }
 
-func extractPermission(permissions []*accesscontrol.Permission, permission string) (bool, map[string]struct{}) {
+func extractScopes(permissions []*accesscontrol.Permission, targetAction string) (bool, map[string]struct{}) {
 	scopes := map[string]struct{}{}
 	ok := false
 
@@ -50,7 +52,7 @@ func extractPermission(permissions []*accesscontrol.Permission, permission strin
 		if p == nil {
 			continue
 		}
-		if p.Action == permission {
+		if p.Action == targetAction {
 			ok = true
 			scopes[p.Scope] = struct{}{}
 		}

pkg/services/accesscontrol/evaluator/evaluator_test.go

@@ -0,0 +1,65 @@
+package evaluator
+
+import (
+	"testing"
+
+	"github.com/grafana/grafana/pkg/services/accesscontrol"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestExtractPermission(t *testing.T) {
+	const targetPermission = "permissions:create"
+	userPermissions := []*accesscontrol.Permission{
+		{
+			Action: "permissions:create",
+			Scope:  "teams:*/permissions:*",
+		},
+		{
+			Action: "permissions:remove",
+			Scope:  "permissions:*",
+		},
+	}
+	expectedScopes := map[string]struct{}{
+		"teams:*/permissions:*": {},
+	}
+	ok, scopes := extractScopes(userPermissions, targetPermission)
+	assert.True(t, ok)
+	assert.Equal(t, expectedScopes, scopes)
+}
+
+func TestEvaluatePermissions(t *testing.T) {
+	scopes := map[string]struct{}{
+		"teams:*/permissions:*": {},
+		"users:*":               {},
+		"permissions:delegate":  {},
+	}
+
+	ok, err := evaluateScope(scopes, "teams:1/permissions:delegate")
+	require.NoError(t, err)
+	assert.True(t, ok)
+}
+
+func TestEvaluatePermissions_WhenAtLeastOneScopeIsMatched_ReturnsTrue(t *testing.T) {
+	scopes := map[string]struct{}{
+		"teams:*/permissions:*": {},
+		"users:*":               {},
+		"permissions:delegate":  {},
+	}
+
+	ok, err := evaluateScope(scopes, "global:admin", "permissions:delegate")
+	require.NoError(t, err)
+	assert.True(t, ok)
+}
+
+func TestEvaluatePermissions_WhenNoMatchFound_ReturnsFalse(t *testing.T) {
+	scopes := map[string]struct{}{
+		"teams:*/permissions:*": {},
+		"users:*":               {},
+		"permissions:delegate":  {},
+	}
+
+	ok, err := evaluateScope(scopes, "teams1/permissions:delegate")
+	require.NoError(t, err)
+	assert.False(t, ok)
+}

pkg/services/accesscontrol/roles.go

@@ -18,7 +18,7 @@ var ldapAdminReadRole = RoleDTO{
 var ldapAdminEditRole = RoleDTO{
 	Name:    ldapAdminEdit,
 	Version: 1,
-	Permissions: concat(ldapAdminReadRole.Permissions, []Permission{
+	Permissions: ConcatPermissions(ldapAdminReadRole.Permissions, []Permission{
 		{
 			Action: ActionLDAPUsersSync,
 		},
@@ -39,7 +39,7 @@ var orgsAdminReadRole = RoleDTO{
 var orgsAdminEditRole = RoleDTO{
 	Name:    orgsAdminEdit,
 	Version: 1,
-	Permissions: concat(orgsAdminReadRole.Permissions, []Permission{
+	Permissions: ConcatPermissions(orgsAdminReadRole.Permissions, []Permission{
 		{
 			Action: ActionOrgUsersAdd,
 			Scope:  ScopeOrgAllUsersAll,
@@ -69,7 +69,7 @@ var orgsCurrentReadRole = RoleDTO{
 var orgsCurrentEditRole = RoleDTO{
 	Name:    orgsCurrentEdit,
 	Version: 1,
-	Permissions: concat(orgsCurrentReadRole.Permissions, []Permission{
+	Permissions: ConcatPermissions(orgsCurrentReadRole.Permissions, []Permission{
 		{
 			Action: ActionOrgUsersAdd,
 			Scope:  ScopeOrgCurrentUsersAll,
@@ -111,7 +111,7 @@ var usersAdminReadRole = RoleDTO{
 var usersAdminEditRole = RoleDTO{
 	Name:    usersAdminEdit,
 	Version: 1,
-	Permissions: concat(usersAdminReadRole.Permissions, []Permission{
+	Permissions: ConcatPermissions(usersAdminReadRole.Permissions, []Permission{
 		{
 			Action: ActionUsersPasswordUpdate,
 			Scope:  ScopeUsersAll,
@@ -205,7 +205,7 @@ var PredefinedRoleGrants = map[string][]string{
 	},
 }
 
-func concat(permissions ...[]Permission) []Permission {
+func ConcatPermissions(permissions ...[]Permission) []Permission {
 	if permissions == nil {
 		return nil
 	}

pkg/services/accesscontrol/roles_test.go

@@ -34,7 +34,7 @@ func TestPredefinedRoleGrants(t *testing.T) {
 	}
 }
 
-func TestConcat(t *testing.T) {
+func TestConcatPermissions(t *testing.T) {
 	perms1 := []Permission{
 		{
 			Action: "test",
@@ -67,6 +67,6 @@ func TestConcat(t *testing.T) {
 		},
 	}
 
-	perms := concat(perms1, perms2)
+	perms := ConcatPermissions(perms1, perms2)
 	assert.ElementsMatch(t, perms, expected)
 }