From fa5cba9cc79b3bdc48ad42f276652bc699795a39 Mon Sep 17 00:00:00 2001 From: George Goldberg Date: Thu, 8 Feb 2018 16:07:40 +0000 Subject: XYZ-76: Add license check to patchRoles endpoint. (#8224) --- api4/role.go | 26 ++++++++++++++++++++++++++ api4/role_test.go | 37 ++++++++++++++++++++++++++++++++++--- i18n/en.json | 4 ++++ model/role.go | 35 +++++++++++++++++++++++++++++++++++ 4 files changed, 99 insertions(+), 3 deletions(-) diff --git a/api4/role.go b/api4/role.go index a401a8034..ac9364c35 100644 --- a/api4/role.go +++ b/api4/role.go @@ -7,6 +7,7 @@ import ( "net/http" "github.com/mattermost/mattermost-server/model" + "github.com/mattermost/mattermost-server/utils" ) func (api *API) InitRole() { @@ -85,6 +86,31 @@ func patchRole(c *Context, w http.ResponseWriter, r *http.Request) { 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) { c.SetPermissionError(model.PERMISSION_MANAGE_SYSTEM) return diff --git a/api4/role_test.go b/api4/role_test.go index 64b8303e2..a3e6d35be 100644 --- a/api4/role_test.go +++ b/api4/role_test.go @@ -10,6 +10,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/mattermost/mattermost-server/model" + "github.com/mattermost/mattermost-server/utils" ) func TestGetRole(t *testing.T) { @@ -146,7 +147,7 @@ func TestPatchRole(t *testing.T) { Name: model.NewId(), DisplayName: model.NewId(), Description: model.NewId(), - Permissions: []string{"manage_system", "create_public_channel"}, + Permissions: []string{"manage_system", "create_public_channel", "manage_slash_commands"}, SchemeManaged: true, } @@ -156,7 +157,7 @@ func TestPatchRole(t *testing.T) { defer th.App.Srv.Store.Job().Delete(role.Id) 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) @@ -166,7 +167,7 @@ func TestPatchRole(t *testing.T) { 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", "delete_public_channel"}) + assert.EqualValues(t, received.Permissions, []string{"manage_system", "create_public_channel", "manage_webhooks"}) assert.Equal(t, received.SchemeManaged, role.SchemeManaged) // Check a no-op patch succeeds. @@ -181,4 +182,34 @@ func TestPatchRole(t *testing.T) { received, resp = th.Client.PatchRole(role.Id, patch) 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) } diff --git a/i18n/en.json b/i18n/en.json index 4708f321e..76f3d7ea6 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -47,6 +47,10 @@ "id": "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", "translation": "No file under 'certificate' in request." diff --git a/model/role.go b/model/role.go index 254513ae0..eedf97d25 100644 --- a/model/role.go +++ b/model/role.go @@ -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 { if len(role.Id) != 26 { return false -- cgit v1.2.3-1-g7c22