feat(endpoint-auth): validate cross-host redirect_uri against client_id

helpers/mock-agent/endpoint-auth.js

@@ -49,6 +49,53 @@ export const mockClient = () => {
   // Client information (Not Found)
   agent.get(origin).intercept({ path: "/404" }).reply(404);
 
+  // Declared `redirect_uri` (HTML `<link>` tag)
+  agent
+    .get(origin)
+    .intercept({ path: "/declares-redirect" })
+    .reply(
+      200,
+      `<html><head>
+        <link rel="redirect_uri" href="https://redirect.example/callback">
+      </head><body><p>Client</p></body></html>`,
+      { headers: { "content-type": "text/html" } },
+    )
+    .persist();
+
+  // Declared `redirect_uri` (HTTP `Link` header)
+  agent
+    .get(origin)
+    .intercept({ path: "/declares-redirect-header" })
+    .reply(200, "<html><head></head><body><p>Client</p></body></html>", {
+      headers: {
+        "content-type": "text/html",
+        link: '<https://redirect.example/callback>; rel="redirect_uri"',
+      },
+    })
+    .persist();
+
+  // Declared `redirect_uri` containing a pattern, which is matched literally
+  agent
+    .get(origin)
+    .intercept({ path: "/declares-wildcard" })
+    .reply(
+      200,
+      `<html><head>
+        <link rel="redirect_uri" href="https://*.extension.example/">
+      </head><body><p>Client</p></body></html>`,
+      { headers: { "content-type": "text/html" } },
+    )
+    .persist();
+
+  // No declared `redirect_uri`
+  agent
+    .get(origin)
+    .intercept({ path: "/declares-nothing" })
+    .reply(200, "<html><head></head><body><p>Client</p></body></html>", {
+      headers: { "content-type": "text/html" },
+    })
+    .persist();
+
   // Profile URL response
   agent
     .get(origin)

packages/endpoint-auth/lib/controllers/authorization.js

@@ -65,7 +65,7 @@ export const authorizationController = {
         request.query;
 
       // Validate `redirect_uri`
-      const validRedirect = validateRedirect(
+      const validRedirect = await validateRedirect(
         String(redirect_uri),
         String(client_id),
       );

packages/endpoint-auth/lib/middleware/code.js

@@ -10,7 +10,7 @@ import { getRequestParameters } from "../utils.js";
  * Validate authorization code before redeeming
  * @type {import("express").RequestHandler}
  */
-export const codeValidator = (request, response, next) => {
+export const codeValidator = async (request, response, next) => {
   try {
     const { client, usePkce } = request.app.locals;
     const parameters = getRequestParameters(request);
@@ -46,7 +46,7 @@ export const codeValidator = (request, response, next) => {
     }
 
     // Validate `redirect_uri`
-    const validRedirect = validateRedirect(redirect_uri, client_id);
+    const validRedirect = await validateRedirect(redirect_uri, client_id);
     if (!validRedirect) {
       throw IndiekitError.badRequest(
         response.locals.__("BadRequestError.invalidValue", "redirect_uri"),

packages/endpoint-auth/lib/redirect.js

@@ -1,16 +1,128 @@
+import { mf2 } from "microformats-parser";
+
+const FETCH_TIMEOUT = 5000;
+
 /**
  * Validate `redirect_uri`
+ *
+ * A redirect URI sharing the same host as `client_id` is always allowed. One
+ * on a different host is only allowed if `client_id` declares it, via a
+ * `<link rel="redirect_uri">` tag or a `Link` HTTP header. Anything that
+ * prevents that check from succeeding (network error, non-2xx response,
+ * unparseable markup) denies the redirect.
  * @param {string} redirectUri - Redirect URL
  * @param {string} clientId - URL of client
- * @returns {boolean} Valid redirect
+ * @returns {Promise<boolean>} Valid redirect
  * @see {@link https://indieauth.spec.indieweb.org/#redirect-url}
- * @todo If redirect URIs doesn’t share same host as `client_id`, validate
- * against list of redirect URIs fetched from `<link>` tags or `Link` HTTP
- * headers with a `rel` attribute of `redirect_uri` at the `client_id` URL.
  */
-export const validateRedirect = (redirectUri, clientId) => {
-  const redirectHost = new URL(redirectUri).host;
-  const clientHost = new URL(clientId).host;
+export const validateRedirect = async (redirectUri, clientId) => {
+  let redirectUrl;
+  let clientUrl;
+
+  try {
+    redirectUrl = new URL(redirectUri);
+    clientUrl = new URL(clientId);
+  } catch {
+    return false;
+  }
+
+  if (redirectUrl.host === clientUrl.host) {
+    return true;
+  }
+
+  const declaredUris = await getDeclaredRedirectUris(clientId);
+
+  return declaredUris.some((declaredUri) =>
+    matchesDeclaredUri(redirectUrl, declaredUri),
+  );
+};
+
+/**
+ * Get redirect URIs declared at a `client_id` URL
+ * @param {string} clientId - URL of client
+ * @returns {Promise<string[]>} Declared redirect URIs
+ */
+const getDeclaredRedirectUris = async (clientId) => {
+  try {
+    const response = await fetch(clientId, {
+      headers: { accept: "text/html" },
+      signal: AbortSignal.timeout(FETCH_TIMEOUT),
+    });
+
+    if (!response.ok) {
+      return [];
+    }
+
+    const fromHeader = parseLinkHeader(response.headers.get("link"));
+    const body = await response.text();
+    const { rels } = mf2(body, { baseUrl: clientId });
+
+    return [...fromHeader, ...(rels.redirect_uri || [])];
+  } catch {
+    // Fail closed: an undiscoverable declaration is not a valid declaration
+    return [];
+  }
+};
+
+/**
+ * Get URIs from a `Link` HTTP header with a `rel` of `redirect_uri`
+ * @param {string|null} header - `Link` header value
+ * @returns {string[]} Declared redirect URIs
+ * @see {@link https://datatracker.ietf.org/doc/html/rfc8288#section-3}
+ */
+const parseLinkHeader = (header) => {
+  if (!header) {
+    return [];
+  }
+
+  const uris = [];
+
+  // Split on commas separating link values, not those within parameters
+  for (const value of header.split(/,(?=\s*<)/)) {
+    const [, uri, parameters] = value.match(/<([^>]*)>\s*;\s*(.*)/s) || [];
+
+    if (uri) {
+      const [, relationship] =
+        parameters.match(/rel\s*=\s*"?([^";]+)"?/i) || [];
+
+      if (relationship?.trim().split(/\s+/).includes("redirect_uri")) {
+        uris.push(uri);
+      }
+    }
+  }
+
+  return uris;
+};
+
+/**
+ * Check a redirect URL against a declared redirect URI
+ *
+ * Compared exactly, ignoring only a trailing slash on the path. Patterns are
+ * deliberately not supported: the OAuth working group’s position is that
+ * wildcards in redirect URLs open up attack vectors, and clients that appear
+ * to need one generally do not. A browser extension’s redirect host is
+ * derived from its extension ID, so it is fixed for a given extension and can
+ * be declared literally.
+ * @param {URL} redirectUrl - Redirect URL
+ * @param {string} declaredUri - Declared redirect URI
+ * @returns {boolean} Redirect URL matches declared URI
+ * @see {@link https://github.com/indieweb/indieauth/issues/22#issuecomment-544204967}
+ */
+const matchesDeclaredUri = (redirectUrl, declaredUri) => {
+  let declaredUrl;
+  try {
+    declaredUrl = new URL(declaredUri);
+  } catch {
+    return false;
+  }
+
+  // Ignore a trailing slash when comparing paths
+  const redirectPath = redirectUrl.pathname.replace(/\/$/, "");
+  const declaredPath = declaredUrl.pathname.replace(/\/$/, "");
 
-  return redirectHost === clientHost;
+  return (
+    redirectUrl.protocol === declaredUrl.protocol &&
+    redirectUrl.host === declaredUrl.host &&
+    redirectPath === declaredPath
+  );
 };

packages/endpoint-auth/test/unit/redirect.js

@@ -1,30 +1,127 @@
 import { strict as assert } from "node:assert";
 import { describe, it } from "node:test";
 
+import { mockAgent } from "@indiekit-test/mock-agent";
+
 import { validateRedirect } from "../../lib/redirect.js";
 
+await mockAgent("endpoint-auth");
+
+const origin = "https://auth-endpoint.example";
+
 describe("endpoint-auth/lib/redirect", () => {
-  it("Validates `redirect_uri`", () => {
+  it("Validates `redirect_uri`", async () => {
     assert.equal(
-      validateRedirect(
+      await validateRedirect(
         "https://client.example:3000",
         "https://client.example:3000/redirect",
       ),
       true,
     );
     assert.equal(
-      validateRedirect(
+      await validateRedirect(
         "https://client.example:3000",
         "https://client.example:8080/redirect",
       ),
       false,
     );
     assert.equal(
-      validateRedirect(
+      await validateRedirect(
         "https://client.example",
         "https://www.client.example/redirect",
       ),
       false,
     );
   });
+
+  it("Validates `redirect_uri` declared in `<link>` tag", async () => {
+    assert.equal(
+      await validateRedirect(
+        "https://redirect.example/callback",
+        `${origin}/declares-redirect`,
+      ),
+      true,
+    );
+  });
+
+  it("Validates `redirect_uri` declared in `Link` header", async () => {
+    assert.equal(
+      await validateRedirect(
+        "https://redirect.example/callback",
+        `${origin}/declares-redirect-header`,
+      ),
+      true,
+    );
+  });
+
+  it("Invalidates `redirect_uri` not declared at `client_id`", async () => {
+    assert.equal(
+      await validateRedirect(
+        "https://attacker.example/callback",
+        `${origin}/declares-redirect`,
+      ),
+      false,
+    );
+    assert.equal(
+      await validateRedirect(
+        "https://redirect.example/callback",
+        `${origin}/declares-nothing`,
+      ),
+      false,
+    );
+  });
+
+  it("Invalidates declared `redirect_uri` with different path or scheme", async () => {
+    assert.equal(
+      await validateRedirect(
+        "https://redirect.example/elsewhere",
+        `${origin}/declares-redirect`,
+      ),
+      false,
+    );
+    assert.equal(
+      await validateRedirect(
+        // eslint-disable-next-line unicorn/prefer-https -- downgraded scheme is the condition under test
+        "http://redirect.example/callback",
+        `${origin}/declares-redirect`,
+      ),
+      false,
+    );
+  });
+
+  it("Invalidates `redirect_uri` matching a declared pattern", async () => {
+    // A declared `*.` prefix is matched literally, not as a wildcard: any
+    // pattern support in redirect URLs opens up attack vectors
+    for (const redirectUri of [
+      "https://abc123.extension.example/",
+      "https://extension.example/",
+      "https://a.b.extension.example/",
+    ]) {
+      assert.equal(
+        await validateRedirect(redirectUri, `${origin}/declares-wildcard`),
+        false,
+      );
+    }
+  });
+
+  it("Invalidates `redirect_uri` if `client_id` can’t be fetched", async () => {
+    assert.equal(
+      await validateRedirect(
+        "https://redirect.example/callback",
+        `${origin}/404`,
+      ),
+      false,
+    );
+  });
+
+  it("Invalidates invalid URLs", async () => {
+    assert.equal(
+      await validateRedirect("foo", "https://client.example"),
+      false,
+    );
+    assert.equal(
+      await validateRedirect("https://client.example", "bar"),
+      false,
+    );
+  });
 });