diff --git a/.changeset/image-type-check-before-dataset.md b/.changeset/image-type-check-before-dataset.md new file mode 100644 index 0000000000..7342b50ef8 --- /dev/null +++ b/.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. diff --git a/packages/provider/src/api/captcha/getImageCaptchaChallenge.ts b/packages/provider/src/api/captcha/getImageCaptchaChallenge.ts index cf94e7f9f1..d186d5ae8f 100644 --- a/packages/provider/src/api/captcha/getImageCaptchaChallenge.ts +++ b/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 diff --git a/packages/provider/src/tests/unit/api/captcha/imageChallengeClientDatasetId.unit.test.ts b/packages/provider/src/tests/unit/api/captcha/imageChallengeClientDatasetId.unit.test.ts index ff9ac1a4ba..ff9c4a14fe 100644 --- a/packages/provider/src/tests/unit/api/captcha/imageChallengeClientDatasetId.unit.test.ts +++ b/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>; }; + imgCaptchaManager: { + getPrioritisedAccessPolicies: Mock<() => Promise<[]>>; + isValidRequest: Mock<() => Promise>; + }; }; const PAST_DATASET_CHECK = "past the dataset check"; @@ -129,6 +136,12 @@ beforeEach(() => { }); }), }, + imgCaptchaManager: { + getPrioritisedAccessPolicies: vi.fn(async (): Promise<[]> => []), + isValidRequest: vi.fn( + async (): Promise => ({ 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();