diff --git a/pkg/api/api.go b/pkg/api/api.go index daabd46ce2c..e96fa424431 100644 --- a/pkg/api/api.go +++ b/pkg/api/api.go @@ -198,8 +198,8 @@ func (hs *HTTPServer) registerRoutes() { // team without requirement of user to be org admin apiRoute.Group("/teams", func(teamsRoute routing.RouteRegister) { - teamsRoute.Get("/:teamId", routing.Wrap(hs.GetTeamByID)) - teamsRoute.Get("/search", routing.Wrap(hs.SearchTeams)) + teamsRoute.Get("/:teamId", authorize(reqSignedIn, ac.EvalPermission(ac.ActionTeamsRead, ac.ScopeTeamsID)), routing.Wrap(hs.GetTeamByID)) + teamsRoute.Get("/search", authorize(reqSignedIn, ac.EvalPermission(ac.ActionTeamsRead)), routing.Wrap(hs.SearchTeams)) }) // org information available to all users. diff --git a/pkg/api/team.go b/pkg/api/team.go index e13edd7e763..95160280446 100644 --- a/pkg/api/team.go +++ b/pkg/api/team.go @@ -132,11 +132,17 @@ func (hs *HTTPServer) SearchTeams(c *models.ReqContext) response.Response { page = 1 } + // 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) + } + query := models.SearchTeamsQuery{ OrgId: c.OrgId, Query: c.Query("query"), Name: c.Query("name"), - UserIdFilter: userFilter(hs.Cfg.EditorsCanAdmin, c), + UserIdFilter: userIdFilter, Page: page, Limit: perPage, SignedInUser: c.SignedInUser, @@ -203,12 +209,18 @@ func (hs *HTTPServer) GetTeamByID(c *models.ReqContext) response.Response { return response.Error(http.StatusBadRequest, "teamId is invalid", err) } + // 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) + } + query := models.GetTeamByIdQuery{ OrgId: c.OrgId, Id: teamId, SignedInUser: c.SignedInUser, HiddenUsers: hs.Cfg.HiddenUsers, - UserIdFilter: userFilter(hs.Cfg.EditorsCanAdmin, c), + UserIdFilter: userIdFilter, } if err := hs.SQLStore.GetTeamById(c.Req.Context(), &query); err != nil { diff --git a/pkg/api/team_test.go b/pkg/api/team_test.go index 849ceff636f..70d27d7744f 100644 --- a/pkg/api/team_test.go +++ b/pkg/api/team_test.go @@ -36,6 +36,10 @@ func TestTeamAPIEndpoint(t *testing.T) { hs.SQLStore = sqlstore.InitTestDB(t) hs.Cfg.EditorsCanAdmin = true + store := sqlstore.InitTestDB(t) + store.Cfg = hs.Cfg + hs.SQLStore = store + 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) @@ -138,6 +142,7 @@ func TestTeamAPIEndpoint(t *testing.T) { } const ( + searchTeamsURL = "/api/teams/search" createTeamURL = "/api/teams/" detailTeamURL = "/api/teams/%d" detailTeamPreferenceURL = "/api/teams/%d/preferences" @@ -197,6 +202,79 @@ func TestTeamAPIEndpoint_CreateTeam_FGAC(t *testing.T) { }) } +func TestTeamAPIEndpoint_SearchTeams_FGAC(t *testing.T) { + sc := setupHTTPServer(t, true, true) + sc.db = sqlstore.InitTestDB(t) + + // Seed three teams + for i := 1; i <= 3; i++ { + _, err := sc.db.CreateTeam(fmt.Sprintf("team%d", i), fmt.Sprintf("team%d@example.org", i), 1) + require.NoError(t, err) + } + + setInitCtxSignedInViewer(sc.initCtx) + + t.Run("Access control prevents searching for teams with the incorrect permissions", func(t *testing.T) { + setAccessControlPermissions(sc.acmock, []*accesscontrol.Permission{{Action: accesscontrol.ActionTeamsDelete, Scope: "teams:id:*"}}, 1) + response := callAPI(sc.server, http.MethodGet, searchTeamsURL, http.NoBody, t) + assert.Equal(t, http.StatusForbidden, response.Code) + }) + + t.Run("Access control allows searching for teams with the correct permissions", func(t *testing.T) { + setAccessControlPermissions(sc.acmock, []*accesscontrol.Permission{{Action: accesscontrol.ActionTeamsRead, Scope: "teams:id:*"}}, 1) + response := callAPI(sc.server, http.MethodGet, searchTeamsURL, http.NoBody, t) + assert.Equal(t, http.StatusOK, response.Code) + + res := &models.SearchTeamQueryResult{} + err := json.Unmarshal(response.Body.Bytes(), res) + require.NoError(t, err) + require.Len(t, res.Teams, 3, "expected all teams to have been returned") + require.Equal(t, res.TotalCount, int64(3), "expected count to match teams length") + }) + + t.Run("Access control filters teams based on user permissions", func(t *testing.T) { + setAccessControlPermissions(sc.acmock, []*accesscontrol.Permission{{Action: accesscontrol.ActionTeamsRead, Scope: "teams:id:1"}, {Action: accesscontrol.ActionTeamsRead, Scope: "teams:id:3"}}, 1) + response := callAPI(sc.server, http.MethodGet, searchTeamsURL, http.NoBody, t) + assert.Equal(t, http.StatusOK, response.Code) + + res := &models.SearchTeamQueryResult{} + err := json.Unmarshal(response.Body.Bytes(), res) + require.NoError(t, err) + require.Len(t, res.Teams, 2, "expected a subset of teams to have been returned") + require.Equal(t, res.TotalCount, int64(2), "expected count to match teams length") + for _, team := range res.Teams { + require.NotEqual(t, team.Name, "team2", "expected team2 to have been filtered") + } + }) +} + +func TestTeamAPIEndpoint_GetTeamByID_FGAC(t *testing.T) { + sc := setupHTTPServer(t, true, true) + sc.db = sqlstore.InitTestDB(t) + + _, err := sc.db.CreateTeam("team1", "team1@example.org", 1) + require.NoError(t, err) + + setInitCtxSignedInViewer(sc.initCtx) + + t.Run("Access control prevents getting a team with the incorrect permissions", func(t *testing.T) { + setAccessControlPermissions(sc.acmock, []*accesscontrol.Permission{{Action: accesscontrol.ActionTeamsRead, Scope: "teams:id:2"}}, 1) + response := callAPI(sc.server, http.MethodGet, fmt.Sprintf(detailTeamURL, 1), http.NoBody, t) + assert.Equal(t, http.StatusForbidden, response.Code) + }) + + t.Run("Access control allows getting a team with the correct permissions", func(t *testing.T) { + setAccessControlPermissions(sc.acmock, []*accesscontrol.Permission{{Action: accesscontrol.ActionTeamsRead, Scope: "teams:id:1"}}, 1) + response := callAPI(sc.server, http.MethodGet, fmt.Sprintf(detailTeamURL, 1), http.NoBody, t) + assert.Equal(t, http.StatusOK, response.Code) + + res := &models.TeamDTO{} + err := json.Unmarshal(response.Body.Bytes(), res) + require.NoError(t, err) + assert.Equal(t, "team1", res.Name) + }) +} + // Given a team with a user, when the user is granted X permission, // Then the endpoint should return 200 if the user has accesscontrol.ActionTeamsWrite with teams:id:1 scope // else return 403 diff --git a/pkg/services/accesscontrol/filter.go b/pkg/services/accesscontrol/filter.go index c2bf3768055..2653e607ec5 100644 --- a/pkg/services/accesscontrol/filter.go +++ b/pkg/services/accesscontrol/filter.go @@ -12,6 +12,7 @@ import ( var sqlIDAcceptList = map[string]struct{}{ "org_user.user_id": {}, "role.id": {}, + "team.id": {}, } var ( diff --git a/pkg/services/sqlstore/team.go b/pkg/services/sqlstore/team.go index 67dd3ca18f7..8cc4eca10f5 100644 --- a/pkg/services/sqlstore/team.go +++ b/pkg/services/sqlstore/team.go @@ -10,6 +10,7 @@ import ( "github.com/grafana/grafana/pkg/bus" "github.com/grafana/grafana/pkg/models" ac "github.com/grafana/grafana/pkg/services/accesscontrol" + "github.com/grafana/grafana/pkg/services/featuremgmt" ) func (ss *SQLStore) addTeamQueryAndCommandHandlers() { @@ -210,6 +211,19 @@ func (ss *SQLStore) SearchTeams(ctx context.Context, query *models.SearchTeamsQu params = append(params, query.Name) } + var ( + acFilter ac.SQLFilter + err error + ) + if ss.Cfg.IsFeatureToggleEnabled(featuremgmt.FlagAccesscontrol) { + acFilter, err = ac.Filter(ctx, "team.id", "teams", ac.ActionTeamsRead, query.SignedInUser) + if err != nil { + return err + } + sql.WriteString(` and` + acFilter.Where) + params = append(params, acFilter.Args...) + } + sql.WriteString(` order by team.name asc`) if query.Limit != 0 { @@ -245,6 +259,11 @@ func (ss *SQLStore) SearchTeams(ctx context.Context, query *models.SearchTeamsQu )`, query.UserIdFilter) } + // Only count teams user can see + if ss.Cfg.IsFeatureToggleEnabled(featuremgmt.FlagAccesscontrol) { + countSess.Where(acFilter.Where, acFilter.Args...) + } + count, err := countSess.Count(&team) query.Result.TotalCount = count diff --git a/pkg/services/sqlstore/team_test.go b/pkg/services/sqlstore/team_test.go index 2d8f0b6defc..6603884520f 100644 --- a/pkg/services/sqlstore/team_test.go +++ b/pkg/services/sqlstore/team_test.go @@ -9,6 +9,9 @@ import ( "testing" "github.com/grafana/grafana/pkg/models" + ac "github.com/grafana/grafana/pkg/services/accesscontrol" + "github.com/grafana/grafana/pkg/services/featuremgmt" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -344,3 +347,75 @@ func TestTeamCommandsAndQueries(t *testing.T) { }) }) } + +func TestSQLStore_SearchTeams(t *testing.T) { + type searchTeamsTestCase struct { + desc string + query *models.SearchTeamsQuery + expectedNumUsers int + } + + tests := []searchTeamsTestCase{ + { + desc: "should return all teams", + query: &models.SearchTeamsQuery{ + OrgId: 1, + SignedInUser: &models.SignedInUser{ + OrgId: 1, + Permissions: map[int64]map[string][]string{1: {ac.ActionTeamsRead: {ac.ScopeTeamsAll}}}, + }, + }, + expectedNumUsers: 10, + }, + { + desc: "should return no teams", + query: &models.SearchTeamsQuery{ + OrgId: 1, + SignedInUser: &models.SignedInUser{ + OrgId: 1, + Permissions: map[int64]map[string][]string{1: {ac.ActionTeamsRead: {""}}}, + }, + }, + expectedNumUsers: 0, + }, + { + desc: "should return some teams", + query: &models.SearchTeamsQuery{ + OrgId: 1, + SignedInUser: &models.SignedInUser{ + OrgId: 1, + Permissions: map[int64]map[string][]string{1: {ac.ActionTeamsRead: { + "teams:id:1", + "teams:id:5", + "teams:id:9", + }}}, + }, + }, + expectedNumUsers: 3, + }, + } + + store := InitTestDB(t) + store.Cfg.IsFeatureToggleEnabled = featuremgmt.WithFeatures(featuremgmt.FlagAccesscontrol).IsEnabled + + // Seed 10 teams + for i := 1; i <= 10; i++ { + _, err := store.CreateTeam(fmt.Sprintf("team-%d", i), fmt.Sprintf("team-%d@example.org", i), 1) + require.NoError(t, err) + } + + for _, tt := range tests { + t.Run(tt.desc, func(t *testing.T) { + err := store.SearchTeams(context.Background(), tt.query) + require.NoError(t, err) + assert.Len(t, tt.query.Result.Teams, tt.expectedNumUsers) + assert.Equal(t, tt.query.Result.TotalCount, int64(tt.expectedNumUsers)) + + if !hasWildcardScope(tt.query.SignedInUser, ac.ActionTeamsRead) { + for _, team := range tt.query.Result.Teams { + assert.Contains(t, tt.query.SignedInUser.Permissions[tt.query.SignedInUser.OrgId][ac.ActionTeamsRead], fmt.Sprintf("teams:id:%d", team.Id)) + } + } + }) + } +}