diff --git a/.changeset/great-rats-argue.md b/.changeset/great-rats-argue.md new file mode 100644 index 00000000000..e758c90aff4 --- /dev/null +++ b/.changeset/great-rats-argue.md @@ -0,0 +1,5 @@ +--- +'@clerk/clerk-js': minor +--- + +Adjusts cookie setting in development so Clerk can function in embedded iframe scenarios. diff --git a/packages/clerk-js/src/core/auth/AuthCookieService.ts b/packages/clerk-js/src/core/auth/AuthCookieService.ts index 6cbdd8207e7..8c5e59a32e8 100644 --- a/packages/clerk-js/src/core/auth/AuthCookieService.ts +++ b/packages/clerk-js/src/core/auth/AuthCookieService.ts @@ -73,8 +73,8 @@ export class AuthCookieService { this.refreshTokenOnFocus(); this.startPollingForToken(); - this.clientUat = createClientUatCookie(cookieSuffix); - this.sessionCookie = createSessionCookie(cookieSuffix); + this.clientUat = createClientUatCookie({ cookieSuffix, isProduction: this.instanceType === 'production' }); + this.sessionCookie = createSessionCookie({ cookieSuffix, isProduction: this.instanceType === 'production' }); this.activeCookie = createCookieHandler('clerk_active_context'); this.devBrowser = createDevBrowser({ frontendApi: clerk.frontendApi, diff --git a/packages/clerk-js/src/core/auth/cookies/__tests__/clientUat.test.ts b/packages/clerk-js/src/core/auth/cookies/__tests__/clientUat.test.ts index c8dd0633d7e..6afb241ce1a 100644 --- a/packages/clerk-js/src/core/auth/cookies/__tests__/clientUat.test.ts +++ b/packages/clerk-js/src/core/auth/cookies/__tests__/clientUat.test.ts @@ -35,14 +35,14 @@ describe('createClientUatCookie', () => { }); it('should create both suffixed and non-suffixed cookie handlers', () => { - createClientUatCookie(mockCookieSuffix); + createClientUatCookie({ cookieSuffix: mockCookieSuffix, isProduction: false }); expect(createCookieHandler).toHaveBeenCalledTimes(2); expect(createCookieHandler).toHaveBeenCalledWith('__client_uat'); expect(createCookieHandler).toHaveBeenCalledWith('__client_uat_test-suffix'); }); it('should set cookies with correct parameters in non-cross-origin context', () => { - const cookieHandler = createClientUatCookie(mockCookieSuffix); + const cookieHandler = createClientUatCookie({ cookieSuffix: mockCookieSuffix, isProduction: false }); cookieHandler.set({ id: 'test-client', updatedAt: new Date('2024-01-01'), @@ -50,24 +50,6 @@ describe('createClientUatCookie', () => { }); expect(mockSet).toHaveBeenCalledTimes(2); - expect(mockSet).toHaveBeenCalledWith('1704067200', { - domain: mockDomain, - expires: mockExpires, - sameSite: 'Strict', - secure: true, - partitioned: false, - }); - }); - - it('should set cookies with None sameSite in cross-origin context', () => { - (inCrossOriginIframe as jest.Mock).mockReturnValue(true); - const cookieHandler = createClientUatCookie(mockCookieSuffix); - cookieHandler.set({ - id: 'test-client', - updatedAt: new Date('2024-01-01'), - signedInSessions: ['session1'], - }); - expect(mockSet).toHaveBeenCalledWith('1704067200', { domain: mockDomain, expires: mockExpires, @@ -78,20 +60,20 @@ describe('createClientUatCookie', () => { }); it('should set value to 0 when client is undefined', () => { - const cookieHandler = createClientUatCookie(mockCookieSuffix); + const cookieHandler = createClientUatCookie({ cookieSuffix: mockCookieSuffix, isProduction: false }); cookieHandler.set(undefined); expect(mockSet).toHaveBeenCalledWith('0', { domain: mockDomain, expires: mockExpires, - sameSite: 'Strict', + sameSite: 'None', secure: true, partitioned: false, }); }); it('should set value to 0 when client has no signed in sessions', () => { - const cookieHandler = createClientUatCookie(mockCookieSuffix); + const cookieHandler = createClientUatCookie({ cookieSuffix: mockCookieSuffix, isProduction: false }); cookieHandler.set({ id: 'test-client', updatedAt: new Date('2024-01-01'), @@ -101,7 +83,7 @@ describe('createClientUatCookie', () => { expect(mockSet).toHaveBeenCalledWith('0', { domain: mockDomain, expires: mockExpires, - sameSite: 'Strict', + sameSite: 'None', secure: true, partitioned: false, }); @@ -110,7 +92,7 @@ describe('createClientUatCookie', () => { it('should get cookie value from suffixed cookie first, then fallback to non-suffixed', () => { mockGet.mockImplementationOnce(() => '1234567890').mockImplementationOnce(() => '9876543210'); - const cookieHandler = createClientUatCookie(mockCookieSuffix); + const cookieHandler = createClientUatCookie({ cookieSuffix: mockCookieSuffix, isProduction: false }); const result = cookieHandler.get(); expect(result).toBe(1234567890); @@ -119,9 +101,26 @@ describe('createClientUatCookie', () => { it('should return 0 when no cookie value is present', () => { mockGet.mockImplementationOnce(() => undefined).mockImplementationOnce(() => undefined); - const cookieHandler = createClientUatCookie(mockCookieSuffix); + const cookieHandler = createClientUatCookie({ cookieSuffix: mockCookieSuffix, isProduction: false }); const result = cookieHandler.get(); expect(result).toBe(0); }); + + it('should set SameSite=Strict in production', () => { + const cookieHandler = createClientUatCookie({ cookieSuffix: mockCookieSuffix, isProduction: true }); + cookieHandler.set({ + id: 'test-client', + updatedAt: new Date('2024-01-01'), + signedInSessions: ['session1'], + }); + + expect(mockSet).toHaveBeenCalledWith('1704067200', { + domain: mockDomain, + expires: mockExpires, + sameSite: 'Strict', + secure: true, + partitioned: false, + }); + }); }); diff --git a/packages/clerk-js/src/core/auth/cookies/__tests__/session.test.ts b/packages/clerk-js/src/core/auth/cookies/__tests__/session.test.ts index 3ab6bca3966..1385b11e57d 100644 --- a/packages/clerk-js/src/core/auth/cookies/__tests__/session.test.ts +++ b/packages/clerk-js/src/core/auth/cookies/__tests__/session.test.ts @@ -32,29 +32,42 @@ describe('createSessionCookie', () => { }); it('should create both suffixed and non-suffixed cookie handlers', () => { - createSessionCookie(mockCookieSuffix); + createSessionCookie({ cookieSuffix: mockCookieSuffix, isProduction: false }); expect(createCookieHandler).toHaveBeenCalledTimes(2); expect(createCookieHandler).toHaveBeenCalledWith('__session'); expect(createCookieHandler).toHaveBeenCalledWith('__session_test-suffix'); }); - it('should set cookies with correct parameters in non-cross-origin context', () => { - const cookieHandler = createSessionCookie(mockCookieSuffix); + it('should set cookies with correct parameters', () => { + const cookieHandler = createSessionCookie({ cookieSuffix: mockCookieSuffix, isProduction: false }); cookieHandler.set(mockToken); expect(mockSet).toHaveBeenCalledTimes(2); expect(mockSet).toHaveBeenCalledWith(mockToken, { expires: mockExpires, - sameSite: 'Lax', + sameSite: 'None', secure: true, partitioned: false, }); }); - it('should set cookies with None sameSite in cross-origin context', () => { - (inCrossOriginIframe as jest.Mock).mockReturnValue(true); - const cookieHandler = createSessionCookie(mockCookieSuffix); + it('should remove both cookies when remove is called', () => { + const cookieHandler = createSessionCookie({ cookieSuffix: mockCookieSuffix, isProduction: false }); + cookieHandler.remove(); + + expect(mockRemove).toHaveBeenCalledTimes(2); + }); + + it('should remove cookies with the same attributes as set', () => { + const cookieHandler = createSessionCookie({ cookieSuffix: mockCookieSuffix, isProduction: false }); cookieHandler.set(mockToken); + cookieHandler.remove(); + + const expectedAttributes = { + sameSite: 'None', + secure: true, + partitioned: false, + }; expect(mockSet).toHaveBeenCalledWith(mockToken, { expires: mockExpires, @@ -62,29 +75,24 @@ describe('createSessionCookie', () => { secure: true, partitioned: false, }); - }); - - it('should remove both cookies when remove is called', () => { - const cookieHandler = createSessionCookie(mockCookieSuffix); - cookieHandler.remove(); - expect(mockRemove).toHaveBeenCalledTimes(2); + expect(mockRemove).toHaveBeenCalledWith(expectedAttributes); }); it('should remove cookies with the same attributes as set', () => { - const cookieHandler = createSessionCookie(mockCookieSuffix); + const cookieHandler = createSessionCookie({ cookieSuffix: mockCookieSuffix, isProduction: false }); cookieHandler.set(mockToken); cookieHandler.remove(); const expectedAttributes = { - sameSite: 'Lax', + sameSite: 'None', secure: true, partitioned: false, }; expect(mockSet).toHaveBeenCalledWith(mockToken, { expires: mockExpires, - sameSite: 'Lax', + sameSite: 'None', secure: true, partitioned: false, }); @@ -98,7 +106,7 @@ describe('createSessionCookie', () => { it('should get cookie value from suffixed cookie first, then fallback to non-suffixed', () => { mockGet.mockImplementationOnce(() => 'suffixed-value').mockImplementationOnce(() => 'non-suffixed-value'); - const cookieHandler = createSessionCookie(mockCookieSuffix); + const cookieHandler = createSessionCookie({ cookieSuffix: mockCookieSuffix, isProduction: false }); const result = cookieHandler.get(); expect(result).toBe('suffixed-value'); @@ -107,9 +115,21 @@ describe('createSessionCookie', () => { it('should fallback to non-suffixed cookie when suffixed cookie is not present', () => { mockGet.mockImplementationOnce(() => undefined).mockImplementationOnce(() => 'non-suffixed-value'); - const cookieHandler = createSessionCookie(mockCookieSuffix); + const cookieHandler = createSessionCookie({ cookieSuffix: mockCookieSuffix, isProduction: false }); const result = cookieHandler.get(); expect(result).toBe('non-suffixed-value'); }); + + it('should not set SameSite=None in production', () => { + const cookieHandler = createSessionCookie({ cookieSuffix: mockCookieSuffix, isProduction: true }); + cookieHandler.set(mockToken); + + expect(mockSet).toHaveBeenCalledWith(mockToken, { + expires: mockExpires, + sameSite: 'Lax', + secure: true, + partitioned: false, + }); + }); }); diff --git a/packages/clerk-js/src/core/auth/cookies/clientUat.ts b/packages/clerk-js/src/core/auth/cookies/clientUat.ts index 9ffec6e1456..d2d4dd8d6a8 100644 --- a/packages/clerk-js/src/core/auth/cookies/clientUat.ts +++ b/packages/clerk-js/src/core/auth/cookies/clientUat.ts @@ -20,7 +20,13 @@ export type ClientUatCookieHandler = { * The cookie is used as hint from the Clerk Backend packages to identify * if the user is authenticated or not. */ -export const createClientUatCookie = (cookieSuffix: string): ClientUatCookieHandler => { +export const createClientUatCookie = ({ + cookieSuffix, + isProduction, +}: { + cookieSuffix: string; + isProduction: boolean; +}): ClientUatCookieHandler => { const clientUatCookie = createCookieHandler(CLIENT_UAT_COOKIE_NAME); const suffixedClientUatCookie = createCookieHandler(getSuffixedCookieName(CLIENT_UAT_COOKIE_NAME, cookieSuffix)); @@ -37,7 +43,13 @@ export const createClientUatCookie = (cookieSuffix: string): ClientUatCookieHand * Generally, this is handled by redirectWithAuth() being called and relying on the dev browser ID in the URL, * but if that isn't used we rely on this. In production, nothing is cross-domain and Lax is used when client_uat is set from FAPI. */ - const sameSite = __BUILD_VARIANT_CHIPS__ ? 'None' : inCrossOriginIframe() ? 'None' : 'Strict'; + const sameSite = __BUILD_VARIANT_CHIPS__ + ? 'None' + : inCrossOriginIframe() + ? 'None' + : isProduction + ? 'Strict' + : 'None'; const secure = getSecureAttribute(sameSite); const partitioned = __BUILD_VARIANT_CHIPS__ && secure; const domain = getCookieDomain(); diff --git a/packages/clerk-js/src/core/auth/cookies/devBrowser.ts b/packages/clerk-js/src/core/auth/cookies/devBrowser.ts index 47edda5fadd..acf1b677288 100644 --- a/packages/clerk-js/src/core/auth/cookies/devBrowser.ts +++ b/packages/clerk-js/src/core/auth/cookies/devBrowser.ts @@ -3,7 +3,6 @@ import { addYears } from '@clerk/shared/date'; import { DEV_BROWSER_JWT_KEY } from '@clerk/shared/devBrowser'; import { getSuffixedCookieName } from '@clerk/shared/keys'; -import { inCrossOriginIframe } from '../../../utils'; import { getSecureAttribute } from '../getSecureAttribute'; export type DevBrowserCookieHandler = { @@ -13,7 +12,7 @@ export type DevBrowserCookieHandler = { }; const getCookieAttributes = (): { sameSite: string; secure: boolean } => { - const sameSite = inCrossOriginIframe() ? 'None' : 'Lax'; + const sameSite = 'None'; const secure = getSecureAttribute(sameSite); return { sameSite, secure }; }; @@ -24,7 +23,7 @@ const getCookieAttributes = (): { sameSite: string; secure: boolean } => { * The cookie is used to authenticate FAPI requests and pass * authentication from AP to the app. */ -export const createDevBrowserCookie = (cookieSuffix: string): DevBrowserCookieHandler => { +export const createDevBrowserCookie = ({ cookieSuffix }: { cookieSuffix: string }): DevBrowserCookieHandler => { const devBrowserCookie = createCookieHandler(DEV_BROWSER_JWT_KEY); const suffixedDevBrowserCookie = createCookieHandler(getSuffixedCookieName(DEV_BROWSER_JWT_KEY, cookieSuffix)); diff --git a/packages/clerk-js/src/core/auth/cookies/session.ts b/packages/clerk-js/src/core/auth/cookies/session.ts index 209755d96f4..22f40c8c50d 100644 --- a/packages/clerk-js/src/core/auth/cookies/session.ts +++ b/packages/clerk-js/src/core/auth/cookies/session.ts @@ -13,8 +13,12 @@ export type SessionCookieHandler = { get: () => string | undefined; }; -const getCookieAttributes = (): { sameSite: string; secure: boolean; partitioned: boolean } => { - const sameSite = __BUILD_VARIANT_CHIPS__ ? 'None' : inCrossOriginIframe() ? 'None' : 'Lax'; +const getCookieAttributes = ({ + isProduction, +}: { + isProduction: boolean; +}): { sameSite: string; secure: boolean; partitioned: boolean } => { + const sameSite = __BUILD_VARIANT_CHIPS__ ? 'None' : inCrossOriginIframe() ? 'None' : isProduction ? 'Lax' : 'None'; const secure = getSecureAttribute(sameSite); const partitioned = __BUILD_VARIANT_CHIPS__ && secure; return { sameSite, secure, partitioned }; @@ -25,19 +29,25 @@ const getCookieAttributes = (): { sameSite: string; secure: boolean; partitioned * The cookie is used by the Clerk backend SDKs to identify * the authenticated user. */ -export const createSessionCookie = (cookieSuffix: string): SessionCookieHandler => { +export const createSessionCookie = ({ + cookieSuffix, + isProduction, +}: { + cookieSuffix: string; + isProduction: boolean; +}): SessionCookieHandler => { const sessionCookie = createCookieHandler(SESSION_COOKIE_NAME); const suffixedSessionCookie = createCookieHandler(getSuffixedCookieName(SESSION_COOKIE_NAME, cookieSuffix)); const remove = () => { - const attributes = getCookieAttributes(); + const attributes = getCookieAttributes({ isProduction }); sessionCookie.remove(attributes); suffixedSessionCookie.remove(attributes); }; const set = (token: string) => { const expires = addYears(Date.now(), 1); - const { sameSite, secure, partitioned } = getCookieAttributes(); + const { sameSite, secure, partitioned } = getCookieAttributes({ isProduction }); // If setting Partitioned to true, remove the existing session cookies. // This is to avoid conflicts with the same cookie name without Partitioned attribute. diff --git a/packages/clerk-js/src/core/auth/devBrowser.ts b/packages/clerk-js/src/core/auth/devBrowser.ts index 2d5e3cb644e..7b7527e3709 100644 --- a/packages/clerk-js/src/core/auth/devBrowser.ts +++ b/packages/clerk-js/src/core/auth/devBrowser.ts @@ -26,7 +26,7 @@ export type CreateDevBrowserOptions = { }; export function createDevBrowser({ cookieSuffix, frontendApi, fapiClient }: CreateDevBrowserOptions): DevBrowser { - const devBrowserCookie = createDevBrowserCookie(cookieSuffix); + const devBrowserCookie = createDevBrowserCookie({ cookieSuffix }); function getDevBrowserJWT() { return devBrowserCookie.get();