From 823edca377741c0d6981bef0b131ebebcd23b515 Mon Sep 17 00:00:00 2001 From: Yahya Fakhroji Date: Sat, 26 Sep 2026 00:02:55 +0700 Subject: [PATCH] fix(recaptcha): drop the local token-age check that rejected fresh tokens verifyRecaptcha accepted Google's success verdict and then rejected the token as "stale" when challenge_ts was more than two minutes old. For v3, challenge_ts is when the page's challenge was created, not when execute() minted the token at submit time. Any form submitted more than about two minutes after page load failed with a clean score. Google already enforces expiry and single use, and reports both as success=false with timeout-or-duplicate, which lands as reason "rejected". The local check was redundant and anchored on the wrong timestamp. Removes MAX_TOKEN_AGE_MS, the "stale" reason, and the tolerateStale option from recaptchaRejects, so the recovery code step calls it plainly again. The no-token, rejected, action-mismatch, hostname-mismatch and fail-open paths are unchanged. Staging, on both recovery steps: signup_recaptcha_scored verdict=invalid reason=stale score=0.9 action=recovery signup_recaptcha_scored verdict=invalid reason=stale score=0.9 action=recovery_code --- app/modules/i18n/locales/en.po | 24 ++++++++++----------- app/routes/recover/index.tsx | 10 +-------- app/server/infra/recaptcha.server.ts | 27 ++++-------------------- cypress/component/server/recaptcha.cy.ts | 22 ++++++++++++++++--- 4 files changed, 36 insertions(+), 47 deletions(-) diff --git a/app/modules/i18n/locales/en.po b/app/modules/i18n/locales/en.po index 00fa789a3..6c3ba944b 100644 --- a/app/modules/i18n/locales/en.po +++ b/app/modules/i18n/locales/en.po @@ -164,7 +164,7 @@ msgid "Change your password" msgstr "Change your password" #: app/routes/password/reset.tsx:109 -#: app/routes/recover/index.tsx:315 +#: app/routes/recover/index.tsx:307 #: app/routes/signup/index.tsx:408 #: app/routes/signup/method.tsx:259 #: app/routes/signup/password.tsx:218 @@ -192,7 +192,7 @@ msgid "Choose your login method" msgstr "Choose your login method" #: app/routes/recover/complete.tsx:225 -#: app/routes/recover/index.tsx:339 +#: app/routes/recover/index.tsx:331 msgid "Code" msgstr "Code" @@ -225,7 +225,7 @@ msgstr "Connected accounts" #: app/routes/device/index.tsx:73 #: app/routes/login/index.tsx:568 #: app/routes/recover/complete.tsx:248 -#: app/routes/recover/index.tsx:354 +#: app/routes/recover/index.tsx:346 #: app/routes/signup/index.tsx:459 #: app/routes/signup/index.tsx:605 msgid "Continue" @@ -294,7 +294,7 @@ msgstr "Didn't get the email?" #: app/routes/login/index.tsx:392 #: app/routes/login/index.tsx:407 -#: app/routes/recover/index.tsx:390 +#: app/routes/recover/index.tsx:382 #: app/routes/signup/index.tsx:541 #: app/routes/signup/index.tsx:577 msgid "Email" @@ -346,7 +346,7 @@ msgstr "Enable SMS one-time code" msgid "Enter a new code" msgstr "Enter a new code" -#: app/routes/recover/index.tsx:385 +#: app/routes/recover/index.tsx:377 msgid "Enter a valid email address." msgstr "Enter a valid email address." @@ -374,7 +374,7 @@ msgstr "Enter the verification code sent to your email address." msgid "Enter your authenticator code" msgstr "Enter your authenticator code" -#: app/routes/recover/index.tsx:372 +#: app/routes/recover/index.tsx:364 msgid "Enter your email address and we'll send you a link to set up a new passkey." msgstr "Enter your email address and we'll send you a link to set up a new passkey." @@ -612,7 +612,7 @@ msgid "Please finish setting up your passkey before leaving this page. It will b msgstr "Please finish setting up your passkey before leaving this page. It will be the only way to sign in, and your account won't be usable without it." #: app/routes/error.tsx:52 -#: app/routes/recover/index.tsx:370 +#: app/routes/recover/index.tsx:362 #: app/routes/signup/index.tsx:598 #: app/routes/verify/index.tsx:234 msgid "Recover your account" @@ -689,7 +689,7 @@ msgstr "Security key" msgid "Select an account to continue or add a new one." msgstr "Select an account to continue or add a new one." -#: app/routes/recover/index.tsx:404 +#: app/routes/recover/index.tsx:396 msgid "Send recovery link" msgstr "Send recovery link" @@ -850,7 +850,7 @@ msgstr "Thanks," msgid "That action isn't available right now." msgstr "That action isn't available right now." -#: app/routes/recover/index.tsx:334 +#: app/routes/recover/index.tsx:326 msgid "That code is invalid or has expired. Check the email, or start over." msgstr "That code is invalid or has expired. Check the email, or start over." @@ -1040,7 +1040,7 @@ msgid "We sent a verification code to your email address." msgstr "We sent a verification code to your email address." #. placeholder {0}: sent.email -#: app/routes/recover/index.tsx:317 +#: app/routes/recover/index.tsx:309 msgid "We've sent a link to <0>{0}. Open it on the device you want to sign in with, or enter the code from that email here." msgstr "We've sent a link to <0>{0}. Open it on the device you want to sign in with, or enter the code from that email here." @@ -1068,7 +1068,7 @@ msgstr "Welcome" msgid "While Datum is currently free of charge to use, we require a valid payment method during the signup process." msgstr "While Datum is currently free of charge to use, we require a valid payment method during the signup process." -#: app/routes/recover/index.tsx:359 +#: app/routes/recover/index.tsx:351 #: app/routes/signup/index.tsx:471 msgid "Wrong address? Start over" msgstr "Wrong address? Start over" @@ -1121,7 +1121,7 @@ msgstr "You'll be asked to sign in before authorizing." msgid "You've been signed out" msgstr "You've been signed out" -#: app/routes/recover/index.tsx:399 +#: app/routes/recover/index.tsx:391 msgid "you@example.com" msgstr "you@example.com" diff --git a/app/routes/recover/index.tsx b/app/routes/recover/index.tsx index 2529b65e0..65038e7b3 100644 --- a/app/routes/recover/index.tsx +++ b/app/routes/recover/index.tsx @@ -143,15 +143,7 @@ export async function action({ request }: ActionFunctionArgs) { // Bot gate before any provider work, so a rejection costs what an acceptance costs. // A distinct action name per intent: a request token cannot be replayed against code entry. - // The sealed recovery ticket already gates this step, and the request step that issued - // it passed a fresh reCAPTCHA. A user who left to fetch the code from their mail comes - // back with an aged token and a clean score; rejecting that told them their code was - // invalid, which is both wrong and the normal path. - if ( - await recaptchaRejects(String(form.get('recaptchaToken') ?? ''), 'recovery_code', { - tolerateStale: true, - }) - ) { + if (await recaptchaRejects(String(form.get('recaptchaToken') ?? ''), 'recovery_code')) { return invalidCode(); } if (!parsed.success) return invalidCode(); diff --git a/app/server/infra/recaptcha.server.ts b/app/server/infra/recaptcha.server.ts index fc54ce130..b6af256d3 100644 --- a/app/server/infra/recaptcha.server.ts +++ b/app/server/infra/recaptcha.server.ts @@ -3,7 +3,6 @@ import { logAuthEvent } from '@/server/observability'; const SITEVERIFY_URL = 'https://www.google.com/recaptcha/api/siteverify'; const REQUEST_TIMEOUT_MS = 2000; -const MAX_TOKEN_AGE_MS = 2 * 60_000; export type RecaptchaReason = | 'ok' @@ -11,7 +10,6 @@ export type RecaptchaReason = | 'no-token' | 'rejected' | 'action-mismatch' - | 'stale' | 'hostname-mismatch' | 'transport'; @@ -76,7 +74,6 @@ export async function verifyRecaptcha( score?: number; action?: string; hostname?: string; - challenge_ts?: string; 'error-codes'?: unknown; }; @@ -101,17 +98,14 @@ export async function verifyRecaptcha( const score = typeof body.score === 'number' ? body.score : null; + // No local age check: Google enforces expiry and single use itself, answering + // success=false with timeout-or-duplicate. challenge_ts is when the page's challenge was + // created, not when execute() minted the token, so aging from it rejects real users. if (!body.success) return { outcome: 'invalid', score, reason: 'rejected', errorCodes }; if (body.action !== expectedAction) { return { outcome: 'invalid', score, reason: 'action-mismatch' }; } - // Math.abs so a backward-skewed clock cannot silently disable this check. - const issued = body.challenge_ts ? Date.parse(body.challenge_ts) : NaN; - if (Number.isNaN(issued) || Math.abs(Date.now() - issued) > MAX_TOKEN_AGE_MS) { - return { outcome: 'invalid', score, reason: 'stale' }; - } - // The only control against someone farming tokens with our public site key on their own // domain. env.server refuses to boot when the secret is set without PUBLIC_ORIGIN, so // this guard is a fallback rather than a live skip path. @@ -133,19 +127,7 @@ export async function verifyRecaptcha( * the gated set stays enumerable. Callers must still run it before any account lookup, or * the fast reject path becomes an enumeration timing oracle (G7). */ -/** - * `tolerateStale` is for a step the user reaches minutes after the page rendered — the - * recovery code screen, where they leave to fetch the code from their mail. grecaptcha - * hands back a challenge aged from page load rather than from the execute() call, so an - * ordinary user returning after two minutes fails the age check with a perfect score. - * Staleness alone is then not evidence of a bot, and every other verdict still rejects. - * Only pass it where a separate credential already gates the step. - */ -export async function recaptchaRejects( - token: string, - expectedAction: string, - { tolerateStale = false }: { tolerateStale?: boolean } = {} -): Promise { +export async function recaptchaRejects(token: string, expectedAction: string): Promise { const verdict = await verifyRecaptcha(token, expectedAction); // Unconfigured deployments stay dark, audit trail included — otherwise the metric reads @@ -165,6 +147,5 @@ export async function recaptchaRejects( } // 'unavailable' is Google failing us, not the caller — fail open. - if (tolerateStale && verdict.reason === 'stale') return false; return verdict.outcome === 'invalid'; } diff --git a/cypress/component/server/recaptcha.cy.ts b/cypress/component/server/recaptcha.cy.ts index 71e7fd3a2..cae42e59d 100644 --- a/cypress/component/server/recaptcha.cy.ts +++ b/cypress/component/server/recaptcha.cy.ts @@ -101,16 +101,32 @@ describe('verifyRecaptcha', () => { }); }); - it('rejects a stale token', () => { - const old = new Date(Date.now() - 5 * 60_000).toISOString(); + // challenge_ts is when the page's challenge was created, not when execute() minted the + // token. A user who submits hours after page load still holds a token Google accepts. + it('accepts a Google-verified token whose challenge_ts is hours old', () => { + const old = new Date(Date.now() - 5 * 60 * 60_000).toISOString(); callService({ fn: 'verifyRecaptcha', env: RECAPTCHA_ENV, recaptchaInput: { token: 'tok', expectedAction: 'signup' }, recaptchaFetch: { body: ok({ challenge_ts: old }) }, + }).then((v) => { + expect(v.outcome.outcome).to.equal('valid'); + expect(v.outcome.reason).to.equal('ok'); + }); + }); + + // Genuine expiry and replay are Google's call, reported as timeout-or-duplicate. + it('rejects an expired or replayed token Google reports as timeout-or-duplicate', () => { + callService({ + fn: 'verifyRecaptcha', + env: RECAPTCHA_ENV, + recaptchaInput: { token: 'tok', expectedAction: 'signup' }, + recaptchaFetch: { body: { success: false, 'error-codes': ['timeout-or-duplicate'] } }, }).then((v) => { expect(v.outcome.outcome).to.equal('invalid'); - expect(v.outcome.reason).to.equal('stale'); + expect(v.outcome.reason).to.equal('rejected'); + expect(v.outcome.errorCodes).to.include('timeout-or-duplicate'); }); });