From 6da850a2f2419f7f8adb8e9773e3a41f5c4f473b Mon Sep 17 00:00:00 2001 From: Gabriel MABILLE Date: Thu, 5 Jan 2023 17:32:13 +0100 Subject: [PATCH] RBAC: Add function to reduce permissions (#58197) * RBAC: Add function to reduce permissions * Make names readable Co-authored-by: Jguer * Remove copy pasted comment * Nit. Co-authored-by: Jguer Co-authored-by: Mihaly Gyongyosi --- pkg/services/accesscontrol/accesscontrol.go | 64 +++++++++++ .../accesscontrol/accesscontrol_test.go | 103 ++++++++++++++++++ pkg/services/accesscontrol/scope.go | 4 + 3 files changed, 171 insertions(+) create mode 100644 pkg/services/accesscontrol/accesscontrol_test.go diff --git a/pkg/services/accesscontrol/accesscontrol.go b/pkg/services/accesscontrol/accesscontrol.go index 724898f0b6e..ef2ca546a57 100644 --- a/pkg/services/accesscontrol/accesscontrol.go +++ b/pkg/services/accesscontrol/accesscontrol.go @@ -192,6 +192,70 @@ func GroupScopesByAction(permissions []Permission) map[string][]string { return m } +func Reduce(ps []Permission) map[string][]string { + reduced := make(map[string][]string) + scopesByAction := make(map[string]map[string]bool) + wildcardsByAction := make(map[string]map[string]bool) + + // helpers + add := func(scopesByAction map[string]map[string]bool, action, scope string) { + if _, ok := scopesByAction[action]; !ok { + scopesByAction[action] = map[string]bool{scope: true} + return + } + scopesByAction[action][scope] = true + } + includes := func(wildcardsSet map[string]bool, scope string) bool { + for wildcard := range wildcardsSet { + if wildcard == "*" || strings.HasPrefix(scope, wildcard[:len(wildcard)-1]) { + return true + } + } + return false + } + + // Sort permissions (scopeless, wildcard, specific) + for i := range ps { + if ps[i].Scope == "" { + if _, ok := reduced[ps[i].Action]; !ok { + reduced[ps[i].Action] = nil + } + continue + } + if isWildcard(ps[i].Scope) { + add(wildcardsByAction, ps[i].Action, ps[i].Scope) + continue + } + add(scopesByAction, ps[i].Action, ps[i].Scope) + } + + // Reduce wildcards + for action, wildcards := range wildcardsByAction { + for wildcard := range wildcards { + if wildcard == "*" { + reduced[action] = []string{wildcard} + break + } + if includes(wildcards, wildcard[:len(wildcard)-2]) { + continue + } + reduced[action] = append(reduced[action], wildcard) + } + } + + // Reduce specific + for action, scopes := range scopesByAction { + for scope := range scopes { + if includes(wildcardsByAction[action], scope) { + continue + } + reduced[action] = append(reduced[action], scope) + } + } + + return reduced +} + func ValidateScope(scope string) bool { prefix, last := scope[:len(scope)-1], scope[len(scope)-1] // verify that last char is either ':' or '/' if last character of scope is '*' diff --git a/pkg/services/accesscontrol/accesscontrol_test.go b/pkg/services/accesscontrol/accesscontrol_test.go new file mode 100644 index 00000000000..9d20738441a --- /dev/null +++ b/pkg/services/accesscontrol/accesscontrol_test.go @@ -0,0 +1,103 @@ +package accesscontrol + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestReduce(t *testing.T) { + tests := []struct { + name string + ps []Permission + want map[string][]string + }{ + { + name: "no permission", + ps: []Permission{}, + want: map[string][]string{}, + }, + { + name: "scopeless permissions", + ps: []Permission{{Action: "orgs:read"}}, + want: map[string][]string{"orgs:read": nil}, + }, + { // edge case that should not exist + name: "mixed scope and scopeless permissions", + ps: []Permission{ + {Action: "resources:read", Scope: "resources:id:1"}, + {Action: "resources:read"}, + }, + want: map[string][]string{"resources:read": {"resources:id:1"}}, + }, + { + name: "specific permission", + ps: []Permission{ + {Action: "teams:read", Scope: "teams:id:1"}, + {Action: "teams:read", Scope: "teams:id:2"}, + {Action: "teams:write", Scope: "teams:id:1"}, + }, + want: map[string][]string{ + "teams:read": {"teams:id:1", "teams:id:2"}, + "teams:write": {"teams:id:1"}, + }, + }, + { + name: "wildcard permission", + ps: []Permission{ + {Action: "teams:read", Scope: "teams:id:1"}, + {Action: "teams:read", Scope: "teams:id:2"}, + {Action: "teams:read", Scope: "teams:id:*"}, + {Action: "teams:write", Scope: "teams:id:1"}, + }, + want: map[string][]string{ + "teams:read": {"teams:id:*"}, + "teams:write": {"teams:id:1"}, + }, + }, + { + name: "mixed wildcard and scoped permission", + ps: []Permission{ + {Action: "dashboards:read", Scope: "dashboards:*"}, + {Action: "dashboards:read", Scope: "folders:uid:1"}, + }, + want: map[string][]string{ + "dashboards:read": {"dashboards:*", "folders:uid:1"}, + }, + }, + { + name: "different wildcard permission", + ps: []Permission{ + {Action: "dashboards:read", Scope: "dashboards:uid:*"}, + {Action: "dashboards:read", Scope: "dashboards:*"}, + {Action: "dashboards:read", Scope: "folders:uid:*"}, + {Action: "dashboards:read", Scope: "folders:*"}, + }, + want: map[string][]string{ + "dashboards:read": {"dashboards:*", "folders:*"}, + }, + }, + { + name: "root wildcard permission", + ps: []Permission{ + {Action: "dashboards:read", Scope: "*"}, + {Action: "dashboards:read", Scope: "dashboards:*"}, + {Action: "dashboards:read", Scope: "folders:*"}, + }, + want: map[string][]string{ + "dashboards:read": {"*"}, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := Reduce(tt.ps) + require.Len(t, got, len(tt.want)) + for action, scopes := range got { + want, ok := tt.want[action] + require.True(t, ok) + require.ElementsMatch(t, scopes, want) + } + }) + } +} diff --git a/pkg/services/accesscontrol/scope.go b/pkg/services/accesscontrol/scope.go index de03856dd5a..8b58b9bd8d2 100644 --- a/pkg/services/accesscontrol/scope.go +++ b/pkg/services/accesscontrol/scope.go @@ -170,3 +170,7 @@ func (wildcards Wildcards) Contains(scope string) bool { } return false } + +func isWildcard(scope string) bool { + return scope == "*" || strings.HasSuffix(scope, ":*") +}