Skip to content
Merged
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
5 changes: 5 additions & 0 deletions .changeset/fix-dev-browser-partitioned-cookie.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@clerk/clerk-js': patch
---

Fix dev browser token being read from a stale non-partitioned cookie when `partitionedCookies` is enabled. The token is now kept in memory so FAPI requests always use the authoritative value.
87 changes: 87 additions & 0 deletions integration/tests/dev-browser-partitioned-cookies.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
import { expect, test } from '@playwright/test';
import { parsePublishableKey } from '@clerk/shared/keys';

import { appConfigs } from '../presets';
import type { FakeUser } from '../testUtils';
import { createTestUtils, testAgainstRunningApps } from '../testUtils';

testAgainstRunningApps({ withEnv: [appConfigs.envs.withEmailCodes] })(
'dev browser partitioned cookies @generic',
({ app }) => {
test.describe.configure({ mode: 'serial' });

let fakeUser: FakeUser;

test.beforeAll(async () => {
const u = createTestUtils({ app });
fakeUser = u.services.users.createFakeUser();
await u.services.users.createBapiUser(fakeUser);
});

test.afterAll(async () => {
await fakeUser.deleteIfExists();
await app.teardown();
});

test('URL query param dev browser token takes precedence over existing partitioned cookie on initial load', async ({
page,
context,
}) => {
const pk = app.env.publicVariables.get('CLERK_PUBLISHABLE_KEY');
const { frontendApi } = parsePublishableKey(pk)!;
const fapiOrigin = `https://${frontendApi}`;

// Obtain a valid dev browser token directly from FAPI before any page load
const devBrowserRes = await page.request.post(`${fapiOrigin}/v1/dev_browser`);
expect(devBrowserRes.ok()).toBe(true);
const { id: freshToken } = await devBrowserRes.json();
expect(freshToken).toBeTruthy();

// Pre-set a stale __clerk_db_jwt cookie before the page ever loads.
// This simulates the partitioned cookie that already exists in the browser
// from a previous session.
const appUrl = new URL(app.serverUrl);
await context.addCookies([
{
name: '__clerk_db_jwt',
value: 'stale_partitioned_value',
domain: appUrl.hostname,
path: '/',
},
]);

// Collect every dev browser token attached to FAPI requests
const fapiTokens: string[] = [];
page.on('request', req => {
if (req.url().includes('__clerk_db_jwt') && req.url().includes('/v1/')) {
const url = new URL(req.url());
const token = url.searchParams.get('__clerk_db_jwt');
if (token) {
fapiTokens.push(token);
}
}
});

// Initial page load with the fresh token in the URL query param,
// simulating a redirect back from Clerk's Account Portal.
const signInUrl = new URL(app.serverUrl + '/sign-in');
signInUrl.searchParams.set('__clerk_db_jwt', freshToken);

await page.goto(signInUrl.toString());
await page.waitForLoadState('networkidle');

// Every FAPI request during initial load must use the URL token,
// not the stale partitioned cookie.
expect(fapiTokens.length).toBeGreaterThan(0);
for (const token of fapiTokens) {
expect(token).toBe(freshToken);
expect(token).not.toBe('stale_partitioned_value');
}

// Verify clerk-js is functional: sign in should succeed
const u = createTestUtils({ app, page, context });
await u.po.signIn.signInWithEmailAndInstantPassword({ email: fakeUser.email, password: fakeUser.password });
await u.po.expect.toBeSignedIn();
});
},
);
13 changes: 11 additions & 2 deletions packages/clerk-js/src/core/auth/devBrowser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,15 +37,22 @@ export function createDevBrowser({
}: CreateDevBrowserOptions): DevBrowser {
const devBrowserCookie = createDevBrowserCookie(cookieSuffix, cookieOptions);

// Hold the dev browser token in memory so it's always available to FAPI
// interceptors, even before Environment resolves and cookies can be written
// with the correct Partitioned attribute.
let devBrowserInMemory: string | undefined;
Copy link
Member Author

Choose a reason for hiding this comment

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

I can't think of a reason why this wouldn't be safe. Potentially if there are already multiple dev browser cookies and a different value would have been read from cookies, but that would be a separate issue for us.

  • Dev browser should be unique per device per instance
  • Using this in-memory value should actually be more efficient and avoid unnecessary cookie reads
  • Any time the dev browser cooke is mutated on the client this value is updated accordingly


function getDevBrowser() {
return devBrowserCookie.get();
return devBrowserInMemory || devBrowserCookie.get();
}

function setDevBrowser(devBrowser: string) {
devBrowserInMemory = devBrowser;
devBrowserCookie.set(devBrowser);
}

function removeDevBrowser() {
devBrowserInMemory = undefined;
devBrowserCookie.remove();
}

Expand Down Expand Up @@ -81,7 +88,9 @@ export function createDevBrowser({
}

// 2. If no dev browser is found in the first step, check if one is already available in the __clerk_db_jwt JS cookie
if (devBrowserCookie.get()) {
const existingDevBrowser = devBrowserCookie.get();
if (existingDevBrowser) {
devBrowserInMemory = existingDevBrowser;
return;
}

Expand Down
Loading