diff --git a/server/channels/app/permissions_migrations.go b/server/channels/app/permissions_migrations.go index b2e8e6db044..028ec0762b3 100644 --- a/server/channels/app/permissions_migrations.go +++ b/server/channels/app/permissions_migrations.go @@ -1284,6 +1284,15 @@ func (a *App) getAddSecureConnectionManagerPermissionsMigration() (permissionsMa }, nil } +func (a *App) getRestoreManageOAuthPermissionMigration() (permissionsMap, error) { + return permissionsMap{ + permissionTransformation{ + On: isExactRole(model.SystemAdminRoleId), + Add: []string{model.PermissionManageOAuth.Id}, + }, + }, nil +} + // DoPermissionsMigrations execute all the permissions migrations need by the current version. func (a *App) DoPermissionsMigrations() error { return a.Srv().doPermissionsMigrations() @@ -1343,6 +1352,7 @@ func (s *Server) doPermissionsMigrations() error { {Key: model.MigrationKeyAddChannelAutoTranslationPermissions, Migration: a.getAddChannelAutoTranslationPermissionMigration}, {Key: model.MigrationKeyAddSharedChannelManagerPermissions, Migration: a.getAddSharedChannelManagerPermissionsMigration}, {Key: model.MigrationKeyAddSecureConnectionManagerPermissions, Migration: a.getAddSecureConnectionManagerPermissionsMigration}, + {Key: model.MigrationKeyRestoreManageOAuthPermission, Migration: a.getRestoreManageOAuthPermissionMigration}, } roles, err := s.Store().Role().GetAll() diff --git a/server/channels/app/permissions_migrations_test.go b/server/channels/app/permissions_migrations_test.go index 6e4987c7e8e..a7acdb8dda8 100644 --- a/server/channels/app/permissions_migrations_test.go +++ b/server/channels/app/permissions_migrations_test.go @@ -7,11 +7,64 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" "github.com/mattermost/mattermost/server/public/model" "github.com/mattermost/mattermost/server/v8/channels/store/sqlstore" + "github.com/mattermost/mattermost/server/v8/channels/store/storetest/mocks" ) +func TestRestoreManageOAuthPermissionMigration(t *testing.T) { + mainHelper.Parallel(t) + + th := SetupWithStoreMock(t) + + migrationMap, err := th.App.getRestoreManageOAuthPermissionMigration() + require.NoError(t, err) + + systemAdminRole := &model.Role{ + Name: model.SystemAdminRoleId, + Permissions: []string{model.PermissionManageSystemWideOAuth.Id}, + } + systemUserRole := &model.Role{ + Name: model.SystemUserRoleId, + Permissions: []string{model.PermissionCreateDirectChannel.Id}, + } + roles := []*model.Role{systemAdminRole, systemUserRole} + + mockStore := th.App.Srv().Store().(*mocks.Store) + roleStore := mocks.RoleStore{} + systemStore := mocks.SystemStore{} + + mockStore.On("Role").Return(&roleStore) + mockStore.On("System").Return(&systemStore) + + systemStore.On("GetByName", model.MigrationKeyRestoreManageOAuthPermission). + Return(nil, model.NewAppError("test", "missing", nil, "", 404)).Once() + systemStore.On("GetByName", model.MigrationKeyRestoreManageOAuthPermission). + Return(&model.System{Name: model.MigrationKeyRestoreManageOAuthPermission, Value: "true"}, nil).Once() + systemStore.On("SaveOrUpdate", mock.MatchedBy(func(system *model.System) bool { + return system.Name == model.MigrationKeyRestoreManageOAuthPermission && system.Value == "true" + })).Return(nil).Once() + + roleStore.On("Save", mock.AnythingOfType("*model.Role")). + Return(func(role *model.Role) *model.Role { return role }, nil).Twice() + + appErr := th.App.Srv().doPermissionsMigration(model.MigrationKeyRestoreManageOAuthPermission, migrationMap, roles) + require.Nil(t, appErr) + assert.Contains(t, systemAdminRole.Permissions, model.PermissionManageOAuth.Id) + assert.NotContains(t, systemUserRole.Permissions, model.PermissionManageOAuth.Id) + assert.Len(t, systemAdminRole.Permissions, 2) + + appErr = th.App.Srv().doPermissionsMigration(model.MigrationKeyRestoreManageOAuthPermission, migrationMap, roles) + require.Nil(t, appErr) + assert.Len(t, systemAdminRole.Permissions, 2) + + roleStore.AssertNumberOfCalls(t, "Save", 2) + systemStore.AssertNumberOfCalls(t, "SaveOrUpdate", 1) +} + func TestApplyPermissionsMap(t *testing.T) { mainHelper.Parallel(t) tt := []struct { diff --git a/server/channels/testlib/store.go b/server/channels/testlib/store.go index ca46030d461..9edf15ce825 100644 --- a/server/channels/testlib/store.go +++ b/server/channels/testlib/store.go @@ -95,6 +95,7 @@ func GetMockStoreForSetupFunctions() *mocks.Store { systemStore.On("GetByName", model.MigrationKeyAddChannelBannerPermissions).Return(&model.System{Name: model.MigrationKeyAddChannelBannerPermissions, Value: "true"}, nil) systemStore.On("GetByName", model.MigrationKeyAddChannelAccessRulesPermission).Return(&model.System{Name: model.MigrationKeyAddChannelAccessRulesPermission, Value: "true"}, nil) systemStore.On("GetByName", model.MigrationKeyAddChannelAutoTranslationPermissions).Return(&model.System{Name: model.MigrationKeyAddChannelAutoTranslationPermissions, Value: "true"}, nil) + systemStore.On("GetByName", model.MigrationKeyRestoreManageOAuthPermission).Return(&model.System{Name: model.MigrationKeyRestoreManageOAuthPermission, Value: "true"}, nil) systemStore.On("InsertIfExists", mock.AnythingOfType("*model.System")).Return(&model.System{}, nil).Once() systemStore.On("Save", mock.AnythingOfType("*model.System")).Return(nil) diff --git a/server/public/model/migration.go b/server/public/model/migration.go index e540998ea74..816f366657f 100644 --- a/server/public/model/migration.go +++ b/server/public/model/migration.go @@ -60,4 +60,5 @@ const ( MigrationKeyAddChannelAutoTranslationPermissions = "add_channel_auto_translation_permissions" MigrationKeyAddSharedChannelManagerPermissions = "shared_channel_manager_permissions" MigrationKeyAddSecureConnectionManagerPermissions = "secure_connection_manager_permissions" + MigrationKeyRestoreManageOAuthPermission = "restore_manage_oauth_permission" ) diff --git a/server/public/model/permission.go b/server/public/model/permission.go index 12d5a005513..1bbfee97e53 100644 --- a/server/public/model/permission.go +++ b/server/public/model/permission.go @@ -2460,6 +2460,7 @@ func initializePermissions() { PermissionReadOtherUsersTeams, PermissionGetPublicLink, PermissionManageSystemWideOAuth, + PermissionManageOAuth, PermissionCreateTeam, PermissionListUsersWithoutTeam, PermissionCreateUserAccessToken, @@ -2612,7 +2613,6 @@ func initializePermissions() { PermissionManageIncomingWebhooks, PermissionManageOutgoingWebhooks, PermissionManageSlashCommands, - PermissionManageOAuth, PermissionManageEmojis, PermissionManageOthersEmojis, PermissionSysconsoleReadAuthentication, diff --git a/server/public/model/role_test.go b/server/public/model/role_test.go index 300e0d45886..2de49e15115 100644 --- a/server/public/model/role_test.go +++ b/server/public/model/role_test.go @@ -341,4 +341,17 @@ func TestMakeDefaultRolesContainsNewManagerRoles(t *testing.T) { assert.True(t, slices.Contains(NewSystemRoleIDs, SecureConnectionManagerRoleId), "secure_connection_manager should be in NewSystemRoleIDs") }) + + t.Run("system_admin includes manage_oauth by default", func(t *testing.T) { + role, ok := roles[SystemAdminRoleId] + require.True(t, ok, "system_admin role should exist in MakeDefaultRoles") + assert.True(t, slices.Contains(role.Permissions, PermissionManageOAuth.Id), + "system_admin should include manage_oauth") + assert.True(t, slices.ContainsFunc(AllPermissions, func(permission *Permission) bool { + return permission.Id == PermissionManageOAuth.Id + }), "manage_oauth should be part of AllPermissions") + assert.False(t, slices.ContainsFunc(DeprecatedPermissions, func(permission *Permission) bool { + return permission.Id == PermissionManageOAuth.Id + }), "manage_oauth should not remain deprecated") + }) }