diff --git a/packages/@n8n/config/src/configs/__tests__/user-management.config.test.ts b/packages/@n8n/config/src/configs/__tests__/user-management.config.test.ts new file mode 100644 index 0000000000..52a9055d8d --- /dev/null +++ b/packages/@n8n/config/src/configs/__tests__/user-management.config.test.ts @@ -0,0 +1,67 @@ +import { Container } from '@n8n/di'; + +import { UserManagementConfig } from '../user-management.config'; + +describe('UserManagementConfig', () => { + beforeEach(() => { + Container.reset(); + jest.clearAllMocks(); + }); + + const originalEnv = process.env; + afterEach(() => { + process.env = originalEnv; + }); + + test('with refresh timout > session, sets refresh timout to `0`', () => { + const consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation(); + + process.env = { + N8N_USER_MANAGEMENT_JWT_DURATION_HOURS: '1', + N8N_USER_MANAGEMENT_JWT_REFRESH_TIMEOUT_HOURS: '2', + }; + + const config = Container.get(UserManagementConfig); + + expect(config.jwtRefreshTimeoutHours).toBe(0); + expect(consoleWarnSpy).toHaveBeenCalledWith( + 'N8N_USER_MANAGEMENT_JWT_REFRESH_TIMEOUT_HOURS needs to be smaller than N8N_USER_MANAGEMENT_JWT_DURATION_HOURS. Setting N8N_USER_MANAGEMENT_JWT_REFRESH_TIMEOUT_HOURS to 0.', + ); + + consoleWarnSpy.mockRestore(); + }); + + test('with refresh timout == session, sets refresh timout to `0`', () => { + const consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation(); + + process.env = { + N8N_USER_MANAGEMENT_JWT_DURATION_HOURS: '1', + N8N_USER_MANAGEMENT_JWT_REFRESH_TIMEOUT_HOURS: '1', + }; + + const config = Container.get(UserManagementConfig); + + expect(config.jwtRefreshTimeoutHours).toBe(0); + expect(consoleWarnSpy).toHaveBeenCalledWith( + 'N8N_USER_MANAGEMENT_JWT_REFRESH_TIMEOUT_HOURS needs to be smaller than N8N_USER_MANAGEMENT_JWT_DURATION_HOURS. Setting N8N_USER_MANAGEMENT_JWT_REFRESH_TIMEOUT_HOURS to 0.', + ); + + consoleWarnSpy.mockRestore(); + }); + + test('with refresh timout < session, keeps refresh timout intact', () => { + const consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation(); + + process.env = { + N8N_USER_MANAGEMENT_JWT_DURATION_HOURS: '10', + N8N_USER_MANAGEMENT_JWT_REFRESH_TIMEOUT_HOURS: '5', + }; + + const config = Container.get(UserManagementConfig); + + expect(config.jwtRefreshTimeoutHours).toBe(5); + expect(consoleWarnSpy).not.toHaveBeenCalled(); + + consoleWarnSpy.mockRestore(); + }); +}); diff --git a/packages/@n8n/config/src/configs/user-management.config.ts b/packages/@n8n/config/src/configs/user-management.config.ts index 2eece00a86..fba482e834 100644 --- a/packages/@n8n/config/src/configs/user-management.config.ts +++ b/packages/@n8n/config/src/configs/user-management.config.ts @@ -86,8 +86,34 @@ class EmailConfig { template: TemplateConfig; } +const INVALID_JWT_REFRESH_TIMEOUT_WARNING = + 'N8N_USER_MANAGEMENT_JWT_REFRESH_TIMEOUT_HOURS needs to be smaller than N8N_USER_MANAGEMENT_JWT_DURATION_HOURS. Setting N8N_USER_MANAGEMENT_JWT_REFRESH_TIMEOUT_HOURS to 0.'; + @Config export class UserManagementConfig { @Nested emails: EmailConfig; + + /** JWT secret to use. If unset, n8n will generate its own. */ + @Env('N8N_USER_MANAGEMENT_JWT_SECRET') + jwtSecret: string = ''; + + /** How long (in hours) before the JWT expires. */ + @Env('N8N_USER_MANAGEMENT_JWT_DURATION_HOURS') + jwtSessionDurationHours: number = 168; + + /** + * How long (in hours) before expiration to automatically refresh it. + * - `0` means 25% of `N8N_USER_MANAGEMENT_JWT_DURATION_HOURS`. + * - `-1` means it will never refresh. This forces users to log back in after expiration. + */ + @Env('N8N_USER_MANAGEMENT_JWT_REFRESH_TIMEOUT_HOURS') + jwtRefreshTimeoutHours: number = 0; + + sanitize() { + if (this.jwtRefreshTimeoutHours >= this.jwtSessionDurationHours) { + console.warn(INVALID_JWT_REFRESH_TIMEOUT_WARNING); + this.jwtRefreshTimeoutHours = 0; + } + } } diff --git a/packages/@n8n/config/src/decorators.ts b/packages/@n8n/config/src/decorators.ts index 06b52004b9..d7f2c04dd2 100644 --- a/packages/@n8n/config/src/decorators.ts +++ b/packages/@n8n/config/src/decorators.ts @@ -81,6 +81,9 @@ export const Config: ClassDecorator = (ConfigClass: Class) => { } } } + + if (typeof config.sanitize === 'function') config.sanitize(); + return config; }; // eslint-disable-next-line @typescript-eslint/no-unsafe-return diff --git a/packages/@n8n/config/test/config.test.ts b/packages/@n8n/config/test/config.test.ts index 853f0b8784..740d0b1553 100644 --- a/packages/@n8n/config/test/config.test.ts +++ b/packages/@n8n/config/test/config.test.ts @@ -2,6 +2,7 @@ import { Container } from '@n8n/di'; import fs from 'fs'; import { mock } from 'jest-mock-extended'; +import type { UserManagementConfig } from '../src/configs/user-management.config'; import { GlobalConfig } from '../src/index'; jest.mock('fs'); @@ -101,6 +102,9 @@ describe('GlobalConfig', () => { }, }, userManagement: { + jwtSecret: '', + jwtSessionDurationHours: 168, + jwtRefreshTimeoutHours: 0, emails: { mode: 'smtp', smtp: { @@ -124,7 +128,7 @@ describe('GlobalConfig', () => { 'project-shared': '', }, }, - }, + } as UserManagementConfig, eventBus: { checkUnsentInterval: 0, crashRecoveryMode: 'extensive', diff --git a/packages/cli/src/auth/__tests__/auth.service.test.ts b/packages/cli/src/auth/__tests__/auth.service.test.ts index 8a2b9c93ab..44bd29fdef 100644 --- a/packages/cli/src/auth/__tests__/auth.service.test.ts +++ b/packages/cli/src/auth/__tests__/auth.service.test.ts @@ -11,15 +11,12 @@ import { mock } from 'jest-mock-extended'; import jwt from 'jsonwebtoken'; import { AuthService } from '@/auth/auth.service'; -import config from '@/config'; import { AUTH_COOKIE_NAME } from '@/constants'; import type { MfaService } from '@/mfa/mfa.service'; import { JwtService } from '@/services/jwt.service'; import type { UrlService } from '@/services/url.service'; describe('AuthService', () => { - config.set('userManagement.jwtSecret', 'random-secret'); - const browserId = 'test-browser-id'; const userData = { id: '123', @@ -29,8 +26,11 @@ describe('AuthService', () => { mfaEnabled: false, }; const user = mock(userData); - const globalConfig = mock({ auth: { cookie: { secure: true, samesite: 'lax' } } }); - const jwtService = new JwtService(mock()); + const globalConfig = mock({ + auth: { cookie: { secure: true, samesite: 'lax' } }, + userManagement: { jwtSecret: 'random-secret' }, + }); + const jwtService = new JwtService(mock(), globalConfig); const urlService = mock(); const userRepository = mock(); const invalidAuthTokenRepository = mock(); @@ -58,8 +58,8 @@ describe('AuthService', () => { beforeEach(() => { jest.resetAllMocks(); jest.setSystemTime(now); - config.set('userManagement.jwtSessionDurationHours', 168); - config.set('userManagement.jwtRefreshTimeoutHours', 0); + globalConfig.userManagement.jwtSessionDurationHours = 168; + globalConfig.userManagement.jwtRefreshTimeoutHours = 0; globalConfig.auth.cookie = { secure: true, samesite: 'lax' }; }); @@ -253,7 +253,7 @@ describe('AuthService', () => { const testDurationSeconds = testDurationHours * Time.hours.toSeconds; it('should apply it to tokens', () => { - config.set('userManagement.jwtSessionDurationHours', testDurationHours); + globalConfig.userManagement.jwtSessionDurationHours = testDurationHours; const token = authService.issueJWT(user, false, browserId); const decodedToken = jwtService.verify(token); @@ -376,7 +376,7 @@ describe('AuthService', () => { }); it('should not refresh the cookie if jwtRefreshTimeoutHours is set to -1', async () => { - config.set('userManagement.jwtRefreshTimeoutHours', -1); + globalConfig.userManagement.jwtRefreshTimeoutHours = -1; userRepository.findOne.mockResolvedValue(user); expect(await authService.resolveJwt(validToken, req, res)).toEqual([ diff --git a/packages/cli/src/auth/auth.service.ts b/packages/cli/src/auth/auth.service.ts index 50000c9be0..873bc0ce6e 100644 --- a/packages/cli/src/auth/auth.service.ts +++ b/packages/cli/src/auth/auth.service.ts @@ -268,9 +268,9 @@ export class AuthService { return createHash('sha256').update(input).digest('base64'); } - /** How many **milliseconds** before expiration should a JWT be renewed */ + /** How many **milliseconds** before expiration should a JWT be renewed. */ get jwtRefreshTimeout() { - const { jwtRefreshTimeoutHours, jwtSessionDurationHours } = config.get('userManagement'); + const { jwtRefreshTimeoutHours, jwtSessionDurationHours } = this.globalConfig.userManagement; if (jwtRefreshTimeoutHours === 0) { return Math.floor(jwtSessionDurationHours * 0.25 * Time.hours.toMilliseconds); } else { @@ -278,8 +278,8 @@ export class AuthService { } } - /** How many **seconds** is an issued JWT valid for */ + /** How many **seconds** is an issued JWT valid for. */ get jwtExpiration() { - return config.get('userManagement.jwtSessionDurationHours') * Time.hours.toSeconds; + return this.globalConfig.userManagement.jwtSessionDurationHours * Time.hours.toSeconds; } } diff --git a/packages/cli/src/config/__tests__/index.test.ts b/packages/cli/src/config/__tests__/index.test.ts deleted file mode 100644 index d49a4e0a3c..0000000000 --- a/packages/cli/src/config/__tests__/index.test.ts +++ /dev/null @@ -1,9 +0,0 @@ -describe('userManagement.jwtRefreshTimeoutHours', () => { - it("resets jwtRefreshTimeoutHours to 0 if it's greater than or equal to jwtSessionDurationHours", async () => { - process.env.N8N_USER_MANAGEMENT_JWT_DURATION_HOURS = '1'; - process.env.N8N_USER_MANAGEMENT_JWT_REFRESH_TIMEOUT_HOURS = '1'; - const { default: config } = await import('@/config'); - - expect(config.getEnv('userManagement.jwtRefreshTimeoutHours')).toBe(0); - }); -}); diff --git a/packages/cli/src/config/index.ts b/packages/cli/src/config/index.ts index b698b1b448..0586aa0b13 100644 --- a/packages/cli/src/config/index.ts +++ b/packages/cli/src/config/index.ts @@ -93,20 +93,6 @@ if (!inE2ETests && !inTest) { }); } -// Validate Configuration -config.validate({ - allowed: 'strict', -}); -const userManagement = config.get('userManagement'); -if (userManagement.jwtRefreshTimeoutHours >= userManagement.jwtSessionDurationHours) { - if (!inTest) - logger.warn( - 'N8N_USER_MANAGEMENT_JWT_REFRESH_TIMEOUT_HOURS needs to smaller than N8N_USER_MANAGEMENT_JWT_DURATION_HOURS. Setting N8N_USER_MANAGEMENT_JWT_REFRESH_TIMEOUT_HOURS to 0 for now.', - ); - - config.set('userManagement.jwtRefreshTimeoutHours', 0); -} - setGlobalState({ defaultTimezone: globalConfig.generic.timezone, }); diff --git a/packages/cli/src/config/schema.ts b/packages/cli/src/config/schema.ts index c20dad1038..9ba5cb9387 100644 --- a/packages/cli/src/config/schema.ts +++ b/packages/cli/src/config/schema.ts @@ -103,25 +103,6 @@ export const schema = { }, userManagement: { - jwtSecret: { - doc: 'Set a specific JWT secret (optional - n8n can generate one)', // Generated @ start.ts - format: String, - default: '', - env: 'N8N_USER_MANAGEMENT_JWT_SECRET', - }, - jwtSessionDurationHours: { - doc: 'Set a specific expiration date for the JWTs in hours.', - format: Number, - default: 168, - env: 'N8N_USER_MANAGEMENT_JWT_DURATION_HOURS', - }, - jwtRefreshTimeoutHours: { - doc: 'How long before the JWT expires to automatically refresh it. 0 means 25% of N8N_USER_MANAGEMENT_JWT_DURATION_HOURS. -1 means it will never refresh, which forces users to login again after the defined period in N8N_USER_MANAGEMENT_JWT_DURATION_HOURS.', - format: Number, - default: 0, - env: 'N8N_USER_MANAGEMENT_JWT_REFRESH_TIMEOUT_HOURS', - }, - /** * @important Do not remove until after cloud hooks are updated to stop using convict config. */ diff --git a/packages/cli/src/services/__tests__/jwt.service.test.ts b/packages/cli/src/services/__tests__/jwt.service.test.ts index 6e6e744fb9..196cfdb331 100644 --- a/packages/cli/src/services/__tests__/jwt.service.test.ts +++ b/packages/cli/src/services/__tests__/jwt.service.test.ts @@ -1,8 +1,8 @@ +import type { GlobalConfig } from '@n8n/config'; import { mock } from 'jest-mock-extended'; import jwt from 'jsonwebtoken'; import type { InstanceSettings } from 'n8n-core'; -import config from '@/config'; import { JwtService } from '@/services/jwt.service'; describe('JwtService', () => { @@ -13,21 +13,29 @@ describe('JwtService', () => { 'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOjEsImlhdCI6MTY5OTk4NDMxM30.xNZOAmcidW5ovEF_mwIOzCWkJ70FEO6MFNLK2QRDOeQ'; const instanceSettings = mock({ encryptionKey: 'test-key' }); + let globalConfig: GlobalConfig; beforeEach(() => { jest.clearAllMocks(); + globalConfig = mock({ + userManagement: { + jwtSecret: '', + jwtSessionDurationHours: 168, + jwtRefreshTimeoutHours: 0, + }, + }); }); describe('secret initialization', () => { it('should read the secret from config, when set', () => { - config.set('userManagement.jwtSecret', jwtSecret); - const jwtService = new JwtService(instanceSettings); + globalConfig.userManagement.jwtSecret = jwtSecret; + const jwtService = new JwtService(instanceSettings, globalConfig); expect(jwtService.jwtSecret).toEqual(jwtSecret); }); it('should derive the secret from encryption key when not set in config', () => { - config.set('userManagement.jwtSecret', ''); - const jwtService = new JwtService(instanceSettings); + globalConfig.userManagement.jwtSecret = ''; + const jwtService = new JwtService(instanceSettings, globalConfig); expect(jwtService.jwtSecret).toEqual( 'e9e2975005eddefbd31b2c04a0b0f2d9c37d9d718cf3676cddf76d65dec555cb', ); @@ -35,8 +43,7 @@ describe('JwtService', () => { }); describe('with a secret set', () => { - config.set('userManagement.jwtSecret', jwtSecret); - const jwtService = new JwtService(instanceSettings); + let jwtService: JwtService; beforeAll(() => { jest.useFakeTimers().setSystemTime(new Date(iat * 1000)); @@ -44,6 +51,11 @@ describe('JwtService', () => { afterAll(() => jest.useRealTimers()); + beforeEach(() => { + globalConfig.userManagement.jwtSecret = jwtSecret; + jwtService = new JwtService(instanceSettings, globalConfig); + }); + it('should sign', () => { const token = jwtService.sign(payload); expect(token).toEqual(signedToken); diff --git a/packages/cli/src/services/__tests__/public-api-key.service.test.ts b/packages/cli/src/services/__tests__/public-api-key.service.test.ts index 38978fce38..519b69ffc2 100644 --- a/packages/cli/src/services/__tests__/public-api-key.service.test.ts +++ b/packages/cli/src/services/__tests__/public-api-key.service.test.ts @@ -36,7 +36,7 @@ const securitySchema = mock({ name: 'X-N8N-API-KEY', }); -const jwtService = new JwtService(instanceSettings); +const jwtService = new JwtService(instanceSettings, mock()); let userRepository: UserRepository; let apiKeyRepository: ApiKeyRepository; diff --git a/packages/cli/src/services/jwt.service.ts b/packages/cli/src/services/jwt.service.ts index 0601309013..14e78a05b7 100644 --- a/packages/cli/src/services/jwt.service.ts +++ b/packages/cli/src/services/jwt.service.ts @@ -1,16 +1,15 @@ -import { Service } from '@n8n/di'; +import { GlobalConfig } from '@n8n/config'; +import { Container, Service } from '@n8n/di'; import { createHash } from 'crypto'; import jwt from 'jsonwebtoken'; import { InstanceSettings } from 'n8n-core'; -import config from '@/config'; - @Service() export class JwtService { - readonly jwtSecret = config.getEnv('userManagement.jwtSecret'); + jwtSecret: string = ''; - constructor({ encryptionKey }: InstanceSettings) { - this.jwtSecret = config.getEnv('userManagement.jwtSecret'); + constructor({ encryptionKey }: InstanceSettings, globalConfig: GlobalConfig) { + this.jwtSecret = globalConfig.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 @@ -20,7 +19,7 @@ export class JwtService { baseKey += encryptionKey[i]; } this.jwtSecret = createHash('sha256').update(baseKey).digest('hex'); - config.set('userManagement.jwtSecret', this.jwtSecret); + Container.get(GlobalConfig).userManagement.jwtSecret = this.jwtSecret; } } diff --git a/packages/cli/test/integration/shared/utils/test-server.ts b/packages/cli/test/integration/shared/utils/test-server.ts index c188119a1e..4e48570160 100644 --- a/packages/cli/test/integration/shared/utils/test-server.ts +++ b/packages/cli/test/integration/shared/utils/test-server.ts @@ -21,6 +21,7 @@ import { LicenseMocker } from '@test-integration/license'; import { PUBLIC_API_REST_PATH_SEGMENT, REST_PATH_SEGMENT } from '../constants'; import type { SetupProps, TestServer } from '../types'; +import { GlobalConfig } from '@n8n/config'; /** * Plugin to prefix a path segment into a request URL pathname. @@ -127,7 +128,7 @@ export const setupTestServer = ({ if (modules) await testModules.loadModules(modules); await testDb.init(); - config.set('userManagement.jwtSecret', 'My JWT secret'); + Container.get(GlobalConfig).userManagement.jwtSecret = 'My JWT secret'; config.set('userManagement.isInstanceOwnerSetUp', true); testServer.license.mock(Container.get(License));