From f4f0134e6185963a65aca89ea0c093e8341eb215 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, 27 Jan 2023 11:58:23 +0100 Subject: [PATCH 1/5] move nodeTypes api to a controller class --- packages/cli/src/Server.ts | 11 ++--- packages/cli/src/controllers/index.ts | 1 + .../nodeTypes.controller.ts} | 46 ++++++++++--------- packages/cli/src/index.ts | 1 - 4 files changed, 29 insertions(+), 30 deletions(-) rename packages/cli/src/{api/nodeTypes.api.ts => controllers/nodeTypes.controller.ts} (60%) diff --git a/packages/cli/src/Server.ts b/packages/cli/src/Server.ts index 4c73c7c3565a6..83ed2470edf95 100644 --- a/packages/cli/src/Server.ts +++ b/packages/cli/src/Server.ts @@ -84,6 +84,7 @@ import { registerController } from '@/decorators'; import { AuthController, MeController, + NodeTypesController, OwnerController, PasswordResetController, TranslationController, @@ -91,7 +92,6 @@ import { } from '@/controllers'; import { executionsController } from '@/executions/executions.controller'; -import { nodeTypesController } from '@/api/nodeTypes.api'; import { tagsController } from '@/api/tags.api'; import { workflowStatsController } from '@/api/workflowStats.api'; import { loadPublicApiVersions } from '@/PublicApi'; @@ -366,7 +366,7 @@ class Server extends AbstractServer { } private registerControllers(ignoredEndpoints: Readonly) { - const { app, externalHooks, activeWorkflowRunner } = this; + const { app, externalHooks, activeWorkflowRunner, nodeTypes } = this; const repositories = Db.collections; setupAuthMiddlewares(app, ignoredEndpoints, this.restEndpoint, repositories.User); @@ -391,6 +391,7 @@ class Server extends AbstractServer { logger, postHog, }), + new NodeTypesController({ config, nodeTypes }), ]; controllers.forEach((controller) => registerController(app, config, controller)); } @@ -640,12 +641,6 @@ class Server extends AbstractServer { ), ); - // ---------------------------------------- - // Node-Types - // ---------------------------------------- - - this.app.use(`/${this.restEndpoint}/node-types`, nodeTypesController); - // ---------------------------------------- // Active Workflows // ---------------------------------------- diff --git a/packages/cli/src/controllers/index.ts b/packages/cli/src/controllers/index.ts index 37ce548a540dc..557db2d313e93 100644 --- a/packages/cli/src/controllers/index.ts +++ b/packages/cli/src/controllers/index.ts @@ -1,5 +1,6 @@ export { AuthController } from './auth.controller'; export { MeController } from './me.controller'; +export { NodeTypesController } from './nodeTypes.controller'; export { OwnerController } from './owner.controller'; export { PasswordResetController } from './passwordReset.controller'; export { TranslationController } from './translation.controller'; diff --git a/packages/cli/src/api/nodeTypes.api.ts b/packages/cli/src/controllers/nodeTypes.controller.ts similarity index 60% rename from packages/cli/src/api/nodeTypes.api.ts rename to packages/cli/src/controllers/nodeTypes.controller.ts index de37b549f5b78..5f8af5a909f22 100644 --- a/packages/cli/src/api/nodeTypes.api.ts +++ b/packages/cli/src/controllers/nodeTypes.controller.ts @@ -1,39 +1,43 @@ -import express from 'express'; import { readFile } from 'fs/promises'; import get from 'lodash.get'; - +import { Request } from 'express'; import type { INodeTypeDescription, INodeTypeNameVersion } from 'n8n-workflow'; - -import config from '@/config'; -import { NodeTypes } from '@/NodeTypes'; -import * as ResponseHelper from '@/ResponseHelper'; +import { Post, RestController } from '@/decorators'; import { getNodeTranslationPath } from '@/TranslationHelpers'; -import { Container } from 'typedi'; +import type { Config } from '@/config'; +import type { NodeTypes } from '@/NodeTypes'; + +@RestController('/node-types') +export class NodeTypesController { + private readonly config: Config; -export const nodeTypesController = express.Router(); + private readonly nodeTypes: NodeTypes; -// Returns node information based on node names and versions -nodeTypesController.post( - '/', - ResponseHelper.send(async (req: express.Request): Promise => { + constructor({ config, nodeTypes }: { config: Config; nodeTypes: NodeTypes }) { + this.config = config; + this.nodeTypes = nodeTypes; + } + + @Post('/') + async getNodeInfo(req: Request) { const nodeInfos = get(req, 'body.nodeInfos', []) as INodeTypeNameVersion[]; - const defaultLocale = config.getEnv('defaultLocale'); + const defaultLocale = this.config.getEnv('defaultLocale'); if (defaultLocale === 'en') { return nodeInfos.reduce((acc, { name, version }) => { - const { description } = Container.get(NodeTypes).getByNameAndVersion(name, version); + const { description } = this.nodeTypes.getByNameAndVersion(name, version); acc.push(description); return acc; }, []); } - async function populateTranslation( + const populateTranslation = async ( name: string, version: number, nodeTypes: INodeTypeDescription[], - ) { - const { description, sourcePath } = Container.get(NodeTypes).getWithSourcePath(name, version); + ) => { + const { description, sourcePath } = this.nodeTypes.getWithSourcePath(name, version); const translationPath = await getNodeTranslationPath({ nodeSourcePath: sourcePath, longNodeType: description.name, @@ -44,12 +48,12 @@ nodeTypesController.post( const translation = await readFile(translationPath, 'utf8'); // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment description.translation = JSON.parse(translation); - } catch (error) { + } catch { // ignore - no translation exists at path } nodeTypes.push(description); - } + }; const nodeTypes: INodeTypeDescription[] = []; @@ -60,5 +64,5 @@ nodeTypesController.post( await Promise.all(promises); return nodeTypes; - }), -); + } +} diff --git a/packages/cli/src/index.ts b/packages/cli/src/index.ts index 81b43dd31745f..af889d15f9de4 100644 --- a/packages/cli/src/index.ts +++ b/packages/cli/src/index.ts @@ -3,7 +3,6 @@ export * from './CredentialsHelper'; export * from './CredentialTypes'; export * from './CredentialsOverwrites'; export * from './Interfaces'; -export * from './NodeTypes'; export * from './WaitingWebhooks'; export * from './WorkflowCredentials'; export * from './WorkflowRunner'; From 9d66723c00b71bd545da1b475b75b1ad9bbfc7e1 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: Thu, 9 Feb 2023 23:05:38 +0100 Subject: [PATCH 2/5] move tags api to a controller class --- packages/cli/src/Server.ts | 10 +- packages/cli/src/api/tags.api.ts | 110 ------------------ packages/cli/src/controllers/index.ts | 1 + .../cli/src/controllers/tags.controller.ts | 106 +++++++++++++++++ packages/cli/src/decorators/RestController.ts | 6 +- packages/cli/src/decorators/constants.ts | 1 + .../cli/src/decorators/registerController.ts | 9 +- 7 files changed, 122 insertions(+), 121 deletions(-) delete mode 100644 packages/cli/src/api/tags.api.ts create mode 100644 packages/cli/src/controllers/tags.controller.ts diff --git a/packages/cli/src/Server.ts b/packages/cli/src/Server.ts index 83ed2470edf95..cc1ca8d9f486c 100644 --- a/packages/cli/src/Server.ts +++ b/packages/cli/src/Server.ts @@ -87,12 +87,12 @@ import { NodeTypesController, OwnerController, PasswordResetController, + TagsController, TranslationController, UsersController, } from '@/controllers'; import { executionsController } from '@/executions/executions.controller'; -import { tagsController } from '@/api/tags.api'; import { workflowStatsController } from '@/api/workflowStats.api'; import { loadPublicApiVersions } from '@/PublicApi'; import { @@ -379,7 +379,9 @@ class Server extends AbstractServer { new AuthController({ config, internalHooks, repositories, logger, postHog }), new OwnerController({ config, internalHooks, repositories, logger }), new MeController({ externalHooks, internalHooks, repositories, logger }), + new NodeTypesController({ config, nodeTypes }), new PasswordResetController({ config, externalHooks, internalHooks, repositories, logger }), + new TagsController({ config, repositories, externalHooks }), new TranslationController(config, this.credentialTypes), new UsersController({ config, @@ -391,7 +393,6 @@ class Server extends AbstractServer { logger, postHog, }), - new NodeTypesController({ config, nodeTypes }), ]; controllers.forEach((controller) => registerController(app, config, controller)); } @@ -500,11 +501,6 @@ class Server extends AbstractServer { // ---------------------------------------- this.app.use(`/${this.restEndpoint}/workflow-stats`, workflowStatsController); - // ---------------------------------------- - // Tags - // ---------------------------------------- - this.app.use(`/${this.restEndpoint}/tags`, tagsController); - // ---------------------------------------- // LDAP // ---------------------------------------- diff --git a/packages/cli/src/api/tags.api.ts b/packages/cli/src/api/tags.api.ts deleted file mode 100644 index 6c13c44b5136a..0000000000000 --- a/packages/cli/src/api/tags.api.ts +++ /dev/null @@ -1,110 +0,0 @@ -/* eslint-disable @typescript-eslint/no-unnecessary-boolean-literal-compare */ -/* eslint-disable @typescript-eslint/no-invalid-void-type */ -/* eslint-disable @typescript-eslint/no-unsafe-call */ -/* eslint-disable @typescript-eslint/no-unsafe-member-access */ -/* eslint-disable @typescript-eslint/no-unsafe-assignment */ -/* eslint-disable no-param-reassign */ - -import express from 'express'; - -import * as Db from '@/Db'; -import { ExternalHooks } from '@/ExternalHooks'; -import type { ITagWithCountDb } from '@/Interfaces'; -import * as ResponseHelper from '@/ResponseHelper'; -import config from '@/config'; -import * as TagHelpers from '@/TagHelpers'; -import { validateEntity } from '@/GenericHelpers'; -import { TagEntity } from '@db/entities/TagEntity'; -import type { TagsRequest } from '@/requests'; -import { Container } from 'typedi'; - -export const tagsController = express.Router(); - -const workflowsEnabledMiddleware: express.RequestHandler = (req, res, next) => { - if (config.getEnv('workflowTagsDisabled')) { - throw new ResponseHelper.BadRequestError('Workflow tags are disabled'); - } - next(); -}; - -// Retrieves all tags, with or without usage count -tagsController.get( - '/', - workflowsEnabledMiddleware, - ResponseHelper.send(async (req: express.Request): Promise => { - if (req.query.withUsageCount === 'true') { - const tablePrefix = config.getEnv('database.tablePrefix'); - return TagHelpers.getTagsWithCountDb(tablePrefix); - } - - return Db.collections.Tag.find({ select: ['id', 'name', 'createdAt', 'updatedAt'] }); - }), -); - -// Creates a tag -tagsController.post( - '/', - workflowsEnabledMiddleware, - ResponseHelper.send(async (req: express.Request): Promise => { - const newTag = new TagEntity(); - newTag.name = req.body.name.trim(); - - await Container.get(ExternalHooks).run('tag.beforeCreate', [newTag]); - - await validateEntity(newTag); - const tag = await Db.collections.Tag.save(newTag); - - await Container.get(ExternalHooks).run('tag.afterCreate', [tag]); - - return tag; - }), -); - -// Updates a tag -tagsController.patch( - '/:id(\\d+)', - workflowsEnabledMiddleware, - ResponseHelper.send(async (req: express.Request): Promise => { - const { name } = req.body; - const { id } = req.params; - - const newTag = new TagEntity(); - // @ts-ignore - newTag.id = id; - newTag.name = name.trim(); - - await Container.get(ExternalHooks).run('tag.beforeUpdate', [newTag]); - - await validateEntity(newTag); - const tag = await Db.collections.Tag.save(newTag); - - await Container.get(ExternalHooks).run('tag.afterUpdate', [tag]); - - return tag; - }), -); - -tagsController.delete( - '/:id(\\d+)', - workflowsEnabledMiddleware, - ResponseHelper.send(async (req: TagsRequest.Delete): Promise => { - if ( - config.getEnv('userManagement.isInstanceOwnerSetUp') === true && - req.user.globalRole.name !== 'owner' - ) { - throw new ResponseHelper.UnauthorizedError( - 'You are not allowed to perform this action', - 'Only owners can remove tags', - ); - } - const id = req.params.id; - - await Container.get(ExternalHooks).run('tag.beforeDelete', [id]); - - await Db.collections.Tag.delete({ id }); - - await Container.get(ExternalHooks).run('tag.afterDelete', [id]); - - return true; - }), -); diff --git a/packages/cli/src/controllers/index.ts b/packages/cli/src/controllers/index.ts index 557db2d313e93..496dcf1d54f20 100644 --- a/packages/cli/src/controllers/index.ts +++ b/packages/cli/src/controllers/index.ts @@ -3,5 +3,6 @@ export { MeController } from './me.controller'; export { NodeTypesController } from './nodeTypes.controller'; export { OwnerController } from './owner.controller'; export { PasswordResetController } from './passwordReset.controller'; +export { TagsController } from './tags.controller'; export { TranslationController } from './translation.controller'; export { UsersController } from './users.controller'; diff --git a/packages/cli/src/controllers/tags.controller.ts b/packages/cli/src/controllers/tags.controller.ts new file mode 100644 index 0000000000000..fa8ed81e7e6f6 --- /dev/null +++ b/packages/cli/src/controllers/tags.controller.ts @@ -0,0 +1,106 @@ +/* eslint-disable @typescript-eslint/no-shadow */ +import type { RequestHandler } from 'express'; +import { Request } 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 type { IDatabaseCollections, IExternalHooksClass, ITagWithCountDb } from '@/Interfaces'; +import { TagEntity } from '@db/entities/TagEntity'; +import { getTagsWithCountDb } from '@/TagHelpers'; +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) +export class TagsController { + private config: Config; + + private externalHooks: IExternalHooksClass; + + private tagsRepository: Repository; + + constructor({ + config, + externalHooks, + repositories, + }: { + config: Config; + externalHooks: IExternalHooksClass; + repositories: Pick; + }) { + this.config = config; + this.externalHooks = externalHooks; + this.tagsRepository = repositories.Tag; + } + + // Retrieves all tags, with or without usage count + @Get('/') + async getALL( + req: Request<{}, {}, {}, { withUsageCount: string }>, + ): Promise { + const { withUsageCount } = req.query; + if (withUsageCount === 'true') { + const tablePrefix = this.config.getEnv('database.tablePrefix'); + return getTagsWithCountDb(tablePrefix); + } + + return this.tagsRepository.find({ select: ['id', 'name', 'createdAt', 'updatedAt'] }); + } + + // Creates a tag + @Post('/') + async createTag(req: Request<{}, {}, { name: string }>): Promise { + const newTag = new TagEntity(); + newTag.name = req.body.name.trim(); + + await this.externalHooks.run('tag.beforeCreate', [newTag]); + await validateEntity(newTag); + + const tag = await this.tagsRepository.save(newTag); + await this.externalHooks.run('tag.afterCreate', [tag]); + return tag; + } + + // Updates a tag + @Patch('/:id(\\d+)') + async updateTag(req: Request<{ id: string }, {}, { name: string }>): Promise { + const { name } = req.body; + const { id } = req.params; + + const newTag = new TagEntity(); + newTag.id = id; + newTag.name = name.trim(); + + await this.externalHooks.run('tag.beforeUpdate', [newTag]); + await validateEntity(newTag); + + const tag = await this.tagsRepository.save(newTag); + await this.externalHooks.run('tag.afterUpdate', [tag]); + return tag; + } + + @Delete('/:id(\\d+)') + async deleteTag(req: TagsRequest.Delete) { + const isInstanceOwnerSetUp = this.config.getEnv('userManagement.isInstanceOwnerSetUp'); + if (isInstanceOwnerSetUp && req.user.globalRole.name !== 'owner') { + throw new UnauthorizedError( + 'You are not allowed to perform this action', + 'Only owners can remove tags', + ); + } + const { id } = req.params; + await this.externalHooks.run('tag.beforeDelete', [id]); + + await this.tagsRepository.delete({ id }); + await this.externalHooks.run('tag.afterDelete', [id]); + return true; + } +} diff --git a/packages/cli/src/decorators/RestController.ts b/packages/cli/src/decorators/RestController.ts index 11c2d55665b39..09ec49d0185b1 100644 --- a/packages/cli/src/decorators/RestController.ts +++ b/packages/cli/src/decorators/RestController.ts @@ -1,8 +1,10 @@ /* eslint-disable @typescript-eslint/naming-convention */ -import { CONTROLLER_BASE_PATH } from './constants'; +import type { RequestHandler } from 'express'; +import { CONTROLLER_BASE_PATH, CONTROLLER_MIDDLEWARES } from './constants'; export const RestController = - (basePath: `/${string}` = '/'): ClassDecorator => + (basePath: `/${string}` = '/', ...middlewares: RequestHandler[]): ClassDecorator => (target: object) => { Reflect.defineMetadata(CONTROLLER_BASE_PATH, basePath, target); + Reflect.defineMetadata(CONTROLLER_MIDDLEWARES, middlewares, target); }; diff --git a/packages/cli/src/decorators/constants.ts b/packages/cli/src/decorators/constants.ts index 9c77ab696e172..6bff0e4a6cb90 100644 --- a/packages/cli/src/decorators/constants.ts +++ b/packages/cli/src/decorators/constants.ts @@ -1,2 +1,3 @@ export const CONTROLLER_ROUTES = 'CONTROLLER_ROUTES'; export const CONTROLLER_BASE_PATH = 'CONTROLLER_BASE_PATH'; +export const CONTROLLER_MIDDLEWARES = 'CONTROLLER_MIDDLEWARES'; diff --git a/packages/cli/src/decorators/registerController.ts b/packages/cli/src/decorators/registerController.ts index 9f1ab5bba72e2..5153e4d150252 100644 --- a/packages/cli/src/decorators/registerController.ts +++ b/packages/cli/src/decorators/registerController.ts @@ -1,9 +1,9 @@ /* eslint-disable @typescript-eslint/naming-convention */ import { Router } from 'express'; import type { Config } from '@/config'; -import { CONTROLLER_BASE_PATH, CONTROLLER_ROUTES } from './constants'; +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 } from 'express'; +import type { Application, Request, Response, RequestHandler } from 'express'; import type { Controller, RouteMetadata } from './types'; export const registerController = (app: Application, config: Config, controller: object) => { @@ -14,6 +14,10 @@ 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 }); @@ -23,6 +27,7 @@ export const registerController = (app: Application, config: Config, controller: routes.forEach(({ method, path, handlerName }) => { router[method]( path, + ...middlewares, send(async (req: Request, res: Response) => (controller as Controller)[handlerName](req, res), ), From a6d7b42fc6b8e5d753f5b5904fbebc2cebc12371 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, 24 Feb 2023 15:39:58 +0100 Subject: [PATCH 3/5] move LDAP routes to a controller class --- .../cli/src/Ldap/routes/ldap.controller.ee.ts | 77 ------------------- packages/cli/src/Server.ts | 19 +++-- packages/cli/src/controllers/index.ts | 1 + .../cli/src/controllers/ldap.controller.ts | 65 ++++++++++++++++ packages/cli/src/decorators/Route.ts | 1 + packages/cli/src/decorators/index.ts | 2 +- packages/cli/src/decorators/types.ts | 2 +- packages/cli/src/middlewares/auth.ts | 2 +- .../test/integration/ldap/ldap.api.test.ts | 2 - packages/cli/test/integration/shared/utils.ts | 36 ++++----- 10 files changed, 94 insertions(+), 113 deletions(-) delete mode 100644 packages/cli/src/Ldap/routes/ldap.controller.ee.ts create mode 100644 packages/cli/src/controllers/ldap.controller.ts diff --git a/packages/cli/src/Ldap/routes/ldap.controller.ee.ts b/packages/cli/src/Ldap/routes/ldap.controller.ee.ts deleted file mode 100644 index aac5938415d7e..0000000000000 --- a/packages/cli/src/Ldap/routes/ldap.controller.ee.ts +++ /dev/null @@ -1,77 +0,0 @@ -import express from 'express'; -import { LdapManager } from '../LdapManager.ee'; -import { getLdapConfig, getLdapSynchronizations, updateLdapConfig } from '../helpers'; -import type { LdapConfiguration } from '../types'; -import pick from 'lodash.pick'; -import { NON_SENSIBLE_LDAP_CONFIG_PROPERTIES } from '../constants'; -import { InternalHooks } from '@/InternalHooks'; -import { Container } from 'typedi'; - -export const ldapController = express.Router(); - -/** - * GET /ldap/config - */ -ldapController.get('/config', async (req: express.Request, res: express.Response) => { - const data = await getLdapConfig(); - return res.status(200).json({ data }); -}); -/** - * POST /ldap/test-connection - */ -ldapController.post('/test-connection', async (req: express.Request, res: express.Response) => { - try { - await LdapManager.getInstance().service.testConnection(); - } catch (error) { - const errorObject = error as { message: string }; - return res.status(400).json({ message: errorObject.message }); - } - return res.status(200).json(); -}); - -/** - * PUT /ldap/config - */ -ldapController.put('/config', async (req: LdapConfiguration.Update, res: express.Response) => { - try { - await updateLdapConfig(req.body); - } catch (e) { - if (e instanceof Error) { - return res.status(400).json({ message: e.message }); - } - } - - const data = await getLdapConfig(); - - void Container.get(InternalHooks).onUserUpdatedLdapSettings({ - user_id: req.user.id, - ...pick(data, NON_SENSIBLE_LDAP_CONFIG_PROPERTIES), - }); - - return res.status(200).json({ data }); -}); - -/** - * POST /ldap/sync - */ -ldapController.post('/sync', async (req: LdapConfiguration.Sync, res: express.Response) => { - const runType = req.body.type; - - try { - await LdapManager.getInstance().sync.run(runType); - } catch (e) { - if (e instanceof Error) { - return res.status(400).json({ message: e.message }); - } - } - return res.status(200).json({}); -}); - -/** - * GET /ldap/sync - */ -ldapController.get('/sync', async (req: LdapConfiguration.GetSync, res: express.Response) => { - const { page = '0', perPage = '20' } = req.query; - const data = await getLdapSynchronizations(parseInt(page, 10), parseInt(perPage, 10)); - return res.status(200).json({ data }); -}); diff --git a/packages/cli/src/Server.ts b/packages/cli/src/Server.ts index cc1ca8d9f486c..e51f837c30363 100644 --- a/packages/cli/src/Server.ts +++ b/packages/cli/src/Server.ts @@ -83,6 +83,7 @@ import type { import { registerController } from '@/decorators'; import { AuthController, + LdapController, MeController, NodeTypesController, OwnerController, @@ -134,7 +135,6 @@ import { licenseController } from './license/license.controller'; import { Push, setupPushServer, setupPushHandler } from '@/push'; import { setupAuthMiddlewares } from './middlewares'; import { initEvents } from './events'; -import { ldapController } from './Ldap/routes/ldap.controller.ee'; import { getLdapLoginLabel, isLdapEnabled, isLdapLoginEnabled } from './Ldap/helpers'; import { AbstractServer } from './AbstractServer'; import { configureMetrics } from './metrics'; @@ -149,6 +149,7 @@ import { getSamlLoginLabel, isSamlLoginEnabled, isSamlLicensed } from './sso/sam import { samlControllerPublic } from './sso/saml/routes/saml.controller.public.ee'; import { SamlService } from './sso/saml/saml.service.ee'; import { samlControllerProtected } from './sso/saml/routes/saml.controller.protected.ee'; +import { LdapManager } from './Ldap/LdapManager.ee'; const exec = promisify(callbackExec); @@ -375,7 +376,7 @@ class Server extends AbstractServer { const mailer = getMailerInstance(); const postHog = this.postHog; - const controllers = [ + const controllers: object[] = [ new AuthController({ config, internalHooks, repositories, logger, postHog }), new OwnerController({ config, internalHooks, repositories, logger }), new MeController({ externalHooks, internalHooks, repositories, logger }), @@ -394,6 +395,12 @@ class Server extends AbstractServer { postHog, }), ]; + + if (isLdapEnabled()) { + const { service, sync } = LdapManager.getInstance(); + controllers.push(new LdapController(service, sync, internalHooks)); + } + controllers.forEach((controller) => registerController(app, config, controller)); } @@ -501,13 +508,6 @@ class Server extends AbstractServer { // ---------------------------------------- this.app.use(`/${this.restEndpoint}/workflow-stats`, workflowStatsController); - // ---------------------------------------- - // LDAP - // ---------------------------------------- - if (isLdapEnabled()) { - this.app.use(`/${this.restEndpoint}/ldap`, ldapController); - } - // ---------------------------------------- // SAML // ---------------------------------------- @@ -526,7 +526,6 @@ class Server extends AbstractServer { this.app.use(`/${this.restEndpoint}/sso/saml`, samlControllerProtected); // ---------------------------------------- - // Returns parameter values which normally get loaded from an external API or // get generated dynamically this.app.get( diff --git a/packages/cli/src/controllers/index.ts b/packages/cli/src/controllers/index.ts index 496dcf1d54f20..3cabcf987b2e3 100644 --- a/packages/cli/src/controllers/index.ts +++ b/packages/cli/src/controllers/index.ts @@ -1,4 +1,5 @@ export { AuthController } from './auth.controller'; +export { LdapController } from './ldap.controller'; export { MeController } from './me.controller'; export { NodeTypesController } from './nodeTypes.controller'; export { OwnerController } from './owner.controller'; diff --git a/packages/cli/src/controllers/ldap.controller.ts b/packages/cli/src/controllers/ldap.controller.ts new file mode 100644 index 0000000000000..619a70db286b2 --- /dev/null +++ b/packages/cli/src/controllers/ldap.controller.ts @@ -0,0 +1,65 @@ +import pick from 'lodash.pick'; +import { Get, Post, Put, RestController } from '@/decorators'; +import { getLdapConfig, getLdapSynchronizations, updateLdapConfig } from '@/Ldap/helpers'; +import { LdapService } from '@/Ldap/LdapService.ee'; +import { LdapSync } from '@/Ldap/LdapSync.ee'; +import { LdapConfiguration } from '@/Ldap/types'; +import { BadRequestError } from '@/ResponseHelper'; +import { NON_SENSIBLE_LDAP_CONFIG_PROPERTIES } from '@/Ldap/constants'; +import { InternalHooks } from '@/InternalHooks'; + +@RestController('/ldap') +export class LdapController { + constructor( + private ldapService: LdapService, + private ldapSync: LdapSync, + private internalHooks: InternalHooks, + ) {} + + @Get('/config') + async getConfig() { + return getLdapConfig(); + } + + @Post('/test-connection') + async testConnection() { + try { + await this.ldapService.testConnection(); + } catch (error) { + throw new BadRequestError((error as { message: string }).message); + } + } + + @Put('/config') + async updateConfig(req: LdapConfiguration.Update) { + try { + await updateLdapConfig(req.body); + } catch (error) { + throw new BadRequestError((error as { message: string }).message); + } + + const data = await getLdapConfig(); + + void this.internalHooks.onUserUpdatedLdapSettings({ + user_id: req.user.id, + ...pick(data, NON_SENSIBLE_LDAP_CONFIG_PROPERTIES), + }); + + return data; + } + + @Get('/sync') + async getLdapSync(req: LdapConfiguration.GetSync) { + const { page = '0', perPage = '20' } = req.query; + return getLdapSynchronizations(parseInt(page, 10), parseInt(perPage, 10)); + } + + @Post('/sync') + async syncLdap(req: LdapConfiguration.Sync) { + try { + await this.ldapSync.run(req.body.type); + } catch (error) { + throw new BadRequestError((error as { message: string }).message); + } + } +} diff --git a/packages/cli/src/decorators/Route.ts b/packages/cli/src/decorators/Route.ts index 773bb2193ac26..59461cbc8709c 100644 --- a/packages/cli/src/decorators/Route.ts +++ b/packages/cli/src/decorators/Route.ts @@ -15,5 +15,6 @@ const RouteFactory = export const Get = RouteFactory('get'); export const Post = RouteFactory('post'); +export const Put = RouteFactory('put'); export const Patch = RouteFactory('patch'); export const Delete = RouteFactory('delete'); diff --git a/packages/cli/src/decorators/index.ts b/packages/cli/src/decorators/index.ts index bda31ce887b70..6c345c85a38a9 100644 --- a/packages/cli/src/decorators/index.ts +++ b/packages/cli/src/decorators/index.ts @@ -1,3 +1,3 @@ export { RestController } from './RestController'; -export { Get, Post, Patch, Delete } from './Route'; +export { Get, Post, Put, Patch, Delete } from './Route'; export { registerController } from './registerController'; diff --git a/packages/cli/src/decorators/types.ts b/packages/cli/src/decorators/types.ts index 829451f4d77cc..31ddc75274b0b 100644 --- a/packages/cli/src/decorators/types.ts +++ b/packages/cli/src/decorators/types.ts @@ -1,6 +1,6 @@ import type { Request, Response } from 'express'; -export type Method = 'get' | 'post' | 'patch' | 'delete'; +export type Method = 'get' | 'post' | 'put' | 'patch' | 'delete'; export interface RouteMetadata { method: Method; diff --git a/packages/cli/src/middlewares/auth.ts b/packages/cli/src/middlewares/auth.ts index 34c76ea592b8d..1299f474084b6 100644 --- a/packages/cli/src/middlewares/auth.ts +++ b/packages/cli/src/middlewares/auth.ts @@ -18,7 +18,7 @@ import { } from '@/UserManagement/UserManagementHelper'; import type { Repository } from 'typeorm'; import type { User } from '@db/entities/User'; -import { SamlUrls } from '../sso/saml/constants'; +import { SamlUrls } from '@/sso/saml/constants'; const jwtFromRequest = (req: Request) => { // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access diff --git a/packages/cli/test/integration/ldap/ldap.api.test.ts b/packages/cli/test/integration/ldap/ldap.api.test.ts index 36b89244692be..f823355758017 100644 --- a/packages/cli/test/integration/ldap/ldap.api.test.ts +++ b/packages/cli/test/integration/ldap/ldap.api.test.ts @@ -49,13 +49,11 @@ beforeAll(async () => { authAgent = utils.createAuthAgent(app); - config.set(LDAP_ENABLED, true); defaultLdapConfig.bindingAdminPassword = await encryptPassword( defaultLdapConfig.bindingAdminPassword, ); utils.initConfigFile(); - await utils.initLdapManager(); }); beforeEach(async () => { diff --git a/packages/cli/test/integration/shared/utils.ts b/packages/cli/test/integration/shared/utils.ts index ba28f16732656..1cae3b82e0147 100644 --- a/packages/cli/test/integration/shared/utils.ts +++ b/packages/cli/test/integration/shared/utils.ts @@ -31,9 +31,7 @@ import { DeepPartial } from 'ts-essentials'; import config from '@/config'; import * as Db from '@/Db'; import { WorkflowEntity } from '@db/entities/WorkflowEntity'; -import { CredentialTypes } from '@/CredentialTypes'; import { ExternalHooks } from '@/ExternalHooks'; -import { NodeTypes } from '@/NodeTypes'; import { ActiveWorkflowRunner } from '@/ActiveWorkflowRunner'; import { nodesController } from '@/api/nodes.api'; import { workflowsController } from '@/workflows/workflows.controller'; @@ -65,6 +63,7 @@ import { eventBusRouter } from '@/eventbus/eventBusRoutes'; import { registerController } from '@/decorators'; import { AuthController, + LdapController, MeController, OwnerController, PasswordResetController, @@ -74,11 +73,12 @@ import { setupAuthMiddlewares } from '@/middlewares'; import * as testDb from '../shared/testDb'; import { v4 as uuid } from 'uuid'; -import { handleLdapInit } from '@/Ldap/helpers'; -import { ldapController } from '@/Ldap/routes/ldap.controller.ee'; 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'; export const mockInstance = ( ctor: new (...args: any[]) => T, @@ -158,7 +158,6 @@ export async function initTestServer({ nodes: { controller: nodesController, path: 'nodes' }, license: { controller: licenseController, path: 'license' }, eventBus: { controller: eventBusRouter, path: 'eventbus' }, - ldap: { controller: ldapController, path: 'ldap' }, }; if (enablePublicAPI) { @@ -190,6 +189,16 @@ export async function initTestServer({ new AuthController({ config, logger, internalHooks, repositories }), ); break; + case 'ldap': + config.set(LDAP_ENABLED, true); + await handleLdapInit(); + const { service, sync } = LdapManager.getInstance(); + registerController( + testServer.app, + config, + new LdapController(service, sync, internalHooks), + ); + break; case 'me': registerController( testServer.app, @@ -246,15 +255,7 @@ const classifyEndpointGroups = (endpointGroups: EndpointGroup[]) => { const routerEndpoints: EndpointGroup[] = []; const functionEndpoints: EndpointGroup[] = []; - const ROUTER_GROUP = [ - 'credentials', - 'nodes', - 'workflows', - 'publicApi', - 'ldap', - 'eventBus', - 'license', - ]; + const ROUTER_GROUP = ['credentials', 'nodes', 'workflows', 'publicApi', 'eventBus', 'license']; endpointGroups.forEach((group) => (ROUTER_GROUP.includes(group) ? routerEndpoints : functionEndpoints).push(group), @@ -320,13 +321,6 @@ export async function initCredentialsTypes(): Promise { }; } -/** - * Initialize LDAP manager. - */ -export async function initLdapManager(): Promise { - await handleLdapInit(); -} - /** * Initialize node types. */ From e36aaccdcf44c6292272dfc7bd5942b81a815ca0 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 Mar 2023 16:12:55 +0100 Subject: [PATCH 4/5] move nodes routes to a controller class --- packages/cli/src/Server.ts | 15 +- packages/cli/src/controllers/index.ts | 1 + .../nodes.controller.ts} | 201 +++++++----------- .../cli/src/controllers/tags.controller.ts | 24 +-- packages/cli/src/decorators/Middleware.ts | 11 + packages/cli/src/decorators/RestController.ts | 6 +- packages/cli/src/decorators/index.ts | 1 + .../cli/src/decorators/registerController.ts | 13 +- packages/cli/src/decorators/types.ts | 4 + packages/cli/test/integration/shared/utils.ts | 21 +- 10 files changed, 141 insertions(+), 156 deletions(-) rename packages/cli/src/{api/nodes.api.ts => controllers/nodes.controller.ts} (64%) create mode 100644 packages/cli/src/decorators/Middleware.ts 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), From 9ff1cc65c7cffed2fb03d479726790c2ab2b5449 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 Mar 2023 13:43:26 +0100 Subject: [PATCH 5/5] move request types to requests.ts --- packages/cli/src/controllers/tags.controller.ts | 8 +++----- packages/cli/src/requests.ts | 3 +++ 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/packages/cli/src/controllers/tags.controller.ts b/packages/cli/src/controllers/tags.controller.ts index b9c87294921ae..3c73235c853b3 100644 --- a/packages/cli/src/controllers/tags.controller.ts +++ b/packages/cli/src/controllers/tags.controller.ts @@ -41,9 +41,7 @@ export class TagsController { // Retrieves all tags, with or without usage count @Get('/') - async getALL( - req: Request<{}, {}, {}, { withUsageCount: string }>, - ): Promise { + async getAll(req: TagsRequest.GetAll): Promise { const { withUsageCount } = req.query; if (withUsageCount === 'true') { const tablePrefix = this.config.getEnv('database.tablePrefix'); @@ -55,7 +53,7 @@ export class TagsController { // Creates a tag @Post('/') - async createTag(req: Request<{}, {}, { name: string }>): Promise { + async createTag(req: TagsRequest.Create): Promise { const newTag = new TagEntity(); newTag.name = req.body.name.trim(); @@ -69,7 +67,7 @@ export class TagsController { // Updates a tag @Patch('/:id(\\d+)') - async updateTag(req: Request<{ id: string }, {}, { name: string }>): Promise { + async updateTag(req: TagsRequest.Update): Promise { const { name } = req.body; const { id } = req.params; diff --git a/packages/cli/src/requests.ts b/packages/cli/src/requests.ts index f8c5af17b4a84..5c520407e0ad4 100644 --- a/packages/cli/src/requests.ts +++ b/packages/cli/src/requests.ts @@ -330,6 +330,9 @@ export type NodeListSearchRequest = AuthenticatedRequest< // ---------------------------------- export declare namespace TagsRequest { + type GetAll = AuthenticatedRequest<{}, {}, {}, { withUsageCount: string }>; + type Create = AuthenticatedRequest<{}, {}, { name: string }>; + type Update = AuthenticatedRequest<{ id: string }, {}, { name: string }>; type Delete = AuthenticatedRequest<{ id: string }>; }