Add Server-Side Paywall Validation for Price and Currency in payment_trigger() #109
Conversation
WalkthroughAdds currency-aware server-side paywall validation to the AJAX payment handler: the handler now loads paywall requirements from the target post, verifies currency/unit and paid amount (with epsilon tolerance), and only writes unlock data after successful validation; admin docs example includes Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Webhook / Frontend
participant Server as PayButton AJAX (payment_trigger)
participant Post as WP Post / Shortcode
participant DB as Unlock DB Writer
Client->>Server: POST payload (post_id, user_address, amount, currency)
Server->>Post: paybutton_get_paywall_requirements(post_id)
Post-->>Server: paywall requirements (price, unit, defaults)
alt missing fields or not paywalled
Server-->>Client: error (missing post/user/currency or not paywalled)
else currency/unit mismatch or amount invalid
Server-->>Client: error (currency/unit mismatch or price check failed)
else validation passes
Server->>DB: write unlock record (post_id, user_address, tx)
DB-->>Server: write confirmation
Server-->>Client: success (unlocked)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
includes/class-paybutton-ajax.php (2)
193-196: Consider handling missing currency field more gracefully.If
currencyis empty in the webhook payload,$incoming_unitwill be empty and the unit comparison at line 222 will always fail. Consider providing a more informative error message when the currency field is missing from the webhook.$incoming_unit = ''; if ( ! empty( $currency ) ) { $incoming_unit = strtoupper( $currency ); +} else { + wp_send_json_error( array( 'message' => 'Missing currency in webhook payload.' ), 400 ); + return; }
222-225: Minor: Redundantstrtoupper()call.
$incoming_unitis already uppercased at line 195, so thestrtoupper( $incoming_unit )call here is redundant.-if ( strtoupper( $incoming_unit ) !== strtoupper( $expected_unit ) ) { +if ( $incoming_unit !== $expected_unit ) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
includes/class-paybutton-ajax.php(4 hunks)templates/admin/paywall-settings.php(1 hunks)
🔇 Additional comments (8)
templates/admin/paywall-settings.php (1)
259-260: LGTM!The documentation update correctly reflects the new
currencyfield requirement in the webhook payload, which aligns with the backend validation changes inpayment_trigger().includes/class-paybutton-ajax.php (7)
104-105: LGTM!The payload extraction handles both nested object (
['address']) and direct array formats foruser_address, providing good flexibility for different payload structures from the PayButton server.
227-238: LGTM!The validation flow is solid: after verifying both the payment amount and currency match expectations, the unlock is stored in the database with proper sanitization. This correctly implements the PR objective of server-side paywall validation.
311-317: Edge case: Explicit price of 0 is overridden by default.If a shortcode explicitly sets
price="0", the condition$price === 0.0will override it with the default price. If free/zero-price paywalls are never intended, this is correct. Otherwise, you may want to distinguish between "not set" and "explicitly set to 0".Verify if zero-price paywalls are a supported use case in this plugin.
282-326: LGTM!The helper method correctly parses the
[paywalled_content]shortcode attributes and falls back to plugin options when values are not specified. The use ofshortcode_parse_atts()is the proper WordPress approach.
198-201: Good defensive check added.This early validation for required fields (
post_idanduser_address) with a clear error message improves error handling and provides better debugging feedback.
203-208: Good handling of non-paywalled posts.Returning an error for posts without
[paywalled_content]shortcode prevents processing payments for content that shouldn't require payment.
214-220: The epsilon tolerance of 0.05 is actually sufficient and conservative for typical payment scenarios.Rounding errors in cryptocurrency-to-fiat conversions are bounded by currency precision (typically ≤$0.01 per transaction when converting to USD/CAD cents), making 0.05 a 5–10× safety margin. Rounding error magnitude is independent of transaction size—it scales with decimal precision, not amount. No changes needed.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
includes/class-paybutton-ajax.php (1)
98-112: Don’t trustpost_id/tx_*/currency/user_addressunless they’re covered by the verified signature (and fixuser_addressshape parsing).
Right now you verify$payload(Line 121-126), but you use top-level fields (Line 98-105, 129-136). If an attacker can replay a validsignature+payloadpair, they may be able to alter unsigned top-level fields (e.g.,tx_amount,currency,post_id,tx_hash) and still pass verification, leading to incorrect unlock writes. Also,$json['user_address'][0]will behave badly ifuser_addressis a string (it becomes the first character) and can cause false “non-empty” passes.Suggested direction:
- Parse the signed
$payloadinto a structured object and sourcepost_id,tx_hash,tx_amount,tx_timestamp,user_address,currencyfrom that signed content (or validate the top-level fields exactly match the signed payload fields).- Harden
user_addressextraction to handle both shapes explicitly.- $user_addr_raw = $json['user_address'][0]['address'] ?? ($json['user_address'][0] ?? ''); - $currency_raw = $json['currency'] ?? ''; + // Prefer extracting these from the SIGNED payload structure (see note below). + $user_addr_raw = ''; + if ( isset($json['user_address']) && is_array($json['user_address']) ) { + $first = $json['user_address'][0] ?? null; + if ( is_array($first) ) { + $user_addr_raw = $first['address'] ?? ''; + } elseif ( is_string($first) ) { + $user_addr_raw = $first; + } + } elseif ( isset($json['user_address']) && is_string($json['user_address']) ) { + $user_addr_raw = $json['user_address']; + } + $currency_raw = isset($json['currency']) && is_string($json['currency']) ? $json['currency'] : '';To verify the signing contract, please confirm via public docs/spec:
PayButton webhook Ed25519 signing: what exactly is signed in `signature.payload` (is it the full request body / canonical JSON containing post_id, tx_hash, tx_amount, tx_timestamp, user_address, currency), and how should servers validate it?Also applies to: 104-105, 121-126, 129-136
🧹 Nitpick comments (1)
includes/class-paybutton-ajax.php (1)
278-329: Acceptcurrencyshortcode attribute (alias ofunit) and validate parsed atts.
Given the admin example referencescurrency, but shortcode parsing usesunit(Line 310-312), consider supporting both (currencypreferred, fallback tounit) to avoid misconfiguration. Also, defensively handleshortcode_parse_atts()returning non-array.Also applies to: 306-321
This PR fixes #107 by introducing strict server-side validation for paywalled content by parsing the
[paywalled_content]shortcode at runtime and enforcing both the expected price and currency for each paywalled post or page.Test Plan:
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.