From be051c8724d326a473df5cc9c2699d0f28f70a26 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix=20Malfait?= Date: Tue, 4 Aug 2026 17:12:51 +0200 Subject: [PATCH] Keep the token pair as a fallback after switching to cookie auth (#23755) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `CookieSessionBootEffect` cleared the token pair the moment it switched a client onto cookie auth. That leaves the client with a single credential, and a server that still has `AUTH_COOKIE_SESSIONS_ENABLED=false` ignores the session cookie entirely — `extractSessionTokenFromRequest` early-returns when the flag is off. A cookie-only client is therefore unauthenticated against such a server, `handleTokenRenewal` finds no refresh token, and `onUnauthenticatedError` signs the user out. That is not a hypothetical state. It is every request routed to a not-yet-rolled pod while the flag is being enabled, and every request after the flag is rolled back. Requests are load-balanced per request, so a migrated client hits an old pod almost immediately and gets signed out; signing back in can migrate it again and repeat for the length of the rollout. It also means rollback was not free, contrary to how it was described: flipping the flag back to `false` signed out everyone who had already migrated, because the pair they were supposed to fall back to had been deleted. ## Approach Keep the token pair as a dormant fallback, and stop *sending* it while cookie auth is active. Both halves are needed. Retaining it without suppressing the header would be worse than the bug: `validateTokenByRequest` checks the Bearer token first and only falls back to the session cookie when there is none, so a client that keeps sending Bearer would never exercise the cookie at all, and `CookieSessionCsrfMiddleware` bypasses on any Bearer-carrying request. Cookie sessions would silently become a no-op. So: - `switchToCookieAuth` no longer nulls the token pair - the auth link omits `authorization` while cookie auth is active, leaving the cookie as the credential in use - on an unauthenticated error while cookie auth is active, the client deactivates cookie auth once per operation and falls through to the existing renewal path, which replays with a fresh Bearer The fallback deliberately goes through renewal rather than replaying immediately: access tokens live 10 minutes, so the retained one has usually expired while the client was authenticating by cookie, and an immediate replay would just fail again. `isCookieAuthActive` is read and written through `localStorage` from the link because the links run per request and must agree with the atom synchronously — a React state update lands a render too late to affect the request being built. ## Follow-up This trades the immediate removal of the token pair from `localStorage` for rollout safety, so the XSS-exfiltration surface that cookie sessions close stays open a while longer. Once cookie sessions are stable across every environment, the retained pair should be dropped — reverting to a clear on `switchToCookieAuth` is a one-line change. ## Test Three cases added to `apollo.factory.test.ts`: no Bearer header while cookie auth is active; an unauthenticated response falls back and replays with the token pair rather than calling `onUnauthenticatedError`; and the fallback is attempted only once before going through renewal. The existing `CookieSessionBootEffect` assertion that the pair is cleared is inverted to assert it is retained. Review in cubic --- .../services/__tests__/apollo.factory.test.ts | 68 +++++++++++++++++++ .../modules/apollo/services/apollo.factory.ts | 44 +++++++++++- .../apollo/utils/getIsCookieAuthActive.ts | 13 ++++ .../apollo/utils/setIsCookieAuthActive.ts | 14 ++++ .../CookieSessionBootEffect.tsx | 11 ++- .../CookieSessionBootEffect.test.tsx | 4 +- .../auth/states/isCookieAuthActiveState.ts | 5 +- 7 files changed, 152 insertions(+), 7 deletions(-) create mode 100644 packages/twenty-front/src/modules/apollo/utils/getIsCookieAuthActive.ts create mode 100644 packages/twenty-front/src/modules/apollo/utils/setIsCookieAuthActive.ts diff --git a/packages/twenty-front/src/modules/apollo/services/__tests__/apollo.factory.test.ts b/packages/twenty-front/src/modules/apollo/services/__tests__/apollo.factory.test.ts index 24ee8ef7b0..188c679d7c 100644 --- a/packages/twenty-front/src/modules/apollo/services/__tests__/apollo.factory.test.ts +++ b/packages/twenty-front/src/modules/apollo/services/__tests__/apollo.factory.test.ts @@ -4,6 +4,8 @@ import fetchMock, { enableFetchMocks } from 'jest-fetch-mock'; import { ApolloFactory, type Options } from '@/apollo/services/apollo.factory'; import { getTokenPair } from '@/apollo/utils/getTokenPair'; +import { isCookieAuthActiveState } from '@/auth/states/isCookieAuthActiveState'; +import { jotaiStore } from '@/ui/utilities/state/jotai/jotaiStore'; import { renewToken } from '@/auth/services/AuthService'; import { CUSTOM_WORKSPACE_APPLICATION_MOCK } from '@/object-metadata/hooks/__tests__/constants/CustomWorkspaceApplicationMock.test.constant'; import { @@ -154,6 +156,7 @@ describe('ApolloFactory', () => { fetchMock.resetMocks(); jest.mocked(renewToken).mockReset().mockResolvedValue(RENEWED_TOKEN_PAIR); jest.mocked(getTokenPair).mockReset().mockReturnValue(CURRENT_TOKEN_PAIR); + jotaiStore.set(isCookieAuthActiveState.atom, false); }); it('should create an instance of ApolloFactory', () => { @@ -366,4 +369,69 @@ describe('ApolloFactory', () => { expect(renewToken).not.toHaveBeenCalled(); expect(mockOnUnauthenticatedError).toHaveBeenCalledTimes(1); }); + describe('cookie auth fallback during a mixed-version rollout', () => { + const setCookieAuthActive = () => + jotaiStore.set(isCookieAuthActiveState.atom, true); + + // fetch normalises header names, so assert case-insensitively rather than + // depending on the casing the mock happens to expose. + const readHeader = ( + headers: Record, + name: string, + ): string | undefined => + Object.entries(headers).find( + ([key]) => key.toLowerCase() === name.toLowerCase(), + )?.[1]; + + it('should not attach the Bearer header while cookie auth is active', async () => { + setCookieAuthActive(); + fetchMock.mockResponse(() => + Promise.resolve({ body: JSON.stringify({ data: {} }) }), + ); + + await makeRequest(); + + const headers = fetchMock.mock.calls[0]?.[1]?.headers as Record< + string, + string + >; + + expect(readHeader(headers, 'authorization')).toBeUndefined(); + // Version-mismatch detection must keep working in cookie-auth mode. + expect(readHeader(headers, 'X-App-Version')).toBe('1.0.0'); + }); + + it('should fall back to the token pair instead of signing out when a server ignores the session cookie', async () => { + setCookieAuthActive(); + fetchMock + .mockResponseOnce(UNAUTHENTICATED_RESPONSE) + .mockResponseOnce(JSON.stringify({ data: { trackAnalytics: null } })); + + await makeRequest(); + + expect(mockOnUnauthenticatedError).not.toHaveBeenCalled(); + expect(jotaiStore.get(isCookieAuthActiveState.atom)).toBe(false); + + const retryHeaders = fetchMock.mock.calls[1]?.[1]?.headers as Record< + string, + string + >; + + expect(readHeader(retryHeaders, 'authorization')).toBe( + `Bearer ${CURRENT_TOKEN_PAIR.accessOrWorkspaceAgnosticToken.token}`, + ); + }); + + it('should only attempt the cookie fallback once, then go through renewal', async () => { + setCookieAuthActive(); + fetchMock.mockResponse(UNAUTHENTICATED_RESPONSE); + + try { + await makeRequest(); + } catch {} + + expect(jotaiStore.get(isCookieAuthActiveState.atom)).toBe(false); + expect(renewToken).toHaveBeenCalled(); + }); + }); }); diff --git a/packages/twenty-front/src/modules/apollo/services/apollo.factory.ts b/packages/twenty-front/src/modules/apollo/services/apollo.factory.ts index b81d6398f4..0a47a6eea2 100644 --- a/packages/twenty-front/src/modules/apollo/services/apollo.factory.ts +++ b/packages/twenty-front/src/modules/apollo/services/apollo.factory.ts @@ -20,7 +20,9 @@ import { retryWithBackoff } from '~/utils/retryWithBackoff'; import { REST_API_BASE_URL } from '@/apollo/constant/rest-api-base-url'; import { type ApolloManager } from '@/apollo/types/apolloManager.interface'; +import { getIsCookieAuthActive } from '@/apollo/utils/getIsCookieAuthActive'; import { getTokenPair } from '@/apollo/utils/getTokenPair'; +import { setIsCookieAuthActive } from '@/apollo/utils/setIsCookieAuthActive'; import { isUnauthenticatedGraphQLError } from '@/apollo/utils/isUnauthenticatedGraphQLError'; import { loggerLink } from '@/apollo/utils/loggerLink'; import { StreamingRestLink } from '@/apollo/utils/streamingRestLink'; @@ -133,12 +135,23 @@ export class ApolloFactory implements ApolloManager { const locale = this.currentWorkspaceMember?.locale ?? i18n.locale; - if (isUndefinedOrNull(tokenPair) || skipAuthToken === true) { + // The token pair is kept as a dormant fallback once cookie auth is + // active, but must not be sent: Bearer takes precedence over the + // session cookie server-side, so attaching it would keep the cookie + // unused and bypass the CSRF origin check. + if ( + isUndefinedOrNull(tokenPair) || + skipAuthToken === true || + getIsCookieAuthActive() + ) { return { headers: { ...headers, ...optionHeaders, 'x-locale': locale, + ...(isDefined(this.appVersion) && { + 'X-App-Version': this.appVersion, + }), }, }; } @@ -151,7 +164,9 @@ export class ApolloFactory implements ApolloManager { ...optionHeaders, authorization: token ? `Bearer ${token}` : '', 'x-locale': locale, - ...(this.appVersion && { 'X-App-Version': this.appVersion }), + ...(isDefined(this.appVersion) && { + 'X-App-Version': this.appVersion, + }), }, }; }); @@ -205,6 +220,31 @@ export class ApolloFactory implements ApolloManager { return throwError(() => error); } + // A server that still has cookie sessions disabled ignores the session + // cookie, so a cookie-only client reads as unauthenticated there. That + // happens on every request routed to a not-yet-rolled pod, and after a + // rollback. Fall back to the retained token pair instead of signing the + // user out. Attempted once per operation so a genuinely expired token + // still reaches the renewal path below. + if ( + getIsCookieAuthActive() && + operation.getContext().hasAttemptedCookieAuthFallback !== true && + isDefined(getTokenPair()?.refreshToken?.token) + ) { + setIsCookieAuthActive(false); + operation.setContext({ hasAttemptedCookieAuthFallback: true }); + // Deactivation is sticky for the rest of the mount by design. Both + // credentials stay valid, so re-probing after every fallback would + // thrash between them for the whole rollout: the probe succeeds on a + // rolled pod, the next request lands on an old one and falls back + // again. CookieSessionBootEffect re-probes on the next mount, which + // restores cookie auth once the fleet is uniform. + // Deliberately falls through to the renewal below rather than + // replaying immediately: the retained access token is likely to have + // expired while the client was authenticating by cookie, so the + // replay needs a fresh one to succeed on the first try. + } + if (!getTokenPair()?.refreshToken?.token) { onUnauthenticatedError?.(); diff --git a/packages/twenty-front/src/modules/apollo/utils/getIsCookieAuthActive.ts b/packages/twenty-front/src/modules/apollo/utils/getIsCookieAuthActive.ts new file mode 100644 index 0000000000..3b9ba6fa21 --- /dev/null +++ b/packages/twenty-front/src/modules/apollo/utils/getIsCookieAuthActive.ts @@ -0,0 +1,13 @@ +import { isCookieAuthActiveState } from '@/auth/states/isCookieAuthActiveState'; +import { jotaiStore } from '@/ui/utilities/state/jotai/jotaiStore'; + +// Read straight off the store rather than through a subscription because the +// Apollo links run per request and must see the value synchronously, before +// the render a React subscription would wait for. +export const getIsCookieAuthActive = (): boolean => { + try { + return jotaiStore.get(isCookieAuthActiveState.atom) === true; + } catch { + return false; + } +}; diff --git a/packages/twenty-front/src/modules/apollo/utils/setIsCookieAuthActive.ts b/packages/twenty-front/src/modules/apollo/utils/setIsCookieAuthActive.ts new file mode 100644 index 0000000000..1688d350ff --- /dev/null +++ b/packages/twenty-front/src/modules/apollo/utils/setIsCookieAuthActive.ts @@ -0,0 +1,14 @@ +import { isCookieAuthActiveState } from '@/auth/states/isCookieAuthActiveState'; +import { jotaiStore } from '@/ui/utilities/state/jotai/jotaiStore'; + +// Goes through the atom so the persisted key keeps a single writer, and so +// React subscribers follow without the caller mirroring this into component +// state. Guarded because it runs inside unauthenticated error handling, where +// blocked storage would otherwise throw out of the error path. +export const setIsCookieAuthActive = (isActive: boolean): void => { + try { + jotaiStore.set(isCookieAuthActiveState.atom, isActive); + } catch { + // noop + } +}; diff --git a/packages/twenty-front/src/modules/auth/effect-components/CookieSessionBootEffect.tsx b/packages/twenty-front/src/modules/auth/effect-components/CookieSessionBootEffect.tsx index 77d9bee27e..f684c96110 100644 --- a/packages/twenty-front/src/modules/auth/effect-components/CookieSessionBootEffect.tsx +++ b/packages/twenty-front/src/modules/auth/effect-components/CookieSessionBootEffect.tsx @@ -47,7 +47,7 @@ export const CookieSessionBootEffect = () => { const [isCookieAuthActive, setIsCookieAuthActive] = useAtomState( isCookieAuthActiveState, ); - const [tokenPair, setTokenPair] = useAtomState(tokenPairState); + const tokenPair = useAtomStateValue(tokenPairState); // oxlint-disable-next-line twenty/no-state-useref const hasProbeRunRef = useRef(false); @@ -68,9 +68,15 @@ export const CookieSessionBootEffect = () => { } }; + // The token pair is deliberately retained rather than cleared. A server + // that still has cookie sessions disabled ignores the session cookie, so a + // cookie-only client is unauthenticated against it — which is every request + // routed to a not-yet-rolled pod during a deploy, and every request after a + // rollback. Keeping the pair lets those fall back instead of signing the + // user out. It stops being sent while cookie auth is active, so the cookie + // is still the credential in use. const switchToCookieAuth = () => { setIsCookieAuthActive(true); - setTokenPair(null); }; const attemptCookieSessionBoot = async (): Promise => { @@ -145,7 +151,6 @@ export const CookieSessionBootEffect = () => { isCookieSessionEnabled, isLoadedOnce, setIsCookieAuthActive, - setTokenPair, store, tokenPair, ]); diff --git a/packages/twenty-front/src/modules/auth/effect-components/__tests__/CookieSessionBootEffect.test.tsx b/packages/twenty-front/src/modules/auth/effect-components/__tests__/CookieSessionBootEffect.test.tsx index 7252853de0..67326c7579 100644 --- a/packages/twenty-front/src/modules/auth/effect-components/__tests__/CookieSessionBootEffect.test.tsx +++ b/packages/twenty-front/src/modules/auth/effect-components/__tests__/CookieSessionBootEffect.test.tsx @@ -89,7 +89,9 @@ describe('CookieSessionBootEffect', () => { expect(store.get(isCookieAuthActiveState.atom)).toBe(true); }); - expect(store.get(tokenPairState.atom)).toBeNull(); + // Retained as a fallback for servers that still have cookie sessions + // disabled, which would otherwise sign the user out mid-rollout. + expect(store.get(tokenPairState.atom)).not.toBeNull(); }); it('should stay retryable when the probe fails for an unrelated reason', async () => { diff --git a/packages/twenty-front/src/modules/auth/states/isCookieAuthActiveState.ts b/packages/twenty-front/src/modules/auth/states/isCookieAuthActiveState.ts index 91285790a9..4f294de238 100644 --- a/packages/twenty-front/src/modules/auth/states/isCookieAuthActiveState.ts +++ b/packages/twenty-front/src/modules/auth/states/isCookieAuthActiveState.ts @@ -1,7 +1,10 @@ import { createAtomState } from '@/ui/utilities/state/jotai/utils/createAtomState'; +export const IS_COOKIE_AUTH_ACTIVE_LOCAL_STORAGE_KEY = + 'isCookieAuthActiveState'; + export const isCookieAuthActiveState = createAtomState({ - key: 'isCookieAuthActiveState', + key: IS_COOKIE_AUTH_ACTIVE_LOCAL_STORAGE_KEY, defaultValue: false, useLocalStorage: true, localStorageOptions: { getOnInit: true },