From 1648b11e82919a369cfe9a9586fc18afe38eb0e5 Mon Sep 17 00:00:00 2001 From: Agniva De Sarker Date: Thu, 25 Jun 2020 13:36:47 +0530 Subject: [PATCH] MM-25507: Remove old migration state marker before running fresh migration (#14768) While resetting permissions, we were not removing old migration state residue for the EMOJIS_PERMISSIONS_MIGRATION_KEY and GUEST_ROLES_CREATION_MIGRATION_KEY. Therefore, during their migration, they got skipped because the code checks if a migration key is already present or not. To fix this, we remove the migration key just as we do for ADVANCED_PERMISSIONS_MIGRATION_KEY. Co-authored-by: Mattermod --- app/migrations.go | 7 +++++-- app/permissions.go | 10 ++++++++++ app/permissions_test.go | 21 +++++++++++++++++++++ 3 files changed, 36 insertions(+), 2 deletions(-) diff --git a/app/migrations.go b/app/migrations.go index 404de7dffc..7f1abc6061 100644 --- a/app/migrations.go +++ b/app/migrations.go @@ -134,8 +134,11 @@ func (a *App) DoEmojisPermissionsMigration() { return } - 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) + systemAdminRole.Permissions = append(systemAdminRole.Permissions, + model.PERMISSION_CREATE_EMOJIS.Id, + model.PERMISSION_DELETE_EMOJIS.Id, + model.PERMISSION_DELETE_OTHERS_EMOJIS.Id, + ) if _, err := a.Srv().Store.Role().Save(systemAdminRole); err != nil { mlog.Critical("Failed to migrate emojis creation permissions from mattermost config.", mlog.Err(err)) return diff --git a/app/permissions.go b/app/permissions.go index 890e837a42..59c65c91a0 100644 --- a/app/permissions.go +++ b/app/permissions.go @@ -58,6 +58,16 @@ func (a *App) ResetPermissionsSystem() *model.AppError { return err } + // Remove the "System" table entry that marks the emoji permissions migration as done. + if _, err := a.Srv().Store.System().PermanentDeleteByName(EMOJIS_PERMISSIONS_MIGRATION_KEY); err != nil { + return err + } + + // Remove the "System" table entry that marks the guest roles permissions migration as done. + if _, err := a.Srv().Store.System().PermanentDeleteByName(GUEST_ROLES_CREATION_MIGRATION_KEY); err != nil { + return err + } + // Now that the permissions system has been reset, re-run the migration to reinitialise it. a.DoAppMigrations() diff --git a/app/permissions_test.go b/app/permissions_test.go index 0146b040ff..1bf57a98b2 100644 --- a/app/permissions_test.go +++ b/app/permissions_test.go @@ -10,6 +10,8 @@ import ( "testing" "github.com/mattermost/mattermost-server/v5/model" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) type testWriter struct { @@ -253,6 +255,25 @@ func TestImportPermissions_schemeDeletedOnRoleFailure(t *testing.T) { } +func TestEmojiMigration(t *testing.T) { + th := Setup(t) + defer th.TearDown() + + role, err := th.App.GetRoleByName(model.SYSTEM_ADMIN_ROLE_ID) + require.Nil(t, err) + assert.Contains(t, role.Permissions, model.PERMISSION_CREATE_EMOJIS.Id) + assert.Contains(t, role.Permissions, model.PERMISSION_DELETE_EMOJIS.Id) + assert.Contains(t, role.Permissions, model.PERMISSION_DELETE_OTHERS_EMOJIS.Id) + + th.App.ResetPermissionsSystem() + + role, err = th.App.GetRoleByName(model.SYSTEM_ADMIN_ROLE_ID) + require.Nil(t, err) + assert.Contains(t, role.Permissions, model.PERMISSION_CREATE_EMOJIS.Id) + assert.Contains(t, role.Permissions, model.PERMISSION_DELETE_EMOJIS.Id) + assert.Contains(t, role.Permissions, model.PERMISSION_DELETE_OTHERS_EMOJIS.Id) +} + func withMigrationMarkedComplete(th *TestHelper, f func()) { // Mark the migration as done. th.App.Srv().Store.System().PermanentDeleteByName(model.MIGRATION_KEY_ADVANCED_PERMISSIONS_PHASE_2)