fix(front): store auth tokenPair in localStorage instead of a cookie (#21507)
## Problem A client hit an AWS S3 `RequestHeaderSectionTooLarge` error (`MaxSizeAllowed 8192`) when opening a `https://<workspace>.twenty.com/verify?loginToken=<JWT>` link — the request to load the `/verify` SPA page is served from S3, which rejects it before the app loads. The dominant cause is the **`tokenPair` cookie**. The auth tokenPair (access + refresh JWTs, ~2–5KB) was persisted in a host-scoped, JS-readable cookie. Nothing server-side ever reads it — the access token is sent to the API via an `Authorization: Bearer` header set in the Apollo auth link (`ExtractJwt.fromAuthHeaderAsBearerToken()` on the backend; no `cookie-parser`). Yet the browser attached that cookie to **every** request to the origin, including static assets and the `/verify` page. Combined with the `loginToken` in the URL, the request header section exceeds S3's 8192-byte limit. ## Fix Move `tokenPair` from cookie storage to **localStorage**, which is never transmitted in request headers. - `tokenPairState` now uses `useLocalStorage` (with `getOnInit: true`). - `getTokenPair` (the synchronous read used by the Apollo auth link) reads from localStorage under the same key. - A one-time migration (`migrateTokenPairCookieToLocalStorage`) runs before React renders: it ports any existing `tokenPair` cookie into localStorage and **deletes the cookie**, so already-authenticated users aren't logged out and the oversized cookie stops being sent. ## Why this is safe **Behavior:** equivalent. The cookie was host-scoped (no `domain` attribute), so it never provided cross-subdomain sharing — cross-workspace auth already re-establishes the token per-origin via the `loginToken`-in-URL → `/verify` handoff. localStorage has identical origin scoping. **Security:** neutral-to-positive. - No XSS protection lost — the cookie was **not** `httpOnly` (it can't be; JS reads it to build the Bearer header), so it was already XSS-exposed exactly like localStorage. - No CSRF surface change — the token was never sent as a cookie credential (no `credentials: 'include'`). - **Reduced exposure** — the token no longer leaks into CDN/proxy/server access logs or request headers, which is the actual bug. - Server-side revocation (`revokedAt`) and the 60-day refresh-token JWT expiry govern validity, so localStorage's lack of auto-expiry is moot. ## Testing - `getTokenPair` unit tests updated to localStorage. - New unit tests for the migration util (port, no-op, no-clobber, error-safety). - `nx test twenty-front` auth + apollo suites: 125 passing. - `lint:diff-with-main` clean; changed files typecheck clean. <!-- This is an auto-generated description by cubic. --> <a href="https://cubic.dev/pr/twentyhq/twenty/pull/21507?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. -->
This commit is contained in:
Charles Bochet
@@ -1,126 +1,110 @@
|
||||
import { cookieStorage } from '~/utils/cookie-storage';
|
||||
import { getTokenPair } from '@/apollo/utils/getTokenPair';
|
||||
|
||||
jest.mock('~/utils/cookie-storage', () => ({
|
||||
cookieStorage: {
|
||||
getItem: jest.fn(),
|
||||
removeItem: jest.fn(),
|
||||
},
|
||||
}));
|
||||
|
||||
const mockCookieStorage = cookieStorage as jest.Mocked<typeof cookieStorage>;
|
||||
import { TOKEN_PAIR_LOCAL_STORAGE_KEY } from '@/auth/states/tokenPairState';
|
||||
|
||||
describe('getTokenPair', () => {
|
||||
let getItemSpy: jest.SpyInstance;
|
||||
let removeItemSpy: jest.SpyInstance;
|
||||
|
||||
beforeEach(() => {
|
||||
jest.clearAllMocks();
|
||||
getItemSpy = jest.spyOn(Storage.prototype, 'getItem');
|
||||
removeItemSpy = jest.spyOn(Storage.prototype, 'removeItem');
|
||||
});
|
||||
|
||||
describe('when tokenPair cookie does not exist', () => {
|
||||
it('should return undefined when cookie is not set', () => {
|
||||
mockCookieStorage.getItem.mockReturnValue(undefined);
|
||||
afterEach(() => {
|
||||
jest.restoreAllMocks();
|
||||
});
|
||||
|
||||
describe('when tokenPair is not stored', () => {
|
||||
it('should return undefined when nothing is stored', () => {
|
||||
getItemSpy.mockReturnValue(null);
|
||||
|
||||
const result = getTokenPair();
|
||||
|
||||
expect(result).toBeUndefined();
|
||||
expect(mockCookieStorage.getItem).toHaveBeenCalledWith('tokenPair');
|
||||
});
|
||||
|
||||
it('should return undefined when cookie is undefined', () => {
|
||||
mockCookieStorage.getItem.mockReturnValue(undefined);
|
||||
|
||||
const result = getTokenPair();
|
||||
|
||||
expect(result).toBeUndefined();
|
||||
expect(mockCookieStorage.getItem).toHaveBeenCalledWith('tokenPair');
|
||||
expect(getItemSpy).toHaveBeenCalledWith(TOKEN_PAIR_LOCAL_STORAGE_KEY);
|
||||
});
|
||||
});
|
||||
|
||||
describe('when tokenPair cookie has invalid JSON', () => {
|
||||
it('should remove cookie and return undefined for malformed JSON', () => {
|
||||
mockCookieStorage.getItem.mockReturnValue('invalid-json');
|
||||
describe('when stored tokenPair has invalid JSON', () => {
|
||||
it('should remove the item and return undefined for malformed JSON', () => {
|
||||
getItemSpy.mockReturnValue('invalid-json');
|
||||
|
||||
const result = getTokenPair();
|
||||
|
||||
expect(result).toBeUndefined();
|
||||
expect(mockCookieStorage.removeItem).toHaveBeenCalledWith('tokenPair');
|
||||
expect(removeItemSpy).toHaveBeenCalledWith(TOKEN_PAIR_LOCAL_STORAGE_KEY);
|
||||
});
|
||||
|
||||
it('should remove cookie and return undefined for partial JSON', () => {
|
||||
mockCookieStorage.getItem.mockReturnValue('{"incomplete":');
|
||||
it('should remove the item and return undefined for partial JSON', () => {
|
||||
getItemSpy.mockReturnValue('{"incomplete":');
|
||||
|
||||
const result = getTokenPair();
|
||||
|
||||
expect(result).toBeUndefined();
|
||||
expect(mockCookieStorage.removeItem).toHaveBeenCalledWith('tokenPair');
|
||||
expect(removeItemSpy).toHaveBeenCalledWith(TOKEN_PAIR_LOCAL_STORAGE_KEY);
|
||||
});
|
||||
});
|
||||
|
||||
describe('when tokenPair cookie has invalid structure', () => {
|
||||
it('should remove cookie and return undefined when tokenPair is null', () => {
|
||||
mockCookieStorage.getItem.mockReturnValue('null');
|
||||
describe('when stored tokenPair has invalid structure', () => {
|
||||
it('should remove the item and return undefined when tokenPair is null', () => {
|
||||
getItemSpy.mockReturnValue('null');
|
||||
|
||||
const result = getTokenPair();
|
||||
|
||||
expect(result).toBeUndefined();
|
||||
expect(mockCookieStorage.removeItem).toHaveBeenCalledWith('tokenPair');
|
||||
expect(removeItemSpy).toHaveBeenCalledWith(TOKEN_PAIR_LOCAL_STORAGE_KEY);
|
||||
});
|
||||
|
||||
it('should remove cookie and return undefined when tokenPair is not an object', () => {
|
||||
mockCookieStorage.getItem.mockReturnValue('"string-value"');
|
||||
it('should remove the item and return undefined when tokenPair is not an object', () => {
|
||||
getItemSpy.mockReturnValue('"string-value"');
|
||||
|
||||
const result = getTokenPair();
|
||||
|
||||
expect(result).toBeUndefined();
|
||||
expect(mockCookieStorage.removeItem).toHaveBeenCalledWith('tokenPair');
|
||||
expect(removeItemSpy).toHaveBeenCalledWith(TOKEN_PAIR_LOCAL_STORAGE_KEY);
|
||||
});
|
||||
|
||||
it('should remove cookie and return undefined when accessOrWorkspaceAgnosticToken is missing', () => {
|
||||
it('should remove the item and return undefined when accessOrWorkspaceAgnosticToken is missing', () => {
|
||||
const invalidTokenPair = {
|
||||
refreshToken: { token: 'refresh-token' },
|
||||
};
|
||||
mockCookieStorage.getItem.mockReturnValue(
|
||||
JSON.stringify(invalidTokenPair),
|
||||
);
|
||||
getItemSpy.mockReturnValue(JSON.stringify(invalidTokenPair));
|
||||
|
||||
const result = getTokenPair();
|
||||
|
||||
expect(result).toBeUndefined();
|
||||
expect(mockCookieStorage.removeItem).toHaveBeenCalledWith('tokenPair');
|
||||
expect(removeItemSpy).toHaveBeenCalledWith(TOKEN_PAIR_LOCAL_STORAGE_KEY);
|
||||
});
|
||||
|
||||
it('should remove cookie and return undefined when accessOrWorkspaceAgnosticToken is not an object', () => {
|
||||
it('should remove the item and return undefined when accessOrWorkspaceAgnosticToken is not an object', () => {
|
||||
const invalidTokenPair = {
|
||||
accessOrWorkspaceAgnosticToken: 'not-an-object',
|
||||
refreshToken: { token: 'refresh-token' },
|
||||
};
|
||||
mockCookieStorage.getItem.mockReturnValue(
|
||||
JSON.stringify(invalidTokenPair),
|
||||
);
|
||||
getItemSpy.mockReturnValue(JSON.stringify(invalidTokenPair));
|
||||
|
||||
const result = getTokenPair();
|
||||
|
||||
expect(result).toBeUndefined();
|
||||
expect(mockCookieStorage.removeItem).toHaveBeenCalledWith('tokenPair');
|
||||
expect(removeItemSpy).toHaveBeenCalledWith(TOKEN_PAIR_LOCAL_STORAGE_KEY);
|
||||
});
|
||||
|
||||
it('should remove cookie and return undefined when token is missing', () => {
|
||||
it('should remove the item and return undefined when token is missing', () => {
|
||||
const invalidTokenPair = {
|
||||
accessOrWorkspaceAgnosticToken: {
|
||||
expiresAt: '2024-01-01T00:00:00Z',
|
||||
},
|
||||
refreshToken: { token: 'refresh-token' },
|
||||
};
|
||||
mockCookieStorage.getItem.mockReturnValue(
|
||||
JSON.stringify(invalidTokenPair),
|
||||
);
|
||||
getItemSpy.mockReturnValue(JSON.stringify(invalidTokenPair));
|
||||
|
||||
const result = getTokenPair();
|
||||
|
||||
expect(result).toBeUndefined();
|
||||
expect(mockCookieStorage.removeItem).toHaveBeenCalledWith('tokenPair');
|
||||
expect(removeItemSpy).toHaveBeenCalledWith(TOKEN_PAIR_LOCAL_STORAGE_KEY);
|
||||
});
|
||||
|
||||
it('should remove cookie and return undefined when token is not a string', () => {
|
||||
it('should remove the item and return undefined when token is not a string', () => {
|
||||
const invalidTokenPair = {
|
||||
accessOrWorkspaceAgnosticToken: {
|
||||
token: 123,
|
||||
@@ -128,14 +112,12 @@ describe('getTokenPair', () => {
|
||||
},
|
||||
refreshToken: { token: 'refresh-token' },
|
||||
};
|
||||
mockCookieStorage.getItem.mockReturnValue(
|
||||
JSON.stringify(invalidTokenPair),
|
||||
);
|
||||
getItemSpy.mockReturnValue(JSON.stringify(invalidTokenPair));
|
||||
|
||||
const result = getTokenPair();
|
||||
|
||||
expect(result).toBeUndefined();
|
||||
expect(mockCookieStorage.removeItem).toHaveBeenCalledWith('tokenPair');
|
||||
expect(removeItemSpy).toHaveBeenCalledWith(TOKEN_PAIR_LOCAL_STORAGE_KEY);
|
||||
});
|
||||
|
||||
it('should accept empty string token as valid', () => {
|
||||
@@ -146,16 +128,16 @@ describe('getTokenPair', () => {
|
||||
},
|
||||
refreshToken: { token: 'refresh-token' },
|
||||
};
|
||||
mockCookieStorage.getItem.mockReturnValue(JSON.stringify(validTokenPair));
|
||||
getItemSpy.mockReturnValue(JSON.stringify(validTokenPair));
|
||||
|
||||
const result = getTokenPair();
|
||||
|
||||
expect(result).toEqual(validTokenPair);
|
||||
expect(mockCookieStorage.removeItem).not.toHaveBeenCalled();
|
||||
expect(removeItemSpy).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe('when tokenPair cookie has valid structure', () => {
|
||||
describe('when stored tokenPair has valid structure', () => {
|
||||
it('should return valid tokenPair with all required fields', () => {
|
||||
const validTokenPair = {
|
||||
accessOrWorkspaceAgnosticToken: {
|
||||
@@ -167,12 +149,12 @@ describe('getTokenPair', () => {
|
||||
expiresAt: '2024-01-02T00:00:00Z',
|
||||
},
|
||||
};
|
||||
mockCookieStorage.getItem.mockReturnValue(JSON.stringify(validTokenPair));
|
||||
getItemSpy.mockReturnValue(JSON.stringify(validTokenPair));
|
||||
|
||||
const result = getTokenPair();
|
||||
|
||||
expect(result).toEqual(validTokenPair);
|
||||
expect(mockCookieStorage.removeItem).not.toHaveBeenCalled();
|
||||
expect(removeItemSpy).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should return valid tokenPair with minimal required fields', () => {
|
||||
@@ -181,12 +163,12 @@ describe('getTokenPair', () => {
|
||||
token: 'minimal-access-token',
|
||||
},
|
||||
};
|
||||
mockCookieStorage.getItem.mockReturnValue(JSON.stringify(validTokenPair));
|
||||
getItemSpy.mockReturnValue(JSON.stringify(validTokenPair));
|
||||
|
||||
const result = getTokenPair();
|
||||
|
||||
expect(result).toEqual(validTokenPair);
|
||||
expect(mockCookieStorage.removeItem).not.toHaveBeenCalled();
|
||||
expect(removeItemSpy).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should return valid tokenPair with extra fields', () => {
|
||||
@@ -201,19 +183,18 @@ describe('getTokenPair', () => {
|
||||
},
|
||||
additionalField: 'additional-value',
|
||||
};
|
||||
mockCookieStorage.getItem.mockReturnValue(JSON.stringify(validTokenPair));
|
||||
getItemSpy.mockReturnValue(JSON.stringify(validTokenPair));
|
||||
|
||||
const result = getTokenPair();
|
||||
|
||||
expect(result).toEqual(validTokenPair);
|
||||
expect(mockCookieStorage.removeItem).not.toHaveBeenCalled();
|
||||
expect(removeItemSpy).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe('edge cases', () => {
|
||||
it('should handle JSON parsing error gracefully', () => {
|
||||
mockCookieStorage.getItem.mockReturnValue('{"valid": "json"');
|
||||
// Simulate JSON.parse throwing an error
|
||||
getItemSpy.mockReturnValue('{"valid": "json"');
|
||||
const originalParse = JSON.parse;
|
||||
JSON.parse = jest.fn(() => {
|
||||
throw new SyntaxError('Unexpected end of JSON input');
|
||||
@@ -222,9 +203,8 @@ describe('getTokenPair', () => {
|
||||
const result = getTokenPair();
|
||||
|
||||
expect(result).toBeUndefined();
|
||||
expect(mockCookieStorage.removeItem).toHaveBeenCalledWith('tokenPair');
|
||||
expect(removeItemSpy).toHaveBeenCalledWith(TOKEN_PAIR_LOCAL_STORAGE_KEY);
|
||||
|
||||
// Restore original JSON.parse
|
||||
JSON.parse = originalParse;
|
||||
});
|
||||
|
||||
@@ -235,7 +215,7 @@ describe('getTokenPair', () => {
|
||||
token: longToken,
|
||||
},
|
||||
};
|
||||
mockCookieStorage.getItem.mockReturnValue(JSON.stringify(validTokenPair));
|
||||
getItemSpy.mockReturnValue(JSON.stringify(validTokenPair));
|
||||
|
||||
const result = getTokenPair();
|
||||
|
||||
@@ -250,7 +230,7 @@ describe('getTokenPair', () => {
|
||||
token: unicodeToken,
|
||||
},
|
||||
};
|
||||
mockCookieStorage.getItem.mockReturnValue(JSON.stringify(validTokenPair));
|
||||
getItemSpy.mockReturnValue(JSON.stringify(validTokenPair));
|
||||
|
||||
const result = getTokenPair();
|
||||
|
||||
|
||||
@@ -1,15 +1,12 @@
|
||||
import { isDefined } from 'twenty-shared/utils';
|
||||
import { TOKEN_PAIR_LOCAL_STORAGE_KEY } from '@/auth/states/tokenPairState';
|
||||
import { type AuthTokenPair } from '~/generated-metadata/graphql';
|
||||
import { cookieStorage } from '~/utils/cookie-storage';
|
||||
import { isValidAuthTokenPair } from './isValidAuthTokenPair';
|
||||
|
||||
export const getTokenPair = (): AuthTokenPair | undefined => {
|
||||
const stringTokenPair = cookieStorage.getItem('tokenPair');
|
||||
const stringTokenPair = localStorage.getItem(TOKEN_PAIR_LOCAL_STORAGE_KEY);
|
||||
|
||||
if (!isDefined(stringTokenPair)) {
|
||||
// oxlint-disable-next-line no-console
|
||||
console.log('tokenPair is undefined');
|
||||
|
||||
return undefined;
|
||||
}
|
||||
|
||||
@@ -17,13 +14,13 @@ export const getTokenPair = (): AuthTokenPair | undefined => {
|
||||
const parsedTokenPair = JSON.parse(stringTokenPair);
|
||||
|
||||
if (!isValidAuthTokenPair(parsedTokenPair)) {
|
||||
cookieStorage.removeItem('tokenPair');
|
||||
localStorage.removeItem(TOKEN_PAIR_LOCAL_STORAGE_KEY);
|
||||
return undefined;
|
||||
}
|
||||
|
||||
return parsedTokenPair;
|
||||
} catch {
|
||||
cookieStorage.removeItem('tokenPair');
|
||||
localStorage.removeItem(TOKEN_PAIR_LOCAL_STORAGE_KEY);
|
||||
return undefined;
|
||||
}
|
||||
};
|
||||
|
||||
Reference in New Issue
Block a user