fix: patched fetch

Author: chrisvander

README.md

@@ -46,6 +46,10 @@ export default defineConfig({
 })
 ```
 
+AT Astro detects runtimes that reject `RequestInit.redirect: "error"` and applies a scoped
+OAuth fetch compatibility patch automatically. Set `patchRedirects: true` or `false` only to
+force or disable that behavior for a runtime whose capability detection is inaccurate.
+
 ## Usage
 
 ### Routes

package.json

@@ -1,6 +1,6 @@
 {
   "name": "at-astro",
-  "version": "1.0.3",
+  "version": "1.0.4",
   "private": false,
   "description": "An Astro integration for the AT Protocol, implementing OAuth flow and publishing an authenticated client to fetch and mutate records.",
   "keywords": [

src/integration.ts

@@ -15,6 +15,7 @@ export type AtAstroConfig = {
   oauthSessionPrefix: string
   handleResolver: string
   publicEndpoint: string
+  patchRedirects?: boolean
   redirectAfterSignIn: string
   redirectAfterSignOut: string
 }
@@ -40,6 +41,8 @@ export type AtAstroOptions = {
    * @default "https://public.api.bsky.app"
    */
   publicEndpoint?: string
+  /** Force-enable or disable the OAuth redirect compatibility patch; defaults to runtime detection. */
+  patchRedirects?: boolean
   /** The path to redirect to after the user successfully signs in (defaults to the site's root) */
   redirectAfterSignIn?: `/${string}`
   /** The path to redirect to after the user successfully signs out (defaults to the site's root) */
@@ -104,6 +107,7 @@ export function createConfig({ isDev, astroConfig, options }: CreateConfigOption
     oauthSessionPrefix: `${sessionPrefix}:oauth-session:`,
     handleResolver: options?.handleResolver ?? "https://bsky.social",
     publicEndpoint: options?.publicEndpoint ?? "https://public.api.bsky.app",
+    patchRedirects: options?.patchRedirects,
     redirectAfterSignIn: `${clientUri}${options?.redirectAfterSignIn ?? ""}`,
     redirectAfterSignOut: `${clientUri}${options?.redirectAfterSignOut ?? ""}`,
   }

src/lib/atproto-client.test.ts

@@ -0,0 +1,40 @@
+import { expect, mock, spyOn, test } from "bun:test"
+import { OAuthResolverError } from "@atproto/oauth-client"
+import type { AtAstroSession } from "../types/session"
+
+const restore = mock()
+
+void mock.module("at-astro:config", () => ({
+  config: {
+    didSessionKey: "at-astro:did",
+    publicEndpoint: "https://public.api.bsky.app",
+  },
+}))
+void mock.module("./atproto-oauth", () => ({
+  getOAuthClient: () => ({ restore }),
+}))
+
+const { getClient } = await import("./atproto-client")
+const session = {
+  get: () => Promise.resolve("did:plc:test"),
+} as unknown as AtAstroSession
+
+test("returns a public client when identity resolution fails", async () => {
+  const error = new OAuthResolverError("Failed to resolve identity: did:plc:test")
+  const consoleError = spyOn(console, "error").mockImplementation(() => {})
+  restore.mockRejectedValueOnce(error)
+
+  try {
+    expect((await getClient(session)).did).toBeNull()
+    expect(consoleError).toHaveBeenCalledWith("Failed to restore AT Protocol session", error)
+  } finally {
+    consoleError.mockRestore()
+  }
+})
+
+test("preserves other session restore errors", async () => {
+  const error = new Error("Session storage failed")
+  restore.mockRejectedValueOnce(error)
+
+  expect(getClient(session)).rejects.toBe(error)
+})

src/lib/atproto-client.ts

@@ -1,8 +1,16 @@
 import { Client } from "@atproto/lex"
+import { OAuthResolverError } from "@atproto/oauth-client"
 import { getOAuthClient } from "./atproto-oauth"
 import { config } from "at-astro:config"
 import type { AtAstroSession } from "../types/session"
 
+function getPublicClient() {
+  return {
+    client: new Client(config.publicEndpoint, { validateRequest: import.meta.env.DEV }),
+    did: null,
+  }
+}
+
 /**
  * Returns an ATProto client, authenticated if the user is signed in, and a public client otherwise.
  * @param session - The session to use for authentication, typically Astro.session.
@@ -14,13 +22,14 @@ export async function getClient(session: AtAstroSession | undefined): Promise<{
   did: string | null
 }> {
   const did = await session?.get(config.didSessionKey)
-  if (!session || !did) {
-    return {
-      client: new Client(config.publicEndpoint, { validateRequest: import.meta.env.DEV }),
-      did: null,
-    }
-  }
+  if (!session || !did) return getPublicClient()
 
-  const oauthSession = await getOAuthClient(session).restore(did)
-  return { client: new Client(oauthSession, { validateRequest: import.meta.env.DEV }), did }
+  try {
+    const oauthSession = await getOAuthClient(session).restore(did)
+    return { client: new Client(oauthSession, { validateRequest: import.meta.env.DEV }), did }
+  } catch (error) {
+    if (!(error instanceof OAuthResolverError)) throw error
+    console.error("Failed to restore AT Protocol session", error)
+    return getPublicClient()
+  }
 }

src/lib/atproto-oauth.ts

@@ -3,6 +3,7 @@ import { WebcryptoKey } from "@atproto/jwk-webcrypto"
 import type { SimpleStore } from "@atproto-labs/simple-store"
 import { config } from "at-astro:config"
 import type { AtAstroSession } from "../types/session.ts"
+import { createOAuthFetchOptions, type Fetch } from "./workerd-fetch.ts"
 
 async function importDpopKey(jwk: JsonWebKey) {
   if (jwk.kty !== "EC" || jwk.crv !== "P-256" || !jwk.d) {
@@ -45,11 +46,16 @@ function oauthStore<T extends { dpopKey: Key }>(
   }
 }
 
-export function getOAuthClient(session: AtAstroSession) {
+export function getOAuthClient(
+  session: AtAstroSession,
+  fetch: Fetch = globalThis.fetch,
+  patchRedirects = config.patchRedirects,
+) {
   return new OAuthClient({
     responseMode: "query",
     clientMetadata: config.clientMetadata,
     handleResolver: config.handleResolver,
+    ...createOAuthFetchOptions(fetch, patchRedirects),
     stateStore: oauthStore(config.oauthStatePrefix, session, 3600),
     sessionStore: oauthStore(config.oauthSessionPrefix, session),
     runtimeImplementation: {

src/lib/workerd-fetch.test.ts

@@ -0,0 +1,89 @@
+import { expect, mock, test } from "bun:test"
+import type { Did } from "@atproto/oauth-client"
+import { createOAuthFetchOptions } from "./workerd-fetch"
+
+const plcDid = "did:plc:3u26lcxyhiyq3ygsfyrc7xx2" as Did<"plc">
+
+function createWorkerdOptions(
+  fetch: (input: string | URL | Request, init?: RequestInit) => Promise<Response>,
+) {
+  return createOAuthFetchOptions(fetch, true)
+}
+
+test("uses the supplied fetch unchanged when redirect:error is supported", () => {
+  const fetch = mock(async () => Response.json({ ok: true }))
+  const options = createOAuthFetchOptions(fetch)
+
+  expect(Object.is(options.fetch, fetch)).toBe(true)
+  expect(options.didResolver).toBeUndefined()
+})
+
+test("patches redirect:error when the Request constructor rejects it", async () => {
+  const fetch = mock(async (_input: string | URL | Request, init?: RequestInit) => {
+    if (init?.redirect === "error") {
+      throw new TypeError('Invalid redirect value: "error"')
+    }
+    return Response.json({ id: plcDid })
+  })
+  const options = createWorkerdOptions(fetch)
+
+  expect(await options.fetch("https://plc.directory/test", { redirect: "error" })).toEqual(
+    expect.objectContaining({ status: 200 }),
+  )
+  expect(fetch.mock.calls[0]?.[1]?.redirect).toBe("manual")
+})
+
+test("patches redirect:error on a Request input", async () => {
+  const fetch = mock(async (input: string | URL | Request) => {
+    expect(input).toBeInstanceOf(Request)
+    expect((input as Request).redirect).toBe("manual")
+    return Response.json({ ok: true })
+  })
+  const request = new Request("https://example.com", { redirect: "error" })
+
+  await createWorkerdOptions(fetch).fetch(request)
+})
+
+test("rejects redirects when patching redirect:error", async () => {
+  const fetch = mock(async () => Response.redirect("https://attacker.example/did.json", 302))
+
+  expect(
+    createWorkerdOptions(fetch).fetch("https://plc.directory/test", { redirect: "error" }),
+  ).rejects.toThrow("Redirects are not allowed")
+})
+
+test("preserves other redirect modes", async () => {
+  const response = Response.json({ ok: true })
+  const fetch = mock(async (_input: string | URL | Request, _init?: RequestInit) => response)
+
+  expect(
+    await createWorkerdOptions(fetch).fetch("https://example.com", { redirect: "follow" }),
+  ).toBe(response)
+  expect(fetch.mock.calls[0]?.[1]?.redirect).toBe("follow")
+})
+
+test("lets the ATProto DID resolver reach the patched fetch", async () => {
+  const fetch = mock(async (_input: string | URL | Request, init?: RequestInit) => {
+    if (init?.redirect === "error") {
+      throw new TypeError('Invalid redirect value: "error"')
+    }
+    return Response.json({ id: plcDid })
+  })
+  const options = createWorkerdOptions(fetch)
+
+  expect(await options.didResolver?.resolve(plcDid)).toEqual({ id: plcDid })
+  expect(fetch.mock.calls[0]?.[1]?.redirect).toBe("manual")
+})
+
+test("resolves did:web through the same patched fetch", async () => {
+  const did = "did:web:example.com" as Did<"web">
+  const fetch = mock(async (input: string | URL | Request, init?: RequestInit) => {
+    expect(input).toBeInstanceOf(URL)
+    expect((input as URL).href).toBe("https://example.com/.well-known/did.json")
+    expect(init?.redirect).toBe("manual")
+    return Response.json({ id: did })
+  })
+  const options = createWorkerdOptions(fetch)
+
+  expect(await options.didResolver?.resolve(did)).toEqual({ id: did })
+})

src/lib/workerd-fetch.ts

@@ -0,0 +1,93 @@
+import {
+  DidPlcMethod,
+  DidResolverCommon,
+  DidWebMethod,
+  type AtprotoIdentityDidMethods,
+  type DidMethod,
+  type OAuthClientOptions,
+} from "@atproto/oauth-client"
+
+export type Fetch = (input: string | URL | Request, init?: RequestInit) => Promise<Response>
+type OAuthFetch = NonNullable<OAuthClientOptions["fetch"]>
+
+const redirectStatuses = new Set([301, 302, 303, 307, 308])
+
+function createWorkerdFetch(fetch: Fetch): OAuthFetch {
+  const patchedFetch: Fetch = async (input, init) => {
+    const redirect = init?.redirect ?? (input instanceof Request ? input.redirect : undefined)
+    if (redirect !== "error") return fetch.call(globalThis, input, init)
+
+    const patchedInit = { ...init, redirect: "manual" as const }
+    const response =
+      input instanceof Request
+        ? await fetch.call(globalThis, new Request(input, patchedInit))
+        : await fetch.call(globalThis, input, patchedInit)
+
+    if (response.type === "opaqueredirect" || redirectStatuses.has(response.status)) {
+      throw new TypeError("Redirects are not allowed")
+    }
+
+    return response
+  }
+
+  // Bun augments its global fetch type with preconnect, but ATProto only calls it as a function.
+  return patchedFetch as OAuthFetch
+}
+
+class WorkerdDidPlcMethod extends DidPlcMethod {
+  protected override readonly fetch: Fetch
+
+  constructor(fetch: Fetch) {
+    super()
+    this.fetch = fetch
+  }
+}
+
+class WorkerdDidWebMethod extends DidWebMethod {
+  protected override readonly fetch: Fetch
+
+  constructor(fetch: Fetch) {
+    super()
+    this.fetch = fetch
+  }
+}
+
+type DidMethodRegistry = {
+  set<M extends AtprotoIdentityDidMethods>(name: M, method: DidMethod<M>): void
+}
+
+class WorkerdDidResolver extends DidResolverCommon {
+  constructor(fetch: Fetch) {
+    super()
+    // The SDK constructs Request before calling an injected fetch, which Workerd rejects for
+    // redirect:error. Replace only the DID methods so the patched fetch receives their init.
+    const methods = this.methods as unknown as DidMethodRegistry
+    methods.set("plc", new WorkerdDidPlcMethod(fetch))
+    methods.set("web", new WorkerdDidWebMethod(fetch))
+  }
+}
+
+function createWorkerdDidResolver(fetch: Fetch) {
+  return new WorkerdDidResolver(fetch)
+}
+
+function supportsRedirectError() {
+  try {
+    return new Request("https://at-astro.invalid", { redirect: "error" }).redirect === "error"
+  } catch {
+    return false
+  }
+}
+
+export function createOAuthFetchOptions(
+  fetch: Fetch = globalThis.fetch,
+  patchRedirects = !supportsRedirectError(),
+) {
+  if (!patchRedirects) return { fetch: fetch as OAuthFetch }
+
+  const patchedFetch = createWorkerdFetch(fetch)
+  return {
+    fetch: patchedFetch,
+    didResolver: createWorkerdDidResolver(patchedFetch),
+  }
+}