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
12 changes: 12 additions & 0 deletions .changeset/lucky-pandas-observe.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
---
'@clerk/clerk-js': minor
'@clerk/shared': minor
---

Acquire an optional Protect session token and attach it to sign-in and sign-up requests.

On instances whose loader config references the new `{cid}` / `{pid}` / `{rid}` / `{instance_id}` / `{sdkver}` placeholders, Clerk mints an opaque, random correlation id and substitutes it into the loader's attributes and `textContent`. The loader is served with a signed session token, which is taken up once per browser session — shared across tabs under a lock rather than acquired once per tab — and travels alongside the correlation id and an acquisition status in the form-encoded body of sign-in and sign-up requests.

Acquisition can never block or fail a sign-in: it is bounded by a deadline, and when no token can be obtained a status (`no_token`, `timeout`, `script_error`, `fetch_error`, `unsupported`, `http_<n>`) travels in its place and the request proceeds unchanged. Only the loader carrying the correlation id is governed by the shared token; any other configured loader is still applied on every page load. Nothing is stored in the browser unless a loader references `{cid}`, `{pid}` or `{rid}`.

`ProtectLoader` gains two optional fields: `tokenTimeoutMs`, and `tokenUrl` for instances that opt into fetching the token from a dedicated endpoint instead of taking the one served with the loader.
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Comment on lines +6 to +12

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This describes the technical implementation, but it doesn't really explain to an end user reading the release changelog what it does.

I'd favor a two sentence summary of the capabilities this gives instead, no need for this level of technical detail here.

10 changes: 5 additions & 5 deletions packages/clerk-js/bundlewatch.config.json
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
{
"files": [
{ "path": "./dist/clerk.js", "maxSize": "549KB" },
{ "path": "./dist/clerk.browser.js", "maxSize": "77KB" },
{ "path": "./dist/clerk.legacy.browser.js", "maxSize": "119KB" },
{ "path": "./dist/clerk.no-rhc.js", "maxSize": "316KB" },
{ "path": "./dist/clerk.native.js", "maxSize": "77KB" },
{ "path": "./dist/clerk.js", "maxSize": "552KB" },
{ "path": "./dist/clerk.browser.js", "maxSize": "79KB" },
{ "path": "./dist/clerk.legacy.browser.js", "maxSize": "122KB" },
{ "path": "./dist/clerk.no-rhc.js", "maxSize": "320KB" },
{ "path": "./dist/clerk.native.js", "maxSize": "79KB" },
{ "path": "./dist/vendors*.js", "maxSize": "7KB" },
{ "path": "./dist/coinbase*.js", "maxSize": "36KB" },
{ "path": "./dist/base-account-sdk*.js", "maxSize": "207KB" },
Expand Down
101 changes: 101 additions & 0 deletions packages/clerk-js/src/core/__tests__/clerk.protect-params.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
import type { ProtectAssertion } from '@clerk/shared/types';
import { beforeEach, describe, expect, it, vi } from 'vitest';

import { Clerk } from '../clerk';

/**
* Two independent Protect features feed the single `getProtectParams` hook the FAPI client calls:
* the application-supplied assertion, and the server-configured session token. They are wired in
* the same expression, so collapsing it to either one alone still compiles, still type-checks, and
* silently stops sending the other's params — a degradation with nothing to see. These tests pin
* the hook to the union.
*/

const getRequestParams = vi.fn();

vi.mock('../protect', () => ({
Protect: class {
load = vi.fn();
getRequestParams = getRequestParams;
},
}));

const { capturedOptions } = vi.hoisted(() => ({ capturedOptions: { current: undefined as any } }));

vi.mock('../fapiClient', async importOriginal => {
const actual = await importOriginal<typeof import('../fapiClient')>();
return {
...actual,
createFapiClient: (options: any) => {
capturedOptions.current = options;
return actual.createFapiClient(options);
Comment on lines +23 to +31

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.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Type the FAPI mock options.

Lines 23 and 29 use any, so a createFapiClient contract change can bypass this integration test. Derive the captured value and mock parameter from Parameters<typeof createFapiClient>[0].

As per coding guidelines, “Avoid any type” and “No any types without justification in code review.”

🤖 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.protect-params.test.ts` around
lines 23 - 31, Replace the `any` annotations in the hoisted `capturedOptions`
state and the mocked `createFapiClient` parameter with `Parameters<typeof
createFapiClient>[0]`, importing or referencing `createFapiClient` as needed.
Preserve the existing capture-and-delegate behavior while ensuring the test
tracks contract changes.

Source: Coding guidelines

},
};
});

const productionPublishableKey = 'pk_live_Y2xlcmsuYWJjZWYuMTIzNDUucHJvZC5sY2xjbGVyay5jb20k';

const sessionParams = { __clerk_protect_token: 'v1.payload.mac', __clerk_protect_status: 'ok' };
const assertionParams = { __clerk_protect_assertion: 'token-abc' };

/** The hook a freshly constructed Clerk handed to the FAPI client. */
const hookFor = (assertion?: ProtectAssertion) => {
const clerk = new Clerk(productionPublishableKey);
if (assertion !== undefined) {
clerk.setProtectAssertion(assertion);
}
return capturedOptions.current.getProtectParams as () => Promise<Record<string, string | undefined> | undefined>;
};

describe('Clerk getProtectParams', () => {
beforeEach(() => {
getRequestParams.mockReset();
capturedOptions.current = undefined;
});

it('is wired into the FAPI client', () => {
expect(hookFor()).toBeTypeOf('function');
});

it('unions the assertion and the session token', async () => {
getRequestParams.mockResolvedValue(sessionParams);

await expect(hookFor('token-abc')()).resolves.toEqual({ ...assertionParams, ...sessionParams });
});

it('sends the session token when no assertion is configured', async () => {
getRequestParams.mockResolvedValue(sessionParams);

await expect(hookFor()()).resolves.toEqual(sessionParams);
});

it('sends the assertion when the session contributes nothing', async () => {
getRequestParams.mockResolvedValue(undefined);

await expect(hookFor('token-abc')()).resolves.toEqual(assertionParams);
});

// Returning `{}` would make every sign-in body differ from what it was before the feature existed.
it('resolves to undefined when neither contributes anything', async () => {
getRequestParams.mockResolvedValue(undefined);

await expect(hookFor()()).resolves.toBeUndefined();
});

// Neither feature may take the other down with it.
it('keeps the session token when the assertion resolver throws', async () => {
getRequestParams.mockResolvedValue(sessionParams);

await expect(
hookFor(() => {
throw new Error('boom');
})(),
).resolves.toEqual(sessionParams);
});

it('keeps the assertion when acquiring the session token rejects', async () => {
getRequestParams.mockRejectedValue(new DOMException('storage is blocked', 'SecurityError'));

await expect(hookFor('token-abc')()).resolves.toEqual(assertionParams);
});
});
Comment on lines +50 to +101

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

Test explicit assertion clearing.

Add a test that loads Clerk with protectAssertion, then calls setProtectAssertion(undefined). Assert that getProtectParams no longer returns the option assertion. The current tests cannot detect a regression where clearing falls back to this.#options.protectAssertion.

As per coding guidelines, “Unit tests are required for all new functionality” and tests must verify edge cases.

🤖 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.protect-params.test.ts` around
lines 50 - 101, Add a test covering explicit assertion clearing: initialize
Clerk with protectAssertion, call setProtectAssertion(undefined), then invoke
the getProtectParams hook and assert the cleared assertion is absent. Ensure the
test distinguishes the cleared runtime value from any fallback to
this.#options.protectAssertion.

Source: Coding guidelines

145 changes: 84 additions & 61 deletions packages/clerk-js/src/core/__tests__/fapiClient.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -385,67 +385,84 @@ describe('request', () => {
});

describe('Protect params', () => {
const protectParams = { __clerk_protect_assertion: 'token-abc' };
const clientWithProtect = createFapiClient({
...baseFapiClientOptions,
getProtectParams: () => Promise.resolve(protectParams),
// Two independent features feed this one hook — an application-supplied assertion and the
// server-configured session token — so the fixture carries params from both.
const protectParams = {
__clerk_protect_assertion: 'token-abc',
__clerk_protect_token: 'v1.payload.mac',
__clerk_protect_status: 'ok',
__clerk_protect_cid: `1-${'a'.repeat(26)}-${'b'.repeat(26)}`,
};
const expectedProtectQuery =
'__clerk_protect_assertion=token-abc&__clerk_protect_token=v1.payload.mac&__clerk_protect_status=ok' +
`&__clerk_protect_cid=${protectParams.__clerk_protect_cid}`;

let getProtectParams: Mock;
let clientWithProtect: ReturnType<typeof createFapiClient>;

beforeEach(() => {
getProtectParams = vi.fn().mockResolvedValue(protectParams);
clientWithProtect = createFapiClient({ ...baseFapiClientOptions, getProtectParams });
});

it.each([
['/client/sign_ins'],
['/client/sign_ins/sia_123/attempt_first_factor'],
['/client/sign_ups'],
['/client/sign_ups/sua_123/attempt_verification'],
])('attaches them to POST %s', async path => {
await clientWithProtect.request({ path, method: 'POST', body: { identifier: 'user@example.com' } as any });
const bodyOf = () => (fetch as Mock).mock.calls[0][1].body as string;

expect(fetch).toHaveBeenCalledWith(
expect.any(URL),
expect.objectContaining({
body: 'identifier=user%40example.com&__clerk_protect_assertion=token-abc',
}),
);
it.each([
'/client/sign_ins',
'/client/sign_ups',
'/client/sign_ins/sia_123/attempt_first_factor',
'/client/sign_ups/sua_123/attempt_verification',
])('merges them into the form-encoded body of %s', async path => {
await clientWithProtect.request({ path, method: 'POST', body: { identifier: 'nick@clerk.dev' } as any });

expect(bodyOf()).toBe(`identifier=nick%40clerk.dev&${expectedProtectQuery}`);
// A signed credential must never land in the URL, which is logged all along the path.
expect((fetch as Mock).mock.calls[0][0].toString()).not.toContain('__clerk_protect');
});

it('attaches them when the request has no body of its own', async () => {
await clientWithProtect.request({ path: '/client/sign_ins', method: 'POST' });
it('adds no request headers', async () => {
await clientWithProtect.request({ path: '/client/sign_ins', method: 'POST', body: {} as any });

expect(fetch).toHaveBeenCalledWith(
expect.any(URL),
expect.objectContaining({ body: '__clerk_protect_assertion=token-abc' }),
);
const headers = (fetch as Mock).mock.calls[0][1].headers as Headers;
expect([...headers.keys()]).toEqual(['content-type']);
});

// All lower-case, so the camel-to-snake body key encoder has nothing to rewrite.
it('does not mangle the param name', async () => {
await clientWithProtect.request({ path: '/client/sign_ins', method: 'POST' });
// Also pins the param names against the camel-to-snake body key encoder: they are all
// lower-case, so it has nothing to rewrite.
it('populates the body even when the request had none', async () => {
await clientWithProtect.request({ path: '/client/sign_ups', method: 'POST' });

const [, init] = (fetch as Mock).mock.calls.at(-1)!;
expect(init.body).toBe('__clerk_protect_assertion=token-abc');
expect(bodyOf()).toBe(expectedProtectQuery);
});

it.each([
['a GET', 'GET', '/client/sign_ins'],
['an unrelated path', 'POST', '/client/sessions'],
['a path that merely shares a prefix', 'POST', '/client/sign_ins_other'],
])('does not attach them to %s', async (_label, method, path) => {
await clientWithProtect.request({ path, method: method as any, body: { a: 'b' } as any });

const [, init] = (fetch as Mock).mock.calls.at(-1)!;
expect(init.body ?? '').not.toContain('__clerk_protect_assertion');
it.each(['/client', '/client/sessions', '/environment', '/client/sign_insomething', '/client/sign_ins_other'])(
'leaves %s alone',
async path => {
await clientWithProtect.request({ path, method: 'POST', body: { foo: 'bar' } as any });

expect(bodyOf()).toBe('foo=bar');
expect(getProtectParams).not.toHaveBeenCalled();
},
);

it('leaves GET requests alone', async () => {
await clientWithProtect.request({ path: '/client/sign_ins', method: 'GET' });

expect(getProtectParams).not.toHaveBeenCalled();
});

// Spreading a FormData would discard the caller's payload, so non-plain bodies are left alone.
it('leaves a FormData body untouched', async () => {
it('leaves a FormData body alone', async () => {
const formData = new FormData();
formData.append('identifier', 'user@example.com');
formData.append('identifier', 'nick@clerk.dev');

await clientWithProtect.request({ path: '/client/sign_ins', method: 'POST', body: formData });

expect(fetch).toHaveBeenCalledWith(expect.any(URL), expect.objectContaining({ body: formData }));
expect((fetch as Mock).mock.calls[0][1].body).toBe(formData);
expect(getProtectParams).not.toHaveBeenCalled();
});

it('leaves a string body untouched', async () => {
it('leaves a string body alone', async () => {
// text/plain keeps the form-urlencoded encoder out of it.
await clientWithProtect.request({
path: '/client/sign_ins',
Expand All @@ -454,38 +471,44 @@ describe('request', () => {
headers: { 'content-type': 'text/plain' },
});

expect(fetch).toHaveBeenCalledWith(expect.any(URL), expect.objectContaining({ body: 'raw string body' }));
expect(bodyOf()).toBe('raw string body');
expect(getProtectParams).not.toHaveBeenCalled();
});

// Protect may influence a sign-in but must never fail one.
it('sends the request unchanged when resolving the params rejects', async () => {
const failing = createFapiClient({
...baseFapiClientOptions,
getProtectParams: () => Promise.reject(new Error('boom')),
});
// Merging into any of these would spread away the caller's payload rather than add to it.
it.each([
['a Blob', () => new Blob(['payload'])],
['an array', () => [1, 2, 3]],
['a URLSearchParams', () => new URLSearchParams({ identifier: 'nick@clerk.dev' })],
])('leaves %s body alone', async (_label, makeBody) => {
await clientWithProtect.request({ path: '/client/sign_ins', method: 'POST', body: makeBody() as any });

expect(getProtectParams).not.toHaveBeenCalled();
expect(String((fetch as Mock).mock.calls[0][1].body)).not.toContain('__clerk_protect');
});

await expect(
failing.request({ path: '/client/sign_ins', method: 'POST', body: { identifier: 'a' } as any }),
).resolves.toBeTruthy();
it('sends nothing extra when the instance contributes no params', async () => {
getProtectParams.mockResolvedValue(undefined);

expect(fetch).toHaveBeenCalledWith(expect.any(URL), expect.objectContaining({ body: 'identifier=a' }));
});
await clientWithProtect.request({ path: '/client/sign_ins', method: 'POST', body: { foo: 'bar' } as any });

it('sends the request unchanged when there are no params', async () => {
const none = createFapiClient({
...baseFapiClientOptions,
getProtectParams: () => Promise.resolve(undefined),
});
expect(bodyOf()).toBe('foo=bar');
});

await none.request({ path: '/client/sign_ins', method: 'POST', body: { identifier: 'a' } as any });
it('still sends the request when resolving the params rejects', async () => {
getProtectParams.mockRejectedValue(new DOMException('storage is blocked', 'SecurityError'));

expect(fetch).toHaveBeenCalledWith(expect.any(URL), expect.objectContaining({ body: 'identifier=a' }));
// Protect can degrade a sign-in but must never fail one before it is even sent.
await expect(
clientWithProtect.request({ path: '/client/sign_ins', method: 'POST', body: { foo: 'bar' } as any }),
).resolves.toBeDefined();
expect(bodyOf()).toBe('foo=bar');
});

it('is inert when no hook is configured', async () => {
await fapiClient.request({ path: '/client/sign_ins', method: 'POST', body: { identifier: 'a' } as any });

expect(fetch).toHaveBeenCalledWith(expect.any(URL), expect.objectContaining({ body: 'identifier=a' }));
expect(bodyOf()).toBe('identifier=a');
});
});

Expand Down
Loading
Loading