Access control: Use search options for computing permissions cache key (#87589)

This commit is contained in:
Alexander Zobnin
2024-05-10 11:06:52 +02:00
committed by GitHub
parent 896882b004
commit 0302b75721
4 changed files with 163 additions and 65 deletions

View File

@@ -553,7 +553,7 @@ func (s *Service) searchUserPermissions(ctx context.Context, orgID int64, search
}
permissions = append(permissions, dbPermissions[userID]...)
key := accesscontrol.GetPermissionCacheKey(&user.SignedInUser{UserID: userID, OrgID: orgID})
key := accesscontrol.GetSearchPermissionCacheKey(&user.SignedInUser{UserID: userID, OrgID: orgID}, searchOptions)
s.cache.Set(key, permissions, cacheTTL)
return permissions, nil
@@ -571,7 +571,7 @@ func (s *Service) searchUserPermissionsFromCache(orgID int64, searchOptions acce
OrgID: orgID,
}
key := accesscontrol.GetPermissionCacheKey(tempUser)
key := accesscontrol.GetSearchPermissionCacheKey(tempUser, searchOptions)
permissions, ok := s.cache.Get((key))
if !ok {
metrics.MAccessSearchUserPermissionsCacheUsage.WithLabelValues(accesscontrol.CacheMiss).Inc()

View File

@@ -20,7 +20,6 @@ import (
"github.com/grafana/grafana/pkg/services/auth/identity"
"github.com/grafana/grafana/pkg/services/featuremgmt"
"github.com/grafana/grafana/pkg/services/licensing"
"github.com/grafana/grafana/pkg/services/org"
"github.com/grafana/grafana/pkg/services/user"
"github.com/grafana/grafana/pkg/setting"
"github.com/grafana/grafana/pkg/tests/testsuite"
@@ -748,68 +747,6 @@ func TestService_SearchUserPermissions(t *testing.T) {
}
}
func TestPermissionCacheKey(t *testing.T) {
testcases := []struct {
name string
signedInUser *user.SignedInUser
expected string
}{
{
name: "should return correct key for user",
signedInUser: &user.SignedInUser{
OrgID: 1,
UserID: 1,
NamespacedID: identity.MustParseNamespaceID("user:1"),
},
expected: "rbac-permissions-1-user-1",
},
{
name: "should return correct key for api key",
signedInUser: &user.SignedInUser{
OrgID: 1,
ApiKeyID: 1,
IsServiceAccount: false,
NamespacedID: identity.MustParseNamespaceID("user:1"),
},
expected: "rbac-permissions-1-api-key-1",
},
{
name: "should return correct key for service account",
signedInUser: &user.SignedInUser{
OrgID: 1,
UserID: 1,
IsServiceAccount: true,
NamespacedID: identity.MustParseNamespaceID("service-account:1"),
},
expected: "rbac-permissions-1-service-account-1",
},
{
name: "should return correct key for matching a service account with userId -1",
signedInUser: &user.SignedInUser{
OrgID: 1,
UserID: -1,
IsServiceAccount: true,
NamespacedID: identity.MustParseNamespaceID("service-account:-1"),
},
expected: "rbac-permissions-1-service-account--1",
},
{
name: "should use org role if no unique id",
signedInUser: &user.SignedInUser{
OrgID: 1,
OrgRole: org.RoleNone,
NamespacedID: identity.MustParseNamespaceID("user:1"),
},
expected: "rbac-permissions-1-user-None",
},
}
for _, tc := range testcases {
t.Run(tc.name, func(t *testing.T) {
assert.Equal(t, tc.expected, accesscontrol.GetPermissionCacheKey(tc.signedInUser))
})
}
}
func TestService_SaveExternalServiceRole(t *testing.T) {
type run struct {
cmd accesscontrol.SaveExternalServiceRoleCommand

View File

@@ -11,6 +11,23 @@ func GetPermissionCacheKey(user identity.Requester) string {
return fmt.Sprintf("rbac-permissions-%s", user.GetCacheKey())
}
func GetSearchPermissionCacheKey(user identity.Requester, searchOptions SearchOptions) string {
key := fmt.Sprintf("rbac-permissions-%s", user.GetCacheKey())
if searchOptions.Action != "" {
key += fmt.Sprintf("-%s", searchOptions.Action)
}
if searchOptions.Scope != "" {
key += fmt.Sprintf("-%s", searchOptions.Scope)
}
if len(searchOptions.RolePrefixes) > 0 {
key += "-" + strings.Join(searchOptions.RolePrefixes, "-")
}
if searchOptions.ActionPrefix != "" {
key += fmt.Sprintf("-%s", searchOptions.ActionPrefix)
}
return key
}
func GetUserDirectPermissionCacheKey(user identity.Requester) string {
return fmt.Sprintf("rbac-permissions-direct-%s", user.GetCacheKey())
}

View File

@@ -0,0 +1,144 @@
package accesscontrol
import (
"testing"
"github.com/stretchr/testify/assert"
"github.com/grafana/grafana/pkg/services/auth/identity"
"github.com/grafana/grafana/pkg/services/org"
"github.com/grafana/grafana/pkg/services/user"
)
func TestPermissionCacheKey(t *testing.T) {
testcases := []struct {
name string
signedInUser *user.SignedInUser
expected string
}{
{
name: "should return correct key for user",
signedInUser: &user.SignedInUser{
OrgID: 1,
UserID: 1,
NamespacedID: identity.MustParseNamespaceID("user:1"),
},
expected: "rbac-permissions-1-user-1",
},
{
name: "should return correct key for api key",
signedInUser: &user.SignedInUser{
OrgID: 1,
ApiKeyID: 1,
IsServiceAccount: false,
NamespacedID: identity.MustParseNamespaceID("user:1"),
},
expected: "rbac-permissions-1-api-key-1",
},
{
name: "should return correct key for service account",
signedInUser: &user.SignedInUser{
OrgID: 1,
UserID: 1,
IsServiceAccount: true,
NamespacedID: identity.MustParseNamespaceID("service-account:1"),
},
expected: "rbac-permissions-1-service-account-1",
},
{
name: "should return correct key for matching a service account with userId -1",
signedInUser: &user.SignedInUser{
OrgID: 1,
UserID: -1,
IsServiceAccount: true,
NamespacedID: identity.MustParseNamespaceID("service-account:-1"),
},
expected: "rbac-permissions-1-service-account--1",
},
{
name: "should use org role if no unique id",
signedInUser: &user.SignedInUser{
OrgID: 1,
OrgRole: org.RoleNone,
NamespacedID: identity.MustParseNamespaceID("user:1"),
},
expected: "rbac-permissions-1-user-None",
},
}
for _, tc := range testcases {
t.Run(tc.name, func(t *testing.T) {
assert.Equal(t, tc.expected, GetPermissionCacheKey(tc.signedInUser))
})
}
}
func TestGetSearchPermissionCacheKey(t *testing.T) {
testcases := []struct {
name string
signedInUser *user.SignedInUser
searchOptions SearchOptions
expected string
}{
{
name: "should return correct key for user with no options",
signedInUser: &user.SignedInUser{
OrgID: 1,
UserID: 1,
},
searchOptions: SearchOptions{},
expected: "rbac-permissions-1-user-1",
},
{
name: "should return correct key for user with action",
signedInUser: &user.SignedInUser{
OrgID: 1,
UserID: 1,
},
searchOptions: SearchOptions{
Action: "datasources:read",
},
expected: "rbac-permissions-1-user-1-datasources:read",
},
{
name: "should return correct key for user with scope",
signedInUser: &user.SignedInUser{
OrgID: 1,
UserID: 1,
},
searchOptions: SearchOptions{
Scope: "datasources:*",
},
expected: "rbac-permissions-1-user-1-datasources:*",
},
{
name: "should return correct key for user with action and scope",
signedInUser: &user.SignedInUser{
OrgID: 1,
UserID: 1,
},
searchOptions: SearchOptions{
Action: "datasources:read",
Scope: "datasources:*",
},
expected: "rbac-permissions-1-user-1-datasources:read-datasources:*",
},
{
name: "should return correct key for user with role prefixes",
signedInUser: &user.SignedInUser{
OrgID: 1,
UserID: 1,
},
searchOptions: SearchOptions{
RolePrefixes: []string{"foo", "bar"},
},
expected: "rbac-permissions-1-user-1-foo-bar",
},
}
for _, tc := range testcases {
t.Run(tc.name, func(t *testing.T) {
assert.Equal(t, tc.expected, GetSearchPermissionCacheKey(tc.signedInUser, tc.searchOptions))
})
}
}