refactor(api): update authentication handling and context types (#39342)

Author: ggazzo

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

@@ -1,4 +1,4 @@
-import type { IMethodConnection, IUser, RequiredField } from '@rocket.chat/core-typings';
+import type { IMethodConnection, IUser } from '@rocket.chat/core-typings';
 import type { Route, Router } from '@rocket.chat/http-router';
 import { License } from '@rocket.chat/license';
 import { Logger } from '@rocket.chat/logger';
@@ -41,6 +41,8 @@ import type {
 } from './definition';
 import { getUserInfo } from './helpers/getUserInfo';
 import { parseJsonQuery } from './helpers/parseJsonQuery';
+import { authenticationMiddlewareForHono } from './middlewares/authenticationHono';
+import type { APIActionContext } from './router';
 import { RocketChatAPIRouter } from './router';
 import { license } from '../../../ee/app/api-enterprise/server/middlewares/license';
 import { isObject } from '../../../lib/utils/isObject';
@@ -57,7 +59,7 @@ const logger = new Logger('API');
 // We have some breaking changes planned to the API.
 // To avoid conflicts or missing something during the period we are adopting a 'feature flag approach'
 // TODO: MAJOR check if this is still needed
-const applyBreakingChanges = shouldBreakInVersion('9.0.0');
+export const applyBreakingChanges = shouldBreakInVersion('9.0.0');
 type MinimalRoute = {
 	method: 'GET' | 'POST' | 'PUT' | 'DELETE';
 	path: string;
@@ -166,7 +168,7 @@ export class APIClass<TBasePath extends string = '', TOperations extends Record<
 
 	private _routes: { path: string; options: Options; endpoints: Record<string, string> }[] = [];
 
-	public authMethods: ((routeContext: GenericRouteExecutionContext) => Promise<IUser | undefined>)[];
+	public authMethods: ((routeContext: APIActionContext) => Promise<IUser | undefined>)[];
 
 	protected helperMethods: Map<string, () => any> = new Map();
 
@@ -248,7 +250,7 @@ export class APIClass<TBasePath extends string = '', TOperations extends Record<
 		return parseJsonQuery(routeContext);
 	}
 
-	public addAuthMethod(func: (routeContext: GenericRouteExecutionContext) => Promise<IUser | undefined>): void {
+	public addAuthMethod(func: (routeContext: APIActionContext) => Promise<IUser | undefined>): void {
 		this.authMethods.push(func);
 	}
 
@@ -825,37 +827,9 @@ export class APIClass<TBasePath extends string = '', TOperations extends Record<
 						this.queryFields = options.queryFields;
 						this.logger = logger;
 
-						const user = await api.authenticatedRoute(this);
-
-						const isUserWithUsername = (user: IUser | null): user is RequiredField<IUser, 'username'> => {
-							return user !== null && typeof user === 'object' && 'username' in user && user.username !== undefined;
-						};
-
-						this.user = user!;
-						this.userId = this.user?._id;
 						const authToken = this.request.headers.get('x-auth-token');
 						this.token = Accounts._hashLoginToken(String(authToken))!;
 
-						const shouldPreventAnonymousRead = !this.user && options.authOrAnonRequired && !settings.get('Accounts_AllowAnonymousRead');
-						const shouldPreventUserRead = !this.user && options.authRequired;
-
-						if (shouldPreventAnonymousRead || shouldPreventUserRead) {
-							const result = api.unauthorized('You must be logged in to do this.');
-							// compatibility with the old API
-							// TODO: MAJOR
-							if (!applyBreakingChanges) {
-								Object.assign(result.body, {
-									status: 'error',
-									message: 'You must be logged in to do this.',
-								});
-							}
-							return result;
-						}
-
-						if (user && !options.userWithoutUsername && !isUserWithUsername(user)) {
-							throw new Meteor.Error('error-unauthorized', 'Users must have a username');
-						}
-
 						const objectForRateLimitMatch = {
 							IPAddr: this.requestIp,
 							route: api.getFullRouteName(route, this.request.method.toLowerCase()),
@@ -961,6 +935,12 @@ export class APIClass<TBasePath extends string = '', TOperations extends Record<
 				this.router[method.toLowerCase() as 'get' | 'post' | 'put' | 'delete'](
 					`/${route}`.replaceAll('//', '/'),
 					{ ..._options, tags } as TypedOptions,
+					authenticationMiddlewareForHono(this, {
+						authRequired: options.authRequired,
+						authOrAnonRequired: options.authOrAnonRequired,
+						userWithoutUsername: options.userWithoutUsername,
+						logger,
+					}),
 					license(_options as TypedOptions, License),
 					(operations[method as keyof Operations<TPathPattern, TOptions>] as Record<string, any>).action,
 				);
@@ -978,7 +958,7 @@ export class APIClass<TBasePath extends string = '', TOperations extends Record<
 		});
 	}
 
-	protected async authenticatedRoute(routeContext: GenericRouteExecutionContext): Promise<IUser | null> {
+	public async authenticatedRoute(routeContext: APIActionContext): Promise<IUser | null> {
 		const userId = routeContext.request.headers.get('x-user-id');
 		const userToken = routeContext.request.headers.get('x-auth-token');
 

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

@@ -0,0 +1,48 @@
+import { type IUser, type RequiredField } from '@rocket.chat/core-typings';
+import { type Logger } from '@rocket.chat/logger';
+import type { MiddlewareHandler } from 'hono';
+import { Meteor } from 'meteor/meteor';
+
+import { settings } from '../../../settings/server';
+import { applyBreakingChanges, type APIClass } from '../ApiClass';
+import { convertHonoContextToApiActionContext, type HonoContext } from '../router';
+
+const isUserWithUsername = (user: IUser | null): user is RequiredField<IUser, 'username'> => {
+	return user !== null && typeof user === 'object' && 'username' in user && user.username !== undefined;
+};
+
+export function authenticationMiddlewareForHono(
+	api: APIClass<string, Record<string, unknown>>,
+	options: {
+		authRequired?: boolean;
+		authOrAnonRequired?: boolean;
+		userWithoutUsername?: boolean;
+		logger: Logger;
+	},
+): MiddlewareHandler {
+	return async (c: HonoContext, next) => {
+		const user = await api.authenticatedRoute(convertHonoContextToApiActionContext(c, { logger: options.logger }));
+		const shouldPreventAnonymousRead = !user && options.authOrAnonRequired && !settings.get('Accounts_AllowAnonymousRead');
+		const shouldPreventUserRead = !user && options.authRequired;
+
+		if (shouldPreventAnonymousRead || shouldPreventUserRead) {
+			const result = api.unauthorized('You must be logged in to do this.');
+			// TODO: MAJOR
+			if (!applyBreakingChanges) {
+				Object.assign(result.body, {
+					status: 'error',
+					message: 'You must be logged in to do this.',
+				});
+			}
+
+			return c.json(result.body, result.statusCode);
+		}
+
+		if (user && !options.userWithoutUsername && !isUserWithUsername(user)) {
+			throw new Meteor.Error('error-unauthorized', 'Users must have a username');
+		}
+
+		c.set('user', user);
+		return next();
+	};
+}

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

@@ -1,17 +1,20 @@
 import type { IncomingMessage } from 'node:http';
 
+import type { IUser } from '@rocket.chat/core-typings';
 import type { ResponseSchema } from '@rocket.chat/http-router';
 import { Router } from '@rocket.chat/http-router';
+import { type Logger } from '@rocket.chat/logger';
 import type { Context } from 'hono';
 
 import type { TypedOptions } from './definition';
 
-type HonoContext = Context<{
+export type HonoContext = Context<{
 	Bindings: { incoming: IncomingMessage };
 	Variables: {
 		remoteAddress: string;
 		bodyParams: Record<string, unknown>;
 		queryParams: Record<string, unknown>;
+		user?: IUser | null;
 	};
 }>;
 
@@ -25,6 +28,7 @@ export type APIActionContext = {
 	response: any;
 	route: string;
 	incoming: IncomingMessage;
+	logger: Logger;
 };
 
 export type APIActionHandler = (this: APIActionContext, request: Request) => Promise<ResponseSchema<TypedOptions>>;
@@ -35,28 +39,43 @@ export class RocketChatAPIRouter<
 		[x: string]: unknown;
 	} = NonNullable<unknown>,
 > extends Router<TBasePath, TOperations, APIActionHandler> {
-	protected override convertActionToHandler(action: APIActionHandler): (c: HonoContext) => Promise<ResponseSchema<TypedOptions>> {
+	protected override convertActionToHandler(
+		action: APIActionHandler,
+		logger: Logger,
+	): (c: HonoContext) => Promise<ResponseSchema<TypedOptions>> {
 		return async (c: HonoContext): Promise<ResponseSchema<TypedOptions>> => {
-			const { req, res } = c;
+			const { req } = c;
 
 			const request = req.raw.clone();
 
-			const context: APIActionContext = {
-				requestIp: c.get('remoteAddress'),
-				urlParams: req.param(),
-				queryParams: c.get('queryParams'),
-				bodyParams: c.get('bodyParams'),
-				request,
-				path: req.path,
-				response: res,
-				route: req.routePath,
-				incoming: c.env.incoming,
-			};
+			const context = convertHonoContextToApiActionContext(c, { logger });
 
 			return action.apply(context, [request]);
 		};
 	}
 }
 
+export const convertHonoContextToApiActionContext = (
+	c: HonoContext,
+	options: {
+		logger: Logger;
+	},
+): APIActionContext => {
+	const user = c.get('user');
+	return {
+		requestIp: c.get('remoteAddress'),
+		urlParams: c.req.param(),
+		queryParams: c.get('queryParams'),
+		bodyParams: c.get('bodyParams'),
+		request: c.req.raw,
+		path: c.req.path,
+		response: c.res,
+		route: c.req.routePath,
+		incoming: c.env.incoming,
+		logger: options.logger,
+		...(user && { user, userId: user._id }),
+	};
+};
+
 export type ExtractRouterEndpoints<TRoute extends RocketChatAPIRouter<any, any>> =
 	TRoute extends RocketChatAPIRouter<any, infer TOperations> ? TOperations : never;

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

@@ -16,6 +16,7 @@ import type { FailureResult, GenericRouteExecutionContext, SuccessResult, Unavai
 import { loggerMiddleware } from '../../../api/server/middlewares/logger';
 import { metricsMiddleware } from '../../../api/server/middlewares/metrics';
 import { tracerSpanMiddleware } from '../../../api/server/middlewares/tracer';
+import type { APIActionContext } from '../../../api/server/router';
 import type { WebhookResponseItem } from '../../../lib/server/functions/processWebhookMessage';
 import { processWebhookMessage } from '../../../lib/server/functions/processWebhookMessage';
 import { metrics } from '../../../metrics/server';
@@ -357,7 +358,7 @@ function integrationInfoRest(): { statusCode: number; body: { success: boolean }
 }
 
 class WebHookAPI extends APIClass<'/hooks'> {
-	override async authenticatedRoute(routeContext: IntegrationThis): Promise<IUser | null> {
+	override async authenticatedRoute(routeContext: APIActionContext): Promise<IUser | null> {
 		const { integrationId, token } = routeContext.urlParams;
 		const integration = await Integrations.findOneByIdAndToken<IIncomingIntegration>(integrationId, decodeURIComponent(token));
 
@@ -369,9 +370,10 @@ class WebHookAPI extends APIClass<'/hooks'> {
 
 		routeContext.request.headers.set('x-auth-token', token);
 
-		routeContext.request.integration = integration;
+		const req = routeContext.request as Request & { integration?: IIncomingIntegration };
+		req.integration = integration;
 
-		return Users.findOneById(routeContext.request.integration.userId);
+		return Users.findOneById(req.integration.userId);
 	}
 
 	override shouldAddRateLimitToRoute(options: { rateLimiterOptions?: RateLimiterOptions | boolean }): boolean {

apps/meteor/ee/server/apps/communication/endpoints/appLogsExportHandler.ts

@@ -7,7 +7,7 @@ import { json2csv } from 'json-2-csv';
 import type { AppsRestApi } from '../rest';
 import { makeAppLogsQuery } from './lib/makeAppLogsQuery';
 import { APIClass } from '../../../../../app/api/server/ApiClass';
-import type { GenericRouteExecutionContext } from '../../../../../app/api/server/definition';
+import type { APIActionContext } from '../../../../../app/api/server/router';
 
 const isErrorResponse = ajv.compile<{
 	success: false;
@@ -26,7 +26,7 @@ const isErrorResponse = ajv.compile<{
 });
 
 class ExportHandlerAPI extends APIClass {
-	protected override async authenticatedRoute(routeContext: GenericRouteExecutionContext): Promise<IUser | null> {
+	public override async authenticatedRoute(routeContext: APIActionContext): Promise<IUser | null> {
 		const { rc_uid, rc_token } = parse(routeContext.request.headers.get('cookie') || '');
 
 		if (rc_uid) {

packages/http-router/src/Router.ts

@@ -75,7 +75,7 @@ export type Route = {
 };
 
 export abstract class AbstractRouter<TActionCallback = (c: Context) => Promise<ResponseSchema<TypedOptions>>> {
-	protected abstract convertActionToHandler(action: TActionCallback): (c: Context) => Promise<ResponseSchema<TypedOptions>>;
+	protected abstract convertActionToHandler(action: TActionCallback, logger: Logger): (c: Context) => Promise<ResponseSchema<TypedOptions>>;
 }
 
 type InnerRouter = Hono<{
@@ -118,7 +118,7 @@ export class Router<
 					{
 						description: '',
 						content: {
-							'application/json': { schema: ('schema' in schema ? schema.schema : schema) as AnySchema },
+							'application/json': { schema: 'schema' in schema ? schema.schema : schema },
 						},
 					},
 				]),
@@ -185,7 +185,7 @@ export class Router<
 		...actions: MiddlewareHandlerListAndActionHandler<TOptions, TActionCallback>
 	): Router<TBasePath, TOperations, TActionCallback> {
 		const [middlewares, action] = splitArray<MiddlewareHandler, TActionCallback>(actions);
-		const convertedAction = this.convertActionToHandler(action);
+		const convertedAction = this.convertActionToHandler(action, logger);
 
 		const path = `/${subpath}`.replace('//', '/');
 		(
@@ -314,21 +314,21 @@ export class Router<
 			};
 
 			if (isContentLess(statusCode)) {
-				return c.status(statusCode as 101 | 204 | 205 | 304);
+				return c.status(statusCode);
 			}
 			Object.entries(responseHeaders).forEach(([key, value]) => {
 				if (value) {
 					c.header(key, String(value));
 				}
 			});
 
-			return c.body((contentType?.match(/json|javascript/) ? JSON.stringify(body) : body) as any, statusCode as StatusCode);
+			return c.body(contentType?.match(/json|javascript/) ? JSON.stringify(body) : body, statusCode as StatusCode);
 		});
 		this.registerTypedRoutes(method, subpath, options);
 		return this;
 	}
 
-	protected convertActionToHandler(action: TActionCallback): (c: Context) => Promise<ResponseSchema<TypedOptions>> {
+	protected convertActionToHandler(action: TActionCallback, _logger: Logger): (c: Context) => Promise<ResponseSchema<TypedOptions>> {
 		// Default implementation simply passes through the action
 		// Subclasses can override this to provide custom handling
 		return action as (c: Context) => Promise<ResponseSchema<TypedOptions>>;