diff --git a/packages/cli/src/Server.ts b/packages/cli/src/Server.ts index e51f837c30363..eabc232d30c52 100644 --- a/packages/cli/src/Server.ts +++ b/packages/cli/src/Server.ts @@ -59,7 +59,6 @@ import config from '@/config'; import * as Queue from '@/Queue'; import { getSharedWorkflowIds } from '@/WorkflowHelpers'; -import { nodesController } from '@/api/nodes.api'; import { workflowsController } from '@/workflows/workflows.controller'; import { EDITOR_UI_DIST_DIR, @@ -85,6 +84,7 @@ import { AuthController, LdapController, MeController, + NodesController, NodeTypesController, OwnerController, PasswordResetController, @@ -401,6 +401,12 @@ class Server extends AbstractServer { controllers.push(new LdapController(service, sync, internalHooks)); } + if (config.getEnv('nodes.communityPackages.enabled')) { + controllers.push( + new NodesController(config, this.loadNodesAndCredentials, this.push, internalHooks), + ); + } + controllers.forEach((controller) => registerController(app, config, controller)); } @@ -486,13 +492,6 @@ class Server extends AbstractServer { this.app.use(`/${this.restEndpoint}/credentials`, credentialsController); - // ---------------------------------------- - // Packages and nodes management - // ---------------------------------------- - if (config.getEnv('nodes.communityPackages.enabled')) { - this.app.use(`/${this.restEndpoint}/nodes`, nodesController); - } - // ---------------------------------------- // Workflow // ---------------------------------------- diff --git a/packages/cli/src/controllers/index.ts b/packages/cli/src/controllers/index.ts index 3cabcf987b2e3..04b46af5f13df 100644 --- a/packages/cli/src/controllers/index.ts +++ b/packages/cli/src/controllers/index.ts @@ -1,6 +1,7 @@ export { AuthController } from './auth.controller'; export { LdapController } from './ldap.controller'; export { MeController } from './me.controller'; +export { NodesController } from './nodes.controller'; export { NodeTypesController } from './nodeTypes.controller'; export { OwnerController } from './owner.controller'; export { PasswordResetController } from './passwordReset.controller'; diff --git a/packages/cli/src/api/nodes.api.ts b/packages/cli/src/controllers/nodes.controller.ts similarity index 64% rename from packages/cli/src/api/nodes.api.ts rename to packages/cli/src/controllers/nodes.controller.ts index 70264cdab5c5a..63116bb761f2b 100644 --- a/packages/cli/src/api/nodes.api.ts +++ b/packages/cli/src/controllers/nodes.controller.ts @@ -1,10 +1,12 @@ -import express from 'express'; -import type { PublicInstalledPackage } from 'n8n-workflow'; - -import config from '@/config'; -import { LoadNodesAndCredentials } from '@/LoadNodesAndCredentials'; -import * as ResponseHelper from '@/ResponseHelper'; - +import { Request, Response, NextFunction } from 'express'; +import { + RESPONSE_ERROR_MESSAGES, + STARTER_TEMPLATE_NAME, + UNKNOWN_FAILURE_REASON, +} from '@/constants'; +import { Delete, Get, Middleware, Patch, Post, RestController } from '@/decorators'; +import { NodeRequest } from '@/requests'; +import { BadRequestError, InternalServerError } from '@/ResponseHelper'; import { checkNpmPackageStatus, executeCommand, @@ -22,57 +24,50 @@ import { getAllInstalledPackages, isPackageInstalled, } from '@/CommunityNodes/packageModel'; -import { - RESPONSE_ERROR_MESSAGES, - STARTER_TEMPLATE_NAME, - UNKNOWN_FAILURE_REASON, -} from '@/constants'; -import { isAuthenticatedRequest } from '@/UserManagement/UserManagementHelper'; - import type { InstalledPackages } from '@db/entities/InstalledPackages'; import type { CommunityPackages } from '@/Interfaces'; -import type { NodeRequest } from '@/requests'; -import { Push } from '@/push'; -import { Container } from 'typedi'; +import { LoadNodesAndCredentials } from '@/LoadNodesAndCredentials'; import { InternalHooks } from '@/InternalHooks'; +import { Push } from '@/push'; +import { Config } from '@/config'; +import { isAuthenticatedRequest } from '@/UserManagement/UserManagementHelper'; const { PACKAGE_NOT_INSTALLED, PACKAGE_NAME_NOT_PROVIDED } = RESPONSE_ERROR_MESSAGES; -export const nodesController = express.Router(); - -nodesController.use((req, res, next) => { - if (!isAuthenticatedRequest(req) || req.user.globalRole.name !== 'owner') { - res.status(403).json({ status: 'error', message: 'Unauthorized' }); - return; +@RestController('/nodes') +export class NodesController { + constructor( + private config: Config, + private loadNodesAndCredentials: LoadNodesAndCredentials, + private push: Push, + private internalHooks: InternalHooks, + ) {} + + // TODO: move this into a new decorator `@Authorized` + @Middleware() + checkIfOwner(req: Request, res: Response, next: NextFunction) { + if (!isAuthenticatedRequest(req) || req.user.globalRole.name !== 'owner') + res.status(403).json({ status: 'error', message: 'Unauthorized' }); + else next(); } - next(); -}); - -nodesController.use((req, res, next) => { - if (config.getEnv('executions.mode') === 'queue' && req.method !== 'GET') { - res.status(400).json({ - status: 'error', - message: 'Package management is disabled when running in "queue" mode', - }); - return; + // TODO: move this into a new decorator `@IfConfig('executions.mode', 'queue')` + @Middleware() + checkIfCommunityNodesEnabled(req: Request, res: Response, next: NextFunction) { + if (this.config.getEnv('executions.mode') === 'queue' && req.method !== 'GET') + res.status(400).json({ + status: 'error', + message: 'Package management is disabled when running in "queue" mode', + }); + else next(); } - next(); -}); - -/** - * POST /nodes - * - * Install an n8n community package - */ -nodesController.post( - '/', - ResponseHelper.send(async (req: NodeRequest.Post) => { + @Post('/') + async installPackage(req: NodeRequest.Post) { const { name } = req.body; if (!name) { - throw new ResponseHelper.BadRequestError(PACKAGE_NAME_NOT_PROVIDED); + throw new BadRequestError(PACKAGE_NAME_NOT_PROVIDED); } let parsed: CommunityPackages.ParsedPackageName; @@ -80,13 +75,13 @@ nodesController.post( try { parsed = parseNpmPackageName(name); } catch (error) { - throw new ResponseHelper.BadRequestError( + throw new BadRequestError( error instanceof Error ? error.message : 'Failed to parse package name', ); } if (parsed.packageName === STARTER_TEMPLATE_NAME) { - throw new ResponseHelper.BadRequestError( + throw new BadRequestError( [ `Package "${parsed.packageName}" is only a template`, 'Please enter an actual package to install', @@ -98,7 +93,7 @@ nodesController.post( const hasLoaded = hasPackageLoaded(name); if (isInstalled && hasLoaded) { - throw new ResponseHelper.BadRequestError( + throw new BadRequestError( [ `Package "${parsed.packageName}" is already installed`, 'To update it, click the corresponding button in the UI', @@ -109,22 +104,19 @@ nodesController.post( const packageStatus = await checkNpmPackageStatus(name); if (packageStatus.status !== 'OK') { - throw new ResponseHelper.BadRequestError( - `Package "${name}" is banned so it cannot be installed`, - ); + throw new BadRequestError(`Package "${name}" is banned so it cannot be installed`); } let installedPackage: InstalledPackages; - try { - installedPackage = await Container.get(LoadNodesAndCredentials).loadNpmModule( + installedPackage = await this.loadNodesAndCredentials.loadNpmModule( parsed.packageName, parsed.version, ); } catch (error) { const errorMessage = error instanceof Error ? error.message : UNKNOWN_FAILURE_REASON; - void Container.get(InternalHooks).onCommunityPackageInstallFinished({ + void this.internalHooks.onCommunityPackageInstallFinished({ user: req.user, input_string: name, package_name: parsed.packageName, @@ -136,23 +128,20 @@ nodesController.post( const message = [`Error loading package "${name}"`, errorMessage].join(':'); const clientError = error instanceof Error ? isClientError(error) : false; - - throw new ResponseHelper[clientError ? 'BadRequestError' : 'InternalServerError'](message); + throw new (clientError ? BadRequestError : InternalServerError)(message); } if (!hasLoaded) removePackageFromMissingList(name); - const pushInstance = Container.get(Push); - // broadcast to connected frontends that node list has been updated installedPackage.installedNodes.forEach((node) => { - pushInstance.send('reloadNodeType', { + this.push.send('reloadNodeType', { name: node.type, version: node.latestVersion, }); }); - void Container.get(InternalHooks).onCommunityPackageInstallFinished({ + void this.internalHooks.onCommunityPackageInstallFinished({ user: req.user, input_string: name, package_name: parsed.packageName, @@ -164,17 +153,10 @@ nodesController.post( }); return installedPackage; - }), -); - -/** - * GET /nodes - * - * Retrieve list of installed n8n community packages - */ -nodesController.get( - '/', - ResponseHelper.send(async (): Promise => { + } + + @Get('/') + async getInstalledPackages() { const installedPackages = await getAllInstalledPackages(); if (installedPackages.length === 0) return []; @@ -188,7 +170,6 @@ nodesController.get( // when there are updates, npm exits with code 1 // when there are no updates, command succeeds // https://github.com/npm/rfcs/issues/473 - if (isNpmError(error) && error.code === 1) { pendingUpdates = JSON.parse(error.stdout) as CommunityPackages.AvailableUpdates; } @@ -197,31 +178,21 @@ nodesController.get( let hydratedPackages = matchPackagesWithUpdates(installedPackages, pendingUpdates); try { - const missingPackages = config.get('nodes.packagesMissing') as string | undefined; - + const missingPackages = this.config.get('nodes.packagesMissing') as string | undefined; if (missingPackages) { hydratedPackages = matchMissingPackages(hydratedPackages, missingPackages); } - } catch { - // Do nothing if setting is missing - } + } catch {} return hydratedPackages; - }), -); - -/** - * DELETE /nodes - * - * Uninstall an installed n8n community package - */ -nodesController.delete( - '/', - ResponseHelper.send(async (req: NodeRequest.Delete) => { + } + + @Delete('/') + async uninstallPackage(req: NodeRequest.Delete) { const { name } = req.query; if (!name) { - throw new ResponseHelper.BadRequestError(PACKAGE_NAME_NOT_PROVIDED); + throw new BadRequestError(PACKAGE_NAME_NOT_PROVIDED); } try { @@ -229,37 +200,35 @@ nodesController.delete( } catch (error) { const message = error instanceof Error ? error.message : UNKNOWN_FAILURE_REASON; - throw new ResponseHelper.BadRequestError(message); + throw new BadRequestError(message); } const installedPackage = await findInstalledPackage(name); if (!installedPackage) { - throw new ResponseHelper.BadRequestError(PACKAGE_NOT_INSTALLED); + throw new BadRequestError(PACKAGE_NOT_INSTALLED); } try { - await Container.get(LoadNodesAndCredentials).removeNpmModule(name, installedPackage); + await this.loadNodesAndCredentials.removeNpmModule(name, installedPackage); } catch (error) { const message = [ `Error removing package "${name}"`, error instanceof Error ? error.message : UNKNOWN_FAILURE_REASON, ].join(':'); - throw new ResponseHelper.InternalServerError(message); + throw new InternalServerError(message); } - const pushInstance = Container.get(Push); - // broadcast to connected frontends that node list has been updated installedPackage.installedNodes.forEach((node) => { - pushInstance.send('removeNodeType', { + this.push.send('removeNodeType', { name: node.type, version: node.latestVersion, }); }); - void Container.get(InternalHooks).onCommunityPackageDeleteFinished({ + void this.internalHooks.onCommunityPackageDeleteFinished({ user: req.user, package_name: name, package_version: installedPackage.installedVersion, @@ -267,53 +236,44 @@ nodesController.delete( package_author: installedPackage.authorName, package_author_email: installedPackage.authorEmail, }); - }), -); - -/** - * PATCH /nodes - * - * Update an installed n8n community package - */ -nodesController.patch( - '/', - ResponseHelper.send(async (req: NodeRequest.Update) => { + } + + @Patch('/') + async updatePackage(req: NodeRequest.Update) { const { name } = req.body; if (!name) { - throw new ResponseHelper.BadRequestError(PACKAGE_NAME_NOT_PROVIDED); + throw new BadRequestError(PACKAGE_NAME_NOT_PROVIDED); } const previouslyInstalledPackage = await findInstalledPackage(name); if (!previouslyInstalledPackage) { - throw new ResponseHelper.BadRequestError(PACKAGE_NOT_INSTALLED); + throw new BadRequestError(PACKAGE_NOT_INSTALLED); } try { - const newInstalledPackage = await Container.get(LoadNodesAndCredentials).updateNpmModule( + const newInstalledPackage = await this.loadNodesAndCredentials.updateNpmModule( parseNpmPackageName(name).packageName, previouslyInstalledPackage, ); - const pushInstance = Container.get(Push); - // broadcast to connected frontends that node list has been updated previouslyInstalledPackage.installedNodes.forEach((node) => { - pushInstance.send('removeNodeType', { + this.push.send('removeNodeType', { name: node.type, version: node.latestVersion, }); }); newInstalledPackage.installedNodes.forEach((node) => { - pushInstance.send('reloadNodeType', { + this.push.send('reloadNodeType', { name: node.name, version: node.latestVersion, }); }); - void Container.get(InternalHooks).onCommunityPackageUpdateFinished({ + void this.internalHooks.onCommunityPackageUpdateFinished({ user: req.user, package_name: name, package_version_current: previouslyInstalledPackage.installedVersion, @@ -326,8 +286,7 @@ nodesController.patch( return newInstalledPackage; } catch (error) { previouslyInstalledPackage.installedNodes.forEach((node) => { - const pushInstance = Container.get(Push); - pushInstance.send('removeNodeType', { + this.push.send('removeNodeType', { name: node.type, version: node.latestVersion, }); @@ -338,7 +297,7 @@ nodesController.patch( error instanceof Error ? error.message : UNKNOWN_FAILURE_REASON, ].join(':'); - throw new ResponseHelper.InternalServerError(message); + throw new InternalServerError(message); } - }), -); + } +} diff --git a/packages/cli/src/controllers/tags.controller.ts b/packages/cli/src/controllers/tags.controller.ts index fa8ed81e7e6f6..b9c87294921ae 100644 --- a/packages/cli/src/controllers/tags.controller.ts +++ b/packages/cli/src/controllers/tags.controller.ts @@ -1,10 +1,7 @@ -/* eslint-disable @typescript-eslint/no-shadow */ -import type { RequestHandler } from 'express'; -import { Request } from 'express'; +import { Request, Response, NextFunction } from 'express'; import type { Repository } from 'typeorm'; import type { Config } from '@/config'; -import config from '@/config'; -import { Delete, Get, Patch, Post, RestController } from '@/decorators'; +import { Delete, Get, Middleware, Patch, Post, RestController } from '@/decorators'; import type { IDatabaseCollections, IExternalHooksClass, ITagWithCountDb } from '@/Interfaces'; import { TagEntity } from '@db/entities/TagEntity'; import { getTagsWithCountDb } from '@/TagHelpers'; @@ -12,14 +9,7 @@ import { validateEntity } from '@/GenericHelpers'; import { BadRequestError, UnauthorizedError } from '@/ResponseHelper'; import { TagsRequest } from '@/requests'; -// TODO: convert this into a decorator `@IfEnabled('workflowTagsDisabled')` -const workflowsEnabledMiddleware: RequestHandler = (req, res, next) => { - if (config.getEnv('workflowTagsDisabled')) - throw new BadRequestError('Workflow tags are disabled'); - next(); -}; - -@RestController('/tags', workflowsEnabledMiddleware) +@RestController('/tags') export class TagsController { private config: Config; @@ -41,6 +31,14 @@ export class TagsController { this.tagsRepository = repositories.Tag; } + // TODO: move this into a new decorator `@IfEnabled('workflowTagsDisabled')` + @Middleware() + workflowsEnabledMiddleware(req: Request, res: Response, next: NextFunction) { + if (this.config.getEnv('workflowTagsDisabled')) + throw new BadRequestError('Workflow tags are disabled'); + next(); + } + // Retrieves all tags, with or without usage count @Get('/') async getALL( diff --git a/packages/cli/src/decorators/Middleware.ts b/packages/cli/src/decorators/Middleware.ts new file mode 100644 index 0000000000000..6d5e957f6eccf --- /dev/null +++ b/packages/cli/src/decorators/Middleware.ts @@ -0,0 +1,11 @@ +import { CONTROLLER_MIDDLEWARES } from './constants'; +import type { MiddlewareMetadata } from './types'; + +// eslint-disable-next-line @typescript-eslint/naming-convention +export const Middleware = (): MethodDecorator => (target, handlerName) => { + const controllerClass = target.constructor; + const middlewares = (Reflect.getMetadata(CONTROLLER_MIDDLEWARES, controllerClass) ?? + []) as MiddlewareMetadata[]; + middlewares.push({ handlerName: String(handlerName) }); + Reflect.defineMetadata(CONTROLLER_MIDDLEWARES, middlewares, controllerClass); +}; diff --git a/packages/cli/src/decorators/RestController.ts b/packages/cli/src/decorators/RestController.ts index 09ec49d0185b1..11c2d55665b39 100644 --- a/packages/cli/src/decorators/RestController.ts +++ b/packages/cli/src/decorators/RestController.ts @@ -1,10 +1,8 @@ /* eslint-disable @typescript-eslint/naming-convention */ -import type { RequestHandler } from 'express'; -import { CONTROLLER_BASE_PATH, CONTROLLER_MIDDLEWARES } from './constants'; +import { CONTROLLER_BASE_PATH } from './constants'; export const RestController = - (basePath: `/${string}` = '/', ...middlewares: RequestHandler[]): ClassDecorator => + (basePath: `/${string}` = '/'): ClassDecorator => (target: object) => { Reflect.defineMetadata(CONTROLLER_BASE_PATH, basePath, target); - Reflect.defineMetadata(CONTROLLER_MIDDLEWARES, middlewares, target); }; diff --git a/packages/cli/src/decorators/index.ts b/packages/cli/src/decorators/index.ts index 6c345c85a38a9..71b82b5b69c30 100644 --- a/packages/cli/src/decorators/index.ts +++ b/packages/cli/src/decorators/index.ts @@ -1,3 +1,4 @@ export { RestController } from './RestController'; export { Get, Post, Put, Patch, Delete } from './Route'; +export { Middleware } from './Middleware'; export { registerController } from './registerController'; diff --git a/packages/cli/src/decorators/registerController.ts b/packages/cli/src/decorators/registerController.ts index 5153e4d150252..991b5a185e990 100644 --- a/packages/cli/src/decorators/registerController.ts +++ b/packages/cli/src/decorators/registerController.ts @@ -4,7 +4,7 @@ import type { Config } from '@/config'; import { CONTROLLER_BASE_PATH, CONTROLLER_MIDDLEWARES, CONTROLLER_ROUTES } from './constants'; import { send } from '@/ResponseHelper'; // TODO: move `ResponseHelper.send` to this file import type { Application, Request, Response, RequestHandler } from 'express'; -import type { Controller, RouteMetadata } from './types'; +import type { Controller, MiddlewareMetadata, RouteMetadata } from './types'; export const registerController = (app: Application, config: Config, controller: object) => { const controllerClass = controller.constructor; @@ -14,16 +14,19 @@ export const registerController = (app: Application, config: Config, controller: if (!controllerBasePath) throw new Error(`${controllerClass.name} is missing the RestController decorator`); - const middlewares = Reflect.getMetadata( - CONTROLLER_MIDDLEWARES, - controllerClass, - ) as RequestHandler[]; const routes = Reflect.getMetadata(CONTROLLER_ROUTES, controllerClass) as RouteMetadata[]; if (routes.length > 0) { const router = Router({ mergeParams: true }); const restBasePath = config.getEnv('endpoints.rest'); const prefix = `/${[restBasePath, controllerBasePath].join('/')}`.replace(/\/+/g, '/'); + const middlewares = ( + (Reflect.getMetadata(CONTROLLER_MIDDLEWARES, controllerClass) ?? []) as MiddlewareMetadata[] + ).map( + ({ handlerName }) => + (controller as Controller)[handlerName].bind(controller) as RequestHandler, + ); + routes.forEach(({ method, path, handlerName }) => { router[method]( path, diff --git a/packages/cli/src/decorators/types.ts b/packages/cli/src/decorators/types.ts index 31ddc75274b0b..790833c3a9228 100644 --- a/packages/cli/src/decorators/types.ts +++ b/packages/cli/src/decorators/types.ts @@ -2,6 +2,10 @@ import type { Request, Response } from 'express'; export type Method = 'get' | 'post' | 'put' | 'patch' | 'delete'; +export interface MiddlewareMetadata { + handlerName: string; +} + export interface RouteMetadata { method: Method; path: string; diff --git a/packages/cli/test/integration/shared/utils.ts b/packages/cli/test/integration/shared/utils.ts index 1cae3b82e0147..6aeded0866e99 100644 --- a/packages/cli/test/integration/shared/utils.ts +++ b/packages/cli/test/integration/shared/utils.ts @@ -33,7 +33,6 @@ import * as Db from '@/Db'; import { WorkflowEntity } from '@db/entities/WorkflowEntity'; import { ExternalHooks } from '@/ExternalHooks'; import { ActiveWorkflowRunner } from '@/ActiveWorkflowRunner'; -import { nodesController } from '@/api/nodes.api'; import { workflowsController } from '@/workflows/workflows.controller'; import { AUTH_COOKIE_NAME, NODE_PACKAGE_PREFIX } from '@/constants'; import { credentialsController } from '@/credentials/credentials.controller'; @@ -65,6 +64,7 @@ import { AuthController, LdapController, MeController, + NodesController, OwnerController, PasswordResetController, UsersController, @@ -77,8 +77,9 @@ import { InternalHooks } from '@/InternalHooks'; import { LoadNodesAndCredentials } from '@/LoadNodesAndCredentials'; import { PostHogClient } from '@/posthog'; import { LdapManager } from '@/Ldap/LdapManager.ee'; -import { LDAP_DEFAULT_CONFIGURATION, LDAP_ENABLED } from '@/Ldap/constants'; -import { handleLdapInit, encryptPassword } from '@/Ldap/helpers'; +import { LDAP_ENABLED } from '@/Ldap/constants'; +import { handleLdapInit } from '@/Ldap/helpers'; +import { Push } from '@/push'; export const mockInstance = ( ctor: new (...args: any[]) => T, @@ -155,7 +156,6 @@ export async function initTestServer({ const map: Record = { credentials: { controller: credentialsController, path: 'credentials' }, workflows: { controller: workflowsController, path: 'workflows' }, - nodes: { controller: nodesController, path: 'nodes' }, license: { controller: licenseController, path: 'license' }, eventBus: { controller: eventBusRouter, path: 'eventbus' }, }; @@ -199,6 +199,17 @@ export async function initTestServer({ new LdapController(service, sync, internalHooks), ); break; + case 'nodes': + registerController( + testServer.app, + config, + new NodesController( + config, + Container.get(LoadNodesAndCredentials), + Container.get(Push), + internalHooks, + ), + ); case 'me': registerController( testServer.app, @@ -255,7 +266,7 @@ const classifyEndpointGroups = (endpointGroups: EndpointGroup[]) => { const routerEndpoints: EndpointGroup[] = []; const functionEndpoints: EndpointGroup[] = []; - const ROUTER_GROUP = ['credentials', 'nodes', 'workflows', 'publicApi', 'eventBus', 'license']; + const ROUTER_GROUP = ['credentials', 'workflows', 'publicApi', 'eventBus', 'license']; endpointGroups.forEach((group) => (ROUTER_GROUP.includes(group) ? routerEndpoints : functionEndpoints).push(group),