diff --git a/CHANGELOG.md b/CHANGELOG.md index 9ad8bdb6b408b..683764b54333d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,36 @@ +# [1.31.0](https://github.com/n8n-io/n8n/compare/n8n@1.30.0...n8n@1.31.0) (2024-02-28) + + +### Bug Fixes + +* **core:** Ensure `maxRedirects` is used for any http request defining it ([#8706](https://github.com/n8n-io/n8n/issues/8706)) ([246c988](https://github.com/n8n-io/n8n/commit/246c988b9373a838086f37e603ec2827cf849588)) +* **core:** Fix pairedItem issue with partial manual executions ([#8575](https://github.com/n8n-io/n8n/issues/8575)) ([a29b41e](https://github.com/n8n-io/n8n/commit/a29b41ec55d8a0cf5610a53087e455b7e649b8bc)) +* **Default Data Loader Node:** Fix binary data loader in s3 mode ([#8626](https://github.com/n8n-io/n8n/issues/8626)) ([a5e6f59](https://github.com/n8n-io/n8n/commit/a5e6f5928ae39f19d6cb55a234818e776141e325)) +* **editor:** Do not break NDV for version-less nodes ([#8714](https://github.com/n8n-io/n8n/issues/8714)) ([8a88d15](https://github.com/n8n-io/n8n/commit/8a88d156847852b38e1fd13f3b9240887491a665)) +* **editor:** Hide previous execution data for sub-nodes in debug mode if it has execution error ([#8710](https://github.com/n8n-io/n8n/issues/8710)) ([a973b9c](https://github.com/n8n-io/n8n/commit/a973b9c077d28faa45b527cf6e0f0e6644cf354a)) +* **editor:** Update Filter component state when value is updated ([#8684](https://github.com/n8n-io/n8n/issues/8684)) ([3ba2cdc](https://github.com/n8n-io/n8n/commit/3ba2cdcadbf4df4e4521cb03bf63f13a32a926a5)) +* Fix execution error when using AI chain nodes with non-chat model ([#8724](https://github.com/n8n-io/n8n/issues/8724)) ([0882dc0](https://github.com/n8n-io/n8n/commit/0882dc0ce9ad4c9260390f99be56df2d6f7b5e86)) +* **Postgres Trigger Node:** `closeFunction` errors should not prevent a workflow from being deactivated ([#8738](https://github.com/n8n-io/n8n/issues/8738)) ([7012577](https://github.com/n8n-io/n8n/commit/7012577fce796c6d18ab8081f90014a8cded7391)) +* Send user id when setting up an account ([#8639](https://github.com/n8n-io/n8n/issues/8639)) ([27f3166](https://github.com/n8n-io/n8n/commit/27f3166272455627a2d2f851a286126310a4d5b5)) +* **Trello Node:** Remove GET request body ([#8715](https://github.com/n8n-io/n8n/issues/8715)) ([8c4a744](https://github.com/n8n-io/n8n/commit/8c4a744c56ce84984ed837583cdfd7a296de5090)) +* Wrong prompt input key for sql agent ([#8708](https://github.com/n8n-io/n8n/issues/8708)) ([7c1cf33](https://github.com/n8n-io/n8n/commit/7c1cf33616eb1990a9d6d7f4b93e91575f2cddc8)) + + +### Features + +* Add env variables to support exposing `/workflows/demo` route and `/nodes.json` route ([#8506](https://github.com/n8n-io/n8n/issues/8506)) ([4b01335](https://github.com/n8n-io/n8n/commit/4b01335aa45d93b0e4f2b7c69503430f1bcca28a)) +* Add Outlook Trigger Node ([#8656](https://github.com/n8n-io/n8n/issues/8656)) ([720ae1b](https://github.com/n8n-io/n8n/commit/720ae1b96b4c6fd644bad60191c35d8d598ad666)) +* Add support for Ollama embeddings API ([#8732](https://github.com/n8n-io/n8n/issues/8732)) ([15490ad](https://github.com/n8n-io/n8n/commit/15490ad1d47c4f0d5c3f9eb350b2a1bcad4bbec0)) +* **AI Agent Node:** Allow use of Azure Chat model for OpenAI Functions agent ([#8725](https://github.com/n8n-io/n8n/issues/8725)) ([d03d927](https://github.com/n8n-io/n8n/commit/d03d9276f923d541f9c9ef86b8dc232f2737e30b)) +* Allow instance owners and admins to edit all credentials ([#8716](https://github.com/n8n-io/n8n/issues/8716)) ([7371708](https://github.com/n8n-io/n8n/commit/737170893d17108098c14db6be80071e8ef51930)) +* **editor:** AI Floating Nodes ([#8703](https://github.com/n8n-io/n8n/issues/8703)) ([41b191e](https://github.com/n8n-io/n8n/commit/41b191e0552aa2d92d442d1dea05913e8b386d4d)) +* **editor:** Retrieve previous chat message on arrow-up ([#8696](https://github.com/n8n-io/n8n/issues/8696)) ([246f8cf](https://github.com/n8n-io/n8n/commit/246f8cfcc3acdeb323849c94542fc4ad028c4f77)) +* No expression error when node hasn’t executed ([#8448](https://github.com/n8n-io/n8n/issues/8448)) ([f9a99ec](https://github.com/n8n-io/n8n/commit/f9a99ec0295499d95534d64e016f70339a56956b)) +* Session is selector for memory nodes ([#8736](https://github.com/n8n-io/n8n/issues/8736)) ([2aaf211](https://github.com/n8n-io/n8n/commit/2aaf211dfc270920a4885a2b086b98ab8a3c2af6)) +* SQL agent improvements ([#8709](https://github.com/n8n-io/n8n/issues/8709)) ([0952430](https://github.com/n8n-io/n8n/commit/09524304e6a8d1fdcdfe6340b71a5a443b942d6d)) + + + # [1.30.0](https://github.com/n8n-io/n8n/compare/n8n@1.29.0...n8n@1.30.0) (2024-02-21) diff --git a/cypress/e2e/14-mapping.cy.ts b/cypress/e2e/14-mapping.cy.ts index 7f49416a0965e..bf0d1d05dcdc3 100644 --- a/cypress/e2e/14-mapping.cy.ts +++ b/cypress/e2e/14-mapping.cy.ts @@ -285,8 +285,8 @@ describe('Data mapping', () => { ndv.actions.clearParameterInput('value'); cy.get('body').type('{esc}'); - ndv.getters.parameterInput('keepOnlySet').find('input[type="checkbox"]').should('exist'); - ndv.getters.parameterInput('keepOnlySet').find('input[type="text"]').should('not.exist'); + ndv.getters.parameterInput('includeOtherFields').find('input[type="checkbox"]').should('exist'); + ndv.getters.parameterInput('includeOtherFields').find('input[type="text"]').should('not.exist'); ndv.getters .inputDataContainer() .should('exist') @@ -296,9 +296,12 @@ describe('Data mapping', () => { .realMouseMove(100, 100); cy.wait(50); - ndv.getters.parameterInput('keepOnlySet').find('input[type="checkbox"]').should('not.exist'); ndv.getters - .parameterInput('keepOnlySet') + .parameterInput('includeOtherFields') + .find('input[type="checkbox"]') + .should('not.exist'); + ndv.getters + .parameterInput('includeOtherFields') .find('input[type="text"]') .should('exist') .invoke('css', 'border') diff --git a/cypress/e2e/5-ndv.cy.ts b/cypress/e2e/5-ndv.cy.ts index 765988a7aa0b6..dd884921c2e38 100644 --- a/cypress/e2e/5-ndv.cy.ts +++ b/cypress/e2e/5-ndv.cy.ts @@ -632,4 +632,24 @@ describe('NDV', () => { ndv.actions.changeNodeOperation('Update Row'); ndv.getters.resourceLocatorInput('documentId').find('input').should('have.value', TEST_DOC_ID); }); + + it('Should open appropriate node creator after clicking on connection hint link', () => { + const nodeCreator = new NodeCreator(); + const hintMapper = { + 'Memory': 'AI Nodes', + 'Output Parser': 'AI Nodes', + 'Token Splitter': 'Document Loaders', + 'Tool': 'AI Nodes', + 'Embeddings': 'Vector Stores', + 'Vector Store': 'Retrievers' + } + cy.createFixtureWorkflow('open_node_creator_for_connection.json', `open_node_creator_for_connection ${uuid()}`); + + Object.entries(hintMapper).forEach(([node, group]) => { + workflowPage.actions.openNode(node); + cy.get('[data-action=openSelectiveNodeCreator]').contains('Insert one').click(); + nodeCreator.getters.activeSubcategory().should('contain', group); + cy.realPress('Escape'); + }); + }) }); diff --git a/cypress/fixtures/Test_workflow_3.json b/cypress/fixtures/Test_workflow_3.json index 846b50cf193c8..43313c1f602b1 100644 --- a/cypress/fixtures/Test_workflow_3.json +++ b/cypress/fixtures/Test_workflow_3.json @@ -20,20 +20,21 @@ }, { "parameters": { - "values": { - "string": [ - { - "name": "other", - "value": "" - } + "assignments": { + "assignments": [ + { + "id": "2b0f25a2-9483-4579-9f6d-91b7ac2fcb71", + "name": "other", + "value": "", + "type": "string" + } ] - }, - "options": {} + } }, "id": "2dfc690a-95cf-48c2-85a6-2b3bb8cd1d1d", "name": "Set", "type": "n8n-nodes-base.set", - "typeVersion": 1, + "typeVersion": 3.3, "position": [ 920, 300 @@ -43,21 +44,22 @@ "id": "9bee04af-1bfc-4be2-a704-e975cb887ced", "name": "Set1", "type": "n8n-nodes-base.set", - "typeVersion": 1, + "typeVersion": 3.3, "position": [ 1120, 300 ], "parameters": { - "values": { - "string": [ - { - "name": "other", - "value": "" - } + "assignments": { + "assignments": [ + { + "id": "2b0f25a2-9483-4579-9f6d-91b7ac2fcb71", + "name": "other", + "value": "", + "type": "string" + } ] - }, - "options": {} + } } } ], @@ -118,4 +120,4 @@ "instanceId": "fe45a93dd232270eb40d3ba1f7907ad3935bbd72ad5e4ee09ff61e96674f9aef" }, "tags": [] -} \ No newline at end of file +} diff --git a/cypress/fixtures/open_node_creator_for_connection.json b/cypress/fixtures/open_node_creator_for_connection.json new file mode 100644 index 0000000000000..78827d4083fd1 --- /dev/null +++ b/cypress/fixtures/open_node_creator_for_connection.json @@ -0,0 +1,110 @@ +{ + "name": "open_node_creator_for_connection", + "nodes": [ + { + "parameters": {}, + "id": "25ff0c17-7064-4e14-aec6-45c71d63201b", + "name": "When clicking \"Test workflow\"", + "type": "n8n-nodes-base.manualTrigger", + "typeVersion": 1, + "position": [ + 740, + 520 + ] + }, + { + "parameters": {}, + "id": "49f376ca-845b-4737-aac0-073d0e4fa95c", + "name": "Token Splitter", + "type": "@n8n/n8n-nodes-langchain.textSplitterTokenSplitter", + "typeVersion": 1, + "position": [ + 1180, + 540 + ] + }, + { + "parameters": {}, + "id": "d1db5111-4b01-4620-8ccb-a16ea576c363", + "name": "Memory", + "type": "@n8n/n8n-nodes-langchain.memoryXata", + "typeVersion": 1.2, + "position": [ + 940, + 540 + ], + "credentials": { + "xataApi": { + "id": "q1ckaYlHTWCYDtF0", + "name": "Xata Api account" + } + } + }, + { + "parameters": {}, + "id": "b08b6d3a-bef8-42ac-9cef-ec9d4e5402b1", + "name": "Output Parser", + "type": "@n8n/n8n-nodes-langchain.outputParserStructured", + "typeVersion": 1.1, + "position": [ + 1060, + 540 + ] + }, + { + "parameters": {}, + "id": "ee557938-9cf1-4b78-afef-c783c52fd307", + "name": "Tool", + "type": "@n8n/n8n-nodes-langchain.toolWikipedia", + "typeVersion": 1, + "position": [ + 1300, + 540 + ] + }, + { + "parameters": { + "options": {} + }, + "id": "814f2e9c-cc7b-4f3c-89b4-d6eb82bc24df", + "name": "Embeddings", + "type": "@n8n/n8n-nodes-langchain.embeddingsHuggingFaceInference", + "typeVersion": 1, + "position": [ + 1420, + 540 + ] + }, + { + "parameters": { + "tableName": { + "__rl": true, + "mode": "list", + "value": "" + }, + "options": {} + }, + "id": "e8569b0b-a580-4249-9c5e-f1feed5c644e", + "name": "Vector Store", + "type": "@n8n/n8n-nodes-langchain.vectorStoreSupabase", + "typeVersion": 1, + "position": [ + 1540, + 540 + ] + } + ], + "pinData": {}, + "connections": {}, + "active": false, + "settings": { + "executionOrder": "v1" + }, + "versionId": "8e90604c-f7e9-489d-8e43-1cc699b7db04", + "meta": { + "templateCredsSetupCompleted": true, + "instanceId": "27cc9b56542ad45b38725555722c50a1c3fee1670bbb67980558314ee08517c4" + }, + "id": "L3tgfoW660UOSuX6", + "tags": [] +} \ No newline at end of file diff --git a/docker/images/n8n/README.md b/docker/images/n8n/README.md index bde9617fe65aa..573b5eb0a012d 100644 --- a/docker/images/n8n/README.md +++ b/docker/images/n8n/README.md @@ -18,7 +18,6 @@ n8n is an extendable workflow automation tool. With a [fair-code](https://fairco - [Persist data](#persist-data) - [Start with other Database](#start-with-other-database) - [Use with PostgresDB](#use-with-postgresdb) - - [Use with MySQL](#use-with-mysql) - [Passing Sensitive Data via File](#passing-sensitive-data-via-file) - [Example Setup with Lets Encrypt](#example-setup-with-lets-encrypt) - [Updating a running docker-compose instance](#updating-a-running-docker-compose-instance) @@ -95,10 +94,9 @@ docker run -it --rm \ ### Start with other Database By default n8n uses SQLite to save credentials, past executions and workflows. -n8n however also supports PostgresDB, MySQL and MariaDB. To use them simply a few -environment variables have to be set. +n8n however also supports PostgresDB. -It is important to still persist the data in the `/root/.n8n` folder. The reason +It is important to still persist the data in the `/home/node/.n8n` folder. The reason is that it contains n8n user data. That is the name of the webhook (in case) the n8n tunnel gets used and even more important the encryption key for the credentials. If none gets found n8n creates automatically one on @@ -128,37 +126,11 @@ docker run -it --rm \ -e DB_POSTGRESDB_SCHEMA= \ -e DB_POSTGRESDB_PASSWORD= \ -v ~/.n8n:/home/node/.n8n \ - docker.n8n.io/n8nio/n8n \ - n8n start + docker.n8n.io/n8nio/n8n ``` A full working setup with docker-compose can be found [here](https://github.com/n8n-io/n8n/blob/master/docker/compose/withPostgres/README.md) -#### Use with MySQL - -Replace the following placeholders with the actual data: - -- MYSQLDB_DATABASE -- MYSQLDB_HOST -- MYSQLDB_PASSWORD -- MYSQLDB_PORT -- MYSQLDB_USER - -```bash -docker run -it --rm \ - --name n8n \ - -p 5678:5678 \ - -e DB_TYPE=mysqldb \ - -e DB_MYSQLDB_DATABASE= \ - -e DB_MYSQLDB_HOST= \ - -e DB_MYSQLDB_PORT= \ - -e DB_MYSQLDB_USER= \ - -e DB_MYSQLDB_PASSWORD= \ - -v ~/.n8n:/home/node/.n8n \ - docker.n8n.io/n8nio/n8n \ - n8n start -``` - ## Passing Sensitive Data via File To avoid passing sensitive information via environment variables "\_FILE" may be @@ -222,7 +194,7 @@ docker run -it --rm \ docker buildx build --platform linux/amd64,linux/arm64 --build-arg N8N_VERSION= -t n8n: . # For example: -docker buildx build --platform linux/amd64,linux/arm64 --build-arg N8N_VERSION=0.114.0 -t n8n:0.114.0 . +docker buildx build --platform linux/amd64,linux/arm64 --build-arg N8N_VERSION=1.30.1 -t n8n:1.30.1 . ``` ## What does n8n mean and how do you pronounce it? diff --git a/package.json b/package.json index 78237e3134cd4..44bec31d0b9b9 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "n8n-monorepo", - "version": "1.30.0", + "version": "1.31.0", "private": true, "homepage": "https://n8n.io", "engines": { @@ -68,9 +68,10 @@ "tsc-watch": "^6.0.4", "turbo": "1.10.12", "typescript": "*", - "vite": "^5.0.12", - "vitest": "^1.2.1", - "vue-tsc": "^1.8.27" + "vite": "^5.1.5", + "vite-plugin-checker": "^0.6.4", + "vitest": "^1.3.1", + "vue-tsc": "^2.0.4" }, "pnpm": { "onlyBuiltDependencies": [ diff --git a/packages/@n8n/chat/vite.config.ts b/packages/@n8n/chat/vite.config.ts index 62241dacbb9c5..98ecbdc2a9f10 100644 --- a/packages/@n8n/chat/vite.config.ts +++ b/packages/@n8n/chat/vite.config.ts @@ -1,6 +1,7 @@ import { fileURLToPath, URL } from 'node:url'; import { defineConfig } from 'vite'; +import checker from 'vite-plugin-checker'; import { resolve } from 'path'; import vue from '@vitejs/plugin-vue'; import icons from 'unplugin-icons/vite'; @@ -9,15 +10,20 @@ import dts from 'vite-plugin-dts'; const includeVue = process.env.INCLUDE_VUE === 'true'; const srcPath = fileURLToPath(new URL('./src', import.meta.url)); +const plugins = [ + vue(), + icons({ + compiler: 'vue3', + }), + dts(), +]; +if (process.env.ENABLE_TYPE_CHECKING === 'true') { + plugins.push(checker({ vueTsc: true })); +} + // https://vitejs.dev/config/ export default defineConfig({ - plugins: [ - vue(), - icons({ - compiler: 'vue3', - }), - dts(), - ], + plugins, resolve: { alias: { '@': srcPath, diff --git a/packages/@n8n/nodes-langchain/nodes/agents/Agent/Agent.node.ts b/packages/@n8n/nodes-langchain/nodes/agents/Agent/Agent.node.ts index 1ecd9095353b4..d3927e505c2be 100644 --- a/packages/@n8n/nodes-langchain/nodes/agents/Agent/Agent.node.ts +++ b/packages/@n8n/nodes-langchain/nodes/agents/Agent/Agent.node.ts @@ -161,7 +161,7 @@ export class Agent implements INodeType { name: 'agent', icon: 'fa:robot', group: ['transform'], - version: [1, 1.1, 1.2, 1.3, 1.4], + version: [1, 1.1, 1.2, 1.3, 1.4, 1.5], description: 'Generates an action plan and executes it. Can use external tools.', subtitle: "={{ { conversationalAgent: 'Conversational Agent', openAiFunctionsAgent: 'OpenAI Functions Agent', reActAgent: 'ReAct Agent', sqlAgent: 'SQL Agent', planAndExecuteAgent: 'Plan and Execute Agent' }[$parameter.agent] }}", @@ -314,17 +314,18 @@ export class Agent implements INodeType { async execute(this: IExecuteFunctions): Promise { const agentType = this.getNodeParameter('agent', 0, '') as string; + const nodeVersion = this.getNode().typeVersion; if (agentType === 'conversationalAgent') { - return await conversationalAgentExecute.call(this); + return await conversationalAgentExecute.call(this, nodeVersion); } else if (agentType === 'openAiFunctionsAgent') { - return await openAiFunctionsAgentExecute.call(this); + return await openAiFunctionsAgentExecute.call(this, nodeVersion); } else if (agentType === 'reActAgent') { - return await reActAgentAgentExecute.call(this); + return await reActAgentAgentExecute.call(this, nodeVersion); } else if (agentType === 'sqlAgent') { - return await sqlAgentAgentExecute.call(this); + return await sqlAgentAgentExecute.call(this, nodeVersion); } else if (agentType === 'planAndExecuteAgent') { - return await planAndExecuteAgentExecute.call(this); + return await planAndExecuteAgentExecute.call(this, nodeVersion); } throw new NodeOperationError(this.getNode(), `The agent type "${agentType}" is not supported`); diff --git a/packages/@n8n/nodes-langchain/nodes/agents/Agent/agents/ConversationalAgent/execute.ts b/packages/@n8n/nodes-langchain/nodes/agents/Agent/agents/ConversationalAgent/execute.ts index ddf0f4e6ab991..62cc6edbd8f11 100644 --- a/packages/@n8n/nodes-langchain/nodes/agents/Agent/agents/ConversationalAgent/execute.ts +++ b/packages/@n8n/nodes-langchain/nodes/agents/Agent/agents/ConversationalAgent/execute.ts @@ -6,7 +6,6 @@ import { } from 'n8n-workflow'; import { initializeAgentExecutorWithOptions } from 'langchain/agents'; -import type { Tool } from 'langchain/tools'; import type { BaseChatMemory } from 'langchain/memory'; import type { BaseOutputParser } from 'langchain/schema/output_parser'; import { PromptTemplate } from 'langchain/prompts'; @@ -15,10 +14,12 @@ import { isChatInstance, getPromptInputByType, getOptionalOutputParsers, + getConnectedTools, } from '../../../../../utils/helpers'; export async function conversationalAgentExecute( this: IExecuteFunctions, + nodeVersion: number, ): Promise { this.logger.verbose('Executing Conversational Agent'); @@ -31,7 +32,8 @@ export async function conversationalAgentExecute( const memory = (await this.getInputConnectionData(NodeConnectionType.AiMemory, 0)) as | BaseChatMemory | undefined; - const tools = (await this.getInputConnectionData(NodeConnectionType.AiTool, 0)) as Tool[]; + + const tools = await getConnectedTools(this, nodeVersion >= 1.5); const outputParsers = await getOptionalOutputParsers(this); // TODO: Make it possible in the future to use values for other items than just 0 diff --git a/packages/@n8n/nodes-langchain/nodes/agents/Agent/agents/OpenAiFunctionsAgent/execute.ts b/packages/@n8n/nodes-langchain/nodes/agents/Agent/agents/OpenAiFunctionsAgent/execute.ts index 87e6752fb55ec..bcfe28a02a992 100644 --- a/packages/@n8n/nodes-langchain/nodes/agents/Agent/agents/OpenAiFunctionsAgent/execute.ts +++ b/packages/@n8n/nodes-langchain/nodes/agents/Agent/agents/OpenAiFunctionsAgent/execute.ts @@ -7,16 +7,20 @@ import { import type { AgentExecutorInput } from 'langchain/agents'; import { AgentExecutor, OpenAIAgent } from 'langchain/agents'; -import type { Tool } from 'langchain/tools'; import type { BaseOutputParser } from 'langchain/schema/output_parser'; import { PromptTemplate } from 'langchain/prompts'; import { CombiningOutputParser } from 'langchain/output_parsers'; import { BufferMemory, type BaseChatMemory } from 'langchain/memory'; import { ChatOpenAI } from 'langchain/chat_models/openai'; -import { getOptionalOutputParsers, getPromptInputByType } from '../../../../../utils/helpers'; +import { + getConnectedTools, + getOptionalOutputParsers, + getPromptInputByType, +} from '../../../../../utils/helpers'; export async function openAiFunctionsAgentExecute( this: IExecuteFunctions, + nodeVersion: number, ): Promise { this.logger.verbose('Executing OpenAi Functions Agent'); const model = (await this.getInputConnectionData( @@ -33,7 +37,7 @@ export async function openAiFunctionsAgentExecute( const memory = (await this.getInputConnectionData(NodeConnectionType.AiMemory, 0)) as | BaseChatMemory | undefined; - const tools = (await this.getInputConnectionData(NodeConnectionType.AiTool, 0)) as Tool[]; + const tools = await getConnectedTools(this, nodeVersion >= 1.5); const outputParsers = await getOptionalOutputParsers(this); const options = this.getNodeParameter('options', 0, {}) as { systemMessage?: string; diff --git a/packages/@n8n/nodes-langchain/nodes/agents/Agent/agents/PlanAndExecuteAgent/execute.ts b/packages/@n8n/nodes-langchain/nodes/agents/Agent/agents/PlanAndExecuteAgent/execute.ts index 8326b877b9611..735041556816b 100644 --- a/packages/@n8n/nodes-langchain/nodes/agents/Agent/agents/PlanAndExecuteAgent/execute.ts +++ b/packages/@n8n/nodes-langchain/nodes/agents/Agent/agents/PlanAndExecuteAgent/execute.ts @@ -5,16 +5,20 @@ import { NodeOperationError, } from 'n8n-workflow'; -import type { Tool } from 'langchain/tools'; import type { BaseOutputParser } from 'langchain/schema/output_parser'; import { PromptTemplate } from 'langchain/prompts'; import { CombiningOutputParser } from 'langchain/output_parsers'; import type { BaseChatModel } from 'langchain/chat_models/base'; import { PlanAndExecuteAgentExecutor } from 'langchain/experimental/plan_and_execute'; -import { getOptionalOutputParsers, getPromptInputByType } from '../../../../../utils/helpers'; +import { + getConnectedTools, + getOptionalOutputParsers, + getPromptInputByType, +} from '../../../../../utils/helpers'; export async function planAndExecuteAgentExecute( this: IExecuteFunctions, + nodeVersion: number, ): Promise { this.logger.verbose('Executing PlanAndExecute Agent'); const model = (await this.getInputConnectionData( @@ -22,7 +26,7 @@ export async function planAndExecuteAgentExecute( 0, )) as BaseChatModel; - const tools = (await this.getInputConnectionData(NodeConnectionType.AiTool, 0)) as Tool[]; + const tools = await getConnectedTools(this, nodeVersion >= 1.5); const outputParsers = await getOptionalOutputParsers(this); diff --git a/packages/@n8n/nodes-langchain/nodes/agents/Agent/agents/ReActAgent/execute.ts b/packages/@n8n/nodes-langchain/nodes/agents/Agent/agents/ReActAgent/execute.ts index 49f0b8a6ae533..02b34996630ba 100644 --- a/packages/@n8n/nodes-langchain/nodes/agents/Agent/agents/ReActAgent/execute.ts +++ b/packages/@n8n/nodes-langchain/nodes/agents/Agent/agents/ReActAgent/execute.ts @@ -7,12 +7,12 @@ import { import { AgentExecutor, ChatAgent, ZeroShotAgent } from 'langchain/agents'; import type { BaseLanguageModel } from 'langchain/base_language'; -import type { Tool } from 'langchain/tools'; import type { BaseOutputParser } from 'langchain/schema/output_parser'; import { PromptTemplate } from 'langchain/prompts'; import { CombiningOutputParser } from 'langchain/output_parsers'; import type { BaseChatModel } from 'langchain/chat_models/base'; import { + getConnectedTools, getOptionalOutputParsers, getPromptInputByType, isChatInstance, @@ -20,6 +20,7 @@ import { export async function reActAgentAgentExecute( this: IExecuteFunctions, + nodeVersion: number, ): Promise { this.logger.verbose('Executing ReAct Agent'); @@ -27,7 +28,7 @@ export async function reActAgentAgentExecute( | BaseLanguageModel | BaseChatModel; - const tools = (await this.getInputConnectionData(NodeConnectionType.AiTool, 0)) as Tool[]; + const tools = await getConnectedTools(this, nodeVersion >= 1.5); const outputParsers = await getOptionalOutputParsers(this); diff --git a/packages/@n8n/nodes-langchain/nodes/agents/Agent/agents/SqlAgent/execute.ts b/packages/@n8n/nodes-langchain/nodes/agents/Agent/agents/SqlAgent/execute.ts index 245d0cf89ee0b..eae37cb7f9bee 100644 --- a/packages/@n8n/nodes-langchain/nodes/agents/Agent/agents/SqlAgent/execute.ts +++ b/packages/@n8n/nodes-langchain/nodes/agents/Agent/agents/SqlAgent/execute.ts @@ -26,6 +26,7 @@ const parseTablesString = (tablesString: string) => export async function sqlAgentAgentExecute( this: IExecuteFunctions, + nodeVersion: number, ): Promise { this.logger.verbose('Executing SQL Agent'); diff --git a/packages/@n8n/nodes-langchain/nodes/agents/OpenAiAssistant/OpenAiAssistant.node.ts b/packages/@n8n/nodes-langchain/nodes/agents/OpenAiAssistant/OpenAiAssistant.node.ts index 4a74d062ba0d2..4c7243c7d5957 100644 --- a/packages/@n8n/nodes-langchain/nodes/agents/OpenAiAssistant/OpenAiAssistant.node.ts +++ b/packages/@n8n/nodes-langchain/nodes/agents/OpenAiAssistant/OpenAiAssistant.node.ts @@ -1,7 +1,6 @@ import { AgentExecutor } from 'langchain/agents'; import { OpenAI as OpenAIClient } from 'openai'; import { OpenAIAssistantRunnable } from 'langchain/experimental/openai_assistant'; -import { type Tool } from 'langchain/tools'; import { NodeConnectionType, NodeOperationError } from 'n8n-workflow'; import type { IExecuteFunctions, @@ -10,6 +9,7 @@ import type { INodeTypeDescription, } from 'n8n-workflow'; import type { OpenAIToolType } from 'langchain/dist/experimental/openai_assistant/schema'; +import { getConnectedTools } from '../../../utils/helpers'; import { formatToOpenAIAssistantTool } from './utils'; export class OpenAiAssistant implements INodeType { @@ -19,7 +19,7 @@ export class OpenAiAssistant implements INodeType { hidden: true, icon: 'fa:robot', group: ['transform'], - version: 1, + version: [1, 1.1], description: 'Utilizes Assistant API from Open AI.', subtitle: 'Open AI Assistant', defaults: { @@ -311,7 +311,8 @@ export class OpenAiAssistant implements INodeType { }; async execute(this: IExecuteFunctions): Promise { - const tools = (await this.getInputConnectionData(NodeConnectionType.AiTool, 0)) as Tool[]; + const nodeVersion = this.getNode().typeVersion; + const tools = await getConnectedTools(this, nodeVersion > 1); const credentials = await this.getCredentials('openAiApi'); const items = this.getInputData(); diff --git a/packages/@n8n/nodes-langchain/nodes/chains/ChainLLM/ChainLlm.node.ts b/packages/@n8n/nodes-langchain/nodes/chains/ChainLLM/ChainLlm.node.ts index d01f62b5370e4..c23dc97cce462 100644 --- a/packages/@n8n/nodes-langchain/nodes/chains/ChainLLM/ChainLlm.node.ts +++ b/packages/@n8n/nodes-langchain/nodes/chains/ChainLLM/ChainLlm.node.ts @@ -504,7 +504,7 @@ export class ChainLlm implements INodeType { for (let itemIndex = 0; itemIndex < items.length; itemIndex++) { let prompt: string; - if (this.getNode().typeVersion <= 1.2) { + if (this.getNode().typeVersion <= 1.3) { prompt = this.getNodeParameter('prompt', itemIndex) as string; } else { prompt = getPromptInputByType({ diff --git a/packages/@n8n/nodes-langchain/nodes/memory/MemoryBufferWindow/MemoryBufferWindow.node.ts b/packages/@n8n/nodes-langchain/nodes/memory/MemoryBufferWindow/MemoryBufferWindow.node.ts index 3cf6ff66f883e..2b7e205de63fb 100644 --- a/packages/@n8n/nodes-langchain/nodes/memory/MemoryBufferWindow/MemoryBufferWindow.node.ts +++ b/packages/@n8n/nodes-langchain/nodes/memory/MemoryBufferWindow/MemoryBufferWindow.node.ts @@ -10,6 +10,8 @@ import type { BufferWindowMemoryInput } from 'langchain/memory'; import { BufferWindowMemory } from 'langchain/memory'; import { logWrapper } from '../../../utils/logWrapper'; import { getConnectionHintNoticeField } from '../../../utils/sharedFields'; +import { sessionIdOption, sessionKeyProperty } from '../descriptions'; +import { getSessionId } from '../../../utils/helpers'; class MemoryChatBufferSingleton { private static instance: MemoryChatBufferSingleton; @@ -70,7 +72,7 @@ export class MemoryBufferWindow implements INodeType { name: 'memoryBufferWindow', icon: 'fa:database', group: ['transform'], - version: [1, 1.1], + version: [1, 1.1, 1.2], description: 'Stores in n8n memory, so no credentials required', defaults: { name: 'Window Buffer Memory', @@ -119,6 +121,15 @@ export class MemoryBufferWindow implements INodeType { }, }, }, + { + ...sessionIdOption, + displayOptions: { + show: { + '@version': [{ _cnd: { gte: 1.2 } }], + }, + }, + }, + sessionKeyProperty, { displayName: 'Context Window Length', name: 'contextWindowLength', @@ -130,12 +141,21 @@ export class MemoryBufferWindow implements INodeType { }; async supplyData(this: IExecuteFunctions, itemIndex: number): Promise { - const sessionKey = this.getNodeParameter('sessionKey', itemIndex) as string; const contextWindowLength = this.getNodeParameter('contextWindowLength', itemIndex) as number; const workflowId = this.getWorkflow().id; const memoryInstance = MemoryChatBufferSingleton.getInstance(); - const memory = await memoryInstance.getMemory(`${workflowId}__${sessionKey}`, { + const nodeVersion = this.getNode().typeVersion; + + let sessionId; + + if (nodeVersion >= 1.2) { + sessionId = getSessionId(this, itemIndex); + } else { + sessionId = this.getNodeParameter('sessionKey', itemIndex) as string; + } + + const memory = await memoryInstance.getMemory(`${workflowId}__${sessionId}`, { k: contextWindowLength, inputKey: 'input', memoryKey: 'chat_history', diff --git a/packages/@n8n/nodes-langchain/nodes/memory/MemoryManager/MemoryManager.node.ts b/packages/@n8n/nodes-langchain/nodes/memory/MemoryManager/MemoryManager.node.ts index de84f62f2f475..c1d91a26d0ef0 100644 --- a/packages/@n8n/nodes-langchain/nodes/memory/MemoryManager/MemoryManager.node.ts +++ b/packages/@n8n/nodes-langchain/nodes/memory/MemoryManager/MemoryManager.node.ts @@ -1,11 +1,11 @@ /* eslint-disable n8n-nodes-base/node-dirname-against-convention */ -import { - NodeConnectionType, - type IDataObject, - type IExecuteFunctions, - type INodeExecutionData, - type INodeType, - type INodeTypeDescription, +import { NodeConnectionType } from 'n8n-workflow'; +import type { + IDataObject, + IExecuteFunctions, + INodeExecutionData, + INodeType, + INodeTypeDescription, } from 'n8n-workflow'; import type { BaseChatMemory } from 'langchain/memory'; import { AIMessage, SystemMessage, HumanMessage, type BaseMessage } from 'langchain/schema'; @@ -37,13 +37,39 @@ function simplifyMessages(messages: BaseMessage[]) { return transformedMessages; } +const prepareOutputSetup = (ctx: IExecuteFunctions, version: number, memory: BaseChatMemory) => { + if (version === 1) { + //legacy behavior of insert and delete for version 1 + return async (i: number) => { + const messages = await memory.chatHistory.getMessages(); + + const serializedMessages = messages?.map((message) => message.toJSON()) ?? []; + + const executionData = ctx.helpers.constructExecutionMetaData( + ctx.helpers.returnJsonArray(serializedMessages as unknown as IDataObject[]), + { itemData: { item: i } }, + ); + + return executionData; + }; + } + return async (i: number) => { + return [ + { + json: { success: true }, + pairedItem: { item: i }, + }, + ]; + }; +}; + export class MemoryManager implements INodeType { description: INodeTypeDescription = { displayName: 'Chat Memory Manager', name: 'memoryManager', icon: 'fa:database', group: ['transform'], - version: 1, + version: [1, 1.1], description: 'Manage chat messages memory and use it in the workflow', defaults: { name: 'Chat Memory Manager', @@ -240,21 +266,46 @@ export class MemoryManager implements INodeType { }, }, }, + { + displayName: 'Options', + name: 'options', + placeholder: 'Add Option', + type: 'collection', + default: {}, + options: [ + { + displayName: 'Group Messages', + name: 'groupMessages', + type: 'boolean', + default: true, + description: + 'Whether to group messages into a single item or return each message as a separate item', + }, + ], + displayOptions: { + show: { + mode: ['load'], + }, + }, + }, ], }; async execute(this: IExecuteFunctions): Promise { + const nodeVersion = this.getNode().typeVersion; + const items = this.getInputData(); + const mode = this.getNodeParameter('mode', 0, 'load') as 'load' | 'insert' | 'delete'; const memory = (await this.getInputConnectionData( NodeConnectionType.AiMemory, 0, )) as BaseChatMemory; - const items = this.getInputData(); - const result = []; - for (let i = 0; i < items.length; i++) { - const mode = this.getNodeParameter('mode', i) as 'load' | 'insert' | 'delete'; + const prepareOutput = prepareOutputSetup(this, nodeVersion, memory); + + const returnData: INodeExecutionData[] = []; - let messages = [...(await memory.chatHistory.getMessages())]; + for (let i = 0; i < items.length; i++) { + const messages = await memory.chatHistory.getMessages(); if (mode === 'delete') { const deleteMode = this.getNodeParameter('deleteMode', i) as 'lastN' | 'all'; @@ -272,6 +323,8 @@ export class MemoryManager implements INodeType { } else { await memory.chatHistory.clear(); } + + returnData.push(...(await prepareOutput(i))); } if (mode === 'insert') { @@ -301,29 +354,57 @@ export class MemoryManager implements INodeType { await memory.chatHistory.addMessage(MessageClass); } - } - // Refresh messages from memory - messages = await memory.chatHistory.getMessages(); - - const simplifyOutput = this.getNodeParameter('simplifyOutput', i, false) as boolean; - if (simplifyOutput && messages) { - return [ - this.helpers.constructExecutionMetaData( - this.helpers.returnJsonArray(simplifyMessages(messages)), - { itemData: { item: i } }, - ), - ]; + returnData.push(...(await prepareOutput(i))); } - const serializedMessages = messages?.map((message) => message.toJSON()) ?? []; - const executionData = this.helpers.constructExecutionMetaData( - this.helpers.returnJsonArray(serializedMessages as unknown as IDataObject[]), - { itemData: { item: i } }, - ); - result.push(...executionData); + if (mode === 'load') { + const simplifyOutput = this.getNodeParameter('simplifyOutput', i, false) as boolean; + const options = this.getNodeParameter('options', i); + + //Load mode, legacy behavior for version 1, buggy - outputs only for single input item + if (simplifyOutput && messages.length && nodeVersion === 1) { + const groupMessages = options.groupMessages as boolean; + const output = simplifyMessages(messages); + + return [ + this.helpers.constructExecutionMetaData( + this.helpers.returnJsonArray( + groupMessages ? [{ messages: output, messagesCount: output.length }] : output, + ), + { itemData: { item: i } }, + ), + ]; + } + + let groupMessages = true; + //disable grouping if explicitly set to false + if (options.groupMessages === false) { + groupMessages = false; + } + //disable grouping if not set and node version is 1 (legacy behavior) + if (options.groupMessages === undefined && nodeVersion === 1) { + groupMessages = false; + } + + let output: IDataObject[] = + (simplifyOutput + ? simplifyMessages(messages) + : (messages?.map((message) => message.toJSON()) as unknown as IDataObject[])) ?? []; + + if (groupMessages) { + output = [{ messages: output, messagesCount: output.length }]; + } + + const executionData = this.helpers.constructExecutionMetaData( + this.helpers.returnJsonArray(output), + { itemData: { item: i } }, + ); + + returnData.push(...executionData); + } } - return await this.prepareOutputData(result); + return [returnData]; } } diff --git a/packages/@n8n/nodes-langchain/nodes/memory/MemoryMotorhead/MemoryMotorhead.node.ts b/packages/@n8n/nodes-langchain/nodes/memory/MemoryMotorhead/MemoryMotorhead.node.ts index cc3a030c36ea9..a50bdc0ec0375 100644 --- a/packages/@n8n/nodes-langchain/nodes/memory/MemoryMotorhead/MemoryMotorhead.node.ts +++ b/packages/@n8n/nodes-langchain/nodes/memory/MemoryMotorhead/MemoryMotorhead.node.ts @@ -10,6 +10,8 @@ import { import { MotorheadMemory } from 'langchain/memory'; import { logWrapper } from '../../../utils/logWrapper'; import { getConnectionHintNoticeField } from '../../../utils/sharedFields'; +import { sessionIdOption, sessionKeyProperty } from '../descriptions'; +import { getSessionId } from '../../../utils/helpers'; export class MemoryMotorhead implements INodeType { description: INodeTypeDescription = { @@ -17,7 +19,7 @@ export class MemoryMotorhead implements INodeType { name: 'memoryMotorhead', icon: 'fa:file-export', group: ['transform'], - version: [1, 1.1], + version: [1, 1.1, 1.2], description: 'Use Motorhead Memory', defaults: { name: 'Motorhead', @@ -72,13 +74,29 @@ export class MemoryMotorhead implements INodeType { }, }, }, + { + ...sessionIdOption, + displayOptions: { + show: { + '@version': [{ _cnd: { gte: 1.2 } }], + }, + }, + }, + sessionKeyProperty, ], }; async supplyData(this: IExecuteFunctions, itemIndex: number): Promise { const credentials = await this.getCredentials('motorheadApi'); + const nodeVersion = this.getNode().typeVersion; + + let sessionId; - const sessionId = this.getNodeParameter('sessionId', itemIndex) as string; + if (nodeVersion >= 1.2) { + sessionId = getSessionId(this, itemIndex); + } else { + sessionId = this.getNodeParameter('sessionId', itemIndex) as string; + } const memory = new MotorheadMemory({ sessionId, diff --git a/packages/@n8n/nodes-langchain/nodes/memory/MemoryRedisChat/MemoryRedisChat.node.ts b/packages/@n8n/nodes-langchain/nodes/memory/MemoryRedisChat/MemoryRedisChat.node.ts index 788fe3884980c..1758ebfa18ec0 100644 --- a/packages/@n8n/nodes-langchain/nodes/memory/MemoryRedisChat/MemoryRedisChat.node.ts +++ b/packages/@n8n/nodes-langchain/nodes/memory/MemoryRedisChat/MemoryRedisChat.node.ts @@ -14,6 +14,8 @@ import type { RedisClientOptions } from 'redis'; import { createClient } from 'redis'; import { logWrapper } from '../../../utils/logWrapper'; import { getConnectionHintNoticeField } from '../../../utils/sharedFields'; +import { sessionIdOption, sessionKeyProperty } from '../descriptions'; +import { getSessionId } from '../../../utils/helpers'; export class MemoryRedisChat implements INodeType { description: INodeTypeDescription = { @@ -21,7 +23,7 @@ export class MemoryRedisChat implements INodeType { name: 'memoryRedisChat', icon: 'file:redis.svg', group: ['transform'], - version: [1, 1.1], + version: [1, 1.1, 1.2], description: 'Stores the chat history in Redis.', defaults: { name: 'Redis Chat Memory', @@ -76,6 +78,15 @@ export class MemoryRedisChat implements INodeType { }, }, }, + { + ...sessionIdOption, + displayOptions: { + show: { + '@version': [{ _cnd: { gte: 1.2 } }], + }, + }, + }, + sessionKeyProperty, { displayName: 'Session Time To Live', name: 'sessionTTL', @@ -89,9 +100,18 @@ export class MemoryRedisChat implements INodeType { async supplyData(this: IExecuteFunctions, itemIndex: number): Promise { const credentials = await this.getCredentials('redis'); - const sessionKey = this.getNodeParameter('sessionKey', itemIndex) as string; + const nodeVersion = this.getNode().typeVersion; + const sessionTTL = this.getNodeParameter('sessionTTL', itemIndex, 0) as number; + let sessionId; + + if (nodeVersion >= 1.2) { + sessionId = getSessionId(this, itemIndex); + } else { + sessionId = this.getNodeParameter('sessionKey', itemIndex) as string; + } + const redisOptions: RedisClientOptions = { socket: { host: credentials.host as string, @@ -115,7 +135,7 @@ export class MemoryRedisChat implements INodeType { const redisChatConfig: RedisChatMessageHistoryInput = { client, - sessionId: sessionKey, + sessionId, }; if (sessionTTL > 0) { diff --git a/packages/@n8n/nodes-langchain/nodes/memory/MemoryXata/MemoryXata.node.ts b/packages/@n8n/nodes-langchain/nodes/memory/MemoryXata/MemoryXata.node.ts index f2f3016dafbf2..baeee639f97e7 100644 --- a/packages/@n8n/nodes-langchain/nodes/memory/MemoryXata/MemoryXata.node.ts +++ b/packages/@n8n/nodes-langchain/nodes/memory/MemoryXata/MemoryXata.node.ts @@ -6,13 +6,16 @@ import { BufferMemory } from 'langchain/memory'; import { BaseClient } from '@xata.io/client'; import { logWrapper } from '../../../utils/logWrapper'; import { getConnectionHintNoticeField } from '../../../utils/sharedFields'; +import { sessionIdOption, sessionKeyProperty } from '../descriptions'; +import { getSessionId } from '../../../utils/helpers'; + export class MemoryXata implements INodeType { description: INodeTypeDescription = { displayName: 'Xata', name: 'memoryXata', icon: 'file:xata.svg', group: ['transform'], - version: [1, 1.1], + version: [1, 1.1, 1.2], description: 'Use Xata Memory', defaults: { name: 'Xata', @@ -69,11 +72,29 @@ export class MemoryXata implements INodeType { }, }, }, + { + ...sessionIdOption, + displayOptions: { + show: { + '@version': [{ _cnd: { gte: 1.2 } }], + }, + }, + }, + sessionKeyProperty, ], }; async supplyData(this: IExecuteFunctions, itemIndex: number): Promise { const credentials = await this.getCredentials('xataApi'); + const nodeVersion = this.getNode().typeVersion; + + let sessionId; + + if (nodeVersion >= 1.2) { + sessionId = getSessionId(this, itemIndex); + } else { + sessionId = this.getNodeParameter('sessionId', itemIndex) as string; + } const xataClient = new BaseClient({ apiKey: credentials.apiKey as string, @@ -81,8 +102,6 @@ export class MemoryXata implements INodeType { databaseURL: credentials.databaseEndpoint as string, }); - const sessionId = this.getNodeParameter('sessionId', itemIndex) as string; - const table = (credentials.databaseEndpoint as string).match( /https:\/\/[^.]+\.[^.]+\.xata\.sh\/db\/([^\/:]+)/, ); @@ -94,18 +113,21 @@ export class MemoryXata implements INodeType { ); } + const chatHistory = new XataChatMessageHistory({ + table: table[1], + sessionId, + client: xataClient, + apiKey: credentials.apiKey as string, + }); + const memory = new BufferMemory({ - chatHistory: new XataChatMessageHistory({ - table: table[1], - sessionId, - client: xataClient, - apiKey: credentials.apiKey as string, - }), + chatHistory, memoryKey: 'chat_history', returnMessages: true, inputKey: 'input', outputKey: 'output', }); + return { response: logWrapper(memory, this), }; diff --git a/packages/@n8n/nodes-langchain/nodes/memory/MemoryZep/MemoryZep.node.ts b/packages/@n8n/nodes-langchain/nodes/memory/MemoryZep/MemoryZep.node.ts index d1b0d87ba5d2c..8de27cb8bab2c 100644 --- a/packages/@n8n/nodes-langchain/nodes/memory/MemoryZep/MemoryZep.node.ts +++ b/packages/@n8n/nodes-langchain/nodes/memory/MemoryZep/MemoryZep.node.ts @@ -9,6 +9,8 @@ import { import { ZepMemory } from 'langchain/memory/zep'; import { logWrapper } from '../../../utils/logWrapper'; import { getConnectionHintNoticeField } from '../../../utils/sharedFields'; +import { sessionIdOption, sessionKeyProperty } from '../descriptions'; +import { getSessionId } from '../../../utils/helpers'; export class MemoryZep implements INodeType { description: INodeTypeDescription = { @@ -17,7 +19,7 @@ export class MemoryZep implements INodeType { // eslint-disable-next-line n8n-nodes-base/node-class-description-icon-not-svg icon: 'file:zep.png', group: ['transform'], - version: [1, 1.1], + version: [1, 1.1, 1.2], description: 'Use Zep Memory', defaults: { name: 'Zep', @@ -72,6 +74,15 @@ export class MemoryZep implements INodeType { }, }, }, + { + ...sessionIdOption, + displayOptions: { + show: { + '@version': [{ _cnd: { gte: 1.2 } }], + }, + }, + }, + sessionKeyProperty, ], }; @@ -81,8 +92,15 @@ export class MemoryZep implements INodeType { apiUrl: string; }; - // TODO: Should it get executed once per item or not? - const sessionId = this.getNodeParameter('sessionId', itemIndex) as string; + const nodeVersion = this.getNode().typeVersion; + + let sessionId; + + if (nodeVersion >= 1.2) { + sessionId = getSessionId(this, itemIndex); + } else { + sessionId = this.getNodeParameter('sessionId', itemIndex) as string; + } const memory = new ZepMemory({ sessionId, diff --git a/packages/@n8n/nodes-langchain/nodes/memory/descriptions.ts b/packages/@n8n/nodes-langchain/nodes/memory/descriptions.ts new file mode 100644 index 0000000000000..5f722c4647828 --- /dev/null +++ b/packages/@n8n/nodes-langchain/nodes/memory/descriptions.ts @@ -0,0 +1,35 @@ +import type { INodeProperties } from 'n8n-workflow'; + +export const sessionIdOption: INodeProperties = { + displayName: 'Session ID', + name: 'sessionIdType', + type: 'options', + options: [ + { + // eslint-disable-next-line n8n-nodes-base/node-param-display-name-miscased + name: 'Take from previous node automatically', + value: 'fromInput', + description: 'Looks for an input field called sessionId', + }, + { + // eslint-disable-next-line n8n-nodes-base/node-param-display-name-miscased + name: 'Define below', + value: 'customKey', + description: 'Use an expression to reference data in previous nodes or enter static text', + }, + ], + default: 'fromInput', +}; + +export const sessionKeyProperty: INodeProperties = { + displayName: 'Key', + name: 'sessionKey', + type: 'string', + default: '', + description: 'The key to use to store session ID in the memory', + displayOptions: { + show: { + sessionIdType: ['customKey'], + }, + }, +}; diff --git a/packages/@n8n/nodes-langchain/nodes/output_parser/OutputParserStructured/OutputParserStructured.node.ts b/packages/@n8n/nodes-langchain/nodes/output_parser/OutputParserStructured/OutputParserStructured.node.ts index 6ee592ef39905..e4fa7abfc8c1d 100644 --- a/packages/@n8n/nodes-langchain/nodes/output_parser/OutputParserStructured/OutputParserStructured.node.ts +++ b/packages/@n8n/nodes-langchain/nodes/output_parser/OutputParserStructured/OutputParserStructured.node.ts @@ -41,6 +41,7 @@ class N8nStructuredOutputParser extends StructuredOutput static fromZedJsonSchema( schema: JSONSchema7, + nodeVersion: number, ): StructuredOutputParser> { // Make sure to remove the description from root schema const { description, ...restOfSchema } = schema; @@ -51,30 +52,37 @@ class N8nStructuredOutputParser extends StructuredOutput // eslint-disable-next-line @typescript-eslint/no-implied-eval const itemSchema = new Function('z', `return (${zodSchemaString})`)(z) as z.ZodSchema; - const returnSchema = z.object({ - [STRUCTURED_OUTPUT_KEY]: z - .object({ - [STRUCTURED_OUTPUT_OBJECT_KEY]: itemSchema.optional(), - [STRUCTURED_OUTPUT_ARRAY_KEY]: z.array(itemSchema).optional(), - }) - .describe( - `Wrapper around the output data. It can only contain ${STRUCTURED_OUTPUT_OBJECT_KEY} or ${STRUCTURED_OUTPUT_ARRAY_KEY} but never both.`, - ) - .refine( - (data) => { - // Validate that one and only one of the properties exists - return ( - Boolean(data[STRUCTURED_OUTPUT_OBJECT_KEY]) !== - Boolean(data[STRUCTURED_OUTPUT_ARRAY_KEY]) - ); - }, - { - message: - 'One and only one of __structured__output__object and __structured__output__array should be present.', - path: [STRUCTURED_OUTPUT_KEY], - }, - ), - }); + let returnSchema: z.ZodSchema; + if (nodeVersion === 1) { + returnSchema = z.object({ + [STRUCTURED_OUTPUT_KEY]: z + .object({ + [STRUCTURED_OUTPUT_OBJECT_KEY]: itemSchema.optional(), + [STRUCTURED_OUTPUT_ARRAY_KEY]: z.array(itemSchema).optional(), + }) + .describe( + `Wrapper around the output data. It can only contain ${STRUCTURED_OUTPUT_OBJECT_KEY} or ${STRUCTURED_OUTPUT_ARRAY_KEY} but never both.`, + ) + .refine( + (data) => { + // Validate that one and only one of the properties exists + return ( + Boolean(data[STRUCTURED_OUTPUT_OBJECT_KEY]) !== + Boolean(data[STRUCTURED_OUTPUT_ARRAY_KEY]) + ); + }, + { + message: + 'One and only one of __structured__output__object and __structured__output__array should be present.', + path: [STRUCTURED_OUTPUT_KEY], + }, + ), + }); + } else { + returnSchema = z.object({ + output: itemSchema.optional(), + }); + } return N8nStructuredOutputParser.fromZodSchema(returnSchema); } @@ -85,7 +93,8 @@ export class OutputParserStructured implements INodeType { name: 'outputParserStructured', icon: 'fa:code', group: ['transform'], - version: 1, + version: [1, 1.1], + defaultVersion: 1.1, description: 'Return data in a defined JSON format', defaults: { name: 'Structured Output Parser', @@ -152,11 +161,20 @@ export class OutputParserStructured implements INodeType { let itemSchema: JSONSchema7; try { itemSchema = jsonParse(schema); + + // If the type is not defined, we assume it's an object + if (itemSchema.type === undefined) { + itemSchema = { + type: 'object', + properties: itemSchema.properties || (itemSchema as { [key: string]: JSONSchema7 }), + }; + } } catch (error) { throw new NodeOperationError(this.getNode(), 'Error during parsing of JSON Schema.'); } - const parser = N8nStructuredOutputParser.fromZedJsonSchema(itemSchema); + const nodeVersion = this.getNode().typeVersion; + const parser = N8nStructuredOutputParser.fromZedJsonSchema(itemSchema, nodeVersion); return { response: logWrapper(parser, this), diff --git a/packages/@n8n/nodes-langchain/nodes/vendors/OpenAi/actions/assistant/message.operation.ts b/packages/@n8n/nodes-langchain/nodes/vendors/OpenAi/actions/assistant/message.operation.ts index a11bc7ab2b46e..3623c18fd4e0c 100644 --- a/packages/@n8n/nodes-langchain/nodes/vendors/OpenAi/actions/assistant/message.operation.ts +++ b/packages/@n8n/nodes-langchain/nodes/vendors/OpenAi/actions/assistant/message.operation.ts @@ -1,15 +1,17 @@ import { AgentExecutor } from 'langchain/agents'; -import type { Tool } from 'langchain/tools'; + import { OpenAIAssistantRunnable } from 'langchain/experimental/openai_assistant'; import type { OpenAIToolType } from 'langchain/dist/experimental/openai_assistant/schema'; import { OpenAI as OpenAIClient } from 'openai'; -import { NodeConnectionType, NodeOperationError, updateDisplayOptions } from 'n8n-workflow'; +import { NodeOperationError, updateDisplayOptions } from 'n8n-workflow'; import type { IExecuteFunctions, INodeExecutionData, INodeProperties } from 'n8n-workflow'; import { formatToOpenAIAssistantTool } from '../../helpers/utils'; import { assistantRLC } from '../descriptions'; +import { getConnectedTools } from '../../../../../utils/helpers'; + const properties: INodeProperties[] = [ assistantRLC, { @@ -97,6 +99,7 @@ export const description = updateDisplayOptions(displayOptions, properties); export async function execute(this: IExecuteFunctions, i: number): Promise { const credentials = await this.getCredentials('openAiApi'); + const nodeVersion = this.getNode().typeVersion; const prompt = this.getNodeParameter('prompt', i) as string; @@ -131,7 +134,7 @@ export async function execute(this: IExecuteFunctions, i: number): Promise 1); if (tools.length) { const transformedConnectedTools = tools?.map(formatToOpenAIAssistantTool) ?? []; diff --git a/packages/@n8n/nodes-langchain/nodes/vendors/OpenAi/actions/text/message.operation.ts b/packages/@n8n/nodes-langchain/nodes/vendors/OpenAi/actions/text/message.operation.ts index 762851a39ad08..d80ac6bdeb8a0 100644 --- a/packages/@n8n/nodes-langchain/nodes/vendors/OpenAi/actions/text/message.operation.ts +++ b/packages/@n8n/nodes-langchain/nodes/vendors/OpenAi/actions/text/message.operation.ts @@ -4,13 +4,12 @@ import type { INodeExecutionData, IDataObject, } from 'n8n-workflow'; -import { NodeConnectionType, updateDisplayOptions } from 'n8n-workflow'; - -import type { Tool } from 'langchain/tools'; +import { updateDisplayOptions } from 'n8n-workflow'; import { apiRequest } from '../../transport'; import type { ChatCompletion } from '../../helpers/interfaces'; import { formatToOpenAIAssistantTool } from '../../helpers/utils'; import { modelRLC } from '../descriptions'; +import { getConnectedTools } from '../../../../../utils/helpers'; const properties: INodeProperties[] = [ modelRLC, @@ -166,11 +165,22 @@ const displayOptions = { export const description = updateDisplayOptions(displayOptions, properties); export async function execute(this: IExecuteFunctions, i: number): Promise { + const nodeVersion = this.getNode().typeVersion; const model = this.getNodeParameter('modelId', i, '', { extractValue: true }); let messages = this.getNodeParameter('messages.values', i, []) as IDataObject[]; const options = this.getNodeParameter('options', i, {}); const jsonOutput = this.getNodeParameter('jsonOutput', i, false) as boolean; + if (options.maxTokens !== undefined) { + options.max_tokens = options.maxTokens; + delete options.maxTokens; + } + + if (options.topP !== undefined) { + options.top_p = options.topP; + delete options.topP; + } + let response_format; if (jsonOutput) { response_format = { type: 'json_object' }; @@ -183,8 +193,7 @@ export async function execute(this: IExecuteFunctions, i: number): Promise 1); let tools; if (externalTools.length) { diff --git a/packages/@n8n/nodes-langchain/nodes/vendors/OpenAi/actions/versionDescription.ts b/packages/@n8n/nodes-langchain/nodes/vendors/OpenAi/actions/versionDescription.ts index f3e8177e3f262..4e25b64f2491c 100644 --- a/packages/@n8n/nodes-langchain/nodes/vendors/OpenAi/actions/versionDescription.ts +++ b/packages/@n8n/nodes-langchain/nodes/vendors/OpenAi/actions/versionDescription.ts @@ -59,7 +59,7 @@ export const versionDescription: INodeTypeDescription = { name: 'openAi', icon: 'file:openAi.svg', group: ['transform'], - version: 1, + version: [1, 1.1], subtitle: `={{(${prettifyOperation})($parameter.resource, $parameter.operation)}}`, description: 'Message an assistant or GPT, analyze images, generate audio, etc.', defaults: { @@ -74,7 +74,7 @@ export const versionDescription: INodeTypeDescription = { resources: { primaryDocumentation: [ { - url: 'https://docs.n8n.io/integrations/builtin/app-nodes/n8n-nodes-base.openai/', + url: 'https://docs.n8n.io/integrations/builtin/app-nodes/n8n-nodes-langchain.openai/', }, ], }, diff --git a/packages/@n8n/nodes-langchain/package.json b/packages/@n8n/nodes-langchain/package.json index 8f63836183dea..cf2141858ee17 100644 --- a/packages/@n8n/nodes-langchain/package.json +++ b/packages/@n8n/nodes-langchain/package.json @@ -1,6 +1,6 @@ { "name": "@n8n/n8n-nodes-langchain", - "version": "0.15.0", + "version": "0.16.0", "description": "", "license": "SEE LICENSE IN LICENSE.md", "homepage": "https://n8n.io", diff --git a/packages/@n8n/nodes-langchain/utils/helpers.ts b/packages/@n8n/nodes-langchain/utils/helpers.ts index c52f086ec3cde..7a91f1c17c100 100644 --- a/packages/@n8n/nodes-langchain/utils/helpers.ts +++ b/packages/@n8n/nodes-langchain/utils/helpers.ts @@ -3,7 +3,8 @@ import type { EventNamesAiNodesType, IDataObject, IExecuteFunctions } from 'n8n- import { BaseChatModel } from 'langchain/chat_models/base'; import { BaseChatModel as BaseChatModelCore } from '@langchain/core/language_models/chat_models'; import type { BaseOutputParser } from '@langchain/core/output_parsers'; -import { BaseMessage } from 'langchain/schema'; +import type { BaseMessage } from 'langchain/schema'; +import { DynamicTool, type Tool } from 'langchain/tools'; export function getMetadataFiltersValues( ctx: IExecuteFunctions, @@ -67,6 +68,39 @@ export function getPromptInputByType(options: { return input; } +export function getSessionId( + ctx: IExecuteFunctions, + itemIndex: number, + selectorKey = 'sessionIdType', + autoSelect = 'fromInput', + customKey = 'sessionKey', +) { + let sessionId = ''; + const selectorType = ctx.getNodeParameter(selectorKey, itemIndex) as string; + + if (selectorType === autoSelect) { + sessionId = ctx.evaluateExpression('{{ $json.sessionId }}', itemIndex) as string; + if (sessionId === '' || sessionId === undefined) { + throw new NodeOperationError(ctx.getNode(), 'No session ID found', { + description: + "Expected to find the session ID in an input field called 'sessionId' (this is what the chat trigger node outputs). To use something else, change the 'Session ID' parameter", + itemIndex, + }); + } + } else { + sessionId = ctx.getNodeParameter(customKey, itemIndex, '') as string; + if (sessionId === '' || sessionId === undefined) { + throw new NodeOperationError(ctx.getNode(), 'Key parameter is empty', { + description: + "Provide a key to use as session ID in the 'Key' parameter or use the 'Take from previous node automatically' option to use the session ID from the previous node, e.t. chat trigger node", + itemIndex, + }); + } + } + + return sessionId; +} + export async function logAiEvent( executeFunctions: IExecuteFunctions, event: EventNamesAiNodesType, @@ -79,7 +113,7 @@ export async function logAiEvent( } } -export function serializeChatHistory (chatHistory: Array): string { +export function serializeChatHistory(chatHistory: BaseMessage[]): string { return chatHistory .map((chatMessage) => { if (chatMessage._getType() === 'human') { @@ -92,3 +126,26 @@ export function serializeChatHistory (chatHistory: Array): string { }) .join('\n'); } + +export const getConnectedTools = async (ctx: IExecuteFunctions, enforceUniqueNames: boolean) => { + const connectedTools = ((await ctx.getInputConnectionData(NodeConnectionType.AiTool, 0)) as Tool[]) || []; + + if (!enforceUniqueNames) return connectedTools; + + const seenNames = new Set(); + + for (const tool of connectedTools) { + if (!(tool instanceof DynamicTool)) continue; + + const { name } = tool; + if (seenNames.has(name)) { + throw new NodeOperationError( + ctx.getNode(), + `You have multiple tools with the same name: '${name}', please rename them to avoid conflicts`, + ); + } + seenNames.add(name); + } + + return connectedTools; +}; diff --git a/packages/@n8n/nodes-langchain/utils/logWrapper.ts b/packages/@n8n/nodes-langchain/utils/logWrapper.ts index 2b358f7d68453..37b19b6324b34 100644 --- a/packages/@n8n/nodes-langchain/utils/logWrapper.ts +++ b/packages/@n8n/nodes-langchain/utils/logWrapper.ts @@ -15,10 +15,10 @@ import type { BaseDocumentLoader } from 'langchain/document_loaders/base'; import type { BaseCallbackConfig, Callbacks } from 'langchain/dist/callbacks/manager'; import { BaseLLM } from 'langchain/llms/base'; import { BaseChatMemory } from 'langchain/memory'; -import type { MemoryVariables } from 'langchain/dist/memory/base'; +import type { MemoryVariables, OutputValues } from 'langchain/dist/memory/base'; import { BaseRetriever } from 'langchain/schema/retriever'; import type { FormatInstructionsOptions } from 'langchain/schema/output_parser'; -import { BaseOutputParser } from 'langchain/schema/output_parser'; +import { BaseOutputParser, OutputParserException } from 'langchain/schema/output_parser'; import { isObject } from 'lodash'; import { N8nJsonLoader } from './N8nJsonLoader'; import { N8nBinaryLoader } from './N8nBinaryLoader'; @@ -44,6 +44,10 @@ export async function callMethodAsync( try { return await parameters.method.call(this, ...parameters.arguments); } catch (e) { + // Langchain checks for OutputParserException to run retry chain + // for auto-fixing the output so skip wrapping in this case + if (e instanceof OutputParserException) throw e; + // Propagate errors from sub-nodes if (e.functionality === 'configuration-node') throw e; const connectedNode = parameters.executeFunctions.getNode(); @@ -144,35 +148,37 @@ export function logWrapper( arguments: [values], })) as MemoryVariables; + const chatHistory = (response?.chat_history as BaseMessage[]) ?? response; + executeFunctions.addOutputData(connectionType, index, [ - [{ json: { action: 'loadMemoryVariables', response } }], + [{ json: { action: 'loadMemoryVariables', chatHistory } }], ]); return response; }; - } else if ( - prop === 'outputKey' && - 'outputKey' in target && - target.constructor.name === 'BufferWindowMemory' - ) { - connectionType = NodeConnectionType.AiMemory; - const { index } = executeFunctions.addInputData(connectionType, [ - [{ json: { action: 'chatHistory' } }], - ]); - const response = target[prop]; - - target.chatHistory - .getMessages() - .then((messages) => { - executeFunctions.addOutputData(NodeConnectionType.AiMemory, index, [ - [{ json: { action: 'chatHistory', chatHistory: messages } }], - ]); - }) - .catch((error: Error) => { - executeFunctions.addOutputData(NodeConnectionType.AiMemory, index, [ - [{ json: { action: 'chatHistory', error } }], - ]); - }); - return response; + } else if (prop === 'saveContext' && 'saveContext' in target) { + return async (input: InputValues, output: OutputValues): Promise => { + connectionType = NodeConnectionType.AiMemory; + + const { index } = executeFunctions.addInputData(connectionType, [ + [{ json: { action: 'saveContext', input, output } }], + ]); + + const response = (await callMethodAsync.call(target, { + executeFunctions, + connectionType, + currentNodeRunIndex: index, + method: target[prop], + arguments: [input, output], + })) as MemoryVariables; + + const chatHistory = await target.chatHistory.getMessages(); + + executeFunctions.addOutputData(connectionType, index, [ + [{ json: { action: 'saveContext', chatHistory } }], + ]); + + return response; + }; } } diff --git a/packages/@n8n/nodes-langchain/utils/sharedFields.ts b/packages/@n8n/nodes-langchain/utils/sharedFields.ts index f51d4727b5b8c..ffc9640aafe03 100644 --- a/packages/@n8n/nodes-langchain/utils/sharedFields.ts +++ b/packages/@n8n/nodes-langchain/utils/sharedFields.ts @@ -79,8 +79,15 @@ function determineArticle(nextWord: string): string { const vowels = /^[aeiouAEIOU]/; return vowels.test(nextWord) ? 'an' : 'a'; } +const getConnectionParameterString = (connectionType: string) => { + if (connectionType === '') return "data-action-parameter-creatorview='AI'"; + + return `data-action-parameter-connectiontype='${connectionType}'`; +}; const getAhref = (connectionType: { connection: string; locale: string }) => - `${connectionType.locale}`; + `${connectionType.locale}`; export function getConnectionHintNoticeField( connectionTypes: AllowedConnectionTypes[], @@ -105,12 +112,15 @@ export function getConnectionHintNoticeField( if (groupedConnections.size === 1) { const [[connection, locales]] = Array.from(groupedConnections); + displayName = `This node must be connected to ${determineArticle(locales[0])} ${locales[0] .toLowerCase() .replace( /^ai /, 'AI ', - )}. Insert one`; + )}. Insert one`; } else { const ahrefs = Array.from(groupedConnections, ([connection, locales]) => { // If there are multiple locales, join them with ' or ' diff --git a/packages/@n8n/permissions/src/types.ts b/packages/@n8n/permissions/src/types.ts index 6dce8947d66df..1707d1c35e2db 100644 --- a/packages/@n8n/permissions/src/types.ts +++ b/packages/@n8n/permissions/src/types.ts @@ -1,6 +1,7 @@ export type DefaultOperations = 'create' | 'read' | 'update' | 'delete' | 'list'; export type Resource = | 'auditLogs' + | 'banner' | 'communityPackage' | 'credential' | 'externalSecretsProvider' @@ -27,6 +28,7 @@ export type ResourceScope< export type WildcardScope = `${Resource}:*` | '*'; export type AuditLogsScope = ResourceScope<'auditLogs', 'manage'>; +export type BannerScope = ResourceScope<'banner', 'dismiss'>; export type CommunityPackageScope = ResourceScope< 'communityPackage', 'install' | 'uninstall' | 'update' | 'list' | 'manage' @@ -56,6 +58,7 @@ export type WorkflowScope = ResourceScope<'workflow', DefaultOperations | 'share export type Scope = | AuditLogsScope + | BannerScope | CommunityPackageScope | CredentialScope | ExternalSecretProviderScope diff --git a/packages/cli/BREAKING-CHANGES.md b/packages/cli/BREAKING-CHANGES.md index 540749ed43599..afa1ed17e144d 100644 --- a/packages/cli/BREAKING-CHANGES.md +++ b/packages/cli/BREAKING-CHANGES.md @@ -2,6 +2,16 @@ This list shows all the versions which include breaking changes and how to upgrade. +## 1.32.0 + +### What changed? + +N8n auth cookie has `Secure` flag set by default now. + +### When is action necessary? + +If you are running n8n without HTTP**S** on a domain other than `localhost`, you need to either setup HTTPS, or you can disable the secure flag by setting the env variable `N8N_SECURE_COOKIE` to `false`. + ## 1.27.0 ### What changed? diff --git a/packages/cli/package.json b/packages/cli/package.json index 13319b3fa55ef..e338827891278 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -1,6 +1,6 @@ { "name": "n8n", - "version": "1.30.0", + "version": "1.31.0", "description": "n8n Workflow Automation Tool", "license": "SEE LICENSE IN LICENSE.md", "homepage": "https://n8n.io", @@ -67,15 +67,13 @@ "@types/connect-history-api-fallback": "^1.3.1", "@types/convict": "^6.1.1", "@types/cookie-parser": "^1.4.2", - "@types/express": "^4.17.6", + "@types/express": "^4.17.21", "@types/formidable": "^3.4.0", "@types/json-diff": "^1.0.0", "@types/jsonwebtoken": "^9.0.1", "@types/lodash": "^4.14.195", - "@types/passport-jwt": "^3.0.6", "@types/psl": "^1.1.0", "@types/replacestream": "^4.0.1", - "@types/send": "^0.17.1", "@types/shelljs": "^0.8.11", "@types/sshpk": "^1.17.1", "@types/superagent": "4.1.13", @@ -118,12 +116,12 @@ "csrf": "3.1.0", "curlconverter": "3.21.0", "dotenv": "8.6.0", - "express": "4.18.2", + "express": "4.18.3", "express-async-errors": "3.1.1", - "express-handlebars": "7.0.2", + "express-handlebars": "7.1.2", "express-openapi-validator": "4.13.8", "express-prom-bundle": "6.6.0", - "express-rate-limit": "7.1.3", + "express-rate-limit": "7.2.0", "fast-glob": "3.2.12", "flatted": "3.2.7", "formidable": "3.5.0", @@ -153,9 +151,6 @@ "otpauth": "9.1.1", "p-cancelable": "2.1.1", "p-lazy": "3.1.0", - "passport": "0.6.0", - "passport-cookie": "1.0.9", - "passport-jwt": "4.0.1", "pg": "8.11.3", "picocolors": "1.0.0", "pkce-challenge": "3.0.0", diff --git a/packages/cli/src/ExternalSecrets/ExternalSecrets.controller.ee.ts b/packages/cli/src/ExternalSecrets/ExternalSecrets.controller.ee.ts index 03967cf3ee26d..86a61b75a0a76 100644 --- a/packages/cli/src/ExternalSecrets/ExternalSecrets.controller.ee.ts +++ b/packages/cli/src/ExternalSecrets/ExternalSecrets.controller.ee.ts @@ -1,23 +1,22 @@ -import { Authorized, Get, Post, RestController, RequireGlobalScope } from '@/decorators'; +import { Get, Post, RestController, GlobalScope } from '@/decorators'; import { ExternalSecretsRequest } from '@/requests'; import { Response } from 'express'; import { ExternalSecretsService } from './ExternalSecrets.service.ee'; import { ExternalSecretsProviderNotFoundError } from '@/errors/external-secrets-provider-not-found.error'; import { NotFoundError } from '@/errors/response-errors/not-found.error'; -@Authorized() @RestController('/external-secrets') export class ExternalSecretsController { constructor(private readonly secretsService: ExternalSecretsService) {} @Get('/providers') - @RequireGlobalScope('externalSecretsProvider:list') + @GlobalScope('externalSecretsProvider:list') async getProviders() { return await this.secretsService.getProviders(); } @Get('/providers/:provider') - @RequireGlobalScope('externalSecretsProvider:read') + @GlobalScope('externalSecretsProvider:read') async getProvider(req: ExternalSecretsRequest.GetProvider) { const providerName = req.params.provider; try { @@ -31,7 +30,7 @@ export class ExternalSecretsController { } @Post('/providers/:provider/test') - @RequireGlobalScope('externalSecretsProvider:read') + @GlobalScope('externalSecretsProvider:read') async testProviderSettings(req: ExternalSecretsRequest.TestProviderSettings, res: Response) { const providerName = req.params.provider; try { @@ -51,7 +50,7 @@ export class ExternalSecretsController { } @Post('/providers/:provider') - @RequireGlobalScope('externalSecretsProvider:create') + @GlobalScope('externalSecretsProvider:create') async setProviderSettings(req: ExternalSecretsRequest.SetProviderSettings) { const providerName = req.params.provider; try { @@ -66,7 +65,7 @@ export class ExternalSecretsController { } @Post('/providers/:provider/connect') - @RequireGlobalScope('externalSecretsProvider:update') + @GlobalScope('externalSecretsProvider:update') async setProviderConnected(req: ExternalSecretsRequest.SetProviderConnected) { const providerName = req.params.provider; try { @@ -81,7 +80,7 @@ export class ExternalSecretsController { } @Post('/providers/:provider/update') - @RequireGlobalScope('externalSecretsProvider:sync') + @GlobalScope('externalSecretsProvider:sync') async updateProvider(req: ExternalSecretsRequest.UpdateProvider, res: Response) { const providerName = req.params.provider; try { @@ -101,7 +100,7 @@ export class ExternalSecretsController { } @Get('/secrets') - @RequireGlobalScope('externalSecret:list') + @GlobalScope('externalSecret:list') getSecretNames() { return this.secretsService.getAllSecrets(); } diff --git a/packages/cli/src/Interfaces.ts b/packages/cli/src/Interfaces.ts index 6bb0d1b933fcb..9d1867f1c8dc5 100644 --- a/packages/cli/src/Interfaces.ts +++ b/packages/cli/src/Interfaces.ts @@ -613,18 +613,6 @@ export interface ILicensePostResponse extends ILicenseReadResponse { managementToken: string; } -export interface JwtToken { - token: string; - /** The amount of seconds after which the JWT will expire. **/ - expiresIn: number; -} - -export interface JwtPayload { - id: string; - email: string | null; - password: string | null; -} - export interface PublicUser { id: string; email?: string; diff --git a/packages/cli/src/Ldap/ldap.controller.ts b/packages/cli/src/Ldap/ldap.controller.ts index a158d81d49c66..e7715630bfa48 100644 --- a/packages/cli/src/Ldap/ldap.controller.ts +++ b/packages/cli/src/Ldap/ldap.controller.ts @@ -1,5 +1,5 @@ import pick from 'lodash/pick'; -import { Authorized, Get, Post, Put, RestController, RequireGlobalScope } from '@/decorators'; +import { Get, Post, Put, RestController, GlobalScope } from '@/decorators'; import { InternalHooks } from '@/InternalHooks'; import { BadRequestError } from '@/errors/response-errors/bad-request.error'; @@ -8,7 +8,6 @@ import { getLdapSynchronizations } from './helpers'; import { LdapConfiguration } from './types'; import { LdapService } from './ldap.service'; -@Authorized() @RestController('/ldap') export class LdapController { constructor( @@ -17,13 +16,13 @@ export class LdapController { ) {} @Get('/config') - @RequireGlobalScope('ldap:manage') + @GlobalScope('ldap:manage') async getConfig() { return await this.ldapService.loadConfig(); } @Post('/test-connection') - @RequireGlobalScope('ldap:manage') + @GlobalScope('ldap:manage') async testConnection() { try { await this.ldapService.testConnection(); @@ -33,7 +32,7 @@ export class LdapController { } @Put('/config') - @RequireGlobalScope('ldap:manage') + @GlobalScope('ldap:manage') async updateConfig(req: LdapConfiguration.Update) { try { await this.ldapService.updateConfig(req.body); @@ -52,14 +51,14 @@ export class LdapController { } @Get('/sync') - @RequireGlobalScope('ldap:sync') + @GlobalScope('ldap:sync') async getLdapSync(req: LdapConfiguration.GetSync) { const { page = '0', perPage = '20' } = req.query; return await getLdapSynchronizations(parseInt(page, 10), parseInt(perPage, 10)); } @Post('/sync') - @RequireGlobalScope('ldap:sync') + @GlobalScope('ldap:sync') async syncLdap(req: LdapConfiguration.Sync) { try { await this.ldapService.runSync(req.body.type); diff --git a/packages/cli/src/Server.ts b/packages/cli/src/Server.ts index 357ae8ec44e39..ae285538b364d 100644 --- a/packages/cli/src/Server.ts +++ b/packages/cli/src/Server.ts @@ -62,7 +62,6 @@ import { EventBusController } from '@/eventbus/eventBus.controller'; import { EventBusControllerEE } from '@/eventbus/eventBus.controller.ee'; import { LicenseController } from '@/license/license.controller'; import { setupPushServer, setupPushHandler } from '@/push'; -import { setupAuthMiddlewares } from './middlewares'; import { isLdapEnabled } from './Ldap/helpers'; import { AbstractServer } from './AbstractServer'; import { PostHogClient } from './posthog'; @@ -80,7 +79,7 @@ import { ActiveWorkflowsController } from './controllers/activeWorkflows.control import { OrchestrationController } from './controllers/orchestration.controller'; import { WorkflowHistoryController } from './workflows/workflowHistory/workflowHistory.controller.ee'; import { InvitationController } from './controllers/invitation.controller'; -import { CollaborationService } from './collaboration/collaboration.service'; +// import { CollaborationService } from './collaboration/collaboration.service'; import { BadRequestError } from './errors/response-errors/bad-request.error'; import { OrchestrationService } from '@/services/orchestration.service'; @@ -126,12 +125,11 @@ export class Server extends AbstractServer { } void Container.get(InternalHooks).onServerStarted(); - Container.get(CollaborationService); + // Container.get(CollaborationService); } - private async registerControllers(ignoredEndpoints: Readonly) { + private async registerControllers() { const { app } = this; - setupAuthMiddlewares(app, ignoredEndpoints, this.restEndpoint); const controllers: Array> = [ EventBusController, @@ -278,7 +276,7 @@ export class Server extends AbstractServer { await handleMfaDisable(); - await this.registerControllers(ignoredEndpoints); + await this.registerControllers(); // ---------------------------------------- // SAML diff --git a/packages/cli/src/WebhookHelpers.ts b/packages/cli/src/WebhookHelpers.ts index c429a06aa2f65..bd0e44d85c465 100644 --- a/packages/cli/src/WebhookHelpers.ts +++ b/packages/cli/src/WebhookHelpers.ts @@ -34,6 +34,7 @@ import type { WorkflowExecuteMode, } from 'n8n-workflow'; import { + ApplicationError, BINARY_ENCODING, createDeferredPromise, ErrorReporterProxy as ErrorReporter, @@ -807,7 +808,13 @@ export async function executeWebhook( }) .catch((e) => { if (!didSendResponse) { - responseCallback(new Error('There was a problem executing the workflow'), {}); + responseCallback( + new ApplicationError('There was a problem executing the workflow', { + level: 'warning', + cause: e, + }), + {}, + ); } throw new InternalServerError(e.message); @@ -818,7 +825,10 @@ export async function executeWebhook( const error = e instanceof UnprocessableRequestError ? e - : new Error('There was a problem executing the workflow', { cause: e }); + : new ApplicationError('There was a problem executing the workflow', { + level: 'warning', + cause: e, + }); if (didSendResponse) throw error; responseCallback(error, {}); return; diff --git a/packages/cli/src/auth/auth.service.ts b/packages/cli/src/auth/auth.service.ts new file mode 100644 index 0000000000000..22fa6f3ffef4d --- /dev/null +++ b/packages/cli/src/auth/auth.service.ts @@ -0,0 +1,198 @@ +import { Service } from 'typedi'; +import type { NextFunction, Response } from 'express'; +import { createHash } from 'crypto'; +import { JsonWebTokenError, TokenExpiredError } from 'jsonwebtoken'; + +import config from '@/config'; +import { AUTH_COOKIE_NAME, RESPONSE_ERROR_MESSAGES, Time } from '@/constants'; +import type { User } from '@db/entities/User'; +import { UserRepository } from '@db/repositories/user.repository'; +import { AuthError } from '@/errors/response-errors/auth.error'; +import { UnauthorizedError } from '@/errors/response-errors/unauthorized.error'; +import { License } from '@/License'; +import { Logger } from '@/Logger'; +import type { AuthenticatedRequest } from '@/requests'; +import { JwtService } from '@/services/jwt.service'; +import { UrlService } from '@/services/url.service'; + +interface AuthJwtPayload { + /** User Id */ + id: string; + /** This hash is derived from email and bcrypt of password */ + hash: string; +} + +interface IssuedJWT extends AuthJwtPayload { + exp: number; +} + +interface PasswordResetToken { + sub: string; + hash: string; +} + +@Service() +export class AuthService { + constructor( + private readonly logger: Logger, + private readonly license: License, + private readonly jwtService: JwtService, + private readonly urlService: UrlService, + private readonly userRepository: UserRepository, + ) { + // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment + this.authMiddleware = this.authMiddleware.bind(this); + } + + async authMiddleware(req: AuthenticatedRequest, res: Response, next: NextFunction) { + const token = req.cookies[AUTH_COOKIE_NAME]; + if (token) { + try { + req.user = await this.resolveJwt(token, res); + } catch (error) { + if (error instanceof JsonWebTokenError || error instanceof AuthError) { + this.clearCookie(res); + } else { + throw error; + } + } + } + + if (req.user) next(); + else res.status(401).json({ status: 'error', message: 'Unauthorized' }); + } + + clearCookie(res: Response) { + res.clearCookie(AUTH_COOKIE_NAME); + } + + issueCookie(res: Response, user: User) { + // If the instance has exceeded its user quota, prevent non-owners from logging in + const isWithinUsersLimit = this.license.isWithinUsersLimit(); + if ( + config.getEnv('userManagement.isInstanceOwnerSetUp') && + !user.isOwner && + !isWithinUsersLimit + ) { + throw new UnauthorizedError(RESPONSE_ERROR_MESSAGES.USERS_QUOTA_REACHED); + } + + const token = this.issueJWT(user); + res.cookie(AUTH_COOKIE_NAME, token, { + maxAge: this.jwtExpiration * Time.seconds.toMilliseconds, + httpOnly: true, + sameSite: 'lax', + secure: config.getEnv('secure_cookie'), + }); + } + + issueJWT(user: User) { + const payload: AuthJwtPayload = { + id: user.id, + hash: this.createJWTHash(user), + }; + return this.jwtService.sign(payload, { + expiresIn: this.jwtExpiration, + }); + } + + async resolveJwt(token: string, res: Response): Promise { + const jwtPayload: IssuedJWT = this.jwtService.verify(token, { + algorithms: ['HS256'], + }); + + // TODO: Use an in-memory ttl-cache to cache the User object for upto a minute + const user = await this.userRepository.findOne({ + where: { id: jwtPayload.id }, + }); + + if ( + // If not user is found + !user || + // or, If the user has been deactivated (i.e. LDAP users) + user.disabled || + // or, If the email or password has been updated + jwtPayload.hash !== this.createJWTHash(user) + ) { + throw new AuthError('Unauthorized'); + } + + if (jwtPayload.exp * 1000 - Date.now() < this.jwtRefreshTimeout) { + this.logger.debug('JWT about to expire. Will be refreshed'); + this.issueCookie(res, user); + } + + return user; + } + + generatePasswordResetToken(user: User, expiresIn = '20m') { + const payload: PasswordResetToken = { sub: user.id, hash: this.createJWTHash(user) }; + return this.jwtService.sign(payload, { expiresIn }); + } + + generatePasswordResetUrl(user: User) { + const instanceBaseUrl = this.urlService.getInstanceBaseUrl(); + const url = new URL(`${instanceBaseUrl}/change-password`); + + url.searchParams.append('token', this.generatePasswordResetToken(user)); + url.searchParams.append('mfaEnabled', user.mfaEnabled.toString()); + + return url.toString(); + } + + async resolvePasswordResetToken(token: string): Promise { + let decodedToken: PasswordResetToken; + try { + decodedToken = this.jwtService.verify(token); + } catch (e) { + if (e instanceof TokenExpiredError) { + this.logger.debug('Reset password token expired', { token }); + } else { + this.logger.debug('Error verifying token', { token }); + } + return; + } + + const user = await this.userRepository.findOne({ + where: { id: decodedToken.sub }, + relations: ['authIdentities'], + }); + + if (!user) { + this.logger.debug( + 'Request to resolve password token failed because no user was found for the provided user ID', + { userId: decodedToken.sub, token }, + ); + return; + } + + if (decodedToken.hash !== this.createJWTHash(user)) { + this.logger.debug('Password updated since this token was generated'); + return; + } + + return user; + } + + createJWTHash({ email, password }: User) { + const hash = createHash('sha256') + .update(email + ':' + password) + .digest('base64'); + return hash.substring(0, 10); + } + + /** How many **milliseconds** before expiration should a JWT be renewed */ + get jwtRefreshTimeout() { + const { jwtRefreshTimeoutHours, jwtSessionDurationHours } = config.get('userManagement'); + if (jwtRefreshTimeoutHours === 0) { + return Math.floor(jwtSessionDurationHours * 0.25 * Time.hours.toMilliseconds); + } else { + return Math.floor(jwtRefreshTimeoutHours * Time.hours.toMilliseconds); + } + } + + /** How many **seconds** is an issued JWT valid for */ + get jwtExpiration() { + return config.get('userManagement.jwtSessionDurationHours') * Time.hours.toSeconds; + } +} diff --git a/packages/cli/src/auth/jwt.ts b/packages/cli/src/auth/jwt.ts index affc9ea75f8b8..1d63f97fa3c6d 100644 --- a/packages/cli/src/auth/jwt.ts +++ b/packages/cli/src/auth/jwt.ts @@ -1,94 +1,12 @@ -import type { Response } from 'express'; -import { createHash } from 'crypto'; -import { AUTH_COOKIE_NAME, RESPONSE_ERROR_MESSAGES, Time } from '@/constants'; -import type { JwtPayload, JwtToken } from '@/Interfaces'; -import type { User } from '@db/entities/User'; -import config from '@/config'; -import { License } from '@/License'; import { Container } from 'typedi'; -import { UserRepository } from '@db/repositories/user.repository'; -import { JwtService } from '@/services/jwt.service'; -import { UnauthorizedError } from '@/errors/response-errors/unauthorized.error'; -import { AuthError } from '@/errors/response-errors/auth.error'; -import { ApplicationError } from 'n8n-workflow'; - -export function issueJWT(user: User): JwtToken { - const { id, email, password } = user; - const expiresInHours = config.getEnv('userManagement.jwtSessionDurationHours'); - const expiresInSeconds = expiresInHours * Time.hours.toSeconds; - - const isWithinUsersLimit = Container.get(License).isWithinUsersLimit(); - - const payload: JwtPayload = { - id, - email, - password: password ?? null, - }; - - if ( - config.getEnv('userManagement.isInstanceOwnerSetUp') && - !user.isOwner && - !isWithinUsersLimit - ) { - throw new UnauthorizedError(RESPONSE_ERROR_MESSAGES.USERS_QUOTA_REACHED); - } - if (password) { - payload.password = createHash('sha256') - .update(password.slice(password.length / 2)) - .digest('hex'); - } - - const signedToken = Container.get(JwtService).sign(payload, { - expiresIn: expiresInSeconds, - }); - - return { - token: signedToken, - expiresIn: expiresInSeconds, - }; -} - -export const createPasswordSha = (user: User) => - createHash('sha256') - .update(user.password.slice(user.password.length / 2)) - .digest('hex'); - -export async function resolveJwtContent(jwtPayload: JwtPayload): Promise { - const user = await Container.get(UserRepository).findOne({ - where: { id: jwtPayload.id }, - }); - - let passwordHash = null; - if (user?.password) { - passwordHash = createPasswordSha(user); - } - - // currently only LDAP users during synchronization - // can be set to disabled - if (user?.disabled) { - throw new AuthError('Unauthorized'); - } - - if (!user || jwtPayload.password !== passwordHash || user.email !== jwtPayload.email) { - // When owner hasn't been set up, the default user - // won't have email nor password (both equals null) - throw new ApplicationError('Invalid token content'); - } - return user; -} +import type { Response } from 'express'; -export async function resolveJwt(token: string): Promise { - const jwtPayload: JwtPayload = Container.get(JwtService).verify(token, { - algorithms: ['HS256'], - }); - return await resolveJwtContent(jwtPayload); -} +import type { User } from '@db/entities/User'; +import { AuthService } from './auth.service'; -export async function issueCookie(res: Response, user: User): Promise { - const userData = issueJWT(user); - res.cookie(AUTH_COOKIE_NAME, userData.token, { - maxAge: userData.expiresIn * Time.seconds.toMilliseconds, - httpOnly: true, - sameSite: 'lax', - }); +// This method is still used by cloud hooks. +// DO NOT DELETE until the hooks have been updated +/** @deprecated Use `AuthService` instead */ +export function issueCookie(res: Response, user: User) { + return Container.get(AuthService).issueCookie(res, user); } diff --git a/packages/cli/src/config/index.ts b/packages/cli/src/config/index.ts index 755d1105828df..2ad950837a03b 100644 --- a/packages/cli/src/config/index.ts +++ b/packages/cli/src/config/index.ts @@ -15,6 +15,7 @@ if (inE2ETests) { process.env.N8N_LOG_LEVEL = 'silent'; process.env.N8N_PUBLIC_API_DISABLED = 'true'; process.env.SKIP_STATISTICS_EVENTS = 'true'; + process.env.N8N_SECURE_COOKIE = 'false'; } else { dotenv.config(); } diff --git a/packages/cli/src/config/schema.ts b/packages/cli/src/config/schema.ts index 0fde528a9877b..a2ea6c47c6f8f 100644 --- a/packages/cli/src/config/schema.ts +++ b/packages/cli/src/config/schema.ts @@ -538,6 +538,12 @@ export const schema = { env: 'N8N_PROTOCOL', doc: 'HTTP Protocol via which n8n can be reached', }, + secure_cookie: { + doc: 'This sets the `Secure` flag on n8n auth cookie', + format: Boolean, + default: true, + env: 'N8N_SECURE_COOKIE', + }, ssl_key: { format: String, default: '', diff --git a/packages/cli/src/constants.ts b/packages/cli/src/constants.ts index 9e65e4c342bfa..aecd7cd1efad9 100644 --- a/packages/cli/src/constants.ts +++ b/packages/cli/src/constants.ts @@ -131,6 +131,7 @@ export const Time = { }, days: { toSeconds: 24 * 60 * 60, + toMilliseconds: 24 * 60 * 60 * 1000, }, }; diff --git a/packages/cli/src/controllers/activeWorkflows.controller.ts b/packages/cli/src/controllers/activeWorkflows.controller.ts index b86c2c4bad28c..20e75208c86b6 100644 --- a/packages/cli/src/controllers/activeWorkflows.controller.ts +++ b/packages/cli/src/controllers/activeWorkflows.controller.ts @@ -1,8 +1,7 @@ -import { Authorized, Get, RestController } from '@/decorators'; +import { Get, RestController } from '@/decorators'; import { ActiveWorkflowRequest } from '@/requests'; import { ActiveWorkflowsService } from '@/services/activeWorkflows.service'; -@Authorized() @RestController('/active-workflows') export class ActiveWorkflowsController { constructor(private readonly activeWorkflowsService: ActiveWorkflowsService) {} diff --git a/packages/cli/src/controllers/auth.controller.ts b/packages/cli/src/controllers/auth.controller.ts index 7ba13a2677dc7..7dd4e0b6303ba 100644 --- a/packages/cli/src/controllers/auth.controller.ts +++ b/packages/cli/src/controllers/auth.controller.ts @@ -1,12 +1,12 @@ import validator from 'validator'; -import { Authorized, Get, Post, RestController } from '@/decorators'; -import { issueCookie, resolveJwt } from '@/auth/jwt'; -import { AUTH_COOKIE_NAME, RESPONSE_ERROR_MESSAGES } from '@/constants'; + +import { AuthService } from '@/auth/auth.service'; +import { Get, Post, RestController } from '@/decorators'; +import { RESPONSE_ERROR_MESSAGES } from '@/constants'; import { Request, Response } from 'express'; import type { User } from '@db/entities/User'; -import { LoginRequest, UserRequest } from '@/requests'; +import { AuthenticatedRequest, LoginRequest, UserRequest } from '@/requests'; import type { PublicUser } from '@/Interfaces'; -import config from '@/config'; import { handleEmailLogin, handleLdapLogin } from '@/auth'; import { PostHogClient } from '@/posthog'; import { @@ -20,7 +20,6 @@ import { UserService } from '@/services/user.service'; import { MfaService } from '@/Mfa/mfa.service'; import { Logger } from '@/Logger'; import { AuthError } from '@/errors/response-errors/auth.error'; -import { InternalServerError } from '@/errors/response-errors/internal-server.error'; import { BadRequestError } from '@/errors/response-errors/bad-request.error'; import { UnauthorizedError } from '@/errors/response-errors/unauthorized.error'; import { ApplicationError } from 'n8n-workflow'; @@ -31,6 +30,7 @@ export class AuthController { constructor( private readonly logger: Logger, private readonly internalHooks: InternalHooks, + private readonly authService: AuthService, private readonly mfaService: MfaService, private readonly userService: UserService, private readonly license: License, @@ -38,10 +38,8 @@ export class AuthController { private readonly postHog?: PostHogClient, ) {} - /** - * Log in a user. - */ - @Post('/login') + /** Log in a user */ + @Post('/login', { skipAuth: true }) async login(req: LoginRequest, res: Response): Promise { const { email, password, mfaToken, mfaRecoveryCode } = req.body; if (!email) throw new ApplicationError('Email is required to log in'); @@ -96,7 +94,7 @@ export class AuthController { } } - await issueCookie(res, user); + this.authService.issueCookie(res, user); void this.internalHooks.onUserLoginSuccess({ user, authenticationMethod: usedAuthenticationMethod, @@ -112,51 +110,17 @@ export class AuthController { throw new AuthError('Wrong username or password. Do you have caps lock on?'); } - /** - * Manually check the `n8n-auth` cookie. - */ + /** Check if the user is already logged in */ @Get('/login') - async currentUser(req: Request, res: Response): Promise { - // Manually check the existing cookie. - // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - const cookieContents = req.cookies?.[AUTH_COOKIE_NAME] as string | undefined; - - let user: User; - if (cookieContents) { - // If logged in, return user - try { - user = await resolveJwt(cookieContents); - - return await this.userService.toPublic(user, { posthog: this.postHog, withScopes: true }); - } catch (error) { - res.clearCookie(AUTH_COOKIE_NAME); - } - } - - if (config.getEnv('userManagement.isInstanceOwnerSetUp')) { - throw new AuthError('Not logged in'); - } - - try { - user = await this.userRepository.findOneOrFail({ where: {} }); - } catch (error) { - throw new InternalServerError( - 'No users found in database - did you wipe the users table? Create at least one user.', - ); - } - - if (user.email || user.password) { - throw new InternalServerError('Invalid database state - user has password set.'); - } - - await issueCookie(res, user); - return await this.userService.toPublic(user, { posthog: this.postHog, withScopes: true }); + async currentUser(req: AuthenticatedRequest): Promise { + return await this.userService.toPublic(req.user, { + posthog: this.postHog, + withScopes: true, + }); } - /** - * Validate invite token to enable invitee to set up their account. - */ - @Get('/resolve-signup-token') + /** Validate invite token to enable invitee to set up their account */ + @Get('/resolve-signup-token', { skipAuth: true }) async resolveSignupToken(req: UserRequest.ResolveSignUp) { const { inviterId, inviteeId } = req.query; const isWithinUsersLimit = this.license.isWithinUsersLimit(); @@ -223,13 +187,10 @@ export class AuthController { return { inviter: { firstName, lastName } }; } - /** - * Log out a user. - */ - @Authorized() + /** Log out a user */ @Post('/logout') - logout(req: Request, res: Response) { - res.clearCookie(AUTH_COOKIE_NAME); + logout(_: Request, res: Response) { + this.authService.clearCookie(res); return { loggedOut: true }; } diff --git a/packages/cli/src/controllers/communityPackages.controller.ts b/packages/cli/src/controllers/communityPackages.controller.ts index 0e260c863b52f..681acd9d234cf 100644 --- a/packages/cli/src/controllers/communityPackages.controller.ts +++ b/packages/cli/src/controllers/communityPackages.controller.ts @@ -5,16 +5,7 @@ import { STARTER_TEMPLATE_NAME, UNKNOWN_FAILURE_REASON, } from '@/constants'; -import { - Authorized, - Delete, - Get, - Middleware, - Patch, - Post, - RestController, - RequireGlobalScope, -} from '@/decorators'; +import { Delete, Get, Middleware, Patch, Post, RestController, GlobalScope } from '@/decorators'; import { NodeRequest } from '@/requests'; import type { InstalledPackages } from '@db/entities/InstalledPackages'; import type { CommunityPackages } from '@/Interfaces'; @@ -41,7 +32,6 @@ export function isNpmError(error: unknown): error is { code: number; stdout: str return typeof error === 'object' && error !== null && 'code' in error && 'stdout' in error; } -@Authorized() @RestController('/community-packages') export class CommunityPackagesController { constructor( @@ -62,7 +52,7 @@ export class CommunityPackagesController { } @Post('/') - @RequireGlobalScope('communityPackage:install') + @GlobalScope('communityPackage:install') async installPackage(req: NodeRequest.Post) { const { name } = req.body; @@ -159,7 +149,7 @@ export class CommunityPackagesController { } @Get('/') - @RequireGlobalScope('communityPackage:list') + @GlobalScope('communityPackage:list') async getInstalledPackages() { const installedPackages = await this.communityPackagesService.getAllInstalledPackages(); @@ -194,7 +184,7 @@ export class CommunityPackagesController { } @Delete('/') - @RequireGlobalScope('communityPackage:uninstall') + @GlobalScope('communityPackage:uninstall') async uninstallPackage(req: NodeRequest.Delete) { const { name } = req.query; @@ -246,7 +236,7 @@ export class CommunityPackagesController { } @Patch('/') - @RequireGlobalScope('communityPackage:update') + @GlobalScope('communityPackage:update') async updatePackage(req: NodeRequest.Update) { const { name } = req.body; diff --git a/packages/cli/src/controllers/cta.controller.ts b/packages/cli/src/controllers/cta.controller.ts index 9d06ff081223c..5cd41a1dcd810 100644 --- a/packages/cli/src/controllers/cta.controller.ts +++ b/packages/cli/src/controllers/cta.controller.ts @@ -1,5 +1,5 @@ import express from 'express'; -import { Authorized, Get, RestController } from '@/decorators'; +import { Get, RestController } from '@/decorators'; import { AuthenticatedRequest } from '@/requests'; import { CtaService } from '@/services/cta.service'; @@ -7,7 +7,6 @@ import { CtaService } from '@/services/cta.service'; * Controller for Call to Action (CTA) endpoints. CTAs are certain * messages that are shown to users in the UI. */ -@Authorized() @RestController('/cta') export class CtaController { constructor(private readonly ctaService: CtaService) {} diff --git a/packages/cli/src/controllers/debug.controller.ts b/packages/cli/src/controllers/debug.controller.ts index b74ccd584019b..d689be18f8d37 100644 --- a/packages/cli/src/controllers/debug.controller.ts +++ b/packages/cli/src/controllers/debug.controller.ts @@ -11,7 +11,7 @@ export class DebugController { private readonly workflowRepository: WorkflowRepository, ) {} - @Get('/multi-main-setup') + @Get('/multi-main-setup', { skipAuth: true }) async getMultiMainSetupDetails() { const leaderKey = await this.orchestrationService.multiMainSetup.fetchLeaderKey(); diff --git a/packages/cli/src/controllers/dynamicNodeParameters.controller.ts b/packages/cli/src/controllers/dynamicNodeParameters.controller.ts index 8a3fba26bbdaf..0c70862956bf6 100644 --- a/packages/cli/src/controllers/dynamicNodeParameters.controller.ts +++ b/packages/cli/src/controllers/dynamicNodeParameters.controller.ts @@ -7,7 +7,7 @@ import type { } from 'n8n-workflow'; import { jsonParse } from 'n8n-workflow'; -import { Authorized, Get, Middleware, RestController } from '@/decorators'; +import { Get, Middleware, RestController } from '@/decorators'; import { getBase } from '@/WorkflowExecuteAdditionalData'; import { DynamicNodeParametersService } from '@/services/dynamicNodeParameters.service'; import { DynamicNodeParametersRequest } from '@/requests'; @@ -21,17 +21,12 @@ const assertMethodName: RequestHandler = (req, res, next) => { next(); }; -@Authorized() @RestController('/dynamic-node-parameters') export class DynamicNodeParametersController { constructor(private readonly service: DynamicNodeParametersService) {} @Middleware() - parseQueryParams( - req: DynamicNodeParametersRequest.BaseRequest, - res: Response, - next: NextFunction, - ) { + parseQueryParams(req: DynamicNodeParametersRequest.BaseRequest, _: Response, next: NextFunction) { const { credentials, currentNodeParameters, nodeTypeAndVersion } = req.query; if (!nodeTypeAndVersion) { throw new BadRequestError('Parameter nodeTypeAndVersion is required.'); diff --git a/packages/cli/src/controllers/e2e.controller.ts b/packages/cli/src/controllers/e2e.controller.ts index 1435c9064fe57..0656e6f1813f9 100644 --- a/packages/cli/src/controllers/e2e.controller.ts +++ b/packages/cli/src/controllers/e2e.controller.ts @@ -7,7 +7,7 @@ import { ActiveWorkflowRunner } from '@/ActiveWorkflowRunner'; import { MessageEventBus } from '@/eventbus/MessageEventBus/MessageEventBus'; import { License } from '@/License'; import { LICENSE_FEATURES, inE2ETests } from '@/constants'; -import { NoAuthRequired, Patch, Post, RestController } from '@/decorators'; +import { Patch, Post, RestController } from '@/decorators'; import type { UserSetupPayload } from '@/requests'; import type { BooleanLicenseFeature, IPushDataType } from '@/Interfaces'; import { MfaService } from '@/Mfa/mfa.service'; @@ -60,7 +60,6 @@ type PushRequest = Request< } >; -@NoAuthRequired() @RestController('/e2e') export class E2EController { private enabledFeatures: Record = { @@ -97,7 +96,7 @@ export class E2EController { this.enabledFeatures[feature] ?? false; } - @Post('/reset') + @Post('/reset', { skipAuth: true }) async reset(req: ResetRequest) { this.resetFeatures(); await this.resetLogStreaming(); @@ -107,18 +106,18 @@ export class E2EController { await this.setupUserManagement(req.body.owner, req.body.members, req.body.admin); } - @Post('/push') + @Post('/push', { skipAuth: true }) async pushSend(req: PushRequest) { this.push.broadcast(req.body.type, req.body.data); } - @Patch('/feature') + @Patch('/feature', { skipAuth: true }) setFeature(req: Request<{}, {}, { feature: BooleanLicenseFeature; enabled: boolean }>) { const { enabled, feature } = req.body; this.enabledFeatures[feature] = enabled; } - @Patch('/queue-mode') + @Patch('/queue-mode', { skipAuth: true }) async setQueueMode(req: Request<{}, {}, { enabled: boolean }>) { const { enabled } = req.body; config.set('executions.mode', enabled ? 'queue' : 'regular'); diff --git a/packages/cli/src/controllers/invitation.controller.ts b/packages/cli/src/controllers/invitation.controller.ts index 022ead6ed115a..39a13792cd501 100644 --- a/packages/cli/src/controllers/invitation.controller.ts +++ b/packages/cli/src/controllers/invitation.controller.ts @@ -1,9 +1,9 @@ import { Response } from 'express'; import validator from 'validator'; +import { AuthService } from '@/auth/auth.service'; import config from '@/config'; -import { Authorized, NoAuthRequired, Post, RequireGlobalScope, RestController } from '@/decorators'; -import { issueCookie } from '@/auth/jwt'; +import { Post, GlobalScope, RestController } from '@/decorators'; import { RESPONSE_ERROR_MESSAGES } from '@/constants'; import { UserRequest } from '@/requests'; import { License } from '@/License'; @@ -19,13 +19,13 @@ import { UnauthorizedError } from '@/errors/response-errors/unauthorized.error'; import { InternalHooks } from '@/InternalHooks'; import { ExternalHooks } from '@/ExternalHooks'; -@Authorized() @RestController('/invitations') export class InvitationController { constructor( private readonly logger: Logger, private readonly internalHooks: InternalHooks, private readonly externalHooks: ExternalHooks, + private readonly authService: AuthService, private readonly userService: UserService, private readonly license: License, private readonly passwordUtility: PasswordUtility, @@ -38,7 +38,7 @@ export class InvitationController { */ @Post('/') - @RequireGlobalScope('user:create') + @GlobalScope('user:create') async inviteUser(req: UserRequest.Invite) { const isWithinUsersLimit = this.license.isWithinUsersLimit(); @@ -119,8 +119,7 @@ export class InvitationController { /** * Fill out user shell with first name, last name, and password. */ - @NoAuthRequired() - @Post('/:id/accept') + @Post('/:id/accept', { skipAuth: true }) async acceptInvitation(req: UserRequest.Update, res: Response) { const { id: inviteeId } = req.params; @@ -165,7 +164,7 @@ export class InvitationController { const updatedUser = await this.userRepository.save(invitee, { transaction: false }); - await issueCookie(res, updatedUser); + this.authService.issueCookie(res, updatedUser); void this.internalHooks.onUserSignup(updatedUser, { user_type: 'email', diff --git a/packages/cli/src/controllers/me.controller.ts b/packages/cli/src/controllers/me.controller.ts index 1d09cb8ce6d36..ab09935225fb7 100644 --- a/packages/cli/src/controllers/me.controller.ts +++ b/packages/cli/src/controllers/me.controller.ts @@ -2,10 +2,11 @@ import validator from 'validator'; import { plainToInstance } from 'class-transformer'; import { Response } from 'express'; import { randomBytes } from 'crypto'; -import { Authorized, Delete, Get, Patch, Post, RestController } from '@/decorators'; + +import { AuthService } from '@/auth/auth.service'; +import { Delete, Get, Patch, Post, RestController } from '@/decorators'; import { PasswordUtility } from '@/services/password.utility'; import { validateEntity } from '@/GenericHelpers'; -import { issueCookie } from '@/auth/jwt'; import type { User } from '@db/entities/User'; import { AuthenticatedRequest, @@ -14,7 +15,7 @@ import { UserUpdatePayload, } from '@/requests'; import type { PublicUser } from '@/Interfaces'; -import { isSamlLicensedAndEnabled } from '../sso/saml/samlHelpers'; +import { isSamlLicensedAndEnabled } from '@/sso/saml/samlHelpers'; import { UserService } from '@/services/user.service'; import { Logger } from '@/Logger'; import { ExternalHooks } from '@/ExternalHooks'; @@ -22,13 +23,13 @@ import { InternalHooks } from '@/InternalHooks'; import { BadRequestError } from '@/errors/response-errors/bad-request.error'; import { UserRepository } from '@/databases/repositories/user.repository'; -@Authorized() @RestController('/me') export class MeController { constructor( private readonly logger: Logger, private readonly externalHooks: ExternalHooks, private readonly internalHooks: InternalHooks, + private readonly authService: AuthService, private readonly userService: UserService, private readonly passwordUtility: PasswordUtility, private readonly userRepository: UserRepository, @@ -84,7 +85,7 @@ export class MeController { this.logger.info('User updated successfully', { userId }); - await issueCookie(res, user); + this.authService.issueCookie(res, user); const updatedKeys = Object.keys(payload); void this.internalHooks.onUserUpdate({ @@ -137,7 +138,7 @@ export class MeController { const updatedUser = await this.userRepository.save(user, { transaction: false }); this.logger.info('Password updated successfully', { userId: user.id }); - await issueCookie(res, updatedUser); + this.authService.issueCookie(res, updatedUser); void this.internalHooks.onUserUpdate({ user: updatedUser, diff --git a/packages/cli/src/controllers/mfa.controller.ts b/packages/cli/src/controllers/mfa.controller.ts index 9d4f370cd0a55..3c58b944d57d0 100644 --- a/packages/cli/src/controllers/mfa.controller.ts +++ b/packages/cli/src/controllers/mfa.controller.ts @@ -1,9 +1,8 @@ -import { Authorized, Delete, Get, Post, RestController } from '@/decorators'; +import { Delete, Get, Post, RestController } from '@/decorators'; import { AuthenticatedRequest, MFA } from '@/requests'; import { MfaService } from '@/Mfa/mfa.service'; import { BadRequestError } from '@/errors/response-errors/bad-request.error'; -@Authorized() @RestController('/mfa') export class MFAController { constructor(private mfaService: MfaService) {} diff --git a/packages/cli/src/controllers/nodeTypes.controller.ts b/packages/cli/src/controllers/nodeTypes.controller.ts index ad0b20efbcfbd..fddefb7e10d12 100644 --- a/packages/cli/src/controllers/nodeTypes.controller.ts +++ b/packages/cli/src/controllers/nodeTypes.controller.ts @@ -2,11 +2,10 @@ import { readFile } from 'fs/promises'; import get from 'lodash/get'; import { Request } from 'express'; import type { INodeTypeDescription, INodeTypeNameVersion } from 'n8n-workflow'; -import { Authorized, Post, RestController } from '@/decorators'; +import { Post, RestController } from '@/decorators'; import config from '@/config'; import { NodeTypes } from '@/NodeTypes'; -@Authorized() @RestController('/node-types') export class NodeTypesController { constructor(private readonly nodeTypes: NodeTypes) {} diff --git a/packages/cli/src/controllers/oauth/oAuth1Credential.controller.ts b/packages/cli/src/controllers/oauth/oAuth1Credential.controller.ts index 761e6b15b3230..0c3f0fb2046e4 100644 --- a/packages/cli/src/controllers/oauth/oAuth1Credential.controller.ts +++ b/packages/cli/src/controllers/oauth/oAuth1Credential.controller.ts @@ -5,7 +5,7 @@ import type { RequestOptions } from 'oauth-1.0a'; import clientOAuth1 from 'oauth-1.0a'; import { createHmac } from 'crypto'; import { RESPONSE_ERROR_MESSAGES } from '@/constants'; -import { Authorized, Get, RestController } from '@/decorators'; +import { Get, RestController } from '@/decorators'; import { OAuthRequest } from '@/requests'; import { sendErrorResponse } from '@/ResponseHelper'; import { AbstractOAuthController } from './abstractOAuth.controller'; @@ -29,7 +29,6 @@ const algorithmMap = { /* eslint-enable */ } as const; -@Authorized() @RestController('/oauth1-credential') export class OAuth1CredentialController extends AbstractOAuthController { override oauthVersion = 1; diff --git a/packages/cli/src/controllers/oauth/oAuth2Credential.controller.ts b/packages/cli/src/controllers/oauth/oAuth2Credential.controller.ts index a24f105f2b400..66aec58e99604 100644 --- a/packages/cli/src/controllers/oauth/oAuth2Credential.controller.ts +++ b/packages/cli/src/controllers/oauth/oAuth2Credential.controller.ts @@ -8,7 +8,7 @@ import omit from 'lodash/omit'; import set from 'lodash/set'; import split from 'lodash/split'; import { ApplicationError, jsonParse, jsonStringify } from 'n8n-workflow'; -import { Authorized, Get, RestController } from '@/decorators'; +import { Get, RestController } from '@/decorators'; import { OAuthRequest } from '@/requests'; import { AbstractOAuthController } from './abstractOAuth.controller'; @@ -17,7 +17,6 @@ interface CsrfStateParam { token: string; } -@Authorized() @RestController('/oauth2-credential') export class OAuth2CredentialController extends AbstractOAuthController { override oauthVersion = 2; diff --git a/packages/cli/src/controllers/orchestration.controller.ts b/packages/cli/src/controllers/orchestration.controller.ts index fb044014bebe1..74a9665e1e2d3 100644 --- a/packages/cli/src/controllers/orchestration.controller.ts +++ b/packages/cli/src/controllers/orchestration.controller.ts @@ -1,9 +1,8 @@ -import { Authorized, Post, RestController, RequireGlobalScope } from '@/decorators'; +import { Post, RestController, GlobalScope } from '@/decorators'; import { OrchestrationRequest } from '@/requests'; import { OrchestrationService } from '@/services/orchestration.service'; import { License } from '@/License'; -@Authorized() @RestController('/orchestration') export class OrchestrationController { constructor( @@ -12,10 +11,10 @@ export class OrchestrationController { ) {} /** - * These endpoints do not return anything, they just trigger the messsage to + * These endpoints do not return anything, they just trigger the message to * the workers to respond on Redis with their status. */ - @RequireGlobalScope('orchestration:read') + @GlobalScope('orchestration:read') @Post('/worker/status/:id') async getWorkersStatus(req: OrchestrationRequest.Get) { if (!this.licenseService.isWorkerViewLicensed()) return; @@ -23,14 +22,14 @@ export class OrchestrationController { return await this.orchestrationService.getWorkerStatus(id); } - @RequireGlobalScope('orchestration:read') + @GlobalScope('orchestration:read') @Post('/worker/status') async getWorkersStatusAll() { if (!this.licenseService.isWorkerViewLicensed()) return; return await this.orchestrationService.getWorkerStatus(); } - @RequireGlobalScope('orchestration:list') + @GlobalScope('orchestration:list') @Post('/worker/ids') async getWorkerIdsAll() { if (!this.licenseService.isWorkerViewLicensed()) return; diff --git a/packages/cli/src/controllers/owner.controller.ts b/packages/cli/src/controllers/owner.controller.ts index 15d0bc2c5cc49..d0b910f78012a 100644 --- a/packages/cli/src/controllers/owner.controller.ts +++ b/packages/cli/src/controllers/owner.controller.ts @@ -1,27 +1,27 @@ import validator from 'validator'; import { Response } from 'express'; +import { AuthService } from '@/auth/auth.service'; import config from '@/config'; import { validateEntity } from '@/GenericHelpers'; -import { Authorized, Post, RestController } from '@/decorators'; +import { GlobalScope, Post, RestController } from '@/decorators'; import { PasswordUtility } from '@/services/password.utility'; -import { issueCookie } from '@/auth/jwt'; import { OwnerRequest } from '@/requests'; import { SettingsRepository } from '@db/repositories/settings.repository'; +import { UserRepository } from '@db/repositories/user.repository'; import { PostHogClient } from '@/posthog'; import { UserService } from '@/services/user.service'; import { Logger } from '@/Logger'; import { BadRequestError } from '@/errors/response-errors/bad-request.error'; import { InternalHooks } from '@/InternalHooks'; -import { UserRepository } from '@/databases/repositories/user.repository'; -@Authorized('global:owner') @RestController('/owner') export class OwnerController { constructor( private readonly logger: Logger, private readonly internalHooks: InternalHooks, private readonly settingsRepository: SettingsRepository, + private readonly authService: AuthService, private readonly userService: UserService, private readonly passwordUtility: PasswordUtility, private readonly postHog: PostHogClient, @@ -32,24 +32,19 @@ export class OwnerController { * Promote a shell into the owner of the n8n instance, * and enable `isInstanceOwnerSetUp` setting. */ - @Post('/setup') + @Post('/setup', { skipAuth: true }) async setupOwner(req: OwnerRequest.Post, res: Response) { const { email, firstName, lastName, password } = req.body; - const { id: userId } = req.user; if (config.getEnv('userManagement.isInstanceOwnerSetUp')) { this.logger.debug( 'Request to claim instance ownership failed because instance owner already exists', - { - userId, - }, ); throw new BadRequestError('Instance owner already setup'); } if (!email || !validator.isEmail(email)) { this.logger.debug('Request to claim instance ownership failed because of invalid email', { - userId, invalidEmail: email, }); throw new BadRequestError('Invalid email address'); @@ -60,25 +55,24 @@ export class OwnerController { if (!firstName || !lastName) { this.logger.debug( 'Request to claim instance ownership failed because of missing first name or last name in payload', - { userId, payload: req.body }, + { payload: req.body }, ); throw new BadRequestError('First and last names are mandatory'); } - let owner = req.user; - - Object.assign(owner, { - email, - firstName, - lastName, - password: await this.passwordUtility.hash(validPassword), + let owner = await this.userRepository.findOneOrFail({ + where: { role: 'global:owner' }, }); + owner.email = email; + owner.firstName = firstName; + owner.lastName = lastName; + owner.password = await this.passwordUtility.hash(validPassword); await validateEntity(owner); owner = await this.userRepository.save(owner, { transaction: false }); - this.logger.info('Owner was set up successfully', { userId }); + this.logger.info('Owner was set up successfully'); await this.settingsRepository.update( { key: 'userManagement.isInstanceOwnerSetUp' }, @@ -87,19 +81,19 @@ export class OwnerController { config.set('userManagement.isInstanceOwnerSetUp', true); - this.logger.debug('Setting isInstanceOwnerSetUp updated successfully', { userId }); + this.logger.debug('Setting isInstanceOwnerSetUp updated successfully'); - await issueCookie(res, owner); + this.authService.issueCookie(res, owner); - void this.internalHooks.onInstanceOwnerSetup({ user_id: userId }); + void this.internalHooks.onInstanceOwnerSetup({ user_id: owner.id }); return await this.userService.toPublic(owner, { posthog: this.postHog, withScopes: true }); } @Post('/dismiss-banner') + @GlobalScope('banner:dismiss') async dismissBanner(req: OwnerRequest.DismissBanner) { const bannerName = 'banner' in req.body ? (req.body.banner as string) : ''; - const response = await this.settingsRepository.dismissBanner({ bannerName }); - return response; + return await this.settingsRepository.dismissBanner({ bannerName }); } } diff --git a/packages/cli/src/controllers/passwordReset.controller.ts b/packages/cli/src/controllers/passwordReset.controller.ts index 806d892d26375..041d667dac640 100644 --- a/packages/cli/src/controllers/passwordReset.controller.ts +++ b/packages/cli/src/controllers/passwordReset.controller.ts @@ -2,11 +2,11 @@ import { Response } from 'express'; import { rateLimit } from 'express-rate-limit'; import validator from 'validator'; +import { AuthService } from '@/auth/auth.service'; import { Get, Post, RestController } from '@/decorators'; import { PasswordUtility } from '@/services/password.utility'; import { UserManagementMailer } from '@/UserManagement/email'; import { PasswordResetRequest } from '@/requests'; -import { issueCookie } from '@/auth/jwt'; import { isSamlCurrentAuthenticationMethod } from '@/sso/ssoHelpers'; import { UserService } from '@/services/user.service'; import { License } from '@/License'; @@ -36,6 +36,7 @@ export class PasswordResetController { private readonly externalHooks: ExternalHooks, private readonly internalHooks: InternalHooks, private readonly mailer: UserManagementMailer, + private readonly authService: AuthService, private readonly userService: UserService, private readonly mfaService: MfaService, private readonly urlService: UrlService, @@ -49,6 +50,7 @@ export class PasswordResetController { */ @Post('/forgot-password', { middlewares: !inTest ? [throttle] : [], + skipAuth: true, }) async forgotPassword(req: PasswordResetRequest.Email) { if (!this.mailer.isEmailSetUp) { @@ -114,7 +116,7 @@ export class PasswordResetController { throw new UnprocessableRequestError('forgotPassword.ldapUserPasswordResetUnavailable'); } - const url = this.userService.generatePasswordResetUrl(user); + const url = this.authService.generatePasswordResetUrl(user); const { id, firstName, lastName } = user; try { @@ -149,7 +151,7 @@ export class PasswordResetController { /** * Verify password reset token and user ID. */ - @Get('/resolve-password-token') + @Get('/resolve-password-token', { skipAuth: true }) async resolvePasswordToken(req: PasswordResetRequest.Credentials) { const { token } = req.query; @@ -163,7 +165,7 @@ export class PasswordResetController { throw new BadRequestError(''); } - const user = await this.userService.resolvePasswordResetToken(token); + const user = await this.authService.resolvePasswordResetToken(token); if (!user) throw new NotFoundError(''); if (!user?.isOwner && !this.license.isWithinUsersLimit()) { @@ -181,7 +183,7 @@ export class PasswordResetController { /** * Verify password reset token and update password. */ - @Post('/change-password') + @Post('/change-password', { skipAuth: true }) async changePassword(req: PasswordResetRequest.NewPassword, res: Response) { const { token, password, mfaToken } = req.body; @@ -197,7 +199,7 @@ export class PasswordResetController { const validPassword = this.passwordUtility.validate(password); - const user = await this.userService.resolvePasswordResetToken(token); + const user = await this.authService.resolvePasswordResetToken(token); if (!user) throw new NotFoundError(''); if (user.mfaEnabled) { @@ -216,7 +218,7 @@ export class PasswordResetController { this.logger.info('User password updated successfully', { userId: user.id }); - await issueCookie(res, user); + this.authService.issueCookie(res, user); void this.internalHooks.onUserUpdate({ user, diff --git a/packages/cli/src/controllers/tags.controller.ts b/packages/cli/src/controllers/tags.controller.ts index a50cb25dcd094..f097002eace2b 100644 --- a/packages/cli/src/controllers/tags.controller.ts +++ b/packages/cli/src/controllers/tags.controller.ts @@ -1,20 +1,10 @@ import { Request, Response, NextFunction } from 'express'; import config from '@/config'; -import { - Authorized, - Delete, - Get, - Middleware, - Patch, - Post, - RestController, - RequireGlobalScope, -} from '@/decorators'; +import { Delete, Get, Middleware, Patch, Post, RestController, GlobalScope } from '@/decorators'; import { TagService } from '@/services/tag.service'; import { TagsRequest } from '@/requests'; import { BadRequestError } from '@/errors/response-errors/bad-request.error'; -@Authorized() @RestController('/tags') export class TagsController { private config = config; @@ -30,13 +20,13 @@ export class TagsController { } @Get('/') - @RequireGlobalScope('tag:list') + @GlobalScope('tag:list') async getAll(req: TagsRequest.GetAll) { return await this.tagService.getAll({ withUsageCount: req.query.withUsageCount === 'true' }); } @Post('/') - @RequireGlobalScope('tag:create') + @GlobalScope('tag:create') async createTag(req: TagsRequest.Create) { const tag = this.tagService.toEntity({ name: req.body.name }); @@ -44,7 +34,7 @@ export class TagsController { } @Patch('/:id(\\w+)') - @RequireGlobalScope('tag:update') + @GlobalScope('tag:update') async updateTag(req: TagsRequest.Update) { const newTag = this.tagService.toEntity({ id: req.params.id, name: req.body.name.trim() }); @@ -52,7 +42,7 @@ export class TagsController { } @Delete('/:id(\\w+)') - @RequireGlobalScope('tag:delete') + @GlobalScope('tag:delete') async deleteTag(req: TagsRequest.Delete) { const { id } = req.params; diff --git a/packages/cli/src/controllers/translation.controller.ts b/packages/cli/src/controllers/translation.controller.ts index 7b5f6bef57a1e..f359ec2b3a540 100644 --- a/packages/cli/src/controllers/translation.controller.ts +++ b/packages/cli/src/controllers/translation.controller.ts @@ -1,7 +1,7 @@ import type { Request } from 'express'; import { join } from 'path'; import { access } from 'fs/promises'; -import { Authorized, Get, RestController } from '@/decorators'; +import { Get, RestController } from '@/decorators'; import config from '@/config'; import { NODES_BASE_DIR } from '@/constants'; import { BadRequestError } from '@/errors/response-errors/bad-request.error'; @@ -15,7 +15,6 @@ export declare namespace TranslationRequest { export type Credential = Request<{}, {}, {}, { credentialType: string }>; } -@Authorized() @RestController('/') export class TranslationController { constructor(private readonly credentialTypes: CredentialTypes) {} diff --git a/packages/cli/src/controllers/users.controller.ts b/packages/cli/src/controllers/users.controller.ts index 2ed94e1345fea..baa441fe36a4b 100644 --- a/packages/cli/src/controllers/users.controller.ts +++ b/packages/cli/src/controllers/users.controller.ts @@ -1,15 +1,10 @@ +import { plainToInstance } from 'class-transformer'; + +import { AuthService } from '@/auth/auth.service'; import { User } from '@db/entities/User'; import { SharedCredentials } from '@db/entities/SharedCredentials'; import { SharedWorkflow } from '@db/entities/SharedWorkflow'; -import { - RequireGlobalScope, - Authorized, - Delete, - Get, - RestController, - Patch, - Licensed, -} from '@/decorators'; +import { GlobalScope, Delete, Get, RestController, Patch, Licensed } from '@/decorators'; import { ListQuery, UserRequest, @@ -22,7 +17,6 @@ import { AuthIdentity } from '@db/entities/AuthIdentity'; import { SharedCredentialsRepository } from '@db/repositories/sharedCredentials.repository'; import { SharedWorkflowRepository } from '@db/repositories/sharedWorkflow.repository'; import { UserRepository } from '@db/repositories/user.repository'; -import { plainToInstance } from 'class-transformer'; import { UserService } from '@/services/user.service'; import { listQueryMiddleware } from '@/middlewares'; import { Logger } from '@/Logger'; @@ -33,7 +27,6 @@ import { ExternalHooks } from '@/ExternalHooks'; import { InternalHooks } from '@/InternalHooks'; import { validateEntity } from '@/GenericHelpers'; -@Authorized() @RestController('/users') export class UsersController { constructor( @@ -44,6 +37,7 @@ export class UsersController { private readonly sharedWorkflowRepository: SharedWorkflowRepository, private readonly userRepository: UserRepository, private readonly activeWorkflowRunner: ActiveWorkflowRunner, + private readonly authService: AuthService, private readonly userService: UserService, ) {} @@ -86,7 +80,7 @@ export class UsersController { } @Get('/', { middlewares: listQueryMiddleware }) - @RequireGlobalScope('user:list') + @GlobalScope('user:list') async listUsers(req: ListQuery.Request) { const { listQueryOptions } = req; @@ -107,7 +101,7 @@ export class UsersController { } @Get('/:id/password-reset-link') - @RequireGlobalScope('user:resetPassword') + @GlobalScope('user:resetPassword') async getUserPasswordResetLink(req: UserRequest.PasswordResetLink) { const user = await this.userRepository.findOneOrFail({ where: { id: req.params.id }, @@ -116,12 +110,12 @@ export class UsersController { throw new NotFoundError('User not found'); } - const link = this.userService.generatePasswordResetUrl(user); + const link = this.authService.generatePasswordResetUrl(user); return { link }; } @Patch('/:id/settings') - @RequireGlobalScope('user:update') + @GlobalScope('user:update') async updateUserSettings(req: UserRequest.UserSettingsUpdate) { const payload = plainToInstance(UserSettingsUpdatePayload, req.body); @@ -141,7 +135,7 @@ export class UsersController { * Delete a user. Optionally, designate a transferee for their workflows and credentials. */ @Delete('/:id') - @RequireGlobalScope('user:delete') + @GlobalScope('user:delete') async deleteUser(req: UserRequest.Delete) { const { id: idToDelete } = req.params; @@ -293,7 +287,7 @@ export class UsersController { } @Patch('/:id/role') - @RequireGlobalScope('user:changeRole') + @GlobalScope('user:changeRole') @Licensed('feat:advancedPermissions') async changeGlobalRole(req: UserRequest.ChangeRole) { const { NO_ADMIN_ON_OWNER, NO_USER, NO_OWNER_ON_OWNER } = diff --git a/packages/cli/src/credentials/credentials.controller.ts b/packages/cli/src/credentials/credentials.controller.ts index 6dc62814f0c92..884f8ee56b6d9 100644 --- a/packages/cli/src/credentials/credentials.controller.ts +++ b/packages/cli/src/credentials/credentials.controller.ts @@ -11,14 +11,13 @@ import { License } from '@/License'; import { CredentialsRepository } from '@/databases/repositories/credentials.repository'; import { OwnershipService } from '@/services/ownership.service'; import { EnterpriseCredentialsService } from './credentials.service.ee'; -import { Authorized, Delete, Get, Licensed, Patch, Post, Put, RestController } from '@/decorators'; +import { Delete, Get, Licensed, Patch, Post, Put, RestController } from '@/decorators'; import { BadRequestError } from '@/errors/response-errors/bad-request.error'; import { UserManagementMailer } from '@/UserManagement/email'; import * as Db from '@/Db'; import * as utils from '@/utils'; import { listQueryMiddleware } from '@/middlewares'; -@Authorized() @RestController('/credentials') export class CredentialsController { constructor( diff --git a/packages/cli/src/databases/entities/User.ts b/packages/cli/src/databases/entities/User.ts index a9da54fe0dea5..238affb7c1df3 100644 --- a/packages/cli/src/databases/entities/User.ts +++ b/packages/cli/src/databases/entities/User.ts @@ -141,4 +141,9 @@ export class User extends WithTimestamps implements IUser { scopeOptions, ); } + + toJSON() { + const { password, apiKey, mfaSecret, mfaRecoveryCodes, ...rest } = this; + return rest; + } } diff --git a/packages/cli/src/decorators/Authorized.ts b/packages/cli/src/decorators/Authorized.ts deleted file mode 100644 index 4e97972a89755..0000000000000 --- a/packages/cli/src/decorators/Authorized.ts +++ /dev/null @@ -1,15 +0,0 @@ -/* eslint-disable @typescript-eslint/naming-convention */ -import { CONTROLLER_AUTH_ROLES } from './constants'; -import type { AuthRoleMetadata } from './types'; - -export function Authorized(authRole: AuthRoleMetadata[string] = 'any'): Function { - return function (target: Function | object, handlerName?: string) { - const controllerClass = handlerName ? target.constructor : target; - const authRoles = (Reflect.getMetadata(CONTROLLER_AUTH_ROLES, controllerClass) ?? - {}) as AuthRoleMetadata; - authRoles[handlerName ?? '*'] = authRole; - Reflect.defineMetadata(CONTROLLER_AUTH_ROLES, authRoles, controllerClass); - }; -} - -export const NoAuthRequired = () => Authorized('none'); diff --git a/packages/cli/src/decorators/Route.ts b/packages/cli/src/decorators/Route.ts index 64dd96d2935a5..420a168b006d5 100644 --- a/packages/cli/src/decorators/Route.ts +++ b/packages/cli/src/decorators/Route.ts @@ -5,6 +5,8 @@ import type { Method, RouteMetadata } from './types'; interface RouteOptions { middlewares?: RequestHandler[]; usesTemplates?: boolean; + /** When this flag is set to true, auth cookie isn't validated, and req.user will not be set */ + skipAuth?: boolean; } const RouteFactory = @@ -20,6 +22,7 @@ const RouteFactory = middlewares: options.middlewares ?? [], handlerName: String(handlerName), usesTemplates: options.usesTemplates ?? false, + skipAuth: options.skipAuth ?? false, }); Reflect.defineMetadata(CONTROLLER_ROUTES, routes, controllerClass); }; diff --git a/packages/cli/src/decorators/Scopes.ts b/packages/cli/src/decorators/Scopes.ts index 9e4bdca22a99b..aa2518017d699 100644 --- a/packages/cli/src/decorators/Scopes.ts +++ b/packages/cli/src/decorators/Scopes.ts @@ -2,7 +2,7 @@ import type { Scope } from '@n8n/permissions'; import type { ScopeMetadata } from './types'; import { CONTROLLER_REQUIRED_SCOPES } from './constants'; -export const RequireGlobalScope = (scope: Scope | Scope[]) => { +export const GlobalScope = (scope: Scope | Scope[]) => { // eslint-disable-next-line @typescript-eslint/ban-types return (target: Function | object, handlerName?: string) => { const controllerClass = handlerName ? target.constructor : target; diff --git a/packages/cli/src/decorators/constants.ts b/packages/cli/src/decorators/constants.ts index ba3d8a314712f..1487f91a0fc56 100644 --- a/packages/cli/src/decorators/constants.ts +++ b/packages/cli/src/decorators/constants.ts @@ -1,6 +1,5 @@ export const CONTROLLER_ROUTES = 'CONTROLLER_ROUTES'; export const CONTROLLER_BASE_PATH = 'CONTROLLER_BASE_PATH'; export const CONTROLLER_MIDDLEWARES = 'CONTROLLER_MIDDLEWARES'; -export const CONTROLLER_AUTH_ROLES = 'CONTROLLER_AUTH_ROLES'; export const CONTROLLER_LICENSE_FEATURES = 'CONTROLLER_LICENSE_FEATURES'; export const CONTROLLER_REQUIRED_SCOPES = 'CONTROLLER_REQUIRED_SCOPES'; diff --git a/packages/cli/src/decorators/index.ts b/packages/cli/src/decorators/index.ts index 5ec9e3d0105fe..94c94ef184f79 100644 --- a/packages/cli/src/decorators/index.ts +++ b/packages/cli/src/decorators/index.ts @@ -1,7 +1,6 @@ -export { Authorized, NoAuthRequired } from './Authorized'; export { RestController } from './RestController'; export { Get, Post, Put, Patch, Delete } from './Route'; export { Middleware } from './Middleware'; export { registerController } from './registerController'; export { Licensed } from './Licensed'; -export { RequireGlobalScope } from './Scopes'; +export { GlobalScope } from './Scopes'; diff --git a/packages/cli/src/decorators/registerController.ts b/packages/cli/src/decorators/registerController.ts index f6abd260fa423..d17799c00c6c8 100644 --- a/packages/cli/src/decorators/registerController.ts +++ b/packages/cli/src/decorators/registerController.ts @@ -1,13 +1,17 @@ import { Container } from 'typedi'; import { Router } from 'express'; import type { Application, Request, Response, RequestHandler } from 'express'; +import type { Scope } from '@n8n/permissions'; +import { ApplicationError } from 'n8n-workflow'; import type { Class } from 'n8n-core'; +import { AuthService } from '@/auth/auth.service'; import config from '@/config'; +import type { BooleanLicenseFeature } from '@/Interfaces'; +import { License } from '@/License'; import type { AuthenticatedRequest } from '@/requests'; import { send } from '@/ResponseHelper'; // TODO: move `ResponseHelper.send` to this file import { - CONTROLLER_AUTH_ROLES, CONTROLLER_BASE_PATH, CONTROLLER_LICENSE_FEATURES, CONTROLLER_MIDDLEWARES, @@ -15,31 +19,12 @@ import { CONTROLLER_ROUTES, } from './constants'; import type { - AuthRole, - AuthRoleMetadata, Controller, LicenseMetadata, MiddlewareMetadata, RouteMetadata, ScopeMetadata, } from './types'; -import type { BooleanLicenseFeature } from '@/Interfaces'; - -import { License } from '@/License'; -import type { Scope } from '@n8n/permissions'; -import { ApplicationError } from 'n8n-workflow'; - -export const createAuthMiddleware = - (authRole: AuthRole): RequestHandler => - ({ user }: AuthenticatedRequest, res, next) => { - if (authRole === 'none') return next(); - - if (!user) return res.status(401).json({ status: 'error', message: 'Unauthorized' }); - - if (authRole === 'any' || authRole === user.role) return next(); - - res.status(403).json({ status: 'error', message: 'Unauthorized' }); - }; export const createLicenseMiddleware = (features: BooleanLicenseFeature[]): RequestHandler => @@ -77,11 +62,6 @@ export const createGlobalScopeMiddleware = return next(); }; -const authFreeRoutes: string[] = []; - -export const canSkipAuth = (method: string, path: string): boolean => - authFreeRoutes.includes(`${method.toLowerCase()} ${path}`); - export const registerController = (app: Application, controllerClass: Class) => { const controller = Container.get(controllerClass as Class); const controllerBasePath = Reflect.getMetadata(CONTROLLER_BASE_PATH, controllerClass) as @@ -92,9 +72,6 @@ export const registerController = (app: Application, controllerClass: Class controller[handlerName].bind(controller) as RequestHandler); + const authService = Container.get(AuthService); + routes.forEach( - ({ method, path, middlewares: routeMiddlewares, handlerName, usesTemplates }) => { - const authRole = authRoles?.[handlerName] ?? authRoles?.['*']; + ({ method, path, middlewares: routeMiddlewares, handlerName, usesTemplates, skipAuth }) => { const features = licenseFeatures?.[handlerName] ?? licenseFeatures?.['*']; const scopes = requiredScopes?.[handlerName] ?? requiredScopes?.['*']; const handler = async (req: Request, res: Response) => await controller[handlerName](req, res); router[method]( path, - ...(authRole ? [createAuthMiddleware(authRole)] : []), + // eslint-disable-next-line @typescript-eslint/unbound-method + ...(skipAuth ? [] : [authService.authMiddleware]), ...(features ? [createLicenseMiddleware(features)] : []), ...(scopes ? [createGlobalScopeMiddleware(scopes)] : []), ...controllerMiddlewares, ...routeMiddlewares, usesTemplates ? handler : send(handler), ); - if (!authRole || authRole === 'none') authFreeRoutes.push(`${method} ${prefix}${path}`); }, ); diff --git a/packages/cli/src/decorators/types.ts b/packages/cli/src/decorators/types.ts index bbaccf39ab6a5..6a8b14e0fa3d5 100644 --- a/packages/cli/src/decorators/types.ts +++ b/packages/cli/src/decorators/types.ts @@ -1,13 +1,9 @@ import type { Request, Response, RequestHandler } from 'express'; -import type { GlobalRole } from '@db/entities/User'; import type { BooleanLicenseFeature } from '@/Interfaces'; import type { Scope } from '@n8n/permissions'; export type Method = 'get' | 'post' | 'put' | 'patch' | 'delete'; -export type AuthRole = GlobalRole | 'any' | 'none'; -export type AuthRoleMetadata = Record; - export type LicenseMetadata = Record; export type ScopeMetadata = Record; @@ -22,6 +18,7 @@ export interface RouteMetadata { handlerName: string; middlewares: RequestHandler[]; usesTemplates: boolean; + skipAuth: boolean; } export type Controller = Record< diff --git a/packages/cli/src/environments/sourceControl/sourceControl.controller.ee.ts b/packages/cli/src/environments/sourceControl/sourceControl.controller.ee.ts index d589af30d8acc..0cd0dde9a94c5 100644 --- a/packages/cli/src/environments/sourceControl/sourceControl.controller.ee.ts +++ b/packages/cli/src/environments/sourceControl/sourceControl.controller.ee.ts @@ -1,6 +1,6 @@ import type { PullResult } from 'simple-git'; import express from 'express'; -import { Authorized, Get, Post, Patch, RestController, RequireGlobalScope } from '@/decorators'; +import { Get, Post, Patch, RestController, GlobalScope } from '@/decorators'; import { sourceControlLicensedMiddleware, sourceControlLicensedAndEnabledMiddleware, @@ -17,7 +17,6 @@ import { getRepoType } from './sourceControlHelper.ee'; import { SourceControlGetStatus } from './types/sourceControlGetStatus'; import { BadRequestError } from '@/errors/response-errors/bad-request.error'; -@Authorized() @RestController('/source-control') export class SourceControlController { constructor( @@ -26,15 +25,14 @@ export class SourceControlController { private readonly internalHooks: InternalHooks, ) {} - @Authorized('none') - @Get('/preferences', { middlewares: [sourceControlLicensedMiddleware] }) + @Get('/preferences', { middlewares: [sourceControlLicensedMiddleware], skipAuth: true }) async getPreferences(): Promise { // returns the settings with the privateKey property redacted return this.sourceControlPreferencesService.getPreferences(); } @Post('/preferences', { middlewares: [sourceControlLicensedMiddleware] }) - @RequireGlobalScope('sourceControl:manage') + @GlobalScope('sourceControl:manage') async setPreferences(req: SourceControlRequest.UpdatePreferences) { if ( req.body.branchReadOnly === undefined && @@ -98,7 +96,7 @@ export class SourceControlController { } @Patch('/preferences', { middlewares: [sourceControlLicensedMiddleware] }) - @RequireGlobalScope('sourceControl:manage') + @GlobalScope('sourceControl:manage') async updatePreferences(req: SourceControlRequest.UpdatePreferences) { try { const sanitizedPreferences: Partial = { @@ -142,7 +140,7 @@ export class SourceControlController { } @Post('/disconnect', { middlewares: [sourceControlLicensedMiddleware] }) - @RequireGlobalScope('sourceControl:manage') + @GlobalScope('sourceControl:manage') async disconnect(req: SourceControlRequest.Disconnect) { try { return await this.sourceControlService.disconnect(req.body); @@ -151,7 +149,6 @@ export class SourceControlController { } } - @Authorized('any') @Get('/get-branches', { middlewares: [sourceControlLicensedMiddleware] }) async getBranches() { try { @@ -162,7 +159,7 @@ export class SourceControlController { } @Post('/push-workfolder', { middlewares: [sourceControlLicensedAndEnabledMiddleware] }) - @RequireGlobalScope('sourceControl:push') + @GlobalScope('sourceControl:push') async pushWorkfolder( req: SourceControlRequest.PushWorkFolder, res: express.Response, @@ -184,7 +181,7 @@ export class SourceControlController { } @Post('/pull-workfolder', { middlewares: [sourceControlLicensedAndEnabledMiddleware] }) - @RequireGlobalScope('sourceControl:pull') + @GlobalScope('sourceControl:pull') async pullWorkfolder( req: SourceControlRequest.PullWorkFolder, res: express.Response, @@ -203,7 +200,7 @@ export class SourceControlController { } @Get('/reset-workfolder', { middlewares: [sourceControlLicensedAndEnabledMiddleware] }) - @RequireGlobalScope('sourceControl:manage') + @GlobalScope('sourceControl:manage') async resetWorkfolder(): Promise { try { return await this.sourceControlService.resetWorkfolder(); @@ -212,7 +209,6 @@ export class SourceControlController { } } - @Authorized('any') @Get('/get-status', { middlewares: [sourceControlLicensedAndEnabledMiddleware] }) async getStatus(req: SourceControlRequest.GetStatus) { try { @@ -225,7 +221,6 @@ export class SourceControlController { } } - @Authorized('any') @Get('/status', { middlewares: [sourceControlLicensedMiddleware] }) async status(req: SourceControlRequest.GetStatus) { try { @@ -236,7 +231,7 @@ export class SourceControlController { } @Post('/generate-key-pair', { middlewares: [sourceControlLicensedMiddleware] }) - @RequireGlobalScope('sourceControl:manage') + @GlobalScope('sourceControl:manage') async generateKeyPair( req: SourceControlRequest.GenerateKeyPair, ): Promise { diff --git a/packages/cli/src/environments/variables/variables.controller.ee.ts b/packages/cli/src/environments/variables/variables.controller.ee.ts index 5e3eb6452f9be..16a59f26d827a 100644 --- a/packages/cli/src/environments/variables/variables.controller.ee.ts +++ b/packages/cli/src/environments/variables/variables.controller.ee.ts @@ -1,34 +1,24 @@ import { VariablesRequest } from '@/requests'; -import { - Authorized, - Delete, - Get, - Licensed, - Patch, - Post, - RequireGlobalScope, - RestController, -} from '@/decorators'; +import { Delete, Get, Licensed, Patch, Post, GlobalScope, RestController } from '@/decorators'; import { VariablesService } from './variables.service.ee'; import { BadRequestError } from '@/errors/response-errors/bad-request.error'; import { NotFoundError } from '@/errors/response-errors/not-found.error'; import { VariableValidationError } from '@/errors/variable-validation.error'; import { VariableCountLimitReachedError } from '@/errors/variable-count-limit-reached.error'; -@Authorized() @RestController('/variables') export class VariablesController { constructor(private readonly variablesService: VariablesService) {} @Get('/') - @RequireGlobalScope('variable:list') + @GlobalScope('variable:list') async getVariables() { return await this.variablesService.getAllCached(); } @Post('/') @Licensed('feat:variables') - @RequireGlobalScope('variable:create') + @GlobalScope('variable:create') async createVariable(req: VariablesRequest.Create) { const variable = req.body; delete variable.id; @@ -45,7 +35,7 @@ export class VariablesController { } @Get('/:id') - @RequireGlobalScope('variable:read') + @GlobalScope('variable:read') async getVariable(req: VariablesRequest.Get) { const id = req.params.id; const variable = await this.variablesService.getCached(id); @@ -57,7 +47,7 @@ export class VariablesController { @Patch('/:id') @Licensed('feat:variables') - @RequireGlobalScope('variable:update') + @GlobalScope('variable:update') async updateVariable(req: VariablesRequest.Update) { const id = req.params.id; const variable = req.body; @@ -75,7 +65,7 @@ export class VariablesController { } @Delete('/:id(\\w+)') - @RequireGlobalScope('variable:delete') + @GlobalScope('variable:delete') async deleteVariable(req: VariablesRequest.Delete) { const id = req.params.id; await this.variablesService.delete(id); diff --git a/packages/cli/src/eventbus/eventBus.controller.ee.ts b/packages/cli/src/eventbus/eventBus.controller.ee.ts index 216496092e369..95433a359b334 100644 --- a/packages/cli/src/eventbus/eventBus.controller.ee.ts +++ b/packages/cli/src/eventbus/eventBus.controller.ee.ts @@ -5,7 +5,7 @@ import type { } from 'n8n-workflow'; import { MessageEventBusDestinationTypeNames } from 'n8n-workflow'; -import { RestController, Get, Post, Delete, Authorized, RequireGlobalScope } from '@/decorators'; +import { RestController, Get, Post, Delete, GlobalScope } from '@/decorators'; import { AuthenticatedRequest } from '@/requests'; import { BadRequestError } from '@/errors/response-errors/bad-request.error'; @@ -52,7 +52,6 @@ const isMessageEventBusDestinationOptions = ( // Controller // ---------------------------------------- -@Authorized() @RestController('/eventbus') export class EventBusControllerEE { constructor(private readonly eventBus: MessageEventBus) {} @@ -62,7 +61,7 @@ export class EventBusControllerEE { // ---------------------------------------- @Get('/destination', { middlewares: [logStreamingLicensedMiddleware] }) - @RequireGlobalScope('eventBusDestination:list') + @GlobalScope('eventBusDestination:list') async getDestination(req: express.Request): Promise { if (isWithIdString(req.query)) { return await this.eventBus.findDestination(req.query.id); @@ -72,7 +71,7 @@ export class EventBusControllerEE { } @Post('/destination', { middlewares: [logStreamingLicensedMiddleware] }) - @RequireGlobalScope('eventBusDestination:create') + @GlobalScope('eventBusDestination:create') async postDestination(req: AuthenticatedRequest): Promise { let result: MessageEventBusDestination | undefined; if (isMessageEventBusDestinationOptions(req.body)) { @@ -116,7 +115,7 @@ export class EventBusControllerEE { } @Get('/testmessage', { middlewares: [logStreamingLicensedMiddleware] }) - @RequireGlobalScope('eventBusDestination:test') + @GlobalScope('eventBusDestination:test') async sendTestMessage(req: express.Request): Promise { if (isWithIdString(req.query)) { return await this.eventBus.testDestination(req.query.id); @@ -125,7 +124,7 @@ export class EventBusControllerEE { } @Delete('/destination', { middlewares: [logStreamingLicensedMiddleware] }) - @RequireGlobalScope('eventBusDestination:delete') + @GlobalScope('eventBusDestination:delete') async deleteDestination(req: AuthenticatedRequest) { if (isWithIdString(req.query)) { return await this.eventBus.removeDestination(req.query.id); diff --git a/packages/cli/src/eventbus/eventBus.controller.ts b/packages/cli/src/eventbus/eventBus.controller.ts index a76c90ddc8628..179d44da41668 100644 --- a/packages/cli/src/eventbus/eventBus.controller.ts +++ b/packages/cli/src/eventbus/eventBus.controller.ts @@ -2,7 +2,7 @@ import express from 'express'; import type { IRunExecutionData } from 'n8n-workflow'; import { EventMessageTypeNames } from 'n8n-workflow'; -import { RestController, Get, Post, Authorized, RequireGlobalScope } from '@/decorators'; +import { RestController, Get, Post, GlobalScope } from '@/decorators'; import { BadRequestError } from '@/errors/response-errors/bad-request.error'; import { isEventMessageOptions } from './EventMessageClasses/AbstractEventMessage'; @@ -33,7 +33,6 @@ const isWithQueryString = (candidate: unknown): candidate is { query: string } = // Controller // ---------------------------------------- -@Authorized() @RestController('/eventbus') export class EventBusController { constructor( @@ -45,7 +44,7 @@ export class EventBusController { // Events // ---------------------------------------- @Get('/event') - @RequireGlobalScope('eventBusEvent:query') + @GlobalScope('eventBusEvent:query') async getEvents( req: express.Request, ): Promise> { @@ -67,14 +66,14 @@ export class EventBusController { } @Get('/failed') - @RequireGlobalScope('eventBusEvent:list') + @GlobalScope('eventBusEvent:list') async getFailedEvents(req: express.Request): Promise { const amount = parseInt(req.query?.amount as string) ?? 5; return await this.eventBus.getEventsFailed(amount); } @Get('/execution/:id') - @RequireGlobalScope('eventBusEvent:read') + @GlobalScope('eventBusEvent:read') async getEventForExecutionId(req: express.Request): Promise { if (req.params?.id) { let logHistory; @@ -87,7 +86,7 @@ export class EventBusController { } @Get('/execution-recover/:id') - @RequireGlobalScope('eventBusEvent:read') + @GlobalScope('eventBusEvent:read') async getRecoveryForExecutionId(req: express.Request): Promise { const { id } = req.params; if (req.params?.id) { @@ -102,7 +101,7 @@ export class EventBusController { } @Post('/event') - @RequireGlobalScope('eventBusEvent:create') + @GlobalScope('eventBusEvent:create') async postEvent(req: express.Request): Promise { let msg: EventMessageTypes | undefined; if (isEventMessageOptions(req.body)) { diff --git a/packages/cli/src/executions/executions.controller.ts b/packages/cli/src/executions/executions.controller.ts index aa9c2148009fe..3d778b2bc7770 100644 --- a/packages/cli/src/executions/executions.controller.ts +++ b/packages/cli/src/executions/executions.controller.ts @@ -1,7 +1,7 @@ import type { GetManyActiveFilter } from './execution.types'; import { ExecutionRequest } from './execution.types'; import { ExecutionService } from './execution.service'; -import { Authorized, Get, Post, RestController } from '@/decorators'; +import { Get, Post, RestController } from '@/decorators'; import { EnterpriseExecutionsService } from './execution.service.ee'; import { License } from '@/License'; import { WorkflowSharingService } from '@/workflows/workflowSharing.service'; @@ -11,7 +11,6 @@ import { jsonParse } from 'n8n-workflow'; import { NotFoundError } from '@/errors/response-errors/not-found.error'; import { ActiveExecutionService } from './active-execution.service'; -@Authorized() @RestController('/executions') export class ExecutionsController { private readonly isQueueMode = config.getEnv('executions.mode') === 'queue'; diff --git a/packages/cli/src/license/license.controller.ts b/packages/cli/src/license/license.controller.ts index 96900366289fc..086ab3d4f87b1 100644 --- a/packages/cli/src/license/license.controller.ts +++ b/packages/cli/src/license/license.controller.ts @@ -1,8 +1,7 @@ -import { Authorized, Get, Post, RequireGlobalScope, RestController } from '@/decorators'; +import { Get, Post, GlobalScope, RestController } from '@/decorators'; import { LicenseRequest } from '@/requests'; import { LicenseService } from './license.service'; -@Authorized() @RestController('/license') export class LicenseController { constructor(private readonly licenseService: LicenseService) {} @@ -13,7 +12,7 @@ export class LicenseController { } @Post('/activate') - @RequireGlobalScope('license:manage') + @GlobalScope('license:manage') async activateLicense(req: LicenseRequest.Activate) { const { activationKey } = req.body; await this.licenseService.activateLicense(activationKey); @@ -21,7 +20,7 @@ export class LicenseController { } @Post('/renew') - @RequireGlobalScope('license:manage') + @GlobalScope('license:manage') async renewLicense() { await this.licenseService.renewLicense(); return await this.getTokenAndData(); diff --git a/packages/cli/src/middlewares/auth.ts b/packages/cli/src/middlewares/auth.ts deleted file mode 100644 index 12c1d78d655dc..0000000000000 --- a/packages/cli/src/middlewares/auth.ts +++ /dev/null @@ -1,114 +0,0 @@ -import type { Application, NextFunction, Request, RequestHandler, Response } from 'express'; -import { Container } from 'typedi'; -import jwt from 'jsonwebtoken'; -import passport from 'passport'; -import { Strategy } from 'passport-jwt'; -import { sync as globSync } from 'fast-glob'; -import type { JwtPayload } from '@/Interfaces'; -import type { AuthenticatedRequest } from '@/requests'; -import { AUTH_COOKIE_NAME, EDITOR_UI_DIST_DIR } from '@/constants'; -import { issueCookie, resolveJwtContent } from '@/auth/jwt'; -import { canSkipAuth } from '@/decorators/registerController'; -import { Logger } from '@/Logger'; -import { JwtService } from '@/services/jwt.service'; -import config from '@/config'; - -const jwtFromRequest = (req: Request) => { - // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - return (req.cookies?.[AUTH_COOKIE_NAME] as string | undefined) ?? null; -}; - -const userManagementJwtAuth = (): RequestHandler => { - const jwtStrategy = new Strategy( - { - jwtFromRequest, - secretOrKey: Container.get(JwtService).jwtSecret, - }, - async (jwtPayload: JwtPayload, done) => { - try { - const user = await resolveJwtContent(jwtPayload); - return done(null, user); - } catch (error) { - Container.get(Logger).debug('Failed to extract user from JWT payload', { jwtPayload }); - return done(null, false, { message: 'User not found' }); - } - }, - ); - - passport.use(jwtStrategy); - return passport.initialize(); -}; - -/** - * middleware to refresh cookie before it expires - */ -export const refreshExpiringCookie = (async (req: AuthenticatedRequest, res, next) => { - const jwtRefreshTimeoutHours = config.get('userManagement.jwtRefreshTimeoutHours'); - - let jwtRefreshTimeoutMilliSeconds: number; - - if (jwtRefreshTimeoutHours === 0) { - const jwtSessionDurationHours = config.get('userManagement.jwtSessionDurationHours'); - - jwtRefreshTimeoutMilliSeconds = Math.floor(jwtSessionDurationHours * 0.25 * 60 * 60 * 1000); - } else { - jwtRefreshTimeoutMilliSeconds = Math.floor(jwtRefreshTimeoutHours * 60 * 60 * 1000); - } - - const cookieAuth = jwtFromRequest(req); - - if (cookieAuth && req.user && jwtRefreshTimeoutHours !== -1) { - const cookieContents = jwt.decode(cookieAuth) as JwtPayload & { exp: number }; - if (cookieContents.exp * 1000 - Date.now() < jwtRefreshTimeoutMilliSeconds) { - await issueCookie(res, req.user); - } - } - next(); -}) satisfies RequestHandler; - -const passportMiddleware = passport.authenticate('jwt', { session: false }) as RequestHandler; - -const staticAssets = globSync(['**/*.html', '**/*.svg', '**/*.png', '**/*.ico'], { - cwd: EDITOR_UI_DIST_DIR, -}); - -// TODO: delete this -const isPostInvitationAccept = (req: Request, restEndpoint: string): boolean => - req.method === 'POST' && - new RegExp(`/${restEndpoint}/invitations/[\\w\\d-]*`).test(req.url) && - req.url.includes('accept'); - -const isAuthExcluded = (url: string, ignoredEndpoints: Readonly): boolean => - !!ignoredEndpoints - .filter(Boolean) // skip empty paths - .find((ignoredEndpoint) => url.startsWith(`/${ignoredEndpoint}`)); - -/** - * This sets up the auth middlewares in the correct order - */ -export const setupAuthMiddlewares = ( - app: Application, - ignoredEndpoints: Readonly, - restEndpoint: string, -) => { - app.use(userManagementJwtAuth()); - - app.use(async (req: Request, res: Response, next: NextFunction) => { - if ( - // TODO: refactor me!!! - // skip authentication for preflight requests - req.method === 'OPTIONS' || - staticAssets.includes(req.url.slice(1)) || - canSkipAuth(req.method, req.path) || - isAuthExcluded(req.url, ignoredEndpoints) || - req.url.startsWith(`/${restEndpoint}/settings`) || - isPostInvitationAccept(req, restEndpoint) - ) { - return next(); - } - - return passportMiddleware(req, res, next); - }); - - app.use(refreshExpiringCookie); -}; diff --git a/packages/cli/src/middlewares/index.ts b/packages/cli/src/middlewares/index.ts index c9390709df7b2..75ebae01c66fe 100644 --- a/packages/cli/src/middlewares/index.ts +++ b/packages/cli/src/middlewares/index.ts @@ -1,4 +1,3 @@ -export * from './auth'; export * from './bodyParser'; export * from './cors'; export * from './listQuery'; diff --git a/packages/cli/src/permissions/roles.ts b/packages/cli/src/permissions/roles.ts index 39d9c649f765f..68d61af0b2afb 100644 --- a/packages/cli/src/permissions/roles.ts +++ b/packages/cli/src/permissions/roles.ts @@ -2,6 +2,7 @@ import type { Scope } from '@n8n/permissions'; export const ownerPermissions: Scope[] = [ 'auditLogs:manage', + 'banner:dismiss', 'credential:create', 'credential:read', 'credential:update', diff --git a/packages/cli/src/push/index.ts b/packages/cli/src/push/index.ts index e740821321cd0..a88688d6a6efb 100644 --- a/packages/cli/src/push/index.ts +++ b/packages/cli/src/push/index.ts @@ -2,19 +2,19 @@ import { EventEmitter } from 'events'; import { ServerResponse } from 'http'; import type { Server } from 'http'; import type { Socket } from 'net'; -import type { Application, RequestHandler } from 'express'; +import type { Application } from 'express'; import { Server as WSServer } from 'ws'; import { parse as parseUrl } from 'url'; import { Container, Service } from 'typedi'; import config from '@/config'; -import { resolveJwt } from '@/auth/jwt'; -import { AUTH_COOKIE_NAME } from '@/constants'; import { SSEPush } from './sse.push'; import { WebSocketPush } from './websocket.push'; import type { PushResponse, SSEPushRequest, WebSocketPushRequest } from './types'; import type { IPushDataType } from '@/Interfaces'; import type { User } from '@db/entities/User'; import { OnShutdown } from '@/decorators/OnShutdown'; +import { AuthService } from '@/auth/auth.service'; +import { BadRequestError } from '@/errors/response-errors/bad-request.error'; const useWebSockets = config.getEnv('push.backend') === 'websocket'; @@ -39,14 +39,24 @@ export class Push extends EventEmitter { handleRequest(req: SSEPushRequest | WebSocketPushRequest, res: PushResponse) { const { - userId, + user, + ws, query: { sessionId }, } = req; + if (!sessionId) { + if (ws) { + ws.send('The query parameter "sessionId" is missing!'); + ws.close(1008); + return; + } + throw new BadRequestError('The query parameter "sessionId" is missing!'); + } + if (req.ws) { - (this.backend as WebSocketPush).add(sessionId, userId, req.ws); + (this.backend as WebSocketPush).add(sessionId, user.id, req.ws); } else if (!useWebSockets) { - (this.backend as SSEPush).add(sessionId, userId, { req, res }); + (this.backend as SSEPush).add(sessionId, user.id, { req, res }); } else { res.status(401).send('Unauthorized'); return; @@ -102,46 +112,12 @@ export const setupPushServer = (restEndpoint: string, server: Server, app: Appli export const setupPushHandler = (restEndpoint: string, app: Application) => { const endpoint = `/${restEndpoint}/push`; - - const pushValidationMiddleware: RequestHandler = async ( - req: SSEPushRequest | WebSocketPushRequest, - res, - next, - ) => { - const ws = req.ws; - - const { sessionId } = req.query; - if (sessionId === undefined) { - if (ws) { - ws.send('The query parameter "sessionId" is missing!'); - ws.close(1008); - } else { - next(new Error('The query parameter "sessionId" is missing!')); - } - return; - } - try { - // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-unsafe-member-access - const authCookie: string = req.cookies?.[AUTH_COOKIE_NAME] ?? ''; - const user = await resolveJwt(authCookie); - req.userId = user.id; - } catch (error) { - if (ws) { - ws.send(`Unauthorized: ${(error as Error).message}`); - ws.close(1008); - } else { - res.status(401).send('Unauthorized'); - } - return; - } - - next(); - }; - const push = Container.get(Push); + const authService = Container.get(AuthService); app.use( endpoint, - pushValidationMiddleware, + // eslint-disable-next-line @typescript-eslint/unbound-method + authService.authMiddleware, (req: SSEPushRequest | WebSocketPushRequest, res: PushResponse) => push.handleRequest(req, res), ); }; diff --git a/packages/cli/src/push/types.ts b/packages/cli/src/push/types.ts index 2a65b08131166..d99ca3f6121b5 100644 --- a/packages/cli/src/push/types.ts +++ b/packages/cli/src/push/types.ts @@ -1,13 +1,15 @@ -import type { User } from '@db/entities/User'; -import type { Request, Response } from 'express'; +import type { Response } from 'express'; import type { WebSocket } from 'ws'; +import type { User } from '@db/entities/User'; +import type { AuthenticatedRequest } from '@/requests'; + // TODO: move all push related types here -export type PushRequest = Request<{}, {}, {}, { sessionId: string }>; +export type PushRequest = AuthenticatedRequest<{}, {}, {}, { sessionId: string }>; -export type SSEPushRequest = PushRequest & { ws: undefined; userId: User['id'] }; -export type WebSocketPushRequest = PushRequest & { ws: WebSocket; userId: User['id'] }; +export type SSEPushRequest = PushRequest & { ws: undefined }; +export type WebSocketPushRequest = PushRequest & { ws: WebSocket }; export type PushResponse = Response & { req: PushRequest }; diff --git a/packages/cli/src/requests.ts b/packages/cli/src/requests.ts index 4b64f844a9426..d7a6911c091a2 100644 --- a/packages/cli/src/requests.ts +++ b/packages/cli/src/requests.ts @@ -62,8 +62,12 @@ export type AuthenticatedRequest< ResponseBody = {}, RequestBody = {}, RequestQuery = {}, -> = Omit, 'user'> & { +> = Omit< + express.Request, + 'user' | 'cookies' +> & { user: User; + cookies: Record; }; // ---------------------------------- diff --git a/packages/cli/src/services/user.service.ts b/packages/cli/src/services/user.service.ts index 6c753788faa97..0ce290da8bd7d 100644 --- a/packages/cli/src/services/user.service.ts +++ b/packages/cli/src/services/user.service.ts @@ -1,17 +1,15 @@ import { Container, Service } from 'typedi'; -import { type AssignableRole, User } from '@db/entities/User'; import type { IUserSettings } from 'n8n-workflow'; +import { ApplicationError, ErrorReporterProxy as ErrorReporter } from 'n8n-workflow'; + +import { type AssignableRole, User } from '@db/entities/User'; import { UserRepository } from '@db/repositories/user.repository'; import type { PublicUser } from '@/Interfaces'; import type { PostHogClient } from '@/posthog'; -import { type JwtPayload, JwtService } from './jwt.service'; -import { TokenExpiredError } from 'jsonwebtoken'; import { Logger } from '@/Logger'; -import { createPasswordSha } from '@/auth/jwt'; import { UserManagementMailer } from '@/UserManagement/email'; import { InternalHooks } from '@/InternalHooks'; import { UrlService } from '@/services/url.service'; -import { ApplicationError, ErrorReporterProxy as ErrorReporter } from 'n8n-workflow'; import type { UserRequest } from '@/requests'; import { InternalServerError } from '@/errors/response-errors/internal-server.error'; @@ -20,7 +18,6 @@ export class UserService { constructor( private readonly logger: Logger, private readonly userRepository: UserRepository, - private readonly jwtService: JwtService, private readonly mailer: UserManagementMailer, private readonly urlService: UrlService, ) {} @@ -39,57 +36,6 @@ export class UserService { return await this.userRepository.update(userId, { settings: { ...settings, ...newSettings } }); } - generatePasswordResetToken(user: User, expiresIn = '20m') { - return this.jwtService.sign( - { sub: user.id, passwordSha: createPasswordSha(user) }, - { expiresIn }, - ); - } - - generatePasswordResetUrl(user: User) { - const instanceBaseUrl = this.urlService.getInstanceBaseUrl(); - const url = new URL(`${instanceBaseUrl}/change-password`); - - url.searchParams.append('token', this.generatePasswordResetToken(user)); - url.searchParams.append('mfaEnabled', user.mfaEnabled.toString()); - - return url.toString(); - } - - async resolvePasswordResetToken(token: string): Promise { - let decodedToken: JwtPayload & { passwordSha: string }; - try { - decodedToken = this.jwtService.verify(token); - } catch (e) { - if (e instanceof TokenExpiredError) { - this.logger.debug('Reset password token expired', { token }); - } else { - this.logger.debug('Error verifying token', { token }); - } - return; - } - - const user = await this.userRepository.findOne({ - where: { id: decodedToken.sub }, - relations: ['authIdentities'], - }); - - if (!user) { - this.logger.debug( - 'Request to resolve password token failed because no user was found for the provided user ID', - { userId: decodedToken.sub, token }, - ); - return; - } - - if (createPasswordSha(user) !== decodedToken.passwordSha) { - this.logger.debug('Password updated since this token was generated'); - return; - } - - return user; - } - async toPublic( user: User, options?: { diff --git a/packages/cli/src/sso/saml/routes/saml.controller.ee.ts b/packages/cli/src/sso/saml/routes/saml.controller.ee.ts index cfa68bfdd6fd3..597fdfb93ca91 100644 --- a/packages/cli/src/sso/saml/routes/saml.controller.ee.ts +++ b/packages/cli/src/sso/saml/routes/saml.controller.ee.ts @@ -1,26 +1,18 @@ import express from 'express'; -import { - Authorized, - Get, - NoAuthRequired, - Post, - RestController, - RequireGlobalScope, -} from '@/decorators'; -import { SamlUrls } from '../constants'; -import { - samlLicensedAndEnabledMiddleware, - samlLicensedMiddleware, -} from '../middleware/samlEnabledMiddleware'; -import { SamlService } from '../saml.service.ee'; -import { SamlConfiguration } from '../types/requests'; -import { getInitSSOFormView } from '../views/initSsoPost'; -import { issueCookie } from '@/auth/jwt'; import { validate } from 'class-validator'; import type { PostBindingContext } from 'samlify/types/src/entity'; -import { isConnectionTestRequest, isSamlLicensedAndEnabled } from '../samlHelpers'; -import type { SamlLoginBinding } from '../types'; +import url from 'url'; + +import { Get, Post, RestController, GlobalScope } from '@/decorators'; +import { AuthService } from '@/auth/auth.service'; import { AuthenticatedRequest } from '@/requests'; +import { InternalHooks } from '@/InternalHooks'; +import querystring from 'querystring'; +import { BadRequestError } from '@/errors/response-errors/bad-request.error'; +import { AuthError } from '@/errors/response-errors/auth.error'; +import { UrlService } from '@/services/url.service'; + +import { SamlUrls } from '../constants'; import { getServiceProviderConfigTestReturnUrl, getServiceProviderEntityId, @@ -28,25 +20,27 @@ import { } from '../serviceProvider.ee'; import { getSamlConnectionTestSuccessView } from '../views/samlConnectionTestSuccess'; import { getSamlConnectionTestFailedView } from '../views/samlConnectionTestFailed'; -import { InternalHooks } from '@/InternalHooks'; -import url from 'url'; -import querystring from 'querystring'; -import { BadRequestError } from '@/errors/response-errors/bad-request.error'; -import { AuthError } from '@/errors/response-errors/auth.error'; -import { UrlService } from '@/services/url.service'; +import { isConnectionTestRequest, isSamlLicensedAndEnabled } from '../samlHelpers'; +import type { SamlLoginBinding } from '../types'; +import { + samlLicensedAndEnabledMiddleware, + samlLicensedMiddleware, +} from '../middleware/samlEnabledMiddleware'; +import { SamlService } from '../saml.service.ee'; +import { SamlConfiguration } from '../types/requests'; +import { getInitSSOFormView } from '../views/initSsoPost'; -@Authorized() @RestController('/sso/saml') export class SamlController { constructor( + private readonly authService: AuthService, private readonly samlService: SamlService, private readonly urlService: UrlService, private readonly internalHooks: InternalHooks, ) {} - @NoAuthRequired() - @Get(SamlUrls.metadata) - async getServiceProviderMetadata(req: express.Request, res: express.Response) { + @Get(SamlUrls.metadata, { skipAuth: true }) + async getServiceProviderMetadata(_: express.Request, res: express.Response) { return res .header('Content-Type', 'text/xml') .send(this.samlService.getServiceProviderInstance().getMetadata()); @@ -56,7 +50,6 @@ export class SamlController { * GET /sso/saml/config * Return SAML config */ - @Authorized('any') @Get(SamlUrls.config, { middlewares: [samlLicensedMiddleware] }) async configGet() { const prefs = this.samlService.samlPreferences; @@ -72,7 +65,7 @@ export class SamlController { * Set SAML config */ @Post(SamlUrls.config, { middlewares: [samlLicensedMiddleware] }) - @RequireGlobalScope('saml:manage') + @GlobalScope('saml:manage') async configPost(req: SamlConfiguration.Update) { const validationResult = await validate(req.body); if (validationResult.length === 0) { @@ -91,7 +84,7 @@ export class SamlController { * Set SAML config */ @Post(SamlUrls.configToggleEnabled, { middlewares: [samlLicensedMiddleware] }) - @RequireGlobalScope('saml:manage') + @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'); @@ -104,8 +97,7 @@ export class SamlController { * GET /sso/saml/acs * Assertion Consumer Service endpoint */ - @NoAuthRequired() - @Get(SamlUrls.acs, { middlewares: [samlLicensedMiddleware] }) + @Get(SamlUrls.acs, { middlewares: [samlLicensedMiddleware], skipAuth: true }) async acsGet(req: SamlConfiguration.AcsRequest, res: express.Response) { return await this.acsHandler(req, res, 'redirect'); } @@ -114,8 +106,7 @@ export class SamlController { * POST /sso/saml/acs * Assertion Consumer Service endpoint */ - @NoAuthRequired() - @Post(SamlUrls.acs, { middlewares: [samlLicensedMiddleware] }) + @Post(SamlUrls.acs, { middlewares: [samlLicensedMiddleware], skipAuth: true }) async acsPost(req: SamlConfiguration.AcsRequest, res: express.Response) { return await this.acsHandler(req, res, 'post'); } @@ -147,7 +138,7 @@ export class SamlController { }); // Only sign in user if SAML is enabled, otherwise treat as test connection if (isSamlLicensedAndEnabled()) { - await issueCookie(res, loginResult.authenticatedUser); + this.authService.issueCookie(res, loginResult.authenticatedUser); if (loginResult.onboardingRequired) { return res.redirect(this.urlService.getInstanceBaseUrl() + SamlUrls.samlOnboarding); } else { @@ -180,8 +171,7 @@ export class SamlController { * Access URL for implementing SP-init SSO * This endpoint is available if SAML is licensed and enabled */ - @NoAuthRequired() - @Get(SamlUrls.initSSO, { middlewares: [samlLicensedAndEnabledMiddleware] }) + @Get(SamlUrls.initSSO, { middlewares: [samlLicensedAndEnabledMiddleware], skipAuth: true }) async initSsoGet(req: express.Request, res: express.Response) { let redirectUrl = ''; try { @@ -207,7 +197,7 @@ export class SamlController { * This endpoint is available if SAML is licensed and the requestor is an instance owner */ @Get(SamlUrls.configTest, { middlewares: [samlLicensedMiddleware] }) - @RequireGlobalScope('saml:manage') + @GlobalScope('saml:manage') async configTestGet(req: AuthenticatedRequest, res: express.Response) { return await this.handleInitSSO(res, getServiceProviderConfigTestReturnUrl()); } diff --git a/packages/cli/src/workflows/workflowHistory/workflowHistory.controller.ee.ts b/packages/cli/src/workflows/workflowHistory/workflowHistory.controller.ee.ts index 2ac2cf42332f3..c57e3cb0b584a 100644 --- a/packages/cli/src/workflows/workflowHistory/workflowHistory.controller.ee.ts +++ b/packages/cli/src/workflows/workflowHistory/workflowHistory.controller.ee.ts @@ -1,4 +1,4 @@ -import { Authorized, RestController, Get, Middleware } from '@/decorators'; +import { RestController, Get, Middleware } from '@/decorators'; import { WorkflowHistoryRequest } from '@/requests'; import { WorkflowHistoryService } from './workflowHistory.service.ee'; import { Request, Response, NextFunction } from 'express'; @@ -11,7 +11,6 @@ import { WorkflowHistoryVersionNotFoundError } from '@/errors/workflow-history-v const DEFAULT_TAKE = 20; -@Authorized() @RestController('/workflow-history') export class WorkflowHistoryController { constructor(private readonly historyService: WorkflowHistoryService) {} diff --git a/packages/cli/src/workflows/workflows.controller.ts b/packages/cli/src/workflows/workflows.controller.ts index 807b892cc3a27..d740667db71c6 100644 --- a/packages/cli/src/workflows/workflows.controller.ts +++ b/packages/cli/src/workflows/workflows.controller.ts @@ -8,7 +8,7 @@ import * as ResponseHelper from '@/ResponseHelper'; import * as WorkflowHelpers from '@/WorkflowHelpers'; import type { IWorkflowResponse } from '@/Interfaces'; import config from '@/config'; -import { Authorized, Delete, Get, Patch, Post, Put, RestController } from '@/decorators'; +import { Delete, Get, Patch, Post, Put, RestController } from '@/decorators'; import { SharedWorkflow, type WorkflowSharingRole } from '@db/entities/SharedWorkflow'; import { WorkflowEntity } from '@db/entities/WorkflowEntity'; import { SharedWorkflowRepository } from '@db/repositories/sharedWorkflow.repository'; @@ -39,7 +39,6 @@ import { WorkflowExecutionService } from './workflowExecution.service'; import { WorkflowSharingService } from './workflowSharing.service'; import { UserManagementMailer } from '@/UserManagement/email'; -@Authorized() @RestController('/workflows') export class WorkflowsController { constructor( diff --git a/packages/cli/test/integration/PermissionChecker.test.ts b/packages/cli/test/integration/PermissionChecker.test.ts index 5f544b4d9de48..f80fbc02ddfd1 100644 --- a/packages/cli/test/integration/PermissionChecker.test.ts +++ b/packages/cli/test/integration/PermissionChecker.test.ts @@ -27,6 +27,8 @@ import type { SaveCredentialFunction } from '../integration/shared/types'; import { mockNodeTypesData } from '../unit/Helpers'; import { affixRoleToSaveCredential } from '../integration/shared/db/credentials'; import { createOwner, createUser } from '../integration/shared/db/users'; +import { SharedCredentialsRepository } from '@/databases/repositories/sharedCredentials.repository'; +import type { WorkflowEntity } from '@/databases/entities/WorkflowEntity'; export const toTargetCallErrorMsg = (subworkflowId: string) => `Target workflow ID ${subworkflowId} may not be called`; @@ -69,8 +71,34 @@ export function createSubworkflow({ }); } +const createWorkflow = async (nodes: INode[], workflowOwner?: User): Promise => { + const workflowDetails = { + id: uuid(), + name: 'test', + active: false, + connections: {}, + nodeTypes: mockNodeTypes, + nodes, + }; + + const workflowEntity = await Container.get(WorkflowRepository).save(workflowDetails); + + if (workflowOwner) { + await Container.get(SharedWorkflowRepository).save({ + workflow: workflowEntity, + user: workflowOwner, + role: 'workflow:owner', + }); + } + + return workflowEntity; +}; + let saveCredential: SaveCredentialFunction; +let owner: User; +let member: User; + const mockNodeTypes = mockInstance(NodeTypes); mockInstance(LoadNodesAndCredentials, { loadedNodes: mockNodeTypesData(['start', 'actionNetwork']), @@ -78,17 +106,29 @@ mockInstance(LoadNodesAndCredentials, { let permissionChecker: PermissionChecker; +let license: LicenseMocker; + beforeAll(async () => { await testDb.init(); saveCredential = affixRoleToSaveCredential('credential:owner'); permissionChecker = Container.get(PermissionChecker); + + [owner, member] = await Promise.all([createOwner(), createUser()]); + + license = new LicenseMocker(); + license.mock(Container.get(License)); + license.setDefaults({ + features: ['feat:sharing'], + }); }); -describe('check()', () => { - const workflowId = randomPositiveDigit().toString(); +beforeEach(() => { + license.reset(); +}); +describe('check()', () => { beforeEach(async () => { await testDb.truncate(['Workflow', 'Credentials']); }); @@ -98,7 +138,6 @@ describe('check()', () => { }); test('should allow if workflow has no creds', async () => { - const userId = uuid(); const nodes: INode[] = [ { id: uuid(), @@ -110,7 +149,11 @@ describe('check()', () => { }, ]; - expect(async () => await permissionChecker.check(workflowId, userId, nodes)).not.toThrow(); + const workflow = await createWorkflow(nodes, member); + + await expect( + permissionChecker.check(workflow.id, member.id, workflow.nodes), + ).resolves.not.toThrow(); }); test('should allow if requesting user is instance owner', async () => { @@ -132,14 +175,126 @@ describe('check()', () => { }, ]; - expect(async () => await permissionChecker.check(workflowId, owner.id, nodes)).not.toThrow(); + const workflow = await createWorkflow(nodes); + + await expect( + permissionChecker.check(workflow.id, owner.id, workflow.nodes), + ).resolves.not.toThrow(); + }); + + test('should allow if workflow creds are valid subset (shared credential)', async () => { + const ownerCred = await saveCredential(randomCred(), { user: owner }); + const memberCred = await saveCredential(randomCred(), { user: member }); + + await Container.get(SharedCredentialsRepository).save( + Container.get(SharedCredentialsRepository).create({ + credentialsId: ownerCred.id, + userId: member.id, + role: 'credential:user', + }), + ); + + const nodes: INode[] = [ + { + id: uuid(), + name: 'Action Network', + type: 'n8n-nodes-base.actionNetwork', + parameters: {}, + typeVersion: 1, + position: [0, 0], + credentials: { + actionNetworkApi: { + id: ownerCred.id, + name: ownerCred.name, + }, + }, + }, + { + id: uuid(), + name: 'Action Network 2', + type: 'n8n-nodes-base.actionNetwork', + parameters: {}, + typeVersion: 1, + position: [0, 0], + credentials: { + actionNetworkApi: { + id: memberCred.id, + name: memberCred.name, + }, + }, + }, + ]; + + const workflow = await createWorkflow(nodes, member); + + await expect( + permissionChecker.check(workflow.id, member.id, workflow.nodes), + ).resolves.not.toThrow(); }); - test('should allow if workflow creds are valid subset', async () => { + test('should allow if workflow creds are valid subset (shared workflow)', async () => { + const ownerCred = await saveCredential(randomCred(), { user: owner }); + const memberCred = await saveCredential(randomCred(), { user: member }); + + const nodes: INode[] = [ + { + id: uuid(), + name: 'Action Network', + type: 'n8n-nodes-base.actionNetwork', + parameters: {}, + typeVersion: 1, + position: [0, 0], + credentials: { + actionNetworkApi: { + id: ownerCred.id, + name: ownerCred.name, + }, + }, + }, + { + id: uuid(), + name: 'Action Network 2', + type: 'n8n-nodes-base.actionNetwork', + parameters: {}, + typeVersion: 1, + position: [0, 0], + credentials: { + actionNetworkApi: { + id: memberCred.id, + name: memberCred.name, + }, + }, + }, + ]; + + const workflow = await createWorkflow(nodes, member); + await Container.get(SharedWorkflowRepository).save( + Container.get(SharedWorkflowRepository).create({ + workflowId: workflow.id, + userId: owner.id, + role: 'workflow:editor', + }), + ); + + await expect( + permissionChecker.check(workflow.id, member.id, workflow.nodes), + ).resolves.not.toThrow(); + }); + + test('should deny if workflow creds are valid subset but sharing is disabled', async () => { const [owner, member] = await Promise.all([createOwner(), createUser()]); const ownerCred = await saveCredential(randomCred(), { user: owner }); const memberCred = await saveCredential(randomCred(), { user: member }); + + await Container.get(SharedCredentialsRepository).save( + Container.get(SharedCredentialsRepository).create({ + credentialsId: ownerCred.id, + userId: member.id, + role: 'credential:user', + }), + ); + const nodes: INode[] = [ { id: uuid(), @@ -171,7 +326,10 @@ describe('check()', () => { }, ]; - expect(async () => await permissionChecker.check(workflowId, owner.id, nodes)).not.toThrow(); + const workflow = await createWorkflow(nodes, member); + + license.disable('feat:sharing'); + await expect(permissionChecker.check(workflow.id, member.id, nodes)).rejects.toThrow(); }); test('should deny if workflow creds are not valid subset', async () => { @@ -179,69 +337,46 @@ describe('check()', () => { const memberCred = await saveCredential(randomCred(), { user: member }); - const workflowDetails = { - id: randomPositiveDigit().toString(), - name: 'test', - active: false, - connections: {}, - nodeTypes: mockNodeTypes, - nodes: [ - { - id: uuid(), - name: 'Action Network', - type: 'n8n-nodes-base.actionNetwork', - parameters: {}, - typeVersion: 1, - position: [0, 0] as [number, number], - credentials: { - actionNetworkApi: { - id: memberCred.id, - name: memberCred.name, - }, + const nodes: INode[] = [ + { + id: uuid(), + name: 'Action Network', + type: 'n8n-nodes-base.actionNetwork', + parameters: {}, + typeVersion: 1, + position: [0, 0] as [number, number], + credentials: { + actionNetworkApi: { + id: memberCred.id, + name: memberCred.name, }, }, - { - id: uuid(), - name: 'Action Network 2', - type: 'n8n-nodes-base.actionNetwork', - parameters: {}, - typeVersion: 1, - position: [0, 0] as [number, number], - credentials: { - actionNetworkApi: { - id: 'non-existing-credential-id', - name: 'Non-existing credential name', - }, + }, + { + id: uuid(), + name: 'Action Network 2', + type: 'n8n-nodes-base.actionNetwork', + parameters: {}, + typeVersion: 1, + position: [0, 0] as [number, number], + credentials: { + actionNetworkApi: { + id: 'non-existing-credential-id', + name: 'Non-existing credential name', }, }, - ], - }; - - const workflowEntity = await Container.get(WorkflowRepository).save(workflowDetails); + }, + ]; - await Container.get(SharedWorkflowRepository).save({ - workflow: workflowEntity, - user: member, - role: 'workflow:owner', - }); + const workflow = await createWorkflow(nodes, member); - await expect( - permissionChecker.check(workflowDetails.id, member.id, workflowDetails.nodes), - ).rejects.toThrow(); + await expect(permissionChecker.check(workflow.id, member.id, workflow.nodes)).rejects.toThrow(); }); }); describe('checkSubworkflowExecutePolicy()', () => { const ownershipService = mockInstance(OwnershipService); - let license: LicenseMocker; - - beforeAll(() => { - license = new LicenseMocker(); - license.mock(Container.get(License)); - license.enable('feat:sharing'); - }); - describe('no caller policy', () => { test('should fall back to N8N_WORKFLOW_CALLER_POLICY_DEFAULT_OPTION', async () => { config.set('workflows.callerPolicyDefaultOption', 'none'); diff --git a/packages/cli/test/integration/auth.api.test.ts b/packages/cli/test/integration/auth.api.test.ts index 768acc467d4c5..9435fa7a7dfcf 100644 --- a/packages/cli/test/integration/auth.api.test.ts +++ b/packages/cli/test/integration/auth.api.test.ts @@ -157,18 +157,6 @@ describe('GET /login', () => { expect(authToken).toBeUndefined(); }); - test('should return cookie if UM is disabled and no cookie is already set', async () => { - await createUserShell('global:owner'); - await utils.setInstanceOwnerSetUp(false); - - const response = await testServer.authlessAgent.get('/login'); - - expect(response.statusCode).toBe(200); - - const authToken = utils.getAuthToken(response); - expect(authToken).toBeDefined(); - }); - test('should return 401 Unauthorized if invalid cookie', async () => { testServer.authlessAgent.jar.setCookie(`${AUTH_COOKIE_NAME}=invalid`); diff --git a/packages/cli/test/integration/auth.mw.test.ts b/packages/cli/test/integration/auth.mw.test.ts index 7a117e95be4e4..8f40759f965c6 100644 --- a/packages/cli/test/integration/auth.mw.test.ts +++ b/packages/cli/test/integration/auth.mw.test.ts @@ -17,15 +17,12 @@ describe('Auth Middleware', () => { ['PATCH', '/me'], ['PATCH', '/me/password'], ['POST', '/me/survey'], - ['POST', '/owner/setup'], - ['GET', '/non-existent'], ]; /** Routes requiring a valid `n8n-auth` cookie for an owner. */ const ROUTES_REQUIRING_AUTHORIZATION: Readonly> = [ ['POST', '/invitations'], ['DELETE', '/users/123'], - ['POST', '/owner/setup'], ]; describe('Routes requiring Authentication', () => { diff --git a/packages/cli/test/integration/ldap/ldap.api.test.ts b/packages/cli/test/integration/ldap/ldap.api.test.ts index f3a923d4bd85e..cc28d7b748a8e 100644 --- a/packages/cli/test/integration/ldap/ldap.api.test.ts +++ b/packages/cli/test/integration/ldap/ldap.api.test.ts @@ -453,7 +453,7 @@ describe('POST /ldap/sync', () => { await authOwnerAgent.post('/ldap/sync').send({ type: 'live' }); const response = await testServer.authAgentFor(member).get('/login'); - expect(response.body.code).toBe(401); + expect(response.status).toBe(401); }); }); }); diff --git a/packages/cli/test/integration/me.api.test.ts b/packages/cli/test/integration/me.api.test.ts index 08261b7b08297..53ee82343029f 100644 --- a/packages/cli/test/integration/me.api.test.ts +++ b/packages/cli/test/integration/me.api.test.ts @@ -300,10 +300,6 @@ describe('Member', () => { }); describe('Owner', () => { - beforeEach(async () => { - await utils.setInstanceOwnerSetUp(true); - }); - test('PATCH /me should succeed with valid inputs', async () => { const owner = await createUser({ role: 'global:owner' }); const authOwnerAgent = testServer.authAgentFor(owner); diff --git a/packages/cli/test/integration/mfa/mfa.api.test.ts b/packages/cli/test/integration/mfa/mfa.api.test.ts index 2e7df144ca734..d66261f60e39f 100644 --- a/packages/cli/test/integration/mfa/mfa.api.test.ts +++ b/packages/cli/test/integration/mfa/mfa.api.test.ts @@ -1,14 +1,16 @@ import Container from 'typedi'; + +import { AuthService } from '@/auth/auth.service'; import config from '@/config'; import type { User } from '@db/entities/User'; +import { UserRepository } from '@db/repositories/user.repository'; import { randomPassword } from '@/Ldap/helpers'; import { TOTPService } from '@/Mfa/totp.service'; -import { UserService } from '@/services/user.service'; -import { randomDigit, randomString, randomValidPassword, uniqueId } from '../shared/random'; + import * as testDb from '../shared/testDb'; import * as utils from '../shared/utils'; +import { randomDigit, randomString, randomValidPassword, uniqueId } from '../shared/random'; import { createUser, createUserWithMfaEnabled } from '../shared/db/users'; -import { UserRepository } from '@db/repositories/user.repository'; jest.mock('@/telemetry'); @@ -241,7 +243,7 @@ describe('Change password with MFA enabled', () => { config.set('userManagement.jwtSecret', randomString(5, 10)); - const resetPasswordToken = Container.get(UserService).generatePasswordResetToken(user); + const resetPasswordToken = Container.get(AuthService).generatePasswordResetToken(user); const mfaToken = new TOTPService().generateTOTP(rawSecret); diff --git a/packages/cli/test/integration/owner.api.test.ts b/packages/cli/test/integration/owner.api.test.ts index cbe8a84c915d8..ff5dffe854757 100644 --- a/packages/cli/test/integration/owner.api.test.ts +++ b/packages/cli/test/integration/owner.api.test.ts @@ -1,5 +1,4 @@ import validator from 'validator'; -import type { SuperAgentTest } from 'supertest'; import config from '@/config'; import type { User } from '@db/entities/User'; @@ -18,11 +17,9 @@ import Container from 'typedi'; const testServer = utils.setupTestServer({ endpointGroups: ['owner'] }); let ownerShell: User; -let authOwnerShellAgent: SuperAgentTest; beforeEach(async () => { ownerShell = await createUserShell('global:owner'); - authOwnerShellAgent = testServer.authAgentFor(ownerShell); config.set('userManagement.isInstanceOwnerSetUp', false); }); @@ -39,7 +36,7 @@ describe('POST /owner/setup', () => { password: randomValidPassword(), }; - const response = await authOwnerShellAgent.post('/owner/setup').send(newOwnerData); + const response = await testServer.authlessAgent.post('/owner/setup').send(newOwnerData); expect(response.statusCode).toBe(200); @@ -88,7 +85,7 @@ describe('POST /owner/setup', () => { password: randomValidPassword(), }; - const response = await authOwnerShellAgent.post('/owner/setup').send(newOwnerData); + const response = await testServer.authlessAgent.post('/owner/setup').send(newOwnerData); expect(response.statusCode).toBe(200); @@ -150,7 +147,7 @@ describe('POST /owner/setup', () => { test('should fail with invalid inputs', async () => { for (const invalidPayload of INVALID_POST_OWNER_PAYLOADS) { - const response = await authOwnerShellAgent.post('/owner/setup').send(invalidPayload); + const response = await testServer.authlessAgent.post('/owner/setup').send(invalidPayload); expect(response.statusCode).toBe(400); } }); diff --git a/packages/cli/test/integration/passwordReset.api.test.ts b/packages/cli/test/integration/passwordReset.api.test.ts index 996fc0ab77f26..6aca848e84d71 100644 --- a/packages/cli/test/integration/passwordReset.api.test.ts +++ b/packages/cli/test/integration/passwordReset.api.test.ts @@ -3,13 +3,13 @@ import { compare } from 'bcryptjs'; import { Container } from 'typedi'; import { mock } from 'jest-mock-extended'; +import { AuthService } from '@/auth/auth.service'; import { License } from '@/License'; import config from '@/config'; import type { User } from '@db/entities/User'; import { setCurrentAuthenticationMethod } from '@/sso/ssoHelpers'; import { ExternalHooks } from '@/ExternalHooks'; import { JwtService } from '@/services/jwt.service'; -import { UserService } from '@/services/user.service'; import { UserManagementMailer } from '@/UserManagement/email'; import { UserRepository } from '@db/repositories/user.repository'; @@ -35,7 +35,7 @@ const externalHooks = mockInstance(ExternalHooks); const mailer = mockInstance(UserManagementMailer, { isEmailSetUp: true }); const testServer = setupTestServer({ endpointGroups: ['passwordReset'] }); const jwtService = Container.get(JwtService); -let userService: UserService; +let authService: AuthService; beforeEach(async () => { await testDb.truncate(['User']); @@ -43,7 +43,7 @@ beforeEach(async () => { member = await createUser({ role: 'global:member' }); externalHooks.run.mockReset(); jest.replaceProperty(mailer, 'isEmailSetUp', true); - userService = Container.get(UserService); + authService = Container.get(AuthService); }); describe('POST /forgot-password', () => { @@ -126,7 +126,7 @@ describe('POST /forgot-password', () => { describe('GET /resolve-password-token', () => { test('should succeed with valid inputs', async () => { - const resetPasswordToken = userService.generatePasswordResetToken(owner); + const resetPasswordToken = authService.generatePasswordResetToken(owner); const response = await testServer.authlessAgent .get('/resolve-password-token') @@ -158,7 +158,7 @@ describe('GET /resolve-password-token', () => { }); test('should fail if token is expired', async () => { - const resetPasswordToken = userService.generatePasswordResetToken(owner, '-1h'); + const resetPasswordToken = authService.generatePasswordResetToken(owner, '-1h'); const response = await testServer.authlessAgent .get('/resolve-password-token') @@ -169,7 +169,7 @@ describe('GET /resolve-password-token', () => { test('should fail after password has changed', async () => { const updatedUser = mock({ ...owner, password: 'another-password' }); - const resetPasswordToken = userService.generatePasswordResetToken(updatedUser); + const resetPasswordToken = authService.generatePasswordResetToken(updatedUser); const response = await testServer.authlessAgent .get('/resolve-password-token') @@ -183,7 +183,7 @@ describe('POST /change-password', () => { const passwordToStore = randomValidPassword(); test('should succeed with valid inputs', async () => { - const resetPasswordToken = userService.generatePasswordResetToken(owner); + const resetPasswordToken = authService.generatePasswordResetToken(owner); const response = await testServer.authlessAgent.post('/change-password').send({ token: resetPasswordToken, userId: owner.id, @@ -213,7 +213,7 @@ describe('POST /change-password', () => { }); test('should fail with invalid inputs', async () => { - const resetPasswordToken = userService.generatePasswordResetToken(owner); + const resetPasswordToken = authService.generatePasswordResetToken(owner); const invalidPayloads = [ { token: uuid() }, @@ -247,7 +247,7 @@ describe('POST /change-password', () => { }); test('should fail when token has expired', async () => { - const resetPasswordToken = userService.generatePasswordResetToken(owner, '-1h'); + const resetPasswordToken = authService.generatePasswordResetToken(owner, '-1h'); const response = await testServer.authlessAgent.post('/change-password').send({ token: resetPasswordToken, @@ -263,7 +263,7 @@ describe('POST /change-password', () => { test('owner should be able to reset its password when quota:users = 1', async () => { jest.spyOn(Container.get(License), 'getUsersLimit').mockReturnValueOnce(1); - const resetPasswordToken = userService.generatePasswordResetToken(owner); + const resetPasswordToken = authService.generatePasswordResetToken(owner); const response = await testServer.authlessAgent.post('/change-password').send({ token: resetPasswordToken, userId: owner.id, @@ -292,7 +292,7 @@ describe('POST /change-password', () => { test('member should not be able to reset its password when quota:users = 1', async () => { jest.spyOn(Container.get(License), 'getUsersLimit').mockReturnValueOnce(1); - const resetPasswordToken = userService.generatePasswordResetToken(member); + const resetPasswordToken = authService.generatePasswordResetToken(member); const response = await testServer.authlessAgent.post('/change-password').send({ token: resetPasswordToken, userId: member.id, diff --git a/packages/cli/test/integration/shared/constants.ts b/packages/cli/test/integration/shared/constants.ts index 5fc7149f8c6b7..7b32134cabde5 100644 --- a/packages/cli/test/integration/shared/constants.ts +++ b/packages/cli/test/integration/shared/constants.ts @@ -4,14 +4,6 @@ export const REST_PATH_SEGMENT = config.getEnv('endpoints.rest'); export const PUBLIC_API_REST_PATH_SEGMENT = config.getEnv('publicApi.path'); -export const AUTHLESS_ENDPOINTS: Readonly = [ - 'healthz', - 'metrics', - config.getEnv('endpoints.webhook'), - config.getEnv('endpoints.webhookWaiting'), - config.getEnv('endpoints.webhookTest'), -]; - export const SUCCESS_RESPONSE_BODY = { data: { success: true, diff --git a/packages/cli/test/integration/shared/types.ts b/packages/cli/test/integration/shared/types.ts index 6393e7914325d..2482beb95c3b4 100644 --- a/packages/cli/test/integration/shared/types.ts +++ b/packages/cli/test/integration/shared/types.ts @@ -35,7 +35,6 @@ type EndpointGroup = | 'debug'; export interface SetupProps { - applyAuth?: boolean; endpointGroups?: EndpointGroup[]; enabledFeatures?: BooleanLicenseFeature[]; quotas?: Partial<{ [K in NumericLicenseFeature]: number }>; diff --git a/packages/cli/test/integration/shared/utils/testServer.ts b/packages/cli/test/integration/shared/utils/testServer.ts index d791d7dc895e7..829fd0214cc99 100644 --- a/packages/cli/test/integration/shared/utils/testServer.ts +++ b/packages/cli/test/integration/shared/utils/testServer.ts @@ -8,9 +8,8 @@ import { URL } from 'url'; import config from '@/config'; import { AUTH_COOKIE_NAME } from '@/constants'; import type { User } from '@db/entities/User'; -import { issueJWT } from '@/auth/jwt'; import { registerController } from '@/decorators'; -import { rawBodyReader, bodyParser, setupAuthMiddlewares } from '@/middlewares'; +import { rawBodyReader, bodyParser } from '@/middlewares'; import { PostHogClient } from '@/posthog'; import { Push } from '@/push'; import { License } from '@/License'; @@ -19,9 +18,10 @@ import { InternalHooks } from '@/InternalHooks'; import { mockInstance } from '../../../shared/mocking'; import * as testDb from '../../shared/testDb'; -import { AUTHLESS_ENDPOINTS, PUBLIC_API_REST_PATH_SEGMENT, REST_PATH_SEGMENT } from '../constants'; +import { PUBLIC_API_REST_PATH_SEGMENT, REST_PATH_SEGMENT } from '../constants'; import type { SetupProps, TestServer } from '../types'; import { LicenseMocker } from '../license'; +import { AuthService } from '@/auth/auth.service'; /** * Plugin to prefix a path segment into a request URL pathname. @@ -47,7 +47,7 @@ function createAgent(app: express.Application, options?: { auth: boolean; user: const agent = request.agent(app); void agent.use(prefix(REST_PATH_SEGMENT)); if (options?.auth && options?.user) { - const { token } = issueJWT(options.user); + const token = Container.get(AuthService).issueJWT(options.user); agent.jar.setCookie(`${AUTH_COOKIE_NAME}=${token}`); } return agent; @@ -67,7 +67,6 @@ function publicApiAgent( export const setupTestServer = ({ endpointGroups, - applyAuth = true, enabledFeatures, quotas, }: SetupProps): TestServer => { @@ -104,15 +103,11 @@ export const setupTestServer = ({ }); } - const enablePublicAPI = endpointGroups?.includes('publicApi'); - if (applyAuth && !enablePublicAPI) { - setupAuthMiddlewares(app, AUTHLESS_ENDPOINTS, REST_PATH_SEGMENT); - } - if (!endpointGroups) return; app.use(bodyParser); + const enablePublicAPI = endpointGroups?.includes('publicApi'); if (enablePublicAPI) { const { loadPublicApiVersions } = await import('@/PublicApi'); const { apiRouters } = await loadPublicApiVersions(PUBLIC_API_REST_PATH_SEGMENT); diff --git a/packages/cli/test/integration/workflows/workflow.service.ee.test.ts b/packages/cli/test/integration/workflows/workflow.service.ee.test.ts index 4cb823b7c0663..4504775f2e58b 100644 --- a/packages/cli/test/integration/workflows/workflow.service.ee.test.ts +++ b/packages/cli/test/integration/workflows/workflow.service.ee.test.ts @@ -28,6 +28,7 @@ describe('EnterpriseWorkflowService', () => { Container.get(SharedWorkflowRepository), Container.get(WorkflowRepository), Container.get(CredentialsRepository), + mock(), ); }); diff --git a/packages/cli/test/unit/UserManagementMailer.test.ts b/packages/cli/test/unit/UserManagementMailer.test.ts index 8e9de8d3a93ee..10daf984c9a2c 100644 --- a/packages/cli/test/unit/UserManagementMailer.test.ts +++ b/packages/cli/test/unit/UserManagementMailer.test.ts @@ -1,6 +1,7 @@ import config from '@/config'; import { NodeMailer } from '@/UserManagement/email/NodeMailer'; import { UserManagementMailer } from '@/UserManagement/email/UserManagementMailer'; +import { mock } from 'jest-mock-extended'; describe('UserManagementMailer', () => { describe('expect NodeMailer.verifyConnection', () => { @@ -20,7 +21,7 @@ describe('UserManagementMailer', () => { }); test('not be called when SMTP not set up', async () => { - const userManagementMailer = new UserManagementMailer(); + const userManagementMailer = new UserManagementMailer(mock(), mock(), mock()); // NodeMailer.verifyConnection gets called only explicitly await expect(async () => await userManagementMailer.verifyConnection()).rejects.toThrow(); @@ -32,7 +33,7 @@ describe('UserManagementMailer', () => { config.set('userManagement.emails.smtp.host', 'host'); config.set('userManagement.emails.mode', 'smtp'); - const userManagementMailer = new UserManagementMailer(); + const userManagementMailer = new UserManagementMailer(mock(), mock(), mock()); // NodeMailer.verifyConnection gets called only explicitly expect(async () => await userManagementMailer.verifyConnection()).not.toThrow(); }); diff --git a/packages/cli/test/unit/auth/auth.service.test.ts b/packages/cli/test/unit/auth/auth.service.test.ts new file mode 100644 index 0000000000000..9c9e41e061a84 --- /dev/null +++ b/packages/cli/test/unit/auth/auth.service.test.ts @@ -0,0 +1,276 @@ +import jwt from 'jsonwebtoken'; +import { mock } from 'jest-mock-extended'; +import { type NextFunction, type Response } from 'express'; + +import { AuthService } from '@/auth/auth.service'; +import config from '@/config'; +import { AUTH_COOKIE_NAME, Time } from '@/constants'; +import type { User } from '@db/entities/User'; +import type { UserRepository } from '@db/repositories/user.repository'; +import { JwtService } from '@/services/jwt.service'; +import type { UrlService } from '@/services/url.service'; +import type { AuthenticatedRequest } from '@/requests'; + +describe('AuthService', () => { + config.set('userManagement.jwtSecret', 'random-secret'); + + const userData = { + id: '123', + email: 'test@example.com', + password: 'passwordHash', + disabled: false, + mfaEnabled: false, + }; + const validToken = + 'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpZCI6IjEyMyIsImhhc2giOiJtSkFZeDRXYjdrIiwiaWF0IjoxNzA2NzUwNjI1LCJleHAiOjE3MDczNTU0MjV9.JwY3doH0YrxHdX4nTOlTN4-QMaXsAu5OFOaFcIHSHBI'; + + const user = mock(userData); + const jwtService = new JwtService(mock()); + const urlService = mock(); + const userRepository = mock(); + const authService = new AuthService(mock(), mock(), jwtService, urlService, userRepository); + + jest.useFakeTimers(); + const now = new Date('2024-02-01T01:23:45.678Z'); + beforeEach(() => { + jest.clearAllMocks(); + jest.setSystemTime(now); + config.set('userManagement.jwtSessionDurationHours', 168); + config.set('userManagement.jwtRefreshTimeoutHours', 0); + }); + + describe('createJWTHash', () => { + it('should generate unique hashes', () => { + expect(authService.createJWTHash(user)).toEqual('mJAYx4Wb7k'); + expect( + authService.createJWTHash(mock({ email: user.email, password: 'newPasswordHash' })), + ).toEqual('FVALtU7AE0'); + expect( + authService.createJWTHash( + mock({ email: 'test1@example.com', password: user.password }), + ), + ).toEqual('y8ha6X01jd'); + }); + }); + + describe('authMiddleware', () => { + const req = mock({ cookies: {}, user: undefined }); + const res = mock(); + const next = jest.fn() as NextFunction; + + beforeEach(() => { + res.status.mockReturnThis(); + }); + + it('should 401 if no cookie is set', async () => { + req.cookies[AUTH_COOKIE_NAME] = undefined; + await authService.authMiddleware(req, res, next); + expect(next).not.toHaveBeenCalled(); + expect(res.status).toHaveBeenCalledWith(401); + }); + + it('should 401 and clear the cookie if the JWT is expired', async () => { + req.cookies[AUTH_COOKIE_NAME] = validToken; + jest.advanceTimersByTime(365 * Time.days.toMilliseconds); + + await authService.authMiddleware(req, res, next); + expect(next).not.toHaveBeenCalled(); + expect(res.status).toHaveBeenCalledWith(401); + expect(res.clearCookie).toHaveBeenCalledWith(AUTH_COOKIE_NAME); + }); + + it('should refresh the cookie before it expires', async () => { + req.cookies[AUTH_COOKIE_NAME] = validToken; + jest.advanceTimersByTime(6 * Time.days.toMilliseconds); + userRepository.findOne.mockResolvedValue(user); + + await authService.authMiddleware(req, res, next); + expect(next).toHaveBeenCalled(); + expect(res.cookie).toHaveBeenCalledWith('n8n-auth', expect.any(String), { + httpOnly: true, + maxAge: 604800000, + sameSite: 'lax', + secure: false, + }); + }); + }); + + describe('issueJWT', () => { + describe('when not setting userManagement.jwtSessionDuration', () => { + it('should default to expire in 7 days', () => { + const defaultInSeconds = 7 * Time.days.toSeconds; + const token = authService.issueJWT(user); + + expect(authService.jwtExpiration).toBe(defaultInSeconds); + const decodedToken = jwtService.verify(token); + if (decodedToken.exp === undefined || decodedToken.iat === undefined) { + fail('Expected exp and iat to be defined'); + } + + expect(decodedToken.exp - decodedToken.iat).toBe(defaultInSeconds); + }); + }); + + describe('when setting userManagement.jwtSessionDuration', () => { + const testDurationHours = 1; + const testDurationSeconds = testDurationHours * Time.hours.toSeconds; + + it('should apply it to tokens', () => { + config.set('userManagement.jwtSessionDurationHours', testDurationHours); + const token = authService.issueJWT(user); + + const decodedToken = jwtService.verify(token); + if (decodedToken.exp === undefined || decodedToken.iat === undefined) { + fail('Expected exp and iat to be defined on decodedToken'); + } + expect(decodedToken.exp - decodedToken.iat).toBe(testDurationSeconds); + }); + }); + }); + + describe('resolveJwt', () => { + const res = mock(); + + it('should throw on invalid tokens', async () => { + await expect(authService.resolveJwt('random-string', res)).rejects.toThrow('jwt malformed'); + expect(res.cookie).not.toHaveBeenCalled(); + }); + + it('should throw on expired tokens', async () => { + jest.advanceTimersByTime(365 * Time.days.toMilliseconds); + + await expect(authService.resolveJwt(validToken, res)).rejects.toThrow('jwt expired'); + expect(res.cookie).not.toHaveBeenCalled(); + }); + + it('should throw on tampered tokens', async () => { + const [header, payload, signature] = validToken.split('.'); + const tamperedToken = [header, payload, signature + '123'].join('.'); + await expect(authService.resolveJwt(tamperedToken, res)).rejects.toThrow('invalid signature'); + expect(res.cookie).not.toHaveBeenCalled(); + }); + + test.each([ + ['no user is found', null], + ['the user is disabled', { ...userData, disabled: true }], + [ + 'user password does not match the one on the token', + { ...userData, password: 'something else' }, + ], + [ + 'user email does not match the one on the token', + { ...userData, email: 'someone@example.com' }, + ], + ])('should throw if %s', async (_, data) => { + userRepository.findOne.mockResolvedValueOnce(data && mock(data)); + await expect(authService.resolveJwt(validToken, res)).rejects.toThrow('Unauthorized'); + expect(res.cookie).not.toHaveBeenCalled(); + }); + + it('should refresh the cookie before it expires', async () => { + userRepository.findOne.mockResolvedValue(user); + expect(await authService.resolveJwt(validToken, res)).toEqual(user); + expect(res.cookie).not.toHaveBeenCalled(); + + jest.advanceTimersByTime(6 * Time.days.toMilliseconds); // 6 Days + expect(await authService.resolveJwt(validToken, res)).toEqual(user); + expect(res.cookie).toHaveBeenCalledWith('n8n-auth', expect.any(String), { + httpOnly: true, + maxAge: 604800000, + sameSite: 'lax', + secure: false, + }); + }); + + it('should refresh the cookie only if less than 1/4th of time is left', async () => { + userRepository.findOne.mockResolvedValue(user); + expect(await authService.resolveJwt(validToken, res)).toEqual(user); + expect(res.cookie).not.toHaveBeenCalled(); + + jest.advanceTimersByTime(5 * Time.days.toMilliseconds); + expect(await authService.resolveJwt(validToken, res)).toEqual(user); + expect(res.cookie).not.toHaveBeenCalled(); + + jest.advanceTimersByTime(1 * Time.days.toMilliseconds); + expect(await authService.resolveJwt(validToken, res)).toEqual(user); + expect(res.cookie).toHaveBeenCalled(); + }); + + it('should not refresh the cookie if jwtRefreshTimeoutHours is set to -1', async () => { + config.set('userManagement.jwtRefreshTimeoutHours', -1); + + userRepository.findOne.mockResolvedValue(user); + expect(await authService.resolveJwt(validToken, res)).toEqual(user); + expect(res.cookie).not.toHaveBeenCalled(); + + jest.advanceTimersByTime(6 * Time.days.toMilliseconds); // 6 Days + expect(await authService.resolveJwt(validToken, res)).toEqual(user); + expect(res.cookie).not.toHaveBeenCalled(); + }); + }); + + describe('generatePasswordResetUrl', () => { + it('should generate a valid url', () => { + urlService.getInstanceBaseUrl.mockReturnValue('https://n8n.instance'); + const url = authService.generatePasswordResetUrl(user); + expect(url).toEqual( + 'https://n8n.instance/change-password?token=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjMiLCJoYXNoIjoibUpBWXg0V2I3ayIsImlhdCI6MTcwNjc1MDYyNSwiZXhwIjoxNzA2NzUxODI1fQ.rg90I7MKjc_KC77mov59XYAeRc-CoW9ka4mt1dCfrnk&mfaEnabled=false', + ); + }); + }); + + describe('generatePasswordResetToken', () => { + it('should generate valid password-reset tokens', () => { + const token = authService.generatePasswordResetToken(user); + + const decoded = jwt.decode(token) as jwt.JwtPayload; + + if (!decoded.exp) fail('Token does not contain expiry'); + if (!decoded.iat) fail('Token does not contain issued-at'); + + expect(decoded.sub).toEqual(user.id); + expect(decoded.exp - decoded.iat).toEqual(1200); // Expires in 20 minutes + expect(decoded.hash).toEqual('mJAYx4Wb7k'); + }); + }); + + describe('resolvePasswordResetToken', () => { + it('should not return a user if the token in invalid', async () => { + const resolvedUser = await authService.resolvePasswordResetToken('invalid-token'); + expect(resolvedUser).toBeUndefined(); + }); + + it('should not return a user if the token in expired', async () => { + const token = authService.generatePasswordResetToken(user, '-1h'); + + const resolvedUser = await authService.resolvePasswordResetToken(token); + expect(resolvedUser).toBeUndefined(); + }); + + it('should not return a user if the user does not exist in the DB', async () => { + userRepository.findOne.mockResolvedValueOnce(null); + const token = authService.generatePasswordResetToken(user); + + const resolvedUser = await authService.resolvePasswordResetToken(token); + expect(resolvedUser).toBeUndefined(); + }); + + it('should not return a user if the password sha does not match', async () => { + const token = authService.generatePasswordResetToken(user); + const updatedUser = Object.create(user); + updatedUser.password = 'something-else'; + userRepository.findOne.mockResolvedValueOnce(updatedUser); + + const resolvedUser = await authService.resolvePasswordResetToken(token); + expect(resolvedUser).toBeUndefined(); + }); + + it('should not return the user if all checks pass', async () => { + const token = authService.generatePasswordResetToken(user); + userRepository.findOne.mockResolvedValueOnce(user); + + const resolvedUser = await authService.resolvePasswordResetToken(token); + expect(resolvedUser).toEqual(user); + }); + }); +}); diff --git a/packages/cli/test/unit/auth/jwt.test.ts b/packages/cli/test/unit/auth/jwt.test.ts deleted file mode 100644 index dd9c65116ab77..0000000000000 --- a/packages/cli/test/unit/auth/jwt.test.ts +++ /dev/null @@ -1,61 +0,0 @@ -import { Container } from 'typedi'; -import { mock } from 'jest-mock-extended'; - -import config from '@/config'; -import { JwtService } from '@/services/jwt.service'; -import { License } from '@/License'; -import { Time } from '@/constants'; -import { issueJWT } from '@/auth/jwt'; - -import { mockInstance } from '../../shared/mocking'; - -import type { User } from '@db/entities/User'; - -mockInstance(License); - -describe('jwt.issueJWT', () => { - const jwtService = Container.get(JwtService); - - describe('when not setting userManagement.jwtSessionDuration', () => { - it('should default to expire in 7 days', () => { - const defaultInSeconds = 7 * Time.days.toSeconds; - const mockUser = mock({ password: 'passwordHash' }); - const { token, expiresIn } = issueJWT(mockUser); - - expect(expiresIn).toBe(defaultInSeconds); - const decodedToken = jwtService.verify(token); - if (decodedToken.exp === undefined || decodedToken.iat === undefined) { - fail('Expected exp and iat to be defined'); - } - - expect(decodedToken.exp - decodedToken.iat).toBe(defaultInSeconds); - }); - }); - - describe('when setting userManagement.jwtSessionDuration', () => { - const oldDuration = config.get('userManagement.jwtSessionDurationHours'); - const testDurationHours = 1; - const testDurationSeconds = testDurationHours * Time.hours.toSeconds; - - beforeEach(() => { - mockInstance(License); - config.set('userManagement.jwtSessionDurationHours', testDurationHours); - }); - - afterEach(() => { - config.set('userManagement.jwtSessionDuration', oldDuration); - }); - - it('should apply it to tokens', () => { - const mockUser = mock({ password: 'passwordHash' }); - const { token, expiresIn } = issueJWT(mockUser); - - expect(expiresIn).toBe(testDurationSeconds); - const decodedToken = jwtService.verify(token); - if (decodedToken.exp === undefined || decodedToken.iat === undefined) { - fail('Expected exp and iat to be defined on decodedToken'); - } - expect(decodedToken.exp - decodedToken.iat).toBe(testDurationSeconds); - }); - }); -}); diff --git a/packages/cli/test/unit/controllers/me.controller.test.ts b/packages/cli/test/unit/controllers/me.controller.test.ts index 0ebca0bd77e16..f0f9b8458623a 100644 --- a/packages/cli/test/unit/controllers/me.controller.test.ts +++ b/packages/cli/test/unit/controllers/me.controller.test.ts @@ -1,7 +1,7 @@ -import type { CookieOptions, Response } from 'express'; +import type { Response } from 'express'; import { Container } from 'typedi'; import jwt from 'jsonwebtoken'; -import { mock, anyObject, captor } from 'jest-mock-extended'; +import { mock, anyObject } from 'jest-mock-extended'; import type { PublicUser } from '@/Interfaces'; import type { User } from '@db/entities/User'; import { MeController } from '@/controllers/me.controller'; @@ -11,10 +11,10 @@ import { UserService } from '@/services/user.service'; import { ExternalHooks } from '@/ExternalHooks'; import { InternalHooks } from '@/InternalHooks'; import { License } from '@/License'; -import { badPasswords } from '../shared/testData'; -import { mockInstance } from '../../shared/mocking'; import { BadRequestError } from '@/errors/response-errors/bad-request.error'; import { UserRepository } from '@/databases/repositories/user.repository'; +import { badPasswords } from '../shared/testData'; +import { mockInstance } from '../../shared/mocking'; describe('MeController', () => { const externalHooks = mockInstance(ExternalHooks); @@ -63,10 +63,16 @@ describe('MeController', () => { expect(userService.update).toHaveBeenCalled(); - const cookieOptions = captor(); - expect(res.cookie).toHaveBeenCalledWith(AUTH_COOKIE_NAME, 'signed-token', cookieOptions); - expect(cookieOptions.value.httpOnly).toBe(true); - expect(cookieOptions.value.sameSite).toBe('lax'); + expect(res.cookie).toHaveBeenCalledWith( + AUTH_COOKIE_NAME, + 'signed-token', + expect.objectContaining({ + maxAge: expect.any(Number), + httpOnly: true, + sameSite: 'lax', + secure: false, + }), + ); expect(externalHooks.run).toHaveBeenCalledWith('user.profile.update', [ user.email, @@ -175,10 +181,16 @@ describe('MeController', () => { expect(req.user.password).not.toBe(passwordHash); - const cookieOptions = captor(); - expect(res.cookie).toHaveBeenCalledWith(AUTH_COOKIE_NAME, 'new-signed-token', cookieOptions); - expect(cookieOptions.value.httpOnly).toBe(true); - expect(cookieOptions.value.sameSite).toBe('lax'); + expect(res.cookie).toHaveBeenCalledWith( + AUTH_COOKIE_NAME, + 'new-signed-token', + expect.objectContaining({ + maxAge: expect.any(Number), + httpOnly: true, + sameSite: 'lax', + secure: false, + }), + ); expect(externalHooks.run).toHaveBeenCalledWith('user.password.update', [ req.user.email, diff --git a/packages/cli/test/unit/controllers/owner.controller.test.ts b/packages/cli/test/unit/controllers/owner.controller.test.ts index f142e16b99b92..50917a2107d26 100644 --- a/packages/cli/test/unit/controllers/owner.controller.test.ts +++ b/packages/cli/test/unit/controllers/owner.controller.test.ts @@ -1,34 +1,37 @@ -import type { CookieOptions, Response } from 'express'; -import { anyObject, captor, mock } from 'jest-mock-extended'; +import Container from 'typedi'; +import type { Response } from 'express'; +import { anyObject, mock } from 'jest-mock-extended'; import jwt from 'jsonwebtoken'; -import type { User } from '@db/entities/User'; -import type { SettingsRepository } from '@db/repositories/settings.repository'; + +import type { AuthService } from '@/auth/auth.service'; import config from '@/config'; -import type { OwnerRequest } from '@/requests'; import { OwnerController } from '@/controllers/owner.controller'; -import { AUTH_COOKIE_NAME } from '@/constants'; -import { UserService } from '@/services/user.service'; +import type { User } from '@db/entities/User'; +import type { SettingsRepository } from '@db/repositories/settings.repository'; +import type { UserRepository } from '@db/repositories/user.repository'; +import { BadRequestError } from '@/errors/response-errors/bad-request.error'; +import type { InternalHooks } from '@/InternalHooks'; import { License } from '@/License'; +import type { OwnerRequest } from '@/requests'; +import type { UserService } from '@/services/user.service'; +import { PasswordUtility } from '@/services/password.utility'; import { mockInstance } from '../../shared/mocking'; import { badPasswords } from '../shared/testData'; -import { BadRequestError } from '@/errors/response-errors/bad-request.error'; -import { PasswordUtility } from '@/services/password.utility'; -import Container from 'typedi'; -import type { InternalHooks } from '@/InternalHooks'; -import { UserRepository } from '@/databases/repositories/user.repository'; describe('OwnerController', () => { const configGetSpy = jest.spyOn(config, 'getEnv'); const internalHooks = mock(); - const userService = mockInstance(UserService); - const userRepository = mockInstance(UserRepository); + const authService = mock(); + const userService = mock(); + const userRepository = mock(); const settingsRepository = mock(); mockInstance(License).isWithinUsersLimit.mockReturnValue(true); const controller = new OwnerController( mock(), internalHooks, settingsRepository, + authService, userService, Container.get(PasswordUtility), mock(), @@ -90,17 +93,17 @@ describe('OwnerController', () => { }); const res = mock(); configGetSpy.mockReturnValue(false); + userRepository.findOneOrFail.calledWith(anyObject()).mockResolvedValue(user); userRepository.save.calledWith(anyObject()).mockResolvedValue(user); jest.spyOn(jwt, 'sign').mockImplementation(() => 'signed-token'); await controller.setupOwner(req, res); + expect(userRepository.findOneOrFail).toHaveBeenCalledWith({ + where: { role: 'global:owner' }, + }); expect(userRepository.save).toHaveBeenCalledWith(user, { transaction: false }); - - const cookieOptions = captor(); - expect(res.cookie).toHaveBeenCalledWith(AUTH_COOKIE_NAME, 'signed-token', cookieOptions); - expect(cookieOptions.value.httpOnly).toBe(true); - expect(cookieOptions.value.sameSite).toBe('lax'); + expect(authService.issueCookie).toHaveBeenCalledWith(res, user); }); }); }); diff --git a/packages/cli/test/unit/databases/entities/user.entity.test.ts b/packages/cli/test/unit/databases/entities/user.entity.test.ts new file mode 100644 index 0000000000000..005e45df2c957 --- /dev/null +++ b/packages/cli/test/unit/databases/entities/user.entity.test.ts @@ -0,0 +1,20 @@ +import { User } from '@db/entities/User'; + +describe('User Entity', () => { + describe('JSON.stringify', () => { + it('should not serialize sensitive data', () => { + const user = Object.assign(new User(), { + email: 'test@example.com', + firstName: 'Don', + lastName: 'Joe', + password: '123456789', + apiKey: '123', + mfaSecret: '123', + mfaRecoveryCodes: ['123'], + }); + expect(JSON.stringify(user)).toEqual( + '{"email":"test@example.com","firstName":"Don","lastName":"Joe"}', + ); + }); + }); +}); diff --git a/packages/cli/test/unit/middleware/auth.test.ts b/packages/cli/test/unit/middleware/auth.test.ts deleted file mode 100644 index dfd6e0e8c13e4..0000000000000 --- a/packages/cli/test/unit/middleware/auth.test.ts +++ /dev/null @@ -1,162 +0,0 @@ -import { mock } from 'jest-mock-extended'; - -import config from '@/config'; -import { AUTH_COOKIE_NAME, Time } from '@/constants'; -import { License } from '@/License'; -import { issueJWT } from '@/auth/jwt'; -import { refreshExpiringCookie } from '@/middlewares'; - -import { mockInstance } from '../../shared/mocking'; - -import type { AuthenticatedRequest } from '@/requests'; -import type { NextFunction, Response } from 'express'; -import type { User } from '@/databases/entities/User'; - -mockInstance(License); - -jest.useFakeTimers(); - -describe('refreshExpiringCookie', () => { - const oldDuration = config.getEnv('userManagement.jwtSessionDurationHours'); - const oldTimeout = config.getEnv('userManagement.jwtRefreshTimeoutHours'); - let mockUser: User; - - beforeEach(() => { - mockUser = mock({ password: 'passwordHash' }); - }); - - afterEach(() => { - config.set('userManagement.jwtSessionDuration', oldDuration); - config.set('userManagement.jwtRefreshTimeoutHours', oldTimeout); - }); - - it('does not do anything if the user is not authorized', async () => { - const req = mock(); - const res = mock({ cookie: jest.fn() }); - const next = jest.fn(); - - await refreshExpiringCookie(req, res, next); - - expect(next).toHaveBeenCalledTimes(1); - expect(res.cookie).not.toHaveBeenCalled(); - }); - - describe('with N8N_USER_MANAGEMENT_JWT_REFRESH_TIMEOUT_HOURS=-1', () => { - it('does not refresh the cookie, ever', async () => { - config.set('userManagement.jwtSessionDurationHours', 1); - config.set('userManagement.jwtRefreshTimeoutHours', -1); - const { token } = issueJWT(mockUser); - - jest.advanceTimersByTime(1000 * 60 * 55); /* 55 minutes */ - - const req = mock({ - cookies: { [AUTH_COOKIE_NAME]: token }, - user: mockUser, - }); - const res = mock({ cookie: jest.fn() }); - const next = jest.fn(); - await refreshExpiringCookie(req, res, next); - - expect(next).toHaveBeenCalledTimes(1); - expect(res.cookie).not.toHaveBeenCalled(); - }); - }); - - describe('with N8N_USER_MANAGEMENT_JWT_REFRESH_TIMEOUT_HOURS=0', () => { - let token: string; - let req: AuthenticatedRequest; - let res: Response; - let next: NextFunction; - - beforeEach(() => { - // ARRANGE - config.set('userManagement.jwtSessionDurationHours', 1); - config.set('userManagement.jwtRefreshTimeoutHours', 0); - token = issueJWT(mockUser).token; - - req = mock({ - cookies: { [AUTH_COOKIE_NAME]: token }, - user: mockUser, - }); - res = mock({ cookie: jest.fn() }); - next = jest.fn(); - }); - - it('does not refresh the cookie when more than 1/4th of time is left', async () => { - // ARRANGE - jest.advanceTimersByTime(44 * Time.minutes.toMilliseconds); /* 44 minutes */ - - // ACT - await refreshExpiringCookie(req, res, next); - - // ASSERT - expect(next).toHaveBeenCalledTimes(1); - expect(res.cookie).not.toHaveBeenCalled(); - }); - - it('refreshes the cookie when 1/4th of time is left', async () => { - // ARRANGE - jest.advanceTimersByTime(46 * Time.minutes.toMilliseconds); /* 46 minutes */ - - // ACT - await refreshExpiringCookie(req, res, next); - - // ASSERT - expect(next).toHaveBeenCalledTimes(1); - expect(res.cookie).toHaveBeenCalledTimes(1); - }); - }); - - describe('with N8N_USER_MANAGEMENT_JWT_REFRESH_TIMEOUT_HOURS=50', () => { - const jwtSessionDurationHours = 51; - let token: string; - let req: AuthenticatedRequest; - let res: Response; - let next: NextFunction; - - // ARRANGE - beforeEach(() => { - config.set('userManagement.jwtSessionDurationHours', jwtSessionDurationHours); - config.set('userManagement.jwtRefreshTimeoutHours', 50); - - token = issueJWT(mockUser).token; - req = mock({ - cookies: { [AUTH_COOKIE_NAME]: token }, - user: mockUser, - }); - res = mock({ cookie: jest.fn() }); - next = jest.fn(); - }); - - it('does not do anything if the cookie is still valid', async () => { - // ARRANGE - // cookie has 50.5 hours to live: 51 - 0.5 - jest.advanceTimersByTime(30 * Time.minutes.toMilliseconds); - - // ACT - await refreshExpiringCookie(req, res, next); - - // ASSERT - expect(next).toHaveBeenCalledTimes(1); - expect(res.cookie).not.toHaveBeenCalled(); - }); - - it('refreshes the cookie if it has less than 50 hours to live', async () => { - // ARRANGE - // cookie has 49.5 hours to live: 51 - 1.5 - jest.advanceTimersByTime(1.5 * Time.hours.toMilliseconds); - - // ACT - await refreshExpiringCookie(req, res, next); - - // ASSERT - expect(next).toHaveBeenCalledTimes(1); - expect(res.cookie).toHaveBeenCalledTimes(1); - expect(res.cookie).toHaveBeenCalledWith(AUTH_COOKIE_NAME, expect.any(String), { - httpOnly: true, - maxAge: jwtSessionDurationHours * Time.hours.toMilliseconds, - sameSite: 'lax', - }); - }); - }); -}); diff --git a/packages/cli/test/unit/push/index.test.ts b/packages/cli/test/unit/push/index.test.ts new file mode 100644 index 0000000000000..a2296a61ba388 --- /dev/null +++ b/packages/cli/test/unit/push/index.test.ts @@ -0,0 +1,42 @@ +import type { WebSocket } from 'ws'; +import config from '@/config'; +import type { User } from '@db/entities/User'; +import { Push } from '@/push'; +import { SSEPush } from '@/push/sse.push'; +import { WebSocketPush } from '@/push/websocket.push'; +import type { WebSocketPushRequest, SSEPushRequest } from '@/push/types'; +import { mockInstance } from '../../shared/mocking'; +import { mock } from 'jest-mock-extended'; +import { BadRequestError } from '@/errors/response-errors/bad-request.error'; + +jest.unmock('@/push'); + +describe('Push', () => { + const user = mock(); + + const sseBackend = mockInstance(SSEPush); + const wsBackend = mockInstance(WebSocketPush); + + test('should validate sessionId on requests for websocket backend', () => { + config.set('push.backend', 'websocket'); + const push = new Push(); + const ws = mock(); + const request = mock({ user, ws }); + request.query = { sessionId: '' }; + push.handleRequest(request, mock()); + + expect(ws.send).toHaveBeenCalled(); + expect(ws.close).toHaveBeenCalledWith(1008); + expect(wsBackend.add).not.toHaveBeenCalled(); + }); + + test('should validate sessionId on requests for SSE backend', () => { + config.set('push.backend', 'sse'); + const push = new Push(); + const request = mock({ user, ws: undefined }); + request.query = { sessionId: '' }; + expect(() => push.handleRequest(request, mock())).toThrow(BadRequestError); + + expect(sseBackend.add).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/cli/test/unit/services/user.service.test.ts b/packages/cli/test/unit/services/user.service.test.ts index 6ab4f79c42911..fe5a7c2a80ca3 100644 --- a/packages/cli/test/unit/services/user.service.test.ts +++ b/packages/cli/test/unit/services/user.service.test.ts @@ -1,22 +1,13 @@ -import Container from 'typedi'; -import jwt from 'jsonwebtoken'; -import { Logger } from '@/Logger'; -import config from '@/config'; +import { mock } from 'jest-mock-extended'; +import { v4 as uuid } from 'uuid'; + import { User } from '@db/entities/User'; -import { UserRepository } from '@db/repositories/user.repository'; import { UserService } from '@/services/user.service'; -import { mockInstance } from '../../shared/mocking'; -import { v4 as uuid } from 'uuid'; -import { InternalHooks } from '@/InternalHooks'; +import { UrlService } from '@/services/url.service'; describe('UserService', () => { - config.set('userManagement.jwtSecret', 'random-secret'); - - mockInstance(Logger); - mockInstance(InternalHooks); - - const userRepository = mockInstance(UserRepository); - const userService = Container.get(UserService); + const urlService = new UrlService(); + const userService = new UserService(mock(), mock(), mock(), urlService); const commonMockUser = Object.assign(new User(), { id: uuid(), @@ -75,66 +66,4 @@ describe('UserService', () => { expect(url.searchParams.get('inviteeId')).toBe(secondUser.id); }); }); - - describe('generatePasswordResetToken', () => { - it('should generate valid password-reset tokens', () => { - const token = userService.generatePasswordResetToken(commonMockUser); - - const decoded = jwt.decode(token) as jwt.JwtPayload; - - if (!decoded.exp) fail('Token does not contain expiry'); - if (!decoded.iat) fail('Token does not contain issued-at'); - - expect(decoded.sub).toEqual(commonMockUser.id); - expect(decoded.exp - decoded.iat).toEqual(1200); // Expires in 20 minutes - expect(decoded.passwordSha).toEqual( - '31513c5a9e3c5afe5c06d5675ace74e8bc3fadd9744ab5d89c311f2a62ccbd39', - ); - }); - }); - - describe('resolvePasswordResetToken', () => { - it('should not return a user if the token in invalid', async () => { - const user = await userService.resolvePasswordResetToken('invalid-token'); - - expect(user).toBeUndefined(); - }); - - it('should not return a user if the token in expired', async () => { - const token = userService.generatePasswordResetToken(commonMockUser, '-1h'); - - const user = await userService.resolvePasswordResetToken(token); - - expect(user).toBeUndefined(); - }); - - it('should not return a user if the user does not exist in the DB', async () => { - userRepository.findOne.mockResolvedValueOnce(null); - const token = userService.generatePasswordResetToken(commonMockUser); - - const user = await userService.resolvePasswordResetToken(token); - - expect(user).toBeUndefined(); - }); - - it('should not return a user if the password sha does not match', async () => { - const token = userService.generatePasswordResetToken(commonMockUser); - const updatedUser = Object.create(commonMockUser); - updatedUser.password = 'something-else'; - userRepository.findOne.mockResolvedValueOnce(updatedUser); - - const user = await userService.resolvePasswordResetToken(token); - - expect(user).toBeUndefined(); - }); - - it('should not return the user if all checks pass', async () => { - const token = userService.generatePasswordResetToken(commonMockUser); - userRepository.findOne.mockResolvedValueOnce(commonMockUser); - - const user = await userService.resolvePasswordResetToken(token); - - expect(user).toEqual(commonMockUser); - }); - }); }); diff --git a/packages/core/package.json b/packages/core/package.json index da1dc9e6901fe..ae9813b018735 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -1,6 +1,6 @@ { "name": "n8n-core", - "version": "1.30.0", + "version": "1.31.0", "description": "Core functionality of n8n", "license": "SEE LICENSE IN LICENSE.md", "homepage": "https://n8n.io", diff --git a/packages/design-system/package.json b/packages/design-system/package.json index 2a78cddc40f13..ac42e7e93a344 100644 --- a/packages/design-system/package.json +++ b/packages/design-system/package.json @@ -1,6 +1,6 @@ { "name": "n8n-design-system", - "version": "1.21.0", + "version": "1.22.0", "license": "SEE LICENSE IN LICENSE.md", "homepage": "https://n8n.io", "author": { @@ -46,7 +46,7 @@ "@types/markdown-it": "^12.2.3", "@types/markdown-it-emoji": "^2.0.2", "@types/markdown-it-link-attributes": "^3.0.1", - "@types/sanitize-html": "^2.9.0", + "@types/sanitize-html": "^2.11.0", "@vitejs/plugin-vue": "^4.5.2", "@vue/test-utils": "^2.4.3", "@vue/tsconfig": "^0.5.1", @@ -71,8 +71,8 @@ "markdown-it-emoji": "^2.0.2", "markdown-it-link-attributes": "^4.0.1", "markdown-it-task-lists": "^2.1.1", - "sanitize-html": "2.10.0", - "vue": "^3.3.4", + "sanitize-html": "2.12.1", + "vue": "^3.4.21", "vue-boring-avatars": "^1.3.0", "vue-router": "^4.2.2", "xss": "^1.0.14" diff --git a/packages/design-system/vite.config.mts b/packages/design-system/vite.config.mts index 7bebfbeec8ab8..254fbb9e4d1be 100644 --- a/packages/design-system/vite.config.mts +++ b/packages/design-system/vite.config.mts @@ -1,6 +1,7 @@ import vue from '@vitejs/plugin-vue'; import { resolve } from 'path'; import { defineConfig, mergeConfig } from 'vite'; +import checker from 'vite-plugin-checker'; import { type UserConfig } from 'vitest'; import { defineConfig as defineVitestConfig } from 'vitest/config'; @@ -28,9 +29,14 @@ export const vitestConfig = defineVitestConfig({ }, }) as UserConfig; +const plugins = [vue()]; +if (process.env.ENABLE_TYPE_CHECKING === 'true') { + plugins.push(checker({ vueTsc: true })); +} + export default mergeConfig( defineConfig({ - plugins: [vue()], + plugins, resolve: { alias: { '@': resolve(__dirname, 'src'), diff --git a/packages/editor-ui/package.json b/packages/editor-ui/package.json index 072e67942f938..f61d31a8311ac 100644 --- a/packages/editor-ui/package.json +++ b/packages/editor-ui/package.json @@ -1,6 +1,6 @@ { "name": "n8n-editor-ui", - "version": "1.30.0", + "version": "1.31.0", "description": "Workflow Editor UI for n8n", "license": "SEE LICENSE IN LICENSE.md", "homepage": "https://n8n.io", @@ -70,7 +70,7 @@ "timeago.js": "^4.0.2", "uuid": "^8.3.2", "v3-infinite-loading": "^1.2.2", - "vue": "^3.3.4", + "vue": "^3.4.21", "vue-agile": "^2.0.0", "vue-chartjs": "^5.2.0", "vue-i18n": "^9.2.2", diff --git a/packages/editor-ui/src/components/AssignmentCollection/Assignment.vue b/packages/editor-ui/src/components/AssignmentCollection/Assignment.vue index 6634e747a6fd1..9684c2c7275cf 100644 --- a/packages/editor-ui/src/components/AssignmentCollection/Assignment.vue +++ b/packages/editor-ui/src/components/AssignmentCollection/Assignment.vue @@ -10,6 +10,8 @@ import { isObject } from '@jsplumb/util'; import type { AssignmentValue, INodeProperties } from 'n8n-workflow'; import { computed, ref } from 'vue'; import TypeSelect from './TypeSelect.vue'; +import { useNDVStore } from '@/stores/ndv.store'; +import { useI18n } from '@/composables/useI18n'; interface Props { path: string; @@ -29,6 +31,9 @@ const emit = defineEmits<{ (event: 'remove'): void; }>(); +const ndvStore = useNDVStore(); +const i18n = useI18n(); + const assignmentTypeToNodeProperty = ( type: string, ): Partial & Pick => { @@ -52,7 +57,7 @@ const assignmentTypeToNodeProperty = ( }; const nameParameter = computed(() => ({ - name: '', + name: 'name', displayName: '', default: '', requiresDataPath: 'single', @@ -62,7 +67,7 @@ const nameParameter = computed(() => ({ const valueParameter = computed(() => { return { - name: '', + name: 'value', displayName: '', default: '', placeholder: 'value', @@ -77,7 +82,12 @@ const hint = computed(() => { } try { - const resolvedValue = resolveParameter(value) as unknown; + const resolvedValue = resolveParameter(value, { + targetItem: ndvStore.hoveringItem ?? undefined, + inputNodeName: ndvStore.ndvInputNodeName, + inputRunIndex: ndvStore.ndvInputRunIndex, + inputBranchIndex: ndvStore.ndvInputBranchIndex, + }) as unknown; if (isObject(resolvedValue)) { return JSON.stringify(resolvedValue); @@ -86,12 +96,20 @@ const hint = computed(() => { return resolvedValue.toString(); } + if (resolvedValue === '') { + return i18n.baseText('parameterInput.emptyString'); + } + return resolvedValue as string; } catch (error) { return ''; } }); +const highlightHint = computed(() => + Boolean(hint.value && ndvStore.hoveringItem && ndvStore.isInputParentOfActiveNode), +); + const valueIsExpression = computed(() => { const { value } = assignment.value; @@ -189,7 +207,13 @@ const onBlur = (): void => { @update="onAssignmentValueChange" @blur="onBlur" /> - + diff --git a/packages/editor-ui/src/components/CredentialEdit/CredentialEdit.vue b/packages/editor-ui/src/components/CredentialEdit/CredentialEdit.vue index f494c1c663020..2a7a2e8324d92 100644 --- a/packages/editor-ui/src/components/CredentialEdit/CredentialEdit.vue +++ b/packages/editor-ui/src/components/CredentialEdit/CredentialEdit.vue @@ -97,11 +97,8 @@
@@ -642,10 +639,6 @@ export default defineComponent({ } this.credentialName = currentCredentials.name; - currentCredentials.nodesAccess.forEach((access: { nodeType: string }) => { - // keep node access structure to keep dates when updating - this.nodeAccess[access.nodeType] = access; - }); } catch (error) { this.showError( error, @@ -671,23 +664,6 @@ export default defineComponent({ sharing_enabled: EnterpriseEditionFeature.Sharing, }); }, - onNodeAccessChange({ name, value }: { name: string; value: boolean }) { - this.hasUnsavedChanges = true; - - if (value) { - this.nodeAccess = { - ...this.nodeAccess, - [name]: { - nodeType: name, - }, - }; - } else { - this.nodeAccess = { - ...this.nodeAccess, - [name]: null, - }; - } - }, onChangeSharedWith(sharees: IDataObject[]) { this.credentialData = { ...this.credentialData, @@ -762,17 +738,13 @@ export default defineComponent({ return; } - const nodesAccess = Object.values(this.nodeAccess).filter( - (access) => !!access, - ) as ICredentialNodeAccess[]; - const { ownedBy, sharedWith, ...credentialData } = this.credentialData; const details: ICredentialsDecrypted = { id: this.credentialId, name: this.credentialName, type: this.credentialTypeName!, data: credentialData, - nodesAccess, + nodesAccess: [], }; this.isRetesting = true; @@ -802,9 +774,6 @@ export default defineComponent({ } this.isSaving = true; - const nodesAccess = Object.values(this.nodeAccess).filter( - (access) => !!access, - ) as ICredentialNodeAccess[]; // Save only the none default data const data = NodeHelpers.getNodeParameters( @@ -827,7 +796,7 @@ export default defineComponent({ name: this.credentialName, type: this.credentialTypeName!, data: data as unknown as ICredentialDataDecryptedObject, - nodesAccess, + nodesAccess: [], sharedWith, ownedBy, }; @@ -1114,18 +1083,7 @@ export default defineComponent({ } }, setupNodeAccess(): void { - this.nodeAccess = this.nodesWithAccess.reduce( - (accu: NodeAccessMap, node: { name: string }) => { - if (this.mode === 'new') { - accu[node.name] = { nodeType: node.name }; // enable all nodes by default - } else { - accu[node.name] = null; - } - - return accu; - }, - {}, - ); + this.nodeAccess = {}; }, resetCredentialData(): void { if (!this.credentialType) { diff --git a/packages/editor-ui/src/components/CredentialEdit/CredentialInfo.vue b/packages/editor-ui/src/components/CredentialEdit/CredentialInfo.vue index da3c2f971dfb1..9574c2c0e0aef 100644 --- a/packages/editor-ui/src/components/CredentialEdit/CredentialInfo.vue +++ b/packages/editor-ui/src/components/CredentialEdit/CredentialInfo.vue @@ -1,35 +1,5 @@