diff --git a/docs/sources/http_api/team.md b/docs/sources/http_api/team.md index 967ba415114..0a49dd99528 100644 --- a/docs/sources/http_api/team.md +++ b/docs/sources/http_api/team.md @@ -13,7 +13,8 @@ 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. +- If you enable `editors_can_admin` configuration flag, then Organization Editors can create teams and manage teams where they are Admin. + - If you enable `editors_can_admin` configuration flag, Editors can find out whether a team that they are not members of exists by trying to create a team with the same name. > If you are running Grafana Enterprise and have [Fine-grained access control]({{< relref "../enterprise/access-control/_index.md" >}}) enabled, access to endpoints will be controlled by Fine-grained access control permissions. > Refer to specific endpoints to understand what permissions are required. diff --git a/pkg/api/team.go b/pkg/api/team.go index 74a66238af6..be12efe992a 100644 --- a/pkg/api/team.go +++ b/pkg/api/team.go @@ -133,7 +133,7 @@ func (hs *HTTPServer) SearchTeams(c *models.ReqContext) response.Response { // Using accesscontrol the filtering is done based on user permissions userIdFilter := models.FilterIgnoreUser if !hs.Features.IsEnabled(featuremgmt.FlagAccesscontrol) { - userIdFilter = userFilter(hs.Cfg.EditorsCanAdmin, c) + userIdFilter = userFilter(c) } query := models.SearchTeamsQuery{ @@ -189,14 +189,12 @@ func (hs *HTTPServer) getTeamAccessControlMetadata(c *models.ReqContext, teamID // 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 { +// 2. If the user is an admin, this will return models.FilterIgnoreUser (0) +func userFilter(c *models.ReqContext) int64 { userIdFilter := c.SignedInUser.UserId - if (editorsCanAdmin && c.OrgRole == models.ROLE_EDITOR) || c.OrgRole == models.ROLE_ADMIN { + if c.OrgRole == models.ROLE_ADMIN { userIdFilter = models.FilterIgnoreUser } - return userIdFilter } @@ -210,7 +208,7 @@ func (hs *HTTPServer) GetTeamByID(c *models.ReqContext) response.Response { // Using accesscontrol the filtering has already been performed at middleware layer userIdFilter := models.FilterIgnoreUser if !hs.Features.IsEnabled(featuremgmt.FlagAccesscontrol) { - userIdFilter = userFilter(hs.Cfg.EditorsCanAdmin, c) + userIdFilter = userFilter(c) } query := models.GetTeamByIdQuery{ diff --git a/pkg/api/team_test.go b/pkg/api/team_test.go index 488c0e2e376..4246cafa1e9 100644 --- a/pkg/api/team_test.go +++ b/pkg/api/team_test.go @@ -40,39 +40,69 @@ func TestTeamAPIEndpoint(t *testing.T) { hs.SQLStore = store mock := &mockstore.SQLStoreMock{} - 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) - - 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)) - }, mock) - - 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) - - 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, 2, resp.TotalCount) - assert.Equal(t, 0, len(resp.Teams)) - }, mock) + loggedInUserScenarioWithRole(t, "When admin is calling GET on", "GET", "/api/teams/search", "/api/teams/search", + models.ROLE_ADMIN, func(sc *scenarioContext) { + _, err := hs.SQLStore.CreateTeam("team1", "", 1) + require.NoError(t, err) + _, err = hs.SQLStore.CreateTeam("team2", "", 1) + require.NoError(t, err) + + 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)) + }, mock) + + loggedInUserScenario(t, "When editor (with editors_can_admin) is 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) + + // Adding the test user to the teams in order for him to list them + err = hs.SQLStore.AddTeamMember(testUserID, testOrgID, team1.Id, false, 0) + require.NoError(t, err) + + 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)) + }, mock) + + loggedInUserScenario(t, "When editor (with editors_can_admin) calling GET with pagination on", + "/api/teams/search", "/api/teams/search", func(sc *scenarioContext) { + team1, err := hs.SQLStore.CreateTeam("team1", "", 1) + require.NoError(t, err) + team2, err := hs.SQLStore.CreateTeam("team2", "", 1) + require.NoError(t, err) + + // Adding the test user to the teams in order for him to list them + err = hs.SQLStore.AddTeamMember(testUserID, testOrgID, team1.Id, false, 0) + require.NoError(t, err) + err = hs.SQLStore.AddTeamMember(testUserID, testOrgID, team2.Id, false, 0) + require.NoError(t, err) + + 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, 2, resp.TotalCount) + assert.Equal(t, 0, len(resp.Teams)) + }, mock) }) t.Run("When creating team with API key", func(t *testing.T) { diff --git a/pkg/services/sqlstore/team.go b/pkg/services/sqlstore/team.go index 40ae8ddf99f..8fad2e6b0c4 100644 --- a/pkg/services/sqlstore/team.go +++ b/pkg/services/sqlstore/team.go @@ -76,6 +76,18 @@ func getTeamSelectSQLBase(filteredUsers []string) string { ` FROM team as team ` } +func getTeamSelectWithPermissionsSQLBase(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 (ss *SQLStore) CreateTeam(name, email string, orgID int64) (models.Team, error) { team := models.Team{ Name: name, @@ -188,14 +200,14 @@ func (ss *SQLStore) SearchTeams(ctx context.Context, query *models.SearchTeamsQu params := make([]interface{}, 0) filteredUsers := getFilteredUsers(query.SignedInUser, query.HiddenUsers) - 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 = ?`) + if query.UserIdFilter == models.FilterIgnoreUser { + sql.WriteString(getTeamSelectSQLBase(filteredUsers)) + } else { + sql.WriteString(getTeamSelectWithPermissionsSQLBase(filteredUsers)) params = append(params, query.UserIdFilter) } diff --git a/public/app/features/teams/TeamList.tsx b/public/app/features/teams/TeamList.tsx index 3f899e5da02..f044e4681d7 100644 --- a/public/app/features/teams/TeamList.tsx +++ b/public/app/features/teams/TeamList.tsx @@ -69,11 +69,9 @@ export class TeamList extends PureComponent { const { editorsCanAdmin, signedInUser } = this.props; const permission = team.permission; const teamUrl = `org/teams/edit/${team.id}`; - const canDelete = contextSrv.hasAccessInMetadata( - AccessControlAction.ActionTeamsDelete, - team, - isPermissionTeamAdmin({ permission, editorsCanAdmin, signedInUser }) - ); + const isTeamAdmin = isPermissionTeamAdmin({ permission, editorsCanAdmin, signedInUser }); + const canDelete = contextSrv.hasAccessInMetadata(AccessControlAction.ActionTeamsDelete, team, isTeamAdmin); + const canReadTeam = contextSrv.hasAccessInMetadata(AccessControlAction.ActionTeamsRead, team, isTeamAdmin); const canSeeTeamRoles = contextSrv.hasAccessInMetadata(AccessControlAction.ActionTeamsRolesList, team, false); const canUpdateTeamRoles = contextSrv.hasAccess(AccessControlAction.ActionTeamsRolesAdd, false) || @@ -86,20 +84,34 @@ export class TeamList extends PureComponent { return ( - + {canReadTeam ? ( + + Team avatar + + ) : ( Team avatar - + )} - {team.name} + {canReadTeam ? {team.name} :
{team.name}
} - 0 ? undefined : 'Empty email cell'}> - {team.email} - + {canReadTeam ? ( + 0 ? undefined : 'Empty email cell'}> + {team.email} + + ) : ( +
0 ? undefined : 'Empty email cell'}> + {team.email} +
+ )} - {team.memberCount} + {canReadTeam ? ( + {team.memberCount} + ) : ( +
{team.memberCount}
+ )} {displayRolePicker && ( diff --git a/public/app/features/teams/__snapshots__/TeamList.test.tsx.snap b/public/app/features/teams/__snapshots__/TeamList.test.tsx.snap index 88f24af4dd8..7e7b2c6d41f 100644 --- a/public/app/features/teams/__snapshots__/TeamList.test.tsx.snap +++ b/public/app/features/teams/__snapshots__/TeamList.test.tsx.snap @@ -445,42 +445,50 @@ exports[`Render when feature toggle editorsCanAdmin is turned on and signedin us - - Team avatar - + Team avatar - test-1 - + - test-1@test.com - + - 1 - + - - Team avatar - + Team avatar - test-1 - + - test-1@test.com - + - 1 - +