From e8dc4c2f7006174a3e6c057e32f65f70a3491e24 Mon Sep 17 00:00:00 2001 From: Bryce Kalow Date: Mon, 21 Jul 2025 12:04:34 -0500 Subject: [PATCH 1/6] Ensure same cookie attributes are passed to remove() and fixes the types --- .../src/core/auth/AuthCookieService.ts | 8 +++++++ .../src/core/auth/cookies/devBrowser.ts | 15 ++++++++---- .../clerk-js/src/core/auth/cookies/session.ts | 16 +++++++++---- packages/clerk-js/src/core/clerk.ts | 2 -- packages/shared/src/cookie.ts | 23 +++++++++++++------ 5 files changed, 45 insertions(+), 19 deletions(-) diff --git a/packages/clerk-js/src/core/auth/AuthCookieService.ts b/packages/clerk-js/src/core/auth/AuthCookieService.ts index dcdba03d856..6cbdd8207e7 100644 --- a/packages/clerk-js/src/core/auth/AuthCookieService.ts +++ b/packages/clerk-js/src/core/auth/AuthCookieService.ts @@ -68,6 +68,8 @@ export class AuthCookieService { this.setClientUatCookieForDevelopmentInstances(); }); + eventBus.on(events.UserSignOut, () => this.handleSignOut()); + this.refreshTokenOnFocus(); this.startPollingForToken(); @@ -212,6 +214,12 @@ export class AuthCookieService { // -------- } + private handleSignOut() { + this.activeCookie.remove(); + this.sessionCookie.remove(); + this.setClientUatCookieForDevelopmentInstances(); + } + /** * The below methods handle active context tracking (session and organization) to ensure * only tabs with matching context can update the session cookie. diff --git a/packages/clerk-js/src/core/auth/cookies/devBrowser.ts b/packages/clerk-js/src/core/auth/cookies/devBrowser.ts index 7b292a9e656..32733a96b62 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 = { @@ -12,6 +11,12 @@ export type DevBrowserCookieHandler = { remove: () => void; }; +const getCookieAttributes = (): { sameSite: string; secure: boolean } => { + const sameSite = 'None'; + const secure = getSecureAttribute(sameSite); + return { sameSite, secure }; +}; + /** * Create a long-lived JS cookie to store the dev browser token * ONLY for development instances. @@ -26,16 +31,16 @@ export const createDevBrowserCookie = (cookieSuffix: string): DevBrowserCookieHa const set = (jwt: string) => { const expires = addYears(Date.now(), 1); - const sameSite = inCrossOriginIframe() ? 'None' : 'Lax'; - const secure = getSecureAttribute(sameSite); + const { sameSite, secure } = getCookieAttributes(); suffixedDevBrowserCookie.set(jwt, { expires, sameSite, secure }); devBrowserCookie.set(jwt, { expires, sameSite, secure }); }; const remove = () => { - suffixedDevBrowserCookie.remove(); - devBrowserCookie.remove(); + const attributes = getCookieAttributes(); + suffixedDevBrowserCookie.remove(attributes); + devBrowserCookie.remove(attributes); }; return { diff --git a/packages/clerk-js/src/core/auth/cookies/session.ts b/packages/clerk-js/src/core/auth/cookies/session.ts index 8025b69aa8b..209755d96f4 100644 --- a/packages/clerk-js/src/core/auth/cookies/session.ts +++ b/packages/clerk-js/src/core/auth/cookies/session.ts @@ -13,6 +13,13 @@ export type SessionCookieHandler = { get: () => string | undefined; }; +const getCookieAttributes = (): { sameSite: string; secure: boolean; partitioned: boolean } => { + const sameSite = __BUILD_VARIANT_CHIPS__ ? 'None' : inCrossOriginIframe() ? 'None' : 'Lax'; + const secure = getSecureAttribute(sameSite); + const partitioned = __BUILD_VARIANT_CHIPS__ && secure; + return { sameSite, secure, partitioned }; +}; + /** * Create a short-lived JS cookie to store the current user JWT. * The cookie is used by the Clerk backend SDKs to identify @@ -23,15 +30,14 @@ export const createSessionCookie = (cookieSuffix: string): SessionCookieHandler const suffixedSessionCookie = createCookieHandler(getSuffixedCookieName(SESSION_COOKIE_NAME, cookieSuffix)); const remove = () => { - sessionCookie.remove(); - suffixedSessionCookie.remove(); + const attributes = getCookieAttributes(); + sessionCookie.remove(attributes); + suffixedSessionCookie.remove(attributes); }; const set = (token: string) => { const expires = addYears(Date.now(), 1); - const sameSite = __BUILD_VARIANT_CHIPS__ ? 'None' : inCrossOriginIframe() ? 'None' : 'Lax'; - const secure = getSecureAttribute(sameSite); - const partitioned = __BUILD_VARIANT_CHIPS__ && secure; + const { sameSite, secure, partitioned } = getCookieAttributes(); // 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/clerk.ts b/packages/clerk-js/src/core/clerk.ts index 8d1f5450c43..f3a13254de6 100644 --- a/packages/clerk-js/src/core/clerk.ts +++ b/packages/clerk-js/src/core/clerk.ts @@ -485,8 +485,6 @@ export class Clerk implements ClerkInterface { // Notify other tabs that user is signing out. eventBus.emit(events.UserSignOut, null); - // Clean up cookies - eventBus.emit(events.TokenUpdate, { token: null }); this.#setTransitiveState(); diff --git a/packages/shared/src/cookie.ts b/packages/shared/src/cookie.ts index 87679a095a9..9e726ed2ea9 100644 --- a/packages/shared/src/cookie.ts +++ b/packages/shared/src/cookie.ts @@ -1,10 +1,17 @@ import Cookies from 'js-cookie'; -type LocationAttributes = { - path?: string; - domain?: string; -}; - +/** + * Creates helper methods for dealing with a specific cookie. + * + * @example + * ```ts + * const cookie = createCookieHandler('my_cookie') + * + * cookie.set('my_value'); + * cookie.get() // 'my_value'; + * cookie.remove() + * ``` + */ export function createCookieHandler(cookieName: string) { return { get() { @@ -18,10 +25,12 @@ export function createCookieHandler(cookieName: string) { }, /** * On removing a cookie, you have to pass the exact same path/domain attributes used to set it initially + * > IMPORTANT! When deleting a cookie and you're not relying on the default attributes, you must pass the exact same path, domain, secure and sameSite attributes that were used to set the cookie. + * * @see https://github.com/js-cookie/js-cookie#basic-usage */ - remove(locationAttributes?: LocationAttributes) { - Cookies.remove(cookieName, locationAttributes); + remove(cookieAttributes?: Cookies.CookieAttributes) { + Cookies.remove(cookieName, cookieAttributes); }, }; } From 6f20ed87e119071fbd2c07235ac3ef7834dba3eb Mon Sep 17 00:00:00 2001 From: Bryce Kalow Date: Mon, 21 Jul 2025 12:22:24 -0500 Subject: [PATCH 2/6] Adds test case --- .../auth/cookies/__tests__/session.test.ts | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) 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 8ca14d5a5e9..3ab6bca3966 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 @@ -71,6 +71,30 @@ describe('createSessionCookie', () => { expect(mockRemove).toHaveBeenCalledTimes(2); }); + it('should remove cookies with the same attributes as set', () => { + const cookieHandler = createSessionCookie(mockCookieSuffix); + cookieHandler.set(mockToken); + cookieHandler.remove(); + + const expectedAttributes = { + sameSite: 'Lax', + secure: true, + partitioned: false, + }; + + expect(mockSet).toHaveBeenCalledWith(mockToken, { + expires: mockExpires, + sameSite: 'Lax', + secure: true, + partitioned: false, + }); + + expect(mockRemove).toHaveBeenCalledWith(expectedAttributes); + expect(mockRemove).toHaveBeenCalledTimes(2); + expect(mockRemove).toHaveBeenNthCalledWith(1, expectedAttributes); + expect(mockRemove).toHaveBeenNthCalledWith(2, expectedAttributes); + }); + it('should get cookie value from suffixed cookie first, then fallback to non-suffixed', () => { mockGet.mockImplementationOnce(() => 'suffixed-value').mockImplementationOnce(() => 'non-suffixed-value'); From 81b7b4af8ca9e4aea5321250e75a96da7ffdb0c9 Mon Sep 17 00:00:00 2001 From: Bryce Kalow Date: Mon, 21 Jul 2025 12:23:20 -0500 Subject: [PATCH 3/6] Adds changeset --- .changeset/famous-news-judge.md | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changeset/famous-news-judge.md diff --git a/.changeset/famous-news-judge.md b/.changeset/famous-news-judge.md new file mode 100644 index 00000000000..28efd16bc2a --- /dev/null +++ b/.changeset/famous-news-judge.md @@ -0,0 +1,6 @@ +--- +'@clerk/clerk-js': patch +'@clerk/shared': patch +--- + +Fixes an issue where cookies were not properly cleared on sign out when using non-default cookie attributes. From b5bc084d878eeb0db4c0fd418b5a0bd79904ba2b Mon Sep 17 00:00:00 2001 From: Bryce Kalow Date: Mon, 21 Jul 2025 13:02:29 -0500 Subject: [PATCH 4/6] Apply suggestion from @brkalow --- packages/clerk-js/src/core/auth/cookies/devBrowser.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/clerk-js/src/core/auth/cookies/devBrowser.ts b/packages/clerk-js/src/core/auth/cookies/devBrowser.ts index 32733a96b62..4656b4e3a82 100644 --- a/packages/clerk-js/src/core/auth/cookies/devBrowser.ts +++ b/packages/clerk-js/src/core/auth/cookies/devBrowser.ts @@ -3,6 +3,7 @@ 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 = { From 7cfeed278066c4cd7c4ae17a5618efa075b2a781 Mon Sep 17 00:00:00 2001 From: Bryce Kalow Date: Mon, 21 Jul 2025 13:02:35 -0500 Subject: [PATCH 5/6] Apply suggestion from @brkalow --- packages/clerk-js/src/core/auth/cookies/devBrowser.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/clerk-js/src/core/auth/cookies/devBrowser.ts b/packages/clerk-js/src/core/auth/cookies/devBrowser.ts index 4656b4e3a82..47edda5fadd 100644 --- a/packages/clerk-js/src/core/auth/cookies/devBrowser.ts +++ b/packages/clerk-js/src/core/auth/cookies/devBrowser.ts @@ -13,7 +13,7 @@ export type DevBrowserCookieHandler = { }; const getCookieAttributes = (): { sameSite: string; secure: boolean } => { - const sameSite = 'None'; + const sameSite = inCrossOriginIframe() ? 'None' : 'Lax'; const secure = getSecureAttribute(sameSite); return { sameSite, secure }; }; From 6af5c6786ef81b8db28ed682be6310985ee415c4 Mon Sep 17 00:00:00 2001 From: Bryce Kalow Date: Mon, 28 Jul 2025 14:52:34 -0500 Subject: [PATCH 6/6] Apply suggestion from @brkalow --- packages/clerk-js/src/core/clerk.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/clerk-js/src/core/clerk.ts b/packages/clerk-js/src/core/clerk.ts index f3a13254de6..7910c4bf317 100644 --- a/packages/clerk-js/src/core/clerk.ts +++ b/packages/clerk-js/src/core/clerk.ts @@ -483,7 +483,7 @@ export class Clerk implements ClerkInterface { const executeSignOut = async () => { const tracker = createBeforeUnloadTracker(this.#options.standardBrowser); - // Notify other tabs that user is signing out. + // Notify other tabs that user is signing out and clean up cookies. eventBus.emit(events.UserSignOut, null); this.#setTransitiveState();