PermissionManageOauth removal impact (mattermost#35554)

* Restore manage oauth permission

Co-authored-by: Nick Misasi <nick13misasi@gmail.com>

* Fix migration test lint assertion

Co-authored-by: Nick Misasi <nick13misasi@gmail.com>

---------

Co-authored-by: Cursor Agent <cursoragent@cursor.com>

Author: nickmisasi

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()

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 {

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)

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"
 )

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,

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")
+	})
 }