feat(js): attach an optional server-configured session token to sign-in (#9299)

Author: zourzouvillys

.changeset/lucky-pandas-observe.md

@@ -0,0 +1,6 @@
+---
+'@clerk/clerk-js': minor
+'@clerk/shared': minor
+---
+
+Internal improvements to Clerk Protect. No action is required, and instances that do not use Protect are unaffected.

packages/clerk-js/bundlewatch.config.json

@@ -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" },

packages/clerk-js/src/core/__tests__/clerk.protect-params.test.ts

@@ -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);
+    },
+  };
+});
+
+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);
+  });
+});

packages/clerk-js/src/core/__tests__/fapiClient.test.ts

@@ -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',
@@ -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');
     });
   });