Skip to content

Commit

Permalink
🚀 Release 0.216.1 (#5531)
Browse files Browse the repository at this point in the history
* 🚀 Release 0.216.1

* fix(core): Do not allow arbitrary path traversal in the credential-translation endpoint (#5522)

* fix(core): Do not allow arbitrary path traversal in BinaryDataManager (#5523)

* fix(core): User update endpoint should only allow updating email, firstName, and lastName (#5526)

* fix(core): Do not explicitly bypass auth on urls containing `.svg` (#5525)

* 📚 Update CHANGELOG.md

---------

Co-authored-by: janober <janober@users.noreply.github.com>
Co-authored-by: कारतोफ्फेलस्क्रिप्ट™ <netroy@users.noreply.github.com>
Co-authored-by: Jan Oberhauser <jan.oberhauser@gmail.com>
  • Loading branch information
4 people authored Feb 21, 2023
1 parent fe782c8 commit 7400c35
Show file tree
Hide file tree
Showing 24 changed files with 272 additions and 138 deletions.
12 changes: 12 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,15 @@
## [0.216.1](https://github.com/n8n-io/n8n/compare/n8n@0.216.0...n8n@0.216.1) (2023-02-21)


### Bug Fixes

* **core:** Do not allow arbitrary path traversal in BinaryDataManager ([#5523](https://github.com/n8n-io/n8n/issues/5523)) ([40b9784](https://github.com/n8n-io/n8n/commit/40b97846483fe7c58229c156acb66f43a5a79dc3))
* **core:** Do not allow arbitrary path traversal in the credential-translation endpoint ([#5522](https://github.com/n8n-io/n8n/issues/5522)) ([fb07d77](https://github.com/n8n-io/n8n/commit/fb07d77106bb4933758c63bbfb87f591bf4a27dd))
* **core:** Do not explicitly bypass auth on urls containing `.svg` ([#5525](https://github.com/n8n-io/n8n/issues/5525)) ([27adea7](https://github.com/n8n-io/n8n/commit/27adea70459329fc0dddabee69e10c9d1453835f))
* **core:** User update endpoint should only allow updating email, firstName, and lastName ([#5526](https://github.com/n8n-io/n8n/issues/5526)) ([5599221](https://github.com/n8n-io/n8n/commit/5599221007cb09cb81f0623874fafc6cd481384c))



# [0.216.0](https://github.com/n8n-io/n8n/compare/n8n@0.215.2...n8n@0.216.0) (2023-02-16)


Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "n8n",
"version": "0.216.0",
"version": "0.216.1",
"private": true,
"homepage": "https://n8n.io",
"engines": {
Expand Down
3 changes: 2 additions & 1 deletion packages/cli/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "n8n",
"version": "0.216.0",
"version": "0.216.1",
"description": "n8n Workflow Automation Tool",
"license": "SEE LICENSE IN LICENSE.md",
"homepage": "https://n8n.io",
Expand Down Expand Up @@ -127,6 +127,7 @@
"callsites": "^3.1.0",
"change-case": "^4.1.1",
"class-validator": "^0.14.0",
"class-transformer": "^0.5.1",
"client-oauth2": "^4.2.5",
"compression": "^1.7.4",
"connect-history-api-fallback": "^1.6.0",
Expand Down
3 changes: 2 additions & 1 deletion packages/cli/src/GenericHelpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import type { WorkflowEntity } from '@db/entities/WorkflowEntity';
import type { CredentialsEntity } from '@db/entities/CredentialsEntity';
import type { TagEntity } from '@db/entities/TagEntity';
import type { User } from '@db/entities/User';
import type { UserUpdatePayload } from '@/requests';

/**
* Returns the base URL n8n is reachable from
Expand Down Expand Up @@ -99,7 +100,7 @@ export async function generateUniqueName(
}

export async function validateEntity(
entity: WorkflowEntity | CredentialsEntity | TagEntity | User,
entity: WorkflowEntity | CredentialsEntity | TagEntity | User | UserUpdatePayload,
): Promise<void> {
const errors = await validate(entity);

Expand Down
79 changes: 22 additions & 57 deletions packages/cli/src/Server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import {
LoadNodeParameterOptions,
LoadNodeListSearch,
UserSettings,
FileNotFoundError,
} from 'n8n-core';

import type {
Expand All @@ -55,7 +56,6 @@ import history from 'connect-history-api-fallback';
import config from '@/config';
import * as Queue from '@/Queue';
import { InternalHooksManager } from '@/InternalHooksManager';
import { getCredentialTranslationPath } from '@/TranslationHelpers';
import { getSharedWorkflowIds } from '@/WorkflowHelpers';

import { nodesController } from '@/api/nodes.api';
Expand Down Expand Up @@ -86,6 +86,7 @@ import {
MeController,
OwnerController,
PasswordResetController,
TranslationController,
UsersController,
} from '@/controllers';

Expand Down Expand Up @@ -347,6 +348,7 @@ class Server extends AbstractServer {
new OwnerController({ config, internalHooks, repositories, logger }),
new MeController({ externalHooks, internalHooks, repositories, logger }),
new PasswordResetController({ config, externalHooks, internalHooks, repositories, logger }),
new TranslationController(config, this.credentialTypes),
new UsersController({
config,
mailer,
Expand Down Expand Up @@ -585,48 +587,6 @@ class Server extends AbstractServer {
),
);

this.app.get(
`/${this.restEndpoint}/credential-translation`,
ResponseHelper.send(
async (
req: express.Request & { query: { credentialType: string } },
res: express.Response,
): Promise<object | null> => {
const translationPath = getCredentialTranslationPath({
locale: this.frontendSettings.defaultLocale,
credentialType: req.query.credentialType,
});

try {
return require(translationPath);
} catch (error) {
return null;
}
},
),
);

// Returns node information based on node names and versions
const headersPath = pathJoin(NODES_BASE_DIR, 'dist', 'nodes', 'headers');
this.app.get(
`/${this.restEndpoint}/node-translation-headers`,
ResponseHelper.send(
async (req: express.Request, res: express.Response): Promise<object | void> => {
try {
await fsAccess(`${headersPath}.js`);
} catch (_) {
return; // no headers available
}

try {
return require(headersPath);
} catch (error) {
res.status(500).send('Failed to load headers file');
}
},
),
);

// ----------------------------------------
// Node-Types
// ----------------------------------------
Expand Down Expand Up @@ -1160,21 +1120,26 @@ class Server extends AbstractServer {
// TODO UM: check if this needs permission check for UM
const identifier = req.params.path;
const binaryDataManager = BinaryDataManager.getInstance();
const binaryPath = binaryDataManager.getBinaryPath(identifier);
let { mode, fileName, mimeType } = req.query;
if (!fileName || !mimeType) {
try {
const metadata = await binaryDataManager.getBinaryMetadata(identifier);
fileName = metadata.fileName;
mimeType = metadata.mimeType;
res.setHeader('Content-Length', metadata.fileSize);
} catch {}
}
if (mimeType) res.setHeader('Content-Type', mimeType);
if (mode === 'download') {
res.setHeader('Content-Disposition', `attachment; filename="${fileName}"`);
try {
const binaryPath = binaryDataManager.getBinaryPath(identifier);
let { mode, fileName, mimeType } = req.query;
if (!fileName || !mimeType) {
try {
const metadata = await binaryDataManager.getBinaryMetadata(identifier);
fileName = metadata.fileName;
mimeType = metadata.mimeType;
res.setHeader('Content-Length', metadata.fileSize);
} catch {}
}
if (mimeType) res.setHeader('Content-Type', mimeType);
if (mode === 'download') {
res.setHeader('Content-Disposition', `attachment; filename="${fileName}"`);
}
res.sendFile(binaryPath);
} catch (error) {
if (error instanceof FileNotFoundError) res.writeHead(404).end();
else throw error;
}
res.sendFile(binaryPath);
},
);

Expand Down
16 changes: 0 additions & 16 deletions packages/cli/src/TranslationHelpers.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { join, dirname } from 'path';
import { readdir } from 'fs/promises';
import type { Dirent } from 'fs';
import { NODES_BASE_DIR } from '@/constants';

const ALLOWED_VERSIONED_DIRNAME_LENGTH = [2, 3]; // e.g. v1, v10

Expand Down Expand Up @@ -47,18 +46,3 @@ export async function getNodeTranslationPath({
? join(nodeDir, `v${maxVersion}`, 'translations', locale, `${nodeType}.json`)
: join(nodeDir, 'translations', locale, `${nodeType}.json`);
}

/**
* Get the full path to a credential translation file in `/dist`.
*/
export function getCredentialTranslationPath({
locale,
credentialType,
}: {
locale: string;
credentialType: string;
}): string {
const credsPath = join(NODES_BASE_DIR, 'dist', 'credentials');

return join(credsPath, 'translations', locale, `${credentialType}.json`);
}
1 change: 1 addition & 0 deletions packages/cli/src/controllers/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,5 @@ export { AuthController } from './auth.controller';
export { MeController } from './me.controller';
export { OwnerController } from './owner.controller';
export { PasswordResetController } from './passwordReset.controller';
export { TranslationController } from './translation.controller';
export { UsersController } from './users.controller';
35 changes: 19 additions & 16 deletions packages/cli/src/controllers/me.controller.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import validator from 'validator';
import { plainToInstance } from 'class-transformer';
import { Delete, Get, Patch, Post, RestController } from '@/decorators';
import {
compareHash,
Expand All @@ -7,13 +8,13 @@ import {
validatePassword,
} from '@/UserManagement/UserManagementHelper';
import { BadRequestError } from '@/ResponseHelper';
import { User } from '@db/entities/User';
import type { User } from '@db/entities/User';
import { validateEntity } from '@/GenericHelpers';
import { issueCookie } from '@/auth/jwt';
import { Response } from 'express';
import type { Repository } from 'typeorm';
import type { ILogger } from 'n8n-workflow';
import { AuthenticatedRequest, MeRequest } from '@/requests';
import { AuthenticatedRequest, MeRequest, UserUpdatePayload } from '@/requests';
import type {
PublicUser,
IDatabaseCollections,
Expand Down Expand Up @@ -61,38 +62,40 @@ export class MeController {
* Update the logged-in user's settings, except password.
*/
@Patch('/')
async updateCurrentUser(req: MeRequest.Settings, res: Response): Promise<PublicUser> {
const { email } = req.body;
async updateCurrentUser(req: MeRequest.UserUpdate, res: Response): Promise<PublicUser> {
const { id: userId, email: currentEmail } = req.user;
const payload = plainToInstance(UserUpdatePayload, req.body);

const { email } = payload;
if (!email) {
this.logger.debug('Request to update user email failed because of missing email in payload', {
userId: req.user.id,
payload: req.body,
userId,
payload,
});
throw new BadRequestError('Email is mandatory');
}

if (!validator.isEmail(email)) {
this.logger.debug('Request to update user email failed because of invalid email in payload', {
userId: req.user.id,
userId,
invalidEmail: email,
});
throw new BadRequestError('Invalid email address');
}

const { email: currentEmail } = req.user;
const newUser = new User();

Object.assign(newUser, req.user, req.body);
await validateEntity(payload);

await validateEntity(newUser);

const user = await this.userRepository.save(newUser);
await this.userRepository.update(userId, payload);
const user = await this.userRepository.findOneOrFail({
where: { id: userId },
relations: { globalRole: true },
});

this.logger.info('User updated successfully', { userId: user.id });
this.logger.info('User updated successfully', { userId });

await issueCookie(res, user);

const updatedKeys = Object.keys(req.body);
const updatedKeys = Object.keys(payload);
void this.internalHooks.onUserUpdate({
user,
fields_changed: updatedKeys,
Expand Down
58 changes: 58 additions & 0 deletions packages/cli/src/controllers/translation.controller.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
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 { BadRequestError, InternalServerError } from '@/ResponseHelper';
import { Config } from '@/config';
import { NODES_BASE_DIR } from '@/constants';

export const CREDENTIAL_TRANSLATIONS_DIR = 'n8n-nodes-base/dist/credentials/translations';
export const NODE_HEADERS_PATH = join(NODES_BASE_DIR, 'dist/nodes/headers');

export declare namespace TranslationRequest {
export type Credential = Request<{}, {}, {}, { credentialType: string }>;
}

@RestController('/')
export class TranslationController {
constructor(private config: Config, private credentialTypes: ICredentialTypes) {}

@Get('/credential-translation')
async getCredentialTranslation(req: TranslationRequest.Credential) {
const { credentialType } = req.query;

if (!this.credentialTypes.recognizes(credentialType))
throw new BadRequestError(`Invalid Credential type: "${credentialType}"`);

const defaultLocale = this.config.getEnv('defaultLocale');
const translationPath = join(
CREDENTIAL_TRANSLATIONS_DIR,
defaultLocale,
`${credentialType}.json`,
);

try {
// eslint-disable-next-line @typescript-eslint/no-unsafe-return
return require(translationPath);
} catch (error) {
return null;
}
}

@Get('/node-translation-headers')
async getNodeTranslationHeaders() {
try {
await access(`${NODE_HEADERS_PATH}.js`);
} catch (_) {
return; // no headers available
}

try {
// eslint-disable-next-line @typescript-eslint/no-unsafe-return
return require(NODE_HEADERS_PATH);
} catch (error) {
throw new InternalServerError('Failed to load headers file');
}
}
}
5 changes: 4 additions & 1 deletion packages/cli/src/databases/entities/User.ts
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,9 @@ export class User extends AbstractEntity implements IUser {
@AfterLoad()
@AfterUpdate()
computeIsPending(): void {
this.isPending = this.password === null;
this.isPending =
this.globalRole?.name === 'owner' && this.globalRole.scope === 'global'
? false
: this.password === null;
}
}
14 changes: 7 additions & 7 deletions packages/cli/src/middlewares/auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,12 @@ import jwt from 'jsonwebtoken';
import cookieParser from 'cookie-parser';
import passport from 'passport';
import { Strategy } from 'passport-jwt';
import { sync as globSync } from 'fast-glob';
import { LoggerProxy as Logger } from 'n8n-workflow';
import type { JwtPayload } from '@/Interfaces';
import type { AuthenticatedRequest } from '@/requests';
import config from '@/config';
import { AUTH_COOKIE_NAME } from '@/constants';
import { AUTH_COOKIE_NAME, EDITOR_UI_DIST_DIR } from '@/constants';
import { issueCookie, resolveJwtContent } from '@/auth/jwt';
import {
isAuthenticatedRequest,
Expand Down Expand Up @@ -61,6 +62,10 @@ const refreshExpiringCookie: RequestHandler = async (req: AuthenticatedRequest,

const passportMiddleware = passport.authenticate('jwt', { session: false }) as RequestHandler;

const staticAssets = globSync(['**/*.html', '**/*.svg', '**/*.png', '**/*.ico'], {
cwd: EDITOR_UI_DIST_DIR,
});

/**
* This sets up the auth middlewares in the correct order
*/
Expand All @@ -79,12 +84,7 @@ export const setupAuthMiddlewares = (
// TODO: refactor me!!!
// skip authentication for preflight requests
req.method === 'OPTIONS' ||
req.url === '/index.html' ||
req.url === '/favicon.ico' ||
req.url.startsWith('/css/') ||
req.url.startsWith('/js/') ||
req.url.startsWith('/fonts/') ||
req.url.includes('.svg') ||
staticAssets.includes(req.url.slice(1)) ||
req.url.startsWith(`/${restEndpoint}/settings`) ||
req.url.startsWith(`/${restEndpoint}/login`) ||
req.url.startsWith(`/${restEndpoint}/logout`) ||
Expand Down
Loading

0 comments on commit 7400c35

Please sign in to comment.