From fd73ca63bb87414ce477bbffac7c5a4846425234 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix=20Malfait?= Date: Sat, 1 Aug 2026 13:55:29 +0000 Subject: [PATCH] fix(auth): parse cookie protocol, complete loopback range, share the window default - isSecureDeployment used a case-sensitive startsWith, so a SERVER_URL spelled HTTPS:// was read as plain http and handed out a cookie without the __Host- prefix. The protocol is now parsed. - Loopback filtering missed the IPv4-mapped form: URL canonicalises [::ffff:127.0.0.1] to [::ffff:7f00:1], which the previous literal never matched. Both spellings and the whole 127.0.0.0/8 block are covered now. - The 10m auto-login window was written twice, in the config default and in the gate's fallback. One constant now feeds both so they cannot drift. - The CSRF spec restored a mutated allowlist inline, so a failing assertion would have leaked it into every later test. The restore is unconditional in afterEach, which made the restore flag dead. --- ...lt-workspace-auto-login-window.constant.ts | 4 ++ ...dential-auto-login-into-workspaces.util.ts | 6 ++- .../twenty-config/config-variables.ts | 3 +- .../services/user-session-cookie.service.ts | 19 +++++++-- ...solve-allowed-credentialed-origins.util.ts | 42 +++++++++++++------ .../cookie-session-csrf.middleware.spec.ts | 13 ++---- 6 files changed, 60 insertions(+), 27 deletions(-) create mode 100644 packages/twenty-server/src/engine/core-modules/auth/constants/default-workspace-auto-login-window.constant.ts diff --git a/packages/twenty-server/src/engine/core-modules/auth/constants/default-workspace-auto-login-window.constant.ts b/packages/twenty-server/src/engine/core-modules/auth/constants/default-workspace-auto-login-window.constant.ts new file mode 100644 index 00000000000..5d1c14dcc77 --- /dev/null +++ b/packages/twenty-server/src/engine/core-modules/auth/constants/default-workspace-auto-login-window.constant.ts @@ -0,0 +1,4 @@ +// Single source for both the WORKSPACE_AUTO_LOGIN_WINDOW default and the +// fallback the gate applies when the configured value cannot be parsed, so +// the two cannot drift apart. +export const DEFAULT_WORKSPACE_AUTO_LOGIN_WINDOW = '10m'; diff --git a/packages/twenty-server/src/engine/core-modules/auth/utils/can-credential-auto-login-into-workspaces.util.ts b/packages/twenty-server/src/engine/core-modules/auth/utils/can-credential-auto-login-into-workspaces.util.ts index 37c6589dfdd..cab198deb27 100644 --- a/packages/twenty-server/src/engine/core-modules/auth/utils/can-credential-auto-login-into-workspaces.util.ts +++ b/packages/twenty-server/src/engine/core-modules/auth/utils/can-credential-auto-login-into-workspaces.util.ts @@ -2,6 +2,8 @@ import { addMilliseconds } from 'date-fns'; import ms from 'ms'; import { isDefined } from 'twenty-shared/utils'; +import { DEFAULT_WORKSPACE_AUTO_LOGIN_WINDOW } from 'src/engine/core-modules/auth/constants/default-workspace-auto-login-window.constant'; + // Listing the workspaces a user belongs to is harmless and stays available // for the whole session, but converting that credential into workspace access // without re-authenticating is not: a workspace-agnostic session outlives a @@ -9,7 +11,9 @@ import { isDefined } from 'twenty-shared/utils'; // cookie it does not own), so it would otherwise hand the workspace back. // Workspace-scoped credentials are unaffected: signing out of a workspace // revokes them, so they cannot outlive the sign-out they would bypass. -const DEFAULT_WORKSPACE_AUTO_LOGIN_WINDOW_MS = 10 * 60 * 1000; +const DEFAULT_WORKSPACE_AUTO_LOGIN_WINDOW_MS = ms( + DEFAULT_WORKSPACE_AUTO_LOGIN_WINDOW, +); export const canCredentialAutoLoginIntoWorkspaces = ({ isWorkspaceScopedCredential, diff --git a/packages/twenty-server/src/engine/core-modules/twenty-config/config-variables.ts b/packages/twenty-server/src/engine/core-modules/twenty-config/config-variables.ts index 116191fe7c6..7fc0e9250b5 100644 --- a/packages/twenty-server/src/engine/core-modules/twenty-config/config-variables.ts +++ b/packages/twenty-server/src/engine/core-modules/twenty-config/config-variables.ts @@ -47,6 +47,7 @@ import { CastToUpperSnakeCase } from 'src/engine/core-modules/twenty-config/deco import { ConfigVariablesMetadata } from 'src/engine/core-modules/twenty-config/decorators/config-variables-metadata.decorator'; import { IsAWSRegion } from 'src/engine/core-modules/twenty-config/decorators/is-aws-region.decorator'; import { IsDuration } from 'src/engine/core-modules/twenty-config/decorators/is-duration.decorator'; +import { DEFAULT_WORKSPACE_AUTO_LOGIN_WINDOW } from 'src/engine/core-modules/auth/constants/default-workspace-auto-login-window.constant'; import { IsNonNegativeDuration } from 'src/engine/core-modules/twenty-config/decorators/is-non-negative-duration.decorator'; import { IsPositiveDuration } from 'src/engine/core-modules/twenty-config/decorators/is-positive-duration.decorator'; import { IsOptionalOrEmptyString } from 'src/engine/core-modules/twenty-config/decorators/is-optional-or-empty-string.decorator'; @@ -399,7 +400,7 @@ export class ConfigVariables { }) @IsNonNegativeDuration() @IsOptional() - WORKSPACE_AUTO_LOGIN_WINDOW = '10m'; + WORKSPACE_AUTO_LOGIN_WINDOW: string = DEFAULT_WORKSPACE_AUTO_LOGIN_WINDOW; @ConfigVariablesMetadata({ group: ConfigVariablesGroup.ADVANCED_SETTINGS, diff --git a/packages/twenty-server/src/engine/core-modules/user-session/services/user-session-cookie.service.ts b/packages/twenty-server/src/engine/core-modules/user-session/services/user-session-cookie.service.ts index 2b8284be8bb..c55f097d15a 100644 --- a/packages/twenty-server/src/engine/core-modules/user-session/services/user-session-cookie.service.ts +++ b/packages/twenty-server/src/engine/core-modules/user-session/services/user-session-cookie.service.ts @@ -9,6 +9,18 @@ import { USER_SESSION_SECURE_COOKIE_NAME } from 'src/engine/core-modules/user-se import { extractUserSessionTokenFromRequestCookie } from 'src/engine/core-modules/user-session/utils/extract-user-session-token-from-request.util'; import { TwentyConfigService } from 'src/engine/core-modules/twenty-config/twenty-config.service'; +const isHttpsUrl = (url: string | undefined): boolean => { + if (!isNonEmptyString(url)) { + return false; + } + + try { + return new URL(url).protocol === 'https:'; + } catch { + return false; + } +}; + @Injectable() export class UserSessionCookieService { constructor(private readonly twentyConfigService: TwentyConfigService) {} @@ -19,10 +31,11 @@ export class UserSessionCookieService { const serverUrl = this.twentyConfigService.get('SERVER_URL'); const sameSite = this.twentyConfigService.get('AUTH_COOKIE_SAME_SITE'); + // Parsed rather than prefix-matched: a SERVER_URL spelled HTTPS:// is + // https, and treating it as plain http would silently hand out a cookie + // without the __Host- prefix. // SameSite=None is rejected by browsers without Secure, so it forces it. - return ( - Boolean(serverUrl && serverUrl.startsWith('https')) || sameSite === 'none' - ); + return isHttpsUrl(serverUrl) || sameSite === 'none'; } // The kill switch lives here rather than at each call site: with it off, diff --git a/packages/twenty-server/src/engine/core-modules/user-session/utils/resolve-allowed-credentialed-origins.util.ts b/packages/twenty-server/src/engine/core-modules/user-session/utils/resolve-allowed-credentialed-origins.util.ts index 6ad028f5678..6004ed9bd83 100644 --- a/packages/twenty-server/src/engine/core-modules/user-session/utils/resolve-allowed-credentialed-origins.util.ts +++ b/packages/twenty-server/src/engine/core-modules/user-session/utils/resolve-allowed-credentialed-origins.util.ts @@ -19,23 +19,39 @@ const toOrigin = (url: string): string | undefined => { } }; -// The whole 127.0.0.0/8 block is loopback, not just 127.0.0.1, and IPv6 -// loopback arrives bracketed from URL.hostname. -const LOOPBACK_HOSTNAMES = new Set([ - 'localhost', - '[::1]', - '::1', - '[::ffff:127.0.0.1]', -]); +// URL canonicalises [::ffff:127.0.0.1] to [::ffff:7f00:1], so matching the +// dotted spelling alone would miss it. The whole 127.0.0.0/8 block is +// loopback, not just 127.0.0.1. const IPV4_LOOPBACK_REGEX = /^127\.\d{1,3}\.\d{1,3}\.\d{1,3}$/; +const IPV4_MAPPED_DOTTED_REGEX = /^::ffff:(\d{1,3}(?:\.\d{1,3}){3})$/; +const IPV4_MAPPED_HEX_REGEX = /^::ffff:([0-9a-f]{1,4}):[0-9a-f]{1,4}$/; + +const isLoopbackHostname = (hostname: string): boolean => { + const host = hostname.replace(/^\[|\]$/g, '').toLowerCase(); + + if (host === 'localhost' || host === '::1') { + return true; + } + + if (IPV4_LOOPBACK_REGEX.test(host)) { + return true; + } + + const mappedDotted = IPV4_MAPPED_DOTTED_REGEX.exec(host); + + if (mappedDotted !== null) { + return IPV4_LOOPBACK_REGEX.test(mappedDotted[1]); + } + + const mappedHex = IPV4_MAPPED_HEX_REGEX.exec(host); + + // The high byte of the first hextet is the first octet of the v4 address. + return mappedHex !== null && Number.parseInt(mappedHex[1], 16) >> 8 === 127; +}; const isLoopbackOrigin = (origin: string): boolean => { try { - const hostname = new URL(origin).hostname.toLowerCase(); - - return ( - LOOPBACK_HOSTNAMES.has(hostname) || IPV4_LOOPBACK_REGEX.test(hostname) - ); + return isLoopbackHostname(new URL(origin).hostname); } catch { return false; } diff --git a/packages/twenty-server/src/engine/middlewares/cookie-session-csrf.middleware.spec.ts b/packages/twenty-server/src/engine/middlewares/cookie-session-csrf.middleware.spec.ts index b10b0fa38bc..8db4671dc20 100644 --- a/packages/twenty-server/src/engine/middlewares/cookie-session-csrf.middleware.spec.ts +++ b/packages/twenty-server/src/engine/middlewares/cookie-session-csrf.middleware.spec.ts @@ -38,13 +38,11 @@ describe('CookieSessionCsrfMiddleware', () => { }; let next: NextFunction; - let afterEachRestoreFlag = false; - + // Restored unconditionally: a failing assertion must not leak a mutated + // config into the tests that follow, since mockConfig is describe-scoped. afterEach(() => { - if (afterEachRestoreFlag) { - mockConfig.AUTH_COOKIE_SESSIONS_ENABLED = true; - afterEachRestoreFlag = false; - } + mockConfig.AUTH_COOKIE_SESSIONS_ENABLED = true; + mockConfig.AUTH_COOKIE_ALLOWED_ORIGINS = ''; }); beforeEach(async () => { @@ -167,8 +165,6 @@ describe('CookieSessionCsrfMiddleware', () => { middleware.use(request, buildResponse(), next); expect(next).toHaveBeenCalled(); - - mockConfig.AUTH_COOKIE_ALLOWED_ORIGINS = ''; }); // Browsers omit :443 from Origin while Host keeps a port the client spelled @@ -207,7 +203,6 @@ describe('CookieSessionCsrfMiddleware', () => { it('should skip when cookie sessions are disabled', () => { mockConfig.AUTH_COOKIE_SESSIONS_ENABLED = false; - afterEachRestoreFlag = true; const request = buildRequest({ headers: {