Skip to content

Conversation

@xecdev
Copy link
Collaborator

@xecdev xecdev commented Dec 6, 2025

This PR implements #89.

Test Plan:

  • Install the plugin
  • Check the 'Public Key' field in paywall settings

Greptile Overview

Greptile Summary

This PR makes the PayButton Public Key field mandatory in the Paywall Settings UI by adding the HTML5 required attribute and updating labels from "optional" to "required".

Key Changes:

  • Added required attribute to public key input field
  • Changed label text from "PayButton Public Key (optional)" to "PayButton Public Key (required)"
  • Updated documentation from "strongly recommended" to "required"
  • Added mention of "login transactions" alongside "paywall transactions"

Critical Issue Found:

  • Missing server-side validation: The HTML5 required attribute can be easily bypassed (browser dev tools, disabled JS, direct API calls). The save_settings() method in class-paybutton-admin.php (line 295-298) saves the public key without checking if it's empty. This creates a security gap where users could save empty settings, causing payment verification to fail at runtime when payment_trigger() checks for the key (line 114-118 in class-paybutton-ajax.php).

Confidence Score: 2/5

  • Not safe to merge - missing critical server-side validation for mandatory field
  • Score reflects a critical security gap: the PR makes the field mandatory only at the UI level without server-side enforcement. Users can bypass HTML5 validation and save empty values, breaking payment verification at runtime. This defeats the purpose of making the field mandatory.
  • templates/admin/paywall-settings.php needs server-side validation added to the save_settings() method in includes/class-paybutton-admin.php

Important Files Changed

File Analysis

Filename Score Overview
templates/admin/paywall-settings.php 2/5 Added HTML5 required attribute and updated labels, but missing critical server-side validation that could allow empty submissions

Sequence Diagram

sequenceDiagram
    participant Admin as WordPress Admin
    participant Form as Paywall Settings Form
    participant Server as save_settings()
    participant DB as WordPress Options
    participant Payment as payment_trigger()
    
    Note over Admin,Payment: Current Flow (Missing Validation)
    
    Admin->>Form: Save settings with empty public key
    Form->>Form: HTML5 validation (can be bypassed)
    Form->>Server: POST form data
    Server->>Server: sanitize_text_field()
    Note right of Server: ❌ No check if empty!
    Server->>DB: update_option('paybutton_public_key', '')
    DB-->>Server: Success
    Server-->>Admin: Settings saved
    
    Note over Admin,Payment: Later: Payment Attempt
    
    Payment->>Payment: Receive webhook from PayButton
    Payment->>DB: get_option('paybutton_public_key')
    DB-->>Payment: '' (empty string)
    Payment->>Payment: Check if empty
    Note right of Payment: ❌ Fails here at runtime!
    Payment-->>Payment: Return error
    
    Note over Admin,Payment: Should Add Server-Side Validation
    
    Admin->>Form: Save settings with empty public key
    Form->>Server: POST form data
    Server->>Server: Check if empty
    Note right of Server: ✓ Add validation here!
    Server-->>Admin: Error: Public key required
Loading

Summary by CodeRabbit

  • New Features

    • PayButton Public Key field is now required for configuration; applies to both paywall and login transactions.
  • Bug Fixes

    • Enhanced validation to check for both wallet address and public key during setup.
    • Improved admin error notifications to clearly identify missing required settings.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Dec 6, 2025

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Walkthrough

The changes enhance input validation for PayButton admin settings by renaming and expanding the wallet address validation method to also check for required public key, while simultaneously updating the settings form UI to mark the public key field as required.

Changes

Cohort / File(s) Summary
Admin Validation Logic
includes/class-paybutton-admin.php
Renamed admin_notice_missing_wallet_address() method to admin_notice_missing_required_inputs() with updated hook registration; expanded validation to check for both missing wallet address and public key, displaying separate error notices for each.
Settings Form UI
templates/admin/paywall-settings.php
Updated PayButton Public Key field label from "optional" to "required" and added HTML required attribute; updated accompanying note to reflect mandatory status for both paywall and login transactions.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

  • Verify the hook rename in the constructor doesn't have references elsewhere in the codebase
  • Confirm both validation notices render correctly for missing wallet address and missing public key
  • Test that the required HTML attribute functions properly in the form

Possibly related PRs

Poem

🐰 A key must be kept, a wallet to show,
Admin notices bloom where requirements grow,
No more optional fields, just truth and clear sight—
Required inputs validated, settings shining bright! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'make public key field mandatory' directly and clearly describes the main change: making the PayButton Public Key field required in the UI and adding validation logic.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/require-public-key

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9c966c8 and 1aeb2c8.

📒 Files selected for processing (2)
  • includes/class-paybutton-admin.php (3 hunks)
  • templates/admin/paywall-settings.php (2 hunks)
🔇 Additional comments (2)
includes/class-paybutton-admin.php (2)

17-17: LGTM!

The hook update correctly references the renamed method that now validates both wallet address and public key.


231-252: LGTM!

The method rename and expanded validation logic are well-implemented. The function now validates both wallet address and public key, displaying appropriate admin notices for missing required inputs.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@xecdev xecdev requested a review from Klakurka December 6, 2025 04:36
@xecdev xecdev self-assigned this Dec 6, 2025
@xecdev xecdev added the enhancement (behind the scenes) Stuff that users won't see label Dec 6, 2025
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 file reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

@xecdev
Copy link
Collaborator Author

xecdev commented Dec 6, 2025

Ready for review

@Klakurka
Copy link
Member

Klakurka commented Dec 6, 2025

@coderabbitai review

@coderabbitai
Copy link

coderabbitai bot commented Dec 6, 2025

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@xecdev xecdev linked an issue Dec 7, 2025 that may be closed by this pull request
@Klakurka Klakurka merged commit d4bd4f1 into master Dec 7, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement (behind the scenes) Stuff that users won't see

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Make PayButton Public Key a mandatory field in Paywall Settings

3 participants