RBAC Search: Replace userLogin filter by namespacedID filter (#81810)

* Add namespace ID

* Refactor and add tests

* Rename maxOneOption -> atMostOneOption

* Add ToDo

* Remove UserLogin & UserID for NamespaceID

Co-authored-by: jguer <joao.guerreiro@grafana.com>

* Remove unecessary import of the userSvc

* Update pkg/services/accesscontrol/acimpl/service.go

* fix 1 -> userID

* Update pkg/services/accesscontrol/accesscontrol.go

---------

Co-authored-by: jguer <joao.guerreiro@grafana.com>

pkg/services/accesscontrol/acimpl/service.go

@@ -42,8 +42,8 @@ var SharedWithMeFolderPermission = accesscontrol.Permission{
 }
 
 func ProvideService(cfg *setting.Cfg, db db.DB, routeRegister routing.RouteRegister, cache *localcache.CacheService,
-	accessControl accesscontrol.AccessControl, userSvc user.Service, features featuremgmt.FeatureToggles) (*Service, error) {
-	service := ProvideOSSService(cfg, database.ProvideService(db), cache, userSvc, features)
+	accessControl accesscontrol.AccessControl, features featuremgmt.FeatureToggles) (*Service, error) {
+	service := ProvideOSSService(cfg, database.ProvideService(db), cache, features)
 
 	api.NewAccessControlAPI(routeRegister, accessControl, service, features).RegisterAPIEndpoints()
 	if err := accesscontrol.DeclareFixedRoles(service, cfg); err != nil {
@@ -61,7 +61,7 @@ func ProvideService(cfg *setting.Cfg, db db.DB, routeRegister routing.RouteRegis
 	return service, nil
 }
 
-func ProvideOSSService(cfg *setting.Cfg, store store, cache *localcache.CacheService, userSvc user.Service, features featuremgmt.FeatureToggles) *Service {
+func ProvideOSSService(cfg *setting.Cfg, store store, cache *localcache.CacheService, features featuremgmt.FeatureToggles) *Service {
 	s := &Service{
 		cache:    cache,
 		cfg:      cfg,
@@ -69,7 +69,6 @@ func ProvideOSSService(cfg *setting.Cfg, store store, cache *localcache.CacheSer
 		log:      log.New("accesscontrol.service"),
 		roles:    accesscontrol.BuildBasicRoleDefinitions(),
 		store:    store,
-		userSvc:  userSvc,
 	}
 
 	return s
@@ -94,7 +93,6 @@ type Service struct {
 	registrations accesscontrol.RegistrationList
 	roles         map[string]*accesscontrol.RoleDTO
 	store         store
-	userSvc       user.Service
 }
 
 func (s *Service) GetUsageStats(_ context.Context) map[string]any {
@@ -245,21 +243,20 @@ func (s *Service) DeclarePluginRoles(ctx context.Context, ID, name string, regs
 // SearchUsersPermissions returns all users' permissions filtered by action prefixes
 func (s *Service) SearchUsersPermissions(ctx context.Context, usr identity.Requester,
 	options accesscontrol.SearchOptions) (map[int64][]accesscontrol.Permission, error) {
-	if options.UserLogin != "" {
-		// Resolve userLogin -> userID
-		if err := options.ResolveUserLogin(ctx, s.userSvc); err != nil {
+	if options.NamespacedID != "" {
+		userID, err := options.ComputeUserID()
+		if err != nil {
+			s.log.Error("Failed to resolve user ID", "error", err)
 			return nil, err
 		}
-		options.UserLogin = ""
-	}
-	if options.UserID > 0 {
+
 		// Reroute to the user specific implementation of search permissions
 		// because it leverages the user permission cache.
 		userPerms, err := s.SearchUserPermissions(ctx, usr.GetOrgID(), options)
 		if err != nil {
 			return nil, err
 		}
-		return map[int64][]accesscontrol.Permission{options.UserID: userPerms}, nil
+		return map[int64][]accesscontrol.Permission{userID: userPerms}, nil
 	}
 
 	timer := prometheus.NewTimer(metrics.MAccessSearchPermissionsSummary)
@@ -346,15 +343,8 @@ func (s *Service) SearchUserPermissions(ctx context.Context, orgID int64, search
 	timer := prometheus.NewTimer(metrics.MAccessPermissionsSummary)
 	defer timer.ObserveDuration()
 
-	if searchOptions.UserLogin != "" {
-		// Resolve userLogin -> userID
-		if err := searchOptions.ResolveUserLogin(ctx, s.userSvc); err != nil {
-			return nil, err
-		}
-	}
-
-	if searchOptions.UserID == 0 {
-		return nil, fmt.Errorf("expected user ID or login to be specified")
+	if searchOptions.NamespacedID == "" {
+		return nil, fmt.Errorf("expected namespaced ID to be specified")
 	}
 
 	if permissions, success := s.searchUserPermissionsFromCache(orgID, searchOptions); success {
@@ -364,15 +354,20 @@ func (s *Service) SearchUserPermissions(ctx context.Context, orgID int64, search
 }
 
 func (s *Service) searchUserPermissions(ctx context.Context, orgID int64, searchOptions accesscontrol.SearchOptions) ([]accesscontrol.Permission, error) {
+	userID, err := searchOptions.ComputeUserID()
+	if err != nil {
+		return nil, err
+	}
+
 	// Get permissions for user's basic roles from RAM
-	roleList, err := s.store.GetUsersBasicRoles(ctx, []int64{searchOptions.UserID}, orgID)
+	roleList, err := s.store.GetUsersBasicRoles(ctx, []int64{userID}, orgID)
 	if err != nil {
 		return nil, fmt.Errorf("could not fetch basic roles for the user: %w", err)
 	}
 	var roles []string
 	var ok bool
-	if roles, ok = roleList[searchOptions.UserID]; !ok {
-		return nil, fmt.Errorf("found no basic roles for user %d in organisation %d", searchOptions.UserID, orgID)
+	if roles, ok = roleList[userID]; !ok {
+		return nil, fmt.Errorf("found no basic roles for user %d in organisation %d", userID, orgID)
 	}
 	permissions := make([]accesscontrol.Permission, 0)
 	for _, builtin := range roles {
@@ -390,15 +385,20 @@ func (s *Service) searchUserPermissions(ctx context.Context, orgID int64, search
 	if err != nil {
 		return nil, err
 	}
-	permissions = append(permissions, dbPermissions[searchOptions.UserID]...)
+	permissions = append(permissions, dbPermissions[userID]...)
 
 	return permissions, nil
 }
 
 func (s *Service) searchUserPermissionsFromCache(orgID int64, searchOptions accesscontrol.SearchOptions) ([]accesscontrol.Permission, bool) {
+	userID, err := searchOptions.ComputeUserID()
+	if err != nil {
+		return nil, false
+	}
+
 	// Create a temp signed in user object to retrieve cache key
 	tempUser := &user.SignedInUser{
-		UserID: searchOptions.UserID,
+		UserID: userID,
 		OrgID:  orgID,
 	}
 

pkg/services/accesscontrol/acimpl/service_test.go

@@ -15,11 +15,11 @@ import (
 	"github.com/grafana/grafana/pkg/services/accesscontrol"
 	"github.com/grafana/grafana/pkg/services/accesscontrol/actest"
 	"github.com/grafana/grafana/pkg/services/accesscontrol/database"
+	"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/services/user/usertest"
 	"github.com/grafana/grafana/pkg/setting"
 	"github.com/grafana/grafana/pkg/tests/testsuite"
 )
@@ -40,7 +40,6 @@ func setupTestEnv(t testing.TB) *Service {
 		registrations: accesscontrol.RegistrationList{},
 		roles:         accesscontrol.BuildBasicRoleDefinitions(),
 		store:         database.ProvideService(db.InitTestDB(t)),
-		userSvc:       usertest.NewUserServiceFake(),
 	}
 	require.NoError(t, ac.RegisterFixedRoles(context.Background()))
 	return ac
@@ -65,7 +64,6 @@ func TestUsageMetrics(t *testing.T) {
 				cfg,
 				database.ProvideService(db.InitTestDB(t)),
 				localcache.ProvideService(),
-				usertest.NewUserServiceFake(),
 				featuremgmt.WithFeatures(),
 			)
 			assert.Equal(t, tt.expectedValue, s.GetUsageStats(context.Background())["stats.oss.accesscontrol.enabled.count"])
@@ -537,9 +535,9 @@ func TestService_SearchUsersPermissions(t *testing.T) {
 		{
 			// This test is not exactly representative as normally the store would return
 			// only the user's basic roles and the user's stored permissions
-			name:           "check userID filter works correctly",
+			name:           "check namespacedId filter works correctly",
 			siuPermissions: listAllPerms,
-			searchOption:   accesscontrol.SearchOptions{UserID: 1},
+			searchOption:   accesscontrol.SearchOptions{NamespacedID: identity.NamespaceServiceAccount + ":1"},
 			ramRoles: map[string]*accesscontrol.RoleDTO{
 				string(roletype.RoleEditor): {Permissions: []accesscontrol.Permission{
 					{Action: accesscontrol.ActionTeamsRead, Scope: "teams:*"},
@@ -564,47 +562,11 @@ func TestService_SearchUsersPermissions(t *testing.T) {
 				1: {{Action: accesscontrol.ActionTeamsRead, Scope: "teams:id:1"}, {Action: accesscontrol.ActionTeamsRead, Scope: "teams:*"}},
 			},
 		},
-		{
-			// This test is not exactly representative as normally the store would return
-			// only the user's basic roles and the user's stored permissions
-			name:           "check userLogin filter works correctly",
-			siuPermissions: listAllPerms,
-			searchOption:   accesscontrol.SearchOptions{UserLogin: "testUser"},
-			ramRoles: map[string]*accesscontrol.RoleDTO{
-				string(roletype.RoleEditor): {Permissions: []accesscontrol.Permission{
-					{Action: accesscontrol.ActionTeamsRead, Scope: "teams:*"},
-				}},
-				string(roletype.RoleAdmin): {Permissions: []accesscontrol.Permission{
-					{Action: accesscontrol.ActionTeamsWrite, Scope: "teams:*"},
-				}},
-				accesscontrol.RoleGrafanaAdmin: {Permissions: []accesscontrol.Permission{
-					{Action: accesscontrol.ActionTeamsPermissionsRead, Scope: "teams:*"},
-				}},
-			},
-			storedPerms: map[int64][]accesscontrol.Permission{
-				1: {{Action: accesscontrol.ActionTeamsRead, Scope: "teams:id:1"}},
-				2: {{Action: accesscontrol.ActionTeamsRead, Scope: "teams:id:1"},
-					{Action: accesscontrol.ActionTeamsPermissionsRead, Scope: "teams:id:1"}},
-			},
-			storedRoles: map[int64][]string{
-				1: {string(roletype.RoleEditor)},
-				2: {string(roletype.RoleAdmin), accesscontrol.RoleGrafanaAdmin},
-			},
-			want: map[int64][]accesscontrol.Permission{
-				2: {{Action: accesscontrol.ActionTeamsWrite, Scope: "teams:*"},
-					{Action: accesscontrol.ActionTeamsRead, Scope: "teams:id:1"},
-					{Action: accesscontrol.ActionTeamsPermissionsRead, Scope: "teams:id:1"},
-					{Action: accesscontrol.ActionTeamsPermissionsRead, Scope: "teams:*"}},
-			},
-		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			ac := setupTestEnv(t)
 
-			// Resolve user login to id 2
-			ac.userSvc = &usertest.FakeUserService{ExpectedUser: &user.User{ID: 2}}
-
 			ac.roles = tt.ramRoles
 			ac.store = actest.FakeStore{
 				ExpectedUsersPermissions: tt.storedPerms,
@@ -645,7 +607,7 @@ func TestService_SearchUserPermissions(t *testing.T) {
 			name: "ram only",
 			searchOption: accesscontrol.SearchOptions{
 				ActionPrefix: "teams",
-				UserID:       2,
+				NamespacedID: identity.NamespaceUser + ":2",
 			},
 			ramRoles: map[string]*accesscontrol.RoleDTO{
 				string(roletype.RoleEditor): {Permissions: []accesscontrol.Permission{
@@ -670,7 +632,7 @@ func TestService_SearchUserPermissions(t *testing.T) {
 			name: "stored only",
 			searchOption: accesscontrol.SearchOptions{
 				ActionPrefix: "teams",
-				UserID:       2,
+				NamespacedID: identity.NamespaceUser + ":2",
 			},
 			storedPerms: map[int64][]accesscontrol.Permission{
 				1: {{Action: accesscontrol.ActionTeamsRead, Scope: "teams:id:1"}},
@@ -690,7 +652,7 @@ func TestService_SearchUserPermissions(t *testing.T) {
 			name: "ram and stored",
 			searchOption: accesscontrol.SearchOptions{
 				ActionPrefix: "teams",
-				UserID:       2,
+				NamespacedID: identity.NamespaceUser + ":2",
 			},
 			ramRoles: map[string]*accesscontrol.RoleDTO{
 				string(roletype.RoleAdmin): {Permissions: []accesscontrol.Permission{
@@ -720,7 +682,7 @@ func TestService_SearchUserPermissions(t *testing.T) {
 			name: "check action prefix filter works correctly",
 			searchOption: accesscontrol.SearchOptions{
 				ActionPrefix: "teams",
-				UserID:       1,
+				NamespacedID: identity.NamespaceUser + ":1",
 			},
 			ramRoles: map[string]*accesscontrol.RoleDTO{
 				string(roletype.RoleEditor): {Permissions: []accesscontrol.Permission{
@@ -741,8 +703,8 @@ func TestService_SearchUserPermissions(t *testing.T) {
 		{
 			name: "check action filter works correctly",
 			searchOption: accesscontrol.SearchOptions{
-				Action: accesscontrol.ActionTeamsRead,
-				UserID: 1,
+				Action:       accesscontrol.ActionTeamsRead,
+				NamespacedID: identity.NamespaceUser + ":1",
 			},
 			ramRoles: map[string]*accesscontrol.RoleDTO{
 				string(roletype.RoleEditor): {Permissions: []accesscontrol.Permission{