Strip credentials on cross-origin HTTP redirects

.changeset/eff-216-secure-http-redirects.md

@@ -0,0 +1,5 @@
+---
+"effect": patch
+---
+
+Strip credential headers on cross-origin HTTP redirects and align redirected request methods with fetch.

packages/effect/src/unstable/http/HttpClient.ts

@@ -34,6 +34,7 @@ import type { EqualsWith, ExcludeTag, ExtractTag, NoExcessProperties, NoInfer, T
 import type * as RateLimiter from "../persistence/RateLimiter.ts"
 import * as Cookies from "./Cookies.ts"
 import * as Headers from "./Headers.ts"
+import * as HttpBody from "./HttpBody.ts"
 import * as Error from "./HttpClientError.ts"
 import * as HttpClientRequest from "./HttpClientRequest.ts"
 import * as HttpClientResponse from "./HttpClientResponse.ts"
@@ -1466,17 +1467,29 @@ export const followRedirects: {
       ): Effect.Effect<HttpClientResponse.HttpClientResponse, E, R> =>
         Effect.flatMap(
           self.postprocess(Effect.succeed(request)),
-          (response) =>
-            response.status >= 300 && response.status < 400 && response.headers.location &&
-              redirects < (maxRedirects ?? 10)
-              ? loop(
-                HttpClientRequest.setUrl(
-                  request,
-                  new URL(response.headers.location, response.request.url)
-                ),
-                redirects + 1
-              )
-              : Effect.succeed(response)
+          (response) => {
+            if (
+              response.status < 300 || response.status >= 400 || !response.headers.location ||
+              redirects >= (maxRedirects ?? 10)
+            ) {
+              return Effect.succeed(response)
+            }
+            const url = new URL(response.headers.location, response.request.url)
+            let nextRequest = request
+            if (
+              ((response.status === 301 || response.status === 302) && request.method === "POST") ||
+              (response.status === 303 && request.method !== "GET" && request.method !== "HEAD")
+            ) {
+              nextRequest = HttpClientRequest.setMethod(nextRequest, "GET")
+              nextRequest = HttpClientRequest.setBody(nextRequest, HttpBody.empty)
+            }
+            if (url.origin !== new URL(response.request.url).origin) {
+              nextRequest = HttpClientRequest.removeHeader(nextRequest, "authorization")
+              nextRequest = HttpClientRequest.removeHeader(nextRequest, "proxy-authorization")
+              nextRequest = HttpClientRequest.removeHeader(nextRequest, "cookie")
+            }
+            return loop(HttpClientRequest.setUrl(nextRequest, url), redirects + 1)
+          }
         )
       return Effect.flatMap(request, (request) => loop(request, 0))
     },

packages/effect/test/unstable/http/HttpClient.test.ts

@@ -2,7 +2,7 @@ import { assert, describe, it } from "@effect/vitest"
 import { strictEqual } from "@effect/vitest/utils"
 import { Clock, Duration, Effect, Fiber, Layer, Ref, Stream } from "effect"
 import { TestClock } from "effect/testing"
-import { HttpClient, HttpClientResponse } from "effect/unstable/http"
+import { HttpClient, HttpClientRequest, HttpClientResponse } from "effect/unstable/http"
 import { RateLimiter } from "effect/unstable/persistence"
 
 const makeStatusClient = Effect.fnUntraced(function*(status: number) {
@@ -16,9 +16,133 @@ const makeStatusClient = Effect.fnUntraced(function*(status: number) {
   return { attempts, client } as const
 })
 
+const makeRedirectClient = Effect.fnUntraced(function*(status: number, location: string | ReadonlyArray<string>) {
+  const locations = typeof location === "string" ? [location] : location
+  const requests = yield* Ref.make<Array<HttpClientRequest.HttpClientRequest>>([])
+  const client = HttpClient.make((request) =>
+    Effect.map(
+      Ref.updateAndGet(requests, (requests) => [...requests, request]),
+      (requests) =>
+        HttpClientResponse.fromWeb(
+          request,
+          requests.length <= locations.length
+            ? new Response(null, { status, headers: { location: locations[requests.length - 1] } })
+            : new Response(null, { status: 200 })
+        )
+    )
+  ).pipe(HttpClient.followRedirects())
+  return { client, requests } as const
+})
+
 const RateLimiterTestLayer = RateLimiter.layer.pipe(Layer.provide(RateLimiter.layerStoreMemory))
 
 describe("HttpClient", () => {
+  describe("followRedirects", () => {
+    it.effect("preserves credential headers on same-origin redirects", () =>
+      Effect.gen(function*() {
+        const { client, requests } = yield* makeRedirectClient(302, "https://origin.test/destination")
+        yield* client.get("https://origin.test/start", {
+          headers: {
+            Authorization: "Bearer secret",
+            Cookie: "session=secret"
+          }
+        })
+
+        const redirected = (yield* Ref.get(requests))[1]
+        assert.strictEqual(redirected.headers.authorization, "Bearer secret")
+        assert.strictEqual(redirected.headers.cookie, "session=secret")
+      }))
+
+    it.effect("strips credential headers on cross-origin redirects", () =>
+      Effect.gen(function*() {
+        const { client, requests } = yield* makeRedirectClient(302, "https://redirect.test/destination")
+        yield* client.get("https://origin.test/start", {
+          headers: {
+            Authorization: "Bearer secret",
+            Cookie: "session=secret",
+            "Proxy-Authorization": "Basic secret",
+            "X-Test": "retained"
+          }
+        })
+
+        const redirected = (yield* Ref.get(requests))[1]
+        assert.isUndefined(redirected.headers.authorization)
+        assert.isUndefined(redirected.headers.cookie)
+        assert.isUndefined(redirected.headers["proxy-authorization"])
+        assert.strictEqual(redirected.headers["x-test"], "retained")
+      }))
+
+    it.effect("strips credential headers on scheme downgrade redirects", () =>
+      Effect.gen(function*() {
+        const { client, requests } = yield* makeRedirectClient(302, "http://origin.test/destination")
+        yield* client.get("https://origin.test/start", {
+          headers: {
+            Authorization: "Bearer secret",
+            Cookie: "session=secret",
+            "Proxy-Authorization": "Basic secret"
+          }
+        })
+
+        const redirected = (yield* Ref.get(requests))[1]
+        assert.isUndefined(redirected.headers.authorization)
+        assert.isUndefined(redirected.headers.cookie)
+        assert.isUndefined(redirected.headers["proxy-authorization"])
+      }))
+
+    it.effect("resolves relative locations against the current hop", () =>
+      Effect.gen(function*() {
+        const { client, requests } = yield* makeRedirectClient(302, ["../next/step", "destination"])
+        yield* client.get("https://origin.test/path/start", {
+          headers: { Authorization: "Bearer secret" }
+        })
+
+        const redirected = (yield* Ref.get(requests))[2]
+        assert.strictEqual(redirected.url, "https://origin.test/next/destination")
+        assert.strictEqual(redirected.headers.authorization, "Bearer secret")
+      }))
+
+    it.effect("switches 303 requests to GET and drops the body", () =>
+      Effect.gen(function*() {
+        const { client, requests } = yield* makeRedirectClient(303, "/destination")
+        yield* HttpClientRequest.post("https://origin.test/start").pipe(
+          HttpClientRequest.bodyText("payload"),
+          client.execute
+        )
+
+        const redirected = (yield* Ref.get(requests))[1]
+        assert.strictEqual(redirected.method, "GET")
+        assert.strictEqual(redirected.body._tag, "Empty")
+        assert.isUndefined(redirected.headers["content-type"])
+        assert.isUndefined(redirected.headers["content-length"])
+      }))
+
+    it.effect.each([301, 302])("switches POST to GET on %s redirects", (status) =>
+      Effect.gen(function*() {
+        const { client, requests } = yield* makeRedirectClient(status, "/destination")
+        yield* HttpClientRequest.post("https://origin.test/start").pipe(
+          HttpClientRequest.bodyText("payload"),
+          client.execute
+        )
+
+        const redirected = (yield* Ref.get(requests))[1]
+        assert.strictEqual(redirected.method, "GET")
+        assert.strictEqual(redirected.body._tag, "Empty")
+      }))
+
+    it.effect.each([301, 302])("preserves non-POST methods on %s redirects", (status) =>
+      Effect.gen(function*() {
+        const { client, requests } = yield* makeRedirectClient(status, "/destination")
+        yield* HttpClientRequest.put("https://origin.test/start").pipe(
+          HttpClientRequest.bodyText("payload"),
+          client.execute
+        )
+
+        const redirected = (yield* Ref.get(requests))[1]
+        assert.strictEqual(redirected.method, "PUT")
+        assert.strictEqual(redirected.body._tag, "Uint8Array")
+      }))
+  })
+
   describe("retryTransient", () => {
     it.effect("retries transient responses with retryOn errors-and-responses", () =>
       Effect.gen(function*() {