Provider: reject requests where the header and body site keys differ

.changeset/client-body-site-key-matches-header.md

@@ -0,0 +1,5 @@
+---
+"@prosopo/provider": patch
+---
+
+Client captcha requests are now refused when the site key in the request body differs from the `prosopo-site-key` header. The allowed-domain check reads the header, but the captcha handlers act on the site key in the body, so a page on one site could send its own site key in the header and another site's key in the body and get that other site's challenges without its domain being allowed. The same gap let the reserved test site keys, which skip the domain check, carry any site's key in the body.

packages/provider/src/api/domainMiddleware.ts

@@ -56,6 +56,13 @@ export const domainMiddleware = (env: ProviderEnvironment) => {
 				throw invalidSiteKeyError(siteKey);
 			}
 
+			// Handlers act on the site key in the body, so it must be the one
+			// whose origin allowlist is checked here. Otherwise a request can
+			// pass as the caller's own site while acting for another.
+			const bodySiteKey = bodyDapp(req.body);
+			if (bodySiteKey !== undefined && bodySiteKey !== siteKey)
+				throw invalidSiteKeyError(siteKey);
+
 			// Reserved CI test site keys have no DB record and no domain
 			// allowlist; let them through so the deterministic test flow works in
 			// every environment and from any origin.
@@ -113,6 +120,11 @@ export const domainMiddleware = (env: ProviderEnvironment) => {
 	};
 };
 
+const bodyDapp = (body: unknown): unknown =>
+	typeof body === "object" && body !== null && "dapp" in body
+		? body.dapp
+		: undefined;
+
 // If the origin is a Google Translate proxy URL, return the decoded original
 // origin(s) so the domain check can still match the site's allowed domains.
 const googleTranslateOrigins = (origin: string): string[] => {

packages/provider/src/tests/unit/api/domainMiddleware.unit.test.ts

@@ -68,6 +68,7 @@ vi.mock("../../../tasks/index.js", async () => {
 });
 
 const SITE_KEY = "5GrwvaEF5zXb26Fz9rcQpDWS57CtERHpNehXCPcNoHGKutQY";
+const OTHER_SITE_KEY = "5FHneW46xGXgs5mUiveU4sbTyGBzmstUspZC92UhjJM694ty";
 
 const ClientModel = mongoose.model<ClientRecord>(
 	"DomainMiddlewareTestClient",
@@ -97,6 +98,7 @@ beforeAll(async () => {
 	db.getClientRecord = getClientRecord;
 
 	const app = express();
+	app.use(express.json());
 	app.use((req, _res, next) => {
 		req.t = t;
 		req.i18n = i18n;
@@ -139,10 +141,22 @@ interface GateResult {
  * POSTs to the gated route with exactly the given headers. node:http is used
  * rather than fetch because fetch decides the Origin header for itself.
  */
-const send = (headers: Record<string, string>): Promise<GateResult> =>
+const send = (
+	headers: Record<string, string>,
+	body?: object,
+): Promise<GateResult> =>
 	new Promise<GateResult>((resolve, reject) => {
 		const req = http.request(
-			{ host: "127.0.0.1", port, path: "/", method: "POST", headers },
+			{
+				host: "127.0.0.1",
+				port,
+				path: "/",
+				method: "POST",
+				headers:
+					body === undefined
+						? headers
+						: { ...headers, "content-type": "application/json" },
+			},
 			(res) => {
 				let body = "";
 				res.setEncoding("utf8");
@@ -158,7 +172,7 @@ const send = (headers: Record<string, string>): Promise<GateResult> =>
 			},
 		);
 		req.on("error", reject);
-		req.end();
+		req.end(body === undefined ? undefined : JSON.stringify(body));
 	});
 
 const withKey = (origin?: string): Record<string, string> => ({
@@ -239,6 +253,44 @@ describe("domainMiddleware", () => {
 		expect(result.status).toBeGreaterThanOrEqual(400);
 	});
 
+	it("lets a request through when the body site key matches the header", async () => {
+		getClientRecord.mockResolvedValue(clientRecord(["example.com"]));
+		const result = await send(withKey("https://example.com"), {
+			dapp: SITE_KEY,
+		});
+		expect(result).toEqual({ status: 200, reached: true });
+	});
+
+	it.each([
+		["another site's key", OTHER_SITE_KEY],
+		["a non-string", [SITE_KEY]],
+	])(
+		"rejects a body site key that is %s",
+		async (_name: string, dapp: unknown) => {
+			getClientRecord.mockResolvedValue(clientRecord(["example.com"]));
+			const result = await send(withKey("https://example.com"), { dapp });
+			expect(result).toEqual({ status: 400, reached: false });
+			expect(getClientRecord).not.toHaveBeenCalled();
+		},
+	);
+
+	it("rejects another site's body key behind a reserved test site key", async () => {
+		const result = await send(
+			{
+				"prosopo-site-key": ALWAYS_PASS_SITE_KEY,
+				origin: "https://anything.invalid",
+			},
+			{ dapp: OTHER_SITE_KEY },
+		);
+		expect(result).toEqual({ status: 400, reached: false });
+	});
+
+	it("lets a request with no body site key through on the header alone", async () => {
+		getClientRecord.mockResolvedValue(clientRecord(["example.com"]));
+		const result = await send(withKey("https://example.com"), {});
+		expect(result).toEqual({ status: 200, reached: true });
+	});
+
 	it("fails closed when the client lookup throws", async () => {
 		getClientRecord.mockRejectedValue(new Error("mongo down"));
 		const result = await send(withKey("https://example.com"));