From 4ceac38e0397be91bfc1b50d8a06ebf20de8c32e Mon Sep 17 00:00:00 2001 From: Ben Hesseldieck <1849459+BHesseldieck@users.noreply.github.com> Date: Mon, 2 May 2022 12:11:46 +0200 Subject: [PATCH] fix(core): Do not applying auth if UM is disabled (#3218) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 🔓 not applying auth if UM is disabled * 🛠 add helpers for UM enabled/disabled * 👕 fix lint issue * 🔥 remove unused imports --- packages/cli/src/Server.ts | 46 +++++++------------ .../UserManagement/UserManagementHelper.ts | 14 ++++++ .../cli/src/UserManagement/routes/index.ts | 21 ++++++++- .../cli/src/UserManagement/routes/users.ts | 3 +- 4 files changed, 52 insertions(+), 32 deletions(-) diff --git a/packages/cli/src/Server.ts b/packages/cli/src/Server.ts index b248b70408..7e4af8414b 100644 --- a/packages/cli/src/Server.ts +++ b/packages/cli/src/Server.ts @@ -35,15 +35,12 @@ import { readFile } from 'fs/promises'; import _, { cloneDeep } from 'lodash'; import { dirname as pathDirname, join as pathJoin, resolve as pathResolve } from 'path'; import { - FindConditions, FindManyOptions, getConnection, getConnectionManager, In, IsNull, - LessThan, LessThanOrEqual, - MoreThan, Not, Raw, } from 'typeorm'; @@ -61,13 +58,7 @@ import { createHmac } from 'crypto'; // tested with all possible systems like Windows, Alpine on ARM, FreeBSD, ... import { compare } from 'bcryptjs'; -import { - BinaryDataManager, - Credentials, - IBinaryDataConfig, - LoadNodeParameterOptions, - UserSettings, -} from 'n8n-core'; +import { BinaryDataManager, Credentials, LoadNodeParameterOptions, UserSettings } from 'n8n-core'; import { ICredentialType, @@ -154,14 +145,11 @@ import { WEBHOOK_METHODS } from './WebhookHelpers'; import { userManagementRouter } from './UserManagement'; import { resolveJwt } from './UserManagement/auth/jwt'; import { User } from './databases/entities/User'; -import { CredentialsEntity } from './databases/entities/CredentialsEntity'; import type { - CredentialRequest, ExecutionRequest, WorkflowRequest, NodeParameterOptionsRequest, OAuthRequest, - AuthenticatedRequest, TagsRequest, } from './requests'; import { DEFAULT_EXECUTIONS_GET_ALL_LIMIT, validateEntity } from './GenericHelpers'; @@ -169,7 +157,11 @@ import { ExecutionEntity } from './databases/entities/ExecutionEntity'; import { SharedWorkflow } from './databases/entities/SharedWorkflow'; import { AUTH_COOKIE_NAME, RESPONSE_ERROR_MESSAGES } from './constants'; import { credentialsController } from './api/credentials.api'; -import { getInstanceBaseUrl, isEmailSetUp } from './UserManagement/UserManagementHelper'; +import { + getInstanceBaseUrl, + isEmailSetUp, + isUserManagementEnabled, +} from './UserManagement/UserManagementHelper'; require('body-parser-xml')(bodyParser); @@ -311,9 +303,7 @@ class App { config.getEnv('personalization.enabled') && config.getEnv('diagnostics.enabled'), defaultLocale: config.getEnv('defaultLocale'), userManagement: { - enabled: - config.getEnv('userManagement.disabled') === false || - config.getEnv('userManagement.isInstanceOwnerSetUp') === true, + enabled: isUserManagementEnabled(), showSetupOnFirstLoad: config.getEnv('userManagement.disabled') === false && config.getEnv('userManagement.isInstanceOwnerSetUp') === false && @@ -346,9 +336,7 @@ class App { getSettingsForFrontend(): IN8nUISettings { // refresh user management status Object.assign(this.frontendSettings.userManagement, { - enabled: - config.getEnv('userManagement.disabled') === false || - config.getEnv('userManagement.isInstanceOwnerSetUp') === true, + enabled: isUserManagementEnabled(), showSetupOnFirstLoad: config.getEnv('userManagement.disabled') === false && config.getEnv('userManagement.isInstanceOwnerSetUp') === false && @@ -567,12 +555,14 @@ class App { return; } - try { - const authCookie = req.cookies?.[AUTH_COOKIE_NAME] ?? ''; - await resolveJwt(authCookie); - } catch (error) { - res.status(401).send('Unauthorized'); - return; + if (isUserManagementEnabled()) { + try { + const authCookie = req.cookies?.[AUTH_COOKIE_NAME] ?? ''; + await resolveJwt(authCookie); + } catch (error) { + res.status(401).send('Unauthorized'); + return; + } } this.push.add(req.query.sessionId as string, req, res); @@ -3041,9 +3031,7 @@ export async function start(): Promise { }, deploymentType: config.getEnv('deployment.type'), binaryDataMode: binarDataConfig.mode, - n8n_multi_user_allowed: - config.getEnv('userManagement.disabled') === false || - config.getEnv('userManagement.isInstanceOwnerSetUp') === true, + n8n_multi_user_allowed: isUserManagementEnabled(), smtp_set_up: config.getEnv('userManagement.emails.mode') === 'smtp', }; diff --git a/packages/cli/src/UserManagement/UserManagementHelper.ts b/packages/cli/src/UserManagement/UserManagementHelper.ts index 4e2737c4c5..95edbb16c2 100644 --- a/packages/cli/src/UserManagement/UserManagementHelper.ts +++ b/packages/cli/src/UserManagement/UserManagementHelper.ts @@ -32,6 +32,20 @@ export function isEmailSetUp(): boolean { return smtp && host && user && pass; } +export function isUserManagementEnabled(): boolean { + return ( + !config.getEnv('userManagement.disabled') || + config.getEnv('userManagement.isInstanceOwnerSetUp') + ); +} + +export function isUserManagementDisabled(): boolean { + return ( + config.getEnv('userManagement.disabled') && + !config.getEnv('userManagement.isInstanceOwnerSetUp') + ); +} + async function getInstanceOwnerRole(): Promise { const ownerRole = await Db.collections.Role.findOneOrFail({ where: { diff --git a/packages/cli/src/UserManagement/routes/index.ts b/packages/cli/src/UserManagement/routes/index.ts index c1ca208951..0a8400e4ef 100644 --- a/packages/cli/src/UserManagement/routes/index.ts +++ b/packages/cli/src/UserManagement/routes/index.ts @@ -19,7 +19,13 @@ import { usersNamespace } from './users'; import { passwordResetNamespace } from './passwordReset'; import { AuthenticatedRequest } from '../../requests'; import { ownerNamespace } from './owner'; -import { isAuthExcluded, isPostUsersId, isAuthenticatedRequest } from '../UserManagementHelper'; +import { + isAuthExcluded, + isPostUsersId, + isAuthenticatedRequest, + isUserManagementDisabled, +} from '../UserManagementHelper'; +import { Db } from '../..'; export function addRoutes(this: N8nApp, ignoredEndpoints: string[], restEndpoint: string): void { // needed for testing; not adding overhead since it directly returns if req.cookies exists @@ -47,7 +53,7 @@ export function addRoutes(this: N8nApp, ignoredEndpoints: string[], restEndpoint this.app.use(passport.initialize()); - this.app.use((req: Request, res: Response, next: NextFunction) => { + this.app.use(async (req: Request, res: Response, next: NextFunction) => { if ( // TODO: refactor me!!! // skip authentication for preflight requests @@ -73,6 +79,17 @@ export function addRoutes(this: N8nApp, ignoredEndpoints: string[], restEndpoint return next(); } + // skip authentication if user management is disabled + if (isUserManagementDisabled()) { + req.user = await Db.collections.User.findOneOrFail( + {}, + { + relations: ['globalRole'], + }, + ); + return next(); + } + return passport.authenticate('jwt', { session: false })(req, res, next); }); diff --git a/packages/cli/src/UserManagement/routes/users.ts b/packages/cli/src/UserManagement/routes/users.ts index d6f1caac7d..4e3be7a653 100644 --- a/packages/cli/src/UserManagement/routes/users.ts +++ b/packages/cli/src/UserManagement/routes/users.ts @@ -12,6 +12,7 @@ import { getInstanceBaseUrl, hashPassword, isEmailSetUp, + isUserManagementDisabled, sanitizeUser, validatePassword, } from '../UserManagementHelper'; @@ -55,7 +56,7 @@ export function usersNamespace(this: N8nApp): void { } // TODO: this should be checked in the middleware rather than here - if (config.getEnv('userManagement.disabled')) { + if (isUserManagementDisabled()) { Logger.debug( 'Request to send email invite(s) to user(s) failed because user management is disabled', );