diff --git a/cypress/e2e/14-mapping.cy.ts b/cypress/e2e/14-mapping.cy.ts index 46448b4966..4ebce02d9d 100644 --- a/cypress/e2e/14-mapping.cy.ts +++ b/cypress/e2e/14-mapping.cy.ts @@ -115,6 +115,8 @@ describe('Data mapping', () => { }); it('maps expressions from json view', () => { + // ADO-3063 - followup to make this viewport global + cy.viewport('macbook-16'); cy.fixture('Test_workflow_3.json').then((data) => { cy.get('body').paste(JSON.stringify(data)); }); @@ -123,17 +125,17 @@ describe('Data mapping', () => { workflowPage.actions.openNode('Set'); ndv.actions.switchInputMode('JSON'); + ndv.getters.inputDataContainer().should('exist'); + ndv.getters .inputDataContainer() - .should('exist') .find('.json-data') .should( 'have.text', '[{"input": [{"count": 0,"with space": "!!","with.dot": "!!","with"quotes": "!!"}]},{"input": [{"count": 1}]}]', - ) - .find('span') - .contains('"count"') - .realMouseDown(); + ); + + ndv.getters.inputDataContainer().find('span').contains('"count"').realMouseDown(); ndv.actions.mapToParameter('value'); ndv.getters.inlineExpressionEditorInput().should('have.text', '{{ $json.input[0].count }}'); diff --git a/packages/@n8n/api-types/src/dto/index.ts b/packages/@n8n/api-types/src/dto/index.ts index ea725e2ec5..f16758a89c 100644 --- a/packages/@n8n/api-types/src/dto/index.ts +++ b/packages/@n8n/api-types/src/dto/index.ts @@ -21,6 +21,10 @@ export { ForgotPasswordRequestDto } from './password-reset/forgot-password-reque export { ResolvePasswordTokenQueryDto } from './password-reset/resolve-password-token-query.dto'; export { ChangePasswordRequestDto } from './password-reset/change-password-request.dto'; +export { SamlAcsDto } from './saml/saml-acs.dto'; +export { SamlPreferences } from './saml/saml-preferences.dto'; +export { SamlToggleDto } from './saml/saml-toggle.dto'; + export { PasswordUpdateRequestDto } from './user/password-update-request.dto'; export { RoleChangeRequestDto } from './user/role-change-request.dto'; export { SettingsUpdateRequestDto } from './user/settings-update-request.dto'; @@ -31,3 +35,5 @@ export { CommunityRegisteredRequestDto } from './license/community-registered-re export { VariableListRequestDto } from './variables/variables-list-request.dto'; export { CredentialsGetOneRequestQuery } from './credentials/credentials-get-one-request.dto'; export { CredentialsGetManyRequestQuery } from './credentials/credentials-get-many-request.dto'; + +export { ImportWorkflowFromUrlDto } from './workflows/import-workflow-from-url.dto'; diff --git a/packages/@n8n/api-types/src/dto/saml/__tests__/saml-preferences.dto.test.ts b/packages/@n8n/api-types/src/dto/saml/__tests__/saml-preferences.dto.test.ts new file mode 100644 index 0000000000..6d11483347 --- /dev/null +++ b/packages/@n8n/api-types/src/dto/saml/__tests__/saml-preferences.dto.test.ts @@ -0,0 +1,155 @@ +import { SamlPreferences } from '../saml-preferences.dto'; + +describe('SamlPreferences', () => { + describe('Valid requests', () => { + test.each([ + { + name: 'valid minimal configuration', + request: { + mapping: { + email: 'user@example.com', + firstName: 'John', + lastName: 'Doe', + userPrincipalName: 'johndoe', + }, + metadata: 'metadata', + metadataUrl: 'https://example.com/metadata', + loginEnabled: true, + loginLabel: 'Login with SAML', + }, + }, + { + name: 'valid full configuration', + request: { + mapping: { + email: 'user@example.com', + firstName: 'John', + lastName: 'Doe', + userPrincipalName: 'johndoe', + }, + metadata: 'metadata', + metadataUrl: 'https://example.com/metadata', + ignoreSSL: true, + loginBinding: 'post', + loginEnabled: true, + loginLabel: 'Login with SAML', + authnRequestsSigned: true, + wantAssertionsSigned: true, + wantMessageSigned: true, + acsBinding: 'redirect', + signatureConfig: { + prefix: 'ds', + location: { + reference: '/samlp:Response/saml:Issuer', + action: 'after', + }, + }, + relayState: 'https://example.com/relay', + }, + }, + ])('should validate $name', ({ request }) => { + const result = SamlPreferences.safeParse(request); + expect(result.success).toBe(true); + }); + }); + + describe('Invalid requests', () => { + test.each([ + { + name: 'invalid loginBinding', + request: { + loginBinding: 'invalid', + }, + expectedErrorPath: ['loginBinding'], + }, + { + name: 'invalid acsBinding', + request: { + acsBinding: 'invalid', + }, + expectedErrorPath: ['acsBinding'], + }, + { + name: 'invalid signatureConfig location action', + request: { + signatureConfig: { + prefix: 'ds', + location: { + reference: '/samlp:Response/saml:Issuer', + action: 'invalid', + }, + }, + }, + expectedErrorPath: ['signatureConfig', 'location', 'action'], + }, + { + name: 'missing signatureConfig location reference', + request: { + signatureConfig: { + prefix: 'ds', + location: { + action: 'after', + }, + }, + }, + expectedErrorPath: ['signatureConfig', 'location', 'reference'], + }, + { + name: 'invalid mapping email', + request: { + mapping: { + email: 123, + firstName: 'John', + lastName: 'Doe', + userPrincipalName: 'johndoe', + }, + }, + expectedErrorPath: ['mapping', 'email'], + }, + ])('should fail validation for $name', ({ request, expectedErrorPath }) => { + const result = SamlPreferences.safeParse(request); + + expect(result.success).toBe(false); + + if (expectedErrorPath) { + expect(result.error?.issues[0].path).toEqual(expectedErrorPath); + } + }); + + describe('Edge cases', () => { + test('should handle optional fields correctly', () => { + const validRequest = { + mapping: undefined, + metadata: undefined, + metadataUrl: undefined, + loginEnabled: undefined, + loginLabel: undefined, + }; + + const result = SamlPreferences.safeParse(validRequest); + expect(result.success).toBe(true); + }); + + test('should handle default values correctly', () => { + const validRequest = {}; + + const result = SamlPreferences.safeParse(validRequest); + expect(result.success).toBe(true); + expect(result.data?.ignoreSSL).toBe(false); + expect(result.data?.loginBinding).toBe('redirect'); + expect(result.data?.authnRequestsSigned).toBe(false); + expect(result.data?.wantAssertionsSigned).toBe(true); + expect(result.data?.wantMessageSigned).toBe(true); + expect(result.data?.acsBinding).toBe('post'); + expect(result.data?.signatureConfig).toEqual({ + prefix: 'ds', + location: { + reference: '/samlp:Response/saml:Issuer', + action: 'after', + }, + }); + expect(result.data?.relayState).toBe(''); + }); + }); + }); +}); diff --git a/packages/@n8n/api-types/src/dto/saml/saml-acs.dto.ts b/packages/@n8n/api-types/src/dto/saml/saml-acs.dto.ts new file mode 100644 index 0000000000..2bfbece7d6 --- /dev/null +++ b/packages/@n8n/api-types/src/dto/saml/saml-acs.dto.ts @@ -0,0 +1,6 @@ +import { z } from 'zod'; +import { Z } from 'zod-class'; + +export class SamlAcsDto extends Z.class({ + RelayState: z.string().optional(), +}) {} diff --git a/packages/@n8n/api-types/src/dto/saml/saml-preferences.dto.ts b/packages/@n8n/api-types/src/dto/saml/saml-preferences.dto.ts new file mode 100644 index 0000000000..e07504c1b3 --- /dev/null +++ b/packages/@n8n/api-types/src/dto/saml/saml-preferences.dto.ts @@ -0,0 +1,50 @@ +import { z } from 'zod'; +import { Z } from 'zod-class'; + +const SamlLoginBindingSchema = z.enum(['redirect', 'post']); + +/** Schema for configuring the signature in SAML requests/responses. */ +const SignatureConfigSchema = z.object({ + prefix: z.string().default('ds'), + location: z.object({ + reference: z.string(), + action: z.enum(['before', 'after', 'prepend', 'append']), + }), +}); + +export class SamlPreferences extends Z.class({ + /** Mapping of SAML attributes to user fields. */ + mapping: z + .object({ + email: z.string(), + firstName: z.string(), + lastName: z.string(), + userPrincipalName: z.string(), + }) + .optional(), + /** SAML metadata in XML format. */ + metadata: z.string().optional(), + metadataUrl: z.string().optional(), + + ignoreSSL: z.boolean().default(false), + loginBinding: SamlLoginBindingSchema.default('redirect'), + /** Whether SAML login is enabled. */ + loginEnabled: z.boolean().optional(), + /** Label for the SAML login button. on the Auth screen */ + loginLabel: z.string().optional(), + + authnRequestsSigned: z.boolean().default(false), + wantAssertionsSigned: z.boolean().default(true), + wantMessageSigned: z.boolean().default(true), + + acsBinding: SamlLoginBindingSchema.default('post'), + signatureConfig: SignatureConfigSchema.default({ + prefix: 'ds', + location: { + reference: '/samlp:Response/saml:Issuer', + action: 'after', + }, + }), + + relayState: z.string().default(''), +}) {} diff --git a/packages/@n8n/api-types/src/dto/saml/saml-toggle.dto.ts b/packages/@n8n/api-types/src/dto/saml/saml-toggle.dto.ts new file mode 100644 index 0000000000..be07933d06 --- /dev/null +++ b/packages/@n8n/api-types/src/dto/saml/saml-toggle.dto.ts @@ -0,0 +1,6 @@ +import { z } from 'zod'; +import { Z } from 'zod-class'; + +export class SamlToggleDto extends Z.class({ + loginEnabled: z.boolean(), +}) {} diff --git a/packages/@n8n/api-types/src/dto/workflows/__tests__/import-workflow-from-url.dto.test.ts b/packages/@n8n/api-types/src/dto/workflows/__tests__/import-workflow-from-url.dto.test.ts new file mode 100644 index 0000000000..3c17e873e0 --- /dev/null +++ b/packages/@n8n/api-types/src/dto/workflows/__tests__/import-workflow-from-url.dto.test.ts @@ -0,0 +1,63 @@ +import { ImportWorkflowFromUrlDto } from '../import-workflow-from-url.dto'; + +describe('ImportWorkflowFromUrlDto', () => { + describe('Valid requests', () => { + test('should validate $name', () => { + const result = ImportWorkflowFromUrlDto.safeParse({ + url: 'https://example.com/workflow.json', + }); + expect(result.success).toBe(true); + }); + }); + + describe('Invalid requests', () => { + test.each([ + { + name: 'invalid URL (not ending with .json)', + url: 'https://example.com/workflow', + expectedErrorPath: ['url'], + }, + { + name: 'invalid URL (missing protocol)', + url: 'example.com/workflow.json', + expectedErrorPath: ['url'], + }, + { + name: 'invalid URL (not a URL)', + url: 'not-a-url', + expectedErrorPath: ['url'], + }, + { + name: 'missing URL', + url: undefined, + expectedErrorPath: ['url'], + }, + { + name: 'null URL', + url: null, + expectedErrorPath: ['url'], + }, + { + name: 'invalid URL (ends with .json but not a valid URL)', + url: 'not-a-url.json', + expectedErrorPath: ['url'], + }, + { + name: 'valid URL with query parameters', + url: 'https://example.com/workflow.json?param=value', + }, + { + name: 'valid URL with fragments', + url: 'https://example.com/workflow.json#section', + }, + ])('should fail validation for $name', ({ url, expectedErrorPath }) => { + const result = ImportWorkflowFromUrlDto.safeParse({ url }); + + expect(result.success).toBe(false); + + if (expectedErrorPath) { + expect(result.error?.issues[0].path).toEqual(expectedErrorPath); + } + }); + }); +}); diff --git a/packages/@n8n/api-types/src/dto/workflows/import-workflow-from-url.dto.ts b/packages/@n8n/api-types/src/dto/workflows/import-workflow-from-url.dto.ts new file mode 100644 index 0000000000..310e620fde --- /dev/null +++ b/packages/@n8n/api-types/src/dto/workflows/import-workflow-from-url.dto.ts @@ -0,0 +1,6 @@ +import { z } from 'zod'; +import { Z } from 'zod-class'; + +export class ImportWorkflowFromUrlDto extends Z.class({ + url: z.string().url().endsWith('.json'), +}) {} diff --git a/packages/@n8n/config/src/configs/logging.config.ts b/packages/@n8n/config/src/configs/logging.config.ts index ef4661c115..733278c3e3 100644 --- a/packages/@n8n/config/src/configs/logging.config.ts +++ b/packages/@n8n/config/src/configs/logging.config.ts @@ -9,6 +9,7 @@ export const LOG_SCOPES = [ 'multi-main-setup', 'pruning', 'pubsub', + 'push', 'redis', 'scaling', 'waiting-executions', @@ -70,10 +71,13 @@ export class LoggingConfig { * - `external-secrets` * - `license` * - `multi-main-setup` + * - `pruning` * - `pubsub` + * - `push` * - `redis` * - `scaling` * - `waiting-executions` + * - `task-runner` * * @example * `N8N_LOG_SCOPES=license` diff --git a/packages/@n8n/nodes-langchain/utils/descriptions.ts b/packages/@n8n/nodes-langchain/utils/descriptions.ts index e629bab812..a90b27bb6c 100644 --- a/packages/@n8n/nodes-langchain/utils/descriptions.ts +++ b/packages/@n8n/nodes-langchain/utils/descriptions.ts @@ -66,7 +66,7 @@ export const inputSchemaField: INodeProperties = { }; export const promptTypeOptions: INodeProperties = { - displayName: 'Prompt Source (User Message)', + displayName: 'Source for Prompt (User Message)', name: 'promptType', type: 'options', options: [ @@ -98,7 +98,7 @@ export const textInput: INodeProperties = { }; export const textFromPreviousNode: INodeProperties = { - displayName: 'Text From Previous Node', + displayName: 'Prompt (User Message)', name: 'text', type: 'string', required: true, diff --git a/packages/@n8n/task-runner/src/js-task-runner/__tests__/js-task-runner.test.ts b/packages/@n8n/task-runner/src/js-task-runner/__tests__/js-task-runner.test.ts index 00715621b2..5ebd965e87 100644 --- a/packages/@n8n/task-runner/src/js-task-runner/__tests__/js-task-runner.test.ts +++ b/packages/@n8n/task-runner/src/js-task-runner/__tests__/js-task-runner.test.ts @@ -1,4 +1,3 @@ -import { mock } from 'jest-mock-extended'; import { DateTime } from 'luxon'; import type { IBinaryData } from 'n8n-workflow'; import { setGlobalState, type CodeExecutionMode, type IDataObject } from 'n8n-workflow'; @@ -18,11 +17,12 @@ import { type DataRequestResponse, type InputDataChunkDefinition, } from '@/runner-types'; -import type { Task } from '@/task-runner'; +import type { TaskParams } from '@/task-runner'; import { newDataRequestResponse, - newTaskWithSettings, + newTaskParamsWithSettings, + newTaskState, withPairedItem, wrapIntoJson, } from './test-data'; @@ -64,12 +64,12 @@ describe('JsTaskRunner', () => { taskData, runner = defaultTaskRunner, }: { - task: Task; + task: TaskParams; taskData: DataRequestResponse; runner?: JsTaskRunner; }) => { jest.spyOn(runner, 'requestData').mockResolvedValue(taskData); - return await runner.executeTask(task, mock()); + return await runner.executeTask(task, new AbortController().signal); }; afterEach(() => { @@ -88,7 +88,7 @@ describe('JsTaskRunner', () => { runner?: JsTaskRunner; }) => { return await execTaskWithParams({ - task: newTaskWithSettings({ + task: newTaskParamsWithSettings({ code, nodeMode: 'runOnceForAllItems', ...settings, @@ -112,7 +112,7 @@ describe('JsTaskRunner', () => { chunk?: InputDataChunkDefinition; }) => { return await execTaskWithParams({ - task: newTaskWithSettings({ + task: newTaskParamsWithSettings({ code, nodeMode: 'runOnceForEachItem', chunk, @@ -128,7 +128,7 @@ describe('JsTaskRunner', () => { 'should make an rpc call for console log in %s mode', async (nodeMode) => { jest.spyOn(defaultTaskRunner, 'makeRpcCall').mockResolvedValue(undefined); - const task = newTaskWithSettings({ + const task = newTaskParamsWithSettings({ code: "console.log('Hello', 'world!'); return {}", nodeMode, }); @@ -146,7 +146,7 @@ describe('JsTaskRunner', () => { ); it('should not throw when using unsupported console methods', async () => { - const task = newTaskWithSettings({ + const task = newTaskParamsWithSettings({ code: ` console.warn('test'); console.error('test'); @@ -176,7 +176,7 @@ describe('JsTaskRunner', () => { }); it('should not throw when trying to log the context object', async () => { - const task = newTaskWithSettings({ + const task = newTaskParamsWithSettings({ code: ` console.log(this); return {json: {}} @@ -195,7 +195,7 @@ describe('JsTaskRunner', () => { it('should log the context object as [[ExecutionContext]]', async () => { const rpcCallSpy = jest.spyOn(defaultTaskRunner, 'makeRpcCall').mockResolvedValue(undefined); - const task = newTaskWithSettings({ + const task = newTaskParamsWithSettings({ code: ` console.log(this); return {json: {}} @@ -336,7 +336,7 @@ describe('JsTaskRunner', () => { describe('$env', () => { it('should have the env available in context when access has not been blocked', async () => { const outcome = await execTaskWithParams({ - task: newTaskWithSettings({ + task: newTaskParamsWithSettings({ code: 'return { val: $env.VAR1 }', nodeMode: 'runOnceForAllItems', }), @@ -355,7 +355,7 @@ describe('JsTaskRunner', () => { it('should be possible to access env if it has been blocked', async () => { await expect( execTaskWithParams({ - task: newTaskWithSettings({ + task: newTaskParamsWithSettings({ code: 'return { val: $env.VAR1 }', nodeMode: 'runOnceForAllItems', }), @@ -372,7 +372,7 @@ describe('JsTaskRunner', () => { it('should not be possible to iterate $env', async () => { const outcome = await execTaskWithParams({ - task: newTaskWithSettings({ + task: newTaskParamsWithSettings({ code: 'return Object.values($env).concat(Object.keys($env))', nodeMode: 'runOnceForAllItems', }), @@ -391,7 +391,7 @@ describe('JsTaskRunner', () => { it("should not expose task runner's env variables even if no env state is received", async () => { process.env.N8N_RUNNERS_TASK_BROKER_URI = 'http://127.0.0.1:5679'; const outcome = await execTaskWithParams({ - task: newTaskWithSettings({ + task: newTaskParamsWithSettings({ code: 'return { val: $env.N8N_RUNNERS_TASK_BROKER_URI }', nodeMode: 'runOnceForAllItems', }), @@ -412,7 +412,7 @@ describe('JsTaskRunner', () => { }; const outcome = await execTaskWithParams({ - task: newTaskWithSettings({ + task: newTaskParamsWithSettings({ code: 'return { val: $now.toSeconds() }', nodeMode: 'runOnceForAllItems', }), @@ -429,7 +429,7 @@ describe('JsTaskRunner', () => { }); const outcome = await execTaskWithParams({ - task: newTaskWithSettings({ + task: newTaskParamsWithSettings({ code: 'return { val: $now.toSeconds() }', nodeMode: 'runOnceForAllItems', }), @@ -444,7 +444,7 @@ describe('JsTaskRunner', () => { describe("$getWorkflowStaticData('global')", () => { it('should have the global workflow static data available in runOnceForAllItems', async () => { const outcome = await execTaskWithParams({ - task: newTaskWithSettings({ + task: newTaskParamsWithSettings({ code: 'return { val: $getWorkflowStaticData("global") }', nodeMode: 'runOnceForAllItems', }), @@ -460,7 +460,7 @@ describe('JsTaskRunner', () => { it('should have the global workflow static data available in runOnceForEachItem', async () => { const outcome = await execTaskWithParams({ - task: newTaskWithSettings({ + task: newTaskParamsWithSettings({ code: 'return { val: $getWorkflowStaticData("global") }', nodeMode: 'runOnceForEachItem', }), @@ -480,7 +480,7 @@ describe('JsTaskRunner', () => { "does not return static data if it hasn't been modified in %s", async (mode) => { const outcome = await execTaskWithParams({ - task: newTaskWithSettings({ + task: newTaskParamsWithSettings({ code: ` const staticData = $getWorkflowStaticData("global"); return { val: staticData }; @@ -502,7 +502,7 @@ describe('JsTaskRunner', () => { 'returns the updated static data in %s', async (mode) => { const outcome = await execTaskWithParams({ - task: newTaskWithSettings({ + task: newTaskParamsWithSettings({ code: ` const staticData = $getWorkflowStaticData("global"); staticData.newKey = 'newValue'; @@ -541,7 +541,7 @@ describe('JsTaskRunner', () => { it('should have the node workflow static data available in runOnceForAllItems', async () => { const outcome = await execTaskWithParams({ - task: newTaskWithSettings({ + task: newTaskParamsWithSettings({ code: 'return { val: $getWorkflowStaticData("node") }', nodeMode: 'runOnceForAllItems', }), @@ -553,7 +553,7 @@ describe('JsTaskRunner', () => { it('should have the node workflow static data available in runOnceForEachItem', async () => { const outcome = await execTaskWithParams({ - task: newTaskWithSettings({ + task: newTaskParamsWithSettings({ code: 'return { val: $getWorkflowStaticData("node") }', nodeMode: 'runOnceForEachItem', }), @@ -569,7 +569,7 @@ describe('JsTaskRunner', () => { "does not return static data if it hasn't been modified in %s", async (mode) => { const outcome = await execTaskWithParams({ - task: newTaskWithSettings({ + task: newTaskParamsWithSettings({ code: ` const staticData = $getWorkflowStaticData("node"); return { val: staticData }; @@ -587,7 +587,7 @@ describe('JsTaskRunner', () => { 'returns the updated static data in %s', async (mode) => { const outcome = await execTaskWithParams({ - task: newTaskWithSettings({ + task: newTaskParamsWithSettings({ code: ` const staticData = $getWorkflowStaticData("node"); staticData.newKey = 'newValue'; @@ -662,7 +662,7 @@ describe('JsTaskRunner', () => { // Act await execTaskWithParams({ - task: newTaskWithSettings({ + task: newTaskParamsWithSettings({ code: `await ${group.invocation}; return []`, nodeMode: 'runOnceForAllItems', }), @@ -684,7 +684,7 @@ describe('JsTaskRunner', () => { // Act await execTaskWithParams({ - task: newTaskWithSettings({ + task: newTaskParamsWithSettings({ code: `await ${group.invocation}; return {}`, nodeMode: 'runOnceForEachItem', }), @@ -725,7 +725,7 @@ describe('JsTaskRunner', () => { it('should allow access to Node.js Buffers', async () => { const outcomeAll = await execTaskWithParams({ - task: newTaskWithSettings({ + task: newTaskParamsWithSettings({ code: 'return { val: Buffer.from("test-buffer").toString() }', nodeMode: 'runOnceForAllItems', }), @@ -737,7 +737,7 @@ describe('JsTaskRunner', () => { expect(outcomeAll.result).toEqual([wrapIntoJson({ val: 'test-buffer' })]); const outcomePer = await execTaskWithParams({ - task: newTaskWithSettings({ + task: newTaskParamsWithSettings({ code: 'return { val: Buffer.from("test-buffer").toString() }', nodeMode: 'runOnceForEachItem', }), @@ -1205,7 +1205,7 @@ describe('JsTaskRunner', () => { async (nodeMode) => { await expect( execTaskWithParams({ - task: newTaskWithSettings({ + task: newTaskParamsWithSettings({ code: 'unknown', nodeMode, }), @@ -1218,12 +1218,13 @@ describe('JsTaskRunner', () => { it('sends serializes an error correctly', async () => { const runner = createRunnerWithOpts({}); const taskId = '1'; - const task = newTaskWithSettings({ + const task = newTaskState(taskId); + const taskSettings: JSExecSettings = { code: 'unknown; return []', nodeMode: 'runOnceForAllItems', continueOnFail: false, workflowMode: 'manual', - }); + }; runner.runningTasks.set(taskId, task); const sendSpy = jest.spyOn(runner.ws, 'send').mockImplementation(() => {}); @@ -1232,7 +1233,7 @@ describe('JsTaskRunner', () => { .spyOn(runner, 'requestData') .mockResolvedValue(newDataRequestResponse([wrapIntoJson({ a: 1 })])); - await runner.receivedSettings(taskId, task.settings); + await runner.receivedSettings(taskId, taskSettings); expect(sendSpy).toHaveBeenCalled(); const calledWith = sendSpy.mock.calls[0][0] as string; @@ -1304,11 +1305,7 @@ describe('JsTaskRunner', () => { const emitSpy = jest.spyOn(runner, 'emit'); jest.spyOn(runner, 'executeTask').mockResolvedValue({ result: [] }); - runner.runningTasks.set(taskId, { - taskId, - active: true, - cancelled: false, - }); + runner.runningTasks.set(taskId, newTaskState(taskId)); jest.advanceTimersByTime(idleTimeout * 1000 - 100); expect(emitSpy).not.toHaveBeenCalledWith('runner:reached-idle-timeout'); @@ -1335,15 +1332,13 @@ describe('JsTaskRunner', () => { const runner = createRunnerWithOpts({}, { idleTimeout }); const taskId = '123'; const emitSpy = jest.spyOn(runner, 'emit'); + const task = newTaskState(taskId); - runner.runningTasks.set(taskId, { - taskId, - active: true, - cancelled: false, - }); + runner.runningTasks.set(taskId, task); jest.advanceTimersByTime(idleTimeout * 1000); expect(emitSpy).not.toHaveBeenCalledWith('runner:reached-idle-timeout'); + task.cleanup(); }); }); }); diff --git a/packages/@n8n/task-runner/src/js-task-runner/__tests__/task-runner.test.ts b/packages/@n8n/task-runner/src/js-task-runner/__tests__/task-runner.test.ts index ef1d65a737..9db385eb7f 100644 --- a/packages/@n8n/task-runner/src/js-task-runner/__tests__/task-runner.test.ts +++ b/packages/@n8n/task-runner/src/js-task-runner/__tests__/task-runner.test.ts @@ -1,6 +1,9 @@ import { WebSocket } from 'ws'; +import { newTaskState } from '@/js-task-runner/__tests__/test-data'; +import { TimeoutError } from '@/js-task-runner/errors/timeout-error'; import { TaskRunner, type TaskRunnerOpts } from '@/task-runner'; +import type { TaskStatus } from '@/task-state'; class TestRunner extends TaskRunner {} @@ -154,11 +157,8 @@ describe('TestRunner', () => { runner.onMessage({ type: 'broker:runnerregistered', }); - runner.runningTasks.set('test-task', { - taskId: 'test-task', - active: true, - cancelled: false, - }); + const taskState = newTaskState('test-task'); + runner.runningTasks.set('test-task', taskState); const sendSpy = jest.spyOn(runner, 'send'); runner.sendOffers(); @@ -174,6 +174,7 @@ describe('TestRunner', () => { }, ], ]); + taskState.cleanup(); }); it('should delete stale offers and send new ones', () => { @@ -198,15 +199,45 @@ describe('TestRunner', () => { }); describe('taskCancelled', () => { - it('should reject pending requests when task is cancelled', () => { - const runner = newTestRunner(); + test.each<[TaskStatus, string]>([ + ['aborting:cancelled', 'cancelled'], + ['aborting:timeout', 'timeout'], + ])('should not do anything if task status is %s', async (status, reason) => { + runner = newTestRunner(); const taskId = 'test-task'; - runner.runningTasks.set(taskId, { - taskId, - active: false, - cancelled: false, - }); + const task = newTaskState(taskId); + task.status = status; + + runner.runningTasks.set(taskId, task); + + await runner.taskCancelled(taskId, reason); + + expect(runner.runningTasks.size).toBe(1); + expect(task.status).toBe(status); + }); + + it('should delete task if task is waiting for settings when task is cancelled', async () => { + runner = newTestRunner(); + + const taskId = 'test-task'; + const task = newTaskState(taskId); + const taskCleanupSpy = jest.spyOn(task, 'cleanup'); + runner.runningTasks.set(taskId, task); + + await runner.taskCancelled(taskId, 'test-reason'); + + expect(runner.runningTasks.size).toBe(0); + expect(taskCleanupSpy).toHaveBeenCalled(); + }); + + it('should reject pending requests when task is cancelled', async () => { + runner = newTestRunner(); + + const taskId = 'test-task'; + const task = newTaskState(taskId); + task.status = 'running'; + runner.runningTasks.set(taskId, task); const dataRequestReject = jest.fn(); const nodeTypesRequestReject = jest.fn(); @@ -225,7 +256,71 @@ describe('TestRunner', () => { reject: nodeTypesRequestReject, }); - runner.taskCancelled(taskId, 'test-reason'); + await runner.taskCancelled(taskId, 'test-reason'); + + expect(dataRequestReject).toHaveBeenCalledWith( + expect.objectContaining({ + message: 'Task cancelled: test-reason', + }), + ); + + expect(nodeTypesRequestReject).toHaveBeenCalledWith( + expect.objectContaining({ + message: 'Task cancelled: test-reason', + }), + ); + + expect(runner.dataRequests.size).toBe(0); + expect(runner.nodeTypesRequests.size).toBe(0); + }); + }); + + describe('taskTimedOut', () => { + it('should error task if task is waiting for settings', async () => { + runner = newTestRunner(); + + const taskId = 'test-task'; + const task = newTaskState(taskId); + task.status = 'waitingForSettings'; + runner.runningTasks.set(taskId, task); + const sendSpy = jest.spyOn(runner, 'send'); + + await runner.taskTimedOut(taskId); + + expect(runner.runningTasks.size).toBe(0); + expect(sendSpy).toHaveBeenCalledWith({ + type: 'runner:taskerror', + taskId, + error: expect.any(TimeoutError), + }); + }); + + it('should reject pending requests when task is running', async () => { + runner = newTestRunner(); + + const taskId = 'test-task'; + const task = newTaskState(taskId); + task.status = 'running'; + runner.runningTasks.set(taskId, task); + + const dataRequestReject = jest.fn(); + const nodeTypesRequestReject = jest.fn(); + + runner.dataRequests.set('data-req', { + taskId, + requestId: 'data-req', + resolve: jest.fn(), + reject: dataRequestReject, + }); + + runner.nodeTypesRequests.set('node-req', { + taskId, + requestId: 'node-req', + resolve: jest.fn(), + reject: nodeTypesRequestReject, + }); + + await runner.taskCancelled(taskId, 'test-reason'); expect(dataRequestReject).toHaveBeenCalledWith( expect.objectContaining({ diff --git a/packages/@n8n/task-runner/src/js-task-runner/__tests__/test-data.ts b/packages/@n8n/task-runner/src/js-task-runner/__tests__/test-data.ts index f13939e51e..85a1235dc6 100644 --- a/packages/@n8n/task-runner/src/js-task-runner/__tests__/test-data.ts +++ b/packages/@n8n/task-runner/src/js-task-runner/__tests__/test-data.ts @@ -4,22 +4,21 @@ import { nanoid } from 'nanoid'; import type { JSExecSettings } from '@/js-task-runner/js-task-runner'; import type { DataRequestResponse } from '@/runner-types'; -import type { Task } from '@/task-runner'; +import type { TaskParams } from '@/task-runner'; +import { TaskState } from '@/task-state'; /** * Creates a new task with the given settings */ -export const newTaskWithSettings = ( +export const newTaskParamsWithSettings = ( settings: Partial & Pick, -): Task => ({ +): TaskParams => ({ taskId: '1', settings: { workflowMode: 'manual', continueOnFail: false, ...settings, }, - active: true, - cancelled: false, }); /** @@ -167,3 +166,13 @@ export const withPairedItem = (index: number, data: INodeExecutionData): INodeEx item: index, }, }); + +/** + * Creates a new task state with the given taskId + */ +export const newTaskState = (taskId: string) => + new TaskState({ + taskId, + timeoutInS: 60, + onTimeout: () => {}, + }); diff --git a/packages/@n8n/task-runner/src/js-task-runner/js-task-runner.ts b/packages/@n8n/task-runner/src/js-task-runner/js-task-runner.ts index db93f3c338..ab2cc3a304 100644 --- a/packages/@n8n/task-runner/src/js-task-runner/js-task-runner.ts +++ b/packages/@n8n/task-runner/src/js-task-runner/js-task-runner.ts @@ -23,15 +23,15 @@ import { runInNewContext, type Context } from 'node:vm'; import type { MainConfig } from '@/config/main-config'; import { UnsupportedFunctionError } from '@/js-task-runner/errors/unsupported-function.error'; -import { - EXPOSED_RPC_METHODS, - UNSUPPORTED_HELPER_FUNCTIONS, - type DataRequestResponse, - type InputDataChunkDefinition, - type PartialAdditionalData, - type TaskResultData, +import { EXPOSED_RPC_METHODS, UNSUPPORTED_HELPER_FUNCTIONS } from '@/runner-types'; +import type { + DataRequestResponse, + InputDataChunkDefinition, + PartialAdditionalData, + TaskResultData, } from '@/runner-types'; -import { type Task, TaskRunner } from '@/task-runner'; +import type { TaskParams } from '@/task-runner'; +import { noOp, TaskRunner } from '@/task-runner'; import { BuiltInsParser } from './built-ins-parser/built-ins-parser'; import { BuiltInsParserState } from './built-ins-parser/built-ins-parser-state'; @@ -81,8 +81,6 @@ type CustomConsole = { log: (...args: unknown[]) => void; }; -const noOp = () => {}; - export class JsTaskRunner extends TaskRunner { private readonly requireResolver: RequireResolver; @@ -107,8 +105,11 @@ export class JsTaskRunner extends TaskRunner { }); } - async executeTask(task: Task, signal: AbortSignal): Promise { - const settings = task.settings; + async executeTask( + taskParams: TaskParams, + abortSignal: AbortSignal, + ): Promise { + const { taskId, settings } = taskParams; a.ok(settings, 'JS Code not sent to runner'); this.validateTaskSettings(settings); @@ -119,13 +120,13 @@ export class JsTaskRunner extends TaskRunner { : BuiltInsParserState.newNeedsAllDataState(); const dataResponse = await this.requestData( - task.taskId, + taskId, neededBuiltIns.toDataRequestParams(settings.chunk), ); const data = this.reconstructTaskData(dataResponse, settings.chunk); - await this.requestNodeTypeIfNeeded(neededBuiltIns, data.workflow, task.taskId); + await this.requestNodeTypeIfNeeded(neededBuiltIns, data.workflow, taskId); const workflowParams = data.workflow; const workflow = new Workflow({ @@ -137,8 +138,8 @@ export class JsTaskRunner extends TaskRunner { const result = settings.nodeMode === 'runOnceForAllItems' - ? await this.runForAllItems(task.taskId, settings, data, workflow, signal) - : await this.runForEachItem(task.taskId, settings, data, workflow, signal); + ? await this.runForAllItems(taskId, settings, data, workflow, abortSignal) + : await this.runForEachItem(taskId, settings, data, workflow, abortSignal); return { result, diff --git a/packages/@n8n/task-runner/src/task-runner.ts b/packages/@n8n/task-runner/src/task-runner.ts index 78ec83961c..dd048bcf7e 100644 --- a/packages/@n8n/task-runner/src/task-runner.ts +++ b/packages/@n8n/task-runner/src/task-runner.ts @@ -5,19 +5,14 @@ import { EventEmitter } from 'node:events'; import { type MessageEvent, WebSocket } from 'ws'; import type { BaseRunnerConfig } from '@/config/base-runner-config'; +import { TimeoutError } from '@/js-task-runner/errors/timeout-error'; import type { BrokerMessage, RunnerMessage } from '@/message-types'; import { TaskRunnerNodeTypes } from '@/node-types'; import type { TaskResultData } from '@/runner-types'; +import { TaskState } from '@/task-state'; import { TaskCancelledError } from './js-task-runner/errors/task-cancelled-error'; -export interface Task { - taskId: string; - settings?: T; - active: boolean; - cancelled: boolean; -} - export interface TaskOffer { offerId: string; validUntil: bigint; @@ -49,6 +44,14 @@ const OFFER_VALID_EXTRA_MS = 100; /** Converts milliseconds to nanoseconds */ const msToNs = (ms: number) => BigInt(ms * 1_000_000); +export const noOp = () => {}; + +/** Params the task receives when it is executed */ +export interface TaskParams { + taskId: string; + settings: T; +} + export interface TaskRunnerOpts extends BaseRunnerConfig { taskType: string; name?: string; @@ -61,7 +64,7 @@ export abstract class TaskRunner extends EventEmitter { canSendOffers = false; - runningTasks: Map = new Map(); + runningTasks: Map = new Map(); offerInterval: NodeJS.Timeout | undefined; @@ -89,10 +92,9 @@ export abstract class TaskRunner extends EventEmitter { /** How long (in seconds) a runner may be idle for before exit. */ private readonly idleTimeout: number; - protected taskCancellations = new Map(); - constructor(opts: TaskRunnerOpts) { super(); + this.taskType = opts.taskType; this.name = opts.name ?? 'Node.js Task Runner SDK'; this.maxConcurrency = opts.maxConcurrency; @@ -219,7 +221,7 @@ export abstract class TaskRunner extends EventEmitter { this.offerAccepted(message.offerId, message.taskId); break; case 'broker:taskcancel': - this.taskCancelled(message.taskId, message.reason); + void this.taskCancelled(message.taskId, message.reason); break; case 'broker:tasksettings': void this.receivedSettings(message.taskId, message.settings); @@ -284,11 +286,14 @@ export abstract class TaskRunner extends EventEmitter { } this.resetIdleTimer(); - this.runningTasks.set(taskId, { + const taskState = new TaskState({ taskId, - active: false, - cancelled: false, + timeoutInS: this.taskTimeout, + onTimeout: () => { + void this.taskTimedOut(taskId); + }, }); + this.runningTasks.set(taskId, taskState); this.send({ type: 'runner:taskaccepted', @@ -296,99 +301,103 @@ export abstract class TaskRunner extends EventEmitter { }); } - taskCancelled(taskId: string, reason: string) { - const task = this.runningTasks.get(taskId); - if (!task) { + async taskCancelled(taskId: string, reason: string) { + const taskState = this.runningTasks.get(taskId); + if (!taskState) { return; } - task.cancelled = true; - for (const [requestId, request] of this.dataRequests.entries()) { - if (request.taskId === taskId) { - request.reject(new TaskCancelledError(reason)); - this.dataRequests.delete(requestId); - } - } + await taskState.caseOf({ + // If the cancelled task hasn't received settings yet, we can finish it + waitingForSettings: () => this.finishTask(taskState), - for (const [requestId, request] of this.nodeTypesRequests.entries()) { - if (request.taskId === taskId) { - request.reject(new TaskCancelledError(reason)); - this.nodeTypesRequests.delete(requestId); - } - } + // If the task has already timed out or is already cancelled, we can + // ignore the cancellation + 'aborting:timeout': noOp, + 'aborting:cancelled': noOp, - const controller = this.taskCancellations.get(taskId); - if (controller) { - controller.abort(); - this.taskCancellations.delete(taskId); - } - - if (!task.active) this.runningTasks.delete(taskId); - - this.sendOffers(); + running: () => { + taskState.status = 'aborting:cancelled'; + taskState.abortController.abort('cancelled'); + this.cancelTaskRequests(taskId, reason); + }, + }); } - taskErrored(taskId: string, error: unknown) { - this.send({ - type: 'runner:taskerror', - taskId, - error, - }); - this.runningTasks.delete(taskId); - this.sendOffers(); - } + async taskTimedOut(taskId: string) { + const taskState = this.runningTasks.get(taskId); + if (!taskState) { + return; + } - taskDone(taskId: string, data: RunnerMessage.ToBroker.TaskDone['data']) { - this.send({ - type: 'runner:taskdone', - taskId, - data, + await taskState.caseOf({ + // If we are still waiting for settings for the task, we can error the + // task immediately + waitingForSettings: () => { + try { + this.send({ + type: 'runner:taskerror', + taskId, + error: new TimeoutError(this.taskTimeout), + }); + } finally { + this.finishTask(taskState); + } + }, + + // This should never happen, the timeout timer should only fire once + 'aborting:timeout': TaskState.throwUnexpectedTaskStatus, + + // If we are currently executing the task, abort the execution and + // mark the task as timed out + running: () => { + taskState.status = 'aborting:timeout'; + taskState.abortController.abort('timeout'); + this.cancelTaskRequests(taskId, 'timeout'); + }, + + // If the task is already cancelling, we can ignore the timeout + 'aborting:cancelled': noOp, }); - this.runningTasks.delete(taskId); - this.sendOffers(); } async receivedSettings(taskId: string, settings: unknown) { - const task = this.runningTasks.get(taskId); - if (!task) { - return; - } - if (task.cancelled) { - this.runningTasks.delete(taskId); + const taskState = this.runningTasks.get(taskId); + if (!taskState) { return; } - const controller = new AbortController(); - this.taskCancellations.set(taskId, controller); + await taskState.caseOf({ + // These states should never happen, as they are handled already in + // the other lifecycle methods and the task should be removed from the + // running tasks + 'aborting:cancelled': TaskState.throwUnexpectedTaskStatus, + 'aborting:timeout': TaskState.throwUnexpectedTaskStatus, + running: TaskState.throwUnexpectedTaskStatus, - const taskTimeout = setTimeout(() => { - if (!task.cancelled) { - controller.abort(); - this.taskCancellations.delete(taskId); - } - }, this.taskTimeout * 1_000); + waitingForSettings: async () => { + taskState.status = 'running'; - task.settings = settings; - task.active = true; - try { - const data = await this.executeTask(task, controller.signal); - this.taskDone(taskId, data); - } catch (error) { - if (!task.cancelled) this.taskErrored(taskId, error); - } finally { - clearTimeout(taskTimeout); - this.taskCancellations.delete(taskId); - this.resetIdleTimer(); - } + await this.executeTask( + { + taskId, + settings, + }, + taskState.abortController.signal, + ) + .then(async (data) => await this.taskExecutionSucceeded(taskState, data)) + .catch(async (error) => await this.taskExecutionFailed(taskState, error)); + }, + }); } // eslint-disable-next-line @typescript-eslint/naming-convention - async executeTask(_task: Task, _signal: AbortSignal): Promise { + async executeTask(_taskParams: TaskParams, _signal: AbortSignal): Promise { throw new ApplicationError('Unimplemented'); } async requestNodeTypes( - taskId: Task['taskId'], + taskId: TaskState['taskId'], requestParams: RunnerMessage.ToBroker.NodeTypesRequest['requestParams'], ) { const requestId = nanoid(); @@ -417,12 +426,12 @@ export abstract class TaskRunner extends EventEmitter { } async requestData( - taskId: Task['taskId'], + taskId: TaskState['taskId'], requestParams: RunnerMessage.ToBroker.TaskDataRequest['requestParams'], ): Promise { const requestId = nanoid(); - const p = new Promise((resolve, reject) => { + const dataRequestPromise = new Promise((resolve, reject) => { this.dataRequests.set(requestId, { requestId, taskId, @@ -439,7 +448,7 @@ export abstract class TaskRunner extends EventEmitter { }); try { - return await p; + return await dataRequestPromise; } finally { this.dataRequests.delete(requestId); } @@ -527,4 +536,86 @@ export abstract class TaskRunner extends EventEmitter { await new Promise((resolve) => setTimeout(resolve, 100)); } } + + private async taskExecutionSucceeded(taskState: TaskState, data: TaskResultData) { + try { + const sendData = () => { + this.send({ + type: 'runner:taskdone', + taskId: taskState.taskId, + data, + }); + }; + + await taskState.caseOf({ + waitingForSettings: TaskState.throwUnexpectedTaskStatus, + + 'aborting:cancelled': noOp, + + // If the task timed out but we ended up reaching this point, we + // might as well send the data + 'aborting:timeout': sendData, + running: sendData, + }); + } finally { + this.finishTask(taskState); + } + } + + private async taskExecutionFailed(taskState: TaskState, error: unknown) { + try { + const sendError = () => { + this.send({ + type: 'runner:taskerror', + taskId: taskState.taskId, + error, + }); + }; + + await taskState.caseOf({ + waitingForSettings: TaskState.throwUnexpectedTaskStatus, + + 'aborting:cancelled': noOp, + + 'aborting:timeout': () => { + console.warn(`Task ${taskState.taskId} timed out`); + + sendError(); + }, + + running: sendError, + }); + } finally { + this.finishTask(taskState); + } + } + + /** + * Cancels all node type and data requests made by the given task + */ + private cancelTaskRequests(taskId: string, reason: string) { + for (const [requestId, request] of this.dataRequests.entries()) { + if (request.taskId === taskId) { + request.reject(new TaskCancelledError(reason)); + this.dataRequests.delete(requestId); + } + } + + for (const [requestId, request] of this.nodeTypesRequests.entries()) { + if (request.taskId === taskId) { + request.reject(new TaskCancelledError(reason)); + this.nodeTypesRequests.delete(requestId); + } + } + } + + /** + * Finishes task by removing it from the running tasks and sending new offers + */ + private finishTask(taskState: TaskState) { + taskState.cleanup(); + this.runningTasks.delete(taskState.taskId); + this.sendOffers(); + this.resetIdleTimer(); + } } diff --git a/packages/@n8n/task-runner/src/task-state.ts b/packages/@n8n/task-runner/src/task-state.ts new file mode 100644 index 0000000000..4c2c0e44a8 --- /dev/null +++ b/packages/@n8n/task-runner/src/task-state.ts @@ -0,0 +1,118 @@ +import * as a from 'node:assert'; + +export type TaskStatus = + | 'waitingForSettings' + | 'running' + | 'aborting:cancelled' + | 'aborting:timeout'; + +export type TaskStateOpts = { + taskId: string; + timeoutInS: number; + onTimeout: () => void; +}; + +/** + * The state of a task. The task can be in one of the following states: + * - waitingForSettings: The task is waiting for settings from the broker + * - running: The task is currently running + * - aborting:cancelled: The task was canceled by the broker and is being aborted + * - aborting:timeout: The task took too long to complete and is being aborted + * + * The task is discarded once it reaches an end state. + * + * The class only holds the state, and does not have any logic. + * + * The task has the following lifecycle: + * + * ┌───┐ + * └───┘ + * │ + * broker:taskofferaccept : create task state + * │ + * ▼ + * ┌────────────────────┐ broker:taskcancel / timeout + * │ waitingForSettings ├──────────────────────────────────┐ + * └────────┬───────────┘ │ + * │ │ + * broker:tasksettings │ + * │ │ + * ▼ │ + * ┌───────────────┐ ┌────────────────────┐ │ + * │ running │ │ aborting:timeout │ │ + * │ │ timeout │ │ │ + * ┌───────┤- execute task ├───────────►│- fire abort signal │ │ + * │ └──────┬────────┘ └──────────┬─────────┘ │ + * │ │ │ │ + * │ broker:taskcancel │ │ + * Task execution │ Task execution │ + * resolves / rejects │ resolves / rejects │ + * │ ▼ │ │ + * │ ┌─────────────────────┐ │ │ + * │ │ aborting:cancelled │ │ │ + * │ │ │ │ │ + * │ │- fire abort signal │ │ │ + * │ └──────────┬──────────┘ │ │ + * │ Task execution │ │ + * │ resolves / rejects │ │ + * │ │ │ │ + * │ ▼ │ │ + * │ ┌──┐ │ │ + * └─────────────►│ │◄────────────────────────────┴─────────────┘ + * └──┘ + */ +export class TaskState { + status: TaskStatus = 'waitingForSettings'; + + readonly taskId: string; + + /** Controller for aborting the execution of the task */ + readonly abortController = new AbortController(); + + /** Timeout timer for the task */ + private timeoutTimer: NodeJS.Timeout | undefined; + + constructor(opts: TaskStateOpts) { + this.taskId = opts.taskId; + this.timeoutTimer = setTimeout(opts.onTimeout, opts.timeoutInS * 1000); + } + + /** Cleans up any resources before the task can be removed */ + cleanup() { + clearTimeout(this.timeoutTimer); + this.timeoutTimer = undefined; + } + + /** Custom JSON serialization for the task state for logging purposes */ + toJSON() { + return `[Task ${this.taskId} (${this.status})]`; + } + + /** + * Executes the function matching the current task status + * + * @example + * ```ts + * taskState.caseOf({ + * waitingForSettings: () => {...}, + * running: () => {...}, + * aborting:cancelled: () => {...}, + * aborting:timeout: () => {...}, + * }); + * ``` + */ + async caseOf( + conditions: Record void | Promise | never>, + ) { + if (!conditions[this.status]) { + TaskState.throwUnexpectedTaskStatus(this); + } + + return await conditions[this.status](this); + } + + /** Throws an error that the task status is unexpected */ + static throwUnexpectedTaskStatus = (taskState: TaskState) => { + a.fail(`Unexpected task status: ${JSON.stringify(taskState)}`); + }; +} diff --git a/packages/cli/src/constants.ts b/packages/cli/src/constants.ts index 6d742ecc8c..4bd1890c4e 100644 --- a/packages/cli/src/constants.ts +++ b/packages/cli/src/constants.ts @@ -1,6 +1,7 @@ import { readFileSync } from 'fs'; import type { n8n } from 'n8n-core'; -import { jsonParse } from 'n8n-workflow'; +import type { ITaskDataConnections } from 'n8n-workflow'; +import { jsonParse, TRIMMED_TASK_DATA_CONNECTIONS_KEY } from 'n8n-workflow'; import { resolve, join, dirname } from 'path'; const { NODE_ENV, E2E_TESTS } = process.env; @@ -161,6 +162,22 @@ export const ARTIFICIAL_TASK_DATA = { ], }; +/** + * Connections for an item standing in for a manual execution data item too + * large to be sent live via pubsub. This signals to the client to direct the + * user to the execution history. + */ +export const TRIMMED_TASK_DATA_CONNECTIONS: ITaskDataConnections = { + main: [ + [ + { + json: { [TRIMMED_TASK_DATA_CONNECTIONS_KEY]: true }, + pairedItem: undefined, + }, + ], + ], +}; + /** Lowest priority, meaning shut down happens after other groups */ export const LOWEST_SHUTDOWN_PRIORITY = 0; export const DEFAULT_SHUTDOWN_PRIORITY = 100; diff --git a/packages/cli/src/events/__tests__/telemetry-event-relay.test.ts b/packages/cli/src/events/__tests__/telemetry-event-relay.test.ts index 4448dbc41e..9b4d8aecd2 100644 --- a/packages/cli/src/events/__tests__/telemetry-event-relay.test.ts +++ b/packages/cli/src/events/__tests__/telemetry-event-relay.test.ts @@ -1,7 +1,8 @@ import type { GlobalConfig } from '@n8n/config'; import { mock } from 'jest-mock-extended'; import { InstanceSettings } from 'n8n-core'; -import type { IWorkflowBase } from 'n8n-workflow'; +import type { INode, INodesGraphResult } from 'n8n-workflow'; +import { NodeApiError, TelemetryHelpers, type IRun, type IWorkflowBase } from 'n8n-workflow'; import { N8N_VERSION } from '@/constants'; import type { WorkflowEntity } from '@/databases/entities/workflow-entity'; @@ -28,6 +29,9 @@ describe('TelemetryEventRelay', () => { mode: 'smtp', }, }, + diagnostics: { + enabled: true, + }, endpoints: { metrics: { enable: true, @@ -1106,4 +1110,393 @@ describe('TelemetryEventRelay', () => { }); }); }); + + describe('workflow post execute events', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + const mockWorkflowBase = mock({ + id: 'workflow123', + name: 'Test Workflow', + active: true, + nodes: [ + { + id: 'node1', + name: 'Start', + type: 'n8n-nodes-base.start', + parameters: {}, + typeVersion: 1, + position: [100, 200], + }, + ], + connections: {}, + createdAt: new Date(), + updatedAt: new Date(), + staticData: {}, + settings: {}, + }); + + it('should not track when workflow has no id', async () => { + const event: RelayEventMap['workflow-post-execute'] = { + workflow: { ...mockWorkflowBase, id: '' }, + executionId: 'execution123', + userId: 'user123', + }; + + eventService.emit('workflow-post-execute', event); + + expect(telemetry.trackWorkflowExecution).not.toHaveBeenCalled(); + }); + + it('should not track when execution status is "waiting"', async () => { + const event: RelayEventMap['workflow-post-execute'] = { + workflow: mockWorkflowBase, + executionId: 'execution123', + userId: 'user123', + runData: { + status: 'waiting', + data: { resultData: {} }, + } as IRun, + }; + + eventService.emit('workflow-post-execute', event); + + expect(telemetry.trackWorkflowExecution).not.toHaveBeenCalled(); + }); + + it('should track successful workflow execution', async () => { + const runData = mock({ + finished: true, + status: 'success', + mode: 'manual', + data: { resultData: {} }, + }); + + const event: RelayEventMap['workflow-post-execute'] = { + workflow: mockWorkflowBase, + executionId: 'execution123', + userId: 'user123', + runData: runData as unknown as IRun, + }; + + eventService.emit('workflow-post-execute', event); + + await flushPromises(); + + expect(telemetry.trackWorkflowExecution).toHaveBeenCalledWith( + expect.objectContaining({ + workflow_id: 'workflow123', + user_id: 'user123', + success: true, + is_manual: true, + execution_mode: 'manual', + }), + ); + }); + + it('should call telemetry.track when manual node execution finished', async () => { + sharedWorkflowRepository.findSharingRole.mockResolvedValue('workflow:editor'); + + const runData = { + status: 'error', + mode: 'manual', + data: { + startData: { + destinationNode: 'OpenAI', + runNodeFilter: ['OpenAI'], + }, + resultData: { + runData: {}, + lastNodeExecuted: 'OpenAI', + error: new NodeApiError( + { + id: '1', + typeVersion: 1, + name: 'Jira', + type: 'n8n-nodes-base.jira', + parameters: {}, + position: [100, 200], + }, + { + message: 'Error message', + description: 'Incorrect API key provided', + httpCode: '401', + stack: '', + }, + { + message: 'Error message', + description: 'Error description', + level: 'warning', + functionality: 'regular', + }, + ), + }, + }, + } as IRun; + + const nodeGraph: INodesGraphResult = { + nodeGraph: { node_types: [], node_connections: [], webhookNodeNames: [] }, + nameIndices: { + Jira: '1', + OpenAI: '1', + }, + } as unknown as INodesGraphResult; + + jest.spyOn(TelemetryHelpers, 'generateNodesGraph').mockImplementation(() => nodeGraph); + + jest + .spyOn(TelemetryHelpers, 'getNodeTypeForName') + .mockImplementation( + () => ({ type: 'n8n-nodes-base.jira', version: 1, name: 'Jira' }) as unknown as INode, + ); + + const event: RelayEventMap['workflow-post-execute'] = { + workflow: mockWorkflowBase, + executionId: 'execution123', + userId: 'user123', + runData, + }; + + eventService.emit('workflow-post-execute', event); + + await flushPromises(); + + expect(telemetry.track).toHaveBeenCalledWith( + 'Manual node exec finished', + expect.objectContaining({ + webhook_domain: null, + user_id: 'user123', + workflow_id: 'workflow123', + status: 'error', + executionStatus: 'error', + sharing_role: 'sharee', + error_message: 'Error message', + error_node_type: 'n8n-nodes-base.jira', + error_node_id: '1', + node_id: '1', + node_type: 'n8n-nodes-base.jira', + node_graph_string: JSON.stringify(nodeGraph.nodeGraph), + }), + ); + + expect(telemetry.trackWorkflowExecution).toHaveBeenCalledWith( + expect.objectContaining({ + workflow_id: 'workflow123', + success: false, + is_manual: true, + execution_mode: 'manual', + version_cli: N8N_VERSION, + error_message: 'Error message', + error_node_type: 'n8n-nodes-base.jira', + node_graph_string: JSON.stringify(nodeGraph.nodeGraph), + error_node_id: '1', + }), + ); + }); + + it('should call telemetry.track when manual node execution finished with canceled error message', async () => { + sharedWorkflowRepository.findSharingRole.mockResolvedValue('workflow:owner'); + + const runData = { + status: 'error', + mode: 'manual', + data: { + startData: { + destinationNode: 'OpenAI', + runNodeFilter: ['OpenAI'], + }, + resultData: { + runData: {}, + lastNodeExecuted: 'OpenAI', + error: new NodeApiError( + { + id: '1', + typeVersion: 1, + name: 'Jira', + type: 'n8n-nodes-base.jira', + parameters: {}, + position: [100, 200], + }, + { + message: 'Error message', + description: 'Incorrect API key provided', + httpCode: '401', + stack: '', + }, + { + message: 'Error message canceled', + description: 'Error description', + level: 'warning', + functionality: 'regular', + }, + ), + }, + }, + } as IRun; + + const nodeGraph: INodesGraphResult = { + nodeGraph: { node_types: [], node_connections: [] }, + nameIndices: { + Jira: '1', + OpenAI: '1', + }, + } as unknown as INodesGraphResult; + + jest.spyOn(TelemetryHelpers, 'generateNodesGraph').mockImplementation(() => nodeGraph); + + jest + .spyOn(TelemetryHelpers, 'getNodeTypeForName') + .mockImplementation( + () => ({ type: 'n8n-nodes-base.jira', version: 1, name: 'Jira' }) as unknown as INode, + ); + + const event: RelayEventMap['workflow-post-execute'] = { + workflow: mockWorkflowBase, + executionId: 'execution123', + userId: 'user123', + runData, + }; + + eventService.emit('workflow-post-execute', event); + + await flushPromises(); + + expect(telemetry.track).toHaveBeenCalledWith( + 'Manual node exec finished', + expect.objectContaining({ + webhook_domain: null, + user_id: 'user123', + workflow_id: 'workflow123', + status: 'canceled', + executionStatus: 'canceled', + sharing_role: 'owner', + error_message: 'Error message canceled', + error_node_type: 'n8n-nodes-base.jira', + error_node_id: '1', + node_id: '1', + node_type: 'n8n-nodes-base.jira', + node_graph_string: JSON.stringify(nodeGraph.nodeGraph), + }), + ); + + expect(telemetry.trackWorkflowExecution).toHaveBeenCalledWith( + expect.objectContaining({ + workflow_id: 'workflow123', + success: false, + is_manual: true, + execution_mode: 'manual', + version_cli: N8N_VERSION, + error_message: 'Error message canceled', + error_node_type: 'n8n-nodes-base.jira', + node_graph_string: JSON.stringify(nodeGraph.nodeGraph), + error_node_id: '1', + }), + ); + }); + + it('should call telemetry.track when manual workflow execution finished', async () => { + sharedWorkflowRepository.findSharingRole.mockResolvedValue('workflow:owner'); + + const runData = { + status: 'error', + mode: 'manual', + data: { + startData: { + runNodeFilter: ['OpenAI'], + }, + resultData: { + runData: { + Jira: [ + { + data: { main: [[{ json: { headers: { origin: 'https://www.test.com' } } }]] }, + }, + ], + }, + lastNodeExecuted: 'OpenAI', + error: new NodeApiError( + { + id: '1', + typeVersion: 1, + name: 'Jira', + type: 'n8n-nodes-base.jira', + parameters: {}, + position: [100, 200], + }, + { + message: 'Error message', + description: 'Incorrect API key provided', + httpCode: '401', + stack: '', + }, + { + message: 'Error message', + description: 'Error description', + level: 'warning', + functionality: 'regular', + }, + ), + }, + }, + } as unknown as IRun; + + const nodeGraph: INodesGraphResult = { + webhookNodeNames: ['Jira'], + nodeGraph: { node_types: [], node_connections: [] }, + nameIndices: { + Jira: '1', + OpenAI: '1', + }, + } as unknown as INodesGraphResult; + + jest.spyOn(TelemetryHelpers, 'generateNodesGraph').mockImplementation(() => nodeGraph); + + jest + .spyOn(TelemetryHelpers, 'getNodeTypeForName') + .mockImplementation( + () => ({ type: 'n8n-nodes-base.jira', version: 1, name: 'Jira' }) as unknown as INode, + ); + + const event: RelayEventMap['workflow-post-execute'] = { + workflow: mockWorkflowBase, + executionId: 'execution123', + userId: 'user123', + runData, + }; + + eventService.emit('workflow-post-execute', event); + + await flushPromises(); + + expect(telemetry.track).toHaveBeenCalledWith( + 'Manual workflow exec finished', + expect.objectContaining({ + webhook_domain: 'test.com', + user_id: 'user123', + workflow_id: 'workflow123', + status: 'error', + executionStatus: 'error', + sharing_role: 'owner', + error_message: 'Error message', + error_node_type: 'n8n-nodes-base.jira', + error_node_id: '1', + node_graph_string: JSON.stringify(nodeGraph.nodeGraph), + }), + ); + + expect(telemetry.trackWorkflowExecution).toHaveBeenCalledWith( + expect.objectContaining({ + workflow_id: 'workflow123', + success: false, + is_manual: true, + execution_mode: 'manual', + version_cli: N8N_VERSION, + error_message: 'Error message', + error_node_type: 'n8n-nodes-base.jira', + node_graph_string: JSON.stringify(nodeGraph.nodeGraph), + error_node_id: '1', + }), + ); + }); + }); }); diff --git a/packages/cli/src/executions/__tests__/execution.service.test.ts b/packages/cli/src/executions/__tests__/execution.service.test.ts index 101773a0f7..e425c1e588 100644 --- a/packages/cli/src/executions/__tests__/execution.service.test.ts +++ b/packages/cli/src/executions/__tests__/execution.service.test.ts @@ -183,7 +183,7 @@ describe('ExecutionService', () => { describe('scaling mode', () => { describe('manual execution', () => { - it('should delegate to regular mode in scaling mode', async () => { + it('should stop a `running` execution in scaling mode', async () => { /** * Arrange */ @@ -197,6 +197,8 @@ describe('ExecutionService', () => { concurrencyControl.has.mockReturnValue(false); activeExecutions.has.mockReturnValue(true); waitTracker.has.mockReturnValue(false); + const job = mock({ data: { executionId: '123' } }); + scalingService.findJobsByStatus.mockResolvedValue([job]); executionRepository.stopDuringRun.mockResolvedValue(mock()); // @ts-expect-error Private method const stopInRegularModeSpy = jest.spyOn(executionService, 'stopInRegularMode'); @@ -209,7 +211,7 @@ describe('ExecutionService', () => { /** * Assert */ - expect(stopInRegularModeSpy).toHaveBeenCalledWith(execution); + expect(stopInRegularModeSpy).not.toHaveBeenCalled(); expect(activeExecutions.stopExecution).toHaveBeenCalledWith(execution.id); expect(executionRepository.stopDuringRun).toHaveBeenCalledWith(execution); diff --git a/packages/cli/src/executions/execution.service.ts b/packages/cli/src/executions/execution.service.ts index 86338da924..9eba37773f 100644 --- a/packages/cli/src/executions/execution.service.ts +++ b/packages/cli/src/executions/execution.service.ts @@ -464,11 +464,6 @@ export class ExecutionService { } private async stopInScalingMode(execution: IExecutionResponse) { - if (execution.mode === 'manual') { - // manual executions in scaling mode are processed by main - return await this.stopInRegularMode(execution); - } - if (this.activeExecutions.has(execution.id)) { this.activeExecutions.stopExecution(execution.id); } diff --git a/packages/cli/src/push/__tests__/index.test.ts b/packages/cli/src/push/__tests__/index.test.ts index 03457926b1..ae46487a38 100644 --- a/packages/cli/src/push/__tests__/index.test.ts +++ b/packages/cli/src/push/__tests__/index.test.ts @@ -20,7 +20,7 @@ describe('Push', () => { test('should validate pushRef on requests for websocket backend', () => { config.set('push.backend', 'websocket'); - const push = new Push(mock(), mock()); + const push = new Push(mock(), mock(), mock()); const ws = mock(); const request = mock({ user, ws }); request.query = { pushRef: '' }; @@ -33,7 +33,7 @@ describe('Push', () => { test('should validate pushRef on requests for SSE backend', () => { config.set('push.backend', 'sse'); - const push = new Push(mock(), mock()); + const push = new Push(mock(), mock(), mock()); const request = mock({ user, ws: undefined }); request.query = { pushRef: '' }; expect(() => push.handleRequest(request, mock())).toThrow(BadRequestError); diff --git a/packages/cli/src/push/index.ts b/packages/cli/src/push/index.ts index 7325981d0b..20d0c7a9c5 100644 --- a/packages/cli/src/push/index.ts +++ b/packages/cli/src/push/index.ts @@ -2,7 +2,8 @@ import type { PushMessage } from '@n8n/api-types'; import type { Application } from 'express'; import { ServerResponse } from 'http'; import type { Server } from 'http'; -import { InstanceSettings } from 'n8n-core'; +import { InstanceSettings, Logger } from 'n8n-core'; +import { deepCopy } from 'n8n-workflow'; import type { Socket } from 'net'; import { Container, Service } from 'typedi'; import { parse as parseUrl } from 'url'; @@ -10,6 +11,7 @@ import { Server as WSServer } from 'ws'; import { AuthService } from '@/auth/auth.service'; import config from '@/config'; +import { TRIMMED_TASK_DATA_CONNECTIONS } from '@/constants'; import type { User } from '@/databases/entities/user'; import { OnShutdown } from '@/decorators/on-shutdown'; import { BadRequestError } from '@/errors/response-errors/bad-request.error'; @@ -27,6 +29,12 @@ type PushEvents = { const useWebSockets = config.getEnv('push.backend') === 'websocket'; +/** + * Max allowed size of a push message in bytes. Events going through the pubsub + * channel are trimmed if exceeding this size. + */ +const MAX_PAYLOAD_SIZE_BYTES = 5 * 1024 * 1024; // 5 MiB + /** * Push service for uni- or bi-directional communication with frontend clients. * Uses either server-sent events (SSE, unidirectional from backend --> frontend) @@ -43,8 +51,10 @@ export class Push extends TypedEmitter { constructor( private readonly instanceSettings: InstanceSettings, private readonly publisher: Publisher, + private readonly logger: Logger, ) { super(); + this.logger = this.logger.scoped('push'); if (useWebSockets) this.backend.on('message', (msg) => this.emit('message', msg)); } @@ -85,18 +95,14 @@ export class Push extends TypedEmitter { this.backend.sendToAll(pushMsg); } + /** Returns whether a given push ref is registered. */ + hasPushRef(pushRef: string) { + return this.backend.hasPushRef(pushRef); + } + send(pushMsg: PushMessage, pushRef: string) { - /** - * Multi-main setup: In a manual webhook execution, the main process that - * handles a webhook might not be the same as the main process that created - * the webhook. If so, the handler process commands the creator process to - * relay the former's execution lifecycle events to the creator's frontend. - */ - if (this.instanceSettings.isMultiMain && !this.backend.hasPushRef(pushRef)) { - void this.publisher.publishCommand({ - command: 'relay-execution-lifecycle-event', - payload: { ...pushMsg, pushRef }, - }); + if (this.shouldRelayViaPubSub(pushRef)) { + this.relayViaPubSub(pushMsg, pushRef); return; } @@ -111,6 +117,66 @@ export class Push extends TypedEmitter { onShutdown() { this.backend.closeAllConnections(); } + + /** + * Whether to relay a push message via pubsub channel to other instances, + * instead of pushing the message directly to the frontend. + * + * This is needed in two scenarios: + * + * In scaling mode, in single- or multi-main setup, in a manual execution, a + * worker has no connection to a frontend and so relays to all mains lifecycle + * events for manual executions. Only the main who holds the session for the + * execution will push to the frontend who commissioned the execution. + * + * In scaling mode, in multi-main setup, in a manual webhook execution, if + * the main who handles a webhook is not the main who created the webhook, + * the handler main relays execution lifecycle events to all mains. Only + * the main who holds the session for the execution will push events to + * the frontend who commissioned the execution. + */ + private shouldRelayViaPubSub(pushRef: string) { + const { isWorker, isMultiMain } = this.instanceSettings; + + return isWorker || (isMultiMain && !this.hasPushRef(pushRef)); + } + + /** + * Relay a push message via the `n8n.commands` pubsub channel, + * reducing the payload size if too large. + * + * See {@link shouldRelayViaPubSub} for more details. + */ + private relayViaPubSub(pushMsg: PushMessage, pushRef: string) { + const eventSizeBytes = new TextEncoder().encode(JSON.stringify(pushMsg.data)).length; + + if (eventSizeBytes <= MAX_PAYLOAD_SIZE_BYTES) { + void this.publisher.publishCommand({ + command: 'relay-execution-lifecycle-event', + payload: { ...pushMsg, pushRef }, + }); + return; + } + + // too large for pubsub channel, trim it + + const pushMsgCopy = deepCopy(pushMsg); + + const toMb = (bytes: number) => (bytes / (1024 * 1024)).toFixed(0); + const eventMb = toMb(eventSizeBytes); + const maxMb = toMb(MAX_PAYLOAD_SIZE_BYTES); + const { type } = pushMsgCopy; + + this.logger.warn(`Size of "${type}" (${eventMb} MB) exceeds max size ${maxMb} MB. Trimming...`); + + if (type === 'nodeExecuteAfter') pushMsgCopy.data.data.data = TRIMMED_TASK_DATA_CONNECTIONS; + else if (type === 'executionFinished') pushMsgCopy.data.rawData = ''; // prompt client to fetch from DB + + void this.publisher.publishCommand({ + command: 'relay-execution-lifecycle-event', + payload: { ...pushMsgCopy, pushRef }, + }); + } } export const setupPushServer = (restEndpoint: string, server: Server, app: Application) => { diff --git a/packages/cli/src/scaling/__tests__/job-processor.service.test.ts b/packages/cli/src/scaling/__tests__/job-processor.service.test.ts index 73264e6382..897986c915 100644 --- a/packages/cli/src/scaling/__tests__/job-processor.service.test.ts +++ b/packages/cli/src/scaling/__tests__/job-processor.service.test.ts @@ -19,6 +19,7 @@ describe('JobProcessor', () => { mock(), mock(), mock(), + mock(), ); const result = await jobProcessor.processJob(mock()); diff --git a/packages/cli/src/scaling/__tests__/pubsub-handler.test.ts b/packages/cli/src/scaling/__tests__/pubsub-handler.test.ts index b92c18c885..bc15911913 100644 --- a/packages/cli/src/scaling/__tests__/pubsub-handler.test.ts +++ b/packages/cli/src/scaling/__tests__/pubsub-handler.test.ts @@ -11,7 +11,6 @@ import type { ExternalSecretsManager } from '@/external-secrets.ee/external-secr import type { IWorkflowDb } from '@/interfaces'; import type { License } from '@/license'; import type { Push } from '@/push'; -import type { WebSocketPush } from '@/push/websocket.push'; import type { CommunityPackagesService } from '@/services/community-packages.service'; import type { TestWebhooks } from '@/webhooks/test-webhooks'; @@ -829,9 +828,7 @@ describe('PubSubHandler', () => { flattedRunData: '[]', }; - push.getBackend.mockReturnValue( - mock({ hasPushRef: jest.fn().mockReturnValue(true) }), - ); + push.hasPushRef.mockReturnValue(true); eventService.emit('relay-execution-lifecycle-event', { type, data, pushRef }); @@ -858,9 +855,7 @@ describe('PubSubHandler', () => { const workflowEntity = mock({ id: 'test-workflow-id' }); const pushRef = 'test-push-ref'; - push.getBackend.mockReturnValue( - mock({ hasPushRef: jest.fn().mockReturnValue(true) }), - ); + push.hasPushRef.mockReturnValue(true); testWebhooks.toWorkflow.mockReturnValue(mock({ id: 'test-workflow-id' })); eventService.emit('clear-test-webhooks', { webhookKey, workflowEntity, pushRef }); diff --git a/packages/cli/src/scaling/job-processor.ts b/packages/cli/src/scaling/job-processor.ts index 5e760e40c1..16a5efd3b3 100644 --- a/packages/cli/src/scaling/job-processor.ts +++ b/packages/cli/src/scaling/job-processor.ts @@ -1,6 +1,11 @@ import type { RunningJobSummary } from '@n8n/api-types'; -import { ErrorReporter, InstanceSettings, WorkflowExecute, Logger } from 'n8n-core'; -import type { ExecutionStatus, IExecuteResponsePromiseData, IRun } from 'n8n-workflow'; +import { InstanceSettings, WorkflowExecute, ErrorReporter, Logger } from 'n8n-core'; +import type { + ExecutionStatus, + IExecuteResponsePromiseData, + IRun, + IWorkflowExecutionDataProcess, +} from 'n8n-workflow'; import { BINARY_ENCODING, ApplicationError, Workflow } from 'n8n-workflow'; import type PCancelable from 'p-cancelable'; import { Service } from 'typedi'; @@ -8,6 +13,7 @@ import { Service } from 'typedi'; import config from '@/config'; import { ExecutionRepository } from '@/databases/repositories/execution.repository'; import { WorkflowRepository } from '@/databases/repositories/workflow.repository'; +import { ManualExecutionService } from '@/manual-execution.service'; import { NodeTypes } from '@/node-types'; import * as WorkflowExecuteAdditionalData from '@/workflow-execute-additional-data'; @@ -34,6 +40,7 @@ export class JobProcessor { private readonly workflowRepository: WorkflowRepository, private readonly nodeTypes: NodeTypes, private readonly instanceSettings: InstanceSettings, + private readonly manualExecutionService: ManualExecutionService, ) { this.logger = this.logger.scoped('scaling'); } @@ -115,13 +122,20 @@ export class JobProcessor { executionTimeoutTimestamp, ); + const { pushRef } = job.data; + additionalData.hooks = WorkflowExecuteAdditionalData.getWorkflowHooksWorkerExecuter( execution.mode, job.data.executionId, execution.workflowData, - { retryOf: execution.retryOf as string }, + { retryOf: execution.retryOf as string, pushRef }, ); + if (pushRef) { + // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment + additionalData.sendDataToUI = WorkflowExecuteAdditionalData.sendDataToUI.bind({ pushRef }); + } + additionalData.hooks.hookFunctions.sendResponse = [ async (response: IExecuteResponsePromiseData): Promise => { const msg: RespondToWebhookMessage = { @@ -146,7 +160,31 @@ export class JobProcessor { let workflowExecute: WorkflowExecute; let workflowRun: PCancelable; - if (execution.data !== undefined) { + + const { startData, resultData, manualData, isTestWebhook } = execution.data; + + if (execution.mode === 'manual' && !isTestWebhook) { + const data: IWorkflowExecutionDataProcess = { + executionMode: execution.mode, + workflowData: execution.workflowData, + destinationNode: startData?.destinationNode, + startNodes: startData?.startNodes, + runData: resultData.runData, + pinData: resultData.pinData, + partialExecutionVersion: manualData?.partialExecutionVersion, + dirtyNodeNames: manualData?.dirtyNodeNames, + triggerToStartFrom: manualData?.triggerToStartFrom, + userId: manualData?.userId, + }; + + workflowRun = this.manualExecutionService.runManually( + data, + workflow, + additionalData, + executionId, + resultData.pinData, + ); + } else if (execution.data !== undefined) { workflowExecute = new WorkflowExecute(additionalData, execution.mode, execution.data); workflowRun = workflowExecute.processRunExecutionData(workflow); } else { diff --git a/packages/cli/src/scaling/pubsub/pubsub-handler.ts b/packages/cli/src/scaling/pubsub/pubsub-handler.ts index 10a763f8f1..9059027704 100644 --- a/packages/cli/src/scaling/pubsub/pubsub-handler.ts +++ b/packages/cli/src/scaling/pubsub/pubsub-handler.ts @@ -160,12 +160,12 @@ export class PubSubHandler { 'display-workflow-activation-error': async ({ workflowId, errorMessage }) => this.push.broadcast({ type: 'workflowFailedToActivate', data: { workflowId, errorMessage } }), 'relay-execution-lifecycle-event': async ({ pushRef, ...pushMsg }) => { - if (!this.push.getBackend().hasPushRef(pushRef)) return; + if (!this.push.hasPushRef(pushRef)) return; this.push.send(pushMsg, pushRef); }, 'clear-test-webhooks': async ({ webhookKey, workflowEntity, pushRef }) => { - if (!this.push.getBackend().hasPushRef(pushRef)) return; + if (!this.push.hasPushRef(pushRef)) return; this.testWebhooks.clearTimeout(webhookKey); diff --git a/packages/cli/src/scaling/scaling.types.ts b/packages/cli/src/scaling/scaling.types.ts index ae7e790a16..3c69294172 100644 --- a/packages/cli/src/scaling/scaling.types.ts +++ b/packages/cli/src/scaling/scaling.types.ts @@ -12,6 +12,7 @@ export type JobId = Job['id']; export type JobData = { executionId: string; loadStaticData: boolean; + pushRef?: string; }; export type JobResult = { diff --git a/packages/cli/src/sso.ee/saml/__tests__/saml-helpers.test.ts b/packages/cli/src/sso.ee/saml/__tests__/saml-helpers.test.ts index f544e050ed..d75fdc8a7f 100644 --- a/packages/cli/src/sso.ee/saml/__tests__/saml-helpers.test.ts +++ b/packages/cli/src/sso.ee/saml/__tests__/saml-helpers.test.ts @@ -4,7 +4,7 @@ import { AuthIdentityRepository } from '@/databases/repositories/auth-identity.r import { UserRepository } from '@/databases/repositories/user.repository'; import { generateNanoId } from '@/databases/utils/generators'; import * as helpers from '@/sso.ee/saml/saml-helpers'; -import type { SamlUserAttributes } from '@/sso.ee/saml/types/saml-user-attributes'; +import type { SamlUserAttributes } from '@/sso.ee/saml/types'; import { mockInstance } from '@test/mocking'; const userRepository = mockInstance(UserRepository); diff --git a/packages/cli/src/sso.ee/saml/__tests__/saml-validator.test.ts b/packages/cli/src/sso.ee/saml/__tests__/saml-validator.test.ts index 563c7934ea..9f93550bf2 100644 --- a/packages/cli/src/sso.ee/saml/__tests__/saml-validator.test.ts +++ b/packages/cli/src/sso.ee/saml/__tests__/saml-validator.test.ts @@ -1,11 +1,15 @@ -import { Logger } from 'n8n-core'; +import { mock } from 'jest-mock-extended'; -import { mockInstance } from '@test/mocking'; - -import { validateMetadata, validateResponse } from '../saml-validator'; +import { SamlValidator } from '../saml-validator'; describe('saml-validator', () => { - mockInstance(Logger); + const validator = new SamlValidator(mock()); + const VALID_CERTIFICATE = + 'MIIC8DCCAdigAwIBAgIQf+iroClVKohAtsyk0Ne13TANBgkqhkiG9w0BAQsFADA0MTIwMAYDVQQDEylNaWNyb3NvZnQgQXp1cmUgRmVkZXJhdGVkIFNTTyBDZXJ0aWZpY2F0ZTAeFw0yNDExMTMxMDEwNTNaFw0yNzExMTMxMDEwNTNaMDQxMjAwBgNVBAMTKU1pY3Jvc29mdCBBenVyZSBGZWRlcmF0ZWQgU1NPIENlcnRpZmljYXRlMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAwE8Ad1OMQKfaHi6YrsEcmMNwIAQ86h7JmnuABf5xLNd27jaMF4FVxHbEtC/BYxtcmwld5zbkCVXQ6PT6VoeYIjHMVnptFXg15EGgjnqpxWsjLDQNoSdSQu8VhG+8Yb5M7KPt+UEZfsRZVrgqMjdSEMVrOzPMD8KMB7wnghYX6npcZhn7D5w/F9gVDpI1Um8M/FIUKYVSYFjky1i24WvKmcBf71mAacZp48Zuj5by/ELIb6gAjpW5xpd02smpLthy/Yo4XDIQQurFOfjqyZd8xAZu/SfPsbjtymWw59tgd9RdYISl6O/241kY9h6Ojtx6WShOVDi6q+bJrfj9Z8WKcQIDAQABMA0GCSqGSIb3DQEBCwUAA4IBAQCiVxiQ9KpjihliQzIW45YO0EvRJtoPtyVAh9RiSGozbTl4otfrUJf8nbRtj7iZBRuuW4rrtRAH5kDb+i1wNUUQED2Pl/l4x5cN0oBytP3GSymq6NJx1gUOBO1BrNY+c3r5yHOUyj5qpbw9UkqpG1AqQkLLeZqB/yVCyOBQT7SKTbXVYhGefFM/+6z0/rGsWZN5OF6/2NC06ws1v4In28Atgpg4XxFh5TL7rPMJ11ca5MN9lHJoIUsvls053eQBcd7vJneqzd904B6WtPld6KOJK4dzIt9edHzPhaz158awWwx3iHsMn1Y/T0WVy5/4ZTzxY/i4U3t1Yt8ktxewVJYT'; + + beforeAll(async () => { + await validator.init(); + }); describe('validateMetadata', () => { test('successfully validates metadata containing ws federation tags', async () => { @@ -31,8 +35,7 @@ describe('saml-validator', () => { DQnnT/5se4dqYN86R35MCdbyKVl64lGPLSIVrxFxrOQ9YRK1br7Z1Bt1/LQD4f92z+GwAl+9tZTWhuoy6OGHCV6LlqBEztW43KnlCKw6eaNg4/6NluzJ/XeknXYLURDnfFVyGbLQAYWGND4Qm8CUXO/GjGfWTZuArvrDDC36/2FA41jKXtf1InxGFx1Bbaskx3n3KCFFth/V9knbnc1zftEe022aQluPRoGccROOI4ZeLUFL6+1gYlxjx0gFIOTRiuvrzR765lHNrF7iZ4aD+XukqtkGEtxTkiLoB+Bnr8Fd7IF5rV5FKTZWSxo+ZFcLimrDGtFPItVrC/oKRc+MGA== - - MIIC8DCCAdigAwIBAgIQf+iroClVKohAtsyk0Ne13TANBgkqhkiG9w0BAQsFADA0MTIwMAYDVQQDEylNaWNyb3NvZnQgQXp1cmUgRmVkZXJhdGVkIFNTTyBDZXJ0aWZpY2F0ZTAeFw0yNDExMTMxMDEwNTNaFw0yNzExMTMxMDEwNTNaMDQxMjAwBgNVBAMTKU1pY3Jvc29mdCBBenVyZSBGZWRlcmF0ZWQgU1NPIENlcnRpZmljYXRlMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAwE8Ad1OMQKfaHi6YrsEcmMNwIAQ86h7JmnuABf5xLNd27jaMF4FVxHbEtC/BYxtcmwld5zbkCVXQ6PT6VoeYIjHMVnptFXg15EGgjnqpxWsjLDQNoSdSQu8VhG+8Yb5M7KPt+UEZfsRZVrgqMjdSEMVrOzPMD8KMB7wnghYX6npcZhn7D5w/F9gVDpI1Um8M/FIUKYVSYFjky1i24WvKmcBf71mAacZp48Zuj5by/ELIb6gAjpW5xpd02smpLthy/Yo4XDIQQurFOfjqyZd8xAZu/SfPsbjtymWw59tgd9RdYISl6O/241kY9h6Ojtx6WShOVDi6q+bJrfj9Z8WKcQIDAQABMA0GCSqGSIb3DQEBCwUAA4IBAQCiVxiQ9KpjihliQzIW45YO0EvRJtoPtyVAh9RiSGozbTl4otfrUJf8nbRtj7iZBRuuW4rrtRAH5kDb+i1wNUUQED2Pl/l4x5cN0oBytP3GSymq6NJx1gUOBO1BrNY+c3r5yHOUyj5qpbw9UkqpG1AqQkLLeZqB/yVCyOBQT7SKTbXVYhGefFM/+6z0/rGsWZN5OF6/2NC06ws1v4In28Atgpg4XxFh5TL7rPMJ11ca5MN9lHJoIUsvls053eQBcd7vJneqzd904B6WtPld6KOJK4dzIt9edHzPhaz158awWwx3iHsMn1Y/T0WVy5/4ZTzxY/i4U3t1Yt8ktxewVJYT + ${VALID_CERTIFICATE} @@ -43,8 +46,7 @@ describe('saml-validator', () => { - - MIIC8DCCAdigAwIBAgIQf+iroClVKohAtsyk0Ne13TANBgkqhkiG9w0BAQsFADA0MTIwMAYDVQQDEylNaWNyb3NvZnQgQXp1cmUgRmVkZXJhdGVkIFNTTyBDZXJ0aWZpY2F0ZTAeFw0yNDExMTMxMDEwNTNaFw0yNzExMTMxMDEwNTNaMDQxMjAwBgNVBAMTKU1pY3Jvc29mdCBBenVyZSBGZWRlcmF0ZWQgU1NPIENlcnRpZmljYXRlMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAwE8Ad1OMQKfaHi6YrsEcmMNwIAQ86h7JmnuABf5xLNd27jaMF4FVxHbEtC/BYxtcmwld5zbkCVXQ6PT6VoeYIjHMVnptFXg15EGgjnqpxWsjLDQNoSdSQu8VhG+8Yb5M7KPt+UEZfsRZVrgqMjdSEMVrOzPMD8KMB7wnghYX6npcZhn7D5w/F9gVDpI1Um8M/FIUKYVSYFjky1i24WvKmcBf71mAacZp48Zuj5by/ELIb6gAjpW5xpd02smpLthy/Yo4XDIQQurFOfjqyZd8xAZu/SfPsbjtymWw59tgd9RdYISl6O/241kY9h6Ojtx6WShOVDi6q+bJrfj9Z8WKcQIDAQABMA0GCSqGSIb3DQEBCwUAA4IBAQCiVxiQ9KpjihliQzIW45YO0EvRJtoPtyVAh9RiSGozbTl4otfrUJf8nbRtj7iZBRuuW4rrtRAH5kDb+i1wNUUQED2Pl/l4x5cN0oBytP3GSymq6NJx1gUOBO1BrNY+c3r5yHOUyj5qpbw9UkqpG1AqQkLLeZqB/yVCyOBQT7SKTbXVYhGefFM/+6z0/rGsWZN5OF6/2NC06ws1v4In28Atgpg4XxFh5TL7rPMJ11ca5MN9lHJoIUsvls053eQBcd7vJneqzd904B6WtPld6KOJK4dzIt9edHzPhaz158awWwx3iHsMn1Y/T0WVy5/4ZTzxY/i4U3t1Yt8ktxewVJYT + ${VALID_CERTIFICATE} @@ -169,8 +171,7 @@ describe('saml-validator', () => { - - MIIC8DCCAdigAwIBAgIQf+iroClVKohAtsyk0Ne13TANBgkqhkiG9w0BAQsFADA0MTIwMAYDVQQDEylNaWNyb3NvZnQgQXp1cmUgRmVkZXJhdGVkIFNTTyBDZXJ0aWZpY2F0ZTAeFw0yNDExMTMxMDEwNTNaFw0yNzExMTMxMDEwNTNaMDQxMjAwBgNVBAMTKU1pY3Jvc29mdCBBenVyZSBGZWRlcmF0ZWQgU1NPIENlcnRpZmljYXRlMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAwE8Ad1OMQKfaHi6YrsEcmMNwIAQ86h7JmnuABf5xLNd27jaMF4FVxHbEtC/BYxtcmwld5zbkCVXQ6PT6VoeYIjHMVnptFXg15EGgjnqpxWsjLDQNoSdSQu8VhG+8Yb5M7KPt+UEZfsRZVrgqMjdSEMVrOzPMD8KMB7wnghYX6npcZhn7D5w/F9gVDpI1Um8M/FIUKYVSYFjky1i24WvKmcBf71mAacZp48Zuj5by/ELIb6gAjpW5xpd02smpLthy/Yo4XDIQQurFOfjqyZd8xAZu/SfPsbjtymWw59tgd9RdYISl6O/241kY9h6Ojtx6WShOVDi6q+bJrfj9Z8WKcQIDAQABMA0GCSqGSIb3DQEBCwUAA4IBAQCiVxiQ9KpjihliQzIW45YO0EvRJtoPtyVAh9RiSGozbTl4otfrUJf8nbRtj7iZBRuuW4rrtRAH5kDb+i1wNUUQED2Pl/l4x5cN0oBytP3GSymq6NJx1gUOBO1BrNY+c3r5yHOUyj5qpbw9UkqpG1AqQkLLeZqB/yVCyOBQT7SKTbXVYhGefFM/+6z0/rGsWZN5OF6/2NC06ws1v4In28Atgpg4XxFh5TL7rPMJ11ca5MN9lHJoIUsvls053eQBcd7vJneqzd904B6WtPld6KOJK4dzIt9edHzPhaz158awWwx3iHsMn1Y/T0WVy5/4ZTzxY/i4U3t1Yt8ktxewVJYT + ${VALID_CERTIFICATE} @@ -194,8 +195,7 @@ describe('saml-validator', () => { - - MIIC8DCCAdigAwIBAgIQf+iroClVKohAtsyk0Ne13TANBgkqhkiG9w0BAQsFADA0MTIwMAYDVQQDEylNaWNyb3NvZnQgQXp1cmUgRmVkZXJhdGVkIFNTTyBDZXJ0aWZpY2F0ZTAeFw0yNDExMTMxMDEwNTNaFw0yNzExMTMxMDEwNTNaMDQxMjAwBgNVBAMTKU1pY3Jvc29mdCBBenVyZSBGZWRlcmF0ZWQgU1NPIENlcnRpZmljYXRlMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAwE8Ad1OMQKfaHi6YrsEcmMNwIAQ86h7JmnuABf5xLNd27jaMF4FVxHbEtC/BYxtcmwld5zbkCVXQ6PT6VoeYIjHMVnptFXg15EGgjnqpxWsjLDQNoSdSQu8VhG+8Yb5M7KPt+UEZfsRZVrgqMjdSEMVrOzPMD8KMB7wnghYX6npcZhn7D5w/F9gVDpI1Um8M/FIUKYVSYFjky1i24WvKmcBf71mAacZp48Zuj5by/ELIb6gAjpW5xpd02smpLthy/Yo4XDIQQurFOfjqyZd8xAZu/SfPsbjtymWw59tgd9RdYISl6O/241kY9h6Ojtx6WShOVDi6q+bJrfj9Z8WKcQIDAQABMA0GCSqGSIb3DQEBCwUAA4IBAQCiVxiQ9KpjihliQzIW45YO0EvRJtoPtyVAh9RiSGozbTl4otfrUJf8nbRtj7iZBRuuW4rrtRAH5kDb+i1wNUUQED2Pl/l4x5cN0oBytP3GSymq6NJx1gUOBO1BrNY+c3r5yHOUyj5qpbw9UkqpG1AqQkLLeZqB/yVCyOBQT7SKTbXVYhGefFM/+6z0/rGsWZN5OF6/2NC06ws1v4In28Atgpg4XxFh5TL7rPMJ11ca5MN9lHJoIUsvls053eQBcd7vJneqzd904B6WtPld6KOJK4dzIt9edHzPhaz158awWwx3iHsMn1Y/T0WVy5/4ZTzxY/i4U3t1Yt8ktxewVJYT + ${VALID_CERTIFICATE} @@ -209,7 +209,7 @@ describe('saml-validator', () => { `; // ACT - const result = await validateMetadata(metadata); + const result = await validator.validateMetadata(metadata); // ASSERT expect(result).toBe(true); @@ -225,7 +225,85 @@ describe('saml-validator', () => { `; // ACT - const result = await validateMetadata(metadata); + const result = await validator.validateMetadata(metadata); + + // ASSERT + expect(result).toBe(false); + }); + + test('rejects malformed XML metadata', async () => { + // ARRANGE + const metadata = ` + + + + + + ${VALID_CERTIFICATE} + + + + + + `; // Missing closing tags + + // ACT + const result = await validator.validateMetadata(metadata); + + // ASSERT + expect(result).toBe(false); + }); + + test('rejects metadata missing SingleSignOnService', async () => { + // ARRANGE + const metadata = ` + + + + + + ${VALID_CERTIFICATE} + + + + + + `; + + // ACT + const result = await validator.validateMetadata(metadata); + + // ASSERT + expect(result).toBe(false); + }); + + test('rejects metadata with invalid X.509 certificate', async () => { + // ARRANGE + const metadata = ` + + + + + + + INVALID_CERTIFICATE + + + + + + + `; + + // ACT + const result = await validator.validateMetadata(metadata); // ASSERT expect(result).toBe(false); @@ -328,13 +406,13 @@ describe('saml-validator', () => { `; // ACT - const result = await validateResponse(response); + const result = await validator.validateResponse(response); // ASSERT expect(result).toBe(true); }); - test('rejects invalidate response', async () => { + test('rejects invalid response', async () => { // ARRANGE // Invalid because required children are missing const response = ` { `; // ACT - const result = await validateResponse(response); + const result = await validator.validateResponse(response); + + // ASSERT + expect(result).toBe(false); + }); + + test('rejects expired SAML response', async () => { + // ARRANGE + const response = ` + + https://sts.windows.net/random-issuer/ + + + + + https://sts.windows.net/random-issuer/ + + + random_name_id + + + + + // Expired + + http://localhost:5678/rest/sso/saml/metadata + + + + `; + + // ACT + const result = await validator.validateResponse(response); // ASSERT expect(result).toBe(false); diff --git a/packages/cli/src/sso.ee/saml/__tests__/saml.service.ee.test.ts b/packages/cli/src/sso.ee/saml/__tests__/saml.service.ee.test.ts index 6070104571..ebf34e3075 100644 --- a/packages/cli/src/sso.ee/saml/__tests__/saml.service.ee.test.ts +++ b/packages/cli/src/sso.ee/saml/__tests__/saml.service.ee.test.ts @@ -1,10 +1,8 @@ import type express from 'express'; import { mock } from 'jest-mock-extended'; -import { Logger } from 'n8n-core'; import type { IdentityProviderInstance, ServiceProviderInstance } from 'samlify'; import { SettingsRepository } from '@/databases/repositories/settings.repository'; -import { UrlService } from '@/services/url.service'; import * as samlHelpers from '@/sso.ee/saml/saml-helpers'; import { SamlService } from '@/sso.ee/saml/saml.service.ee'; import { mockInstance } from '@test/mocking'; @@ -13,10 +11,8 @@ import { SAML_PREFERENCES_DB_KEY } from '../constants'; import { InvalidSamlMetadataError } from '../errors/invalid-saml-metadata.error'; describe('SamlService', () => { - const logger = mockInstance(Logger); - const urlService = mockInstance(UrlService); - const samlService = new SamlService(logger, urlService); const settingsRepository = mockInstance(SettingsRepository); + const samlService = new SamlService(mock(), mock(), mock(), mock(), settingsRepository); beforeEach(() => { jest.restoreAllMocks(); diff --git a/packages/cli/src/sso.ee/saml/routes/__tests__/saml.controller.ee.test.ts b/packages/cli/src/sso.ee/saml/routes/__tests__/saml.controller.ee.test.ts index c4a33ed441..928f6d6df0 100644 --- a/packages/cli/src/sso.ee/saml/routes/__tests__/saml.controller.ee.test.ts +++ b/packages/cli/src/sso.ee/saml/routes/__tests__/saml.controller.ee.test.ts @@ -2,18 +2,14 @@ import { type Response } from 'express'; import { mock } from 'jest-mock-extended'; import type { User } from '@/databases/entities/user'; -import { UrlService } from '@/services/url.service'; -import { mockInstance } from '@test/mocking'; +import type { AuthlessRequest } from '@/requests'; -import { SamlService } from '../../saml.service.ee'; +import type { SamlService } from '../../saml.service.ee'; import { getServiceProviderConfigTestReturnUrl } from '../../service-provider.ee'; -import type { SamlConfiguration } from '../../types/requests'; -import type { SamlUserAttributes } from '../../types/saml-user-attributes'; +import type { SamlUserAttributes } from '../../types'; import { SamlController } from '../saml.controller.ee'; -const urlService = mockInstance(UrlService); -urlService.getInstanceBaseUrl.mockReturnValue(''); -const samlService = mockInstance(SamlService); +const samlService = mock(); const controller = new SamlController(mock(), samlService, mock(), mock()); const user = mock({ @@ -31,46 +27,45 @@ const attributes: SamlUserAttributes = { }; describe('Test views', () => { + const RelayState = getServiceProviderConfigTestReturnUrl(); + test('Should render success with template', async () => { - const req = mock(); + const req = mock(); const res = mock(); - req.body.RelayState = getServiceProviderConfigTestReturnUrl(); samlService.handleSamlLogin.mockResolvedValueOnce({ authenticatedUser: user, attributes, onboardingRequired: false, }); - await controller.acsPost(req, res); + await controller.acsPost(req, res, { RelayState }); expect(res.render).toBeCalledWith('saml-connection-test-success', attributes); }); test('Should render failure with template', async () => { - const req = mock(); + const req = mock(); const res = mock(); - req.body.RelayState = getServiceProviderConfigTestReturnUrl(); samlService.handleSamlLogin.mockResolvedValueOnce({ authenticatedUser: undefined, attributes, onboardingRequired: false, }); - await controller.acsPost(req, res); + await controller.acsPost(req, res, { RelayState }); expect(res.render).toBeCalledWith('saml-connection-test-failed', { message: '', attributes }); }); test('Should render error with template', async () => { - const req = mock(); + const req = mock(); const res = mock(); - req.body.RelayState = getServiceProviderConfigTestReturnUrl(); samlService.handleSamlLogin.mockRejectedValueOnce(new Error('Test Error')); - await controller.acsPost(req, res); + await controller.acsPost(req, res, { RelayState }); expect(res.render).toBeCalledWith('saml-connection-test-failed', { message: 'Test Error' }); }); diff --git a/packages/cli/src/sso.ee/saml/routes/saml.controller.ee.ts b/packages/cli/src/sso.ee/saml/routes/saml.controller.ee.ts index c7b954914b..c8f636eec4 100644 --- a/packages/cli/src/sso.ee/saml/routes/saml.controller.ee.ts +++ b/packages/cli/src/sso.ee/saml/routes/saml.controller.ee.ts @@ -1,15 +1,14 @@ -import { validate } from 'class-validator'; -import express from 'express'; +import { SamlAcsDto, SamlPreferences, SamlToggleDto } from '@n8n/api-types'; +import { Response } from 'express'; import querystring from 'querystring'; import type { PostBindingContext } from 'samlify/types/src/entity'; import url from 'url'; import { AuthService } from '@/auth/auth.service'; -import { Get, Post, RestController, GlobalScope } from '@/decorators'; +import { Get, Post, RestController, GlobalScope, Body } from '@/decorators'; import { AuthError } from '@/errors/response-errors/auth.error'; -import { BadRequestError } from '@/errors/response-errors/bad-request.error'; import { EventService } from '@/events/event.service'; -import { AuthenticatedRequest } from '@/requests'; +import { AuthenticatedRequest, AuthlessRequest } from '@/requests'; import { sendErrorResponse } from '@/response-helper'; import { UrlService } from '@/services/url.service'; @@ -25,7 +24,6 @@ import { getServiceProviderReturnUrl, } from '../service-provider.ee'; import type { SamlLoginBinding } from '../types'; -import { SamlConfiguration } from '../types/requests'; import { getInitSSOFormView } from '../views/init-sso-post'; @RestController('/sso/saml') @@ -38,7 +36,7 @@ export class SamlController { ) {} @Get('/metadata', { skipAuth: true }) - async getServiceProviderMetadata(_: express.Request, res: express.Response) { + async getServiceProviderMetadata(_: AuthlessRequest, res: Response) { return res .header('Content-Type', 'text/xml') .send(this.samlService.getServiceProviderInstance().getMetadata()); @@ -62,17 +60,8 @@ export class SamlController { */ @Post('/config', { middlewares: [samlLicensedMiddleware] }) @GlobalScope('saml:manage') - async configPost(req: SamlConfiguration.Update) { - const validationResult = await validate(req.body); - if (validationResult.length === 0) { - const result = await this.samlService.setSamlPreferences(req.body); - return result; - } else { - throw new BadRequestError( - 'Body is not a valid SamlPreferences object: ' + - validationResult.map((e) => e.toString()).join(','), - ); - } + async configPost(_req: AuthenticatedRequest, _res: Response, @Body payload: SamlPreferences) { + return await this.samlService.setSamlPreferences(payload); } /** @@ -80,11 +69,12 @@ export class SamlController { */ @Post('/config/toggle', { middlewares: [samlLicensedMiddleware] }) @GlobalScope('saml:manage') - async toggleEnabledPost(req: SamlConfiguration.Toggle, res: express.Response) { - if (req.body.loginEnabled === undefined) { - throw new BadRequestError('Body should contain a boolean "loginEnabled" property'); - } - await this.samlService.setSamlPreferences({ loginEnabled: req.body.loginEnabled }); + async toggleEnabledPost( + _req: AuthenticatedRequest, + res: Response, + @Body { loginEnabled }: SamlToggleDto, + ) { + await this.samlService.setSamlPreferences({ loginEnabled }); return res.sendStatus(200); } @@ -92,7 +82,7 @@ export class SamlController { * Assertion Consumer Service endpoint */ @Get('/acs', { middlewares: [samlLicensedMiddleware], skipAuth: true, usesTemplates: true }) - async acsGet(req: SamlConfiguration.AcsRequest, res: express.Response) { + async acsGet(req: AuthlessRequest, res: Response) { return await this.acsHandler(req, res, 'redirect'); } @@ -100,8 +90,8 @@ export class SamlController { * Assertion Consumer Service endpoint */ @Post('/acs', { middlewares: [samlLicensedMiddleware], skipAuth: true, usesTemplates: true }) - async acsPost(req: SamlConfiguration.AcsRequest, res: express.Response) { - return await this.acsHandler(req, res, 'post'); + async acsPost(req: AuthlessRequest, res: Response, @Body payload: SamlAcsDto) { + return await this.acsHandler(req, res, 'post', payload); } /** @@ -110,14 +100,15 @@ export class SamlController { * For test connections, returns status 202 if SAML is not enabled */ private async acsHandler( - req: SamlConfiguration.AcsRequest, - res: express.Response, + req: AuthlessRequest, + res: Response, binding: SamlLoginBinding, + payload: SamlAcsDto = {}, ) { try { const loginResult = await this.samlService.handleSamlLogin(req, binding); // if RelayState is set to the test connection Url, this is a test connection - if (isConnectionTestRequest(req)) { + if (isConnectionTestRequest(payload)) { if (loginResult.authenticatedUser) { return res.render('saml-connection-test-success', loginResult.attributes); } else { @@ -139,7 +130,7 @@ export class SamlController { if (loginResult.onboardingRequired) { return res.redirect(this.urlService.getInstanceBaseUrl() + '/saml/onboarding'); } else { - const redirectUrl = req.body?.RelayState ?? '/'; + const redirectUrl = payload.RelayState ?? '/'; return res.redirect(this.urlService.getInstanceBaseUrl() + redirectUrl); } } else { @@ -153,7 +144,7 @@ export class SamlController { // Need to manually send the error response since we're using templates return sendErrorResponse(res, new AuthError('SAML Authentication failed')); } catch (error) { - if (isConnectionTestRequest(req)) { + if (isConnectionTestRequest(payload)) { return res.render('saml-connection-test-failed', { message: (error as Error).message }); } this.eventService.emit('user-login-failed', { @@ -173,7 +164,7 @@ export class SamlController { * This endpoint is available if SAML is licensed and enabled */ @Get('/initsso', { middlewares: [samlLicensedAndEnabledMiddleware], skipAuth: true }) - async initSsoGet(req: express.Request, res: express.Response) { + async initSsoGet(req: AuthlessRequest, res: Response) { let redirectUrl = ''; try { const refererUrl = req.headers.referer; @@ -198,11 +189,11 @@ export class SamlController { */ @Get('/config/test', { middlewares: [samlLicensedMiddleware] }) @GlobalScope('saml:manage') - async configTestGet(_: AuthenticatedRequest, res: express.Response) { + async configTestGet(_: AuthenticatedRequest, res: Response) { return await this.handleInitSSO(res, getServiceProviderConfigTestReturnUrl()); } - private async handleInitSSO(res: express.Response, relayState?: string) { + private async handleInitSSO(res: Response, relayState?: string) { const result = await this.samlService.getLoginRequestUrl(relayState); if (result?.binding === 'redirect') { return result.context.context; diff --git a/packages/cli/src/sso.ee/saml/saml-helpers.ts b/packages/cli/src/sso.ee/saml/saml-helpers.ts index 996e17b359..8479314d4b 100644 --- a/packages/cli/src/sso.ee/saml/saml-helpers.ts +++ b/packages/cli/src/sso.ee/saml/saml-helpers.ts @@ -1,3 +1,4 @@ +import type { SamlAcsDto, SamlPreferences } from '@n8n/api-types'; import { randomString } from 'n8n-workflow'; import type { FlowResult } from 'samlify/types/src/flow'; import { Container } from 'typedi'; @@ -14,10 +15,7 @@ import { PasswordUtility } from '@/services/password.utility'; import { SAML_LOGIN_ENABLED, SAML_LOGIN_LABEL } from './constants'; import { getServiceProviderConfigTestReturnUrl } from './service-provider.ee'; -import type { SamlConfiguration } from './types/requests'; -import type { SamlAttributeMapping } from './types/saml-attribute-mapping'; -import type { SamlPreferences } from './types/saml-preferences'; -import type { SamlUserAttributes } from './types/saml-user-attributes'; +import type { SamlAttributeMapping, SamlUserAttributes } from './types'; import { getCurrentAuthenticationMethod, isEmailCurrentAuthenticationMethod, @@ -165,6 +163,6 @@ export function getMappedSamlAttributesFromFlowResult( return result; } -export function isConnectionTestRequest(req: SamlConfiguration.AcsRequest): boolean { - return req.body.RelayState === getServiceProviderConfigTestReturnUrl(); +export function isConnectionTestRequest(payload: SamlAcsDto): boolean { + return payload.RelayState === getServiceProviderConfigTestReturnUrl(); } diff --git a/packages/cli/src/sso.ee/saml/saml-validator.ts b/packages/cli/src/sso.ee/saml/saml-validator.ts index 582fe624a5..570b279c72 100644 --- a/packages/cli/src/sso.ee/saml/saml-validator.ts +++ b/packages/cli/src/sso.ee/saml/saml-validator.ts @@ -1,115 +1,87 @@ import { Logger } from 'n8n-core'; -import { Container } from 'typedi'; -import type { XMLFileInfo } from 'xmllint-wasm'; +import { Service } from 'typedi'; +import type { XMLFileInfo, XMLLintOptions, XMLValidationResult } from 'xmllint-wasm'; -let xmlMetadata: XMLFileInfo; -let xmlProtocol: XMLFileInfo; +@Service() +export class SamlValidator { + private xmlMetadata: XMLFileInfo; -let preload: XMLFileInfo[] = []; + private xmlProtocol: XMLFileInfo; -// eslint-disable-next-line @typescript-eslint/consistent-type-imports -let xmllintWasm: typeof import('xmllint-wasm') | undefined; + private preload: XMLFileInfo[] = []; -// dynamically load schema files -async function loadSchemas(): Promise { - xmlProtocol = (await import('./schema/saml-schema-protocol-2.0.xsd')).xmlFileInfo; - xmlMetadata = (await import('./schema/saml-schema-metadata-2.0.xsd')).xmlFileInfo; - preload = ( - await Promise.all([ - // SAML - import('./schema/saml-schema-assertion-2.0.xsd'), - import('./schema/xmldsig-core-schema.xsd'), - import('./schema/xenc-schema.xsd'), - import('./schema/xml.xsd'), + constructor(private readonly logger: Logger) {} - // WS-Federation - import('./schema/ws-federation.xsd'), - import('./schema/oasis-200401-wss-wssecurity-secext-1.0.xsd'), - import('./schema/oasis-200401-wss-wssecurity-utility-1.0.xsd'), - import('./schema/ws-addr.xsd'), - import('./schema/metadata-exchange.xsd'), - import('./schema/ws-securitypolicy-1.2.xsd'), - import('./schema/ws-authorization.xsd'), - ]) - ).map((m) => m.xmlFileInfo); -} + private xmllint: { + validateXML: (options: XMLLintOptions) => Promise; + }; -// dynamically load xmllint-wasm -async function loadXmllintWasm(): Promise { - if (xmllintWasm === undefined) { - Container.get(Logger).debug('Loading xmllint-wasm library into memory'); - xmllintWasm = await import('xmllint-wasm'); + async init() { + await this.loadSchemas(); + this.xmllint = await import('xmllint-wasm'); } -} -export async function validateMetadata(metadata: string): Promise { - const logger = Container.get(Logger); - try { - await loadXmllintWasm(); - await loadSchemas(); - const validationResult = await xmllintWasm?.validateXML({ - xml: [ - { - fileName: 'metadata.xml', - contents: metadata, - }, - ], - extension: 'schema', - schema: [xmlMetadata], - preload: [xmlProtocol, ...preload], - }); - if (validationResult?.valid) { - logger.debug('SAML Metadata is valid'); - return true; - } else { - logger.warn('SAML Validate Metadata: Invalid metadata'); - logger.warn( - validationResult - ? validationResult.errors - .map((error) => `${error.message} - ${error.rawMessage}`) - .join('\n') - : '', - ); + async validateMetadata(metadata: string): Promise { + return await this.validateXml('metadata', metadata); + } + + async validateResponse(response: string): Promise { + return await this.validateXml('response', response); + } + + // dynamically load schema files + private async loadSchemas(): Promise { + this.xmlProtocol = (await import('./schema/saml-schema-protocol-2.0.xsd')).xmlFileInfo; + this.xmlMetadata = (await import('./schema/saml-schema-metadata-2.0.xsd')).xmlFileInfo; + this.preload = ( + await Promise.all([ + // SAML + import('./schema/saml-schema-assertion-2.0.xsd'), + import('./schema/xmldsig-core-schema.xsd'), + import('./schema/xenc-schema.xsd'), + import('./schema/xml.xsd'), + + // WS-Federation + import('./schema/ws-federation.xsd'), + import('./schema/oasis-200401-wss-wssecurity-secext-1.0.xsd'), + import('./schema/oasis-200401-wss-wssecurity-utility-1.0.xsd'), + import('./schema/ws-addr.xsd'), + import('./schema/metadata-exchange.xsd'), + import('./schema/ws-securitypolicy-1.2.xsd'), + import('./schema/ws-authorization.xsd'), + ]) + ).map((m) => m.xmlFileInfo); + } + + private async validateXml(type: 'metadata' | 'response', contents: string): Promise { + const fileName = `${type}.xml`; + const schema = type === 'metadata' ? [this.xmlMetadata] : [this.xmlProtocol]; + const preload = [type === 'metadata' ? this.xmlProtocol : this.xmlMetadata, ...this.preload]; + + try { + const validationResult = await this.xmllint.validateXML({ + xml: [{ fileName, contents }], + extension: 'schema', + schema, + preload, + }); + if (validationResult?.valid) { + this.logger.debug(`SAML ${type} is valid`); + return true; + } else { + this.logger.debug(`SAML ${type} is invalid`); + this.logger.warn( + validationResult + ? validationResult.errors + .map((error) => `${error.message} - ${error.rawMessage}`) + .join('\n') + : '', + ); + } + } catch (error) { + // eslint-disable-next-line @typescript-eslint/no-unsafe-argument + this.logger.warn(error); } - } catch (error) { - // eslint-disable-next-line @typescript-eslint/no-unsafe-argument - logger.warn(error); + return false; } - return false; -} - -export async function validateResponse(response: string): Promise { - const logger = Container.get(Logger); - try { - await loadXmllintWasm(); - await loadSchemas(); - const validationResult = await xmllintWasm?.validateXML({ - xml: [ - { - fileName: 'response.xml', - contents: response, - }, - ], - extension: 'schema', - schema: [xmlProtocol], - preload: [xmlMetadata, ...preload], - }); - if (validationResult?.valid) { - logger.debug('SAML Response is valid'); - return true; - } else { - logger.warn('SAML Validate Response: Failed'); - logger.warn( - validationResult - ? validationResult.errors - .map((error) => `${error.message} - ${error.rawMessage}`) - .join('\n') - : '', - ); - } - } catch (error) { - // eslint-disable-next-line @typescript-eslint/no-unsafe-argument - logger.warn(error); - } - return false; } diff --git a/packages/cli/src/sso.ee/saml/saml.service.ee.ts b/packages/cli/src/sso.ee/saml/saml.service.ee.ts index 2944d9adf1..fd03471b67 100644 --- a/packages/cli/src/sso.ee/saml/saml.service.ee.ts +++ b/packages/cli/src/sso.ee/saml/saml.service.ee.ts @@ -1,3 +1,4 @@ +import type { SamlPreferences } from '@n8n/api-types'; import axios from 'axios'; import type express from 'express'; import https from 'https'; @@ -5,7 +6,7 @@ import { Logger } from 'n8n-core'; import { ApplicationError, jsonParse } from 'n8n-workflow'; import type { IdentityProviderInstance, ServiceProviderInstance } from 'samlify'; import type { BindingContext, PostBindingContext } from 'samlify/types/src/entity'; -import Container, { Service } from 'typedi'; +import { Service } from 'typedi'; import type { Settings } from '@/databases/entities/settings'; import type { User } from '@/databases/entities/user'; @@ -27,11 +28,9 @@ import { setSamlLoginLabel, updateUserFromSamlAttributes, } from './saml-helpers'; -import { validateMetadata, validateResponse } from './saml-validator'; +import { SamlValidator } from './saml-validator'; import { getServiceProviderInstance } from './service-provider.ee'; -import type { SamlLoginBinding } from './types'; -import type { SamlPreferences } from './types/saml-preferences'; -import type { SamlUserAttributes } from './types/saml-user-attributes'; +import type { SamlLoginBinding, SamlUserAttributes } from './types'; import { isSsoJustInTimeProvisioningEnabled } from '../sso-helpers'; @Service() @@ -79,12 +78,16 @@ export class SamlService { constructor( private readonly logger: Logger, private readonly urlService: UrlService, + private readonly validator: SamlValidator, + private readonly userRepository: UserRepository, + private readonly settingsRepository: SettingsRepository, ) {} async init(): Promise { try { // load preferences first but do not apply so as to not load samlify unnecessarily await this.loadFromDbAndApplySamlPreferences(false); + await this.validator.init(); if (isSamlLicensedAndEnabled()) { await this.loadSamlify(); await this.loadFromDbAndApplySamlPreferences(true); @@ -108,9 +111,10 @@ export class SamlService { this.logger.debug('Loading samlify library into memory'); this.samlify = await import('samlify'); } + this.samlify.setSchemaValidator({ validate: async (response: string) => { - const valid = await validateResponse(response); + const valid = await this.validator.validateResponse(response); if (!valid) { throw new InvalidSamlMetadataError(); } @@ -188,7 +192,7 @@ export class SamlService { const attributes = await this.getAttributesFromLoginResponse(req, binding); if (attributes.email) { const lowerCasedEmail = attributes.email.toLowerCase(); - const user = await Container.get(UserRepository).findOne({ + const user = await this.userRepository.findOne({ where: { email: lowerCasedEmail }, relations: ['authIdentities'], }); @@ -233,7 +237,7 @@ export class SamlService { }; } - async setSamlPreferences(prefs: SamlPreferences): Promise { + async setSamlPreferences(prefs: Partial): Promise { await this.loadSamlify(); await this.loadPreferencesWithoutValidation(prefs); if (prefs.metadataUrl) { @@ -242,7 +246,7 @@ export class SamlService { this._samlPreferences.metadata = fetchedMetadata; } } else if (prefs.metadata) { - const validationResult = await validateMetadata(prefs.metadata); + const validationResult = await this.validator.validateMetadata(prefs.metadata); if (!validationResult) { throw new InvalidSamlMetadataError(); } @@ -252,7 +256,7 @@ export class SamlService { return result; } - async loadPreferencesWithoutValidation(prefs: SamlPreferences) { + async loadPreferencesWithoutValidation(prefs: Partial) { this._samlPreferences.loginBinding = prefs.loginBinding ?? this._samlPreferences.loginBinding; this._samlPreferences.metadata = prefs.metadata ?? this._samlPreferences.metadata; this._samlPreferences.mapping = prefs.mapping ?? this._samlPreferences.mapping; @@ -278,7 +282,7 @@ export class SamlService { } async loadFromDbAndApplySamlPreferences(apply = true): Promise { - const samlPreferences = await Container.get(SettingsRepository).findOne({ + const samlPreferences = await this.settingsRepository.findOne({ where: { key: SAML_PREFERENCES_DB_KEY }, }); if (samlPreferences) { @@ -296,18 +300,18 @@ export class SamlService { } async saveSamlPreferencesToDb(): Promise { - const samlPreferences = await Container.get(SettingsRepository).findOne({ + const samlPreferences = await this.settingsRepository.findOne({ where: { key: SAML_PREFERENCES_DB_KEY }, }); const settingsValue = JSON.stringify(this.samlPreferences); let result: Settings; if (samlPreferences) { samlPreferences.value = settingsValue; - result = await Container.get(SettingsRepository).save(samlPreferences, { + result = await this.settingsRepository.save(samlPreferences, { transaction: false, }); } else { - result = await Container.get(SettingsRepository).save( + result = await this.settingsRepository.save( { key: SAML_PREFERENCES_DB_KEY, value: settingsValue, @@ -332,7 +336,7 @@ export class SamlService { const response = await axios.get(this._samlPreferences.metadataUrl, { httpsAgent: agent }); if (response.status === 200 && response.data) { const xml = (await response.data) as string; - const validationResult = await validateMetadata(xml); + const validationResult = await this.validator.validateMetadata(xml); if (!validationResult) { throw new BadRequestError( `Data received from ${this._samlPreferences.metadataUrl} is not valid SAML metadata.`, @@ -392,6 +396,6 @@ export class SamlService { */ async reset() { await setSamlLoginEnabled(false); - await Container.get(SettingsRepository).delete({ key: SAML_PREFERENCES_DB_KEY }); + await this.settingsRepository.delete({ key: SAML_PREFERENCES_DB_KEY }); } } diff --git a/packages/cli/src/sso.ee/saml/service-provider.ee.ts b/packages/cli/src/sso.ee/saml/service-provider.ee.ts index 2e6511df09..0522c80b51 100644 --- a/packages/cli/src/sso.ee/saml/service-provider.ee.ts +++ b/packages/cli/src/sso.ee/saml/service-provider.ee.ts @@ -1,10 +1,9 @@ +import type { SamlPreferences } from '@n8n/api-types'; import type { ServiceProviderInstance } from 'samlify'; import { Container } from 'typedi'; import { UrlService } from '@/services/url.service'; -import type { SamlPreferences } from './types/saml-preferences'; - let serviceProviderInstance: ServiceProviderInstance | undefined; export function getServiceProviderEntityId(): string { diff --git a/packages/cli/src/sso.ee/saml/types.ts b/packages/cli/src/sso.ee/saml/types.ts new file mode 100644 index 0000000000..35687777b1 --- /dev/null +++ b/packages/cli/src/sso.ee/saml/types.ts @@ -0,0 +1,5 @@ +import type { SamlPreferences } from '@n8n/api-types'; + +export type SamlLoginBinding = SamlPreferences['loginBinding']; +export type SamlAttributeMapping = NonNullable; +export type SamlUserAttributes = SamlAttributeMapping; diff --git a/packages/cli/src/sso.ee/saml/types/index.ts b/packages/cli/src/sso.ee/saml/types/index.ts deleted file mode 100644 index 560f7003f8..0000000000 --- a/packages/cli/src/sso.ee/saml/types/index.ts +++ /dev/null @@ -1 +0,0 @@ -export type SamlLoginBinding = 'post' | 'redirect'; diff --git a/packages/cli/src/sso.ee/saml/types/requests.ts b/packages/cli/src/sso.ee/saml/types/requests.ts deleted file mode 100644 index 69fb89a1eb..0000000000 --- a/packages/cli/src/sso.ee/saml/types/requests.ts +++ /dev/null @@ -1,17 +0,0 @@ -import type { AuthenticatedRequest, AuthlessRequest } from '@/requests'; - -import type { SamlPreferences } from './saml-preferences'; - -export declare namespace SamlConfiguration { - type Update = AuthenticatedRequest<{}, {}, SamlPreferences, {}>; - type Toggle = AuthenticatedRequest<{}, {}, { loginEnabled: boolean }, {}>; - - type AcsRequest = AuthlessRequest< - {}, - {}, - { - RelayState?: string; - }, - {} - >; -} diff --git a/packages/cli/src/sso.ee/saml/types/saml-attribute-mapping.ts b/packages/cli/src/sso.ee/saml/types/saml-attribute-mapping.ts deleted file mode 100644 index af7dd76e23..0000000000 --- a/packages/cli/src/sso.ee/saml/types/saml-attribute-mapping.ts +++ /dev/null @@ -1,6 +0,0 @@ -export interface SamlAttributeMapping { - email: string; - firstName: string; - lastName: string; - userPrincipalName: string; -} diff --git a/packages/cli/src/sso.ee/saml/types/saml-preferences.ts b/packages/cli/src/sso.ee/saml/types/saml-preferences.ts deleted file mode 100644 index 1231684360..0000000000 --- a/packages/cli/src/sso.ee/saml/types/saml-preferences.ts +++ /dev/null @@ -1,65 +0,0 @@ -import { IsBoolean, IsObject, IsOptional, IsString } from 'class-validator'; -import { SignatureConfig } from 'samlify/types/src/types'; - -import { SamlLoginBinding } from '.'; -import { SamlAttributeMapping } from './saml-attribute-mapping'; - -export class SamlPreferences { - @IsObject() - @IsOptional() - mapping?: SamlAttributeMapping; - - @IsString() - @IsOptional() - metadata?: string; - - @IsString() - @IsOptional() - metadataUrl?: string; - - @IsBoolean() - @IsOptional() - ignoreSSL?: boolean = false; - - @IsString() - @IsOptional() - loginBinding?: SamlLoginBinding = 'redirect'; - - @IsBoolean() - @IsOptional() - loginEnabled?: boolean; - - @IsString() - @IsOptional() - loginLabel?: string; - - @IsBoolean() - @IsOptional() - authnRequestsSigned?: boolean = false; - - @IsBoolean() - @IsOptional() - wantAssertionsSigned?: boolean = true; - - @IsBoolean() - @IsOptional() - wantMessageSigned?: boolean = true; - - @IsString() - @IsOptional() - acsBinding?: SamlLoginBinding = 'post'; - - @IsObject() - @IsOptional() - signatureConfig?: SignatureConfig = { - prefix: 'ds', - location: { - reference: '/samlp:Response/saml:Issuer', - action: 'after', - }, - }; - - @IsString() - @IsOptional() - relayState?: string = ''; -} diff --git a/packages/cli/src/sso.ee/saml/types/saml-user-attributes.ts b/packages/cli/src/sso.ee/saml/types/saml-user-attributes.ts deleted file mode 100644 index fa3c849f65..0000000000 --- a/packages/cli/src/sso.ee/saml/types/saml-user-attributes.ts +++ /dev/null @@ -1,6 +0,0 @@ -export interface SamlUserAttributes { - email: string; - firstName: string; - lastName: string; - userPrincipalName: string; -} diff --git a/packages/cli/src/webhooks/test-webhooks.ts b/packages/cli/src/webhooks/test-webhooks.ts index b90b1db59d..ff5d47fd50 100644 --- a/packages/cli/src/webhooks/test-webhooks.ts +++ b/packages/cli/src/webhooks/test-webhooks.ts @@ -154,11 +154,7 @@ export class TestWebhooks implements IWebhookManager { * the webhook. If so, after the test webhook has been successfully executed, * the handler process commands the creator process to clear its test webhooks. */ - if ( - this.instanceSettings.isMultiMain && - pushRef && - !this.push.getBackend().hasPushRef(pushRef) - ) { + if (this.instanceSettings.isMultiMain && pushRef && !this.push.hasPushRef(pushRef)) { void this.publisher.publishCommand({ command: 'clear-test-webhooks', payload: { webhookKey: key, workflowEntity, pushRef }, diff --git a/packages/cli/src/webhooks/webhook-helpers.ts b/packages/cli/src/webhooks/webhook-helpers.ts index 51b81fad83..0566e72a10 100644 --- a/packages/cli/src/webhooks/webhook-helpers.ts +++ b/packages/cli/src/webhooks/webhook-helpers.ts @@ -37,10 +37,12 @@ import { FORM_NODE_TYPE, NodeOperationError, } from 'n8n-workflow'; +import assert from 'node:assert'; import { finished } from 'stream/promises'; import { Container } from 'typedi'; import { ActiveExecutions } from '@/active-executions'; +import config from '@/config'; import type { Project } from '@/databases/entities/project'; import { InternalServerError } from '@/errors/response-errors/internal-server.error'; import { NotFoundError } from '@/errors/response-errors/not-found.error'; @@ -531,6 +533,15 @@ export async function executeWebhook( }); } + if ( + config.getEnv('executions.mode') === 'queue' && + process.env.OFFLOAD_MANUAL_EXECUTIONS_TO_WORKERS === 'true' && + runData.executionMode === 'manual' + ) { + assert(runData.executionData); + runData.executionData.isTestWebhook = true; + } + // Start now to run the workflow executionId = await Container.get(WorkflowRunner).run( runData, diff --git a/packages/cli/src/workflow-execute-additional-data.ts b/packages/cli/src/workflow-execute-additional-data.ts index de9bc85c14..ac2dab4d88 100644 --- a/packages/cli/src/workflow-execute-additional-data.ts +++ b/packages/cli/src/workflow-execute-additional-data.ts @@ -5,7 +5,13 @@ import type { PushMessage, PushType } from '@n8n/api-types'; import { GlobalConfig } from '@n8n/config'; import { stringify } from 'flatted'; -import { ErrorReporter, Logger, WorkflowExecute, isObjectLiteral } from 'n8n-core'; +import { + ErrorReporter, + Logger, + InstanceSettings, + WorkflowExecute, + isObjectLiteral, +} from 'n8n-core'; import { ApplicationError, NodeOperationError, Workflow, WorkflowHooks } from 'n8n-workflow'; import type { IDataObject, @@ -1076,8 +1082,7 @@ function getWorkflowHooksIntegrated( } /** - * Returns WorkflowHooks instance for running integrated workflows - * (Workflows which get started inside of another workflow) + * Returns WorkflowHooks instance for worker in scaling mode. */ export function getWorkflowHooksWorkerExecuter( mode: WorkflowExecuteMode, @@ -1093,6 +1098,17 @@ export function getWorkflowHooksWorkerExecuter( hooks.push.apply(hookFunctions[key], preExecuteFunctions[key]); } + if (mode === 'manual' && Container.get(InstanceSettings).isWorker) { + const pushHooks = hookFunctionsPush(); + for (const key of Object.keys(pushHooks)) { + if (hookFunctions[key] === undefined) { + hookFunctions[key] = []; + } + // eslint-disable-next-line prefer-spread + hookFunctions[key].push.apply(hookFunctions[key], pushHooks[key]); + } + } + return new WorkflowHooks(hookFunctions, mode, executionId, workflowData, optionalParameters); } diff --git a/packages/cli/src/workflow-runner.ts b/packages/cli/src/workflow-runner.ts index 93917d7987..1b31feb7c7 100644 --- a/packages/cli/src/workflow-runner.ts +++ b/packages/cli/src/workflow-runner.ts @@ -82,7 +82,7 @@ export class WorkflowRunner { // in queue mode, first do a sanity run for the edge case that the execution was not marked as stalled // by Bull even though it executed successfully, see https://github.com/OptimalBits/bull/issues/1415 - if (isQueueMode && executionMode !== 'manual') { + if (isQueueMode) { const executionWithoutData = await this.executionRepository.findSingleExecution(executionId, { includeData: false, }); @@ -153,9 +153,13 @@ export class WorkflowRunner { this.activeExecutions.attachResponsePromise(executionId, responsePromise); } - if (this.executionsMode === 'queue' && data.executionMode !== 'manual') { - // Do not run "manual" executions in bull because sending events to the - // frontend would not be possible + // @TODO: Reduce to true branch once feature is stable + const shouldEnqueue = + process.env.OFFLOAD_MANUAL_EXECUTIONS_TO_WORKERS === 'true' + ? this.executionsMode === 'queue' + : this.executionsMode === 'queue' && data.executionMode !== 'manual'; + + if (shouldEnqueue) { await this.enqueueExecution(executionId, data, loadStaticData, realtime); } else { await this.runMainProcess(executionId, data, loadStaticData, restartExecutionId); @@ -349,6 +353,7 @@ export class WorkflowRunner { const jobData: JobData = { executionId, loadStaticData: !!loadStaticData, + pushRef: data.pushRef, }; if (!this.scalingService) { diff --git a/packages/cli/src/workflows/__tests__/workflows.controller.test.ts b/packages/cli/src/workflows/__tests__/workflows.controller.test.ts new file mode 100644 index 0000000000..4168d487bf --- /dev/null +++ b/packages/cli/src/workflows/__tests__/workflows.controller.test.ts @@ -0,0 +1,72 @@ +import type { ImportWorkflowFromUrlDto } from '@n8n/api-types/src/dto/workflows/import-workflow-from-url.dto'; +import axios from 'axios'; +import type { Response } from 'express'; +import { mock } from 'jest-mock-extended'; + +import { BadRequestError } from '@/errors/response-errors/bad-request.error'; +import type { AuthenticatedRequest } from '@/requests'; + +import { WorkflowsController } from '../workflows.controller'; + +jest.mock('axios'); + +describe('WorkflowsController', () => { + const controller = Object.create(WorkflowsController.prototype); + const axiosMock = axios.get as jest.Mock; + const req = mock(); + const res = mock(); + + describe('getFromUrl', () => { + describe('should return workflow data', () => { + it('when the URL points to a valid JSON file', async () => { + const mockWorkflowData = { + nodes: [], + connections: {}, + }; + + axiosMock.mockResolvedValue({ data: mockWorkflowData }); + + const query: ImportWorkflowFromUrlDto = { url: 'https://example.com/workflow.json' }; + const result = await controller.getFromUrl(req, res, query); + + expect(result).toEqual(mockWorkflowData); + expect(axiosMock).toHaveBeenCalledWith(query.url); + }); + }); + + describe('should throw a BadRequestError', () => { + const query: ImportWorkflowFromUrlDto = { url: 'https://example.com/invalid.json' }; + + it('when the URL does not point to a valid JSON file', async () => { + axiosMock.mockRejectedValue(new Error('Network Error')); + + await expect(controller.getFromUrl(req, res, query)).rejects.toThrow(BadRequestError); + expect(axiosMock).toHaveBeenCalledWith(query.url); + }); + + it('when the data is not a valid n8n workflow JSON', async () => { + const invalidWorkflowData = { + nodes: 'not an array', + connections: 'not an object', + }; + + axiosMock.mockResolvedValue({ data: invalidWorkflowData }); + + await expect(controller.getFromUrl(req, res, query)).rejects.toThrow(BadRequestError); + expect(axiosMock).toHaveBeenCalledWith(query.url); + }); + + it('when the data is missing required fields', async () => { + const incompleteWorkflowData = { + nodes: [], + // Missing connections field + }; + + axiosMock.mockResolvedValue({ data: incompleteWorkflowData }); + + await expect(controller.getFromUrl(req, res, query)).rejects.toThrow(BadRequestError); + expect(axiosMock).toHaveBeenCalledWith(query.url); + }); + }); + }); +}); diff --git a/packages/cli/src/workflows/workflow-execution.service.ts b/packages/cli/src/workflows/workflow-execution.service.ts index 842ddfe726..10d882121c 100644 --- a/packages/cli/src/workflows/workflow-execution.service.ts +++ b/packages/cli/src/workflows/workflow-execution.service.ts @@ -15,6 +15,7 @@ import type { import { SubworkflowOperationError, Workflow } from 'n8n-workflow'; import { Service } from 'typedi'; +import config from '@/config'; import type { Project } from '@/databases/entities/project'; import type { User } from '@/databases/entities/user'; import { ExecutionRepository } from '@/databases/repositories/execution.repository'; @@ -146,6 +147,35 @@ export class WorkflowExecutionService { triggerToStartFrom, }; + /** + * Historically, manual executions in scaling mode ran in the main process, + * so some execution details were never persisted in the database. + * + * Currently, manual executions in scaling mode are offloaded to workers, + * so we persist all details to give workers full access to them. + */ + if ( + config.getEnv('executions.mode') === 'queue' && + process.env.OFFLOAD_MANUAL_EXECUTIONS_TO_WORKERS === 'true' + ) { + data.executionData = { + startData: { + startNodes, + destinationNode, + }, + resultData: { + pinData, + runData, + }, + manualData: { + userId: data.userId, + partialExecutionVersion: data.partialExecutionVersion, + dirtyNodeNames, + triggerToStartFrom, + }, + }; + } + const hasRunData = (node: INode) => runData !== undefined && !!runData[node.name]; if (pinnedTrigger && !hasRunData(pinnedTrigger)) { diff --git a/packages/cli/src/workflows/workflow.request.ts b/packages/cli/src/workflows/workflow.request.ts index 4098384abb..47fd2cdb93 100644 --- a/packages/cli/src/workflows/workflow.request.ts +++ b/packages/cli/src/workflows/workflow.request.ts @@ -69,6 +69,4 @@ export declare namespace WorkflowRequest { {}, { destinationProjectId: string } >; - - type FromUrl = AuthenticatedRequest<{}, {}, {}, { url?: string }>; } diff --git a/packages/cli/src/workflows/workflows.controller.ts b/packages/cli/src/workflows/workflows.controller.ts index b12dfdce5a..865b38450e 100644 --- a/packages/cli/src/workflows/workflows.controller.ts +++ b/packages/cli/src/workflows/workflows.controller.ts @@ -1,3 +1,4 @@ +import { ImportWorkflowFromUrlDto } from '@n8n/api-types'; import { GlobalConfig } from '@n8n/config'; // eslint-disable-next-line n8n-local-rules/misplaced-n8n-typeorm-import import { In, type FindOptionsRelations } from '@n8n/typeorm'; @@ -18,7 +19,7 @@ import { SharedWorkflowRepository } from '@/databases/repositories/shared-workfl import { TagRepository } from '@/databases/repositories/tag.repository'; import { WorkflowRepository } from '@/databases/repositories/workflow.repository'; import * as Db from '@/db'; -import { Delete, Get, Patch, Post, ProjectScope, Put, RestController } from '@/decorators'; +import { Delete, Get, Patch, Post, ProjectScope, Put, Query, RestController } from '@/decorators'; import { BadRequestError } from '@/errors/response-errors/bad-request.error'; import { ForbiddenError } from '@/errors/response-errors/forbidden.error'; import { InternalServerError } from '@/errors/response-errors/internal-server.error'; @@ -29,6 +30,7 @@ import { validateEntity } from '@/generic-helpers'; import type { IWorkflowResponse } from '@/interfaces'; import { License } from '@/license'; import { listQueryMiddleware } from '@/middlewares'; +import { AuthenticatedRequest } from '@/requests'; import * as ResponseHelper from '@/response-helper'; import { NamingService } from '@/services/naming.service'; import { ProjectService } from '@/services/project.service.ee'; @@ -215,18 +217,14 @@ export class WorkflowsController { } @Get('/from-url') - async getFromUrl(req: WorkflowRequest.FromUrl) { - if (req.query.url === undefined) { - throw new BadRequestError('The parameter "url" is missing!'); - } - if (!/^http[s]?:\/\/.*\.json$/i.exec(req.query.url)) { - throw new BadRequestError( - 'The parameter "url" is not valid! It does not seem to be a URL pointing to a n8n workflow JSON file.', - ); - } + async getFromUrl( + _req: AuthenticatedRequest, + _res: express.Response, + @Query query: ImportWorkflowFromUrlDto, + ) { let workflowData: IWorkflowResponse | undefined; try { - const { data } = await axios.get(req.query.url); + const { data } = await axios.get(query.url); workflowData = data; } catch (error) { throw new BadRequestError('The URL does not point to valid JSON file!'); diff --git a/packages/cli/test/integration/collaboration/collaboration.service.test.ts b/packages/cli/test/integration/collaboration/collaboration.service.test.ts index ab7a8314b3..e6951644fd 100644 --- a/packages/cli/test/integration/collaboration/collaboration.service.test.ts +++ b/packages/cli/test/integration/collaboration/collaboration.service.test.ts @@ -16,7 +16,7 @@ import { createWorkflow, shareWorkflowWithUsers } from '@test-integration/db/wor import * as testDb from '@test-integration/test-db'; describe('CollaborationService', () => { - mockInstance(Push, new Push(mock(), mock())); + mockInstance(Push, new Push(mock(), mock(), mock())); let pushService: Push; let collaborationService: CollaborationService; let owner: User; diff --git a/packages/cli/test/integration/saml/saml-helpers.test.ts b/packages/cli/test/integration/saml/saml-helpers.test.ts index 3396c0edc7..87d020248c 100644 --- a/packages/cli/test/integration/saml/saml-helpers.test.ts +++ b/packages/cli/test/integration/saml/saml-helpers.test.ts @@ -1,5 +1,5 @@ import * as helpers from '@/sso.ee/saml/saml-helpers'; -import type { SamlUserAttributes } from '@/sso.ee/saml/types/saml-user-attributes'; +import type { SamlUserAttributes } from '@/sso.ee/saml/types'; import { getPersonalProject } from '../shared/db/projects'; import * as testDb from '../shared/test-db'; diff --git a/packages/cli/test/integration/saml/saml.api.test.ts b/packages/cli/test/integration/saml/saml.api.test.ts index 247faaacba..7737444c6b 100644 --- a/packages/cli/test/integration/saml/saml.api.test.ts +++ b/packages/cli/test/integration/saml/saml.api.test.ts @@ -34,6 +34,8 @@ beforeAll(async () => { authMemberAgent = testServer.authAgentFor(someUser); }); +beforeEach(async () => await enableSaml(false)); + describe('Instance owner', () => { describe('PATCH /me', () => { test('should succeed with valid inputs', async () => { @@ -89,6 +91,17 @@ describe('Instance owner', () => { .expect(200); expect(getCurrentAuthenticationMethod()).toBe('saml'); }); + + test('should return 400 on invalid config', async () => { + await authOwnerAgent + .post('/sso/saml/config') + .send({ + ...sampleConfig, + loginBinding: 'invalid', + }) + .expect(400); + expect(getCurrentAuthenticationMethod()).toBe('email'); + }); }); describe('POST /sso/saml/config/toggle', () => { diff --git a/packages/cli/test/integration/saml/sample-metadata.ts b/packages/cli/test/integration/saml/sample-metadata.ts index fd7968c2fb..528a3f158f 100644 --- a/packages/cli/test/integration/saml/sample-metadata.ts +++ b/packages/cli/test/integration/saml/sample-metadata.ts @@ -1,7 +1,8 @@ +import type { SamlPreferences } from '@n8n/api-types'; export const sampleMetadata = '\n\n\n\n\n\n\n\n\n\nd/0TlU9d7qi9oQxDwjsZi69RMCiheKmcjJ7W0fRCHlM=\n\n\num+M46ZJmOhK1vGm6ZTIOY926ZN8pkMClyVprLs0NAWH3sEO11rZZZkcAnSuWrLR\n8BcrwpKRU6qE4zrZBWfh+/Fqp180OvUa7vUDpxuZFJZhv7dSldfLgAdFX2VHctBo\n77hdLmrmJuWv/u6Gzsie/J8/2D0U0OwDGwfsOLLW3rjrfea5opcaAxY+0Rh+2zzk\nzIxVBqtSnSKxAJtkOpCDzbtnQIO0meB0ZvO7ssxwSFjBbHs34TRj1S3GFgCZXzl5\naXDi7AoWEs1YPviRNb368OrD3aljFBK0gzjullFter0rzp2TzSzZilkxaZmhupJe\n388cIDBKJPUmkxumafWXxJIOMfktUTnciUl4kz0OfDQ0J5m5NaDrmvYU8g/2A0+P\nVRI88N9n0GcT9cDvzTCEDSBFefOVpvuQkue+ZYLpZ8bJJS0ykunkcNiXLbGlBlCS\nje3Od78eNjwzG/WYmHsf9ajmBezBrUmzvdJx+SmfGRZplu86z9NrOQMliKcU4/T6\nOGEwz0pRcvhMJLn+MNR2DPzX6YHnPZ0neyiUqnIkzt0fU4q1QNdcyqSTfRQlZjkx\ndbdLsEFALxcNRv8vFaAbsQpxPuFNlfZeyAWQ/MLoBG1rUiEl06I9REMN6KM7CTog\n5i926hP4LLsIki45Ob83glFOrIoj/3nAw2jbd2Crl+E=\n\n\nMIIFUzCCAzugAwIBAgIRAJ1peD6pO0pygujUcWb85QswDQYJKoZIhvcNAQELBQAw\nHTEbMBkGA1UEAwwSYXV0aGVudGlrIDIwMjMuMi4yMB4XDTIzMDIyNzEzMTQ0MFoX\nDTI0MDIyODEzMTQ0MFowVjEqMCgGA1UEAwwhYXV0aGVudGlrIFNlbGYtc2lnbmVk\nIENlcnRpZmljYXRlMRIwEAYDVQQKDAlhdXRoZW50aWsxFDASBgNVBAsMC1NlbGYt\nc2lnbmVkMIICIjANBgkqhkiG9w0BAQEFAAOCAg8AMIICCgKCAgEA3thve9UWPL09\nouGwUPlCxfrBDDKmDdvoMc3eahfuop2tSP38EvdBcnPCVYTtu2hhHNqN/QtoyAZc\nTvwD8oDjwiYxdO6VbNjMZAnMD4W84l2niGnG7ATy/niNcZoge4xy+OmCJKXsolbs\nXT+hQGQ2oiUDnbX8QwMQCMN8FBF+EvYoHXKvRjmjO75DHyHY9JP05HZTO3lycVLW\nGrIq4oJfp60PN/0z5tbpk/Tyst21o4lcESAM4fkmndonPmoKMr7q9g+CFYRT+As6\niB+L38J44YNWs0Qm42tHAlveinBRuLLMi+eMC2L0sckvyJKB1qHG+bKl7jVXNDJg\n5KWKEHdM4CBg3dJkign+12EO205ruLYSBydZErAb2NKd2htgYs/zGHSgb3LhQ3vE\nuHiTIcq828PWmVM7l3B8CJ+ZyPLixywT0pKgkb8lrDqzXIffRljCYMT2pIR4FNuy\n+CzXMYm+N30qVO8h9+cl3YRSHpFBk9KJ0/+HQp1k6ELnaYW+LryS8Jr1uPxhwyMq\nGu+4bxCF8JfZncojMhlQghXCQUvOaboNlBWv5jtsoZ9mN266V1EJpnF064UimQ1f\noN1O4l4292NvkChcmiQf2YDE5PrMWm10gQg401oulE9o91OsxLRmyw/qZTJvA06K\ngVamNLfhN/St/CVfl8q6ldgoHmWaxY8CAwEAAaNVMFMwUQYDVR0RAQH/BEcwRYJD\nT1BRVVpWNW1qdWFvQ01hdEVvenU5ajNoUnlhU0UyQThaTjd4WlZqUy5zZWxmLXNp\nZ25lZC5nb2F1dGhlbnRpay5pbzANBgkqhkiG9w0BAQsFAAOCAgEAwaQtK4s2DnJx\njg6i6BSo/rhNg7ClXgnOyF79T7JO3gexVjzboY2UTi1ut/DEII01PI0qgQ62+q9l\nTloWd1SpxPOrOVeu2uVgTK0LkGb63q355iJ2myfhFYYPPprNDzvUhnX8cVY979Ma\niqAOCJW7irlHAH2bLAujanRdlcgFtmoe5lZ+qnS5iOUmp5tehPsDJGlPZ3nCWJcR\nQHDLLSOp3TvR5no8nj0cWxUWnNeaGoJy1GsJlGapLXS5pUKpxVg9GeEcQxjBkFgM\nLWrkWBsQDvC5+GlmHgSkdRvuYBlB6CRK2eGY7G06v7ZRPhf82LvEFRBwzJvGdM0g\n491OTTJquTN2wyq45UlJK4anMYrUbpi8p8MOW7IUw6a+SvZyJab9gNoLTUzA6Mlz\nQP9bPrEALpwNhmHsmD09zNyYiNfpkpLJog96wPscx4b+gsg+5PcilET8qvth6VYD\nup8TdsonPvDPH0oyo66SAYoyOgAeB+BHTicjtVt+UnrhXYj92BHDXfmfdTzA8QcY\n7reLPIOQVk1zV24cwySiLh4F2Hr8z8V1wMRVNVHcezMsVBvCzxQ15XlMq9X2wBuj\nfED93dXJVs+WuzbpTIoXvHHT3zWnzykX8hVbrj9ddzF8TuJW4NYis0cH5SLzvtPj\n7EzvuRaQc7pNrduO1pTKoPAy+2SLgqo=\n\n\nMIIFUzCCAzugAwIBAgIRAJ1peD6pO0pygujUcWb85QswDQYJKoZIhvcNAQELBQAwHTEbMBkGA1UEAwwSYXV0aGVudGlrIDIwMjMuMi4yMB4XDTIzMDIyNzEzMTQ0MFoXDTI0MDIyODEzMTQ0MFowVjEqMCgGA1UEAwwhYXV0aGVudGlrIFNlbGYtc2lnbmVkIENlcnRpZmljYXRlMRIwEAYDVQQKDAlhdXRoZW50aWsxFDASBgNVBAsMC1NlbGYtc2lnbmVkMIICIjANBgkqhkiG9w0BAQEFAAOCAg8AMIICCgKCAgEA3thve9UWPL09ouGwUPlCxfrBDDKmDdvoMc3eahfuop2tSP38EvdBcnPCVYTtu2hhHNqN/QtoyAZcTvwD8oDjwiYxdO6VbNjMZAnMD4W84l2niGnG7ATy/niNcZoge4xy+OmCJKXsolbsXT+hQGQ2oiUDnbX8QwMQCMN8FBF+EvYoHXKvRjmjO75DHyHY9JP05HZTO3lycVLWGrIq4oJfp60PN/0z5tbpk/Tyst21o4lcESAM4fkmndonPmoKMr7q9g+CFYRT+As6iB+L38J44YNWs0Qm42tHAlveinBRuLLMi+eMC2L0sckvyJKB1qHG+bKl7jVXNDJg5KWKEHdM4CBg3dJkign+12EO205ruLYSBydZErAb2NKd2htgYs/zGHSgb3LhQ3vEuHiTIcq828PWmVM7l3B8CJ+ZyPLixywT0pKgkb8lrDqzXIffRljCYMT2pIR4FNuy+CzXMYm+N30qVO8h9+cl3YRSHpFBk9KJ0/+HQp1k6ELnaYW+LryS8Jr1uPxhwyMqGu+4bxCF8JfZncojMhlQghXCQUvOaboNlBWv5jtsoZ9mN266V1EJpnF064UimQ1foN1O4l4292NvkChcmiQf2YDE5PrMWm10gQg401oulE9o91OsxLRmyw/qZTJvA06KgVamNLfhN/St/CVfl8q6ldgoHmWaxY8CAwEAAaNVMFMwUQYDVR0RAQH/BEcwRYJDT1BRVVpWNW1qdWFvQ01hdEVvenU5ajNoUnlhU0UyQThaTjd4WlZqUy5zZWxmLXNpZ25lZC5nb2F1dGhlbnRpay5pbzANBgkqhkiG9w0BAQsFAAOCAgEAwaQtK4s2DnJxjg6i6BSo/rhNg7ClXgnOyF79T7JO3gexVjzboY2UTi1ut/DEII01PI0qgQ62+q9lTloWd1SpxPOrOVeu2uVgTK0LkGb63q355iJ2myfhFYYPPprNDzvUhnX8cVY979MaiqAOCJW7irlHAH2bLAujanRdlcgFtmoe5lZ+qnS5iOUmp5tehPsDJGlPZ3nCWJcRQHDLLSOp3TvR5no8nj0cWxUWnNeaGoJy1GsJlGapLXS5pUKpxVg9GeEcQxjBkFgMLWrkWBsQDvC5+GlmHgSkdRvuYBlB6CRK2eGY7G06v7ZRPhf82LvEFRBwzJvGdM0g491OTTJquTN2wyq45UlJK4anMYrUbpi8p8MOW7IUw6a+SvZyJab9gNoLTUzA6MlzQP9bPrEALpwNhmHsmD09zNyYiNfpkpLJog96wPscx4b+gsg+5PcilET8qvth6VYDup8TdsonPvDPH0oyo66SAYoyOgAeB+BHTicjtVt+UnrhXYj92BHDXfmfdTzA8QcY7reLPIOQVk1zV24cwySiLh4F2Hr8z8V1wMRVNVHcezMsVBvCzxQ15XlMq9X2wBujfED93dXJVs+WuzbpTIoXvHHT3zWnzykX8hVbrj9ddzF8TuJW4NYis0cH5SLzvtPj7EzvuRaQc7pNrduO1pTKoPAy+2SLgqo=urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddressurn:oasis:names:tc:SAML:2.0:nameid-format:persistenturn:oasis:names:tc:SAML:2.0:nameid-format:X509SubjectNameurn:oasis:names:tc:SAML:2.0:nameid-format:transient'; -export const sampleConfig = { +export const sampleConfig: SamlPreferences = { mapping: { email: 'http://schemas.xmlsoap.org/ws/2005/05/identity/claims/emailaddress', firstName: 'http://schemas.xmlsoap.org/ws/2005/05/identity/claims/firstname', @@ -25,6 +26,5 @@ export const sampleConfig = { action: 'after', }, }, - entityID: 'https://n8n-tunnel.localhost.dev/rest/sso/saml/metadata', - returnUrl: 'https://n8n-tunnel.localhost.dev/rest/sso/saml/acs', + relayState: '', }; diff --git a/packages/cli/test/integration/shared/utils/test-server.ts b/packages/cli/test/integration/shared/utils/test-server.ts index 52e97f8f31..d4c3437728 100644 --- a/packages/cli/test/integration/shared/utils/test-server.ts +++ b/packages/cli/test/integration/shared/utils/test-server.ts @@ -209,8 +209,10 @@ export const setupTestServer = ({ break; case 'saml': - const { setSamlLoginEnabled } = await import('@/sso.ee/saml/saml-helpers'); + const { SamlService } = await import('@/sso.ee/saml/saml.service.ee'); + await Container.get(SamlService).init(); await import('@/sso.ee/saml/routes/saml.controller.ee'); + const { setSamlLoginEnabled } = await import('@/sso.ee/saml/saml-helpers'); await setSamlLoginEnabled(true); break; diff --git a/packages/core/src/InstanceSettings.ts b/packages/core/src/InstanceSettings.ts index 814f75ef94..ebdfb7fd63 100644 --- a/packages/core/src/InstanceSettings.ts +++ b/packages/core/src/InstanceSettings.ts @@ -113,6 +113,10 @@ export class InstanceSettings { return !this.isMultiMain; } + get isWorker() { + return this.instanceType === 'worker'; + } + get isLeader() { return this.instanceRole === 'leader'; } diff --git a/packages/design-system/src/components/N8nButton/Button.scss b/packages/design-system/src/components/N8nButton/Button.scss index bdafef35d9..af37c8b4fe 100644 --- a/packages/design-system/src/components/N8nButton/Button.scss +++ b/packages/design-system/src/components/N8nButton/Button.scss @@ -196,7 +196,7 @@ @mixin n8n-button-danger { --button-font-color: var(--color-button-danger-font); - --button-border-color: var(--color-danger); + --button-border-color: var(--color-button-danger-border); --button-background-color: var(--color-danger); --button-hover-font-color: var(--color-button-danger-font); @@ -210,11 +210,11 @@ --button-focus-font-color: var(--color-button-danger-font); --button-focus-border-color: var(--color-danger); --button-focus-background-color: var(--color-danger); - --button-focus-outline-color: var(--color-danger-tint-1); + --button-focus-outline-color: var(--color-button-danger-focus-outline); --button-disabled-font-color: var(--color-button-danger-disabled-font); - --button-disabled-border-color: var(--color-danger-tint-1); - --button-disabled-background-color: var(--color-danger-tint-1); + --button-disabled-border-color: var(--color-button-danger-disabled-border); + --button-disabled-background-color: var(--color-button-danger-disabled-background); --button-loading-font-color: var(--color-button-danger-font); --button-loading-border-color: var(--color-danger); diff --git a/packages/design-system/src/components/index.ts b/packages/design-system/src/components/index.ts index 38484308ef..ce4360fe46 100644 --- a/packages/design-system/src/components/index.ts +++ b/packages/design-system/src/components/index.ts @@ -36,6 +36,7 @@ export { default as N8nOption } from './N8nOption'; export { default as N8nPopover } from './N8nPopover'; export { default as N8nPulse } from './N8nPulse'; export { default as N8nRadioButtons } from './N8nRadioButtons'; +export { default as N8nRoute } from './N8nRoute'; export { default as N8nRecycleScroller } from './N8nRecycleScroller'; export { default as N8nResizeWrapper } from './N8nResizeWrapper'; export { default as N8nSelect } from './N8nSelect'; diff --git a/packages/design-system/src/css/_tokens.dark.scss b/packages/design-system/src/css/_tokens.dark.scss index 4390bc1da8..4ea6566677 100644 --- a/packages/design-system/src/css/_tokens.dark.scss +++ b/packages/design-system/src/css/_tokens.dark.scss @@ -188,6 +188,14 @@ --color-button-secondary-disabled-font: var(--prim-gray-0-alpha-030); --color-button-secondary-disabled-border: var(--prim-gray-0-alpha-030); + // Button success, warning, danger + --color-button-danger-font: var(--prim-gray-0); + --color-button-danger-border: transparent; + --color-button-danger-focus-outline: var(--prim-color-alt-c-tint-250); + --color-button-danger-disabled-font: var(--prim-gray-0-alpha-025); + --color-button-danger-disabled-border: transparent; + --color-button-danger-disabled-background: var(--prim-color-alt-c-alpha-02); + // Text button --color-text-button-secondary-font: var(--prim-gray-320); diff --git a/packages/design-system/src/css/_tokens.scss b/packages/design-system/src/css/_tokens.scss index 944652e43e..e4faf088cf 100644 --- a/packages/design-system/src/css/_tokens.scss +++ b/packages/design-system/src/css/_tokens.scss @@ -244,7 +244,11 @@ --color-button-warning-font: var(--color-text-xlight); --color-button-warning-disabled-font: var(--prim-gray-0-alpha-075); --color-button-danger-font: var(--color-text-xlight); + --color-button-danger-border: var(--color-danger); + --color-button-danger-focus-outline: var(--color-danger-tint-1); --color-button-danger-disabled-font: var(--prim-gray-0-alpha-075); + --color-button-danger-disabled-border: var(--color-danger-tint-1); + --color-button-danger-disabled-background: var(--color-danger-tint-1); // Text button --color-text-button-secondary-font: var(--prim-gray-670); diff --git a/packages/editor-ui/src/Interface.ts b/packages/editor-ui/src/Interface.ts index 22d2387548..2b898f7b44 100644 --- a/packages/editor-ui/src/Interface.ts +++ b/packages/editor-ui/src/Interface.ts @@ -56,7 +56,6 @@ import type { ROLE, } from '@/constants'; import type { BulkCommand, Undoable } from '@/models/history'; -import type { PartialBy } from '@/utils/typeHelpers'; import type { ProjectSharingData } from '@/types/projects.types'; @@ -1300,41 +1299,6 @@ export type ExecutionsQueryFilter = { vote?: ExecutionFilterVote; }; -export type SamlAttributeMapping = { - email: string; - firstName: string; - lastName: string; - userPrincipalName: string; -}; - -export type SamlLoginBinding = 'post' | 'redirect'; - -export type SamlSignatureConfig = { - prefix: 'ds'; - location: { - reference: '/samlp:Response/saml:Issuer'; - action: 'after'; - }; -}; - -export type SamlPreferencesLoginEnabled = { - loginEnabled: boolean; -}; - -export type SamlPreferences = { - mapping?: SamlAttributeMapping; - metadata?: string; - metadataUrl?: string; - ignoreSSL?: boolean; - loginBinding?: SamlLoginBinding; - acsBinding?: SamlLoginBinding; - authnRequestsSigned?: boolean; - loginLabel?: string; - wantAssertionsSigned?: boolean; - wantMessageSigned?: boolean; - signatureConfig?: SamlSignatureConfig; -} & PartialBy; - export type SamlPreferencesExtractedData = { entityID: string; returnUrl: string; diff --git a/packages/editor-ui/src/__tests__/server/endpoints/sso.ts b/packages/editor-ui/src/__tests__/server/endpoints/sso.ts index a9c13e7285..91b1aaaaf2 100644 --- a/packages/editor-ui/src/__tests__/server/endpoints/sso.ts +++ b/packages/editor-ui/src/__tests__/server/endpoints/sso.ts @@ -1,16 +1,17 @@ +import type { SamlPreferences } from '@n8n/api-types'; import type { Server, Request } from 'miragejs'; import { Response } from 'miragejs'; -import type { SamlPreferences, SamlPreferencesExtractedData } from '@/Interface'; +import type { SamlPreferencesExtractedData } from '@/Interface'; import { faker } from '@faker-js/faker'; import type { AppSchema } from '@/__tests__/server/types'; import { jsonParse } from 'n8n-workflow'; -let samlConfig: SamlPreferences & SamlPreferencesExtractedData = { +let samlConfig = { metadata: '', metadataUrl: '', entityID: faker.internet.url(), returnUrl: faker.internet.url(), -}; +} as SamlPreferences & SamlPreferencesExtractedData; export function routesForSSO(server: Server) { server.get('/rest/sso/saml/config', () => { diff --git a/packages/editor-ui/src/api/sso.ts b/packages/editor-ui/src/api/sso.ts index 95b0425e95..c990f19c06 100644 --- a/packages/editor-ui/src/api/sso.ts +++ b/packages/editor-ui/src/api/sso.ts @@ -1,10 +1,6 @@ +import type { SamlPreferences, SamlToggleDto } from '@n8n/api-types'; import { makeRestApiRequest } from '@/utils/apiUtils'; -import type { - IRestApiContext, - SamlPreferencesLoginEnabled, - SamlPreferences, - SamlPreferencesExtractedData, -} from '@/Interface'; +import type { IRestApiContext, SamlPreferencesExtractedData } from '@/Interface'; export const initSSO = async (context: IRestApiContext): Promise => { return await makeRestApiRequest(context, 'GET', '/sso/saml/initsso'); @@ -22,14 +18,14 @@ export const getSamlConfig = async ( export const saveSamlConfig = async ( context: IRestApiContext, - data: SamlPreferences, + data: Partial, ): Promise => { return await makeRestApiRequest(context, 'POST', '/sso/saml/config', data); }; export const toggleSamlConfig = async ( context: IRestApiContext, - data: SamlPreferencesLoginEnabled, + data: SamlToggleDto, ): Promise => { return await makeRestApiRequest(context, 'POST', '/sso/saml/config/toggle', data); }; diff --git a/packages/editor-ui/src/components/RunData.vue b/packages/editor-ui/src/components/RunData.vue index 5c5fa0dbac..d852d3ee45 100644 --- a/packages/editor-ui/src/components/RunData.vue +++ b/packages/editor-ui/src/components/RunData.vue @@ -1,18 +1,19 @@