diff --git a/pkg/services/accesscontrol/accesscontrol.go b/pkg/services/accesscontrol/accesscontrol.go index 9bd58ad413b..cddb344d50c 100644 --- a/pkg/services/accesscontrol/accesscontrol.go +++ b/pkg/services/accesscontrol/accesscontrol.go @@ -2,6 +2,7 @@ package accesscontrol import ( "context" + "strings" "github.com/grafana/grafana/pkg/models" ) @@ -53,3 +54,15 @@ func BuildPermissionsMap(permissions []*Permission) map[string]bool { return permissionsMap } + +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 '*' + if len(prefix) > 0 && last == '*' { + lastChar := prefix[len(prefix)-1] + if lastChar != ':' && lastChar != '/' { + return false + } + } + return !strings.ContainsAny(prefix, "*?") +} diff --git a/pkg/services/accesscontrol/evaluator/evaluator.go b/pkg/services/accesscontrol/evaluator/evaluator.go index 101512927f5..9bb3c72e8a0 100644 --- a/pkg/services/accesscontrol/evaluator/evaluator.go +++ b/pkg/services/accesscontrol/evaluator/evaluator.go @@ -2,15 +2,18 @@ package evaluator import ( "context" + "fmt" + "strings" - "github.com/gobwas/glob" - + "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/infra/metrics" "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/prometheus/client_golang/prometheus" ) +var logger = log.New("accesscontrol.evaluator") + // Evaluate evaluates access to the given resource, using provided AccessControl instance. // Scopes are evaluated with an `OR` relationship. func Evaluate(ctx context.Context, ac accesscontrol.AccessControl, user *models.SignedInUser, action string, scope ...string) (bool, error) { @@ -37,20 +40,46 @@ func evaluateScope(dbScopes map[string]struct{}, targetScopes ...string) (bool, } for _, s := range targetScopes { - var match bool for dbScope := range dbScopes { - rule, err := glob.Compile(dbScope, ':', '/') - if err != nil { - return false, err + if dbScope == "" { + continue } - match = rule.Match(s) - if match { + if !accesscontrol.ValidateScope(dbScope) { + logger.Error( + "invalid scope", + "reason", fmt.Sprintf("%v should not contain meta-characters like * or ?, except in the last position", dbScope), + "scope", dbScope, + ) + continue + } + + prefix, last := dbScope[:len(dbScope)-1], dbScope[len(dbScope)-1] + //Prefix match + if last == '*' { + if strings.HasPrefix(s, prefix) { + logger.Debug( + "matched scope", + "reason", fmt.Sprintf("matched request scope %v against resource scope %v", dbScope, s), + "request scope", dbScope, + "resource scope", s, + ) + return true, nil + } + } + + if s == dbScope { return true, nil } } } + logger.Debug( + "access control failed", + "request scope", dbScopes, + "resource scope", targetScopes, + "reason", fmt.Sprintf("Could not match resource scopes %v with request scopes %v", dbScopes, targetScopes), + ) return false, nil } diff --git a/pkg/services/accesscontrol/evaluator/evaluator_test.go b/pkg/services/accesscontrol/evaluator/evaluator_test.go index a88ece4419e..d0b85750cac 100644 --- a/pkg/services/accesscontrol/evaluator/evaluator_test.go +++ b/pkg/services/accesscontrol/evaluator/evaluator_test.go @@ -14,7 +14,11 @@ func TestExtractPermission(t *testing.T) { userPermissions := []*accesscontrol.Permission{ { Action: "permissions:create", - Scope: "teams:*/permissions:*", + Scope: "teams:*", + }, + { + Action: "permissions:create", + Scope: "permissions:*", }, { Action: "permissions:remove", @@ -22,7 +26,8 @@ func TestExtractPermission(t *testing.T) { }, } expectedScopes := map[string]struct{}{ - "teams:*/permissions:*": {}, + "permissions:*": {}, + "teams:*": {}, } ok, scopes := extractScopes(userPermissions, targetPermission) assert.True(t, ok) @@ -45,9 +50,10 @@ func TestEvaluatePermissions(t *testing.T) { { Name: "No expected scope always returns true", HasScopes: map[string]struct{}{ - "teams:*/permissions:*": {}, - "users:*": {}, - "permissions:delegate": {}, + "teams:*": {}, + "permissions:*": {}, + "users:*": {}, + "permissions:delegate": {}, }, NeedAnyScope: []string{}, Valid: true, @@ -55,27 +61,30 @@ func TestEvaluatePermissions(t *testing.T) { { Name: "Single scope from list", HasScopes: map[string]struct{}{ - "teams:1/permissions:delegate": {}, + "teams:1": {}, + "permissions:delegate": {}, }, - NeedAnyScope: []string{"teams:1/permissions:delegate"}, + NeedAnyScope: []string{"teams:1", "permissions:delegate"}, Valid: true, }, { Name: "Single scope from glob list", HasScopes: map[string]struct{}{ - "teams:*/permissions:*": {}, - "users:*": {}, - "permissions:delegate": {}, + "teams:*": {}, + "permissions:*": {}, + "users:*": {}, + "permissions:delegate": {}, }, - NeedAnyScope: []string{"teams:1/permissions:delegate"}, + NeedAnyScope: []string{"teams:1", "permissions:delegate"}, Valid: true, }, { Name: "Either of two scopes from glob list", HasScopes: map[string]struct{}{ - "teams:*/permissions:*": {}, - "users:*": {}, - "permissions:delegate": {}, + "teams:*": {}, + "permissions:*": {}, + "users:*": {}, + "permissions:delegate": {}, }, NeedAnyScope: []string{"global:admin", "permissions:delegate"}, Valid: true, @@ -83,11 +92,11 @@ func TestEvaluatePermissions(t *testing.T) { { Name: "No match found", HasScopes: map[string]struct{}{ - "teams:*/permissions:*": {}, - "users:*": {}, - "permissions:delegate": {}, + "teams:*": {}, + "users:*": {}, + "permissions:delegate": {}, }, - NeedAnyScope: []string{"teams1/permissions:delegate"}, + NeedAnyScope: []string{"teams1", "permissions:nodelegate"}, Valid: false, }, } diff --git a/pkg/services/accesscontrol/models.go b/pkg/services/accesscontrol/models.go index 0fd557ebe15..6ee746b730b 100644 --- a/pkg/services/accesscontrol/models.go +++ b/pkg/services/accesscontrol/models.go @@ -102,7 +102,7 @@ const ( ScopeUsersAll = "users:*" // Settings scope - ScopeSettingsAll = "settings:**" + ScopeSettingsAll = "settings:*" ) const RoleGrafanaAdmin = "Grafana Admin" diff --git a/pkg/services/accesscontrol/roles.go b/pkg/services/accesscontrol/roles.go index 3823c98698d..c503bae728f 100644 --- a/pkg/services/accesscontrol/roles.go +++ b/pkg/services/accesscontrol/roles.go @@ -57,7 +57,7 @@ var ( } settingsAdminReadRole = RoleDTO{ - Version: 1, + Version: 2, Name: settingsAdminRead, Permissions: []Permission{ {