From 312dd9e31504839539704ead6a9d472a9c33bbf9 Mon Sep 17 00:00:00 2001 From: J Guerreiro Date: Fri, 28 Jan 2022 11:17:54 +0000 Subject: [PATCH] AccessControl: Add AC to team preferences (#44554) * AccessControl: Add AC to team preferences * Apply suggestions from code review Co-authored-by: Gabriel MABILLE Co-authored-by: Gabriel MABILLE --- pkg/api/api.go | 4 +-- pkg/api/team.go | 15 ++++++--- pkg/api/team_test.go | 72 ++++++++++++++++++++++++++++++++++++++++++-- 3 files changed, 82 insertions(+), 9 deletions(-) diff --git a/pkg/api/api.go b/pkg/api/api.go index a16ce442f48..515a3caa8ff 100644 --- a/pkg/api/api.go +++ b/pkg/api/api.go @@ -188,8 +188,8 @@ func (hs *HTTPServer) registerRoutes() { teamsRoute.Post("/:teamId/members", authorize(reqCanAccessTeams, ac.EvalPermission(ac.ActionTeamsPermissionsWrite, ac.ScopeTeamsID)), routing.Wrap(hs.AddTeamMember)) teamsRoute.Put("/:teamId/members/:userId", authorize(reqCanAccessTeams, ac.EvalPermission(ac.ActionTeamsPermissionsWrite, ac.ScopeTeamsID)), routing.Wrap(hs.UpdateTeamMember)) teamsRoute.Delete("/:teamId/members/:userId", authorize(reqCanAccessTeams, ac.EvalPermission(ac.ActionTeamsPermissionsWrite, ac.ScopeTeamsID)), routing.Wrap(hs.RemoveTeamMember)) - teamsRoute.Get("/:teamId/preferences", reqCanAccessTeams, routing.Wrap(hs.GetTeamPreferences)) - teamsRoute.Put("/:teamId/preferences", reqCanAccessTeams, routing.Wrap(hs.UpdateTeamPreferences)) + teamsRoute.Get("/:teamId/preferences", authorize(reqCanAccessTeams, ac.EvalPermission(ac.ActionTeamsRead, ac.ScopeTeamsID)), routing.Wrap(hs.GetTeamPreferences)) + teamsRoute.Put("/:teamId/preferences", authorize(reqCanAccessTeams, ac.EvalPermission(ac.ActionTeamsWrite, ac.ScopeTeamsID)), routing.Wrap(hs.UpdateTeamPreferences)) }) // team without requirement of user to be org admin diff --git a/pkg/api/team.go b/pkg/api/team.go index 412e331cfa7..c695db38b4c 100644 --- a/pkg/api/team.go +++ b/pkg/api/team.go @@ -177,10 +177,13 @@ func (hs *HTTPServer) GetTeamPreferences(c *models.ReqContext) response.Response if err != nil { return response.Error(http.StatusBadRequest, "teamId is invalid", err) } + orgId := c.OrgId - if err := hs.teamGuardian.CanAdmin(c.Req.Context(), orgId, teamId, c.SignedInUser); err != nil { - return response.Error(403, "Not allowed to view team preferences.", err) + if !hs.Features.IsEnabled(featuremgmt.FlagAccesscontrol) { + if err := hs.teamGuardian.CanAdmin(c.Req.Context(), orgId, teamId, c.SignedInUser); err != nil { + return response.Error(403, "Not allowed to view team preferences.", err) + } } return hs.getPreferencesFor(c.Req.Context(), orgId, 0, teamId) @@ -192,14 +195,18 @@ func (hs *HTTPServer) UpdateTeamPreferences(c *models.ReqContext) response.Respo if err := web.Bind(c.Req, &dtoCmd); err != nil { return response.Error(http.StatusBadRequest, "bad request data", err) } + teamId, err := strconv.ParseInt(web.Params(c.Req)[":teamId"], 10, 64) if err != nil { return response.Error(http.StatusBadRequest, "teamId is invalid", err) } + orgId := c.OrgId - if err := hs.teamGuardian.CanAdmin(c.Req.Context(), orgId, teamId, c.SignedInUser); err != nil { - return response.Error(403, "Not allowed to update team preferences.", err) + if !hs.Features.IsEnabled(featuremgmt.FlagAccesscontrol) { + if err := hs.teamGuardian.CanAdmin(c.Req.Context(), orgId, teamId, c.SignedInUser); err != nil { + return response.Error(403, "Not allowed to update team preferences.", err) + } } return hs.updatePreferencesFor(c.Req.Context(), orgId, 0, teamId, &dtoCmd) diff --git a/pkg/api/team_test.go b/pkg/api/team_test.go index f29d93d3642..0e2a0a8c620 100644 --- a/pkg/api/team_test.go +++ b/pkg/api/team_test.go @@ -135,9 +135,12 @@ func TestTeamAPIEndpoint(t *testing.T) { } const ( - createTeamURL = "/api/teams/" - detailTeamURL = "/api/teams/%d" - teamCmd = `{"name": "MyTestTeam%d"}` + createTeamURL = "/api/teams/" + detailTeamURL = "/api/teams/%d" + detailTeamPreferenceURL = "/api/teams/%d/preferences" + teamCmd = `{"name": "MyTestTeam%d"}` + teamPreferenceCmd = `{"theme": "dark"}` + teamPreferenceCmdLight = `{"theme": "light"}` ) func TestTeamAPIEndpoint_CreateTeam_LegacyAccessControl(t *testing.T) { @@ -271,3 +274,66 @@ func TestTeamAPIEndpoint_DeleteTeam_FGAC(t *testing.T) { require.ErrorIs(t, err, models.ErrTeamNotFound) }) } + +// Given a team with a user, when the user is granted X permission, +// Then the endpoint should return 200 if the user has accesscontrol.ActionTeamsRead with teams:id:1 scope +// else return 403 +func TestTeamAPIEndpoint_GetTeamPreferences_FGAC(t *testing.T) { + sc := setupHTTPServer(t, true, true) + sc.db = sqlstore.InitTestDB(t) + _, err := sc.db.CreateTeam("team1", "", 1) + + require.NoError(t, err) + + setInitCtxSignedInViewer(sc.initCtx) + + t.Run("Access control allows getting team preferences 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(detailTeamPreferenceURL, 1), http.NoBody, t) + assert.Equal(t, http.StatusOK, response.Code) + }) + + t.Run("Access control prevents getting team preferences 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(detailTeamPreferenceURL, 1), http.NoBody, t) + assert.Equal(t, http.StatusForbidden, response.Code) + }) +} + +// 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 +func TestTeamAPIEndpoint_UpdateTeamPreferences_FGAC(t *testing.T) { + sc := setupHTTPServer(t, true, true) + sc.db = sqlstore.InitTestDB(t) + _, err := sc.db.CreateTeam("team1", "", 1) + + require.NoError(t, err) + + setInitCtxSignedInViewer(sc.initCtx) + + input := strings.NewReader(teamPreferenceCmd) + t.Run("Access control allows updating team preferences with the correct permissions", func(t *testing.T) { + setAccessControlPermissions(sc.acmock, []*accesscontrol.Permission{{Action: accesscontrol.ActionTeamsWrite, Scope: "teams:id:1"}}, 1) + response := callAPI(sc.server, http.MethodPut, fmt.Sprintf(detailTeamPreferenceURL, 1), input, t) + assert.Equal(t, http.StatusOK, response.Code) + + prefQuery := &models.GetPreferencesQuery{OrgId: 1, TeamId: 1, Result: &models.Preferences{}} + err := sc.db.GetPreferences(context.Background(), prefQuery) + require.NoError(t, err) + assert.Equal(t, "dark", prefQuery.Result.Theme) + }) + + input = strings.NewReader(teamPreferenceCmdLight) + t.Run("Access control prevents updating team preferences with the incorrect permissions", func(t *testing.T) { + setAccessControlPermissions(sc.acmock, []*accesscontrol.Permission{{Action: accesscontrol.ActionTeamsWrite, Scope: "teams:id:2"}}, 1) + response := callAPI(sc.server, http.MethodPut, fmt.Sprintf(detailTeamPreferenceURL, 1), input, t) + assert.Equal(t, http.StatusForbidden, response.Code) + + prefQuery := &models.GetPreferencesQuery{OrgId: 1, TeamId: 1, Result: &models.Preferences{}} + err := sc.db.GetPreferences(context.Background(), prefQuery) + assert.NoError(t, err) + assert.Equal(t, "dark", prefQuery.Result.Theme) + }) +}