[FIX][ENTERPRISE] OAuth "Merge Roles" removes roles from users (#23588)

* Fix OAuth 'Merge Roles'

* Add setting to control which roles to sync

Co-authored-by: pierre-lehnen-rc <55164754+pierre-lehnen-rc@users.noreply.github.com>

app/custom-oauth/server/custom_oauth_server.js

@@ -334,6 +334,8 @@ export class CustomOAuth {
 					return;
 				}
 
+				callbacks.run('afterProcessOAuthUser', { serviceName, serviceData, user });
+
 				// User already created or merged and has identical name as before
 				if (user.services && user.services[serviceName] && user.services[serviceName].id === serviceData.id && user.name === serviceData.name) {
 					return;
@@ -343,8 +345,6 @@ export class CustomOAuth {
 					throw new Meteor.Error('CustomOAuth', `User with username ${ user.username } already exists`);
 				}
 
-				callbacks.run('afterProcessOAuthUser', { serviceName, serviceData, user });
-
 				const serviceIdKey = `services.${ serviceName }.id`;
 				const update = {
 					$set: {

app/lib/server/functions/addOAuthService.ts

@@ -57,6 +57,18 @@ export function addOAuthService(name: string, values: { [k: string]: string | bo
 		enterprise: true,
 		invalidValue: false,
 		modules: ['oauth-enterprise'] });
+	settingsRegistry.add(`Accounts_OAuth_Custom-${ name }-roles_to_sync`          , values.rolesToSync || '', { type: 'string',
+		group: 'OAuth',
+		section: `Custom OAuth: ${ name }`,
+		i18nLabel: 'Accounts_OAuth_Custom_Roles_To_Sync',
+		i18nDescription: 'Accounts_OAuth_Custom_Roles_To_Sync_Description',
+		enterprise: true,
+		enableQuery: {
+			_id: `Accounts_OAuth_Custom-${ name }-merge_roles`,
+			value: true,
+		},
+		invalidValue: '',
+		modules: ['oauth-enterprise'] });
 	settingsRegistry.add(`Accounts_OAuth_Custom-${ name }-merge_users`, values.mergeUsers || false, { type: 'boolean', group: 'OAuth', section: `Custom OAuth: ${ name }`, i18nLabel: 'Accounts_OAuth_Custom_Merge_Users', persistent: true });
 	settingsRegistry.add(`Accounts_OAuth_Custom-${ name }-show_button`            , values.showButton || true                     , { type: 'boolean', group: 'OAuth', section: `Custom OAuth: ${ name }`, i18nLabel: 'Accounts_OAuth_Custom_Show_Button_On_Login_Page', persistent: true });
 	settingsRegistry.add(`Accounts_OAuth_Custom-${ name }-groups_channel_map`     , values.channelsMap || '{\n\t"rocket-admin": "admin",\n\t"tech-support": "support"\n}' , { type: 'code'   , multiline: true, code: 'application/json', group: 'OAuth', section: `Custom OAuth: ${ name }`, i18nLabel: 'Accounts_OAuth_Custom_Channel_Map', persistent: true });

app/lib/server/methods/removeOAuthService.ts

@@ -43,6 +43,7 @@ Meteor.methods({
 			Settings.removeById(`Accounts_OAuth_Custom-${ name }-avatar_field`),
 			Settings.removeById(`Accounts_OAuth_Custom-${ name }-roles_claim`),
 			Settings.removeById(`Accounts_OAuth_Custom-${ name }-merge_roles`),
+			Settings.removeById(`Accounts_OAuth_Custom-${ name }-roles_to_sync`),
 			Settings.removeById(`Accounts_OAuth_Custom-${ name }-merge_users`),
 			Settings.removeById(`Accounts_OAuth_Custom-${ name }-show_button`),
 			Settings.removeById(`Accounts_OAuth_Custom-${ name }-groups_claim`),

app/lib/server/startup/oAuthServicesUpdate.js

@@ -55,6 +55,7 @@ function _OAuthServicesUpdate() {
 				data.mergeUsers = settings.get(`${ key }-merge_users`);
 				data.mapChannels = settings.get(`${ key }-map_channels`);
 				data.mergeRoles = settings.get(`${ key }-merge_roles`);
+				data.rolesToSync = settings.get(`${ key }-roles_to_sync`);
 				data.showButton = settings.get(`${ key }-show_button`);
 
 				new CustomOAuth(serviceName.toLowerCase(), {
@@ -78,6 +79,7 @@ function _OAuthServicesUpdate() {
 					channelsAdmin: data.channelsAdmin,
 					mergeUsers: data.mergeUsers,
 					mergeRoles: data.mergeRoles,
+					rolesToSync: data.rolesToSync,
 					accessTokenParam: data.accessTokenParam,
 					showButton: data.showButton,
 				});
@@ -184,6 +186,7 @@ function customOAuthServicesInit() {
 					mergeUsers: process.env[`${ serviceKey }_merge_users`] === 'true',
 					mapChannels: process.env[`${ serviceKey }_map_channels`],
 					mergeRoles: process.env[`${ serviceKey }_merge_roles`] === 'true',
+					rolesToSync: process.env[`${ serviceKey }_roles_to_sync`],
 					showButton: process.env[`${ serviceKey }_show_button`] === 'true',
 					avatarField: process.env[`${ serviceKey }_avatar_field`],
 				};

app/statistics/server/lib/getServicesStatistics.ts

@@ -11,7 +11,7 @@ function getCustomOAuthServices(): Record<string, {
 		const name = key.replace('Accounts_OAuth_Custom-', '');
 		return [name, {
 			enabled: Boolean(value),
-			mergeRoles: Boolean(settings.get(`Accounts_OAuth_Custom-${ name }-merge_roles`)),
+			mergeRoles: settings.get<boolean>(`Accounts_OAuth_Custom-${ name }-merge_roles`),
 			users: Users.countActiveUsersByService(name),
 		}];
 	}));

ee/server/configuration/oauth.ts

@@ -21,6 +21,7 @@ interface IOAuthUserIdentity {
 interface IOAuthSettings {
 	mapChannels: string;
 	mergeRoles: string;
+	rolesToSync: string;
 	rolesClaim: string;
 	groupsClaim: string;
 	channelsAdmin: string;
@@ -33,6 +34,7 @@ function getOAuthSettings(serviceName: string): IOAuthSettings {
 	return {
 		mapChannels: settings.get(`Accounts_OAuth_Custom-${ serviceName }-map_channels`) as string,
 		mergeRoles: settings.get(`Accounts_OAuth_Custom-${ serviceName }-merge_roles`) as string,
+		rolesToSync: settings.get(`Accounts_OAuth_Custom-${ serviceName }-roles_to_sync`) as string,
 		rolesClaim: settings.get(`Accounts_OAuth_Custom-${ serviceName }-roles_claim`) as string,
 		groupsClaim: settings.get(`Accounts_OAuth_Custom-${ serviceName }-groups_claim`) as string,
 		channelsAdmin: settings.get(`Accounts_OAuth_Custom-${ serviceName }-channels_admin`) as string,
@@ -61,7 +63,7 @@ onLicense('oauth-enterprise', () => {
 		}
 
 		if (settings.mergeRoles) {
-			OAuthEEManager.updateRolesFromSSO(auth.user, auth.serviceData, settings.rolesClaim);
+			OAuthEEManager.updateRolesFromSSO(auth.user, auth.serviceData, settings.rolesClaim, settings.rolesToSync.split(',').map((role) => role.trim()));
 		}
 	});
 

ee/server/lib/oauth/Manager.ts

@@ -35,7 +35,7 @@ export class OAuthEEManager {
 		}
 	}
 
-	static updateRolesFromSSO(user: Record<string, any>, identity: Record<string, any>, roleClaimName: string): void {
+	static updateRolesFromSSO(user: Record<string, any>, identity: Record<string, any>, roleClaimName: string, rolesToSync: string[]): void {
 		if (user && identity && roleClaimName) {
 			const rolesFromSSO = this.mapRolesFromSSO(identity, roleClaimName);
 
@@ -43,19 +43,15 @@ export class OAuthEEManager {
 				user.roles = [];
 			}
 
-			const toRemove = user.roles.filter((val: any) => !rolesFromSSO.includes(val));
+			const toRemove = user.roles.filter((val: any) => !rolesFromSSO.includes(val) && rolesToSync.includes(val));
 
-			// loop through roles that user has that sso doesnt have and remove each one
-			toRemove.forEach(function(role: any) {
-				removeUserFromRoles(user._id, role);
-			});
+			// remove all roles that the user has, but sso doesnt
+			removeUserFromRoles(user._id, toRemove);
 
-			const toAdd = rolesFromSSO.filter((val: any) => !user.roles.includes(val));
+			const toAdd = rolesFromSSO.filter((val: any) => !user.roles.includes(val) && (!rolesToSync.length || rolesToSync.includes(val)));
 
-			// loop through sso roles and add the new ones
-			toAdd.forEach(function(role: any) {
-				addUserRoles(user._id, role);
-			});
+			// add all roles that sso has, but the user doesnt
+			addUserRoles(user._id, toAdd);
 		}
 	}
 

packages/rocketchat-i18n/i18n/en.i18n.json

@@ -114,6 +114,8 @@
   "Accounts_OAuth_Custom_Merge_Users": "Merge users",
   "Accounts_OAuth_Custom_Name_Field": "Name field",
   "Accounts_OAuth_Custom_Roles_Claim": "Roles/Groups field name",
+  "Accounts_OAuth_Custom_Roles_To_Sync": "Roles to Sync",
+  "Accounts_OAuth_Custom_Roles_To_Sync_Description": "OAuth Roles to sync on user login and creation (comma-separated).",
   "Accounts_OAuth_Custom_Scope": "Scope",
   "Accounts_OAuth_Custom_Secret": "Secret",
   "Accounts_OAuth_Custom_Show_Button_On_Login_Page": "Show Button on Login Page",

server/startup/migrations/index.ts

@@ -70,4 +70,5 @@ import './v243';
 import './v244';
 import './v245';
 import './v246';
+import './v247';
 import './xrun';

server/startup/migrations/v247.ts

@@ -0,0 +1,27 @@
+import { settings, settingsRegistry } from '../../../app/settings/server';
+import { addMigration } from '../../lib/migrations';
+
+addMigration({
+	version: 247,
+	up() {
+		const customOauthServices = settings.getByRegexp(/Accounts_OAuth_Custom-[^-]+$/mi);
+		const serviceNames = customOauthServices.map(([key]) => key.replace('Accounts_OAuth_Custom-', ''));
+
+		serviceNames.forEach((serviceName) => {
+			settingsRegistry.add(`Accounts_OAuth_Custom-${ serviceName }-roles_to_sync`, '', {
+				type: 'string',
+				group: 'OAuth',
+				section: `Custom OAuth: ${ serviceName }`,
+				i18nLabel: 'Accounts_OAuth_Custom_Roles_To_Sync',
+				i18nDescription: 'Accounts_OAuth_Custom_Roles_To_Sync_Description',
+				enterprise: true,
+				enableQuery: {
+					_id: `Accounts_OAuth_Custom-${ serviceName }-merge_roles`,
+					value: true,
+				},
+				invalidValue: '',
+				modules: ['oauth-enterprise'],
+			});
+		});
+	},
+});