Skip to content

Commit 8ea741a

Browse files
ShireenMissiShashwat-06michael-radency
authored
fix(core): Fix CORS issue in waiting webhook responses (#23861)
Co-authored-by: Shashwat <[email protected]> Co-authored-by: Michael Kret <[email protected]> Co-authored-by: Michael Kret <[email protected]>
1 parent abd0422 commit 8ea741a

File tree

5 files changed

+189
-7
lines changed

5 files changed

+189
-7
lines changed

‎packages/cli/src/utils/__tests__/cors.util.test.ts‎

Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,135 @@
1+
import type { Request, Response } from 'express';
2+
3+
import { applyCors } from '../cors.util';
4+
5+
describe('applyCors', () => {
6+
let mockReq: Partial<Request>;
7+
let mockRes: Partial<Response>;
8+
let setHeaderSpy: jest.Mock;
9+
let getHeaderSpy: jest.Mock;
10+
11+
beforeEach(() => {
12+
setHeaderSpy = jest.fn();
13+
getHeaderSpy = jest.fn();
14+
15+
mockReq = {
16+
headers: {},
17+
};
18+
19+
mockRes = {
20+
setHeader: setHeaderSpy,
21+
getHeader: getHeaderSpy,
22+
};
23+
});
24+
25+
afterEach(() => {
26+
jest.clearAllMocks();
27+
});
28+
29+
it('should not modify headers if Access-Control-Allow-Origin is already set', () => {
30+
getHeaderSpy.mockReturnValue('https://example.com');
31+
mockReq.headers = { origin: 'https://test.com' };
32+
33+
applyCors(mockReq as Request, mockRes as Response);
34+
35+
expect(getHeaderSpy).toHaveBeenCalledWith('Access-Control-Allow-Origin');
36+
expect(setHeaderSpy).not.toHaveBeenCalled();
37+
});
38+
39+
it('should set Access-Control-Allow-Origin to * when origin is undefined', () => {
40+
getHeaderSpy.mockReturnValue(undefined);
41+
mockReq.headers = {};
42+
43+
applyCors(mockReq as Request, mockRes as Response);
44+
45+
expect(setHeaderSpy).toHaveBeenCalledWith('Access-Control-Allow-Origin', '*');
46+
expect(setHeaderSpy).toHaveBeenCalledWith('Access-Control-Allow-Methods', 'POST, GET, OPTIONS');
47+
expect(setHeaderSpy).toHaveBeenCalledWith('Access-Control-Allow-Headers', 'Content-Type');
48+
expect(setHeaderSpy).toHaveBeenCalledTimes(3);
49+
});
50+
51+
it('should set Access-Control-Allow-Origin to * when origin is "null"', () => {
52+
getHeaderSpy.mockReturnValue(undefined);
53+
mockReq.headers = { origin: 'null' };
54+
55+
applyCors(mockReq as Request, mockRes as Response);
56+
57+
expect(setHeaderSpy).toHaveBeenCalledWith('Access-Control-Allow-Origin', '*');
58+
expect(setHeaderSpy).toHaveBeenCalledWith('Access-Control-Allow-Methods', 'POST, GET, OPTIONS');
59+
expect(setHeaderSpy).toHaveBeenCalledWith('Access-Control-Allow-Headers', 'Content-Type');
60+
expect(setHeaderSpy).toHaveBeenCalledTimes(3);
61+
});
62+
63+
it('should set Access-Control-Allow-Origin to the request origin when origin is provided', () => {
64+
getHeaderSpy.mockReturnValue(undefined);
65+
const origin = 'https://example.com';
66+
mockReq.headers = { origin };
67+
68+
applyCors(mockReq as Request, mockRes as Response);
69+
70+
expect(setHeaderSpy).toHaveBeenCalledWith('Access-Control-Allow-Origin', origin);
71+
expect(setHeaderSpy).toHaveBeenCalledWith('Access-Control-Allow-Methods', 'POST, GET, OPTIONS');
72+
expect(setHeaderSpy).toHaveBeenCalledWith('Access-Control-Allow-Headers', 'Content-Type');
73+
expect(setHeaderSpy).toHaveBeenCalledTimes(3);
74+
});
75+
76+
it('should set Access-Control-Allow-Origin to the request origin with localhost', () => {
77+
getHeaderSpy.mockReturnValue(undefined);
78+
const origin = 'http://localhost:3000';
79+
mockReq.headers = { origin };
80+
81+
applyCors(mockReq as Request, mockRes as Response);
82+
83+
expect(setHeaderSpy).toHaveBeenCalledWith('Access-Control-Allow-Origin', origin);
84+
expect(setHeaderSpy).toHaveBeenCalledWith('Access-Control-Allow-Methods', 'POST, GET, OPTIONS');
85+
expect(setHeaderSpy).toHaveBeenCalledWith('Access-Control-Allow-Headers', 'Content-Type');
86+
expect(setHeaderSpy).toHaveBeenCalledTimes(3);
87+
});
88+
89+
it('should set Access-Control-Allow-Origin to the request origin with custom port', () => {
90+
getHeaderSpy.mockReturnValue(undefined);
91+
const origin = 'https://app.example.com:8080';
92+
mockReq.headers = { origin };
93+
94+
applyCors(mockReq as Request, mockRes as Response);
95+
96+
expect(setHeaderSpy).toHaveBeenCalledWith('Access-Control-Allow-Origin', origin);
97+
expect(setHeaderSpy).toHaveBeenCalledWith('Access-Control-Allow-Methods', 'POST, GET, OPTIONS');
98+
expect(setHeaderSpy).toHaveBeenCalledWith('Access-Control-Allow-Headers', 'Content-Type');
99+
expect(setHeaderSpy).toHaveBeenCalledTimes(3);
100+
});
101+
102+
it('should handle IP address as origin', () => {
103+
getHeaderSpy.mockReturnValue(undefined);
104+
const origin = 'http://192.168.1.1:5000';
105+
mockReq.headers = { origin };
106+
107+
applyCors(mockReq as Request, mockRes as Response);
108+
109+
expect(setHeaderSpy).toHaveBeenCalledWith('Access-Control-Allow-Origin', origin);
110+
expect(setHeaderSpy).toHaveBeenCalledWith('Access-Control-Allow-Methods', 'POST, GET, OPTIONS');
111+
expect(setHeaderSpy).toHaveBeenCalledWith('Access-Control-Allow-Headers', 'Content-Type');
112+
expect(setHeaderSpy).toHaveBeenCalledTimes(3);
113+
});
114+
115+
it('should always set Allow-Methods and Allow-Headers when origin is present', () => {
116+
getHeaderSpy.mockReturnValue(undefined);
117+
const origin = 'https://trusted-domain.com';
118+
mockReq.headers = { origin };
119+
120+
applyCors(mockReq as Request, mockRes as Response);
121+
122+
expect(setHeaderSpy).toHaveBeenCalledWith('Access-Control-Allow-Methods', 'POST, GET, OPTIONS');
123+
expect(setHeaderSpy).toHaveBeenCalledWith('Access-Control-Allow-Headers', 'Content-Type');
124+
});
125+
126+
it('should always set Allow-Methods and Allow-Headers when origin is not present', () => {
127+
getHeaderSpy.mockReturnValue(undefined);
128+
mockReq.headers = {};
129+
130+
applyCors(mockReq as Request, mockRes as Response);
131+
132+
expect(setHeaderSpy).toHaveBeenCalledWith('Access-Control-Allow-Methods', 'POST, GET, OPTIONS');
133+
expect(setHeaderSpy).toHaveBeenCalledWith('Access-Control-Allow-Headers', 'Content-Type');
134+
});
135+
});

‎packages/cli/src/utils/cors.util.ts‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
import type { Request, Response } from 'express';
2+
3+
export function applyCors(req: Request, res: Response) {
4+
if (res.getHeader('Access-Control-Allow-Origin')) {
5+
return;
6+
}
7+
8+
const origin = req.headers.origin;
9+
10+
if (!origin || origin === 'null') {
11+
res.setHeader('Access-Control-Allow-Origin', '*');
12+
} else {
13+
res.setHeader('Access-Control-Allow-Origin', origin);
14+
}
15+
16+
res.setHeader('Access-Control-Allow-Methods', 'POST, GET, OPTIONS');
17+
res.setHeader('Access-Control-Allow-Headers', 'Content-Type');
18+
}

‎packages/cli/src/webhooks/__tests__/waiting-forms.test.ts‎

Lines changed: 29 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -246,10 +246,32 @@ describe('WaitingForms', () => {
246246

247247
await waitingForms.executeWebhook(req, res);
248248

249-
expect(res.header).toHaveBeenCalledWith('Access-Control-Allow-Origin', 'null');
249+
expect(res.setHeader).toHaveBeenCalledWith('Access-Control-Allow-Origin', '*');
250250
});
251251

252-
it('should not set CORS headers when origin header is missing for status endpoint', async () => {
252+
it('should not override existing Access-Control-Allow-Origin header', async () => {
253+
const execution = mock<IExecutionResponse>({
254+
status: 'success',
255+
});
256+
executionRepository.findSingleExecution.mockResolvedValue(execution);
257+
258+
const req = mock<WaitingWebhookRequest>({
259+
headers: { origin: 'null' },
260+
params: {
261+
path: '123',
262+
suffix: WAITING_FORMS_EXECUTION_STATUS,
263+
},
264+
});
265+
266+
const res = mock<express.Response>();
267+
res.setHeader.mockImplementation(() => res);
268+
res.getHeader.mockReturnValue('https://example.com');
269+
270+
await waitingForms.executeWebhook(req, res);
271+
272+
expect(res.setHeader).not.toHaveBeenCalledWith('Access-Control-Allow-Origin', '*');
273+
});
274+
it('should set CORS headers to wildcard when origin header is missing for status endpoint', async () => {
253275
const execution = mock<IExecutionResponse>({
254276
status: 'success',
255277
});
@@ -267,7 +289,7 @@ describe('WaitingForms', () => {
267289

268290
await waitingForms.executeWebhook(req, res);
269291

270-
expect(res.header).not.toHaveBeenCalledWith('Access-Control-Allow-Origin', expect.anything());
292+
expect(res.setHeader).toHaveBeenCalledWith('Access-Control-Allow-Origin', '*');
271293
});
272294

273295
it('should not set CORS headers for non-status endpoints', async () => {
@@ -317,7 +339,10 @@ describe('WaitingForms', () => {
317339

318340
await waitingForms.executeWebhook(req, res);
319341

320-
expect(res.header).not.toHaveBeenCalledWith('Access-Control-Allow-Origin', expect.anything());
342+
expect(res.setHeader).not.toHaveBeenCalledWith(
343+
'Access-Control-Allow-Origin',
344+
expect.anything(),
345+
);
321346
});
322347

323348
it('should handle old executions with missing activeVersionId field when active=true', () => {

‎packages/cli/src/webhooks/waiting-forms.ts‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import { WaitingWebhooks } from '@/webhooks/waiting-webhooks';
1717

1818
import { sanitizeWebhookRequest } from './webhook-request-sanitizer';
1919
import type { IWebhookResponseCallbackData, WaitingWebhookRequest } from './webhook.types';
20+
import { applyCors } from '@/utils/cors.util';
2021

2122
@Service()
2223
export class WaitingForms extends WaitingWebhooks {
@@ -97,9 +98,7 @@ export class WaitingForms extends WaitingWebhooks {
9798
}
9899
}
99100

100-
if (req.headers.origin) {
101-
res.header('Access-Control-Allow-Origin', req.headers.origin);
102-
}
101+
applyCors(req, res);
103102

104103
res.send(status);
105104
return { noWebhookResponse: true };
@@ -148,6 +147,8 @@ export class WaitingForms extends WaitingWebhooks {
148147
}
149148
}
150149

150+
applyCors(req, res);
151+
151152
return await this.getWebhookExecutionData({
152153
execution,
153154
req,

‎packages/cli/src/webhooks/waiting-webhooks.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ import { getWorkflowActiveStatusFromWorkflowData } from '@/executions/execution.
3333
import { NodeTypes } from '@/node-types';
3434
import * as WebhookHelpers from '@/webhooks/webhook-helpers';
3535
import * as WorkflowExecuteAdditionalData from '@/workflow-execute-additional-data';
36+
import { applyCors } from '@/utils/cors.util';
3637

3738
/**
3839
* Service for handling the execution of webhooks of Wait nodes that use the
@@ -170,6 +171,8 @@ export class WaitingWebhooks implements IWebhookManager {
170171

171172
const lastNodeExecuted = execution.data.resultData.lastNodeExecuted as string;
172173

174+
applyCors(req, res);
175+
173176
return await this.getWebhookExecutionData({
174177
execution,
175178
req,

0 commit comments

Comments
 (0)