Add more control on http trigger (#21216)
add "new Response" utils to define response code or content type of http route triggered logic function responses follow up of https://github.com/twentyhq/twenty/pull/21214 --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
+49
@@ -0,0 +1,49 @@
|
||||
import { LOGIC_FUNCTION_HTTP_RESPONSE_MARKER } from 'twenty-shared/types';
|
||||
|
||||
import { buildRouteTriggerResponse } from 'src/engine/core-modules/logic-function/logic-function-trigger/triggers/route/route-trigger.service';
|
||||
|
||||
describe('buildRouteTriggerResponse', () => {
|
||||
it('wraps a plain body with status 200 and no headers', () => {
|
||||
expect(buildRouteTriggerResponse({ message: 'hi' })).toEqual({
|
||||
statusCode: 200,
|
||||
headers: {},
|
||||
body: { message: 'hi' },
|
||||
});
|
||||
});
|
||||
|
||||
it('passes through null/undefined as a 200 with that body', () => {
|
||||
expect(buildRouteTriggerResponse(null)).toEqual({
|
||||
statusCode: 200,
|
||||
headers: {},
|
||||
body: null,
|
||||
});
|
||||
});
|
||||
|
||||
it('reads status, headers and body from a wrapped response', () => {
|
||||
const data = {
|
||||
[LOGIC_FUNCTION_HTTP_RESPONSE_MARKER]: true,
|
||||
body: '<h1>Hi</h1>',
|
||||
status: 201,
|
||||
headers: { 'Content-Type': 'text/html' },
|
||||
};
|
||||
|
||||
expect(buildRouteTriggerResponse(data)).toEqual({
|
||||
statusCode: 201,
|
||||
headers: { 'Content-Type': 'text/html' },
|
||||
body: '<h1>Hi</h1>',
|
||||
});
|
||||
});
|
||||
|
||||
it('defaults a wrapped response without status/headers to 200 and {}', () => {
|
||||
const data = {
|
||||
[LOGIC_FUNCTION_HTTP_RESPONSE_MARKER]: true,
|
||||
body: { ok: true },
|
||||
};
|
||||
|
||||
expect(buildRouteTriggerResponse(data)).toEqual({
|
||||
statusCode: 200,
|
||||
headers: {},
|
||||
body: { ok: true },
|
||||
});
|
||||
});
|
||||
});
|
||||
+23
-3
@@ -5,7 +5,7 @@ import { Request } from 'express';
|
||||
import { match } from 'path-to-regexp';
|
||||
import { assertIsDefinedOrThrow, isDefined } from 'twenty-shared/utils';
|
||||
import { IsNull, Not, Repository } from 'typeorm';
|
||||
import { HTTPMethod } from 'twenty-shared/types';
|
||||
import { HTTPMethod, isLogicFunctionHttpResponse } from 'twenty-shared/types';
|
||||
|
||||
import { AccessTokenService } from 'src/engine/core-modules/auth/token/services/access-token.service';
|
||||
import { WorkspaceDomainsService } from 'src/engine/core-modules/domain/workspace-domains/services/workspace-domains.service';
|
||||
@@ -26,6 +26,26 @@ import {
|
||||
} from 'src/engine/core-modules/logic-function/logic-function-executor/logic-function-executor.service';
|
||||
import { CustomException } from 'src/utils/custom-exception';
|
||||
|
||||
export type RouteTriggerResponse = {
|
||||
statusCode: number;
|
||||
headers: Record<string, string>;
|
||||
body: unknown;
|
||||
};
|
||||
|
||||
export const buildRouteTriggerResponse = (
|
||||
data: unknown,
|
||||
): RouteTriggerResponse => {
|
||||
if (isLogicFunctionHttpResponse(data)) {
|
||||
return {
|
||||
statusCode: data.status ?? 200,
|
||||
headers: data.headers ?? {},
|
||||
body: data.body,
|
||||
};
|
||||
}
|
||||
|
||||
return { statusCode: 200, headers: {}, body: data };
|
||||
};
|
||||
|
||||
@Injectable()
|
||||
export class RouteTriggerService {
|
||||
private readonly logger = new Logger(RouteTriggerService.name);
|
||||
@@ -223,7 +243,7 @@ export class RouteTriggerService {
|
||||
}
|
||||
|
||||
if (!isDefined(result)) {
|
||||
return result;
|
||||
return buildRouteTriggerResponse(result);
|
||||
}
|
||||
|
||||
if (result.error) {
|
||||
@@ -233,6 +253,6 @@ export class RouteTriggerService {
|
||||
);
|
||||
}
|
||||
|
||||
return result.data;
|
||||
return buildRouteTriggerResponse(result.data);
|
||||
}
|
||||
}
|
||||
|
||||
+137
-30
@@ -1,20 +1,29 @@
|
||||
import { HttpStatus } from '@nestjs/common';
|
||||
import { HTTP_CODE_METADATA } from '@nestjs/common/constants';
|
||||
|
||||
import { type Request } from 'express';
|
||||
import { type Response } from 'express';
|
||||
import { HTTPMethod } from 'twenty-shared/types';
|
||||
|
||||
import { RouteTriggerService } from 'src/engine/core-modules/logic-function/logic-function-trigger/triggers/route/route-trigger.service';
|
||||
import { RouteTriggerController } from 'src/engine/metadata-modules/route-trigger/route-trigger.controller';
|
||||
|
||||
const createResponseMock = () => {
|
||||
const headers: Record<string, string> = {};
|
||||
|
||||
return {
|
||||
status: jest.fn(),
|
||||
setHeader: jest.fn((key: string, value: string) => {
|
||||
headers[key.toLowerCase()] = value;
|
||||
}),
|
||||
getHeader: jest.fn((key: string) => headers[key.toLowerCase()]),
|
||||
send: jest.fn(),
|
||||
json: jest.fn(),
|
||||
} as unknown as Response;
|
||||
};
|
||||
|
||||
describe('RouteTriggerController', () => {
|
||||
let controller: RouteTriggerController;
|
||||
const handle = jest.fn();
|
||||
|
||||
beforeEach(() => {
|
||||
const routeTriggerService = {
|
||||
handle,
|
||||
} as unknown as RouteTriggerService;
|
||||
const routeTriggerService = { handle } as unknown as RouteTriggerService;
|
||||
|
||||
controller = new RouteTriggerController(routeTriggerService);
|
||||
});
|
||||
@@ -27,34 +36,132 @@ describe('RouteTriggerController', () => {
|
||||
expect(controller).toBeDefined();
|
||||
});
|
||||
|
||||
describe('response status code', () => {
|
||||
it.each([
|
||||
['get', RouteTriggerController.prototype.get],
|
||||
['post', RouteTriggerController.prototype.post],
|
||||
['put', RouteTriggerController.prototype.put],
|
||||
['patch', RouteTriggerController.prototype.patch],
|
||||
['delete', RouteTriggerController.prototype.delete],
|
||||
])('should respond with 200 for %s', (_method, handler) => {
|
||||
const httpCode = Reflect.getMetadata(HTTP_CODE_METADATA, handler);
|
||||
it('delegates to the service with the POST http method and applies status 200', async () => {
|
||||
const request = { path: '/s/webhooks/google/leads' } as never;
|
||||
const response = createResponseMock();
|
||||
|
||||
expect(httpCode).toBe(HttpStatus.OK);
|
||||
handle.mockResolvedValue({
|
||||
statusCode: 200,
|
||||
headers: {},
|
||||
body: { ok: true },
|
||||
});
|
||||
|
||||
await controller.post(request, response);
|
||||
|
||||
expect(handle).toHaveBeenCalledWith({
|
||||
request,
|
||||
httpMethod: HTTPMethod.POST,
|
||||
});
|
||||
expect(response.status).toHaveBeenCalledWith(200);
|
||||
expect(response.json).toHaveBeenCalledWith({ ok: true });
|
||||
});
|
||||
|
||||
describe('post', () => {
|
||||
it('should delegate to the service with the POST http method', async () => {
|
||||
const request = { path: '/s/webhooks/google/leads' } as Request;
|
||||
const expectedResult = {};
|
||||
it('applies the status code and allow-listed headers from the service result', async () => {
|
||||
const response = createResponseMock();
|
||||
|
||||
handle.mockResolvedValue(expectedResult);
|
||||
|
||||
const result = await controller.post(request);
|
||||
|
||||
expect(handle).toHaveBeenCalledWith({
|
||||
request,
|
||||
httpMethod: HTTPMethod.POST,
|
||||
});
|
||||
expect(result).toBe(expectedResult);
|
||||
handle.mockResolvedValue({
|
||||
statusCode: 201,
|
||||
headers: { 'Content-Type': 'text/html', 'Cache-Control': 'no-store' },
|
||||
body: '<h1>Hi</h1>',
|
||||
});
|
||||
|
||||
await controller.get({} as never, response);
|
||||
|
||||
expect(response.status).toHaveBeenCalledWith(201);
|
||||
expect(response.setHeader).toHaveBeenCalledWith(
|
||||
'Content-Type',
|
||||
'text/html',
|
||||
);
|
||||
expect(response.setHeader).toHaveBeenCalledWith(
|
||||
'Cache-Control',
|
||||
'no-store',
|
||||
);
|
||||
expect(response.send).toHaveBeenCalledWith('<h1>Hi</h1>');
|
||||
});
|
||||
|
||||
it('drops headers that are not in the allow-list', async () => {
|
||||
const response = createResponseMock();
|
||||
|
||||
handle.mockResolvedValue({
|
||||
statusCode: 200,
|
||||
headers: {
|
||||
'Content-Type': 'text/html',
|
||||
'Set-Cookie': 'session=abc',
|
||||
'Access-Control-Allow-Origin': '*',
|
||||
'X-Custom': 'foo',
|
||||
},
|
||||
body: '<h1>Hi</h1>',
|
||||
});
|
||||
|
||||
await controller.get({} as never, response);
|
||||
|
||||
expect(response.setHeader).toHaveBeenCalledWith(
|
||||
'Content-Type',
|
||||
'text/html',
|
||||
);
|
||||
expect(response.setHeader).not.toHaveBeenCalledWith(
|
||||
'Set-Cookie',
|
||||
'session=abc',
|
||||
);
|
||||
expect(response.setHeader).not.toHaveBeenCalledWith(
|
||||
'Access-Control-Allow-Origin',
|
||||
'*',
|
||||
);
|
||||
expect(response.setHeader).not.toHaveBeenCalledWith('X-Custom', 'foo');
|
||||
});
|
||||
|
||||
it('sends an empty response when the body is nil', async () => {
|
||||
const response = createResponseMock();
|
||||
|
||||
handle.mockResolvedValue({ statusCode: 200, headers: {}, body: null });
|
||||
|
||||
await controller.get({} as never, response);
|
||||
|
||||
expect(response.status).toHaveBeenCalledWith(200);
|
||||
expect(response.send).toHaveBeenCalledWith();
|
||||
expect(response.json).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('defaults a string body content-type to text/plain when none is set', async () => {
|
||||
const response = createResponseMock();
|
||||
|
||||
handle.mockResolvedValue({ statusCode: 200, headers: {}, body: 'plain' });
|
||||
|
||||
await controller.get({} as never, response);
|
||||
|
||||
expect(response.setHeader).toHaveBeenCalledWith(
|
||||
'content-type',
|
||||
'text/plain',
|
||||
);
|
||||
expect(response.send).toHaveBeenCalledWith('plain');
|
||||
});
|
||||
|
||||
it('sends an object body as JSON when no content-type is set', async () => {
|
||||
const response = createResponseMock();
|
||||
|
||||
handle.mockResolvedValue({
|
||||
statusCode: 200,
|
||||
headers: {},
|
||||
body: { ok: true },
|
||||
});
|
||||
|
||||
await controller.get({} as never, response);
|
||||
|
||||
expect(response.json).toHaveBeenCalledWith({ ok: true });
|
||||
});
|
||||
|
||||
it('pre-serializes an object body when a custom content-type is set', async () => {
|
||||
const response = createResponseMock();
|
||||
|
||||
handle.mockResolvedValue({
|
||||
statusCode: 200,
|
||||
headers: { 'Content-Type': 'application/ld+json' },
|
||||
body: { ok: true },
|
||||
});
|
||||
|
||||
await controller.get({} as never, response);
|
||||
|
||||
expect(response.send).toHaveBeenCalledWith(JSON.stringify({ ok: true }));
|
||||
expect(response.json).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
+94
-34
@@ -2,23 +2,34 @@ import {
|
||||
Controller,
|
||||
Delete,
|
||||
Get,
|
||||
HttpCode,
|
||||
HttpStatus,
|
||||
Patch,
|
||||
Post,
|
||||
Put,
|
||||
Req,
|
||||
Res,
|
||||
UseFilters,
|
||||
UseGuards,
|
||||
} from '@nestjs/common';
|
||||
|
||||
import { Request } from 'express';
|
||||
import { Request, Response } from 'express';
|
||||
import { HTTPMethod } from 'twenty-shared/types';
|
||||
import { isDefined } from 'twenty-shared/utils';
|
||||
|
||||
import { NoPermissionGuard } from 'src/engine/guards/no-permission.guard';
|
||||
import { PublicEndpointGuard } from 'src/engine/guards/public-endpoint.guard';
|
||||
import { RouteTriggerRestApiExceptionFilter } from 'src/engine/core-modules/logic-function/logic-function-trigger/triggers/route/exceptions/route-trigger-rest-api-exception-filter';
|
||||
import { RouteTriggerService } from 'src/engine/core-modules/logic-function/logic-function-trigger/triggers/route/route-trigger.service';
|
||||
import {
|
||||
RouteTriggerResponse,
|
||||
RouteTriggerService,
|
||||
} from 'src/engine/core-modules/logic-function/logic-function-trigger/triggers/route/route-trigger.service';
|
||||
|
||||
const ALLOWED_RESPONSE_HEADERS = new Set([
|
||||
'content-type',
|
||||
'content-language',
|
||||
'content-disposition',
|
||||
'cache-control',
|
||||
'retry-after',
|
||||
]);
|
||||
|
||||
@Controller('s')
|
||||
@UseGuards(PublicEndpointGuard, NoPermissionGuard)
|
||||
@@ -27,47 +38,96 @@ export class RouteTriggerController {
|
||||
constructor(private readonly routeTriggerService: RouteTriggerService) {}
|
||||
|
||||
@Get('*path')
|
||||
@HttpCode(HttpStatus.OK)
|
||||
async get(@Req() request: Request) {
|
||||
return await this.routeTriggerService.handle({
|
||||
request,
|
||||
httpMethod: HTTPMethod.GET,
|
||||
});
|
||||
async get(@Req() request: Request, @Res() response: Response) {
|
||||
this.sendResponse(
|
||||
response,
|
||||
await this.routeTriggerService.handle({
|
||||
request,
|
||||
httpMethod: HTTPMethod.GET,
|
||||
}),
|
||||
);
|
||||
}
|
||||
|
||||
@Post('*path')
|
||||
@HttpCode(HttpStatus.OK)
|
||||
async post(@Req() request: Request) {
|
||||
return await this.routeTriggerService.handle({
|
||||
request,
|
||||
httpMethod: HTTPMethod.POST,
|
||||
});
|
||||
async post(@Req() request: Request, @Res() response: Response) {
|
||||
this.sendResponse(
|
||||
response,
|
||||
await this.routeTriggerService.handle({
|
||||
request,
|
||||
httpMethod: HTTPMethod.POST,
|
||||
}),
|
||||
);
|
||||
}
|
||||
|
||||
@Put('*path')
|
||||
@HttpCode(HttpStatus.OK)
|
||||
async put(@Req() request: Request) {
|
||||
return await this.routeTriggerService.handle({
|
||||
request,
|
||||
httpMethod: HTTPMethod.PUT,
|
||||
});
|
||||
async put(@Req() request: Request, @Res() response: Response) {
|
||||
this.sendResponse(
|
||||
response,
|
||||
await this.routeTriggerService.handle({
|
||||
request,
|
||||
httpMethod: HTTPMethod.PUT,
|
||||
}),
|
||||
);
|
||||
}
|
||||
|
||||
@Patch('*path')
|
||||
@HttpCode(HttpStatus.OK)
|
||||
async patch(@Req() request: Request) {
|
||||
return await this.routeTriggerService.handle({
|
||||
request,
|
||||
httpMethod: HTTPMethod.PATCH,
|
||||
});
|
||||
async patch(@Req() request: Request, @Res() response: Response) {
|
||||
this.sendResponse(
|
||||
response,
|
||||
await this.routeTriggerService.handle({
|
||||
request,
|
||||
httpMethod: HTTPMethod.PATCH,
|
||||
}),
|
||||
);
|
||||
}
|
||||
|
||||
@Delete('*path')
|
||||
@HttpCode(HttpStatus.OK)
|
||||
async delete(@Req() request: Request) {
|
||||
return await this.routeTriggerService.handle({
|
||||
request,
|
||||
httpMethod: HTTPMethod.DELETE,
|
||||
});
|
||||
async delete(@Req() request: Request, @Res() response: Response) {
|
||||
this.sendResponse(
|
||||
response,
|
||||
await this.routeTriggerService.handle({
|
||||
request,
|
||||
httpMethod: HTTPMethod.DELETE,
|
||||
}),
|
||||
);
|
||||
}
|
||||
|
||||
private sendResponse(
|
||||
response: Response,
|
||||
{ statusCode, headers, body }: RouteTriggerResponse,
|
||||
) {
|
||||
response.status(statusCode);
|
||||
|
||||
for (const [key, value] of Object.entries(headers)) {
|
||||
if (ALLOWED_RESPONSE_HEADERS.has(key.toLowerCase())) {
|
||||
response.setHeader(key, value);
|
||||
}
|
||||
}
|
||||
|
||||
if (!isDefined(body)) {
|
||||
response.send();
|
||||
|
||||
return;
|
||||
}
|
||||
|
||||
const hasContentType = isDefined(response.getHeader('content-type'));
|
||||
|
||||
if (typeof body === 'string') {
|
||||
if (!hasContentType) {
|
||||
response.setHeader('content-type', 'text/plain');
|
||||
}
|
||||
|
||||
response.send(body);
|
||||
|
||||
return;
|
||||
}
|
||||
|
||||
if (hasContentType) {
|
||||
response.send(JSON.stringify(body));
|
||||
|
||||
return;
|
||||
}
|
||||
|
||||
response.json(body);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user