From 23d8f7c2fe6b0b1d87e41daf7ad85e72c4a78662 Mon Sep 17 00:00:00 2001
From: Misi <mgyongyosi@users.noreply.github.com>
Date: Wed, 10 May 2023 11:24:37 +0200
Subject: [PATCH] RBAC: Fix SearchUsersPermissions when the filter is empty
 (#68176)

Fix SearchUsersPermission action filter
---
 pkg/services/accesscontrol/acimpl/service.go  | 11 ++-----
 .../accesscontrol/acimpl/service_test.go      | 29 ++++++++++++++++++-
 2 files changed, 30 insertions(+), 10 deletions(-)

diff --git a/pkg/services/accesscontrol/acimpl/service.go b/pkg/services/accesscontrol/acimpl/service.go
index e2bcfd3fc94..479a5941a9a 100644
--- a/pkg/services/accesscontrol/acimpl/service.go
+++ b/pkg/services/accesscontrol/acimpl/service.go
@@ -252,15 +252,8 @@ func (s *Service) SearchUsersPermissions(ctx context.Context, user *user.SignedI
 	basicPermissions := map[string][]accesscontrol.Permission{}
 	for role, basicRole := range s.roles {
 		for i := range basicRole.Permissions {
-			if options.ActionPrefix != "" {
-				if strings.HasPrefix(basicRole.Permissions[i].Action, options.ActionPrefix) {
-					basicPermissions[role] = append(basicPermissions[role], basicRole.Permissions[i])
-				}
-			}
-			if options.Action != "" {
-				if basicRole.Permissions[i].Action == options.Action {
-					basicPermissions[role] = append(basicPermissions[role], basicRole.Permissions[i])
-				}
+			if PermissionMatchesSearchOptions(basicRole.Permissions[i], options) {
+				basicPermissions[role] = append(basicPermissions[role], basicRole.Permissions[i])
 			}
 		}
 	}
diff --git a/pkg/services/accesscontrol/acimpl/service_test.go b/pkg/services/accesscontrol/acimpl/service_test.go
index f4fce35c319..6da1b4bbcef 100644
--- a/pkg/services/accesscontrol/acimpl/service_test.go
+++ b/pkg/services/accesscontrol/acimpl/service_test.go
@@ -384,6 +384,7 @@ func TestService_SearchUsersPermissions(t *testing.T) {
 	tests := []struct {
 		name           string
 		siuPermissions map[string][]string
+		searchOption   accesscontrol.SearchOptions
 		ramRoles       map[string]*accesscontrol.RoleDTO    // BasicRole => RBAC BasicRole
 		storedPerms    map[int64][]accesscontrol.Permission // UserID => Permissions
 		storedRoles    map[int64][]string                   // UserID => Roles
@@ -393,6 +394,7 @@ func TestService_SearchUsersPermissions(t *testing.T) {
 		{
 			name:           "ram only",
 			siuPermissions: listAllPerms,
+			searchOption:   searchOption,
 			ramRoles: map[string]*accesscontrol.RoleDTO{
 				string(roletype.RoleAdmin): {Permissions: []accesscontrol.Permission{
 					{Action: accesscontrol.ActionTeamsRead, Scope: "teams:*"},
@@ -413,6 +415,7 @@ func TestService_SearchUsersPermissions(t *testing.T) {
 		{
 			name:           "stored only",
 			siuPermissions: listAllPerms,
+			searchOption:   searchOption,
 			storedPerms: map[int64][]accesscontrol.Permission{
 				1: {{Action: accesscontrol.ActionTeamsRead, Scope: "teams:id:1"}},
 				2: {{Action: accesscontrol.ActionTeamsRead, Scope: "teams:*"},
@@ -431,6 +434,7 @@ func TestService_SearchUsersPermissions(t *testing.T) {
 		{
 			name:           "ram and stored",
 			siuPermissions: listAllPerms,
+			searchOption:   searchOption,
 			ramRoles: map[string]*accesscontrol.RoleDTO{
 				string(roletype.RoleAdmin): {Permissions: []accesscontrol.Permission{
 					{Action: accesscontrol.ActionTeamsRead, Scope: "teams:*"},
@@ -459,6 +463,7 @@ func TestService_SearchUsersPermissions(t *testing.T) {
 		{
 			name:           "view permission on subset of users only",
 			siuPermissions: listSomePerms,
+			searchOption:   searchOption,
 			ramRoles: map[string]*accesscontrol.RoleDTO{
 				accesscontrol.RoleGrafanaAdmin: {Permissions: []accesscontrol.Permission{
 					{Action: accesscontrol.ActionTeamsPermissionsRead, Scope: "teams:*"},
@@ -482,6 +487,7 @@ func TestService_SearchUsersPermissions(t *testing.T) {
 		{
 			name:           "check action filter on RAM permissions works correctly",
 			siuPermissions: listAllPerms,
+			searchOption:   searchOption,
 			ramRoles: map[string]*accesscontrol.RoleDTO{
 				accesscontrol.RoleGrafanaAdmin: {Permissions: []accesscontrol.Permission{
 					{Action: accesscontrol.ActionUsersCreate},
@@ -493,6 +499,27 @@ func TestService_SearchUsersPermissions(t *testing.T) {
 				1: {{Action: accesscontrol.ActionTeamsPermissionsRead, Scope: "teams:*"}},
 			},
 		},
+		{
+			name:           "check empty action filter on RAM permissions works correctly",
+			siuPermissions: listAllPerms,
+			searchOption:   accesscontrol.SearchOptions{},
+			ramRoles: map[string]*accesscontrol.RoleDTO{
+				accesscontrol.RoleGrafanaAdmin: {Permissions: []accesscontrol.Permission{
+					{Action: accesscontrol.ActionTeamsRead, Scope: "teams:*"},
+					{Action: accesscontrol.ActionUsersCreate},
+					{Action: accesscontrol.ActionTeamsPermissionsRead, Scope: "teams:*"},
+					{Action: accesscontrol.ActionAnnotationsRead, Scope: "annotations:*"},
+				}},
+			},
+			storedRoles: map[int64][]string{1: {accesscontrol.RoleGrafanaAdmin}},
+			want: map[int64][]accesscontrol.Permission{
+				1: {{Action: accesscontrol.ActionTeamsRead, Scope: "teams:*"},
+					{Action: accesscontrol.ActionUsersCreate},
+					{Action: accesscontrol.ActionTeamsPermissionsRead, Scope: "teams:*"},
+					{Action: accesscontrol.ActionAnnotationsRead, Scope: "annotations:*"},
+				},
+			},
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -505,7 +532,7 @@ func TestService_SearchUsersPermissions(t *testing.T) {
 			}
 
 			siu := &user.SignedInUser{OrgID: 2, Permissions: map[int64]map[string][]string{2: tt.siuPermissions}}
-			got, err := ac.SearchUsersPermissions(ctx, siu, 2, searchOption)
+			got, err := ac.SearchUsersPermissions(ctx, siu, 2, tt.searchOption)
 			if tt.wantErr {
 				require.NotNil(t, err)
 				return