chore: remove Meteor user invocation from rest api (#38017)

apps/meteor/app/api/server/ApiClass.ts

@@ -496,18 +496,16 @@ export class APIClass<TBasePath extends string = '', TOperations extends Record<
 	public async processTwoFactor({
 		userId,
 		request,
-		invocation,
 		options,
 		connection,
 	}: {
 		userId: string;
 		request: Request;
-		invocation: { twoFactorChecked?: boolean };
 		options?: Options;
 		connection: IMethodConnection;
-	}): Promise<void> {
+	}): Promise<boolean> {
 		if (options && (!('twoFactorRequired' in options) || !options.twoFactorRequired)) {
-			return;
+			return false;
 		}
 		const code = request.headers.get('x-2fa-code') ? String(request.headers.get('x-2fa-code')) : undefined;
 		const method = request.headers.get('x-2fa-method') ? String(request.headers.get('x-2fa-method')) : undefined;
@@ -520,7 +518,7 @@ export class APIClass<TBasePath extends string = '', TOperations extends Record<
 			connection,
 		});
 
-		invocation.twoFactorChecked = true;
+		return true;
 	}
 
 	public getFullRouteName(route: string, method: string): string {
@@ -902,30 +900,28 @@ export class APIClass<TBasePath extends string = '', TOperations extends Record<
 								}
 							}
 
-							const invocation = new DDPCommon.MethodInvocation({
-								connection,
-								isSimulation: false,
-								userId: this.userId,
-							});
-
-							Accounts._accountData[connection.id] = {
-								connection,
-							};
-
-							Accounts._setAccountData(connection.id, 'loginToken', this.token!);
-
-							this.userId &&
+							if (
+								this.userId &&
 								(await api.processTwoFactor({
 									userId: this.userId,
 									request: this.request,
-									invocation: invocation as unknown as Record<string, any>,
 									options: _options,
 									connection: connection as unknown as IMethodConnection,
-								}));
+								}))
+							) {
+								this.twoFactorChecked = true;
+							}
 
 							this.parseJsonQuery = () => api.parseJsonQuery(this);
 
-							result = (await DDP._CurrentInvocation.withValue(invocation as any, async () => originalAction.apply(this))) || api.success();
+							if (options.applyMeteorContext) {
+								const invocation = APIClass.createMeteorInvocation(connection, this.userId, this.token);
+								result = await invocation
+									.applyInvocation(() => originalAction.apply(this))
+									.finally(() => invocation[Symbol.asyncDispose]());
+							} else {
+								result = await originalAction.apply(this);
+							}
 						} catch (e: any) {
 							result = ((e: any) => {
 								switch (e.error) {
@@ -1209,4 +1205,38 @@ export class APIClass<TBasePath extends string = '', TOperations extends Record<
 			},
 		);
 	}
+
+	static createMeteorInvocation(
+		connection: {
+			id: string;
+			close: () => void;
+			clientAddress: string;
+			httpHeaders: Record<string, any>;
+		},
+		userId?: string,
+		token?: string,
+	) {
+		const invocation = new DDPCommon.MethodInvocation({
+			connection,
+			isSimulation: false,
+			userId,
+		});
+
+		Accounts._accountData[connection.id] = {
+			connection,
+		};
+		if (token) {
+			Accounts._setAccountData(connection.id, 'loginToken', token);
+		}
+
+		return {
+			invocation,
+			applyInvocation: <F extends () => Promise<any>>(action: F): ReturnType<F> => {
+				return DDP._CurrentInvocation.withValue(invocation as any, async () => action()) as ReturnType<F>;
+			},
+			[Symbol.asyncDispose]() {
+				return Promise.resolve();
+			},
+		};
+	}
 }

apps/meteor/app/api/server/definition.ts

@@ -150,6 +150,7 @@ export type SharedOptions<TMethod extends string> = (
 		version: DeprecationLoggerNextPlannedVersion;
 		alternatives?: PathPattern[];
 	};
+	applyMeteorContext?: boolean;
 };
 
 export type GenericRouteExecutionContext = ActionThis<any, any, any>;
@@ -191,6 +192,8 @@ export type ActionThis<TMethod extends Method, TPathPattern extends PathPattern,
 	readonly queryOperations: TOptions extends { queryOperations: infer T } ? T : never;
 	readonly queryFields: TOptions extends { queryFields: infer T } ? T : never;
 
+	readonly twoFactorChecked: boolean;
+
 	parseJsonQuery(): Promise<{
 		sort: Record<string, 1 | -1>;
 		/**

apps/meteor/app/api/server/v1/misc.ts

@@ -476,6 +476,7 @@ API.v1.addRoute(
 		authRequired: true,
 		rateLimiterOptions: false,
 		validateParams: isMeteorCall,
+		applyMeteorContext: true,
 	},
 	{
 		async post() {
@@ -515,8 +516,7 @@ API.v1.addRoute(
 					});
 				}
 
-				const result = await Meteor.callAsync(method, ...params);
-				return API.v1.success(mountResult({ id, result }));
+				return API.v1.success(mountResult({ id, result: await Meteor.callAsync(method, ...params) }));
 			} catch (err) {
 				if (!(err as any).isClientSafe && !(err as any).meteorError) {
 					SystemLogger.error({ msg: 'Exception while invoking method', err, method });
@@ -531,12 +531,14 @@ API.v1.addRoute(
 		},
 	},
 );
+
 API.v1.addRoute(
 	'method.callAnon/:method',
 	{
 		authRequired: false,
 		rateLimiterOptions: false,
 		validateParams: isMeteorCall,
+		applyMeteorContext: true,
 	},
 	{
 		async post() {
@@ -572,8 +574,7 @@ API.v1.addRoute(
 					});
 				}
 
-				const result = await Meteor.callAsync(method, ...params);
-				return API.v1.success(mountResult({ id, result }));
+				return API.v1.success(mountResult({ id, result: await Meteor.callAsync(method, ...params) }));
 			} catch (err) {
 				if (!(err as any).isClientSafe && !(err as any).meteorError) {
 					SystemLogger.error({ msg: 'Exception while invoking method', err, method });

apps/meteor/app/authentication/server/startup/index.js

@@ -24,7 +24,6 @@ import { notifyOnSettingChangedById } from '../../../lib/server/lib/notifyListen
 import * as Mailer from '../../../mailer/server/api';
 import { settings } from '../../../settings/server';
 import { getBaseUserFields } from '../../../utils/server/functions/getBaseUserFields';
-import { safeGetMeteorUser } from '../../../utils/server/functions/safeGetMeteorUser';
 import { isValidAttemptByUser, isValidLoginAttemptByIp } from '../lib/restrictLoginAttempts';
 
 Accounts.config({
@@ -380,7 +379,7 @@ Accounts.insertUserDoc = async function (options, user) {
 
 	if (!options.skipAppsEngineEvent) {
 		// `post` triggered events don't need to wait for the promise to resolve
-		Apps.self?.triggerEvent(AppEvents.IPostUserCreated, { user, performedBy: await safeGetMeteorUser() }).catch((e) => {
+		Apps.self?.triggerEvent(AppEvents.IPostUserCreated, { user, performedBy: options.performedBy }).catch((e) => {
 			Apps.self?.getRocketChatLogger().error({ msg: 'Error while executing post user created event', err: e });
 		});
 	}

apps/meteor/app/lib/server/functions/saveUser/saveNewUser.ts

@@ -1,3 +1,4 @@
+import type { IUser } from '@rocket.chat/core-typings';
 import { Users } from '@rocket.chat/models';
 import Gravatar from 'gravatar';
 
@@ -11,7 +12,7 @@ import { handleNickname } from './handleNickname';
 import type { SaveUserData } from './saveUser';
 import { sendPasswordEmail, sendWelcomeEmail } from './sendUserEmail';
 
-export const saveNewUser = async function (userData: SaveUserData, sendPassword: boolean) {
+export const saveNewUser = async function (userData: SaveUserData, sendPassword: boolean, performedBy: IUser) {
 	await validateEmailDomain(userData.email);
 
 	const roles = (!!userData.roles && userData.roles.length > 0 && userData.roles) || getNewUserRoles();
@@ -25,6 +26,7 @@ export const saveNewUser = async function (userData: SaveUserData, sendPassword:
 		isGuest,
 		globalRoles: roles,
 		skipNewUserRolesSetting: true,
+		performedBy,
 	};
 	if (userData.email) {
 		createUser.email = userData.email;

apps/meteor/app/lib/server/functions/saveUser/saveUser.ts

@@ -18,7 +18,6 @@ import type { UserChangedAuditStore } from '../../../../../server/lib/auditServe
 import { callbacks } from '../../../../../server/lib/callbacks';
 import { shouldBreakInVersion } from '../../../../../server/lib/shouldBreakInVersion';
 import { hasPermissionAsync } from '../../../../authorization/server/functions/hasPermission';
-import { safeGetMeteorUser } from '../../../../utils/server/functions/safeGetMeteorUser';
 import { generatePassword } from '../../lib/generatePassword';
 import { notifyOnUserChange } from '../../lib/notifyListener';
 import { passwordPolicy } from '../../lib/passwordPolicy';
@@ -63,8 +62,19 @@ type SaveUserOptions = {
 	auditStore?: UserChangedAuditStore;
 };
 
+const findUserById = async (uid: IUser['_id']): Promise<IUser> => {
+	const user = await Users.findOneById(uid);
+	if (!user) {
+		throw new Meteor.Error('error-invalid-user', 'Invalid user');
+	}
+
+	return user;
+};
+
 const _saveUser = (session?: ClientSession) =>
 	async function (userId: IUser['_id'], userData: SaveUserData, options?: SaveUserOptions) {
+		const performedBy = await findUserById(userId);
+
 		const oldUserData = userData._id && (await Users.findOneById(userData._id));
 		if (oldUserData && isUserFederated(oldUserData)) {
 			throw new Meteor.Error('Edit_Federated_User_Not_Allowed', 'Not possible to edit a federated user');
@@ -91,7 +101,7 @@ const _saveUser = (session?: ClientSession) =>
 
 		if (!isUpdateUserData(userData)) {
 			// TODO audit new users
-			return saveNewUser(userData, sendPassword);
+			return saveNewUser(userData, sendPassword, performedBy);
 		}
 
 		if (!oldUserData) {
@@ -212,7 +222,7 @@ const _saveUser = (session?: ClientSession) =>
 			await Apps.self?.triggerEvent(AppEvents.IPostUserUpdated, {
 				user: userUpdated,
 				previousUser: oldUserData,
-				performedBy: await safeGetMeteorUser(),
+				performedBy,
 			});
 
 			if (sendPassword) {

apps/meteor/app/utils/server/functions/safeGetMeteorUser.ts

@@ -1,30 +0,0 @@
-import { Meteor } from 'meteor/meteor';
-
-const invalidEnvironmentErrorMessage = 'Meteor.userId can only be invoked in method calls or publications.';
-
-/**
- * Helper that executes the `Meteor.userAsync()`, but
- * supresses errors thrown if the code isn't
- * executed inside Meteor's environment
- *
- * Use this function only if it the code path is
- * expected to run out of Meteor's environment and
- * is prepared to handle those cases. Otherwise, it
- * is advisable to call `Meteor.userAsync()` directly
- *
- * @returns The current user in the Meteor session, or null if not available
- */
-export async function safeGetMeteorUser(): Promise<Meteor.User | null> {
-	try {
-		// Explicitly await here otherwise the try...catch wouldn't work.
-		return await Meteor.userAsync();
-	} catch (error: any) {
-		// This is the only type of error we want to capture and supress,
-		// so if the error thrown is different from what we expect, we let it go
-		if (error?.message !== invalidEnvironmentErrorMessage) {
-			throw error;
-		}
-
-		return null;
-	}
-}

apps/meteor/ee/server/apps/communication/rest.ts

@@ -249,7 +249,7 @@ export class AppsRestApi {
 		// WE NEED TO MOVE EACH ENDPOINT HANDLER TO IT'S OWN FILE
 		this.api.addRoute(
 			'',
-			{ authRequired: true, permissionsRequired: ['manage-apps'] },
+			{ authRequired: true, permissionsRequired: ['manage-apps'], applyMeteorContext: true },
 			{
 				async post() {
 					let buff;
@@ -721,7 +721,7 @@ export class AppsRestApi {
 
 		this.api.addRoute(
 			':id',
-			{ authRequired: true, permissionsRequired: ['manage-apps'] },
+			{ authRequired: true, permissionsRequired: ['manage-apps'], applyMeteorContext: true },
 			{
 				async get() {
 					if (this.queryParams.marketplace && this.queryParams.version) {

apps/meteor/server/lib/rooms/roomTypes/direct.ts

@@ -1,7 +1,6 @@
 import type { AtLeast } from '@rocket.chat/core-typings';
 import { isRoomFederated, isRoomNativeFederated } from '@rocket.chat/core-typings';
 import { Subscriptions } from '@rocket.chat/models';
-import { Meteor } from 'meteor/meteor';
 
 import { settings } from '../../../../app/settings/server';
 import type { IRoomTypeServerDirectives } from '../../../../definition/IRoomTypeConfig';
@@ -12,14 +11,6 @@ import { roomCoordinator } from '../roomCoordinator';
 
 const DirectMessageRoomType = getDirectMessageRoomType(roomCoordinator);
 
-const getCurrentUserId = (): string | undefined => {
-	try {
-		return Meteor.userId() || undefined;
-	} catch (_e) {
-		//
-	}
-};
-
 roomCoordinator.add(DirectMessageRoomType, {
 	allowRoomSettingChange(_room, setting) {
 		if (isRoomFederated(_room)) {
@@ -60,7 +51,7 @@ roomCoordinator.add(DirectMessageRoomType, {
 		}
 	},
 
-	async roomName(room, userId?) {
+	async roomName(room, uid) {
 		const subscription = await (async (): Promise<{ fname?: string; name?: string } | null> => {
 			if (room.fname || room.name) {
 				return {
@@ -73,7 +64,6 @@ roomCoordinator.add(DirectMessageRoomType, {
 				return null;
 			}
 
-			const uid = userId || getCurrentUserId();
 			if (uid) {
 				return Subscriptions.findOneByRoomIdAndUserId(room._id, uid, { projection: { name: 1, fname: 1 } });
 			}