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. 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/__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'); diff --git a/packages/clerk-js/src/core/auth/cookies/devBrowser.ts b/packages/clerk-js/src/core/auth/cookies/devBrowser.ts index 7b292a9e656..47edda5fadd 100644 --- a/packages/clerk-js/src/core/auth/cookies/devBrowser.ts +++ b/packages/clerk-js/src/core/auth/cookies/devBrowser.ts @@ -12,6 +12,12 @@ export type DevBrowserCookieHandler = { remove: () => void; }; +const getCookieAttributes = (): { sameSite: string; secure: boolean } => { + const sameSite = inCrossOriginIframe() ? 'None' : 'Lax'; + 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 +32,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..7910c4bf317 100644 --- a/packages/clerk-js/src/core/clerk.ts +++ b/packages/clerk-js/src/core/clerk.ts @@ -483,10 +483,8 @@ 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); - // 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); }, }; }