diff --git a/apps/meteor/app/api/server/middlewares/logger.ts b/apps/meteor/app/api/server/middlewares/logger.ts index 1188556d0a263..bd749a74eae51 100644 --- a/apps/meteor/app/api/server/middlewares/logger.ts +++ b/apps/meteor/app/api/server/middlewares/logger.ts @@ -8,29 +8,22 @@ export const loggerMiddleware = async (c, next) => { const startTime = Date.now(); - let payload = {}; - - // We don't want to consume the request body stream for multipart requests - if (!c.req.header('content-type')?.includes('multipart/form-data')) { - try { - payload = await c.req.raw.clone().json(); - // eslint-disable-next-line no-empty - } catch {} - } else { - payload = '[multipart/form-data]'; - } - - const log = logger.logger.child({ - method: c.req.method, - url: c.req.url, - userId: c.req.header('x-user-id'), - userAgent: c.req.header('user-agent'), - length: c.req.header('content-length'), - host: c.req.header('host'), - referer: c.req.header('referer'), - remoteIP: c.get('remoteAddress'), - ...(['POST', 'PUT', 'PATCH', 'DELETE'].includes(c.req.method) && getRestPayload(payload)), - }); + const log = logger.logger.child( + { + method: c.req.method, + url: c.req.url, + userId: c.req.header('x-user-id'), + userAgent: c.req.header('user-agent'), + length: c.req.header('content-length'), + host: c.req.header('host'), + referer: c.req.header('referer'), + remoteIP: c.get('remoteAddress'), + ...(['POST', 'PUT', 'PATCH', 'DELETE'].includes(c.req.method) && (await getRestPayload(c.req))), + }, + { + redact: ['password'], + }, + ); await next(); diff --git a/apps/meteor/app/api/server/router.ts b/apps/meteor/app/api/server/router.ts index 0fba18a206093..583976cb7efc4 100644 --- a/apps/meteor/app/api/server/router.ts +++ b/apps/meteor/app/api/server/router.ts @@ -9,16 +9,17 @@ import type { TypedOptions } from './definition'; type HonoContext = Context<{ Bindings: { incoming: IncomingMessage }; Variables: { - 'remoteAddress': string; - 'bodyParams-override'?: Record; + remoteAddress: string; + bodyParams: Record; + queryParams: Record; }; }>; export type APIActionContext = { requestIp: string; urlParams: Record; - queryParams: Record; - bodyParams: Record; + queryParams: Record; + bodyParams: Record; request: Request; path: string; response: any; @@ -37,19 +38,14 @@ export class RocketChatAPIRouter< protected override convertActionToHandler(action: APIActionHandler): (c: HonoContext) => Promise> { return async (c: HonoContext): Promise> => { const { req, res } = c; - const queryParams = this.parseQueryParams(req); - const bodyParams = await this.parseBodyParams<{ bodyParamsOverride: Record }>({ - request: req, - extra: { bodyParamsOverride: c.var['bodyParams-override'] || {} }, - }); const request = req.raw.clone(); const context: APIActionContext = { requestIp: c.get('remoteAddress'), urlParams: req.param(), - queryParams, - bodyParams, + queryParams: c.get('queryParams'), + bodyParams: c.get('bodyParams'), request, path: req.path, response: res, diff --git a/apps/meteor/app/integrations/server/api/api.ts b/apps/meteor/app/integrations/server/api/api.ts index 8e1a37c8e76fa..24f09003c4dd9 100644 --- a/apps/meteor/app/integrations/server/api/api.ts +++ b/apps/meteor/app/integrations/server/api/api.ts @@ -403,14 +403,12 @@ const middleware = async (c: Context, next: Next): Promise => { if (body.payload) { // need to compose the full payload in this weird way because body-parser thought it was a form - c.set('bodyParams-override', JSON.parse(body.payload)); return next(); } incomingLogger.debug({ msg: 'Body received as application/x-www-form-urlencoded without the "payload" key, parsed as string', content, }); - c.set('bodyParams-override', JSON.parse(content)); } catch (e: any) { c.body(JSON.stringify({ success: false, error: e.message }), 400); } diff --git a/apps/meteor/app/livechat/server/api/v1/customField.ts b/apps/meteor/app/livechat/server/api/v1/customField.ts index c4e3130a6f014..42b334c45c407 100644 --- a/apps/meteor/app/livechat/server/api/v1/customField.ts +++ b/apps/meteor/app/livechat/server/api/v1/customField.ts @@ -111,10 +111,6 @@ const livechatCustomFieldsEndpoints = API.v1 async function action() { const { customFieldId, customFieldData } = this.bodyParams; - if (!/^[0-9a-zA-Z-_]+$/.test(customFieldId)) { - return API.v1.failure('Invalid custom field name. Use only letters, numbers, hyphens and underscores.'); - } - if (customFieldId) { const customField = await LivechatCustomField.findOneById(customFieldId); if (!customField) { diff --git a/apps/meteor/server/lib/logger/logPayloads.ts b/apps/meteor/server/lib/logger/logPayloads.ts index 67d67b6a68bb5..a09ada0603924 100644 --- a/apps/meteor/server/lib/logger/logPayloads.ts +++ b/apps/meteor/server/lib/logger/logPayloads.ts @@ -1,3 +1,5 @@ +import type { HonoRequest } from 'hono'; + import { omit } from '../../../lib/utils/omit'; const { LOG_METHOD_PAYLOAD = 'false', LOG_REST_PAYLOAD = 'false', LOG_REST_METHOD_PAYLOADS = 'false' } = process.env; @@ -23,7 +25,16 @@ export const getMethodArgs = export const getRestPayload = LOG_REST_PAYLOAD === 'false' && LOG_REST_METHOD_PAYLOADS === 'false' - ? (): null => null - : (payload: unknown): { payload: unknown } => ({ - payload, - }); + ? (): Promise => Promise.resolve(null) + : async (request: HonoRequest): Promise<{ payload: unknown } | null> => { + if (request.header('content-type')?.includes('multipart/form-data')) { + return { payload: '[multipart/form-data]' }; + } + + return { + payload: await request.raw + .clone() + .json() + .catch(() => null), + }; + }; diff --git a/packages/http-router/src/Router.ts b/packages/http-router/src/Router.ts index 4ebec2f5e02ff..a266295847d4b 100644 --- a/packages/http-router/src/Router.ts +++ b/packages/http-router/src/Router.ts @@ -78,6 +78,14 @@ export abstract class AbstractRouter Promise Promise>; } +type InnerRouter = Hono<{ + Variables: { + remoteAddress: string; + bodyParams: Record; + queryParams: Record; + }; +}>; + export class Router< TBasePath extends string, TOperations extends { @@ -85,11 +93,7 @@ export class Router< } = NonNullable, TActionCallback = (c: Context) => Promise>, > extends AbstractRouter { - protected innerRouter: Hono<{ - Variables: { - remoteAddress: string; - }; - }>; + protected innerRouter: InnerRouter; constructor(readonly base: TBasePath) { super(); @@ -150,39 +154,24 @@ export class Router< }; } - protected async parseBodyParams>({ request }: { request: HonoRequest; extra?: T }) { + protected async parseBodyParams({ request }: { request: HonoRequest }): Promise> { try { - let parsedBody = {}; - const contentType = request.header('content-type'); + const contentType = request.header('content-type') || ''; - if (contentType?.includes('multipart/form-data')) { - // Don't parse multipart here, routes handle it manually via UploadService.parse() - // since multipart/form-data is only used for file uploads - return parsedBody; + if (contentType.includes('application/json')) { + return await request.raw.clone().json(); } - if (contentType?.includes('application/json')) { - parsedBody = await request.raw.clone().json(); - } else if (contentType?.includes('application/x-www-form-urlencoded')) { + if (contentType.includes('application/x-www-form-urlencoded')) { const req = await request.raw.clone().formData(); - parsedBody = Object.fromEntries(req.entries()); - } else { - parsedBody = await request.raw.clone().text(); - } - // This is necessary to keep the compatibility with the previous version, otherwise the bodyParams will be an empty string when no content-type is sent - if (parsedBody === '') { - return {}; + return Object.fromEntries(req.entries()); } - if (Array.isArray(parsedBody)) { - return parsedBody; - } - - return { ...parsedBody }; - // eslint-disable-next-line no-empty - } catch {} - - return {}; + return {}; + } catch { + // No problem if there is error, just means the endpoint is going to have to parse the body itself if necessary + return {}; + } } protected parseQueryParams(request: HonoRequest) { @@ -204,6 +193,7 @@ export class Router< let queryParams: Record; try { queryParams = this.parseQueryParams(req); + c.set('queryParams', queryParams); } catch (e) { logger.warn({ msg: 'Error parsing query params for request', path: req.path, err: e }); @@ -233,6 +223,7 @@ export class Router< } const bodyParams = await this.parseBodyParams({ request: req }); + c.set('bodyParams', bodyParams); if (options.body) { const validatorFn = options.body; @@ -439,11 +430,7 @@ export class Router< return router; } - getHonoRouter(): Hono<{ - Variables: { - remoteAddress: string; - }; - }> { + getHonoRouter(): InnerRouter { return this.innerRouter; } } diff --git a/packages/model-typings/src/models/ILivechatCustomFieldModel.ts b/packages/model-typings/src/models/ILivechatCustomFieldModel.ts index 06b5ac1450275..d43a82f4fe224 100644 --- a/packages/model-typings/src/models/ILivechatCustomFieldModel.ts +++ b/packages/model-typings/src/models/ILivechatCustomFieldModel.ts @@ -27,7 +27,7 @@ export interface ILivechatCustomFieldModel extends IBaseModel, ): FindCursor; createOrUpdateCustomField( - _id: string, + _id: string | null, field: string, label: ILivechatCustomField['label'], scope: ILivechatCustomField['scope'], diff --git a/packages/models/src/models/LivechatCustomField.ts b/packages/models/src/models/LivechatCustomField.ts index df13b279442a2..20b445cf46135 100644 --- a/packages/models/src/models/LivechatCustomField.ts +++ b/packages/models/src/models/LivechatCustomField.ts @@ -50,7 +50,7 @@ export class LivechatCustomFieldRaw extends BaseRaw implem } async createOrUpdateCustomField( - _id: string, + _id: string | null, field: string, label: ILivechatCustomField['label'], scope: ILivechatCustomField['scope'], diff --git a/packages/rest-typings/src/v1/omnichannel.ts b/packages/rest-typings/src/v1/omnichannel.ts index 801396f3bc31f..72504f380786e 100644 --- a/packages/rest-typings/src/v1/omnichannel.ts +++ b/packages/rest-typings/src/v1/omnichannel.ts @@ -4445,12 +4445,15 @@ const POSTLivechatSaveCustomFieldsSchema = { properties: { customFieldId: { type: 'string', + pattern: '^[0-9a-zA-Z_-]+$', + nullable: true, }, customFieldData: { type: 'object', properties: { field: { type: 'string', + pattern: '^[0-9a-zA-Z_-]+$', }, label: { type: 'string', @@ -4497,7 +4500,7 @@ const POSTLivechatSaveCustomFieldsSchema = { }; export const isPOSTLivechatSaveCustomFieldsParams = ajv.compile<{ - customFieldId: string; + customFieldId: string | null; customFieldData: Omit & { field: string }; }>(POSTLivechatSaveCustomFieldsSchema);