feat(core): Prevent session hijacking

n8n-io · Apr 4, 2024 · 61840d4 · 61840d4
1 parent dff8f7a
commit 61840d4
Show file tree

Hide file tree

Showing 11 changed files with 53 additions and 21 deletions.
diff --git 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 {
@@ -48,7 +50,9 @@ export class AuthService {
 		const token = req.cookies[AUTH_COOKIE_NAME];
 		if (token) {
 			try {
-				req.user = await this.resolveJwt(token, res);
+				// TODO: set this on `skipAuth` endpoints as well
+				req.browserId = req.headers['browser-id'] as string;
+				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,
 		};
 		return this.jwtService.sign(payload, {
 			expiresIn: this.jwtExpiration,
 		});
 	}
 
-	async resolveJwt(token: string, res: Response): Promise<User> {
+	async resolveJwt(token: string, req: AuthenticatedRequest, res: Response): Promise<User> {
 		const jwtPayload: IssuedJWT = this.jwtService.verify(token, {
 			algorithms: ['HS256'],
 		});
@@ -112,14 +118,16 @@ 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
+			(jwtPayload.browserId && jwtPayload.browserId !== 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;

diff --git 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.headers['browser-id'] as string);
 			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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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/push/index.ts b/packages/cli/src/push/index.ts
@@ -117,6 +117,7 @@ export const setupPushHandler = (restEndpoint: string, app: Application) => {
 	app.use(
 		endpoint,
 		// eslint-disable-next-line @typescript-eslint/unbound-method
+		// TODO: handle the missing browserId on WebSocket requests
 		authService.authMiddleware,
 		(req: SSEPushRequest | WebSocketPushRequest, res: PushResponse) => push.handleRequest(req, res),
 	);

diff --git a/packages/cli/src/requests.ts b/packages/cli/src/requests.ts
@@ -56,7 +56,9 @@ export type AuthlessRequest<
 	ResponseBody = {},
 	RequestBody = {},
 	RequestQuery = {},
-> = express.Request<RouteParams, ResponseBody, RequestBody, RequestQuery>;
+> = express.Request<RouteParams, ResponseBody, RequestBody, RequestQuery> & {
+	browserId?: string;
+};
 
 export type AuthenticatedRequest<
 	RouteParams = {},
@@ -69,6 +71,7 @@ export type AuthenticatedRequest<
 > & {
 	user: User;
 	cookies: Record<string, string | undefined>;
+	browserId?: string;
 };
 
 // ----------------------------------

diff --git 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/editor-ui/src/utils/apiUtils.ts b/packages/editor-ui/src/utils/apiUtils.ts
@@ -1,9 +1,29 @@
-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 = sessionStorage.getItem(BROWSER_ID_STORAGE_KEY);
+if (!browserId) {
+	const encoder = new TextEncoder();
+	const data = encoder.encode(
+		JSON.stringify([
+			navigator.userAgent,
+			[screen.height, screen.width, screen.colorDepth].join('x'),
+			new Date().getTimezoneOffset(),
+			[...navigator.plugins].map((p) => p.name).join(','),
+		]),
+	);
+	crypto.subtle.digest('SHA-256', data).then((arrayBuffer) => {
+		sessionStorage.setItem(
+			BROWSER_ID_STORAGE_KEY,
+			btoa(String.fromCharCode(...new Uint8Array(arrayBuffer))),
+		);
+	});
+}
+
 export const NO_NETWORK_ERROR_CODE = 999;
 
 export class ResponseError extends Error {
@@ -62,7 +82,7 @@ export async function request(config: {
 	method: Method;
 	baseURL: string;
 	endpoint: string;
-	headers?: IDataObject;
+	headers?: RawAxiosRequestHeaders;
 	data?: IDataObject | IDataObject[];
 	withCredentials?: boolean;
 }) {
@@ -71,7 +91,7 @@ export async function request(config: {
 		method,
 		url: endpoint,
 		baseURL,
-		headers,
+		headers: { ...headers, 'Browser-Id': browserId },
 	};
 	if (
 		import.meta.env.NODE_ENV !== 'production' &&
@@ -137,7 +157,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 +166,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 });
 }