Requester: Update GetCacheKey (#74834)

* AuthN: re-export all namespaces

* Identity: Change signature of GetCacheKey

* User: check HasUniqueID

* Default to org role None if role is empty

pkg/services/accesscontrol/acimpl/service.go

@@ -143,11 +143,7 @@ func (s *Service) getUserPermissions(ctx context.Context, user identity.Requeste
 }
 
 func (s *Service) getCachedUserPermissions(ctx context.Context, user identity.Requester, options accesscontrol.Options) ([]accesscontrol.Permission, error) {
-	key, err := permissionCacheKey(user)
+	key := permissionCacheKey(user)
-	if err != nil {
-		return nil, err
-	}
-
 	if !options.ReloadCache {
 		permissions, ok := s.cache.Get(key)
 		if ok {
@@ -169,11 +165,7 @@ func (s *Service) getCachedUserPermissions(ctx context.Context, user identity.Re
 }
 
 func (s *Service) ClearUserPermissionCache(user identity.Requester) {
-	key, err := permissionCacheKey(user)
+	s.cache.Delete(permissionCacheKey(user))
-	if err != nil {
-		return
-	}
-	s.cache.Delete(key)
 }
 
 func (s *Service) DeleteUserPermissions(ctx context.Context, orgID int64, userID int64) error {
@@ -215,12 +207,8 @@ func (s *Service) RegisterFixedRoles(ctx context.Context) error {
 	return nil
 }
 
-func permissionCacheKey(user identity.Requester) (string, error) {
+func permissionCacheKey(user identity.Requester) string {
-	key, err := user.GetCacheKey()
+	return fmt.Sprintf("rbac-permissions-%s", user.GetCacheKey())
-	if err != nil {
-		return "", err
-	}
-	return fmt.Sprintf("rbac-permissions-%s", key), nil
 }
 
 // DeclarePluginRoles allow the caller to declare, to the service, plugin roles and their assignments
@@ -380,13 +368,9 @@ func (s *Service) searchUserPermissionsFromCache(orgID int64, searchOptions acce
 		UserID: searchOptions.UserID,
 		OrgID:  orgID,
 	}
-	key, err := permissionCacheKey(tempUser)
-	if err != nil {
-		s.log.Debug("Could not obtain cache key to search user permissions", "error", err.Error())
-		return nil, false
-	}
 
-	permissions, ok := s.cache.Get(key)
+	key := permissionCacheKey(tempUser)
+	permissions, ok := s.cache.Get((key))
 	if !ok {
 		return nil, false
 	}

pkg/services/accesscontrol/acimpl/service_test.go

@@ -17,6 +17,7 @@ import (
 	"github.com/grafana/grafana/pkg/services/accesscontrol/database"
 	"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"
 )
@@ -695,7 +696,6 @@ func TestPermissionCacheKey(t *testing.T) {
 		name         string
 		signedInUser *user.SignedInUser
 		expected     string
-		expectedErr  error
 	}{
 		{
 			name: "should return correct key for user",
@@ -703,8 +703,7 @@ func TestPermissionCacheKey(t *testing.T) {
 				OrgID:  1,
 				UserID: 1,
 			},
-			expected:    "rbac-permissions-1-user-1",
+			expected: "rbac-permissions-1-user-1",
-			expectedErr: nil,
 		},
 		{
 			name: "should return correct key for api key",
@@ -713,8 +712,7 @@ func TestPermissionCacheKey(t *testing.T) {
 				ApiKeyID:         1,
 				IsServiceAccount: false,
 			},
-			expected:    "rbac-permissions-1-apikey-1",
+			expected: "rbac-permissions-1-api-key-1",
-			expectedErr: nil,
 		},
 		{
 			name: "should return correct key for service account",
@@ -723,8 +721,7 @@ func TestPermissionCacheKey(t *testing.T) {
 				UserID:           1,
 				IsServiceAccount: true,
 			},
-			expected:    "rbac-permissions-1-service-1",
+			expected: "rbac-permissions-1-service-account-1",
-			expectedErr: nil,
 		},
 		{
 			name: "should return correct key for matching a service account with userId -1",
@@ -733,24 +730,20 @@ func TestPermissionCacheKey(t *testing.T) {
 				UserID:           -1,
 				IsServiceAccount: true,
 			},
-			expected:    "rbac-permissions-1-service--1",
+			expected: "rbac-permissions-1-service-account--1",
-			expectedErr: nil,
 		},
 		{
-			name: "should return error if not matching any",
+			name: "should use org role if no unique id",
 			signedInUser: &user.SignedInUser{
-				OrgID:  1,
+				OrgID:   1,
-				UserID: -1,
+				OrgRole: org.RoleNone,
 			},
-			expected:    "",
+			expected: "rbac-permissions-1-user-None",
-			expectedErr: user.ErrNoUniqueID,
 		},
 	}
 	for _, tc := range testcases {
 		t.Run(tc.name, func(t *testing.T) {
-			str, err := permissionCacheKey(tc.signedInUser)
+			assert.Equal(t, tc.expected, permissionCacheKey(tc.signedInUser))
-			require.Equal(t, tc.expectedErr, err)
-			assert.Equal(t, tc.expected, str)
 		})
 	}
 }

pkg/services/auth/identity/requester.go

@@ -57,7 +57,7 @@ type Requester interface {
 	HasRole(role roletype.RoleType) bool
 	// GetCacheKey returns a unique key for the entity.
 	// Add an extra prefix to avoid collisions with other caches
-	GetCacheKey() (string, error)
+	GetCacheKey() string
 	// HasUniqueId returns true if the entity has a unique id
 	HasUniqueId() bool
 	// AuthenticatedBy returns the authentication method used to authenticate the entity.

pkg/services/authn/authn.go

@@ -180,6 +180,8 @@ const (
 	NamespaceUser           = identity.NamespaceUser
 	NamespaceAPIKey         = identity.NamespaceAPIKey
 	NamespaceServiceAccount = identity.NamespaceServiceAccount
+	NamespaceAnonymous      = identity.NamespaceAnonymous
+	NamespaceRenderService  = identity.NamespaceRenderService
 )
 
 type Identity struct {

pkg/services/user/identity.go

@@ -105,17 +105,15 @@ func (u *SignedInUser) HasUniqueId() bool {
 
 // GetCacheKey returns a unique key for the entity.
 // Add an extra prefix to avoid collisions with other caches
-func (u *SignedInUser) GetCacheKey() (string, error) {
+func (u *SignedInUser) GetCacheKey() string {
-	if u.IsRealUser() {
+	namespace, id := u.GetNamespacedID()
-		return fmt.Sprintf("%d-user-%d", u.OrgID, u.UserID), nil
+	if !u.HasUniqueId() {
+		// Hack use the org role as id for identities that do not have a unique id
+		// e.g. anonymous and render key.
+		id = string(u.GetOrgRole())
 	}
-	if u.IsApiKeyUser() {
+
-		return fmt.Sprintf("%d-apikey-%d", u.OrgID, u.ApiKeyID), nil
+	return fmt.Sprintf("%d-%s-%s", u.GetOrgID(), namespace, id)
-	}
-	if u.IsServiceAccountUser() { // not considered a real user
-		return fmt.Sprintf("%d-service-%d", u.OrgID, u.UserID), nil
-	}
-	return "", ErrNoUniqueID
 }
 
 // GetIsGrafanaAdmin returns true if the user is a server admin
@@ -161,6 +159,9 @@ func (u *SignedInUser) GetTeams() []int64 {
 
 // GetOrgRole returns the role of the active entity in the active organization
 func (u *SignedInUser) GetOrgRole() roletype.RoleType {
+	if u.OrgRole == "" {
+		return roletype.RoleNone
+	}
 	return u.OrgRole
 }
 
@@ -178,7 +179,7 @@ func (u *SignedInUser) GetNamespacedID() (string, string) {
 		return identity.NamespaceAnonymous, ""
 	case u.AuthenticatedBy == "render": //import cycle render
 		if u.UserID == 0 {
-			return identity.NamespaceRenderService, fmt.Sprintf("%d", u.UserID)
+			return identity.NamespaceRenderService, "0"
 		} else { // this should never happen as u.UserID > 0 already catches this
 			return identity.NamespaceUser, fmt.Sprintf("%d", u.UserID)
 		}