From acec9bad7106b2ad51800a650be68b3ec5003357 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E0=A4=95=E0=A4=BE=E0=A4=B0=E0=A4=A4=E0=A5=8B=E0=A4=AB?= =?UTF-8?q?=E0=A5=8D=E0=A4=AB=E0=A5=87=E0=A4=B2=E0=A4=B8=E0=A5=8D=E0=A4=95?= =?UTF-8?q?=E0=A5=8D=E0=A4=B0=E0=A4=BF=E0=A4=AA=E0=A5=8D=E0=A4=9F=E2=84=A2?= Date: Fri, 3 Nov 2023 17:20:54 +0100 Subject: [PATCH 01/24] refactor(core): Convert OAuth1/OAuth2 routes to decorated controller classes (no-changelog) (#5973) --- packages/cli/src/CredentialsHelper.ts | 46 +-- packages/cli/src/Server.ts | 284 +-------------- packages/cli/src/controllers/index.ts | 2 + .../oauth/abstractOAuth.controller.ts | 106 ++++++ .../oauth/oAuth1Credential.controller.ts | 188 ++++++++++ .../oauth/oAuth2Credential.controller.ts | 262 ++++++++++++++ .../src/credentials/oauth2Credential.api.ts | 327 ------------------ .../sharedCredentials.repository.ts | 14 + packages/cli/src/decorators/Route.ts | 2 + .../cli/src/decorators/registerController.ts | 35 +- packages/cli/src/decorators/types.ts | 1 + packages/cli/src/middlewares/auth.ts | 4 +- packages/cli/src/requests.ts | 2 +- ...allback.html => oauth-callback.handlebars} | 0 .../oAuth1Credential.controller.test.ts | 96 +++++ .../oAuth2Credential.controller.test.ts | 219 ++++++++++++ .../sharedCredentials.repository.test.ts | 60 ++++ 17 files changed, 977 insertions(+), 671 deletions(-) create mode 100644 packages/cli/src/controllers/oauth/abstractOAuth.controller.ts create mode 100644 packages/cli/src/controllers/oauth/oAuth1Credential.controller.ts create mode 100644 packages/cli/src/controllers/oauth/oAuth2Credential.controller.ts delete mode 100644 packages/cli/src/credentials/oauth2Credential.api.ts rename packages/cli/templates/{oauth-callback.html => oauth-callback.handlebars} (100%) create mode 100644 packages/cli/test/unit/controllers/oAuth1Credential.controller.test.ts create mode 100644 packages/cli/test/unit/controllers/oAuth2Credential.controller.test.ts create mode 100644 packages/cli/test/unit/repositories/sharedCredentials.repository.test.ts diff --git a/packages/cli/src/CredentialsHelper.ts b/packages/cli/src/CredentialsHelper.ts index b7db5a419d727..a8fea0f4e9b67 100644 --- a/packages/cli/src/CredentialsHelper.ts +++ b/packages/cli/src/CredentialsHelper.ts @@ -50,7 +50,6 @@ import type { CredentialsEntity } from '@db/entities/CredentialsEntity'; import { NodeTypes } from '@/NodeTypes'; import { CredentialTypes } from '@/CredentialTypes'; import { CredentialsOverwrites } from '@/CredentialsOverwrites'; -import { whereClause } from './UserManagement/UserManagementHelper'; import { RESPONSE_ERROR_MESSAGES } from './constants'; import { isObjectLiteral } from './utils'; import { Logger } from '@/Logger'; @@ -213,7 +212,7 @@ export class CredentialsHelper extends ICredentialsHelper { /** * Resolves the given value in case it is an expression */ - resolveValue( + private resolveValue( parameterValue: string, additionalKeys: IWorkflowDataProxyAdditionalKeys, workflow: Workflow, @@ -248,9 +247,6 @@ export class CredentialsHelper extends ICredentialsHelper { /** * Returns the credentials instance - * - * @param {INodeCredentialsDetails} nodeCredential id and name to return instance of - * @param {string} type Type of the credential to return instance of */ async getCredentials( nodeCredential: INodeCredentialsDetails, @@ -284,8 +280,6 @@ export class CredentialsHelper extends ICredentialsHelper { /** * Returns all the properties of the credentials with the given name - * - * @param {string} type The name of the type to return credentials off */ getCredentialsProperties(type: string): INodeProperties[] { const credentialTypeData = this.credentialTypes.getByName(type); @@ -327,10 +321,6 @@ export class CredentialsHelper extends ICredentialsHelper { /** * Returns the decrypted credential data with applied overwrites - * - * @param {INodeCredentialsDetails} nodeCredentials id and name to return instance of - * @param {string} type Type of the credentials to return data of - * @param {boolean} [raw] Return the data as supplied without defaults or overwrites */ async getDecrypted( additionalData: IWorkflowExecuteAdditionalData, @@ -443,10 +433,6 @@ export class CredentialsHelper extends ICredentialsHelper { /** * Updates credentials in the database - * - * @param {string} name Name of the credentials to set data of - * @param {string} type Type of the credentials to set data of - * @param {ICredentialDataDecryptedObject} data The data to set */ async updateCredentials( nodeCredentials: INodeCredentialsDetails, @@ -804,36 +790,6 @@ export class CredentialsHelper extends ICredentialsHelper { } } -/** - * Get a credential if it has been shared with a user. - */ -export async function getCredentialForUser( - credentialId: string, - user: User, -): Promise { - const sharedCredential = await Db.collections.SharedCredentials.findOne({ - relations: ['credentials'], - where: whereClause({ - user, - entityType: 'credentials', - entityId: credentialId, - }), - }); - - if (!sharedCredential) return null; - - return sharedCredential.credentials as ICredentialsDb; -} - -/** - * Get a credential without user check - */ -export async function getCredentialWithoutUser( - credentialId: string, -): Promise { - return Db.collections.Credentials.findOneBy({ id: credentialId }); -} - export function createCredentialsFromCredentialsEntity( credential: CredentialsEntity, encrypt = false, diff --git a/packages/cli/src/Server.ts b/packages/cli/src/Server.ts index 7ce3a476e28c3..f94e5e088b7be 100644 --- a/packages/cli/src/Server.ts +++ b/packages/cli/src/Server.ts @@ -1,5 +1,4 @@ /* eslint-disable @typescript-eslint/no-unsafe-argument */ - /* eslint-disable @typescript-eslint/no-unnecessary-type-assertion */ /* eslint-disable prefer-const */ /* eslint-disable @typescript-eslint/no-shadow */ @@ -11,8 +10,7 @@ import assert from 'assert'; import { exec as callbackExec } from 'child_process'; import { access as fsAccess } from 'fs/promises'; import os from 'os'; -import { join as pathJoin, resolve as pathResolve } from 'path'; -import { createHmac } from 'crypto'; +import { join as pathJoin } from 'path'; import { promisify } from 'util'; import cookieParser from 'cookie-parser'; import express from 'express'; @@ -20,26 +18,15 @@ import { engine as expressHandlebars } from 'express-handlebars'; import type { ServeStaticOptions } from 'serve-static'; import type { FindManyOptions, FindOptionsWhere } from 'typeorm'; import { Not, In } from 'typeorm'; -import type { AxiosRequestConfig } from 'axios'; -import axios from 'axios'; -import type { RequestOptions } from 'oauth-1.0a'; -import clientOAuth1 from 'oauth-1.0a'; -import { - Credentials, - LoadMappingOptions, - LoadNodeParameterOptions, - LoadNodeListSearch, -} from 'n8n-core'; +import { LoadMappingOptions, LoadNodeParameterOptions, LoadNodeListSearch } from 'n8n-core'; import type { INodeCredentials, - INodeCredentialsDetails, INodeListSearchResult, INodeParameters, INodePropertyOptions, INodeTypeNameVersion, - WorkflowExecuteMode, ICredentialTypes, ExecutionStatus, IExecutionsSummary, @@ -63,17 +50,14 @@ import { inDevelopment, inE2ETests, N8N_VERSION, - RESPONSE_ERROR_MESSAGES, TEMPLATES_DIR, } from '@/constants'; import { credentialsController } from '@/credentials/credentials.controller'; -import { oauth2CredentialController } from '@/credentials/oauth2Credential.api'; import type { CurlHelper, ExecutionRequest, NodeListSearchRequest, NodeParameterOptionsRequest, - OAuthRequest, ResourceMapperRequest, WorkflowRequest, } from '@/requests'; @@ -84,6 +68,8 @@ import { MeController, MFAController, NodeTypesController, + OAuth1CredentialController, + OAuth2CredentialController, OwnerController, PasswordResetController, TagsController, @@ -99,25 +85,14 @@ import { isApiEnabled, loadPublicApiVersions } from '@/PublicApi'; import { whereClause } from '@/UserManagement/UserManagementHelper'; import { UserManagementMailer } from '@/UserManagement/email'; import * as Db from '@/Db'; -import type { - ICredentialsDb, - ICredentialsOverwrite, - IDiagnosticInfo, - IExecutionsStopData, -} from '@/Interfaces'; +import type { ICredentialsOverwrite, IDiagnosticInfo, IExecutionsStopData } from '@/Interfaces'; import { ActiveExecutions } from '@/ActiveExecutions'; -import { - CredentialsHelper, - getCredentialForUser, - getCredentialWithoutUser, -} from '@/CredentialsHelper'; import { CredentialsOverwrites } from '@/CredentialsOverwrites'; import { CredentialTypes } from '@/CredentialTypes'; import { LoadNodesAndCredentials } from '@/LoadNodesAndCredentials'; import { NodeTypes } from '@/NodeTypes'; import * as ResponseHelper from '@/ResponseHelper'; import { WaitTracker } from '@/WaitTracker'; -import * as WebhookHelpers from '@/WebhookHelpers'; import * as WorkflowExecuteAdditionalData from '@/WorkflowExecuteAdditionalData'; import { toHttpNodeParameters } from '@/CurlConverterHelper'; import { EventBusController } from '@/eventbus/eventBus.controller'; @@ -285,6 +260,8 @@ export class Server extends AbstractServer { new EventBusController(), new EventBusControllerEE(), Container.get(AuthController), + Container.get(OAuth1CredentialController), + Container.get(OAuth2CredentialController), new OwnerController( config, logger, @@ -699,253 +676,6 @@ export class Server extends AbstractServer { ), ); - // ---------------------------------------- - // OAuth1-Credential/Auth - // ---------------------------------------- - - // Authorize OAuth Data - this.app.get( - `/${this.restEndpoint}/oauth1-credential/auth`, - ResponseHelper.send(async (req: OAuthRequest.OAuth1Credential.Auth): Promise => { - const { id: credentialId } = req.query; - - if (!credentialId) { - this.logger.error('OAuth1 credential authorization failed due to missing credential ID'); - throw new ResponseHelper.BadRequestError('Required credential ID is missing'); - } - - const credential = await getCredentialForUser(credentialId, req.user); - - if (!credential) { - this.logger.error( - 'OAuth1 credential authorization failed because the current user does not have the correct permissions', - { userId: req.user.id }, - ); - throw new ResponseHelper.NotFoundError(RESPONSE_ERROR_MESSAGES.NO_CREDENTIAL); - } - - const additionalData = await WorkflowExecuteAdditionalData.getBase(req.user.id); - - const mode: WorkflowExecuteMode = 'internal'; - const credentialsHelper = Container.get(CredentialsHelper); - const decryptedDataOriginal = await credentialsHelper.getDecrypted( - additionalData, - credential as INodeCredentialsDetails, - credential.type, - mode, - true, - ); - - const oauthCredentials = credentialsHelper.applyDefaultsAndOverwrites( - additionalData, - decryptedDataOriginal, - credential.type, - mode, - ); - - const signatureMethod = oauthCredentials.signatureMethod as string; - - const oAuthOptions: clientOAuth1.Options = { - consumer: { - key: oauthCredentials.consumerKey as string, - secret: oauthCredentials.consumerSecret as string, - }, - signature_method: signatureMethod, - // eslint-disable-next-line @typescript-eslint/naming-convention - hash_function(base, key) { - let algorithm: string; - switch (signatureMethod) { - case 'HMAC-SHA256': - algorithm = 'sha256'; - break; - case 'HMAC-SHA512': - algorithm = 'sha512'; - break; - default: - algorithm = 'sha1'; - break; - } - - return createHmac(algorithm, key).update(base).digest('base64'); - }, - }; - - const oauthRequestData = { - oauth_callback: `${WebhookHelpers.getWebhookBaseUrl()}${ - this.restEndpoint - }/oauth1-credential/callback?cid=${credentialId}`, - }; - - await this.externalHooks.run('oauth1.authenticate', [oAuthOptions, oauthRequestData]); - - const oauth = new clientOAuth1(oAuthOptions); - - const options: RequestOptions = { - method: 'POST', - url: oauthCredentials.requestTokenUrl as string, - data: oauthRequestData, - }; - - const data = oauth.toHeader(oauth.authorize(options)); - - // @ts-ignore - options.headers = data; - - const response = await axios.request(options as Partial); - - // Response comes as x-www-form-urlencoded string so convert it to JSON - - const paramsParser = new URLSearchParams(response.data); - - const responseJson = Object.fromEntries(paramsParser.entries()); - - const returnUri = `${oauthCredentials.authUrl as string}?oauth_token=${ - responseJson.oauth_token - }`; - - // Encrypt the data - const credentials = new Credentials( - credential as INodeCredentialsDetails, - credential.type, - credential.nodesAccess, - ); - - credentials.setData(decryptedDataOriginal); - const newCredentialsData = credentials.getDataToSave() as unknown as ICredentialsDb; - - // Add special database related data - newCredentialsData.updatedAt = new Date(); - - // Update the credentials in DB - await Db.collections.Credentials.update(credentialId, newCredentialsData); - - this.logger.verbose('OAuth1 authorization successful for new credential', { - userId: req.user.id, - credentialId, - }); - - return returnUri; - }), - ); - - // Verify and store app code. Generate access tokens and store for respective credential. - this.app.get( - `/${this.restEndpoint}/oauth1-credential/callback`, - async (req: OAuthRequest.OAuth1Credential.Callback, res: express.Response) => { - try { - const { oauth_verifier, oauth_token, cid: credentialId } = req.query; - - if (!oauth_verifier || !oauth_token) { - const errorResponse = new ResponseHelper.ServiceUnavailableError( - `Insufficient parameters for OAuth1 callback. Received following query parameters: ${JSON.stringify( - req.query, - )}`, - ); - this.logger.error( - 'OAuth1 callback failed because of insufficient parameters received', - { - userId: req.user?.id, - credentialId, - }, - ); - return ResponseHelper.sendErrorResponse(res, errorResponse); - } - - const credential = await getCredentialWithoutUser(credentialId); - - if (!credential) { - this.logger.error('OAuth1 callback failed because of insufficient user permissions', { - userId: req.user?.id, - credentialId, - }); - const errorResponse = new ResponseHelper.NotFoundError( - RESPONSE_ERROR_MESSAGES.NO_CREDENTIAL, - ); - return ResponseHelper.sendErrorResponse(res, errorResponse); - } - - const additionalData = await WorkflowExecuteAdditionalData.getBase(req.user.id); - - const mode: WorkflowExecuteMode = 'internal'; - const credentialsHelper = Container.get(CredentialsHelper); - const decryptedDataOriginal = await credentialsHelper.getDecrypted( - additionalData, - credential as INodeCredentialsDetails, - credential.type, - mode, - true, - ); - const oauthCredentials = credentialsHelper.applyDefaultsAndOverwrites( - additionalData, - decryptedDataOriginal, - credential.type, - mode, - ); - - const options: AxiosRequestConfig = { - method: 'POST', - url: oauthCredentials.accessTokenUrl as string, - params: { - oauth_token, - oauth_verifier, - }, - }; - - let oauthToken; - - try { - oauthToken = await axios.request(options); - } catch (error) { - this.logger.error('Unable to fetch tokens for OAuth1 callback', { - userId: req.user?.id, - credentialId, - }); - const errorResponse = new ResponseHelper.NotFoundError('Unable to get access tokens!'); - return ResponseHelper.sendErrorResponse(res, errorResponse); - } - - // Response comes as x-www-form-urlencoded string so convert it to JSON - - const paramParser = new URLSearchParams(oauthToken.data); - - const oauthTokenJson = Object.fromEntries(paramParser.entries()); - - decryptedDataOriginal.oauthTokenData = oauthTokenJson; - - const credentials = new Credentials( - credential as INodeCredentialsDetails, - credential.type, - credential.nodesAccess, - ); - credentials.setData(decryptedDataOriginal); - const newCredentialsData = credentials.getDataToSave() as unknown as ICredentialsDb; - // Add special database related data - newCredentialsData.updatedAt = new Date(); - // Save the credentials in DB - await Db.collections.Credentials.update(credentialId, newCredentialsData); - - this.logger.verbose('OAuth1 callback successful for new credential', { - userId: req.user?.id, - credentialId, - }); - res.sendFile(pathResolve(TEMPLATES_DIR, 'oauth-callback.html')); - } catch (error) { - this.logger.error('OAuth1 callback failed because of insufficient user permissions', { - userId: req.user?.id, - credentialId: req.query.cid, - }); - // Error response - return ResponseHelper.sendErrorResponse(res, error); - } - }, - ); - - // ---------------------------------------- - // OAuth2-Credential - // ---------------------------------------- - - this.app.use(`/${this.restEndpoint}/oauth2-credential`, oauth2CredentialController); - // ---------------------------------------- // Executions // ---------------------------------------- diff --git a/packages/cli/src/controllers/index.ts b/packages/cli/src/controllers/index.ts index 4ccee477171d9..9351eff35f68f 100644 --- a/packages/cli/src/controllers/index.ts +++ b/packages/cli/src/controllers/index.ts @@ -3,6 +3,8 @@ export { LdapController } from './ldap.controller'; export { MeController } from './me.controller'; export { MFAController } from './mfa.controller'; export { NodeTypesController } from './nodeTypes.controller'; +export { OAuth1CredentialController } from './oauth/oAuth1Credential.controller'; +export { OAuth2CredentialController } from './oauth/oAuth2Credential.controller'; export { OwnerController } from './owner.controller'; export { PasswordResetController } from './passwordReset.controller'; export { TagsController } from './tags.controller'; diff --git a/packages/cli/src/controllers/oauth/abstractOAuth.controller.ts b/packages/cli/src/controllers/oauth/abstractOAuth.controller.ts new file mode 100644 index 0000000000000..274a2186f085c --- /dev/null +++ b/packages/cli/src/controllers/oauth/abstractOAuth.controller.ts @@ -0,0 +1,106 @@ +import { Service } from 'typedi'; +import { Credentials } from 'n8n-core'; +import type { ICredentialDataDecryptedObject, IWorkflowExecuteAdditionalData } from 'n8n-workflow'; +import config from '@/config'; +import type { CredentialsEntity } from '@db/entities/CredentialsEntity'; +import type { User } from '@db/entities/User'; +import { CredentialsRepository, SharedCredentialsRepository } from '@db/repositories'; +import type { ICredentialsDb } from '@/Interfaces'; +import { getInstanceBaseUrl } from '@/UserManagement/UserManagementHelper'; +import type { OAuthRequest } from '@/requests'; +import { BadRequestError, NotFoundError } from '@/ResponseHelper'; +import { RESPONSE_ERROR_MESSAGES } from '@/constants'; +import { CredentialsHelper } from '@/CredentialsHelper'; +import * as WorkflowExecuteAdditionalData from '@/WorkflowExecuteAdditionalData'; +import { Logger } from '@/Logger'; +import { ExternalHooks } from '@/ExternalHooks'; + +@Service() +export abstract class AbstractOAuthController { + abstract oauthVersion: number; + + constructor( + protected readonly logger: Logger, + protected readonly externalHooks: ExternalHooks, + private readonly credentialsHelper: CredentialsHelper, + private readonly credentialsRepository: CredentialsRepository, + private readonly sharedCredentialsRepository: SharedCredentialsRepository, + ) {} + + get baseUrl() { + const restUrl = `${getInstanceBaseUrl()}/${config.getEnv('endpoints.rest')}`; + return `${restUrl}/oauth${this.oauthVersion}-credential`; + } + + protected async getCredential( + req: OAuthRequest.OAuth2Credential.Auth, + ): Promise { + const { id: credentialId } = req.query; + + if (!credentialId) { + throw new BadRequestError('Required credential ID is missing'); + } + + const credential = await this.sharedCredentialsRepository.findCredentialForUser( + credentialId, + req.user, + ); + + if (!credential) { + this.logger.error( + `OAuth${this.oauthVersion} credential authorization failed because the current user does not have the correct permissions`, + { userId: req.user.id }, + ); + throw new NotFoundError(RESPONSE_ERROR_MESSAGES.NO_CREDENTIAL); + } + + return credential; + } + + protected async getAdditionalData(user: User) { + return WorkflowExecuteAdditionalData.getBase(user.id); + } + + protected async getDecryptedData( + credential: ICredentialsDb, + additionalData: IWorkflowExecuteAdditionalData, + ) { + return this.credentialsHelper.getDecrypted( + additionalData, + credential, + credential.type, + 'internal', + true, + ); + } + + protected applyDefaultsAndOverwrites( + credential: ICredentialsDb, + decryptedData: ICredentialDataDecryptedObject, + additionalData: IWorkflowExecuteAdditionalData, + ) { + return this.credentialsHelper.applyDefaultsAndOverwrites( + additionalData, + decryptedData, + credential.type, + 'internal', + ) as unknown as T; + } + + protected async encryptAndSaveData( + credential: ICredentialsDb, + decryptedData: ICredentialDataDecryptedObject, + ) { + const credentials = new Credentials(credential, credential.type, credential.nodesAccess); + credentials.setData(decryptedData); + await this.credentialsRepository.update(credential.id, { + ...credentials.getDataToSave(), + updatedAt: new Date(), + }); + } + + /** Get a credential without user check */ + protected async getCredentialWithoutUser(credentialId: string): Promise { + return this.credentialsRepository.findOneBy({ id: credentialId }); + } +} diff --git a/packages/cli/src/controllers/oauth/oAuth1Credential.controller.ts b/packages/cli/src/controllers/oauth/oAuth1Credential.controller.ts new file mode 100644 index 0000000000000..c9d3d31e000e9 --- /dev/null +++ b/packages/cli/src/controllers/oauth/oAuth1Credential.controller.ts @@ -0,0 +1,188 @@ +import { Service } from 'typedi'; +import { Response } from 'express'; +import type { AxiosRequestConfig } from 'axios'; +import axios from 'axios'; +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 { OAuthRequest } from '@/requests'; +import { NotFoundError, sendErrorResponse, ServiceUnavailableError } from '@/ResponseHelper'; +import { AbstractOAuthController } from './abstractOAuth.controller'; + +interface OAuth1CredentialData { + signatureMethod: 'HMAC-SHA256' | 'HMAC-SHA512' | 'HMAC-SHA1'; + consumerKey: string; + consumerSecret: string; + authUrl: string; + accessTokenUrl: string; + requestTokenUrl: string; +} + +const algorithmMap = { + /* eslint-disable @typescript-eslint/naming-convention */ + 'HMAC-SHA256': 'sha256', + 'HMAC-SHA512': 'sha512', + 'HMAC-SHA1': 'sha1', + /* eslint-enable */ +} as const; + +@Service() +@Authorized() +@RestController('/oauth1-credential') +export class OAuth1CredentialController extends AbstractOAuthController { + override oauthVersion = 1; + + /** Get Authorization url */ + @Get('/auth') + async getAuthUri(req: OAuthRequest.OAuth1Credential.Auth): Promise { + const credential = await this.getCredential(req); + const additionalData = await this.getAdditionalData(req.user); + const decryptedDataOriginal = await this.getDecryptedData(credential, additionalData); + const oauthCredentials = this.applyDefaultsAndOverwrites( + credential, + decryptedDataOriginal, + additionalData, + ); + + const signatureMethod = oauthCredentials.signatureMethod; + + const oAuthOptions: clientOAuth1.Options = { + consumer: { + key: oauthCredentials.consumerKey, + secret: oauthCredentials.consumerSecret, + }, + signature_method: signatureMethod, + // eslint-disable-next-line @typescript-eslint/naming-convention + hash_function(base, key) { + const algorithm = algorithmMap[signatureMethod] ?? 'sha1'; + return createHmac(algorithm, key).update(base).digest('base64'); + }, + }; + + const oauthRequestData = { + oauth_callback: `${this.baseUrl}/callback?cid=${credential.id}`, + }; + + await this.externalHooks.run('oauth1.authenticate', [oAuthOptions, oauthRequestData]); + + const oauth = new clientOAuth1(oAuthOptions); + + const options: RequestOptions = { + method: 'POST', + url: oauthCredentials.requestTokenUrl, + data: oauthRequestData, + }; + + const data = oauth.toHeader(oauth.authorize(options)); + + // @ts-ignore + options.headers = data; + + // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment + const { data: response } = await axios.request(options as Partial); + + // Response comes as x-www-form-urlencoded string so convert it to JSON + + const paramsParser = new URLSearchParams(response as string); + + const responseJson = Object.fromEntries(paramsParser.entries()); + + const returnUri = `${oauthCredentials.authUrl}?oauth_token=${responseJson.oauth_token}`; + + await this.encryptAndSaveData(credential, decryptedDataOriginal); + + this.logger.verbose('OAuth1 authorization successful for new credential', { + userId: req.user.id, + credentialId: credential.id, + }); + + return returnUri; + } + + /** Verify and store app code. Generate access tokens and store for respective credential */ + @Get('/callback', { usesTemplates: true }) + async handleCallback(req: OAuthRequest.OAuth1Credential.Callback, res: Response) { + try { + const { oauth_verifier, oauth_token, cid: credentialId } = req.query; + + if (!oauth_verifier || !oauth_token) { + const errorResponse = new ServiceUnavailableError( + `Insufficient parameters for OAuth1 callback. Received following query parameters: ${JSON.stringify( + req.query, + )}`, + ); + this.logger.error('OAuth1 callback failed because of insufficient parameters received', { + userId: req.user?.id, + credentialId, + }); + return sendErrorResponse(res, errorResponse); + } + + const credential = await this.getCredentialWithoutUser(credentialId); + + if (!credential) { + this.logger.error('OAuth1 callback failed because of insufficient user permissions', { + userId: req.user?.id, + credentialId, + }); + const errorResponse = new NotFoundError(RESPONSE_ERROR_MESSAGES.NO_CREDENTIAL); + return sendErrorResponse(res, errorResponse); + } + + const additionalData = await this.getAdditionalData(req.user); + const decryptedDataOriginal = await this.getDecryptedData(credential, additionalData); + const oauthCredentials = this.applyDefaultsAndOverwrites( + credential, + decryptedDataOriginal, + additionalData, + ); + + const options: AxiosRequestConfig = { + method: 'POST', + url: oauthCredentials.accessTokenUrl, + params: { + oauth_token, + oauth_verifier, + }, + }; + + let oauthToken; + + try { + oauthToken = await axios.request(options); + } catch (error) { + this.logger.error('Unable to fetch tokens for OAuth1 callback', { + userId: req.user?.id, + credentialId, + }); + const errorResponse = new NotFoundError('Unable to get access tokens!'); + return sendErrorResponse(res, errorResponse); + } + + // Response comes as x-www-form-urlencoded string so convert it to JSON + + const paramParser = new URLSearchParams(oauthToken.data as string); + + const oauthTokenJson = Object.fromEntries(paramParser.entries()); + + decryptedDataOriginal.oauthTokenData = oauthTokenJson; + + await this.encryptAndSaveData(credential, decryptedDataOriginal); + + this.logger.verbose('OAuth1 callback successful for new credential', { + userId: req.user?.id, + credentialId, + }); + return res.render('oauth-callback'); + } catch (error) { + this.logger.error('OAuth1 callback failed because of insufficient user permissions', { + userId: req.user?.id, + credentialId: req.query.cid, + }); + // Error response + return sendErrorResponse(res, error as Error); + } + } +} diff --git a/packages/cli/src/controllers/oauth/oAuth2Credential.controller.ts b/packages/cli/src/controllers/oauth/oAuth2Credential.controller.ts new file mode 100644 index 0000000000000..219ef2f844b05 --- /dev/null +++ b/packages/cli/src/controllers/oauth/oAuth2Credential.controller.ts @@ -0,0 +1,262 @@ +import { Service } from 'typedi'; +import type { ClientOAuth2Options } from '@n8n/client-oauth2'; +import { ClientOAuth2 } from '@n8n/client-oauth2'; +import Csrf from 'csrf'; +import { Response } from 'express'; +import pkceChallenge from 'pkce-challenge'; +import * as qs from 'querystring'; +import omit from 'lodash/omit'; +import set from 'lodash/set'; +import split from 'lodash/split'; +import type { OAuth2GrantType } from 'n8n-workflow'; +import { jsonParse, jsonStringify } from 'n8n-workflow'; +import { Authorized, Get, RestController } from '@/decorators'; +import { OAuthRequest } from '@/requests'; +import { AbstractOAuthController } from './abstractOAuth.controller'; + +interface OAuth2CredentialData { + clientId: string; + clientSecret?: string; + accessTokenUrl?: string; + authUrl?: string; + scope?: string; + authQueryParameters?: string; + authentication?: 'header' | 'body'; + grantType: OAuth2GrantType; + ignoreSSLIssues?: boolean; +} + +interface CsrfStateParam { + cid: string; + token: string; +} + +@Service() +@Authorized() +@RestController('/oauth2-credential') +export class OAuth2CredentialController extends AbstractOAuthController { + override oauthVersion = 2; + + /** Get Authorization url */ + @Get('/auth') + async getAuthUri(req: OAuthRequest.OAuth2Credential.Auth): Promise { + const credential = await this.getCredential(req); + const additionalData = await this.getAdditionalData(req.user); + const decryptedDataOriginal = await this.getDecryptedData(credential, additionalData); + + // At some point in the past we saved hidden scopes to credentials (but shouldn't) + // Delete scope before applying defaults to make sure new scopes are present on reconnect + // Generic Oauth2 API is an exception because it needs to save the scope + const genericOAuth2 = ['oAuth2Api', 'googleOAuth2Api', 'microsoftOAuth2Api']; + if ( + decryptedDataOriginal?.scope && + credential.type.includes('OAuth2') && + !genericOAuth2.includes(credential.type) + ) { + delete decryptedDataOriginal.scope; + } + + const oauthCredentials = this.applyDefaultsAndOverwrites( + credential, + decryptedDataOriginal, + additionalData, + ); + + // Generate a CSRF prevention token and send it as an OAuth2 state string + const [csrfSecret, state] = this.createCsrfState(credential.id); + + const oAuthOptions = { + ...this.convertCredentialToOptions(oauthCredentials), + state, + }; + + if (oauthCredentials.authQueryParameters) { + oAuthOptions.query = qs.parse(oauthCredentials.authQueryParameters); + } + + await this.externalHooks.run('oauth2.authenticate', [oAuthOptions]); + + decryptedDataOriginal.csrfSecret = csrfSecret; + if (oauthCredentials.grantType === 'pkce') { + const { code_verifier, code_challenge } = pkceChallenge(); + oAuthOptions.query = { + ...oAuthOptions.query, + code_challenge, + code_challenge_method: 'S256', + }; + decryptedDataOriginal.codeVerifier = code_verifier; + } + + await this.encryptAndSaveData(credential, decryptedDataOriginal); + + const oAuthObj = new ClientOAuth2(oAuthOptions); + const returnUri = oAuthObj.code.getUri(); + + this.logger.verbose('OAuth2 authorization url created for credential', { + userId: req.user.id, + credentialId: credential.id, + }); + + return returnUri.toString(); + } + + /** Verify and store app code. Generate access tokens and store for respective credential */ + @Get('/callback', { usesTemplates: true }) + async handleCallback(req: OAuthRequest.OAuth2Credential.Callback, res: Response) { + try { + // realmId it's currently just use for the quickbook OAuth2 flow + const { code, state: encodedState } = req.query; + if (!code || !encodedState) { + return this.renderCallbackError( + res, + 'Insufficient parameters for OAuth2 callback.', + `Received following query parameters: ${JSON.stringify(req.query)}`, + ); + } + + let state: CsrfStateParam; + try { + state = this.decodeCsrfState(encodedState); + } catch (error) { + return this.renderCallbackError(res, (error as Error).message); + } + + const credential = await this.getCredentialWithoutUser(state.cid); + if (!credential) { + const errorMessage = 'OAuth2 callback failed because of insufficient permissions'; + this.logger.error(errorMessage, { + userId: req.user?.id, + credentialId: state.cid, + }); + return this.renderCallbackError(res, errorMessage); + } + + const additionalData = await this.getAdditionalData(req.user); + const decryptedDataOriginal = await this.getDecryptedData(credential, additionalData); + const oauthCredentials = this.applyDefaultsAndOverwrites( + credential, + decryptedDataOriginal, + additionalData, + ); + + const token = new Csrf(); + if ( + decryptedDataOriginal.csrfSecret === undefined || + !token.verify(decryptedDataOriginal.csrfSecret as string, state.token) + ) { + const errorMessage = 'The OAuth2 callback state is invalid!'; + this.logger.debug(errorMessage, { + userId: req.user?.id, + credentialId: credential.id, + }); + return this.renderCallbackError(res, errorMessage); + } + + let options: Partial = {}; + + const oAuthOptions = this.convertCredentialToOptions(oauthCredentials); + + if (oauthCredentials.grantType === 'pkce') { + options = { + body: { code_verifier: decryptedDataOriginal.codeVerifier }, + }; + } else if (oauthCredentials.authentication === 'body') { + options = { + body: { + client_id: oAuthOptions.clientId, + client_secret: oAuthOptions.clientSecret, + }, + }; + delete oAuthOptions.clientSecret; + } + + await this.externalHooks.run('oauth2.callback', [oAuthOptions]); + + const oAuthObj = new ClientOAuth2(oAuthOptions); + + const queryParameters = req.originalUrl.split('?').splice(1, 1).join(''); + + const oauthToken = await oAuthObj.code.getToken( + `${oAuthOptions.redirectUri as string}?${queryParameters}`, + options, + ); + + if (Object.keys(req.query).length > 2) { + set(oauthToken.data, 'callbackQueryString', omit(req.query, 'state', 'code')); + } + + if (oauthToken === undefined) { + const errorMessage = 'Unable to get OAuth2 access tokens!'; + this.logger.error(errorMessage, { + userId: req.user?.id, + credentialId: credential.id, + }); + return this.renderCallbackError(res, errorMessage); + } + + if (decryptedDataOriginal.oauthTokenData) { + // Only overwrite supplied data as some providers do for example just return the + // refresh_token on the very first request and not on subsequent ones. + Object.assign(decryptedDataOriginal.oauthTokenData, oauthToken.data); + } else { + // No data exists so simply set + decryptedDataOriginal.oauthTokenData = oauthToken.data; + } + + delete decryptedDataOriginal.csrfSecret; + await this.encryptAndSaveData(credential, decryptedDataOriginal); + + this.logger.verbose('OAuth2 callback successful for credential', { + userId: req.user?.id, + credentialId: credential.id, + }); + + return res.render('oauth-callback'); + } catch (error) { + return this.renderCallbackError( + res, + (error as Error).message, + // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access + 'body' in error ? jsonStringify(error.body) : undefined, + ); + } + } + + private convertCredentialToOptions(credential: OAuth2CredentialData): ClientOAuth2Options { + return { + clientId: credential.clientId, + clientSecret: credential.clientSecret ?? '', + accessTokenUri: credential.accessTokenUrl ?? '', + authorizationUri: credential.authUrl ?? '', + redirectUri: `${this.baseUrl}/callback`, + scopes: split(credential.scope ?? 'openid', ','), + scopesSeparator: credential.scope?.includes(',') ? ',' : ' ', + ignoreSSLIssues: credential.ignoreSSLIssues ?? false, + }; + } + + private renderCallbackError(res: Response, message: string, reason?: string) { + res.render('oauth-error-callback', { error: { message, reason } }); + } + + private createCsrfState(credentialsId: string): [string, string] { + const token = new Csrf(); + const csrfSecret = token.secretSync(); + const state: CsrfStateParam = { + token: token.create(csrfSecret), + cid: credentialsId, + }; + return [csrfSecret, Buffer.from(JSON.stringify(state)).toString('base64')]; + } + + private decodeCsrfState(encodedState: string): CsrfStateParam { + const errorMessage = 'Invalid state format'; + const decoded = jsonParse(Buffer.from(encodedState, 'base64').toString(), { + errorMessage, + }); + if (typeof decoded.cid !== 'string' || typeof decoded.token !== 'string') { + throw new Error(errorMessage); + } + return decoded; + } +} diff --git a/packages/cli/src/credentials/oauth2Credential.api.ts b/packages/cli/src/credentials/oauth2Credential.api.ts deleted file mode 100644 index 8eda71a4ce27d..0000000000000 --- a/packages/cli/src/credentials/oauth2Credential.api.ts +++ /dev/null @@ -1,327 +0,0 @@ -import type { ClientOAuth2Options } from '@n8n/client-oauth2'; -import { ClientOAuth2 } from '@n8n/client-oauth2'; -import Csrf from 'csrf'; -import express from 'express'; -import pkceChallenge from 'pkce-challenge'; -import * as qs from 'querystring'; -import get from 'lodash/get'; -import omit from 'lodash/omit'; -import set from 'lodash/set'; -import split from 'lodash/split'; -import unset from 'lodash/unset'; -import { Credentials } from 'n8n-core'; -import type { WorkflowExecuteMode, INodeCredentialsDetails } from 'n8n-workflow'; -import { jsonStringify } from 'n8n-workflow'; -import { resolve as pathResolve } from 'path'; - -import * as Db from '@/Db'; -import * as ResponseHelper from '@/ResponseHelper'; -import type { ICredentialsDb } from '@/Interfaces'; -import { RESPONSE_ERROR_MESSAGES, TEMPLATES_DIR } from '@/constants'; -import { - CredentialsHelper, - getCredentialForUser, - getCredentialWithoutUser, -} from '@/CredentialsHelper'; -import type { OAuthRequest } from '@/requests'; -import { ExternalHooks } from '@/ExternalHooks'; -import config from '@/config'; -import { getInstanceBaseUrl } from '@/UserManagement/UserManagementHelper'; -import { Container } from 'typedi'; - -import * as WorkflowExecuteAdditionalData from '@/WorkflowExecuteAdditionalData'; -import { Logger } from '@/Logger'; - -export const oauth2CredentialController = express.Router(); - -const restEndpoint = config.getEnv('endpoints.rest'); - -/** - * GET /oauth2-credential/auth - * - * Authorize OAuth Data - */ -oauth2CredentialController.get( - '/auth', - ResponseHelper.send(async (req: OAuthRequest.OAuth1Credential.Auth): Promise => { - const { id: credentialId } = req.query; - - if (!credentialId) { - throw new ResponseHelper.BadRequestError('Required credential ID is missing'); - } - - const credential = await getCredentialForUser(credentialId, req.user); - - if (!credential) { - Container.get(Logger).error('Failed to authorize OAuth2 due to lack of permissions', { - userId: req.user.id, - credentialId, - }); - throw new ResponseHelper.NotFoundError(RESPONSE_ERROR_MESSAGES.NO_CREDENTIAL); - } - - const additionalData = await WorkflowExecuteAdditionalData.getBase(req.user.id); - - const credentialType = credential.type; - - const mode: WorkflowExecuteMode = 'internal'; - const credentialsHelper = Container.get(CredentialsHelper); - const decryptedDataOriginal = await credentialsHelper.getDecrypted( - additionalData, - credential as INodeCredentialsDetails, - credentialType, - mode, - true, - ); - - // At some point in the past we saved hidden scopes to credentials (but shouldn't) - // Delete scope before applying defaults to make sure new scopes are present on reconnect - // Generic Oauth2 API is an exception because it needs to save the scope - const genericOAuth2 = ['oAuth2Api', 'googleOAuth2Api', 'microsoftOAuth2Api']; - if ( - decryptedDataOriginal?.scope && - credentialType.includes('OAuth2') && - !genericOAuth2.includes(credentialType) - ) { - delete decryptedDataOriginal.scope; - } - - const oauthCredentials = credentialsHelper.applyDefaultsAndOverwrites( - additionalData, - decryptedDataOriginal, - credentialType, - mode, - ); - - const token = new Csrf(); - // Generate a CSRF prevention token and send it as an OAuth2 state string - const csrfSecret = token.secretSync(); - const state = { - token: token.create(csrfSecret), - cid: req.query.id, - }; - const stateEncodedStr = Buffer.from(JSON.stringify(state)).toString('base64'); - - const scopes = get(oauthCredentials, 'scope', 'openid') as string; - const oAuthOptions: ClientOAuth2Options = { - clientId: get(oauthCredentials, 'clientId') as string, - clientSecret: get(oauthCredentials, 'clientSecret', '') as string, - accessTokenUri: get(oauthCredentials, 'accessTokenUrl', '') as string, - authorizationUri: get(oauthCredentials, 'authUrl', '') as string, - redirectUri: `${getInstanceBaseUrl()}/${restEndpoint}/oauth2-credential/callback`, - scopes: split(scopes, ','), - scopesSeparator: scopes.includes(',') ? ',' : ' ', - state: stateEncodedStr, - }; - const authQueryParameters = get(oauthCredentials, 'authQueryParameters', '') as string; - if (authQueryParameters) { - oAuthOptions.query = qs.parse(authQueryParameters); - } - - await Container.get(ExternalHooks).run('oauth2.authenticate', [oAuthOptions]); - - const oAuthObj = new ClientOAuth2(oAuthOptions); - - // Encrypt the data - const credentials = new Credentials( - credential as INodeCredentialsDetails, - credentialType, - credential.nodesAccess, - ); - decryptedDataOriginal.csrfSecret = csrfSecret; - - if (oauthCredentials.grantType === 'pkce') { - const { code_verifier, code_challenge } = pkceChallenge(); - oAuthOptions.query = { - ...oAuthOptions.query, - code_challenge, - code_challenge_method: 'S256', - }; - decryptedDataOriginal.codeVerifier = code_verifier; - } - - credentials.setData(decryptedDataOriginal); - const newCredentialsData = credentials.getDataToSave() as unknown as ICredentialsDb; - - // Add special database related data - newCredentialsData.updatedAt = new Date(); - - // Update the credentials in DB - await Db.collections.Credentials.update(req.query.id, newCredentialsData); - - Container.get(Logger).verbose('OAuth2 authorization url created for credential', { - userId: req.user.id, - credentialId, - }); - - return oAuthObj.code.getUri(); - }), -); - -const renderCallbackError = (res: express.Response, message: string, reason?: string) => - res.render('oauth-error-callback', { error: { message, reason } }); - -/** - * GET /oauth2-credential/callback - * - * Verify and store app code. Generate access tokens and store for respective credential. - */ - -oauth2CredentialController.get( - '/callback', - async (req: OAuthRequest.OAuth2Credential.Callback, res: express.Response) => { - try { - // realmId it's currently just use for the quickbook OAuth2 flow - const { code, state: stateEncoded } = req.query; - if (!code || !stateEncoded) { - return renderCallbackError( - res, - 'Insufficient parameters for OAuth2 callback.', - `Received following query parameters: ${JSON.stringify(req.query)}`, - ); - } - - let state; - try { - state = JSON.parse(Buffer.from(stateEncoded, 'base64').toString()) as { - cid: string; - token: string; - }; - } catch (error) { - return renderCallbackError(res, 'Invalid state format returned'); - } - - const credential = await getCredentialWithoutUser(state.cid); - - if (!credential) { - const errorMessage = 'OAuth2 callback failed because of insufficient permissions'; - Container.get(Logger).error(errorMessage, { - userId: req.user?.id, - credentialId: state.cid, - }); - return renderCallbackError(res, errorMessage); - } - - const additionalData = await WorkflowExecuteAdditionalData.getBase(state.cid); - - const mode: WorkflowExecuteMode = 'internal'; - const credentialsHelper = Container.get(CredentialsHelper); - const decryptedDataOriginal = await credentialsHelper.getDecrypted( - additionalData, - credential as INodeCredentialsDetails, - credential.type, - mode, - true, - ); - const oauthCredentials = credentialsHelper.applyDefaultsAndOverwrites( - additionalData, - decryptedDataOriginal, - credential.type, - mode, - ); - - const token = new Csrf(); - if ( - decryptedDataOriginal.csrfSecret === undefined || - !token.verify(decryptedDataOriginal.csrfSecret as string, state.token) - ) { - const errorMessage = 'The OAuth2 callback state is invalid!'; - Container.get(Logger).debug(errorMessage, { - userId: req.user?.id, - credentialId: state.cid, - }); - return renderCallbackError(res, errorMessage); - } - - let options: Partial = {}; - - const scopes = get(oauthCredentials, 'scope', 'openid') as string; - const oAuth2Parameters: ClientOAuth2Options = { - clientId: get(oauthCredentials, 'clientId') as string, - clientSecret: get(oauthCredentials, 'clientSecret', '') as string, - accessTokenUri: get(oauthCredentials, 'accessTokenUrl', '') as string, - authorizationUri: get(oauthCredentials, 'authUrl', '') as string, - redirectUri: `${getInstanceBaseUrl()}/${restEndpoint}/oauth2-credential/callback`, - scopes: split(scopes, ','), - scopesSeparator: scopes.includes(',') ? ',' : ' ', - ignoreSSLIssues: get(oauthCredentials, 'ignoreSSLIssues') as boolean, - }; - - if (oauthCredentials.grantType === 'pkce') { - options = { - body: { code_verifier: decryptedDataOriginal.codeVerifier }, - }; - } else if ((get(oauthCredentials, 'authentication', 'header') as string) === 'body') { - options = { - body: { - client_id: get(oauthCredentials, 'clientId') as string, - client_secret: get(oauthCredentials, 'clientSecret', '') as string, - }, - }; - // @ts-ignore - delete oAuth2Parameters.clientSecret; - } - - await Container.get(ExternalHooks).run('oauth2.callback', [oAuth2Parameters]); - - const oAuthObj = new ClientOAuth2(oAuth2Parameters); - - const queryParameters = req.originalUrl.split('?').splice(1, 1).join(''); - - const oauthToken = await oAuthObj.code.getToken( - `${oAuth2Parameters.redirectUri as string}?${queryParameters}`, - // @ts-ignore - options, - ); - - if (Object.keys(req.query).length > 2) { - set(oauthToken.data, 'callbackQueryString', omit(req.query, 'state', 'code')); - } - - if (oauthToken === undefined) { - const errorMessage = 'Unable to get OAuth2 access tokens!'; - Container.get(Logger).error(errorMessage, { - userId: req.user?.id, - credentialId: state.cid, - }); - return renderCallbackError(res, errorMessage); - } - - if (decryptedDataOriginal.oauthTokenData) { - // Only overwrite supplied data as some providers do for example just return the - // refresh_token on the very first request and not on subsequent ones. - Object.assign(decryptedDataOriginal.oauthTokenData, oauthToken.data); - } else { - // No data exists so simply set - decryptedDataOriginal.oauthTokenData = oauthToken.data; - } - - unset(decryptedDataOriginal, 'csrfSecret'); - - const credentials = new Credentials( - credential as INodeCredentialsDetails, - credential.type, - credential.nodesAccess, - ); - credentials.setData(decryptedDataOriginal); - const newCredentialsData = credentials.getDataToSave() as unknown as ICredentialsDb; - // Add special database related data - newCredentialsData.updatedAt = new Date(); - // Save the credentials in DB - await Db.collections.Credentials.update(state.cid, newCredentialsData); - Container.get(Logger).verbose('OAuth2 callback successful for new credential', { - userId: req.user?.id, - credentialId: state.cid, - }); - - return res.sendFile(pathResolve(TEMPLATES_DIR, 'oauth-callback.html')); - } catch (error) { - return renderCallbackError( - res, - (error as Error).message, - // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - 'body' in error ? jsonStringify(error.body) : undefined, - ); - } - }, -); diff --git a/packages/cli/src/databases/repositories/sharedCredentials.repository.ts b/packages/cli/src/databases/repositories/sharedCredentials.repository.ts index 29e473b885455..d2a738886324f 100644 --- a/packages/cli/src/databases/repositories/sharedCredentials.repository.ts +++ b/packages/cli/src/databases/repositories/sharedCredentials.repository.ts @@ -1,10 +1,24 @@ import { Service } from 'typedi'; import { DataSource, Repository } from 'typeorm'; import { SharedCredentials } from '../entities/SharedCredentials'; +import type { User } from '../entities/User'; @Service() export class SharedCredentialsRepository extends Repository { constructor(dataSource: DataSource) { super(SharedCredentials, dataSource.manager); } + + /** Get a credential if it has been shared with a user */ + async findCredentialForUser(credentialsId: string, user: User) { + const sharedCredential = await this.findOne({ + relations: ['credentials'], + where: { + credentialsId, + ...(!user.isOwner ? { userId: user.id } : {}), + }, + }); + if (!sharedCredential) return null; + return sharedCredential.credentials; + } } diff --git a/packages/cli/src/decorators/Route.ts b/packages/cli/src/decorators/Route.ts index 38f0c8ab5183f..64dd96d2935a5 100644 --- a/packages/cli/src/decorators/Route.ts +++ b/packages/cli/src/decorators/Route.ts @@ -4,6 +4,7 @@ import type { Method, RouteMetadata } from './types'; interface RouteOptions { middlewares?: RequestHandler[]; + usesTemplates?: boolean; } const RouteFactory = @@ -18,6 +19,7 @@ const RouteFactory = path, middlewares: options.middlewares ?? [], handlerName: String(handlerName), + usesTemplates: options.usesTemplates ?? false, }); Reflect.defineMetadata(CONTROLLER_ROUTES, routes, controllerClass); }; diff --git a/packages/cli/src/decorators/registerController.ts b/packages/cli/src/decorators/registerController.ts index 80e5ff7235b0e..8a2158075882a 100644 --- a/packages/cli/src/decorators/registerController.ts +++ b/packages/cli/src/decorators/registerController.ts @@ -36,7 +36,8 @@ const authFreeRoutes: string[] = []; export const canSkipAuth = (method: string, path: string): boolean => authFreeRoutes.includes(`${method.toLowerCase()} ${path}`); -export const registerController = (app: Application, config: Config, controller: object) => { +export const registerController = (app: Application, config: Config, cObj: object) => { + const controller = cObj as Controller; const controllerClass = controller.constructor; const controllerBasePath = Reflect.getMetadata(CONTROLLER_BASE_PATH, controllerClass) as | string @@ -57,24 +58,22 @@ export const registerController = (app: Application, config: Config, controller: const controllerMiddlewares = ( (Reflect.getMetadata(CONTROLLER_MIDDLEWARES, controllerClass) ?? []) as MiddlewareMetadata[] - ).map( - ({ handlerName }) => - (controller as Controller)[handlerName].bind(controller) as RequestHandler, - ); + ).map(({ handlerName }) => controller[handlerName].bind(controller) as RequestHandler); - routes.forEach(({ method, path, middlewares: routeMiddlewares, handlerName }) => { - const authRole = authRoles && (authRoles[handlerName] ?? authRoles['*']); - router[method]( - path, - ...(authRole ? [createAuthMiddleware(authRole)] : []), - ...controllerMiddlewares, - ...routeMiddlewares, - send(async (req: Request, res: Response) => - (controller as Controller)[handlerName](req, res), - ), - ); - if (!authRole || authRole === 'none') authFreeRoutes.push(`${method} ${prefix}${path}`); - }); + routes.forEach( + ({ method, path, middlewares: routeMiddlewares, handlerName, usesTemplates }) => { + const authRole = authRoles && (authRoles[handlerName] ?? authRoles['*']); + const handler = async (req: Request, res: Response) => controller[handlerName](req, res); + router[method]( + path, + ...(authRole ? [createAuthMiddleware(authRole)] : []), + ...controllerMiddlewares, + ...routeMiddlewares, + usesTemplates ? handler : send(handler), + ); + if (!authRole || authRole === 'none') authFreeRoutes.push(`${method} ${prefix}${path}`); + }, + ); app.use(prefix, router); } diff --git a/packages/cli/src/decorators/types.ts b/packages/cli/src/decorators/types.ts index d118e6e6c9cbd..4d0f7d43b80d2 100644 --- a/packages/cli/src/decorators/types.ts +++ b/packages/cli/src/decorators/types.ts @@ -15,6 +15,7 @@ export interface RouteMetadata { path: string; handlerName: string; middlewares: RequestHandler[]; + usesTemplates: boolean; } export type Controller = Record< diff --git a/packages/cli/src/middlewares/auth.ts b/packages/cli/src/middlewares/auth.ts index e02383d6fefb9..89a37d50fbc2e 100644 --- a/packages/cli/src/middlewares/auth.ts +++ b/packages/cli/src/middlewares/auth.ts @@ -89,9 +89,7 @@ export const setupAuthMiddlewares = ( canSkipAuth(req.method, req.path) || isAuthExcluded(req.url, ignoredEndpoints) || req.url.startsWith(`/${restEndpoint}/settings`) || - isPostUsersId(req, restEndpoint) || - req.url.startsWith(`/${restEndpoint}/oauth2-credential/callback`) || - req.url.startsWith(`/${restEndpoint}/oauth1-credential/callback`) + isPostUsersId(req, restEndpoint) ) { return next(); } diff --git a/packages/cli/src/requests.ts b/packages/cli/src/requests.ts index f50cdfda8c507..de675a4a141db 100644 --- a/packages/cli/src/requests.ts +++ b/packages/cli/src/requests.ts @@ -392,7 +392,7 @@ export declare namespace OAuthRequest { } namespace OAuth2Credential { - type Auth = OAuth1Credential.Auth; + type Auth = AuthenticatedRequest<{}, {}, {}, { id: string }>; type Callback = AuthenticatedRequest<{}, {}, {}, { code: string; state: string }>; } } diff --git a/packages/cli/templates/oauth-callback.html b/packages/cli/templates/oauth-callback.handlebars similarity index 100% rename from packages/cli/templates/oauth-callback.html rename to packages/cli/templates/oauth-callback.handlebars diff --git a/packages/cli/test/unit/controllers/oAuth1Credential.controller.test.ts b/packages/cli/test/unit/controllers/oAuth1Credential.controller.test.ts new file mode 100644 index 0000000000000..92d06080a9b0a --- /dev/null +++ b/packages/cli/test/unit/controllers/oAuth1Credential.controller.test.ts @@ -0,0 +1,96 @@ +import nock from 'nock'; +import Container from 'typedi'; +import { Cipher } from 'n8n-core'; +import { mock } from 'jest-mock-extended'; + +import { OAuth1CredentialController } from '@/controllers/oauth/oAuth1Credential.controller'; +import type { CredentialsEntity } from '@db/entities/CredentialsEntity'; +import type { User } from '@db/entities/User'; +import type { OAuthRequest } from '@/requests'; +import { BadRequestError, NotFoundError } from '@/ResponseHelper'; +import { CredentialsRepository, SharedCredentialsRepository } from '@/databases/repositories'; +import { ExternalHooks } from '@/ExternalHooks'; +import { Logger } from '@/Logger'; +import { VariablesService } from '@/environments/variables/variables.service'; +import { SecretsHelper } from '@/SecretsHelpers'; +import { CredentialsHelper } from '@/CredentialsHelper'; + +import { mockInstance } from '../../integration/shared/utils'; + +describe('OAuth1CredentialController', () => { + mockInstance(Logger); + mockInstance(ExternalHooks); + mockInstance(SecretsHelper); + mockInstance(VariablesService, { + getAllCached: async () => [], + }); + const cipher = mockInstance(Cipher); + const credentialsHelper = mockInstance(CredentialsHelper); + const credentialsRepository = mockInstance(CredentialsRepository); + const sharedCredentialsRepository = mockInstance(SharedCredentialsRepository); + const user = mock({ + id: '123', + password: 'password', + authIdentities: [], + globalRoleId: '1', + }); + const credential = mock({ + id: '1', + name: 'Test Credential', + nodesAccess: [], + type: 'oAuth1Api', + }); + + const controller = Container.get(OAuth1CredentialController); + + beforeEach(() => { + jest.resetAllMocks(); + }); + + describe('getAuthUri', () => { + it('should throw a BadRequestError when credentialId is missing in the query', async () => { + const req = mock({ query: { id: '' } }); + await expect(controller.getAuthUri(req)).rejects.toThrowError( + new BadRequestError('Required credential ID is missing'), + ); + }); + + it('should throw a NotFoundError when no matching credential is found for the user', async () => { + sharedCredentialsRepository.findCredentialForUser.mockResolvedValueOnce(null); + + const req = mock({ user, query: { id: '1' } }); + await expect(controller.getAuthUri(req)).rejects.toThrowError( + new NotFoundError('Credential not found'), + ); + }); + + it('should return a valid auth URI', async () => { + sharedCredentialsRepository.findCredentialForUser.mockResolvedValueOnce(credential); + credentialsHelper.getDecrypted.mockResolvedValueOnce({}); + credentialsHelper.applyDefaultsAndOverwrites.mockReturnValueOnce({ + requestTokenUrl: 'https://example.domain/oauth/request_token', + authUrl: 'https://example.domain/oauth/authorize', + signatureMethod: 'HMAC-SHA1', + }); + nock('https://example.domain') + .post('/oauth/request_token', { + oauth_callback: 'http://localhost:5678/rest/oauth1-credential/callback?cid=1', + }) + .reply(200, { oauth_token: 'random-token' }); + cipher.encrypt.mockReturnValue('encrypted'); + + const req = mock({ user, query: { id: '1' } }); + const authUri = await controller.getAuthUri(req); + expect(authUri).toEqual('https://example.domain/oauth/authorize?oauth_token=random-token'); + expect(credentialsRepository.update).toHaveBeenCalledWith( + '1', + expect.objectContaining({ + data: 'encrypted', + id: '1', + name: 'Test Credential', + type: 'oAuth1Api', + }), + ); + }); + }); +}); diff --git a/packages/cli/test/unit/controllers/oAuth2Credential.controller.test.ts b/packages/cli/test/unit/controllers/oAuth2Credential.controller.test.ts new file mode 100644 index 0000000000000..46c7bd6d0568a --- /dev/null +++ b/packages/cli/test/unit/controllers/oAuth2Credential.controller.test.ts @@ -0,0 +1,219 @@ +import nock from 'nock'; +import Container from 'typedi'; +import Csrf from 'csrf'; +import { type Response } from 'express'; +import { Cipher } from 'n8n-core'; +import { mock } from 'jest-mock-extended'; + +import { OAuth2CredentialController } from '@/controllers/oauth/oAuth2Credential.controller'; +import type { CredentialsEntity } from '@db/entities/CredentialsEntity'; +import type { User } from '@db/entities/User'; +import type { OAuthRequest } from '@/requests'; +import { BadRequestError, NotFoundError } from '@/ResponseHelper'; +import { CredentialsRepository, SharedCredentialsRepository } from '@/databases/repositories'; +import { ExternalHooks } from '@/ExternalHooks'; +import { Logger } from '@/Logger'; +import { VariablesService } from '@/environments/variables/variables.service'; +import { SecretsHelper } from '@/SecretsHelpers'; +import { CredentialsHelper } from '@/CredentialsHelper'; + +import { mockInstance } from '../../integration/shared/utils'; + +describe('OAuth2CredentialController', () => { + mockInstance(Logger); + mockInstance(SecretsHelper); + mockInstance(VariablesService, { + getAllCached: async () => [], + }); + const cipher = mockInstance(Cipher); + const externalHooks = mockInstance(ExternalHooks); + const credentialsHelper = mockInstance(CredentialsHelper); + const credentialsRepository = mockInstance(CredentialsRepository); + const sharedCredentialsRepository = mockInstance(SharedCredentialsRepository); + + const csrfSecret = 'csrf-secret'; + const user = mock({ + id: '123', + password: 'password', + authIdentities: [], + globalRoleId: '1', + }); + const credential = mock({ + id: '1', + name: 'Test Credential', + nodesAccess: [], + type: 'oAuth2Api', + }); + + const controller = Container.get(OAuth2CredentialController); + + beforeEach(() => { + jest.resetAllMocks(); + }); + + describe('getAuthUri', () => { + it('should throw a BadRequestError when credentialId is missing in the query', async () => { + const req = mock({ query: { id: '' } }); + await expect(controller.getAuthUri(req)).rejects.toThrowError( + new BadRequestError('Required credential ID is missing'), + ); + }); + + it('should throw a NotFoundError when no matching credential is found for the user', async () => { + sharedCredentialsRepository.findCredentialForUser.mockResolvedValueOnce(null); + + const req = mock({ user, query: { id: '1' } }); + await expect(controller.getAuthUri(req)).rejects.toThrowError( + new NotFoundError('Credential not found'), + ); + }); + + it('should return a valid auth URI', async () => { + jest.spyOn(Csrf.prototype, 'secretSync').mockReturnValueOnce(csrfSecret); + jest.spyOn(Csrf.prototype, 'create').mockReturnValueOnce('token'); + sharedCredentialsRepository.findCredentialForUser.mockResolvedValueOnce(credential); + credentialsHelper.getDecrypted.mockResolvedValueOnce({}); + credentialsHelper.applyDefaultsAndOverwrites.mockReturnValue({ + clientId: 'test-client-id', + authUrl: 'https://example.domain/o/oauth2/v2/auth', + }); + cipher.encrypt.mockReturnValue('encrypted'); + + const req = mock({ user, query: { id: '1' } }); + const authUri = await controller.getAuthUri(req); + expect(authUri).toEqual( + 'https://example.domain/o/oauth2/v2/auth?client_id=test-client-id&redirect_uri=http%3A%2F%2Flocalhost%3A5678%2Frest%2Foauth2-credential%2Fcallback&response_type=code&state=eyJ0b2tlbiI6InRva2VuIiwiY2lkIjoiMSJ9&scope=openid', + ); + expect(credentialsRepository.update).toHaveBeenCalledWith( + '1', + expect.objectContaining({ + data: 'encrypted', + id: '1', + name: 'Test Credential', + type: 'oAuth2Api', + }), + ); + }); + }); + + describe('handleCallback', () => { + const validState = Buffer.from( + JSON.stringify({ + token: 'token', + cid: '1', + }), + ).toString('base64'); + + it('should render the error page when required query params are missing', async () => { + const req = mock({ + query: { code: undefined, state: undefined }, + }); + const res = mock(); + await controller.handleCallback(req, res); + + expect(res.render).toHaveBeenCalledWith('oauth-error-callback', { + error: { + message: 'Insufficient parameters for OAuth2 callback.', + reason: 'Received following query parameters: undefined', + }, + }); + expect(credentialsRepository.findOneBy).not.toHaveBeenCalled(); + }); + + it('should render the error page when `state` query param is invalid', async () => { + const req = mock({ + query: { code: 'code', state: 'invalid-state' }, + }); + const res = mock(); + await controller.handleCallback(req, res); + + expect(res.render).toHaveBeenCalledWith('oauth-error-callback', { + error: { + message: 'Invalid state format', + }, + }); + expect(credentialsRepository.findOneBy).not.toHaveBeenCalled(); + }); + + it('should render the error page when credential is not found in DB', async () => { + credentialsRepository.findOneBy.mockResolvedValueOnce(null); + + const req = mock({ + query: { code: 'code', state: validState }, + }); + const res = mock(); + await controller.handleCallback(req, res); + + expect(res.render).toHaveBeenCalledWith('oauth-error-callback', { + error: { + message: 'OAuth2 callback failed because of insufficient permissions', + }, + }); + expect(credentialsRepository.findOneBy).toHaveBeenCalledTimes(1); + expect(credentialsRepository.findOneBy).toHaveBeenCalledWith({ id: '1' }); + }); + + it('should render the error page when csrfSecret on the saved credential does not match the state', async () => { + credentialsRepository.findOneBy.mockResolvedValueOnce(credential); + credentialsHelper.getDecrypted.mockResolvedValueOnce({ csrfSecret }); + jest.spyOn(Csrf.prototype, 'verify').mockReturnValueOnce(false); + + const req = mock({ + query: { code: 'code', state: validState }, + }); + const res = mock(); + await controller.handleCallback(req, res); + expect(res.render).toHaveBeenCalledWith('oauth-error-callback', { + error: { + message: 'The OAuth2 callback state is invalid!', + }, + }); + expect(externalHooks.run).not.toHaveBeenCalled(); + }); + + it('should exchange the code for a valid token, and save it to DB', async () => { + credentialsRepository.findOneBy.mockResolvedValueOnce(credential); + credentialsHelper.getDecrypted.mockResolvedValueOnce({ csrfSecret }); + credentialsHelper.applyDefaultsAndOverwrites.mockReturnValue({ + clientId: 'test-client-id', + clientSecret: 'oauth-secret', + accessTokenUrl: 'https://example.domain/token', + }); + jest.spyOn(Csrf.prototype, 'verify').mockReturnValueOnce(true); + nock('https://example.domain') + .post( + '/token', + 'code=code&grant_type=authorization_code&redirect_uri=http%3A%2F%2Flocalhost%3A5678%2Frest%2Foauth2-credential%2Fcallback', + ) + .reply(200, { access_token: 'access-token', refresh_token: 'refresh-token' }); + cipher.encrypt.mockReturnValue('encrypted'); + + const req = mock({ + query: { code: 'code', state: validState }, + originalUrl: '?code=code', + }); + const res = mock(); + await controller.handleCallback(req, res); + + expect(externalHooks.run).toHaveBeenCalledWith('oauth2.callback', [ + expect.objectContaining({ + clientId: 'test-client-id', + redirectUri: 'http://localhost:5678/rest/oauth2-credential/callback', + }), + ]); + expect(cipher.encrypt).toHaveBeenCalledWith({ + oauthTokenData: { access_token: 'access-token', refresh_token: 'refresh-token' }, + }); + expect(credentialsRepository.update).toHaveBeenCalledWith( + '1', + expect.objectContaining({ + data: 'encrypted', + id: '1', + name: 'Test Credential', + type: 'oAuth2Api', + }), + ); + expect(res.render).toHaveBeenCalledWith('oauth-callback'); + }); + }); +}); diff --git a/packages/cli/test/unit/repositories/sharedCredentials.repository.test.ts b/packages/cli/test/unit/repositories/sharedCredentials.repository.test.ts new file mode 100644 index 0000000000000..d04b6974a6f9f --- /dev/null +++ b/packages/cli/test/unit/repositories/sharedCredentials.repository.test.ts @@ -0,0 +1,60 @@ +import { Container } from 'typedi'; +import { DataSource, EntityManager, type EntityMetadata } from 'typeorm'; +import { mock } from 'jest-mock-extended'; +import type { User } from '@db/entities/User'; +import type { CredentialsEntity } from '@db/entities/CredentialsEntity'; +import { SharedCredentials } from '@db/entities/SharedCredentials'; +import { SharedCredentialsRepository } from '@db/repositories/sharedCredentials.repository'; +import { mockInstance } from '../../integration/shared/utils/'; + +describe('SharedCredentialsRepository', () => { + const entityManager = mockInstance(EntityManager); + const dataSource = mockInstance(DataSource, { + manager: entityManager, + getMetadata: () => mock({ target: SharedCredentials }), + }); + Object.assign(entityManager, { connection: dataSource }); + const repository = Container.get(SharedCredentialsRepository); + + describe('findCredentialForUser', () => { + const credentialsId = 'cred_123'; + const sharedCredential = mock(); + sharedCredential.credentials = mock({ id: credentialsId }); + const owner = mock({ isOwner: true }); + const member = mock({ isOwner: false, id: 'test' }); + + beforeEach(() => { + jest.resetAllMocks(); + }); + + test('should allow instance owner access to all credentials', async () => { + entityManager.findOne.mockResolvedValueOnce(sharedCredential); + const credential = await repository.findCredentialForUser(credentialsId, owner); + expect(entityManager.findOne).toHaveBeenCalledWith(SharedCredentials, { + relations: ['credentials'], + where: { credentialsId }, + }); + expect(credential).toEqual(sharedCredential.credentials); + }); + + test('should allow members', async () => { + entityManager.findOne.mockResolvedValueOnce(sharedCredential); + const credential = await repository.findCredentialForUser(credentialsId, member); + expect(entityManager.findOne).toHaveBeenCalledWith(SharedCredentials, { + relations: ['credentials'], + where: { credentialsId, userId: member.id }, + }); + expect(credential).toEqual(sharedCredential.credentials); + }); + + test('should return null when no shared credential is found', async () => { + entityManager.findOne.mockResolvedValueOnce(null); + const credential = await repository.findCredentialForUser(credentialsId, member); + expect(entityManager.findOne).toHaveBeenCalledWith(SharedCredentials, { + relations: ['credentials'], + where: { credentialsId, userId: member.id }, + }); + expect(credential).toEqual(null); + }); + }); +}); From 5790e251b8072679d7c061e2d2fa1f4229e03cf8 Mon Sep 17 00:00:00 2001 From: Ricardo Espinoza Date: Fri, 3 Nov 2023 13:44:12 -0400 Subject: [PATCH 02/24] feat(core): Rate limit forgot password endpoint (#7604) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Github issue / Community forum post (link here to close automatically): --------- Co-authored-by: कारतोफ्फेलस्क्रिप्ट™ --- packages/cli/package.json | 1 + packages/cli/src/AbstractServer.ts | 3 +++ packages/cli/src/config/schema.ts | 7 +++++++ .../controllers/passwordReset.controller.ts | 12 +++++++++-- .../src/plugins/i18n/locales/en.json | 1 + .../src/views/ForgotMyPasswordView.vue | 20 ++++++++++++------- pnpm-lock.yaml | 20 ++++++++++++++++--- 7 files changed, 52 insertions(+), 12 deletions(-) diff --git a/packages/cli/package.json b/packages/cli/package.json index 651e2a3dee549..27bab5ea18a60 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -129,6 +129,7 @@ "express-handlebars": "^7.0.2", "express-openapi-validator": "^4.13.6", "express-prom-bundle": "^6.6.0", + "express-rate-limit": "^7.1.3", "fast-glob": "^3.2.5", "flatted": "^3.2.4", "formidable": "^3.5.0", diff --git a/packages/cli/src/AbstractServer.ts b/packages/cli/src/AbstractServer.ts index 137f5f5de3979..6fab9bb0385d2 100644 --- a/packages/cli/src/AbstractServer.ts +++ b/packages/cli/src/AbstractServer.ts @@ -55,6 +55,9 @@ export abstract class AbstractServer { this.app = express(); this.app.disable('x-powered-by'); + const proxyHops = config.getEnv('proxy_hops'); + if (proxyHops > 0) this.app.set('trust proxy', proxyHops); + this.protocol = config.getEnv('protocol'); this.sslKey = config.getEnv('ssl_key'); this.sslCert = config.getEnv('ssl_cert'); diff --git a/packages/cli/src/config/schema.ts b/packages/cli/src/config/schema.ts index 438a4daf0e1c9..8ab9d0aa2233e 100644 --- a/packages/cli/src/config/schema.ts +++ b/packages/cli/src/config/schema.ts @@ -1344,4 +1344,11 @@ export const schema = { env: 'N8N_LEADER_SELECTION_CHECK_INTERVAL', }, }, + + proxy_hops: { + format: Number, + default: 0, + env: 'N8N_PROXY_HOPS', + doc: 'Number of reverse-proxies n8n is running behind', + }, }; diff --git a/packages/cli/src/controllers/passwordReset.controller.ts b/packages/cli/src/controllers/passwordReset.controller.ts index c8bf9ceeaaff2..ef25cbd8260c8 100644 --- a/packages/cli/src/controllers/passwordReset.controller.ts +++ b/packages/cli/src/controllers/passwordReset.controller.ts @@ -24,12 +24,18 @@ import { isSamlCurrentAuthenticationMethod } from '@/sso/ssoHelpers'; import { UserService } from '@/services/user.service'; import { License } from '@/License'; import { Container } from 'typedi'; -import { RESPONSE_ERROR_MESSAGES } from '@/constants'; +import { RESPONSE_ERROR_MESSAGES, inTest } from '@/constants'; import { TokenExpiredError } from 'jsonwebtoken'; import type { JwtPayload } from '@/services/jwt.service'; import { JwtService } from '@/services/jwt.service'; import { MfaService } from '@/Mfa/mfa.service'; import { Logger } from '@/Logger'; +import { rateLimit } from 'express-rate-limit'; + +const throttle = rateLimit({ + windowMs: 5 * 60 * 1000, // 5 minutes + limit: 5, // Limit each IP to 5 requests per `window` (here, per 5 minutes). +}); @RestController() export class PasswordResetController { @@ -46,7 +52,9 @@ export class PasswordResetController { /** * Send a password reset email. */ - @Post('/forgot-password') + @Post('/forgot-password', { + middlewares: !inTest ? [throttle] : [], + }) async forgotPassword(req: PasswordResetRequest.Email) { if (!this.mailer.isEmailSetUp) { this.logger.debug( diff --git a/packages/editor-ui/src/plugins/i18n/locales/en.json b/packages/editor-ui/src/plugins/i18n/locales/en.json index be64fc1a3e624..7b9c66e3b751c 100644 --- a/packages/editor-ui/src/plugins/i18n/locales/en.json +++ b/packages/editor-ui/src/plugins/i18n/locales/en.json @@ -660,6 +660,7 @@ "forgotPassword.sendingEmailError": "Problem sending email", "forgotPassword.ldapUserPasswordResetUnavailable": "Please contact your LDAP administrator to reset your password", "forgotPassword.smtpErrorContactAdministrator": "Please contact your administrator (problem with your SMTP setup)", + "forgotPassword.tooManyRequests": "You’ve reached the password reset limit. Please try again in a few minutes.", "forms.resourceFiltersDropdown.filters": "Filters", "forms.resourceFiltersDropdown.ownedBy": "Owned by", "forms.resourceFiltersDropdown.sharedWith": "Shared with", diff --git a/packages/editor-ui/src/views/ForgotMyPasswordView.vue b/packages/editor-ui/src/views/ForgotMyPasswordView.vue index 55ede7a05a17a..eaae4a39f8d65 100644 --- a/packages/editor-ui/src/views/ForgotMyPasswordView.vue +++ b/packages/editor-ui/src/views/ForgotMyPasswordView.vue @@ -88,14 +88,20 @@ export default defineComponent({ }); } catch (error) { let message = this.$locale.baseText('forgotPassword.smtpErrorContactAdministrator'); - if (error.httpStatusCode === 422) { - message = this.$locale.baseText(error.message); + if (error.isAxiosError) { + const { status } = error.response; + if (status === 429) { + message = this.$locale.baseText('forgotPassword.tooManyRequests'); + } else if (error.httpStatusCode === 422) { + message = this.$locale.baseText(error.message); + } + + this.showMessage({ + type: 'error', + title: this.$locale.baseText('forgotPassword.sendingEmailError'), + message, + }); } - this.showMessage({ - type: 'error', - title: this.$locale.baseText('forgotPassword.sendingEmailError'), - message, - }); } this.loading = false; }, diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 61c36655b3ba0..fbaeec28499b8 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -284,6 +284,9 @@ importers: express-prom-bundle: specifier: ^6.6.0 version: 6.6.0(prom-client@13.2.0) + express-rate-limit: + specifier: ^7.1.3 + version: 7.1.3(express@4.18.2) fast-glob: specifier: ^3.2.5 version: 3.2.12 @@ -9641,7 +9644,7 @@ packages: /axios@0.21.4: resolution: {integrity: sha512-ut5vewkiu8jjGBdqpM44XxjuCjq9LAKeHVmoVfHVzy8eHgxxq8SbAVQNovDA8mVi05kP0Ea/n/UzcSHcTJQfNg==} dependencies: - follow-redirects: 1.15.2(debug@4.3.4) + follow-redirects: 1.15.2(debug@3.2.7) transitivePeerDependencies: - debug dev: false @@ -9670,11 +9673,12 @@ packages: form-data: 4.0.0 transitivePeerDependencies: - debug + dev: true /axios@1.4.0: resolution: {integrity: sha512-S4XCWMEmzvo64T9GfvQDOXgYRDJ/wsSZc7Jvdgx5u1sd0JwsuPLqb3SYmusag+edF6ziyMensPVqLTSc1PiSEA==} dependencies: - follow-redirects: 1.15.2(debug@4.3.4) + follow-redirects: 1.15.2(debug@3.2.7) form-data: 4.0.0 proxy-from-env: 1.1.0 transitivePeerDependencies: @@ -12772,6 +12776,15 @@ packages: url-value-parser: 2.2.0 dev: false + /express-rate-limit@7.1.3(express@4.18.2): + resolution: {integrity: sha512-BDes6WeNYSGRRGQU8QDNwUnwqaBro28HN/TTweM3RlxXRHDld8RLoH7tbfCxAc0hamQyn6aL0KrfR45+ZxknYg==} + engines: {node: '>= 16'} + peerDependencies: + express: 4 || 5 || ^5.0.0-beta.1 + dependencies: + express: 4.18.2 + dev: false + /express@4.18.2: resolution: {integrity: sha512-5/PsL6iGPdfQ/lKM1UuielYgv3BUoJfz1aUwU9vHZ+J7gyvwdQXFEBIEIaxeGf0GIcreATNyBExtalisDbuMqQ==} engines: {node: '>= 0.10.0'} @@ -13236,6 +13249,7 @@ packages: optional: true dependencies: debug: 4.3.4(supports-color@8.1.1) + dev: true /for-each@0.3.3: resolution: {integrity: sha512-jqYfLp7mo9vIyQf8ykW2v7A+2N4QjeCeI5+Dz9XraiO1ign81wjiH7Fb9vSOWvQfNtmSa4H2RoQTrrXivdUZmw==} @@ -18476,7 +18490,7 @@ packages: resolution: {integrity: sha512-aXYe/D+28kF63W8Cz53t09ypEORz+ULeDCahdAqhVrRm2scbOXFbtnn0GGhvMpYe45grepLKuwui9KxrZ2ZuMw==} engines: {node: '>=14.17.0'} dependencies: - axios: 0.27.2(debug@4.3.4) + axios: 0.27.2(debug@3.2.7) transitivePeerDependencies: - debug dev: false From b11c4de0392289ff1ac4c4293ff92ba3bdeecbc0 Mon Sep 17 00:00:00 2001 From: Ricardo Espinoza Date: Sat, 4 Nov 2023 20:34:05 -0400 Subject: [PATCH 03/24] fix(editor): Focus mfa token input (no-changelog) (#7610) Github issue / Community forum post (link here to close automatically): --- packages/editor-ui/src/views/MfaView.vue | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/editor-ui/src/views/MfaView.vue b/packages/editor-ui/src/views/MfaView.vue index 5aa8b2985413b..e75cdb2ee4fa8 100644 --- a/packages/editor-ui/src/views/MfaView.vue +++ b/packages/editor-ui/src/views/MfaView.vue @@ -179,7 +179,7 @@ export default defineComponent({ MFA_AUTHENTICATION_RECOVERY_CODE_INPUT_MAX_LENGTH, ); }, - formField(name: string, label: string, placeholder: string, maxlength: number) { + formField(name: string, label: string, placeholder: string, maxlength: number, focus = true) { return { name, initialValue: '', @@ -189,6 +189,7 @@ export default defineComponent({ maxlength, capitalize: true, validateOnBlur: false, + focusInitially: focus, }, }; }, From 52f655f3d263415ba93c762454746b0f58273506 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Mon, 6 Nov 2023 11:36:22 +0100 Subject: [PATCH 04/24] refactor(Google Sheets Node): Stop reporting to Sentry sheet not found error (no-changelog) (#7617) https://n8nio.sentry.io/issues/4264383378 --- .../nodes/Google/Sheet/GoogleSheetsTrigger.node.ts | 5 ++++- .../nodes-base/nodes/Google/Sheet/v2/actions/router.ts | 2 +- .../nodes/Google/Sheet/v2/helpers/GoogleSheet.ts | 9 +++++++-- .../nodes/Google/Sheet/v2/methods/loadOptions.ts | 2 +- .../nodes/Google/Sheet/v2/methods/resourceMapping.ts | 2 +- 5 files changed, 14 insertions(+), 6 deletions(-) diff --git a/packages/nodes-base/nodes/Google/Sheet/GoogleSheetsTrigger.node.ts b/packages/nodes-base/nodes/Google/Sheet/GoogleSheetsTrigger.node.ts index d923c28c745bc..6745b5f84ead8 100644 --- a/packages/nodes-base/nodes/Google/Sheet/GoogleSheetsTrigger.node.ts +++ b/packages/nodes-base/nodes/Google/Sheet/GoogleSheetsTrigger.node.ts @@ -418,7 +418,10 @@ export class GoogleSheetsTrigger implements INodeType { } const googleSheet = new GoogleSheet(documentId, this); - const sheetName: string = await googleSheet.spreadsheetGetSheetNameById(sheetId); + const sheetName: string = await googleSheet.spreadsheetGetSheetNameById( + this.getNode(), + sheetId, + ); const options = this.getNodeParameter('options') as IDataObject; const previousRevision = workflowStaticData.lastRevision as number; diff --git a/packages/nodes-base/nodes/Google/Sheet/v2/actions/router.ts b/packages/nodes-base/nodes/Google/Sheet/v2/actions/router.ts index da3a9afbbe1fc..b793e5a179149 100644 --- a/packages/nodes-base/nodes/Google/Sheet/v2/actions/router.ts +++ b/packages/nodes-base/nodes/Google/Sheet/v2/actions/router.ts @@ -51,7 +51,7 @@ export async function router(this: IExecuteFunctions): Promise item.properties.sheetId === +sheetId, ); + if (!foundItem?.properties?.title) { - throw new Error(`Sheet with id ${sheetId} not found`); + throw new NodeOperationError(node, `Sheet with ID ${sheetId} not found`, { + severity: 'warning', + }); } + return foundItem.properties.title; } diff --git a/packages/nodes-base/nodes/Google/Sheet/v2/methods/loadOptions.ts b/packages/nodes-base/nodes/Google/Sheet/v2/methods/loadOptions.ts index 4c9611de7deed..42bc396e48ea7 100644 --- a/packages/nodes-base/nodes/Google/Sheet/v2/methods/loadOptions.ts +++ b/packages/nodes-base/nodes/Google/Sheet/v2/methods/loadOptions.ts @@ -45,7 +45,7 @@ export async function getSheetHeaderRow( sheetWithinDocument = '0'; } - const sheetName = await sheet.spreadsheetGetSheetNameById(sheetWithinDocument); + const sheetName = await sheet.spreadsheetGetSheetNameById(this.getNode(), sheetWithinDocument); const sheetData = await sheet.getData(`${sheetName}!1:1`, 'FORMATTED_VALUE'); if (sheetData === undefined) { diff --git a/packages/nodes-base/nodes/Google/Sheet/v2/methods/resourceMapping.ts b/packages/nodes-base/nodes/Google/Sheet/v2/methods/resourceMapping.ts index d28676493e4e9..4821ebe276b53 100644 --- a/packages/nodes-base/nodes/Google/Sheet/v2/methods/resourceMapping.ts +++ b/packages/nodes-base/nodes/Google/Sheet/v2/methods/resourceMapping.ts @@ -23,7 +23,7 @@ export async function getMappingColumns( sheetWithinDocument = '0'; } - const sheetName = await sheet.spreadsheetGetSheetNameById(sheetWithinDocument); + const sheetName = await sheet.spreadsheetGetSheetNameById(this.getNode(), sheetWithinDocument); const sheetData = await sheet.getData(`${sheetName}!1:1`, 'FORMATTED_VALUE'); const columns = sheet.testFilter(sheetData || [], 0, 0).filter((col) => col !== ''); From a994ba5e8d7092edeae05e7aa5fdfbb9fd854034 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Mon, 6 Nov 2023 12:03:35 +0100 Subject: [PATCH 05/24] fix(core): Ensure `init` before checking leader or follower in multi-main scenario (#7621) This PR ensures `MultiMainInstancePublisher` is initialized before checking if the instance is leader or follower. Followers skip license init, license check, and pruning start and stop. --- packages/cli/src/License.ts | 6 +++++- packages/cli/src/commands/BaseCommand.ts | 7 ++++++- packages/cli/src/commands/start.ts | 18 +++++++++++++----- .../main/MultiMainInstance.publisher.ee.ts | 2 ++ packages/cli/src/services/pruning.service.ts | 12 ++++++++++-- 5 files changed, 36 insertions(+), 9 deletions(-) diff --git a/packages/cli/src/License.ts b/packages/cli/src/License.ts index e99c387a866ff..25d9b97e9975a 100644 --- a/packages/cli/src/License.ts +++ b/packages/cli/src/License.ts @@ -118,7 +118,11 @@ export class License { '@/services/orchestration/main/MultiMainInstance.publisher.ee' ); - if (Container.get(MultiMainInstancePublisher).isFollower) { + const multiMainInstancePublisher = Container.get(MultiMainInstancePublisher); + + await multiMainInstancePublisher.init(); + + if (multiMainInstancePublisher.isFollower) { this.logger.debug('Instance is follower, skipping sending of reloadLicense command...'); return; } diff --git a/packages/cli/src/commands/BaseCommand.ts b/packages/cli/src/commands/BaseCommand.ts index cbcbb3d70baba..445685d1d5b9e 100644 --- a/packages/cli/src/commands/BaseCommand.ts +++ b/packages/cli/src/commands/BaseCommand.ts @@ -248,7 +248,11 @@ export abstract class BaseCommand extends Command { '@/services/orchestration/main/MultiMainInstance.publisher.ee' ); - if (Container.get(MultiMainInstancePublisher).isFollower) { + const multiMainInstancePublisher = Container.get(MultiMainInstancePublisher); + + await multiMainInstancePublisher.init(); + + if (multiMainInstancePublisher.isFollower) { this.logger.debug('Instance is follower, skipping license initialization...'); return; } @@ -269,6 +273,7 @@ export abstract class BaseCommand extends Command { try { this.logger.debug('Attempting license activation'); await license.activate(activationKey); + this.logger.debug('License init complete'); } catch (e) { this.logger.error('Could not activate license', e as Error); } diff --git a/packages/cli/src/commands/start.ts b/packages/cli/src/commands/start.ts index ae54bd7928639..097e069de8270 100644 --- a/packages/cli/src/commands/start.ts +++ b/packages/cli/src/commands/start.ts @@ -207,7 +207,7 @@ export class Start extends BaseCommand { this.activeWorkflowRunner = Container.get(ActiveWorkflowRunner); await this.initLicense(); - this.logger.debug('License init complete'); + await this.initOrchestration(); this.logger.debug('Orchestration init complete'); await this.initBinaryDataService(); @@ -233,15 +233,23 @@ export class Start extends BaseCommand { return; } - if (!Container.get(License).isMultipleMainInstancesLicensed()) { - throw new FeatureNotLicensedError(LICENSE_FEATURES.MULTIPLE_MAIN_INSTANCES); - } + // multi-main scenario const { MultiMainInstancePublisher } = await import( '@/services/orchestration/main/MultiMainInstance.publisher.ee' ); - await Container.get(MultiMainInstancePublisher).init(); + const multiMainInstancePublisher = Container.get(MultiMainInstancePublisher); + + await multiMainInstancePublisher.init(); + + if ( + multiMainInstancePublisher.isLeader && + !Container.get(License).isMultipleMainInstancesLicensed() + ) { + throw new FeatureNotLicensedError(LICENSE_FEATURES.MULTIPLE_MAIN_INSTANCES); + } + await Container.get(OrchestrationHandlerMainService).init(); } diff --git a/packages/cli/src/services/orchestration/main/MultiMainInstance.publisher.ee.ts b/packages/cli/src/services/orchestration/main/MultiMainInstance.publisher.ee.ts index 1c7af70d4c5d7..62c92c6e97b50 100644 --- a/packages/cli/src/services/orchestration/main/MultiMainInstance.publisher.ee.ts +++ b/packages/cli/src/services/orchestration/main/MultiMainInstance.publisher.ee.ts @@ -28,6 +28,8 @@ export class MultiMainInstancePublisher extends SingleMainInstancePublisher { private leaderCheckInterval: NodeJS.Timer | undefined; async init() { + if (this.initialized) return; + await this.initPublisher(); this.initialized = true; diff --git a/packages/cli/src/services/pruning.service.ts b/packages/cli/src/services/pruning.service.ts index 7a5a36062c888..c5435e6371bd1 100644 --- a/packages/cli/src/services/pruning.service.ts +++ b/packages/cli/src/services/pruning.service.ts @@ -46,7 +46,11 @@ export class PruningService { '@/services/orchestration/main/MultiMainInstance.publisher.ee' ); - return Container.get(MultiMainInstancePublisher).isLeader; + const multiMainInstancePublisher = Container.get(MultiMainInstancePublisher); + + await multiMainInstancePublisher.init(); + + return multiMainInstancePublisher.isLeader; } return true; @@ -63,7 +67,11 @@ export class PruningService { '@/services/orchestration/main/MultiMainInstance.publisher.ee' ); - if (Container.get(MultiMainInstancePublisher).isFollower) return; + const multiMainInstancePublisher = Container.get(MultiMainInstancePublisher); + + await multiMainInstancePublisher.init(); + + if (multiMainInstancePublisher.isFollower) return; } this.logger.debug('Clearing soft-deletion interval and hard-deletion timeout (pruning cycle)'); From db56a9ee37e8b041ea8958fc8400b9e5b6b81316 Mon Sep 17 00:00:00 2001 From: Jan Oberhauser Date: Mon, 6 Nov 2023 13:44:05 +0100 Subject: [PATCH 06/24] fix(editor): Fix issue that frontend breaks with unkown nodes (#7596) Because of recent changes does the frontend break when a node is not known. ![Screenshot from 2023-11-03 08-27-43](https://github.com/n8n-io/n8n/assets/6249596/ab340c3c-9b72-4220-b1f8-70d80f7a5522) Github issue / Community forum post (link here to close automatically): --------- Signed-off-by: Oleg Ivaniv Co-authored-by: Oleg Ivaniv --- cypress/e2e/12-canvas.cy.ts | 36 ++++++++ .../fixtures/workflow-with-unknown-nodes.json | 90 +++++++++++++++++++ packages/editor-ui/src/components/Node.vue | 52 +++++------ 3 files changed, 153 insertions(+), 25 deletions(-) create mode 100644 cypress/fixtures/workflow-with-unknown-nodes.json diff --git a/cypress/e2e/12-canvas.cy.ts b/cypress/e2e/12-canvas.cy.ts index 8170b9caa1672..0d4a0f414efe9 100644 --- a/cypress/e2e/12-canvas.cy.ts +++ b/cypress/e2e/12-canvas.cy.ts @@ -370,4 +370,40 @@ describe('Canvas Node Manipulation and Navigation', () => { NDVDialog.actions.close(); }); }); + + it('should render connections correctly if unkown nodes are present', () => { + const unknownNodeName = 'Unknown node'; + cy.createFixtureWorkflow('workflow-with-unknown-nodes.json', 'Unknown nodes'); + + WorkflowPage.getters.canvasNodeByName(`${unknownNodeName} 1`).should('exist'); + WorkflowPage.getters.canvasNodeByName(`${unknownNodeName} 2`).should('exist'); + WorkflowPage.actions.zoomToFit(); + + cy.draganddrop( + WorkflowPage.getters.getEndpointSelector('plus', `${unknownNodeName} 1`), + WorkflowPage.getters.getEndpointSelector('input', EDIT_FIELDS_SET_NODE_NAME), + ); + + cy.draganddrop( + WorkflowPage.getters.getEndpointSelector('plus', `${unknownNodeName} 2`), + WorkflowPage.getters.getEndpointSelector('input', `${EDIT_FIELDS_SET_NODE_NAME}1`), + ); + + WorkflowPage.actions.executeWorkflow(); + cy.contains('Node not found').should('be.visible'); + + WorkflowPage.getters + .canvasNodeByName(`${unknownNodeName} 1`) + .find('[data-test-id=delete-node-button]') + .click({ force: true }); + + WorkflowPage.getters + .canvasNodeByName(`${unknownNodeName} 2`) + .find('[data-test-id=delete-node-button]') + .click({ force: true }); + + WorkflowPage.actions.executeWorkflow(); + + cy.contains('Node not found').should('not.exist'); + }); }); diff --git a/cypress/fixtures/workflow-with-unknown-nodes.json b/cypress/fixtures/workflow-with-unknown-nodes.json new file mode 100644 index 0000000000000..df23de5de0444 --- /dev/null +++ b/cypress/fixtures/workflow-with-unknown-nodes.json @@ -0,0 +1,90 @@ +{ + "meta": { + "instanceId": "15bbf37b6a515ccc2f534cabcd8bd171ca33583ff7744b1e9420e5ce68e615bb" + }, + "nodes": [ + { + "parameters": {}, + "id": "40720511-19b6-4421-bdb0-3fb6efef4bc5", + "name": "When clicking \"Execute Workflow\"", + "type": "n8n-nodes-base.manualTrigger", + "typeVersion": 1, + "position": [ + 280, + 320 + ] + }, + { + "parameters": {}, + "id": "acdd1bdc-c642-4ea6-ad67-f4201b640cfa", + "name": "Unknown node 1", + "type": "n8n-nodes-base.thisNodeDoesntExist", + "typeVersion": 1, + "position": [ + 400, + 500 + ] + }, + { + "parameters": {}, + "id": "acdd1bdc-c642-4ea6-ad67-f4201b640ffa", + "name": "Unknown node 2", + "type": "n8n-nodes-base.thisNodeDoesntExistEither", + "typeVersion": 1, + "position": [ + 600, + 500 + ] + }, + { + "parameters": { + "options": {} + }, + "id": "fbe5163b-7474-4741-980a-e4956789be0a", + "name": "Edit Fields", + "type": "n8n-nodes-base.set", + "typeVersion": 3.2, + "position": [ + 500, + 320 + ] + }, + { + "parameters": { + "options": {} + }, + "id": "163313b9-64ff-4ffc-b00f-09b267d8132c", + "name": "Edit Fields1", + "type": "n8n-nodes-base.set", + "typeVersion": 3.2, + "position": [ + 720, + 320 + ] + } + ], + "connections": { + "When clicking \"Execute Workflow\"": { + "main": [ + [ + { + "node": "Edit Fields", + "type": "main", + "index": 0 + } + ] + ] + }, + "Edit Fields": { + "main": [ + [ + { + "node": "Edit Fields1", + "type": "main", + "index": 0 + } + ] + ] + } + } +} diff --git a/packages/editor-ui/src/components/Node.vue b/packages/editor-ui/src/components/Node.vue index 1bb21ee86ef40..da67476079ad8 100644 --- a/packages/editor-ui/src/components/Node.vue +++ b/packages/editor-ui/src/components/Node.vue @@ -365,36 +365,38 @@ export default defineComponent({ top: this.position[1] + 'px', }; - const workflow = this.workflowsStore.getCurrentWorkflow(); - const inputs = - NodeHelpers.getNodeInputs(workflow, this.node, this.nodeType) || - ([] as Array); - const inputTypes = NodeHelpers.getConnectionTypes(inputs); - - const nonMainInputs = inputTypes.filter((input) => input !== NodeConnectionType.Main); - if (nonMainInputs.length) { - const requiredNonMainInputs = inputs.filter( - (input) => typeof input !== 'string' && input.required, - ); + if (this.node && this.nodeType) { + const workflow = this.workflowsStore.getCurrentWorkflow(); + const inputs = + NodeHelpers.getNodeInputs(workflow, this.node, this.nodeType) || + ([] as Array); + const inputTypes = NodeHelpers.getConnectionTypes(inputs); + + const nonMainInputs = inputTypes.filter((input) => input !== NodeConnectionType.Main); + if (nonMainInputs.length) { + const requiredNonMainInputs = inputs.filter( + (input) => typeof input !== 'string' && input.required, + ); + + let spacerCount = 0; + if (NODE_INSERT_SPACER_BETWEEN_INPUT_GROUPS) { + const requiredNonMainInputsCount = requiredNonMainInputs.length; + const optionalNonMainInputsCount = nonMainInputs.length - requiredNonMainInputsCount; + spacerCount = requiredNonMainInputsCount > 0 && optionalNonMainInputsCount > 0 ? 1 : 0; + } - let spacerCount = 0; - if (NODE_INSERT_SPACER_BETWEEN_INPUT_GROUPS) { - const requiredNonMainInputsCount = requiredNonMainInputs.length; - const optionalNonMainInputsCount = nonMainInputs.length - requiredNonMainInputsCount; - spacerCount = requiredNonMainInputsCount > 0 && optionalNonMainInputsCount > 0 ? 1 : 0; + styles['--configurable-node-input-count'] = nonMainInputs.length + spacerCount; } - styles['--configurable-node-input-count'] = nonMainInputs.length + spacerCount; - } + const outputs = + NodeHelpers.getNodeOutputs(workflow, this.node, this.nodeType) || + ([] as Array); - const outputs = - NodeHelpers.getNodeOutputs(workflow, this.node, this.nodeType) || - ([] as Array); + const outputTypes = NodeHelpers.getConnectionTypes(outputs); - const outputTypes = NodeHelpers.getConnectionTypes(outputs); - - const mainOutputs = outputTypes.filter((output) => output === NodeConnectionType.Main); - styles['--node-main-output-count'] = mainOutputs.length; + const mainOutputs = outputTypes.filter((output) => output === NodeConnectionType.Main); + styles['--node-main-output-count'] = mainOutputs.length; + } return styles; }, From 26361dfcd31c9952c8ef109314ca88f5f03e40f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Mon, 6 Nov 2023 13:48:13 +0100 Subject: [PATCH 07/24] fix(core): Fix accessor error when running partial execution (#7618) https://n8nio.sentry.io/issues/3760335947 Followup to #6229 --- packages/core/src/WorkflowExecute.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/src/WorkflowExecute.ts b/packages/core/src/WorkflowExecute.ts index aa63280151dd9..b0490cd63f5db 100644 --- a/packages/core/src/WorkflowExecute.ts +++ b/packages/core/src/WorkflowExecute.ts @@ -212,7 +212,7 @@ export class WorkflowExecute { continue; } const nodeIncomingData = - runData[connection.node][runIndex]?.data?.[connection.type][connection.index]; + runData[connection.node]?.[runIndex]?.data?.[connection.type]?.[connection.index]; if (nodeIncomingData) { incomingData.push(nodeIncomingData); } From 4934462b41e797e79580b43747f18ebcd2ec3de9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E0=A4=95=E0=A4=BE=E0=A4=B0=E0=A4=A4=E0=A5=8B=E0=A4=AB?= =?UTF-8?q?=E0=A5=8D=E0=A4=AB=E0=A5=87=E0=A4=B2=E0=A4=B8=E0=A5=8D=E0=A4=95?= =?UTF-8?q?=E0=A5=8D=E0=A4=B0=E0=A4=BF=E0=A4=AA=E0=A5=8D=E0=A4=9F=E2=84=A2?= Date: Mon, 6 Nov 2023 14:32:38 +0100 Subject: [PATCH 08/24] fix(editor): More dark-mode fixes (no-changelog) (#7624) This fixes: 1. The `OR` divider background when SSO Login is enabled 2. Scanning of QR code in settings in dark mode --- packages/editor-ui/src/components/MfaSetupModal.vue | 8 ++++++++ packages/editor-ui/src/components/SSOLogin.vue | 2 +- packages/editor-ui/src/views/MfaView.vue | 8 -------- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/packages/editor-ui/src/components/MfaSetupModal.vue b/packages/editor-ui/src/components/MfaSetupModal.vue index c7240d833c9f0..ddb507ee953fe 100644 --- a/packages/editor-ui/src/components/MfaSetupModal.vue +++ b/packages/editor-ui/src/components/MfaSetupModal.vue @@ -289,6 +289,14 @@ export default defineComponent({ padding-bottom: var(--spacing-xl); } +.qrContainer { + text-align: center; + + canvas { + border: 4px solid var(--prim-gray-10); + } +} + .headerContainer { text-align: center; } diff --git a/packages/editor-ui/src/components/SSOLogin.vue b/packages/editor-ui/src/components/SSOLogin.vue index 2c36ca656d3fe..aecacdc3b7cfa 100644 --- a/packages/editor-ui/src/components/SSOLogin.vue +++ b/packages/editor-ui/src/components/SSOLogin.vue @@ -53,7 +53,7 @@ const onSSOLogin = async () => { position: relative; display: inline-block; padding: var(--spacing-xl) var(--spacing-l); - background: var(--color-foreground-xlight); + background: var(--color-background-xlight); } } diff --git a/packages/editor-ui/src/views/MfaView.vue b/packages/editor-ui/src/views/MfaView.vue index e75cdb2ee4fa8..3ae8af4f38796 100644 --- a/packages/editor-ui/src/views/MfaView.vue +++ b/packages/editor-ui/src/views/MfaView.vue @@ -219,18 +219,10 @@ body { justify-content: center; } -.textContainer { - text-align: center; -} - .formContainer { padding-bottom: var(--spacing-xl); } -.qrContainer { - text-align: center; -} - .headerContainer { text-align: center; margin-bottom: var(--spacing-xl); From 78b84af8d1cfed005c7d9c715d832e8c91fd9e3f Mon Sep 17 00:00:00 2001 From: Csaba Tuncsik Date: Mon, 6 Nov 2023 18:15:49 +0100 Subject: [PATCH 09/24] fix(editor): Reset canvas zoom before workspace reset in node view (#7625) --- .../e2e/30-editor-after-route-changes.cy.ts | 67 ++ cypress/fixtures/Lots_of_nodes.json | 1051 +++++++++++++++++ packages/editor-ui/src/views/NodeView.vue | 1 + 3 files changed, 1119 insertions(+) create mode 100644 cypress/fixtures/Lots_of_nodes.json diff --git a/cypress/e2e/30-editor-after-route-changes.cy.ts b/cypress/e2e/30-editor-after-route-changes.cy.ts index 3f20879dab879..8ce31cbbdc636 100644 --- a/cypress/e2e/30-editor-after-route-changes.cy.ts +++ b/cypress/e2e/30-editor-after-route-changes.cy.ts @@ -87,6 +87,40 @@ const editWorkflowMoreAndActivate = () => { cy.get('.el-notification .el-notification--error').should('not.exist'); }; +const switchBetweenEditorAndHistory = () => { + workflowPage.getters.workflowHistoryButton().click(); + cy.wait(['@getHistory']); + cy.wait(['@getVersion']); + + cy.intercept('GET', '/rest/workflows/*').as('workflowGet'); + workflowHistoryPage.getters.workflowHistoryCloseButton().click(); + cy.wait(['@workflowGet']); + cy.wait(1000); + + workflowPage.getters.canvasNodes().first().should('be.visible'); + workflowPage.getters.canvasNodes().last().should('be.visible'); +} + +const switchBetweenEditorAndWorkflowlist = () => { + cy.getByTestId('menu-item').first().click(); + cy.wait(['@getUsers', '@getWorkflows', '@getActive', '@getCredentials']); + + cy.getByTestId('resources-list-item').first().click(); + + workflowPage.getters.canvasNodes().first().should('be.visible'); + workflowPage.getters.canvasNodes().last().should('be.visible'); +} + +const zoomInAndCheckNodes = () => { + cy.getByTestId('zoom-in-button').click(); + cy.getByTestId('zoom-in-button').click(); + cy.getByTestId('zoom-in-button').click(); + cy.getByTestId('zoom-in-button').click(); + + workflowPage.getters.canvasNodes().first().should('not.be.visible'); + workflowPage.getters.canvasNodes().last().should('not.be.visible'); +} + describe('Editor actions should work', () => { beforeEach(() => { cy.enableFeature('debugInEditor'); @@ -149,3 +183,36 @@ describe('Editor actions should work', () => { editWorkflowMoreAndActivate(); }); }); + +describe('Editor zoom should work after route changes', () => { + beforeEach(() => { + cy.enableFeature('debugInEditor'); + cy.enableFeature('workflowHistory'); + cy.signin({ email: INSTANCE_OWNER.email, password: INSTANCE_OWNER.password }); + workflowPage.actions.visit(); + cy.createFixtureWorkflow('Lots_of_nodes.json', `Lots of nodes`); + workflowPage.actions.saveWorkflowOnButtonClick(); + }); + + it('after switching between Editor and Workflow history and Workflow list', () => { + cy.intercept('GET', '/rest/workflow-history/workflow/*/version/*').as('getVersion'); + cy.intercept('GET', '/rest/workflow-history/workflow/*').as('getHistory'); + cy.intercept('GET', '/rest/users').as('getUsers'); + cy.intercept('GET', '/rest/workflows').as('getWorkflows'); + cy.intercept('GET', '/rest/active').as('getActive'); + cy.intercept('GET', '/rest/credentials').as('getCredentials'); + + switchBetweenEditorAndHistory(); + zoomInAndCheckNodes(); + switchBetweenEditorAndHistory(); + switchBetweenEditorAndHistory(); + zoomInAndCheckNodes(); + switchBetweenEditorAndWorkflowlist(); + zoomInAndCheckNodes(); + switchBetweenEditorAndWorkflowlist(); + switchBetweenEditorAndWorkflowlist(); + zoomInAndCheckNodes(); + switchBetweenEditorAndHistory(); + switchBetweenEditorAndWorkflowlist(); + }); +}); diff --git a/cypress/fixtures/Lots_of_nodes.json b/cypress/fixtures/Lots_of_nodes.json new file mode 100644 index 0000000000000..97548a6fa0144 --- /dev/null +++ b/cypress/fixtures/Lots_of_nodes.json @@ -0,0 +1,1051 @@ +{ + "name": "Lots of nodes", + "nodes": [ + { + "parameters": {}, + "id": "369fe424-dd3b-4399-9de3-50bd4ce1f75b", + "name": "When clicking \"Execute Workflow\"", + "type": "n8n-nodes-base.manualTrigger", + "typeVersion": 1, + "position": [ + 860, + 740 + ] + }, + { + "parameters": { + "jsCode": "// Loop over input items and add a new field called 'myNewField' to the JSON of each one\nfor (const item of $input.all()) {\n item.json.myNewField = 1;\n}\n\nreturn $input.all();" + }, + "id": "dce967a7-8c5e-43cc-ba2b-e0fb0c9cf14c", + "name": "Code", + "type": "n8n-nodes-base.code", + "typeVersion": 2, + "position": [ + 1080, + 740 + ] + }, + { + "parameters": { + "options": {} + }, + "id": "df7a719e-b25a-43e3-b941-7091a7d9a1a8", + "name": "Edit Fields", + "type": "n8n-nodes-base.set", + "typeVersion": 3.2, + "position": [ + 1300, + 740 + ] + }, + { + "parameters": {}, + "id": "32968b79-6a8b-43ed-b884-eb906b597661", + "name": "IF", + "type": "n8n-nodes-base.if", + "typeVersion": 1, + "position": [ + 1520, + 740 + ] + }, + { + "parameters": { + "jsCode": "// Loop over input items and add a new field called 'myNewField' to the JSON of each one\nfor (const item of $input.all()) {\n item.json.myNewField = 1;\n}\n\nreturn $input.all();" + }, + "id": "e9a72745-6dbb-4be1-b286-aaa679b95e36", + "name": "Code1", + "type": "n8n-nodes-base.code", + "typeVersion": 2, + "position": [ + 1820, + 80 + ] + }, + { + "parameters": { + "options": {} + }, + "id": "f831d21b-c3a9-4bd8-9fc3-6daef12bd43f", + "name": "Edit Fields1", + "type": "n8n-nodes-base.set", + "typeVersion": 3.2, + "position": [ + 2040, + 80 + ] + }, + { + "parameters": {}, + "id": "6e6b2a4f-9e61-4245-8502-ca01e851fcbe", + "name": "IF1", + "type": "n8n-nodes-base.if", + "typeVersion": 1, + "position": [ + 2260, + 80 + ] + }, + { + "parameters": { + "jsCode": "// Loop over input items and add a new field called 'myNewField' to the JSON of each one\nfor (const item of $input.all()) {\n item.json.myNewField = 1;\n}\n\nreturn $input.all();" + }, + "id": "535b9786-ead9-44f9-bff2-ef2e019a4cf9", + "name": "Code3", + "type": "n8n-nodes-base.code", + "typeVersion": 2, + "position": [ + 2560, + -260 + ] + }, + { + "parameters": { + "options": {} + }, + "id": "6a181d75-f2f2-4ad1-be3c-81ebe077ccc8", + "name": "Edit Fields3", + "type": "n8n-nodes-base.set", + "typeVersion": 3.2, + "position": [ + 2780, + -260 + ] + }, + { + "parameters": {}, + "id": "4b45828e-4e2b-4046-b9ae-24b373a81863", + "name": "IF7", + "type": "n8n-nodes-base.if", + "typeVersion": 1, + "position": [ + 3000, + -260 + ] + }, + { + "parameters": { + "jsCode": "// Loop over input items and add a new field called 'myNewField' to the JSON of each one\nfor (const item of $input.all()) {\n item.json.myNewField = 1;\n}\n\nreturn $input.all();" + }, + "id": "059534cb-820c-4fb7-933c-eeed2ae74f1c", + "name": "Code7", + "type": "n8n-nodes-base.code", + "typeVersion": 2, + "position": [ + 3260, + -400 + ] + }, + { + "parameters": { + "options": {} + }, + "id": "4f5c0d94-b69d-4ad3-aa8f-f1dd5824ec4a", + "name": "Edit Fields7", + "type": "n8n-nodes-base.set", + "typeVersion": 3.2, + "position": [ + 3480, + -400 + ] + }, + { + "parameters": {}, + "id": "cd74f840-7b0f-425d-8ecd-e247a7d8abf5", + "name": "IF8", + "type": "n8n-nodes-base.if", + "typeVersion": 1, + "position": [ + 3700, + -400 + ] + }, + { + "parameters": { + "jsCode": "// Loop over input items and add a new field called 'myNewField' to the JSON of each one\nfor (const item of $input.all()) {\n item.json.myNewField = 1;\n}\n\nreturn $input.all();" + }, + "id": "3c97fd14-9c23-45e2-a1ac-934d743e9a01", + "name": "Code8", + "type": "n8n-nodes-base.code", + "typeVersion": 2, + "position": [ + 3260, + -80 + ] + }, + { + "parameters": { + "options": {} + }, + "id": "9e7bd7e9-5142-4751-b132-735d27007d82", + "name": "Edit Fields8", + "type": "n8n-nodes-base.set", + "typeVersion": 3.2, + "position": [ + 3480, + -80 + ] + }, + { + "parameters": {}, + "id": "8d3968b6-16d4-4e03-9026-eeaf70b17805", + "name": "IF9", + "type": "n8n-nodes-base.if", + "typeVersion": 1, + "position": [ + 3700, + -80 + ] + }, + { + "parameters": { + "jsCode": "// Loop over input items and add a new field called 'myNewField' to the JSON of each one\nfor (const item of $input.all()) {\n item.json.myNewField = 1;\n}\n\nreturn $input.all();" + }, + "id": "141edef3-ea0f-4e90-9b6a-09f5d5551195", + "name": "Code4", + "type": "n8n-nodes-base.code", + "typeVersion": 2, + "position": [ + 2560, + 440 + ] + }, + { + "parameters": { + "options": {} + }, + "id": "b5b93cd7-9448-4290-91b7-c3c8429925fd", + "name": "Edit Fields4", + "type": "n8n-nodes-base.set", + "typeVersion": 3.2, + "position": [ + 2780, + 440 + ] + }, + { + "parameters": {}, + "id": "79d2c11c-0378-4ff5-b166-ae1bf773f53a", + "name": "IF14", + "type": "n8n-nodes-base.if", + "typeVersion": 1, + "position": [ + 3000, + 440 + ] + }, + { + "parameters": { + "jsCode": "// Loop over input items and add a new field called 'myNewField' to the JSON of each one\nfor (const item of $input.all()) {\n item.json.myNewField = 1;\n}\n\nreturn $input.all();" + }, + "id": "8483e962-24e7-4495-9c8e-481481ebe897", + "name": "Code13", + "type": "n8n-nodes-base.code", + "typeVersion": 2, + "position": [ + 3260, + 300 + ] + }, + { + "parameters": { + "options": {} + }, + "id": "74dfb8f9-6d14-493e-97d5-729e1f44856b", + "name": "Edit Fields13", + "type": "n8n-nodes-base.set", + "typeVersion": 3.2, + "position": [ + 3480, + 300 + ] + }, + { + "parameters": {}, + "id": "0c2e8e54-958d-4932-91b5-b23979460c97", + "name": "IF15", + "type": "n8n-nodes-base.if", + "typeVersion": 1, + "position": [ + 3700, + 300 + ] + }, + { + "parameters": { + "jsCode": "// Loop over input items and add a new field called 'myNewField' to the JSON of each one\nfor (const item of $input.all()) {\n item.json.myNewField = 1;\n}\n\nreturn $input.all();" + }, + "id": "bfed29c6-c453-4850-8acf-7aa11b1d0d8e", + "name": "Code14", + "type": "n8n-nodes-base.code", + "typeVersion": 2, + "position": [ + 3260, + 620 + ] + }, + { + "parameters": { + "options": {} + }, + "id": "d8415057-c597-40a9-95f6-bafbe3fafac0", + "name": "Edit Fields14", + "type": "n8n-nodes-base.set", + "typeVersion": 3.2, + "position": [ + 3480, + 620 + ] + }, + { + "parameters": {}, + "id": "51ed9040-bb6c-4f77-9740-74b54ac56a00", + "name": "IF16", + "type": "n8n-nodes-base.if", + "typeVersion": 1, + "position": [ + 3700, + 620 + ] + }, + { + "parameters": { + "jsCode": "// Loop over input items and add a new field called 'myNewField' to the JSON of each one\nfor (const item of $input.all()) {\n item.json.myNewField = 1;\n}\n\nreturn $input.all();" + }, + "id": "5864e701-eb16-4412-ae8b-be1f2a1f16a5", + "name": "Code2", + "type": "n8n-nodes-base.code", + "typeVersion": 2, + "position": [ + 1820, + 1480 + ] + }, + { + "parameters": { + "options": {} + }, + "id": "4b7de291-f1c7-4ae8-a545-81aaa2ebd1fb", + "name": "Edit Fields2", + "type": "n8n-nodes-base.set", + "typeVersion": 3.2, + "position": [ + 2040, + 1480 + ] + }, + { + "parameters": {}, + "id": "328aa16f-82ed-465e-b548-9436f21eb519", + "name": "IF2", + "type": "n8n-nodes-base.if", + "typeVersion": 1, + "position": [ + 2260, + 1480 + ] + }, + { + "parameters": { + "jsCode": "// Loop over input items and add a new field called 'myNewField' to the JSON of each one\nfor (const item of $input.all()) {\n item.json.myNewField = 1;\n}\n\nreturn $input.all();" + }, + "id": "90aaf0b0-57b6-4a08-b000-abb2956ba640", + "name": "Code5", + "type": "n8n-nodes-base.code", + "typeVersion": 2, + "position": [ + 2560, + 1140 + ] + }, + { + "parameters": { + "options": {} + }, + "id": "7d327c87-da3b-4f4b-9f9a-51c9c622990d", + "name": "Edit Fields5", + "type": "n8n-nodes-base.set", + "typeVersion": 3.2, + "position": [ + 2780, + 1140 + ] + }, + { + "parameters": {}, + "id": "fa2a3b1b-53de-454e-a16d-e2bf62cb05ec", + "name": "IF21", + "type": "n8n-nodes-base.if", + "typeVersion": 1, + "position": [ + 3000, + 1140 + ] + }, + { + "parameters": { + "jsCode": "// Loop over input items and add a new field called 'myNewField' to the JSON of each one\nfor (const item of $input.all()) {\n item.json.myNewField = 1;\n}\n\nreturn $input.all();" + }, + "id": "8efaa5a3-982e-41b4-af6e-28e35c64093d", + "name": "Code19", + "type": "n8n-nodes-base.code", + "typeVersion": 2, + "position": [ + 3260, + 1000 + ] + }, + { + "parameters": { + "options": {} + }, + "id": "987e27fd-778a-4562-85a9-369b1ec232de", + "name": "Edit Fields19", + "type": "n8n-nodes-base.set", + "typeVersion": 3.2, + "position": [ + 3480, + 1000 + ] + }, + { + "parameters": {}, + "id": "b3f4e9b3-9995-4019-9b0f-dadd64e036b4", + "name": "IF22", + "type": "n8n-nodes-base.if", + "typeVersion": 1, + "position": [ + 3700, + 1000 + ] + }, + { + "parameters": { + "jsCode": "// Loop over input items and add a new field called 'myNewField' to the JSON of each one\nfor (const item of $input.all()) {\n item.json.myNewField = 1;\n}\n\nreturn $input.all();" + }, + "id": "681c1b30-063d-4c1e-b550-942a9dd3eb9a", + "name": "Code20", + "type": "n8n-nodes-base.code", + "typeVersion": 2, + "position": [ + 3260, + 1320 + ] + }, + { + "parameters": { + "options": {} + }, + "id": "024770b6-7bf4-44f6-9675-d4f7dc73d6ac", + "name": "Edit Fields20", + "type": "n8n-nodes-base.set", + "typeVersion": 3.2, + "position": [ + 3480, + 1320 + ] + }, + { + "parameters": {}, + "id": "24699015-3ccf-4ffa-b52f-8ba4c4853963", + "name": "IF23", + "type": "n8n-nodes-base.if", + "typeVersion": 1, + "position": [ + 3700, + 1320 + ] + }, + { + "parameters": { + "jsCode": "// Loop over input items and add a new field called 'myNewField' to the JSON of each one\nfor (const item of $input.all()) {\n item.json.myNewField = 1;\n}\n\nreturn $input.all();" + }, + "id": "f4b2d116-2fda-4a3a-9509-0e8c64e7796e", + "name": "Code6", + "type": "n8n-nodes-base.code", + "typeVersion": 2, + "position": [ + 2560, + 1840 + ] + }, + { + "parameters": { + "options": {} + }, + "id": "535e5b12-6743-4c01-9fc5-e27b10421423", + "name": "Edit Fields6", + "type": "n8n-nodes-base.set", + "typeVersion": 3.2, + "position": [ + 2780, + 1840 + ] + }, + { + "parameters": {}, + "id": "3dcbecdf-686b-445f-9c77-2902d0dc1f56", + "name": "IF28", + "type": "n8n-nodes-base.if", + "typeVersion": 1, + "position": [ + 3000, + 1840 + ] + }, + { + "parameters": { + "jsCode": "// Loop over input items and add a new field called 'myNewField' to the JSON of each one\nfor (const item of $input.all()) {\n item.json.myNewField = 1;\n}\n\nreturn $input.all();" + }, + "id": "7223c6ef-664b-426a-8d08-eca1b34e6b23", + "name": "Code25", + "type": "n8n-nodes-base.code", + "typeVersion": 2, + "position": [ + 3260, + 1700 + ] + }, + { + "parameters": { + "options": {} + }, + "id": "496414a6-384a-4f94-97ec-d2e5ad646f82", + "name": "Edit Fields25", + "type": "n8n-nodes-base.set", + "typeVersion": 3.2, + "position": [ + 3480, + 1700 + ] + }, + { + "parameters": {}, + "id": "82f9562d-e4a8-49f3-924d-983effb4b6c6", + "name": "IF29", + "type": "n8n-nodes-base.if", + "typeVersion": 1, + "position": [ + 3700, + 1700 + ] + }, + { + "parameters": { + "jsCode": "// Loop over input items and add a new field called 'myNewField' to the JSON of each one\nfor (const item of $input.all()) {\n item.json.myNewField = 1;\n}\n\nreturn $input.all();" + }, + "id": "c91d4bc5-3c60-4c22-aa31-44e84e0816ec", + "name": "Code26", + "type": "n8n-nodes-base.code", + "typeVersion": 2, + "position": [ + 3260, + 2020 + ] + }, + { + "parameters": { + "options": {} + }, + "id": "49b61f23-bf3f-474d-8bba-a3b7de6f6441", + "name": "Edit Fields26", + "type": "n8n-nodes-base.set", + "typeVersion": 3.2, + "position": [ + 3480, + 2020 + ] + }, + { + "parameters": {}, + "id": "1cad6ae3-1064-4f30-a9ec-502891868332", + "name": "IF30", + "type": "n8n-nodes-base.if", + "typeVersion": 1, + "position": [ + 3700, + 2020 + ] + } + ], + "pinData": {}, + "connections": { + "When clicking \"Execute Workflow\"": { + "main": [ + [ + { + "node": "Code", + "type": "main", + "index": 0 + } + ] + ] + }, + "Code": { + "main": [ + [ + { + "node": "Edit Fields", + "type": "main", + "index": 0 + } + ] + ] + }, + "Edit Fields": { + "main": [ + [ + { + "node": "IF", + "type": "main", + "index": 0 + } + ] + ] + }, + "IF": { + "main": [ + [ + { + "node": "Code1", + "type": "main", + "index": 0 + } + ], + [ + { + "node": "Code2", + "type": "main", + "index": 0 + } + ] + ] + }, + "Code1": { + "main": [ + [ + { + "node": "Edit Fields1", + "type": "main", + "index": 0 + } + ] + ] + }, + "Edit Fields1": { + "main": [ + [ + { + "node": "IF1", + "type": "main", + "index": 0 + } + ] + ] + }, + "Code3": { + "main": [ + [ + { + "node": "Edit Fields3", + "type": "main", + "index": 0 + } + ] + ] + }, + "Edit Fields3": { + "main": [ + [ + { + "node": "IF7", + "type": "main", + "index": 0 + } + ] + ] + }, + "IF1": { + "main": [ + [ + { + "node": "Code3", + "type": "main", + "index": 0 + } + ], + [ + { + "node": "Code4", + "type": "main", + "index": 0 + } + ] + ] + }, + "IF7": { + "main": [ + [ + { + "node": "Code7", + "type": "main", + "index": 0 + } + ], + [ + { + "node": "Code8", + "type": "main", + "index": 0 + } + ] + ] + }, + "Code7": { + "main": [ + [ + { + "node": "Edit Fields7", + "type": "main", + "index": 0 + } + ] + ] + }, + "Edit Fields7": { + "main": [ + [ + { + "node": "IF8", + "type": "main", + "index": 0 + } + ] + ] + }, + "Code8": { + "main": [ + [ + { + "node": "Edit Fields8", + "type": "main", + "index": 0 + } + ] + ] + }, + "Edit Fields8": { + "main": [ + [ + { + "node": "IF9", + "type": "main", + "index": 0 + } + ] + ] + }, + "Code4": { + "main": [ + [ + { + "node": "Edit Fields4", + "type": "main", + "index": 0 + } + ] + ] + }, + "Edit Fields4": { + "main": [ + [ + { + "node": "IF14", + "type": "main", + "index": 0 + } + ] + ] + }, + "IF14": { + "main": [ + [ + { + "node": "Code13", + "type": "main", + "index": 0 + } + ], + [ + { + "node": "Code14", + "type": "main", + "index": 0 + } + ] + ] + }, + "Code13": { + "main": [ + [ + { + "node": "Edit Fields13", + "type": "main", + "index": 0 + } + ] + ] + }, + "Edit Fields13": { + "main": [ + [ + { + "node": "IF15", + "type": "main", + "index": 0 + } + ] + ] + }, + "Code14": { + "main": [ + [ + { + "node": "Edit Fields14", + "type": "main", + "index": 0 + } + ] + ] + }, + "Edit Fields14": { + "main": [ + [ + { + "node": "IF16", + "type": "main", + "index": 0 + } + ] + ] + }, + "Code2": { + "main": [ + [ + { + "node": "Edit Fields2", + "type": "main", + "index": 0 + } + ] + ] + }, + "Edit Fields2": { + "main": [ + [ + { + "node": "IF2", + "type": "main", + "index": 0 + } + ] + ] + }, + "IF2": { + "main": [ + [ + { + "node": "Code5", + "type": "main", + "index": 0 + } + ], + [ + { + "node": "Code6", + "type": "main", + "index": 0 + } + ] + ] + }, + "Code5": { + "main": [ + [ + { + "node": "Edit Fields5", + "type": "main", + "index": 0 + } + ] + ] + }, + "Edit Fields5": { + "main": [ + [ + { + "node": "IF21", + "type": "main", + "index": 0 + } + ] + ] + }, + "IF21": { + "main": [ + [ + { + "node": "Code19", + "type": "main", + "index": 0 + } + ], + [ + { + "node": "Code20", + "type": "main", + "index": 0 + } + ] + ] + }, + "Code19": { + "main": [ + [ + { + "node": "Edit Fields19", + "type": "main", + "index": 0 + } + ] + ] + }, + "Edit Fields19": { + "main": [ + [ + { + "node": "IF22", + "type": "main", + "index": 0 + } + ] + ] + }, + "Code20": { + "main": [ + [ + { + "node": "Edit Fields20", + "type": "main", + "index": 0 + } + ] + ] + }, + "Edit Fields20": { + "main": [ + [ + { + "node": "IF23", + "type": "main", + "index": 0 + } + ] + ] + }, + "Code6": { + "main": [ + [ + { + "node": "Edit Fields6", + "type": "main", + "index": 0 + } + ] + ] + }, + "Edit Fields6": { + "main": [ + [ + { + "node": "IF28", + "type": "main", + "index": 0 + } + ] + ] + }, + "IF28": { + "main": [ + [ + { + "node": "Code25", + "type": "main", + "index": 0 + } + ], + [ + { + "node": "Code26", + "type": "main", + "index": 0 + } + ] + ] + }, + "Code25": { + "main": [ + [ + { + "node": "Edit Fields25", + "type": "main", + "index": 0 + } + ] + ] + }, + "Edit Fields25": { + "main": [ + [ + { + "node": "IF29", + "type": "main", + "index": 0 + } + ] + ] + }, + "Code26": { + "main": [ + [ + { + "node": "Edit Fields26", + "type": "main", + "index": 0 + } + ] + ] + }, + "Edit Fields26": { + "main": [ + [ + { + "node": "IF30", + "type": "main", + "index": 0 + } + ] + ] + } + }, + "active": false, + "settings": { + "executionOrder": "v1" + }, + "versionId": "d38289e0-49d3-4e1d-8e4b-46e4eb85a2c9", + "id": "iKlx4AGIjCNJSu9M", + "meta": { + "instanceId": "8a47b83b4479b11330fdf21ccc96d4a8117035a968612e452b4c87bfd09c16c7" + }, + "tags": [] +} \ No newline at end of file diff --git a/packages/editor-ui/src/views/NodeView.vue b/packages/editor-ui/src/views/NodeView.vue index b8228bc40d8bc..fe5d66b823173 100644 --- a/packages/editor-ui/src/views/NodeView.vue +++ b/packages/editor-ui/src/views/NodeView.vue @@ -4090,6 +4090,7 @@ export default defineComponent({ this.onToggleNodeCreator({ createNodeActive: false }); this.nodeCreatorStore.setShowScrim(false); + this.canvasStore.resetZoom(); // Reset nodes this.unbindEndpointEventListeners(); From 151e60f829663e79982aae6ac1cd8489f3083224 Mon Sep 17 00:00:00 2001 From: Mutasem Aldmour <4711238+mutdmour@users.noreply.github.com> Date: Tue, 7 Nov 2023 10:06:08 +0100 Subject: [PATCH 10/24] fix(editor): Fix local storage flags defaulting to undefined string (#7603) useStorage takes the default value `undefined` and sets it in local storage.. also returns "undefined" as string which breaks onboarding flows Github issue / Community forum post (link here to close automatically): --- .../src/components/ActivationModal.vue | 3 +- .../src/components/NDVDraggablePanels.vue | 9 ++-- packages/editor-ui/src/components/Node.vue | 9 ++-- packages/editor-ui/src/components/RunData.vue | 11 ++--- .../src/composables/useStorage.test.ts | 48 +++++++++++++++++++ .../editor-ui/src/composables/useStorage.ts | 13 +++++ .../editor-ui/src/mixins/workflowActivate.ts | 7 +-- packages/editor-ui/src/router.ts | 5 +- .../src/stores/__tests__/posthog.test.ts | 13 ++--- packages/editor-ui/src/stores/ndv.store.ts | 6 +-- .../editor-ui/src/stores/posthog.store.ts | 6 +-- packages/editor-ui/src/stores/ui.utils.ts | 9 ++-- 12 files changed, 97 insertions(+), 42 deletions(-) create mode 100644 packages/editor-ui/src/composables/useStorage.test.ts create mode 100644 packages/editor-ui/src/composables/useStorage.ts diff --git a/packages/editor-ui/src/components/ActivationModal.vue b/packages/editor-ui/src/components/ActivationModal.vue index 6b2e7274bcb73..a8c119c2e2bee 100644 --- a/packages/editor-ui/src/components/ActivationModal.vue +++ b/packages/editor-ui/src/components/ActivationModal.vue @@ -50,6 +50,7 @@ import { getActivatableTriggerNodes, getTriggerNodeServiceName } from '@/utils'; import { useUIStore } from '@/stores/ui.store'; import { useWorkflowsStore } from '@/stores/workflows.store'; import { useNodeTypesStore } from '@/stores/nodeTypes.store'; +import { useStorage } from '@/composables/useStorage'; export default defineComponent({ name: 'ActivationModal', @@ -88,7 +89,7 @@ export default defineComponent({ }, handleCheckboxChange(checkboxValue: boolean) { this.checked = checkboxValue; - window.localStorage.setItem(LOCAL_STORAGE_ACTIVATION_FLAG, checkboxValue.toString()); + useStorage(LOCAL_STORAGE_ACTIVATION_FLAG).value = checkboxValue.toString(); }, }, computed: { diff --git a/packages/editor-ui/src/components/NDVDraggablePanels.vue b/packages/editor-ui/src/components/NDVDraggablePanels.vue index 8c96ebf154c69..5851aa85b1aed 100644 --- a/packages/editor-ui/src/components/NDVDraggablePanels.vue +++ b/packages/editor-ui/src/components/NDVDraggablePanels.vue @@ -41,7 +41,7 @@ import { defineComponent } from 'vue'; import type { PropType } from 'vue'; import { mapStores } from 'pinia'; import { get } from 'lodash-es'; -import { useStorage } from '@vueuse/core'; +import { useStorage } from '@/composables/useStorage'; import type { INodeTypeDescription } from 'n8n-workflow'; import PanelDragButton from './PanelDragButton.vue'; @@ -348,7 +348,6 @@ export default defineComponent({ restorePositionData() { const storedPanelWidthData = useStorage( `${LOCAL_STORAGE_MAIN_PANEL_RELATIVE_WIDTH}_${this.currentNodePaneType}`, - undefined, ).value; if (storedPanelWidthData) { @@ -362,10 +361,8 @@ export default defineComponent({ return false; }, storePositionData() { - window.localStorage.setItem( - `${LOCAL_STORAGE_MAIN_PANEL_RELATIVE_WIDTH}_${this.currentNodePaneType}`, - this.mainPanelDimensions.relativeWidth.toString(), - ); + useStorage(`${LOCAL_STORAGE_MAIN_PANEL_RELATIVE_WIDTH}_${this.currentNodePaneType}`).value = + this.mainPanelDimensions.relativeWidth.toString(); }, onDragStart() { this.isDragging = true; diff --git a/packages/editor-ui/src/components/Node.vue b/packages/editor-ui/src/components/Node.vue index da67476079ad8..615aab0c5f609 100644 --- a/packages/editor-ui/src/components/Node.vue +++ b/packages/editor-ui/src/components/Node.vue @@ -170,7 +170,7 @@