From 07e67052568dcb292dbf63bee8912110931726bc Mon Sep 17 00:00:00 2001 From: Danny Martini Date: Mon, 22 Jan 2024 09:54:13 +0100 Subject: [PATCH] feat(core): Custom session timeout and refresh configuration (#8342) --- packages/cli/src/Interfaces.ts | 1 + packages/cli/src/auth/jwt.ts | 12 +- packages/cli/src/config/index.ts | 9 + packages/cli/src/config/schema.ts | 12 +- packages/cli/src/constants.ts | 23 +++ packages/cli/src/middlewares/auth.ts | 23 ++- packages/cli/test/unit/auth/jwt.test.ts | 61 +++++++ packages/cli/test/unit/config/index.test.ts | 9 + .../cli/test/unit/middleware/auth.test.ts | 162 ++++++++++++++++++ 9 files changed, 299 insertions(+), 13 deletions(-) create mode 100644 packages/cli/test/unit/auth/jwt.test.ts create mode 100644 packages/cli/test/unit/config/index.test.ts create mode 100644 packages/cli/test/unit/middleware/auth.test.ts diff --git a/packages/cli/src/Interfaces.ts b/packages/cli/src/Interfaces.ts index 88a0c2f610d7f..5dcf0272f92c2 100644 --- a/packages/cli/src/Interfaces.ts +++ b/packages/cli/src/Interfaces.ts @@ -667,6 +667,7 @@ export interface ILicensePostResponse extends ILicenseReadResponse { export interface JwtToken { token: string; + /** The amount of seconds after which the JWT will expire. **/ expiresIn: number; } diff --git a/packages/cli/src/auth/jwt.ts b/packages/cli/src/auth/jwt.ts index c79479008d113..cf2a415b75196 100644 --- a/packages/cli/src/auth/jwt.ts +++ b/packages/cli/src/auth/jwt.ts @@ -1,6 +1,6 @@ import type { Response } from 'express'; import { createHash } from 'crypto'; -import { AUTH_COOKIE_NAME, RESPONSE_ERROR_MESSAGES } from '@/constants'; +import { AUTH_COOKIE_NAME, RESPONSE_ERROR_MESSAGES, Time } from '@/constants'; import type { JwtPayload, JwtToken } from '@/Interfaces'; import type { User } from '@db/entities/User'; import config from '@/config'; @@ -14,7 +14,9 @@ import { ApplicationError } from 'n8n-workflow'; export function issueJWT(user: User): JwtToken { const { id, email, password } = user; - const expiresIn = 7 * 86400000; // 7 days + const expiresInHours = config.getEnv('userManagement.jwtSessionDurationHours'); + const expiresInSeconds = expiresInHours * Time.hours.toSeconds; + const isWithinUsersLimit = Container.get(License).isWithinUsersLimit(); const payload: JwtPayload = { @@ -37,12 +39,12 @@ export function issueJWT(user: User): JwtToken { } const signedToken = Container.get(JwtService).sign(payload, { - expiresIn: expiresIn / 1000 /* in seconds */, + expiresIn: expiresInSeconds, }); return { token: signedToken, - expiresIn, + expiresIn: expiresInSeconds, }; } @@ -86,7 +88,7 @@ export async function resolveJwt(token: string): Promise { export async function issueCookie(res: Response, user: User): Promise { const userData = issueJWT(user); res.cookie(AUTH_COOKIE_NAME, userData.token, { - maxAge: userData.expiresIn, + maxAge: userData.expiresIn * Time.seconds.toMilliseconds, httpOnly: true, sameSite: 'lax', }); diff --git a/packages/cli/src/config/index.ts b/packages/cli/src/config/index.ts index 5fa96e75aba82..7300b0dcf739f 100644 --- a/packages/cli/src/config/index.ts +++ b/packages/cli/src/config/index.ts @@ -62,9 +62,18 @@ if (!inE2ETests && !inTest) { }); } +// Validate Configuration config.validate({ allowed: 'strict', }); +const userManagement = config.get('userManagement'); +if (userManagement.jwtRefreshTimeoutHours >= userManagement.jwtSessionDurationHours) { + console.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: config.getEnv('generic.timezone'), diff --git a/packages/cli/src/config/schema.ts b/packages/cli/src/config/schema.ts index fca700fc7890d..8b77f36cb3d0d 100644 --- a/packages/cli/src/config/schema.ts +++ b/packages/cli/src/config/schema.ts @@ -762,11 +762,17 @@ export const schema = { default: '', env: 'N8N_USER_MANAGEMENT_JWT_SECRET', }, - jwtDuration: { - doc: 'Set a specific JWT secret (optional - n8n can generate one)', // Generated @ start.ts + jwtSessionDurationHours: { + doc: 'Set a specific expiration date for the JWTs in hours.', format: Number, default: 168, - env: 'N8N_USER_MANAGEMENT_JWT_DURATION', + 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', }, isInstanceOwnerSetUp: { // n8n loads this setting from DB on startup diff --git a/packages/cli/src/constants.ts b/packages/cli/src/constants.ts index 8603d2996330d..9e65e4c342bfa 100644 --- a/packages/cli/src/constants.ts +++ b/packages/cli/src/constants.ts @@ -103,6 +103,7 @@ export const UM_FIX_INSTRUCTION = /** * Units of time in milliseconds + * @deprecated Please use constants.Time instead. */ export const TIME = { SECOND: 1000, @@ -111,6 +112,28 @@ export const TIME = { DAY: 24 * 60 * 60 * 1000, } as const; +/** + * Convert time from any unit to any other unit + * + * Please amend conversions as necessary. + * Eventually this will superseed `TIME` above + */ +export const Time = { + seconds: { + toMilliseconds: 1000, + }, + minutes: { + toMilliseconds: 60 * 1000, + }, + hours: { + toMilliseconds: 60 * 60 * 1000, + toSeconds: 60 * 60, + }, + days: { + toSeconds: 24 * 60 * 60, + }, +}; + export const MIN_PASSWORD_CHAR_LENGTH = 8; export const MAX_PASSWORD_CHAR_LENGTH = 64; diff --git a/packages/cli/src/middlewares/auth.ts b/packages/cli/src/middlewares/auth.ts index 195b174b74115..12c1d78d655dc 100644 --- a/packages/cli/src/middlewares/auth.ts +++ b/packages/cli/src/middlewares/auth.ts @@ -11,6 +11,7 @@ import { issueCookie, resolveJwtContent } from '@/auth/jwt'; import { canSkipAuth } from '@/decorators/registerController'; import { Logger } from '@/Logger'; import { JwtService } from '@/services/jwt.service'; +import config from '@/config'; const jwtFromRequest = (req: Request) => { // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access @@ -41,17 +42,29 @@ const userManagementJwtAuth = (): RequestHandler => { /** * middleware to refresh cookie before it expires */ -const refreshExpiringCookie: RequestHandler = async (req: AuthenticatedRequest, res, next) => { +export const refreshExpiringCookie = (async (req: AuthenticatedRequest, res, next) => { + const jwtRefreshTimeoutHours = config.get('userManagement.jwtRefreshTimeoutHours'); + + let jwtRefreshTimeoutMilliSeconds: number; + + if (jwtRefreshTimeoutHours === 0) { + const jwtSessionDurationHours = config.get('userManagement.jwtSessionDurationHours'); + + jwtRefreshTimeoutMilliSeconds = Math.floor(jwtSessionDurationHours * 0.25 * 60 * 60 * 1000); + } else { + jwtRefreshTimeoutMilliSeconds = Math.floor(jwtRefreshTimeoutHours * 60 * 60 * 1000); + } + const cookieAuth = jwtFromRequest(req); - if (cookieAuth && req.user) { + + if (cookieAuth && req.user && jwtRefreshTimeoutHours !== -1) { const cookieContents = jwt.decode(cookieAuth) as JwtPayload & { exp: number }; - if (cookieContents.exp * 1000 - Date.now() < 259200000) { - // if cookie expires in < 3 days, renew it. + if (cookieContents.exp * 1000 - Date.now() < jwtRefreshTimeoutMilliSeconds) { await issueCookie(res, req.user); } } next(); -}; +}) satisfies RequestHandler; const passportMiddleware = passport.authenticate('jwt', { session: false }) as RequestHandler; diff --git a/packages/cli/test/unit/auth/jwt.test.ts b/packages/cli/test/unit/auth/jwt.test.ts new file mode 100644 index 0000000000000..dd9c65116ab77 --- /dev/null +++ b/packages/cli/test/unit/auth/jwt.test.ts @@ -0,0 +1,61 @@ +import { Container } from 'typedi'; +import { mock } from 'jest-mock-extended'; + +import config from '@/config'; +import { JwtService } from '@/services/jwt.service'; +import { License } from '@/License'; +import { Time } from '@/constants'; +import { issueJWT } from '@/auth/jwt'; + +import { mockInstance } from '../../shared/mocking'; + +import type { User } from '@db/entities/User'; + +mockInstance(License); + +describe('jwt.issueJWT', () => { + const jwtService = Container.get(JwtService); + + describe('when not setting userManagement.jwtSessionDuration', () => { + it('should default to expire in 7 days', () => { + const defaultInSeconds = 7 * Time.days.toSeconds; + const mockUser = mock({ password: 'passwordHash' }); + const { token, expiresIn } = issueJWT(mockUser); + + expect(expiresIn).toBe(defaultInSeconds); + const decodedToken = jwtService.verify(token); + if (decodedToken.exp === undefined || decodedToken.iat === undefined) { + fail('Expected exp and iat to be defined'); + } + + expect(decodedToken.exp - decodedToken.iat).toBe(defaultInSeconds); + }); + }); + + describe('when setting userManagement.jwtSessionDuration', () => { + const oldDuration = config.get('userManagement.jwtSessionDurationHours'); + const testDurationHours = 1; + const testDurationSeconds = testDurationHours * Time.hours.toSeconds; + + beforeEach(() => { + mockInstance(License); + config.set('userManagement.jwtSessionDurationHours', testDurationHours); + }); + + afterEach(() => { + config.set('userManagement.jwtSessionDuration', oldDuration); + }); + + it('should apply it to tokens', () => { + const mockUser = mock({ password: 'passwordHash' }); + const { token, expiresIn } = issueJWT(mockUser); + + expect(expiresIn).toBe(testDurationSeconds); + const decodedToken = jwtService.verify(token); + if (decodedToken.exp === undefined || decodedToken.iat === undefined) { + fail('Expected exp and iat to be defined on decodedToken'); + } + expect(decodedToken.exp - decodedToken.iat).toBe(testDurationSeconds); + }); + }); +}); diff --git a/packages/cli/test/unit/config/index.test.ts b/packages/cli/test/unit/config/index.test.ts new file mode 100644 index 0000000000000..d49a4e0a3c3ef --- /dev/null +++ b/packages/cli/test/unit/config/index.test.ts @@ -0,0 +1,9 @@ +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/test/unit/middleware/auth.test.ts b/packages/cli/test/unit/middleware/auth.test.ts new file mode 100644 index 0000000000000..dfd6e0e8c13e4 --- /dev/null +++ b/packages/cli/test/unit/middleware/auth.test.ts @@ -0,0 +1,162 @@ +import { mock } from 'jest-mock-extended'; + +import config from '@/config'; +import { AUTH_COOKIE_NAME, Time } from '@/constants'; +import { License } from '@/License'; +import { issueJWT } from '@/auth/jwt'; +import { refreshExpiringCookie } from '@/middlewares'; + +import { mockInstance } from '../../shared/mocking'; + +import type { AuthenticatedRequest } from '@/requests'; +import type { NextFunction, Response } from 'express'; +import type { User } from '@/databases/entities/User'; + +mockInstance(License); + +jest.useFakeTimers(); + +describe('refreshExpiringCookie', () => { + const oldDuration = config.getEnv('userManagement.jwtSessionDurationHours'); + const oldTimeout = config.getEnv('userManagement.jwtRefreshTimeoutHours'); + let mockUser: User; + + beforeEach(() => { + mockUser = mock({ password: 'passwordHash' }); + }); + + afterEach(() => { + config.set('userManagement.jwtSessionDuration', oldDuration); + config.set('userManagement.jwtRefreshTimeoutHours', oldTimeout); + }); + + it('does not do anything if the user is not authorized', async () => { + const req = mock(); + const res = mock({ cookie: jest.fn() }); + const next = jest.fn(); + + await refreshExpiringCookie(req, res, next); + + expect(next).toHaveBeenCalledTimes(1); + expect(res.cookie).not.toHaveBeenCalled(); + }); + + describe('with N8N_USER_MANAGEMENT_JWT_REFRESH_TIMEOUT_HOURS=-1', () => { + it('does not refresh the cookie, ever', async () => { + config.set('userManagement.jwtSessionDurationHours', 1); + config.set('userManagement.jwtRefreshTimeoutHours', -1); + const { token } = issueJWT(mockUser); + + jest.advanceTimersByTime(1000 * 60 * 55); /* 55 minutes */ + + const req = mock({ + cookies: { [AUTH_COOKIE_NAME]: token }, + user: mockUser, + }); + const res = mock({ cookie: jest.fn() }); + const next = jest.fn(); + await refreshExpiringCookie(req, res, next); + + expect(next).toHaveBeenCalledTimes(1); + expect(res.cookie).not.toHaveBeenCalled(); + }); + }); + + describe('with N8N_USER_MANAGEMENT_JWT_REFRESH_TIMEOUT_HOURS=0', () => { + let token: string; + let req: AuthenticatedRequest; + let res: Response; + let next: NextFunction; + + beforeEach(() => { + // ARRANGE + config.set('userManagement.jwtSessionDurationHours', 1); + config.set('userManagement.jwtRefreshTimeoutHours', 0); + token = issueJWT(mockUser).token; + + req = mock({ + cookies: { [AUTH_COOKIE_NAME]: token }, + user: mockUser, + }); + res = mock({ cookie: jest.fn() }); + next = jest.fn(); + }); + + it('does not refresh the cookie when more than 1/4th of time is left', async () => { + // ARRANGE + jest.advanceTimersByTime(44 * Time.minutes.toMilliseconds); /* 44 minutes */ + + // ACT + await refreshExpiringCookie(req, res, next); + + // ASSERT + expect(next).toHaveBeenCalledTimes(1); + expect(res.cookie).not.toHaveBeenCalled(); + }); + + it('refreshes the cookie when 1/4th of time is left', async () => { + // ARRANGE + jest.advanceTimersByTime(46 * Time.minutes.toMilliseconds); /* 46 minutes */ + + // ACT + await refreshExpiringCookie(req, res, next); + + // ASSERT + expect(next).toHaveBeenCalledTimes(1); + expect(res.cookie).toHaveBeenCalledTimes(1); + }); + }); + + describe('with N8N_USER_MANAGEMENT_JWT_REFRESH_TIMEOUT_HOURS=50', () => { + const jwtSessionDurationHours = 51; + let token: string; + let req: AuthenticatedRequest; + let res: Response; + let next: NextFunction; + + // ARRANGE + beforeEach(() => { + config.set('userManagement.jwtSessionDurationHours', jwtSessionDurationHours); + config.set('userManagement.jwtRefreshTimeoutHours', 50); + + token = issueJWT(mockUser).token; + req = mock({ + cookies: { [AUTH_COOKIE_NAME]: token }, + user: mockUser, + }); + res = mock({ cookie: jest.fn() }); + next = jest.fn(); + }); + + it('does not do anything if the cookie is still valid', async () => { + // ARRANGE + // cookie has 50.5 hours to live: 51 - 0.5 + jest.advanceTimersByTime(30 * Time.minutes.toMilliseconds); + + // ACT + await refreshExpiringCookie(req, res, next); + + // ASSERT + expect(next).toHaveBeenCalledTimes(1); + expect(res.cookie).not.toHaveBeenCalled(); + }); + + it('refreshes the cookie if it has less than 50 hours to live', async () => { + // ARRANGE + // cookie has 49.5 hours to live: 51 - 1.5 + jest.advanceTimersByTime(1.5 * Time.hours.toMilliseconds); + + // ACT + await refreshExpiringCookie(req, res, next); + + // ASSERT + expect(next).toHaveBeenCalledTimes(1); + expect(res.cookie).toHaveBeenCalledTimes(1); + expect(res.cookie).toHaveBeenCalledWith(AUTH_COOKIE_NAME, expect.any(String), { + httpOnly: true, + maxAge: jwtSessionDurationHours * Time.hours.toMilliseconds, + sameSite: 'lax', + }); + }); + }); +});