-
Notifications
You must be signed in to change notification settings - Fork 215
feat: Dokan Captcha service Provider #2969
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
…upport** - Added a provider-based architecture for captcha management, supporting multiple providers (Google reCAPTCHA v3 and Cloudflare Turnstile). - Introduced `CaptchaServiceProvider`, `Manager`, and provider-specific classes for streamlined captcha handling. - Deprecated legacy captcha helper functions in favor of DI container-resolved manager. - Updated contact form and asset handling to dynamically adapt based on the selected captcha provider. - Added comprehensive provider configuration options in the admin settings. - Included migration documentation (`captcha.md`) outlining integration steps and best practices for transitioning to the new system.
…d compatibility - Updated token handling to target generic form elements instead of a specific form ID. - Added a hidden input field for storing the captcha token, ensuring compatibility with various forms.
WalkthroughThis pull request replaces Dokan's hardcoded Google reCAPTCHA v3 integration with a pluggable, provider-based CAPTCHA architecture. The refactor introduces a ProviderInterface contract, an AbstractProvider base class, a Manager registry facade, and two concrete implementations: GoogleRecaptchaV3Provider and CloudflareTurnstileProvider. Legacy reCAPTCHA settings are removed, validation flows are updated to use the new provider system, and functions are marked deprecated. Changes
Sequence Diagram(s)sequenceDiagram
participant User as Frontend
participant Ajax as Ajax Handler
participant Manager as Captcha Manager
participant Provider as Active Provider
participant External as External API
User->>Ajax: Submit form with captcha token
Ajax->>Manager: validate(context, token)
rect rgb(200, 230, 255)
note over Manager: Check if enabled & provider ready
Manager->>Manager: is_enabled()?
Manager->>Manager: get_active_provider()
end
alt Provider enabled and ready
Manager->>Provider: validate(context, token)
rect rgb(200, 255, 220)
note over Provider: Server-side verification
Provider->>External: POST siteverify (token, secret)
External-->>Provider: Success/failure response
end
Provider-->>Manager: bool (valid/invalid)
else Provider not available
Manager-->>Ajax: false (invalid)
end
Manager-->>Ajax: bool result
alt Token valid
Ajax->>User: Process form submission
else Token invalid
Ajax->>User: Validation error
end
sequenceDiagram
participant Admin as Admin User
participant Settings as Settings Panel
participant Manager as Captcha Manager
participant Providers as Registered Providers
Admin->>Settings: Open appearance settings
Settings->>Manager: filter_settings_fields()
rect rgb(200, 230, 255)
note over Manager: Build provider registry
Manager->>Manager: register_providers_from_filter()
Manager->>Providers: Discover from dokan_captcha_providers filter
Providers-->>Manager: Provider instances
end
Manager->>Manager: Get active provider settings
Manager->>Providers: get_admin_settings_fields()
Providers-->>Manager: Provider-specific fields
Manager-->>Settings: Global enable + provider select + provider fields
Settings->>Admin: Rendered form with all captcha options
Admin->>Admin: Select provider & configure
Admin->>Settings: Save settings
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (10)
docs/captcha.md (2)
6-6: Replace DOKAN_SINCE placeholder with the actual version.Readers need a concrete version (e.g., “≥ 4.0.8”).
74-75: Keep docs consistent with code: token/asset flow.Note: GoogleRecaptchaV3Provider now enqueues the Google API directly; it’s no longer pre‑registered in Assets::get_scripts(). Please adjust the note accordingly.
includes/functions.php (2)
4021-4044: Mark usage at runtime for deprecated helper.Add wc_deprecated_function() so developers get runtime notices when calling this.
function dokan_get_recaptcha_site_and_secret_keys( $boolean = false ) { + wc_deprecated_function( __FUNCTION__, 'DOKAN 4.0.8', '\WeDevs\Dokan\Captcha\Manager + providers' );
4050-4088: Ditto: deprecate with runtime notice.Emit a deprecation notice on entry.
function dokan_handle_recaptcha_validation( $action, $token, $secretkey ) { + wc_deprecated_function( __FUNCTION__, 'DOKAN 4.0.8', '\WeDevs\Dokan\Captcha\Manager::validate()' );includes/DependencyManagement/Providers/ServiceProvider.php (1)
71-72: Ensure CAPTCHA hooks are registered during boot.After registering CaptchaServiceProvider, proactively call Manager->register_hooks() so provider fields/settings are injected even if no global Hookable bootstrap runs.
- $this->getContainer()->addServiceProvider( new CaptchaServiceProvider() ); + $this->getContainer()->addServiceProvider( new CaptchaServiceProvider() ); + // Ensure Captcha hooks are registered early. + $this->getContainer()->get( \WeDevs\Dokan\Captcha\Manager::class )->register_hooks();If hooks are already auto-booted for all Hookable services elsewhere, ignore this and confirm where that happens.
includes/Ajax.php (1)
347-356: Fail closed when CAPTCHA is enabled, and tighten condition/message.
- If captcha is enabled but no active provider is ready, the form currently bypasses validation. Prefer failing closed in that case.
- Use a strict boolean check and generic message (works for Turnstile too).
- // Validate captcha using the selected provider if available - $captcha_manager = dokan_get_container()->get( \WeDevs\Dokan\Captcha\Manager::class ); - if ( $captcha_manager && $captcha_manager->get_active_provider() ) { - $is_valid = $captcha_manager->validate( 'dokan_contact_seller_recaptcha', $recaptcha_token ); - - if ( empty( $is_valid ) ) { - $message = sprintf( $error_template, __( 'reCAPTCHA verification failed!', 'dokan-lite' ) ); - wp_send_json_error( $message ); - } - } + // Validate captcha using the selected provider + $captcha_manager = dokan_get_container()->get( \WeDevs\Dokan\Captcha\Manager::class ); + if ( $captcha_manager && $captcha_manager->is_enabled() ) { + $provider = $captcha_manager->get_active_provider(); + if ( ! $provider ) { + $message = sprintf( $error_template, __( 'Captcha is not configured. Please try again later.', 'dokan-lite' ) ); + wp_send_json_error( $message ); + } + $is_valid = $captcha_manager->validate( 'dokan_contact_seller_recaptcha', $recaptcha_token ); + if ( ! $is_valid ) { + $message = sprintf( $error_template, __( 'Captcha verification failed!', 'dokan-lite' ) ); + wp_send_json_error( $message ); + } + }Confirm UX: if you prefer soft‑fail when disabled, this only enforces when the global “Enable Captcha Service” is on.
includes/DependencyManagement/Providers/CaptchaServiceProvider.php (1)
15-33: Register hooks in a boot phase to guarantee provider readiness.Consider making this provider bootable and invoking register_hooks() for Manager and providers. This avoids relying on a separate global Hookable bootstrap.
-namespace WeDevs\Dokan\DependencyManagement\Providers; +namespace WeDevs\Dokan\DependencyManagement\Providers; @@ -use WeDevs\Dokan\DependencyManagement\BaseServiceProvider; +use WeDevs\Dokan\DependencyManagement\BootableServiceProvider; @@ -class CaptchaServiceProvider extends BaseServiceProvider { +class CaptchaServiceProvider extends BootableServiceProvider { @@ public function register(): void { foreach ( $this->services as $service ) { $definition = $this->share_with_implements_tags( $service ); $this->add_tags( $definition, $this->tags ); } } + + public function boot(): void { + // Ensure hooks are registered once services are in the container. + $this->getContainer()->get( \WeDevs\Dokan\Captcha\Manager::class )->register_hooks(); + foreach ( [ \WeDevs\Dokan\Captcha\Providers\GoogleRecaptchaV3Provider::class, \WeDevs\Dokan\Captcha\Providers\CloudflareTurnstileProvider::class ] as $cls ) { + $this->getContainer()->get( $cls )->register_hooks(); + } + }If there is already a global boot mechanism that calls register_hooks() for all Hookable services, please confirm and feel free to skip this change.
includes/Captcha/Providers/GoogleRecaptchaV3Provider.php (1)
52-64: Update comment and avoid implying pre-registration.The handle isn’t pre-registered in Assets anymore. Tweak the comment to avoid confusion, or wrap with wp_script_is to avoid duplicate enqueue.
- // The handle is already registered in Assets::get_scripts() as 'dokan-google-recaptcha'. + // Enqueue Google reCAPTCHA v3 with the site key; helper code consumes dokan_google_recaptcha.recaptcha_sitekey. + // Guard against duplicate enqueue if needed. + if ( wp_script_is( 'dokan-google-recaptcha', 'enqueued' ) ) { + return; + }includes/Captcha/Providers/CloudflareTurnstileProvider.php (2)
56-66: Reuse the site key and bail out defensively.
$siteis fetched but unused, tripping PHPMD. Using it to short-circuit keeps the code tidy and protects against stale readiness cache if the key is removed mid-request.- $site = $this->get_option( 'turnstile_site_key', '' ); - // Register & enqueue Turnstile API + $site = $this->get_option( 'turnstile_site_key', '' ); + if ( empty( $site ) ) { + return; + } + + // Register & enqueue Turnstile API
60-61: Version the registered script to avoid stale caches.WordPress coding standards flag unversioned script registration because browsers may cache an out-of-date asset. Pass your plugin version (or another stable version) so cache busting works predictably.
- wp_register_script( $handle, 'https://challenges.cloudflare.com/turnstile/v0/api.js', [], null, true ); + wp_register_script( $handle, 'https://challenges.cloudflare.com/turnstile/v0/api.js', [], DOKAN_PLUGIN_VERSION, true );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
assets/images/cloudflare.pngis excluded by!**/*.png
📒 Files selected for processing (13)
docs/captcha.md(1 hunks)includes/Admin/Settings.php(0 hunks)includes/Ajax.php(1 hunks)includes/Assets.php(1 hunks)includes/Captcha/AbstractProvider.php(1 hunks)includes/Captcha/Manager.php(1 hunks)includes/Captcha/ProviderInterface.php(1 hunks)includes/Captcha/Providers/CloudflareTurnstileProvider.php(1 hunks)includes/Captcha/Providers/GoogleRecaptchaV3Provider.php(1 hunks)includes/DependencyManagement/Providers/CaptchaServiceProvider.php(1 hunks)includes/DependencyManagement/Providers/ServiceProvider.php(1 hunks)includes/functions.php(2 hunks)templates/widgets/store-contact-form.php(0 hunks)
💤 Files with no reviewable changes (2)
- templates/widgets/store-contact-form.php
- includes/Admin/Settings.php
🧰 Additional context used
🧬 Code graph analysis (9)
includes/Ajax.php (5)
dokan.php (1)
dokan_get_container(79-83)includes/Captcha/Manager.php (3)
Manager(17-206)get_active_provider(91-104)validate(115-121)includes/Captcha/Providers/CloudflareTurnstileProvider.php (1)
validate(96-115)includes/Captcha/ProviderInterface.php (1)
validate(40-40)includes/Captcha/Providers/GoogleRecaptchaV3Provider.php (1)
validate(89-110)
includes/DependencyManagement/Providers/ServiceProvider.php (1)
includes/DependencyManagement/Providers/CaptchaServiceProvider.php (1)
CaptchaServiceProvider(15-34)
includes/DependencyManagement/Providers/CaptchaServiceProvider.php (5)
includes/Captcha/Providers/CloudflareTurnstileProvider.php (1)
CloudflareTurnstileProvider(14-157)includes/Captcha/Providers/GoogleRecaptchaV3Provider.php (1)
GoogleRecaptchaV3Provider(14-168)includes/DependencyManagement/BaseServiceProvider.php (3)
BaseServiceProvider(21-103)share_with_implements_tags(86-88)add_tags(98-102)includes/Captcha/Manager.php (1)
Manager(17-206)includes/DependencyManagement/Providers/ServiceProvider.php (1)
register(93-97)
includes/Captcha/Providers/GoogleRecaptchaV3Provider.php (4)
includes/Captcha/AbstractProvider.php (5)
AbstractProvider(14-93)compute_readiness(48-48)get_option(19-21)is_ready(31-37)to_bool(84-92)includes/Captcha/Providers/CloudflareTurnstileProvider.php (7)
get_slug(20-22)get_label(29-31)compute_readiness(38-45)register_assets(52-67)render_field_html(77-86)validate(96-115)get_admin_settings_fields(122-155)includes/Captcha/ProviderInterface.php (7)
get_slug(9-9)get_label(12-12)register_assets(21-21)is_ready(15-15)render_field_html(30-30)validate(40-40)get_admin_settings_fields(48-48)includes/Captcha/Manager.php (4)
register_assets(107-112)render_field_html(124-130)validate(115-121)to_bool(198-205)
includes/Captcha/Manager.php (7)
includes/Admin/Settings.php (1)
Settings(18-1112)includes/Captcha/Providers/GoogleRecaptchaV3Provider.php (8)
GoogleRecaptchaV3Provider(14-168)get_slug(20-22)to_bool(119-126)register_assets(52-64)validate(89-110)render_field_html(77-79)get_label(29-31)get_admin_settings_fields(134-167)includes/Captcha/AbstractProvider.php (3)
register_hooks(58-60)to_bool(84-92)is_ready(31-37)includes/Captcha/Providers/CloudflareTurnstileProvider.php (6)
get_slug(20-22)register_assets(52-67)validate(96-115)render_field_html(77-86)get_label(29-31)get_admin_settings_fields(122-155)includes/Captcha/ProviderInterface.php (7)
get_slug(9-9)is_ready(15-15)register_assets(21-21)validate(40-40)render_field_html(30-30)get_label(12-12)get_admin_settings_fields(48-48)includes/functions.php (1)
dokan_get_option(1014-1024)dokan.php (1)
dokan_get_container(79-83)
includes/Captcha/Providers/CloudflareTurnstileProvider.php (4)
includes/Captcha/AbstractProvider.php (4)
AbstractProvider(14-93)compute_readiness(48-48)get_option(19-21)is_ready(31-37)includes/Captcha/ProviderInterface.php (7)
get_slug(9-9)get_label(12-12)register_assets(21-21)is_ready(15-15)render_field_html(30-30)validate(40-40)get_admin_settings_fields(48-48)includes/Captcha/Providers/GoogleRecaptchaV3Provider.php (7)
get_slug(20-22)get_label(29-31)compute_readiness(38-45)register_assets(52-64)render_field_html(77-79)validate(89-110)get_admin_settings_fields(134-167)includes/Captcha/Manager.php (3)
register_assets(107-112)render_field_html(124-130)validate(115-121)
includes/Captcha/ProviderInterface.php (4)
includes/Captcha/Providers/CloudflareTurnstileProvider.php (6)
get_slug(20-22)get_label(29-31)register_assets(52-67)render_field_html(77-86)validate(96-115)get_admin_settings_fields(122-155)includes/Captcha/Providers/GoogleRecaptchaV3Provider.php (6)
get_slug(20-22)get_label(29-31)register_assets(52-64)render_field_html(77-79)validate(89-110)get_admin_settings_fields(134-167)includes/Captcha/AbstractProvider.php (1)
is_ready(31-37)includes/Captcha/Manager.php (3)
register_assets(107-112)render_field_html(124-130)validate(115-121)
includes/Captcha/AbstractProvider.php (5)
includes/functions.php (1)
dokan_get_option(1014-1024)includes/Captcha/ProviderInterface.php (1)
is_ready(15-15)includes/Captcha/Providers/CloudflareTurnstileProvider.php (1)
compute_readiness(38-45)includes/Captcha/Providers/GoogleRecaptchaV3Provider.php (1)
compute_readiness(38-45)includes/Captcha/Manager.php (1)
register_hooks(27-34)
includes/Assets.php (5)
dokan.php (1)
dokan_get_container(79-83)includes/Captcha/Manager.php (2)
Manager(17-206)register_assets(107-112)includes/Captcha/Providers/CloudflareTurnstileProvider.php (1)
register_assets(52-67)includes/Captcha/ProviderInterface.php (1)
register_assets(21-21)includes/Captcha/Providers/GoogleRecaptchaV3Provider.php (1)
register_assets(52-64)
🪛 GitHub Check: Run PHPCS inspection
includes/Captcha/Providers/CloudflareTurnstileProvider.php
[failure] 157-157:
The closing brace for the class must go on the next line after the body
[warning] 60-60:
Resource version not set in call to wp_register_script(). This means new versions of the script may not always be loaded due to browser caching.
includes/Captcha/AbstractProvider.php
[warning] 19-19:
It is recommended not to use reserved keyword "default" as function parameter name. Found: $default
🪛 LanguageTool
docs/captcha.md
[uncategorized] ~31-~31: Use a comma before ‘so’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...via the dokan_captcha_providers filter so third parties can add providers - Deleg...
(COMMA_COMPOUND_SENTENCE_2)
🪛 PHPMD (2.15.0)
includes/Captcha/Providers/GoogleRecaptchaV3Provider.php
77-77: Avoid unused parameters such as '$context'. (undefined)
(UnusedFormalParameter)
77-77: Avoid unused parameters such as '$args'. (undefined)
(UnusedFormalParameter)
includes/Captcha/Manager.php
145-145: Avoid unused parameters such as '$settings_instance'. (undefined)
(UnusedFormalParameter)
includes/Captcha/Providers/CloudflareTurnstileProvider.php
56-56: Avoid unused local variables such as '$site'. (undefined)
(UnusedLocalVariable)
77-77: Avoid unused parameters such as '$context'. (undefined)
(UnusedFormalParameter)
77-77: Avoid unused parameters such as '$args'. (undefined)
(UnusedFormalParameter)
96-96: Avoid unused parameters such as '$context'. (undefined)
(UnusedFormalParameter)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: e2e tests (1, 3)
- GitHub Check: e2e tests (3, 3)
- GitHub Check: e2e tests (2, 3)
- GitHub Check: api tests (1, 1)
🔇 Additional comments (2)
includes/Assets.php (1)
853-856: Review comment is incorrect - Manager::register_hooks() and Provider register_hooks() are properly invoked.The bootstrap mechanism in
dokan-class.php(lines 273-276) retrieves all services implementing theHookableinterface from the container and explicitly callsregister_hooks()on each one during theinitaction. Since bothManagerandAbstractProviderimplementHookable, theirregister_hooks()methods are automatically invoked as part of the normal plugin initialization sequence, ensuring CAPTCHA hooks and fields are properly registered.Likely an incorrect or invalid review comment.
includes/Captcha/Manager.php (1)
149-192: Good job wiring provider-specific settings gating.The combined
show_ifconditions keep provider fields hidden unless captcha is enabled and the matching provider is selected, which will save merchants a lot of confusion.
| public function validate( string $context, string $token ): bool { | ||
| $secret = $this->get_option( 'turnstile_secret_key', '' ); | ||
| if ( empty( $token ) || empty( $secret ) ) { | ||
| return false; | ||
| } | ||
| $response = wp_remote_post( | ||
| 'https://challenges.cloudflare.com/turnstile/v0/siteverify', [ | ||
| 'body' => [ | ||
| 'secret' => $secret, | ||
| 'response' => $token, | ||
| ], | ||
| ] | ||
| ); | ||
| if ( is_wp_error( $response ) ) { | ||
| return false; | ||
| } | ||
| $response_body = wp_remote_retrieve_body( $response ); | ||
| $data = json_decode( $response_body, true ); | ||
| return ! empty( $data['success'] ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prevent fatal when Cloudflare returns non-JSON payloads.
json_decode() can return null (e.g., HTML error page or empty body). Accessing $data['success'] on a non-array throws a PHP 8 fatal (“Cannot use array offset on value of type null”), breaking captcha validation and the request path. Guard the decode result before reading offsets.
- $data = json_decode( $response_body, true );
- return ! empty( $data['success'] );
+ $data = json_decode( $response_body, true );
+ if ( ! is_array( $data ) ) {
+ return false;
+ }
+
+ return ! empty( $data['success'] );🧰 Tools
🪛 PHPMD (2.15.0)
96-96: Avoid unused parameters such as '$context'. (undefined)
(UnusedFormalParameter)
🤖 Prompt for AI Agents
In includes/Captcha/Providers/CloudflareTurnstileProvider.php around lines 96 to
114, json_decode() can return null for non-JSON responses which makes
$data['success'] access fatal; after decoding, ensure the result is an array and
that the 'success' key exists before accessing it—if json_decode() does not
return an array (or the 'success' key is absent), return false; implement this
guard by checking is_array($data) and using isset/array_key_exists on 'success'
(or default to false) before returning the boolean.
| } | ||
|
|
||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
Fix class-closing brace indentation.
PHPCS is failing because the class closing brace is indented. WordPress standards expect it flush-left.
-}
+}Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 GitHub Check: Run PHPCS inspection
[failure] 157-157:
The closing brace for the class must go on the next line after the body
🤖 Prompt for AI Agents
In includes/Captcha/Providers/CloudflareTurnstileProvider.php around lines 155
to 157, the class closing brace is indented; adjust the file so the final "}" of
the class is flush-left (no leading spaces or tabs) to match WordPress PHPCS
standards. Ensure there is a single newline after the closing brace if file ends
there and that indentation inside the class remains unchanged.
All Submissions:
Changes proposed in this Pull Request:
Related Pull Request(s)
Closes
How to test the changes in this Pull Request:
Changelog entry
Before Changes
Describe the issue before changes with screenshots(s).
After Changes
Feature Video (optional)
Link of detailed video if this PR is for a feature.
PR Self Review Checklist:
FOR PR REVIEWER ONLY:
Summary by CodeRabbit
Release Notes
New Features
Documentation
Chores