Handle FetchError in getSignedKeyOwner() gracefully

CHANGES.md

@@ -8,6 +8,17 @@ Version 2.1.0
 
 To be released.
 
+### @fedify/fedify
+
+ -  Fixed `RequestContext.getSignedKeyOwner()` to return `null` instead of
+    throwing an error when the remote server requires authorized fetch and
+    returns `401 Unauthorized` for the key owner lookup.  Previously, this
+    caused a `500 Internal Server Error` when interoperating with servers like
+    GoToSocial that have authorized fetch enabled.  [[#473], [#589]]
+
+[#473]: https://github.com/fedify-dev/fedify/issues/473
+[#589]: https://github.com/fedify-dev/fedify/pull/589
+
 ### @fedify/init
 
  -  Changed `fedify init` to add `"temporal"` to `deno.json`'s `"unstable"`

docs/manual/access-control.md

@@ -166,14 +166,33 @@ Instance actor
 --------------
 
 When you enable authorized fetch, you need to fetch actors from other servers
-to retrieve their public keys.  However, fetching resources from other servers
-may cause an infinite loop if the other server also requires authorized fetch,
-which causes another request to your server for the public key, and so on.
-
-The most common way to prevent it is a pattern called [instance actor], which
-is an actor that represents the whole instance and exceptionally does not
-require authorized fetch.  You can use the instance actor to fetch resources
-from other servers without causing an infinite loop.
+to retrieve their public keys.  However, this can cause problems when the other
+server also has authorized fetch enabled:
+
+ -  *Unauthenticated appearance*: If the remote server requires a signed
+    request to fetch its actor, and your server fetches it without a signature,
+    the remote server returns an HTTP 401 error.  In this case,
+    `~RequestContext.getSignedKeyOwner()` returns `null`, so the requester
+    appears unauthenticated to your `AuthorizePredicate`—which will typically
+    deny the request.
+
+ -  *Infinite loop*: If both servers require authorized fetch, fetching the
+    remote actor requires your server to be authenticated, which in turn
+    requires fetching *your* actor, which requires authentication, and so on.
+
+> [!NOTE]
+> Even without the infinite loop, if the remote server requires authorized
+> fetch, `~RequestContext.getSignedKeyOwner()` returns `null` for requests
+> from that server (since fetching the key owner fails with HTTP 401).
+> This means such requests appear unauthenticated to your
+> `AuthorizePredicate`.  To properly authenticate those requests, implement
+> the instance actor pattern below.
+
+The most common way to prevent both problems is a pattern called
+[instance actor], which is an actor that represents the whole instance and
+exceptionally does not require authorized fetch.  You can use the instance
+actor to fetch resources from other servers with a valid signature, without
+causing an infinite loop.
 
 Usually, many ActivityPub implementations name their instance actor as their
 domain name, such as `example.com@example.com`.  Here is an example of how to

packages/fedify/src/federation/middleware.test.ts

@@ -35,7 +35,7 @@ import {
   rsaPublicKey2,
   rsaPublicKey3,
 } from "../testing/keys.ts";
-import { getDocumentLoader } from "@fedify/vocab-runtime";
+import { FetchError, getDocumentLoader } from "@fedify/vocab-runtime";
 import { getAuthenticatedDocumentLoader } from "../utils/docloader.ts";
 
 const documentLoader = getDocumentLoader();
@@ -942,6 +942,52 @@ test({
       );
     });
 
+    await t.step(
+      "RequestContext.getSignedKeyOwner() returns null on FetchError",
+      async () => {
+        // Regression test for <https://github.com/fedify-dev/fedify/issues/473>:
+        // When the key owner actor fetch fails (e.g., GoToSocial returns 401 for
+        // authorized fetch), getSignedKeyOwner() should return null instead of
+        // throwing a FetchError.
+        //
+        // Custom document loader that simulates a server with authorized fetch
+        // enabled (returns 401 for actor URL but allows key URL with fragment):
+        const customDocumentLoader = async (url: string) => {
+          if (url === "https://example.com/person2#key3") {
+            // Key URL (with fragment): return actor document for sig verification
+            return await mockDocumentLoader("https://example.com/person2");
+          }
+          if (url === "https://example.com/person2") {
+            // Actor URL (without fragment): simulate 401 Unauthorized
+            throw new FetchError(
+              new URL(url),
+              "HTTP 401: Unauthorized",
+            );
+          }
+          return mockDocumentLoader(url);
+        };
+
+        const signedReq = await signRequest(
+          new Request("https://example.com/", {
+            headers: { accept: "application/activity+json" },
+          }),
+          rsaPrivateKey3,
+          rsaPublicKey3.id!,
+        );
+
+        const fed = createFederation<void>({
+          kv,
+          documentLoaderFactory: () => customDocumentLoader,
+          contextLoaderFactory: () => mockDocumentLoader,
+        });
+        const ctx = fed.createContext(signedReq, undefined);
+
+        // Before fix: throws FetchError (causes 500 Internal Server Error)
+        // After fix: returns null gracefully
+        assertEquals(await ctx.getSignedKeyOwner(), null);
+      },
+    );
+
     await t.step("RequestContext.clone()", () => {
       const federation = createFederation<number>({
         kv,

packages/fedify/src/federation/middleware.ts

@@ -22,7 +22,7 @@ import type {
   DocumentLoaderFactoryOptions,
   GetUserAgentOptions,
 } from "@fedify/vocab-runtime";
-import { getDocumentLoader } from "@fedify/vocab-runtime";
+import { FetchError, getDocumentLoader } from "@fedify/vocab-runtime";
 import type {
   LookupWebFingerOptions,
   ResourceDescriptor,
@@ -2705,11 +2705,24 @@ class RequestContextImpl<TContextData> extends ContextImpl<TContextData>
     if (this.#signedKeyOwner != null) return this.#signedKeyOwner;
     const key = await this.getSignedKey(options);
     if (key == null) return this.#signedKeyOwner = null;
-    return this.#signedKeyOwner = await getKeyOwner(key, {
-      contextLoader: options.contextLoader ?? this.contextLoader,
-      documentLoader: options.documentLoader ?? this.documentLoader,
-      tracerProvider: options.tracerProvider ?? this.tracerProvider,
-    });
+    try {
+      return this.#signedKeyOwner = await getKeyOwner(key, {
+        contextLoader: options.contextLoader ?? this.contextLoader,
+        documentLoader: options.documentLoader ?? this.documentLoader,
+        tracerProvider: options.tracerProvider ?? this.tracerProvider,
+      });
+    } catch (error) {
+      if (error instanceof FetchError) {
+        getLogger(["fedify", "federation", "actor"]).warn(
+          "Failed to fetch the key owner {keyOwner} of {keyId} while " +
+            "verifying the request signature; treating the request as " +
+            "unauthenticated: {error}",
+          { keyId: key.id?.href, keyOwner: error.url.href, error },
+        );
+        return this.#signedKeyOwner = null;
+      }
+      throw error;
+    }
   }
 }