refactor(core): Add unit tests for all external auth middlewares (no-changelog) (#5386)

This commit is contained in:
कारतोफ्फेलस्क्रिप्ट™ 2023-02-07 15:49:35 +01:00 committed by GitHub
parent 3a435f7057
commit 7e2f2f7453
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
10 changed files with 492 additions and 398 deletions

View file

@ -40,7 +40,7 @@
"@n8n_io/eslint-config": "*",
"@ngneat/falso": "^6.1.0",
"@types/jest": "^29.2.2",
"@types/node": "^16.11.22",
"@types/supertest": "^2.0.12",
"cross-env": "^7.0.3",
"cypress": "^10.0.3",
"cypress-real-events": "^1.7.6",
@ -53,7 +53,7 @@
"rimraf": "^3.0.2",
"run-script-os": "^1.0.7",
"start-server-and-test": "^1.14.0",
"supertest": "^6.2.2",
"supertest": "^6.3.3",
"ts-jest": "^29.0.3",
"tsc-watch": "^5.0.3",
"turbo": "1.6.3",
@ -65,6 +65,7 @@
"vue-demi"
],
"overrides": {
"@types/node": "^16.18.12",
"browserslist": "^4.21.4",
"ejs": "^3.1.8",
"fork-ts-checker-webpack-plugin": "^6.0.4",

View file

@ -63,7 +63,7 @@
"devDependencies": {
"@apidevtools/swagger-cli": "4.0.0",
"@oclif/dev-cli": "^1.22.2",
"@types/basic-auth": "^1.1.2",
"@types/basic-auth": "^1.1.3",
"@types/bcryptjs": "^2.4.2",
"@types/body-parser-xml": "^2.0.2",
"@types/compression": "1.0.1",
@ -72,7 +72,7 @@
"@types/cookie-parser": "^1.4.2",
"@types/express": "^4.17.6",
"@types/json-diff": "^0.5.1",
"@types/jsonwebtoken": "^9.0.0",
"@types/jsonwebtoken": "^9.0.1",
"@types/localtunnel": "^1.9.0",
"@types/lodash.get": "^4.4.6",
"@types/lodash.intersection": "^4.4.7",
@ -94,16 +94,15 @@
"@types/send": "^0.17.1",
"@types/shelljs": "^0.8.11",
"@types/superagent": "4.1.13",
"@types/supertest": "^2.0.11",
"@types/swagger-ui-express": "^4.1.3",
"@types/syslog-client": "^1.1.2",
"@types/uuid": "^8.3.2",
"@types/validator": "^13.7.0",
"@types/yamljs": "^0.2.31",
"concurrently": "^5.1.0",
"mock-jwks": "^1.0.9",
"nodemon": "^2.0.2",
"run-script-os": "^1.0.7",
"supertest": "^6.2.2",
"ts-node": "^9.1.1",
"tsc-alias": "^1.7.0",
"tsconfig-paths": "^3.14.1"
@ -147,7 +146,7 @@
"json-diff": "^0.5.4",
"jsonschema": "^1.4.1",
"jsonwebtoken": "^9.0.0",
"jwks-rsa": "~1.12.1",
"jwks-rsa": "^3.0.1",
"ldapts": "^4.2.2",
"localtunnel": "^2.0.0",
"lodash.get": "^4.4.2",

View file

@ -42,9 +42,6 @@ import type { AxiosRequestConfig } from 'axios';
import axios from 'axios';
import type { RequestOptions } from 'oauth-1.0a';
import clientOAuth1 from 'oauth-1.0a';
// IMPORTANT! Do not switch to anther bcrypt library unless really necessary and
// tested with all possible systems like Windows, Alpine on ARM, FreeBSD, ...
import { compare } from 'bcryptjs';
import {
BinaryDataManager,
@ -68,9 +65,6 @@ import type {
} from 'n8n-workflow';
import { LoggerProxy, jsonParse } from 'n8n-workflow';
import basicAuth from 'basic-auth';
import jwt from 'jsonwebtoken';
import jwks from 'jwks-rsa';
// @ts-ignore
import timezones from 'google-timezones-json';
import history from 'connect-history-api-fallback';
@ -166,6 +160,8 @@ import { ldapController } from './Ldap/routes/ldap.controller.ee';
import { getLdapLoginLabel, isLdapEnabled, isLdapLoginEnabled } from './Ldap/helpers';
import { AbstractServer } from './AbstractServer';
import { configureMetrics } from './metrics';
import { setupBasicAuth } from './middlewares/basicAuth';
import { setupExternalJWTAuth } from './middlewares/externalJWTAuth';
const exec = promisify(callbackExec);
@ -408,150 +404,13 @@ class Server extends AbstractServer {
const authIgnoreRegex = new RegExp(`^\/(${ignoredEndpoints.join('|')})\/?.*$`);
// Check for basic auth credentials if activated
const basicAuthActive = config.getEnv('security.basicAuth.active');
if (basicAuthActive) {
const basicAuthUser = config.getEnv('security.basicAuth.user');
if (basicAuthUser === '') {
throw new Error('Basic auth is activated but no user got defined. Please set one!');
}
const basicAuthPassword = config.getEnv('security.basicAuth.password');
if (basicAuthPassword === '') {
throw new Error('Basic auth is activated but no password got defined. Please set one!');
}
const basicAuthHashEnabled = config.getEnv('security.basicAuth.hash') as boolean;
let validPassword: null | string = null;
this.app.use(
async (req: express.Request, res: express.Response, next: express.NextFunction) => {
// Skip basic auth for a few listed endpoints or when instance owner has been setup
if (
authIgnoreRegex.exec(req.url) ||
config.getEnv('userManagement.isInstanceOwnerSetUp')
) {
return next();
}
const realm = 'n8n - Editor UI';
const basicAuthData = basicAuth(req);
if (basicAuthData === undefined) {
// Authorization data is missing
return ResponseHelper.basicAuthAuthorizationError(
res,
realm,
'Authorization is required!',
);
}
if (basicAuthData.name === basicAuthUser) {
if (basicAuthHashEnabled) {
if (
validPassword === null &&
(await compare(basicAuthData.pass, basicAuthPassword))
) {
// Password is valid so save for future requests
validPassword = basicAuthData.pass;
}
if (validPassword === basicAuthData.pass && validPassword !== null) {
// Provided hash is correct
return next();
}
} else if (basicAuthData.pass === basicAuthPassword) {
// Provided password is correct
return next();
}
}
// Provided authentication data is wrong
return ResponseHelper.basicAuthAuthorizationError(
res,
realm,
'Authorization data is wrong!',
);
},
);
if (config.getEnv('security.basicAuth.active')) {
await setupBasicAuth(this.app, config, authIgnoreRegex);
}
// Check for and validate JWT if configured
const jwtAuthActive = config.getEnv('security.jwtAuth.active');
if (jwtAuthActive) {
const jwtAuthHeader = config.getEnv('security.jwtAuth.jwtHeader');
if (jwtAuthHeader === '') {
throw new Error('JWT auth is activated but no request header was defined. Please set one!');
}
const jwksUri = config.getEnv('security.jwtAuth.jwksUri');
if (jwksUri === '') {
throw new Error('JWT auth is activated but no JWK Set URI was defined. Please set one!');
}
const jwtHeaderValuePrefix = config.getEnv('security.jwtAuth.jwtHeaderValuePrefix');
const jwtIssuer = config.getEnv('security.jwtAuth.jwtIssuer');
const jwtNamespace = config.getEnv('security.jwtAuth.jwtNamespace');
const jwtAllowedTenantKey = config.getEnv('security.jwtAuth.jwtAllowedTenantKey');
const jwtAllowedTenant = config.getEnv('security.jwtAuth.jwtAllowedTenant');
// eslint-disable-next-line no-inner-declarations
function isTenantAllowed(decodedToken: object): boolean {
if (jwtNamespace === '' || jwtAllowedTenantKey === '' || jwtAllowedTenant === '') {
return true;
}
for (const [k, v] of Object.entries(decodedToken)) {
if (k === jwtNamespace) {
for (const [kn, kv] of Object.entries(v)) {
if (kn === jwtAllowedTenantKey && kv === jwtAllowedTenant) {
return true;
}
}
}
}
return false;
}
// eslint-disable-next-line consistent-return
this.app.use((req: express.Request, res: express.Response, next: express.NextFunction) => {
if (authIgnoreRegex.exec(req.url)) {
return next();
}
let token = req.header(jwtAuthHeader) as string;
if (token === undefined || token === '') {
return ResponseHelper.jwtAuthAuthorizationError(res, 'Missing token');
}
if (jwtHeaderValuePrefix !== '' && token.startsWith(jwtHeaderValuePrefix)) {
token = token.replace(`${jwtHeaderValuePrefix} `, '').trimLeft();
}
const jwkClient = jwks({ cache: true, jwksUri });
// eslint-disable-next-line @typescript-eslint/ban-types
function getKey(header: any, callback: Function) {
jwkClient.getSigningKey(header.kid, (err: Error, key: any) => {
// eslint-disable-next-line @typescript-eslint/no-throw-literal
if (err) throw ResponseHelper.jwtAuthAuthorizationError(res, err.message);
const signingKey = key.publicKey || key.rsaPublicKey;
callback(null, signingKey);
});
}
const jwtVerifyOptions: jwt.VerifyOptions = {
issuer: jwtIssuer !== '' ? jwtIssuer : undefined,
ignoreExpiration: false,
};
jwt.verify(token, getKey, jwtVerifyOptions, (err: jwt.VerifyErrors, decoded: object) => {
if (err) {
ResponseHelper.jwtAuthAuthorizationError(res, 'Invalid token');
} else if (!isTenantAllowed(decoded)) {
ResponseHelper.jwtAuthAuthorizationError(res, 'Tenant not allowed');
} else {
next();
}
});
});
if (config.getEnv('security.jwtAuth.active')) {
await setupExternalJWTAuth(this.app, config, authIgnoreRegex);
}
// ----------------------------------------

View file

@ -38,7 +38,7 @@ async function checkWorkflowId(workflowId: string, user: User): Promise<boolean>
}
/**
* Initialise Logger if needed
* Initialize Logger if needed
*/
workflowStatsController.use((req, res, next) => {
try {

View file

@ -0,0 +1,57 @@
import type { Application } from 'express';
import basicAuth from 'basic-auth';
// IMPORTANT! Do not switch to anther bcrypt library unless really necessary and
// tested with all possible systems like Windows, Alpine on ARM, FreeBSD, ...
import { compare } from 'bcryptjs';
import type { Config } from '@/config';
import { basicAuthAuthorizationError } from '@/ResponseHelper';
export const setupBasicAuth = async (app: Application, config: Config, authIgnoreRegex: RegExp) => {
const basicAuthUser = config.getEnv('security.basicAuth.user');
if (basicAuthUser === '') {
throw new Error('Basic auth is activated but no user got defined. Please set one!');
}
const basicAuthPassword = config.getEnv('security.basicAuth.password');
if (basicAuthPassword === '') {
throw new Error('Basic auth is activated but no password got defined. Please set one!');
}
const basicAuthHashEnabled = config.getEnv('security.basicAuth.hash');
let validPassword: null | string = null;
app.use(async (req, res, next) => {
// Skip basic auth for a few listed endpoints or when instance owner has been setup
if (authIgnoreRegex.exec(req.url) || config.getEnv('userManagement.isInstanceOwnerSetUp')) {
return next();
}
const realm = 'n8n - Editor UI';
const basicAuthData = basicAuth(req);
if (basicAuthData === undefined) {
// Authorization data is missing
return basicAuthAuthorizationError(res, realm, 'Authorization is required!');
}
if (basicAuthData.name === basicAuthUser) {
if (basicAuthHashEnabled) {
if (validPassword === null && (await compare(basicAuthData.pass, basicAuthPassword))) {
// Password is valid so save for future requests
validPassword = basicAuthData.pass;
}
if (validPassword === basicAuthData.pass && validPassword !== null) {
// Provided hash is correct
return next();
}
} else if (basicAuthData.pass === basicAuthPassword) {
// Provided password is correct
return next();
}
}
// Provided authentication data is wrong
return basicAuthAuthorizationError(res, realm, 'Authorization data is wrong!');
});
};

View file

@ -0,0 +1,89 @@
import type { Application } from 'express';
import jwt from 'jsonwebtoken';
import jwks from 'jwks-rsa';
import type { Config } from '@/config';
import { jwtAuthAuthorizationError } from '@/ResponseHelper';
export const setupExternalJWTAuth = async (
app: Application,
config: Config,
authIgnoreRegex: RegExp,
) => {
const jwtAuthHeader = config.getEnv('security.jwtAuth.jwtHeader');
if (jwtAuthHeader === '') {
throw new Error('JWT auth is activated but no request header was defined. Please set one!');
}
const jwksUri = config.getEnv('security.jwtAuth.jwksUri');
if (jwksUri === '') {
throw new Error('JWT auth is activated but no JWK Set URI was defined. Please set one!');
}
const jwtHeaderValuePrefix = config.getEnv('security.jwtAuth.jwtHeaderValuePrefix');
const jwtIssuer = config.getEnv('security.jwtAuth.jwtIssuer');
const jwtNamespace = config.getEnv('security.jwtAuth.jwtNamespace');
const jwtAllowedTenantKey = config.getEnv('security.jwtAuth.jwtAllowedTenantKey');
const jwtAllowedTenant = config.getEnv('security.jwtAuth.jwtAllowedTenant');
// eslint-disable-next-line no-inner-declarations
function isTenantAllowed(decodedToken: object): boolean {
if (jwtNamespace === '' || jwtAllowedTenantKey === '' || jwtAllowedTenant === '') {
return true;
}
for (const [k, v] of Object.entries(decodedToken)) {
if (k === jwtNamespace) {
// eslint-disable-next-line @typescript-eslint/no-unsafe-argument
for (const [kn, kv] of Object.entries(v)) {
if (kn === jwtAllowedTenantKey && kv === jwtAllowedTenant) {
return true;
}
}
}
}
return false;
}
// eslint-disable-next-line consistent-return
app.use((req, res, next) => {
if (authIgnoreRegex.exec(req.url)) {
return next();
}
let token = req.header(jwtAuthHeader) as string;
if (token === undefined || token === '') {
return jwtAuthAuthorizationError(res, 'Missing token');
}
if (jwtHeaderValuePrefix !== '' && token.startsWith(jwtHeaderValuePrefix)) {
token = token.replace(`${jwtHeaderValuePrefix} `, '').trimStart();
}
const jwkClient = jwks({ cache: true, jwksUri });
const getKey: jwt.GetPublicKeyOrSecret = (header, callbackFn) => {
// eslint-disable-next-line @typescript-eslint/no-throw-literal
if (!header.kid) throw jwtAuthAuthorizationError(res, 'No JWT key found');
jwkClient.getSigningKey(header.kid, (error, key) => {
// eslint-disable-next-line @typescript-eslint/no-throw-literal
if (error) throw jwtAuthAuthorizationError(res, error.message);
callbackFn(null, key?.getPublicKey());
});
};
const jwtVerifyOptions: jwt.VerifyOptions = {
issuer: jwtIssuer !== '' ? jwtIssuer : undefined,
ignoreExpiration: false,
};
jwt.verify(token, getKey, jwtVerifyOptions, (error: jwt.VerifyErrors, decoded: object) => {
if (error) {
jwtAuthAuthorizationError(res, 'Invalid token');
} else if (!isTenantAllowed(decoded)) {
jwtAuthAuthorizationError(res, 'Tenant not allowed');
} else {
next();
}
});
});
};

View file

@ -0,0 +1,42 @@
import express from 'express';
import request from 'supertest';
import config from '@/config';
import { setupBasicAuth } from '@/middlewares/basicAuth';
describe('Basic Auth Middleware', () => {
let app: express.Application;
beforeAll(() => {
app = express();
config.set('security.basicAuth', { user: 'jim', password: 'n8n', hash: false });
setupBasicAuth(app, config, new RegExp('^/skip-auth'));
app.get('/test', (req, res) => res.send({ auth: true }));
app.get('/skip-auth', (req, res) => res.send({ auth: false }));
});
it('should not block calls to /skip-auth', async () => {
const response = await request(app).get('/skip-auth');
expect(response.statusCode).toEqual(200);
expect(response.headers).not.toHaveProperty('www-authenticate');
expect(response.body).toEqual({ auth: false });
});
it('should block calls to /test if auth is absent', async () => {
const response = await request(app).get('/test');
expect(response.statusCode).toEqual(401);
expect(response.headers).toHaveProperty('www-authenticate');
});
it('should block calls to /test if auth is invalid', async () => {
const response = await request(app).get('/test').auth('user', 'invalid');
expect(response.statusCode).toEqual(401);
expect(response.headers).toHaveProperty('www-authenticate');
});
it('should allow access to /test if basic auth header is valid', async () => {
const response = await request(app).get('/test').auth('jim', 'n8n');
expect(response.statusCode).toEqual(200);
expect(response.headers).not.toHaveProperty('www-authenticate');
expect(response.body).toEqual({ auth: true });
});
});

View file

@ -0,0 +1,47 @@
import express from 'express';
import request from 'supertest';
import createJWKSMock from 'mock-jwks';
import config from '@/config';
import { setupExternalJWTAuth } from '@/middlewares/externalJWTAuth';
const testJWKUri = 'https://n8n.test/';
const jwksMock = createJWKSMock(testJWKUri);
describe('External JWT Auth Middleware', () => {
let app: express.Application;
beforeAll(() => {
app = express();
config.set('security.jwtAuth.jwtHeader', 'Authorization');
config.set('security.jwtAuth.jwtHeaderValuePrefix', 'Bearer');
config.set('security.jwtAuth.jwtIssuer', 'n8n');
config.set('security.jwtAuth.jwksUri', `${testJWKUri}.well-known/jwks.json`);
setupExternalJWTAuth(app, config, new RegExp('^/skip-auth'));
app.get('/test', (req, res) => res.send({ auth: true }));
app.get('/skip-auth', (req, res) => res.send({ auth: false }));
jwksMock.start();
});
it('should not block calls to /skip-auth', async () => {
const response = await request(app).get('/skip-auth');
expect(response.statusCode).toEqual(200);
expect(response.body).toEqual({ auth: false });
});
it('should block calls to /test if auth is absent', async () =>
request(app).get('/test').expect(403));
it('should block calls to /test if auth is invalid', async () => {
const token = jwksMock.token({ iss: 'invalid' });
const response = await request(app).get('/test').set('Authorization', `Bearer ${token}`);
expect(response.statusCode).toEqual(403);
});
it('should allow access to /test if JWT auth header is valid', async () => {
const token = jwksMock.token({ iss: 'n8n' });
const response = await request(app).get('/test').set('Authorization', `Bearer ${token}`);
expect(response.statusCode).toEqual(200);
expect(response.body).toEqual({ auth: true });
});
});

View file

@ -730,7 +730,7 @@
"devDependencies": {
"@types/amqplib": "^0.10.1",
"@types/aws4": "^1.5.1",
"@types/basic-auth": "^1.1.2",
"@types/basic-auth": "^1.1.3",
"@types/cheerio": "^0.22.15",
"@types/cron": "~1.7.1",
"@types/eventsource": "^1.1.2",
@ -738,7 +738,7 @@
"@types/formidable": "^1.0.31",
"@types/gm": "^1.18.2",
"@types/imap-simple": "^4.2.0",
"@types/jsonwebtoken": "^9.0.0",
"@types/jsonwebtoken": "^9.0.1",
"@types/lodash.set": "^4.3.6",
"@types/lossless-json": "^1.0.0",
"@types/mailparser": "^2.7.3",

File diff suppressed because it is too large Load diff