mirror of
https://github.com/n8n-io/n8n.git
synced 2026-10-17 13:39:44 +02:00
refactor(engine): Make the control plane token scope a parameter on both sides (no-changelog) (#37978)
This commit is contained in:
parent
33960b0684
commit
a63e88e7c1
|
|
@ -1,5 +1,6 @@
|
|||
import type { Logger } from '@n8n/backend-common';
|
||||
import type { EngineConfig } from '@n8n/config';
|
||||
import type { ActionScope } from '@n8n/engine';
|
||||
import { ACTION_TOKEN, mintActionToken, mintIdentityToken } from '@n8n/engine';
|
||||
import type { NextFunction, Request, Response } from 'express';
|
||||
import jwt from 'jsonwebtoken';
|
||||
|
|
@ -29,11 +30,16 @@ describe('createEngineControlPlaneAuthMiddleware', () => {
|
|||
),
|
||||
}) as unknown as Request;
|
||||
|
||||
const authenticate = (authorization?: string, secret = authSecret) => {
|
||||
const authenticate = (
|
||||
authorization?: string,
|
||||
secret = authSecret,
|
||||
requiredScope: ActionScope = 'lifecycle-events:write',
|
||||
) => {
|
||||
const res = newResponse();
|
||||
const middleware = createEngineControlPlaneAuthMiddleware(
|
||||
mock<EngineConfig>({ authSecret: secret }),
|
||||
logger,
|
||||
requiredScope,
|
||||
);
|
||||
middleware(newRequest(authorization), res, next);
|
||||
return res;
|
||||
|
|
@ -60,7 +66,11 @@ describe('createEngineControlPlaneAuthMiddleware', () => {
|
|||
it('reads the secret per request, so one generated after startup still works', () => {
|
||||
// The secret is set after construction.
|
||||
const engineConfig = mock<EngineConfig>({ authSecret: '' });
|
||||
const middleware = createEngineControlPlaneAuthMiddleware(engineConfig, logger);
|
||||
const middleware = createEngineControlPlaneAuthMiddleware(
|
||||
engineConfig,
|
||||
logger,
|
||||
'lifecycle-events:write',
|
||||
);
|
||||
engineConfig.authSecret = authSecret;
|
||||
|
||||
middleware(
|
||||
|
|
@ -87,7 +97,24 @@ describe('createEngineControlPlaneAuthMiddleware', () => {
|
|||
expectRejected(authenticate(`Bearer ${token}`));
|
||||
});
|
||||
|
||||
it('rejects a token carrying a different scope', () => {
|
||||
it('accepts a credential read token on a route that requires that scope', () => {
|
||||
const token = mintActionToken(authSecret, 'credentials:read');
|
||||
const res = authenticate(`Bearer ${token}`, authSecret, 'credentials:read');
|
||||
|
||||
expect(next).toHaveBeenCalled();
|
||||
expect(res.status).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it.each<[ActionScope, ActionScope]>([
|
||||
['credentials:read', 'lifecycle-events:write'],
|
||||
['lifecycle-events:write', 'credentials:read'],
|
||||
])('rejects a %s token on a route that requires %s', (minted, required) => {
|
||||
const token = mintActionToken(authSecret, minted);
|
||||
|
||||
expectRejected(authenticate(`Bearer ${token}`, authSecret, required));
|
||||
});
|
||||
|
||||
it('rejects a token with a scope the engine does not know', () => {
|
||||
const token = jwt.sign({ scope: 'credential:read' }, authSecret, {
|
||||
algorithm: 'HS256',
|
||||
issuer: ACTION_TOKEN.issuer,
|
||||
|
|
|
|||
|
|
@ -1,17 +1,10 @@
|
|||
import type {
|
||||
HttpRequestClient,
|
||||
HttpRequestClientOptions,
|
||||
OutboundHttp,
|
||||
} from '@n8n/backend-network';
|
||||
import type { EngineConfig } from '@n8n/config';
|
||||
import type { LifecycleEvent } from '@n8n/engine';
|
||||
import { InvalidActionTokenError, verifyActionToken } from '@n8n/engine';
|
||||
import type { HttpRequestClient } from '@n8n/backend-network';
|
||||
import type { ActionScope, LifecycleEvent } from '@n8n/engine';
|
||||
import { OperationalError } from 'n8n-workflow';
|
||||
import { mock } from 'vitest-mock-extended';
|
||||
|
||||
import { EngineControlPlaneClient } from '../engine-control-plane-client';
|
||||
|
||||
const authSecret = 'a'.repeat(32);
|
||||
import type { EngineControlPlaneTransport } from '../engine-control-plane-transport';
|
||||
|
||||
const events: LifecycleEvent[] = [
|
||||
{
|
||||
|
|
@ -24,7 +17,7 @@ const events: LifecycleEvent[] = [
|
|||
|
||||
describe('EngineControlPlaneClient', () => {
|
||||
let http: HttpRequestClient;
|
||||
let clientOptions: HttpRequestClientOptions | undefined;
|
||||
let requestedScope: ActionScope | undefined;
|
||||
let client: EngineControlPlaneClient;
|
||||
let signal: AbortSignal;
|
||||
|
||||
|
|
@ -32,29 +25,15 @@ describe('EngineControlPlaneClient', () => {
|
|||
vi.mocked(http.request).mockResolvedValue({ statusCode, body: '', headers: {} });
|
||||
};
|
||||
|
||||
/** Rebuilds the client so each test can vary the config. */
|
||||
const newClient = (engineConfig: Partial<EngineConfig> = {}) => {
|
||||
beforeEach(() => {
|
||||
http = mock<HttpRequestClient>();
|
||||
const outboundHttp = mock<OutboundHttp>({
|
||||
requests: vi.fn((options?: HttpRequestClientOptions) => {
|
||||
clientOptions = options;
|
||||
const transport = mock<EngineControlPlaneTransport>({
|
||||
forScope: vi.fn((scope: ActionScope) => {
|
||||
requestedScope = scope;
|
||||
return http;
|
||||
}),
|
||||
});
|
||||
|
||||
return new EngineControlPlaneClient(
|
||||
mock<EngineConfig>({
|
||||
controlPlaneBaseUrl: '',
|
||||
authSecret,
|
||||
controlPlanePort: 3001,
|
||||
...engineConfig,
|
||||
}),
|
||||
outboundHttp,
|
||||
);
|
||||
};
|
||||
|
||||
beforeEach(() => {
|
||||
client = newClient();
|
||||
client = new EngineControlPlaneClient(transport);
|
||||
signal = new AbortController().signal;
|
||||
});
|
||||
|
||||
|
|
@ -84,23 +63,8 @@ describe('EngineControlPlaneClient', () => {
|
|||
);
|
||||
});
|
||||
|
||||
it('dials the control plane server on the loopback, not n8n main', () => {
|
||||
expect(clientOptions?.baseURL).toBe('http://127.0.0.1:3001');
|
||||
});
|
||||
|
||||
it('dials the configured base URL when the control plane answers elsewhere', () => {
|
||||
newClient({ controlPlaneBaseUrl: 'https://cp.internal:8443' });
|
||||
|
||||
expect(clientOptions?.baseURL).toBe('https://cp.internal:8443');
|
||||
});
|
||||
|
||||
it('opts out of SSRF protection for the n8n-controlled host', () => {
|
||||
expect(clientOptions?.useDefaultSsrfPolicy).toBe('unsafe');
|
||||
});
|
||||
|
||||
it('leaves the send deadline to the engine, which owns it', () => {
|
||||
// A client timeout would fire first and hide the engine's deadline.
|
||||
expect(clientOptions?.timeout).toBeUndefined();
|
||||
it('asks the transport for a client scoped to lifecycle-event writes', () => {
|
||||
expect(requestedScope).toBe('lifecycle-events:write');
|
||||
});
|
||||
|
||||
it("forwards the engine's abort signal, so an abandoned batch cancels its request", async () => {
|
||||
|
|
@ -111,25 +75,6 @@ describe('EngineControlPlaneClient', () => {
|
|||
expect(http.request).toHaveBeenCalledWith(expect.objectContaining({ abortSignal: signal }));
|
||||
});
|
||||
|
||||
it('mints a fresh token per request', () => {
|
||||
expect(typeof clientOptions?.headers).toBe('function');
|
||||
});
|
||||
|
||||
it('sends an action token scoped to lifecycle-event writes that the control plane accepts', () => {
|
||||
const headers = clientOptions?.headers;
|
||||
const resolved = typeof headers === 'function' ? headers() : headers;
|
||||
const authorization = resolved?.authorization ?? '';
|
||||
|
||||
expect(authorization).toMatch(/^Bearer .+/);
|
||||
|
||||
const token = authorization.replace('Bearer ', '');
|
||||
|
||||
expect(() => verifyActionToken(authSecret, token, 'lifecycle-events:write')).not.toThrow();
|
||||
expect(() => verifyActionToken('b'.repeat(32), token, 'lifecycle-events:write')).toThrow(
|
||||
InvalidActionTokenError,
|
||||
);
|
||||
});
|
||||
|
||||
it.each([302, 400, 401, 500])(
|
||||
'rejects a batch the control plane answered %s',
|
||||
async (statusCode) => {
|
||||
|
|
|
|||
|
|
@ -108,6 +108,10 @@ describe('EngineControlPlaneServer', () => {
|
|||
'a token signed with a different secret',
|
||||
mintActionToken('b'.repeat(32), 'lifecycle-events:write'),
|
||||
],
|
||||
[
|
||||
'an action token minted for credential reads',
|
||||
mintActionToken(authSecret, 'credentials:read'),
|
||||
],
|
||||
])('rejects %s', async (_label, token) => {
|
||||
const response = await post({ events }, token);
|
||||
|
||||
|
|
|
|||
|
|
@ -0,0 +1,118 @@
|
|||
import type {
|
||||
HttpRequestClient,
|
||||
HttpRequestClientOptions,
|
||||
OutboundHttp,
|
||||
} from '@n8n/backend-network';
|
||||
import type { EngineConfig } from '@n8n/config';
|
||||
import type { ActionScope } from '@n8n/engine';
|
||||
import { InvalidActionTokenError, verifyActionToken } from '@n8n/engine';
|
||||
import { mock } from 'vitest-mock-extended';
|
||||
|
||||
import { EngineControlPlaneTransport } from '../engine-control-plane-transport';
|
||||
|
||||
const authSecret = 'a'.repeat(32);
|
||||
|
||||
describe('EngineControlPlaneTransport', () => {
|
||||
let http: HttpRequestClient;
|
||||
let clientOptions: HttpRequestClientOptions | undefined;
|
||||
|
||||
let engineConfig: EngineConfig;
|
||||
|
||||
/** Rebuilds the transport so each test can vary the config. */
|
||||
const newTransport = (overrides: Partial<EngineConfig> = {}) => {
|
||||
http = mock<HttpRequestClient>();
|
||||
const outboundHttp = mock<OutboundHttp>({
|
||||
requests: vi.fn((options?: HttpRequestClientOptions) => {
|
||||
clientOptions = options;
|
||||
return http;
|
||||
}),
|
||||
});
|
||||
engineConfig = mock<EngineConfig>({
|
||||
controlPlaneBaseUrl: '',
|
||||
authSecret,
|
||||
controlPlanePort: 3001,
|
||||
...overrides,
|
||||
});
|
||||
|
||||
return new EngineControlPlaneTransport(engineConfig, outboundHttp);
|
||||
};
|
||||
|
||||
/** Resolves the authorization header the way the HTTP client does per request. */
|
||||
const mintedToken = () => {
|
||||
const headers = clientOptions?.headers;
|
||||
const resolved = typeof headers === 'function' ? headers() : headers;
|
||||
const authorization = resolved?.authorization ?? '';
|
||||
|
||||
expect(authorization).toMatch(/^Bearer .+/);
|
||||
|
||||
return authorization.replace('Bearer ', '');
|
||||
};
|
||||
|
||||
describe('forScope', () => {
|
||||
it('dials the control plane server on the loopback, not n8n main', () => {
|
||||
newTransport().forScope('lifecycle-events:write');
|
||||
|
||||
expect(clientOptions?.baseURL).toBe('http://127.0.0.1:3001');
|
||||
});
|
||||
|
||||
it('dials the configured base URL when the control plane answers elsewhere', () => {
|
||||
newTransport({ controlPlaneBaseUrl: 'https://cp.internal:8443' }).forScope(
|
||||
'lifecycle-events:write',
|
||||
);
|
||||
|
||||
expect(clientOptions?.baseURL).toBe('https://cp.internal:8443');
|
||||
});
|
||||
|
||||
it('opts out of SSRF protection for the n8n-controlled host', () => {
|
||||
newTransport().forScope('lifecycle-events:write');
|
||||
|
||||
expect(clientOptions?.useDefaultSsrfPolicy).toBe('unsafe');
|
||||
});
|
||||
|
||||
it('sets no client timeout, so the caller owns the deadline', () => {
|
||||
newTransport().forScope('lifecycle-events:write');
|
||||
|
||||
expect(clientOptions?.timeout).toBeUndefined();
|
||||
});
|
||||
|
||||
it('mints a fresh token per request', () => {
|
||||
newTransport().forScope('lifecycle-events:write');
|
||||
|
||||
expect(typeof clientOptions?.headers).toBe('function');
|
||||
});
|
||||
|
||||
it.each<ActionScope>(['lifecycle-events:write', 'credentials:read'])(
|
||||
'mints a token with the requested scope %s and no other',
|
||||
(scope) => {
|
||||
newTransport().forScope(scope);
|
||||
const token = mintedToken();
|
||||
const otherScope: ActionScope =
|
||||
scope === 'credentials:read' ? 'lifecycle-events:write' : 'credentials:read';
|
||||
|
||||
expect(() => verifyActionToken(authSecret, token, scope)).not.toThrow();
|
||||
expect(() => verifyActionToken(authSecret, token, otherScope)).toThrow(
|
||||
InvalidActionTokenError,
|
||||
);
|
||||
},
|
||||
);
|
||||
|
||||
it('signs the token with the configured secret', () => {
|
||||
newTransport().forScope('lifecycle-events:write');
|
||||
const token = mintedToken();
|
||||
|
||||
expect(() => verifyActionToken('b'.repeat(32), token, 'lifecycle-events:write')).toThrow(
|
||||
InvalidActionTokenError,
|
||||
);
|
||||
});
|
||||
|
||||
it('reads the secret at request time, after the module generated it', () => {
|
||||
newTransport({ authSecret: '' }).forScope('lifecycle-events:write');
|
||||
|
||||
engineConfig.authSecret = authSecret;
|
||||
|
||||
expect(() =>
|
||||
verifyActionToken(authSecret, mintedToken(), 'lifecycle-events:write'),
|
||||
).not.toThrow();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
@ -1,5 +1,6 @@
|
|||
import type { Logger } from '@n8n/backend-common';
|
||||
import type { EngineConfig } from '@n8n/config';
|
||||
import type { ActionScope } from '@n8n/engine';
|
||||
import { InvalidActionTokenError, verifyActionToken } from '@n8n/engine';
|
||||
import type { NextFunction, Request, Response } from 'express';
|
||||
|
||||
|
|
@ -11,12 +12,14 @@ const BEARER_PREFIX = /^bearer /i;
|
|||
type SyncRequestHandler = (req: Request, res: Response, next: NextFunction) => void;
|
||||
|
||||
/**
|
||||
* Rejects a caller the shared secret does not vouch for. Reads the secret per
|
||||
* request, because it is generated after this is constructed.
|
||||
* Rejects a caller the shared secret does not vouch for, and a caller whose
|
||||
* token was minted for another scope. Reads the secret per request, because it
|
||||
* is generated after this is constructed.
|
||||
*/
|
||||
export function createEngineControlPlaneAuthMiddleware(
|
||||
engineConfig: EngineConfig,
|
||||
logger: Logger,
|
||||
requiredScope: ActionScope,
|
||||
): SyncRequestHandler {
|
||||
const reject = (req: Request, res: Response, reason: string): void => {
|
||||
// Logged, never returned: an operator needs the reason, a caller must not.
|
||||
|
|
@ -33,11 +36,7 @@ export function createEngineControlPlaneAuthMiddleware(
|
|||
}
|
||||
|
||||
try {
|
||||
verifyActionToken(
|
||||
engineConfig.authSecret,
|
||||
header.replace(BEARER_PREFIX, ''),
|
||||
'lifecycle-events:write',
|
||||
);
|
||||
verifyActionToken(engineConfig.authSecret, header.replace(BEARER_PREFIX, ''), requiredScope);
|
||||
} catch (error) {
|
||||
if (!(error instanceof InvalidActionTokenError)) throw error;
|
||||
reject(req, res, 'token rejected');
|
||||
|
|
|
|||
|
|
@ -1,11 +1,9 @@
|
|||
import type { HttpRequestClient } from '@n8n/backend-network';
|
||||
import { OutboundHttp } from '@n8n/backend-network';
|
||||
import { EngineConfig } from '@n8n/config';
|
||||
import { Service } from '@n8n/di';
|
||||
import type { LifecycleEvent } from '@n8n/engine';
|
||||
import { mintActionToken } from '@n8n/engine';
|
||||
import { OperationalError } from 'n8n-workflow';
|
||||
|
||||
import { EngineControlPlaneTransport } from './engine-control-plane-transport';
|
||||
import { STATUS_CALLBACK_PATH } from './engine-v2.constants';
|
||||
|
||||
/**
|
||||
|
|
@ -16,21 +14,8 @@ import { STATUS_CALLBACK_PATH } from './engine-v2.constants';
|
|||
export class EngineControlPlaneClient {
|
||||
private readonly http: HttpRequestClient;
|
||||
|
||||
constructor(
|
||||
private readonly engineConfig: EngineConfig,
|
||||
outboundHttp: OutboundHttp,
|
||||
) {
|
||||
this.http = outboundHttp.requests({
|
||||
// Fixed, n8n-controlled host.
|
||||
useDefaultSsrfPolicy: 'unsafe',
|
||||
// A bind address is not dialable, so default to loopback.
|
||||
baseURL:
|
||||
engineConfig.controlPlaneBaseUrl || `http://127.0.0.1:${engineConfig.controlPlanePort}`,
|
||||
// A factory: each request needs a fresh token, and the secret is set later.
|
||||
headers: () => ({
|
||||
authorization: `Bearer ${mintActionToken(this.engineConfig.authSecret, 'lifecycle-events:write')}`,
|
||||
}),
|
||||
});
|
||||
constructor(transport: EngineControlPlaneTransport) {
|
||||
this.http = transport.forScope('lifecycle-events:write');
|
||||
}
|
||||
|
||||
/** Throws on a refused batch; the engine decides what a failed delivery costs. */
|
||||
|
|
|
|||
|
|
@ -9,7 +9,7 @@ import { bodyParser, rawBodyReader } from '@/middlewares';
|
|||
import { send } from '@/response-helper';
|
||||
|
||||
import { createEngineControlPlaneAuthMiddleware } from './engine-control-plane-auth.middleware';
|
||||
import { CONTROL_PLANE_PREFIX, STATUS_CALLBACK_PATH } from './engine-v2.constants';
|
||||
import { STATUS_CALLBACK_PATH } from './engine-v2.constants';
|
||||
import { EngineLifecycleEventController } from './engine-lifecycle-event.controller';
|
||||
|
||||
/**
|
||||
|
|
@ -86,16 +86,18 @@ export class EngineControlPlaneServer {
|
|||
res.status(200).json({ status: 'ok' });
|
||||
});
|
||||
|
||||
// On the prefix, not the route, so a later route cannot forget either.
|
||||
app.use(
|
||||
CONTROL_PLANE_PREFIX,
|
||||
createEngineControlPlaneAuthMiddleware(this.engineConfig, this.logger),
|
||||
);
|
||||
// Auth is per route, because each route requires its own token scope. It
|
||||
// runs before the body parser, so an unauthenticated body is never read.
|
||||
// n8n's parser bounds the body by `N8N_PAYLOAD_SIZE_MAX`.
|
||||
app.use(CONTROL_PLANE_PREFIX, rawBodyReader, bodyParser);
|
||||
|
||||
app.post(
|
||||
STATUS_CALLBACK_PATH,
|
||||
createEngineControlPlaneAuthMiddleware(
|
||||
this.engineConfig,
|
||||
this.logger,
|
||||
'lifecycle-events:write',
|
||||
),
|
||||
rawBodyReader,
|
||||
bodyParser,
|
||||
send(
|
||||
async (req, res) => await this.lifecycleEventController.receiveLifecycleEvents(req, res),
|
||||
),
|
||||
|
|
|
|||
|
|
@ -0,0 +1,37 @@
|
|||
import type { HttpRequestClient } from '@n8n/backend-network';
|
||||
import { OutboundHttp } from '@n8n/backend-network';
|
||||
import { EngineConfig } from '@n8n/config';
|
||||
import { Service } from '@n8n/di';
|
||||
import type { ActionScope } from '@n8n/engine';
|
||||
import { mintActionToken } from '@n8n/engine';
|
||||
|
||||
/**
|
||||
* Builds the HTTP clients that call the control plane server.
|
||||
*
|
||||
* Every caller needs the same base URL, SSRF policy and token minting, and
|
||||
* differs only in the scope of the token. Configuring this once keeps the
|
||||
* callers identical in everything but their scope.
|
||||
*/
|
||||
@Service()
|
||||
export class EngineControlPlaneTransport {
|
||||
constructor(
|
||||
private readonly engineConfig: EngineConfig,
|
||||
private readonly outboundHttp: OutboundHttp,
|
||||
) {}
|
||||
|
||||
/** Returns a client that sends a fresh action token with `scope` on every request. */
|
||||
forScope(scope: ActionScope): HttpRequestClient {
|
||||
return this.outboundHttp.requests({
|
||||
// Fixed, n8n-controlled host.
|
||||
useDefaultSsrfPolicy: 'unsafe',
|
||||
// A bind address is not dialable, so default to loopback.
|
||||
baseURL:
|
||||
this.engineConfig.controlPlaneBaseUrl ||
|
||||
`http://127.0.0.1:${this.engineConfig.controlPlanePort}`,
|
||||
// A factory: each request needs a fresh token, and the secret is set later.
|
||||
headers: () => ({
|
||||
authorization: `Bearer ${mintActionToken(this.engineConfig.authSecret, scope)}`,
|
||||
}),
|
||||
});
|
||||
}
|
||||
}
|
||||
Loading…
Reference in New Issue
Block a user