diff --git a/packages/@n8n_io/eslint-config/base.js b/packages/@n8n_io/eslint-config/base.js index 4242429369be4..ffb4f73540f0e 100644 --- a/packages/@n8n_io/eslint-config/base.js +++ b/packages/@n8n_io/eslint-config/base.js @@ -194,6 +194,11 @@ const config = (module.exports = { */ '@typescript-eslint/consistent-type-assertions': 'error', + /** + * https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/consistent-type-imports.md + */ + '@typescript-eslint/consistent-type-imports': 'error', + /** * https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/member-delimiter-style.md */ diff --git a/packages/cli/.eslintrc.js b/packages/cli/.eslintrc.js index 3da2d993a3ca1..2a24787f80e76 100644 --- a/packages/cli/.eslintrc.js +++ b/packages/cli/.eslintrc.js @@ -16,8 +16,6 @@ module.exports = { ], rules: { - '@typescript-eslint/consistent-type-imports': 'error', - // TODO: Remove this 'import/no-cycle': 'warn', 'import/order': 'off', diff --git a/packages/cli/src/CredentialsHelper.ts b/packages/cli/src/CredentialsHelper.ts index 2851638360576..cfc86bfd263af 100644 --- a/packages/cli/src/CredentialsHelper.ts +++ b/packages/cli/src/CredentialsHelper.ts @@ -542,10 +542,10 @@ export class CredentialsHelper extends ICredentialsHelper { ): Promise { const credentialTestFunction = this.getCredentialTestFunction(credentialType); if (credentialTestFunction === undefined) { - return Promise.resolve({ + return { status: 'Error', message: 'No testing function found for this credential.', - }); + }; } if (credentialsDecrypted.data) { diff --git a/packages/cli/src/InternalHooks.ts b/packages/cli/src/InternalHooks.ts index 6a1ab61b64715..965f84642b6fa 100644 --- a/packages/cli/src/InternalHooks.ts +++ b/packages/cli/src/InternalHooks.ts @@ -271,12 +271,12 @@ export class InternalHooks implements IInternalHooksClass { runData?: IRun, userId?: string, ): Promise { - const promises = [Promise.resolve()]; - if (!workflow.id) { - return Promise.resolve(); + return; } + const promises = []; + const properties: IExecutionTrackProperties = { workflow_id: workflow.id, is_manual: false, diff --git a/packages/cli/src/Ldap/LdapService.ee.ts b/packages/cli/src/Ldap/LdapService.ee.ts index bc5432cbea6ef..63394a4145fe4 100644 --- a/packages/cli/src/Ldap/LdapService.ee.ts +++ b/packages/cli/src/Ldap/LdapService.ee.ts @@ -82,7 +82,7 @@ export class LdapService { await this.client.unbind(); return searchEntries; } - return Promise.resolve([]); + return []; } /** diff --git a/packages/cli/src/UserManagement/UserManagementHelper.ts b/packages/cli/src/UserManagement/UserManagementHelper.ts index 3e705efbd648c..88927f4c73acb 100644 --- a/packages/cli/src/UserManagement/UserManagementHelper.ts +++ b/packages/cli/src/UserManagement/UserManagementHelper.ts @@ -196,30 +196,6 @@ export async function getUserById(userId: string): Promise { return user; } -/** - * Check if a URL contains an auth-excluded endpoint. - */ -export function isAuthExcluded(url: string, ignoredEndpoints: Readonly): boolean { - return !!ignoredEndpoints - .filter(Boolean) // skip empty paths - .find((ignoredEndpoint) => url.startsWith(`/${ignoredEndpoint}`)); -} - -/** - * Check if the endpoint is `POST /users/:id`. - */ -export function isPostUsersId(req: express.Request, restEndpoint: string): boolean { - return ( - req.method === 'POST' && - new RegExp(`/${restEndpoint}/users/[\\w\\d-]*`).test(req.url) && - !req.url.includes('reinvite') - ); -} - -export function isAuthenticatedRequest(request: express.Request): request is AuthenticatedRequest { - return request.user !== undefined; -} - // ---------------------------------- // hashing // ---------------------------------- diff --git a/packages/cli/src/commands/start.ts b/packages/cli/src/commands/start.ts index 359c79d584d54..358d8091eb692 100644 --- a/packages/cli/src/commands/start.ts +++ b/packages/cli/src/commands/start.ts @@ -189,8 +189,16 @@ export class Start extends BaseCommand { await license.init(this.instanceId); const activationKey = config.getEnv('license.activationKey'); + if (activationKey) { + const hasCert = (await license.loadCertStr()).length > 0; + + if (hasCert) { + return LoggerProxy.debug('Skipping license activation'); + } + try { + LoggerProxy.debug('Attempting license activation'); await license.activate(activationKey); } catch (e) { LoggerProxy.error('Could not activate license', e as Error); diff --git a/packages/cli/src/controllers/auth.controller.ts b/packages/cli/src/controllers/auth.controller.ts index 068522ce8700d..7aeb7e1cfe0cb 100644 --- a/packages/cli/src/controllers/auth.controller.ts +++ b/packages/cli/src/controllers/auth.controller.ts @@ -1,5 +1,5 @@ import validator from 'validator'; -import { Get, Post, RestController } from '@/decorators'; +import { Authorized, Get, Post, RestController } from '@/decorators'; import { AuthError, BadRequestError, InternalServerError } from '@/ResponseHelper'; import { sanitizeUser, withFeatureFlags } from '@/UserManagement/UserManagementHelper'; import { issueCookie, resolveJwt } from '@/auth/jwt'; @@ -58,7 +58,6 @@ export class AuthController { /** * Log in a user. - * Authless endpoint. */ @Post('/login') async login(req: LoginRequest, res: Response): Promise { @@ -135,7 +134,6 @@ export class AuthController { /** * Validate invite token to enable invitee to set up their account. - * Authless endpoint. */ @Get('/resolve-signup-token') async resolveSignupToken(req: UserRequest.ResolveSignUp) { @@ -196,8 +194,8 @@ export class AuthController { /** * Log out a user. - * Authless endpoint. */ + @Authorized() @Post('/logout') logout(req: Request, res: Response) { res.clearCookie(AUTH_COOKIE_NAME); diff --git a/packages/cli/src/controllers/ldap.controller.ts b/packages/cli/src/controllers/ldap.controller.ts index 619a70db286b2..b354ef660021c 100644 --- a/packages/cli/src/controllers/ldap.controller.ts +++ b/packages/cli/src/controllers/ldap.controller.ts @@ -1,5 +1,5 @@ import pick from 'lodash.pick'; -import { Get, Post, Put, RestController } from '@/decorators'; +import { Authorized, 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'; @@ -8,6 +8,7 @@ import { BadRequestError } from '@/ResponseHelper'; import { NON_SENSIBLE_LDAP_CONFIG_PROPERTIES } from '@/Ldap/constants'; import { InternalHooks } from '@/InternalHooks'; +@Authorized(['global', 'owner']) @RestController('/ldap') export class LdapController { constructor( diff --git a/packages/cli/src/controllers/me.controller.ts b/packages/cli/src/controllers/me.controller.ts index 3cf1ba409b262..0ff4cd78f0a5c 100644 --- a/packages/cli/src/controllers/me.controller.ts +++ b/packages/cli/src/controllers/me.controller.ts @@ -1,6 +1,6 @@ import validator from 'validator'; import { plainToInstance } from 'class-transformer'; -import { Delete, Get, Patch, Post, RestController } from '@/decorators'; +import { Authorized, Delete, Get, Patch, Post, RestController } from '@/decorators'; import { compareHash, hashPassword, @@ -30,6 +30,7 @@ import { randomBytes } from 'crypto'; import { isSamlLicensedAndEnabled } from '../sso/saml/samlHelpers'; import { UserService } from '@/user/user.service'; +@Authorized() @RestController('/me') export class MeController { private readonly logger: ILogger; diff --git a/packages/cli/src/controllers/nodeTypes.controller.ts b/packages/cli/src/controllers/nodeTypes.controller.ts index 5f8af5a909f22..63002c7c122a9 100644 --- a/packages/cli/src/controllers/nodeTypes.controller.ts +++ b/packages/cli/src/controllers/nodeTypes.controller.ts @@ -2,11 +2,12 @@ import { readFile } from 'fs/promises'; import get from 'lodash.get'; import { Request } from 'express'; import type { INodeTypeDescription, INodeTypeNameVersion } from 'n8n-workflow'; -import { Post, RestController } from '@/decorators'; +import { Authorized, Post, RestController } from '@/decorators'; import { getNodeTranslationPath } from '@/TranslationHelpers'; import type { Config } from '@/config'; import type { NodeTypes } from '@/NodeTypes'; +@Authorized() @RestController('/node-types') export class NodeTypesController { private readonly config: Config; diff --git a/packages/cli/src/controllers/nodes.controller.ts b/packages/cli/src/controllers/nodes.controller.ts index e82bd3e5895b8..9d872c8693d56 100644 --- a/packages/cli/src/controllers/nodes.controller.ts +++ b/packages/cli/src/controllers/nodes.controller.ts @@ -4,7 +4,7 @@ import { STARTER_TEMPLATE_NAME, UNKNOWN_FAILURE_REASON, } from '@/constants'; -import { Delete, Get, Middleware, Patch, Post, RestController } from '@/decorators'; +import { Authorized, Delete, Get, Middleware, Patch, Post, RestController } from '@/decorators'; import { NodeRequest } from '@/requests'; import { BadRequestError, InternalServerError } from '@/ResponseHelper'; import { @@ -30,10 +30,10 @@ 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; +@Authorized(['global', 'owner']) @RestController('/nodes') export class NodesController { constructor( @@ -43,14 +43,6 @@ export class NodesController { 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(); - } - // TODO: move this into a new decorator `@IfConfig('executions.mode', 'queue')` @Middleware() checkIfCommunityNodesEnabled(req: Request, res: Response, next: NextFunction) { diff --git a/packages/cli/src/controllers/owner.controller.ts b/packages/cli/src/controllers/owner.controller.ts index d83a1fecee17d..ae8745a9afab7 100644 --- a/packages/cli/src/controllers/owner.controller.ts +++ b/packages/cli/src/controllers/owner.controller.ts @@ -1,6 +1,6 @@ import validator from 'validator'; import { validateEntity } from '@/GenericHelpers'; -import { Get, Post, RestController } from '@/decorators'; +import { Authorized, Get, Post, RestController } from '@/decorators'; import { BadRequestError } from '@/ResponseHelper'; import { hashPassword, @@ -20,6 +20,7 @@ import type { WorkflowRepository, } from '@db/repositories'; +@Authorized(['global', 'owner']) @RestController('/owner') export class OwnerController { private readonly config: Config; diff --git a/packages/cli/src/controllers/passwordReset.controller.ts b/packages/cli/src/controllers/passwordReset.controller.ts index d78424f6d7303..64e46582066bd 100644 --- a/packages/cli/src/controllers/passwordReset.controller.ts +++ b/packages/cli/src/controllers/passwordReset.controller.ts @@ -65,7 +65,6 @@ export class PasswordResetController { /** * Send a password reset email. - * Authless endpoint. */ @Post('/forgot-password') async forgotPassword(req: PasswordResetRequest.Email) { @@ -171,7 +170,6 @@ export class PasswordResetController { /** * Verify password reset token and user ID. - * Authless endpoint. */ @Get('/resolve-password-token') async resolvePasswordToken(req: PasswordResetRequest.Credentials) { @@ -213,7 +211,6 @@ export class PasswordResetController { /** * Verify password reset token and user ID and update password. - * Authless endpoint. */ @Post('/change-password') async changePassword(req: PasswordResetRequest.NewPassword, res: Response) { diff --git a/packages/cli/src/controllers/tags.controller.ts b/packages/cli/src/controllers/tags.controller.ts index d8ccb4a28ca9c..7b68d95aafb06 100644 --- a/packages/cli/src/controllers/tags.controller.ts +++ b/packages/cli/src/controllers/tags.controller.ts @@ -1,13 +1,14 @@ import { Request, Response, NextFunction } from 'express'; import type { Config } from '@/config'; -import { Delete, Get, Middleware, Patch, Post, RestController } from '@/decorators'; +import { Authorized, Delete, Get, Middleware, Patch, Post, RestController } from '@/decorators'; import type { IDatabaseCollections, IExternalHooksClass, ITagWithCountDb } from '@/Interfaces'; import { TagEntity } from '@db/entities/TagEntity'; import type { TagRepository } from '@db/repositories'; import { validateEntity } from '@/GenericHelpers'; -import { BadRequestError, UnauthorizedError } from '@/ResponseHelper'; +import { BadRequestError } from '@/ResponseHelper'; import { TagsRequest } from '@/requests'; +@Authorized() @RestController('/tags') export class TagsController { private config: Config; @@ -91,15 +92,9 @@ export class TagsController { return tag; } + @Authorized(['global', 'owner']) @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]); diff --git a/packages/cli/src/controllers/translation.controller.ts b/packages/cli/src/controllers/translation.controller.ts index 8240a376a7e64..6d36d650d2e2c 100644 --- a/packages/cli/src/controllers/translation.controller.ts +++ b/packages/cli/src/controllers/translation.controller.ts @@ -2,7 +2,7 @@ import type { Request } from 'express'; import { ICredentialTypes } from 'n8n-workflow'; import { join } from 'path'; import { access } from 'fs/promises'; -import { Get, RestController } from '@/decorators'; +import { Authorized, Get, RestController } from '@/decorators'; import { BadRequestError, InternalServerError } from '@/ResponseHelper'; import { Config } from '@/config'; import { NODES_BASE_DIR } from '@/constants'; @@ -14,6 +14,7 @@ export declare namespace TranslationRequest { export type Credential = Request<{}, {}, {}, { credentialType: string }>; } +@Authorized() @RestController('/') export class TranslationController { constructor(private config: Config, private credentialTypes: ICredentialTypes) {} diff --git a/packages/cli/src/controllers/users.controller.ts b/packages/cli/src/controllers/users.controller.ts index fc9faf06fd9e3..c2ae07d344d1d 100644 --- a/packages/cli/src/controllers/users.controller.ts +++ b/packages/cli/src/controllers/users.controller.ts @@ -5,7 +5,7 @@ import { ErrorReporterProxy as ErrorReporter } from 'n8n-workflow'; import { User } from '@db/entities/User'; import { SharedCredentials } from '@db/entities/SharedCredentials'; import { SharedWorkflow } from '@db/entities/SharedWorkflow'; -import { Delete, Get, Post, RestController } from '@/decorators'; +import { Authorized, NoAuthRequired, Delete, Get, Post, RestController } from '@/decorators'; import { addInviteLinkToUser, generateUserInviteUrl, @@ -41,6 +41,7 @@ import type { UserRepository, } from '@db/repositories'; +@Authorized(['global', 'owner']) @RestController('/users') export class UsersController { private config: Config; @@ -282,6 +283,7 @@ export class UsersController { /** * Fill out user shell with first name, last name, and password. */ + @NoAuthRequired() @Post('/:id') async updateUser(req: UserRequest.Update, res: Response) { const { id: inviteeId } = req.params; @@ -343,6 +345,7 @@ export class UsersController { return withFeatureFlags(this.postHog, sanitizeUser(updatedUser)); } + @Authorized('any') @Get('/') async listUsers(req: UserRequest.List) { const users = await this.userRepository.find({ relations: ['globalRole', 'authIdentities'] }); diff --git a/packages/cli/src/decorators/Authorized.ts b/packages/cli/src/decorators/Authorized.ts new file mode 100644 index 0000000000000..880938ae5ce72 --- /dev/null +++ b/packages/cli/src/decorators/Authorized.ts @@ -0,0 +1,16 @@ +/* eslint-disable @typescript-eslint/ban-types */ +/* eslint-disable @typescript-eslint/naming-convention */ +import { CONTROLLER_AUTH_ROLES } from './constants'; +import type { AuthRoleMetadata } from './types'; + +export function Authorized(authRole: AuthRoleMetadata[string] = 'any'): Function { + return function (target: Function | Object, handlerName?: string) { + const controllerClass = handlerName ? target.constructor : target; + const authRoles = (Reflect.getMetadata(CONTROLLER_AUTH_ROLES, controllerClass) ?? + {}) as AuthRoleMetadata; + authRoles[handlerName ?? '*'] = authRole; + Reflect.defineMetadata(CONTROLLER_AUTH_ROLES, authRoles, controllerClass); + }; +} + +export const NoAuthRequired = () => Authorized('none'); diff --git a/packages/cli/src/decorators/constants.ts b/packages/cli/src/decorators/constants.ts index 6bff0e4a6cb90..5a5efc938db6c 100644 --- a/packages/cli/src/decorators/constants.ts +++ b/packages/cli/src/decorators/constants.ts @@ -1,3 +1,4 @@ export const CONTROLLER_ROUTES = 'CONTROLLER_ROUTES'; export const CONTROLLER_BASE_PATH = 'CONTROLLER_BASE_PATH'; export const CONTROLLER_MIDDLEWARES = 'CONTROLLER_MIDDLEWARES'; +export const CONTROLLER_AUTH_ROLES = 'CONTROLLER_AUTH_ROLES'; diff --git a/packages/cli/src/decorators/index.ts b/packages/cli/src/decorators/index.ts index 71b82b5b69c30..0e683d410cbc9 100644 --- a/packages/cli/src/decorators/index.ts +++ b/packages/cli/src/decorators/index.ts @@ -1,3 +1,4 @@ +export { Authorized, NoAuthRequired } from './Authorized'; export { RestController } from './RestController'; export { Get, Post, Put, Patch, Delete } from './Route'; export { Middleware } from './Middleware'; diff --git a/packages/cli/src/decorators/registerController.ts b/packages/cli/src/decorators/registerController.ts index e20293ae21c71..fe792040b6067 100644 --- a/packages/cli/src/decorators/registerController.ts +++ b/packages/cli/src/decorators/registerController.ts @@ -1,10 +1,36 @@ /* eslint-disable @typescript-eslint/naming-convention */ import { Router } from 'express'; +import type { Application, Request, Response, RequestHandler } from 'express'; import type { Config } from '@/config'; -import { CONTROLLER_BASE_PATH, CONTROLLER_MIDDLEWARES, CONTROLLER_ROUTES } from './constants'; +import type { AuthenticatedRequest } from '@/requests'; import { send } from '@/ResponseHelper'; // TODO: move `ResponseHelper.send` to this file -import type { Application, Request, Response, RequestHandler } from 'express'; -import type { Controller, MiddlewareMetadata, RouteMetadata } from './types'; +import { + CONTROLLER_AUTH_ROLES, + CONTROLLER_BASE_PATH, + CONTROLLER_MIDDLEWARES, + CONTROLLER_ROUTES, +} from './constants'; +import type { + AuthRole, + AuthRoleMetadata, + Controller, + MiddlewareMetadata, + RouteMetadata, +} from './types'; + +export const createAuthMiddleware = + (authRole: AuthRole): RequestHandler => + ({ user }: AuthenticatedRequest, res, next) => { + if (authRole === 'none') return next(); + + if (!user) return res.status(401).json({ status: 'error', message: 'Unauthorized' }); + + const { globalRole } = user; + if (authRole === 'any' || (globalRole.scope === authRole[0] && globalRole.name === authRole[1])) + return next(); + + res.status(403).json({ status: 'error', message: 'Unauthorized' }); + }; export const registerController = (app: Application, config: Config, controller: object) => { const controllerClass = controller.constructor; @@ -14,11 +40,16 @@ export const registerController = (app: Application, config: Config, controller: if (!controllerBasePath) throw new Error(`${controllerClass.name} is missing the RestController decorator`); + const authRoles = Reflect.getMetadata(CONTROLLER_AUTH_ROLES, controllerClass) as + | AuthRoleMetadata + | undefined; 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 prefix = `/${[restBasePath, controllerBasePath].join('/')}` + .replace(/\/+/g, '/') + .replace(/\/$/, ''); const controllerMiddlewares = ( (Reflect.getMetadata(CONTROLLER_MIDDLEWARES, controllerClass) ?? []) as MiddlewareMetadata[] @@ -28,8 +59,10 @@ export const registerController = (app: Application, config: Config, controller: ); routes.forEach(({ method, path, middlewares: routeMiddlewares, handlerName }) => { + const authRole = authRoles && (authRoles[handlerName] ?? authRoles['*']); router[method]( path, + ...(authRole ? [createAuthMiddleware(authRole)] : []), ...controllerMiddlewares, ...routeMiddlewares, send(async (req: Request, res: Response) => diff --git a/packages/cli/src/decorators/types.ts b/packages/cli/src/decorators/types.ts index 250abf0d27c95..d118e6e6c9cbd 100644 --- a/packages/cli/src/decorators/types.ts +++ b/packages/cli/src/decorators/types.ts @@ -1,7 +1,11 @@ import type { Request, Response, RequestHandler } from 'express'; +import type { RoleNames, RoleScopes } from '@db/entities/Role'; export type Method = 'get' | 'post' | 'put' | 'patch' | 'delete'; +export type AuthRole = [RoleScopes, RoleNames] | 'any' | 'none'; +export type AuthRoleMetadata = Record; + export interface MiddlewareMetadata { handlerName: string; } diff --git a/packages/cli/src/eventbus/eventBus.controller.ts b/packages/cli/src/eventbus/eventBus.controller.ts index bc18472d4c910..7c9ce21ccc5ee 100644 --- a/packages/cli/src/eventbus/eventBus.controller.ts +++ b/packages/cli/src/eventbus/eventBus.controller.ts @@ -28,15 +28,13 @@ import type { IRunExecutionData, } from 'n8n-workflow'; import { MessageEventBusDestinationTypeNames, EventMessageTypeNames } from 'n8n-workflow'; -import type { User } from '@db/entities/User'; -import * as ResponseHelper from '@/ResponseHelper'; import type { EventMessageNodeOptions } from './EventMessageClasses/EventMessageNode'; import { EventMessageNode } from './EventMessageClasses/EventMessageNode'; import { recoverExecutionDataFromEventLogMessages } from './MessageEventBus/recoverEvents'; -import { RestController, Get, Post, Delete } from '@/decorators'; +import { RestController, Get, Post, Delete, Authorized } from '@/decorators'; import type { MessageEventBusDestination } from './MessageEventBusDestination/MessageEventBusDestination.ee'; -import { isOwnerMiddleware } from '../middlewares/isOwner'; import type { DeleteResult } from 'typeorm'; +import { AuthenticatedRequest } from '@/requests'; // ---------------------------------------- // TypeGuards @@ -74,12 +72,14 @@ const isMessageEventBusDestinationOptions = ( // Controller // ---------------------------------------- +@Authorized() @RestController('/eventbus') export class EventBusController { // ---------------------------------------- // Events // ---------------------------------------- - @Get('/event', { middlewares: [isOwnerMiddleware] }) + @Authorized(['global', 'owner']) + @Get('/event') async getEvents( req: express.Request, ): Promise> { @@ -132,7 +132,8 @@ export class EventBusController { return; } - @Post('/event', { middlewares: [isOwnerMiddleware] }) + @Authorized(['global', 'owner']) + @Post('/event') async postEvent(req: express.Request): Promise { let msg: EventMessageTypes | undefined; if (isEventMessageOptions(req.body)) { @@ -172,12 +173,9 @@ export class EventBusController { } } - @Post('/destination', { middlewares: [isOwnerMiddleware] }) - async postDestination(req: express.Request): Promise { - if (!req.user || (req.user as User).globalRole.name !== 'owner') { - throw new ResponseHelper.UnauthorizedError('Invalid request'); - } - + @Authorized(['global', 'owner']) + @Post('/destination') + async postDestination(req: AuthenticatedRequest): Promise { let result: MessageEventBusDestination | undefined; if (isMessageEventBusDestinationOptions(req.body)) { switch (req.body.__type) { @@ -228,11 +226,9 @@ export class EventBusController { return false; } - @Delete('/destination', { middlewares: [isOwnerMiddleware] }) - async deleteDestination(req: express.Request): Promise { - if (!req.user || (req.user as User).globalRole.name !== 'owner') { - throw new ResponseHelper.UnauthorizedError('Invalid request'); - } + @Authorized(['global', 'owner']) + @Delete('/destination') + async deleteDestination(req: AuthenticatedRequest): Promise { if (isWithIdString(req.query)) { return eventBus.removeDestination(req.query.id); } else { diff --git a/packages/cli/src/middlewares/auth.ts b/packages/cli/src/middlewares/auth.ts index bd0f580c8a73c..231b48beddf91 100644 --- a/packages/cli/src/middlewares/auth.ts +++ b/packages/cli/src/middlewares/auth.ts @@ -10,13 +10,7 @@ import type { AuthenticatedRequest } from '@/requests'; import config from '@/config'; import { AUTH_COOKIE_NAME, EDITOR_UI_DIST_DIR } from '@/constants'; import { issueCookie, resolveJwtContent } from '@/auth/jwt'; -import { - isAuthenticatedRequest, - isAuthExcluded, - isPostUsersId, - isUserManagementEnabled, -} from '@/UserManagement/UserManagementHelper'; -import { SamlUrls } from '@/sso/saml/constants'; +import { isUserManagementEnabled } from '@/UserManagement/UserManagementHelper'; import type { UserRepository } from '@db/repositories'; const jwtFromRequest = (req: Request) => { @@ -66,6 +60,17 @@ const staticAssets = globSync(['**/*.html', '**/*.svg', '**/*.png', '**/*.ico'], cwd: EDITOR_UI_DIST_DIR, }); +// TODO: delete this +const isPostUsersId = (req: Request, restEndpoint: string): boolean => + req.method === 'POST' && + new RegExp(`/${restEndpoint}/users/[\\w\\d-]*`).test(req.url) && + !req.url.includes('reinvite'); + +const isAuthExcluded = (url: string, ignoredEndpoints: Readonly): boolean => + !!ignoredEndpoints + .filter(Boolean) // skip empty paths + .find((ignoredEndpoint) => url.startsWith(`/${ignoredEndpoint}`)); + /** * This sets up the auth middlewares in the correct order */ @@ -85,20 +90,16 @@ export const setupAuthMiddlewares = ( // skip authentication for preflight requests req.method === 'OPTIONS' || staticAssets.includes(req.url.slice(1)) || + isAuthExcluded(req.url, ignoredEndpoints) || req.url.startsWith(`/${restEndpoint}/settings`) || req.url.startsWith(`/${restEndpoint}/login`) || - req.url.startsWith(`/${restEndpoint}/logout`) || req.url.startsWith(`/${restEndpoint}/resolve-signup-token`) || isPostUsersId(req, restEndpoint) || req.url.startsWith(`/${restEndpoint}/forgot-password`) || req.url.startsWith(`/${restEndpoint}/resolve-password-token`) || req.url.startsWith(`/${restEndpoint}/change-password`) || req.url.startsWith(`/${restEndpoint}/oauth2-credential/callback`) || - req.url.startsWith(`/${restEndpoint}/oauth1-credential/callback`) || - req.url.startsWith(`/${restEndpoint}/sso/saml${SamlUrls.metadata}`) || - req.url.startsWith(`/${restEndpoint}/sso/saml${SamlUrls.initSSO}`) || - req.url.startsWith(`/${restEndpoint}/sso/saml${SamlUrls.acs}`) || - isAuthExcluded(req.url, ignoredEndpoints) + req.url.startsWith(`/${restEndpoint}/oauth1-credential/callback`) ) { return next(); } @@ -115,43 +116,5 @@ export const setupAuthMiddlewares = ( return passportMiddleware(req, res, next); }); - app.use((req: Request | AuthenticatedRequest, res: Response, next: NextFunction) => { - // req.user is empty for public routes, so just proceed - // owner can do anything, so proceed as well - if (!req.user || (isAuthenticatedRequest(req) && req.user.globalRole.name === 'owner')) { - next(); - return; - } - // Not owner and user exists. We now protect restricted urls. - const postRestrictedUrls = [ - `/${restEndpoint}/users`, - `/${restEndpoint}/owner`, - `/${restEndpoint}/ldap/sync`, - `/${restEndpoint}/ldap/test-connection`, - ]; - const getRestrictedUrls = [`/${restEndpoint}/ldap/sync`, `/${restEndpoint}/ldap/config`]; - const putRestrictedUrls = [`/${restEndpoint}/ldap/config`]; - const trimmedUrl = req.url.endsWith('/') ? req.url.slice(0, -1) : req.url; - if ( - (req.method === 'POST' && postRestrictedUrls.includes(trimmedUrl)) || - (req.method === 'GET' && getRestrictedUrls.includes(trimmedUrl)) || - (req.method === 'PUT' && putRestrictedUrls.includes(trimmedUrl)) || - (req.method === 'DELETE' && - new RegExp(`/${restEndpoint}/users/[^/]+`, 'gm').test(trimmedUrl)) || - (req.method === 'POST' && - new RegExp(`/${restEndpoint}/users/[^/]+/reinvite`, 'gm').test(trimmedUrl)) || - new RegExp(`/${restEndpoint}/owner/[^/]+`, 'gm').test(trimmedUrl) - ) { - Logger.verbose('User attempted to access endpoint without authorization', { - endpoint: `${req.method} ${trimmedUrl}`, - userId: isAuthenticatedRequest(req) ? req.user.id : 'unknown', - }); - res.status(403).json({ status: 'error', message: 'Unauthorized' }); - return; - } - - next(); - }); - app.use(refreshExpiringCookie); }; diff --git a/packages/cli/src/middlewares/isOwner.ts b/packages/cli/src/middlewares/isOwner.ts deleted file mode 100644 index d3e3c70a0fb46..0000000000000 --- a/packages/cli/src/middlewares/isOwner.ts +++ /dev/null @@ -1,12 +0,0 @@ -import type { RequestHandler } from 'express'; -import { LoggerProxy } from 'n8n-workflow'; -import type { AuthenticatedRequest } from '@/requests'; - -export const isOwnerMiddleware: RequestHandler = (req: AuthenticatedRequest, res, next) => { - if (req.user.globalRole.name === 'owner') { - next(); - } else { - LoggerProxy.debug('Request failed because user is not owner'); - res.status(401).send('Unauthorized'); - } -}; diff --git a/packages/cli/src/posthog/index.ts b/packages/cli/src/posthog/index.ts index e513e366366cf..df390c53b39a8 100644 --- a/packages/cli/src/posthog/index.ts +++ b/packages/cli/src/posthog/index.ts @@ -44,7 +44,7 @@ export class PostHogClient { } async getFeatureFlags(user: Pick): Promise { - if (!this.postHog) return Promise.resolve({}); + if (!this.postHog) return {}; const fullId = [this.instanceId, user.id].join('#'); diff --git a/packages/cli/src/sso/saml/middleware/samlEnabledMiddleware.ts b/packages/cli/src/sso/saml/middleware/samlEnabledMiddleware.ts index 6e4600b895f06..69015838d7f69 100644 --- a/packages/cli/src/sso/saml/middleware/samlEnabledMiddleware.ts +++ b/packages/cli/src/sso/saml/middleware/samlEnabledMiddleware.ts @@ -1,24 +1,11 @@ import type { RequestHandler } from 'express'; -import type { AuthenticatedRequest } from '@/requests'; import { isSamlLicensed, isSamlLicensedAndEnabled } from '../samlHelpers'; -export const samlLicensedOwnerMiddleware: RequestHandler = ( - req: AuthenticatedRequest, - res, - next, -) => { - if (isSamlLicensed() && req.user?.globalRole.name === 'owner') { - next(); - } else { - res.status(401).json({ status: 'error', message: 'Unauthorized' }); - } -}; - export const samlLicensedAndEnabledMiddleware: RequestHandler = (req, res, next) => { if (isSamlLicensedAndEnabled()) { next(); } else { - res.status(401).json({ status: 'error', message: 'Unauthorized' }); + res.status(403).json({ status: 'error', message: 'Unauthorized' }); } }; @@ -26,6 +13,6 @@ export const samlLicensedMiddleware: RequestHandler = (req, res, next) => { if (isSamlLicensed()) { next(); } else { - res.status(401).json({ status: 'error', message: 'Unauthorized' }); + res.status(403).json({ status: 'error', message: 'Unauthorized' }); } }; diff --git a/packages/cli/src/sso/saml/routes/saml.controller.ee.ts b/packages/cli/src/sso/saml/routes/saml.controller.ee.ts index 86e7b75cfa8c4..c6b04cae460ef 100644 --- a/packages/cli/src/sso/saml/routes/saml.controller.ee.ts +++ b/packages/cli/src/sso/saml/routes/saml.controller.ee.ts @@ -1,10 +1,10 @@ import express from 'express'; -import { Get, Post, RestController } from '@/decorators'; +import { getInstanceBaseUrl } from '@/UserManagement/UserManagementHelper'; +import { Authorized, Get, Post, RestController } from '@/decorators'; import { SamlUrls } from '../constants'; import { samlLicensedAndEnabledMiddleware, samlLicensedMiddleware, - samlLicensedOwnerMiddleware, } from '../middleware/samlEnabledMiddleware'; import { SamlService } from '../saml.service.ee'; import { SamlConfiguration } from '../types/requests'; @@ -39,7 +39,8 @@ export class SamlController { * GET /sso/saml/config * Return SAML config */ - @Get(SamlUrls.config, { middlewares: [samlLicensedOwnerMiddleware] }) + @Authorized(['global', 'owner']) + @Get(SamlUrls.config, { middlewares: [samlLicensedMiddleware] }) async configGet() { const prefs = this.samlService.samlPreferences; return { @@ -53,7 +54,8 @@ export class SamlController { * POST /sso/saml/config * Set SAML config */ - @Post(SamlUrls.config, { middlewares: [samlLicensedOwnerMiddleware] }) + @Authorized(['global', 'owner']) + @Post(SamlUrls.config, { middlewares: [samlLicensedMiddleware] }) async configPost(req: SamlConfiguration.Update) { const validationResult = await validate(req.body); if (validationResult.length === 0) { @@ -71,7 +73,8 @@ export class SamlController { * POST /sso/saml/config/toggle * Set SAML config */ - @Post(SamlUrls.configToggleEnabled, { middlewares: [samlLicensedOwnerMiddleware] }) + @Authorized(['global', 'owner']) + @Post(SamlUrls.configToggleEnabled, { middlewares: [samlLicensedMiddleware] }) async toggleEnabledPost(req: SamlConfiguration.Toggle, res: express.Response) { if (req.body.loginEnabled === undefined) { throw new BadRequestError('Body should contain a boolean "loginEnabled" property'); @@ -123,9 +126,9 @@ export class SamlController { if (isSamlLicensedAndEnabled()) { await issueCookie(res, loginResult.authenticatedUser); if (loginResult.onboardingRequired) { - return res.redirect(SamlUrls.samlOnboarding); + return res.redirect(getInstanceBaseUrl() + SamlUrls.samlOnboarding); } else { - return res.redirect(SamlUrls.defaultRedirect); + return res.redirect(getInstanceBaseUrl() + SamlUrls.defaultRedirect); } } else { return res.status(202).send(loginResult.attributes); @@ -155,7 +158,8 @@ export class SamlController { * Test SAML config * This endpoint is available if SAML is licensed and the requestor is an instance owner */ - @Get(SamlUrls.configTest, { middlewares: [samlLicensedOwnerMiddleware] }) + @Authorized(['global', 'owner']) + @Get(SamlUrls.configTest, { middlewares: [samlLicensedMiddleware] }) async configTestGet(req: AuthenticatedRequest, res: express.Response) { return this.handleInitSSO(res, getServiceProviderConfigTestReturnUrl()); } diff --git a/packages/cli/src/sso/saml/saml.service.ee.ts b/packages/cli/src/sso/saml/saml.service.ee.ts index b1a764c724094..b16d14600e1d8 100644 --- a/packages/cli/src/sso/saml/saml.service.ee.ts +++ b/packages/cli/src/sso/saml/saml.service.ee.ts @@ -73,9 +73,8 @@ export class SamlService { validate: async (response: string) => { const valid = await validateResponse(response); if (!valid) { - return Promise.reject(new Error('Invalid SAML response')); + throw new Error('Invalid SAML response'); } - return Promise.resolve(); }, }); } diff --git a/packages/cli/src/telemetry/index.ts b/packages/cli/src/telemetry/index.ts index cdc7b96f8be74..7edb6f6172f41 100644 --- a/packages/cli/src/telemetry/index.ts +++ b/packages/cli/src/telemetry/index.ts @@ -76,7 +76,7 @@ export class Telemetry { private async pulse(): Promise { if (!this.rudderStack) { - return Promise.resolve(); + return; } const allPromises = Object.keys(this.executionCountsBuffer).map(async (workflowId) => { diff --git a/packages/cli/test/integration/ldap/ldap.api.test.ts b/packages/cli/test/integration/ldap/ldap.api.test.ts index 2bb766be937b9..eaffa15494f23 100644 --- a/packages/cli/test/integration/ldap/ldap.api.test.ts +++ b/packages/cli/test/integration/ldap/ldap.api.test.ts @@ -205,9 +205,7 @@ test('GET /ldap/config route should retrieve current configuration', async () => describe('POST /ldap/test-connection', () => { test('route should success', async () => { - jest - .spyOn(LdapService.prototype, 'testConnection') - .mockImplementation(async () => Promise.resolve()); + jest.spyOn(LdapService.prototype, 'testConnection').mockResolvedValue(); const response = await authAgent(owner).post('/ldap/test-connection'); expect(response.statusCode).toBe(200); @@ -216,9 +214,7 @@ describe('POST /ldap/test-connection', () => { test('route should fail', async () => { const errorMessage = 'Invalid connection'; - jest - .spyOn(LdapService.prototype, 'testConnection') - .mockImplementation(async () => Promise.reject(new Error(errorMessage))); + jest.spyOn(LdapService.prototype, 'testConnection').mockRejectedValue(new Error(errorMessage)); const response = await authAgent(owner).post('/ldap/test-connection'); expect(response.statusCode).toBe(400); @@ -240,9 +236,7 @@ describe('POST /ldap/sync', () => { describe('dry mode', () => { const runTest = async (ldapUsers: LdapUser[]) => { - jest - .spyOn(LdapService.prototype, 'searchWithAdminBinding') - .mockImplementation(async () => Promise.resolve(ldapUsers)); + jest.spyOn(LdapService.prototype, 'searchWithAdminBinding').mockResolvedValue(ldapUsers); const response = await authAgent(owner).post('/ldap/sync').send({ type: 'dry' }); @@ -337,9 +331,7 @@ describe('POST /ldap/sync', () => { describe('live mode', () => { const runTest = async (ldapUsers: LdapUser[]) => { - jest - .spyOn(LdapService.prototype, 'searchWithAdminBinding') - .mockImplementation(async () => Promise.resolve(ldapUsers)); + jest.spyOn(LdapService.prototype, 'searchWithAdminBinding').mockResolvedValue(ldapUsers); const response = await authAgent(owner).post('/ldap/sync').send({ type: 'live' }); @@ -467,9 +459,7 @@ describe('POST /ldap/sync', () => { test('should remove user instance access once the user is disabled during synchronization', async () => { const member = await testDb.createLdapUser({ globalRole: globalMemberRole }, uniqueId()); - jest - .spyOn(LdapService.prototype, 'searchWithAdminBinding') - .mockImplementation(async () => Promise.resolve([])); + jest.spyOn(LdapService.prototype, 'searchWithAdminBinding').mockResolvedValue([]); await authAgent(owner).post('/ldap/sync').send({ type: 'live' }); @@ -508,13 +498,9 @@ describe('POST /login', () => { const authlessAgent = utils.createAgent(app); - jest - .spyOn(LdapService.prototype, 'searchWithAdminBinding') - .mockImplementation(async () => Promise.resolve([ldapUser])); + jest.spyOn(LdapService.prototype, 'searchWithAdminBinding').mockResolvedValue([ldapUser]); - jest - .spyOn(LdapService.prototype, 'validUser') - .mockImplementation(async () => Promise.resolve()); + jest.spyOn(LdapService.prototype, 'validUser').mockResolvedValue(); const response = await authlessAgent .post('/login') diff --git a/packages/cli/test/integration/shared/constants.ts b/packages/cli/test/integration/shared/constants.ts index 3608225eaa1e1..37798aa4d0f83 100644 --- a/packages/cli/test/integration/shared/constants.ts +++ b/packages/cli/test/integration/shared/constants.ts @@ -42,7 +42,7 @@ export const ROUTES_REQUIRING_AUTHORIZATION: Readonly = [ 'POST /users', 'DELETE /users/123', 'POST /users/123/reinvite', - 'POST /owner/pre-setup', + 'GET /owner/pre-setup', 'POST /owner/setup', 'POST /owner/skip-setup', ]; diff --git a/packages/cli/test/integration/shared/utils.ts b/packages/cli/test/integration/shared/utils.ts index 3625caf661a84..9e8e4aa2b1ab8 100644 --- a/packages/cli/test/integration/shared/utils.ts +++ b/packages/cli/test/integration/shared/utils.ts @@ -799,11 +799,11 @@ export function installedNodePayload(packageName: string): InstalledNodePayload }; } -export const emptyPackage = () => { +export const emptyPackage = async () => { const installedPackage = new InstalledPackages(); installedPackage.installedNodes = []; - return Promise.resolve(installedPackage); + return installedPackage; }; // ---------------------------------- diff --git a/packages/cli/test/integration/users.api.test.ts b/packages/cli/test/integration/users.api.test.ts index 759d12b8cb42a..966bb6d1694a7 100644 --- a/packages/cli/test/integration/users.api.test.ts +++ b/packages/cli/test/integration/users.api.test.ts @@ -31,6 +31,7 @@ let credentialOwnerRole: Role; let owner: User; let authlessAgent: SuperAgentTest; let authOwnerAgent: SuperAgentTest; +let authAgentFor: (user: User) => SuperAgentTest; beforeAll(async () => { const app = await utils.initTestServer({ endpointGroups: ['users'] }); @@ -49,7 +50,8 @@ beforeAll(async () => { owner = await testDb.createUser({ globalRole: globalOwnerRole }); authlessAgent = utils.createAgent(app); - authOwnerAgent = utils.createAuthAgent(app)(owner); + authAgentFor = utils.createAuthAgent(app); + authOwnerAgent = authAgentFor(owner); }); beforeEach(async () => { @@ -69,7 +71,7 @@ afterAll(async () => { }); describe('GET /users', () => { - test('should return all users', async () => { + test('should return all users (for owner)', async () => { await testDb.createUser({ globalRole: globalMemberRole }); const response = await authOwnerAgent.get('/users'); @@ -103,6 +105,14 @@ describe('GET /users', () => { expect(apiKey).not.toBeDefined(); }); }); + + test('should return all users (for member)', async () => { + const member = await testDb.createUser({ globalRole: globalMemberRole }); + const response = await authAgentFor(member).get('/users'); + + expect(response.statusCode).toBe(200); + expect(response.body.data.length).toBe(2); + }); }); describe('DELETE /users/:id', () => { diff --git a/packages/cli/test/unit/ActiveExecutions.test.ts b/packages/cli/test/unit/ActiveExecutions.test.ts index 19c1d316e6a5b..3f27677052db8 100644 --- a/packages/cli/test/unit/ActiveExecutions.test.ts +++ b/packages/cli/test/unit/ActiveExecutions.test.ts @@ -18,7 +18,7 @@ jest.mock('@/Db', () => { return { collections: { Execution: { - save: jest.fn(async () => Promise.resolve({ id: FAKE_EXECUTION_ID })), + save: jest.fn(async () => ({ id: FAKE_EXECUTION_ID })), update: jest.fn(), }, }, diff --git a/packages/cli/test/unit/ActiveWorkflowRunner.test.ts b/packages/cli/test/unit/ActiveWorkflowRunner.test.ts index efe6fe22960c8..ab1232cde6e62 100644 --- a/packages/cli/test/unit/ActiveWorkflowRunner.test.ts +++ b/packages/cli/test/unit/ActiveWorkflowRunner.test.ts @@ -99,12 +99,11 @@ jest.mock('@/Db', () => { return { collections: { Workflow: { - find: jest.fn(async () => Promise.resolve(generateWorkflows(databaseActiveWorkflowsCount))), + find: jest.fn(async () => generateWorkflows(databaseActiveWorkflowsCount)), findOne: jest.fn(async (searchParams) => { - const foundWorkflow = databaseActiveWorkflowsList.find( + return databaseActiveWorkflowsList.find( (workflow) => workflow.id.toString() === searchParams.where.id.toString(), ); - return Promise.resolve(foundWorkflow); }), update: jest.fn(), createQueryBuilder: jest.fn(() => { @@ -112,7 +111,7 @@ jest.mock('@/Db', () => { update: () => fakeQueryBuilder, set: () => fakeQueryBuilder, where: () => fakeQueryBuilder, - execute: () => Promise.resolve(), + execute: async () => {}, }; return fakeQueryBuilder; }), @@ -246,7 +245,7 @@ describe('ActiveWorkflowRunner', () => { const workflow = generateWorkflows(1); const additionalData = await WorkflowExecuteAdditionalData.getBase('fake-user-id'); - workflowRunnerRun.mockImplementationOnce(() => Promise.resolve('invalid-execution-id')); + workflowRunnerRun.mockResolvedValueOnce('invalid-execution-id'); await activeWorkflowRunner.runWorkflow( workflow[0], diff --git a/packages/cli/test/unit/WorkflowExecuteAdditionalData.test.ts b/packages/cli/test/unit/WorkflowExecuteAdditionalData.test.ts index 9d364bd788939..6007c0dd74b81 100644 --- a/packages/cli/test/unit/WorkflowExecuteAdditionalData.test.ts +++ b/packages/cli/test/unit/WorkflowExecuteAdditionalData.test.ts @@ -6,7 +6,7 @@ jest.mock('@/Db', () => { return { collections: { ExecutionMetadata: { - save: jest.fn(async () => Promise.resolve([])), + save: jest.fn(async () => []), }, }, }; diff --git a/packages/core/.eslintrc.js b/packages/core/.eslintrc.js index a1a1b8155d206..4d68086144d56 100644 --- a/packages/core/.eslintrc.js +++ b/packages/core/.eslintrc.js @@ -11,8 +11,6 @@ module.exports = { ignorePatterns: ['bin/*.js'], rules: { - '@typescript-eslint/consistent-type-imports': 'error', - // TODO: Remove this 'import/order': 'off', '@typescript-eslint/ban-ts-comment': ['error', { 'ts-ignore': true }], diff --git a/packages/core/src/BinaryDataManager/FileSystem.ts b/packages/core/src/BinaryDataManager/FileSystem.ts index df033569752f1..011a5cd5eedae 100644 --- a/packages/core/src/BinaryDataManager/FileSystem.ts +++ b/packages/core/src/BinaryDataManager/FileSystem.ts @@ -134,63 +134,58 @@ export class BinaryDataFileSystem implements IBinaryDataManager { const execsAdded: { [key: string]: number } = {}; - const proms = metaFileNames.reduce( - (prev, curr) => { - const [prefix, executionId, ts] = curr.split('_'); + const promises = metaFileNames.reduce>>((prev, curr) => { + const [prefix, executionId, ts] = curr.split('_'); - if (prefix !== filePrefix) { + if (prefix !== filePrefix) { + return prev; + } + + const execTimestamp = parseInt(ts, 10); + + if (execTimestamp < currentTimeValue) { + if (execsAdded[executionId]) { + // do not delete data, only meta file + prev.push(this.deleteMetaFileByPath(path.join(metaPath, curr))); return prev; } - const execTimestamp = parseInt(ts, 10); - - if (execTimestamp < currentTimeValue) { - if (execsAdded[executionId]) { - // do not delete data, only meta file - prev.push(this.deleteMetaFileByPath(path.join(metaPath, curr))); - return prev; - } - - execsAdded[executionId] = 1; - prev.push( - this.deleteBinaryDataByExecutionId(executionId).then(async () => - this.deleteMetaFileByPath(path.join(metaPath, curr)), - ), - ); - } + execsAdded[executionId] = 1; + prev.push( + this.deleteBinaryDataByExecutionId(executionId).then(async () => + this.deleteMetaFileByPath(path.join(metaPath, curr)), + ), + ); + } - return prev; - }, - [Promise.resolve()], - ); + return prev; + }, []); - return Promise.all(proms).then(() => {}); + await Promise.all(promises); } async duplicateBinaryDataByIdentifier(binaryDataId: string, prefix: string): Promise { const newBinaryDataId = this.generateFileName(prefix); - return fs - .copyFile(this.resolveStoragePath(binaryDataId), this.resolveStoragePath(newBinaryDataId)) - .then(() => newBinaryDataId); + await fs.copyFile( + this.resolveStoragePath(binaryDataId), + this.resolveStoragePath(newBinaryDataId), + ); + return newBinaryDataId; } async deleteBinaryDataByExecutionId(executionId: string): Promise { const regex = new RegExp(`${executionId}_*`); const filenames = await fs.readdir(this.storagePath); - const proms = filenames.reduce( - (allProms, filename) => { - if (regex.test(filename)) { - allProms.push(fs.rm(this.resolveStoragePath(filename))); - } - - return allProms; - }, - [Promise.resolve()], - ); + const promises = filenames.reduce>>((allProms, filename) => { + if (regex.test(filename)) { + allProms.push(fs.rm(this.resolveStoragePath(filename))); + } + return allProms; + }, []); - return Promise.all(proms).then(async () => Promise.resolve()); + await Promise.all(promises); } async deleteBinaryDataByIdentifier(identifier: string): Promise { @@ -198,20 +193,17 @@ export class BinaryDataFileSystem implements IBinaryDataManager { } async persistBinaryDataForExecutionId(executionId: string): Promise { - return fs.readdir(this.getBinaryDataPersistMetaPath()).then(async (metafiles) => { - const proms = metafiles.reduce( - (prev, curr) => { - if (curr.startsWith(`${PREFIX_PERSISTED_METAFILE}_${executionId}_`)) { - prev.push(fs.rm(path.join(this.getBinaryDataPersistMetaPath(), curr))); - return prev; - } - + return fs.readdir(this.getBinaryDataPersistMetaPath()).then(async (metaFiles) => { + const promises = metaFiles.reduce>>((prev, curr) => { + if (curr.startsWith(`${PREFIX_PERSISTED_METAFILE}_${executionId}_`)) { + prev.push(fs.rm(path.join(this.getBinaryDataPersistMetaPath(), curr))); return prev; - }, - [Promise.resolve()], - ); + } + + return prev; + }, []); - return Promise.all(proms).then(() => {}); + await Promise.all(promises); }); } @@ -227,8 +219,8 @@ export class BinaryDataFileSystem implements IBinaryDataManager { return path.join(this.storagePath, 'persistMeta'); } - private async deleteMetaFileByPath(metafilePath: string): Promise { - return fs.rm(metafilePath); + private async deleteMetaFileByPath(metaFilePath: string): Promise { + return fs.rm(metaFilePath); } private async deleteFromLocalStorage(identifier: string) { diff --git a/packages/core/src/BinaryDataManager/index.ts b/packages/core/src/BinaryDataManager/index.ts index b3ddb8738d6c6..d1c85f5b797ed 100644 --- a/packages/core/src/BinaryDataManager/index.ts +++ b/packages/core/src/BinaryDataManager/index.ts @@ -158,38 +158,30 @@ export class BinaryDataManager { async markDataForDeletionByExecutionId(executionId: string): Promise { if (this.managers[this.binaryDataMode]) { - return this.managers[this.binaryDataMode].markDataForDeletionByExecutionId(executionId); + await this.managers[this.binaryDataMode].markDataForDeletionByExecutionId(executionId); } - - return Promise.resolve(); } async markDataForDeletionByExecutionIds(executionIds: string[]): Promise { if (this.managers[this.binaryDataMode]) { - return Promise.all( + await Promise.all( executionIds.map(async (id) => this.managers[this.binaryDataMode].markDataForDeletionByExecutionId(id), ), - ).then(() => {}); + ); } - - return Promise.resolve(); } async persistBinaryDataForExecutionId(executionId: string): Promise { if (this.managers[this.binaryDataMode]) { - return this.managers[this.binaryDataMode].persistBinaryDataForExecutionId(executionId); + await this.managers[this.binaryDataMode].persistBinaryDataForExecutionId(executionId); } - - return Promise.resolve(); } async deleteBinaryDataByExecutionId(executionId: string): Promise { if (this.managers[this.binaryDataMode]) { - return this.managers[this.binaryDataMode].deleteBinaryDataByExecutionId(executionId); + await this.managers[this.binaryDataMode].deleteBinaryDataByExecutionId(executionId); } - - return Promise.resolve(); } async duplicateBinaryData( @@ -218,7 +210,7 @@ export class BinaryDataManager { return Promise.all(returnInputData); } - return Promise.resolve(inputData as INodeExecutionData[][]); + return inputData as INodeExecutionData[][]; } private generateBinaryId(filename: string) { diff --git a/packages/core/src/WorkflowExecute.ts b/packages/core/src/WorkflowExecute.ts index 480a2b2f80e0c..00e7a3f84f01c 100644 --- a/packages/core/src/WorkflowExecute.ts +++ b/packages/core/src/WorkflowExecute.ts @@ -792,7 +792,7 @@ export class WorkflowExecute { } if (gotCancel) { - return Promise.resolve(); + return; } nodeSuccessData = null; @@ -919,7 +919,7 @@ export class WorkflowExecute { for (let tryIndex = 0; tryIndex < maxTries; tryIndex++) { if (gotCancel) { - return Promise.resolve(); + return; } try { if (tryIndex !== 0) { @@ -1175,10 +1175,8 @@ export class WorkflowExecute { outputIndex ]) { if (!workflow.nodes.hasOwnProperty(connectionData.node)) { - return Promise.reject( - new Error( - `The node "${executionNode.name}" connects to not found node "${connectionData.node}"`, - ), + throw new Error( + `The node "${executionNode.name}" connects to not found node "${connectionData.node}"`, ); } @@ -1212,7 +1210,7 @@ export class WorkflowExecute { ]); } - return Promise.resolve(); + return; })() .then(async () => { if (gotCancel && executionError === undefined) { diff --git a/packages/design-system/src/components/N8nActionDropdown/ActionDropdown.vue b/packages/design-system/src/components/N8nActionDropdown/ActionDropdown.vue index 08f0147a20575..75a785a590289 100644 --- a/packages/design-system/src/components/N8nActionDropdown/ActionDropdown.vue +++ b/packages/design-system/src/components/N8nActionDropdown/ActionDropdown.vue @@ -34,7 +34,8 @@