Split Emojis and Webhooks permissions (#10239)

* Split Emojis and Webhooks permissions

* Fixing some tests

* Fixing more tests

* Fix more tests

* Fixed review comments

* Fixing review comments
Этот коммит содержится в:
Jesús Espino
2019-03-07 16:07:09 +01:00
коммит произвёл GitHub
родитель a4e3dfaebc
Коммит 84afd47021
29 изменённых файлов: 653 добавлений и 167 удалений

Просмотреть файл

@@ -5,6 +5,7 @@ package app
import (
"fmt"
"sort"
"testing"
"github.com/stretchr/testify/assert"
@@ -110,10 +111,12 @@ func TestDoAdvancedPermissionsMigration(t *testing.T) {
model.PERMISSION_IMPORT_TEAM.Id,
model.PERMISSION_MANAGE_TEAM_ROLES.Id,
model.PERMISSION_MANAGE_CHANNEL_ROLES.Id,
model.PERMISSION_MANAGE_OTHERS_WEBHOOKS.Id,
model.PERMISSION_MANAGE_OTHERS_INCOMING_WEBHOOKS.Id,
model.PERMISSION_MANAGE_OTHERS_OUTGOING_WEBHOOKS.Id,
model.PERMISSION_MANAGE_SLASH_COMMANDS.Id,
model.PERMISSION_MANAGE_OTHERS_SLASH_COMMANDS.Id,
model.PERMISSION_MANAGE_WEBHOOKS.Id,
model.PERMISSION_MANAGE_INCOMING_WEBHOOKS.Id,
model.PERMISSION_MANAGE_OUTGOING_WEBHOOKS.Id,
model.PERMISSION_DELETE_POST.Id,
model.PERMISSION_DELETE_OTHERS_POSTS.Id,
},
@@ -147,7 +150,8 @@ func TestDoAdvancedPermissionsMigration(t *testing.T) {
model.PERMISSION_DELETE_PRIVATE_CHANNEL.Id,
model.PERMISSION_CREATE_PRIVATE_CHANNEL.Id,
model.PERMISSION_MANAGE_SYSTEM_WIDE_OAUTH.Id,
model.PERMISSION_MANAGE_OTHERS_WEBHOOKS.Id,
model.PERMISSION_MANAGE_OTHERS_INCOMING_WEBHOOKS.Id,
model.PERMISSION_MANAGE_OTHERS_OUTGOING_WEBHOOKS.Id,
model.PERMISSION_EDIT_OTHER_USERS.Id,
model.PERMISSION_EDIT_OTHERS_POSTS.Id,
model.PERMISSION_MANAGE_OAUTH.Id,
@@ -187,7 +191,8 @@ func TestDoAdvancedPermissionsMigration(t *testing.T) {
model.PERMISSION_MANAGE_CHANNEL_ROLES.Id,
model.PERMISSION_MANAGE_SLASH_COMMANDS.Id,
model.PERMISSION_MANAGE_OTHERS_SLASH_COMMANDS.Id,
model.PERMISSION_MANAGE_WEBHOOKS.Id,
model.PERMISSION_MANAGE_INCOMING_WEBHOOKS.Id,
model.PERMISSION_MANAGE_OUTGOING_WEBHOOKS.Id,
model.PERMISSION_EDIT_POST.Id,
},
}
@@ -281,10 +286,12 @@ func TestDoAdvancedPermissionsMigration(t *testing.T) {
model.PERMISSION_IMPORT_TEAM.Id,
model.PERMISSION_MANAGE_TEAM_ROLES.Id,
model.PERMISSION_MANAGE_CHANNEL_ROLES.Id,
model.PERMISSION_MANAGE_OTHERS_WEBHOOKS.Id,
model.PERMISSION_MANAGE_OTHERS_INCOMING_WEBHOOKS.Id,
model.PERMISSION_MANAGE_OTHERS_OUTGOING_WEBHOOKS.Id,
model.PERMISSION_MANAGE_SLASH_COMMANDS.Id,
model.PERMISSION_MANAGE_OTHERS_SLASH_COMMANDS.Id,
model.PERMISSION_MANAGE_WEBHOOKS.Id,
model.PERMISSION_MANAGE_INCOMING_WEBHOOKS.Id,
model.PERMISSION_MANAGE_OUTGOING_WEBHOOKS.Id,
model.PERMISSION_MANAGE_PUBLIC_CHANNEL_PROPERTIES.Id,
model.PERMISSION_MANAGE_PRIVATE_CHANNEL_PROPERTIES.Id,
model.PERMISSION_DELETE_POST.Id,
@@ -320,7 +327,8 @@ func TestDoAdvancedPermissionsMigration(t *testing.T) {
model.PERMISSION_DELETE_PRIVATE_CHANNEL.Id,
model.PERMISSION_CREATE_PRIVATE_CHANNEL.Id,
model.PERMISSION_MANAGE_SYSTEM_WIDE_OAUTH.Id,
model.PERMISSION_MANAGE_OTHERS_WEBHOOKS.Id,
model.PERMISSION_MANAGE_OTHERS_INCOMING_WEBHOOKS.Id,
model.PERMISSION_MANAGE_OTHERS_OUTGOING_WEBHOOKS.Id,
model.PERMISSION_EDIT_OTHER_USERS.Id,
model.PERMISSION_EDIT_OTHERS_POSTS.Id,
model.PERMISSION_MANAGE_OAUTH.Id,
@@ -360,7 +368,8 @@ func TestDoAdvancedPermissionsMigration(t *testing.T) {
model.PERMISSION_MANAGE_CHANNEL_ROLES.Id,
model.PERMISSION_MANAGE_SLASH_COMMANDS.Id,
model.PERMISSION_MANAGE_OTHERS_SLASH_COMMANDS.Id,
model.PERMISSION_MANAGE_WEBHOOKS.Id,
model.PERMISSION_MANAGE_INCOMING_WEBHOOKS.Id,
model.PERMISSION_MANAGE_OUTGOING_WEBHOOKS.Id,
model.PERMISSION_EDIT_POST.Id,
},
}
@@ -459,7 +468,8 @@ func TestDoEmojisPermissionsMigration(t *testing.T) {
model.PERMISSION_DELETE_PRIVATE_CHANNEL.Id,
model.PERMISSION_CREATE_PRIVATE_CHANNEL.Id,
model.PERMISSION_MANAGE_SYSTEM_WIDE_OAUTH.Id,
model.PERMISSION_MANAGE_OTHERS_WEBHOOKS.Id,
model.PERMISSION_MANAGE_OTHERS_INCOMING_WEBHOOKS.Id,
model.PERMISSION_MANAGE_OTHERS_OUTGOING_WEBHOOKS.Id,
model.PERMISSION_EDIT_OTHER_USERS.Id,
model.PERMISSION_EDIT_OTHERS_POSTS.Id,
model.PERMISSION_MANAGE_OAUTH.Id,
@@ -499,14 +509,18 @@ func TestDoEmojisPermissionsMigration(t *testing.T) {
model.PERMISSION_MANAGE_CHANNEL_ROLES.Id,
model.PERMISSION_MANAGE_SLASH_COMMANDS.Id,
model.PERMISSION_MANAGE_OTHERS_SLASH_COMMANDS.Id,
model.PERMISSION_MANAGE_WEBHOOKS.Id,
model.PERMISSION_MANAGE_INCOMING_WEBHOOKS.Id,
model.PERMISSION_MANAGE_OUTGOING_WEBHOOKS.Id,
model.PERMISSION_EDIT_POST.Id,
model.PERMISSION_MANAGE_EMOJIS.Id,
model.PERMISSION_MANAGE_OTHERS_EMOJIS.Id,
model.PERMISSION_CREATE_EMOJIS.Id,
model.PERMISSION_DELETE_EMOJIS.Id,
model.PERMISSION_DELETE_OTHERS_EMOJIS.Id,
}
sort.Strings(expectedSystemAdmin)
role1, err1 := th.App.GetRoleByName(model.SYSTEM_ADMIN_ROLE_ID)
assert.Nil(t, err1)
sort.Strings(role1.Permissions)
assert.Equal(t, expectedSystemAdmin, role1.Permissions, fmt.Sprintf("'%v' did not have expected permissions", model.SYSTEM_ADMIN_ROLE_ID))
th.App.UpdateConfig(func(cfg *model.Config) {
@@ -524,18 +538,24 @@ func TestDoEmojisPermissionsMigration(t *testing.T) {
model.PERMISSION_IMPORT_TEAM.Id,
model.PERMISSION_MANAGE_TEAM_ROLES.Id,
model.PERMISSION_MANAGE_CHANNEL_ROLES.Id,
model.PERMISSION_MANAGE_OTHERS_WEBHOOKS.Id,
model.PERMISSION_MANAGE_OTHERS_INCOMING_WEBHOOKS.Id,
model.PERMISSION_MANAGE_OTHERS_OUTGOING_WEBHOOKS.Id,
model.PERMISSION_MANAGE_SLASH_COMMANDS.Id,
model.PERMISSION_MANAGE_OTHERS_SLASH_COMMANDS.Id,
model.PERMISSION_MANAGE_WEBHOOKS.Id,
model.PERMISSION_MANAGE_INCOMING_WEBHOOKS.Id,
model.PERMISSION_MANAGE_OUTGOING_WEBHOOKS.Id,
model.PERMISSION_DELETE_POST.Id,
model.PERMISSION_DELETE_OTHERS_POSTS.Id,
model.PERMISSION_MANAGE_EMOJIS.Id,
model.PERMISSION_CREATE_EMOJIS.Id,
model.PERMISSION_DELETE_EMOJIS.Id,
}
sort.Strings(expected2)
sort.Strings(role2.Permissions)
assert.Equal(t, expected2, role2.Permissions, fmt.Sprintf("'%v' did not have expected permissions", model.TEAM_ADMIN_ROLE_ID))
systemAdmin1, systemAdminErr1 := th.App.GetRoleByName(model.SYSTEM_ADMIN_ROLE_ID)
assert.Nil(t, systemAdminErr1)
sort.Strings(systemAdmin1.Permissions)
assert.Equal(t, expectedSystemAdmin, systemAdmin1.Permissions, fmt.Sprintf("'%v' did not have expected permissions", model.SYSTEM_ADMIN_ROLE_ID))
th.App.UpdateConfig(func(cfg *model.Config) {
@@ -552,11 +572,15 @@ func TestDoEmojisPermissionsMigration(t *testing.T) {
model.PERMISSION_CREATE_GROUP_CHANNEL.Id,
model.PERMISSION_PERMANENT_DELETE_USER.Id,
model.PERMISSION_CREATE_TEAM.Id,
model.PERMISSION_MANAGE_EMOJIS.Id,
model.PERMISSION_CREATE_EMOJIS.Id,
model.PERMISSION_DELETE_EMOJIS.Id,
}
sort.Strings(expected3)
sort.Strings(role3.Permissions)
assert.Equal(t, expected3, role3.Permissions, fmt.Sprintf("'%v' did not have expected permissions", model.SYSTEM_USER_ROLE_ID))
systemAdmin2, systemAdminErr2 := th.App.GetRoleByName(model.SYSTEM_ADMIN_ROLE_ID)
assert.Nil(t, systemAdminErr2)
sort.Strings(systemAdmin2.Permissions)
assert.Equal(t, expectedSystemAdmin, systemAdmin2.Permissions, fmt.Sprintf("'%v' did not have expected permissions", model.SYSTEM_ADMIN_ROLE_ID))
}

Просмотреть файл

@@ -429,7 +429,15 @@ func (me *TestHelper) ResetRoleMigration() {
func (me *TestHelper) ResetEmojisMigration() {
sqlSupplier := mainHelper.GetSqlSupplier()
if _, err := sqlSupplier.GetMaster().Exec("UPDATE Roles SET Permissions=REPLACE(Permissions, ', manage_emojis', '') WHERE builtin=True"); err != nil {
if _, err := sqlSupplier.GetMaster().Exec("UPDATE Roles SET Permissions=REPLACE(Permissions, ' create_emojis', '') WHERE builtin=True"); err != nil {
panic(err)
}
if _, err := sqlSupplier.GetMaster().Exec("UPDATE Roles SET Permissions=REPLACE(Permissions, ' delete_emojis', '') WHERE builtin=True"); err != nil {
panic(err)
}
if _, err := sqlSupplier.GetMaster().Exec("UPDATE Roles SET Permissions=REPLACE(Permissions, ' delete_others_emojis', '') WHERE builtin=True"); err != nil {
panic(err)
}

Просмотреть файл

@@ -123,7 +123,7 @@ func (a *App) DoEmojisPermissionsMigration() {
}
if role != nil {
role.Permissions = append(role.Permissions, model.PERMISSION_MANAGE_EMOJIS.Id)
role.Permissions = append(role.Permissions, model.PERMISSION_CREATE_EMOJIS.Id, model.PERMISSION_DELETE_EMOJIS.Id)
if result := <-a.Srv.Store.Role().Save(role); result.Err != nil {
mlog.Critical("Failed to migrate emojis creation permissions from mattermost config.")
mlog.Critical(result.Err.Error())
@@ -138,8 +138,8 @@ func (a *App) DoEmojisPermissionsMigration() {
return
}
systemAdminRole.Permissions = append(systemAdminRole.Permissions, model.PERMISSION_MANAGE_EMOJIS.Id)
systemAdminRole.Permissions = append(systemAdminRole.Permissions, model.PERMISSION_MANAGE_OTHERS_EMOJIS.Id)
systemAdminRole.Permissions = append(systemAdminRole.Permissions, model.PERMISSION_CREATE_EMOJIS.Id, model.PERMISSION_DELETE_EMOJIS.Id)
systemAdminRole.Permissions = append(systemAdminRole.Permissions, model.PERMISSION_DELETE_OTHERS_EMOJIS.Id)
if result := <-a.Srv.Store.Role().Save(systemAdminRole); result.Err != nil {
mlog.Critical("Failed to migrate emojis creation permissions from mattermost config.")
mlog.Critical(result.Err.Error())

Просмотреть файл

@@ -60,6 +60,7 @@ func (a *App) ResetPermissionsSystem() *model.AppError {
// Now that the permissions system has been reset, re-run the migration to reinitialise it.
a.DoAdvancedPermissionsMigration()
a.DoEmojisPermissionsMigration()
a.DoPermissionsMigrations()
return nil
}

163
app/permissions_migrations.go Обычный файл
Просмотреть файл

@@ -0,0 +1,163 @@
// Copyright (c) 2018-present Mattermost, Inc. All Rights Reserved.
// See License.txt for license information.
package app
import "github.com/mattermost/mattermost-server/model"
type permissionTransformation struct {
On func(map[string]bool) bool
Add []string
Remove []string
}
type permissionsMap []permissionTransformation
const (
MIGRATION_KEY_EMOJI_PERMISSIONS_SPLIT = "emoji_permissions_split"
MIGRATION_KEY_WEBHOOK_PERMISSIONS_SPLIT = "webhook_permissions_split"
PERMISSION_MANAGE_EMOJIS = "manage_emojis"
PERMISSION_MANAGE_OTHERS_EMOJIS = "manage_others_emojis"
PERMISSION_CREATE_EMOJIS = "create_emojis"
PERMISSION_DELETE_EMOJIS = "delete_emojis"
PERMISSION_DELETE_OTHERS_EMOJIS = "delete_others_emojis"
PERMISSION_MANAGE_WEBHOOKS = "manage_webhooks"
PERMISSION_MANAGE_OTHERS_WEBHOOKS = "manage_others_webhooks"
PERMISSION_MANAGE_INCOMING_WEBHOOKS = "manage_incoming_webhooks"
PERMISSION_MANAGE_OTHERS_INCOMING_WEBHOOKS = "manage_others_incoming_webhooks"
PERMISSION_MANAGE_OUTGOING_WEBHOOKS = "manage_outgoing_webhooks"
PERMISSION_MANAGE_OTHERS_OUTGOING_WEBHOOKS = "manage_others_outgoing_webhooks"
)
func permissionExists(permission string) func(map[string]bool) bool {
return func(permissions map[string]bool) bool {
val, ok := permissions[permission]
return ok && val
}
}
func permissionNotExists(permission string) func(map[string]bool) bool {
return func(permissions map[string]bool) bool {
val, ok := permissions[permission]
return !(ok && val)
}
}
func permissionOr(funcs ...func(map[string]bool) bool) func(map[string]bool) bool {
return func(permissions map[string]bool) bool {
for _, f := range funcs {
if f(permissions) {
return true
}
}
return false
}
}
func permissionAnd(funcs ...func(map[string]bool) bool) func(map[string]bool) bool {
return func(permissions map[string]bool) bool {
for _, f := range funcs {
if !f(permissions) {
return false
}
}
return true
}
}
func applyPermissionsMap(permissions []string, migrationMap permissionsMap) []string {
finalMap := make(map[string]bool)
var result []string
for _, permission := range permissions {
finalMap[permission] = true
}
for _, transformation := range migrationMap {
if transformation.On(finalMap) {
for _, add := range transformation.Add {
finalMap[add] = true
}
for _, remove := range transformation.Remove {
finalMap[remove] = false
}
}
}
for key, active := range finalMap {
if active {
result = append(result, key)
}
}
return result
}
func (a *App) doPermissionsMigration(key string, migrationMap permissionsMap) *model.AppError {
if result := <-a.Srv.Store.System().GetByName(key); result.Err == nil {
return nil
}
roles, err := a.GetAllRoles()
if err != nil {
return err
}
for _, role := range roles {
role.Permissions = applyPermissionsMap(role.Permissions, migrationMap)
if result := <-a.Srv.Store.Role().Save(role); result.Err != nil {
return result.Err
}
}
if result := <-a.Srv.Store.System().Save(&model.System{Name: key, Value: "true"}); result.Err != nil {
return result.Err
}
return nil
}
func getEmojisPermissionsSplitMigration() permissionsMap {
return permissionsMap{
permissionTransformation{
On: permissionExists(PERMISSION_MANAGE_EMOJIS),
Add: []string{PERMISSION_CREATE_EMOJIS, PERMISSION_DELETE_EMOJIS},
Remove: []string{PERMISSION_MANAGE_EMOJIS},
},
permissionTransformation{
On: permissionExists(PERMISSION_MANAGE_OTHERS_EMOJIS),
Add: []string{PERMISSION_DELETE_OTHERS_EMOJIS},
Remove: []string{PERMISSION_MANAGE_OTHERS_EMOJIS},
},
}
}
func getWebhooksPermissionsSplitMigration() permissionsMap {
return permissionsMap{
permissionTransformation{
On: permissionExists(PERMISSION_MANAGE_WEBHOOKS),
Add: []string{PERMISSION_MANAGE_INCOMING_WEBHOOKS, PERMISSION_MANAGE_OUTGOING_WEBHOOKS},
Remove: []string{PERMISSION_MANAGE_WEBHOOKS},
},
permissionTransformation{
On: permissionExists(PERMISSION_MANAGE_OTHERS_WEBHOOKS),
Add: []string{PERMISSION_MANAGE_OTHERS_INCOMING_WEBHOOKS, PERMISSION_MANAGE_OTHERS_OUTGOING_WEBHOOKS},
Remove: []string{PERMISSION_MANAGE_OTHERS_WEBHOOKS},
},
}
}
// DoPermissionsMigrations execute all the permissions migrations need by the current version.
func (a *App) DoPermissionsMigrations() *model.AppError {
PermissionsMigrations := []struct {
Key string
Migration func() permissionsMap
}{
{Key: MIGRATION_KEY_EMOJI_PERMISSIONS_SPLIT, Migration: getEmojisPermissionsSplitMigration},
{Key: MIGRATION_KEY_WEBHOOK_PERMISSIONS_SPLIT, Migration: getWebhooksPermissionsSplitMigration},
}
for _, migration := range PermissionsMigrations {
if err := a.doPermissionsMigration(migration.Key, migration.Migration()); err != nil {
return err
}
}
return nil
}

89
app/permissions_migrations_test.go Обычный файл
Просмотреть файл

@@ -0,0 +1,89 @@
// Copyright (c) 2018-present Mattermost, Inc. All Rights Reserved.
// See License.txt for license information.
package app
import (
"sort"
"testing"
"github.com/stretchr/testify/assert"
)
func TestApplyPermissionsMap(t *testing.T) {
tt := []struct {
Name string
Permissions []string
TranslationMap permissionsMap
ExpectedResult []string
}{
{
"Split existing",
[]string{"test1", "test2", "test3"},
permissionsMap{permissionTransformation{On: permissionExists("test2"), Add: []string{"test4", "test5"}}},
[]string{"test1", "test2", "test3", "test4", "test5"},
},
{
"Remove existing",
[]string{"test1", "test2", "test3"},
permissionsMap{permissionTransformation{On: permissionExists("test2"), Remove: []string{"test2"}}},
[]string{"test1", "test3"},
},
{
"Rename existing",
[]string{"test1", "test2", "test3"},
permissionsMap{permissionTransformation{On: permissionExists("test2"), Add: []string{"test5"}, Remove: []string{"test2"}}},
[]string{"test1", "test3", "test5"},
},
{
"Remove when other not exists",
[]string{"test1", "test2", "test3"},
permissionsMap{permissionTransformation{On: permissionNotExists("test5"), Remove: []string{"test2"}}},
[]string{"test1", "test3"},
},
{
"Add when at least one exists",
[]string{"test1", "test2", "test3"},
permissionsMap{permissionTransformation{
On: permissionOr(permissionExists("test5"), permissionExists("test3")),
Add: []string{"test4"},
}},
[]string{"test1", "test2", "test3", "test4"},
},
{
"Add when all exists",
[]string{"test1", "test2", "test3"},
permissionsMap{permissionTransformation{
On: permissionAnd(permissionExists("test1"), permissionExists("test2")),
Add: []string{"test4"},
}},
[]string{"test1", "test2", "test3", "test4"},
},
{
"Not add when one in the and not exists",
[]string{"test1", "test2", "test3"},
permissionsMap{permissionTransformation{
On: permissionAnd(permissionExists("test1"), permissionExists("test5")),
Add: []string{"test4"},
}},
[]string{"test1", "test2", "test3"},
},
{
"Not Add when none on the or exists",
[]string{"test1", "test2", "test3"},
permissionsMap{permissionTransformation{
On: permissionOr(permissionExists("test7"), permissionExists("test9")),
Add: []string{"test4"},
}},
[]string{"test1", "test2", "test3"},
},
}
for _, tc := range tt {
t.Run(tc.Name, func(t *testing.T) {
result := applyPermissionsMap(tc.Permissions, tc.TranslationMap)
sort.Strings(result)
assert.Equal(t, tc.ExpectedResult, result)
})
}
}

Просмотреть файл

@@ -17,7 +17,14 @@ func (a *App) GetRole(id string) (*model.Role, *model.AppError) {
return nil, result.Err
}
return result.Data.(*model.Role), nil
}
func (a *App) GetAllRoles() ([]*model.Role, *model.AppError) {
result := <-a.Srv.Store.Role().GetAll()
if result.Err != nil {
return nil, result.Err
}
return result.Data.([]*model.Role), nil
}
func (a *App) GetRoleByName(name string) (*model.Role, *model.AppError) {

Просмотреть файл

@@ -137,6 +137,7 @@ func (s *Server) RunOldAppInitalization() error {
s.FakeApp().DoAdvancedPermissionsMigration()
s.FakeApp().DoEmojisPermissionsMigration()
s.FakeApp().DoPermissionsMigrations()
s.FakeApp().InitPostMetadata()