diff --git a/.changeset/express-forward-auth-options.md b/.changeset/express-forward-auth-options.md new file mode 100644 index 00000000000..d22f280fab6 --- /dev/null +++ b/.changeset/express-forward-auth-options.md @@ -0,0 +1,7 @@ +--- +"@clerk/express": patch +--- + +Forward all `AuthenticateRequestOptions` and `VerifyTokenOptions` passed to `clerkMiddleware()` through to the backend `authenticateRequest()` call. Previously only a hand-picked subset was forwarded, so options like `organizationSyncOptions`, `skipJwksCache`, and `headerType` were accepted by the TypeScript types but silently ignored at runtime — the same class of bug that caused `clockSkewInMs` to be dropped. + +Additionally, when `apiUrl` or `apiVersion` are passed to `clerkMiddleware()` and no custom `clerkClient` is supplied, the middleware now builds a per-middleware `ClerkClient` configured with those values instead of using the env-only default singleton. This is required because `@clerk/backend` pins `apiUrl`/`apiVersion` at client construction time and ignores runtime overrides on `authenticateRequest()`. Passing your own `clerkClient` continues to take precedence. diff --git a/packages/express/src/__tests__/clerkMiddleware.test.ts b/packages/express/src/__tests__/clerkMiddleware.test.ts index 4fb8688d067..82c8300dc37 100644 --- a/packages/express/src/__tests__/clerkMiddleware.test.ts +++ b/packages/express/src/__tests__/clerkMiddleware.test.ts @@ -1,3 +1,4 @@ +import type * as ClerkBackend from '@clerk/backend'; import type { Request, RequestHandler, Response } from 'express'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; @@ -12,7 +13,19 @@ vi.mock('@clerk/backend/proxy', async () => { }; }); -import { authenticateRequest } from '../authenticateRequest'; +const { mockCreateClerkClient } = vi.hoisted(() => ({ + mockCreateClerkClient: vi.fn(), +})); +vi.mock('@clerk/backend', async () => { + const actual = await vi.importActual('@clerk/backend'); + mockCreateClerkClient.mockImplementation(actual.createClerkClient); + return { + ...actual, + createClerkClient: mockCreateClerkClient, + }; +}); + +import { authenticateAndDecorateRequest, authenticateRequest } from '../authenticateRequest'; import { clerkMiddleware } from '../clerkMiddleware'; import { getAuth } from '../getAuth'; import { assertNoDebugHeaders, assertSignedOutDebugHeaders, runMiddleware, runMiddlewareOnPath } from './helpers'; @@ -125,6 +138,176 @@ describe('clerkMiddleware', () => { ); }); + it('forwards arbitrary AuthenticateRequestOptions/VerifyTokenOptions to authenticateRequest', async () => { + const authenticateRequestMock = vi.fn().mockResolvedValue({}); + const clerkClient = { + authenticateRequest: authenticateRequestMock, + } as any; + + const organizationSyncOptions = { + organizationPatterns: ['/orgs/:slug'], + }; + + await authenticateRequest({ + clerkClient, + request: { + method: 'GET', + url: '/', + headers: { + host: 'example.com', + }, + } as Request, + options: { + publishableKey: 'pk_test_Y2xlcmsuZXhhbXBsZS5jb20k', + secretKey: 'sk_test_....', + clockSkewInMs: 12_345, + audience: 'https://api.example.com', + authorizedParties: ['https://example.com'], + jwtKey: 'jwt-key-value', + acceptsToken: 'session_token', + organizationSyncOptions, + skipJwksCache: true, + headerType: 'JWT', + } as any, + }); + + expect(authenticateRequestMock).toHaveBeenCalledWith( + expect.any(Object), + expect.objectContaining({ + audience: 'https://api.example.com', + authorizedParties: ['https://example.com'], + clockSkewInMs: 12_345, + jwtKey: 'jwt-key-value', + acceptsToken: 'session_token', + organizationSyncOptions, + skipJwksCache: true, + headerType: 'JWT', + }), + ); + }); + + it('does not forward middleware-only options (clerkClient, debug, frontendApiProxy) to authenticateRequest', async () => { + const authenticateRequestMock = vi.fn().mockResolvedValue({}); + const clerkClient = { + authenticateRequest: authenticateRequestMock, + } as any; + + await authenticateRequest({ + clerkClient, + request: { + method: 'GET', + url: '/', + headers: { + host: 'example.com', + }, + } as Request, + options: { + publishableKey: 'pk_test_Y2xlcmsuZXhhbXBsZS5jb20k', + secretKey: 'sk_test_....', + clerkClient, + debug: true, + frontendApiProxy: { enabled: true, path: '/__clerk' }, + }, + }); + + const forwarded = authenticateRequestMock.mock.calls[0][1]; + expect(forwarded).not.toHaveProperty('clerkClient'); + expect(forwarded).not.toHaveProperty('debug'); + expect(forwarded).not.toHaveProperty('frontendApiProxy'); + }); + + describe('apiUrl/apiVersion default-client construction', () => { + beforeEach(() => { + mockCreateClerkClient.mockClear(); + }); + + it('builds a per-middleware ClerkClient with apiUrl when no custom clerkClient is supplied', () => { + authenticateAndDecorateRequest({ + apiUrl: 'https://api.example.test', + secretKey: 'sk_test_....', + publishableKey: 'pk_test_Y2xlcmsuZXhhbXBsZS5jb20k', + }); + + expect(mockCreateClerkClient).toHaveBeenCalledWith( + expect.objectContaining({ apiUrl: 'https://api.example.test' }), + ); + }); + + it('builds a per-middleware ClerkClient with apiVersion when no custom clerkClient is supplied', () => { + authenticateAndDecorateRequest({ + apiVersion: 'v2', + secretKey: 'sk_test_....', + publishableKey: 'pk_test_Y2xlcmsuZXhhbXBsZS5jb20k', + }); + + expect(mockCreateClerkClient).toHaveBeenCalledWith(expect.objectContaining({ apiVersion: 'v2' })); + }); + + it('does not call createClerkClient at construction when apiUrl/apiVersion are not set', () => { + authenticateAndDecorateRequest({ secretKey: 'sk_test_....' }); + + expect(mockCreateClerkClient).not.toHaveBeenCalled(); + }); + + it('does not build a per-middleware client when the caller supplies their own clerkClient', () => { + const customClient = { authenticateRequest: vi.fn() } as any; + + authenticateAndDecorateRequest({ + apiUrl: 'https://api.example.test', + apiVersion: 'v2', + clerkClient: customClient, + }); + + expect(mockCreateClerkClient).not.toHaveBeenCalled(); + }); + + it('routes outbound API traffic to the apiUrl override', async () => { + const fetchSpy = vi.spyOn(globalThis, 'fetch').mockResolvedValue( + new Response('{"data":[],"total_count":0}', { + status: 200, + headers: { 'content-type': 'application/json' }, + }), + ); + + authenticateAndDecorateRequest({ + apiUrl: 'https://api.example.test', + secretKey: 'sk_test_....', + publishableKey: 'pk_test_Y2xlcmsuZXhhbXBsZS5jb20k', + }); + + const client = mockCreateClerkClient.mock.results[0].value; + await client.users.getUserList().catch(() => undefined); + + const calledUrls = fetchSpy.mock.calls.map(call => { + const input = call[0]; + if (typeof input === 'string') { + return input; + } + if (input instanceof URL) { + return input.href; + } + return input.url; + }); + expect(calledUrls.some(url => new URL(url).origin === 'https://api.example.test')).toBe(true); + + fetchSpy.mockRestore(); + }); + + it('callback form: builds a per-middleware ClerkClient when the callback returns apiUrl', async () => { + await runMiddleware( + clerkMiddleware(() => ({ + apiUrl: 'https://api.example.test', + secretKey: 'sk_test_....', + publishableKey: 'pk_test_Y2xlcmsuZXhhbXBsZS5jb20k', + })), + ).expect(200); + + expect(mockCreateClerkClient).toHaveBeenCalledWith( + expect.objectContaining({ apiUrl: 'https://api.example.test' }), + ); + }); + }); + it('throws error if clerkMiddleware is not executed before getAuth', async () => { const customMiddleware: RequestHandler = (request, response, next) => { const auth = getAuth(request); diff --git a/packages/express/src/authenticateRequest.ts b/packages/express/src/authenticateRequest.ts index 1f8cd0f8ef1..606c80ddbd8 100644 --- a/packages/express/src/authenticateRequest.ts +++ b/packages/express/src/authenticateRequest.ts @@ -1,3 +1,4 @@ +import { createClerkClient } from '@clerk/backend'; import type { RequestState } from '@clerk/backend/internal'; import { AuthStatus, createClerkRequest } from '@clerk/backend/internal'; import { clerkFrontendApiProxy, DEFAULT_PROXY_PATH, stripTrailingSlashes } from '@clerk/backend/proxy'; @@ -24,20 +25,36 @@ import { incomingMessageToRequest, loadApiEnv, loadClientEnv, requestToProxyRequ */ export const authenticateRequest = (opts: AuthenticateRequestParams) => { const { clerkClient, request, options } = opts; - const { jwtKey, authorizedParties, audience, acceptsToken, clockSkewInMs } = options || {}; + // Peel off middleware-only keys and the few options that need middleware-side + // resolution (env fallbacks, URL normalization). Everything else is spread + // straight through, so new AuthenticateRequestOptions/VerifyTokenOptions + // fields flow to the backend without another code change here. + const { + clerkClient: _clerkClient, + debug: _debug, + frontendApiProxy: _frontendApiProxy, + isSatellite: isSatelliteInput, + domain: domainInput, + signInUrl: signInUrlInput, + proxyUrl: proxyUrlInput, + secretKey: secretKeyInput, + machineSecretKey: machineSecretKeyInput, + publishableKey: publishableKeyInput, + ...restOptions + } = options || {}; const clerkRequest = createClerkRequest(incomingMessageToRequest(request)); const env = { ...loadApiEnv(), ...loadClientEnv() }; - const secretKey = options?.secretKey || env.secretKey; - const machineSecretKey = options?.machineSecretKey || env.machineSecretKey; - const publishableKey = options?.publishableKey || env.publishableKey; + const secretKey = secretKeyInput || env.secretKey; + const machineSecretKey = machineSecretKeyInput || env.machineSecretKey; + const publishableKey = publishableKeyInput || env.publishableKey; - const isSatellite = handleValueOrFn(options?.isSatellite, clerkRequest.clerkUrl, env.isSatellite); - const domain = handleValueOrFn(options?.domain, clerkRequest.clerkUrl) || env.domain; - const signInUrl = options?.signInUrl || env.signInUrl; + const isSatellite = handleValueOrFn(isSatelliteInput, clerkRequest.clerkUrl, env.isSatellite); + const domain = handleValueOrFn(domainInput, clerkRequest.clerkUrl) || env.domain; + const signInUrl = signInUrlInput || env.signInUrl; const proxyUrl = absoluteProxyUrl( - handleValueOrFn(options?.proxyUrl, clerkRequest.clerkUrl, env.proxyUrl), + handleValueOrFn(proxyUrlInput, clerkRequest.clerkUrl, env.proxyUrl), clerkRequest.clerkUrl.toString(), ); @@ -50,18 +67,14 @@ export const authenticateRequest = (opts: AuthenticateRequestParams) => { } return clerkClient.authenticateRequest(clerkRequest, { - audience, + ...restOptions, secretKey, machineSecretKey, publishableKey, - jwtKey, - clockSkewInMs, - authorizedParties, proxyUrl, isSatellite, domain, signInUrl, - acceptsToken, }); }; @@ -99,8 +112,27 @@ const absoluteProxyUrl = (relativeOrAbsoluteUrl: string, baseUrl: string): strin return new URL(relativeOrAbsoluteUrl, baseUrl).toString(); }; +// `apiUrl` and `apiVersion` are pinned at client construction time inside +// `@clerk/backend`'s `createAuthenticateRequest` factory (build-time values +// override runtime ones). The default singleton in `./clerkClient` is built +// from env only, so passing these via `clerkMiddleware()` would be silently +// ignored. When the caller hasn't supplied their own `clerkClient` but did +// pass `apiUrl`/`apiVersion`, build a per-middleware client with those values. +const resolveDefaultClerkClient = (options: ClerkMiddlewareOptions) => { + if (!options.apiUrl && !options.apiVersion) { + return defaultClerkClient; + } + const env = { ...loadApiEnv(), ...loadClientEnv() }; + return createClerkClient({ + ...env, + ...(options.apiUrl ? { apiUrl: options.apiUrl } : {}), + ...(options.apiVersion ? { apiVersion: options.apiVersion } : {}), + userAgent: `${PACKAGE_NAME}@${PACKAGE_VERSION}`, + }); +}; + export const authenticateAndDecorateRequest = (options: ClerkMiddlewareOptions = {}): RequestHandler => { - const clerkClient = options.clerkClient || defaultClerkClient; + const clerkClient = options.clerkClient || resolveDefaultClerkClient(options); // Extract proxy configuration const frontendApiProxy = options.frontendApiProxy;