Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions .changeset/safari-itp-sign-out-cookie-refresh.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
---
'@clerk/clerk-js': patch
---

Apply the Safari ITP cookie refresh on sign-out.

Signing out re-issues the client cookie from a fetch, which Safari's ITP caps at 7 days. The client outlives sign-out, and it is what Client Trust uses to recognize a known device, so the cap meant a user who signed out and came back more than a week later was treated as being on a new device and challenged for a second factor.

`signOut()` now routes its redirect through `/v1/client/touch` when the client cookie is close to expiring, the same workaround `setActive()` already applies after sign-in. The redirect is left unchanged when no refresh is needed.

This covers `signOut()` calls that navigate to a redirect URL, including `<UserButton />` and `<SignOutButton />`. Passing your own callback to `signOut(callback)` still navigates on your behalf and is not decorated.
41 changes: 41 additions & 0 deletions packages/clerk-js/src/core/__tests__/clerk.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1169,6 +1169,13 @@ describe('Clerk singleton', () => {
const mockSession1 = { id: '1', remove: vi.fn(), status: 'active', user: {}, getToken: vi.fn() };
const mockSession2 = { id: '2', remove: vi.fn(), status: 'active', user: {}, getToken: vi.fn() };
const mockSession3 = { id: '3', remove: vi.fn(), status: 'pending', user: {}, getToken: vi.fn() };
// Sign-out consults these to decide whether to route the redirect through
// /v1/client/touch. Default to "no refresh needed" so the existing cases
// assert against the plain redirect URL.
const clientTouchDefaults = {
isEligibleForTouch: () => false,
buildTouchUrl: () => 'https://clerk.example.com/v1/client/touch',
};

beforeEach(() => {
mockClientDestroy.mockReset();
Expand Down Expand Up @@ -1198,6 +1205,7 @@ describe('Clerk singleton', () => {
it('signs out all sessions if no sessionId is passed and multiple sessions have authenticated status', async () => {
mockClientFetch.mockReturnValue(
Promise.resolve({
...clientTouchDefaults,
signedInSessions: [mockSession1, mockSession2, mockSession3],
sessions: [mockSession1, mockSession2, mockSession3],
destroy: mockClientDestroy,
Expand All @@ -1221,6 +1229,7 @@ describe('Clerk singleton', () => {
async status => {
mockClientFetch.mockReturnValue(
Promise.resolve({
...clientTouchDefaults,
signedInSessions: [{ ...mockSession1, status }],
sessions: [{ ...mockSession1, status }],
destroy: mockClientDestroy,
Expand All @@ -1244,6 +1253,7 @@ describe('Clerk singleton', () => {
it('only removes the session that corresponds to the passed sessionId if it is not the current', async () => {
mockClientFetch.mockReturnValue(
Promise.resolve({
...clientTouchDefaults,
signedInSessions: [mockSession1, mockSession2, mockSession3],
sessions: [mockSession1, mockSession2, mockSession3],
destroy: mockClientDestroy,
Expand All @@ -1264,6 +1274,7 @@ describe('Clerk singleton', () => {
it('removes and signs out the session that corresponds to the passed sessionId if it is the current', async () => {
mockClientFetch.mockReturnValue(
Promise.resolve({
...clientTouchDefaults,
signedInSessions: [mockSession1, mockSession2, mockSession3],
sessions: [mockSession1, mockSession2, mockSession3],
destroy: mockClientDestroy,
Expand All @@ -1284,6 +1295,7 @@ describe('Clerk singleton', () => {
it('removes and signs out the session and redirects to the provided redirectUrl ', async () => {
mockClientFetch.mockReturnValue(
Promise.resolve({
...clientTouchDefaults,
signedInSessions: [mockSession1, mockSession2, mockSession3],
sessions: [mockSession1, mockSession2, mockSession3],
destroy: mockClientDestroy,
Expand All @@ -1300,6 +1312,35 @@ describe('Clerk singleton', () => {
expect(sut.navigate).toHaveBeenCalledWith('/after-sign-out');
});
});

it('routes the redirect through /v1/client/touch when the client cookie is close to expiring', async () => {
mockClientFetch.mockReturnValue(
Promise.resolve({
signedInSessions: [mockSession1],
sessions: [mockSession1],
destroy: mockClientDestroy,
removeSessions: mockClientRemoveSessions,
isEligibleForTouch: () => true,
buildTouchUrl: ({ redirectUrl }: { redirectUrl: URL }) =>
`https://clerk.example.com/v1/client/touch?redirect_url=${redirectUrl.toString()}`,
}),
);

const sut = new Clerk(productionPublishableKey);
sut.navigate = vi.fn();
await sut.load();
await sut.signOut({ redirectUrl: '/after-sign-out' });

await waitFor(() => {
expect(mockClientRemoveSessions).toHaveBeenCalled();
const navigatedTo = new URL((sut.navigate as ReturnType<typeof vi.fn>).mock.calls[0][0]);
expect(navigatedTo.pathname).toEqual('/v1/client/touch');
// The user still lands on the original destination, via the touch redirect.
expect(navigatedTo.searchParams.get('redirect_url')).toEqual(
new URL('/after-sign-out', mockWindowLocation.href).toString(),
);
Comment on lines +1324 to +1341

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Encode nested redirect URLs in the test mock.

The mock interpolates redirectUrl.toString() directly into the query, unlike Client.buildTouchUrl(). A destination containing ? or & would be parsed incorrectly, so this test does not cover the redirect-preservation contract.

Suggested test adjustment
-          buildTouchUrl: ({ redirectUrl }: { redirectUrl: URL }) =>
-            `https://clerk.example.com/v1/client/touch?redirect_url=${redirectUrl.toString()}`,
+          buildTouchUrl: ({ redirectUrl }: { redirectUrl: URL }) => {
+            const touchUrl = new URL('https://clerk.example.com/v1/client/touch');
+            touchUrl.searchParams.set('redirect_url', redirectUrl.toString());
+            return touchUrl.toString();
+          },
...
-      await sut.signOut({ redirectUrl: '/after-sign-out' });
+      await sut.signOut({ redirectUrl: '/after-sign-out?tab=security#settings' });
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/clerk-js/src/core/__tests__/clerk.test.ts` around lines 1324 - 1341,
The buildTouchUrl mock in the Clerk sign-out test must URL-encode the nested
redirect URL, matching Client.buildTouchUrl(). Update the mock around
buildTouchUrl to encode redirectUrl.toString() before placing it in the
redirect_url query parameter, while preserving the existing navigation
assertions and destination value.

});
});
});

describe('.navigate(to)', () => {
Expand Down
24 changes: 23 additions & 1 deletion packages/clerk-js/src/core/clerk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -656,6 +656,28 @@ export class Clerk implements ClerkInterface {
return Boolean(!this.#options.signUpUrl && this.#options.signInUrl && !isAbsoluteUrl(this.#options.signInUrl));
}

/**
* Routes the post-sign-out navigation through `/v1/client/touch` when the client
* cookie is close to expiring, and returns `redirectUrl` unchanged otherwise.
*
* In practice this only adds a redirect on Safari. Signing out re-issues the client
* cookie from a fetch, which ITP caps at 7 days; every other browser gets 400 days
* and is never eligible. The cap matters because the client outlives sign-out and is
* what Client Trust uses to recognize a known device, so a user returning a week
* later would be challenged for a second factor. Touch is a top-level navigation,
* which ITP does not cap, so it restores the full lifetime.
*/
#buildSignOutRedirectUrl(redirectUrl: string): string {
// In React Native, window exists but window.location does not, so the redirect
// cannot be resolved to an absolute URL. ITP does not apply there either.
if (!inBrowser() || typeof window.location === 'undefined' || !this.client?.isEligibleForTouch()) {
return redirectUrl;
}

const absoluteRedirectUrl = new URL(redirectUrl, window.location.href);
return this.buildUrlWithAuth(this.client.buildTouchUrl({ redirectUrl: absoluteRedirectUrl }));
}

public signOut: SignOut = async (callbackOrOptions?: SignOutCallback | SignOutOptions, options?: SignOutOptions) => {
if (!this.client || this.client.sessions.length === 0) {
return;
Expand Down Expand Up @@ -696,7 +718,7 @@ export class Clerk implements ClerkInterface {
if (signOutCallback) {
await signOutCallback();
} else {
await this.navigate(redirectUrl);
await this.navigate(this.#buildSignOutRedirectUrl(redirectUrl));
}
});

Expand Down
Loading