From b8e3bcc052fcea887a4a964547866e966e20ccce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Tue, 14 Jun 2022 18:32:19 +0200 Subject: [PATCH] refactor(core): Post-release refactorings of Public API (#3495) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * :zap: Post-release refactorings * :test_tube: Add `--forceExit` * 🛠 typing refactor (#3486) * :bug: Fix middleware arguments * :shirt: Fix lint * :zap: Restore commented out block Co-authored-by: Ben Hesseldieck <1849459+BHesseldieck@users.noreply.github.com> --- packages/cli/package.json | 2 +- packages/cli/src/Interfaces.ts | 4 +- packages/cli/src/PublicApi/index.ts | 52 ++++++------ .../credentials/credentials.handler.ts | 39 ++++----- .../credentials/credentials.middleware.ts | 43 +++++----- .../credentials/credentials.service.ts | 3 +- .../handlers/executions/executions.handler.ts | 54 ++++--------- .../handlers/executions/executions.service.ts | 33 ++++---- .../handlers/workflows/workflows.handler.ts | 79 +++++++------------ .../handlers/workflows/workflows.service.ts | 54 ++++++------- .../shared/middlewares/global.middleware.ts | 42 ++++++---- .../v1/shared/services/pagination.service.ts | 1 - packages/cli/src/requests.d.ts | 6 +- .../integration/publicApi/credentials.test.ts | 9 +-- .../integration/publicApi/executions.test.ts | 65 ++++++++------- .../integration/publicApi/workflows.test.ts | 79 +++++++++---------- .../cli/test/integration/shared/testDb.ts | 29 +++---- .../cli/test/integration/shared/types.d.ts | 9 +++ packages/cli/test/integration/shared/utils.ts | 47 +++++------ 19 files changed, 307 insertions(+), 343 deletions(-) diff --git a/packages/cli/package.json b/packages/cli/package.json index 85b9ca1894..b29c6c7a56 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -30,7 +30,7 @@ "start:default": "cd bin && ./n8n", "start:windows": "cd bin && n8n", "test": "npm run test:sqlite", - "test:sqlite": "export N8N_LOG_LEVEL=silent; export DB_TYPE=sqlite; jest", + "test:sqlite": "export N8N_LOG_LEVEL=silent; export DB_TYPE=sqlite; jest --forceExit", "test:postgres": "export N8N_LOG_LEVEL=silent; export DB_TYPE=postgresdb; jest", "test:postgres:alt-schema": "export DB_POSTGRESDB_SCHEMA=alt_schema; npm run test:postgres", "test:mysql": "export N8N_LOG_LEVEL=silent; export DB_TYPE=mysqldb; jest", diff --git a/packages/cli/src/Interfaces.ts b/packages/cli/src/Interfaces.ts index 3a14fd5b32..d413810725 100644 --- a/packages/cli/src/Interfaces.ts +++ b/packages/cli/src/Interfaces.ts @@ -178,8 +178,6 @@ export interface ICredentialsDecryptedResponse extends ICredentialsDecryptedDb { export type DatabaseType = 'mariadb' | 'postgresdb' | 'mysqldb' | 'sqlite'; export type SaveExecutionDataType = 'all' | 'none'; -export type ExecutionDataFieldFormat = 'empty' | 'flattened' | 'json'; - export interface IExecutionBase { id?: number | string; mode: WorkflowExecuteMode; @@ -240,7 +238,7 @@ export interface IExecutionResponseApi { finished: boolean; retryOf?: number | string; retrySuccessId?: number | string; - data?: string; // Just that we can remove it + data?: object; waitTill?: Date | null; workflowData: IWorkflowBase; } diff --git a/packages/cli/src/PublicApi/index.ts b/packages/cli/src/PublicApi/index.ts index 5a8585148c..19681e798d 100644 --- a/packages/cli/src/PublicApi/index.ts +++ b/packages/cli/src/PublicApi/index.ts @@ -1,38 +1,38 @@ -/* eslint-disable @typescript-eslint/no-unused-vars */ -/* eslint-disable no-restricted-syntax */ /* eslint-disable @typescript-eslint/naming-convention */ -/* eslint-disable @typescript-eslint/no-unsafe-call */ /* eslint-disable import/no-cycle */ import express, { Router } from 'express'; +import fs from 'fs/promises'; +import path from 'path'; + import * as OpenApiValidator from 'express-openapi-validator'; import { HttpError } from 'express-openapi-validator/dist/framework/types'; -import fs from 'fs/promises'; import { OpenAPIV3 } from 'openapi-types'; -import path from 'path'; -import * as swaggerUi from 'swagger-ui-express'; +import swaggerUi from 'swagger-ui-express'; import validator from 'validator'; -import * as YAML from 'yamljs'; -import { Db, InternalHooksManager } from '..'; +import YAML from 'yamljs'; + import config from '../../config'; +import { Db, InternalHooksManager } from '..'; import { getInstanceBaseUrl } from '../UserManagement/UserManagementHelper'; function createApiRouter( version: string, openApiSpecPath: string, - hanldersDirectory: string, + handlersDirectory: string, swaggerThemeCss: string, publicApiEndpoint: string, ): Router { const n8nPath = config.getEnv('path'); const swaggerDocument = YAML.load(openApiSpecPath) as swaggerUi.JsonObject; // add the server depeding on the config so the user can interact with the API - // from the swagger UI + // from the Swagger UI swaggerDocument.server = [ { url: `${getInstanceBaseUrl()}/${publicApiEndpoint}/${version}}`, }, ]; const apiController = express.Router(); + apiController.use( `/${publicApiEndpoint}/${version}/docs`, swaggerUi.serveFiles(swaggerDocument), @@ -42,12 +42,14 @@ function createApiRouter( customfavIcon: `${n8nPath}favicon.ico`, }), ); + apiController.use(`/${publicApiEndpoint}/${version}`, express.json()); + apiController.use( `/${publicApiEndpoint}/${version}`, OpenApiValidator.middleware({ apiSpec: openApiSpecPath, - operationHandlers: hanldersDirectory, + operationHandlers: handlersDirectory, validateRequests: true, validateApiSpec: true, formats: [ @@ -71,16 +73,12 @@ function createApiRouter( schema: OpenAPIV3.ApiKeySecurityScheme, ): Promise => { const apiKey = req.headers[schema.name.toLowerCase()]; - const user = await Db.collections.User?.findOne({ - where: { - apiKey, - }, + const user = await Db.collections.User.findOne({ + where: { apiKey }, relations: ['globalRole'], }); - if (!user) { - return false; - } + if (!user) return false; void InternalHooksManager.getInstance().onUserInvokedApi({ user_id: user.id, @@ -97,13 +95,20 @@ function createApiRouter( }, }), ); + apiController.use( - (error: HttpError, req: express.Request, res: express.Response, next: express.NextFunction) => { + ( + error: HttpError, + _req: express.Request, + res: express.Response, + _next: express.NextFunction, + ) => { return res.status(error.status || 400).json({ message: error.message, }); }, ); + return apiController; } @@ -114,11 +119,12 @@ export const loadPublicApiVersions = async ( const folders = await fs.readdir(__dirname); const css = (await fs.readFile(swaggerThemePath)).toString(); const versions = folders.filter((folderName) => folderName.startsWith('v')); - const apiRouters: express.Router[] = []; - for (const version of versions) { + + const apiRouters = versions.map((version) => { const openApiPath = path.join(__dirname, version, 'openapi.yml'); - apiRouters.push(createApiRouter(version, openApiPath, __dirname, css, publicApiEndpoint)); - } + return createApiRouter(version, openApiPath, __dirname, css, publicApiEndpoint); + }); + return { apiRouters, apiLatestVersion: Number(versions.pop()?.charAt(1)) ?? 1, diff --git a/packages/cli/src/PublicApi/v1/handlers/credentials/credentials.handler.ts b/packages/cli/src/PublicApi/v1/handlers/credentials/credentials.handler.ts index 6896706c34..6e9ecad881 100644 --- a/packages/cli/src/PublicApi/v1/handlers/credentials/credentials.handler.ts +++ b/packages/cli/src/PublicApi/v1/handlers/credentials/credentials.handler.ts @@ -1,7 +1,7 @@ -import express = require('express'); +import express from 'express'; + import { CredentialsHelper } from '../../../../CredentialsHelper'; import { CredentialTypes } from '../../../../CredentialTypes'; - import { CredentialsEntity } from '../../../../databases/entities/CredentialsEntity'; import { CredentialRequest } from '../../../../requests'; import { CredentialTypeRequest } from '../../../types'; @@ -29,7 +29,7 @@ export = { res: express.Response, ): Promise>> => { try { - const newCredential = await createCredential(req.body as Partial); + const newCredential = await createCredential(req.body); const encryptedData = await encryptCredential(newCredential); @@ -56,7 +56,7 @@ export = { res: express.Response, ): Promise>> => { const { id: credentialId } = req.params; - let credentials: CredentialsEntity | undefined; + let credential: CredentialsEntity | undefined; if (req.user.globalRole.name !== 'owner') { const shared = await getSharedCredentials(req.user.id, credentialId, [ @@ -65,27 +65,20 @@ export = { ]); if (shared?.role.name === 'owner') { - credentials = shared.credentials; - } else { - // LoggerProxy.info('Attempt to delete credential blocked due to lack of permissions', { - // credentialId, - // userId: req.user.id, - // }); + credential = shared.credentials; } } else { - credentials = (await getCredentials(credentialId)) as CredentialsEntity; + credential = (await getCredentials(credentialId)) as CredentialsEntity; } - if (!credentials) { - return res.status(404).json({ - message: 'Not Found', - }); + if (!credential) { + return res.status(404).json({ message: 'Not Found' }); } - await removeCredential(credentials); - credentials.id = Number(credentialId); + await removeCredential(credential); + credential.id = Number(credentialId); - return res.json(sanitizeCredentials(credentials)); + return res.json(sanitizeCredentials(credential)); }, ], @@ -97,14 +90,12 @@ export = { try { CredentialTypes().getByName(credentialTypeName); } catch (error) { - return res.status(404).json({ - message: 'Not Found', - }); + return res.status(404).json({ message: 'Not Found' }); } - let schema = new CredentialsHelper('').getCredentialsProperties(credentialTypeName); - - schema = schema.filter((nodeProperty) => nodeProperty.type !== 'hidden'); + const schema = new CredentialsHelper('') + .getCredentialsProperties(credentialTypeName) + .filter((property) => property.type !== 'hidden'); return res.json(toJsonSchema(schema)); }, diff --git a/packages/cli/src/PublicApi/v1/handlers/credentials/credentials.middleware.ts b/packages/cli/src/PublicApi/v1/handlers/credentials/credentials.middleware.ts index db4c35cf82..f40e21504f 100644 --- a/packages/cli/src/PublicApi/v1/handlers/credentials/credentials.middleware.ts +++ b/packages/cli/src/PublicApi/v1/handlers/credentials/credentials.middleware.ts @@ -1,37 +1,36 @@ -/* eslint-disable @typescript-eslint/no-explicit-any */ -/* eslint-disable consistent-return */ -import { RequestHandler } from 'express'; +/* eslint-disable @typescript-eslint/no-invalid-void-type */ + +import express from 'express'; import { validate } from 'jsonschema'; + import { CredentialsHelper, CredentialTypes } from '../../../..'; import { CredentialRequest } from '../../../types'; import { toJsonSchema } from './credentials.service'; -export const validCredentialType: RequestHandler = async ( +export const validCredentialType = ( req: CredentialRequest.Create, - res, - next, -): Promise => { - const { type } = req.body; + res: express.Response, + next: express.NextFunction, +): express.Response | void => { try { - CredentialTypes().getByName(type); - } catch (error) { - return res.status(400).json({ - message: 'req.body.type is not a known type', - }); + CredentialTypes().getByName(req.body.type); + } catch (_) { + return res.status(400).json({ message: 'req.body.type is not a known type' }); } - next(); + + return next(); }; -export const validCredentialsProperties: RequestHandler = async ( +export const validCredentialsProperties = ( req: CredentialRequest.Create, - res, - next, -): Promise => { + res: express.Response, + next: express.NextFunction, +): express.Response | void => { const { type, data } = req.body; - let properties = new CredentialsHelper('').getCredentialsProperties(type); - - properties = properties.filter((nodeProperty) => nodeProperty.type !== 'hidden'); + const properties = new CredentialsHelper('') + .getCredentialsProperties(type) + .filter((property) => property.type !== 'hidden'); const schema = toJsonSchema(properties); @@ -43,5 +42,5 @@ export const validCredentialsProperties: RequestHandler = async ( }); } - next(); + return next(); }; diff --git a/packages/cli/src/PublicApi/v1/handlers/credentials/credentials.service.ts b/packages/cli/src/PublicApi/v1/handlers/credentials/credentials.service.ts index 2239b89821..304e147460 100644 --- a/packages/cli/src/PublicApi/v1/handlers/credentials/credentials.service.ts +++ b/packages/cli/src/PublicApi/v1/handlers/credentials/credentials.service.ts @@ -11,6 +11,7 @@ import { SharedCredentials } from '../../../../databases/entities/SharedCredenti import { User } from '../../../../databases/entities/User'; import { externalHooks } from '../../../../Server'; import { IDependency, IJsonSchema } from '../../../types'; +import { CredentialRequest } from '../../../../requests'; export async function getCredentials( credentialId: number | string, @@ -38,7 +39,7 @@ export async function getSharedCredentials( } export async function createCredential( - properties: Partial, + properties: CredentialRequest.CredentialProperties, ): Promise { const newCredential = new CredentialsEntity(); diff --git a/packages/cli/src/PublicApi/v1/handlers/executions/executions.handler.ts b/packages/cli/src/PublicApi/v1/handlers/executions/executions.handler.ts index c8f98b8b1a..bd94da1d50 100644 --- a/packages/cli/src/PublicApi/v1/handlers/executions/executions.handler.ts +++ b/packages/cli/src/PublicApi/v1/handlers/executions/executions.handler.ts @@ -1,16 +1,15 @@ -import express = require('express'); +import express from 'express'; import { BinaryDataManager } from 'n8n-core'; + import { getExecutions, getExecutionInWorkflows, deleteExecution, getExecutionsCount, } from './executions.service'; - import { ActiveExecutions } from '../../../..'; import { authorize, validCursor } from '../../shared/middlewares/global.middleware'; - import { ExecutionRequest } from '../../../types'; import { getSharedWorkflowIds } from '../workflows/workflows.service'; import { encodeNextCursor } from '../../shared/services/pagination.service'; @@ -20,31 +19,24 @@ export = { deleteExecution: [ authorize(['owner', 'member']), async (req: ExecutionRequest.Delete, res: express.Response): Promise => { - const { id } = req.params; - const sharedWorkflowsIds = await getSharedWorkflowIds(req.user); // user does not have workflows hence no executions // or the execution he is trying to access belongs to a workflow he does not own if (!sharedWorkflowsIds.length) { - return res.status(404).json({ - message: 'Not Found', - }); + return res.status(404).json({ message: 'Not Found' }); } + const { id } = req.params; + // look for the execution on the workflow the user owns const execution = await getExecutionInWorkflows(id, sharedWorkflowsIds, false); - // execution was not found if (!execution) { - return res.status(404).json({ - message: 'Not Found', - }); + return res.status(404).json({ message: 'Not Found' }); } - const binaryDataManager = BinaryDataManager.getInstance(); - - await binaryDataManager.deleteBinaryDataByExecutionId(execution.id.toString()); + await BinaryDataManager.getInstance().deleteBinaryDataByExecutionId(execution.id.toString()); await deleteExecution(execution); @@ -56,35 +48,28 @@ export = { getExecution: [ authorize(['owner', 'member']), async (req: ExecutionRequest.Get, res: express.Response): Promise => { - const { id } = req.params; - const { includeData = false } = req.query; - const sharedWorkflowsIds = await getSharedWorkflowIds(req.user); // user does not have workflows hence no executions // or the execution he is trying to access belongs to a workflow he does not own if (!sharedWorkflowsIds.length) { - return res.status(404).json({ - message: 'Not Found', - }); + return res.status(404).json({ message: 'Not Found' }); } + const { id } = req.params; + const { includeData = false } = req.query; + // look for the execution on the workflow the user owns const execution = await getExecutionInWorkflows(id, sharedWorkflowsIds, includeData); - // execution was not found if (!execution) { - return res.status(404).json({ - message: 'Not Found', - }); + return res.status(404).json({ message: 'Not Found' }); } - const telemetryData = { + void InternalHooksManager.getInstance().onUserRetrievedExecution({ user_id: req.user.id, public_api: true, - }; - - void InternalHooksManager.getInstance().onUserRetrievedExecution(telemetryData); + }); return res.json(execution); }, @@ -106,10 +91,7 @@ export = { // user does not have workflows hence no executions // or the execution he is trying to access belongs to a workflow he does not own if (!sharedWorkflowsIds.length) { - return res.status(200).json({ - data: [], - nextCursor: null, - }); + return res.status(200).json({ data: [], nextCursor: null }); } // get running workflows so we exclude them from the result @@ -134,12 +116,10 @@ export = { const count = await getExecutionsCount(filters); - const telemetryData = { + void InternalHooksManager.getInstance().onUserRetrievedAllExecutions({ user_id: req.user.id, public_api: true, - }; - - void InternalHooksManager.getInstance().onUserRetrievedAllExecutions(telemetryData); + }); return res.json({ data: executions, diff --git a/packages/cli/src/PublicApi/v1/handlers/executions/executions.service.ts b/packages/cli/src/PublicApi/v1/handlers/executions/executions.service.ts index 8cdac1d6cc..5f05a8510b 100644 --- a/packages/cli/src/PublicApi/v1/handlers/executions/executions.service.ts +++ b/packages/cli/src/PublicApi/v1/handlers/executions/executions.service.ts @@ -1,23 +1,20 @@ import { parse } from 'flatted'; import { In, Not, ObjectLiteral, LessThan, IsNull } from 'typeorm'; + import { Db, IExecutionFlattedDb, IExecutionResponseApi } from '../../../..'; import { ExecutionStatus } from '../../../types'; function prepareExecutionData( execution: IExecutionFlattedDb | undefined, ): IExecutionResponseApi | undefined { - if (execution === undefined) { - return undefined; - } + if (!execution) return undefined; - if (!execution.data) { - return execution; - } + // @ts-ignore + if (!execution.data) return execution; return { ...execution, - // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment - data: parse(execution.data), + data: parse(execution.data) as object, }; } @@ -32,11 +29,12 @@ function getStatusCondition(status: ExecutionStatus): ObjectLiteral { condition.stoppedAt = Not(IsNull()); condition.finished = false; } + return condition; } function getExecutionSelectableProperties(includeData?: boolean): Array { - const returnData: Array = [ + const selectFields: Array = [ 'id', 'mode', 'retryOf', @@ -47,10 +45,10 @@ function getExecutionSelectableProperties(includeData?: boolean): Array { - await Db.collections.Execution.remove(execution as IExecutionFlattedDb); +export async function deleteExecution( + execution: IExecutionResponseApi | undefined, +): Promise { + // @ts-ignore + return Db.collections.Execution.remove(execution); } diff --git a/packages/cli/src/PublicApi/v1/handlers/workflows/workflows.handler.ts b/packages/cli/src/PublicApi/v1/handlers/workflows/workflows.handler.ts index 23ff02a7d4..d45a33eeaf 100644 --- a/packages/cli/src/PublicApi/v1/handlers/workflows/workflows.handler.ts +++ b/packages/cli/src/PublicApi/v1/handlers/workflows/workflows.handler.ts @@ -1,5 +1,7 @@ -import express = require('express'); +import express from 'express'; + import { FindManyOptions, In } from 'typeorm'; + import { ActiveWorkflowRunner, Db } from '../../../..'; import config = require('../../../../../config'); import { WorkflowEntity } from '../../../../databases/entities/WorkflowEntity'; @@ -30,25 +32,24 @@ export = { createWorkflow: [ authorize(['owner', 'member']), async (req: WorkflowRequest.Create, res: express.Response): Promise => { - let workflow = req.body; + const workflow = req.body; workflow.active = false; - // if the workflow does not have a start node, add it. if (!hasStartNode(workflow)) { workflow.nodes.push(getStartNode()); } - const role = await getWorkflowOwnerRole(); - await replaceInvalidCredentials(workflow); - workflow = await createWorkflow(workflow, req.user, role); + const role = await getWorkflowOwnerRole(); - await externalHooks.run('workflow.afterCreate', [workflow]); - void InternalHooksManager.getInstance().onWorkflowCreated(req.user.id, workflow, true); + const createdWorkflow = await createWorkflow(workflow, req.user, role); - return res.json(workflow); + await externalHooks.run('workflow.afterCreate', [createdWorkflow]); + void InternalHooksManager.getInstance().onWorkflowCreated(req.user.id, createdWorkflow, true); + + return res.json(createdWorkflow); }, ], deleteWorkflow: [ @@ -61,16 +62,12 @@ export = { if (!sharedWorkflow) { // user trying to access a workflow he does not own // or workflow does not exist - return res.status(404).json({ - message: 'Not Found', - }); + return res.status(404).json({ message: 'Not Found' }); } - const workflowRunner = ActiveWorkflowRunner.getInstance(); - if (sharedWorkflow.workflow.active) { // deactivate before deleting - await workflowRunner.remove(id.toString()); + await ActiveWorkflowRunner.getInstance().remove(id.toString()); } await Db.collections.Workflow.delete(id); @@ -91,17 +88,13 @@ export = { if (!sharedWorkflow) { // user trying to access a workflow he does not own // or workflow does not exist - return res.status(404).json({ - message: 'Not Found', - }); + return res.status(404).json({ message: 'Not Found' }); } - const telemetryData = { + void InternalHooksManager.getInstance().onUserRetrievedWorkflow({ user_id: req.user.id, public_api: true, - }; - - void InternalHooksManager.getInstance().onUserRetrievedWorkflow(telemetryData); + }); return res.json(sharedWorkflow.workflow); }, @@ -158,12 +151,10 @@ export = { count = await getWorkflowsCount(query); } - const telemetryData = { + void InternalHooksManager.getInstance().onUserRetrievedAllWorkflows({ user_id: req.user.id, public_api: true, - }; - - void InternalHooksManager.getInstance().onUserRetrievedAllWorkflows(telemetryData); + }); return res.json({ data: workflows, @@ -187,18 +178,13 @@ export = { if (!sharedWorkflow) { // user trying to access a workflow he does not own // or workflow does not exist - return res.status(404).json({ - message: 'Not Found', - }); + return res.status(404).json({ message: 'Not Found' }); } - // if the workflow does not have a start node, add it. - // else there is nothing you can do in IU if (!hasStartNode(updateData)) { updateData.nodes.push(getStartNode()); } - // check credentials for old format await replaceInvalidCredentials(updateData); const workflowRunner = ActiveWorkflowRunner.getInstance(); @@ -215,10 +201,9 @@ export = { try { await workflowRunner.add(sharedWorkflow.workflowId.toString(), 'update'); } catch (error) { - // todo - // remove the type assertion - const errorObject = error as unknown as { message: string }; - return res.status(400).json({ error: errorObject.message }); + if (error instanceof Error) { + return res.status(400).json({ message: error.message }); + } } } @@ -240,21 +225,19 @@ export = { if (!sharedWorkflow) { // user trying to access a workflow he does not own // or workflow does not exist - return res.status(404).json({ - message: 'Not Found', - }); + return res.status(404).json({ message: 'Not Found' }); } - const workflowRunner = ActiveWorkflowRunner.getInstance(); - if (!sharedWorkflow.workflow.active) { try { - await workflowRunner.add(sharedWorkflow.workflowId.toString(), 'activate'); + await ActiveWorkflowRunner.getInstance().add( + sharedWorkflow.workflowId.toString(), + 'activate', + ); } catch (error) { - // todo - // remove the type assertion - const errorObject = error as unknown as { message: string }; - return res.status(400).json({ error: errorObject.message }); + if (error instanceof Error) { + return res.status(400).json({ message: error.message }); + } } // change the status to active in the DB @@ -279,9 +262,7 @@ export = { if (!sharedWorkflow) { // user trying to access a workflow he does not own // or workflow does not exist - return res.status(404).json({ - message: 'Not Found', - }); + return res.status(404).json({ message: 'Not Found' }); } const workflowRunner = ActiveWorkflowRunner.getInstance(); diff --git a/packages/cli/src/PublicApi/v1/handlers/workflows/workflows.service.ts b/packages/cli/src/PublicApi/v1/handlers/workflows/workflows.service.ts index 2e62180a24..e50dc3e4b8 100644 --- a/packages/cli/src/PublicApi/v1/handlers/workflows/workflows.service.ts +++ b/packages/cli/src/PublicApi/v1/handlers/workflows/workflows.service.ts @@ -1,19 +1,19 @@ +import { FindManyOptions, In, UpdateResult } from 'typeorm'; import { intersection } from 'lodash'; import type { INode } from 'n8n-workflow'; -import { FindManyOptions, In, UpdateResult } from 'typeorm'; + +import { Db } from '../../../..'; import { User } from '../../../../databases/entities/User'; import { WorkflowEntity } from '../../../../databases/entities/WorkflowEntity'; -import { Db } from '../../../..'; import { SharedWorkflow } from '../../../../databases/entities/SharedWorkflow'; import { isInstanceOwner } from '../users/users.service'; import { Role } from '../../../../databases/entities/Role'; export async function getSharedWorkflowIds(user: User): Promise { const sharedWorkflows = await Db.collections.SharedWorkflow.find({ - where: { - user, - }, + where: { user }, }); + return sharedWorkflows.map((workflow) => workflow.workflowId); } @@ -21,14 +21,13 @@ export async function getSharedWorkflow( user: User, workflowId?: string | undefined, ): Promise { - const sharedWorkflow = await Db.collections.SharedWorkflow.findOne({ + return Db.collections.SharedWorkflow.findOne({ where: { ...(!isInstanceOwner(user) && { user }), ...(workflowId && { workflow: { id: workflowId } }), }, relations: ['workflow'], }); - return sharedWorkflow; } export async function getSharedWorkflows( @@ -38,23 +37,19 @@ export async function getSharedWorkflows( workflowIds?: number[]; }, ): Promise { - const sharedWorkflows = await Db.collections.SharedWorkflow.find({ + return Db.collections.SharedWorkflow.find({ where: { ...(!isInstanceOwner(user) && { user }), ...(options.workflowIds && { workflow: { id: In(options.workflowIds) } }), }, ...(options.relations && { relations: options.relations }), }); - return sharedWorkflows; } export async function getWorkflowById(id: number): Promise { - const workflow = await Db.collections.Workflow.findOne({ - where: { - id, - }, + return Db.collections.Workflow.findOne({ + where: { id }, }); - return workflow; } /** @@ -63,9 +58,7 @@ export async function getWorkflowById(id: number): Promise { const dbTags = await Db.collections.Tag.find({ - where: { - name: In(tags), - }, + where: { name: In(tags) }, relations: ['workflows'], }); @@ -79,11 +72,11 @@ export async function createWorkflow( user: User, role: Role, ): Promise { - let savedWorkflow: unknown; - const newWorkflow = new WorkflowEntity(); - Object.assign(newWorkflow, workflow); - await Db.transaction(async (transactionManager) => { - savedWorkflow = await transactionManager.save(newWorkflow); + return Db.transaction(async (transactionManager) => { + const newWorkflow = new WorkflowEntity(); + Object.assign(newWorkflow, workflow); + const savedWorkflow = await transactionManager.save(newWorkflow); + const newSharedWorkflow = new SharedWorkflow(); Object.assign(newSharedWorkflow, { role, @@ -91,8 +84,9 @@ export async function createWorkflow( workflow: savedWorkflow, }); await transactionManager.save(newSharedWorkflow); + + return savedWorkflow; }); - return savedWorkflow as WorkflowEntity; } export async function setWorkflowAsActive(workflow: WorkflowEntity): Promise { @@ -110,13 +104,11 @@ export async function deleteWorkflow(workflow: WorkflowEntity): Promise, ): Promise { - const workflows = await Db.collections.Workflow.find(options); - return workflows; + return Db.collections.Workflow.find(options); } export async function getWorkflowsCount(options: FindManyOptions): Promise { - const count = await Db.collections.Workflow.count(options); - return count; + return Db.collections.Workflow.count(options); } export async function updateWorkflow( @@ -127,9 +119,11 @@ export async function updateWorkflow( } export function hasStartNode(workflow: WorkflowEntity): boolean { - return !( - !workflow.nodes.length || !workflow.nodes.find((node) => node.type === 'n8n-nodes-base.start') - ); + if (!workflow.nodes.length) return false; + + const found = workflow.nodes.find((node) => node.type === 'n8n-nodes-base.start'); + + return Boolean(found); } export function getStartNode(): INode { diff --git a/packages/cli/src/PublicApi/v1/shared/middlewares/global.middleware.ts b/packages/cli/src/PublicApi/v1/shared/middlewares/global.middleware.ts index 542e1c967c..7fad023d6b 100644 --- a/packages/cli/src/PublicApi/v1/shared/middlewares/global.middleware.ts +++ b/packages/cli/src/PublicApi/v1/shared/middlewares/global.middleware.ts @@ -1,24 +1,31 @@ -/* eslint-disable consistent-return */ -import { RequestHandler } from 'express'; -import { PaginatatedRequest } from '../../../types'; +/* eslint-disable @typescript-eslint/no-invalid-void-type */ + +import express from 'express'; + +import { AuthenticatedRequest, PaginatatedRequest } from '../../../types'; import { decodeCursor } from '../services/pagination.service'; -type Role = 'member' | 'owner'; +export const authorize = + (authorizedRoles: readonly string[]) => + ( + req: AuthenticatedRequest, + res: express.Response, + next: express.NextFunction, + ): express.Response | void => { + const { name } = req.user.globalRole; + + if (!authorizedRoles.includes(name)) { + return res.status(403).json({ message: 'Forbidden' }); + } -export const authorize: (role: Role[]) => RequestHandler = (role: Role[]) => (req, res, next) => { - const { - globalRole: { name: userRole }, - } = req.user as { globalRole: { name: Role } }; - if (role.includes(userRole)) { return next(); - } - return res.status(403).json({ - message: 'Forbidden', - }); -}; + }; -// @ts-ignore -export const validCursor: RequestHandler = (req: PaginatatedRequest, res, next) => { +export const validCursor = ( + req: PaginatatedRequest, + res: express.Response, + next: express.NextFunction, +): express.Response | void => { if (req.query.cursor) { const { cursor } = req.query; try { @@ -36,5 +43,6 @@ export const validCursor: RequestHandler = (req: PaginatatedRequest, res, next) }); } } - next(); + + return next(); }; diff --git a/packages/cli/src/PublicApi/v1/shared/services/pagination.service.ts b/packages/cli/src/PublicApi/v1/shared/services/pagination.service.ts index 519ee1984d..a06ca5e78a 100644 --- a/packages/cli/src/PublicApi/v1/shared/services/pagination.service.ts +++ b/packages/cli/src/PublicApi/v1/shared/services/pagination.service.ts @@ -12,7 +12,6 @@ export const decodeCursor = (cursor: string): PaginationOffsetDecoded | Paginati }; const encodeOffSetPagination = (pagination: OffsetPagination): string | null => { - // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access if (pagination.numberOfTotalRecords > pagination.offset + pagination.limit) { return Buffer.from( JSON.stringify({ diff --git a/packages/cli/src/requests.d.ts b/packages/cli/src/requests.d.ts index 2e9e7ca71d..9499dc25b1 100644 --- a/packages/cli/src/requests.d.ts +++ b/packages/cli/src/requests.d.ts @@ -82,7 +82,7 @@ export declare namespace WorkflowRequest { // ---------------------------------- export declare namespace CredentialRequest { - type RequestBody = Partial<{ + type CredentialProperties = Partial<{ id: string; // delete if sent name: string; type: string; @@ -90,7 +90,7 @@ export declare namespace CredentialRequest { data: ICredentialDataDecryptedObject; }>; - type Create = AuthenticatedRequest<{}, {}, RequestBody>; + type Create = AuthenticatedRequest<{}, {}, CredentialProperties>; type Get = AuthenticatedRequest<{ id: string }, {}, {}, Record>; @@ -98,7 +98,7 @@ export declare namespace CredentialRequest { type GetAll = AuthenticatedRequest<{}, {}, {}, { filter: string }>; - type Update = AuthenticatedRequest<{ id: string }, {}, RequestBody>; + type Update = AuthenticatedRequest<{ id: string }, {}, CredentialProperties>; type NewName = WorkflowRequest.NewName; diff --git a/packages/cli/test/integration/publicApi/credentials.test.ts b/packages/cli/test/integration/publicApi/credentials.test.ts index f3817b3d50..767fcbb230 100644 --- a/packages/cli/test/integration/publicApi/credentials.test.ts +++ b/packages/cli/test/integration/publicApi/credentials.test.ts @@ -1,5 +1,7 @@ import express from 'express'; + import { UserSettings } from 'n8n-core'; + import { Db } from '../../../src'; import { randomApiKey, randomName, randomString } from '../shared/random'; import * as utils from '../shared/utils'; @@ -9,13 +11,10 @@ import type { User } from '../../../src/databases/entities/User'; import * as testDb from '../shared/testDb'; import { RESPONSE_ERROR_MESSAGES } from '../../../src/constants'; -jest.mock('../../../src/telemetry'); - let app: express.Application; let testDbName = ''; let globalOwnerRole: Role; let globalMemberRole: Role; -let workflowOwnerRole: Role; let credentialOwnerRole: Role; let saveCredential: SaveCredentialFunction; @@ -30,13 +29,12 @@ beforeAll(async () => { const [ fetchedGlobalOwnerRole, fetchedGlobalMemberRole, - fetchedWorkflowOwnerRole, + _, fetchedCredentialOwnerRole, ] = await testDb.getAllRoles(); globalOwnerRole = fetchedGlobalOwnerRole; globalMemberRole = fetchedGlobalMemberRole; - workflowOwnerRole = fetchedWorkflowOwnerRole; credentialOwnerRole = fetchedCredentialOwnerRole; saveCredential = affixRoleToSaveCredential(credentialOwnerRole); @@ -377,6 +375,7 @@ const credentialPayload = (): CredentialPayload => ({ const dbCredential = () => { const credential = credentialPayload(); credential.nodesAccess = [{ nodeType: credential.type }]; + return credential; }; diff --git a/packages/cli/test/integration/publicApi/executions.test.ts b/packages/cli/test/integration/publicApi/executions.test.ts index 9ef5bf77af..7753ad8f40 100644 --- a/packages/cli/test/integration/publicApi/executions.test.ts +++ b/packages/cli/test/integration/publicApi/executions.test.ts @@ -1,7 +1,7 @@ -import express = require('express'); +import express from 'express'; import { ActiveWorkflowRunner } from '../../../src'; -import config = require('../../../config'); +import config from '../../../config'; import { Role } from '../../../src/databases/entities/Role'; import { randomApiKey } from '../shared/random'; @@ -11,11 +11,8 @@ import * as testDb from '../shared/testDb'; let app: express.Application; let testDbName = ''; let globalOwnerRole: Role; - let workflowRunner: ActiveWorkflowRunner.ActiveWorkflowRunner; -jest.mock('../../../src/telemetry'); - beforeAll(async () => { app = await utils.initTestServer({ endpointGroups: ['publicApi'], applyAuth: false }); const initResult = await testDb.init(); @@ -25,21 +22,29 @@ beforeAll(async () => { utils.initTestTelemetry(); utils.initTestLogger(); - // initializing binary manager leave some async operations open - // TODO mockup binary data mannager to avoid error + await utils.initBinaryManager(); await utils.initNodeTypes(); + workflowRunner = await utils.initActiveWorkflowRunner(); }); beforeEach(async () => { - // do not combine calls - shared tables must be cleared first and separately - await testDb.truncate(['SharedCredentials', 'SharedWorkflow'], testDbName); - await testDb.truncate(['User', 'Workflow', 'Credentials', 'Execution', 'Settings'], testDbName); + await testDb.truncate( + [ + 'SharedCredentials', + 'SharedWorkflow', + 'User', + 'Workflow', + 'Credentials', + 'Execution', + 'Settings', + ], + testDbName, + ); config.set('userManagement.disabled', false); config.set('userManagement.isInstanceOwnerSetUp', true); - config.set('userManagement.emails.mode', 'smtp'); }); afterEach(async () => { @@ -50,7 +55,7 @@ afterAll(async () => { await testDb.terminate(testDbName); }); -test.skip('GET /executions/:executionId should fail due to missing API Key', async () => { +test('GET /executions/:id should fail due to missing API Key', async () => { const owner = await testDb.createUser({ globalRole: globalOwnerRole }); const authOwnerAgent = utils.createAgent(app, { @@ -65,7 +70,7 @@ test.skip('GET /executions/:executionId should fail due to missing API Key', asy expect(response.statusCode).toBe(401); }); -test.skip('GET /executions/:executionId should fail due to invalid API Key', async () => { +test('GET /executions/:id should fail due to invalid API Key', async () => { const owner = await testDb.createUser({ globalRole: globalOwnerRole, apiKey: randomApiKey() }); owner.apiKey = 'abcXYZ'; @@ -81,7 +86,7 @@ test.skip('GET /executions/:executionId should fail due to invalid API Key', asy expect(response.statusCode).toBe(401); }); -test.skip('GET /executions/:executionId should get an execution', async () => { +test('GET /executions/:id should get an execution', async () => { const owner = await testDb.createUser({ globalRole: globalOwnerRole, apiKey: randomApiKey() }); const authOwnerAgent = utils.createAgent(app, { @@ -93,7 +98,7 @@ test.skip('GET /executions/:executionId should get an execution', async () => { const workflow = await testDb.createWorkflow({}, owner); - const execution = await testDb.createSuccessfullExecution(workflow); + const execution = await testDb.createSuccessfulExecution(workflow); const response = await authOwnerAgent.get(`/executions/${execution.id}`); @@ -122,7 +127,7 @@ test.skip('GET /executions/:executionId should get an execution', async () => { expect(waitTill).toBeNull(); }); -test.skip('DELETE /executions/:executionId should fail due to missing API Key', async () => { +test('DELETE /executions/:id should fail due to missing API Key', async () => { const owner = await testDb.createUser({ globalRole: globalOwnerRole }); const authOwnerAgent = utils.createAgent(app, { @@ -137,7 +142,7 @@ test.skip('DELETE /executions/:executionId should fail due to missing API Key', expect(response.statusCode).toBe(401); }); -test.skip('DELETE /executions/:executionId should fail due to invalid API Key', async () => { +test('DELETE /executions/:id should fail due to invalid API Key', async () => { const owner = await testDb.createUser({ globalRole: globalOwnerRole, apiKey: randomApiKey() }); owner.apiKey = 'abcXYZ'; @@ -153,7 +158,7 @@ test.skip('DELETE /executions/:executionId should fail due to invalid API Key', expect(response.statusCode).toBe(401); }); -test.skip('DELETE /executions/:executionId should delete an execution', async () => { +test('DELETE /executions/:id should delete an execution', async () => { const owner = await testDb.createUser({ globalRole: globalOwnerRole, apiKey: randomApiKey() }); const authOwnerAgent = utils.createAgent(app, { @@ -165,7 +170,7 @@ test.skip('DELETE /executions/:executionId should delete an execution', async () const workflow = await testDb.createWorkflow({}, owner); - const execution = await testDb.createSuccessfullExecution(workflow); + const execution = await testDb.createSuccessfulExecution(workflow); const response = await authOwnerAgent.delete(`/executions/${execution.id}`); @@ -194,7 +199,7 @@ test.skip('DELETE /executions/:executionId should delete an execution', async () expect(waitTill).toBeNull(); }); -test.skip('GET /executions should fail due to missing API Key', async () => { +test('GET /executions should fail due to missing API Key', async () => { const owner = await testDb.createUser({ globalRole: globalOwnerRole }); const authOwnerAgent = utils.createAgent(app, { @@ -209,7 +214,7 @@ test.skip('GET /executions should fail due to missing API Key', async () => { expect(response.statusCode).toBe(401); }); -test.skip('GET /executions should fail due to invalid API Key', async () => { +test('GET /executions should fail due to invalid API Key', async () => { const owner = await testDb.createUser({ globalRole: globalOwnerRole, apiKey: randomApiKey() }); owner.apiKey = 'abcXYZ'; @@ -225,7 +230,7 @@ test.skip('GET /executions should fail due to invalid API Key', async () => { expect(response.statusCode).toBe(401); }); -test.skip('GET /executions should retrieve all successfull executions', async () => { +test('GET /executions should retrieve all successfull executions', async () => { const owner = await testDb.createUser({ globalRole: globalOwnerRole, apiKey: randomApiKey() }); const authOwnerAgent = utils.createAgent(app, { @@ -237,7 +242,7 @@ test.skip('GET /executions should retrieve all successfull executions', async () const workflow = await testDb.createWorkflow({}, owner); - const successfullExecution = await testDb.createSuccessfullExecution(workflow); + const successfullExecution = await testDb.createSuccessfulExecution(workflow); await testDb.createErrorExecution(workflow); @@ -272,7 +277,7 @@ test.skip('GET /executions should retrieve all successfull executions', async () expect(waitTill).toBeNull(); }); -test.skip('GET /executions should retrieve all error executions', async () => { +test('GET /executions should retrieve all error executions', async () => { const owner = await testDb.createUser({ globalRole: globalOwnerRole, apiKey: randomApiKey() }); const authOwnerAgent = utils.createAgent(app, { @@ -284,7 +289,7 @@ test.skip('GET /executions should retrieve all error executions', async () => { const workflow = await testDb.createWorkflow({}, owner); - await testDb.createSuccessfullExecution(workflow); + await testDb.createSuccessfulExecution(workflow); const errorExecution = await testDb.createErrorExecution(workflow); @@ -319,7 +324,7 @@ test.skip('GET /executions should retrieve all error executions', async () => { expect(waitTill).toBeNull(); }); -test.skip('GET /executions should return all waiting executions', async () => { +test('GET /executions should return all waiting executions', async () => { const owner = await testDb.createUser({ globalRole: globalOwnerRole, apiKey: randomApiKey() }); const authOwnerAgent = utils.createAgent(app, { @@ -331,7 +336,7 @@ test.skip('GET /executions should return all waiting executions', async () => { const workflow = await testDb.createWorkflow({}, owner); - await testDb.createSuccessfullExecution(workflow); + await testDb.createSuccessfulExecution(workflow); await testDb.createErrorExecution(workflow); @@ -368,7 +373,7 @@ test.skip('GET /executions should return all waiting executions', async () => { expect(new Date(waitTill).getTime()).toBeGreaterThan(Date.now() - 1000); }); -test.skip('GET /executions should retrieve all executions of specific workflow', async () => { +test('GET /executions should retrieve all executions of specific workflow', async () => { const owner = await testDb.createUser({ globalRole: globalOwnerRole, apiKey: randomApiKey() }); const authOwnerAgent = utils.createAgent(app, { @@ -384,10 +389,10 @@ test.skip('GET /executions should retrieve all executions of specific workflow', 2, workflow, // @ts-ignore - testDb.createSuccessfullExecution, + testDb.createSuccessfulExecution, ); // @ts-ignore - await testDb.createManyExecutions(2, workflow2, testDb.createSuccessfullExecution); + await testDb.createManyExecutions(2, workflow2, testDb.createSuccessfulExecution); const response = await authOwnerAgent.get(`/executions`).query({ workflowId: workflow.id.toString(), diff --git a/packages/cli/test/integration/publicApi/workflows.test.ts b/packages/cli/test/integration/publicApi/workflows.test.ts index 55ae573dd0..933ff7f3f5 100644 --- a/packages/cli/test/integration/publicApi/workflows.test.ts +++ b/packages/cli/test/integration/publicApi/workflows.test.ts @@ -1,7 +1,7 @@ -import express = require('express'); +import express from 'express'; import { ActiveWorkflowRunner, Db } from '../../../src'; -import config = require('../../../config'); +import config from '../../../config'; import { Role } from '../../../src/databases/entities/Role'; import { randomApiKey } from '../shared/random'; @@ -14,11 +14,8 @@ let testDbName = ''; let globalOwnerRole: Role; let globalMemberRole: Role; let workflowOwnerRole: Role; -let credentialOwnerRole: Role; let workflowRunner: ActiveWorkflowRunner.ActiveWorkflowRunner; -jest.mock('../../../src/telemetry'); - beforeAll(async () => { app = await utils.initTestServer({ endpointGroups: ['publicApi'], applyAuth: false }); const initResult = await testDb.init(); @@ -28,13 +25,11 @@ beforeAll(async () => { fetchedGlobalOwnerRole, fetchedGlobalMemberRole, fetchedWorkflowOwnerRole, - fetchedCredentialOwnerRole, ] = await testDb.getAllRoles(); globalOwnerRole = fetchedGlobalOwnerRole; globalMemberRole = fetchedGlobalMemberRole; workflowOwnerRole = fetchedWorkflowOwnerRole; - credentialOwnerRole = fetchedCredentialOwnerRole; utils.initTestTelemetry(); utils.initTestLogger(); @@ -43,13 +38,13 @@ beforeAll(async () => { }); beforeEach(async () => { - // do not combine calls - shared tables must be cleared first and separately - await testDb.truncate(['SharedCredentials', 'SharedWorkflow'], testDbName); - await testDb.truncate(['User', 'Workflow', 'Credentials'], testDbName); + await testDb.truncate( + ['SharedCredentials', 'SharedWorkflow', 'User', 'Workflow', 'Credentials'], + testDbName, + ); config.set('userManagement.disabled', false); config.set('userManagement.isInstanceOwnerSetUp', true); - config.set('userManagement.emails.mode', 'smtp'); }); afterEach(async () => { @@ -352,7 +347,7 @@ test('GET /workflows should return all workflows for owner', async () => { } }); -test('GET /workflows/:workflowId should fail due to missing API Key', async () => { +test('GET /workflows/:id should fail due to missing API Key', async () => { const owner = await testDb.createUser({ globalRole: globalOwnerRole }); owner.apiKey = null; @@ -369,7 +364,7 @@ test('GET /workflows/:workflowId should fail due to missing API Key', async () = expect(response.statusCode).toBe(401); }); -test('GET /workflows/:workflowId should fail due to invalid API Key', async () => { +test('GET /workflows/:id should fail due to invalid API Key', async () => { const owner = await testDb.createUser({ globalRole: globalOwnerRole, apiKey: randomApiKey() }); owner.apiKey = 'abcXYZ'; @@ -386,7 +381,7 @@ test('GET /workflows/:workflowId should fail due to invalid API Key', async () = expect(response.statusCode).toBe(401); }); -test('GET /workflows/:workflowId should fail due to non existing workflow', async () => { +test('GET /workflows/:id should fail due to non existing workflow', async () => { const owner = await testDb.createUser({ globalRole: globalOwnerRole, apiKey: randomApiKey() }); const authOwnerAgent = utils.createAgent(app, { @@ -401,7 +396,7 @@ test('GET /workflows/:workflowId should fail due to non existing workflow', asyn expect(response.statusCode).toBe(404); }); -test('GET /workflows/:workflowId should retrieve workflow', async () => { +test('GET /workflows/:id should retrieve workflow', async () => { const member = await testDb.createUser({ globalRole: globalMemberRole, apiKey: randomApiKey() }); const authAgent = utils.createAgent(app, { @@ -432,7 +427,7 @@ test('GET /workflows/:workflowId should retrieve workflow', async () => { expect(updatedAt).toEqual(workflow.updatedAt.toISOString()); }); -test('GET /workflows/:workflowId should retrieve non-owned workflow for owner', async () => { +test('GET /workflows/:id should retrieve non-owned workflow for owner', async () => { const owner = await testDb.createUser({ globalRole: globalOwnerRole, apiKey: randomApiKey() }); const member = await testDb.createUser({ globalRole: globalMemberRole }); @@ -464,7 +459,7 @@ test('GET /workflows/:workflowId should retrieve non-owned workflow for owner', expect(updatedAt).toEqual(workflow.updatedAt.toISOString()); }); -test('DELETE /workflows/:workflowId should fail due to missing API Key', async () => { +test('DELETE /workflows/:id should fail due to missing API Key', async () => { const owner = await testDb.createUser({ globalRole: globalOwnerRole }); const authOwnerAgent = utils.createAgent(app, { @@ -479,7 +474,7 @@ test('DELETE /workflows/:workflowId should fail due to missing API Key', async ( expect(response.statusCode).toBe(401); }); -test('DELETE /workflows/:workflowId should fail due to invalid API Key', async () => { +test('DELETE /workflows/:id should fail due to invalid API Key', async () => { const owner = await testDb.createUser({ globalRole: globalOwnerRole, apiKey: randomApiKey() }); owner.apiKey = 'abcXYZ'; @@ -496,7 +491,7 @@ test('DELETE /workflows/:workflowId should fail due to invalid API Key', async ( expect(response.statusCode).toBe(401); }); -test('DELETE /workflows/:workflowId should fail due to non existing workflow', async () => { +test('DELETE /workflows/:id should fail due to non existing workflow', async () => { const owner = await testDb.createUser({ globalRole: globalOwnerRole, apiKey: randomApiKey() }); const authOwnerAgent = utils.createAgent(app, { @@ -511,7 +506,7 @@ test('DELETE /workflows/:workflowId should fail due to non existing workflow', a expect(response.statusCode).toBe(404); }); -test('DELETE /workflows/:workflowId should delete the workflow', async () => { +test('DELETE /workflows/:id should delete the workflow', async () => { const member = await testDb.createUser({ globalRole: globalMemberRole, apiKey: randomApiKey() }); const authAgent = utils.createAgent(app, { @@ -549,7 +544,7 @@ test('DELETE /workflows/:workflowId should delete the workflow', async () => { expect(sharedWorkflow).toBeUndefined(); }); -test('DELETE /workflows/:workflowId should delete non-owned workflow when owner', async () => { +test('DELETE /workflows/:id should delete non-owned workflow when owner', async () => { const owner = await testDb.createUser({ globalRole: globalOwnerRole, apiKey: randomApiKey() }); const member = await testDb.createUser({ globalRole: globalMemberRole }); @@ -588,7 +583,7 @@ test('DELETE /workflows/:workflowId should delete non-owned workflow when owner' expect(sharedWorkflow).toBeUndefined(); }); -test('POST /workflows/:workflowId/activate should fail due to missing API Key', async () => { +test('POST /workflows/:id/activate should fail due to missing API Key', async () => { const owner = await testDb.createUser({ globalRole: globalOwnerRole }); const authOwnerAgent = utils.createAgent(app, { @@ -603,7 +598,7 @@ test('POST /workflows/:workflowId/activate should fail due to missing API Key', expect(response.statusCode).toBe(401); }); -test('POST /workflows/:workflowId/activate should fail due to invalid API Key', async () => { +test('POST /workflows/:id/activate should fail due to invalid API Key', async () => { const owner = await testDb.createUser({ globalRole: globalOwnerRole, apiKey: randomApiKey() }); owner.apiKey = 'abcXYZ'; @@ -620,7 +615,7 @@ test('POST /workflows/:workflowId/activate should fail due to invalid API Key', expect(response.statusCode).toBe(401); }); -test('POST /workflows/:workflowId/activate should fail due to non existing workflow', async () => { +test('POST /workflows/:id/activate should fail due to non existing workflow', async () => { const owner = await testDb.createUser({ globalRole: globalOwnerRole, apiKey: randomApiKey() }); const authOwnerAgent = utils.createAgent(app, { @@ -635,7 +630,7 @@ test('POST /workflows/:workflowId/activate should fail due to non existing workf expect(response.statusCode).toBe(404); }); -test('POST /workflows/:workflowId/activate should fail due to trying to activate a workflow without a trigger', async () => { +test('POST /workflows/:id/activate should fail due to trying to activate a workflow without a trigger', async () => { const owner = await testDb.createUser({ globalRole: globalOwnerRole, apiKey: randomApiKey() }); const authOwnerAgent = utils.createAgent(app, { @@ -652,7 +647,7 @@ test('POST /workflows/:workflowId/activate should fail due to trying to activate expect(response.statusCode).toBe(400); }); -test.skip('POST /workflows/:workflowId/activate should set workflow as active', async () => { +test.skip('POST /workflows/:id/activate should set workflow as active', async () => { const member = await testDb.createUser({ globalRole: globalMemberRole, apiKey: randomApiKey() }); const authAgent = utils.createAgent(app, { @@ -696,7 +691,7 @@ test.skip('POST /workflows/:workflowId/activate should set workflow as active', expect(await workflowRunner.isActive(workflow.id.toString())).toBe(true); }); -test.skip('POST /workflows/:workflowId/activate should set non-owned workflow as active when owner', async () => { +test.skip('POST /workflows/:id/activate should set non-owned workflow as active when owner', async () => { const owner = await testDb.createUser({ globalRole: globalOwnerRole, apiKey: randomApiKey() }); const member = await testDb.createUser({ globalRole: globalMemberRole }); @@ -750,7 +745,7 @@ test.skip('POST /workflows/:workflowId/activate should set non-owned workflow as expect(await workflowRunner.isActive(workflow.id.toString())).toBe(true); }); -test('POST /workflows/:workflowId/deactivate should fail due to missing API Key', async () => { +test('POST /workflows/:id/deactivate should fail due to missing API Key', async () => { const owner = await testDb.createUser({ globalRole: globalOwnerRole }); const authOwnerAgent = utils.createAgent(app, { @@ -765,7 +760,7 @@ test('POST /workflows/:workflowId/deactivate should fail due to missing API Key' expect(response.statusCode).toBe(401); }); -test('POST /workflows/:workflowId/deactivate should fail due to invalid API Key', async () => { +test('POST /workflows/:id/deactivate should fail due to invalid API Key', async () => { const owner = await testDb.createUser({ globalRole: globalOwnerRole, apiKey: randomApiKey() }); owner.apiKey = 'abcXYZ'; @@ -782,7 +777,7 @@ test('POST /workflows/:workflowId/deactivate should fail due to invalid API Key' expect(response.statusCode).toBe(401); }); -test('POST /workflows/:workflowId/deactivate should fail due to non existing workflow', async () => { +test('POST /workflows/:id/deactivate should fail due to non existing workflow', async () => { const owner = await testDb.createUser({ globalRole: globalOwnerRole, apiKey: randomApiKey() }); const authOwnerAgent = utils.createAgent(app, { @@ -797,7 +792,7 @@ test('POST /workflows/:workflowId/deactivate should fail due to non existing wor expect(response.statusCode).toBe(404); }); -test('POST /workflows/:workflowId/deactivate should deactive workflow', async () => { +test('POST /workflows/:id/deactivate should deactive workflow', async () => { const member = await testDb.createUser({ globalRole: globalMemberRole, apiKey: randomApiKey() }); const authAgent = utils.createAgent(app, { @@ -841,7 +836,7 @@ test('POST /workflows/:workflowId/deactivate should deactive workflow', async () expect(await workflowRunner.isActive(workflow.id.toString())).toBe(false); }); -test('POST /workflows/:workflowId/deactivate should deactive non-owned workflow when owner', async () => { +test('POST /workflows/:id/deactivate should deactive non-owned workflow when owner', async () => { const owner = await testDb.createUser({ globalRole: globalOwnerRole, apiKey: randomApiKey() }); const member = await testDb.createUser({ globalRole: globalMemberRole }); @@ -904,7 +899,7 @@ test('POST /workflows should fail due to missing API Key', async () => { version: 1, }); - const response = await authOwnerAgent.post(`/workflows`); + const response = await authOwnerAgent.post('/workflows'); expect(response.statusCode).toBe(401); }); @@ -921,7 +916,7 @@ test('POST /workflows should fail due to invalid API Key', async () => { version: 1, }); - const response = await authOwnerAgent.post(`/workflows`); + const response = await authOwnerAgent.post('/workflows'); expect(response.statusCode).toBe(401); }); @@ -936,7 +931,7 @@ test('POST /workflows should fail due to invalid body', async () => { version: 1, }); - const response = await authOwnerAgent.post(`/workflows`).send({}); + const response = await authOwnerAgent.post('/workflows').send({}); expect(response.statusCode).toBe(400); }); @@ -974,7 +969,7 @@ test('POST /workflows should create workflow', async () => { }, }; - const response = await authAgent.post(`/workflows`).send(payload); + const response = await authAgent.post('/workflows').send(payload); expect(response.statusCode).toBe(200); @@ -1005,7 +1000,7 @@ test('POST /workflows should create workflow', async () => { expect(sharedWorkflow?.role).toEqual(workflowOwnerRole); }); -test('PUT /workflows/:workflowId should fail due to missing API Key', async () => { +test('PUT /workflows/:id should fail due to missing API Key', async () => { const owner = await testDb.createUser({ globalRole: globalOwnerRole }); const authOwnerAgent = utils.createAgent(app, { @@ -1020,7 +1015,7 @@ test('PUT /workflows/:workflowId should fail due to missing API Key', async () = expect(response.statusCode).toBe(401); }); -test('PUT /workflows/:workflowId should fail due to invalid API Key', async () => { +test('PUT /workflows/:id should fail due to invalid API Key', async () => { const owner = await testDb.createUser({ globalRole: globalOwnerRole, apiKey: randomApiKey() }); owner.apiKey = 'abcXYZ'; @@ -1037,7 +1032,7 @@ test('PUT /workflows/:workflowId should fail due to invalid API Key', async () = expect(response.statusCode).toBe(401); }); -test('PUT /workflows/:workflowId should fail due to non existing workflow', async () => { +test('PUT /workflows/:id should fail due to non existing workflow', async () => { const owner = await testDb.createUser({ globalRole: globalOwnerRole, apiKey: randomApiKey() }); const authOwnerAgent = utils.createAgent(app, { @@ -1073,7 +1068,7 @@ test('PUT /workflows/:workflowId should fail due to non existing workflow', asyn expect(response.statusCode).toBe(404); }); -test('PUT /workflows/:workflowId should fail due to invalid body', async () => { +test('PUT /workflows/:id should fail due to invalid body', async () => { const owner = await testDb.createUser({ globalRole: globalOwnerRole, apiKey: randomApiKey() }); const authOwnerAgent = utils.createAgent(app, { @@ -1108,7 +1103,7 @@ test('PUT /workflows/:workflowId should fail due to invalid body', async () => { expect(response.statusCode).toBe(400); }); -test('PUT /workflows/:workflowId should update workflow', async () => { +test('PUT /workflows/:id should update workflow', async () => { const member = await testDb.createUser({ globalRole: globalOwnerRole, apiKey: randomApiKey() }); const workflow = await testDb.createWorkflow({}, member); @@ -1182,7 +1177,7 @@ test('PUT /workflows/:workflowId should update workflow', async () => { ); }); -test('PUT /workflows/:workflowId should update non-owned workflow if owner', async () => { +test('PUT /workflows/:id should update non-owned workflow if owner', async () => { const owner = await testDb.createUser({ globalRole: globalOwnerRole, apiKey: randomApiKey() }); const member = await testDb.createUser({ globalRole: globalMemberRole }); diff --git a/packages/cli/test/integration/shared/testDb.ts b/packages/cli/test/integration/shared/testDb.ts index d250fa99ec..49552bd141 100644 --- a/packages/cli/test/integration/shared/testDb.ts +++ b/packages/cli/test/integration/shared/testDb.ts @@ -323,8 +323,7 @@ export async function createManyExecutions( } /** - * Store a execution in the DB and assigns it to a workflow. - * @param user user to assign the workflow to + * Store a execution in the DB and assign it to a workflow. */ export async function createExecution( attributes: Partial = {}, @@ -346,48 +345,41 @@ export async function createExecution( } /** - * Store a execution in the DB and assigns it to a workflow. - * @param user user to assign the workflow to + * Store a successful execution in the DB and assign it to a workflow. */ -export async function createSuccessfullExecution(workflow: WorkflowEntity) { - const execution = await createExecution( +export async function createSuccessfulExecution(workflow: WorkflowEntity) { + return await createExecution( { finished: true, }, workflow, ); - - return execution; } /** - * Store a execution in the DB and assigns it to a workflow. - * @param user user to assign the workflow to + * Store an error execution in the DB and assign it to a workflow. */ export async function createErrorExecution(workflow: WorkflowEntity) { - const execution = await createExecution( + return await createExecution( { finished: false, stoppedAt: new Date(), }, workflow, ); - return execution; } /** - * Store a execution in the DB and assigns it to a workflow. - * @param user user to assign the workflow to + * Store a waiting execution in the DB and assign it to a workflow. */ export async function createWaitingExecution(workflow: WorkflowEntity) { - const execution = await createExecution( + return await createExecution( { finished: false, waitTill: new Date(), }, workflow, ); - return execution; } // ---------------------------------- @@ -417,7 +409,7 @@ export async function createManyWorkflows( } /** - * Store a workflow in the DB (without a trigger) and optionally assigns it to a user. + * Store a workflow in the DB (without a trigger) and optionally assign it to a user. * @param user user to assign the workflow to */ export async function createWorkflow(attributes: Partial = {}, user?: User) { @@ -450,7 +442,7 @@ export async function createWorkflow(attributes: Partial = {}, u } /** - * Store a workflow in the DB (with a trigger) and optionally assigns it to a user. + * Store a workflow in the DB (with a trigger) and optionally assign it to a user. * @param user user to assign the workflow to */ export async function createWorkflowWithTrigger( @@ -487,6 +479,7 @@ export async function createWorkflowWithTrigger( }, user, ); + return workflow; } diff --git a/packages/cli/test/integration/shared/types.d.ts b/packages/cli/test/integration/shared/types.d.ts index ca10dacb9b..c16a44d29f 100644 --- a/packages/cli/test/integration/shared/types.d.ts +++ b/packages/cli/test/integration/shared/types.d.ts @@ -34,3 +34,12 @@ export type SaveCredentialFunction = ( export type PostgresSchemaSection = { [K in 'host' | 'port' | 'schema' | 'user' | 'password']: { env: string }; }; + +export interface TriggerTime { + mode: string; + hour: number; + minute: number; + dayOfMonth: number; + weekeday: number; + [key: string]: string | number; +} diff --git a/packages/cli/test/integration/shared/utils.ts b/packages/cli/test/integration/shared/utils.ts index f3d205476f..723bf11900 100644 --- a/packages/cli/test/integration/shared/utils.ts +++ b/packages/cli/test/integration/shared/utils.ts @@ -1,5 +1,6 @@ import { randomBytes } from 'crypto'; import { existsSync } from 'fs'; + import express from 'express'; import superagent from 'superagent'; import request from 'supertest'; @@ -7,6 +8,9 @@ import { URL } from 'url'; import bodyParser from 'body-parser'; import util from 'util'; import { createTestAccount } from 'nodemailer'; +import { set } from 'lodash'; +import { CronJob } from 'cron'; +import { BinaryDataManager, UserSettings } from 'n8n-core'; import { ICredentialType, IDataObject, @@ -19,10 +23,8 @@ import { ITriggerResponse, LoggerProxy, } from 'n8n-workflow'; -import { BinaryDataManager, UserSettings } from 'n8n-core'; -import { CronJob } from 'cron'; -import config = require('../../../config'); +import config from '../../../config'; import { AUTHLESS_ENDPOINTS, PUBLIC_API_REST_PATH_SEGMENT, REST_PATH_SEGMENT } from './constants'; import { AUTH_COOKIE_NAME } from '../../../src/constants'; import { addRoutes as authMiddleware } from '../../../src/UserManagement/routes'; @@ -43,20 +45,17 @@ import { issueJWT } from '../../../src/UserManagement/auth/jwt'; import { getLogger } from '../../../src/Logger'; import { credentialsController } from '../../../src/api/credentials.api'; import { loadPublicApiVersions } from '../../../src/PublicApi/'; -import type { User } from '../../../src/databases/entities/User'; -import type { ApiPath, EndpointGroup, PostgresSchemaSection, SmtpTestAccount } from './types'; -import { Telemetry } from '../../../src/telemetry'; -import type { N8nApp } from '../../../src/UserManagement/Interfaces'; -import { set } from 'lodash'; -interface TriggerTime { - mode: string; - hour: number; - minute: number; - dayOfMonth: number; - weekeday: number; - [key: string]: string | number; -} import * as UserManagementMailer from '../../../src/UserManagement/email/UserManagementMailer'; +import type { User } from '../../../src/databases/entities/User'; +import type { + ApiPath, + EndpointGroup, + PostgresSchemaSection, + SmtpTestAccount, + TriggerTime, +} from './types'; +import type { N8nApp } from '../../../src/UserManagement/Interfaces'; + /** * Initialize a test server. @@ -75,7 +74,7 @@ export async function initTestServer({ app: express(), restEndpoint: REST_PATH_SEGMENT, publicApiEndpoint: PUBLIC_API_REST_PATH_SEGMENT, - ...(endpointGroups?.includes('credentials') ? { externalHooks: ExternalHooks() } : {}), + externalHooks: {}, }; testServer.app.use(bodyParser.json()); @@ -90,13 +89,17 @@ export async function initTestServer({ if (!endpointGroups) return testServer.app; + if (endpointGroups.includes('credentials')) { + testServer.externalHooks = ExternalHooks(); + } + const [routerEndpoints, functionEndpoints] = classifyEndpointGroups(endpointGroups); if (routerEndpoints.length) { const { apiRouters } = await loadPublicApiVersions(testServer.publicApiEndpoint); const map: Record = { credentials: credentialsController, - publicApi: apiRouters + publicApi: apiRouters, }; for (const group of routerEndpoints) { @@ -743,8 +746,8 @@ export async function initNodeTypes() { }, }, }; - const nodeTypes = NodeTypes(); - await nodeTypes.init(types); + + await NodeTypes().init(types); } /** @@ -759,7 +762,7 @@ export function initTestLogger() { */ export async function initBinaryManager() { const binaryDataConfig = config.getEnv('binaryDataManager'); - await BinaryDataManager.init(binaryDataConfig, true); + await BinaryDataManager.init(binaryDataConfig); } /** @@ -783,7 +786,7 @@ export function initConfigFile() { */ export function createAgent( app: express.Application, - options?: { apiPath?: ApiPath; version?: string | number; auth: boolean; user: User }, + options?: { auth: boolean; user: User; apiPath?: ApiPath; version?: string | number }, ) { const agent = request.agent(app);