diff --git a/packages/endpoint-auth/lib/middleware/code.js b/packages/endpoint-auth/lib/middleware/code.js index 56451f32c..a75cf49a3 100644 --- a/packages/endpoint-auth/lib/middleware/code.js +++ b/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( diff --git a/packages/endpoint-auth/test/integration/200-authorization-profile-json.js b/packages/endpoint-auth/test/integration/200-authorization-profile-json.js index 544ad47f4..808f750ef 100644 --- a/packages/endpoint-auth/test/integration/200-authorization-profile-json.js +++ b/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", }); diff --git a/packages/endpoint-auth/test/integration/200-authorization-profile-no-grant-type.js b/packages/endpoint-auth/test/integration/200-authorization-profile-no-grant-type.js index c64a9ed0f..98aa68a50 100644 --- a/packages/endpoint-auth/test/integration/200-authorization-profile-no-grant-type.js +++ b/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", }); diff --git a/packages/endpoint-auth/test/integration/200-authorization-profile-url-encoded.js b/packages/endpoint-auth/test/integration/200-authorization-profile-url-encoded.js index 2578261d0..3a7347354 100644 --- a/packages/endpoint-auth/test/integration/200-authorization-profile-url-encoded.js +++ b/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", }); diff --git a/packages/endpoint-auth/test/integration/200-token-grant-json.js b/packages/endpoint-auth/test/integration/200-token-grant-json.js index 4dd4d6a99..5e12e60ff 100644 --- a/packages/endpoint-auth/test/integration/200-token-grant-json.js +++ b/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", }); diff --git a/packages/endpoint-auth/test/integration/200-token-grant-url-encoded.js b/packages/endpoint-auth/test/integration/200-token-grant-url-encoded.js index 06ef5958b..d7452ff32 100644 --- a/packages/endpoint-auth/test/integration/200-token-grant-url-encoded.js +++ b/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", }); diff --git a/packages/endpoint-auth/test/integration/400-token-grant-invalid-pkce-code.js b/packages/endpoint-auth/test/integration/400-token-grant-invalid-pkce-code.js index 223260b7d..51cb09e29 100644 --- a/packages/endpoint-auth/test/integration/400-token-grant-invalid-pkce-code.js +++ b/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", }); diff --git a/packages/endpoint-auth/test/integration/400-token-grant-invalid-redirect-uri.js b/packages/endpoint-auth/test/integration/400-token-grant-invalid-redirect-uri.js index f82556248..f1dcdc517 100644 --- a/packages/endpoint-auth/test/integration/400-token-grant-invalid-redirect-uri.js +++ b/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", }); diff --git a/packages/endpoint-auth/test/integration/401-token-grant-code-issued-to-other-client.js b/packages/endpoint-auth/test/integration/401-token-grant-code-issued-to-other-client.js new file mode 100644 index 000000000..a5f16940a --- /dev/null +++ b/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()); +}); diff --git a/packages/endpoint-auth/test/integration/401-token-grant-invalid-client-id.js b/packages/endpoint-auth/test/integration/401-token-grant-invalid-client-id.js index 49e48b205..5d2394b5f 100644 --- a/packages/endpoint-auth/test/integration/401-token-grant-invalid-client-id.js +++ b/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", }); diff --git a/packages/endpoint-auth/test/integration/401-token-grant-no-authorization-request.js b/packages/endpoint-auth/test/integration/401-token-grant-no-authorization-request.js new file mode 100644 index 000000000..accd130db --- /dev/null +++ b/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()); +});