diff --git a/cypress/support/commands.ts b/cypress/support/commands.ts index 238ea0d2a2..a92dc2ce06 100644 --- a/cypress/support/commands.ts +++ b/cypress/support/commands.ts @@ -62,7 +62,11 @@ Cypress.Commands.add('signinAsOwner', () => { }); Cypress.Commands.add('signout', () => { - cy.request('POST', `${BACKEND_BASE_URL}/rest/logout`); + cy.request({ + method: 'POST', + url: `${BACKEND_BASE_URL}/rest/logout`, + headers: { 'browser-id': localStorage.getItem('n8n-browserId') } + }); cy.getCookie(N8N_AUTH_COOKIE).should('not.exist'); }); diff --git a/packages/cli/src/Server.ts b/packages/cli/src/Server.ts index b4503582d8..7aa8a663c3 100644 --- a/packages/cli/src/Server.ts +++ b/packages/cli/src/Server.ts @@ -33,7 +33,7 @@ import { TEMPLATES_DIR, } from '@/constants'; import { CredentialsController } from '@/credentials/credentials.controller'; -import type { CurlHelper } from '@/requests'; +import type { APIRequest, CurlHelper } from '@/requests'; import { registerController } from '@/decorators'; import { AuthController } from '@/controllers/auth.controller'; import { BinaryDataController } from '@/controllers/binaryData.controller'; @@ -235,6 +235,13 @@ export class Server extends AbstractServer { frontendService.settings.publicApi.latestVersion = apiLatestVersion; } } + + // Extract BrowserId from headers + this.app.use((req: APIRequest, _, next) => { + req.browserId = req.headers['browser-id'] as string; + next(); + }); + // Parse cookies for easier access this.app.use(cookieParser()); diff --git a/packages/cli/src/auth/auth.service.ts b/packages/cli/src/auth/auth.service.ts index 22fa6f3ffe..d7227fd063 100644 --- a/packages/cli/src/auth/auth.service.ts +++ b/packages/cli/src/auth/auth.service.ts @@ -20,6 +20,8 @@ interface AuthJwtPayload { id: string; /** This hash is derived from email and bcrypt of password */ hash: string; + /** This is a client generated unique string to prevent session hijacking */ + browserId?: string; } interface IssuedJWT extends AuthJwtPayload { @@ -31,6 +33,8 @@ interface PasswordResetToken { hash: string; } +const pushEndpoint = `/${config.get('endpoints.rest')}/push`; + @Service() export class AuthService { constructor( @@ -48,7 +52,7 @@ export class AuthService { const token = req.cookies[AUTH_COOKIE_NAME]; if (token) { try { - req.user = await this.resolveJwt(token, res); + req.user = await this.resolveJwt(token, req, res); } catch (error) { if (error instanceof JsonWebTokenError || error instanceof AuthError) { this.clearCookie(res); @@ -66,7 +70,8 @@ export class AuthService { res.clearCookie(AUTH_COOKIE_NAME); } - issueCookie(res: Response, user: User) { + issueCookie(res: Response, user: User, browserId?: string) { + // TODO: move this check to the login endpoint in AuthController // If the instance has exceeded its user quota, prevent non-owners from logging in const isWithinUsersLimit = this.license.isWithinUsersLimit(); if ( @@ -77,7 +82,7 @@ export class AuthService { throw new UnauthorizedError(RESPONSE_ERROR_MESSAGES.USERS_QUOTA_REACHED); } - const token = this.issueJWT(user); + const token = this.issueJWT(user, browserId); res.cookie(AUTH_COOKIE_NAME, token, { maxAge: this.jwtExpiration * Time.seconds.toMilliseconds, httpOnly: true, @@ -86,17 +91,18 @@ export class AuthService { }); } - issueJWT(user: User) { + issueJWT(user: User, browserId?: string) { const payload: AuthJwtPayload = { id: user.id, hash: this.createJWTHash(user), + browserId: browserId && this.hash(browserId), }; return this.jwtService.sign(payload, { expiresIn: this.jwtExpiration, }); } - async resolveJwt(token: string, res: Response): Promise { + async resolveJwt(token: string, req: AuthenticatedRequest, res: Response): Promise { const jwtPayload: IssuedJWT = this.jwtService.verify(token, { algorithms: ['HS256'], }); @@ -112,14 +118,20 @@ export class AuthService { // or, If the user has been deactivated (i.e. LDAP users) user.disabled || // or, If the email or password has been updated - jwtPayload.hash !== this.createJWTHash(user) + jwtPayload.hash !== this.createJWTHash(user) || + // If the token was issued for another browser session + // NOTE: we need to exclude push endpoint from this check because we can't send custom header on websocket requests + // TODO: Implement a custom handshake for push, to avoid having to send any data on querystring or headers + (req.baseUrl !== pushEndpoint && + jwtPayload.browserId && + (!req.browserId || jwtPayload.browserId !== this.hash(req.browserId))) ) { throw new AuthError('Unauthorized'); } if (jwtPayload.exp * 1000 - Date.now() < this.jwtRefreshTimeout) { this.logger.debug('JWT about to expire. Will be refreshed'); - this.issueCookie(res, user); + this.issueCookie(res, user, jwtPayload.browserId); } return user; @@ -175,10 +187,11 @@ export class AuthService { } createJWTHash({ email, password }: User) { - const hash = createHash('sha256') - .update(email + ':' + password) - .digest('base64'); - return hash.substring(0, 10); + return this.hash(email + ':' + password).substring(0, 10); + } + + private hash(input: string) { + return createHash('sha256').update(input).digest('base64'); } /** How many **milliseconds** before expiration should a JWT be renewed */ diff --git a/packages/cli/src/controllers/auth.controller.ts b/packages/cli/src/controllers/auth.controller.ts index d8c7786b64..456937bbaa 100644 --- a/packages/cli/src/controllers/auth.controller.ts +++ b/packages/cli/src/controllers/auth.controller.ts @@ -94,7 +94,7 @@ export class AuthController { } } - this.authService.issueCookie(res, user); + this.authService.issueCookie(res, user, req.browserId); void this.internalHooks.onUserLoginSuccess({ user, authenticationMethod: usedAuthenticationMethod, diff --git a/packages/cli/src/controllers/invitation.controller.ts b/packages/cli/src/controllers/invitation.controller.ts index 39a13792cd..e1ca3dcc18 100644 --- a/packages/cli/src/controllers/invitation.controller.ts +++ b/packages/cli/src/controllers/invitation.controller.ts @@ -164,7 +164,7 @@ export class InvitationController { const updatedUser = await this.userRepository.save(invitee, { transaction: false }); - this.authService.issueCookie(res, updatedUser); + this.authService.issueCookie(res, updatedUser, req.browserId); void this.internalHooks.onUserSignup(updatedUser, { user_type: 'email', diff --git a/packages/cli/src/controllers/me.controller.ts b/packages/cli/src/controllers/me.controller.ts index ab09935225..0541aba72d 100644 --- a/packages/cli/src/controllers/me.controller.ts +++ b/packages/cli/src/controllers/me.controller.ts @@ -85,7 +85,7 @@ export class MeController { this.logger.info('User updated successfully', { userId }); - this.authService.issueCookie(res, user); + this.authService.issueCookie(res, user, req.browserId); const updatedKeys = Object.keys(payload); void this.internalHooks.onUserUpdate({ @@ -138,7 +138,7 @@ export class MeController { const updatedUser = await this.userRepository.save(user, { transaction: false }); this.logger.info('Password updated successfully', { userId: user.id }); - this.authService.issueCookie(res, updatedUser); + this.authService.issueCookie(res, updatedUser, req.browserId); void this.internalHooks.onUserUpdate({ user: updatedUser, diff --git a/packages/cli/src/controllers/owner.controller.ts b/packages/cli/src/controllers/owner.controller.ts index d0b910f780..6077026c90 100644 --- a/packages/cli/src/controllers/owner.controller.ts +++ b/packages/cli/src/controllers/owner.controller.ts @@ -83,7 +83,7 @@ export class OwnerController { this.logger.debug('Setting isInstanceOwnerSetUp updated successfully'); - this.authService.issueCookie(res, owner); + this.authService.issueCookie(res, owner, req.browserId); void this.internalHooks.onInstanceOwnerSetup({ user_id: owner.id }); diff --git a/packages/cli/src/controllers/passwordReset.controller.ts b/packages/cli/src/controllers/passwordReset.controller.ts index fb6261fbf5..643a40fa54 100644 --- a/packages/cli/src/controllers/passwordReset.controller.ts +++ b/packages/cli/src/controllers/passwordReset.controller.ts @@ -208,7 +208,7 @@ export class PasswordResetController { this.logger.info('User password updated successfully', { userId: user.id }); - this.authService.issueCookie(res, user); + this.authService.issueCookie(res, user, req.browserId); void this.internalHooks.onUserUpdate({ user, diff --git a/packages/cli/src/middlewares/cors.ts b/packages/cli/src/middlewares/cors.ts index 1b1379046b..d22b5a14f5 100644 --- a/packages/cli/src/middlewares/cors.ts +++ b/packages/cli/src/middlewares/cors.ts @@ -8,7 +8,7 @@ export const corsMiddleware: RequestHandler = (req, res, next) => { res.header('Access-Control-Allow-Methods', 'GET, POST, OPTIONS, PUT, PATCH, DELETE'); res.header( 'Access-Control-Allow-Headers', - 'Origin, X-Requested-With, Content-Type, Accept, push-ref', + 'Origin, X-Requested-With, Content-Type, Accept, push-ref, browser-id', ); } diff --git a/packages/cli/src/requests.ts b/packages/cli/src/requests.ts index 5946b07117..5cb5b217ca 100644 --- a/packages/cli/src/requests.ts +++ b/packages/cli/src/requests.ts @@ -50,22 +50,30 @@ export class UserRoleChangePayload { newRoleName: AssignableRole; } +export type APIRequest< + RouteParams = {}, + ResponseBody = {}, + RequestBody = {}, + RequestQuery = {}, +> = express.Request & { + browserId?: string; +}; + export type AuthlessRequest< RouteParams = {}, ResponseBody = {}, RequestBody = {}, RequestQuery = {}, -> = express.Request; +> = APIRequest & { + user: never; +}; export type AuthenticatedRequest< RouteParams = {}, ResponseBody = {}, RequestBody = {}, RequestQuery = {}, -> = Omit< - express.Request, - 'user' | 'cookies' -> & { +> = Omit, 'user' | 'cookies'> & { user: User; cookies: Record; }; diff --git a/packages/cli/src/sso/saml/routes/saml.controller.ee.ts b/packages/cli/src/sso/saml/routes/saml.controller.ee.ts index 597fdfb93c..632a4db90b 100644 --- a/packages/cli/src/sso/saml/routes/saml.controller.ee.ts +++ b/packages/cli/src/sso/saml/routes/saml.controller.ee.ts @@ -138,7 +138,7 @@ export class SamlController { }); // Only sign in user if SAML is enabled, otherwise treat as test connection if (isSamlLicensedAndEnabled()) { - this.authService.issueCookie(res, loginResult.authenticatedUser); + this.authService.issueCookie(res, loginResult.authenticatedUser, req.browserId); if (loginResult.onboardingRequired) { return res.redirect(this.urlService.getInstanceBaseUrl() + SamlUrls.samlOnboarding); } else { diff --git a/packages/cli/test/unit/auth/auth.service.test.ts b/packages/cli/test/unit/auth/auth.service.test.ts index 9c9e41e061..e7106c51d7 100644 --- a/packages/cli/test/unit/auth/auth.service.test.ts +++ b/packages/cli/test/unit/auth/auth.service.test.ts @@ -1,6 +1,6 @@ import jwt from 'jsonwebtoken'; import { mock } from 'jest-mock-extended'; -import { type NextFunction, type Response } from 'express'; +import type { NextFunction, Response } from 'express'; import { AuthService } from '@/auth/auth.service'; import config from '@/config'; @@ -14,6 +14,7 @@ import type { AuthenticatedRequest } from '@/requests'; describe('AuthService', () => { config.set('userManagement.jwtSecret', 'random-secret'); + const browserId = 'test-browser-id'; const userData = { id: '123', email: 'test@example.com', @@ -21,17 +22,18 @@ describe('AuthService', () => { disabled: false, mfaEnabled: false, }; - const validToken = - 'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpZCI6IjEyMyIsImhhc2giOiJtSkFZeDRXYjdrIiwiaWF0IjoxNzA2NzUwNjI1LCJleHAiOjE3MDczNTU0MjV9.JwY3doH0YrxHdX4nTOlTN4-QMaXsAu5OFOaFcIHSHBI'; - const user = mock(userData); const jwtService = new JwtService(mock()); const urlService = mock(); const userRepository = mock(); const authService = new AuthService(mock(), mock(), jwtService, urlService, userRepository); - jest.useFakeTimers(); const now = new Date('2024-02-01T01:23:45.678Z'); + jest.useFakeTimers({ now }); + + const validToken = + 'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpZCI6IjEyMyIsImhhc2giOiJtSkFZeDRXYjdrIiwiYnJvd3NlcklkIjoiOFpDVXE1YU1uSFhnMFZvcURLcm9hMHNaZ0NwdWlPQ1AzLzB2UmZKUXU0MD0iLCJpYXQiOjE3MDY3NTA2MjUsImV4cCI6MTcwNzM1NTQyNX0.YE-ZGGIQRNQ4DzUe9rjXvOOFFN9ufU34WibsCxAsc4o'; // Generated using `authService.issueJWT(user, browserId)` + beforeEach(() => { jest.clearAllMocks(); jest.setSystemTime(now); @@ -54,7 +56,11 @@ describe('AuthService', () => { }); describe('authMiddleware', () => { - const req = mock({ cookies: {}, user: undefined }); + const req = mock({ + cookies: {}, + user: undefined, + browserId, + }); const res = mock(); const next = jest.fn() as NextFunction; @@ -99,7 +105,7 @@ describe('AuthService', () => { describe('when not setting userManagement.jwtSessionDuration', () => { it('should default to expire in 7 days', () => { const defaultInSeconds = 7 * Time.days.toSeconds; - const token = authService.issueJWT(user); + const token = authService.issueJWT(user, browserId); expect(authService.jwtExpiration).toBe(defaultInSeconds); const decodedToken = jwtService.verify(token); @@ -117,7 +123,7 @@ describe('AuthService', () => { it('should apply it to tokens', () => { config.set('userManagement.jwtSessionDurationHours', testDurationHours); - const token = authService.issueJWT(user); + const token = authService.issueJWT(user, browserId); const decodedToken = jwtService.verify(token); if (decodedToken.exp === undefined || decodedToken.iat === undefined) { @@ -129,24 +135,40 @@ describe('AuthService', () => { }); describe('resolveJwt', () => { + const req = mock({ + cookies: {}, + user: undefined, + browserId, + }); const res = mock(); it('should throw on invalid tokens', async () => { - await expect(authService.resolveJwt('random-string', res)).rejects.toThrow('jwt malformed'); + await expect(authService.resolveJwt('random-string', req, res)).rejects.toThrow( + 'jwt malformed', + ); expect(res.cookie).not.toHaveBeenCalled(); }); it('should throw on expired tokens', async () => { jest.advanceTimersByTime(365 * Time.days.toMilliseconds); - await expect(authService.resolveJwt(validToken, res)).rejects.toThrow('jwt expired'); + await expect(authService.resolveJwt(validToken, req, res)).rejects.toThrow('jwt expired'); expect(res.cookie).not.toHaveBeenCalled(); }); it('should throw on tampered tokens', async () => { const [header, payload, signature] = validToken.split('.'); const tamperedToken = [header, payload, signature + '123'].join('.'); - await expect(authService.resolveJwt(tamperedToken, res)).rejects.toThrow('invalid signature'); + await expect(authService.resolveJwt(tamperedToken, req, res)).rejects.toThrow( + 'invalid signature', + ); + expect(res.cookie).not.toHaveBeenCalled(); + }); + + it('should throw on hijacked tokens', async () => { + userRepository.findOne.mockResolvedValue(user); + const req = mock({ browserId: 'another-browser' }); + await expect(authService.resolveJwt(validToken, req, res)).rejects.toThrow('Unauthorized'); expect(res.cookie).not.toHaveBeenCalled(); }); @@ -163,17 +185,17 @@ describe('AuthService', () => { ], ])('should throw if %s', async (_, data) => { userRepository.findOne.mockResolvedValueOnce(data && mock(data)); - await expect(authService.resolveJwt(validToken, res)).rejects.toThrow('Unauthorized'); + await expect(authService.resolveJwt(validToken, req, res)).rejects.toThrow('Unauthorized'); expect(res.cookie).not.toHaveBeenCalled(); }); it('should refresh the cookie before it expires', async () => { userRepository.findOne.mockResolvedValue(user); - expect(await authService.resolveJwt(validToken, res)).toEqual(user); + expect(await authService.resolveJwt(validToken, req, res)).toEqual(user); expect(res.cookie).not.toHaveBeenCalled(); jest.advanceTimersByTime(6 * Time.days.toMilliseconds); // 6 Days - expect(await authService.resolveJwt(validToken, res)).toEqual(user); + expect(await authService.resolveJwt(validToken, req, res)).toEqual(user); expect(res.cookie).toHaveBeenCalledWith('n8n-auth', expect.any(String), { httpOnly: true, maxAge: 604800000, @@ -184,15 +206,15 @@ describe('AuthService', () => { it('should refresh the cookie only if less than 1/4th of time is left', async () => { userRepository.findOne.mockResolvedValue(user); - expect(await authService.resolveJwt(validToken, res)).toEqual(user); + expect(await authService.resolveJwt(validToken, req, res)).toEqual(user); expect(res.cookie).not.toHaveBeenCalled(); jest.advanceTimersByTime(5 * Time.days.toMilliseconds); - expect(await authService.resolveJwt(validToken, res)).toEqual(user); + expect(await authService.resolveJwt(validToken, req, res)).toEqual(user); expect(res.cookie).not.toHaveBeenCalled(); jest.advanceTimersByTime(1 * Time.days.toMilliseconds); - expect(await authService.resolveJwt(validToken, res)).toEqual(user); + expect(await authService.resolveJwt(validToken, req, res)).toEqual(user); expect(res.cookie).toHaveBeenCalled(); }); @@ -200,11 +222,11 @@ describe('AuthService', () => { config.set('userManagement.jwtRefreshTimeoutHours', -1); userRepository.findOne.mockResolvedValue(user); - expect(await authService.resolveJwt(validToken, res)).toEqual(user); + expect(await authService.resolveJwt(validToken, req, res)).toEqual(user); expect(res.cookie).not.toHaveBeenCalled(); jest.advanceTimersByTime(6 * Time.days.toMilliseconds); // 6 Days - expect(await authService.resolveJwt(validToken, res)).toEqual(user); + expect(await authService.resolveJwt(validToken, req, res)).toEqual(user); expect(res.cookie).not.toHaveBeenCalled(); }); }); diff --git a/packages/cli/test/unit/controllers/me.controller.test.ts b/packages/cli/test/unit/controllers/me.controller.test.ts index f0f9b84586..1d5ff3f113 100644 --- a/packages/cli/test/unit/controllers/me.controller.test.ts +++ b/packages/cli/test/unit/controllers/me.controller.test.ts @@ -16,6 +16,8 @@ import { UserRepository } from '@/databases/repositories/user.repository'; import { badPasswords } from '../shared/testData'; import { mockInstance } from '../../shared/mocking'; +const browserId = 'test-browser-id'; + describe('MeController', () => { const externalHooks = mockInstance(ExternalHooks); const internalHooks = mockInstance(InternalHooks); @@ -47,7 +49,7 @@ describe('MeController', () => { role: 'global:owner', }); const reqBody = { email: 'valid@email.com', firstName: 'John', lastName: 'Potato' }; - const req = mock({ user, body: reqBody }); + const req = mock({ user, body: reqBody, browserId }); const res = mock(); userRepository.findOneOrFail.mockResolvedValue(user); jest.spyOn(jwt, 'sign').mockImplementation(() => 'signed-token'); @@ -88,7 +90,7 @@ describe('MeController', () => { role: 'global:owner', }); const reqBody = { email: 'valid@email.com', firstName: 'John', lastName: 'Potato' }; - const req = mock({ user, body: reqBody }); + const req = mock({ user, body: reqBody, browserId }); const res = mock(); userRepository.findOneOrFail.mockResolvedValue(user); jest.spyOn(jwt, 'sign').mockImplementation(() => 'signed-token'); @@ -160,6 +162,7 @@ describe('MeController', () => { const req = mock({ user: mock({ password: passwordHash }), body: { currentPassword: 'old_password', newPassword }, + browserId, }); await expect(controller.updatePassword(req, mock())).rejects.toThrowError( new BadRequestError(errorMessage), @@ -172,6 +175,7 @@ describe('MeController', () => { const req = mock({ user: mock({ password: passwordHash }), body: { currentPassword: 'old_password', newPassword: 'NewPassword123' }, + browserId, }); const res = mock(); userRepository.save.calledWith(req.user).mockResolvedValue(req.user); diff --git a/packages/cli/test/unit/controllers/owner.controller.test.ts b/packages/cli/test/unit/controllers/owner.controller.test.ts index 50917a2107..0057c23708 100644 --- a/packages/cli/test/unit/controllers/owner.controller.test.ts +++ b/packages/cli/test/unit/controllers/owner.controller.test.ts @@ -82,6 +82,7 @@ describe('OwnerController', () => { role: 'global:owner', authIdentities: [], }); + const browserId = 'test-browser-id'; const req = mock({ body: { email: 'valid@email.com', @@ -90,6 +91,7 @@ describe('OwnerController', () => { lastName: 'Doe', }, user, + browserId, }); const res = mock(); configGetSpy.mockReturnValue(false); @@ -103,7 +105,7 @@ describe('OwnerController', () => { where: { role: 'global:owner' }, }); expect(userRepository.save).toHaveBeenCalledWith(user, { transaction: false }); - expect(authService.issueCookie).toHaveBeenCalledWith(res, user); + expect(authService.issueCookie).toHaveBeenCalledWith(res, user, browserId); }); }); }); diff --git a/packages/editor-ui/src/utils/apiUtils.ts b/packages/editor-ui/src/utils/apiUtils.ts index e6ce5d734f..c82219ba0e 100644 --- a/packages/editor-ui/src/utils/apiUtils.ts +++ b/packages/editor-ui/src/utils/apiUtils.ts @@ -1,9 +1,16 @@ -import type { AxiosRequestConfig, Method } from 'axios'; +import type { AxiosRequestConfig, Method, RawAxiosRequestHeaders } from 'axios'; import axios from 'axios'; import type { IDataObject } from 'n8n-workflow'; import type { IExecutionFlattedResponse, IExecutionResponse, IRestApiContext } from '@/Interface'; import { parse } from 'flatted'; +const BROWSER_ID_STORAGE_KEY = 'n8n-browserId'; +let browserId = localStorage.getItem(BROWSER_ID_STORAGE_KEY); +if (!browserId && 'randomUUID' in crypto) { + browserId = crypto.randomUUID(); + localStorage.setItem(BROWSER_ID_STORAGE_KEY, browserId); +} + export const NO_NETWORK_ERROR_CODE = 999; export class ResponseError extends Error { @@ -62,7 +69,7 @@ export async function request(config: { method: Method; baseURL: string; endpoint: string; - headers?: IDataObject; + headers?: RawAxiosRequestHeaders; data?: IDataObject | IDataObject[]; withCredentials?: boolean; }) { @@ -121,11 +128,15 @@ export async function makeRestApiRequest( endpoint: string, data?: IDataObject | IDataObject[], ) { + const headers: RawAxiosRequestHeaders = { 'push-ref': context.pushRef }; + if (browserId) { + headers['browser-id'] = browserId; + } const response = await request({ method, baseURL: context.baseUrl, endpoint, - headers: { 'push-ref': context.pushRef }, + headers, data, }); @@ -137,7 +148,7 @@ export async function get( baseURL: string, endpoint: string, params?: IDataObject, - headers?: IDataObject, + headers?: RawAxiosRequestHeaders, ) { return await request({ method: 'GET', baseURL, endpoint, headers, data: params }); } @@ -146,7 +157,7 @@ export async function post( baseURL: string, endpoint: string, params?: IDataObject, - headers?: IDataObject, + headers?: RawAxiosRequestHeaders, ) { return await request({ method: 'POST', baseURL, endpoint, headers, data: params }); }