Skip to content

Commit a6a1aa5

Browse files
committed
fix(endpoint-auth): limit missing response_type to indieauth.com
A request without `response_type` is rejected again, as the specification requires, except when `client_id` is indieauth.com — the one widely-used client that omits it, and one carrying a deprecation notice pointing at indielogin.com, which sends a conforming request. The exception is a single commented block. Deleting it and restoring `request.query.response_type` in the two places below returns the endpoint to strict behaviour. The value is threaded as a local rather than written back to `request.query`: the application sets Express's query parser to `simple`, which re-parses on every access, so an assignment there is discarded before it can be read. Tests cover both sides of the exception: a request from any other client still reports the missing parameter, one from indieauth.com redirects to the consent form.
1 parent 2beb7bc commit a6a1aa5

3 files changed

Lines changed: 59 additions & 16 deletions

File tree

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

Lines changed: 25 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { IndiekitError } from "@indiekit/error";
2-
import { getCanonicalUrl } from "@indiekit/util";
2+
import { getCanonicalUrl, isSameOrigin } from "@indiekit/util";
33

44
import { getClientInformation } from "../client.js";
55
import { createRequestUri } from "../pushed-authorization-request.js";
@@ -24,22 +24,37 @@ export const authorizationController = {
2424
return next(true);
2525
}
2626

27+
// Deprecated exception, delete this block and restore
28+
// `request.query.response_type` below: indieauth.com omits
29+
// `response_type`, the pre-specification form of an authentication-only
30+
// request (`id`). Its replacement, indielogin.com, sends
31+
// `response_type=code` and needs none of this. `client_id` is not yet
32+
// known to be a URL, so it is checked before being compared.
33+
// @see {@link https://github.com/aaronpk/IndieAuth.com/blob/main/controllers/auth-web.rb#L518}
34+
const clientId = String(request.query.client_id);
35+
const isDeprecatedClient =
36+
URL.canParse(clientId) &&
37+
isSameOrigin(clientId, "https://indieauth.com");
38+
const responseType =
39+
request.query.response_type ?? (isDeprecatedClient ? "id" : undefined);
40+
2741
// Validate presence of required parameters
28-
for (const parameter of ["client_id", "redirect_uri", "state"]) {
29-
if (!Object.hasOwn(request.query, parameter)) {
42+
const requiredParameters = {
43+
client_id: request.query.client_id,
44+
redirect_uri: request.query.redirect_uri,
45+
response_type: responseType,
46+
state: request.query.state,
47+
};
48+
49+
for (const [parameter, value] of Object.entries(requiredParameters)) {
50+
if (value === undefined) {
3051
throw IndiekitError.badRequest(
3152
response.locals.__("BadRequestError.missingParameter", parameter),
3253
);
3354
}
3455
}
3556

36-
// `response_type` must be `code` (or deprecated `id`). Clients predating
37-
// the current specification omit it entirely for authentication-only
38-
// requests, which is what indieauth.com sends, so treat a missing value
39-
// as `id` rather than rejecting the request. Nothing downstream branches
40-
// on it: whether an access token is issued depends on the requested
41-
// scope.
42-
const responseType = request.query.response_type ?? "id";
57+
// `response_type` must be `code` (or deprecated `id`)
4358
if (!/^(code|id)$/.test(String(responseType))) {
4459
throw IndiekitError.badRequest(
4560
response.locals.__("BadRequestError.invalidValue", "response_type"),
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
import { strict as assert } from "node:assert";
2+
import { after, describe, it } from "node:test";
3+
4+
import { mockAgent } from "@indiekit-test/mock-agent";
5+
import { testServer } from "@indiekit-test/server";
6+
import supertest from "supertest";
7+
8+
await mockAgent("endpoint-auth");
9+
const server = await testServer();
10+
const request = supertest.agent(server);
11+
12+
describe("endpoint-auth GET /auth", () => {
13+
it("Returns documentation with no `response_type` error", async () => {
14+
const result = await request
15+
.get("/auth")
16+
.query({ client_id: "https://auth-endpoint.example" })
17+
.query({ redirect_uri: "https://auth-endpoint.example/redirect" })
18+
.query({ state: "12345" });
19+
20+
assert.equal(result.status, 200);
21+
assert.equal(
22+
result.text.includes("Missing parameter: <code>response_type</code>"),
23+
true,
24+
);
25+
});
26+
27+
after(() => server.close());
28+
});

‎packages/endpoint-auth/test/integration/302-authorization-no-response-type.js‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,14 +10,14 @@ const server = await testServer();
1010
const request = supertest.agent(server);
1111

1212
describe("endpoint-auth GET /auth", () => {
13-
it("Accepts a request with no `response_type`", async () => {
14-
// Clients predating the current specification omit `response_type` for
15-
// authentication-only requests, sending an empty `scope` alongside it.
16-
// indieauth.com still signs users in this way.
13+
it("Accepts a request from indieauth.com with no `response_type`", async () => {
14+
// indieauth.com omits `response_type` for authentication-only requests,
15+
// sending an empty `scope` alongside it. Every other client is still
16+
// required to send it, covered by 200-authorization-no-response-type.
1717
const result = await request
1818
.get("/auth")
19-
.query({ client_id: "https://auth-endpoint.example" })
20-
.query({ redirect_uri: "https://auth-endpoint.example/redirect" })
19+
.query({ client_id: "https://indieauth.com/" })
20+
.query({ redirect_uri: "https://indieauth.com/auth/redirect" })
2121
.query({ scope: "" })
2222
.query({ state: "12345" });
2323

0 commit comments

Comments
 (0)