feat: enhance payment controller security by using transaction ID for server-side validation

Spomky · Spomky · commit 5a34b0d86acb · 2025-11-17T23:35:01.000+01:00
diff --git a/docs/secure-payment-confirmation.md b/docs/secure-payment-confirmation.md
@@ -169,24 +169,22 @@ $clientPaymentData = CollectedClientPaymentData::create($paymentData);
 
 ### Using the Payment Controller
 
-The Stimulus payment controller simplifies SPC integration on the client side:
+The Stimulus payment controller simplifies SPC integration on the client side.
+
+**SECURITY NOTICE:** For security reasons, payment details (amount, payee, etc.) are NOT passed via HTML attributes, as these can be tampered with by the client. Instead, you pass a secure `transaction-id`, and the server fetches the actual payment details from its database.
 
 ```html
 <form data-controller="webauthn--payment"
       data-action="submit->webauthn--payment#confirmPayment"
       data-webauthn--payment-options-url-value="/api/payment/options"
       data-webauthn--payment-result-url-value="/api/payment/verify"
-      data-webauthn--payment-payee-name-value="Merchant Store"
-      data-webauthn--payment-payee-origin-value="https://merchant.example.com"
-      data-webauthn--payment-amount-value="99.99"
-      data-webauthn--payment-currency-value="USD"
-      data-webauthn--payment-instrument-display-name-value="Visa •••• 1234"
-      data-webauthn--payment-instrument-icon-value="https://example.com/visa-icon.png">
+      data-webauthn--payment-transaction-id-value="txn_abc123def456">
 
     <h2>Confirm Payment</h2>
-    <p>Amount: <strong>$99.99 USD</strong></p>
-    <p>Merchant: <strong>Merchant Store</strong></p>
-    <p>Payment Method: <strong>Visa •••• 1234</strong></p>
+    <!-- Display payment details from server-side data -->
+    <p>Amount: <strong><?= htmlspecialchars($transaction->getFormattedAmount()) ?></strong></p>
+    <p>Merchant: <strong><?= htmlspecialchars($transaction->getPayeeName()) ?></strong></p>
+    <p>Payment Method: <strong><?= htmlspecialchars($transaction->getInstrumentDisplay()) ?></strong></p>
 
     <input type="hidden" data-webauthn--payment-target="result">
     <button type="submit">Confirm Payment</button>
@@ -199,28 +197,26 @@ The Stimulus payment controller simplifies SPC integration on the client side:
 |--------|------|---------|-------------|
 | `optionsUrl` | String | `/payment/options` | URL to fetch payment options |
 | `resultUrl` | String | `/payment/verify` | URL to verify payment result |
-| `payeeName` | String | - | Name of the payee (merchant) |
-| `payeeOrigin` | String | - | Origin of the payee |
-| `amount` | String | - | Payment amount |
-| `currency` | String | `USD` | Payment currency (ISO 4217) |
-| `instrumentDisplayName` | String | - | Display name for payment instrument |
-| `instrumentIcon` | String | - | Icon URL for payment instrument |
+| `transactionId` | String | - | **Secure transaction ID** (server fetches payment details) |
 | `submitViaForm` | Boolean | `false` | Submit credential via form instead of API |
 | `successRedirectUri` | String | - | URI to redirect to on success |
 
+**Security Note:** Payment details (amount, payee, merchant) are **intentionally NOT configurable** via HTML attributes to prevent client-side tampering. The server must fetch these from its database using the `transactionId`.
+
 ### Controller Events
 
 The payment controller dispatches several custom events:
 
 ```javascript
 // Connection event
 document.addEventListener('webauthn:payment:connect', (event) => {
-    console.log('Payment controller connected', event.detail);
+    console.log('Payment controller connected');
+    console.log('Transaction ID:', event.detail.transactionId);
 });
 
 // Options request/response events
 document.addEventListener('webauthn:payment:options:request', (event) => {
-    console.log('Requesting payment options', event.detail.data);
+    console.log('Requesting payment options for transaction:', event.detail.data.transactionId);
 });
 
 document.addEventListener('webauthn:payment:options:success', (event) => {
@@ -283,22 +279,40 @@ class PaymentController
     {
         $data = json_decode($request->getContent(), true);
 
-        // Create payment extension
+        // SECURITY: Fetch payment details from database using transaction ID
+        // This prevents client-side tampering of amounts/payee
+        $transactionId = $data['transactionId'] ?? null;
+        if (!$transactionId) {
+            return new JsonResponse(['error' => 'Transaction ID required'], 400);
+        }
+
+        // Get transaction from secure database
+        $transaction = $this->transactionRepository->findOneBy(['id' => $transactionId]);
+        if (!$transaction || $transaction->getUserId() !== $this->getUser()->getId()) {
+            return new JsonResponse(['error' => 'Transaction not found'], 404);
+        }
+
+        // Verify transaction is in pending state
+        if ($transaction->getStatus() !== 'pending') {
+            return new JsonResponse(['error' => 'Transaction already processed'], 400);
+        }
+
+        // Create payment extension with SERVER-VALIDATED data
         $amount = PaymentCurrencyAmount::create(
-            $data['payment']['total']['currency'],
-            $data['payment']['total']['value']
+            $transaction->getCurrency(),
+            $transaction->getAmount()
         );
 
         $instrument = PaymentCredentialInstrument::create(
-            $data['payment']['instrument']['displayName'] ?? 'Card',
-            $data['payment']['instrument']['icon'] ?? 'https://example.com/default-icon.png'
+            $transaction->getPaymentMethod()->getDisplayName(),
+            $transaction->getPaymentMethod()->getIconUrl()
         );
 
         $paymentExtension = PaymentExtension::register(
             rpId: 'example.com',
-            topOrigin: 'https://merchant.example.com',
-            payeeName: $data['payment']['payeeName'],
-            payeeOrigin: $data['payment']['payeeOrigin'],
+            topOrigin: $transaction->getMerchantOrigin(),
+            payeeName: $transaction->getPayeeName(),
+            payeeOrigin: $transaction->getPayeeOrigin(),
             amount: $amount,
             instrument: $instrument
         );
@@ -311,8 +325,9 @@ class PaymentController
             extensions: new AuthenticationExtensions([$paymentExtension])
         );
 
-        // Store challenge in session for verification
+        // Store challenge and transaction ID in session for verification
         $_SESSION['payment_challenge'] = base64_encode($options->challenge);
+        $_SESSION['payment_transaction_id'] = $transactionId;
 
         return new JsonResponse($options);
     }
@@ -368,28 +383,31 @@ class PaymentController
     <div class="payment-container">
         <h1>Checkout</h1>
 
+        <?php
+        // Fetch transaction from secure database
+        $transaction = $transactionRepository->find($transactionId);
+        ?>
+
         <form data-controller="webauthn--payment"
               data-action="submit->webauthn--payment#confirmPayment"
               data-webauthn--payment-options-url-value="/api/payment/options"
               data-webauthn--payment-result-url-value="/api/payment/verify"
-              data-webauthn--payment-payee-name-value="Acme Store"
-              data-webauthn--payment-payee-origin-value="https://acme-store.example.com"
-              data-webauthn--payment-amount-value="149.99"
-              data-webauthn--payment-currency-value="USD"
-              data-webauthn--payment-instrument-display-name-value="Visa •••• 4242"
-              data-webauthn--payment-instrument-icon-value="/images/visa-logo.png"
+              data-webauthn--payment-transaction-id-value="<?= htmlspecialchars($transaction->getId()) ?>"
               data-webauthn--payment-success-redirect-uri-value="/payment/success">
 
             <div class="order-summary">
                 <h2>Order Summary</h2>
-                <p>Total: <strong>$149.99 USD</strong></p>
-                <p>Merchant: <strong>Acme Store</strong></p>
+                <!-- Display from server-side data, NOT from HTML attributes -->
+                <p>Total: <strong><?= htmlspecialchars($transaction->getFormattedAmount()) ?></strong></p>
+                <p>Merchant: <strong><?= htmlspecialchars($transaction->getPayeeName()) ?></strong></p>
             </div>
 
             <div class="payment-method">
                 <h3>Payment Method</h3>
-                <img src="/images/visa-logo.png" alt="Visa" width="40">
-                <span>Visa •••• 4242</span>
+                <img src="<?= htmlspecialchars($transaction->getPaymentMethod()->getIconUrl()) ?>"
+                     alt="<?= htmlspecialchars($transaction->getPaymentMethod()->getBrand()) ?>"
+                     width="40">
+                <span><?= htmlspecialchars($transaction->getPaymentMethod()->getDisplayName()) ?></span>
             </div>
 
             <button type="submit" class="btn-primary">
@@ -450,12 +468,78 @@ if (browserSupportsWebAuthn()) {
 
 ## Security Considerations
 
-1. **Always validate payment data** on the server side using `PaymentExtensionOutputChecker`
-2. **Use HTTPS** - SPC requires a secure context
-3. **Verify the challenge** matches what was sent to the client
-4. **Check the RP ID** matches your domain
-5. **Validate the payee origin** matches the expected merchant
-6. **Store credentials securely** using proper key management
+### Critical Security Measures
+
+1. **NEVER trust client-side payment data**
+   - Payment amounts, payee names, and merchant details must NEVER come from HTML attributes or JavaScript
+   - Always fetch these from your secure server-side database using a transaction ID
+   - The Stimulus controller is designed to only send a `transactionId` - do not modify it to accept payment details
+
+2. **Server-side validation is mandatory**
+   - Always validate payment data using `PaymentExtensionOutputChecker`
+   - Verify the transaction belongs to the authenticated user
+   - Check the transaction status (must be "pending")
+   - Validate the amount hasn't been modified
+
+3. **Transaction ID security**
+   - Generate cryptographically secure transaction IDs (e.g., `bin2hex(random_bytes(16))`)
+   - Store transaction state in your database with user association
+   - Implement transaction expiry (e.g., 15 minutes)
+   - Mark transactions as "completed" or "cancelled" after processing
+
+4. **Standard WebAuthn security**
+   - Use HTTPS - SPC requires a secure context
+   - Verify the challenge matches what was sent to the client
+   - Check the RP ID matches your domain
+   - Validate the payee origin matches the expected merchant
+   - Store credentials securely using proper key management
+
+### Example: Secure Transaction Flow
+
+```php
+// 1. Create transaction in database (server-side only)
+$transaction = new Transaction();
+$transaction->setId(bin2hex(random_bytes(16))); // Secure ID
+$transaction->setUserId($currentUser->getId());
+$transaction->setAmount('99.99');
+$transaction->setCurrency('USD');
+$transaction->setPayeeName('Merchant Store');
+$transaction->setStatus('pending');
+$transaction->setExpiresAt(new DateTime('+15 minutes'));
+$entityManager->persist($transaction);
+$entityManager->flush();
+
+// 2. Render page with transaction ID only
+echo '<form data-webauthn--payment-transaction-id-value="' .
+     htmlspecialchars($transaction->getId()) . '">';
+
+// 3. In options endpoint: Fetch from database
+$transaction = $repository->findOneBy([
+    'id' => $transactionId,
+    'userId' => $currentUser->getId(),
+    'status' => 'pending'
+]);
+
+if (!$transaction || $transaction->isExpired()) {
+    return new JsonResponse(['error' => 'Invalid transaction'], 400);
+}
+
+// Use $transaction data for payment extension (NOT client data!)
+```
+
+### Why This Matters
+
+**Attack scenario without this protection:**
+1. User initiates payment for $10.00
+2. Attacker modifies HTML: `data-amount-value="0.01"`
+3. Without server validation, user pays only $0.01
+
+**Protection with transaction ID:**
+1. User initiates payment for $10.00
+2. Server creates transaction with ID `txn_abc123` storing amount $10.00
+3. Attacker can modify HTML attributes, but server ignores them
+4. Server always uses database amount ($10.00) from transaction ID
+5. Payment is processed for the correct amount ✅
 
 ## Troubleshooting
 
diff --git a/src/stimulus/PAYMENT_CONTROLLER.md b/src/stimulus/PAYMENT_CONTROLLER.md
@@ -2,6 +2,10 @@
 
 The Payment Controller provides Stimulus-based integration for Secure Payment Confirmation (SPC) using WebAuthn.
 
+## ⚠️ Security Notice
+
+**This controller is designed to be secure by default.** Payment details (amount, payee, merchant) are **intentionally NOT** accepted via HTML attributes to prevent client-side tampering. Instead, you provide a secure `transaction-id`, and the server fetches the actual payment details from its database.
+
 ## Installation
 
 ```bash
@@ -15,10 +19,10 @@ npm install @web-auth/webauthn-stimulus
       data-action="submit->webauthn--payment#confirmPayment"
       data-webauthn--payment-options-url-value="/api/payment/options"
       data-webauthn--payment-result-url-value="/api/payment/verify"
-      data-webauthn--payment-payee-name-value="Merchant Store"
-      data-webauthn--payment-payee-origin-value="https://merchant.example.com"
-      data-webauthn--payment-amount-value="99.99"
-      data-webauthn--payment-currency-value="USD">
+      data-webauthn--payment-transaction-id-value="txn_abc123def456">
+
+    <!-- Display payment info from server-side data -->
+    <p>Amount: <?= htmlspecialchars($transaction->getFormattedAmount()) ?></p>
 
     <input type="hidden" data-webauthn--payment-target="result">
     <button type="submit">Confirm Payment</button>
@@ -29,18 +33,15 @@ npm install @web-auth/webauthn-stimulus
 
 ### Values
 
-| Value                   | Type     | Default           | Description                              |
-|-------------------------|---------|--------------------|------------------------------------------|
-| `optionsUrl`            | String  | `/payment/options` | URL to fetch payment options from server |
-| `resultUrl`             | String  | `/payment/verify`  | URL to verify payment credential         |
-| `payeeName`             | String  | -                  | Name of the payee (merchant)             |
-| `payeeOrigin`           | String  | -                  | Origin of the payee                      |
-| `amount`                | String  | -                  | Payment amount (e.g., "99.99")           |
-| `currency`              | String  | `USD`              | ISO 4217 currency code                   |
-| `instrumentDisplayName` | String  | -                  | Display name for payment instrument      |
-| `instrumentIcon`        | String  | -                  | URL to payment instrument icon           |
-| `submitViaForm`         | Boolean | `false`            | Submit via form instead of API           |
-| `successRedirectUri`    | String  | -                  | Redirect URL on success                  |
+| Value                | Type     | Default           | Description                              |
+|----------------------|---------|-------------------|------------------------------------------|
+| `optionsUrl`         | String  | `/payment/options` | URL to fetch payment options from server |
+| `resultUrl`          | String  | `/payment/verify`  | URL to verify payment credential         |
+| `transactionId`      | String  | -                  | **Secure transaction ID** (server fetches payment details) |
+| `submitViaForm`      | Boolean | `false`            | Submit via form instead of API           |
+| `successRedirectUri` | String  | -                  | Redirect URL on success                  |
+
+**🔒 Security Note:** Payment details (amount, payee, merchant) are **intentionally NOT configurable** via HTML attributes. The server must fetch these from its database using the `transactionId` to prevent client-side tampering.
 
 ### Targets
 
diff --git a/src/stimulus/assets/src/payment-controller.js b/src/stimulus/assets/src/payment-controller.js
