From 214e8b9139bf1df6f5a1859539aad19991301f26 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 5 Sep 2026 16:23:57 +0000 Subject: [PATCH] refactor(captcha): serve the cached-page refresh from the provider (#1053) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `DynamicFragments::handle()` called `SecurityService::generate_simple_captcha()` directly, in two places. It is the fifth refresh site: PR2 routed the four retry sites through `with_fresh_challenge()` and missed this one, leaving `challenge_payload()`'s own docblock claiming a consumer it did not have. The endpoint now forwards `CaptchaProvider::resolve()->challenge_payload()` verbatim and names none of the fields. Mapping them would only ever hold for the math challenge — a proof-of-work challenge has no question and no answer hash — so the client dispatches on the payload's `provider` key instead, and leaves a provider it does not recognise alone rather than half-applying it, which would blank a challenge the visitor may already have solved. Scoping moved with it. The client used to query the whole document for labels, tokens and answers separately; it now walks one security block at a time (`.ffc-security-container`, plus any bare `.ffc-captcha-row` from markup cached before that wrapper existed — cached pages being this endpoint's whole audience). That is what keeps two challenges on one page independent, and it matches where the fields actually live: `templates/captcha/math-fields.php` puts the label and both inputs inside the row. Also removes `Shortcodes::get_new_captcha_data()`, a public method whose only consumer was its own test. Test notes. The JS fixtures placed the inputs outside the row, which no render site does; they now mirror the template. Their seed answers were non-numeric, and the real input is `type="number"` — which reads back as '' — so every "blanks the answer" assertion was passing without proving anything. All six captcha tests were verified failing against the previous script. On the PHP side the endpoint is now checked against a mocked `CaptchaProvider` rather than a mocked `generate_simple_captcha()`: the latter passes just as well with the endpoint bypassing the contract, which is the bug being fixed. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_012XWx9qJdjZdAq8crxM9GCU --- CHANGELOG.md | 5 + assets/js/ffc-dynamic-fragments.js | 117 ++++++++++------- assets/js/ffc-dynamic-fragments.min.js | 2 +- .../frontend/class-ffc-dynamic-fragments.php | 25 ++-- includes/frontend/class-ffc-shortcodes.php | 9 -- tests/Unit/DynamicFragmentsTest.php | 119 +++++++++++++++--- tests/Unit/FrontendShortcodesTest.php | 21 ---- tests/js/dynamic-fragments.test.js | 117 +++++++++++++---- 8 files changed, 285 insertions(+), 130 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9f169326f..301b9fe53 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,8 +13,13 @@ The format follows [Keep a Changelog] (https://keepachangelog.com/en/1.1.0/). ### Changed +- Internal (#1053) — o endpoint de fragmentos de página em cache passa a servir o desafio do provider configurado, via `CaptchaProvider::resolve()->challenge_payload()`, em vez de chamar o captcha matemático direto; o cliente despacha pelo campo `provider` do payload e ignora o que não reconhece, em vez de aplicar meio payload. É o quinto site de refresh, que a unificação do contrato não havia alcançado. - Internal (#1053) — o captcha passa a ter um contrato de estratégia (`CaptchaProviderInterface` + `CaptchaProvider::resolve()`), com o desafio matemático atrás dele. Os 6 sites de verificação e os 4 de retry não mudam: `validate_security_fields()` continua sendo o ponto único e agora delega a metade captcha. As duas cópias do bloco de segurança viraram uma, em `templates/`. +### Removed + +- `Shortcodes::get_new_captcha_data()` (#1053): método público sem nenhum chamador em produção — o único consumidor era o próprio teste. A geração de desafio já é responsabilidade do contrato de captcha. + ### Fixed - **Um agendamento era criado mas reportado como falha** (#1058): `$appointment['id']` chega do `$wpdb` como string e `AppointmentReceiptHandler::get_receipt_url()` declara `int` sob `strict_types` — o `TypeError` fatalava no e-mail de confirmação, *depois* do commit da reserva, e o visitante recebia HTTP 500 para um agendamento que existia. O `catch` do handler passou a `\Throwable` e o bloco pós-commit foi isolado por etapa: uma reserva commitada nunca mais é reportada como falha. diff --git a/assets/js/ffc-dynamic-fragments.js b/assets/js/ffc-dynamic-fragments.js index 2713bb94c..4f844dd4f 100644 --- a/assets/js/ffc-dynamic-fragments.js +++ b/assets/js/ffc-dynamic-fragments.js @@ -81,55 +81,86 @@ } /** - * Patch the DOM with fresh captcha, nonce, and user values. + * Every security block on the page, in DOM order. + * + * `.ffc-security-container` is the provider-independent wrapper each + * render site emits. A bare `.ffc-captcha-row` outside one is markup from + * before that wrapper existed — and cached HTML is exactly this + * endpoint's audience, so it is collected too rather than skipped. + * + * @returns {Element[]} Roots to patch, one per rendered challenge. */ - function applyFragments(data) { + function securityBlocks() { + var blocks = []; var i; - // --- Per-form captchas (multiple forms on one page) --- - var handledWrappers = {}; - if (data.captchas) { - var formId; - for (formId in data.captchas) { - if (data.captchas.hasOwnProperty(formId)) { - var wrapper = document.getElementById('ffc-form-' + formId); - if (wrapper) { - var fc = data.captchas[formId]; - var wLabel = wrapper.querySelector('.ffc-captcha-row .ffc-captcha-label-text'); - var wHash = wrapper.querySelector('input[name="ffc_captcha_hash"]'); - var wAns = wrapper.querySelector('input[name="ffc_captcha_ans"]'); - if (wLabel) { wLabel.textContent = fc.label; } - if (wHash) { wHash.value = fc.hash; } - if (wAns) { wAns.value = ''; } - handledWrappers['ffc-form-' + formId] = true; - } - } - } + var containers = document.querySelectorAll('.ffc-security-container'); + for (i = 0; i < containers.length; i++) { + blocks.push(containers[i]); } - // --- Default captcha (single form or self-scheduling) --- - if (data.captcha) { - var labelTexts = document.querySelectorAll('.ffc-captcha-row .ffc-captcha-label-text'); - var hashes = document.querySelectorAll('input[name="ffc_captcha_hash"]'); - var answers = document.querySelectorAll('input[name="ffc_captcha_ans"]'); - - for (i = 0; i < labelTexts.length; i++) { - var el = labelTexts[i]; - var parent = el.closest('.ffc-form-wrapper'); - if (parent && handledWrappers[parent.id]) { continue; } - el.textContent = data.captcha.label; - } - for (i = 0; i < hashes.length; i++) { - var hEl = hashes[i]; - var hParent = hEl.closest('.ffc-form-wrapper'); - if (hParent && handledWrappers[hParent.id]) { continue; } - hEl.value = data.captcha.hash; + var rows = document.querySelectorAll('.ffc-captcha-row'); + for (i = 0; i < rows.length; i++) { + if (!rows[i].closest('.ffc-security-container')) { + blocks.push(rows[i]); } - for (i = 0; i < answers.length; i++) { - var aEl = answers[i]; - var aParent = aEl.closest('.ffc-form-wrapper'); - if (aParent && handledWrappers[aParent.id]) { continue; } - aEl.value = ''; + } + + return blocks; + } + + /** + * Apply a challenge payload inside one security block. + * + * The payload is whatever the configured captcha strategy issued, and it + * names itself in `provider`. Dispatching on that — rather than assuming + * the math shape — is what keeps this honest once a strategy with + * different fields exists: an unrecognised provider is left alone instead + * of being half-applied, which would blank a challenge the visitor may + * already have solved. + * + * @param {Element} root Security block to patch within. + * @param {Object} payload Challenge payload from the server. + * @returns {void} + */ + function applyChallenge(root, payload) { + if (!payload || payload.provider !== 'math') { + return; + } + + var label = root.querySelector('.ffc-captcha-label-text'); + var hash = root.querySelector('input[name="ffc_captcha_hash"]'); + var ans = root.querySelector('input[name="ffc_captcha_ans"]'); + + if (label) { label.textContent = payload.new_label; } + if (hash) { hash.value = payload.new_hash; } + // Clearing the answer matters as much as the token: a stale answer + // beside a fresh challenge submits a pair that cannot verify. + if (ans) { ans.value = ''; } + } + + /** + * Patch the DOM with fresh captcha, nonce, and user values. + */ + function applyFragments(data) { + var i; + + // --- Captchas --- + // One pass over the security blocks. A block inside a form wrapper + // takes that form's own payload when the server sent one (several + // forms on a page must never share a challenge — #1056); everything + // else takes the default. Scoping by block rather than by document + // is what keeps two challenges on one page independent. + if (data.captcha || data.captchas) { + var blocks = securityBlocks(); + for (i = 0; i < blocks.length; i++) { + var wrapper = blocks[i].closest('.ffc-form-wrapper'); + var formId = wrapper ? wrapper.id.replace('ffc-form-', '') : ''; + var payload = (data.captchas && formId && data.captchas[formId]) + ? data.captchas[formId] + : data.captcha; + + applyChallenge(blocks[i], payload); } } diff --git a/assets/js/ffc-dynamic-fragments.min.js b/assets/js/ffc-dynamic-fragments.min.js index 84f11495d..1a32d8aad 100644 --- a/assets/js/ffc-dynamic-fragments.min.js +++ b/assets/js/ffc-dynamic-fragments.min.js @@ -1,2 +1,2 @@ -!function(){"use strict";function e(){if(document.querySelector(".ffc-security-container")||document.querySelector(".ffc-captcha-row")||document.querySelector(".ffc-verification-form")||document.querySelector(".ffc-form-container")||document.querySelector(".ffc-booking-form")){var e="undefined"!=typeof ffcDynamic&&ffcDynamic.ajaxUrl||"undefined"!=typeof ffc_ajax&&ffc_ajax.ajax_url||"undefined"!=typeof ffcCalendar&&ffcCalendar.ajaxurl||null;if(e){for(var c=[],n=document.querySelectorAll('.ffc-form-wrapper[id^="ffc-form-"]'),f=0;f array( - 'label' => $captcha['label'], - 'hash' => $captcha['hash'], - ), + // Forwarded verbatim from whichever strategy is configured. The + // endpoint deliberately does not name the fields: a proof-of-work + // challenge has no label and no answer hash, so any mapping here + // would only hold for the math one. The client dispatches on the + // payload's own `provider` key. + 'captcha' => Captcha\CaptchaProvider::resolve()->challenge_payload(), 'nonces' => array( 'ffc_frontend_nonce' => wp_create_nonce( 'ffc_frontend_nonce' ), 'ffc_self_scheduling_nonce' => wp_create_nonce( 'ffc_self_scheduling_nonce' ), @@ -82,17 +84,14 @@ public function handle(): void { // phpcs:ignore WordPress.Security.NonceVerification.Missing -- Intentionally nonce-free; see class docblock. $form_ids = isset( $_POST['form_ids'] ) ? array_map( 'absint', (array) $_POST['form_ids'] ) : array(); - // Generate a unique captcha per form so multiple forms on the same - // page each show a different math question after cache refresh. + // Issue a distinct challenge per form, so two forms on one page never + // share a token — the resolver memoises the strategy instance, not the + // challenge, so each call mints a fresh one. if ( count( $form_ids ) > 1 ) { $per_form = array(); foreach ( $form_ids as $fid ) { if ( $fid > 0 ) { - $c = \FreeFormCertificate\Core\SecurityService::generate_simple_captcha(); - $per_form[ $fid ] = array( - 'label' => $c['label'], - 'hash' => $c['hash'], - ); + $per_form[ $fid ] = Captcha\CaptchaProvider::resolve()->challenge_payload(); } } if ( ! empty( $per_form ) ) { diff --git a/includes/frontend/class-ffc-shortcodes.php b/includes/frontend/class-ffc-shortcodes.php index 5c67b683e..5431affe8 100644 --- a/includes/frontend/class-ffc-shortcodes.php +++ b/includes/frontend/class-ffc-shortcodes.php @@ -37,15 +37,6 @@ class Shortcodes { public function __construct() { } - /** - * Generate new captcha data (math question + hash) - * - * @return array - */ - public function get_new_captcha_data(): array { - return SecurityService::generate_simple_captcha(); - } - /** * Generate HTML for security fields (honeypot + captcha) * diff --git a/tests/Unit/DynamicFragmentsTest.php b/tests/Unit/DynamicFragmentsTest.php index 306da7ace..cc599f914 100644 --- a/tests/Unit/DynamicFragmentsTest.php +++ b/tests/Unit/DynamicFragmentsTest.php @@ -64,6 +64,33 @@ private function callHandle( DynamicFragments $fragments ): void { } } + /** + * Alias-mock the captcha resolver so the endpoint's own wiring is what is + * under test. + * + * The endpoint must not know what a challenge looks like — since #1053 PR2 + * it forwards whatever the configured strategy issues, so the assertions + * below use a payload no real provider emits. Mocking + * `SecurityService::generate_simple_captcha()` instead would pass just as + * well with the endpoint calling the math challenge directly, which is + * exactly the bypass this replaces. + * + * @param array ...$payloads One per expected call, in order. + */ + private function mockProvider( array ...$payloads ): void { + $provider = Mockery::mock( '\FreeFormCertificate\Core\Captcha\CaptchaProviderInterface' ); + $expectation = $provider->shouldReceive( 'challenge_payload' ); + + if ( 1 === count( $payloads ) ) { + $expectation->andReturn( $payloads[0] ); + } else { + $expectation->andReturnValues( $payloads ); + } + + $resolver = Mockery::mock( 'alias:\FreeFormCertificate\Core\Captcha\CaptchaProvider' ); + $resolver->shouldReceive( 'resolve' )->andReturn( $provider ); + } + // ================================================================== // Constructor // ================================================================== @@ -79,10 +106,7 @@ public function test_constructor_registers_ajax_hooks(): void { // ================================================================== public function test_handle_returns_captcha_and_nonces_for_anonymous(): void { - $utilsMock = Mockery::mock( 'alias:\FreeFormCertificate\Core\SecurityService' ); - $utilsMock->shouldReceive( 'generate_simple_captcha' ) - ->once() - ->andReturn( array( 'label' => '3 + 4', 'hash' => 'abc123' ) ); + $this->mockProvider( array( 'provider' => 'math', 'new_label' => '3 + 4', 'new_hash' => 'abc123' ) ); Functions\when( 'wp_create_nonce' )->alias( function ( $action ) { return 'nonce_' . $action; @@ -97,8 +121,11 @@ public function test_handle_returns_captcha_and_nonces_for_anonymous(): void { $this->assertSame( 'success', $this->json_responses[0]['type'] ); // Captcha - $this->assertSame( '3 + 4', $data['captcha']['label'] ); - $this->assertSame( 'abc123', $data['captcha']['hash'] ); + $this->assertSame( + array( 'provider' => 'math', 'new_label' => '3 + 4', 'new_hash' => 'abc123' ), + $data['captcha'], + 'The provider payload must be forwarded verbatim — the endpoint does not reshape it.' + ); // Nonces $this->assertSame( 'nonce_ffc_frontend_nonce', $data['nonces']['ffc_frontend_nonce'] ); @@ -108,15 +135,75 @@ public function test_handle_returns_captcha_and_nonces_for_anonymous(): void { $this->assertArrayNotHasKey( 'user', $data ); } + // ================================================================== + // handle() — per-form challenges + // ================================================================== + + public function test_handle_issues_one_challenge_per_form_when_several_are_posted(): void { + // Two forms on one page must never share a token (#1056), so the + // endpoint asks the provider once per form and keys the answers by + // form id. Distinct payloads here are what prove they are separate + // calls rather than one reused value. + $this->mockProvider( + array( 'provider' => 'math', 'new_label' => 'a', 'new_hash' => 'a-hash' ), + array( 'provider' => 'math', 'new_label' => 'b', 'new_hash' => 'b-hash' ), + array( 'provider' => 'math', 'new_label' => 'c', 'new_hash' => 'c-hash' ) + ); + + Functions\when( 'wp_create_nonce' )->justReturn( 'n' ); + Functions\when( 'is_user_logged_in' )->justReturn( false ); + Functions\when( 'absint' )->alias( fn( $v ) => abs( (int) $v ) ); + Functions\when( 'wp_unslash' )->returnArg(); + // The same `form_ids` also drive the geofence branch further down. + Functions\when( 'get_post_meta' )->justReturn( '' ); + + $_POST['form_ids'] = array( '7', '8' ); + + $fragments = new DynamicFragments(); + $this->callHandle( $fragments ); + + unset( $_POST['form_ids'] ); + + $data = $this->json_responses[0]['data']; + + $this->assertArrayHasKey( 'captchas', $data ); + $this->assertSame( array( 7, 8 ), array_keys( $data['captchas'] ) ); + $this->assertNotSame( + $data['captchas'][7]['new_hash'], + $data['captchas'][8]['new_hash'], + 'Each form must get its own token, or the second form submits one the first already spent.' + ); + } + + public function test_handle_omits_per_form_challenges_for_a_single_form(): void { + // One form needs no per-form branch — the default challenge already + // lands on it, and a `captchas` map of one would be a second token + // for the same block. + $this->mockProvider( array( 'provider' => 'math', 'new_label' => 'a', 'new_hash' => 'a-hash' ) ); + + Functions\when( 'wp_create_nonce' )->justReturn( 'n' ); + Functions\when( 'is_user_logged_in' )->justReturn( false ); + Functions\when( 'absint' )->alias( fn( $v ) => abs( (int) $v ) ); + Functions\when( 'wp_unslash' )->returnArg(); + // The same `form_ids` also drive the geofence branch further down. + Functions\when( 'get_post_meta' )->justReturn( '' ); + + $_POST['form_ids'] = array( '7' ); + + $fragments = new DynamicFragments(); + $this->callHandle( $fragments ); + + unset( $_POST['form_ids'] ); + + $this->assertArrayNotHasKey( 'captchas', $this->json_responses[0]['data'] ); + } + // ================================================================== // handle() — logged-in user // ================================================================== public function test_handle_includes_user_data_when_logged_in(): void { - $utilsMock = Mockery::mock( 'alias:\FreeFormCertificate\Core\SecurityService' ); - $utilsMock->shouldReceive( 'generate_simple_captcha' ) - ->once() - ->andReturn( array( 'label' => '5 + 2', 'hash' => 'def456' ) ); + $this->mockProvider( array( 'provider' => 'math', 'new_label' => '5 + 2', 'new_hash' => 'def456' ) ); Functions\when( 'wp_create_nonce' )->justReturn( 'fresh_nonce' ); Functions\when( 'is_user_logged_in' )->justReturn( true ); @@ -143,9 +230,7 @@ public function test_handle_includes_user_data_when_logged_in(): void { // ================================================================== public function test_handle_always_returns_both_nonce_keys(): void { - $utilsMock = Mockery::mock( 'alias:\FreeFormCertificate\Core\SecurityService' ); - $utilsMock->shouldReceive( 'generate_simple_captcha' ) - ->andReturn( array( 'label' => '1 + 1', 'hash' => 'h' ) ); + $this->mockProvider( array( 'provider' => 'math', 'new_label' => '1 + 1', 'new_hash' => 'h' ) ); Functions\when( 'wp_create_nonce' )->justReturn( 'n' ); Functions\when( 'is_user_logged_in' )->justReturn( false ); @@ -163,9 +248,7 @@ public function test_handle_always_returns_both_nonce_keys(): void { // ================================================================== public function test_handle_includes_public_csv_download_nonce(): void { - $utilsMock = Mockery::mock( 'alias:\FreeFormCertificate\Core\SecurityService' ); - $utilsMock->shouldReceive( 'generate_simple_captcha' ) - ->andReturn( array( 'label' => 'x', 'hash' => 'y' ) ); + $this->mockProvider( array( 'provider' => 'math', 'new_label' => 'x', 'new_hash' => 'y' ) ); Functions\when( 'wp_create_nonce' )->alias( function ( $action ) { return 'nonce_' . $action; @@ -185,9 +268,7 @@ public function test_handle_includes_public_csv_download_nonce(): void { // ================================================================== public function test_handle_includes_audience_nonces(): void { - $utilsMock = Mockery::mock( 'alias:\FreeFormCertificate\Core\SecurityService' ); - $utilsMock->shouldReceive( 'generate_simple_captcha' ) - ->andReturn( array( 'label' => 'x', 'hash' => 'y' ) ); + $this->mockProvider( array( 'provider' => 'math', 'new_label' => 'x', 'new_hash' => 'y' ) ); Functions\when( 'wp_create_nonce' )->alias( function ( $action ) { return 'nonce_' . $action; diff --git a/tests/Unit/FrontendShortcodesTest.php b/tests/Unit/FrontendShortcodesTest.php index 18eb6a7a3..598602b6d 100644 --- a/tests/Unit/FrontendShortcodesTest.php +++ b/tests/Unit/FrontendShortcodesTest.php @@ -146,27 +146,6 @@ function ( $key, $default = false ) use ( $global_on ) { return (string) $this->shortcodes->render_form( array( 'id' => $form_id ) ); } - // ================================================================== - // get_new_captcha_data() - // ================================================================== - - public function test_get_new_captcha_data_returns_label_and_hash(): void { - $captcha = $this->shortcodes->get_new_captcha_data(); - - $this->assertArrayHasKey( 'label', $captcha ); - $this->assertArrayHasKey( 'hash', $captcha ); - $this->assertArrayHasKey( 'answer', $captcha ); - $this->assertIsInt( $captcha['answer'] ); - $this->assertGreaterThanOrEqual( 0, $captcha['answer'] ); - - // Since 6.23.0 the token is `..` rather than - // a digest of the answer, so it expires and can only be redeemed once. - $this->assertMatchesRegularExpression( - '/^\d+\.[0-9a-f]{16}\.[0-9a-f]{64}$/', - $captcha['hash'] - ); - } - // ================================================================== // generate_security_fields() // ================================================================== diff --git a/tests/js/dynamic-fragments.test.js b/tests/js/dynamic-fragments.test.js index d8ef75205..cd37b2cbb 100644 --- a/tests/js/dynamic-fragments.test.js +++ b/tests/js/dynamic-fragments.test.js @@ -114,18 +114,35 @@ describe('ffc-dynamic-fragments — applyFragments via XHR onload', () => { MockXHR.lastInstance.deliver(response); } + // The markup below mirrors `templates/captcha/math-fields.php`: the label + // and BOTH inputs live inside `.ffc-captcha-row`, itself inside the + // provider-independent `.ffc-security-container`. Earlier fixtures put the + // inputs outside the row, which no real render site does — and the script + // now scopes per security block, so an unfaithful fixture would test a + // DOM the plugin never emits. + // `answer` must be numeric: the real input is `type="number"`, and a + // non-numeric value reads back as '' — which would make every + // "blanks the answer" assertion below pass without proving anything. + function securityBlock(label, hash, answer) { + return `
+
+ ${label} + + +
+
`; + } + it('updates the per-form captcha (label, hash, blanks the answer)', () => { setupAndDeliver( `
-
old-label
- - + ${securityBlock('old-label', 'old-hash', '123')}
`, { success: true, data: { captchas: { - '7': { label: 'new-label', hash: 'new-hash' }, + '7': { provider: 'math', new_label: 'new-label', new_hash: 'new-hash' }, }, }, } @@ -137,15 +154,11 @@ describe('ffc-dynamic-fragments — applyFragments via XHR onload', () => { it('updates the default captcha when no per-form captchas matched', () => { setupAndDeliver( - `
-
stale
- - -
`, + `
${securityBlock('stale', 'stale-hash', '9')}
`, { success: true, data: { - captcha: { label: 'fresh', hash: 'fresh-hash' }, + captcha: { provider: 'math', new_label: 'fresh', new_hash: 'fresh-hash' }, }, } ); @@ -154,6 +167,70 @@ describe('ffc-dynamic-fragments — applyFragments via XHR onload', () => { expect(document.querySelector('input[name="ffc_captcha_ans"]').value).toBe(''); }); + it('leaves a legacy security block — no .ffc-security-container — patched', () => { + // Cached HTML rendered before the wrapper existed. Cached pages are + // this endpoint's whole reason to exist, so the bare row is collected + // rather than skipped. + setupAndDeliver( + `
+
+ stale + + +
+
`, + { + success: true, + data: { captcha: { provider: 'math', new_label: 'fresh', new_hash: 'fresh-hash' } }, + } + ); + expect(document.querySelector('.ffc-captcha-label-text').textContent).toBe('fresh'); + expect(document.querySelector('input[name="ffc_captcha_hash"]').value).toBe('fresh-hash'); + expect(document.querySelector('input[name="ffc_captcha_ans"]').value).toBe(''); + }); + + it('ignores a payload from an unrecognised provider', () => { + // A provider the cached script does not know about must be left + // alone: half-applying it would blank a challenge the visitor may + // already have solved, which is worse than not refreshing at all. + setupAndDeliver( + `
${securityBlock('keep-me', 'keep-hash', '4')}
`, + { + success: true, + data: { captcha: { provider: 'altcha', challengeurl: '/challenge' } }, + } + ); + expect(document.querySelector('.ffc-captcha-label-text').textContent).toBe('keep-me'); + expect(document.querySelector('input[name="ffc_captcha_hash"]').value).toBe('keep-hash'); + expect(document.querySelector('input[name="ffc_captcha_ans"]').value).toBe('4'); + }); + + it('gives each form on the page its own challenge', () => { + // The #1056 property, enforced at the fragment-refresh level: two + // forms must never end up sharing a token. Scoping per security block + // is what makes this hold — a document-wide query would write the + // same values into both. + setupAndDeliver( + `
${securityBlock('a-old', 'a-old-hash', '1')}
+
${securityBlock('b-old', 'b-old-hash', '2')}
`, + { + success: true, + data: { + captchas: { + '7': { provider: 'math', new_label: 'a-new', new_hash: 'a-new-hash' }, + '8': { provider: 'math', new_label: 'b-new', new_hash: 'b-new-hash' }, + }, + }, + } + ); + const labels = document.querySelectorAll('.ffc-captcha-label-text'); + const hashes = document.querySelectorAll('input[name="ffc_captcha_hash"]'); + expect(labels[0].textContent).toBe('a-new'); + expect(labels[1].textContent).toBe('b-new'); + expect(hashes[0].value).toBe('a-new-hash'); + expect(hashes[1].value).toBe('b-new-hash'); + }); + it('refreshes ffcGeofenceConfig and triggers FFCGeofence.recheck()', () => { // Pre-existing geofence config + recheck spy. window.ffcGeofenceConfig = { 7: { datetime: { enabled: false } } }; @@ -175,7 +252,7 @@ describe('ffc-dynamic-fragments — applyFragments via XHR onload', () => { `
stale
`, - { success: false, data: { captchas: { 9: { label: 'x', hash: 'y' } } } } + { success: false, data: { captchas: { 9: { provider: 'math', new_label: 'x', new_hash: 'y' } } } } ); expect(document.querySelector('.ffc-captcha-label-text').textContent).toBe('stale'); }); @@ -184,7 +261,7 @@ describe('ffc-dynamic-fragments — applyFragments via XHR onload', () => { document.body.innerHTML = '
stale
'; window.ffcDynamic = { ajaxUrl: '/x' }; loadScript('assets/js/ffc-dynamic-fragments.js'); - MockXHR.lastInstance.deliver({ success: true, data: { captcha: { label: 'fresh', hash: 'h' } } }, 500); + MockXHR.lastInstance.deliver({ success: true, data: { captcha: { provider: 'math', new_label: 'fresh', new_hash: 'h' } } }, 500); expect(document.querySelector('.ffc-captcha-label-text').textContent).toBe('stale'); }); @@ -250,25 +327,17 @@ describe('ffc-dynamic-fragments — applyFragments via XHR onload', () => { expect(email.getAttribute('readonly')).toBe('readonly'); }); - it('default-captcha loop skips wrappers already handled by per-form captchas', () => { - // Both per-form (captchas[7]) and default (captcha) payloads, with the - // captcha fields living inside the handled wrapper. The default loop - // must `continue` over them, leaving the per-form values intact. + it('prefers a form\'s own payload over the default one', () => { setupAndDeliver( - `
-
old
- - -
`, + `
${securityBlock('old', 'old-hash', '5')}
`, { success: true, data: { - captchas: { '7': { label: 'per-form-label', hash: 'per-form-hash' } }, - captcha: { label: 'default-label', hash: 'default-hash' }, + captchas: { '7': { provider: 'math', new_label: 'per-form-label', new_hash: 'per-form-hash' } }, + captcha: { provider: 'math', new_label: 'default-label', new_hash: 'default-hash' }, }, } ); - // Per-form values win; the default loop's continue prevented overwrite. expect(document.querySelector('.ffc-captcha-label-text').textContent).toBe('per-form-label'); expect(document.querySelector('input[name="ffc_captcha_hash"]').value).toBe('per-form-hash'); expect(document.querySelector('input[name="ffc_captcha_ans"]').value).toBe('');