XYZ-76: Add license check to patchRoles endpoint. (#8224)
Этот коммит содержится в:
коммит произвёл
Jesús Espino
родитель
a735725d11
Коммит
fa5cba9cc7
26
api4/role.go
26
api4/role.go
@@ -7,6 +7,7 @@ import (
|
|||||||
"net/http"
|
"net/http"
|
||||||
|
|
||||||
"github.com/mattermost/mattermost-server/model"
|
"github.com/mattermost/mattermost-server/model"
|
||||||
|
"github.com/mattermost/mattermost-server/utils"
|
||||||
)
|
)
|
||||||
|
|
||||||
func (api *API) InitRole() {
|
func (api *API) InitRole() {
|
||||||
@@ -85,6 +86,31 @@ func patchRole(c *Context, w http.ResponseWriter, r *http.Request) {
|
|||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if !utils.IsLicensed() && patch.Permissions != nil {
|
||||||
|
allowedPermissions := []string{
|
||||||
|
model.PERMISSION_CREATE_TEAM.Id,
|
||||||
|
model.PERMISSION_MANAGE_WEBHOOKS.Id,
|
||||||
|
model.PERMISSION_MANAGE_SLASH_COMMANDS.Id,
|
||||||
|
model.PERMISSION_MANAGE_OAUTH.Id,
|
||||||
|
model.PERMISSION_MANAGE_SYSTEM_WIDE_OAUTH.Id,
|
||||||
|
}
|
||||||
|
|
||||||
|
changedPermissions := model.PermissionsChangedByPatch(oldRole, patch)
|
||||||
|
for _, permission := range changedPermissions {
|
||||||
|
allowed := false
|
||||||
|
for _, allowedPermission := range allowedPermissions {
|
||||||
|
if permission == allowedPermission {
|
||||||
|
allowed = true
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
if !allowed {
|
||||||
|
c.Err = model.NewAppError("Api4.PatchRoles", "api.roles.patch_roles.license.error", nil, "", http.StatusNotImplemented)
|
||||||
|
return
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
if !c.App.SessionHasPermissionTo(c.Session, model.PERMISSION_MANAGE_SYSTEM) {
|
if !c.App.SessionHasPermissionTo(c.Session, model.PERMISSION_MANAGE_SYSTEM) {
|
||||||
c.SetPermissionError(model.PERMISSION_MANAGE_SYSTEM)
|
c.SetPermissionError(model.PERMISSION_MANAGE_SYSTEM)
|
||||||
return
|
return
|
||||||
|
|||||||
@@ -10,6 +10,7 @@ import (
|
|||||||
"github.com/stretchr/testify/assert"
|
"github.com/stretchr/testify/assert"
|
||||||
|
|
||||||
"github.com/mattermost/mattermost-server/model"
|
"github.com/mattermost/mattermost-server/model"
|
||||||
|
"github.com/mattermost/mattermost-server/utils"
|
||||||
)
|
)
|
||||||
|
|
||||||
func TestGetRole(t *testing.T) {
|
func TestGetRole(t *testing.T) {
|
||||||
@@ -146,7 +147,7 @@ func TestPatchRole(t *testing.T) {
|
|||||||
Name: model.NewId(),
|
Name: model.NewId(),
|
||||||
DisplayName: model.NewId(),
|
DisplayName: model.NewId(),
|
||||||
Description: model.NewId(),
|
Description: model.NewId(),
|
||||||
Permissions: []string{"manage_system", "create_public_channel"},
|
Permissions: []string{"manage_system", "create_public_channel", "manage_slash_commands"},
|
||||||
SchemeManaged: true,
|
SchemeManaged: true,
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -156,7 +157,7 @@ func TestPatchRole(t *testing.T) {
|
|||||||
defer th.App.Srv.Store.Job().Delete(role.Id)
|
defer th.App.Srv.Store.Job().Delete(role.Id)
|
||||||
|
|
||||||
patch := &model.RolePatch{
|
patch := &model.RolePatch{
|
||||||
Permissions: &[]string{"manage_system", "delete_public_channel"},
|
Permissions: &[]string{"manage_system", "create_public_channel", "manage_webhooks"},
|
||||||
}
|
}
|
||||||
|
|
||||||
received, resp := th.SystemAdminClient.PatchRole(role.Id, patch)
|
received, resp := th.SystemAdminClient.PatchRole(role.Id, patch)
|
||||||
@@ -166,7 +167,7 @@ func TestPatchRole(t *testing.T) {
|
|||||||
assert.Equal(t, received.Name, role.Name)
|
assert.Equal(t, received.Name, role.Name)
|
||||||
assert.Equal(t, received.DisplayName, role.DisplayName)
|
assert.Equal(t, received.DisplayName, role.DisplayName)
|
||||||
assert.Equal(t, received.Description, role.Description)
|
assert.Equal(t, received.Description, role.Description)
|
||||||
assert.EqualValues(t, received.Permissions, []string{"manage_system", "delete_public_channel"})
|
assert.EqualValues(t, received.Permissions, []string{"manage_system", "create_public_channel", "manage_webhooks"})
|
||||||
assert.Equal(t, received.SchemeManaged, role.SchemeManaged)
|
assert.Equal(t, received.SchemeManaged, role.SchemeManaged)
|
||||||
|
|
||||||
// Check a no-op patch succeeds.
|
// Check a no-op patch succeeds.
|
||||||
@@ -181,4 +182,34 @@ func TestPatchRole(t *testing.T) {
|
|||||||
|
|
||||||
received, resp = th.Client.PatchRole(role.Id, patch)
|
received, resp = th.Client.PatchRole(role.Id, patch)
|
||||||
CheckForbiddenStatus(t, resp)
|
CheckForbiddenStatus(t, resp)
|
||||||
|
|
||||||
|
// Check a change that the license would not allow.
|
||||||
|
patch = &model.RolePatch{
|
||||||
|
Permissions: &[]string{"manage_system", "manage_webhooks"},
|
||||||
|
}
|
||||||
|
|
||||||
|
received, resp = th.SystemAdminClient.PatchRole(role.Id, patch)
|
||||||
|
CheckNotImplementedStatus(t, resp)
|
||||||
|
|
||||||
|
// Add a license.
|
||||||
|
isLicensed := utils.IsLicensed()
|
||||||
|
license := utils.License()
|
||||||
|
defer func() {
|
||||||
|
utils.SetIsLicensed(isLicensed)
|
||||||
|
utils.SetLicense(license)
|
||||||
|
}()
|
||||||
|
utils.SetIsLicensed(true)
|
||||||
|
utils.SetLicense(&model.License{Features: &model.Features{}})
|
||||||
|
utils.License().Features.SetDefaults()
|
||||||
|
|
||||||
|
// Try again, should succeed
|
||||||
|
received, resp = th.SystemAdminClient.PatchRole(role.Id, patch)
|
||||||
|
CheckNoError(t, resp)
|
||||||
|
|
||||||
|
assert.Equal(t, received.Id, role.Id)
|
||||||
|
assert.Equal(t, received.Name, role.Name)
|
||||||
|
assert.Equal(t, received.DisplayName, role.DisplayName)
|
||||||
|
assert.Equal(t, received.Description, role.Description)
|
||||||
|
assert.EqualValues(t, received.Permissions, []string{"manage_system", "manage_webhooks"})
|
||||||
|
assert.Equal(t, received.SchemeManaged, role.SchemeManaged)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -47,6 +47,10 @@
|
|||||||
"id": "September",
|
"id": "September",
|
||||||
"translation": "September"
|
"translation": "September"
|
||||||
},
|
},
|
||||||
|
{
|
||||||
|
"id": "api.roles.patch_roles.license.error",
|
||||||
|
"translation": "Your current license does not support advanced permissions."
|
||||||
|
},
|
||||||
{
|
{
|
||||||
"id": "api.admin.add_certificate.no_file.app_error",
|
"id": "api.admin.add_certificate.no_file.app_error",
|
||||||
"translation": "No file under 'certificate' in request."
|
"translation": "No file under 'certificate' in request."
|
||||||
|
|||||||
@@ -81,6 +81,41 @@ func (o *Role) Patch(patch *RolePatch) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Returns an array of permissions that are in either role.Permissions
|
||||||
|
// or patch.Permissions, but not both.
|
||||||
|
func PermissionsChangedByPatch(role *Role, patch *RolePatch) []string {
|
||||||
|
var result []string
|
||||||
|
|
||||||
|
if patch.Permissions == nil {
|
||||||
|
return result
|
||||||
|
}
|
||||||
|
|
||||||
|
roleMap := make(map[string]bool)
|
||||||
|
patchMap := make(map[string]bool)
|
||||||
|
|
||||||
|
for _, permission := range role.Permissions {
|
||||||
|
roleMap[permission] = true
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, permission := range *patch.Permissions {
|
||||||
|
patchMap[permission] = true
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, permission := range role.Permissions {
|
||||||
|
if !patchMap[permission] {
|
||||||
|
result = append(result, permission)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, permission := range *patch.Permissions {
|
||||||
|
if !roleMap[permission] {
|
||||||
|
result = append(result, permission)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
return result
|
||||||
|
}
|
||||||
|
|
||||||
func (role *Role) IsValid() bool {
|
func (role *Role) IsValid() bool {
|
||||||
if len(role.Id) != 26 {
|
if len(role.Id) != 26 {
|
||||||
return false
|
return false
|
||||||
|
|||||||
Ссылка в новой задаче
Block a user