Skip to content

fix(core): Add support for proxy using forward headers #15006

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
merged 7 commits into from
May 22, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
118 changes: 111 additions & 7 deletions packages/cli/src/push/__tests__/index.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import type { Application } from 'express';
import { captor, mock } from 'jest-mock-extended';
import type { Logger } from 'n8n-core';
import type { Server, ServerResponse } from 'node:http';
import type { Socket } from 'node:net';
import { type WebSocket, Server as WSServer } from 'ws';
Expand Down Expand Up @@ -27,12 +28,16 @@ describe('Push', () => {
const host = 'example.com';
const user = mock<User>({ id: 'user-id' });
const config = mock<PushConfig>();
const logger = mock<Logger>();

let push: Push;
const sseBackend = mockInstance(SSEPush);
const wsBackend = mockInstance(WebSocketPush);

beforeEach(() => jest.resetAllMocks());
beforeEach(() => {
jest.resetAllMocks();
logger.scoped.mockReturnValue(logger);
});

describe('setupPushServer', () => {
const restEndpoint = 'rest';
Expand All @@ -44,7 +49,7 @@ describe('Push', () => {
describe('sse backend', () => {
test('should not create a WebSocket server', () => {
config.backend = 'sse';
push = new Push(config, mock(), mock(), mock(), mock());
push = new Push(config, mock(), logger, mock(), mock());

push.setupPushServer(restEndpoint, server, app);

Expand All @@ -61,7 +66,7 @@ describe('Push', () => {

beforeEach(() => {
config.backend = 'websocket';
push = new Push(config, mock(), mock(), mock(), mock());
push = new Push(config, mock(), logger, mock(), mock());
wssSpy.mockReturnValue(wsServer);

push.setupPushServer(restEndpoint, server, app);
Expand Down Expand Up @@ -130,15 +135,61 @@ describe('Push', () => {

beforeEach(() => {
config.backend = backendName;
push = new Push(config, mock(), mock(), mock(), mock());
push = new Push(config, mock(), logger, mock(), mock());
req.ws = backendName === 'sse' ? undefined : ws;
});

describe('should throw on invalid origin', () => {
test.each(['https://subdomain.example.com', 'https://123example.com', undefined])(
'%s',
(origin) => {
test.each([
{
name: 'origin does not match host',
origin: 'https://subdomain.example.com',
xForwardedProto: undefined,
xForwardedHost: undefined,
},
{
name: 'origin does not match host (subdomain)',
origin: 'https://123example.com',
xForwardedProto: undefined,
xForwardedHost: undefined,
},
{
name: 'origin is not defined',
origin: undefined,
xForwardedProto: undefined,
xForwardedHost: undefined,
},
{
name: 'only one of the forward headers is defined',
origin: 'https://123example.com',
xForwardedProto: 'https',
xForwardedHost: undefined,
},
{
name: 'only one of the forward headers is defined',
origin: 'https://123example.com',
xForwardedProto: undefined,
xForwardedHost: '123example.com',
},
{
name: 'protocol mismatch',
// correct origin, but forward headers take precedence
origin: 'https://example.com',
xForwardedProto: 'http',
xForwardedHost: host,
},
{
name: 'origin is undefined',
origin: undefined,
xForwardedProto: undefined,
xForwardedHost: undefined,
},
])(
'$name (origin: $origin, x-forwarded-proto: $xForwardedProto, x-forwarded-host: $xForwardedHost)',
({ origin, xForwardedProto, xForwardedHost }) => {
req.headers.origin = origin;
req.headers['x-forwarded-proto'] = xForwardedProto;
req.headers['x-forwarded-host'] = xForwardedHost;

if (backendName === 'sse') {
expect(() => push.handleRequest(req, res)).toThrow(
Expand All @@ -154,6 +205,59 @@ describe('Push', () => {
);
});

describe('should not throw on invalid origin if `X-Forwarded-Host` and `X-Forwarded-Proto` are set correctly', () => {
test.each([
{
name: 'origin matches forward headers',
origin: 'https://example.com',
xForwardedProto: 'https',
xForwardedHost: 'example.com',
},
{
name: 'origin matches forward headers but has different case',
origin: 'https://example.com',
xForwardedProto: 'https',
xForwardedHost: 'EXAMPLE.com',
},
{
name: 'origin matches forward headers but protocol has different case',
origin: 'HTTPS://example.com',
xForwardedProto: undefined,
xForwardedHost: undefined,
},
{
name: 'origin matches host (https)',
origin: 'https://example.com',
xForwardedProto: undefined,
xForwardedHost: undefined,
},
{
name: 'origin matches host (http)',
origin: 'http://example.com',
xForwardedProto: undefined,
xForwardedHost: undefined,
},
])(
'$name (origin: $origin, x-forwarded-proto: $xForwardedProto, x-forwarded-host: $xForwardedHost)',
({ origin, xForwardedProto, xForwardedHost }) => {
// ARRANGE
req.headers.origin = origin;
req.headers['x-forwarded-proto'] = xForwardedProto;
req.headers['x-forwarded-host'] = xForwardedHost;

const emitSpy = jest.spyOn(push, 'emit');
const connection = backendName === 'sse' ? { req, res } : ws;

// ACT
push.handleRequest(req, res);

// ASSERT
expect(backend.add).toHaveBeenCalledWith(pushRef, user.id, connection);
expect(emitSpy).toHaveBeenCalledWith('editorUiConnected', pushRef);
},
);
});

test('should throw if pushRef is invalid', () => {
req.query = { pushRef: '' };

Expand Down
50 changes: 49 additions & 1 deletion packages/cli/src/push/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,35 @@ export class Push extends TypedEmitter<PushEvents> {
);
}

/**
* Construct the expected origin out of the host and forward headers.
* If `x-forwarded-host` and `x-forwarded-proto` are both defined they take
* precedence over `host`.
* If they are not both defined then `host` is used and the protocol is
* inferred from `origin`.
*/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's also Forwarded header with an optional host field that some people might use, but we can discuss adding support for that on another ticket too.

https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/Forwarded

private constructExpectedOrigin(req: SSEPushRequest | WebSocketPushRequest) {
const headers = req.headers;

if (headers.origin) {
const forwardedHost =
typeof headers['x-forwarded-host'] === 'string' ? headers['x-forwarded-host'] : undefined;
const forwardedProto =
typeof headers['x-forwarded-proto'] === 'string' ? headers['x-forwarded-proto'] : undefined;
const allForwardHeadersAreDefined = forwardedHost && forwardedProto;
const host = allForwardHeadersAreDefined ? forwardedHost : headers.host;
const proto = allForwardHeadersAreDefined
? forwardedProto
: headers.origin?.toLowerCase().startsWith('https://')
? 'https'
: 'http';

return { success: true, expectedOrigin: `${proto}://${host}` } as const;
} else {
return { success: false } as const;
}
}

handleRequest(req: SSEPushRequest | WebSocketPushRequest, res: PushResponse) {
const {
ws,
Expand All @@ -108,12 +137,31 @@ export class Push extends TypedEmitter<PushEvents> {
} = req;

let connectionError = '';

const expectedOriginResult = this.constructExpectedOrigin(req);

if (!pushRef) {
connectionError = 'The query parameter "pushRef" is missing!';
} else if (!expectedOriginResult.success) {
this.logger.warn('Origin header is missing');

connectionError = 'Invalid origin!';
} else if (
inProduction &&
!(headers.origin === `http://${headers.host}` || headers.origin === `https://${headers.host}`)
headers.origin?.toLowerCase() !== expectedOriginResult.expectedOrigin.toLowerCase()
) {
this.logger.warn(
`Origin header does NOT match the expected origin. (Origin: "${headers.origin}", Expected: "${expectedOriginResult.expectedOrigin}")`,
{
expectedOrigin: expectedOriginResult.expectedOrigin,
headers: {
host: req.headers.host,
origin: req.headers.origin,
['x-forwarded-proto']: req.headers['x-forwarded-proto'],
['x-forwarded-host']: req.headers['x-forwarded-host'],
},
},
);
connectionError = 'Invalid origin!';
}

Expand Down
Loading