feat(core)!: Set the secure flag on issued cookies (#8812)

n8n-io · Mar 5, 2024 · 0818824 · 0818824
1 parent 2b0e14e
commit 0818824
Show file tree

Hide file tree

Showing 6 changed files with 44 additions and 12 deletions.
diff --git a/packages/cli/BREAKING-CHANGES.md b/packages/cli/BREAKING-CHANGES.md
@@ -2,6 +2,16 @@
 
 This list shows all the versions which include breaking changes and how to upgrade.
 
+## 1.32.0
+
+### What changed?
+
+N8n auth cookie has `Secure` flag set by default now.
+
+### When is action necessary?
+
+If you are running n8n without HTTP**S** on a domain other than `localhost`, you need to either setup HTTPS, or you can disable the secure flag by setting the env variable `N8N_SECURE_COOKIE` to `false`.
+
 ## 1.27.0
 
 ### What changed?

diff --git a/packages/cli/src/auth/auth.service.ts b/packages/cli/src/auth/auth.service.ts
@@ -82,6 +82,7 @@ export class AuthService {
 			maxAge: this.jwtExpiration * Time.seconds.toMilliseconds,
 			httpOnly: true,
 			sameSite: 'lax',
+			secure: config.getEnv('secure_cookie'),
 		});
 	}
 

diff --git a/packages/cli/src/config/index.ts b/packages/cli/src/config/index.ts
@@ -15,6 +15,7 @@ if (inE2ETests) {
 	process.env.N8N_LOG_LEVEL = 'silent';
 	process.env.N8N_PUBLIC_API_DISABLED = 'true';
 	process.env.SKIP_STATISTICS_EVENTS = 'true';
+	process.env.N8N_SECURE_COOKIE = 'false';
 } else {
 	dotenv.config();
 }

diff --git a/packages/cli/src/config/schema.ts b/packages/cli/src/config/schema.ts
@@ -538,6 +538,12 @@ export const schema = {
 		env: 'N8N_PROTOCOL',
 		doc: 'HTTP Protocol via which n8n can be reached',
 	},
+	secure_cookie: {
+		doc: 'This sets the `Secure` flag on n8n auth cookie',
+		format: Boolean,
+		default: true,
+		env: 'N8N_SECURE_COOKIE',
+	},
 	ssl_key: {
 		format: String,
 		default: '',

diff --git a/packages/cli/test/unit/auth/auth.service.test.ts b/packages/cli/test/unit/auth/auth.service.test.ts
@@ -90,6 +90,7 @@ describe('AuthService', () => {
 				httpOnly: true,
 				maxAge: 604800000,
 				sameSite: 'lax',
+				secure: false,
 			});
 		});
 	});
@@ -177,6 +178,7 @@ describe('AuthService', () => {
 				httpOnly: true,
 				maxAge: 604800000,
 				sameSite: 'lax',
+				secure: false,
 			});
 		});
 

diff --git a/packages/cli/test/unit/controllers/me.controller.test.ts b/packages/cli/test/unit/controllers/me.controller.test.ts
@@ -1,7 +1,7 @@
-import type { CookieOptions, Response } from 'express';
+import type { Response } from 'express';
 import { Container } from 'typedi';
 import jwt from 'jsonwebtoken';
-import { mock, anyObject, captor } from 'jest-mock-extended';
+import { mock, anyObject } from 'jest-mock-extended';
 import type { PublicUser } from '@/Interfaces';
 import type { User } from '@db/entities/User';
 import { MeController } from '@/controllers/me.controller';
@@ -11,10 +11,10 @@ import { UserService } from '@/services/user.service';
 import { ExternalHooks } from '@/ExternalHooks';
 import { InternalHooks } from '@/InternalHooks';
 import { License } from '@/License';
-import { badPasswords } from '../shared/testData';
-import { mockInstance } from '../../shared/mocking';
 import { BadRequestError } from '@/errors/response-errors/bad-request.error';
 import { UserRepository } from '@/databases/repositories/user.repository';
+import { badPasswords } from '../shared/testData';
+import { mockInstance } from '../../shared/mocking';
 
 describe('MeController', () => {
 	const externalHooks = mockInstance(ExternalHooks);
@@ -63,10 +63,16 @@ describe('MeController', () => {
 
 			expect(userService.update).toHaveBeenCalled();
 
-			const cookieOptions = captor<CookieOptions>();
-			expect(res.cookie).toHaveBeenCalledWith(AUTH_COOKIE_NAME, 'signed-token', cookieOptions);
-			expect(cookieOptions.value.httpOnly).toBe(true);
-			expect(cookieOptions.value.sameSite).toBe('lax');
+			expect(res.cookie).toHaveBeenCalledWith(
+				AUTH_COOKIE_NAME,
+				'signed-token',
+				expect.objectContaining({
+					maxAge: expect.any(Number),
+					httpOnly: true,
+					sameSite: 'lax',
+					secure: false,
+				}),
+			);
 
 			expect(externalHooks.run).toHaveBeenCalledWith('user.profile.update', [
 				user.email,
@@ -175,10 +181,16 @@ describe('MeController', () => {
 
 			expect(req.user.password).not.toBe(passwordHash);
 
-			const cookieOptions = captor<CookieOptions>();
-			expect(res.cookie).toHaveBeenCalledWith(AUTH_COOKIE_NAME, 'new-signed-token', cookieOptions);
-			expect(cookieOptions.value.httpOnly).toBe(true);
-			expect(cookieOptions.value.sameSite).toBe('lax');
+			expect(res.cookie).toHaveBeenCalledWith(
+				AUTH_COOKIE_NAME,
+				'new-signed-token',
+				expect.objectContaining({
+					maxAge: expect.any(Number),
+					httpOnly: true,
+					sameSite: 'lax',
+					secure: false,
+				}),
+			);
 
 			expect(externalHooks.run).toHaveBeenCalledWith('user.password.update', [
 				req.user.email,