From ab338e8417218184d7596e3e9f438a09c2aebd63 Mon Sep 17 00:00:00 2001 From: Farhan Munshi <3207297+fm2munsh@users.noreply.github.com> Date: Mon, 30 Mar 2020 23:08:48 -0400 Subject: [PATCH] [MM-23019] Create use_group_mentions permission and migrate existing roles (#13987) * MM-22962 Create use_group_mentions permission and give to all non guest roles that can create post * Add use_group_mentions to team admin role for test * Trigger CI * MM-22962 Remove old migration keys --- app/app_test.go | 2 + app/permissions_migrations.go | 74 +++++++++++++++++++++--------- app/permissions_migrations_test.go | 3 +- model/migration.go | 1 + model/permission.go | 9 ++++ testlib/store.go | 1 + 6 files changed, 67 insertions(+), 23 deletions(-) diff --git a/app/app_test.go b/app/app_test.go index 557d9850c6..e19a1bc249 100644 --- a/app/app_test.go +++ b/app/app_test.go @@ -563,6 +563,7 @@ func TestDoEmojisPermissionsMigration(t *testing.T) { model.PERMISSION_DELETE_OTHERS_EMOJIS.Id, model.PERMISSION_VIEW_MEMBERS.Id, model.PERMISSION_USE_CHANNEL_MENTIONS.Id, + model.PERMISSION_USE_GROUP_MENTIONS.Id, } sort.Strings(expectedSystemAdmin) @@ -602,6 +603,7 @@ func TestDoEmojisPermissionsMigration(t *testing.T) { model.PERMISSION_MANAGE_PRIVATE_CHANNEL_MEMBERS.Id, model.PERMISSION_REMOVE_REACTION.Id, model.PERMISSION_USE_CHANNEL_MENTIONS.Id, + model.PERMISSION_USE_GROUP_MENTIONS.Id, } sort.Strings(expected2) sort.Strings(role2.Permissions) diff --git a/app/permissions_migrations.go b/app/permissions_migrations.go index 0fcc39147e..2ed2c5b120 100644 --- a/app/permissions_migrations.go +++ b/app/permissions_migrations.go @@ -4,11 +4,13 @@ package app import ( + "strings" + "github.com/mattermost/mattermost-server/v5/model" ) type permissionTransformation struct { - On func(string, map[string]map[string]bool) bool + On func(*model.Role, map[string]map[string]bool) bool Add []string Remove []string } @@ -49,42 +51,55 @@ const ( PERMISSION_USE_CHANNEL_MENTIONS = "use_channel_mentions" PERMISSION_CREATE_POST = "create_post" PERMISSION_CREATE_POST_PUBLIC = "create_post_public" + PERMISSION_USE_GROUP_MENTIONS = "use_group_mentions" PERMISSION_ADD_REACTION = "add_reaction" PERMISSION_REMOVE_REACTION = "remove_reaction" PERMISSION_MANAGE_PUBLIC_CHANNEL_MEMBERS = "manage_public_channel_members" PERMISSION_MANAGE_PRIVATE_CHANNEL_MEMBERS = "manage_private_channel_members" ) -func isRole(role string) func(string, map[string]map[string]bool) bool { - return func(roleName string, permissionsMap map[string]map[string]bool) bool { - return roleName == role +func isRole(roleName string) func(*model.Role, map[string]map[string]bool) bool { + return func(role *model.Role, permissionsMap map[string]map[string]bool) bool { + return role.Name == roleName } } -func permissionExists(permission string) func(string, map[string]map[string]bool) bool { - return func(roleName string, permissionsMap map[string]map[string]bool) bool { - val, ok := permissionsMap[roleName][permission] +func isNotRole(roleName string) func(*model.Role, map[string]map[string]bool) bool { + return func(role *model.Role, permissionsMap map[string]map[string]bool) bool { + return role.Name != roleName + } +} + +func isNotSchemeRole(roleName string) func(*model.Role, map[string]map[string]bool) bool { + return func(role *model.Role, permissionsMap map[string]map[string]bool) bool { + return !strings.Contains(role.DisplayName, roleName) + } +} + +func permissionExists(permission string) func(*model.Role, map[string]map[string]bool) bool { + return func(role *model.Role, permissionsMap map[string]map[string]bool) bool { + val, ok := permissionsMap[role.Name][permission] return ok && val } } -func permissionNotExists(permission string) func(string, map[string]map[string]bool) bool { - return func(roleName string, permissionsMap map[string]map[string]bool) bool { - val, ok := permissionsMap[roleName][permission] +func permissionNotExists(permission string) func(*model.Role, map[string]map[string]bool) bool { + return func(role *model.Role, permissionsMap map[string]map[string]bool) bool { + val, ok := permissionsMap[role.Name][permission] return !(ok && val) } } -func onOtherRole(otherRole string, function func(string, map[string]map[string]bool) bool) func(string, map[string]map[string]bool) bool { - return func(roleName string, permissionsMap map[string]map[string]bool) bool { - return function(otherRole, permissionsMap) +func onOtherRole(otherRole string, function func(*model.Role, map[string]map[string]bool) bool) func(*model.Role, map[string]map[string]bool) bool { + return func(role *model.Role, permissionsMap map[string]map[string]bool) bool { + return function(&model.Role{Name: otherRole}, permissionsMap) } } -func permissionOr(funcs ...func(string, map[string]map[string]bool) bool) func(string, map[string]map[string]bool) bool { - return func(roleName string, permissionsMap map[string]map[string]bool) bool { +func permissionOr(funcs ...func(*model.Role, map[string]map[string]bool) bool) func(*model.Role, map[string]map[string]bool) bool { + return func(role *model.Role, permissionsMap map[string]map[string]bool) bool { for _, f := range funcs { - if f(roleName, permissionsMap) { + if f(role, permissionsMap) { return true } } @@ -92,10 +107,10 @@ func permissionOr(funcs ...func(string, map[string]map[string]bool) bool) func(s } } -func permissionAnd(funcs ...func(string, map[string]map[string]bool) bool) func(string, map[string]map[string]bool) bool { - return func(roleName string, permissionsMap map[string]map[string]bool) bool { +func permissionAnd(funcs ...func(*model.Role, map[string]map[string]bool) bool) func(*model.Role, map[string]map[string]bool) bool { + return func(role *model.Role, permissionsMap map[string]map[string]bool) bool { for _, f := range funcs { - if !f(roleName, permissionsMap) { + if !f(role, permissionsMap) { return false } } @@ -103,11 +118,12 @@ func permissionAnd(funcs ...func(string, map[string]map[string]bool) bool) func( } } -func applyPermissionsMap(roleName string, roleMap map[string]map[string]bool, migrationMap permissionsMap) []string { +func applyPermissionsMap(role *model.Role, roleMap map[string]map[string]bool, migrationMap permissionsMap) []string { var result []string + roleName := role.Name for _, transformation := range migrationMap { - if transformation.On(roleName, roleMap) { + if transformation.On(role, roleMap) { for _, permission := range transformation.Add { roleMap[roleName][permission] = true } @@ -144,7 +160,7 @@ func (a *App) doPermissionsMigration(key string, migrationMap permissionsMap) *m } for _, role := range roles { - role.Permissions = applyPermissionsMap(role.Name, roleMap, migrationMap) + role.Permissions = applyPermissionsMap(role, roleMap, migrationMap) if _, err := a.Srv().Store.Role().Save(role); err != nil { return err } @@ -396,6 +412,19 @@ func (a *App) channelModerationPermissionsMigration() (permissionsMap, error) { return transformations, nil } +func (a *App) getAddUseGroupMentionsPermissionMigration() (permissionsMap, error) { + return permissionsMap{ + permissionTransformation{ + On: permissionAnd( + isNotRole(model.CHANNEL_GUEST_ROLE_ID), + isNotSchemeRole("Channel Guest Role for Scheme"), + permissionOr(permissionExists(PERMISSION_CREATE_POST), permissionExists(PERMISSION_CREATE_POST_PUBLIC)), + ), + Add: []string{PERMISSION_USE_GROUP_MENTIONS}, + }, + }, nil +} + // DoPermissionsMigrations execute all the permissions migrations need by the current version. func (a *App) DoPermissionsMigrations() error { PermissionsMigrations := []struct { @@ -412,6 +441,7 @@ func (a *App) DoPermissionsMigrations() error { {Key: model.MIGRATION_KEY_VIEW_MEMBERS_NEW_PERMISSION, Migration: a.getViewMembersPermissionMigration}, {Key: model.MIGRATION_KEY_ADD_MANAGE_GUESTS_PERMISSIONS, Migration: a.getAddManageGuestsPermissionsMigration}, {Key: model.MIGRATION_KEY_CHANNEL_MODERATIONS_PERMISSIONS, Migration: a.channelModerationPermissionsMigration}, + {Key: model.MIGRATION_KEY_ADD_USE_GROUP_MENTIONS_PERMISSION, Migration: a.getAddUseGroupMentionsPermissionMigration}, } for _, migration := range PermissionsMigrations { diff --git a/app/permissions_migrations_test.go b/app/permissions_migrations_test.go index 07a98860ff..0332f3514c 100644 --- a/app/permissions_migrations_test.go +++ b/app/permissions_migrations_test.go @@ -7,6 +7,7 @@ import ( "sort" "testing" + "github.com/mattermost/mattermost-server/v5/model" "github.com/stretchr/testify/assert" ) @@ -195,7 +196,7 @@ func TestApplyPermissionsMap(t *testing.T) { for _, tc := range tt { t.Run(tc.Name, func(t *testing.T) { - result := applyPermissionsMap("system_admin", tc.RoleMap, tc.TranslationMap) + result := applyPermissionsMap(&model.Role{Name: "system_admin"}, tc.RoleMap, tc.TranslationMap) sort.Strings(result) assert.Equal(t, tc.ExpectedResult, result) }) diff --git a/model/migration.go b/model/migration.go index cca8550ee3..7dd08bef6e 100644 --- a/model/migration.go +++ b/model/migration.go @@ -16,4 +16,5 @@ const ( MIGRATION_KEY_VIEW_MEMBERS_NEW_PERMISSION = "view_members_new_permission" MIGRATION_KEY_ADD_MANAGE_GUESTS_PERMISSIONS = "add_manage_guests_permissions" MIGRATION_KEY_CHANNEL_MODERATIONS_PERMISSIONS = "channel_moderations_permissions" + MIGRATION_KEY_ADD_USE_GROUP_MENTIONS_PERMISSION = "add_use_group_mentions_permission" ) diff --git a/model/permission.go b/model/permission.go index e5bf106325..cc3c5a7085 100644 --- a/model/permission.go +++ b/model/permission.go @@ -91,6 +91,7 @@ var PERMISSION_INVITE_GUEST *Permission var PERMISSION_PROMOTE_GUEST *Permission var PERMISSION_DEMOTE_TO_GUEST *Permission var PERMISSION_USE_CHANNEL_MENTIONS *Permission +var PERMISSION_USE_GROUP_MENTIONS *Permission // General permission that encompasses all system admin functions // in the future this could be broken up to allow access to some @@ -567,6 +568,13 @@ func initializePermissions() { PERMISSION_SCOPE_CHANNEL, } + PERMISSION_USE_GROUP_MENTIONS = &Permission{ + "use_group_mentions", + "authentication.permissions.use_group_mentions.name", + "authentication.permissions.use_group_mentions.description", + PERMISSION_SCOPE_CHANNEL, + } + ALL_PERMISSIONS = []*Permission{ PERMISSION_INVITE_USER, PERMISSION_ADD_USER_TO_TEAM, @@ -643,6 +651,7 @@ func initializePermissions() { PERMISSION_PROMOTE_GUEST, PERMISSION_DEMOTE_TO_GUEST, PERMISSION_USE_CHANNEL_MENTIONS, + PERMISSION_USE_GROUP_MENTIONS, } CHANNEL_MODERATED_PERMISSIONS = []string{ diff --git a/testlib/store.go b/testlib/store.go index cfe60e5f79..c5cfeefebe 100644 --- a/testlib/store.go +++ b/testlib/store.go @@ -40,6 +40,7 @@ func GetMockStoreForSetupFunctions() *mocks.Store { systemStore.On("GetByName", model.MIGRATION_KEY_VIEW_MEMBERS_NEW_PERMISSION).Return(&model.System{Name: model.MIGRATION_KEY_VIEW_MEMBERS_NEW_PERMISSION, Value: "true"}, nil) systemStore.On("GetByName", model.MIGRATION_KEY_ADD_MANAGE_GUESTS_PERMISSIONS).Return(&model.System{Name: model.MIGRATION_KEY_ADD_MANAGE_GUESTS_PERMISSIONS, Value: "true"}, nil) systemStore.On("GetByName", model.MIGRATION_KEY_CHANNEL_MODERATIONS_PERMISSIONS).Return(&model.System{Name: model.MIGRATION_KEY_CHANNEL_MODERATIONS_PERMISSIONS, Value: "true"}, nil) + systemStore.On("GetByName", model.MIGRATION_KEY_ADD_USE_GROUP_MENTIONS_PERMISSION).Return(&model.System{Name: model.MIGRATION_KEY_ADD_USE_GROUP_MENTIONS_PERMISSION, Value: "true"}, nil) systemStore.On("Get").Return(make(model.StringMap), nil) systemStore.On("Save", mock.AnythingOfType("*model.System")).Return(nil)