IntenseWebs/grafana
3
0
Fork 0
mirror of https://github.com/grafana/grafana.git synced 2025-02-25 18:55:37 -06:00
Code Issues Packages Projects Releases Wiki Activity

Remove unused error from evaluator Evaluate (#49305)

Browse Source
This commit is contained in:
Karl Persson 2022-05-20 10:26:57 +02:00 committed by GitHub
parent e5864f76b0
commit 4a61f4111f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 27 additions and 40 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

32
pkg/services/accesscontrol/evaluator.go
View File

@ -12,7 +12,7 @@ var logger = log.New("accesscontrol.evaluator")
type Evaluator interface {
// Evaluate permissions that are grouped by action
Evaluate(permissions map[string][]string) (bool, error)
Evaluate(permissions map[string][]string) bool
// MutateScopes executes a sequence of ScopeModifier functions on all embedded scopes of an evaluator and returns a new Evaluator
MutateScopes(ctx context.Context, mutate ScopeAttributeMutator) (Evaluator, error)
// String returns a string representation of permission required by the evaluator
@ -32,25 +32,25 @@ type permissionEvaluator struct {
Scopes []string
}
func (p permissionEvaluator) Evaluate(permissions map[string][]string) (bool, error) {
func (p permissionEvaluator) Evaluate(permissions map[string][]string) bool {
userScopes, ok := permissions[p.Action]
if !ok {
return false, nil
return false
}
if len(p.Scopes) == 0 {
return true, nil
return true
}
for _, target := range p.Scopes {
for _, scope := range userScopes {
if match(scope, target) {
return true, nil
return true
}
}
}
return false, nil
return false
}
func match(scope, target string) bool {
@ -114,13 +114,13 @@ type allEvaluator struct {
allOf []Evaluator
}
func (a allEvaluator) Evaluate(permissions map[string][]string) (bool, error) {
func (a allEvaluator) Evaluate(permissions map[string][]string) bool {
for _, e := range a.allOf {
if ok, err := e.Evaluate(permissions); !ok || err != nil {
return false, err
if !e.Evaluate(permissions) {
return false
}
}
return true, nil
return true
}
func (a allEvaluator) MutateScopes(ctx context.Context, mutate ScopeAttributeMutator) (Evaluator, error) {
@ -164,17 +164,13 @@ type anyEvaluator struct {
anyOf []Evaluator
}
func (a anyEvaluator) Evaluate(permissions map[string][]string) (bool, error) {
func (a anyEvaluator) Evaluate(permissions map[string][]string) bool {
for _, e := range a.anyOf {
ok, err := e.Evaluate(permissions)
if err != nil {
return false, err
}
if ok {
return true, nil
if e.Evaluate(permissions) {
return true
}
}
return false, nil
return false
}
func (a anyEvaluator) MutateScopes(ctx context.Context, mutate ScopeAttributeMutator) (Evaluator, error) {

19
pkg/services/accesscontrol/evaluator_test.go
View File

@ -52,8 +52,7 @@ func TestPermission_Evaluate(t *testing.T) {
for _, test := range tests {
t.Run(test.desc, func(t *testing.T) {
ok, err := test.evaluator.Evaluate(test.permissions)
assert.NoError(t, err)
ok := test.evaluator.Evaluate(test.permissions)
assert.Equal(t, test.expected, ok)
})
}
@ -123,8 +122,7 @@ func TestPermission_Inject(t *testing.T) {
t.Run(test.desc, func(t *testing.T) {
injected, err := test.evaluator.MutateScopes(context.TODO(), ScopeInjector(test.params))
assert.NoError(t, err)
ok, err := injected.Evaluate(test.permissions)
assert.NoError(t, err)
ok := injected.Evaluate(test.permissions)
assert.Equal(t, test.expected, ok)
})
}
@ -172,8 +170,7 @@ func TestAll_Evaluate(t *testing.T) {
for _, test := range tests {
t.Run(test.desc, func(t *testing.T) {
ok, err := test.evaluator.Evaluate(test.permissions)
assert.NoError(t, err)
ok := test.evaluator.Evaluate(test.permissions)
assert.Equal(t, test.expected, ok)
})
}
@ -236,7 +233,7 @@ func TestAll_Inject(t *testing.T) {
t.Run(test.desc, func(t *testing.T) {
injected, err := test.evaluator.MutateScopes(context.TODO(), ScopeInjector(test.params))
assert.NoError(t, err)
ok, err := injected.Evaluate(test.permissions)
ok := injected.Evaluate(test.permissions)
assert.NoError(t, err)
assert.Equal(t, test.expected, ok)
})
@ -283,8 +280,7 @@ func TestAny_Evaluate(t *testing.T) {
for _, test := range tests {
t.Run(test.desc, func(t *testing.T) {
ok, err := test.evaluator.Evaluate(test.permissions)
assert.NoError(t, err)
ok := test.evaluator.Evaluate(test.permissions)
assert.Equal(t, test.expected, ok)
})
}
@ -347,7 +343,7 @@ func TestAny_Inject(t *testing.T) {
t.Run(test.desc, func(t *testing.T) {
injected, err := test.evaluator.MutateScopes(context.TODO(), ScopeInjector(test.params))
assert.NoError(t, err)
ok, err := injected.Evaluate(test.permissions)
ok := injected.Evaluate(test.permissions)
assert.NoError(t, err)
assert.Equal(t, test.expected, ok)
})
@ -409,8 +405,7 @@ func TestEval(t *testing.T) {
for _, test := range tests {
t.Run(test.desc, func(t *testing.T) {
ok, err := test.evaluator.Evaluate(test.permissions)
assert.NoError(t, err)
ok := test.evaluator.Evaluate(test.permissions)
assert.Equal(t, test.expected, ok)
})
}

2
pkg/services/accesscontrol/mock/mock.go
View File

@ -103,7 +103,7 @@ func (m *Mock) Evaluate(ctx context.Context, user *models.SignedInUser, evaluato
if err != nil {
return false, err
}
return resolvedEvaluator.Evaluate(accesscontrol.GroupScopesByAction(permissions))
return resolvedEvaluator.Evaluate(accesscontrol.GroupScopesByAction(permissions)), nil
}
// GetUserPermissions returns user permissions.

2
pkg/services/accesscontrol/ossaccesscontrol/ossaccesscontrol.go
View File

@ -98,7 +98,7 @@ func (ac *OSSAccessControlService) Evaluate(ctx context.Context, user *models.Si
if err != nil {
return false, err
}
return resolvedEvaluator.Evaluate(user.Permissions[user.OrgId])
return resolvedEvaluator.Evaluate(user.Permissions[user.OrgId]), nil
}
// GetUserRoles returns user permissions based on built-in roles

12
pkg/services/ngalert/api/authorization_test.go
View File

@ -189,9 +189,8 @@ func TestAuthorizeRuleChanges(t *testing.T) {
groupChanges := testCase.changes()
result, err := authorizeRuleChanges(groupChanges, func(evaluator ac.Evaluator) bool {
response, err := evaluator.Evaluate(make(map[string][]string))
response := evaluator.Evaluate(make(map[string][]string))
require.False(t, response)
require.NoError(t, err)
executed = true
return false
})
@ -202,9 +201,8 @@ func TestAuthorizeRuleChanges(t *testing.T) {
permissions := testCase.permissions(groupChanges)
executed = false
result, err = authorizeRuleChanges(groupChanges, func(evaluator ac.Evaluator) bool {
response, err := evaluator.Evaluate(permissions)
response := evaluator.Evaluate(permissions)
require.Truef(t, response, "provided permissions [%v] is not enough for requested permissions [%s]", testCase.permissions, evaluator.GoString())
require.NoError(t, err)
executed = true
return true
})
@ -359,8 +357,7 @@ func TestAuthorizeRuleDelete(t *testing.T) {
groupChanges := testCase.changes()
permissions := testCase.permissions(groupChanges)
result, err := authorizeRuleChanges(groupChanges, func(evaluator ac.Evaluator) bool {
response, err := evaluator.Evaluate(permissions)
require.NoError(t, err)
response := evaluator.Evaluate(permissions)
return response
})
@ -401,9 +398,8 @@ func TestCheckDatasourcePermissionsForRule(t *testing.T) {
executed := 0
eval := authorizeDatasourceAccessForRule(rule, func(evaluator ac.Evaluator) bool {
response, err := evaluator.Evaluate(permissions)
response := evaluator.Evaluate(permissions)
require.Truef(t, response, "provided permissions [%v] is not enough for requested permissions [%s]", permissions, evaluator.GoString())
require.NoError(t, err)
executed++
return true
})