From cfba630f5cab358cd9ce33ba249ba80d5377647f Mon Sep 17 00:00:00 2001
From: Alexander Zobnin <alexanderzobnin@gmail.com>
Date: Tue, 20 May 2025 09:28:46 +0200
Subject: [PATCH] RBAC: Don't additionally cache all users permissions
 (#105607)

* RBAC: Don't additionally cache all users permissions

* remove unused tests
---
 pkg/services/accesscontrol/acimpl/service.go  |  8 ---
 pkg/services/accesscontrol/cacheutils.go      |  4 --
 pkg/services/accesscontrol/cacheutils_test.go | 66 -------------------
 3 files changed, 78 deletions(-)

diff --git a/pkg/services/accesscontrol/acimpl/service.go b/pkg/services/accesscontrol/acimpl/service.go
index ece482bdd2b..9f489a79dac 100644
--- a/pkg/services/accesscontrol/acimpl/service.go
+++ b/pkg/services/accesscontrol/acimpl/service.go
@@ -243,11 +243,6 @@ func (s *Service) getCachedUserPermissions(ctx context.Context, user identity.Re
 	ctx, span := tracer.Start(ctx, "accesscontrol.acimpl.getCachedUserPermissions")
 	defer span.End()
 
-	cacheKey := accesscontrol.GetUserPermissionCacheKey(user)
-	if cachedPermissions, ok := s.cache.Get(cacheKey); ok {
-		return cachedPermissions.([]accesscontrol.Permission), nil
-	}
-
 	permissions, err := s.getCachedBasicRolesPermissions(ctx, user, options)
 	if err != nil {
 		return nil, err
@@ -263,9 +258,7 @@ func (s *Service) getCachedUserPermissions(ctx context.Context, user identity.Re
 	if err != nil {
 		return nil, err
 	}
-
 	permissions = append(permissions, userManagedPermissions...)
-	s.cache.Set(cacheKey, permissions, cacheTTL)
 	span.SetAttributes(attribute.Int("num_permissions", len(permissions)))
 
 	return permissions, nil
@@ -390,7 +383,6 @@ func (s *Service) getCachedTeamsPermissions(ctx context.Context, user identity.R
 }
 
 func (s *Service) ClearUserPermissionCache(user identity.Requester) {
-	s.cache.Delete(accesscontrol.GetUserPermissionCacheKey(user))
 	s.cache.Delete(accesscontrol.GetUserDirectPermissionCacheKey(user))
 }
 
diff --git a/pkg/services/accesscontrol/cacheutils.go b/pkg/services/accesscontrol/cacheutils.go
index a3fc6d499b3..cd67c358ac6 100644
--- a/pkg/services/accesscontrol/cacheutils.go
+++ b/pkg/services/accesscontrol/cacheutils.go
@@ -30,10 +30,6 @@ func (s *SearchOptions) HashString() (string, error) {
 	return base64.StdEncoding.EncodeToString(h.Sum(nil)), nil
 }
 
-func GetUserPermissionCacheKey(user identity.Requester) string {
-	return fmt.Sprintf("rbac-permissions-%s", user.GetCacheKey())
-}
-
 func GetSearchPermissionCacheKey(log log.Logger, user identity.Requester, searchOptions SearchOptions) (string, error) {
 	searchHash, err := searchOptions.HashString()
 	if err != nil {
diff --git a/pkg/services/accesscontrol/cacheutils_test.go b/pkg/services/accesscontrol/cacheutils_test.go
index 7180c347550..2614204440e 100644
--- a/pkg/services/accesscontrol/cacheutils_test.go
+++ b/pkg/services/accesscontrol/cacheutils_test.go
@@ -6,78 +6,12 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
-	claims "github.com/grafana/authlib/types"
-
 	"github.com/grafana/grafana/pkg/infra/log"
-	"github.com/grafana/grafana/pkg/services/org"
 	"github.com/grafana/grafana/pkg/services/user"
 )
 
 var testLogger = log.New("test")
 
-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,
-				FallbackType: claims.TypeUser,
-			},
-			expected: "rbac-permissions-1-user-1",
-		},
-		{
-			name: "should return correct key for api key",
-			signedInUser: &user.SignedInUser{
-				OrgID:            1,
-				ApiKeyID:         1,
-				IsServiceAccount: false,
-				FallbackType:     claims.TypeUser,
-			},
-			expected: "rbac-permissions-1-api-key-1",
-		},
-		{
-			name: "should return correct key for service account",
-			signedInUser: &user.SignedInUser{
-				OrgID:            1,
-				UserID:           1,
-				IsServiceAccount: true,
-				FallbackType:     claims.TypeUser,
-			},
-			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,
-				FallbackType:     claims.TypeUser, // NOTE, this is still a service account!
-			},
-			expected: "rbac-permissions-1-service-account--1",
-		},
-		{
-			name: "should use org role if no unique id",
-			signedInUser: &user.SignedInUser{
-				OrgID:        1,
-				OrgRole:      org.RoleNone,
-				FallbackType: claims.TypeUser,
-			},
-			expected: "rbac-permissions-1-user-None",
-		},
-	}
-
-	for _, tc := range testcases {
-		t.Run(tc.name, func(t *testing.T) {
-			assert.Equal(t, tc.expected, GetUserPermissionCacheKey(tc.signedInUser))
-		})
-	}
-}
-
 func TestGetSearchPermissionCacheKey(t *testing.T) {
 	keyInputs := []struct {
 		signedInUser  *user.SignedInUser