From 3b83cc7dd3fc8c6281bbd74b5b85a6a06efcbb6d Mon Sep 17 00:00:00 2001 From: Martin Kraft Date: Fri, 9 Feb 2018 10:57:07 -0500 Subject: XYZ-51: Unit tests for and changes to SetRolePermissionsFromConfig. (#8160) * XYZ-10: Role store. * XYZ-37: Update unit tests to work with database roles. * XYZ-51: Tests 'SetRolePermissionsFromConfig' against JSON from policy page. * XYZ-51: Adds permissions in non-licensed cases also. * XYZ-51: Removes some permissions from team_user role. * XYZ-51: Merge fix for change to default permissions from PR 8208. * XYZ-51: Removes unused function. --- app/app.go | 2 +- app/app_test.go | 5 +- utils/authorization.go | 54 ++-- utils/authorization_test.go | 125 ++++++++++ utils/policies-roles-mapping.json | 510 ++++++++++++++++++++++++++++++++++++++ 5 files changed, 667 insertions(+), 29 deletions(-) create mode 100644 utils/authorization_test.go create mode 100644 utils/policies-roles-mapping.json diff --git a/app/app.go b/app/app.go index 4cc9ff7df..ffb903b57 100644 --- a/app/app.go +++ b/app/app.go @@ -460,7 +460,7 @@ func (a *App) DoAdvancedPermissionsMigration() { l4g.Info("Migrating roles to database.") roles := model.MakeDefaultRoles() - roles = utils.SetRolePermissionsFromConfig(roles, a.Config()) + roles = utils.SetRolePermissionsFromConfig(roles, a.Config(), utils.IsLicensed()) allSucceeded := true diff --git a/app/app_test.go b/app/app_test.go index bbc012364..180bd21ee 100644 --- a/app/app_test.go +++ b/app/app_test.go @@ -5,6 +5,7 @@ package app import ( "flag" + "fmt" "os" "testing" @@ -275,6 +276,8 @@ func TestDoAdvancedPermissionsMigration(t *testing.T) { model.PERMISSION_GET_PUBLIC_LINK.Id, model.PERMISSION_CREATE_POST.Id, model.PERMISSION_USE_SLASH_COMMANDS.Id, + model.PERMISSION_DELETE_PUBLIC_CHANNEL.Id, + model.PERMISSION_DELETE_PRIVATE_CHANNEL.Id, model.PERMISSION_MANAGE_PRIVATE_CHANNEL_MEMBERS.Id, model.PERMISSION_DELETE_POST.Id, model.PERMISSION_EDIT_POST.Id, @@ -389,7 +392,7 @@ func TestDoAdvancedPermissionsMigration(t *testing.T) { for name, permissions := range expected2 { role, err := th.App.GetRoleByName(name) assert.Nil(t, err) - assert.Equal(t, permissions, role.Permissions) + assert.Equal(t, permissions, role.Permissions, fmt.Sprintf("'%v' did not have expected permissions", name)) } // Remove the license. diff --git a/utils/authorization.go b/utils/authorization.go index b17e94587..e8556458a 100644 --- a/utils/authorization.go +++ b/utils/authorization.go @@ -7,8 +7,8 @@ import ( "github.com/mattermost/mattermost-server/model" ) -func SetRolePermissionsFromConfig(roles map[string]*model.Role, cfg *model.Config) map[string]*model.Role { - if IsLicensed() { +func SetRolePermissionsFromConfig(roles map[string]*model.Role, cfg *model.Config, isLicensed bool) map[string]*model.Role { + if isLicensed { switch *cfg.TeamSettings.RestrictPublicChannelCreation { case model.PERMISSIONS_ALL: roles[model.TEAM_USER_ROLE_ID].Permissions = append( @@ -28,11 +28,11 @@ func SetRolePermissionsFromConfig(roles map[string]*model.Role, cfg *model.Confi ) } - if IsLicensed() { + if isLicensed { switch *cfg.TeamSettings.RestrictPublicChannelManagement { case model.PERMISSIONS_ALL: - roles[model.TEAM_USER_ROLE_ID].Permissions = append( - roles[model.TEAM_USER_ROLE_ID].Permissions, + roles[model.CHANNEL_USER_ROLE_ID].Permissions = append( + roles[model.CHANNEL_USER_ROLE_ID].Permissions, model.PERMISSION_MANAGE_PUBLIC_CHANNEL_PROPERTIES.Id, ) case model.PERMISSIONS_CHANNEL_ADMIN: @@ -51,17 +51,17 @@ func SetRolePermissionsFromConfig(roles map[string]*model.Role, cfg *model.Confi ) } } else { - roles[model.TEAM_USER_ROLE_ID].Permissions = append( - roles[model.TEAM_USER_ROLE_ID].Permissions, + roles[model.CHANNEL_USER_ROLE_ID].Permissions = append( + roles[model.CHANNEL_USER_ROLE_ID].Permissions, model.PERMISSION_MANAGE_PUBLIC_CHANNEL_PROPERTIES.Id, ) } - if IsLicensed() { + if isLicensed { switch *cfg.TeamSettings.RestrictPublicChannelDeletion { case model.PERMISSIONS_ALL: - roles[model.TEAM_USER_ROLE_ID].Permissions = append( - roles[model.TEAM_USER_ROLE_ID].Permissions, + roles[model.CHANNEL_USER_ROLE_ID].Permissions = append( + roles[model.CHANNEL_USER_ROLE_ID].Permissions, model.PERMISSION_DELETE_PUBLIC_CHANNEL.Id, ) case model.PERMISSIONS_CHANNEL_ADMIN: @@ -80,13 +80,13 @@ func SetRolePermissionsFromConfig(roles map[string]*model.Role, cfg *model.Confi ) } } else { - roles[model.TEAM_USER_ROLE_ID].Permissions = append( - roles[model.TEAM_USER_ROLE_ID].Permissions, + roles[model.CHANNEL_USER_ROLE_ID].Permissions = append( + roles[model.CHANNEL_USER_ROLE_ID].Permissions, model.PERMISSION_DELETE_PUBLIC_CHANNEL.Id, ) } - if IsLicensed() { + if isLicensed { switch *cfg.TeamSettings.RestrictPrivateChannelCreation { case model.PERMISSIONS_ALL: roles[model.TEAM_USER_ROLE_ID].Permissions = append( @@ -106,11 +106,11 @@ func SetRolePermissionsFromConfig(roles map[string]*model.Role, cfg *model.Confi ) } - if IsLicensed() { + if isLicensed { switch *cfg.TeamSettings.RestrictPrivateChannelManagement { case model.PERMISSIONS_ALL: - roles[model.TEAM_USER_ROLE_ID].Permissions = append( - roles[model.TEAM_USER_ROLE_ID].Permissions, + roles[model.CHANNEL_USER_ROLE_ID].Permissions = append( + roles[model.CHANNEL_USER_ROLE_ID].Permissions, model.PERMISSION_MANAGE_PRIVATE_CHANNEL_PROPERTIES.Id, ) case model.PERMISSIONS_CHANNEL_ADMIN: @@ -129,17 +129,17 @@ func SetRolePermissionsFromConfig(roles map[string]*model.Role, cfg *model.Confi ) } } else { - roles[model.TEAM_USER_ROLE_ID].Permissions = append( - roles[model.TEAM_USER_ROLE_ID].Permissions, + roles[model.CHANNEL_USER_ROLE_ID].Permissions = append( + roles[model.CHANNEL_USER_ROLE_ID].Permissions, model.PERMISSION_MANAGE_PRIVATE_CHANNEL_PROPERTIES.Id, ) } - if IsLicensed() { + if isLicensed { switch *cfg.TeamSettings.RestrictPrivateChannelDeletion { case model.PERMISSIONS_ALL: - roles[model.TEAM_USER_ROLE_ID].Permissions = append( - roles[model.TEAM_USER_ROLE_ID].Permissions, + roles[model.CHANNEL_USER_ROLE_ID].Permissions = append( + roles[model.CHANNEL_USER_ROLE_ID].Permissions, model.PERMISSION_DELETE_PRIVATE_CHANNEL.Id, ) case model.PERMISSIONS_CHANNEL_ADMIN: @@ -158,14 +158,14 @@ func SetRolePermissionsFromConfig(roles map[string]*model.Role, cfg *model.Confi ) } } else { - roles[model.TEAM_USER_ROLE_ID].Permissions = append( - roles[model.TEAM_USER_ROLE_ID].Permissions, + roles[model.CHANNEL_USER_ROLE_ID].Permissions = append( + roles[model.CHANNEL_USER_ROLE_ID].Permissions, model.PERMISSION_DELETE_PRIVATE_CHANNEL.Id, ) } // Restrict permissions for Private Channel Manage Members - if IsLicensed() { + if isLicensed { switch *cfg.TeamSettings.RestrictPrivateChannelManageMembers { case model.PERMISSIONS_ALL: roles[model.CHANNEL_USER_ROLE_ID].Permissions = append( @@ -207,7 +207,7 @@ func SetRolePermissionsFromConfig(roles map[string]*model.Role, cfg *model.Confi } // Grant permissions for inviting and adding users to a team. - if IsLicensed() { + if isLicensed { if *cfg.TeamSettings.RestrictTeamInvite == model.PERMISSIONS_TEAM_ADMIN { roles[model.TEAM_ADMIN_ROLE_ID].Permissions = append( roles[model.TEAM_ADMIN_ROLE_ID].Permissions, @@ -229,7 +229,7 @@ func SetRolePermissionsFromConfig(roles map[string]*model.Role, cfg *model.Confi ) } - if IsLicensed() { + if isLicensed { switch *cfg.ServiceSettings.RestrictPostDelete { case model.PERMISSIONS_DELETE_POST_ALL: roles[model.CHANNEL_USER_ROLE_ID].Permissions = append( @@ -267,7 +267,7 @@ func SetRolePermissionsFromConfig(roles map[string]*model.Role, cfg *model.Confi ) } - if IsLicensed() { + if isLicensed { switch *cfg.ServiceSettings.AllowEditPost { case model.ALLOW_EDIT_POST_ALWAYS, model.ALLOW_EDIT_POST_TIME_LIMIT: roles[model.CHANNEL_USER_ROLE_ID].Permissions = append( diff --git a/utils/authorization_test.go b/utils/authorization_test.go new file mode 100644 index 000000000..9e5b570bb --- /dev/null +++ b/utils/authorization_test.go @@ -0,0 +1,125 @@ +// Copyright (c) 2018-present Mattermost, Inc. All Rights Reserved. +// See License.txt for license information. + +package utils + +import ( + "encoding/json" + "fmt" + "io/ioutil" + "reflect" + "strings" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/mattermost/mattermost-server/model" +) + +type RoleState struct { + RoleName string `json:"roleName"` + Permission string `json:"permission"` + ShouldHave bool `json:"shouldHave"` +} + +func mockConfig() *model.Config { + config := model.Config{} + config.SetDefaults() + return &config +} + +func mapping() (map[string]map[string][]RoleState, error) { + + policiesRolesMapping := make(map[string]map[string][]RoleState) + + raw, err := ioutil.ReadFile("./policies-roles-mapping.json") + if err != nil { + return policiesRolesMapping, err + } + + var f map[string]interface{} + err = json.Unmarshal(raw, &f) + if err != nil { + return policiesRolesMapping, err + } + + for policyName, value := range f { + + capitalizedName := fmt.Sprintf("%v%v", strings.ToUpper(policyName[:1]), policyName[1:]) + policiesRolesMapping[capitalizedName] = make(map[string][]RoleState) + + for policyValue, roleStatesMappings := range value.(map[string]interface{}) { + + var roleStates []RoleState + for _, roleStateMapping := range roleStatesMappings.([]interface{}) { + + roleStateMappingJSON, _ := json.Marshal(roleStateMapping) + var roleState RoleState + _ = json.Unmarshal(roleStateMappingJSON, &roleState) + + roleStates = append(roleStates, roleState) + + } + + policiesRolesMapping[capitalizedName][policyValue] = roleStates + + } + + } + + return policiesRolesMapping, nil +} + +func TestSetRolePermissionsFromConfig(t *testing.T) { + + mapping, err := mapping() + if err != nil { + require.NoError(t, err) + } + + for policyName, v := range mapping { + for policyValue, rolesMappings := range v { + + config := mockConfig() + updateConfig(config, policyName, policyValue) + roles := model.MakeDefaultRoles() + SetRolePermissionsFromConfig(roles, config, true) + + for _, roleMappingItem := range rolesMappings { + role := roles[roleMappingItem.RoleName] + + permission := roleMappingItem.Permission + hasPermission := roleHasPermission(role, permission) + + if (roleMappingItem.ShouldHave && !hasPermission) || (!roleMappingItem.ShouldHave && hasPermission) { + wording := "not to" + if roleMappingItem.ShouldHave { + wording = "to" + } + t.Errorf("Expected '%v' %v have '%v' permission when '%v' is set to '%v'.", role.Name, wording, permission, policyName, policyValue) + } + + } + + } + } +} + +func updateConfig(config *model.Config, key string, value string) { + v := reflect.ValueOf(config.ServiceSettings) + field := v.FieldByName(key) + if !field.IsValid() { + v = reflect.ValueOf(config.TeamSettings) + field = v.FieldByName(key) + } + field.Elem().SetString(value) +} + +func roleHasPermission(role *model.Role, permission string) bool { + for _, p := range role.Permissions { + if p == permission { + return true + } + } + return false +} diff --git a/utils/policies-roles-mapping.json b/utils/policies-roles-mapping.json new file mode 100644 index 000000000..f083a854a --- /dev/null +++ b/utils/policies-roles-mapping.json @@ -0,0 +1,510 @@ +{ + "restrictTeamInvite": { + "all": [ + { + "roleName": "team_user", + "permission": "invite_user", + "shouldHave": true + } + ], + "team_admin": [ + { + "roleName": "team_user", + "permission": "invite_user", + "shouldHave": false + }, + { + "roleName": "team_admin", + "permission": "invite_user", + "shouldHave": true + } + ], + "system_admin": [ + { + "roleName": "team_user", + "permission": "invite_user", + "shouldHave": false + }, + { + "roleName": "team_admin", + "permission": "invite_user", + "shouldHave": false + } + ] + }, + "restrictPublicChannelCreation": { + "all": [ + { + "roleName": "team_user", + "permission": "create_public_channel", + "shouldHave": true + } + ], + "team_admin": [ + { + "roleName": "team_user", + "permission": "create_public_channel", + "shouldHave": false + }, + { + "roleName": "team_admin", + "permission": "create_public_channel", + "shouldHave": true + } + ], + "system_admin": [ + { + "roleName": "team_user", + "permission": "create_public_channel", + "shouldHave": false + }, + { + "roleName": "team_admin", + "permission": "create_public_channel", + "shouldHave": false + } + ] + }, + "restrictPrivateChannelCreation": { + "all": [ + { + "roleName": "team_user", + "permission": "create_private_channel", + "shouldHave": true + } + ], + "team_admin": [ + { + "roleName": "team_user", + "permission": "create_private_channel", + "shouldHave": false + }, + { + "roleName": "team_admin", + "permission": "create_private_channel", + "shouldHave": true + } + ], + "system_admin": [ + { + "roleName": "team_user", + "permission": "create_private_channel", + "shouldHave": false + }, + { + "roleName": "team_admin", + "permission": "create_private_channel", + "shouldHave": false + } + ] + }, + "restrictPublicChannelManagement": { + "all": [ + { + "roleName": "channel_user", + "permission": "manage_public_channel_properties", + "shouldHave": true + } + ], + "channel_admin": [ + { + "roleName": "channel_user", + "permission": "manage_public_channel_properties", + "shouldHave": false + }, + { + "roleName": "channel_admin", + "permission": "manage_public_channel_properties", + "shouldHave": true + }, + { + "roleName": "team_admin", + "permission": "manage_public_channel_properties", + "shouldHave": true + } + ], + "team_admin": [ + { + "roleName": "channel_user", + "permission": "manage_public_channel_properties", + "shouldHave": false + }, + { + "roleName": "channel_admin", + "permission": "manage_public_channel_properties", + "shouldHave": false + }, + { + "roleName": "team_admin", + "permission": "manage_public_channel_properties", + "shouldHave": true + } + ], + "system_admin": [ + { + "roleName": "channel_user", + "permission": "manage_public_channel_properties", + "shouldHave": false + }, + { + "roleName": "channel_admin", + "permission": "manage_public_channel_properties", + "shouldHave": false + }, + { + "roleName": "team_admin", + "permission": "manage_public_channel_properties", + "shouldHave": false + } + ] + }, + "restrictPublicChannelDeletion": { + "all": [ + { + "roleName": "channel_user", + "permission": "delete_public_channel", + "shouldHave": true + } + ], + "channel_admin": [ + { + "roleName": "channel_user", + "permission": "delete_public_channel", + "shouldHave": false + }, + { + "roleName": "channel_admin", + "permission": "delete_public_channel", + "shouldHave": true + }, + { + "roleName": "team_admin", + "permission": "delete_public_channel", + "shouldHave": true + } + ], + "team_admin": [ + { + "roleName": "channel_user", + "permission": "delete_public_channel", + "shouldHave": false + }, + { + "roleName": "channel_admin", + "permission": "delete_public_channel", + "shouldHave": false + }, + { + "roleName": "team_admin", + "permission": "delete_public_channel", + "shouldHave": true + } + ], + "system_admin": [ + { + "roleName": "channel_user", + "permission": "delete_public_channel", + "shouldHave": false + }, + { + "roleName": "channel_admin", + "permission": "delete_public_channel", + "shouldHave": false + }, + { + "roleName": "team_admin", + "permission": "delete_public_channel", + "shouldHave": false + } + ] + }, + "restrictPrivateChannelManagement": { + "all": [ + { + "roleName": "channel_user", + "permission": "manage_private_channel_properties", + "shouldHave": true + } + ], + "channel_admin": [ + { + "roleName": "channel_user", + "permission": "manage_private_channel_properties", + "shouldHave": false + }, + { + "roleName": "channel_admin", + "permission": "manage_private_channel_properties", + "shouldHave": true + }, + { + "roleName": "team_admin", + "permission": "manage_private_channel_properties", + "shouldHave": true + } + ], + "team_admin": [ + { + "roleName": "channel_user", + "permission": "manage_private_channel_properties", + "shouldHave": false + }, + { + "roleName": "channel_admin", + "permission": "manage_private_channel_properties", + "shouldHave": false + }, + { + "roleName": "team_admin", + "permission": "manage_private_channel_properties", + "shouldHave": true + } + ], + "system_admin": [ + { + "roleName": "channel_user", + "permission": "manage_private_channel_properties", + "shouldHave": false + }, + { + "roleName": "channel_admin", + "permission": "manage_private_channel_properties", + "shouldHave": false + }, + { + "roleName": "team_admin", + "permission": "manage_private_channel_properties", + "shouldHave": false + } + ] + }, + "restrictPrivateChannelManageMembers": { + "all": [ + { + "roleName": "channel_user", + "permission": "manage_private_channel_members", + "shouldHave": true + } + ], + "channel_admin": [ + { + "roleName": "channel_user", + "permission": "manage_private_channel_members", + "shouldHave": false + }, + { + "roleName": "channel_admin", + "permission": "manage_private_channel_members", + "shouldHave": true + }, + { + "roleName": "team_admin", + "permission": "manage_private_channel_members", + "shouldHave": true + } + ], + "team_admin": [ + { + "roleName": "channel_user", + "permission": "manage_private_channel_members", + "shouldHave": false + }, + { + "roleName": "channel_admin", + "permission": "manage_private_channel_members", + "shouldHave": false + }, + { + "roleName": "team_admin", + "permission": "manage_private_channel_members", + "shouldHave": true + } + ], + "system_admin": [ + { + "roleName": "channel_user", + "permission": "manage_private_channel_members", + "shouldHave": false + }, + { + "roleName": "channel_admin", + "permission": "manage_private_channel_members", + "shouldHave": false + }, + { + "roleName": "team_admin", + "permission": "manage_private_channel_members", + "shouldHave": false + } + ] + }, + "restrictPrivateChannelDeletion": { + "all": [ + { + "roleName": "channel_user", + "permission": "delete_private_channel", + "shouldHave": true + } + ], + "channel_admin": [ + { + "roleName": "channel_user", + "permission": "delete_private_channel", + "shouldHave": false + }, + { + "roleName": "channel_admin", + "permission": "delete_private_channel", + "shouldHave": true + }, + { + "roleName": "team_admin", + "permission": "delete_private_channel", + "shouldHave": true + } + ], + "team_admin": [ + { + "roleName": "channel_user", + "permission": "delete_private_channel", + "shouldHave": false + }, + { + "roleName": "channel_admin", + "permission": "delete_private_channel", + "shouldHave": false + }, + { + "roleName": "team_admin", + "permission": "delete_private_channel", + "shouldHave": true + } + ], + "system_admin": [ + { + "roleName": "channel_user", + "permission": "delete_private_channel", + "shouldHave": false + }, + { + "roleName": "channel_admin", + "permission": "delete_private_channel", + "shouldHave": false + }, + { + "roleName": "team_admin", + "permission": "delete_private_channel", + "shouldHave": false + } + ] + }, + "allowEditPost": { + "always": [ + { + "roleName": "channel_user", + "permission": "edit_post", + "shouldHave": true + }, + { + "roleName": "system_admin", + "permission": "edit_post", + "shouldHave": true + } + ], + "never": [ + { + "roleName": "channel_user", + "permission": "edit_post", + "shouldHave": false + }, + { + "roleName": "system_admin", + "permission": "edit_post", + "shouldHave": false + } + ] + }, + "restrictPostDelete": { + "all": [ + { + "roleName": "channel_user", + "permission": "delete_post", + "shouldHave": true + }, + { + "roleName": "channel_admin", + "permission": "delete_post", + "shouldHave": false + }, + { + "roleName": "channel_admin", + "permission": "delete_others_posts", + "shouldHave": false + }, + { + "roleName": "team_admin", + "permission": "delete_post", + "shouldHave": true + }, + { + "roleName": "team_admin", + "permission": "delete_others_posts", + "shouldHave": true + } + ], + "team_admin": [ + { + "roleName": "channel_user", + "permission": "delete_post", + "shouldHave": false + }, + { + "roleName": "channel_admin", + "permission": "delete_post", + "shouldHave": false + }, + { + "roleName": "channel_admin", + "permission": "delete_others_posts", + "shouldHave": false + }, + { + "roleName": "team_admin", + "permission": "delete_post", + "shouldHave": true + }, + { + "roleName": "team_admin", + "permission": "delete_others_posts", + "shouldHave": true + } + ], + "system_admin": [ + { + "roleName": "channel_user", + "permission": "delete_post", + "shouldHave": false + }, + { + "roleName": "channel_admin", + "permission": "delete_post", + "shouldHave": false + }, + { + "roleName": "channel_admin", + "permission": "delete_others_posts", + "shouldHave": false + }, + { + "roleName": "team_admin", + "permission": "delete_post", + "shouldHave": false + }, + { + "roleName": "team_admin", + "permission": "delete_others_posts", + "shouldHave": false + } + ] + } +} \ No newline at end of file -- cgit v1.2.3-1-g7c22