feat(core): Allow customizing auth cookie samesite attribute and CSP headers (#13855)

Co-authored-by: कारतोफ्फेलस्क्रिप्ट™ <aditya@netroy.in>
This commit is contained in:
Marcus
2025-03-19 12:55:58 +01:00
committed by GitHub
parent d3bc80c22b
commit 17fc5c148b
9 changed files with 111 additions and 29 deletions

View File

@@ -1,3 +1,4 @@
import type { GlobalConfig } from '@n8n/config';
import type { NextFunction, Response } from 'express';
import { mock } from 'jest-mock-extended';
import jwt from 'jsonwebtoken';
@@ -24,11 +25,13 @@ describe('AuthService', () => {
mfaEnabled: false,
};
const user = mock<User>(userData);
const globalConfig = mock<GlobalConfig>({ auth: { cookie: { secure: true, samesite: 'lax' } } });
const jwtService = new JwtService(mock());
const urlService = mock<UrlService>();
const userRepository = mock<UserRepository>();
const invalidAuthTokenRepository = mock<InvalidAuthTokenRepository>();
const authService = new AuthService(
globalConfig,
mock(),
mock(),
jwtService,
@@ -44,10 +47,11 @@ describe('AuthService', () => {
'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpZCI6IjEyMyIsImhhc2giOiJtSkFZeDRXYjdrIiwiYnJvd3NlcklkIjoiOFpDVXE1YU1uSFhnMFZvcURLcm9hMHNaZ0NwdWlPQ1AzLzB2UmZKUXU0MD0iLCJpYXQiOjE3MDY3NTA2MjUsImV4cCI6MTcwNzM1NTQyNX0.YE-ZGGIQRNQ4DzUe9rjXvOOFFN9ufU34WibsCxAsc4o'; // Generated using `authService.issueJWT(user, browserId)`
beforeEach(() => {
jest.clearAllMocks();
jest.resetAllMocks();
jest.setSystemTime(now);
config.set('userManagement.jwtSessionDurationHours', 168);
config.set('userManagement.jwtRefreshTimeoutHours', 0);
globalConfig.auth.cookie = { secure: true, samesite: 'lax' };
});
describe('createJWTHash', () => {
@@ -127,6 +131,33 @@ describe('AuthService', () => {
httpOnly: true,
maxAge: 604800000,
sameSite: 'lax',
secure: true,
});
});
});
describe('issueCookie', () => {
const res = mock<Response>();
it('should issue a cookie with the correct options', () => {
authService.issueCookie(res, user, browserId);
expect(res.cookie).toHaveBeenCalledWith('n8n-auth', validToken, {
httpOnly: true,
maxAge: 604800000,
sameSite: 'lax',
secure: true,
});
});
it('should allow changing cookie options', () => {
globalConfig.auth.cookie = { secure: false, samesite: 'none' };
authService.issueCookie(res, user, browserId);
expect(res.cookie).toHaveBeenCalledWith('n8n-auth', validToken, {
httpOnly: true,
maxAge: 604800000,
sameSite: 'none',
secure: false,
});
});
@@ -231,7 +262,7 @@ describe('AuthService', () => {
httpOnly: true,
maxAge: 604800000,
sameSite: 'lax',
secure: false,
secure: true,
});
const newToken = res.cookie.mock.calls[0].at(1);

View File

@@ -1,5 +1,5 @@
import { GlobalConfig } from '@n8n/config';
import { Container, Service } from '@n8n/di';
import { Service } from '@n8n/di';
import { createHash } from 'crypto';
import type { NextFunction, Response } from 'express';
import { JsonWebTokenError, TokenExpiredError } from 'jsonwebtoken';
@@ -35,24 +35,13 @@ interface PasswordResetToken {
hash: string;
}
const restEndpoint = Container.get(GlobalConfig).endpoints.rest;
// The browser-id check needs to be skipped on these endpoints
const skipBrowserIdCheckEndpoints = [
// we need to exclude push endpoint 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
`/${restEndpoint}/push`,
// We need to exclude binary-data downloading endpoint because we can't send custom headers on `<embed>` tags
`/${restEndpoint}/binary-data/`,
// oAuth callback urls aren't called by the frontend. therefore we can't send custom header on these requests
`/${restEndpoint}/oauth1-credential/callback`,
`/${restEndpoint}/oauth2-credential/callback`,
];
@Service()
export class AuthService {
// The browser-id check needs to be skipped on these endpoints
private skipBrowserIdCheckEndpoints: string[];
constructor(
private readonly globalConfig: GlobalConfig,
private readonly logger: Logger,
private readonly license: License,
private readonly jwtService: JwtService,
@@ -62,6 +51,20 @@ export class AuthService {
) {
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
this.authMiddleware = this.authMiddleware.bind(this);
const restEndpoint = globalConfig.endpoints.rest;
this.skipBrowserIdCheckEndpoints = [
// we need to exclude push endpoint 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
`/${restEndpoint}/push`,
// We need to exclude binary-data downloading endpoint because we can't send custom headers on `<embed>` tags
`/${restEndpoint}/binary-data/`,
// oAuth callback urls aren't called by the frontend. therefore we can't send custom header on these requests
`/${restEndpoint}/oauth1-credential/callback`,
`/${restEndpoint}/oauth2-credential/callback`,
];
}
async authMiddleware(req: AuthenticatedRequest, res: Response, next: NextFunction) {
@@ -117,11 +120,12 @@ export class AuthService {
}
const token = this.issueJWT(user, browserId);
const { samesite, secure } = this.globalConfig.auth.cookie;
res.cookie(AUTH_COOKIE_NAME, token, {
maxAge: this.jwtExpiration * Time.seconds.toMilliseconds,
httpOnly: true,
sameSite: 'lax',
secure: config.getEnv('secure_cookie'),
sameSite: samesite,
secure,
});
}
@@ -159,7 +163,7 @@ export class AuthService {
// Check if the token was issued for another browser session, ignoring the endpoints that can't send custom headers
const endpoint = req.route ? `${req.baseUrl}${req.route.path}` : req.baseUrl;
if (req.method === 'GET' && skipBrowserIdCheckEndpoints.includes(endpoint)) {
if (req.method === 'GET' && this.skipBrowserIdCheckEndpoints.includes(endpoint)) {
this.logger.debug(`Skipped browserId check on ${endpoint}`);
} else if (
jwtPayload.browserId &&

View File

@@ -120,12 +120,6 @@ export const schema = {
},
},
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: '',

View File

@@ -1,9 +1,12 @@
import { SecurityConfig } from '@n8n/config';
import { Container, Service } from '@n8n/di';
import cookieParser from 'cookie-parser';
import express from 'express';
import { access as fsAccess } from 'fs/promises';
import helmet from 'helmet';
import { isEmpty } from 'lodash';
import { InstanceSettings } from 'n8n-core';
import { jsonParse } from 'n8n-workflow';
import { resolve } from 'path';
import { AbstractServer } from '@/abstract-server';
@@ -348,8 +351,21 @@ export class Server extends AbstractServer {
const isTLSEnabled =
this.globalConfig.protocol === 'https' && !!(this.sslKey && this.sslCert);
const isPreviewMode = process.env.N8N_PREVIEW_MODE === 'true';
const cspDirectives = jsonParse<{ [key: string]: Iterable<string> }>(
Container.get(SecurityConfig).contentSecurityPolicy,
{
errorMessage: 'The contentSecurityPolicy is not valid JSON.',
},
);
const securityHeadersMiddleware = helmet({
contentSecurityPolicy: false,
contentSecurityPolicy: isEmpty(cspDirectives)
? false
: {
useDefaults: false,
directives: {
...cspDirectives,
},
},
xFrameOptions:
isPreviewMode || inE2ETests || inDevelopment ? false : { action: 'sameorigin' },
dnsPrefetchControl: false,

View File

@@ -103,7 +103,7 @@ export class FrontendService {
versionCli: N8N_VERSION,
concurrency: config.getEnv('executions.concurrency.productionLimit'),
authCookie: {
secure: config.getEnv('secure_cookie'),
secure: this.globalConfig.auth.cookie.secure,
},
releaseChannel: this.globalConfig.generic.releaseChannel,
oauthCallbackUrls: {