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/yellow-vans-walk.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@clerk/backend': patch
---

Fix logic for forcing a session sync on cross origin requests.
247 changes: 247 additions & 0 deletions packages/backend/src/tokens/__tests__/request.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1520,6 +1520,31 @@ describe('tokens.authenticateRequest(options)', () => {
});
});

test('does not trigger handshake when referer is same origin', async () => {
const request = mockRequestWithCookies(
{
host: 'localhost:3000',
referer: 'http://localhost:3000',
'sec-fetch-dest': 'document',
},
{
__clerk_db_jwt: mockJwt,
__session: mockJwt,
__client_uat: '12345',
},
'http://localhost:3000',
);

const requestState = await authenticateRequest(request, {
...mockOptions(),
signInUrl: 'http://localhost:3000/sign-in',
});

expect(requestState).toBeSignedIn({
signInUrl: 'http://localhost:3000/sign-in',
});
});

test('does not trigger handshake when no referer header', async () => {
const request = mockRequestWithCookies(
{
Expand Down Expand Up @@ -1605,5 +1630,227 @@ describe('tokens.authenticateRequest(options)', () => {
signInUrl: 'https://primary.com/sign-in',
});
});

test('does not trigger handshake when referer is from production accounts portal', async () => {
const request = mockRequestWithCookies(
{
referer: 'https://accounts.example.com/sign-in',
'sec-fetch-dest': 'document',
'sec-fetch-site': 'cross-site',
},
{
__session: mockJwt,
__client_uat: '12345',
},
'https://primary.com/dashboard',
);

const requestState = await authenticateRequest(request, {
...mockOptions(),
publishableKey: PK_LIVE,
domain: 'primary.com',
isSatellite: false,
signInUrl: 'https://primary.com/sign-in',
});

expect(requestState).toBeSignedIn({
domain: 'primary.com',
isSatellite: false,
signInUrl: 'https://primary.com/sign-in',
});
});

Comment on lines +1633 to +1662
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Test currently passes due to permissive “accounts.*” fallback; align with real Clerk host.

This case uses https://accounts.example.com/..., which is not derivable from the publishable key in mockOptions(). It only passes because production code whitelists any accounts.* host. If we tighten matching (recommended), this test will fail.

Change the referer to the expected accounts origin derived from the PK used in tests (e.g., accounts.inspired.puma-74.lcl.dev) or compute it from the PK to avoid coupling to the permissive fallback.

-          referer: 'https://accounts.example.com/sign-in',
+          referer: 'https://accounts.inspired.puma-74.lcl.dev/sign-in',

Alternatively, if you want to exercise a truly “prod-style” hostname, derive it from buildAccountsBaseUrl(frontendApi) seeded by the parsed PK rather than hardcoding example.com.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
test('does not trigger handshake when referer is from production accounts portal', async () => {
const request = mockRequestWithCookies(
{
referer: 'https://accounts.example.com/sign-in',
'sec-fetch-dest': 'document',
'sec-fetch-site': 'cross-site',
},
{
__session: mockJwt,
__client_uat: '12345',
},
'https://primary.com/dashboard',
);
const requestState = await authenticateRequest(request, {
...mockOptions(),
publishableKey: PK_LIVE,
domain: 'primary.com',
isSatellite: false,
signInUrl: 'https://primary.com/sign-in',
});
expect(requestState).toBeSignedIn({
domain: 'primary.com',
isSatellite: false,
signInUrl: 'https://primary.com/sign-in',
});
});
test('does not trigger handshake when referer is from production accounts portal', async () => {
const request = mockRequestWithCookies(
{
referer: 'https://accounts.inspired.puma-74.lcl.dev/sign-in',
'sec-fetch-dest': 'document',
'sec-fetch-site': 'cross-site',
},
{
__session: mockJwt,
__client_uat: '12345',
},
'https://primary.com/dashboard',
);
const requestState = await authenticateRequest(request, {
...mockOptions(),
publishableKey: PK_LIVE,
domain: 'primary.com',
isSatellite: false,
signInUrl: 'https://primary.com/sign-in',
});
expect(requestState).toBeSignedIn({
domain: 'primary.com',
isSatellite: false,
signInUrl: 'https://primary.com/sign-in',
});
});
🤖 Prompt for AI Agents
In packages/backend/src/tokens/__tests__/request.test.ts around lines 1608 to
1637, the test hardcodes referer "https://accounts.example.com/..." which only
passes because production code currently allows any accounts.* host; update the
test to use the actual accounts origin derived from the publishable key in
mockOptions() (e.g., call the same helper used in production like
buildAccountsBaseUrl(frontendApi) or construct
"https://accounts.<frontendApi-derived-host>") so the referer matches the PK
used in the test instead of relying on the permissive accounts.* fallback.

test('does not trigger handshake when referer is from dev accounts portal (current format)', async () => {
const request = mockRequestWithCookies(
{
referer: 'https://foo-bar-13.accounts.dev/sign-in',
'sec-fetch-dest': 'document',
'sec-fetch-site': 'cross-site',
},
{
__session: mockJwt,
__client_uat: '12345',
},
'https://primary.com/dashboard',
);

const requestState = await authenticateRequest(request, {
...mockOptions(),
publishableKey: PK_LIVE,
domain: 'primary.com',
isSatellite: false,
signInUrl: 'https://primary.com/sign-in',
});

expect(requestState).toBeSignedIn({
domain: 'primary.com',
isSatellite: false,
signInUrl: 'https://primary.com/sign-in',
});
});

test('does not trigger handshake when referer is from dev accounts portal (legacy format)', async () => {
const request = mockRequestWithCookies(
{
referer: 'https://accounts.foo-bar-13.lcl.dev/sign-in',
'sec-fetch-dest': 'document',
'sec-fetch-site': 'cross-site',
},
{
__session: mockJwt,
__client_uat: '12345',
},
'https://primary.com/dashboard',
);

const requestState = await authenticateRequest(request, {
...mockOptions(),
publishableKey: PK_LIVE,
domain: 'primary.com',
isSatellite: false,
signInUrl: 'https://primary.com/sign-in',
});

expect(requestState).toBeSignedIn({
domain: 'primary.com',
isSatellite: false,
signInUrl: 'https://primary.com/sign-in',
});
});

test('does not trigger cross-origin handshake when referer is from expected accounts portal derived from frontend API', async () => {
const request = mockRequestWithCookies(
{
referer: 'https://accounts.inspired.puma-74.lcl.dev/sign-in',
'sec-fetch-dest': 'document',
'sec-fetch-site': 'cross-site',
},
{
__session: mockJwt,
__client_uat: '12345',
},
'https://primary.com/dashboard',
);

const requestState = await authenticateRequest(request, {
...mockOptions(),
domain: 'primary.com',
isSatellite: false,
signInUrl: 'https://primary.com/sign-in',
});

// Should not trigger the specific cross-origin sync handshake we're trying to prevent
expect(requestState.reason).not.toBe(AuthErrorReason.PrimaryDomainCrossOriginSync);
});
Comment on lines +1721 to +1744
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add a negative test to prevent regressions: “accounts.attacker.com” must still trigger cross-origin handshake.

To guard against over-broad whitelisting, add a case where the referer is an unrelated accounts.* domain and assert that PrimaryDomainCrossOriginSync is triggered.

@@
   describe('Cross-origin sync', () => {
@@
+    test('triggers handshake when referer is unrelated accounts.* domain', async () => {
+      const request = mockRequestWithCookies(
+        {
+          referer: 'https://accounts.attacker.com/signin',
+          'sec-fetch-dest': 'document',
+          'sec-fetch-site': 'cross-site',
+        },
+        {
+          __session: mockJwt,
+          __client_uat: '12345',
+        },
+        'https://primary.com/dashboard',
+      );
+
+      const requestState = await authenticateRequest(request, {
+        ...mockOptions(),
+        publishableKey: PK_LIVE,
+        domain: 'primary.com',
+        isSatellite: false,
+        signInUrl: 'https://primary.com/sign-in',
+      });
+
+      expect(requestState).toMatchHandshake({
+        reason: AuthErrorReason.PrimaryDomainCrossOriginSync,
+        domain: 'primary.com',
+        signInUrl: 'https://primary.com/sign-in',
+      });
+    });
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
test('does not trigger cross-origin handshake when referer is from expected accounts portal derived from frontend API', async () => {
const request = mockRequestWithCookies(
{
referer: 'https://accounts.inspired.puma-74.lcl.dev/sign-in',
'sec-fetch-dest': 'document',
'sec-fetch-site': 'cross-site',
},
{
__session: mockJwt,
__client_uat: '12345',
},
'https://primary.com/dashboard',
);
const requestState = await authenticateRequest(request, {
...mockOptions(),
domain: 'primary.com',
isSatellite: false,
signInUrl: 'https://primary.com/sign-in',
});
// Should not trigger the specific cross-origin sync handshake we're trying to prevent
expect(requestState.reason).not.toBe(AuthErrorReason.PrimaryDomainCrossOriginSync);
});
describe('Cross-origin sync', () => {
test('does not trigger cross-origin handshake when referer is from expected accounts portal derived from frontend API', async () => {
const request = mockRequestWithCookies(
{
referer: 'https://accounts.inspired.puma-74.lcl.dev/sign-in',
'sec-fetch-dest': 'document',
'sec-fetch-site': 'cross-site',
},
{
__session: mockJwt,
__client_uat: '12345',
},
'https://primary.com/dashboard',
);
const requestState = await authenticateRequest(request, {
...mockOptions(),
domain: 'primary.com',
isSatellite: false,
signInUrl: 'https://primary.com/sign-in',
});
// Should not trigger the specific cross-origin sync handshake we're trying to prevent
expect(requestState.reason).not.toBe(AuthErrorReason.PrimaryDomainCrossOriginSync);
});
test('triggers handshake when referer is unrelated accounts.* domain', async () => {
const request = mockRequestWithCookies(
{
referer: 'https://accounts.attacker.com/signin',
'sec-fetch-dest': 'document',
'sec-fetch-site': 'cross-site',
},
{
__session: mockJwt,
__client_uat: '12345',
},
'https://primary.com/dashboard',
);
const requestState = await authenticateRequest(request, {
...mockOptions(),
publishableKey: PK_LIVE,
domain: 'primary.com',
isSatellite: false,
signInUrl: 'https://primary.com/sign-in',
});
expect(requestState).toMatchHandshake({
reason: AuthErrorReason.PrimaryDomainCrossOriginSync,
domain: 'primary.com',
signInUrl: 'https://primary.com/sign-in',
});
});
});
🤖 Prompt for AI Agents
In packages/backend/src/tokens/__tests__/request.test.ts around lines 1696 to
1719, add a negative test case that ensures an unrelated accounts.* origin still
triggers the cross-origin handshake: create a request similar to the existing
test but with referer 'https://accounts.attacker.com/sign-in' (keep
'sec-fetch-site': 'cross-site', cookies same, and origin
'https://primary.com/dashboard'), call authenticateRequest with the same
mockOptions (domain: 'primary.com', isSatellite: false, signInUrl:
'https://primary.com/sign-in'), and assert that requestState.reason ===
AuthErrorReason.PrimaryDomainCrossOriginSync to prevent over-broad whitelisting.


test('does not trigger handshake when referer is from FAPI domain (redirect-based auth)', async () => {
const request = mockRequestWithCookies(
{
referer: 'https://clerk.inspired.puma-74.lcl.dev/v1/client/sign_ins/12345/attempt_first_factor',
'sec-fetch-dest': 'document',
'sec-fetch-site': 'cross-site',
},
{
__session: mockJwt,
__client_uat: '12345',
},
'https://primary.com/dashboard',
);

const requestState = await authenticateRequest(request, {
...mockOptions(),
domain: 'primary.com',
isSatellite: false,
signInUrl: 'https://primary.com/sign-in',
});

// Should not trigger the specific cross-origin sync handshake we're trying to prevent
expect(requestState.reason).not.toBe(AuthErrorReason.PrimaryDomainCrossOriginSync);
});

test('does not trigger handshake when referer is from FAPI domain with https prefix', async () => {
const request = mockRequestWithCookies(
{
referer: 'https://clerk.inspired.puma-74.lcl.dev/sign-in',
'sec-fetch-dest': 'document',
'sec-fetch-site': 'cross-site',
},
{
__session: mockJwt,
__client_uat: '12345',
},
'https://primary.com/dashboard',
);

const requestState = await authenticateRequest(request, {
...mockOptions(),
domain: 'primary.com',
isSatellite: false,
signInUrl: 'https://primary.com/sign-in',
});

// Should not trigger the specific cross-origin sync handshake we're trying to prevent
expect(requestState.reason).not.toBe(AuthErrorReason.PrimaryDomainCrossOriginSync);
});

test('still triggers handshake for legitimate cross-origin requests from non-accounts domains', async () => {
const request = mockRequestWithCookies(
{
referer: 'https://satellite.com/sign-in',
'sec-fetch-dest': 'document',
'sec-fetch-site': 'cross-site',
},
{
__session: mockJwt,
__client_uat: '12345',
},
'https://primary.com/dashboard',
);

const requestState = await authenticateRequest(request, {
...mockOptions(),
publishableKey: PK_LIVE,
domain: 'primary.com',
isSatellite: false,
signInUrl: 'https://primary.com/sign-in',
});

expect(requestState).toMatchHandshake({
reason: AuthErrorReason.PrimaryDomainCrossOriginSync,
domain: 'primary.com',
signInUrl: 'https://primary.com/sign-in',
});
});

test('does not trigger handshake when referrer matches current origin despite sec-fetch-site cross-site (redirect chain)', async () => {
const request = mockRequestWithCookies(
{
host: 'primary.com',
referer: 'https://primary.com/some-page',
'sec-fetch-dest': 'document',
'sec-fetch-site': 'cross-site', // This can happen due to redirect chains through Clerk domains
},
{
__session: mockJwt,
__client_uat: '12345',
},
'https://primary.com/dashboard',
);

const requestState = await authenticateRequest(request, {
...mockOptions(),
publishableKey: PK_LIVE,
domain: 'primary.com',
isSatellite: false,
signInUrl: 'https://primary.com/sign-in',
});

// Should not trigger handshake because referrer origin matches current origin
expect(requestState).toBeSignedIn({
domain: 'primary.com',
isSatellite: false,
signInUrl: 'https://primary.com/sign-in',
});
});
});
});
56 changes: 52 additions & 4 deletions packages/backend/src/tokens/authenticateContext.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { buildAccountsBaseUrl } from '@clerk/shared/buildAccountsBaseUrl';
import { isCurrentDevAccountPortalOrigin, isLegacyDevAccountPortalOrigin } from '@clerk/shared/url';
import type { Jwt } from '@clerk/types';

import { constants } from '../constants';
Expand Down Expand Up @@ -186,10 +188,6 @@ class AuthenticateContext implements AuthenticateContext {
}

try {
if (this.getHeader(constants.Headers.SecFetchSite) === 'cross-site') {
return true;
}

const referrerOrigin = new URL(this.referrer).origin;
return referrerOrigin !== this.clerkUrl.origin;
} catch {
Expand All @@ -198,6 +196,56 @@ class AuthenticateContext implements AuthenticateContext {
}
}

/**
* Determines if the referrer URL is from a Clerk domain (accounts portal or FAPI).
* This includes both development and production account portal domains, as well as FAPI domains
* used for redirect-based authentication flows.
*
* @returns {boolean} True if the referrer is from a Clerk accounts portal or FAPI domain, false otherwise
*/
public isKnownClerkReferrer(): boolean {
if (!this.referrer) {
return false;
}

try {
const referrerOrigin = new URL(this.referrer);
const referrerHost = referrerOrigin.hostname;

// Check if referrer is the FAPI domain itself (redirect-based auth flows)
if (this.frontendApi) {
const fapiHost = this.frontendApi.startsWith('http') ? new URL(this.frontendApi).hostname : this.frontendApi;
if (referrerHost === fapiHost) {
return true;
}
}

// Check for development account portal patterns
if (isLegacyDevAccountPortalOrigin(referrerHost) || isCurrentDevAccountPortalOrigin(referrerHost)) {
return true;
}

// Check for production account portal by comparing with expected accounts URL
const expectedAccountsUrl = buildAccountsBaseUrl(this.frontendApi);
if (expectedAccountsUrl) {
const expectedAccountsOrigin = new URL(expectedAccountsUrl).origin;
if (referrerOrigin.origin === expectedAccountsOrigin) {
return true;
}
}

// Check for generic production accounts patterns (accounts.*)
if (referrerHost.startsWith('accounts.')) {
return true;
}

return false;
} catch {
// Invalid URL format
return false;
}
}

private initPublishableKeyValues(options: AuthenticateRequestOptions) {
assertValidPublishableKey(options.publishableKey);
this.publishableKey = options.publishableKey;
Expand Down
3 changes: 2 additions & 1 deletion packages/backend/src/tokens/request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -576,7 +576,8 @@ export const authenticateRequest: AuthenticateRequest = (async (
const shouldForceHandshakeForCrossDomain =
!authenticateContext.isSatellite && // We're on primary
authenticateContext.secFetchDest === 'document' && // Document navigation
authenticateContext.isCrossOriginReferrer(); // Came from different domain
authenticateContext.isCrossOriginReferrer() && // Came from different domain
!authenticateContext.isKnownClerkReferrer(); // Not from Clerk accounts portal or FAPI

if (shouldForceHandshakeForCrossDomain) {
return handleMaybeHandshakeStatus(
Expand Down
Loading