Service accounts: Add service account to teams (#51536)

* Revert "Serviceaccounts: #48995

Do not display service accounts assigned to team (#48995)"

This reverts commit cbf71fbd7f.

* fix: test to not include more actions than necessary

* adding service accounts to teams - backend and frontend changes

* also support SA addition through the old team membership endpoints

* fix tests

* tests

* serviceaccounts permission tests

* serviceaccounts permission service tests run

* added back test that was removed by accident

* lint

* refactor: add testoptionsTeams

* fix a bug

* service account picker change

* explicitly set SA managed permissions to false for dash and folders

* lint

* allow team creator to list service accounts

Co-authored-by: IevaVasiljeva <ieva.vasiljeva@grafana.com>

pkg/services/sqlstore/team.go

@@ -9,6 +9,7 @@ import (
 
 	"github.com/grafana/grafana/pkg/models"
 	ac "github.com/grafana/grafana/pkg/services/accesscontrol"
+	"github.com/grafana/grafana/pkg/services/serviceaccounts"
 )
 
 type TeamStore interface {
@@ -528,17 +529,25 @@ func (ss *SQLStore) GetTeamMembers(ctx context.Context, query *models.GetTeamMem
 	// If the signed in user is not set no member will be returned
 	if !ac.IsDisabled(ss.Cfg) {
 		sqlID := fmt.Sprintf("%s.%s", ss.engine.Dialect().Quote("user"), ss.engine.Dialect().Quote("id"))
+		var filter ac.SQLFilter
 		*acFilter, err = ac.Filter(query.SignedInUser, sqlID, "users:id:", ac.ActionOrgUsersRead)
 		if err != nil {
 			return err
 		}
+
+		filter, err = ac.Filter(query.SignedInUser, sqlID, "serviceaccounts:id:", serviceaccounts.ActionRead)
+		if err != nil {
+			return err
+		}
+		acFilter.Where = fmt.Sprintf("(%s OR %s)", acFilter.Where, filter.Where)
+		acFilter.Args = append(acFilter.Args, filter.Args...)
 	}
 
 	return ss.getTeamMembers(ctx, query, acFilter)
 }
 
 // getTeamMembers return a list of members for the specified team
-func (ss *SQLStore) getTeamMembers(ctx context.Context, query *models.GetTeamMembersQuery, acUserFilter *ac.SQLFilter) error {
+func (ss *SQLStore) getTeamMembers(ctx context.Context, query *models.GetTeamMembersQuery, acFilter *ac.SQLFilter) error {
 	return ss.WithDbSession(ctx, func(dbSess *DBSession) error {
 		query.Result = make([]*models.TeamMemberDTO, 0)
 		sess := dbSess.Table("team_member")
@@ -546,11 +555,8 @@ func (ss *SQLStore) getTeamMembers(ctx context.Context, query *models.GetTeamMem
 			fmt.Sprintf("team_member.user_id=%s.%s", ss.Dialect.Quote("user"), ss.Dialect.Quote("id")),
 		)
 
-		// explicitly check for serviceaccounts
-		sess.Where(fmt.Sprintf("%s.is_service_account=?", ss.Dialect.Quote("user")), ss.Dialect.BooleanStr(false))
-
-		if acUserFilter != nil {
-			sess.Where(acUserFilter.Where, acUserFilter.Args...)
+		if acFilter != nil {
+			sess.Where(acFilter.Where, acFilter.Args...)
 		}
 
 		// Join with only most recent auth module

pkg/services/sqlstore/team_test.go

@@ -24,9 +24,8 @@ func TestIntegrationTeamCommandsAndQueries(t *testing.T) {
 			OrgId: 1,
 			Permissions: map[int64]map[string][]string{
 				1: {
-					ac.ActionTeamsRead:         []string{ac.ScopeTeamsAll},
-					ac.ActionOrgUsersRead:      []string{ac.ScopeUsersAll},
-					serviceaccounts.ActionRead: []string{serviceaccounts.ScopeAll},
+					ac.ActionTeamsRead:    []string{ac.ScopeTeamsAll},
+					ac.ActionOrgUsersRead: []string{ac.ScopeUsersAll},
 				},
 			},
 		}
@@ -373,9 +372,21 @@ func TestIntegrationTeamCommandsAndQueries(t *testing.T) {
 				require.EqualValues(t, getTeamQuery.Result.MemberCount, 2)
 			})
 
-			t.Run("Should be able to exclude service accounts from teamembers", func(t *testing.T) {
+			t.Run("Should be able to add service accounts to teams, list it as a team member and remove it from team", func(t *testing.T) {
 				sqlStore = InitTestDB(t)
 				setup()
+
+				signedInUser := &models.SignedInUser{
+					Login: "loginuser0",
+					OrgId: testOrgID,
+					Permissions: map[int64]map[string][]string{
+						testOrgID: {
+							ac.ActionTeamsRead:    []string{ac.ScopeTeamsAll},
+							ac.ActionOrgUsersRead: []string{ac.ScopeUsersAll},
+						},
+					},
+				}
+
 				userCmd = user.CreateUserCommand{
 					Email:            fmt.Sprint("sa", 1, "@test.com"),
 					Name:             fmt.Sprint("sa", 1),
@@ -385,24 +396,23 @@ func TestIntegrationTeamCommandsAndQueries(t *testing.T) {
 				serviceAccount, err := sqlStore.CreateUser(context.Background(), userCmd)
 				require.NoError(t, err)
 
-				groupId := team2.Id
-				// add service account to team
-				err = sqlStore.AddTeamMember(serviceAccount.ID, testOrgID, groupId, false, 0)
+				teamId := team1.Id
+				err = sqlStore.AddTeamMember(serviceAccount.ID, testOrgID, teamId, false, 0)
 				require.NoError(t, err)
 
-				// add user to team
-				err = sqlStore.AddTeamMember(userIds[0], testOrgID, groupId, false, 0)
+				getTeamMembersQuery := &models.GetTeamMembersQuery{OrgId: testOrgID, TeamId: teamId, SignedInUser: signedInUser}
+				err = sqlStore.GetTeamMembers(context.Background(), getTeamMembersQuery)
+				require.NoError(t, err)
+				require.EqualValues(t, 1, len(getTeamMembersQuery.Result))
+				require.EqualValues(t, serviceAccount.ID, getTeamMembersQuery.Result[0].UserId)
+
+				removeTeamMemberCmd := &models.RemoveTeamMemberCommand{OrgId: testOrgID, TeamId: teamId, UserId: serviceAccount.ID}
+				err = sqlStore.RemoveTeamMember(context.Background(), removeTeamMemberCmd)
 				require.NoError(t, err)
 
-				teamMembersQuery := &models.GetTeamMembersQuery{
-					OrgId:        testOrgID,
-					SignedInUser: testUser,
-					TeamId:       groupId,
-				}
-				err = sqlStore.GetTeamMembers(context.Background(), teamMembersQuery)
+				err = sqlStore.GetTeamMembers(context.Background(), getTeamMembersQuery)
 				require.NoError(t, err)
-				// should not receive service account from query
-				require.Equal(t, len(teamMembersQuery.Result), 1)
+				require.EqualValues(t, 0, len(getTeamMembersQuery.Result))
 			})
 		})
 	})
@@ -489,7 +499,7 @@ func TestIntegrationSQLStore_GetTeamMembers_ACFilter(t *testing.T) {
 		t.Skip("skipping integration test")
 	}
 	testOrgID := int64(2)
-	userIds := make([]int64, 4)
+	userIds := make([]int64, 5)
 
 	// Seed 2 teams with 2 members
 	setup := func(store *SQLStore) {
@@ -498,12 +508,15 @@ func TestIntegrationSQLStore_GetTeamMembers_ACFilter(t *testing.T) {
 		team2, errCreateTeam := store.CreateTeam("group2 name", "test2@example.org", testOrgID)
 		require.NoError(t, errCreateTeam)
 
-		for i := 0; i < 4; i++ {
+		for i := 0; i < 5; i++ {
 			userCmd := user.CreateUserCommand{
 				Email: fmt.Sprint("user", i, "@example.org"),
 				Name:  fmt.Sprint("user", i),
 				Login: fmt.Sprint("loginuser", i),
 			}
+			if i >= 3 {
+				userCmd.IsServiceAccount = true
+			}
 			user, errCreateUser := store.CreateUser(context.Background(), userCmd)
 			require.NoError(t, errCreateUser)
 			userIds[i] = user.ID
@@ -517,6 +530,8 @@ func TestIntegrationSQLStore_GetTeamMembers_ACFilter(t *testing.T) {
 		require.NoError(t, errAddMember)
 		errAddMember = store.AddTeamMember(userIds[3], testOrgID, team2.Id, false, 0)
 		require.NoError(t, errAddMember)
+		errAddMember = store.AddTeamMember(userIds[4], testOrgID, team2.Id, false, 0)
+		require.NoError(t, errAddMember)
 	}
 
 	store := InitTestDB(t, InitTestDBOpt{})
@@ -534,11 +549,14 @@ func TestIntegrationSQLStore_GetTeamMembers_ACFilter(t *testing.T) {
 			query: &models.GetTeamMembersQuery{
 				OrgId: testOrgID,
 				SignedInUser: &models.SignedInUser{
-					OrgId:       testOrgID,
-					Permissions: map[int64]map[string][]string{testOrgID: {ac.ActionOrgUsersRead: {ac.ScopeUsersAll}}},
+					OrgId: testOrgID,
+					Permissions: map[int64]map[string][]string{testOrgID: {
+						ac.ActionOrgUsersRead:      {ac.ScopeUsersAll},
+						serviceaccounts.ActionRead: {serviceaccounts.ScopeAll},
+					}},
 				},
 			},
-			expectedNumUsers: 4,
+			expectedNumUsers: 5,
 		},
 		{
 			desc: "should return no team members",
@@ -558,11 +576,15 @@ func TestIntegrationSQLStore_GetTeamMembers_ACFilter(t *testing.T) {
 				OrgId: testOrgID,
 				SignedInUser: &models.SignedInUser{
 					OrgId: testOrgID,
-					Permissions: map[int64]map[string][]string{testOrgID: {ac.ActionOrgUsersRead: {
-						ac.Scope("users", "id", fmt.Sprintf("%d", userIds[0])),
-						ac.Scope("users", "id", fmt.Sprintf("%d", userIds[2])),
-						ac.Scope("users", "id", fmt.Sprintf("%d", userIds[3])),
-					}}},
+					Permissions: map[int64]map[string][]string{testOrgID: {
+						ac.ActionOrgUsersRead: {
+							ac.Scope("users", "id", fmt.Sprintf("%d", userIds[0])),
+							ac.Scope("users", "id", fmt.Sprintf("%d", userIds[2])),
+						},
+						serviceaccounts.ActionRead: {
+							ac.Scope("serviceaccounts", "id", fmt.Sprintf("%d", userIds[3])),
+						},
+					}},
 				},
 			},
 			expectedNumUsers: 3,
@@ -576,10 +598,20 @@ func TestIntegrationSQLStore_GetTeamMembers_ACFilter(t *testing.T) {
 
 			if !hasWildcardScope(tt.query.SignedInUser, ac.ActionOrgUsersRead) {
 				for _, member := range tt.query.Result {
-					assert.Contains(t,
-						tt.query.SignedInUser.Permissions[tt.query.SignedInUser.OrgId][ac.ActionOrgUsersRead],
-						ac.Scope("users", "id", fmt.Sprintf("%d", member.UserId)),
-					)
+					hasPermission := false
+					for _, scope := range tt.query.SignedInUser.Permissions[tt.query.SignedInUser.OrgId][ac.ActionOrgUsersRead] {
+						if scope == ac.Scope("users", "id", fmt.Sprintf("%d", member.UserId)) {
+							hasPermission = true
+							break
+						}
+					}
+					for _, scope := range tt.query.SignedInUser.Permissions[tt.query.SignedInUser.OrgId][serviceaccounts.ActionRead] {
+						if scope == ac.Scope("serviceaccounts", "id", fmt.Sprintf("%d", member.UserId)) {
+							hasPermission = true
+							break
+						}
+					}
+					assert.True(t, hasPermission)
 				}
 			}
 		})