fix: don't follow redirects on API requests

src/common/net/fetch-client.spec.ts

@@ -1122,6 +1122,56 @@ describe('request timeout covers the response body (GH-1679)', () => {
       expect(requestCount).toBe(1);
     });
 
+    it('does not follow a redirect, so the request body is never re-sent', async () => {
+      handler = (req, res) => {
+        if (req.url === '/stolen') {
+          res.writeHead(200, { 'content-type': 'application/json' });
+          res.end('{}');
+          return;
+        }
+        res.writeHead(307, { location: `${baseURL}/stolen` });
+        res.end();
+      };
+      // Default timeout: the shared 100ms one can retry a slow first request.
+      const client = new FetchHttpClient(baseURL, { maxRetries: 0 }, fetchImpl);
+
+      await expect(
+        client.post(
+          '/user_management/authenticate',
+          { client_secret: 'sk_test' },
+          {},
+        ),
+      ).rejects.toMatchObject({
+        name: 'HttpClientError',
+        message: expect.stringContaining(`redirect (307 to ${baseURL}/stolen)`),
+        response: { status: 307 },
+      });
+      expect(requestCount).toBe(1);
+    });
+
+    it('releases the connection when a redirect body stalls instead of waiting on it', async () => {
+      let connectionClosed!: Promise<void>;
+      handler = (_req, res) => {
+        connectionClosed = new Promise((resolve) =>
+          res.on('close', () => resolve()),
+        );
+        res.writeHead(307, { location: `${baseURL}/elsewhere` });
+        res.write('<html>');
+        // Hold the body open; the client must not read it.
+      };
+      const client = new FetchHttpClient(baseURL, { maxRetries: 0 }, fetchImpl);
+
+      await expect(client.get('/users', {})).rejects.toMatchObject({
+        name: 'HttpClientError',
+        message: expect.stringContaining('redirect (307'),
+        response: { status: 307 },
+      });
+      // The request was aborted, so the server sees the connection drop
+      // rather than the SDK sitting on the deadline.
+      await connectionClosed;
+      expect(requestCount).toBe(1);
+    });
+
     it('does not retry a successful body that times out, even when retries are enabled', async () => {
       stallBody(200);
       const { client } = createClient(2);

src/common/net/fetch-client.ts

@@ -206,6 +206,8 @@ export class FetchHttpClient extends HttpClient implements HttpClientInterface {
       });
     // Set once the headers arrive.
     let res: Response | undefined;
+    // Set when the response is a redirect, whose body is never read.
+    let redirected = false;
 
     try {
       res = await this._fetchFn(url, {
@@ -219,8 +221,36 @@ export class FetchHttpClient extends HttpClient implements HttpClientInterface {
         },
         body: requestBody,
         signal: abortController.signal,
+        // The API never redirects. Following one would re-send the body (which
+        // can carry client_secret) to the redirect target. 'manual' rather than
+        // 'error': Cloudflare workerd rejects 'error'.
+        redirect: 'manual',
       });
 
+      // Browsers report a manual redirect as an opaque response with status 0.
+      if (
+        (res.status >= 300 && res.status < 400) ||
+        res.type === 'opaqueredirect'
+      ) {
+        clearTimeout(timeoutId);
+        redirected = true;
+        const location = res.headers.get('Location');
+        // An opaque redirect hides both the status and the Location.
+        const what =
+          res.type === 'opaqueredirect'
+            ? 'an opaque redirect'
+            : `a redirect (${res.status}${location ? ` to ${location}` : ''})`;
+        const message = `WorkOS API responded with ${what}. Redirects are not followed; check the apiHostname, port and https options.`;
+        throw new HttpClientError({
+          message,
+          response: {
+            status: res.status,
+            headers: res.headers,
+            data: { message },
+          },
+        });
+      }
+
       if (!res.ok) {
         const requestID = res.headers.get('X-Request-ID') ?? '';
         // Read the error body under the same deadline, inside the attempt,
@@ -288,6 +318,16 @@ export class FetchHttpClient extends HttpClient implements HttpClientInterface {
       }
 
       throw error;
+    } finally {
+      // Release the unread redirect body and its connection. Reading it under
+      // the deadline would turn a stalled body into a retried 408 that hides
+      // the redirect, and `body.cancel()` only exists on web streams, whereas
+      // every fetch implementation honors the signal. This runs after the
+      // catch above has already checked the signal, so the abort is not
+      // reported as a timeout.
+      if (redirected) {
+        abortController.abort();
+      }
     }
   }
 

src/factory.spec.ts

@@ -73,6 +73,21 @@ describe('createWorkOS', () => {
       expect(workos.baseURL).toBeDefined();
     });
 
+    it('uses the provided fetchFn', async () => {
+      const fetchFn = jest.fn(
+        async () =>
+          new Response('{}', {
+            status: 200,
+            headers: { 'content-type': 'application/json' },
+          }),
+      );
+      const workos = createWorkOS({ apiKey: 'sk_test_123', fetchFn });
+
+      await workos.get('/path');
+
+      expect(fetchFn).toHaveBeenCalledTimes(1);
+    });
+
     it('has access to all services', () => {
       const workos = createWorkOS({
         apiKey: 'sk_test_123',

src/index.worker.ts

@@ -2,9 +2,6 @@
 import { Actions } from './actions/actions';
 import { CryptoProvider } from './common/crypto/crypto-provider';
 import { SubtleCryptoProvider } from './common/crypto/subtle-crypto-provider';
-import { FetchHttpClient } from './common/net/fetch-client';
-import { HttpClient } from './common/net/http-client';
-import { WorkOSOptions } from './index.worker';
 import { Webhooks } from './webhooks/webhooks';
 import { WorkOS } from './workos';
 
@@ -34,33 +31,6 @@ export {
 } from './factory';
 
 class WorkOSWorker extends WorkOS {
-  /** @override */
-  createHttpClient(options: WorkOSOptions, userAgent: string): HttpClient {
-    const headers: Record<string, string> = {
-      'User-Agent': userAgent,
-    };
-
-    const configHeaders = options.config?.headers;
-    if (
-      configHeaders &&
-      typeof configHeaders === 'object' &&
-      !Array.isArray(configHeaders) &&
-      !(configHeaders instanceof Headers)
-    ) {
-      Object.assign(headers, configHeaders);
-    }
-
-    if (this.key) {
-      headers['Authorization'] = `Bearer ${this.key}`;
-    }
-
-    return new FetchHttpClient(this.baseURL, {
-      ...options.config,
-      maxRetries: options.maxRetries,
-      headers,
-    });
-  }
-
   /** @override */
   createWebhookClient(): Webhooks {
     return new Webhooks(this);

src/workos.spec.ts

@@ -391,6 +391,56 @@ describe('WorkOS', () => {
       }, 20000);
     });
 
+    describe('when the api responds with a redirect', () => {
+      it('throws a GenericServerException that names the redirect', async () => {
+        const fetchFn = jest.fn(
+          async () =>
+            new Response(null, {
+              status: 301,
+              headers: { Location: 'https://api.workos.com/path' },
+            }),
+        );
+        const workos = new WorkOS('sk_test_Sz3IQjepeSWaI4cMS4ms4sMuU', {
+          fetchFn,
+        });
+
+        await expect(workos.post('/path', {})).rejects.toMatchObject({
+          name: 'GenericServerException',
+          status: 301,
+          message: expect.stringContaining(
+            'redirect (301 to https://api.workos.com/path)',
+          ),
+        });
+        expect(fetchFn).toHaveBeenCalledTimes(1);
+      });
+
+      it('throws a GenericServerException for an opaque redirect from a browser fetch', async () => {
+        // Browsers hide the status and Location of a manual redirect behind
+        // an opaque response. The Response constructor rejects status 0, so
+        // this is a bare fake.
+        const fetchFn = jest.fn(
+          async () =>
+            ({
+              type: 'opaqueredirect',
+              status: 0,
+              ok: false,
+              headers: new Headers(),
+              body: null,
+            }) as unknown as Response,
+        );
+        const workos = new WorkOS('sk_test_Sz3IQjepeSWaI4cMS4ms4sMuU', {
+          fetchFn,
+        });
+
+        await expect(workos.post('/path', {})).rejects.toMatchObject({
+          name: 'GenericServerException',
+          status: 0,
+          message: expect.stringContaining('an opaque redirect'),
+        });
+        expect(fetchFn).toHaveBeenCalledTimes(1);
+      });
+    });
+
     describe('when the api responds with a 500 and no error/error_description', () => {
       it('throws an GenericServerException', async () => {
         fetchOnce(
@@ -780,6 +830,43 @@ describe('WorkOS', () => {
       ).toBeInstanceOf(SubtleCryptoProvider);
     });
 
+    it('uses the provided fetchFn', async () => {
+      const fetchFn = jest.fn(
+        async () =>
+          new Response('{}', {
+            status: 200,
+            headers: { 'content-type': 'application/json' },
+          }),
+      );
+      const workos = new WorkOSWorker('sk_test_key', { fetchFn });
+
+      await workos.get('/path');
+
+      expect(fetchFn).toHaveBeenCalledTimes(1);
+    });
+
+    it('honors the timeout option', async () => {
+      const neverResolvingFetch = jest.fn(
+        (_url: any, init: any) =>
+          new Promise((_resolve, reject) => {
+            init?.signal?.addEventListener('abort', () => {
+              const abortError = new Error('Aborted');
+              abortError.name = 'AbortError';
+              reject(abortError);
+            });
+          }) as any,
+      );
+      const workos = new WorkOSWorker('sk_test_key', {
+        timeout: 1,
+        maxRetries: 0,
+        fetchFn: neverResolvingFetch as any,
+      });
+
+      await expect(workos.post('/path', {})).rejects.toMatchObject({
+        status: 408,
+      });
+    });
+
     it('uses console.warn to emit warnings', () => {
       const workos = new WorkOSWorker('sk_test_key');
       const warnSpy = jest.spyOn(console, 'warn');

src/workos.ts

@@ -202,12 +202,16 @@ export class WorkOS {
       headers['Authorization'] = `Bearer ${this.key}`;
     }
 
-    return new FetchHttpClient(this.baseURL, {
-      ...options.config,
-      timeout: options.timeout,
-      maxRetries: options.maxRetries,
-      headers,
-    }) as HttpClient;
+    return new FetchHttpClient(
+      this.baseURL,
+      {
+        ...options.config,
+        timeout: options.timeout,
+        maxRetries: options.maxRetries,
+        headers,
+      },
+      options.fetchFn,
+    ) as HttpClient;
   }
 
   get version() {