diff --git a/packages/@n8n/permissions/src/roles/__tests__/node-type-policy-scope.test.ts b/packages/@n8n/permissions/src/roles/__tests__/node-type-policy-scope.test.ts new file mode 100644 index 00000000000..dea9a963a2b --- /dev/null +++ b/packages/@n8n/permissions/src/roles/__tests__/node-type-policy-scope.test.ts @@ -0,0 +1,24 @@ +import { GLOBAL_ADMIN_SCOPES, GLOBAL_OWNER_SCOPES } from '@/roles/scopes/global-scopes.ee'; +import { + PERSONAL_PROJECT_OWNER_SCOPES, + PROJECT_EDITOR_SCOPES, + PROJECT_VIEWER_SCOPES, + REGULAR_PROJECT_ADMIN_SCOPES, +} from '@/roles/scopes/project-scopes.ee'; + +describe('nodeTypePolicy:manage default grants', () => { + it('is granted to instance owners and admins', () => { + expect(GLOBAL_OWNER_SCOPES).toContain('nodeTypePolicy:manage'); + expect(GLOBAL_ADMIN_SCOPES).toContain('nodeTypePolicy:manage'); + }); + + it('is granted to project admins, per IAM-1142 (project admins self-govern their own row)', () => { + expect(REGULAR_PROJECT_ADMIN_SCOPES).toContain('nodeTypePolicy:manage'); + }); + + it('is not granted to personal project owners, editors, or viewers', () => { + expect(PERSONAL_PROJECT_OWNER_SCOPES).not.toContain('nodeTypePolicy:manage'); + expect(PROJECT_EDITOR_SCOPES).not.toContain('nodeTypePolicy:manage'); + expect(PROJECT_VIEWER_SCOPES).not.toContain('nodeTypePolicy:manage'); + }); +}); diff --git a/packages/@n8n/permissions/src/roles/scopes/project-scopes.ee.ts b/packages/@n8n/permissions/src/roles/scopes/project-scopes.ee.ts index 40d9eab9e5e..7171f496b7a 100644 --- a/packages/@n8n/permissions/src/roles/scopes/project-scopes.ee.ts +++ b/packages/@n8n/permissions/src/roles/scopes/project-scopes.ee.ts @@ -53,6 +53,7 @@ export const REGULAR_PROJECT_ADMIN_SCOPES: Scope[] = [ 'folder:list', 'folder:move', 'sourceControl:push', + 'nodeTypePolicy:manage', 'dataTable:create', 'dataTable:delete', 'dataTable:read', diff --git a/packages/@n8n/permissions/src/scope-information.ts b/packages/@n8n/permissions/src/scope-information.ts index bcd3f0621f1..b9a2fb70971 100644 --- a/packages/@n8n/permissions/src/scope-information.ts +++ b/packages/@n8n/permissions/src/scope-information.ts @@ -60,7 +60,7 @@ export const scopeInformation: Partial> = { }, 'nodeTypePolicy:manage': { displayName: 'Manage Node Type Policies', - description: 'Allows managing which node types are available on the instance.', + description: 'Allows managing which node types are available, on the instance or in a project.', }, 'encryptionKey:manage': { displayName: 'Manage Encryption Keys', diff --git a/packages/cli/src/modules/type-availability-policies/__tests__/policy-evaluator.test.ts b/packages/cli/src/modules/type-availability-policies/__tests__/policy-evaluator.test.ts index 5cad8d96aac..7aafa4d2a08 100644 --- a/packages/cli/src/modules/type-availability-policies/__tests__/policy-evaluator.test.ts +++ b/packages/cli/src/modules/type-availability-policies/__tests__/policy-evaluator.test.ts @@ -1,4 +1,4 @@ -import { evaluateType } from '../policy-evaluator'; +import { evaluateComposedType, evaluateType, type ScopePolicy } from '../policy-evaluator'; import type { PolicyAttachment } from '../policy-rule.types'; const attachment = (overrides: Partial): PolicyAttachment => ({ @@ -9,6 +9,12 @@ const attachment = (overrides: Partial): PolicyAttachment => ( ...overrides, }); +const scopePolicy = (overrides: Partial = {}): ScopePolicy => ({ + attachments: [], + defaultAction: 'allow', + ...overrides, +}); + describe('evaluateType', () => { it('lets an earlier deny rule take precedence over a later package allow', () => { const attachments = [ @@ -206,3 +212,107 @@ describe('evaluateType', () => { }); }); }); + +describe('evaluateComposedType', () => { + const TYPE = 'n8n-nodes-base.slack'; + const allowRule = (id: string) => + attachment({ rules: [{ id, action: 'allow', selector: { kind: 'name', value: TYPE } }] }); + const denyRule = (id: string) => + attachment({ rules: [{ id, action: 'deny', selector: { kind: 'name', value: TYPE } }] }); + + it('an instance deny is final: a project allow never overrides it', () => { + const instance = scopePolicy({ attachments: [denyRule('instance-deny')] }); + const project = scopePolicy({ attachments: [allowRule('project-allow')] }); + + expect(evaluateComposedType(instance, project, TYPE)).toEqual({ + action: 'deny', + scope: 'instance', + matchedRuleId: 'instance-deny', + }); + }); + + it('an instance delegate is satisfied by an explicit project allow rule', () => { + const instance = scopePolicy({ defaultAction: 'delegate' }); + const project = scopePolicy({ attachments: [allowRule('project-allow')] }); + + expect(evaluateComposedType(instance, project, TYPE)).toEqual({ + action: 'allow', + scope: 'project', + matchedRuleId: 'project-allow', + }); + }); + + it('an instance delegate stays denied when the project has no policy row at all', () => { + const instance = scopePolicy({ defaultAction: 'delegate' }); + const project = scopePolicy(); + + expect(evaluateComposedType(instance, project, TYPE)).toEqual({ + action: 'deny', + scope: 'instance', + matchedRuleId: null, + }); + }); + + it('an instance delegate is not satisfied by a bare project defaultAction of allow', () => { + const instance = scopePolicy({ defaultAction: 'delegate' }); + const project = scopePolicy({ defaultAction: 'allow' }); + + expect(evaluateComposedType(instance, project, TYPE)).toEqual({ + action: 'deny', + scope: 'instance', + matchedRuleId: null, + }); + }); + + it('an instance delegate with an explicit project deny rule is attributed to the project', () => { + const instance = scopePolicy({ defaultAction: 'delegate' }); + const project = scopePolicy({ attachments: [denyRule('project-deny')] }); + + expect(evaluateComposedType(instance, project, TYPE)).toEqual({ + action: 'deny', + scope: 'project', + matchedRuleId: 'project-deny', + }); + }); + + it('an instance delegate with a project defaultAction of deny is attributed to the project', () => { + const instance = scopePolicy({ defaultAction: 'delegate' }); + const project = scopePolicy({ defaultAction: 'deny' }); + + expect(evaluateComposedType(instance, project, TYPE)).toEqual({ + action: 'deny', + scope: 'project', + matchedRuleId: null, + }); + }); + + it('an instance allow lets a project deny rule restrict further', () => { + const instance = scopePolicy({ attachments: [allowRule('instance-allow')] }); + const project = scopePolicy({ attachments: [denyRule('project-deny')] }); + + expect(evaluateComposedType(instance, project, TYPE)).toEqual({ + action: 'deny', + scope: 'project', + matchedRuleId: 'project-deny', + }); + }); + + it('allows and attributes to the instance when neither scope is configured', () => { + expect(evaluateComposedType(scopePolicy(), scopePolicy(), TYPE)).toEqual({ + action: 'allow', + scope: 'instance', + matchedRuleId: null, + }); + }); + + it('allows and attributes to the project when the project explicitly allows on top of an instance allow', () => { + const instance = scopePolicy(); + const project = scopePolicy({ attachments: [allowRule('project-allow')] }); + + expect(evaluateComposedType(instance, project, TYPE)).toEqual({ + action: 'allow', + scope: 'project', + matchedRuleId: 'project-allow', + }); + }); +}); diff --git a/packages/cli/src/modules/type-availability-policies/__tests__/type-availability-policies.module.test.ts b/packages/cli/src/modules/type-availability-policies/__tests__/type-availability-policies.module.test.ts index 5ddd646ddb8..0d6a8587271 100644 --- a/packages/cli/src/modules/type-availability-policies/__tests__/type-availability-policies.module.test.ts +++ b/packages/cli/src/modules/type-availability-policies/__tests__/type-availability-policies.module.test.ts @@ -1,3 +1,4 @@ +import { LICENSE_FEATURES } from '@n8n/constants'; import { ControllerRegistryMetadata, ModuleMetadata } from '@n8n/decorators'; import { Container } from '@n8n/di'; @@ -11,7 +12,13 @@ describe('TypeAvailabilityPoliciesModule', () => { expect(entry).toBeDefined(); }); - it('registers the instance controller on init', async () => { + it('is gated by the node type policies license feature, so an unlicensed instance skips init', () => { + const entry = Container.get(ModuleMetadata).get('type-availability-policies'); + + expect(entry?.licenseFlag).toBe(LICENSE_FEATURES.NODE_TYPE_POLICIES); + }); + + it('registers the instance and project controllers on init', async () => { const module = new TypeAvailabilityPoliciesModule(); await module.init(); @@ -19,11 +26,17 @@ describe('TypeAvailabilityPoliciesModule', () => { const { TypeAvailabilityPolicyInstanceController } = await import( '../type-availability-policy-instance.controller.js' ); - const metadata = Container.get(ControllerRegistryMetadata).getControllerMetadata( - TypeAvailabilityPolicyInstanceController as never, + const { TypeAvailabilityPolicyProjectController } = await import( + '../type-availability-policy-project.controller.js' ); + const registry = Container.get(ControllerRegistryMetadata); - expect(metadata.routes.size).toBeGreaterThan(0); + expect( + registry.getControllerMetadata(TypeAvailabilityPolicyInstanceController as never).routes.size, + ).toBeGreaterThan(0); + expect( + registry.getControllerMetadata(TypeAvailabilityPolicyProjectController as never).routes.size, + ).toBeGreaterThan(0); }); it('exposes its entities so the datasource picks them up', async () => { diff --git a/packages/cli/src/modules/type-availability-policies/__tests__/type-availability-policy-project.controller.test.ts b/packages/cli/src/modules/type-availability-policies/__tests__/type-availability-policy-project.controller.test.ts new file mode 100644 index 00000000000..5187d5f71a3 --- /dev/null +++ b/packages/cli/src/modules/type-availability-policies/__tests__/type-availability-policy-project.controller.test.ts @@ -0,0 +1,40 @@ +import { LICENSE_FEATURES } from '@n8n/constants'; +import { ControllerRegistryMetadata } from '@n8n/decorators'; +import { Container } from '@n8n/di'; + +import { TypeAvailabilityPolicyProjectController } from '../type-availability-policy-project.controller'; + +/** + * Every route on this controller must be project-scoped (`@ProjectScope`), never + * `@GlobalScope` and never left ungated — a project admin self-governs their own project's + * row, per IAM-1142's RFC decision. Every route must also require the node type policies + * license feature, same as the instance controller. + */ +describe('TypeAvailabilityPolicyProjectController route access scopes', () => { + const metadata = Container.get(ControllerRegistryMetadata).getControllerMetadata( + TypeAvailabilityPolicyProjectController as never, + ); + const routeCases = Array.from(metadata.routes.entries()).map(([handlerName, route]) => ({ + handlerName, + route, + })); + + it('registers at least one route', () => { + expect(routeCases.length).toBeGreaterThan(0); + }); + + it.each(routeCases)( + '$handlerName is gated by a project-scoped nodeTypePolicy:manage check', + ({ route }) => { + expect(route.accessScope).toBeDefined(); + expect(route.accessScope?.globalOnly).toBe(false); + expect(route.accessScope?.scope).toBe('nodeTypePolicy:manage'); + }, + ); + + it('is gated by the node type policies license feature', () => { + for (const { route } of routeCases) { + expect(route.licenseFeature).toBe(LICENSE_FEATURES.NODE_TYPE_POLICIES); + } + }); +}); diff --git a/packages/cli/src/modules/type-availability-policies/__tests__/type-availability-policy.service.test.ts b/packages/cli/src/modules/type-availability-policies/__tests__/type-availability-policy.service.test.ts index fa15743966c..fb1dd671e92 100644 --- a/packages/cli/src/modules/type-availability-policies/__tests__/type-availability-policy.service.test.ts +++ b/packages/cli/src/modules/type-availability-policies/__tests__/type-availability-policy.service.test.ts @@ -22,6 +22,14 @@ const RULE: PolicyRule = { selector: { kind: 'name', value: 'n8n-nodes-base.slack' }, }; +const DELEGATE_RULE: PolicyRule = { + id: 'r-delegate', + action: 'delegate', + selector: { kind: 'name', value: 'n8n-nodes-base.slack' }, +}; + +const DELEGATE_RULE_AT_PROJECT_SCOPE = 'A rule cannot use action "delegate" at project scope'; + function makeScope(overrides: Partial = {}) { return Object.assign(new TypeAvailabilityPolicyScope(), { id: 'scope-1', @@ -106,6 +114,27 @@ describe('TypeAvailabilityPolicyService', () => { }); describe('setDefaultAction', () => { + it('rejects a delegate defaultAction at project scope before opening a transaction', async () => { + await expect( + service.setDefaultAction(KIND, 'project-1', 'delegate', 0, 'user-1'), + ).rejects.toThrow('defaultAction cannot be "delegate" at project scope'); + + expect(transactionRunner.run).not.toHaveBeenCalled(); + expect(scopeRepository.findScopeByKindAndProject).not.toHaveBeenCalled(); + }); + + it('still allows a delegate defaultAction at instance scope', async () => { + scopeRepository.findScopeByKindAndProject.mockResolvedValue(null); + scopeRepository.createScopeIfAbsent.mockResolvedValue({ + scope: makeScope({ defaultAction: 'delegate', version: 1 }), + created: true, + }); + + await expect( + service.setDefaultAction(KIND, null, 'delegate', 0, 'user-1'), + ).resolves.not.toThrow(); + }); + it('throws ConflictError on a stale version and writes nothing', async () => { scopeRepository.findScopeByKindAndProject.mockResolvedValue(makeScope({ version: 2 })); @@ -236,6 +265,55 @@ describe('TypeAvailabilityPolicyService', () => { beforeEach(() => { attachmentRepository.listScopeIdsAttachedToPolicy.mockResolvedValue([]); scopeRepository.lockScopesByIds.mockResolvedValue([]); + scopeRepository.containsProjectScope.mockResolvedValue(false); + }); + + it('rejects a delegate rule when the document is attached to a project scope, before reading it', async () => { + attachmentRepository.listScopeIdsAttachedToPolicy.mockResolvedValue(['scope-1']); + scopeRepository.lockScopesByIds.mockResolvedValue(['scope-1']); + scopeRepository.containsProjectScope.mockResolvedValue(true); + + await expect( + service.updatePolicyDocument('policy-1', [DELEGATE_RULE], 1, 'user-2'), + ).rejects.toThrow(DELEGATE_RULE_AT_PROJECT_SCOPE); + + expect(scopeRepository.containsProjectScope).toHaveBeenCalledWith(['scope-1'], ROOT); + expect(policyRepository.findById).not.toHaveBeenCalled(); + expect(policyRepository.updateRules).not.toHaveBeenCalled(); + expect(eventService.emit).not.toHaveBeenCalled(); + }); + + it('still accepts a delegate rule when the document is attached to instance scope only', async () => { + const before = makePolicy({ rules: [], version: 1 }); + policyRepository.findById.mockResolvedValue(before); + policyRepository.updateRules.mockResolvedValue( + makePolicy({ rules: [DELEGATE_RULE], version: 2 }), + ); + attachmentRepository.listScopeIdsAttachedToPolicy.mockResolvedValue(['scope-1']); + scopeRepository.lockScopesByIds.mockResolvedValue(['scope-1']); + scopeRepository.containsProjectScope.mockResolvedValue(false); + + await expect( + service.updatePolicyDocument(before.id, [DELEGATE_RULE], 1, 'user-2'), + ).resolves.not.toThrow(); + + expect(policyRepository.updateRules).toHaveBeenCalledWith( + before.id, + [DELEGATE_RULE], + 'user-2', + ROOT, + ); + }); + + it('does not look up the scope kinds when the rules carry no delegate', async () => { + policyRepository.findById.mockResolvedValue(makePolicy({ rules: [], version: 1 })); + policyRepository.updateRules.mockResolvedValue(makePolicy({ rules: [RULE], version: 2 })); + attachmentRepository.listScopeIdsAttachedToPolicy.mockResolvedValue(['scope-1']); + scopeRepository.lockScopesByIds.mockResolvedValue(['scope-1']); + + await service.updatePolicyDocument('policy-1', [RULE], 1, 'user-2'); + + expect(scopeRepository.containsProjectScope).not.toHaveBeenCalled(); }); it('throws NotFoundError when the document does not exist', async () => { @@ -463,9 +541,188 @@ describe('TypeAvailabilityPolicyService', () => { expect(result.version).toBe(2); }); + + it('rejects attaching a document with a delegate rule to a project scope, and writes nothing', async () => { + scopeRepository.findScopeById.mockResolvedValue(makeScope({ projectId: 'project-1' })); + policyRepository.findManyByIds.mockResolvedValue([ + makePolicy({ id: 'p1', rules: [RULE] }), + makePolicy({ id: 'p2', rules: [DELEGATE_RULE] }), + ]); + + await expect( + service.replaceAttachments( + 'scope-1', + [ + { policyId: 'p1', priority: 0, isFloor: false }, + { policyId: 'p2', priority: 1, isFloor: false }, + ], + 'user-1', + ), + ).rejects.toThrow(DELEGATE_RULE_AT_PROJECT_SCOPE); + + // Locked read: the check must hold against a concurrent document edit. + expect(policyRepository.findManyByIds).toHaveBeenCalledWith(['p1', 'p2'], ROOT, true); + expect(attachmentRepository.replaceAttachmentsForScope).not.toHaveBeenCalled(); + expect(scopeRepository.bumpVersion).not.toHaveBeenCalled(); + expect(eventService.emit).not.toHaveBeenCalled(); + }); + + it('does not inspect the documents when attaching to instance scope', async () => { + scopeRepository.findScopeById.mockResolvedValue(makeScope({ projectId: null })); + attachmentRepository.listAttachmentsForScope.mockResolvedValue([]); + + await service.replaceAttachments( + 'scope-1', + [{ policyId: 'p-delegating', priority: 0, isFloor: false }], + 'user-1', + ); + + expect(policyRepository.findManyByIds).not.toHaveBeenCalled(); + expect(attachmentRepository.replaceAttachmentsForScope).toHaveBeenCalled(); + }); }); describe('setEffectivePolicy', () => { + beforeEach(() => { + // By default a scope's document is attached to that scope alone. + attachmentRepository.listScopeIdsAttachedToPolicy.mockResolvedValue(['scope-1']); + }); + + it('throws ConflictError when the scope has several attached documents, and writes nothing', async () => { + const scope = makeScope({ projectId: 'project-1', defaultAction: 'allow', version: 1 }); + scopeRepository.findScopeByKindAndProject.mockResolvedValue(scope); + attachmentRepository.listAttachmentsForScope.mockResolvedValue([ + { policyId: 'policy-1', rules: [], priority: 0, isFloor: false }, + { policyId: 'policy-2', rules: [], priority: 1, isFloor: false }, + ]); + + await expect( + service.setEffectivePolicy( + KIND, + 'project-1', + { rules: [RULE], defaultAction: 'deny' }, + 1, + 'user-2', + ), + ).rejects.toThrow(ConflictError); + + expect(scopeRepository.updateDefaultAction).not.toHaveBeenCalled(); + expect(policyRepository.updateRules).not.toHaveBeenCalled(); + expect(scopeRepository.bumpVersion).not.toHaveBeenCalled(); + expect(eventService.emit).not.toHaveBeenCalled(); + }); + + it('throws ConflictError when the attached document is shared with another scope, and writes nothing', async () => { + const scope = makeScope({ projectId: 'project-1', defaultAction: 'allow', version: 1 }); + scopeRepository.findScopeByKindAndProject.mockResolvedValue(scope); + attachmentRepository.listAttachmentsForScope.mockResolvedValue([ + { policyId: 'policy-1', rules: [], priority: 0, isFloor: false }, + ]); + attachmentRepository.listScopeIdsAttachedToPolicy.mockResolvedValue([ + scope.id, + 'instance-scope', + ]); + + await expect( + service.setEffectivePolicy( + KIND, + 'project-1', + { rules: [RULE], defaultAction: 'deny' }, + 1, + 'user-2', + ), + ).rejects.toThrow(ConflictError); + + expect(attachmentRepository.listScopeIdsAttachedToPolicy).toHaveBeenCalledWith( + 'policy-1', + ROOT, + ); + expect(scopeRepository.updateDefaultAction).not.toHaveBeenCalled(); + expect(policyRepository.updateRules).not.toHaveBeenCalled(); + expect(scopeRepository.bumpVersion).not.toHaveBeenCalled(); + expect(eventService.emit).not.toHaveBeenCalled(); + }); + + it('locks the existing document before checking which scopes use it', async () => { + const scope = makeScope({ defaultAction: 'allow', version: 1 }); + scopeRepository.findScopeByKindAndProject.mockResolvedValue(scope); + attachmentRepository.listAttachmentsForScope.mockResolvedValue([ + { policyId: 'policy-1', rules: [], priority: 0, isFloor: false }, + ]); + policyRepository.findById.mockResolvedValue(makePolicy({ rules: [], version: 1 })); + policyRepository.updateRules.mockResolvedValue(makePolicy({ rules: [RULE], version: 2 })); + scopeRepository.findScopeById.mockResolvedValue(makeScope({ version: 2 })); + + await service.setEffectivePolicy( + KIND, + null, + { rules: [RULE], defaultAction: 'allow' }, + 1, + 'user-2', + ); + + expect(policyRepository.findById).toHaveBeenCalledWith('policy-1', ROOT, true); + expect(policyRepository.findById.mock.invocationCallOrder[0]).toBeLessThan( + attachmentRepository.listScopeIdsAttachedToPolicy.mock.invocationCallOrder[0], + ); + }); + + it('rejects a delegate defaultAction at project scope before opening a transaction', async () => { + await expect( + service.setEffectivePolicy( + KIND, + 'project-1', + { rules: [], defaultAction: 'delegate' }, + 0, + 'user-1', + ), + ).rejects.toThrow('defaultAction cannot be "delegate" at project scope'); + + expect(transactionRunner.run).not.toHaveBeenCalled(); + }); + + it('rejects a delegate rule action at project scope before opening a transaction', async () => { + const delegateRule: PolicyRule = { + id: 'r1', + action: 'delegate', + selector: { kind: 'name', value: 'n8n-nodes-base.slack' }, + }; + + await expect( + service.setEffectivePolicy( + KIND, + 'project-1', + { rules: [delegateRule], defaultAction: 'allow' }, + 0, + 'user-1', + ), + ).rejects.toThrow('A rule cannot use action "delegate" at project scope'); + + expect(transactionRunner.run).not.toHaveBeenCalled(); + }); + + it('still allows delegate at instance scope', async () => { + scopeRepository.findScopeByKindAndProject.mockResolvedValue(null); + scopeRepository.createScopeIfAbsent.mockResolvedValue({ + scope: makeScope({ defaultAction: 'delegate', version: 1 }), + created: true, + }); + policyRepository.createPolicy.mockResolvedValue(makePolicy({ rules: [], version: 1 })); + scopeRepository.findScopeById.mockResolvedValue( + makeScope({ defaultAction: 'delegate', version: 2 }), + ); + + await expect( + service.setEffectivePolicy( + KIND, + null, + { rules: [], defaultAction: 'delegate' }, + 0, + 'user-1', + ), + ).resolves.not.toThrow(); + }); + it('throws ConflictError on a stale version and writes nothing', async () => { scopeRepository.findScopeByKindAndProject.mockResolvedValue(makeScope({ version: 2 })); @@ -659,4 +916,61 @@ describe('TypeAvailabilityPolicyService', () => { expect(result.version).toBe(1); }); }); + + describe('evaluateComposedType', () => { + const PROJECT_ID = 'project-1'; + const TYPE = 'n8n-nodes-base.slack'; + + it('reads the instance and project scopes and composes their verdicts', async () => { + const instanceScope = makeScope({ projectId: null, defaultAction: 'delegate', version: 1 }); + const projectScope = makeScope({ + id: 'scope-2', + projectId: PROJECT_ID, + defaultAction: 'deny', + version: 1, + }); + scopeRepository.findScopeByKindAndProject.mockImplementation(async (_kind, projectId) => + projectId === null ? instanceScope : projectScope, + ); + const projectAllowRule: PolicyRule = { + id: 'project-allow', + action: 'allow', + selector: { kind: 'name', value: TYPE }, + }; + attachmentRepository.listAttachmentsForScope.mockImplementation(async (scopeId) => + scopeId === projectScope.id + ? [{ policyId: 'p1', rules: [projectAllowRule], priority: 0, isFloor: false }] + : [], + ); + + const result = await service.evaluateComposedType(KIND, PROJECT_ID, TYPE); + + expect(scopeRepository.findScopeByKindAndProject).toHaveBeenCalledWith(KIND, null, ROOT); + expect(scopeRepository.findScopeByKindAndProject).toHaveBeenCalledWith( + KIND, + PROJECT_ID, + ROOT, + ); + expect(result).toEqual({ action: 'allow', scope: 'project', matchedRuleId: 'project-allow' }); + }); + + it('lets an instance deny win over an unconfigured project', async () => { + const denyRule: PolicyRule = { + id: 'instance-deny', + action: 'deny', + selector: { kind: 'name', value: TYPE }, + }; + const instanceScope = makeScope({ projectId: null, defaultAction: 'allow', version: 1 }); + scopeRepository.findScopeByKindAndProject.mockImplementation(async (_kind, projectId) => + projectId === null ? instanceScope : null, + ); + attachmentRepository.listAttachmentsForScope.mockResolvedValue([ + { policyId: 'p1', rules: [denyRule], priority: 0, isFloor: false }, + ]); + + const result = await service.evaluateComposedType(KIND, PROJECT_ID, TYPE); + + expect(result).toEqual({ action: 'deny', scope: 'instance', matchedRuleId: 'instance-deny' }); + }); + }); }); diff --git a/packages/cli/src/modules/type-availability-policies/database/repositories/__tests__/type-availability-policy-locks.test.ts b/packages/cli/src/modules/type-availability-policies/database/repositories/__tests__/type-availability-policy-locks.test.ts index 57395b9af4c..2d4b2201b08 100644 --- a/packages/cli/src/modules/type-availability-policies/database/repositories/__tests__/type-availability-policy-locks.test.ts +++ b/packages/cli/src/modules/type-availability-policies/database/repositories/__tests__/type-availability-policy-locks.test.ts @@ -137,6 +137,41 @@ describe('row locks depend on the driver', () => { expect(entityManager.increment).toHaveBeenCalledTimes(2); }); + describe('containsProjectScope', () => { + it('is false for an empty id list without querying', async () => { + expect(await repository.containsProjectScope([], ROOT)).toBe(false); + expect(entityManager.count).not.toHaveBeenCalled(); + }); + + it('counts only rows with a non-null projectId', async () => { + entityManager.count.mockResolvedValue(0); + + expect(await repository.containsProjectScope(['a', 'b'], ROOT)).toBe(false); + + expect(entityManager.count).toHaveBeenCalledWith(TypeAvailabilityPolicyScope, { + where: { id: expect.anything(), projectId: expect.anything() }, + }); + }); + + it('stops at the first batch that contains a project scope', async () => { + entityManager.count.mockResolvedValueOnce(1); + const ids = Array.from({ length: 10_001 }, (_, i) => `id-${i}`); + + expect(await repository.containsProjectScope(ids, ROOT)).toBe(true); + + expect(entityManager.count).toHaveBeenCalledTimes(1); + }); + + it('checks every batch when none contains a project scope', async () => { + entityManager.count.mockResolvedValue(0); + const ids = Array.from({ length: 10_001 }, (_, i) => `id-${i}`); + + expect(await repository.containsProjectScope(ids, ROOT)).toBe(false); + + expect(entityManager.count).toHaveBeenCalledTimes(2); + }); + }); + describe('createScopeIfAbsent', () => { const queryBuilder = { insert: vi.fn().mockReturnThis(), @@ -241,7 +276,53 @@ describe('row locks depend on the driver', () => { it('returns an empty list without querying for an empty id list', async () => { expect(await repository.findManyByIds([], ROOT)).toEqual([]); - expect(entityManager.findBy).not.toHaveBeenCalled(); + expect(entityManager.find).not.toHaveBeenCalled(); + }); + + it('does not lock findManyByIds without forUpdate, even on Postgres', async () => { + setDriver(entityManager, 'postgres'); + entityManager.find.mockResolvedValue([]); + + await repository.findManyByIds(['b', 'a'], ROOT); + + expect(entityManager.find).toHaveBeenCalledWith( + TypeAvailabilityPolicy, + expect.not.objectContaining({ lock: expect.anything() }), + ); + }); + + it('locks findManyByIds in id order on Postgres when forUpdate is requested', async () => { + setDriver(entityManager, 'postgres'); + entityManager.find.mockResolvedValue([]); + + await repository.findManyByIds(['b', 'a'], ROOT, true); + + expect(entityManager.find).toHaveBeenCalledWith(TypeAvailabilityPolicy, { + where: { id: expect.anything() }, + order: { id: 'ASC' }, + lock: { mode: 'pessimistic_write' }, + }); + }); + + it('does not lock findManyByIds on SQLite even when forUpdate is requested', async () => { + setDriver(entityManager, 'sqlite'); + entityManager.find.mockResolvedValue([]); + + await repository.findManyByIds(['a'], ROOT, true); + + expect(entityManager.find).toHaveBeenCalledWith( + TypeAvailabilityPolicy, + expect.not.objectContaining({ lock: expect.anything() }), + ); + }); + + it('reads every batch when more ids are named than fit in one query', async () => { + entityManager.find.mockResolvedValue([]); + const ids = Array.from({ length: 10_001 }, (_, i) => `id-${i}`); + + await repository.findManyByIds(ids, ROOT, true); + + expect(entityManager.find).toHaveBeenCalledTimes(2); }); }); }); diff --git a/packages/cli/src/modules/type-availability-policies/database/repositories/type-availability-policy-scope.repository.ts b/packages/cli/src/modules/type-availability-policies/database/repositories/type-availability-policy-scope.repository.ts index b931713af4d..f57996e19cc 100644 --- a/packages/cli/src/modules/type-availability-policies/database/repositories/type-availability-policy-scope.repository.ts +++ b/packages/cli/src/modules/type-availability-policies/database/repositories/type-availability-policy-scope.repository.ts @@ -6,7 +6,7 @@ import { type OperationContext, } from '@n8n/db'; import { Service } from '@n8n/di'; -import { DataSource, In, IsNull, type EntityManager } from '@n8n/typeorm'; +import { DataSource, In, IsNull, Not, type EntityManager } from '@n8n/typeorm'; import { UnexpectedError } from 'n8n-workflow'; import type { PolicyAction } from '../../policy-rule.types'; @@ -106,6 +106,23 @@ export class TypeAvailabilityPolicyScopeRepository extends BaseRepository { + const manager = this.managerFor(ctx); + for (const batch of chunkIds(ids)) { + const count = await manager.count(TypeAvailabilityPolicyScope, { + where: { id: In(batch), projectId: Not(IsNull()) }, + }); + if (count > 0) return true; + } + + return false; + } + /** * Inserts the scope row if `(kind, projectId)` has none yet, then reads back whichever row * exists. `created: false` means a concurrent first write got there first. diff --git a/packages/cli/src/modules/type-availability-policies/database/repositories/type-availability-policy.repository.ts b/packages/cli/src/modules/type-availability-policies/database/repositories/type-availability-policy.repository.ts index 260b8e1b1c7..71ee4cd07ee 100644 --- a/packages/cli/src/modules/type-availability-policies/database/repositories/type-availability-policy.repository.ts +++ b/packages/cli/src/modules/type-availability-policies/database/repositories/type-availability-policy.repository.ts @@ -1,6 +1,6 @@ -import { BaseRepository, TransactionRunner, type OperationContext } from '@n8n/db'; +import { BaseRepository, TransactionRunner, chunkIds, type OperationContext } from '@n8n/db'; import { Service } from '@n8n/di'; -import { DataSource, In } from '@n8n/typeorm'; +import { DataSource, In, type EntityManager } from '@n8n/typeorm'; import { isDeepStrictEqual } from 'node:util'; import type { PolicyRule } from '../../policy-rule.types'; @@ -28,6 +28,13 @@ export class TypeAvailabilityPolicyRepository extends BaseRepository { + /** + * Unknown ids are ignored. Chunked, since a caller may name many documents at once. + * + * Pass `forUpdate: true` inside a write transaction whose decision depends on the rules as + * they are at commit time — `replaceAttachments` checks them before attaching to a project + * scope, and `updatePolicyDocument` holds the same lock while it edits them, so one waits + * for the other instead of deciding on rules the other is about to change. Rows are locked + * in id order, batch by batch, for the reason `lockScopesByIds` gives; a caller that locks + * scopes first and then documents keeps the scope → policy order every write path uses. + */ + async findManyByIds( + ids: string[], + ctx: OperationContext, + forUpdate = false, + ): Promise { if (ids.length === 0) return []; - return await this.managerFor(ctx).findBy(TypeAvailabilityPolicy, { id: In(ids) }); + + const manager = this.managerFor(ctx); + const found: TypeAvailabilityPolicy[] = []; + for (const batch of chunkIds([...ids].sort())) { + const rows = await manager.find(TypeAvailabilityPolicy, { + where: { id: In(batch) }, + order: { id: 'ASC' }, + ...(forUpdate ? this.forUpdateLock(manager) : {}), + }); + found.push(...rows); + } + + return found; } /** Every policy document of one kind, for the document-library listing screen. */ diff --git a/packages/cli/src/modules/type-availability-policies/dto/__tests__/policy-rule.schema.test.ts b/packages/cli/src/modules/type-availability-policies/dto/__tests__/policy-rule.schema.test.ts index eeba0b2ca25..6a8bc312c9c 100644 --- a/packages/cli/src/modules/type-availability-policies/dto/__tests__/policy-rule.schema.test.ts +++ b/packages/cli/src/modules/type-availability-policies/dto/__tests__/policy-rule.schema.test.ts @@ -1,4 +1,10 @@ -import { policyActionSchema, policyRuleListSchema, policyRuleSchema } from '../policy-rule.schema'; +import { + nonDelegatingPolicyActionSchema, + nonDelegatingPolicyRuleListSchema, + policyActionSchema, + policyRuleListSchema, + policyRuleSchema, +} from '../policy-rule.schema'; const nameSelector = { kind: 'name' as const, value: 'n8n-nodes-base.slack' }; @@ -58,3 +64,44 @@ describe('policyRuleListSchema', () => { } }); }); + +describe('nonDelegatingPolicyActionSchema', () => { + it.each(['allow', 'deny'])('accepts %s', (action) => { + expect(nonDelegatingPolicyActionSchema.safeParse(action).success).toBe(true); + }); + + it('rejects delegate', () => { + expect(nonDelegatingPolicyActionSchema.safeParse('delegate').success).toBe(false); + }); +}); + +describe('nonDelegatingPolicyRuleListSchema', () => { + it('accepts a list of allow/deny rules with unique ids', () => { + expect( + nonDelegatingPolicyRuleListSchema.safeParse([ + { id: 'r1', action: 'deny', selector: nameSelector }, + { id: 'r2', action: 'allow', selector: { kind: 'package', value: 'n8n-nodes-base' } }, + ]).success, + ).toBe(true); + }); + + it('rejects a rule using the delegate action', () => { + expect( + nonDelegatingPolicyRuleListSchema.safeParse([ + { id: 'r1', action: 'delegate', selector: nameSelector }, + ]).success, + ).toBe(false); + }); + + it('still rejects duplicate rule ids', () => { + const result = nonDelegatingPolicyRuleListSchema.safeParse([ + { id: 'r1', action: 'deny', selector: nameSelector }, + { id: 'r1', action: 'allow', selector: { kind: 'package', value: 'n8n-nodes-base' } }, + ]); + + expect(result.success).toBe(false); + if (!result.success) { + expect(result.error.issues[0].message).toBe('Duplicate rule id: r1'); + } + }); +}); diff --git a/packages/cli/src/modules/type-availability-policies/dto/policy-rule.schema.ts b/packages/cli/src/modules/type-availability-policies/dto/policy-rule.schema.ts index 9a2d9d024a4..caa70b6df5c 100644 --- a/packages/cli/src/modules/type-availability-policies/dto/policy-rule.schema.ts +++ b/packages/cli/src/modules/type-availability-policies/dto/policy-rule.schema.ts @@ -1,6 +1,11 @@ import { z } from 'zod'; -import type { PolicyAction, PolicyRule } from '../policy-rule.types'; +import type { + NonDelegatingPolicyAction, + NonDelegatingPolicyRule, + PolicyAction, + PolicyRule, +} from '../policy-rule.types'; import { policySelectorSchema } from './policy-selector.schema'; /** @@ -23,8 +28,7 @@ export const policyRuleSchema = z.object({ selector: policySelectorSchema, }) satisfies z.ZodType; -/** An ordered list of rules for one policy document. Rejects duplicate rule ids. */ -export const policyRuleListSchema = z.array(policyRuleSchema).superRefine((rules, ctx) => { +function rejectDuplicateRuleIds(rules: PolicyRule[], ctx: z.RefinementCtx): void { const seenIds = new Set(); for (const [index, rule] of rules.entries()) { @@ -39,4 +43,24 @@ export const policyRuleListSchema = z.array(policyRuleSchema).superRefine((rules seenIds.add(rule.id); } -}); +} + +/** An ordered list of rules for one policy document. Rejects duplicate rule ids. */ +export const policyRuleListSchema = z.array(policyRuleSchema).superRefine(rejectDuplicateRuleIds); + +/** + * `delegate` is only meaningful where a narrower scope exists to opt in — instance scope + * today. There is no scope narrower than project, so a project-scope write rejects it outright. + */ +export const nonDelegatingPolicyActionSchema = policyActionSchema.exclude([ + 'delegate', +]) satisfies z.ZodType; + +const nonDelegatingPolicyRuleSchema = policyRuleSchema.extend({ + action: nonDelegatingPolicyActionSchema, +}) satisfies z.ZodType; + +/** Same as `policyRuleListSchema`, but for project scope: rejects a `delegate` rule too. */ +export const nonDelegatingPolicyRuleListSchema = z + .array(nonDelegatingPolicyRuleSchema) + .superRefine(rejectDuplicateRuleIds); diff --git a/packages/cli/src/modules/type-availability-policies/dto/put-project-policy.dto.ts b/packages/cli/src/modules/type-availability-policies/dto/put-project-policy.dto.ts new file mode 100644 index 00000000000..20c26b2f2c1 --- /dev/null +++ b/packages/cli/src/modules/type-availability-policies/dto/put-project-policy.dto.ts @@ -0,0 +1,19 @@ +import { Z } from '@n8n/api-types'; +import { z } from 'zod'; + +import { + nonDelegatingPolicyActionSchema, + nonDelegatingPolicyRuleListSchema, +} from './policy-rule.schema'; + +/** + * Request body for setting a project's composed policy. Same shape as + * `PutInstancePolicyDto`, except `delegate` is rejected — there is no scope narrower than + * project for it to defer to. + */ +export class PutProjectPolicyDto extends Z.class({ + rules: nonDelegatingPolicyRuleListSchema, + defaultAction: nonDelegatingPolicyActionSchema, + // Optimistic concurrency: callers send the version they last read, so a stale write is rejected. + version: z.number().int().nonnegative(), +}) {} diff --git a/packages/cli/src/modules/type-availability-policies/policy-evaluator.ts b/packages/cli/src/modules/type-availability-policies/policy-evaluator.ts index 3a8ad419bb3..11702d4e5b3 100644 --- a/packages/cli/src/modules/type-availability-policies/policy-evaluator.ts +++ b/packages/cli/src/modules/type-availability-policies/policy-evaluator.ts @@ -5,6 +5,24 @@ import type { PolicyVerdict, } from './policy-rule.types'; +/** One scope's inputs to evaluation: its attached policies' rules and its own default action. */ +export type ScopePolicy = { + readonly attachments: readonly PolicyAttachment[]; + readonly defaultAction: PolicyAction; +}; + +/** + * What the instance ∩ project composition decided for one type, and which scope decided it. + * + * `matchedRuleId` carries the same "explicit rule vs. default action" distinction as + * `PolicyVerdict`, from whichever scope's verdict is reported. + */ +export type ComposedVerdict = { + readonly action: 'allow' | 'deny'; + readonly scope: 'instance' | 'project'; + readonly matchedRuleId: string | null; +}; + /** * Package selectors match the segment of the type name before the first dot (a full type * name is always `.`), same convention used elsewhere in the @@ -59,3 +77,57 @@ export function evaluateType( return { action: defaultAction, matchedRuleId: null }; } + +/** + * Composes an instance scope's verdict with a project scope's, per the finalized RFC: + * conjunction with monotonic restriction. A project can only restrict further, never re-allow + * an instance `deny`. `delegate` is the sanctioned exception, satisfied only by an explicit + * project `allow` rule (`matchedRuleId !== null`) — never by the project's bare `defaultAction`, + * since that would let an unconfigured project silently satisfy a delegation it never actually + * addressed. + * + * `scope` names the scope whose verdict decided. A denial under an instance `delegate` is the + * project's when the project restricted the type itself (by rule or by a `deny` default), and + * the instance's only when the project left the type at its bare `allow` default. + * + * A project-authored `delegate` is treated as `deny` here (fail-closed): validation rejects it + * at write time, so this is defense in depth, not the primary guard. + */ +export function evaluateComposedType( + instance: ScopePolicy, + project: ScopePolicy, + typeName: string, +): ComposedVerdict { + const instanceVerdict = evaluateType(instance.attachments, instance.defaultAction, typeName); + + if (instanceVerdict.action === 'deny') { + return { action: 'deny', scope: 'instance', matchedRuleId: instanceVerdict.matchedRuleId }; + } + + const projectVerdict = evaluateType(project.attachments, project.defaultAction, typeName); + + if (instanceVerdict.action === 'delegate') { + if (projectVerdict.action === 'allow' && projectVerdict.matchedRuleId !== null) { + return { action: 'allow', scope: 'project', matchedRuleId: projectVerdict.matchedRuleId }; + } + // The project restricted the type itself, by rule or by a `deny` default: the denial + // is the project's decision, not the unsatisfied delegation's. + if (projectVerdict.action !== 'allow') { + return { action: 'deny', scope: 'project', matchedRuleId: projectVerdict.matchedRuleId }; + } + // A bare project `allow` default never satisfies a delegation, so the unsatisfied + // instance `delegate` is what denies here. + return { action: 'deny', scope: 'instance', matchedRuleId: instanceVerdict.matchedRuleId }; + } + + // instanceVerdict.action === 'allow': the project's own verdict decides. + if (projectVerdict.action !== 'allow') { + return { action: 'deny', scope: 'project', matchedRuleId: projectVerdict.matchedRuleId }; + } + + return { + action: 'allow', + scope: projectVerdict.matchedRuleId !== null ? 'project' : 'instance', + matchedRuleId: projectVerdict.matchedRuleId ?? instanceVerdict.matchedRuleId, + }; +} diff --git a/packages/cli/src/modules/type-availability-policies/policy-rule.types.ts b/packages/cli/src/modules/type-availability-policies/policy-rule.types.ts index 9eb76607001..353f877a39e 100644 --- a/packages/cli/src/modules/type-availability-policies/policy-rule.types.ts +++ b/packages/cli/src/modules/type-availability-policies/policy-rule.types.ts @@ -20,6 +20,17 @@ export type PolicyRule = { readonly selector: PolicySelector; }; +/** + * The actions a project scope accepts. `delegate` defers to a narrower scope, and none exists + * below project, so project-scope writes never accept it. + */ +export type NonDelegatingPolicyAction = Exclude; + +/** A `PolicyRule` as a project scope accepts it. */ +export type NonDelegatingPolicyRule = Omit & { + readonly action: NonDelegatingPolicyAction; +}; + /** One policy document as attached to a scope, with its evaluation-order metadata. */ export type PolicyAttachment = { readonly policyId: string; diff --git a/packages/cli/src/modules/type-availability-policies/type-availability-policies.module.ts b/packages/cli/src/modules/type-availability-policies/type-availability-policies.module.ts index ecf8bc29823..448e05a3a04 100644 --- a/packages/cli/src/modules/type-availability-policies/type-availability-policies.module.ts +++ b/packages/cli/src/modules/type-availability-policies/type-availability-policies.module.ts @@ -1,11 +1,21 @@ +import { LICENSE_FEATURES } from '@n8n/constants'; import type { ModuleInterface } from '@n8n/decorators'; import { BackendModule } from '@n8n/decorators'; -@BackendModule({ name: 'type-availability-policies' }) +/** + * The module-level `licenseFlag` skips `init()` on an unlicensed instance, so the controllers + * are never loaded there. Each route still carries `@Licensed`: modules are initialized once + * at startup, so only the per-route check covers a license that changes while running. + */ +@BackendModule({ + name: 'type-availability-policies', + licenseFlag: LICENSE_FEATURES.NODE_TYPE_POLICIES, +}) export class TypeAvailabilityPoliciesModule implements ModuleInterface { async init() { - // Side-effecting import: registers the controller's routes via `@RestController`. + // Side-effecting imports: register the controllers' routes via `@RestController`. await import('./type-availability-policy-instance.controller.js'); + await import('./type-availability-policy-project.controller.js'); } async entities() { diff --git a/packages/cli/src/modules/type-availability-policies/type-availability-policy-project.controller.ts b/packages/cli/src/modules/type-availability-policies/type-availability-policy-project.controller.ts new file mode 100644 index 00000000000..a58a3b77e68 --- /dev/null +++ b/packages/cli/src/modules/type-availability-policies/type-availability-policy-project.controller.ts @@ -0,0 +1,65 @@ +import { LICENSE_FEATURES } from '@n8n/constants'; +import { AuthenticatedRequest } from '@n8n/db'; +import { Body, Get, Licensed, ProjectScope, Put, RestController } from '@n8n/decorators'; +import type { Response } from 'express'; + +import { PutProjectPolicyDto } from './dto/put-project-policy.dto'; +import { TypeAvailabilityPolicyService } from './type-availability-policy.service'; + +/** + * The one `kind` this REST surface manages — see `TypeAvailabilityPolicyInstanceController`. + */ +const NODE_TYPES_KIND = 'node-types'; + +/** + * Project-scope REST surface for a project's own node type availability policy row. Every + * route requires `LICENSE_FEATURES.NODE_TYPE_POLICIES` and `nodeTypePolicy:manage`, granted to + * project admins so they self-govern their own project's row (per IAM-1142's RFC decision). + * + * Deliberately narrower than the instance controller: only the composed read/write, mirroring + * `GET`/`PUT /instance`. Policy-document CRUD and multi-attachment management stay + * instance-only — those act on documents and attachments potentially shared across scopes. + */ +@RestController('/projects/:projectId/node-type-policies') +export class TypeAvailabilityPolicyProjectController { + constructor(private readonly service: TypeAvailabilityPolicyService) {} + + @Get('/project') + @Licensed(LICENSE_FEATURES.NODE_TYPE_POLICIES) + @ProjectScope('nodeTypePolicy:manage') + async getProjectPolicy(req: AuthenticatedRequest<{ projectId: string }>) { + const effective = await this.service.getEffectivePolicy(NODE_TYPES_KIND, req.params.projectId); + + return { + scopeId: effective.scopeId, + rules: effective.rules, + defaultAction: effective.defaultAction, + version: effective.version, + }; + } + + @Put('/project') + @Licensed(LICENSE_FEATURES.NODE_TYPE_POLICIES) + @ProjectScope('nodeTypePolicy:manage') + async putProjectPolicy( + req: AuthenticatedRequest<{ projectId: string }>, + _res: Response, + @Body dto: PutProjectPolicyDto, + ) { + const result = await this.service.setEffectivePolicy( + NODE_TYPES_KIND, + req.params.projectId, + { rules: dto.rules, defaultAction: dto.defaultAction }, + dto.version, + req.user.id, + ); + + return { + scopeId: result.scopeId, + rules: result.rules, + defaultAction: result.defaultAction, + version: result.version, + warnings: result.warnings, + }; + } +} diff --git a/packages/cli/src/modules/type-availability-policies/type-availability-policy.service.ts b/packages/cli/src/modules/type-availability-policies/type-availability-policy.service.ts index 6fe64cad3be..f805904f7df 100644 --- a/packages/cli/src/modules/type-availability-policies/type-availability-policy.service.ts +++ b/packages/cli/src/modules/type-availability-policies/type-availability-policy.service.ts @@ -11,7 +11,7 @@ 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 { orderedAttachments } from './policy-evaluator'; +import { evaluateComposedType, orderedAttachments, type ComposedVerdict } from './policy-evaluator'; import type { PolicyAction, PolicyAttachment, PolicyRule } from './policy-rule.types'; import { lintRulesForShadowing, type ShadowWarning } from './policy-shadow-lint'; @@ -55,6 +55,38 @@ function flattenRules(attachments: readonly PolicyAttachment[]): PolicyRule[] { return orderedAttachments(attachments).flatMap((attachment) => [...attachment.rules]); } +function rulesContainDelegate(rules: readonly PolicyRule[]): boolean { + return rules.some((rule) => rule.action === 'delegate'); +} + +const DELEGATE_RULE_AT_PROJECT_SCOPE = 'A rule cannot use action "delegate" at project scope'; + +/** + * `delegate` is only satisfiable at instance scope. A project write never accepts it — not in + * a rule's `action` (already rejected by `PutProjectPolicyDto`'s schema) and not in + * `defaultAction`, which lives on the `policy_scope` row rather than in the policy document, so + * the DTO's rule-level check does not cover it on its own. Defense in depth: a future caller + * reaching this service directly, not only through the project controller, still can't write an + * unsatisfiable `delegate` on a project row. + * + * The composed write is not the only way rules reach a project row: `replaceAttachments` and + * `updatePolicyDocument` guard the same invariant on their own paths. + */ +function assertNoDelegateAtProjectScope( + projectId: string | null, + defaultAction: PolicyAction, + rules: readonly PolicyRule[] = [], +): void { + if (projectId === null) return; + + if (defaultAction === 'delegate') { + throw new UserError('defaultAction cannot be "delegate" at project scope'); + } + if (rulesContainDelegate(rules)) { + throw new UserError(DELEGATE_RULE_AT_PROJECT_SCOPE); + } +} + /** Mirrors the DTO-level check in `ReplaceAttachmentsDto`, as a defensive service-level guard. */ function assertNoDuplicateAttachmentSlots(attachments: readonly AttachmentInput[]): void { const seenPolicyIds = new Set(); @@ -142,6 +174,8 @@ export class TypeAvailabilityPolicyService { expectedVersion: number, updatedBy: string, ): Promise { + assertNoDelegateAtProjectScope(projectId, defaultAction); + const result = await this.transactionRunner.run({}, async (ctx) => { const scope = await this.scopeRepository.findScopeByKindAndProject( kind, @@ -257,6 +291,15 @@ export class TypeAvailabilityPolicyService { ); const lockedScopeIds = await this.scopeRepository.lockScopesByIds(attachedScopeIds, ctx); + // A document attached to a project scope is part of that project's policy, so the + // same `delegate` rejection as a direct project write applies to editing it. + if ( + rulesContainDelegate(rules) && + (await this.scopeRepository.containsProjectScope(lockedScopeIds, ctx)) + ) { + throw new UserError(DELEGATE_RULE_AT_PROJECT_SCOPE); + } + const existing = await this.policyRepository.findById(policyId, ctx, true); if (!existing) { throw new NotFoundError(`Policy document not found: ${policyId}`); @@ -382,6 +425,23 @@ export class TypeAvailabilityPolicyService { throw new NotFoundError(`Policy scope not found: ${scopeId}`); } + // Attaching a document to a project scope makes its rules that project's policy, so + // a document carrying `delegate` is rejected the same way a direct project write is. + // The documents are read under the same row lock `updatePolicyDocument` takes, so a + // concurrent edit cannot slip a `delegate` in between this check and the attach: one + // of the two waits, and the loser either sees the new rules here or fails that path's + // own "attached while updating" conflict check. + if (scope.projectId !== null && attachments.length > 0) { + const policies = await this.policyRepository.findManyByIds( + attachments.map((a) => a.policyId), + ctx, + true, + ); + if (policies.some((policy) => rulesContainDelegate(policy.rules))) { + throw new UserError(DELEGATE_RULE_AT_PROJECT_SCOPE); + } + } + const before = await this.attachmentRepository.listAttachmentsForScope(scopeId, ctx); await this.attachmentRepository.replaceAttachmentsForScope( @@ -442,6 +502,8 @@ export class TypeAvailabilityPolicyService { rules: PolicyRule[]; warnings: readonly ShadowWarning[]; }> { + assertNoDelegateAtProjectScope(projectId, input.defaultAction, input.rules); + const warnings = lintRulesForShadowing(input.rules); const result = await this.transactionRunner.run({}, async (ctx) => { @@ -463,6 +525,45 @@ export class TypeAvailabilityPolicyService { ? { defaultAction: scope.defaultAction, version: scope.version } : null; + const existingAttachments = scope + ? await this.attachmentRepository.listAttachmentsForScope(scope.id, ctx) + : []; + + // This write edits the scope's single document in place. It refuses when that would + // reach further than the scope itself: several attachments (editing only the first + // would silently leave the rest in force), or a document also attached elsewhere + // (editing it would change every other scope that uses it — at project scope, that + // would let a project admin rewrite the instance policy). Both states can only be + // produced through the instance-only attachment management, so the same admin can + // undo them there. Checked before any write, so a refusal leaves nothing to roll back. + if (existingAttachments.length > 1) { + throw new ConflictError( + `Cannot replace the policy of a scope with ${existingAttachments.length} attached documents; manage its attachments instead`, + ); + } + + const existingDocumentId = existingAttachments[0]?.policyId ?? null; + + // Lock the document before checking who else uses it: an attach elsewhere key-shares + // the document row, so it waits for this transaction (or this one waits for it) and + // the check below cannot be overtaken between reading the attachments and the edit. + // Scope first, then document — the order every write path keeps. + const existingDocument = existingDocumentId + ? await this.policyRepository.findById(existingDocumentId, ctx, true) + : null; + + if (scope && existingDocumentId) { + const attachedScopeIds = await this.attachmentRepository.listScopeIdsAttachedToPolicy( + existingDocumentId, + ctx, + ); + if (attachedScopeIds.some((id) => id !== scope.id)) { + throw new ConflictError( + 'Cannot replace a policy document that is attached to other scopes; manage its attachments instead', + ); + } + } + const scopeId = scope ? scope.id : ( @@ -481,19 +582,15 @@ export class TypeAvailabilityPolicyService { ); } - const existingAttachments = scope - ? await this.attachmentRepository.listAttachmentsForScope(scopeId, ctx) - : []; - const existingDocumentId = existingAttachments[0]?.policyId ?? null; - let documentBefore: { rules: readonly PolicyRule[]; version: number } | null = null; let documentAfter: { rules: readonly PolicyRule[]; version: number }; let documentCreated: boolean; let policyId: string; if (existingDocumentId) { - const before = await this.policyRepository.findById(existingDocumentId, ctx); - documentBefore = before ? { rules: before.rules, version: before.version } : null; + documentBefore = existingDocument + ? { rules: existingDocument.rules, version: existingDocument.version } + : null; const updated = await this.policyRepository.updateRules( existingDocumentId, @@ -582,4 +679,22 @@ export class TypeAvailabilityPolicyService { warnings, }; } + + /** + * Composes the instance and project verdicts for one type, per `evaluateComposedType`. Reads + * both scopes' effective policies in parallel — point-in-time snapshots, not one transaction, + * which is fine for an evaluation path (unlike a write). + */ + async evaluateComposedType( + kind: string, + projectId: string, + typeName: string, + ): Promise { + const [instance, project] = await Promise.all([ + this.getEffectivePolicy(kind, null), + this.getEffectivePolicy(kind, projectId), + ]); + + return evaluateComposedType(instance, project, typeName); + } } diff --git a/packages/cli/test/integration/policy/type-availability-policy-project.controller.test.ts b/packages/cli/test/integration/policy/type-availability-policy-project.controller.test.ts new file mode 100644 index 00000000000..7da44d13ad1 --- /dev/null +++ b/packages/cli/test/integration/policy/type-availability-policy-project.controller.test.ts @@ -0,0 +1,394 @@ +import { createTeamProject, linkUserToProject, testDb } from '@n8n/backend-test-utils'; +import { LICENSE_FEATURES } from '@n8n/constants'; +import type { Project, User } from '@n8n/db'; +import { Container } from '@n8n/di'; + +import { EventService } from '@/events/event.service'; +import { createMember, createOwner } from '@test-integration/db/users'; +import * as utils from '@test-integration/utils'; + +const testServer = utils.setupTestServer({ + endpointGroups: ['type-availability-policies'], + modules: ['type-availability-policies'], + enabledFeatures: [LICENSE_FEATURES.NODE_TYPE_POLICIES], +}); + +let owner: User; +let projectAdmin: User; +let projectEditor: User; +let projectViewer: User; +let otherProjectAdmin: User; +let project: Project; +let otherProject: Project; + +const projectRoute = (projectId: string) => `/projects/${projectId}/node-type-policies/project`; + +const ALLOW_ALL = { rules: [], defaultAction: 'allow', version: 0 }; +const DENY_RULE = { id: 'r1', action: 'deny', selector: { kind: 'name', value: 'a.b' } }; +const DELEGATE_RULE = { id: 'r2', action: 'delegate', selector: { kind: 'name', value: 'a.b' } }; + +beforeAll(async () => { + owner = await createOwner(); + projectAdmin = await createMember(); + projectEditor = await createMember(); + projectViewer = await createMember(); + otherProjectAdmin = await createMember(); + + project = await createTeamProject('Policy project', projectAdmin); + otherProject = await createTeamProject('Other project', otherProjectAdmin); + await linkUserToProject(projectEditor, project, 'project:editor'); + await linkUserToProject(projectViewer, project, 'project:viewer'); +}); + +afterEach(async () => { + await testDb.truncate([ + 'TypeAvailabilityPolicyAttachment', + 'TypeAvailabilityPolicyScope', + 'TypeAvailabilityPolicy', + ]); +}); + +/** + * IAM-1142: a project admin self-governs their own project's row. Every other project role, + * and an admin of a different project, is rejected with 403. + */ +describe('node type availability policy project controller RBAC', () => { + test('PUT rejects a project editor with 403', async () => { + const response = await testServer + .authAgentFor(projectEditor) + .put(projectRoute(project.id)) + .send(ALLOW_ALL); + + expect(response.statusCode).toBe(403); + }); + + test('GET rejects a project editor with 403', async () => { + const response = await testServer.authAgentFor(projectEditor).get(projectRoute(project.id)); + + expect(response.statusCode).toBe(403); + }); + + test('GET rejects a project viewer with 403', async () => { + const response = await testServer.authAgentFor(projectViewer).get(projectRoute(project.id)); + + expect(response.statusCode).toBe(403); + }); + + test('GET rejects the admin of a different project with 403', async () => { + const response = await testServer.authAgentFor(otherProjectAdmin).get(projectRoute(project.id)); + + expect(response.statusCode).toBe(403); + }); + + test('PUT rejects the admin of a different project with 403 and writes nothing', async () => { + const response = await testServer + .authAgentFor(otherProjectAdmin) + .put(projectRoute(project.id)) + .send({ ...ALLOW_ALL, defaultAction: 'deny' }); + + expect(response.statusCode).toBe(403); + + const unchanged = await testServer.authAgentFor(projectAdmin).get(projectRoute(project.id)); + expect(unchanged.body.data.version).toBe(0); + }); + + test('an unauthenticated caller is rejected before the scope check even runs', async () => { + const response = await testServer.authlessAgent.get(projectRoute(project.id)); + + expect(response.statusCode).toBe(401); + }); + + test('the admin of the project is not rejected by the scope check', async () => { + const response = await testServer.authAgentFor(projectAdmin).get(projectRoute(project.id)); + + expect(response.statusCode).toBe(200); + }); + + test('an instance owner without a project role is not rejected by the scope check', async () => { + const response = await testServer.authAgentFor(owner).get(projectRoute(project.id)); + + expect(response.statusCode).toBe(200); + }); +}); + +describe('node type availability policy project controller license gating', () => { + afterEach(() => { + testServer.license.enable(LICENSE_FEATURES.NODE_TYPE_POLICIES); + }); + + test('rejects a project admin with 403 when the license feature is disabled', async () => { + testServer.license.disable(LICENSE_FEATURES.NODE_TYPE_POLICIES); + + const response = await testServer.authAgentFor(projectAdmin).get(projectRoute(project.id)); + + expect(response.statusCode).toBe(403); + }); +}); + +describe('node type availability policy project controller project admin happy path', () => { + test('GET on a project that was never configured reports allow-all at version 0', async () => { + const response = await testServer.authAgentFor(projectAdmin).get(projectRoute(project.id)); + + expect(response.statusCode).toBe(200); + expect(response.body.data).toEqual({ + scopeId: null, + rules: [], + defaultAction: 'allow', + version: 0, + }); + }); + + test('PUT persists, bumps version, and fires an audit event scoped to the project', async () => { + const eventService = Container.get(EventService); + const emitSpy = vi.spyOn(eventService, 'emit'); + + const first = await testServer + .authAgentFor(projectAdmin) + .put(projectRoute(project.id)) + .send({ rules: [DENY_RULE], defaultAction: 'allow', version: 0 }); + + expect(first.statusCode).toBe(200); + expect(first.body.data.version).toBeGreaterThan(0); + expect(first.body.data.rules).toEqual([DENY_RULE]); + expect(emitSpy).toHaveBeenCalledWith( + 'node-type-policy-scope-updated', + expect.objectContaining({ updatedBy: projectAdmin.id, projectId: project.id, before: null }), + ); + expect(emitSpy).toHaveBeenCalledWith( + 'node-type-policy-document-created', + expect.objectContaining({ updatedBy: projectAdmin.id }), + ); + + const persisted = await testServer.authAgentFor(projectAdmin).get(projectRoute(project.id)); + expect(persisted.body.data.rules).toEqual(first.body.data.rules); + expect(persisted.body.data.version).toBe(first.body.data.version); + + emitSpy.mockClear(); + const second = await testServer + .authAgentFor(projectAdmin) + .put(projectRoute(project.id)) + .send({ rules: [], defaultAction: 'deny', version: first.body.data.version }); + + expect(second.statusCode).toBe(200); + expect(second.body.data.version).toBeGreaterThan(first.body.data.version); + expect(second.body.data.defaultAction).toBe('deny'); + expect(emitSpy).toHaveBeenCalledWith( + 'node-type-policy-scope-updated', + expect.objectContaining({ updatedBy: projectAdmin.id, projectId: project.id }), + ); + expect(emitSpy).toHaveBeenCalledWith( + 'node-type-policy-document-updated', + expect.objectContaining({ updatedBy: projectAdmin.id }), + ); + }); + + test('a project write leaves the instance scope and other projects untouched', async () => { + const written = await testServer + .authAgentFor(projectAdmin) + .put(projectRoute(project.id)) + .send({ rules: [DENY_RULE], defaultAction: 'deny', version: 0 }); + expect(written.statusCode).toBe(200); + + const instance = await testServer.authAgentFor(owner).get('/node-type-policies/instance'); + expect(instance.body.data.version).toBe(0); + expect(instance.body.data.defaultAction).toBe('allow'); + + const other = await testServer + .authAgentFor(otherProjectAdmin) + .get(projectRoute(otherProject.id)); + expect(other.body.data.version).toBe(0); + expect(other.body.data.rules).toEqual([]); + }); + + test('PUT with a stale version returns 409', async () => { + const first = await testServer + .authAgentFor(projectAdmin) + .put(projectRoute(project.id)) + .send(ALLOW_ALL); + expect(first.statusCode).toBe(200); + + const stale = await testServer + .authAgentFor(projectAdmin) + .put(projectRoute(project.id)) + .send({ ...ALLOW_ALL, defaultAction: 'deny' }); + + expect(stale.statusCode).toBe(409); + }); + + test('PUT with a delegate defaultAction is rejected with 400 and writes nothing', async () => { + const response = await testServer + .authAgentFor(projectAdmin) + .put(projectRoute(project.id)) + .send({ rules: [], defaultAction: 'delegate', version: 0 }); + + expect(response.statusCode).toBe(400); + + const unchanged = await testServer.authAgentFor(projectAdmin).get(projectRoute(project.id)); + expect(unchanged.body.data.version).toBe(0); + }); + + test('PUT with a delegate rule is rejected with 400 and writes nothing', async () => { + const response = await testServer + .authAgentFor(projectAdmin) + .put(projectRoute(project.id)) + .send({ rules: [DELEGATE_RULE], defaultAction: 'allow', version: 0 }); + + expect(response.statusCode).toBe(400); + + const unchanged = await testServer.authAgentFor(projectAdmin).get(projectRoute(project.id)); + expect(unchanged.body.data.version).toBe(0); + }); +}); + +/** + * The project write edits the project's single document in place. When instance-only + * attachment management has made that document reach further than the project — shared with + * another scope, or one of several — the write is refused rather than silently changing + * policy the project admin does not govern. + */ +describe('node type availability policy project controller shared document protection', () => { + async function configureProject() { + const response = await testServer + .authAgentFor(projectAdmin) + .put(projectRoute(project.id)) + .send({ rules: [DENY_RULE], defaultAction: 'allow', version: 0 }); + expect(response.statusCode).toBe(200); + + const documents = await testServer.authAgentFor(owner).get('/node-type-policies/policies'); + expect(documents.body.data).toHaveLength(1); + + return { + scopeId: response.body.data.scopeId as string, + documentId: documents.body.data[0].id as string, + }; + } + + test('PUT is rejected with 409 when the project document is also attached to the instance scope', async () => { + const { documentId } = await configureProject(); + + const instancePut = await testServer + .authAgentFor(owner) + .put('/node-type-policies/instance') + .send(ALLOW_ALL); + const instanceScopeId = instancePut.body.data.scopeId; + + const shared = await testServer + .authAgentFor(owner) + .put(`/node-type-policies/scopes/${instanceScopeId}/attachments`) + .send({ attachments: [{ policyId: documentId, priority: 0, isFloor: false }] }); + expect(shared.statusCode).toBe(200); + + const current = await testServer.authAgentFor(projectAdmin).get(projectRoute(project.id)); + const response = await testServer + .authAgentFor(projectAdmin) + .put(projectRoute(project.id)) + .send({ rules: [], defaultAction: 'allow', version: current.body.data.version }); + + expect(response.statusCode).toBe(409); + + const document = await testServer + .authAgentFor(owner) + .get(`/node-type-policies/policies/${documentId}`); + expect(document.body.data.rules).toEqual([DENY_RULE]); + }); + + test('PUT is rejected with 409 when the project scope has several attached documents', async () => { + const { scopeId, documentId } = await configureProject(); + + const created = await testServer + .authAgentFor(owner) + .post('/node-type-policies/policies') + .send({ rules: [] }); + const secondDocumentId = created.body.data.policy.id; + + const attached = await testServer + .authAgentFor(owner) + .put(`/node-type-policies/scopes/${scopeId}/attachments`) + .send({ + attachments: [ + { policyId: documentId, priority: 0, isFloor: false }, + { policyId: secondDocumentId, priority: 1, isFloor: false }, + ], + }); + expect(attached.statusCode).toBe(200); + + const current = await testServer.authAgentFor(projectAdmin).get(projectRoute(project.id)); + const response = await testServer + .authAgentFor(projectAdmin) + .put(projectRoute(project.id)) + .send({ rules: [], defaultAction: 'allow', version: current.body.data.version }); + + expect(response.statusCode).toBe(409); + }); +}); + +/** + * `delegate` is rejected on every path that puts rules on a project row, not only the + * project's own composed write. + */ +describe('node type availability policy project controller delegate rejection via attachments', () => { + test('attaching a document with a delegate rule to a project scope is rejected with 400', async () => { + const configured = await testServer + .authAgentFor(projectAdmin) + .put(projectRoute(project.id)) + .send(ALLOW_ALL); + const scopeId = configured.body.data.scopeId; + + const created = await testServer + .authAgentFor(owner) + .post('/node-type-policies/policies') + .send({ rules: [DELEGATE_RULE] }); + expect(created.statusCode).toBe(200); + + const response = await testServer + .authAgentFor(owner) + .put(`/node-type-policies/scopes/${scopeId}/attachments`) + .send({ + attachments: [{ policyId: created.body.data.policy.id, priority: 0, isFloor: false }], + }); + + expect(response.statusCode).toBe(400); + + const unchanged = await testServer.authAgentFor(projectAdmin).get(projectRoute(project.id)); + expect(unchanged.body.data.rules).toEqual([]); + }); + + test('adding a delegate rule to a document attached to a project scope is rejected with 400', async () => { + const configured = await testServer + .authAgentFor(projectAdmin) + .put(projectRoute(project.id)) + .send(ALLOW_ALL); + expect(configured.statusCode).toBe(200); + + const documents = await testServer.authAgentFor(owner).get('/node-type-policies/policies'); + const document = documents.body.data[0]; + + const response = await testServer + .authAgentFor(owner) + .patch(`/node-type-policies/policies/${document.id}`) + .send({ rules: [DELEGATE_RULE], version: document.version }); + + expect(response.statusCode).toBe(400); + + const unchanged = await testServer.authAgentFor(projectAdmin).get(projectRoute(project.id)); + expect(unchanged.body.data.rules).toEqual([]); + }); + + test('adding a delegate rule to a document attached to the instance scope only still works', async () => { + const instancePut = await testServer + .authAgentFor(owner) + .put('/node-type-policies/instance') + .send(ALLOW_ALL); + expect(instancePut.statusCode).toBe(200); + + const documents = await testServer.authAgentFor(owner).get('/node-type-policies/policies'); + const document = documents.body.data[0]; + + const response = await testServer + .authAgentFor(owner) + .patch(`/node-type-policies/policies/${document.id}`) + .send({ rules: [DELEGATE_RULE], version: document.version }); + + expect(response.statusCode).toBe(200); + }); +}); diff --git a/packages/cli/test/integration/policy/type-availability-policy-repositories.test.ts b/packages/cli/test/integration/policy/type-availability-policy-repositories.test.ts index 664ad376f7d..b4b94db286e 100644 --- a/packages/cli/test/integration/policy/type-availability-policy-repositories.test.ts +++ b/packages/cli/test/integration/policy/type-availability-policy-repositories.test.ts @@ -170,6 +170,17 @@ describe('type availability policy repositories', () => { expect(found.map((p) => p.id).sort()).toEqual([a.id, b.id].sort()); }); + it('finds many by id under a row lock inside a transaction', async () => { + const a = await createPolicy(); + const b = await createPolicy([ALLOW_BASE]); + + const found = await transactionRunner.run(ROOT, async (ctx) => { + return await policyRepo.findManyByIds([b.id, a.id], ctx, true); + }); + + expect(found.map((p) => p.id)).toEqual([a.id, b.id].sort()); + }); + it('refuses to delete a policy that is still attached', async () => { const policy = await createPolicy(); const scope = await createInstanceScope(); @@ -288,6 +299,44 @@ describe('type availability policy repositories', () => { expect(scope.updatedBy).toBe('user-1'); }); + describe('containsProjectScope', () => { + it('is false for an empty id list', async () => { + expect(await scopeRepo.containsProjectScope([], ROOT)).toBe(false); + }); + + it('is false when every named scope is the instance scope', async () => { + const instanceScope = await createInstanceScope(); + + expect(await scopeRepo.containsProjectScope([instanceScope.id], ROOT)).toBe(false); + }); + + it('is true when one of the named scopes is a project scope', async () => { + const project = await createTeamProject(); + const instanceScope = await createInstanceScope(); + const projectScope = await createProjectScope(project.id); + + expect( + await scopeRepo.containsProjectScope([instanceScope.id, projectScope.id], ROOT), + ).toBe(true); + }); + + it('ignores ids that name no scope', async () => { + await createInstanceScope(); + + expect(await scopeRepo.containsProjectScope(['does-not-exist'], ROOT)).toBe(false); + }); + + it('finds a project scope that lands in a later batch', async () => { + const project = await createTeamProject(); + const projectScope = await createProjectScope(project.id); + // One batch per ID_QUERY_BATCH_SIZE (10,000) ids: the real id is the 10,001st, so + // only the second batch can find it. + const ids = [...Array.from({ length: 10_000 }, (_, i) => `missing-${i}`), projectScope.id]; + + expect(await scopeRepo.containsProjectScope(ids, ROOT)).toBe(true); + }); + }); + it('locks only the scopes that exist and returns their ids', async () => { const project = await createTeamProject(); const instanceScope = await createInstanceScope();