mirror of
https://github.com/n8n-io/n8n.git
synced 2026-10-17 05:29:44 +02:00
feat(core): Add an extends selector to credential type policies (no-changelog) (#40305)
Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Guillaume Jacquart
GitHub
parent
d1422fb652
commit
b33b0db040
|
|
@ -354,12 +354,15 @@ export {
|
|||
export { PaginationDto, MAX_ITEMS_PER_PAGE } from './pagination/pagination.dto';
|
||||
|
||||
export type {
|
||||
NodeTypePolicyRule,
|
||||
NodeTypePolicySelector,
|
||||
NonDelegatingPolicyAction,
|
||||
NonDelegatingPolicyRule,
|
||||
PolicyAction,
|
||||
PolicyRule,
|
||||
PolicySelector,
|
||||
} from './type-availability-policies/policy-rule.types';
|
||||
export { isNodeTypePolicyRule } from './type-availability-policies/policy-rule.types';
|
||||
export {
|
||||
credentialTypePolicySelectorSchema,
|
||||
nodeTypePolicySelectorSchema,
|
||||
|
|
|
|||
|
|
@ -112,20 +112,29 @@ describe('nonDelegatingPolicyRuleListSchema', () => {
|
|||
});
|
||||
|
||||
describe('rule schemas per kind', () => {
|
||||
const packageRule = {
|
||||
const extendsRule = {
|
||||
id: 'r1',
|
||||
action: 'deny',
|
||||
selector: { kind: 'package', value: 'n8n-nodes-base' },
|
||||
selector: { kind: 'extends', value: 'oAuth2Api' },
|
||||
};
|
||||
|
||||
it('accepts the same selectors for both kinds', () => {
|
||||
expect(nodeTypePolicyRuleSchemas.ruleList.safeParse([packageRule]).success).toBe(true);
|
||||
expect(credentialTypePolicyRuleSchemas.ruleList.safeParse([packageRule]).success).toBe(true);
|
||||
it('rejects an extends rule in a node type policy', () => {
|
||||
expect(nodeTypePolicyRuleSchemas.ruleList.safeParse([extendsRule]).success).toBe(false);
|
||||
expect(nodeTypePolicyRuleSchemas.nonDelegatingRuleList.safeParse([extendsRule]).success).toBe(
|
||||
false,
|
||||
);
|
||||
});
|
||||
|
||||
it('accepts an extends rule in a credential type policy', () => {
|
||||
expect(credentialTypePolicyRuleSchemas.ruleList.safeParse([extendsRule]).success).toBe(true);
|
||||
expect(
|
||||
credentialTypePolicyRuleSchemas.nonDelegatingRuleList.safeParse([extendsRule]).success,
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it('rejects duplicate rule ids in a credential type policy', () => {
|
||||
expect(
|
||||
credentialTypePolicyRuleSchemas.ruleList.safeParse([packageRule, packageRule]).success,
|
||||
credentialTypePolicyRuleSchemas.ruleList.safeParse([extendsRule, extendsRule]).success,
|
||||
).toBe(false);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -0,0 +1,19 @@
|
|||
import { isNodeTypePolicyRule } from '../policy-rule.types';
|
||||
|
||||
describe('isNodeTypePolicyRule', () => {
|
||||
it.each(['name', 'package'] as const)('accepts a %s rule', (kind) => {
|
||||
expect(isNodeTypePolicyRule({ id: 'r1', action: 'deny', selector: { kind, value: 'x' } })).toBe(
|
||||
true,
|
||||
);
|
||||
});
|
||||
|
||||
it('rejects an extends rule, which only credential type policies accept', () => {
|
||||
expect(
|
||||
isNodeTypePolicyRule({
|
||||
id: 'r1',
|
||||
action: 'deny',
|
||||
selector: { kind: 'extends', value: 'oAuth2Api' },
|
||||
}),
|
||||
).toBe(false);
|
||||
});
|
||||
});
|
||||
|
|
@ -27,3 +27,15 @@ describe.each([
|
|||
expect(schema.safeParse({ kind: 'name' }).success).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('extends selector', () => {
|
||||
const selector = { kind: 'extends', value: 'oAuth2Api' };
|
||||
|
||||
it('is rejected by the node type schema', () => {
|
||||
expect(nodeTypePolicySelectorSchema.safeParse(selector).success).toBe(false);
|
||||
});
|
||||
|
||||
it('is accepted by the credential type schema', () => {
|
||||
expect(credentialTypePolicySelectorSchema.safeParse(selector).success).toBe(true);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -1,6 +1,7 @@
|
|||
import { z } from 'zod';
|
||||
|
||||
import type {
|
||||
NodeTypePolicyRule,
|
||||
NonDelegatingPolicyAction,
|
||||
NonDelegatingPolicyRule,
|
||||
PolicyAction,
|
||||
|
|
@ -70,7 +71,10 @@ type RuleSchemas<Rule, NonDelegatingRule> = {
|
|||
|
||||
export const nodeTypePolicyRuleSchemas = ruleSchemasFor(
|
||||
nodeTypePolicySelectorSchema,
|
||||
) satisfies RuleSchemas<PolicyRule, NonDelegatingPolicyRule>;
|
||||
) satisfies RuleSchemas<
|
||||
NodeTypePolicyRule,
|
||||
NodeTypePolicyRule & { readonly action: NonDelegatingPolicyAction }
|
||||
>;
|
||||
|
||||
export const credentialTypePolicyRuleSchemas = ruleSchemasFor(
|
||||
credentialTypePolicySelectorSchema,
|
||||
|
|
|
|||
|
|
@ -1,12 +1,19 @@
|
|||
/**
|
||||
* Domain types for node type availability policies, shared by the internal and public API
|
||||
* Domain types for type availability policies, shared by the internal and public API
|
||||
* request DTOs and by the backend evaluator.
|
||||
*/
|
||||
|
||||
/** Matches a node type by its exact full name, or by its package segment. */
|
||||
/**
|
||||
* Matches a type by its exact full name, or by its package segment. `extends` is for credential
|
||||
* types only: it matches the named type and every type built on it through `extends`.
|
||||
*/
|
||||
export type PolicySelector =
|
||||
| { readonly kind: 'name'; readonly value: string }
|
||||
| { readonly kind: 'package'; readonly value: string };
|
||||
| { readonly kind: 'package'; readonly value: string }
|
||||
| { readonly kind: 'extends'; readonly value: string };
|
||||
|
||||
/** The selectors a node type policy accepts. */
|
||||
export type NodeTypePolicySelector = Exclude<PolicySelector, { readonly kind: 'extends' }>;
|
||||
|
||||
export type PolicyAction = 'allow' | 'deny' | 'delegate';
|
||||
|
||||
|
|
@ -17,6 +24,15 @@ export type PolicyRule = {
|
|||
readonly selector: PolicySelector;
|
||||
};
|
||||
|
||||
/** A `PolicyRule` as a node type policy accepts it. */
|
||||
export type NodeTypePolicyRule = Omit<PolicyRule, 'selector'> & {
|
||||
readonly selector: NodeTypePolicySelector;
|
||||
};
|
||||
|
||||
export function isNodeTypePolicyRule(rule: PolicyRule): rule is NodeTypePolicyRule {
|
||||
return rule.selector.kind !== 'extends';
|
||||
}
|
||||
|
||||
/**
|
||||
* The actions a project scope accepts. `delegate` defers to a narrower scope, and none exists
|
||||
* below project, so project-scope writes never accept it.
|
||||
|
|
|
|||
|
|
@ -1,18 +1,22 @@
|
|||
import { z } from 'zod';
|
||||
|
||||
import type { PolicySelector } from './policy-rule.types';
|
||||
import type { NodeTypePolicySelector, PolicySelector } from './policy-rule.types';
|
||||
|
||||
const nameSelectorSchema = z.object({ kind: z.literal('name'), value: z.string().min(1) });
|
||||
const packageSelectorSchema = z.object({ kind: z.literal('package'), value: z.string().min(1) });
|
||||
const extendsSelectorSchema = z
|
||||
.object({ kind: z.literal('extends'), value: z.string().min(1) })
|
||||
.describe('Matches the named credential type and every type that extends it');
|
||||
|
||||
/** The selectors a node type policy accepts. `satisfies` keeps it honest against the type. */
|
||||
export const nodeTypePolicySelectorSchema = z.discriminatedUnion('kind', [
|
||||
nameSelectorSchema,
|
||||
packageSelectorSchema,
|
||||
]) satisfies z.ZodType<PolicySelector>;
|
||||
]) satisfies z.ZodType<NodeTypePolicySelector>;
|
||||
|
||||
/** The selectors a credential type policy accepts. Each kind owns its list. */
|
||||
/** The selectors a credential type policy accepts: the node type ones plus `extends`. */
|
||||
export const credentialTypePolicySelectorSchema = z.discriminatedUnion('kind', [
|
||||
nameSelectorSchema,
|
||||
packageSelectorSchema,
|
||||
extendsSelectorSchema,
|
||||
]) satisfies z.ZodType<PolicySelector>;
|
||||
|
|
|
|||
|
|
@ -65,6 +65,9 @@ export const TYPE_AVAILABILITY_POLICIES_TELEMETRY = defineTelemetryEvents({
|
|||
...ruleCounts,
|
||||
name_selector_count: z.number().describe('Rules that target one exact type name'),
|
||||
package_selector_count: z.number().describe('Rules that target a whole package'),
|
||||
extends_selector_count: z
|
||||
.number()
|
||||
.describe('Rules that target a credential type and every type built on it'),
|
||||
evaluated_type_count: z
|
||||
.number()
|
||||
.describe(
|
||||
|
|
|
|||
|
|
@ -802,6 +802,7 @@ describe('TelemetryEventRelay', () => {
|
|||
delegate_rule_count: 1,
|
||||
name_selector_count: 2,
|
||||
package_selector_count: 1,
|
||||
extends_selector_count: 0,
|
||||
evaluated_type_count: 3,
|
||||
blocked_type_count: 2,
|
||||
allowed_type_count: 1,
|
||||
|
|
@ -849,6 +850,41 @@ describe('TelemetryEventRelay', () => {
|
|||
);
|
||||
});
|
||||
|
||||
it('should count a credential type built on the base an extends rule names as blocked', () => {
|
||||
Object.defineProperty(loadNodesAndCredentials, 'knownCredentials', {
|
||||
configurable: true,
|
||||
value: {
|
||||
oAuth2Api: {},
|
||||
googleOAuth2Api: { extends: ['oAuth2Api'] },
|
||||
googleSheetsOAuth2Api: { extends: ['googleOAuth2Api'] },
|
||||
slackApi: {},
|
||||
},
|
||||
});
|
||||
|
||||
eventService.emit('node-type-policy-saved', {
|
||||
updatedBy: 'user123',
|
||||
kind: 'credential-types',
|
||||
projectId: null,
|
||||
scopeId: 'scope-1',
|
||||
before: null,
|
||||
after: { defaultAction: 'allow', version: 1 },
|
||||
rulesBefore: null,
|
||||
rulesAfter: [
|
||||
{ id: 'rule-1', action: 'deny', selector: { kind: 'extends', value: 'oAuth2Api' } },
|
||||
],
|
||||
warningCount: 0,
|
||||
});
|
||||
|
||||
expect(telemetry.track).toHaveBeenCalledWith(
|
||||
TELEMETRY_EVENT.TYPE_AVAILABILITY_POLICIES.USER_SAVED_TYPE_AVAILABILITY_POLICY,
|
||||
expect.objectContaining({
|
||||
extends_selector_count: 1,
|
||||
blocked_types: ['oAuth2Api', 'googleOAuth2Api', 'googleSheetsOAuth2Api'],
|
||||
allowed_types: ['slackApi'],
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('should expand a credential type package rule using the credential loader package, not a dotted-type prefix', () => {
|
||||
const credentialPackageDenyRule: PolicyRule = {
|
||||
id: 'rule-1',
|
||||
|
|
|
|||
|
|
@ -160,6 +160,7 @@ function countSelectorKinds(rules: readonly PolicyRule[]) {
|
|||
return {
|
||||
name_selector_count: rules.filter((rule) => rule.selector.kind === 'name').length,
|
||||
package_selector_count: rules.filter((rule) => rule.selector.kind === 'package').length,
|
||||
extends_selector_count: rules.filter((rule) => rule.selector.kind === 'extends').length,
|
||||
};
|
||||
}
|
||||
|
||||
|
|
@ -586,7 +587,7 @@ export class TelemetryEventRelay extends EventRelay {
|
|||
kind === CREDENTIAL_TYPES_KIND
|
||||
? Object.keys(this.loadNodesAndCredentials.knownCredentials)
|
||||
: Object.keys(this.nodeTypes.getKnownTypes());
|
||||
const types = typeNames.map(policedTypeFor(kind, this.nodeTypes));
|
||||
const types = typeNames.map(policedTypeFor(kind, this.nodeTypes, this.loadNodesAndCredentials));
|
||||
const resolvePackage = packageResolverFor(kind, this.loadNodesAndCredentials);
|
||||
|
||||
this.telemetry.track(
|
||||
|
|
|
|||
|
|
@ -206,6 +206,14 @@ lint warns about it. The verdict names the variant the user placed and the rule
|
|||
Grandfathering compares literal type names, so adding `gmailTool` to a workflow that already
|
||||
stores `gmail` is a new type and is judged.
|
||||
|
||||
A credential type policy has a third selector, `extends`. It matches the named type and every
|
||||
type built on it through the credential's `extends` list, at any depth: `extends oAuth2Api`
|
||||
covers `googleOAuth2Api` and `googleSheetsOAuth2Api`. A `name` rule stays exact, so
|
||||
`name oAuth2Api` blocks only the generic OAuth2 credential. Rule order decides as usual: an
|
||||
`allow name googleSheetsOAuth2Api` placed before `deny extends oAuth2Api` keeps Sheets usable,
|
||||
and placed after it, the shadow lint warns that it can never match. A write rejects an `extends`
|
||||
rule in a node type policy, and one that names a credential type that is not installed.
|
||||
|
||||
### Reading it on the execution path
|
||||
|
||||
`workflowStart` runs for every execution and every sub-execution, under a 250 ms deadline it
|
||||
|
|
|
|||
|
|
@ -1,7 +1,13 @@
|
|||
import type { LoadNodesAndCredentials } from '@/load-nodes-and-credentials';
|
||||
import type { NodeTypes } from '@/node-types';
|
||||
|
||||
import { CREDENTIAL_TYPES_KIND, NODE_TYPES_KIND } from '../constants';
|
||||
import { isPackageInstalled, packageResolverFor } from '../package-resolver';
|
||||
import {
|
||||
isCredentialTypeKnown,
|
||||
isPackageInstalled,
|
||||
packageResolverFor,
|
||||
policedTypeFor,
|
||||
} from '../package-resolver';
|
||||
|
||||
/** A minimal stand-in for `LoadNodesAndCredentials`, built from just its public `loaders`. */
|
||||
function makeRegistry(loaders: LoadNodesAndCredentials['loaders']): LoadNodesAndCredentials {
|
||||
|
|
@ -89,3 +95,76 @@ describe('isPackageInstalled', () => {
|
|||
expect(isPackageInstalled(makeRegistry({}), 'constructor')).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('policedTypeFor', () => {
|
||||
const withKnownCredentials = (
|
||||
knownCredentials: LoadNodesAndCredentials['knownCredentials'],
|
||||
): LoadNodesAndCredentials => ({ knownCredentials }) as LoadNodesAndCredentials;
|
||||
const nodeTypes = {} as NodeTypes;
|
||||
|
||||
it('lists every credential type a type is built on, nearest first', () => {
|
||||
const policedType = policedTypeFor(
|
||||
CREDENTIAL_TYPES_KIND,
|
||||
nodeTypes,
|
||||
withKnownCredentials({
|
||||
oAuth2Api: { className: '', sourcePath: '' },
|
||||
googleOAuth2Api: { className: '', sourcePath: '', extends: ['oAuth2Api'] },
|
||||
googleSheetsOAuth2Api: { className: '', sourcePath: '', extends: ['googleOAuth2Api'] },
|
||||
}),
|
||||
);
|
||||
|
||||
expect(policedType('googleSheetsOAuth2Api')).toEqual({
|
||||
name: 'googleSheetsOAuth2Api',
|
||||
baseName: 'googleSheetsOAuth2Api',
|
||||
ancestors: ['googleOAuth2Api', 'oAuth2Api'],
|
||||
});
|
||||
});
|
||||
|
||||
it('lists a shared base once when two parents build on it', () => {
|
||||
const policedType = policedTypeFor(
|
||||
CREDENTIAL_TYPES_KIND,
|
||||
nodeTypes,
|
||||
withKnownCredentials({
|
||||
left: { className: '', sourcePath: '', extends: ['root'] },
|
||||
right: { className: '', sourcePath: '', extends: ['root'] },
|
||||
both: { className: '', sourcePath: '', extends: ['left', 'right'] },
|
||||
}),
|
||||
);
|
||||
|
||||
expect(policedType('both').ancestors).toEqual(['left', 'right', 'root']);
|
||||
});
|
||||
|
||||
it('stops on a cycle instead of looping', () => {
|
||||
const policedType = policedTypeFor(
|
||||
CREDENTIAL_TYPES_KIND,
|
||||
nodeTypes,
|
||||
withKnownCredentials({
|
||||
aApi: { className: '', sourcePath: '', extends: ['bApi'] },
|
||||
bApi: { className: '', sourcePath: '', extends: ['aApi'] },
|
||||
}),
|
||||
);
|
||||
|
||||
expect(policedType('aApi').ancestors).toEqual(['bApi']);
|
||||
});
|
||||
|
||||
it('gives an unknown credential type no ancestors', () => {
|
||||
const policedType = policedTypeFor(CREDENTIAL_TYPES_KIND, nodeTypes, withKnownCredentials({}));
|
||||
|
||||
expect(policedType('toString').ancestors).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('isCredentialTypeKnown', () => {
|
||||
const registry = {
|
||||
knownCredentials: { slackApi: { className: '', sourcePath: '' } },
|
||||
} as unknown as LoadNodesAndCredentials;
|
||||
|
||||
it('is true for a known credential type', () => {
|
||||
expect(isCredentialTypeKnown(registry, 'slackApi')).toBe(true);
|
||||
});
|
||||
|
||||
it('is false for an unknown type, or one named after an Object.prototype property', () => {
|
||||
expect(isCredentialTypeKnown(registry, 'unknownApi')).toBe(false);
|
||||
expect(isCredentialTypeKnown(registry, 'constructor')).toBe(false);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -253,6 +253,118 @@ describe('evaluateType', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('extends selector', () => {
|
||||
const SHEETS: PolicedType = {
|
||||
name: 'googleSheetsOAuth2Api',
|
||||
baseName: 'googleSheetsOAuth2Api',
|
||||
ancestors: ['googleOAuth2Api', 'oAuth2Api'],
|
||||
};
|
||||
const GENERIC: PolicedType = { name: 'oAuth2Api', baseName: 'oAuth2Api', ancestors: [] };
|
||||
const denyOAuth2Family: PolicyRule = {
|
||||
id: 'deny-oauth2-family',
|
||||
action: 'deny',
|
||||
selector: { kind: 'extends', value: 'oAuth2Api' },
|
||||
};
|
||||
const allowSheets: PolicyRule = {
|
||||
id: 'allow-sheets',
|
||||
action: 'allow',
|
||||
selector: { kind: 'name', value: 'googleSheetsOAuth2Api' },
|
||||
};
|
||||
|
||||
it('matches a type built on the named base, through every step of the chain', () => {
|
||||
const attachments = [attachment({ rules: [denyOAuth2Family] })];
|
||||
|
||||
expect(evaluateType(attachments, 'allow', SHEETS)).toEqual({
|
||||
action: 'deny',
|
||||
matchedRuleId: 'deny-oauth2-family',
|
||||
});
|
||||
});
|
||||
|
||||
it('matches the named base itself', () => {
|
||||
const attachments = [attachment({ rules: [denyOAuth2Family] })];
|
||||
|
||||
expect(evaluateType(attachments, 'allow', GENERIC).action).toBe('deny');
|
||||
});
|
||||
|
||||
it('does not match a type outside the family', () => {
|
||||
const attachments = [attachment({ rules: [denyOAuth2Family] })];
|
||||
|
||||
expect(evaluateType(attachments, 'allow', type('slackApi'))).toEqual({
|
||||
action: 'allow',
|
||||
matchedRuleId: null,
|
||||
});
|
||||
});
|
||||
|
||||
it('leaves a name rule on the base exact, so derived types stay allowed', () => {
|
||||
const attachments = [
|
||||
attachment({
|
||||
rules: [
|
||||
{ id: 'deny-generic', action: 'deny', selector: { kind: 'name', value: 'oAuth2Api' } },
|
||||
],
|
||||
}),
|
||||
];
|
||||
|
||||
expect(evaluateType(attachments, 'allow', SHEETS).action).toBe('allow');
|
||||
expect(evaluateType(attachments, 'allow', GENERIC).action).toBe('deny');
|
||||
});
|
||||
|
||||
it('lets rule order decide between a derived-type allow and a family deny', () => {
|
||||
const allowFirst = [attachment({ rules: [allowSheets, denyOAuth2Family] })];
|
||||
const denyFirst = [attachment({ rules: [denyOAuth2Family, allowSheets] })];
|
||||
|
||||
expect(evaluateType(allowFirst, 'allow', SHEETS)).toEqual({
|
||||
action: 'allow',
|
||||
matchedRuleId: 'allow-sheets',
|
||||
});
|
||||
expect(evaluateType(denyFirst, 'allow', SHEETS)).toEqual({
|
||||
action: 'deny',
|
||||
matchedRuleId: 'deny-oauth2-family',
|
||||
});
|
||||
});
|
||||
|
||||
it('keeps an instance family deny over a project allow of the derived type', () => {
|
||||
const verdict = evaluateComposedType(
|
||||
scopePolicy({ attachments: [attachment({ rules: [denyOAuth2Family] })] }),
|
||||
scopePolicy({ attachments: [attachment({ rules: [allowSheets] })] }),
|
||||
SHEETS,
|
||||
);
|
||||
|
||||
expect(verdict).toEqual({
|
||||
action: 'deny',
|
||||
scope: 'instance',
|
||||
matchedRuleId: 'deny-oauth2-family',
|
||||
optInAvailable: false,
|
||||
});
|
||||
});
|
||||
|
||||
it('satisfies an instance delegate on a derived type with a project family allow', () => {
|
||||
const verdict = evaluateComposedType(
|
||||
scopePolicy({ defaultAction: 'delegate' }),
|
||||
scopePolicy({
|
||||
attachments: [
|
||||
attachment({
|
||||
rules: [
|
||||
{
|
||||
id: 'allow-family',
|
||||
action: 'allow',
|
||||
selector: { kind: 'extends', value: 'oAuth2Api' },
|
||||
},
|
||||
],
|
||||
}),
|
||||
],
|
||||
}),
|
||||
SHEETS,
|
||||
);
|
||||
|
||||
expect(verdict).toEqual({
|
||||
action: 'allow',
|
||||
scope: 'project',
|
||||
matchedRuleId: 'allow-family',
|
||||
optInAvailable: false,
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('evaluateComposedType', () => {
|
||||
const TYPE = 'n8n-nodes-base.slack';
|
||||
const allowRule = (id: string) =>
|
||||
|
|
|
|||
|
|
@ -177,6 +177,57 @@ describe('lintRulesForShadowing', () => {
|
|||
]);
|
||||
});
|
||||
|
||||
describe('extends selector', () => {
|
||||
/** `googleSheetsOAuth2Api` is built on `googleOAuth2Api`, which is built on `oAuth2Api`. */
|
||||
const credentialType = (name: string) => ({
|
||||
name,
|
||||
baseName: name,
|
||||
ancestors:
|
||||
{
|
||||
googleSheetsOAuth2Api: ['googleOAuth2Api', 'oAuth2Api'],
|
||||
googleOAuth2Api: ['oAuth2Api'],
|
||||
}[name] ?? [],
|
||||
});
|
||||
const lint = (rules: PolicyRule[]) => lintRulesForShadowing(rules, () => null, credentialType);
|
||||
const family = (id: string, value: string) =>
|
||||
rule({ id, selector: { kind: 'extends', value } });
|
||||
const name = (id: string, value: string) => rule({ id, selector: { kind: 'name', value } });
|
||||
|
||||
it('flags a later name rule for a type built on an earlier family rule', () => {
|
||||
expect(
|
||||
lint([family('oauth2', 'oAuth2Api'), name('sheets', 'googleSheetsOAuth2Api')]),
|
||||
).toEqual([{ ruleId: 'sheets', shadowedByRuleId: 'oauth2' }]);
|
||||
});
|
||||
|
||||
it('flags a later name rule for the family base itself', () => {
|
||||
expect(lint([family('oauth2', 'oAuth2Api'), name('generic', 'oAuth2Api')])).toEqual([
|
||||
{ ruleId: 'generic', shadowedByRuleId: 'oauth2' },
|
||||
]);
|
||||
});
|
||||
|
||||
it('flags a later family rule nested inside an earlier one', () => {
|
||||
expect(lint([family('oauth2', 'oAuth2Api'), family('google', 'googleOAuth2Api')])).toEqual([
|
||||
{ ruleId: 'google', shadowedByRuleId: 'oauth2' },
|
||||
]);
|
||||
});
|
||||
|
||||
it('does not flag a family rule after a name rule for its base, which matches one type only', () => {
|
||||
expect(lint([name('generic', 'oAuth2Api'), family('oauth2', 'oAuth2Api')])).toEqual([]);
|
||||
});
|
||||
|
||||
it('does not flag a wider family rule after a narrower one', () => {
|
||||
expect(lint([family('google', 'googleOAuth2Api'), family('oauth2', 'oAuth2Api')])).toEqual(
|
||||
[],
|
||||
);
|
||||
});
|
||||
|
||||
it('does not flag a derived-type allow placed before the family rule', () => {
|
||||
expect(
|
||||
lint([name('sheets', 'googleSheetsOAuth2Api'), family('oauth2', 'oAuth2Api')]),
|
||||
).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
it('returns no warnings for an empty or single-rule list', () => {
|
||||
expect(lintRulesForShadowing([])).toEqual([]);
|
||||
expect(
|
||||
|
|
|
|||
|
|
@ -332,6 +332,67 @@ describe('TypeAvailabilityPolicyService', () => {
|
|||
expect(policyRepository.createPolicy).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
describe('extends rules', () => {
|
||||
const familyRule = (id: string, value: string): PolicyRule => ({
|
||||
id,
|
||||
action: 'deny',
|
||||
selector: { kind: 'extends', value },
|
||||
});
|
||||
|
||||
beforeEach(() => {
|
||||
Object.defineProperty(loadNodesAndCredentials, 'knownCredentials', {
|
||||
configurable: true,
|
||||
value: {
|
||||
oAuth2Api: { className: '', sourcePath: '' },
|
||||
googleSheetsOAuth2Api: { className: '', sourcePath: '', extends: ['oAuth2Api'] },
|
||||
},
|
||||
});
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
delete (loadNodesAndCredentials as { knownCredentials?: unknown }).knownCredentials;
|
||||
});
|
||||
|
||||
it('rejects an extends rule in a node type policy', async () => {
|
||||
await expect(
|
||||
service.createPolicyDocument(KIND, [familyRule('r1', 'oAuth2Api')], 'user-1'),
|
||||
).rejects.toThrow('An "extends" rule is only valid in a credential type policy');
|
||||
expect(policyRepository.createPolicy).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('rejects an extends rule naming a credential type that is not installed', async () => {
|
||||
await expect(
|
||||
service.createPolicyDocument(
|
||||
CREDENTIAL_TYPES_KIND,
|
||||
[familyRule('r1', 'notInstalledApi')],
|
||||
'user-1',
|
||||
),
|
||||
).rejects.toThrow(
|
||||
'Extends rule names a credential type that is not installed: notInstalledApi',
|
||||
);
|
||||
expect(policyRepository.createPolicy).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('warns about a derived-type rule placed after the family rule that covers it', async () => {
|
||||
policyRepository.createPolicy.mockResolvedValue(
|
||||
makePolicy({ kind: CREDENTIAL_TYPES_KIND }),
|
||||
);
|
||||
const allowSheets: PolicyRule = {
|
||||
id: 'r2',
|
||||
action: 'allow',
|
||||
selector: { kind: 'name', value: 'googleSheetsOAuth2Api' },
|
||||
};
|
||||
|
||||
const { warnings } = await service.createPolicyDocument(
|
||||
CREDENTIAL_TYPES_KIND,
|
||||
[familyRule('r1', 'oAuth2Api'), allowSheets],
|
||||
'user-1',
|
||||
);
|
||||
|
||||
expect(warnings).toEqual([{ ruleId: 'r2', shadowedByRuleId: 'r1' }]);
|
||||
});
|
||||
});
|
||||
|
||||
it('accepts a package rule naming an installed package', async () => {
|
||||
policyRepository.createPolicy.mockResolvedValue(makePolicy());
|
||||
const rule: PolicyRule = {
|
||||
|
|
|
|||
|
|
@ -51,10 +51,54 @@ export function packageResolverFor(
|
|||
: nodeTypePackageResolver;
|
||||
}
|
||||
|
||||
export function policedTypeFor(kind: string, nodeTypes: NodeTypes): (name: string) => PolicedType {
|
||||
return kind === NODE_TYPES_KIND
|
||||
? (name) => ({ name, baseName: nodeTypes.resolveBaseName(name).baseName })
|
||||
: (name) => ({ name, baseName: name });
|
||||
/**
|
||||
* Every type `typeName` is built on, walking `extends` breadth-first. The visited set keeps a
|
||||
* community package that declares a cycle from looping forever.
|
||||
*/
|
||||
function credentialTypeAncestors(
|
||||
loadNodesAndCredentials: LoadNodesAndCredentials,
|
||||
typeName: string,
|
||||
): string[] {
|
||||
const { knownCredentials } = loadNodesAndCredentials;
|
||||
const parentsOf = (name: string) =>
|
||||
Object.hasOwn(knownCredentials, name) ? (knownCredentials[name].extends ?? []) : [];
|
||||
|
||||
const seen = new Set<string>([typeName]);
|
||||
const queue = [...parentsOf(typeName)];
|
||||
|
||||
for (let index = 0; index < queue.length; index++) {
|
||||
const parent = queue[index];
|
||||
if (seen.has(parent)) continue;
|
||||
seen.add(parent);
|
||||
queue.push(...parentsOf(parent));
|
||||
}
|
||||
|
||||
seen.delete(typeName);
|
||||
return [...seen];
|
||||
}
|
||||
|
||||
export function policedTypeFor(
|
||||
kind: string,
|
||||
nodeTypes: NodeTypes,
|
||||
loadNodesAndCredentials: LoadNodesAndCredentials,
|
||||
): (name: string) => PolicedType {
|
||||
if (kind === NODE_TYPES_KIND) {
|
||||
return (name) => ({ name, baseName: nodeTypes.resolveBaseName(name).baseName });
|
||||
}
|
||||
|
||||
return (name) => ({
|
||||
name,
|
||||
baseName: name,
|
||||
ancestors: credentialTypeAncestors(loadNodesAndCredentials, name),
|
||||
});
|
||||
}
|
||||
|
||||
/** Whether a credential type is known, checked at write time for an `extends` rule's value. */
|
||||
export function isCredentialTypeKnown(
|
||||
loadNodesAndCredentials: LoadNodesAndCredentials,
|
||||
typeName: string,
|
||||
): boolean {
|
||||
return Object.hasOwn(loadNodesAndCredentials.knownCredentials, typeName);
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
|||
|
|
@ -50,7 +50,12 @@ export type PackageResolver = (typeName: string) => string | null;
|
|||
*/
|
||||
export const nodeTypePackageResolver: PackageResolver = (typeName) => typeName.split('.')[0];
|
||||
|
||||
export type PolicedType = { readonly name: string; readonly baseName: string };
|
||||
/** `ancestors` lists what a credential type is built on through `extends`; empty for node types. */
|
||||
export type PolicedType = {
|
||||
readonly name: string;
|
||||
readonly baseName: string;
|
||||
readonly ancestors?: readonly string[];
|
||||
};
|
||||
|
||||
function selectorMatches(
|
||||
selector: PolicySelector,
|
||||
|
|
@ -62,6 +67,8 @@ function selectorMatches(
|
|||
return selector.value === type.name || selector.value === type.baseName;
|
||||
case 'package':
|
||||
return resolvePackage(type.name) === selector.value;
|
||||
case 'extends':
|
||||
return selector.value === type.name || (type.ancestors ?? []).includes(selector.value);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -3,7 +3,7 @@ import {
|
|||
type PackageResolver,
|
||||
type PolicedType,
|
||||
} from './policy-evaluator';
|
||||
import type { PolicyRule } from './policy-rule.types';
|
||||
import type { PolicyRule, PolicySelector } from './policy-rule.types';
|
||||
|
||||
/**
|
||||
* One case of an unreachable rule: `ruleId` can never match, because `shadowedByRuleId`
|
||||
|
|
@ -54,6 +54,9 @@ function resolvePackageOrUndefined(
|
|||
* package, so it also shadows a later name selector scoped to that same package. A name
|
||||
* selector never shadows a package selector: one type can't cover a whole package.
|
||||
*
|
||||
* An `extends` selector shadows a later `name` or `extends` selector whose type is the same one
|
||||
* or built on it. Nothing else shadows an `extends`: its family can span several packages.
|
||||
*
|
||||
* Runs in one O(n) pass: instead of comparing each rule against every rule before it, it
|
||||
* keeps, per selector kind, only the earliest rule seen so far for each selector value, and
|
||||
* looks that up once per rule.
|
||||
|
|
@ -73,29 +76,47 @@ export function lintRulesForShadowing(
|
|||
): ShadowWarning[] {
|
||||
const warnings: ShadowWarning[] = [];
|
||||
|
||||
const firstByName = new Map<string, FirstOccurrence>();
|
||||
const firstByPackage = new Map<string, FirstOccurrence>();
|
||||
const firstByKind: Record<PolicySelector['kind'], Map<string, FirstOccurrence>> = {
|
||||
name: new Map(),
|
||||
package: new Map(),
|
||||
extends: new Map(),
|
||||
};
|
||||
|
||||
/** The earliest `extends` rule naming the type itself or one it is built on. */
|
||||
const firstFamilyOccurrence = (type: PolicedType) =>
|
||||
[type.name, ...(type.ancestors ?? [])]
|
||||
.map((name) => firstByKind.extends.get(name))
|
||||
.reduce(earlierOccurrence, undefined);
|
||||
|
||||
for (let index = 0; index < rules.length; index++) {
|
||||
const rule = rules[index];
|
||||
const { selector } = rule;
|
||||
|
||||
const shadowedBy =
|
||||
selector.kind === 'package'
|
||||
? firstByPackage.get(selector.value)
|
||||
: earlierOccurrence(
|
||||
earlierOccurrence(
|
||||
firstByName.get(selector.value),
|
||||
firstByName.get(policedType(selector.value).baseName),
|
||||
),
|
||||
resolvePackageOrUndefined(firstByPackage, resolvePackage, selector.value),
|
||||
);
|
||||
let shadowedBy: FirstOccurrence | undefined;
|
||||
switch (selector.kind) {
|
||||
case 'package':
|
||||
shadowedBy = firstByKind.package.get(selector.value);
|
||||
break;
|
||||
case 'extends':
|
||||
shadowedBy = firstFamilyOccurrence(policedType(selector.value));
|
||||
break;
|
||||
case 'name': {
|
||||
const type = policedType(selector.value);
|
||||
shadowedBy = [
|
||||
firstByKind.name.get(selector.value),
|
||||
firstByKind.name.get(type.baseName),
|
||||
resolvePackageOrUndefined(firstByKind.package, resolvePackage, selector.value),
|
||||
firstFamilyOccurrence(type),
|
||||
].reduce(earlierOccurrence, undefined);
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
if (shadowedBy) {
|
||||
warnings.push({ ruleId: rule.id, shadowedByRuleId: shadowedBy.rule.id });
|
||||
}
|
||||
|
||||
const firstOccurrenceByValue = selector.kind === 'package' ? firstByPackage : firstByName;
|
||||
const firstOccurrenceByValue = firstByKind[selector.kind];
|
||||
if (!firstOccurrenceByValue.has(selector.value)) {
|
||||
firstOccurrenceByValue.set(selector.value, { rule, index });
|
||||
}
|
||||
|
|
|
|||
|
|
@ -21,8 +21,13 @@ import { TypeAvailabilityPolicyScopeRepository } from './database/repositories/t
|
|||
import { TypeAvailabilityPolicyRepository } from './database/repositories/type-availability-policy.repository';
|
||||
import type { TypeAvailabilityPolicy } from './database/entities/type-availability-policy.entity';
|
||||
import type { TypeAvailabilityPolicyScope } from './database/entities/type-availability-policy-scope.entity';
|
||||
import { NODE_TYPES_KIND } from './constants';
|
||||
import { isPackageInstalled, packageResolverFor, policedTypeFor } from './package-resolver';
|
||||
import { CREDENTIAL_TYPES_KIND, NODE_TYPES_KIND } from './constants';
|
||||
import {
|
||||
isCredentialTypeKnown,
|
||||
isPackageInstalled,
|
||||
packageResolverFor,
|
||||
policedTypeFor,
|
||||
} from './package-resolver';
|
||||
import { evaluateComposedType, orderedAttachments, type ComposedVerdict } from './policy-evaluator';
|
||||
import type {
|
||||
PolicyAction,
|
||||
|
|
@ -267,6 +272,34 @@ function assertNoCredentialOnlyNodeRules(kind: string, rules: readonly PolicyRul
|
|||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Only credential types declare `extends`, so the selector means nothing for another kind. An
|
||||
* unknown base would match nothing, the same trap `assertPackagesInstalled` closes.
|
||||
*/
|
||||
function assertExtendsSelectorsValid(
|
||||
kind: string,
|
||||
rules: readonly PolicyRule[],
|
||||
loadNodesAndCredentials: LoadNodesAndCredentials,
|
||||
): void {
|
||||
const values = rules.flatMap((rule) =>
|
||||
rule.selector.kind === 'extends' ? [rule.selector.value] : [],
|
||||
);
|
||||
if (values.length === 0) return;
|
||||
|
||||
if (kind !== CREDENTIAL_TYPES_KIND) {
|
||||
throw new UserError('An "extends" rule is only valid in a credential type policy');
|
||||
}
|
||||
|
||||
const unknown = new Set(
|
||||
values.filter((value) => !isCredentialTypeKnown(loadNodesAndCredentials, value)),
|
||||
);
|
||||
if (unknown.size > 0) {
|
||||
throw new UserError(
|
||||
`Extends rule names a credential type that is not installed: ${[...unknown].join(', ')}`,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/** Mirrors the DTO-level check in `ReplaceAttachmentsDto`, as a defensive service-level guard. */
|
||||
function assertNoDuplicateAttachmentSlots(attachments: readonly AttachmentInput[]): void {
|
||||
const seenPolicyIds = new Set<string>();
|
||||
|
|
@ -464,10 +497,11 @@ export class TypeAvailabilityPolicyService {
|
|||
): Promise<PolicyDocumentWrite> {
|
||||
assertPackagesInstalled(rules, this.loadNodesAndCredentials);
|
||||
assertNoCredentialOnlyNodeRules(kind, rules);
|
||||
assertExtendsSelectorsValid(kind, rules, this.loadNodesAndCredentials);
|
||||
const warnings = lintRulesForShadowing(
|
||||
rules,
|
||||
packageResolverFor(kind, this.loadNodesAndCredentials),
|
||||
policedTypeFor(kind, this.nodeTypes),
|
||||
policedTypeFor(kind, this.nodeTypes, this.loadNodesAndCredentials),
|
||||
);
|
||||
|
||||
const policy = await this.policyRepository.createPolicy({ kind, rules, updatedBy }, {});
|
||||
|
|
@ -503,10 +537,11 @@ export class TypeAvailabilityPolicyService {
|
|||
): Promise<PolicyDocumentWrite> {
|
||||
assertPackagesInstalled(rules, this.loadNodesAndCredentials);
|
||||
assertNoCredentialOnlyNodeRules(kind, rules);
|
||||
assertExtendsSelectorsValid(kind, rules, this.loadNodesAndCredentials);
|
||||
const warnings = lintRulesForShadowing(
|
||||
rules,
|
||||
packageResolverFor(kind, this.loadNodesAndCredentials),
|
||||
policedTypeFor(kind, this.nodeTypes),
|
||||
policedTypeFor(kind, this.nodeTypes, this.loadNodesAndCredentials),
|
||||
);
|
||||
|
||||
const result = await this.transactionRunner.run({}, async (ctx) => {
|
||||
|
|
@ -758,11 +793,12 @@ export class TypeAvailabilityPolicyService {
|
|||
assertNoDelegateAtProjectScope(projectId, input.defaultAction, input.rules);
|
||||
assertPackagesInstalled(input.rules, this.loadNodesAndCredentials);
|
||||
assertNoCredentialOnlyNodeRules(kind, input.rules);
|
||||
assertExtendsSelectorsValid(kind, input.rules, this.loadNodesAndCredentials);
|
||||
|
||||
const warnings = lintRulesForShadowing(
|
||||
input.rules,
|
||||
packageResolverFor(kind, this.loadNodesAndCredentials),
|
||||
policedTypeFor(kind, this.nodeTypes),
|
||||
policedTypeFor(kind, this.nodeTypes, this.loadNodesAndCredentials),
|
||||
);
|
||||
|
||||
const result = await this.transactionRunner.run({}, async (ctx) => {
|
||||
|
|
@ -977,7 +1013,7 @@ export class TypeAvailabilityPolicyService {
|
|||
return evaluateComposedType(
|
||||
instance,
|
||||
project,
|
||||
policedTypeFor(kind, this.nodeTypes)(typeName),
|
||||
policedTypeFor(kind, this.nodeTypes, this.loadNodesAndCredentials)(typeName),
|
||||
packageResolverFor(kind, this.loadNodesAndCredentials),
|
||||
);
|
||||
}
|
||||
|
|
@ -1054,7 +1090,7 @@ export class TypeAvailabilityPolicyService {
|
|||
|
||||
private verdictComposer(kind: string, typeNames: readonly string[]) {
|
||||
const resolvePackage = packageResolverFor(kind, this.loadNodesAndCredentials);
|
||||
const types = typeNames.map(policedTypeFor(kind, this.nodeTypes));
|
||||
const types = typeNames.map(policedTypeFor(kind, this.nodeTypes, this.loadNodesAndCredentials));
|
||||
|
||||
return (instance: EffectivePolicy, project: EffectivePolicy): ComposedTypeVerdict[] =>
|
||||
types.map((type) => ({
|
||||
|
|
|
|||
|
|
@ -160,6 +160,17 @@ describe('NodeTypePoliciesPublicController handler bodies', () => {
|
|||
warnings: [{ ruleId: 'r2', shadowedByRuleId: 'r1' }],
|
||||
};
|
||||
|
||||
it('fails loudly instead of returning an extends rule, which no node type write accepts', async () => {
|
||||
service.getEffectivePolicy.mockResolvedValue({
|
||||
...effectivePolicy,
|
||||
rules: [{ id: 'r1', action: 'deny', selector: { kind: 'extends', value: 'oAuth2Api' } }],
|
||||
});
|
||||
|
||||
await expect(controller.getNodeTypeInstancePolicy()).rejects.toThrow(
|
||||
'A node type policy holds an extends rule',
|
||||
);
|
||||
});
|
||||
|
||||
it('getNodeTypeInstancePolicy reads the null-project scope and maps the response', async () => {
|
||||
service.getEffectivePolicy.mockResolvedValue(effectivePolicy);
|
||||
|
||||
|
|
|
|||
|
|
@ -11,7 +11,10 @@ import {
|
|||
PutProjectPolicyDto,
|
||||
ReplaceAttachmentsDto,
|
||||
UpdatePolicyDocumentDto,
|
||||
isNodeTypePolicyRule,
|
||||
nodeTypePolicyIdParamSchema,
|
||||
type NodeTypePolicyRule,
|
||||
type PolicyRule,
|
||||
nodeTypePolicyScopeIdParamSchema,
|
||||
projectIdParamSchema,
|
||||
} from '@n8n/api-types';
|
||||
|
|
@ -39,6 +42,7 @@ import {
|
|||
} from '@n8n/decorators';
|
||||
import { Container } from '@n8n/di';
|
||||
import type { Response } from 'express';
|
||||
import { UnexpectedError } from 'n8n-workflow';
|
||||
|
||||
import { NotFoundError, ServiceUnavailableError } from '@n8n/errors';
|
||||
import { NODE_TYPES_KIND } from '@/modules/type-availability-policies/constants';
|
||||
|
|
@ -50,11 +54,19 @@ import {
|
|||
|
||||
const tags = ['NodeTypePolicy'];
|
||||
|
||||
/** Every write rejects an `extends` rule for this kind, so meeting one here is a bug. */
|
||||
function toNodeTypeRules(rules: readonly PolicyRule[]): NodeTypePolicyRule[] {
|
||||
if (!rules.every(isNodeTypePolicyRule)) {
|
||||
throw new UnexpectedError('A node type policy holds an extends rule');
|
||||
}
|
||||
return [...rules];
|
||||
}
|
||||
|
||||
function toPublicDocument(policy: TypeAvailabilityPolicy): PolicyDocumentPublicDto {
|
||||
return {
|
||||
id: policy.id,
|
||||
kind: policy.kind,
|
||||
rules: [...policy.rules],
|
||||
rules: toNodeTypeRules(policy.rules),
|
||||
version: policy.version,
|
||||
updatedBy: policy.updatedBy,
|
||||
createdAt: policy.createdAt.toISOString(),
|
||||
|
|
@ -98,7 +110,7 @@ export class NodeTypePoliciesPublicController {
|
|||
|
||||
return {
|
||||
scopeId: effective.scopeId,
|
||||
rules: [...effective.rules],
|
||||
rules: toNodeTypeRules(effective.rules),
|
||||
defaultAction: effective.defaultAction,
|
||||
version: effective.version,
|
||||
};
|
||||
|
|
@ -131,7 +143,7 @@ export class NodeTypePoliciesPublicController {
|
|||
|
||||
return {
|
||||
scopeId: result.scopeId,
|
||||
rules: result.rules,
|
||||
rules: toNodeTypeRules(result.rules),
|
||||
defaultAction: result.defaultAction,
|
||||
version: result.version,
|
||||
warnings: [...result.warnings],
|
||||
|
|
@ -158,7 +170,7 @@ export class NodeTypePoliciesPublicController {
|
|||
|
||||
return {
|
||||
scopeId: effective.scopeId,
|
||||
rules: [...effective.rules],
|
||||
rules: toNodeTypeRules(effective.rules),
|
||||
defaultAction: effective.defaultAction,
|
||||
version: effective.version,
|
||||
};
|
||||
|
|
@ -192,7 +204,7 @@ export class NodeTypePoliciesPublicController {
|
|||
|
||||
return {
|
||||
scopeId: result.scopeId,
|
||||
rules: result.rules,
|
||||
rules: toNodeTypeRules(result.rules),
|
||||
defaultAction: result.defaultAction,
|
||||
version: result.version,
|
||||
warnings: [...result.warnings],
|
||||
|
|
@ -353,7 +365,7 @@ export class NodeTypePoliciesPublicController {
|
|||
return {
|
||||
attachments: result.attachments.map((attachment) => ({
|
||||
policyId: attachment.policyId,
|
||||
rules: [...attachment.rules],
|
||||
rules: toNodeTypeRules(attachment.rules),
|
||||
priority: attachment.priority,
|
||||
isFloor: attachment.isFloor,
|
||||
})),
|
||||
|
|
|
|||
|
|
@ -54,6 +54,19 @@ requestBody:
|
|||
required:
|
||||
- kind
|
||||
- value
|
||||
- type: object
|
||||
properties:
|
||||
kind:
|
||||
type: string
|
||||
enum:
|
||||
- extends
|
||||
value:
|
||||
type: string
|
||||
minLength: 1
|
||||
required:
|
||||
- kind
|
||||
- value
|
||||
description: Matches the named credential type and every type that extends it
|
||||
type: object
|
||||
required:
|
||||
- id
|
||||
|
|
|
|||
|
|
@ -65,6 +65,19 @@ responses:
|
|||
required:
|
||||
- kind
|
||||
- value
|
||||
- type: object
|
||||
properties:
|
||||
kind:
|
||||
type: string
|
||||
enum:
|
||||
- extends
|
||||
value:
|
||||
type: string
|
||||
minLength: 1
|
||||
required:
|
||||
- kind
|
||||
- value
|
||||
description: Matches the named credential type and every type that extends it
|
||||
type: object
|
||||
required:
|
||||
- id
|
||||
|
|
|
|||
|
|
@ -67,6 +67,19 @@ responses:
|
|||
required:
|
||||
- kind
|
||||
- value
|
||||
- type: object
|
||||
properties:
|
||||
kind:
|
||||
type: string
|
||||
enum:
|
||||
- extends
|
||||
value:
|
||||
type: string
|
||||
minLength: 1
|
||||
required:
|
||||
- kind
|
||||
- value
|
||||
description: Matches the named credential type and every type that extends it
|
||||
type: object
|
||||
required:
|
||||
- id
|
||||
|
|
|
|||
|
|
@ -54,6 +54,19 @@ requestBody:
|
|||
required:
|
||||
- kind
|
||||
- value
|
||||
- type: object
|
||||
properties:
|
||||
kind:
|
||||
type: string
|
||||
enum:
|
||||
- extends
|
||||
value:
|
||||
type: string
|
||||
minLength: 1
|
||||
required:
|
||||
- kind
|
||||
- value
|
||||
description: Matches the named credential type and every type that extends it
|
||||
type: object
|
||||
required:
|
||||
- id
|
||||
|
|
|
|||
|
|
@ -60,6 +60,19 @@ requestBody:
|
|||
required:
|
||||
- kind
|
||||
- value
|
||||
- type: object
|
||||
properties:
|
||||
kind:
|
||||
type: string
|
||||
enum:
|
||||
- extends
|
||||
value:
|
||||
type: string
|
||||
minLength: 1
|
||||
required:
|
||||
- kind
|
||||
- value
|
||||
description: Matches the named credential type and every type that extends it
|
||||
type: object
|
||||
required:
|
||||
- id
|
||||
|
|
|
|||
|
|
@ -94,6 +94,19 @@ responses:
|
|||
required:
|
||||
- kind
|
||||
- value
|
||||
- type: object
|
||||
properties:
|
||||
kind:
|
||||
type: string
|
||||
enum:
|
||||
- extends
|
||||
value:
|
||||
type: string
|
||||
minLength: 1
|
||||
required:
|
||||
- kind
|
||||
- value
|
||||
description: Matches the named credential type and every type that extends it
|
||||
type: object
|
||||
required:
|
||||
- id
|
||||
|
|
|
|||
|
|
@ -61,6 +61,19 @@ requestBody:
|
|||
required:
|
||||
- kind
|
||||
- value
|
||||
- type: object
|
||||
properties:
|
||||
kind:
|
||||
type: string
|
||||
enum:
|
||||
- extends
|
||||
value:
|
||||
type: string
|
||||
minLength: 1
|
||||
required:
|
||||
- kind
|
||||
- value
|
||||
description: Matches the named credential type and every type that extends it
|
||||
type: object
|
||||
required:
|
||||
- id
|
||||
|
|
|
|||
|
|
@ -47,6 +47,19 @@ properties:
|
|||
required:
|
||||
- kind
|
||||
- value
|
||||
- type: object
|
||||
properties:
|
||||
kind:
|
||||
type: string
|
||||
enum:
|
||||
- extends
|
||||
value:
|
||||
type: string
|
||||
minLength: 1
|
||||
required:
|
||||
- kind
|
||||
- value
|
||||
description: Matches the named credential type and every type that extends it
|
||||
type: object
|
||||
required:
|
||||
- id
|
||||
|
|
|
|||
|
|
@ -43,6 +43,19 @@ properties:
|
|||
required:
|
||||
- kind
|
||||
- value
|
||||
- type: object
|
||||
properties:
|
||||
kind:
|
||||
type: string
|
||||
enum:
|
||||
- extends
|
||||
value:
|
||||
type: string
|
||||
minLength: 1
|
||||
required:
|
||||
- kind
|
||||
- value
|
||||
description: Matches the named credential type and every type that extends it
|
||||
type: object
|
||||
required:
|
||||
- id
|
||||
|
|
|
|||
|
|
@ -43,6 +43,19 @@ properties:
|
|||
required:
|
||||
- kind
|
||||
- value
|
||||
- type: object
|
||||
properties:
|
||||
kind:
|
||||
type: string
|
||||
enum:
|
||||
- extends
|
||||
value:
|
||||
type: string
|
||||
minLength: 1
|
||||
required:
|
||||
- kind
|
||||
- value
|
||||
description: Matches the named credential type and every type that extends it
|
||||
type: object
|
||||
required:
|
||||
- id
|
||||
|
|
|
|||
|
|
@ -0,0 +1,202 @@
|
|||
/**
|
||||
* Pins what an `extends` rule decides for real host paths: a rule on a base credential type
|
||||
* refuses a workflow that asks for a type built on it, exactly as a rule on that type would.
|
||||
*/
|
||||
import { createWorkflow, randomCredentialPayload, testDb } from '@n8n/backend-test-utils';
|
||||
import type { PolicyRule } from '@n8n/api-types';
|
||||
import { LICENSE_FEATURES } from '@n8n/constants';
|
||||
import { ExecutionRepository, WorkflowRepository, type User } from '@n8n/db';
|
||||
import { Container } from '@n8n/di';
|
||||
import { createRunExecutionData, type INode } from 'n8n-workflow';
|
||||
import { v4 as uuid } from 'uuid';
|
||||
|
||||
import { LoadNodesAndCredentials } from '@/load-nodes-and-credentials';
|
||||
import { WorkflowRunner } from '@/workflow-runner';
|
||||
|
||||
import { saveCredential } from '../shared/db/credentials';
|
||||
import { createOwner } from '../shared/db/users';
|
||||
import type { SuperAgentTest } from '../shared/types';
|
||||
import * as utils from '../shared/utils/';
|
||||
import { putCredentialTypePolicy } from './shared/credential-type-policy';
|
||||
import { clearPolicyCache } from './shared/policy-cache';
|
||||
|
||||
const MANUAL_TRIGGER = 'n8n-nodes-base.manualTrigger';
|
||||
const HTTP_REQUEST = 'n8n-nodes-base.httpRequest';
|
||||
|
||||
const BASE = 'oAuth2Api';
|
||||
const DERIVED = 'googleSheetsOAuth2Api';
|
||||
|
||||
const testServer = utils.setupTestServer({
|
||||
endpointGroups: ['workflows', 'type-availability-policies'],
|
||||
modules: ['policy-infrastructure', 'type-availability-policies'],
|
||||
enabledFeatures: [LICENSE_FEATURES.TYPE_AVAILABILITY_POLICIES, LICENSE_FEATURES.SHARING],
|
||||
});
|
||||
|
||||
let owner: User;
|
||||
let ownerAgent: SuperAgentTest;
|
||||
let derivedCredentialId: string;
|
||||
let workflowRepository: WorkflowRepository;
|
||||
let executionRepository: ExecutionRepository;
|
||||
|
||||
const denyFamily: PolicyRule = {
|
||||
id: 'deny-oauth2-family',
|
||||
action: 'deny',
|
||||
selector: { kind: 'extends', value: BASE },
|
||||
};
|
||||
|
||||
const node = (type: string): INode => ({
|
||||
id: uuid(),
|
||||
name: type,
|
||||
type,
|
||||
typeVersion: 1,
|
||||
position: [0, 0],
|
||||
parameters: {},
|
||||
});
|
||||
|
||||
const workflowBody = () => ({
|
||||
name: 'Sheets workflow',
|
||||
nodes: [
|
||||
node(MANUAL_TRIGGER),
|
||||
{
|
||||
...node(HTTP_REQUEST),
|
||||
credentials: { [DERIVED]: { id: derivedCredentialId, name: 'Sheets account' } },
|
||||
},
|
||||
],
|
||||
connections: {},
|
||||
});
|
||||
|
||||
const derivedViolation = {
|
||||
kind: 'credential-type-unavailable',
|
||||
checkId: 'credential-type-availability',
|
||||
message: `Credential type "${DERIVED}" is blocked by an instance policy`,
|
||||
subject: DERIVED,
|
||||
subjectType: 'credentialType',
|
||||
scope: 'instance',
|
||||
matchedRuleId: 'deny-oauth2-family',
|
||||
};
|
||||
|
||||
beforeAll(async () => {
|
||||
await utils.initNodeTypes();
|
||||
await utils.initCredentialsTypes();
|
||||
await utils.initBinaryDataService();
|
||||
|
||||
owner = await createOwner();
|
||||
ownerAgent = testServer.authAgentFor(owner);
|
||||
workflowRepository = Container.get(WorkflowRepository);
|
||||
executionRepository = Container.get(ExecutionRepository);
|
||||
|
||||
const credential = await saveCredential(randomCredentialPayload({ type: DERIVED }), {
|
||||
user: owner,
|
||||
role: 'credential:owner',
|
||||
});
|
||||
derivedCredentialId = credential.id;
|
||||
});
|
||||
|
||||
// Per test: the shared setup restores every mock after each one.
|
||||
beforeEach(() => {
|
||||
vi.spyOn(Container.get(LoadNodesAndCredentials), 'knownCredentials', 'get').mockReturnValue({
|
||||
[BASE]: { className: '', sourcePath: '' },
|
||||
googleOAuth2Api: { className: '', sourcePath: '', extends: [BASE] },
|
||||
[DERIVED]: { className: '', sourcePath: '', extends: ['googleOAuth2Api'] },
|
||||
});
|
||||
});
|
||||
|
||||
afterEach(async () => {
|
||||
await testDb.truncate([
|
||||
'TypeAvailabilityPolicyAttachment',
|
||||
'TypeAvailabilityPolicyScope',
|
||||
'TypeAvailabilityPolicy',
|
||||
'ExecutionEntity',
|
||||
'WorkflowEntity',
|
||||
'SharedWorkflow',
|
||||
'WorkflowHistory',
|
||||
]);
|
||||
await clearPolicyCache();
|
||||
});
|
||||
|
||||
describe('a rule on a base credential type', () => {
|
||||
test('refuses saving a workflow that asks for a type built on it', async () => {
|
||||
await putCredentialTypePolicy(ownerAgent, null, { rules: [denyFamily] });
|
||||
|
||||
const response = await ownerAgent.post('/workflows').send(workflowBody()).expect(403);
|
||||
|
||||
expect(response.body.meta.violations).toEqual([derivedViolation]);
|
||||
await expect(workflowRepository.count()).resolves.toBe(0);
|
||||
});
|
||||
|
||||
test('fails a run of a workflow that asks for a type built on it', async () => {
|
||||
const workflow = await createWorkflow(workflowBody(), owner);
|
||||
await putCredentialTypePolicy(ownerAgent, null, { rules: [denyFamily] });
|
||||
|
||||
const executionId = await Container.get(WorkflowRunner).run(
|
||||
{
|
||||
workflowData: workflow,
|
||||
executionMode: 'trigger',
|
||||
executionData: createRunExecutionData({}),
|
||||
},
|
||||
true,
|
||||
);
|
||||
|
||||
await vi.waitFor(
|
||||
async () => {
|
||||
const execution = await executionRepository.findOneBy({ id: executionId });
|
||||
expect(execution?.status).toBe('error');
|
||||
},
|
||||
{ timeout: 20000, interval: 50 },
|
||||
);
|
||||
const stored = await executionRepository.findSingleExecution(executionId, {
|
||||
includeData: true,
|
||||
unflattenData: true,
|
||||
});
|
||||
const error = stored?.data.resultData.error as unknown as { violations?: unknown[] };
|
||||
|
||||
expect(error?.violations).toEqual([derivedViolation]);
|
||||
});
|
||||
|
||||
test('lets an earlier allow for the derived type win over the family deny', async () => {
|
||||
await putCredentialTypePolicy(ownerAgent, null, {
|
||||
rules: [
|
||||
{ id: 'allow-sheets', action: 'allow', selector: { kind: 'name', value: DERIVED } },
|
||||
denyFamily,
|
||||
],
|
||||
});
|
||||
|
||||
await ownerAgent.post('/workflows').send(workflowBody()).expect(200);
|
||||
});
|
||||
|
||||
test('leaves a type built on the base allowed when the rule names the base exactly', async () => {
|
||||
await putCredentialTypePolicy(ownerAgent, null, {
|
||||
rules: [{ id: 'deny-generic', action: 'deny', selector: { kind: 'name', value: BASE } }],
|
||||
});
|
||||
|
||||
await ownerAgent.post('/workflows').send(workflowBody()).expect(200);
|
||||
});
|
||||
});
|
||||
|
||||
describe('writing an extends rule', () => {
|
||||
test('rejects a base credential type that is not installed', async () => {
|
||||
const response = await ownerAgent.put('/credential-type-policies/instance').send({
|
||||
rules: [
|
||||
{ id: 'r1', action: 'deny', selector: { kind: 'extends', value: 'notInstalledApi' } },
|
||||
],
|
||||
defaultAction: 'allow',
|
||||
version: 0,
|
||||
});
|
||||
|
||||
expect(response.statusCode).toBe(400);
|
||||
expect(response.body.message).toBe(
|
||||
'Extends rule names a credential type that is not installed: notInstalledApi',
|
||||
);
|
||||
});
|
||||
|
||||
test('rejects the rule in a node type policy request', async () => {
|
||||
const response = await ownerAgent.put('/node-type-policies/instance').send({
|
||||
rules: [denyFamily],
|
||||
defaultAction: 'allow',
|
||||
version: 0,
|
||||
});
|
||||
|
||||
expect(response.statusCode).toBe(400);
|
||||
expect(response.body.message).toBe("Invalid discriminator value. Expected 'name' | 'package'");
|
||||
});
|
||||
});
|
||||
|
|
@ -4,6 +4,7 @@ import { LICENSE_FEATURES } from '@n8n/constants';
|
|||
import type { Project, User } from '@n8n/db';
|
||||
import { Container } from '@n8n/di';
|
||||
|
||||
import { LoadNodesAndCredentials } from '@/load-nodes-and-credentials';
|
||||
import { TypeAvailabilityPolicyRepository } from '@/modules/type-availability-policies/database/repositories/type-availability-policy.repository';
|
||||
import { createMemberWithApiKey, createOwnerWithApiKey } from '@test-integration/db/users';
|
||||
import * as utils from '@test-integration/utils';
|
||||
|
|
@ -669,3 +670,54 @@ describe('credential type policies public API attachments', () => {
|
|||
expect(response.statusCode).toBe(400);
|
||||
});
|
||||
});
|
||||
|
||||
describe('credential type policies public API extends rules', () => {
|
||||
const EXTENDS_RULE = {
|
||||
id: 'r1',
|
||||
action: 'deny',
|
||||
selector: { kind: 'extends', value: 'oAuth2Api' },
|
||||
};
|
||||
|
||||
// Per test: the shared setup restores every mock after each one.
|
||||
beforeEach(() => {
|
||||
vi.spyOn(Container.get(LoadNodesAndCredentials), 'knownCredentials', 'get').mockReturnValue({
|
||||
oAuth2Api: { className: '', sourcePath: '' },
|
||||
});
|
||||
});
|
||||
|
||||
test('an extends rule reads back through every response shape', async () => {
|
||||
const agent = testServer.publicApiAgentFor(owner);
|
||||
|
||||
const instancePut = await agent
|
||||
.put('/credential-type-policies/instance')
|
||||
.send({ rules: [EXTENDS_RULE], defaultAction: 'allow', version: 0 });
|
||||
expect(instancePut.statusCode).toBe(200);
|
||||
expect(instancePut.body.rules).toEqual([EXTENDS_RULE]);
|
||||
|
||||
const instanceGet = await agent.get('/credential-type-policies/instance');
|
||||
expect(instanceGet.body.rules).toEqual([EXTENDS_RULE]);
|
||||
|
||||
const created = await agent
|
||||
.post('/credential-type-policies/policies')
|
||||
.send({ rules: [EXTENDS_RULE] });
|
||||
expect(created.statusCode).toBe(201);
|
||||
const policyId = created.body.policy.id as string;
|
||||
|
||||
const document = await agent.get(`/credential-type-policies/policies/${policyId}`);
|
||||
expect(document.body.rules).toEqual([EXTENDS_RULE]);
|
||||
|
||||
const list = await agent.get('/credential-type-policies/policies');
|
||||
expect(list.statusCode).toBe(200);
|
||||
expect(list.body.data.map((policy: { rules: unknown }) => policy.rules)).toContainEqual([
|
||||
EXTENDS_RULE,
|
||||
]);
|
||||
|
||||
const attachments = await agent
|
||||
.put(`/credential-type-policies/scopes/${instancePut.body.scopeId}/attachments`)
|
||||
.send({ attachments: [{ policyId, priority: 1, isFloor: false }] });
|
||||
expect(attachments.statusCode).toBe(200);
|
||||
expect(attachments.body.attachments).toEqual([
|
||||
{ policyId, rules: [EXTENDS_RULE], priority: 1, isFloor: false },
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -274,6 +274,22 @@ describe('node type policies public API instance scope', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('node type policies public API extends rules', () => {
|
||||
test('PUT rejects an extends rule with 400 and writes nothing', async () => {
|
||||
const agent = testServer.publicApiAgentFor(owner);
|
||||
|
||||
const response = await agent.put('/node-type-policies/instance').send({
|
||||
rules: [{ id: 'r1', action: 'deny', selector: { kind: 'extends', value: 'oAuth2Api' } }],
|
||||
defaultAction: 'allow',
|
||||
version: 0,
|
||||
});
|
||||
|
||||
expect(response.statusCode).toBe(400);
|
||||
const current = await agent.get('/node-type-policies/instance');
|
||||
expect(current.body.version).toBe(0);
|
||||
});
|
||||
});
|
||||
|
||||
describe('node type policies public API project scope', () => {
|
||||
test('GET on an unconfigured project reports allow-all at version 0', async () => {
|
||||
const response = await testServer.publicApiAgentFor(projectAdmin).get(projectRoute(project.id));
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user