fix: SCA review round + gitea pipeline green — GDPR audit scrub, backend test gaps, frontend SCA/Square-API, docs parity

7 review agents (pipeline run, self-review, codebase-context, frontend-placement, backend testing-gaps, Square-API, docs-parity) audited the SCA-primary work. ALL findings fixed, including every pre-existing red CI job: GDPR (HIGH): - anonymize_user() now scrubs admin_audit_log.target_user_id (mirrors delete_guest_user) so 2fa_fallback_charge rows (customer id + card_last4 PII) no longer survive registered-user account deletion; gdpr test added BACKEND TEST GAPS (all 10): - delivery-unavailable 503 branch: prod-tag predicate test + dev-variant marker - twoFactorFallbackEnabled alias/case/default matrix tests + exported wrapper - insertTwoFAFallbackAudit details-JSON shape + audit-row assertions for all 6 gate sites (booking/tip/gift-card/payment-method/terminal/till, both actors) - CreateTerminalPayment.VerificationToken: passthrough, too-long 400, 2FA-skip, token-less fallback + SCA-required (new terminal_sca_test.go) - isVerificationRequiredError at all 5 charge sites (402 + code:verification_required) - customer_initiated handler-level assertions (MIT false admin / CIT true customer) - Mock: ApprovePendingVerification, ChallengeResult auto/deny, _deny token suffix, parseVerifyToken unit tests FRONTEND SCA + Square-API (CRITICAL): - tokenizeSavedCardWithVerification reads result.token (the verified token) not result.verificationResult (deprecated verifyBuyer shape — saved-card SCA could never succeed in production before); parseTokenizeVerificationResult pure fn extracted + pinned in square.test.ts; 'verified' with no token proceeds tokenless - HIGH: saved-card idempotency key regenerated after a definitive 402 (fresh token under the same key = IDEMPOTENCY_KEY_REUSED dead-loop); kept on 503/cancelled - challenge-cancelled copy no longer promises a 2FA fallback the UI doesn't show; 'waiting for approval in your banking app' state on CIT surfaces - sca-unavailable demotion resets per attempt; card selection disabled mid-challenge; genuine saved-card declines no longer relabeled 'requires verification'; modal-close guard during processing; retry affordance standardized PIPELINE (every red job now green): - prod-tag build break fixed (shared square stub + test_helpers_test.go, prod-safe) - govulncheck: x/image 0.45.0 bumped (x/text resolved); go mod tidy clean - race: TestDeleteAccount_InvalidatesSquareCustomerCache made deterministic - DAV_ADMIN_PASSWORD placeholder in .env.example (compose config passes) - frontend: prettier 28 files, eslint, a11y 38 errors, knip (currentZIndex), deps in-range, audit vulns (nanoid/postcss) — all fixed; 67 vitest cases DOCS PARITY (6 DRIFTs + 5 GAPs): payments doc Ch4/Ch14/Appendix A, Technical Manual 2FA + counter-reset + payment sections, README test counts + SNAPSHOT_ENC_KEY, Feature Catalog, .env.example REQUIRE_2FA — SCA-primary/2FA-backup posture verified against code everywhere Verified: 26/26 dev + 24/24 prod packages, both vet tags, golangci-lint/staticcheck/ gosec 0 on both tags, gitleaks clean, 2,464 backend + 67 frontend tests.
2026-08-22 00:34:50 +01:00
parent c4c65d9dd8
commit b7122be3a0
78 changed files with 2861 additions and 1120 deletions
@@ -1403,20 +1403,20 @@ func TestDevClient_CreatePayment_RejectsRawPAN(t *testing.T) {
 		{"amex", "378282246310005"},
 	}

-		for _, tt := range tests {
-			t.Run(tt.name, func(t *testing.T) {
-				result, err := client.CreatePayment(ctx, CreatePaymentReq{
-					Amount:         5000,
-					Currency:       "GBP",
-					SourceID:       tt.pan,
-					IdempotencyKey: "raw-pan-" + tt.name,
-					ReferenceID:    "booking-raw",
-				})
-				require.Error(t, err, "raw PAN must be rejected for production parity")
-				assert.Nil(t, result)
-				assert.Contains(t, err.Error(), "invalid source_id")
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result, err := client.CreatePayment(ctx, CreatePaymentReq{
+				Amount:         5000,
+				Currency:       "GBP",
+				SourceID:       tt.pan,
+				IdempotencyKey: "raw-pan-" + tt.name,
+				ReferenceID:    "booking-raw",
 			})
-		}
+			require.Error(t, err, "raw PAN must be rejected for production parity")
+			assert.Nil(t, result)
+			assert.Contains(t, err.Error(), "invalid source_id")
+		})
+	}
 }

 // TestDevClient_CreatePayment_CardOnFileRequiresCustomerID verifies the mock
@@ -2456,3 +2456,196 @@ func TestDevClient_CreatePayment_SavedCardVerification_Grandfathered(t *testing.
 	})
 	require.Equal(t, "CARD_DECLINED_VERIFICATION_REQUIRED", ErrorCode(err), "a non-grandfathered card must stay gated")
 }
+
+// =============================================================================
+// Deterministic verification-token parsing and prefix binding
+// =============================================================================
+
+// TestParseVerifyToken pins the deterministic verify_mock_<prefix>_<amount>
+// [_ok|_deny] encoding: the outcome suffix defaults to approval, "_deny" sets
+// denied=true, and anything malformed is not parseable (an opaque token — the
+// same shape as real Square's verification tokens).
+func TestParseVerifyToken(t *testing.T) {
+	tests := []struct {
+		name       string
+		token      string
+		wantOK     bool
+		wantDenied bool
+		wantAmount int64
+		wantPrefix string
+	}{
+		{"explicit ok suffix", "verify_mock_mock_5000_ok", true, false, 5000, "mock"},
+		{"deny suffix sets denied", "verify_mock_mock_5000_deny", true, true, 5000, "mock"},
+		{"no suffix defaults to approval", "verify_mock_4242_2500", true, false, 2500, "4242"},
+		{"prefix with underscores", "verify_mock_visa_3000_deny", true, true, 3000, "visa"},
+		{"opaque token is not parseable", "vrf_opaque_123", false, false, 0, ""},
+		{"marker with empty rest", "verify_mock_", false, false, 0, ""},
+		{"non-numeric amount", "verify_mock_mock_abc_ok", false, false, 0, ""},
+		{"missing amount", "verify_mock_mock_ok", false, false, 0, ""},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			parsed, ok := parseVerifyToken(tt.token)
+			require.Equal(t, tt.wantOK, ok)
+			if ok {
+				require.Equal(t, tt.wantDenied, parsed.denied)
+				require.Equal(t, tt.wantAmount, parsed.amount)
+				require.Equal(t, tt.wantPrefix, parsed.prefix)
+			}
+		})
+	}
+}
+
+// TestVerificationTokenPrefixForSource pins the card prefix a verify_mock_*
+// token binds to for a source_id: the SAME derivation the dev frontend uses, so
+// the two sides cannot drift.
+func TestVerificationTokenPrefixForSource(t *testing.T) {
+	assert.Equal(t, "4242", verificationTokenPrefixForSource("cnon:test-card"))
+	assert.Equal(t, "4111", verificationTokenPrefixForSource("cnon:visa"))
+	assert.Equal(t, "5555", verificationTokenPrefixForSource("cnon:mastercard"))
+	assert.Equal(t, "3782", verificationTokenPrefixForSource("cnon:amex"))
+	assert.Equal(t, "mock", verificationTokenPrefixForSource("ccof:mock_card"))
+	assert.Equal(t, "abc", verificationTokenPrefixForSource("ccof:abc"))
+	assert.Equal(t, "abcd", verificationTokenPrefixForSource("ccof:abcd"))
+	assert.Equal(t, "", verificationTokenPrefixForSource("unknown-source"))
+}
+
+// TestVerificationTokenInvalidError pins Square's VERIFICATION_TOKEN_INVALID
+// rejection builder: a definitive payment error with the PAYMENT_METHOD_ERROR
+// category and a 400 status.
+func TestVerificationTokenInvalidError(t *testing.T) {
+	err := verificationTokenInvalidError("vrf_bad", "ccof:card_1")
+	require.Equal(t, "VERIFICATION_TOKEN_INVALID", ErrorCode(err))
+	require.Equal(t, "PAYMENT_METHOD_ERROR", ErrorCategory(err))
+	require.Equal(t, http.StatusBadRequest, ErrorStatusCode(err))
+	require.True(t, IsDefinitivePaymentError(err), "VERIFICATION_TOKEN_INVALID must classify as a definitive payment error")
+}
+
+// =============================================================================
+// Challenge resolution: ApprovePendingVerification, ChallengeResult auto/deny,
+// and the stateless _deny token suffix
+// =============================================================================
+
+// TestDevClient_ApprovePendingVerification_OpaqueTokenResolution locks the
+// shared-state challenge resolution for OPAQUE (real-Square-shaped) tokens: a
+// token for a pending challenge is invalid, ApprovePendingVerification marks
+// the challenge approved, and the next tokenized charge then succeeds.
+func TestDevClient_ApprovePendingVerification_OpaqueTokenResolution(t *testing.T) {
+	client := NewDevClient().(*MockClient)
+	client.SimulateSavedCardVerificationRequired = true
+	ctx := context.Background()
+	const ccof = "ccof:mock_approve"
+
+	_, err := client.CreatePayment(ctx, CreatePaymentReq{
+		Amount: 5000, Currency: "GBP", SourceID: ccof, CustomerID: "cus_appr", IdempotencyKey: "approve-gate",
+	})
+	require.Equal(t, "CARD_DECLINED_VERIFICATION_REQUIRED", ErrorCode(err))
+
+	_, err = client.CreatePayment(ctx, CreatePaymentReq{
+		Amount: 5000, Currency: "GBP", SourceID: ccof, CustomerID: "cus_appr", IdempotencyKey: "approve-before",
+		VerificationToken: "vrf_opaque_not_approved",
+	})
+	require.Equal(t, "VERIFICATION_TOKEN_INVALID", ErrorCode(err), "an opaque token for a still-pending challenge must be invalid")
+
+	client.ApprovePendingVerification(ccof)
+
+	result, err := client.CreatePayment(ctx, CreatePaymentReq{
+		Amount: 5000, Currency: "GBP", SourceID: ccof, CustomerID: "cus_appr", IdempotencyKey: "approve-after",
+		VerificationToken: "vrf_opaque_approved_1",
+	})
+	require.NoError(t, err)
+	require.Equal(t, "COMPLETED", result.Status, "an approved challenge must resolve the opaque token")
+}
+
+// TestDevClient_ApprovePendingVerification_CreatesIfMissing locks the
+// create-if-missing behavior: approving a card with NO recorded challenge still
+// lets a subsequent tokenized charge through.
+func TestDevClient_ApprovePendingVerification_CreatesIfMissing(t *testing.T) {
+	client := NewDevClient().(*MockClient)
+	client.SimulateSavedCardVerificationRequired = true
+	ctx := context.Background()
+	const ccof = "ccof:mock_approve_fresh"
+
+	client.ApprovePendingVerification(ccof)
+
+	result, err := client.CreatePayment(ctx, CreatePaymentReq{
+		Amount: 5000, Currency: "GBP", SourceID: ccof, CustomerID: "cus_appr2", IdempotencyKey: "approve-fresh",
+		VerificationToken: "vrf_opaque_fresh_1",
+	})
+	require.NoError(t, err)
+	require.Equal(t, "COMPLETED", result.Status)
+}
+
+// TestDevClient_ChallengeResult_Auto locks ChallengeResult="auto": the banking-
+// app challenge resolves itself as approved at gate time, so the next tokenized
+// retry succeeds WITHOUT an explicit ApprovePendingVerification call.
+func TestDevClient_ChallengeResult_Auto(t *testing.T) {
+	client := NewDevClient().(*MockClient)
+	client.SimulateSavedCardVerificationRequired = true
+	client.ChallengeResult = "auto"
+	ctx := context.Background()
+	const ccof = "ccof:mock_auto"
+
+	_, err := client.CreatePayment(ctx, CreatePaymentReq{
+		Amount: 5000, Currency: "GBP", SourceID: ccof, CustomerID: "cus_auto", IdempotencyKey: "auto-gate",
+	})
+	require.Equal(t, "CARD_DECLINED_VERIFICATION_REQUIRED", ErrorCode(err))
+
+	result, err := client.CreatePayment(ctx, CreatePaymentReq{
+		Amount: 5000, Currency: "GBP", SourceID: ccof, CustomerID: "cus_auto", IdempotencyKey: "auto-retry",
+		VerificationToken: "vrf_opaque_auto_1",
+	})
+	require.NoError(t, err, "ChallengeResult=auto must pre-approve the challenge so the retry succeeds")
+	require.Equal(t, "COMPLETED", result.Status)
+}
+
+// TestDevClient_ChallengeResult_Deny locks ChallengeResult="deny": the buyer
+// denies every banking-app challenge, so ANY verification token — including a
+// statelessly-approved deterministic one — is definitively rejected.
+func TestDevClient_ChallengeResult_Deny(t *testing.T) {
+	client := NewDevClient().(*MockClient)
+	client.SimulateSavedCardVerificationRequired = true
+	client.ChallengeResult = "deny"
+	ctx := context.Background()
+	const ccof = "ccof:mock_deny_all"
+
+	_, err := client.CreatePayment(ctx, CreatePaymentReq{
+		Amount: 5000, Currency: "GBP", SourceID: ccof, CustomerID: "cus_deny", IdempotencyKey: "deny-1",
+		VerificationToken: "verify_mock_mock_5000_ok",
+	})
+	require.Error(t, err)
+	require.Equal(t, "VERIFICATION_TOKEN_INVALID", ErrorCode(err), "a buyer denial must reject even a deterministically-ok token")
+
+	_, err = client.CreatePayment(ctx, CreatePaymentReq{
+		Amount: 5000, Currency: "GBP", SourceID: ccof, CustomerID: "cus_deny", IdempotencyKey: "deny-2",
+		VerificationToken: "verify_mock_mock_5000_deny",
+	})
+	require.Equal(t, "VERIFICATION_TOKEN_INVALID", ErrorCode(err))
+}
+
+// TestDevClient_VerifyMockDenyTokenSuffix locks the stateless _deny token
+// suffix: verify_mock_<prefix>_<amount>_deny encodes a buyer denial and is
+// rejected with VERIFICATION_TOKEN_INVALID even with NO shared-state challenge,
+// while the _ok encoding of the SAME binding succeeds (the control).
+func TestDevClient_VerifyMockDenyTokenSuffix(t *testing.T) {
+	client := NewDevClient().(*MockClient)
+	client.SimulateSavedCardVerificationRequired = true
+	ctx := context.Background()
+	const ccof = "ccof:mock_deny_suffix"
+
+	_, err := client.CreatePayment(ctx, CreatePaymentReq{
+		Amount: 5000, Currency: "GBP", SourceID: ccof, CustomerID: "cus_deny2", IdempotencyKey: "deny-suffix",
+		VerificationToken: "verify_mock_mock_5000_deny",
+	})
+	require.Error(t, err)
+	require.Equal(t, "VERIFICATION_TOKEN_INVALID", ErrorCode(err))
+	require.Equal(t, http.StatusBadRequest, ErrorStatusCode(err))
+	require.True(t, IsDefinitivePaymentError(err), "a _deny token is a definitive rejection")
+
+	result, err := client.CreatePayment(ctx, CreatePaymentReq{
+		Amount: 5000, Currency: "GBP", SourceID: ccof, CustomerID: "cus_deny2", IdempotencyKey: "deny-suffix-ok",
+		VerificationToken: "verify_mock_mock_5000_ok",
+	})
+	require.NoError(t, err)
+	require.Equal(t, "COMPLETED", result.Status, "the _ok encoding of the same binding must succeed (control)")
+}