RBAC: Adjust filter for acl list to check for permissions on service accounts (#78681)

Adjust filter to check for permissions on service accounts

pkg/services/accesscontrol/resourcepermissions/store.go

@@ -10,6 +10,7 @@ import (
 	"github.com/grafana/grafana/pkg/services/accesscontrol"
 	"github.com/grafana/grafana/pkg/services/featuremgmt"
 	"github.com/grafana/grafana/pkg/services/org"
+	"github.com/grafana/grafana/pkg/services/serviceaccounts"
 	"github.com/grafana/grafana/pkg/services/team"
 	"github.com/grafana/grafana/pkg/services/user"
 	"github.com/grafana/grafana/pkg/util"
@@ -391,8 +392,19 @@ func (s *store) getResourcePermissions(sess *db.Session, orgID int64, query GetR
 		if err != nil {
 			return nil, err
 		}
-		userQuery += " AND " + userFilter.Where
+
+		filter := "(" + userFilter.Where + " AND NOT u.is_service_account)"
+
+		saFilter, err := accesscontrol.Filter(query.User, "u.id", "serviceaccounts:id:", serviceaccounts.ActionRead)
+		if err != nil {
+			return nil, err
+		}
+
+		filter += " OR (" + saFilter.Where + " AND u.is_service_account)"
+
+		userQuery += " AND " + filter
 		args = append(args, userFilter.Args...)
+		args = append(args, saFilter.Args...)
 	}
 
 	teamFilter, err := accesscontrol.Filter(query.User, "t.id", "teams:id:", accesscontrol.ActionTeamsRead)

pkg/services/accesscontrol/resourcepermissions/store_test.go

@@ -16,6 +16,7 @@ import (
 	"github.com/grafana/grafana/pkg/services/org"
 	"github.com/grafana/grafana/pkg/services/org/orgimpl"
 	"github.com/grafana/grafana/pkg/services/quota/quotatest"
+	"github.com/grafana/grafana/pkg/services/serviceaccounts"
 	"github.com/grafana/grafana/pkg/services/sqlstore"
 	"github.com/grafana/grafana/pkg/services/supportbundles/supportbundlestest"
 	"github.com/grafana/grafana/pkg/services/user"
@@ -355,11 +356,12 @@ func TestIntegrationStore_SetResourcePermissions(t *testing.T) {
 }
 
 type getResourcePermissionsTest struct {
-	desc        string
+	desc               string
-	user        *user.SignedInUser
+	user               *user.SignedInUser
-	numUsers    int
+	numUsers           int
-	query       GetResourcePermissionsQuery
+	numServiceAccounts int
-	expectedLen int
+	query              GetResourcePermissionsQuery
+	expectedLen        int
 }
 
 func TestIntegrationStore_GetResourcePermissions(t *testing.T) {
@@ -420,6 +422,28 @@ func TestIntegrationStore_GetResourcePermissions(t *testing.T) {
 			},
 			expectedLen: 2,
 		},
+		{
+			desc: "should return users and service accounts caller can read",
+			user: &user.SignedInUser{
+				OrgID: 1,
+				Permissions: map[int64]map[string][]string{
+					1: {
+						accesscontrol.ActionOrgUsersRead: {"users:id:1", "users:id:2", "users:id:3"},
+						serviceaccounts.ActionRead:       {"serviceaccounts:id:5"},
+					},
+				}},
+			numUsers:           3,
+			numServiceAccounts: 3,
+			query: GetResourcePermissionsQuery{
+				Actions:              []string{"datasources:query"},
+				Resource:             "datasources",
+				ResourceID:           "1",
+				ResourceAttribute:    "uid",
+				OnlyManaged:          true,
+				EnforceAccessControl: true,
+			},
+			expectedLen: 4,
+		},
 		{
 			desc: "should return permissions for all users when access control is not enforces",
 			user: &user.SignedInUser{
@@ -479,7 +503,7 @@ func TestIntegrationStore_GetResourcePermissions(t *testing.T) {
 			})
 			require.NoError(t, err)
 
-			seedResourcePermissions(t, store, sql, orgService, tt.query.Actions, tt.query.Resource, tt.query.ResourceID, tt.query.ResourceAttribute, tt.numUsers)
+			seedResourcePermissions(t, store, sql, orgService, tt.query.Actions, tt.query.Resource, tt.query.ResourceID, tt.query.ResourceAttribute, tt.numUsers, tt.numServiceAccounts)
 
 			tt.query.User = tt.user
 			permissions, err := store.GetResourcePermissions(context.Background(), tt.user.OrgID, tt.query)
@@ -489,27 +513,28 @@ func TestIntegrationStore_GetResourcePermissions(t *testing.T) {
 	}
 }
 
-func seedResourcePermissions(t *testing.T, store *store, sql *sqlstore.SQLStore, orgService org.Service, actions []string, resource, resourceID, resourceAttribute string, numUsers int) {
+func seedResourcePermissions(
+	t *testing.T, store *store, sql *sqlstore.SQLStore, orgService org.Service,
+	actions []string, resource, resourceID, resourceAttribute string, numUsers, numServiceAccounts int,
+) {
 	t.Helper()
-	var orgModel *org.Org
+
+	orgID, err := orgService.GetOrCreate(context.Background(), "test")
+	require.NoError(t, err)
+
 	usrSvc, err := userimpl.ProvideService(sql, orgService, sql.Cfg, nil, nil, quotatest.New(false, nil), supportbundlestest.NewFakeBundleService())
 	require.NoError(t, err)
 
-	for i := 0; i < numUsers; i++ {
+	create := func(login string, isServiceAccount bool) {
-		if orgModel == nil {
-			cmd := &org.CreateOrgCommand{Name: "test", UserID: int64(i)}
-			addedOrg, err := orgService.CreateWithMember(context.Background(), cmd)
-			require.NoError(t, err)
-			orgModel = addedOrg
-		}
-
 		u, err := usrSvc.Create(context.Background(), &user.CreateUserCommand{
-			Login: fmt.Sprintf("user:%s%d", resourceID, i),
+			Login:            login,
-			OrgID: orgModel.ID,
+			IsServiceAccount: isServiceAccount,
+			OrgID:            orgID,
 		})
+
 		require.NoError(t, err)
 
-		_, err = store.SetUserResourcePermission(context.Background(), 1, accesscontrol.User{ID: u.ID}, SetResourcePermissionCommand{
+		_, err = store.SetUserResourcePermission(context.Background(), orgID, accesscontrol.User{ID: u.ID}, SetResourcePermissionCommand{
 			Actions:           actions,
 			Resource:          resource,
 			ResourceID:        resourceID,
@@ -517,6 +542,14 @@ func seedResourcePermissions(t *testing.T, store *store, sql *sqlstore.SQLStore,
 		}, nil)
 		require.NoError(t, err)
 	}
+
+	for i := 0; i < numUsers; i++ {
+		create(fmt.Sprintf("user:%s:%d", resourceID, i), false)
+	}
+
+	for i := 0; i < numServiceAccounts; i++ {
+		create(fmt.Sprintf("sa:%s:%d", resourceID, i), true)
+	}
 }
 
 func setupTestEnv(t testing.TB) (*store, *sqlstore.SQLStore) {