fix(endpoint-auth): bind code exchange to the code’s own claims, not application state

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

@@ -2,7 +2,6 @@ import { IndiekitError } from "@indiekit/error";
 import { getCanonicalUrl } from "@indiekit/util";
 
 import { verifyCode } from "../pkce.js";
-import { validateRedirect } from "../redirect.js";
 import { verifyToken } from "../token.js";
 import { getRequestParameters } from "../utils.js";
 
@@ -12,7 +11,6 @@ import { getRequestParameters } from "../utils.js";
  */
 export const codeValidator = async (request, response, next) => {
   try {
-    const { client, usePkce } = request.app.locals;
     const parameters = getRequestParameters(request);
     const { client_id, code, code_verifier, grant_type, redirect_uri } =
       parameters;
@@ -45,33 +43,52 @@ export const codeValidator = async (request, response, next) => {
       );
     }
 
-    // Validate `client_id` against that provided in authorization request
-    if (getCanonicalUrl(client_id) !== getCanonicalUrl(client.id)) {
+    // Verify the code before reading anything from it
+    try {
+      request.verifiedToken = verifyToken(code);
+    } catch {
       throw IndiekitError.unauthorized(
-        response.locals.__("BadRequestError.invalidValue", "client_id"),
+        response.locals.__("UnauthorizedError.invalidToken"),
       );
     }
 
-    // Validate `redirect_uri`
-    const validRedirect = await validateRedirect(redirect_uri, client_id);
-    if (!validRedirect) {
-      throw IndiekitError.badRequest(
-        response.locals.__("BadRequestError.invalidValue", "redirect_uri"),
+    // An authorization code records the client it was issued to and the
+    // redirect it was issued for. A code missing either cannot be checked
+    // against the request, so it is not one this server issued.
+    if (
+      !request.verifiedToken.client_id ||
+      !request.verifiedToken.redirect_uri
+    ) {
+      throw IndiekitError.unauthorized(
+        response.locals.__("UnauthorizedError.invalidToken"),
       );
     }
 
-    // Verify token
-    try {
-      request.verifiedToken = verifyToken(code);
-    } catch {
+    // Validate `client_id` against the client the code was issued to. Reading
+    // it from the code, rather than from state shared across requests, is what
+    // ties this exchange to the authorization request that produced the code.
+    if (
+      getCanonicalUrl(client_id) !==
+      getCanonicalUrl(String(request.verifiedToken.client_id))
+    ) {
       throw IndiekitError.unauthorized(
-        response.locals.__("UnauthorizedError.invalidToken"),
+        response.locals.__("BadRequestError.invalidValue", "client_id"),
+      );
+    }
+
+    // Validate `redirect_uri` against the one the code was issued for. It was
+    // checked against the client's metadata during the authorization request,
+    // so matching it here is what remains to be done.
+    if (redirect_uri !== request.verifiedToken.redirect_uri) {
+      throw IndiekitError.badRequest(
+        response.locals.__("BadRequestError.invalidValue", "redirect_uri"),
       );
     }
 
-    // PKCE (Proof Key for Code Exchange)
-    if (usePkce) {
-      const { code_challenge } = request.verifiedToken;
+    // PKCE (Proof Key for Code Exchange). Whether it applies is recorded in
+    // the code itself, by the presence of the challenge it was issued with.
+    const { code_challenge } = request.verifiedToken;
+    if (code_challenge) {
       const verifiedCode = verifyCode(code_verifier, code_challenge);
       if (!verifiedCode) {
         throw IndiekitError.unauthorized(

packages/endpoint-auth/test/integration/200-authorization-profile-json.js

@@ -24,7 +24,9 @@ describe("endpoint-auth POST /auth", () => {
   it("Returns JSON profile", async () => {
     const code = signToken({
       access_token: "token",
+      client_id: "https://auth-endpoint.example",
       me: "https://website.example",
+      redirect_uri: "https://auth-endpoint.example/redirect",
       scope: "create update delete media",
       token_type: "Bearer",
     });

packages/endpoint-auth/test/integration/200-authorization-profile-no-grant-type.js

@@ -24,7 +24,9 @@ describe("endpoint-auth POST /auth", () => {
   it("Returns profile when `grant_type` omitted", async () => {
     const code = signToken({
       access_token: "token",
+      client_id: "https://auth-endpoint.example",
       me: "https://website.example",
+      redirect_uri: "https://auth-endpoint.example/redirect",
       scope: "create update delete media",
       token_type: "Bearer",
     });

packages/endpoint-auth/test/integration/200-authorization-profile-url-encoded.js

@@ -24,7 +24,9 @@ describe("endpoint-auth POST /auth", () => {
   it("Returns URL encoded profile", async () => {
     const code = signToken({
       access_token: "token",
+      client_id: "https://auth-endpoint.example",
       me: "https://website.example",
+      redirect_uri: "https://auth-endpoint.example/redirect",
       scope: "create update delete media",
       token_type: "Bearer",
     });

packages/endpoint-auth/test/integration/200-token-grant-json.js

@@ -24,7 +24,9 @@ describe("endpoint-auth POST /auth/token", () => {
   it("Returns JSON access token", async () => {
     const code = signToken({
       access_token: "token",
+      client_id: "https://auth-endpoint.example",
       me: "https://website.example",
+      redirect_uri: "https://auth-endpoint.example/redirect",
       scope: "create update delete media",
       token_type: "Bearer",
     });

packages/endpoint-auth/test/integration/200-token-grant-url-encoded.js

@@ -24,7 +24,9 @@ describe("endpoint-auth POST /auth/token", () => {
   it("Returns URL encoded access token", async () => {
     const code = signToken({
       access_token: "token",
+      client_id: "https://auth-endpoint.example",
       me: "https://website.example",
+      redirect_uri: "https://auth-endpoint.example/redirect",
       scope: "create update delete media",
       token_type: "Bearer",
     });

packages/endpoint-auth/test/integration/400-token-grant-invalid-pkce-code.js

@@ -29,8 +29,10 @@ describe("endpoint-auth POST /auth/token", () => {
   it("Returns 401 error fails PKCE code challenge", async () => {
     const code = signToken({
       access_token: "token",
+      client_id: "https://auth-endpoint.example",
       code_challenge: codeChallenge,
       code_challenge_method: "S256",
+      redirect_uri: "https://auth-endpoint.example/redirect",
       scope: "create update delete media",
       token_type: "Bearer",
     });

packages/endpoint-auth/test/integration/400-token-grant-invalid-redirect-uri.js

@@ -24,7 +24,9 @@ describe("endpoint-auth POST /auth/token", () => {
   it("Returns 400 error invalid `redirect_uri`", async () => {
     const code = signToken({
       access_token: "token",
+      client_id: "https://auth-endpoint.example",
       me: "https://website.example",
+      redirect_uri: "https://auth-endpoint.example/redirect",
       scope: "create update delete media",
       token_type: "Bearer",
     });

packages/endpoint-auth/test/integration/401-token-grant-code-issued-to-other-client.js

@@ -0,0 +1,49 @@
+import { strict as assert } from "node:assert";
+import { after, before, describe, it } from "node:test";
+
+import { mockAgent } from "@indiekit-test/mock-agent";
+import { testServer } from "@indiekit-test/server";
+import supertest from "supertest";
+
+import { signToken } from "../../lib/token.js";
+
+await mockAgent("endpoint-auth");
+const server = await testServer();
+const request = supertest.agent(server);
+
+describe("endpoint-auth POST /auth/token", () => {
+  // Begin an authorization request as this client, so that whatever
+  // application-wide state the server keeps refers to it.
+  before(async () => {
+    await request
+      .get("/auth")
+      .query({ client_id: "https://auth-endpoint.example" })
+      .query({ redirect_uri: "https://auth-endpoint.example/redirect" })
+      .query({ response_type: "code" })
+      .query({ state: "12345" });
+  });
+
+  // The authorization code records the client it was issued to. Redeeming it
+  // as a different client must fail, whatever authorization request happened
+  // most recently on the server.
+  it("Rejects a code issued to a different client", async () => {
+    const code = signToken({
+      client_id: "https://other-client.example",
+      me: "https://website.example",
+      redirect_uri: "https://other-client.example/redirect",
+      scope: "create",
+    });
+    const result = await request
+      .post("/auth/token")
+      .set("accept", "application/json")
+      .query({ client_id: "https://auth-endpoint.example" })
+      .query({ code })
+      .query({ grant_type: "authorization_code" })
+      .query({ redirect_uri: "https://auth-endpoint.example/redirect" });
+
+    assert.notEqual(result.status, 200);
+    assert.equal(result.body.access_token, undefined);
+  });
+
+  after(() => server.close());
+});

packages/endpoint-auth/test/integration/401-token-grant-invalid-client-id.js

@@ -24,7 +24,9 @@ describe("endpoint-auth POST /auth/token", () => {
   it("Returns 401 error invalid `client_id`", async () => {
     const code = signToken({
       access_token: "token",
+      client_id: "https://auth-endpoint.example",
       me: "https://website.example",
+      redirect_uri: "https://website.example/redirect",
       scope: "create update delete media",
       token_type: "Bearer",
     });

packages/endpoint-auth/test/integration/401-token-grant-no-authorization-request.js

@@ -0,0 +1,36 @@
+import { strict as assert } from "node:assert";
+import { after, describe, it } from "node:test";
+
+import { testServer } from "@indiekit-test/server";
+import supertest from "supertest";
+
+import { signToken } from "../../lib/token.js";
+
+const server = await testServer();
+const request = supertest.agent(server);
+
+describe("endpoint-auth POST /auth/token", () => {
+  // No authorization request precedes this exchange, so the code itself has to
+  // carry everything needed to validate it. The response should describe the
+  // problem, not fail with an unhandled error.
+  it("Returns an error, not 500, with no preceding authorization request", async () => {
+    const code = signToken({
+      client_id: "https://client.example",
+      me: "https://website.example",
+      redirect_uri: "https://client.example/redirect",
+      scope: "create",
+    });
+    const result = await request
+      .post("/auth/token")
+      .set("accept", "application/json")
+      .query({ client_id: "https://other-client.example" })
+      .query({ code })
+      .query({ grant_type: "authorization_code" })
+      .query({ redirect_uri: "https://other-client.example/redirect" });
+
+    assert.notEqual(result.status, 500);
+    assert.equal(result.body.error, "unauthorized");
+  });
+
+  after(() => server.close());
+});