Teams: Restrict provisioned teams from being updated and deleted (#103454)
* restrict provisioned teams from being updated and deleted * check if team is provisioned before update and delete * add function getTeamDTOByID() * check if team is provisioned in access control * fix TestDeleteTeamMembersAPIEndpoint * add unit tests * add function for validating a team
This commit is contained in:
@@ -2,6 +2,7 @@ package ossaccesscontrol
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"fmt"
|
||||
"strconv"
|
||||
|
||||
@@ -54,7 +55,7 @@ func ProvideTeamPermissions(
|
||||
return err
|
||||
}
|
||||
|
||||
_, err = teamService.GetTeamByID(ctx, &team.GetTeamByIDQuery{
|
||||
existingTeam, err := teamService.GetTeamByID(ctx, &team.GetTeamByIDQuery{
|
||||
OrgID: orgID,
|
||||
ID: id,
|
||||
})
|
||||
@@ -62,6 +63,10 @@ func ProvideTeamPermissions(
|
||||
return err
|
||||
}
|
||||
|
||||
if existingTeam.IsProvisioned {
|
||||
return errors.New("team permissions cannot be updated for provisioned teams")
|
||||
}
|
||||
|
||||
return nil
|
||||
},
|
||||
Assignments: resourcepermissions.Assignments{
|
||||
|
||||
@@ -89,6 +89,19 @@ func (tapi *TeamAPI) updateTeam(c *contextmodel.ReqContext) response.Response {
|
||||
return response.Error(http.StatusBadRequest, "teamId is invalid", err)
|
||||
}
|
||||
|
||||
existingTeam, err := tapi.getTeamDTOByID(c, cmd.ID)
|
||||
if err != nil {
|
||||
if errors.Is(err, team.ErrTeamNotFound) {
|
||||
return response.Error(http.StatusNotFound, "Team not found", err)
|
||||
}
|
||||
|
||||
return response.Error(http.StatusInternalServerError, "Failed to get Team", err)
|
||||
}
|
||||
|
||||
if existingTeam.IsProvisioned && existingTeam.Name != cmd.Name {
|
||||
return response.Error(http.StatusBadRequest, "Team name cannot be changed for provisioned teams", err)
|
||||
}
|
||||
|
||||
if err := tapi.teamService.UpdateTeam(c.Req.Context(), &cmd); err != nil {
|
||||
if errors.Is(err, team.ErrTeamNameTaken) {
|
||||
return response.Error(http.StatusBadRequest, "Team name taken", err)
|
||||
@@ -116,6 +129,11 @@ func (tapi *TeamAPI) deleteTeamByID(c *contextmodel.ReqContext) response.Respons
|
||||
return response.Error(http.StatusBadRequest, "teamId is invalid", err)
|
||||
}
|
||||
|
||||
resp := tapi.validateTeam(c, teamID, "Cannot delete provisioned teams")
|
||||
if resp != nil {
|
||||
return resp
|
||||
}
|
||||
|
||||
if err := tapi.teamService.DeleteTeam(c.Req.Context(), &team.DeleteTeamCommand{OrgID: orgID, ID: teamID}); err != nil {
|
||||
if errors.Is(err, team.ErrTeamNotFound) {
|
||||
return response.Error(http.StatusNotFound, "Failed to delete Team. ID not found", nil)
|
||||
@@ -392,3 +410,30 @@ func (tapi *TeamAPI) getAccessControlMetadata(c *contextmodel.ReqContext,
|
||||
ids := map[string]bool{resourceID: true}
|
||||
return tapi.getMultiAccessControlMetadata(c, prefix, ids)[resourceID]
|
||||
}
|
||||
|
||||
func (tapi *TeamAPI) getTeamDTOByID(c *contextmodel.ReqContext, teamID int64) (*team.TeamDTO, error) {
|
||||
query := team.GetTeamByIDQuery{
|
||||
OrgID: c.SignedInUser.GetOrgID(),
|
||||
ID: teamID,
|
||||
SignedInUser: c.SignedInUser,
|
||||
}
|
||||
|
||||
return tapi.teamService.GetTeamByID(c.Req.Context(), &query)
|
||||
}
|
||||
|
||||
func (tapi *TeamAPI) validateTeam(c *contextmodel.ReqContext, teamID int64, provisionedMessage string) *response.NormalResponse {
|
||||
teamDTO, err := tapi.getTeamDTOByID(c, teamID)
|
||||
if err != nil {
|
||||
if errors.Is(err, team.ErrTeamNotFound) {
|
||||
return response.Error(http.StatusNotFound, "Team not found", err)
|
||||
}
|
||||
|
||||
return response.Error(http.StatusInternalServerError, "Failed to get Team", err)
|
||||
}
|
||||
|
||||
if teamDTO.IsProvisioned {
|
||||
return response.Error(http.StatusBadRequest, provisionedMessage, err)
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
@@ -83,6 +83,11 @@ func (tapi *TeamAPI) addTeamMember(c *contextmodel.ReqContext) response.Response
|
||||
return response.Error(http.StatusBadRequest, "teamId is invalid", err)
|
||||
}
|
||||
|
||||
resp := tapi.validateTeam(c, teamID, "Team memberships cannot be updated for provisioned teams")
|
||||
if resp != nil {
|
||||
return resp
|
||||
}
|
||||
|
||||
isTeamMember, err := tapi.teamService.IsTeamMember(c.Req.Context(), c.SignedInUser.GetOrgID(), teamID, cmd.UserID)
|
||||
if err != nil {
|
||||
return response.Error(http.StatusInternalServerError, "Failed to add team member.", err)
|
||||
@@ -129,6 +134,11 @@ func (tapi *TeamAPI) updateTeamMember(c *contextmodel.ReqContext) response.Respo
|
||||
}
|
||||
orgId := c.SignedInUser.GetOrgID()
|
||||
|
||||
resp := tapi.validateTeam(c, teamId, "Team memberships cannot be updated for provisioned teams")
|
||||
if resp != nil {
|
||||
return resp
|
||||
}
|
||||
|
||||
isTeamMember, err := tapi.teamService.IsTeamMember(c.Req.Context(), orgId, teamId, userId)
|
||||
if err != nil {
|
||||
return response.Error(http.StatusInternalServerError, "Failed to update team member.", err)
|
||||
@@ -168,6 +178,11 @@ func (tapi *TeamAPI) setTeamMemberships(c *contextmodel.ReqContext) response.Res
|
||||
}
|
||||
orgId := c.SignedInUser.GetOrgID()
|
||||
|
||||
resp := tapi.validateTeam(c, teamId, "Team memberships cannot be updated for provisioned teams")
|
||||
if resp != nil {
|
||||
return resp
|
||||
}
|
||||
|
||||
teamMemberships, err := tapi.getTeamMembershipUpdates(c.Req.Context(), orgId, teamId, cmd, c.SignedInUser)
|
||||
if err != nil {
|
||||
if errors.Is(err, user.ErrUserNotFound) || errors.Is(err, team.ErrTeamNotFound) {
|
||||
@@ -275,6 +290,19 @@ func (tapi *TeamAPI) removeTeamMember(c *contextmodel.ReqContext) response.Respo
|
||||
return response.Error(http.StatusBadRequest, "userId is invalid", err)
|
||||
}
|
||||
|
||||
existingTeam, err := tapi.getTeamDTOByID(c, teamId)
|
||||
if err != nil {
|
||||
if errors.Is(err, team.ErrTeamNotFound) {
|
||||
return response.Error(http.StatusNotFound, "Team not found", err)
|
||||
}
|
||||
|
||||
return response.Error(http.StatusInternalServerError, "Failed to get Team", err)
|
||||
}
|
||||
|
||||
if existingTeam.IsProvisioned {
|
||||
return response.Error(http.StatusBadRequest, "Team memberships cannot be updated for provisioned teams", err)
|
||||
}
|
||||
|
||||
teamIDString := strconv.FormatInt(teamId, 10)
|
||||
if _, err := tapi.teamPermissionsService.SetUserPermission(c.Req.Context(), orgId, accesscontrol.User{ID: userId}, teamIDString, ""); err != nil {
|
||||
if errors.Is(err, team.ErrTeamNotFound) {
|
||||
|
||||
@@ -169,9 +169,41 @@ func TestUpdateTeamMembersAPIEndpoint(t *testing.T) {
|
||||
})
|
||||
}
|
||||
|
||||
func TestUpdateTeamMembersFromProvisionedTeam(t *testing.T) {
|
||||
server := SetupAPITestServer(t, &teamtest.FakeService{
|
||||
ExpectedIsMember: true,
|
||||
ExpectedTeamDTO: &team.TeamDTO{ID: 1, UID: "a00001", IsProvisioned: true},
|
||||
})
|
||||
|
||||
t.Run("should not be able to update team member from a provisioned team", func(t *testing.T) {
|
||||
req := webtest.RequestWithSignedInUser(
|
||||
server.NewRequest(http.MethodPut, "/api/teams/1/members/1", strings.NewReader("{\"permission\": 1}")),
|
||||
authedUserWithPermissions(1, 1, []accesscontrol.Permission{{Action: accesscontrol.ActionTeamsPermissionsWrite, Scope: "teams:id:1"}}),
|
||||
)
|
||||
res, err := server.SendJSON(req)
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, http.StatusBadRequest, res.StatusCode)
|
||||
require.NoError(t, res.Body.Close())
|
||||
})
|
||||
|
||||
t.Run("should not be able to update team member from a provisioned team by team UID", func(t *testing.T) {
|
||||
req := webtest.RequestWithSignedInUser(
|
||||
server.NewRequest(http.MethodPut, "/api/teams/a00001/members/1", strings.NewReader("{\"permission\": 1}")),
|
||||
authedUserWithPermissions(1, 1, []accesscontrol.Permission{{Action: accesscontrol.ActionTeamsPermissionsWrite, Scope: "teams:id:1"}}),
|
||||
)
|
||||
res, err := server.SendJSON(req)
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, http.StatusBadRequest, res.StatusCode)
|
||||
require.NoError(t, res.Body.Close())
|
||||
})
|
||||
}
|
||||
|
||||
func TestDeleteTeamMembersAPIEndpoint(t *testing.T) {
|
||||
server := SetupAPITestServer(t, nil, func(hs *TeamAPI) {
|
||||
hs.teamService = &teamtest.FakeService{ExpectedIsMember: true}
|
||||
hs.teamService = &teamtest.FakeService{
|
||||
ExpectedIsMember: true,
|
||||
ExpectedTeamDTO: &team.TeamDTO{ID: 1, UID: "a00001"},
|
||||
}
|
||||
hs.teamPermissionsService = &actest.FakePermissionsService{}
|
||||
})
|
||||
|
||||
@@ -197,6 +229,27 @@ func TestDeleteTeamMembersAPIEndpoint(t *testing.T) {
|
||||
})
|
||||
}
|
||||
|
||||
func TestDeleteTeamMembersFromProvisionedTeam(t *testing.T) {
|
||||
server := SetupAPITestServer(t, nil, func(hs *TeamAPI) {
|
||||
hs.teamService = &teamtest.FakeService{
|
||||
ExpectedIsMember: true,
|
||||
ExpectedTeamDTO: &team.TeamDTO{ID: 1, UID: "a00001", IsProvisioned: true},
|
||||
}
|
||||
hs.teamPermissionsService = &actest.FakePermissionsService{}
|
||||
})
|
||||
|
||||
t.Run("should not be able to delete team member from a provisioned team", func(t *testing.T) {
|
||||
req := webtest.RequestWithSignedInUser(
|
||||
server.NewRequest(http.MethodDelete, "/api/teams/1/members/1", nil),
|
||||
authedUserWithPermissions(1, 1, []accesscontrol.Permission{{Action: accesscontrol.ActionTeamsPermissionsWrite, Scope: "teams:id:1"}}),
|
||||
)
|
||||
res, err := server.SendJSON(req)
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, http.StatusBadRequest, res.StatusCode)
|
||||
require.NoError(t, res.Body.Close())
|
||||
})
|
||||
}
|
||||
|
||||
func Test_getTeamMembershipUpdates(t *testing.T) {
|
||||
type testCase struct {
|
||||
description string
|
||||
|
||||
@@ -16,6 +16,7 @@ import {
|
||||
LinkButton,
|
||||
Pagination,
|
||||
Stack,
|
||||
Tag,
|
||||
TextLink,
|
||||
useStyles2,
|
||||
} from '@grafana/ui';
|
||||
@@ -43,6 +44,7 @@ const skeletonData: TeamWithRoles[] = new Array(3).fill(null).map((_, index) =>
|
||||
memberCount: 0,
|
||||
name: '',
|
||||
orgId: 0,
|
||||
isProvisioned: false,
|
||||
}));
|
||||
|
||||
export const TeamList = ({
|
||||
@@ -166,6 +168,16 @@ export const TeamList = ({
|
||||
},
|
||||
]
|
||||
: []),
|
||||
{
|
||||
id: 'isProvisioned',
|
||||
header: '',
|
||||
cell: ({ cell: { value } }: Cell<'isProvisioned'>) => {
|
||||
if (!hasFetched) {
|
||||
return <Skeleton width={240} />;
|
||||
}
|
||||
return !!value && <Tag colorIndex={14} name={'Provisioned'} />
|
||||
},
|
||||
},
|
||||
{
|
||||
id: 'actions',
|
||||
header: '',
|
||||
@@ -181,7 +193,7 @@ export const TeamList = ({
|
||||
}
|
||||
|
||||
const canReadTeam = contextSrv.hasPermissionInMetadata(AccessControlAction.ActionTeamsRead, original);
|
||||
const canDelete = contextSrv.hasPermissionInMetadata(AccessControlAction.ActionTeamsDelete, original);
|
||||
const canDelete = contextSrv.hasPermissionInMetadata(AccessControlAction.ActionTeamsDelete, original) && !original.isProvisioned;
|
||||
return (
|
||||
<Stack direction="row" justifyContent="flex-end" gap={2}>
|
||||
{canReadTeam && (
|
||||
|
||||
@@ -9,11 +9,15 @@ type TeamPermissionsProps = {
|
||||
|
||||
// TeamPermissions component replaces TeamMembers component when the accesscontrol feature flag is set
|
||||
const TeamPermissions = (props: TeamPermissionsProps) => {
|
||||
const canSetPermissions = contextSrv.hasPermissionInMetadata(
|
||||
let canSetPermissions = contextSrv.hasPermissionInMetadata(
|
||||
AccessControlAction.ActionTeamsPermissionsWrite,
|
||||
props.team
|
||||
);
|
||||
|
||||
if (props.team.isProvisioned) {
|
||||
canSetPermissions = false;
|
||||
}
|
||||
|
||||
return (
|
||||
<Permissions
|
||||
title=""
|
||||
|
||||
@@ -54,7 +54,7 @@ export const TeamSettings = ({ team, updateTeam }: Props) => {
|
||||
</Field>
|
||||
<Field
|
||||
label={t('teams.team-settings.label-name', 'Name')}
|
||||
disabled={!canWriteTeamSettings}
|
||||
disabled={!canWriteTeamSettings || !!team.isProvisioned}
|
||||
required
|
||||
invalid={!!errors.name}
|
||||
error="Name is required"
|
||||
|
||||
@@ -26,6 +26,7 @@ export const getMockTeam = (i = 1, uid = 'aaaaaa', overrides = {}): Team => {
|
||||
memberCount: i,
|
||||
accessControl: { isEditor: false },
|
||||
orgId: 0,
|
||||
isProvisioned: false,
|
||||
...overrides,
|
||||
};
|
||||
};
|
||||
|
||||
@@ -17,6 +17,7 @@ const loadingTeam = {
|
||||
accessControl: { isEditor: false },
|
||||
orgId: 0,
|
||||
updated: '',
|
||||
isProvisioned: false,
|
||||
};
|
||||
|
||||
export function buildNavModel(team: Team): NavModelItem {
|
||||
|
||||
@@ -44,6 +44,10 @@ export interface Team extends WithAccessControlMetadata {
|
||||
* OrgId is the ID of an organisation the team belongs to.
|
||||
*/
|
||||
orgId: number;
|
||||
/**
|
||||
* isProvisioned is set if the team has been provisioned from IdP.
|
||||
*/
|
||||
isProvisioned: boolean;
|
||||
}
|
||||
|
||||
export interface TeamWithRoles extends Team {
|
||||
|
||||
Reference in New Issue
Block a user