Skip to content

Commit fa1d368

Browse files
rmdespaulrobertlloyd
authored andcommitted
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 35ee4bb commit fa1d368

3 files changed

Lines changed: 58 additions & 10 deletions

File tree

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

Lines changed: 24 additions & 9 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 for indieauth.com
28+
// indieauth.com omits `response_type`, the pre-specification form of an
29+
// authentication-only request (`id`). Its replacement, indielogin.com,
30+
// sends `response_type=code` and needs none of this. `client_id` is not
31+
// yet known to be a URL, so it is checked before being compared.
32+
// @see {@link https://github.com/aaronpk/IndieAuth.com/blob/main/controllers/auth-web.rb#L518}
33+
const clientId = String(request.query.client_id);
34+
const isDeprecatedClient =
35+
URL.canParse(clientId) &&
36+
isSameOrigin(clientId, "https://indieauth.com");
37+
const responseType =
38+
request.query.response_type ?? (isDeprecatedClient ? "id" : undefined);
39+
2740
// Validate presence of required parameters
28-
for (const parameter of [
29-
"client_id",
30-
"redirect_uri",
31-
"response_type",
32-
"state",
33-
]) {
34-
if (!Object.hasOwn(request.query, parameter)) {
41+
const requiredParameters = {
42+
client_id: request.query.client_id,
43+
redirect_uri: request.query.redirect_uri,
44+
response_type: responseType,
45+
state: request.query.state,
46+
};
47+
48+
for (const [parameter, value] of Object.entries(requiredParameters)) {
49+
if (value === undefined) {
3550
throw IndiekitError.badRequest(
3651
response.locals.__("BadRequestError.missingParameter", parameter),
3752
);
3853
}
3954
}
4055

4156
// `response_type` must be `code` (or deprecated `id`)
42-
if (!/^(code|id)$/.test(String(request.query.response_type))) {
57+
if (!/^(code|id)$/.test(String(responseType))) {
4358
throw IndiekitError.badRequest(
4459
response.locals.__("BadRequestError.invalidValue", "response_type"),
4560
);

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,15 @@ const server = await testServer();
1010
const request = supertest.agent(server);
1111

1212
describe("endpoint-auth GET /auth", () => {
13-
it("Returns documentation with no `response_type` error", async () => {
13+
it("Rejects 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.
1417
const result = await request
1518
.get("/auth")
1619
.query({ client_id: "https://auth-endpoint.example" })
1720
.query({ redirect_uri: "https://auth-endpoint.example/redirect" })
21+
.query({ scope: "" })
1822
.query({ state: "12345" });
1923

2024
assert.equal(result.status, 200);
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
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("Accepts a indieauth.com request with no `response_type`", async () => {
14+
// indieauth.com predates the current specification, omitting
15+
// `response_type` for authentication-only requests and sending an empty
16+
// value for `scope`.
17+
const result = await request
18+
.get("/auth")
19+
.query({ client_id: "https://indieauth.com" })
20+
.query({ redirect_uri: "https://indieauth.com/redirect" })
21+
.query({ scope: "" })
22+
.query({ state: "12345" });
23+
24+
assert.equal(result.status, 302);
25+
assert.match(result.headers.location, /request_uri=/);
26+
});
27+
28+
after(() => server.close());
29+
});

0 commit comments

Comments
 (0)