* Teams: Appropriately apply user id filter in /api/teams/:id and /ap…

…i/teams/search

* Teams: Ensure that users searching for teams are only able see teams they have access to
* Teams: Require teamGuardian admin privileges to list team members
* Teams: Prevent org viewers from administering teams
* Teams: Add org_id condition to team count query
* Teams: clarify permission requirements in teams api docs
* Teams: expand scenarios for team search tests
* Teams: mock teamGuardian in tests

Co-authored-by: Dan Cech <dcech@grafana.com>

docs/sources/http_api/team.md

@@ -7,7 +7,13 @@ aliases = ["/docs/grafana/latest/http_api/team/"]
 
 # Team API
 
-This API can be used to create/update/delete Teams and to add/remove users to Teams. All actions require that the user has the Admin role for the organization.
+This API can be used to manage Teams and Team Memberships.
+
+Access to these API endpoints is restricted as follows:
+
+- All authenticated users are able to view details of teams they are a member of.
+- Organization Admins are able to manage all teams and team members.
+- If the `editors_can_admin` configuration flag is enabled, Organization Editors are able to view details of all teams and to manage teams that they are Admin members of.
 
 ## Team Search With Paging
 

pkg/api/api.go

@@ -27,7 +27,7 @@ func (hs *HTTPServer) registerRoutes() {
 	reqEditorRole := middleware.ReqEditorRole
 	reqOrgAdmin := middleware.ReqOrgAdmin
 	reqOrgAdminFolderAdminOrTeamAdmin := middleware.OrgAdminFolderAdminOrTeamAdmin
-	reqCanAccessTeams := middleware.AdminOrFeatureEnabled(hs.Cfg.EditorsCanAdmin)
+	reqCanAccessTeams := middleware.AdminOrEditorAndFeatureEnabled(hs.Cfg.EditorsCanAdmin)
 	reqSnapshotPublicModeOrSignedIn := middleware.SnapshotPublicModeOrSignedIn(hs.Cfg)
 	redirectFromLegacyPanelEditURL := middleware.RedirectFromLegacyPanelEditURL(hs.Cfg)
 	authorize := acmiddleware.Middleware(hs.AccessControl)

pkg/api/team.go

@@ -112,16 +112,11 @@ func (hs *HTTPServer) SearchTeams(c *models.ReqContext) response.Response {
 		page = 1
 	}
 
-	var userIdFilter int64
-	if hs.Cfg.EditorsCanAdmin && c.OrgRole != models.ROLE_ADMIN {
-		userIdFilter = c.SignedInUser.UserId
-	}
-
 	query := models.SearchTeamsQuery{
 		OrgId:        c.OrgId,
 		Query:        c.Query("query"),
 		Name:         c.Query("name"),
-		UserIdFilter: userIdFilter,
+		UserIdFilter: userFilter(hs.Cfg.EditorsCanAdmin, c),
 		Page:         page,
 		Limit:        perPage,
 		SignedInUser: c.SignedInUser,
@@ -142,17 +137,32 @@ func (hs *HTTPServer) SearchTeams(c *models.ReqContext) response.Response {
 	return response.JSON(200, query.Result)
 }
 
+// UserFilter returns the user ID used in a filter when querying a team
+// 1. If the user is a viewer or editor, this will return the user's ID.
+// 2. If EditorsCanAdmin is enabled and the user is an editor, this will return models.FilterIgnoreUser (0)
+// 3. If the user is an admin, this will return models.FilterIgnoreUser (0)
+func userFilter(editorsCanAdmin bool, c *models.ReqContext) int64 {
+	userIdFilter := c.SignedInUser.UserId
+	if (editorsCanAdmin && c.OrgRole == models.ROLE_EDITOR) || c.OrgRole == models.ROLE_ADMIN {
+		userIdFilter = models.FilterIgnoreUser
+	}
+
+	return userIdFilter
+}
+
 // GET /api/teams/:teamId
 func (hs *HTTPServer) GetTeamByID(c *models.ReqContext) response.Response {
 	teamId, err := strconv.ParseInt(web.Params(c.Req)[":teamId"], 10, 64)
 	if err != nil {
 		return response.Error(http.StatusBadRequest, "teamId is invalid", err)
 	}
+
 	query := models.GetTeamByIdQuery{
 		OrgId:        c.OrgId,
 		Id:           teamId,
 		SignedInUser: c.SignedInUser,
 		HiddenUsers:  hs.Cfg.HiddenUsers,
+		UserIdFilter: userFilter(hs.Cfg.EditorsCanAdmin, c),
 	}
 
 	if err := hs.SQLStore.GetTeamById(c.Req.Context(), &query); err != nil {

pkg/api/team_members.go

@@ -21,6 +21,9 @@ func (hs *HTTPServer) GetTeamMembers(c *models.ReqContext) response.Response {
 	}
 
 	query := models.GetTeamMembersQuery{OrgId: c.OrgId, TeamId: teamId}
+	if err := hs.teamGuardian.CanAdmin(c.Req.Context(), query.OrgId, query.TeamId, c.SignedInUser); err != nil {
+		return response.Error(403, "Not allowed to list team members", err)
+	}
 
 	if err := hs.SQLStore.GetTeamMembers(c.Req.Context(), &query); err != nil {
 		return response.Error(500, "Failed to get Team Members", err)

pkg/api/team_members_test.go

@@ -15,6 +15,14 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
+type TeamGuardianMock struct {
+	result error
+}
+
+func (t *TeamGuardianMock) CanAdmin(ctx context.Context, orgId int64, teamId int64, user *models.SignedInUser) error {
+	return t.result
+}
+
 func setUpGetTeamMembersHandler(t *testing.T, sqlStore *sqlstore.SQLStore) {
 	const testOrgID int64 = 1
 	var userCmd models.CreateUserCommand
@@ -38,9 +46,10 @@ func TestTeamMembersAPIEndpoint_userLoggedIn(t *testing.T) {
 	settings := setting.NewCfg()
 	sqlStore := sqlstore.InitTestDB(t)
 	hs := &HTTPServer{
-		Cfg:      settings,
-		License:  &licensing.OSSLicensingService{},
-		SQLStore: sqlStore,
+		Cfg:          settings,
+		License:      &licensing.OSSLicensingService{},
+		SQLStore:     sqlStore,
+		teamGuardian: &TeamGuardianMock{},
 	}
 
 	loggedInUserScenarioWithRole(t, "When calling GET on", "GET", "api/teams/1/members",

pkg/api/team_test.go

@@ -38,6 +38,33 @@ func TestTeamAPIEndpoint(t *testing.T) {
 			require.NoError(t, err)
 			_, err = hs.SQLStore.CreateTeam("team2", "", 1)
 			require.NoError(t, err)
+			// third team in a different org
+			_, err = hs.SQLStore.CreateTeam("team3", "", 2)
+			require.NoError(t, err)
+
+			hs.Cfg.EditorsCanAdmin = false
+
+			sc.handlerFunc = hs.SearchTeams
+			sc.fakeReqWithParams("GET", sc.url, map[string]string{}).exec()
+			require.Equal(t, http.StatusOK, sc.resp.Code)
+			var resp models.SearchTeamQueryResult
+			err = json.Unmarshal(sc.resp.Body.Bytes(), &resp)
+			require.NoError(t, err)
+
+			assert.EqualValues(t, 0, resp.TotalCount)
+			assert.Equal(t, 0, len(resp.Teams))
+		})
+
+		loggedInUserScenario(t, "When calling GET on", "/api/teams/search", "/api/teams/search", func(sc *scenarioContext) {
+			_, err := hs.SQLStore.CreateTeam("team1", "", 1)
+			require.NoError(t, err)
+			_, err = hs.SQLStore.CreateTeam("team2", "", 1)
+			require.NoError(t, err)
+			// third team in a different org
+			_, err = hs.SQLStore.CreateTeam("team3", "", 2)
+			require.NoError(t, err)
+
+			hs.Cfg.EditorsCanAdmin = true
 
 			sc.handlerFunc = hs.SearchTeams
 			sc.fakeReqWithParams("GET", sc.url, map[string]string{}).exec()
@@ -55,6 +82,33 @@ func TestTeamAPIEndpoint(t *testing.T) {
 			require.NoError(t, err)
 			_, err = hs.SQLStore.CreateTeam("team2", "", 1)
 			require.NoError(t, err)
+			// third team in a different org
+			_, err = hs.SQLStore.CreateTeam("team3", "", 2)
+			require.NoError(t, err)
+
+			hs.Cfg.EditorsCanAdmin = false
+
+			sc.handlerFunc = hs.SearchTeams
+			sc.fakeReqWithParams("GET", sc.url, map[string]string{"perpage": "10", "page": "2"}).exec()
+			require.Equal(t, http.StatusOK, sc.resp.Code)
+			var resp models.SearchTeamQueryResult
+			err = json.Unmarshal(sc.resp.Body.Bytes(), &resp)
+			require.NoError(t, err)
+
+			assert.EqualValues(t, 0, resp.TotalCount)
+			assert.Equal(t, 0, len(resp.Teams))
+		})
+
+		loggedInUserScenario(t, "When calling GET on", "/api/teams/search", "/api/teams/search", func(sc *scenarioContext) {
+			_, err := hs.SQLStore.CreateTeam("team1", "", 1)
+			require.NoError(t, err)
+			_, err = hs.SQLStore.CreateTeam("team2", "", 1)
+			require.NoError(t, err)
+			// third team in a different org
+			_, err = hs.SQLStore.CreateTeam("team3", "", 2)
+			require.NoError(t, err)
+
+			hs.Cfg.EditorsCanAdmin = true
 
 			sc.handlerFunc = hs.SearchTeams
 			sc.fakeReqWithParams("GET", sc.url, map[string]string{"perpage": "10", "page": "2"}).exec()
@@ -66,6 +120,58 @@ func TestTeamAPIEndpoint(t *testing.T) {
 			assert.EqualValues(t, 2, resp.TotalCount)
 			assert.Equal(t, 0, len(resp.Teams))
 		})
+
+		loggedInUserScenario(t, "When calling GET on", "/api/teams/search", "/api/teams/search", func(sc *scenarioContext) {
+			team1, err := hs.SQLStore.CreateTeam("team1", "", 1)
+			require.NoError(t, err)
+			_, err = hs.SQLStore.CreateTeam("team2", "", 1)
+			require.NoError(t, err)
+			// third team in a different org
+			_, err = hs.SQLStore.CreateTeam("team3", "", 2)
+			require.NoError(t, err)
+
+			// add user to team1
+			err = hs.SQLStore.AddTeamMember(testUserID, testOrgID, team1.Id, false, models.PERMISSION_VIEW)
+			require.NoError(t, err)
+
+			hs.Cfg.EditorsCanAdmin = false
+
+			sc.handlerFunc = hs.SearchTeams
+			sc.fakeReqWithParams("GET", sc.url, map[string]string{}).exec()
+			require.Equal(t, http.StatusOK, sc.resp.Code)
+			var resp models.SearchTeamQueryResult
+			err = json.Unmarshal(sc.resp.Body.Bytes(), &resp)
+			require.NoError(t, err)
+
+			assert.EqualValues(t, 1, resp.TotalCount)
+			assert.Equal(t, 1, len(resp.Teams))
+		})
+
+		loggedInUserScenario(t, "When calling GET on", "/api/teams/search", "/api/teams/search", func(sc *scenarioContext) {
+			team1, err := hs.SQLStore.CreateTeam("team1", "", 1)
+			require.NoError(t, err)
+			_, err = hs.SQLStore.CreateTeam("team2", "", 1)
+			require.NoError(t, err)
+			// third team in a different org
+			_, err = hs.SQLStore.CreateTeam("team3", "", 2)
+			require.NoError(t, err)
+
+			// add user to team1
+			err = hs.SQLStore.AddTeamMember(testUserID, testOrgID, team1.Id, false, models.PERMISSION_VIEW)
+			require.NoError(t, err)
+
+			hs.Cfg.EditorsCanAdmin = true
+
+			sc.handlerFunc = hs.SearchTeams
+			sc.fakeReqWithParams("GET", sc.url, map[string]string{}).exec()
+			require.Equal(t, http.StatusOK, sc.resp.Code)
+			var resp models.SearchTeamQueryResult
+			err = json.Unmarshal(sc.resp.Body.Bytes(), &resp)
+			require.NoError(t, err)
+
+			assert.EqualValues(t, 2, resp.TotalCount)
+			assert.Equal(t, 2, len(resp.Teams))
+		})
 	})
 
 	t.Run("When creating team with API key", func(t *testing.T) {

pkg/middleware/auth.go

@@ -128,20 +128,22 @@ func Auth(options *AuthOptions) web.Handler {
 	}
 }
 
-// AdminOrFeatureEnabled creates a middleware that allows access
-// if the signed in user is either an Org Admin or if the
-// feature flag is enabled.
+// AdminOrEditorAndFeatureEnabled creates a middleware that allows
+// access if the signed in user is either an Org Admin or if they
+// are an Org Editor and the feature flag is enabled.
 // Intended for when feature flags open up access to APIs that
 // are otherwise only available to admins.
-func AdminOrFeatureEnabled(enabled bool) web.Handler {
+func AdminOrEditorAndFeatureEnabled(enabled bool) web.Handler {
 	return func(c *models.ReqContext) {
 		if c.OrgRole == models.ROLE_ADMIN {
 			return
 		}
 
-		if !enabled {
-			accessForbidden(c)
+		if c.OrgRole == models.ROLE_EDITOR && enabled {
+			return
 		}
+
+		accessForbidden(c)
 	}
 }
 

pkg/models/team.go

@@ -55,8 +55,12 @@ type GetTeamByIdQuery struct {
 	SignedInUser *SignedInUser
 	HiddenUsers  map[string]struct{}
 	Result       *TeamDTO
+	UserIdFilter int64
 }
 
+// FilterIgnoreUser is used in a get / search teams query when the caller does not want to filter teams by user ID / membership
+const FilterIgnoreUser int64 = 0
+
 type GetTeamsByUserQuery struct {
 	OrgId  int64
 	UserId int64      `json:"userId"`

pkg/services/sqlstore/team.go

@@ -62,18 +62,6 @@ func getTeamMemberCount(filteredUsers []string) string {
 	return "(SELECT COUNT(*) FROM team_member WHERE team_member.team_id = team.id) AS member_count "
 }
 
-func getTeamSearchSQLBase(filteredUsers []string) string {
-	return `SELECT
-		team.id AS id,
-		team.org_id,
-		team.name AS name,
-		team.email AS email,
-		team_member.permission, ` +
-		getTeamMemberCount(filteredUsers) +
-		` FROM team AS team
-		INNER JOIN team_member ON team.id = team_member.team_id AND team_member.user_id = ? `
-}
-
 func getTeamSelectSQLBase(filteredUsers []string) string {
 	return `SELECT
 		team.id as id,
@@ -192,17 +180,15 @@ func (ss *SQLStore) SearchTeams(ctx context.Context, query *models.SearchTeamsQu
 	params := make([]interface{}, 0)
 
 	filteredUsers := getFilteredUsers(query.SignedInUser, query.HiddenUsers)
-	if query.UserIdFilter > 0 {
-		sql.WriteString(getTeamSearchSQLBase(filteredUsers))
-		for _, user := range filteredUsers {
-			params = append(params, user)
-		}
+	sql.WriteString(getTeamSelectSQLBase(filteredUsers))
+
+	for _, user := range filteredUsers {
+		params = append(params, user)
+	}
+
+	if query.UserIdFilter != models.FilterIgnoreUser {
+		sql.WriteString(` INNER JOIN team_member ON team.id = team_member.team_id AND team_member.user_id = ?`)
 		params = append(params, query.UserIdFilter)
-	} else {
-		sql.WriteString(getTeamSelectSQLBase(filteredUsers))
-		for _, user := range filteredUsers {
-			params = append(params, user)
-		}
 	}
 
 	sql.WriteString(` WHERE team.org_id = ?`)
@@ -231,14 +217,30 @@ func (ss *SQLStore) SearchTeams(ctx context.Context, query *models.SearchTeamsQu
 
 	team := models.Team{}
 	countSess := x.Table("team")
+	countSess.Where("team.org_id=?", query.OrgId)
+
 	if query.Query != "" {
 		countSess.Where(`name `+dialect.LikeStr()+` ?`, queryWithWildcards)
 	}
 
+	countSess.Where("org_id=?", query.OrgId)
+
 	if query.Name != "" {
 		countSess.Where("name=?", query.Name)
 	}
 
+	// If we're not retrieving all results, then only search for teams that this user has access to
+	if query.UserIdFilter != models.FilterIgnoreUser {
+		countSess.
+			Where(`
+			team.id IN (
+				SELECT
+				team_id
+				FROM team_member
+				WHERE team_member.user_id = ?
+			)`, query.UserIdFilter)
+	}
+
 	count, err := countSess.Count(&team)
 	query.Result.TotalCount = count
 
@@ -255,6 +257,11 @@ func (ss *SQLStore) GetTeamById(ctx context.Context, query *models.GetTeamByIdQu
 		params = append(params, user)
 	}
 
+	if query.UserIdFilter != models.FilterIgnoreUser {
+		sql.WriteString(` INNER JOIN team_member ON team.id = team_member.team_id AND team_member.user_id = ?`)
+		params = append(params, query.UserIdFilter)
+	}
+
 	sql.WriteString(` WHERE team.org_id = ? and team.id = ?`)
 	params = append(params, query.OrgId, query.Id)