fix(apostrophe-astro): fix a bug where redirect responses drop headers (#5518)

* fix(apostrophe-astro): fix a bug where redirect responses drop headers

* fix(apostrophe-astro) fix a bug where the response body is dropped but the describing headers are not

Author: eri-vi-maddyie

.changeset/late-comics-chew.md

@@ -0,0 +1,6 @@
+---
+"@apostrophecms/apostrophe-astro": patch
+---
+
+Fixed a bug where redirect responses proxied through `aposProxy.js` (e.g. OAuth login callbacks) lost every header except `Location`. This affected setups where Apostrophe is only reachable through the Astro proxy (e.g. a sidecar deployment), causing the session `Set-Cookie` header from an OAuth callback redirect to be silently dropped and the login to appear to fail.
+Included fix to `aposResponse.js` to remove entity headers describing the body when dropping the body (e.g. redirects) which may otherwise cause strict clients to hang waiting for bytes.

packages/apostrophe-astro/endpoints/aposProxy.js

@@ -1,12 +1,8 @@
-import aposResponse from "../lib/aposResponse";
+import aposResponse from "../lib/aposResponse.js";
 
-export async function ALL({ params, request, redirect }) {
+export async function ALL({ request }) {
   try {
-    const response = await aposResponse(request);
-    if ([301, 302, 307, 308].includes(response.status)) {
-      return redirect(response.headers.get('location'), response.status);
-    }
-    return response;
+    return await aposResponse(request);
   } catch (e) {
     return new Response(e.message, { status: 500 });
   }

packages/apostrophe-astro/lib/aposResponse.js

@@ -83,12 +83,16 @@ export default async function aposResponse(req) {
 
     const { headers, statusCode, ...rest } = res;
 
-    // Statuses whose body we never send to the client: 204/304 carry none,
-    // and redirects (301/302/307/308) become a fresh Astro redirect built
-    // from the Location header in aposProxy. Dump the undici body so its
-    // socket returns to the pool instead of being held open until GC.
+    // Statuses we forward with no body: 204/304 carry none, and for
+    // redirects (301/302/307/308) Location + Set-Cookie are all the client
+    // needs. Dump the undici body so its socket returns to the pool, and
+    // strip the entity headers that described it - a stale Content-Length
+    // would make the client wait for bytes that never arrive.
     if ([204, 304, 301, 302, 307, 308].includes(statusCode)) {
       await res.body.dump().catch(() => {});
+      responseHeaders.delete('content-length');
+      responseHeaders.delete('content-encoding');
+      responseHeaders.delete('transfer-encoding');
       return new Response(null, { ...rest, status: statusCode, headers: responseHeaders });
     }
 

packages/apostrophe-astro/test/endpoints/aposProxy.test.js

@@ -0,0 +1,60 @@
+import assert from 'node:assert/strict';
+import esmock from 'esmock';
+
+async function loadAposProxy(mockResponse) {
+  return esmock('../../endpoints/aposProxy.js', {
+    '../../lib/aposResponse.js': {
+      default: async () => mockResponse
+    }
+  });
+}
+
+function makeRequest(url = 'http://localhost:4321/login/callback') {
+  return new Request(url);
+}
+
+describe('aposProxy', () => {
+  it('forwards redirect responses without dropping non-Location headers', async () => {
+    // Regression test: Astro's own `redirect()` helper only keeps the
+    // Location header, which silently drops the session Set-Cookie header
+    // an OAuth callback relies on to mark the browser as authenticated.
+    const headers = new Headers();
+    headers.set('location', '/dashboard');
+    headers.append('set-cookie', 'apos.sid=abc123; Path=/; HttpOnly');
+    headers.set('x-custom-auth', 'issued');
+    const mockResponse = new Response(null, { status: 302, headers });
+
+    const { ALL } = await loadAposProxy(mockResponse);
+    const res = await ALL({ request: makeRequest() });
+
+    assert.equal(res.status, 302);
+    assert.equal(res.headers.get('location'), '/dashboard');
+    assert.equal(res.headers.get('set-cookie'), 'apos.sid=abc123; Path=/; HttpOnly');
+    assert.equal(res.headers.get('x-custom-auth'), 'issued');
+  });
+
+  it('passes non-redirect responses through unchanged', async () => {
+    const mockResponse = new Response('{"ok":true}', {
+      status: 200,
+      headers: { 'content-type': 'application/json' }
+    });
+
+    const { ALL } = await loadAposProxy(mockResponse);
+    const res = await ALL({ request: makeRequest() });
+
+    assert.equal(res.status, 200);
+    assert.equal(await res.text(), '{"ok":true}');
+  });
+
+  it('returns a 500 text response when aposResponse throws', async () => {
+    const { ALL } = await esmock('../../endpoints/aposProxy.js', {
+      '../../lib/aposResponse.js': {
+        default: async () => { throw new Error('boom'); }
+      }
+    });
+
+    const res = await ALL({ request: makeRequest() });
+    assert.equal(res.status, 500);
+    assert.equal(await res.text(), 'boom');
+  });
+});

packages/apostrophe-astro/test/lib/aposResponse.test.js

@@ -153,6 +153,28 @@ describe('aposResponse', () => {
         assert.equal(res.body, null);
       });
     }
+
+    for (const status of [ 204, 304, 301, 302, 307, 308 ]) {
+      it(`strips entity headers describing the dropped body for ${status}`, async () => {
+        const { default: aposResponse } = await loadAposResponse({}, async () => ({
+          statusCode: status,
+          headers: {
+            location: '/dashboard',
+            'set-cookie': 'apos.sid=abc123; Path=/; HttpOnly',
+            'content-length': '1234',
+            'content-encoding': 'gzip',
+            'transfer-encoding': 'chunked'
+          },
+          body: makeBody('')
+        }));
+        const res = await aposResponse(makeRequest());
+        assert.equal(res.status, status);
+        assert.equal(res.headers.get('set-cookie'), 'apos.sid=abc123; Path=/; HttpOnly');
+        assert.equal(res.headers.get('content-length'), null);
+        assert.equal(res.headers.get('content-encoding'), null);
+        assert.equal(res.headers.get('transfer-encoding'), null);
+      });
+    }
   });
 
   describe('normal response', () => {