fix: third-party login is broken #37707

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

ggazzo merged 6 commits into develop from fix/third-party-login

Dec 19, 2025

.changeset/rich-dogs-wonder.md

-Original file line number
+Diff line change
@@ -0,0 +1,5 @@
+    ---
+    '@rocket.chat/meteor': patch
+    ---
+    Fixes an issue that caused Third-party login to not work properly

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

            
                      Original file line number
                      Diff line number
                      Diff line change
                  
    @@ -28,7 +28,6 @@ import type {
  
    	NotFoundResult,

    	Operations,

    	Options,

    	PartialThis,

    	SuccessResult,

    	TypedThis,

    	TypedAction,

    @@ -37,6 +36,7 @@ import type {
  
    	RedirectStatusCodes,

    	RedirectResult,

    	UnavailableResult,

    	GenericRouteExecutionContext,

    } from './definition';

    import { getUserInfo } from './helpers/getUserInfo';

    import { parseJsonQuery } from './helpers/parseJsonQuery';

    @@ -156,12 +156,7 @@ const generateConnection = (
  
    	clientAddress: ipAddress,

    });

    export class APIClass<

    	TBasePath extends string = '',

    	TOperations extends {

    		[x: string]: unknown;

    	} = {},

    > {

    export class APIClass<TBasePath extends string = '', TOperations extends Record<string, unknown> = Record<string, never>> {

    	public typedRoutes: Record<string, Record<string, Route>> = {};

    	protected apiPath?: string;

    @@ -170,7 +165,7 @@ export class APIClass<
  
    	private _routes: { path: string; options: Options; endpoints: Record<string, string> }[] = [];

    	public authMethods: ((...args: any[]) => any)[];

    	public authMethods: ((routeContext: GenericRouteExecutionContext) => Promise<IUser | undefined>)[];

    	protected helperMethods: Map<string, () => any> = new Map();

    @@ -248,11 +243,11 @@ export class APIClass<
  
    		};

    	}

    	async parseJsonQuery(this: PartialThis) {

    		return parseJsonQuery(this);

    	async parseJsonQuery(routeContext: GenericRouteExecutionContext) {

    		return parseJsonQuery(routeContext);

    	}

    	public addAuthMethod(func: (this: PartialThis, ...args: any[]) => any): void {

    	public addAuthMethod(func: (routeContext: GenericRouteExecutionContext) => Promise<IUser | undefined>): void {

    		this.authMethods.push(func);

    	}

    @@ -822,8 +817,12 @@ export class APIClass<
  
    				const api = this;

    				(operations[method as keyof Operations<TPathPattern, TOptions>] as Record<string, any>).action =

    					async function _internalRouteActionHandler() {

    						this.queryOperations = options.queryOperations;

    						this.queryFields = options.queryFields;

    						this.logger = logger;

    						if (options.authRequired || options.authOrAnonRequired) {

    							const user = await api.authenticatedRoute.call(this, this.request);

    							const user = await api.authenticatedRoute(this);

    							this.user = user!;

    							this.userId = this.user?._id;

    							const authToken = this.request.headers.get('x-auth-token');

    @@ -918,9 +917,7 @@ export class APIClass<
  
    									connection: connection as unknown as IMethodConnection,

    								}));

    							this.queryOperations = options.queryOperations;

    							(this as any).queryFields = options.queryFields;

    							this.parseJsonQuery = api.parseJsonQuery.bind(this as unknown as PartialThis);

    							this.parseJsonQuery = () => api.parseJsonQuery(this);

    							result = (await DDP._CurrentInvocation.withValue(invocation as any, async () => originalAction.apply(this))) || api.success();

    						} catch (e: any) {

    @@ -972,12 +969,9 @@ export class APIClass<
  
    		});

    	}

    	protected async authenticatedRoute(req: Request): Promise<IUser | null> {

    		const headers = Object.fromEntries(req.headers.entries());

    		const { 'x-user-id': userId } = headers;

    		const userToken = String(headers['x-auth-token']);

    	protected async authenticatedRoute(routeContext: GenericRouteExecutionContext): Promise<IUser | null> {

    		const userId = routeContext.request.headers.get('x-user-id');

    		const userToken = routeContext.request.headers.get('x-auth-token');

    		if (userId && userToken) {

    			return Users.findOne(

    @@ -990,6 +984,16 @@ export class APIClass<
  
    				},

    			);

    		}

    		for (const method of this.authMethods) {

    			// eslint-disable-next-line no-await-in-loop -- we want serial execution

    			const user = await method(routeContext);

    			if (user) {

    				return user;

    			}

    		}

    		return null;

    	}

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

-Original file line number
+Diff line change
@@ Expand Up / @@ -142,21 +142,16 @@ export type SharedOptions<TMethod extends string> = ( @@
     	};
     };
-    export type PartialThis = {
-    	user(bodyParams: Record<string, unknown>, user: any): Promise<any>;
-    	readonly request: Request & { query: Record<string, string> };
-    	readonly response: Response;
-    	readonly userId: string;
-    	readonly bodyParams: Record<string, unknown>;
-    	readonly path: string;
-    	readonly queryParams: Record<string, string>;
-    	readonly queryOperations?: string[];
-    	readonly queryFields?: string[];
-    	readonly logger: Logger;
-    	readonly route: string;
-    };
+    export type GenericRouteExecutionContext = ActionThis<any, any, any>;
+    export type RouteExecutionContext<TMethod extends Method, TPathPattern extends PathPattern, TOptions> = ActionThis<
+    	TMethod,
+    	TPathPattern,
+    	TOptions
+    >;
     export type ActionThis<TMethod extends Method, TPathPattern extends PathPattern, TOptions> = {
+    	readonly logger: Logger;
     	route: string;
     	readonly requestIp: string;
     	urlParams: UrlParams<TPathPattern>;
@@ Expand All @@
     	readonly request: Request;
     	readonly queryOperations: TOptions extends { queryOperations: infer T } ? T : never;
+    	readonly queryFields: TOptions extends { queryFields: infer T } ? T : never;
     	parseJsonQuery(): Promise<{
     		sort: Record<string, 1 | -1>;
     		/**
@@ Expand Down @@

apps/meteor/app/api/server/helpers/parseJsonQuery.ts

            
                      Original file line number
                      Diff line number
                      Diff line change
                  
    @@ -1,10 +1,11 @@
  
    import ejson from 'ejson';

    import { Meteor } from 'meteor/meteor';

    import { isPlainObject } from '../../../../lib/utils/isPlainObject';

    import { hasPermissionAsync } from '../../../authorization/server/functions/hasPermission';

    import { apiDeprecationLogger } from '../../../lib/server/lib/deprecationWarningLogger';

    import { API } from '../api';

    import type { PartialThis } from '../definition';

    import type { GenericRouteExecutionContext } from '../definition';

    import { clean } from '../lib/cleanQuery';

    import { isValidQuery } from '../lib/isValidQuery';

    @@ -13,7 +14,7 @@ const pathAllowConf = {
  
    	'def': ['$or', '$and', '$regex'],

    };

    export async function parseJsonQuery(api: PartialThis): Promise<{

    export async function parseJsonQuery(api: GenericRouteExecutionContext): Promise<{

    	sort: Record<string, 1 | -1>;

    	/**

    	 * @deprecated To access "fields" parameter, use ALLOW_UNSAFE_QUERY_AND_FIELDS_API_PARAMS environment variable.

    @@ -24,10 +25,14 @@ export async function parseJsonQuery(api: PartialThis): Promise<{
  
    	 */

    	query: Record<string, unknown>;

    }> {

    	const { userId, queryParams: params, logger, queryFields, queryOperations, response, route } = api;

    	const { userId = '', response, route, logger } = api;

    	const params = isPlainObject(api.queryParams) ? api.queryParams : {};

    	const queryFields = Array.isArray(api.queryFields) ? (api.queryFields as string[]) : [];

    	const queryOperations = Array.isArray(api.queryOperations) ? (api.queryOperations as string[]) : [];

    	let sort;

    	if (params.sort) {

    	if (typeof params?.sort === 'string') {

    		try {

    			sort = JSON.parse(params.sort);

    			Object.entries(sort).forEach(([key, value]) => {

    @@ -50,7 +55,7 @@ export async function parseJsonQuery(api: PartialThis): Promise<{
  
    		`The usage of the "${parameter}" parameter in endpoint "${endpoint}" breaks the security of the API and can lead to data exposure. It has been deprecated and will be removed in the version ${version}.`;

    	let fields: Record<string, 0 | 1> | undefined;

    	if (params.fields && isUnsafeQueryParamsAllowed) {

    	if (typeof params?.fields === 'string' && isUnsafeQueryParamsAllowed) {

    		try {

    			apiDeprecationLogger.parameter(route, 'fields', '8.0.0', response, messageGenerator);

    			fields = JSON.parse(params.fields) as Record<string, 0 | 1>;

    @@ -100,7 +105,7 @@ export async function parseJsonQuery(api: PartialThis): Promise<{
  
    	}

    	let query: Record<string, any> = {};

    	if (params.query && isUnsafeQueryParamsAllowed) {

    	if (typeof params?.query === 'string' && isUnsafeQueryParamsAllowed) {

    		apiDeprecationLogger.parameter(route, 'query', '8.0.0', response, messageGenerator);

    		try {

    			query = ejson.parse(params.query);

apps/meteor/app/api/server/middlewares/authentication.ts

-Original file line number
+Diff line change
@@ Expand Up / @@ -27,7 +27,10 @@ export function authenticationMiddleware( @@
     		if (userId && authToken) {
     			req.user = (await Users.findOneByIdAndLoginToken(userId as string, hashLoginToken(authToken as string))) || undefined;
     		} else {
-    			req.user = (await oAuth2ServerAuth(req))?.user;
+    			req.user = await oAuth2ServerAuth({
+    				headers: req.headers as Record<string, string | undefined>,
+    				query: req.query as Record<string, string | undefined>,
+    			});
     		}
     		if (config.rejectUnauthorized && !req.user) {
@@ Expand Down @@

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

            
                      Original file line number
                      Diff line number
                      Diff line change
                  
    @@ -9,10 +9,11 @@ import type { RateLimiterOptionsToCheck } from 'meteor/rate-limit';
  
    import { WebApp } from 'meteor/webapp';

    import _ from 'underscore';

    import { isPlainObject } from '../../../../lib/utils/isPlainObject';

    import { APIClass } from '../../../api/server/ApiClass';

    import type { RateLimiterOptions } from '../../../api/server/api';

    import { API, defaultRateLimiterOptions } from '../../../api/server/api';

    import type { FailureResult, PartialThis, SuccessResult, UnavailableResult } from '../../../api/server/definition';

    import type { FailureResult, GenericRouteExecutionContext, SuccessResult, UnavailableResult } from '../../../api/server/definition';

    import type { WebhookResponseItem } from '../../../lib/server/functions/processWebhookMessage';

    import { processWebhookMessage } from '../../../lib/server/functions/processWebhookMessage';

    import { settings } from '../../../settings/server';

    @@ -39,11 +40,10 @@ type IntegrationOptions = {
  
    	};

    };

    type IntegrationThis = Omit<PartialThis, 'user'> & {

    type IntegrationThis = GenericRouteExecutionContext & {

    	request: Request & {

    		integration: IIncomingIntegration;

    	};

    	urlParams: Record<string, string>;

    	user: IUser & { username: RequiredField<IUser, 'username'> };

    };

    @@ -138,8 +138,8 @@ async function executeIntegrationRest(
  
    	const scriptEngine = getEngine(this.request.integration);

    	let { bodyParams } = this;

    	const separateResponse = this.bodyParams?.separateResponse === true;

    	let bodyParams = isPlainObject(this.bodyParams) ? this.bodyParams : {};

    	const separateResponse = bodyParams.separateResponse === true;

    	let scriptResponse: Record<string, any> | undefined;

    	if (scriptEngine.integrationHasValidScript(this.request.integration) && this.request.body) {

    @@ -152,21 +152,22 @@ async function executeIntegrationRest(
  
    		const contentRaw = Buffer.concat(buffers).toString('utf8');

    		const protocol = `${this.request.headers.get('x-forwarded-proto')}:` || 'http:';

    		const url = new URL(this.request.url, `${protocol}//${this.request.headers.get('host')}`);

    		const query = isPlainObject(this.queryParams) ? this.queryParams : {};

    		const request = {

    			url: {

    				query,

    				hash: url.hash,

    				search: url.search,

    				query: this.queryParams,

    				pathname: url.pathname,

    				path: this.request.url,

    			},

    			url_raw: this.request.url,

    			url_params: this.urlParams,

    			content: this.bodyParams,

    			content: bodyParams,

    			content_raw: contentRaw,

    			headers: Object.fromEntries(this.request.headers.entries()),

    			body: this.bodyParams,

    			body: bodyParams,

    			user: {

    				_id: this.user._id,

    				name: this.user.name || '',

    @@ -191,7 +192,7 @@ async function executeIntegrationRest(
  
    				return API.v1.failure(result.error);

    			}

    			bodyParams = result && result.content;

    			bodyParams = result?.content;

    			if (!('separateResponse' in bodyParams)) {

    				bodyParams.separateResponse = separateResponse;

    @@ -312,8 +313,8 @@ function integrationInfoRest(): { statusCode: number; body: { success: boolean }
  
    }

    class WebHookAPI extends APIClass<'/hooks'> {

    	override async authenticatedRoute(this: IntegrationThis): Promise<IUser | null> {

    		const { integrationId, token } = this.urlParams;

    	override async authenticatedRoute(routeContext: IntegrationThis): Promise<IUser | null> {

    		const { integrationId, token } = routeContext.urlParams;

    		const integration = await Integrations.findOneByIdAndToken<IIncomingIntegration>(integrationId, decodeURIComponent(token));

    		if (!integration) {

    @@ -322,9 +323,9 @@ class WebHookAPI extends APIClass<'/hooks'> {
  
    			throw new Error('Invalid integration id or token provided.');

    		}

    		this.request.integration = integration;

    		routeContext.request.integration = integration;

    		return Users.findOneById(this.request.integration.userId);

    		return Users.findOneById(routeContext.request.integration.userId);

    	}

    	override shouldAddRateLimitToRoute(options: { rateLimiterOptions?: RateLimiterOptions | boolean }): boolean {

apps/meteor/app/oauth2-server-config/server/oauth/oauth2-server.ts

-Original file line number
+Diff line change
@@ Expand Up / @@ -5,6 +5,7 @@ import type express from 'express'; @@
     import { Meteor } from 'meteor/meteor';
     import { WebApp } from 'meteor/webapp';
+    import { isPlainObject } from '../../../../lib/utils/isPlainObject';
     import { OAuth2Server } from '../../../../server/oauth2-server/oauth';
     import { API } from '../../../api/server';
@@ Expand All / @@ -19,13 +20,18 @@ async function getAccessToken(accessToken: string) { @@
     }
     export async function oAuth2ServerAuth(partialRequest: {
-    	headers: Record<string, any>;
-    	query: Record<string, any>;
-    }): Promise<{ user: IUser } | undefined> {
+    	headers: Record<string, string | undefined>;
+    	query: Record<string, string | undefined>;
+    }): Promise<IUser | undefined> {
     	const headerToken = partialRequest.headers.authorization?.replace('Bearer ', '');
     	const queryToken = partialRequest.query.access_token;
+    	const incomingToken = headerToken || queryToken;
-    	const accessToken = await getAccessToken(headerToken || queryToken);
+    	if (!incomingToken) {
+    		return;
+    	}
+    	const accessToken = await getAccessToken(incomingToken);
     	// If there is no token available or the token has expired, return undefined
     	if (!accessToken || (accessToken.expires != null && accessToken.expires < new Date())) {
@@ Expand All / @@ -38,7 +44,7 @@ export async function oAuth2ServerAuth(partialRequest: { @@
     		return;
     	}
-    	return { user };
+    	return user;
     }
     oauth2server.app.disable('x-powered-by');
@@ Expand Down Expand Up @@
     	});
     });
-    API.v1.addAuthMethod(async function () {
-    	return oAuth2ServerAuth(this.request);
+    API.v1.addAuthMethod((routeContext) => {
+    	const headers = Object.fromEntries(routeContext.request.headers.entries());
+    	const query = (isPlainObject(routeContext.queryParams) ? routeContext.queryParams : {}) as Record<string, string | undefined>;
+    	return oAuth2ServerAuth({ headers, query });
     });
     (WebApp.connectHandlers as unknown as ReturnType<typeof express>).use(oauth2server.app);

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: third-party login is broken #37707

Uh oh!

Diff view

Diff view

There are no files selected for viewing

Uh oh!

Uh oh!

Uh oh!

Uh oh!