diff --git a/pkg/api/api.go b/pkg/api/api.go index 598357e502f..eda73a91c0e 100644 --- a/pkg/api/api.go +++ b/pkg/api/api.go @@ -182,8 +182,8 @@ func (hs *HTTPServer) registerRoutes() { // team (admin permission required) apiRoute.Group("/teams", func(teamsRoute routing.RouteRegister) { teamsRoute.Post("/", authorize(reqCanAccessTeams, ac.EvalPermission(ac.ActionTeamsCreate)), routing.Wrap(hs.CreateTeam)) - teamsRoute.Put("/:teamId", reqCanAccessTeams, routing.Wrap(hs.UpdateTeam)) - teamsRoute.Delete("/:teamId", reqCanAccessTeams, routing.Wrap(hs.DeleteTeamByID)) + teamsRoute.Put("/:teamId", authorize(reqCanAccessTeams, ac.EvalPermission(ac.ActionTeamsWrite, ac.ScopeTeamsID)), routing.Wrap(hs.UpdateTeam)) + teamsRoute.Delete("/:teamId", authorize(reqCanAccessTeams, ac.EvalPermission(ac.ActionTeamsDelete, ac.ScopeTeamsID)), routing.Wrap(hs.DeleteTeamByID)) teamsRoute.Get("/:teamId/members", authorize(reqCanAccessTeams, ac.EvalPermission(ac.ActionTeamsPermissionsRead, ac.ScopeTeamsID)), routing.Wrap(hs.GetTeamMembers)) 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)) diff --git a/pkg/api/roles.go b/pkg/api/roles.go index d56d2ff7202..fc0ed14ca7f 100644 --- a/pkg/api/roles.go +++ b/pkg/api/roles.go @@ -198,17 +198,35 @@ func (hs *HTTPServer) declareFixedRoles() error { Group: "Teams", Version: 1, Permissions: []accesscontrol.Permission{ - { - Action: accesscontrol.ActionTeamsCreate, - }, + {Action: accesscontrol.ActionTeamsCreate}, }, }, Grants: teamCreatorGrants, } + teamsWriterRole := accesscontrol.RoleRegistration{ + Role: accesscontrol.RoleDTO{ + Name: "fixed:teams:writer", + DisplayName: "Team writer", + Description: "Create, read, write, or delete a team as well as controlling team memberships.", + Group: "Teams", + Version: 1, + Permissions: []accesscontrol.Permission{ + {Action: accesscontrol.ActionTeamsCreate}, + {Action: accesscontrol.ActionTeamsDelete, Scope: accesscontrol.ScopeTeamsAll}, + {Action: accesscontrol.ActionTeamsPermissionsRead, Scope: accesscontrol.ScopeTeamsAll}, + {Action: accesscontrol.ActionTeamsPermissionsWrite, Scope: accesscontrol.ScopeTeamsAll}, + {Action: accesscontrol.ActionTeamsRead, Scope: accesscontrol.ScopeTeamsAll}, + {Action: accesscontrol.ActionTeamsWrite, Scope: accesscontrol.ScopeTeamsAll}, + }, + }, + Grants: []string{string(models.ROLE_ADMIN)}, + } + return hs.AccessControl.DeclareFixedRoles( provisioningWriterRole, datasourcesReaderRole, datasourcesWriterRole, datasourcesIdReaderRole, - datasourcesCompatibilityReaderRole, orgReaderRole, orgWriterRole, orgMaintainerRole, teamsCreatorRole, + datasourcesCompatibilityReaderRole, orgReaderRole, orgWriterRole, + orgMaintainerRole, teamsCreatorRole, teamsWriterRole, ) } diff --git a/pkg/api/team.go b/pkg/api/team.go index 53aea94d8ff..412e331cfa7 100644 --- a/pkg/api/team.go +++ b/pkg/api/team.go @@ -65,8 +65,10 @@ func (hs *HTTPServer) UpdateTeam(c *models.ReqContext) response.Response { return response.Error(http.StatusBadRequest, "teamId is invalid", err) } - if err := hs.teamGuardian.CanAdmin(c.Req.Context(), cmd.OrgId, cmd.Id, c.SignedInUser); err != nil { - return response.Error(403, "Not allowed to update team", err) + if !hs.Features.IsEnabled(featuremgmt.FlagAccesscontrol) { + if err := hs.teamGuardian.CanAdmin(c.Req.Context(), cmd.OrgId, cmd.Id, c.SignedInUser); err != nil { + return response.Error(403, "Not allowed to update team", err) + } } if err := hs.SQLStore.UpdateTeam(c.Req.Context(), &cmd); err != nil { @@ -88,8 +90,10 @@ func (hs *HTTPServer) DeleteTeamByID(c *models.ReqContext) response.Response { } user := c.SignedInUser - if err := hs.teamGuardian.CanAdmin(c.Req.Context(), orgId, teamId, user); err != nil { - return response.Error(403, "Not allowed to delete team", err) + if !hs.Features.IsEnabled(featuremgmt.FlagAccesscontrol) { + if err := hs.teamGuardian.CanAdmin(c.Req.Context(), orgId, teamId, user); err != nil { + return response.Error(403, "Not allowed to delete team", err) + } } if err := hs.SQLStore.DeleteTeam(c.Req.Context(), &models.DeleteTeamCommand{OrgId: orgId, Id: teamId}); err != nil { diff --git a/pkg/api/team_members_test.go b/pkg/api/team_members_test.go index 446fb9dd420..37dd49b424f 100644 --- a/pkg/api/team_members_test.go +++ b/pkg/api/team_members_test.go @@ -196,7 +196,7 @@ func TestAddTeamMembersAPIEndpoint_FGAC(t *testing.T) { setInitCtxSignedInOrgAdmin(sc.initCtx) newUserId = createUser(sc.db, testOrgId, t) - input = strings.NewReader(fmt.Sprintf(createTeamCmd, newUserId)) + input = strings.NewReader(fmt.Sprintf(teamCmd, newUserId)) t.Run("Access control prevents from adding a team member with the wrong permissions", func(t *testing.T) { setAccessControlPermissions(sc.acmock, []*accesscontrol.Permission{{Action: accesscontrol.ActionTeamsPermissionsRead, Scope: "teams:id:1"}}, 1) response := callAPI(sc.server, http.MethodPost, fmt.Sprintf(teamMemberAddRoute, "1"), input, t) diff --git a/pkg/api/team_test.go b/pkg/api/team_test.go index f3118ed687e..f29d93d3642 100644 --- a/pkg/api/team_test.go +++ b/pkg/api/team_test.go @@ -134,16 +134,17 @@ func TestTeamAPIEndpoint(t *testing.T) { }) } -var ( +const ( createTeamURL = "/api/teams/" - createTeamCmd = `{"name": "MyTestTeam%d"}` + detailTeamURL = "/api/teams/%d" + teamCmd = `{"name": "MyTestTeam%d"}` ) func TestTeamAPIEndpoint_CreateTeam_LegacyAccessControl(t *testing.T) { sc := setupHTTPServer(t, true, false) setInitCtxSignedInOrgAdmin(sc.initCtx) - input := strings.NewReader(fmt.Sprintf(createTeamCmd, 1)) + input := strings.NewReader(fmt.Sprintf(teamCmd, 1)) t.Run("Organisation admin can create a team", func(t *testing.T) { response := callAPI(sc.server, http.MethodPost, createTeamURL, input, t) assert.Equal(t, http.StatusOK, response.Code) @@ -151,9 +152,9 @@ func TestTeamAPIEndpoint_CreateTeam_LegacyAccessControl(t *testing.T) { setInitCtxSignedInEditor(sc.initCtx) sc.initCtx.IsGrafanaAdmin = true - input = strings.NewReader(fmt.Sprintf(createTeamCmd, 2)) + input = strings.NewReader(fmt.Sprintf(teamCmd, 2)) t.Run("Org editor and server admin cannot create a team", func(t *testing.T) { - response := callAPI(sc.server, http.MethodPost, createTeamURL, strings.NewReader(createTeamCmd), t) + response := callAPI(sc.server, http.MethodPost, createTeamURL, strings.NewReader(teamCmd), t) assert.Equal(t, http.StatusForbidden, response.Code) }) } @@ -164,7 +165,7 @@ func TestTeamAPIEndpoint_CreateTeam_LegacyAccessControl_EditorsCanAdmin(t *testi sc := setupHTTPServerWithCfg(t, true, false, cfg) setInitCtxSignedInEditor(sc.initCtx) - input := strings.NewReader(fmt.Sprintf(createTeamCmd, 1)) + input := strings.NewReader(fmt.Sprintf(teamCmd, 1)) t.Run("Editors can create a team if editorsCanAdmin is set to true", func(t *testing.T) { response := callAPI(sc.server, http.MethodPost, createTeamURL, input, t) assert.Equal(t, http.StatusOK, response.Code) @@ -175,17 +176,98 @@ func TestTeamAPIEndpoint_CreateTeam_FGAC(t *testing.T) { sc := setupHTTPServer(t, true, true) setInitCtxSignedInViewer(sc.initCtx) - input := strings.NewReader(fmt.Sprintf(createTeamCmd, 1)) + input := strings.NewReader(fmt.Sprintf(teamCmd, 1)) t.Run("Access control allows creating teams with the correct permissions", func(t *testing.T) { setAccessControlPermissions(sc.acmock, []*accesscontrol.Permission{{Action: accesscontrol.ActionTeamsCreate}}, 1) response := callAPI(sc.server, http.MethodPost, createTeamURL, input, t) assert.Equal(t, http.StatusOK, response.Code) }) - input = strings.NewReader(fmt.Sprintf(createTeamCmd, 2)) + input = strings.NewReader(fmt.Sprintf(teamCmd, 2)) t.Run("Access control prevents creating teams with the incorrect permissions", func(t *testing.T) { setAccessControlPermissions(sc.acmock, []*accesscontrol.Permission{{Action: "teams:invalid"}}, accesscontrol.GlobalOrgID) response := callAPI(sc.server, http.MethodPost, createTeamURL, input, 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_UpdateTeam_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(fmt.Sprintf(teamCmd, 1)) + t.Run("Access control allows updating teams 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(detailTeamURL, 1), input, t) + assert.Equal(t, http.StatusOK, response.Code) + + teamQuery := &models.GetTeamByIdQuery{OrgId: 1, SignedInUser: sc.initCtx.SignedInUser, Id: 1, Result: &models.TeamDTO{}} + err := sc.db.GetTeamById(context.Background(), teamQuery) + require.NoError(t, err) + assert.Equal(t, "MyTestTeam1", teamQuery.Result.Name) + }) + + input = strings.NewReader(fmt.Sprintf(teamCmd, 2)) + t.Run("Access control allows updating teams with the correct global permissions", func(t *testing.T) { + setAccessControlPermissions(sc.acmock, []*accesscontrol.Permission{{Action: accesscontrol.ActionTeamsWrite, Scope: "teams:id:*"}}, 1) + response := callAPI(sc.server, http.MethodPut, fmt.Sprintf(detailTeamURL, 1), input, t) + assert.Equal(t, http.StatusOK, response.Code) + + teamQuery := &models.GetTeamByIdQuery{OrgId: 1, SignedInUser: sc.initCtx.SignedInUser, Id: 1, Result: &models.TeamDTO{}} + err := sc.db.GetTeamById(context.Background(), teamQuery) + require.NoError(t, err) + assert.Equal(t, "MyTestTeam2", teamQuery.Result.Name) + }) + + input = strings.NewReader(fmt.Sprintf(teamCmd, 3)) + t.Run("Access control prevents updating teams 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(detailTeamURL, 1), input, t) + assert.Equal(t, http.StatusForbidden, response.Code) + + teamQuery := &models.GetTeamByIdQuery{OrgId: 1, SignedInUser: sc.initCtx.SignedInUser, Id: 1, Result: &models.TeamDTO{}} + err := sc.db.GetTeamById(context.Background(), teamQuery) + assert.NoError(t, err) + assert.Equal(t, "MyTestTeam2", teamQuery.Result.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.ActionTeamsDelete with teams:id:1 scope +// else return 403 +func TestTeamAPIEndpoint_DeleteTeam_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 prevents deleting teams with the incorrect permissions", func(t *testing.T) { + setAccessControlPermissions(sc.acmock, []*accesscontrol.Permission{{Action: accesscontrol.ActionTeamsDelete, Scope: "teams:id:7"}}, 1) + response := callAPI(sc.server, http.MethodDelete, fmt.Sprintf(detailTeamURL, 1), http.NoBody, t) + assert.Equal(t, http.StatusForbidden, response.Code) + + teamQuery := &models.GetTeamByIdQuery{OrgId: 1, SignedInUser: sc.initCtx.SignedInUser, Id: 1, Result: &models.TeamDTO{}} + err := sc.db.GetTeamById(context.Background(), teamQuery) + require.NoError(t, err) + }) + + t.Run("Access control allows deleting teams with the correct permissions", func(t *testing.T) { + setAccessControlPermissions(sc.acmock, []*accesscontrol.Permission{{Action: accesscontrol.ActionTeamsDelete, Scope: "teams:id:1"}}, 1) + response := callAPI(sc.server, http.MethodDelete, fmt.Sprintf(detailTeamURL, 1), http.NoBody, t) + assert.Equal(t, http.StatusOK, response.Code) + + teamQuery := &models.GetTeamByIdQuery{OrgId: 1, SignedInUser: sc.initCtx.SignedInUser, Id: 1, Result: &models.TeamDTO{}} + err := sc.db.GetTeamById(context.Background(), teamQuery) + require.ErrorIs(t, err, models.ErrTeamNotFound) + }) +} diff --git a/pkg/services/accesscontrol/models.go b/pkg/services/accesscontrol/models.go index b5f2148139e..ae35ef78628 100644 --- a/pkg/services/accesscontrol/models.go +++ b/pkg/services/accesscontrol/models.go @@ -319,6 +319,9 @@ const ( ActionTeamsWrite = "teams:write" ActionTeamsPermissionsRead = "teams.permissions:read" ActionTeamsPermissionsWrite = "teams.permissions:write" + + // Team related scopes + ScopeTeamsAll = "teams:*" ) var ( diff --git a/pkg/services/sqlstore/team.go b/pkg/services/sqlstore/team.go index 284b2012233..5dd220e251a 100644 --- a/pkg/services/sqlstore/team.go +++ b/pkg/services/sqlstore/team.go @@ -9,6 +9,7 @@ import ( "github.com/grafana/grafana/pkg/bus" "github.com/grafana/grafana/pkg/models" + ac "github.com/grafana/grafana/pkg/services/accesscontrol" ) func (ss *SQLStore) addTeamQueryAndCommandHandlers() { @@ -147,6 +148,7 @@ func (ss *SQLStore) DeleteTeam(ctx context.Context, cmd *models.DeleteTeamComman "DELETE FROM team_member WHERE org_id=? and team_id = ?", "DELETE FROM team WHERE org_id=? and id = ?", "DELETE FROM dashboard_acl WHERE org_id=? and team_id = ?", + "DELETE FROM team_role WHERE org_id=? and team_id = ?", } for _, sql := range deletes { @@ -155,7 +157,10 @@ func (ss *SQLStore) DeleteTeam(ctx context.Context, cmd *models.DeleteTeamComman return err } } - return nil + + _, err := sess.Exec("DELETE FROM permission WHERE scope=?", ac.Scope("teams", "id", fmt.Sprint(cmd.Id))) + + return err }) }