Check the image captcha type before the requested dataset

.changeset/image-type-check-before-dataset.md

@@ -0,0 +1,5 @@
+---
+"@prosopo/provider": patch
+---
+
+An image challenge request now has its captcha type checked before its `datasetId`. Since 3.9.1 the unknown-dataset check (`INVALID_REQUEST:PROV-94`) ran first, so a site key that serves another captcha type got "Unknown dataset" instead of `INCORRECT_CAPTCHA_TYPE` whenever it sent an unknown `datasetId`. That broke the demo monitor, which uses exactly that answer to tell which captcha type a key serves, and it has reported every demo as broken since the 3.9.1 deploy.

packages/provider/src/api/captcha/getImageCaptchaChallenge.ts

@@ -175,20 +175,6 @@ export default (
 				);
 			}
 
-			if (
-				clientDatasetId !== undefined &&
-				!(await clientDatasetExists(tasks, clientDatasetId))
-			) {
-				return next(
-					new ProsopoError({
-						code: "INVALID_REQUEST",
-						errorSite: "PROV-94",
-						message: "Unknown dataset",
-						context: { siteKey: dapp },
-					}),
-				);
-			}
-
 			// Get country code for geoblocking from middleware-provided IP info
 			const countryCode =
 				req.ipInfo && "isValid" in req.ipInfo && req.ipInfo.isValid
@@ -268,6 +254,23 @@ export default (
 				);
 			}
 
+			// After the type check, so a key that is not served image is told
+			// INCORRECT_CAPTCHA_TYPE whatever datasetId it sends. The demo monitor
+			// relies on that to tell which type a key serves.
+			if (
+				clientDatasetId !== undefined &&
+				!(await clientDatasetExists(tasks, clientDatasetId))
+			) {
+				return next(
+					new ProsopoError({
+						code: "INVALID_REQUEST",
+						errorSite: "PROV-94",
+						message: "Unknown dataset",
+						context: { siteKey: dapp },
+					}),
+				);
+			}
+
 			// Evaluate the site's trafficFilter against the connecting IP.
 			// Only `challenge` policies affect the request-time gate — they
 			// contribute solvedImagesCount overrides. `block` policies are

packages/provider/src/tests/unit/api/captcha/imageChallengeClientDatasetId.unit.test.ts

@@ -17,6 +17,7 @@ import { handleErrors } from "@prosopo/api-express-router";
 import { ProsopoError } from "@prosopo/common";
 import { Keyring } from "@prosopo/keyring";
 import { getLogger } from "@prosopo/logger";
+import { ResultReason } from "@prosopo/types";
 import type { ProviderEnvironment } from "@prosopo/types-env";
 import type { AccessRulesStorage } from "@prosopo/user-access-policy";
 import express, {
@@ -43,11 +44,17 @@ declare module "@prosopo/common" {
 	}
 }
 
+type ValidRequestResult = { valid: boolean; reason?: string };
+
 type TasksStub = {
 	db: {
 		getClientRecord: Mock<() => Promise<{ settings: object }>>;
 		getDatasetDetails: Mock<(datasetId: string) => Promise<object>>;
 	};
+	imgCaptchaManager: {
+		getPrioritisedAccessPolicies: Mock<() => Promise<[]>>;
+		isValidRequest: Mock<() => Promise<ValidRequestResult>>;
+	};
 };
 
 const PAST_DATASET_CHECK = "past the dataset check";
@@ -129,6 +136,12 @@ beforeEach(() => {
 				});
 			}),
 		},
+		imgCaptchaManager: {
+			getPrioritisedAccessPolicies: vi.fn(async (): Promise<[]> => []),
+			isValidRequest: vi.fn(
+				async (): Promise<ValidRequestResult> => ({ valid: true }),
+			),
+		},
 	};
 });
 
@@ -177,6 +190,17 @@ describe("image challenge with a client-supplied datasetId", () => {
 		expect(status).toBe(500);
 	});
 
+	it("reports the wrong captcha type before an unknown datasetId", async () => {
+		holder.tasks?.imgCaptchaManager.isValidRequest.mockResolvedValueOnce({
+			valid: false,
+			reason: ResultReason.INCORRECT_CAPTCHA_TYPE,
+		});
+		const { status, text } = await post(UNKNOWN_DATASET);
+		expect(status).toBe(400);
+		expect(text).toContain("INCORRECT_CAPTCHA_TYPE");
+		expect(holder.tasks?.db.getDatasetDetails).not.toHaveBeenCalled();
+	});
+
 	it("does not look up the dataset when the client omits it", async () => {
 		const { status } = await post(undefined);
 		expect(holder.tasks?.db.getDatasetDetails).not.toHaveBeenCalled();