From f07ac45da2c8fac3465ef0aa87f2836e03006aa0 Mon Sep 17 00:00:00 2001
From: Ayush7614 <ayushknj3@gmail.com>
Date: Mon, 7 Sep 2026 21:02:12 +0530
Subject: [PATCH] Do not follow redirects when renewing an OAuth access token

exchangeRefreshTokenOverHttp carries the deployment client secret and
a user refresh token to a catalogue-pinned tokenUrl. fetch follows
redirects by default, so a compromised or misbehaving vendor 302 would
hand both secrets to whatever address the answer named.

Send the renewal with redirect manual, like the authorization-code
redemption and the dynamic-client registration already do, so a 3xx
lands in the refusal branch instead of being followed.
---
 server/src/plugins/store.ts                 |  9 +++
 server/tests/oauth-refresh-redirect.test.ts | 64 +++++++++++++++++++++
 2 files changed, 73 insertions(+)
 create mode 100644 server/tests/oauth-refresh-redirect.test.ts

diff --git a/server/src/plugins/store.ts b/server/src/plugins/store.ts
index 11867e4ce..b2b1dd1ae 100644
--- a/server/src/plugins/store.ts
+++ b/server/src/plugins/store.ts
@@ -387,6 +387,15 @@ export async function exchangeRefreshTokenOverHttp(input: {
     method: "POST",
     headers: { "content-type": "application/x-www-form-urlencoded" },
     body: params,
+    /*
+     * A redirect is a refusal, not a detour to be followed.
+     *
+     * `tokenUrl` is pinned in the catalogue because this request carries the deployment's client
+     * secret and somebody's refresh token, and following a 302 would hand both to whatever address
+     * the answer named. Manual leaves the 3xx as the response, which is not `ok`, so it falls into
+     * the refusal below. The same guard the authorization-code redemption in `oauth.ts` uses.
+     */
+    redirect: "manual",
     signal: AbortSignal.timeout(TOKEN_TIMEOUT_MS),
   });
 
diff --git a/server/tests/oauth-refresh-redirect.test.ts b/server/tests/oauth-refresh-redirect.test.ts
new file mode 100644
index 000000000..e317c439d
--- /dev/null
+++ b/server/tests/oauth-refresh-redirect.test.ts
@@ -0,0 +1,64 @@
+import { describe, expect, test } from "bun:test";
+import { exchangeRefreshTokenOverHttp } from "../src/plugins/store";
+
+/**
+ * The renewal carries the deployment's client secret and somebody's refresh token to a
+ * catalogue-pinned address. Following a redirect would hand both to whatever address the answer
+ * named, so the request goes out with `redirect: "manual"` and a 3xx lands in the refusal branch
+ * instead of being followed.
+ *
+ * The authorization-code redemption and the dynamic-client registration in `oauth.ts` already guard
+ * the same way; this covers the third path that carries secrets, which previously followed
+ * redirects by default.
+ */
+describe("renewing an access token", () => {
+  test("a redirecting token endpoint is a refusal and is never followed", async () => {
+    const seen: { url: unknown; redirect: RequestRedirect | undefined }[] = [];
+    const realFetch = globalThis.fetch;
+    globalThis.fetch = (async (url: unknown, init?: RequestInit) => {
+      seen.push({ url, redirect: init?.redirect });
+      return new Response(null, {
+        status: 302,
+        headers: { location: "https://elsewhere.example/token" },
+      });
+    }) as unknown as typeof fetch;
+    try {
+      await expect(
+        exchangeRefreshTokenOverHttp({
+          tokenUrl: "https://vendor.example/token",
+          client: { clientId: "c-1", clientSecret: "s-1" },
+          refreshToken: "rt-1",
+        }),
+      ).rejects.toThrow("The vendor would not renew this access (302).");
+    } finally {
+      globalThis.fetch = realFetch;
+    }
+    expect(seen).toHaveLength(1);
+    expect(seen[0]?.url).toBe("https://vendor.example/token");
+    expect(seen[0]?.redirect).toBe("manual");
+  });
+
+  test("a successful renewal still sends the secret to the pinned endpoint only", async () => {
+    const seen: { url: unknown; redirect: RequestRedirect | undefined }[] = [];
+    const realFetch = globalThis.fetch;
+    globalThis.fetch = (async (url: unknown, init?: RequestInit) => {
+      seen.push({ url, redirect: init?.redirect });
+      return new Response(
+        JSON.stringify({ access_token: "at-1", expires_in: 3600 }),
+        { status: 200, headers: { "content-type": "application/json" } },
+      );
+    }) as unknown as typeof fetch;
+    try {
+      const token = await exchangeRefreshTokenOverHttp({
+        tokenUrl: "https://vendor.example/token",
+        client: { clientId: "c-1", clientSecret: "s-1" },
+        refreshToken: "rt-1",
+      });
+      expect(token.accessToken).toBe("at-1");
+    } finally {
+      globalThis.fetch = realFetch;
+    }
+    expect(seen).toHaveLength(1);
+    expect(seen[0]?.redirect).toBe("manual");
+  });
+});