fix(core): Initialize JWT Secret before it's used anywhere (#7707)

HELP-394

packages/cli/src/auth/jwt.ts

@@ -1,4 +1,3 @@
-import jwt from 'jsonwebtoken';
 import type { Response } from 'express';
 import { createHash } from 'crypto';
 import { AUTH_COOKIE_NAME, RESPONSE_ERROR_MESSAGES } from '@/constants';
@@ -9,6 +8,7 @@ import * as ResponseHelper from '@/ResponseHelper';
 import { License } from '@/License';
 import { Container } from 'typedi';
 import { UserRepository } from '@db/repositories/user.repository';
+import { JwtService } from '@/services/jwt.service';
 
 export function issueJWT(user: User): JwtToken {
 	const { id, email, password } = user;
@@ -34,7 +34,7 @@ export function issueJWT(user: User): JwtToken {
 			.digest('hex');
 	}
 
-	const signedToken = jwt.sign(payload, config.getEnv('userManagement.jwtSecret'), {
+	const signedToken = Container.get(JwtService).sign(payload, {
 		expiresIn: expiresIn / 1000 /* in seconds */,
 	});
 
@@ -75,9 +75,9 @@ export async function resolveJwtContent(jwtPayload: JwtPayload): Promise<User> {
 }
 
 export async function resolveJwt(token: string): Promise<User> {
-	const jwtPayload = jwt.verify(token, config.getEnv('userManagement.jwtSecret'), {
+	const jwtPayload: JwtPayload = Container.get(JwtService).verify(token, {
 		algorithms: ['HS256'],
-	}) as JwtPayload;
+	});
 	return resolveJwtContent(jwtPayload);
 }
 

packages/cli/src/commands/start.ts

@@ -13,7 +13,6 @@ import { promisify } from 'util';
 import glob from 'fast-glob';
 
 import { sleep, jsonParse } from 'n8n-workflow';
-import { createHash } from 'crypto';
 import config from '@/config';
 
 import { ActiveExecutions } from '@/ActiveExecutions';
@@ -272,20 +271,6 @@ export class Start extends BaseCommand {
 		// eslint-disable-next-line @typescript-eslint/no-shadow
 		const { flags } = this.parse(Start);
 
-		if (!config.getEnv('userManagement.jwtSecret')) {
-			// If we don't have a JWT secret set, generate
-			// one based and save to config.
-			const { encryptionKey } = this.instanceSettings;
-
-			// For a key off every other letter from encryption key
-			// CAREFUL: do not change this or it breaks all existing tokens.
-			let baseKey = '';
-			for (let i = 0; i < encryptionKey.length; i += 2) {
-				baseKey += encryptionKey[i];
-			}
-			config.set('userManagement.jwtSecret', createHash('sha256').update(baseKey).digest('hex'));
-		}
-
 		// Load settings from database and set them to config.
 		const databaseSettings = await Container.get(SettingsRepository).findBy({
 			loadOnStartup: true,

packages/cli/src/middlewares/auth.ts

@@ -6,11 +6,11 @@ import { Strategy } from 'passport-jwt';
 import { sync as globSync } from 'fast-glob';
 import type { JwtPayload } from '@/Interfaces';
 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 { canSkipAuth } from '@/decorators/registerController';
 import { Logger } from '@/Logger';
+import { JwtService } from '@/services/jwt.service';
 
 const jwtFromRequest = (req: Request) => {
 	// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
@@ -21,7 +21,7 @@ const userManagementJwtAuth = (): RequestHandler => {
 	const jwtStrategy = new Strategy(
 		{
 			jwtFromRequest,
-			secretOrKey: config.getEnv('userManagement.jwtSecret'),
+			secretOrKey: Container.get(JwtService).jwtSecret,
 		},
 		async (jwtPayload: JwtPayload, done) => {
 			try {

packages/cli/src/services/jwt.service.ts

@@ -1,17 +1,34 @@
 import { Service } from 'typedi';
-import * as jwt from 'jsonwebtoken';
+import { createHash } from 'crypto';
+import jwt from 'jsonwebtoken';
+import { InstanceSettings } from 'n8n-core';
 import config from '@/config';
 
 @Service()
 export class JwtService {
-	private readonly userManagementSecret = config.getEnv('userManagement.jwtSecret');
+	readonly jwtSecret = config.getEnv('userManagement.jwtSecret');
 
-	public signData(payload: object, options: jwt.SignOptions = {}): string {
-		return jwt.sign(payload, this.userManagementSecret, options);
+	constructor({ encryptionKey }: InstanceSettings) {
+		this.jwtSecret = config.getEnv('userManagement.jwtSecret');
+		if (!this.jwtSecret) {
+			// If we don't have a JWT secret set, generate one based on encryption key.
+			// For a key off every other letter from encryption key
+			// CAREFUL: do not change this or it breaks all existing tokens.
+			let baseKey = '';
+			for (let i = 0; i < encryptionKey.length; i += 2) {
+				baseKey += encryptionKey[i];
+			}
+			this.jwtSecret = createHash('sha256').update(baseKey).digest('hex');
+			config.set('userManagement.jwtSecret', this.jwtSecret);
+		}
 	}
 
-	public verifyToken<T = JwtPayload>(token: string, options: jwt.VerifyOptions = {}) {
-		return jwt.verify(token, this.userManagementSecret, options) as T;
+	public sign(payload: object, options: jwt.SignOptions = {}): string {
+		return jwt.sign(payload, this.jwtSecret, options);
+	}
+
+	public verify<T = JwtPayload>(token: string, options: jwt.VerifyOptions = {}) {
+		return jwt.verify(token, this.jwtSecret, options) as T;
 	}
 }
 

packages/cli/src/services/user.service.ts

@@ -63,7 +63,7 @@ export class UserService {
 	}
 
 	generatePasswordResetToken(user: User, expiresIn = '20m') {
-		return this.jwtService.signData(
+		return this.jwtService.sign(
 			{ sub: user.id, passwordSha: createPasswordSha(user) },
 			{ expiresIn },
 		);
@@ -82,7 +82,7 @@ export class UserService {
 	async resolvePasswordResetToken(token: string): Promise<User | undefined> {
 		let decodedToken: JwtPayload & { passwordSha: string };
 		try {
-			decodedToken = this.jwtService.verifyToken(token);
+			decodedToken = this.jwtService.verify(token);
 		} catch (e) {
 			if (e instanceof TokenExpiredError) {
 				this.logger.debug('Reset password token expired', { token });

packages/cli/test/integration/passwordReset.api.test.ts

@@ -156,7 +156,7 @@ describe('GET /resolve-password-token', () => {
 	});
 
 	test('should fail if user is not found', async () => {
-		const token = jwtService.signData({ sub: uuid() });
+		const token = jwtService.sign({ sub: uuid() });
 
 		const response = await testServer.authlessAgent
 			.get('/resolve-password-token')

packages/cli/test/unit/services/jwt.service.test.ts

@@ -1,42 +1,62 @@
+import jwt from 'jsonwebtoken';
+import type { InstanceSettings } from 'n8n-core';
+import { mock } from 'jest-mock-extended';
 import config from '@/config';
 import { JwtService } from '@/services/jwt.service';
-import { randomString } from '../../integration/shared/random';
-import * as jwt from 'jsonwebtoken';
 
 describe('JwtService', () => {
-	config.set('userManagement.jwtSecret', randomString(5, 10));
+	const iat = 1699984313;
+	const jwtSecret = 'random-string';
+	const payload = { sub: 1 };
+	const signedToken =
+		'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOjEsImlhdCI6MTY5OTk4NDMxM30.xNZOAmcidW5ovEF_mwIOzCWkJ70FEO6MFNLK2QRDOeQ';
 
-	const jwtService = new JwtService();
+	const instanceSettings = mock<InstanceSettings>({ encryptionKey: 'test-key' });
 
 	beforeEach(() => {
 		jest.clearAllMocks();
 	});
 
-	test('Should sign input with user management secret', async () => {
-		const userId = 1;
+	describe('secret initialization', () => {
+		it('should read the secret from config, when set', () => {
+			config.set('userManagement.jwtSecret', jwtSecret);
+			const jwtService = new JwtService(instanceSettings);
+			expect(jwtService.jwtSecret).toEqual(jwtSecret);
+		});
 
-		const token = jwtService.signData({ sub: userId });
-		expect(typeof token).toBe('string');
-
-		const secret = config.get('userManagement.jwtSecret');
-
-		const decodedToken = jwt.verify(token, secret);
-
-		expect(decodedToken).toHaveProperty('sub');
-		expect(decodedToken).toHaveProperty('iat');
-		expect(decodedToken?.sub).toBe(userId);
+		it('should derive the secret from encryption key when not set in config', () => {
+			config.set('userManagement.jwtSecret', '');
+			const jwtService = new JwtService(instanceSettings);
+			expect(jwtService.jwtSecret).toEqual(
+				'e9e2975005eddefbd31b2c04a0b0f2d9c37d9d718cf3676cddf76d65dec555cb',
+			);
+		});
 	});
 
-	test('Should verify token with user management secret', async () => {
-		const userId = 1;
+	describe('with a secret set', () => {
+		config.set('userManagement.jwtSecret', jwtSecret);
+		const jwtService = new JwtService(instanceSettings);
 
-		const secret = config.get('userManagement.jwtSecret');
+		beforeAll(() => {
+			jest.useFakeTimers().setSystemTime(new Date(iat * 1000));
+		});
 
-		const token = jwt.sign({ sub: userId }, secret);
+		afterAll(() => jest.useRealTimers());
 
-		const decodedToken = jwt.verify(token, secret);
+		it('should sign', () => {
+			const token = jwtService.sign(payload);
+			expect(token).toEqual(signedToken);
+		});
 
-		expect(decodedToken).toHaveProperty('sub');
-		expect(decodedToken?.sub).toBe(userId);
+		it('should decode and verify payload', () => {
+			const decodedToken = jwtService.verify(signedToken);
+			expect(decodedToken.sub).toEqual(1);
+			expect(decodedToken.iat).toEqual(iat);
+		});
+
+		it('should throw an error on verify if the token is expired', () => {
+			const expiredToken = jwt.sign(payload, jwtSecret, { expiresIn: -10 });
+			expect(() => jwtService.verify(expiredToken)).toThrow(jwt.TokenExpiredError);
+		});
 	});
 });