From 235ea7a327db23e583c01ad0b38a69c821c946e6 Mon Sep 17 00:00:00 2001 From: Kevin Minehart Date: Tue, 25 Jan 2022 03:52:10 -0600 Subject: [PATCH] [7.5.x] Restrict /api/teams/:id to members or admins (#232) * resolve conflicts * remove reference to 'web' package --- docs/sources/http_api/team.md | 8 +++++- pkg/api/api.go | 2 +- pkg/api/team.go | 21 ++++++++++----- pkg/api/team_members.go | 4 +++ pkg/middleware/auth.go | 14 +++++----- pkg/models/team.go | 4 +++ pkg/services/sqlstore/team.go | 49 +++++++++++++++++++---------------- 7 files changed, 66 insertions(+), 36 deletions(-) diff --git a/docs/sources/http_api/team.md b/docs/sources/http_api/team.md index 866a3acecb8..a80e585490c 100644 --- a/docs/sources/http_api/team.md +++ b/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 diff --git a/pkg/api/api.go b/pkg/api/api.go index 14f9ad81453..50ad77dd29d 100644 --- a/pkg/api/api.go +++ b/pkg/api/api.go @@ -25,7 +25,7 @@ func (hs *HTTPServer) registerRoutes() { reqGrafanaAdmin := middleware.ReqGrafanaAdmin reqEditorRole := middleware.ReqEditorRole reqOrgAdmin := middleware.ReqOrgAdmin - reqCanAccessTeams := middleware.AdminOrFeatureEnabled(hs.Cfg.EditorsCanAdmin) + reqCanAccessTeams := middleware.AdminOrEditorAndFeatureEnabled(hs.Cfg.EditorsCanAdmin) reqSnapshotPublicModeOrSignedIn := middleware.SnapshotPublicModeOrSignedIn(hs.Cfg) redirectFromLegacyDashboardURL := middleware.RedirectFromLegacyDashboardURL() redirectFromLegacyDashboardSoloURL := middleware.RedirectFromLegacyDashboardSoloURL(hs.Cfg) diff --git a/pkg/api/team.go b/pkg/api/team.go index e475724e077..2eb899a5286 100644 --- a/pkg/api/team.go +++ b/pkg/api/team.go @@ -101,16 +101,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, @@ -131,6 +126,19 @@ 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 { query := models.GetTeamByIdQuery{ @@ -138,6 +146,7 @@ func (hs *HTTPServer) GetTeamByID(c *models.ReqContext) response.Response { Id: c.ParamsInt64(":teamId"), SignedInUser: c.SignedInUser, HiddenUsers: hs.Cfg.HiddenUsers, + UserIdFilter: userFilter(hs.Cfg.EditorsCanAdmin, c), } if err := bus.Dispatch(&query); err != nil { diff --git a/pkg/api/team_members.go b/pkg/api/team_members.go index 64bad505c93..2e9ffb26441 100644 --- a/pkg/api/team_members.go +++ b/pkg/api/team_members.go @@ -15,6 +15,10 @@ import ( func (hs *HTTPServer) GetTeamMembers(c *models.ReqContext) response.Response { query := models.GetTeamMembersQuery{OrgId: c.OrgId, TeamId: c.ParamsInt64(":teamId")} + if err := teamguardian.CanAdmin(hs.Bus, query.OrgId, query.TeamId, c.SignedInUser); err != nil { + return response.Error(403, "Not allowed to list team members", err) + } + if err := bus.Dispatch(&query); err != nil { return response.Error(500, "Failed to get Team Members", err) } diff --git a/pkg/middleware/auth.go b/pkg/middleware/auth.go index 8841250eafe..cd01dfd5dc8 100644 --- a/pkg/middleware/auth.go +++ b/pkg/middleware/auth.go @@ -129,20 +129,22 @@ func Auth(options *AuthOptions) macaron.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) macaron.Handler { +func AdminOrEditorAndFeatureEnabled(enabled bool) macaron.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) } } diff --git a/pkg/models/team.go b/pkg/models/team.go index 328e1815b90..8fe1ac47fd6 100644 --- a/pkg/models/team.go +++ b/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"` diff --git a/pkg/services/sqlstore/team.go b/pkg/services/sqlstore/team.go index b04ec56cc3c..734cd4fe08c 100644 --- a/pkg/services/sqlstore/team.go +++ b/pkg/services/sqlstore/team.go @@ -53,18 +53,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, @@ -187,17 +175,15 @@ func SearchTeams(query *models.SearchTeamsQuery) error { 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 = ?`) @@ -226,6 +212,8 @@ func SearchTeams(query *models.SearchTeamsQuery) error { team := models.Team{} countSess := x.Table("team") + countSess.Where("team.org_id=?", query.OrgId) + if query.Query != "" { countSess.Where(`name `+dialect.LikeStr()+` ?`, queryWithWildcards) } @@ -234,6 +222,18 @@ func SearchTeams(query *models.SearchTeamsQuery) error { 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 @@ -250,6 +250,11 @@ func GetTeamById(query *models.GetTeamByIdQuery) error { 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)