Team: Add validation for provisioned teams in setUserPermission endpoint (#103623)
* removed provisioned team validation from team permissions * validate team in setUserPermission
This commit is contained in:
@@ -2,7 +2,6 @@ package ossaccesscontrol
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"fmt"
|
||||
"strconv"
|
||||
|
||||
@@ -55,7 +54,7 @@ func ProvideTeamPermissions(
|
||||
return err
|
||||
}
|
||||
|
||||
existingTeam, err := teamService.GetTeamByID(ctx, &team.GetTeamByIDQuery{
|
||||
_, err = teamService.GetTeamByID(ctx, &team.GetTeamByIDQuery{
|
||||
OrgID: orgID,
|
||||
ID: id,
|
||||
})
|
||||
@@ -63,10 +62,6 @@ func ProvideTeamPermissions(
|
||||
return err
|
||||
}
|
||||
|
||||
if existingTeam.IsProvisioned {
|
||||
return errors.New("team permissions cannot be updated for provisioned teams")
|
||||
}
|
||||
|
||||
return nil
|
||||
},
|
||||
Assignments: resourcepermissions.Assignments{
|
||||
|
||||
@@ -273,6 +273,11 @@ func (a *api) setUserPermission(c *contextmodel.ReqContext) response.Response {
|
||||
}
|
||||
resourceID := web.Params(c.Req)[":resourceID"]
|
||||
|
||||
resp := a.validateTeamResource(c, resourceID)
|
||||
if resp != nil {
|
||||
return resp
|
||||
}
|
||||
|
||||
var cmd setPermissionCommand
|
||||
if err := web.Bind(c.Req, &cmd); err != nil {
|
||||
return response.Error(http.StatusBadRequest, "bad request data", err)
|
||||
@@ -445,6 +450,36 @@ func (a *api) setPermissions(c *contextmodel.ReqContext) response.Response {
|
||||
return response.Success("Permissions updated")
|
||||
}
|
||||
|
||||
func (a *api) validateTeamResource(c *contextmodel.ReqContext, resourceID string) response.Response {
|
||||
if a.service.options.Resource != "teams" {
|
||||
return nil
|
||||
}
|
||||
|
||||
teamID, err := strconv.ParseInt(resourceID, 10, 64)
|
||||
if err != nil {
|
||||
return response.Error(http.StatusBadRequest, "Invalid ResourceID", err)
|
||||
}
|
||||
|
||||
existingTeam, err := a.service.teamService.GetTeamByID(c.Req.Context(), &team.GetTeamByIDQuery{
|
||||
OrgID: c.SignedInUser.GetOrgID(),
|
||||
ID: teamID,
|
||||
SignedInUser: c.SignedInUser,
|
||||
})
|
||||
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 permissions cannot be updated for provisioned teams", nil)
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
func permissionSetResponse(cmd setPermissionCommand) response.Response {
|
||||
message := "Permission updated"
|
||||
if cmd.Permission == "" {
|
||||
|
||||
@@ -348,7 +348,6 @@ func TestApi_setTeamPermission(t *testing.T) {
|
||||
recorder := setPermission(t, server, testOptions.Resource, tt.resourceID, tt.permission, "teams", assignTo)
|
||||
assert.Equal(t, tt.expectedStatus, recorder.Code)
|
||||
|
||||
assert.Equal(t, tt.expectedStatus, recorder.Code)
|
||||
if tt.expectedStatus == http.StatusOK {
|
||||
permissions, _ := getPermission(t, server, testOptions.Resource, tt.resourceID)
|
||||
require.Len(t, permissions, 1)
|
||||
@@ -433,7 +432,6 @@ func TestApi_setUserPermission(t *testing.T) {
|
||||
recorder := setPermission(t, server, testOptions.Resource, tt.resourceID, tt.permission, "users", strconv.Itoa(int(tt.userID)))
|
||||
assert.Equal(t, tt.expectedStatus, recorder.Code)
|
||||
|
||||
assert.Equal(t, tt.expectedStatus, recorder.Code)
|
||||
if tt.expectedStatus == http.StatusOK {
|
||||
permissions, _ := getPermission(t, server, testOptions.Resource, tt.resourceID)
|
||||
require.Len(t, permissions, 1)
|
||||
@@ -444,6 +442,98 @@ func TestApi_setUserPermission(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestApi_setUserPermissionForTeams(t *testing.T) {
|
||||
type setUserPermissionForTeamsTestCase struct {
|
||||
setUserPermissionTestCase
|
||||
teamCmd *team.CreateTeamCommand
|
||||
}
|
||||
tests := []setUserPermissionForTeamsTestCase{
|
||||
{
|
||||
setUserPermissionTestCase: setUserPermissionTestCase{
|
||||
desc: "should set Member permission for user 1",
|
||||
userID: 1,
|
||||
expectedStatus: 200,
|
||||
permission: "Member",
|
||||
permissions: []accesscontrol.Permission{
|
||||
{Action: "teams.permissions:read", Scope: accesscontrol.ScopeTeamsAll},
|
||||
{Action: "teams.permissions:write", Scope: accesscontrol.ScopeTeamsAll},
|
||||
{Action: accesscontrol.ActionOrgUsersRead, Scope: accesscontrol.ScopeUsersAll},
|
||||
},
|
||||
},
|
||||
teamCmd: &team.CreateTeamCommand{
|
||||
Name: "test",
|
||||
Email: "test@test.com",
|
||||
OrgID: 1,
|
||||
},
|
||||
},
|
||||
{
|
||||
setUserPermissionTestCase: setUserPermissionTestCase{
|
||||
desc: "should set Admin permission for user 1",
|
||||
userID: 1,
|
||||
expectedStatus: 200,
|
||||
permission: "Admin",
|
||||
permissions: []accesscontrol.Permission{
|
||||
{Action: "teams.permissions:read", Scope: accesscontrol.ScopeTeamsAll},
|
||||
{Action: "teams.permissions:write", Scope: accesscontrol.ScopeTeamsAll},
|
||||
{Action: accesscontrol.ActionOrgUsersRead, Scope: accesscontrol.ScopeUsersAll},
|
||||
},
|
||||
},
|
||||
teamCmd: &team.CreateTeamCommand{
|
||||
Name: "test",
|
||||
Email: "test@test.com",
|
||||
OrgID: 1,
|
||||
},
|
||||
},
|
||||
{
|
||||
setUserPermissionTestCase: setUserPermissionTestCase{
|
||||
desc: "should return status 400 for a provisioned team",
|
||||
userID: 1,
|
||||
expectedStatus: 400,
|
||||
permission: "Member",
|
||||
permissions: []accesscontrol.Permission{
|
||||
{Action: "teams.permissions:read", Scope: accesscontrol.ScopeTeamsAll},
|
||||
{Action: "teams.permissions:write", Scope: accesscontrol.ScopeTeamsAll},
|
||||
{Action: accesscontrol.ActionOrgUsersRead, Scope: accesscontrol.ScopeUsersAll},
|
||||
},
|
||||
},
|
||||
teamCmd: &team.CreateTeamCommand{
|
||||
Name: "test",
|
||||
Email: "test@test.com",
|
||||
OrgID: 1,
|
||||
IsProvisioned: true,
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.desc, func(t *testing.T) {
|
||||
service, usrSvc, teamSvc := setupTestEnvironment(t, testOptionsForTeams)
|
||||
server := setupTestServer(t, &user.SignedInUser{
|
||||
OrgID: 1,
|
||||
Permissions: map[int64]map[string][]string{1: accesscontrol.GroupScopesByActionContext(context.Background(), tt.permissions)},
|
||||
}, service)
|
||||
|
||||
_, err := usrSvc.Create(context.Background(), &user.CreateUserCommand{Login: "test", OrgID: 1})
|
||||
require.NoError(t, err)
|
||||
|
||||
expectedTeam, err := teamSvc.CreateTeam(context.Background(), tt.teamCmd)
|
||||
require.NoError(t, err)
|
||||
|
||||
resourceID := strconv.Itoa(int(expectedTeam.ID))
|
||||
|
||||
recorder := setPermission(t, server, testOptionsForTeams.Resource, resourceID, tt.permission, "users", strconv.Itoa(int(tt.userID)))
|
||||
assert.Equal(t, tt.expectedStatus, recorder.Code)
|
||||
|
||||
if tt.expectedStatus == http.StatusOK {
|
||||
permissions, _ := getPermission(t, server, testOptionsForTeams.Resource, resourceID)
|
||||
require.Len(t, permissions, 1)
|
||||
assert.Equal(t, tt.permission, permissions[0].Permission)
|
||||
assert.Equal(t, tt.userID, permissions[0].UserID)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func setupTestServer(t *testing.T, user *user.SignedInUser, service *Service) *web.Mux {
|
||||
server := web.New()
|
||||
server.UseMiddleware(web.Renderer("views", "[[", "]]"))
|
||||
@@ -484,6 +574,18 @@ var testOptions = Options{
|
||||
},
|
||||
}
|
||||
|
||||
var testOptionsForTeams = Options{
|
||||
Resource: "teams",
|
||||
ResourceAttribute: "id",
|
||||
Assignments: Assignments{
|
||||
Users: true,
|
||||
},
|
||||
PermissionsToActions: map[string][]string{
|
||||
"Member": {"teams:read"},
|
||||
"Admin": {"teams:read", "teams:write", "teams:delete"},
|
||||
},
|
||||
}
|
||||
|
||||
func getPermission(t *testing.T, server *web.Mux, resource, resourceID string) ([]resourcePermissionDTO, *httptest.ResponseRecorder) {
|
||||
req, err := http.NewRequest(http.MethodGet, fmt.Sprintf("/api/access-control/%s/%s", resource, resourceID), nil)
|
||||
require.NoError(t, err)
|
||||
|
||||
Reference in New Issue
Block a user