Add support for registering sites with myDot and exchanging a jwt token that can be used to share site stats - #1315
Conversation
Added functionality to schedule daily license checks and verify SSL for licensing requests.
…r emails [PRO-286]
…r stats [PRO-286]
The ext-json requirement isn't technically now, we already used this. But the openssl one is. [PRO-286]
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a MyDot Connector for licensing, JWT auth with key rotation and site registration; wires it into plugin bootstrap and REST permission checks; introduces a Connected Services admin page; adds top-pages stats; updates composer requirements and unschedules license cron on deactivation; adds a dev API endpoint filter. Changes
Sequence Diagram(s)sequenceDiagram
participant Admin as Admin User
participant UI as Connected Services Page
participant Connector as MyDot Connector
participant API as MyDot API
participant WP as WordPress (Options/Cron)
Admin->>UI: submit license or JWT
UI->>Connector: handle submission (activate/register)
Connector->>API: validate license / request JWT / register site
API-->>Connector: response (license/JWT/registration, public key)
Connector->>Connector: validate JWT, verify/rotate keys, persist options
Connector->>WP: schedule/clear cron, update options
Connector-->>UI: return status for admin notice
sequenceDiagram
participant Client as Client
participant REST as Plugin REST Endpoint
participant Connector as MyDot Connector
participant API as MyDot API
participant WP as WordPress (capability check)
Client->>REST: request /scans-stats
REST->>Connector: validate_jwt_token_in_request_with_fallback(request)
Connector->>Connector: extract token & verify (iss/aud/exp/nbf/signature)
alt valid
Connector-->>REST: grant access
else invalid/expired
Connector->>API: fetch public key / refresh
API-->>Connector: public key
Connector->>Connector: re-validate token
alt re-validated
Connector-->>REST: grant access
else
REST->>WP: fallback to current_user_can('edit_posts')
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Summary of ChangesHello @pattonwebz, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a significant new feature by integrating the plugin with the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces functionality to connect the plugin with a 'myDot' service for license validation and sharing site statistics via JWT. It adds a new license page, handlers for license activation/deactivation, and site registration/unregistration. The changes also include a mechanism to validate JWTs for authenticating REST API requests. My review has identified a few critical issues, such as a hardcoded local API endpoint and usage of an undefined constant, which will prevent the feature from working in production. I've also pointed out some high-severity bugs related to redundant and faulty form handling logic, and some medium-severity issues regarding code style and WordPress best practices. Addressing these points will improve the correctness and maintainability of the new features.
…openssl and json [PRO-286]
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
composer.json (1)
50-53: Good addition of required PHP extensions.The
ext-opensslandext-jsonrequirements are necessary for JWT validation in the Connector class. This ensures environments have the required extensions before installation.Note: Line 53 uses a tab for indentation instead of spaces, which is inconsistent with the rest of the file.
🔎 Fix indentation consistency
"composer/installers": "^1.12.0", "ext-openssl": "*", "ext-json": "*" - }, + },includes/classes/MyDot/Connector.php (1)
70-71: Inconsistent hook name prefix.The cron hook uses
edacp_check_license_hook(EDACP prefix) but this is the free plugin which usesedac_prefix. This creates naming inconsistency and potential conflicts if EDACP (Pro) is also installed.🔎 Suggested fix
- add_action( 'edacp_check_license_hook', [ $this, 'periodic_check_license' ] ); + add_action( 'edac_check_license_hook', [ $this, 'periodic_check_license' ] );public function check_license_cron() { - if ( ! wp_next_scheduled( 'edacp_check_license_hook' ) ) { - wp_schedule_event( time(), 'daily', 'edacp_check_license_hook' ); + if ( ! wp_next_scheduled( 'edac_check_license_hook' ) ) { + wp_schedule_event( time(), 'daily', 'edac_check_license_hook' ); } }Also applies to: 295-298
admin/AdminPage/LicensePage.php (2)
231-245: Useadmin_url()for constructing admin URLs.The license URL is constructed manually using
get_bloginfo('url'). This may not work correctly on multisite installations or sites with non-standard admin paths.🔎 Suggested fix
public function admin_notices() { $error = get_option( 'edac_license_error' ); - $license_url = get_bloginfo( 'url' ) . '/wp-admin/admin.php?page=accessibility_checker_settings&tab=license'; + $license_url = admin_url( 'admin.php?page=accessibility_checker_settings&tab=license' ); $message = null;
150-154: Consider moving inline styles to a stylesheet.The status indicators use inline styles (
style="color:green;",style="color:red;"). For consistency and maintainability, consider using CSS classes instead.Also applies to: 201-203
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
admin/AdminPage/LicensePage.php(1 hunks)composer.json(1 hunks)includes/classes/MyDot/Connector.php(1 hunks)includes/classes/class-plugin.php(2 hunks)includes/classes/class-rest-api.php(2 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
composer.json
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use PSR-4 autoloading with the EqualizeDigital\AccessibilityChecker namespace
Files:
composer.json
**/*.php
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.php: Follow WordPress Coding Standards (WPCS) in all PHP files
Class names use CamelCase (ClassNameConvention) for new classes
Use edac_ prefix for all custom action/filter hook names
Ensure PHP 7.4+ compatibility
Use type hints where appropriate (parameters, return types, properties)
Sanitize inputs, validate data, and escape outputs following WordPress security best practices; use nonces for forms/AJAX
Use the WordPress database API ($wpdb) for all database operations
Prefix functions and classes in the global namespace with edac_
Use WordPress transients for caching temporary data where appropriate
All user-facing text in PHP must be translatable using the accessibility-checker text domain
Use PHPDoc for all public classes, methods, and properties
Document all custom hooks (actions/filters) with docblocks including parameters and types
Files:
includes/classes/class-plugin.phpincludes/classes/class-rest-api.phpadmin/AdminPage/LicensePage.phpincludes/classes/MyDot/Connector.php
includes/**/*.php
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Place core (non-admin) functionality in the /includes directory
Files:
includes/classes/class-plugin.phpincludes/classes/class-rest-api.phpincludes/classes/MyDot/Connector.php
**/class-*.php
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/class-*.php: Legacy PHP class files must use WordPress style naming class-class-name.php
Legacy class names use WordPress underscore style (Class_Name_Convention)
Files:
includes/classes/class-plugin.phpincludes/classes/class-rest-api.php
admin/**/*.php
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Place admin classes and admin-only PHP code in the /admin directory
Files:
admin/AdminPage/LicensePage.php
{admin,includes}/**/[A-Z][A-Za-z0-9]*.php
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
New PHP class files must use CamelCase file names like ClassName.php
Files:
admin/AdminPage/LicensePage.phpincludes/classes/MyDot/Connector.php
🧠 Learnings (2)
📚 Learning: 2025-08-29T11:15:35.151Z
Learnt from: CR
Repo: equalizedigital/accessibility-checker PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-29T11:15:35.151Z
Learning: Applies to composer.json : Use PSR-4 autoloading with the EqualizeDigital\AccessibilityChecker namespace
Applied to files:
includes/classes/class-plugin.phpincludes/classes/class-rest-api.php
📚 Learning: 2025-08-27T14:11:05.736Z
Learnt from: pattonwebz
Repo: equalizedigital/accessibility-checker PR: 1171
File: includes/helper-functions.php:808-826
Timestamp: 2025-08-27T14:11:05.736Z
Learning: In the Accessibility Checker Pro plugin, the class '\EqualizeDigital\AccessibilityCheckerPro\VirtualContent\PostType\VirtualPageType' was never released, so backward compatibility checks for this class name are not needed in edac_is_virtual_page() function.
Applied to files:
includes/classes/class-plugin.php
🧬 Code graph analysis (2)
includes/classes/class-rest-api.php (1)
includes/classes/MyDot/Connector.php (2)
Connector(20-676)validate_jwt_token_in_request(660-675)
admin/AdminPage/LicensePage.php (1)
includes/classes/MyDot/Connector.php (1)
Connector(20-676)
🪛 PHPStan (2.1.33)
includes/classes/MyDot/Connector.php
271-271: Constant EDACP_STORE_URL not found.
Learn more at https://phpstan.org/user-guide/discovering-symbols
(constant.notFound)
⏰ 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). (1)
- GitHub Check: Agent
🔇 Additional comments (6)
includes/classes/class-plugin.php (1)
13-13: LGTM!The Connector integration is correctly placed. The unconditional initialization is appropriate since:
Connector::init()has an internal guard checkingEDACP_VERSIONto avoid conflicts- The Connector needs to be active for REST API JWT validation and scheduled cron tasks
Also applies to: 47-48
includes/classes/class-rest-api.php (1)
107-112: LGTM!The dual authentication strategy is well-implemented:
- First checks for a valid JWT token (for external MyDot service requests)
- Falls back to WordPress capability check for logged-in admin users
This allows the
/scans-statsendpoint to serve both external authenticated services and WordPress administrators.includes/classes/MyDot/Connector.php (2)
124-149: License handling follows WordPress security best practices.Good security implementation:
- Capability check with
current_user_can('manage_options')- Nonce verification with
check_admin_referer()- Input sanitization with
sanitize_text_field()andwp_unslash()- Safe redirect with
wp_safe_redirect()
508-549: LGTM!The
register_site()method properly:
- Validates input before making API calls
- Handles WP_Error responses
- Returns structured success/failure arrays
- Uses proper JSON encoding and Content-Type headers
admin/AdminPage/LicensePage.php (2)
127-184: License form implementation follows WordPress best practices.The form correctly:
- Uses
admin-post.phpas the action target- Includes
settings_fields()for the option group- Uses
wp_nonce_field()for CSRF protection- Properly escapes all output with
esc_attr(),esc_html(), andesc_url()
186-220: Site registration UI is well-structured.The conditional rendering based on JWT token presence provides clear user feedback about registration status. Forms properly use separate nonces for register and unregister actions.
There are already post callbacks directly hooked in to register and unregister [PRO-286]
…nse data [PRO-286]
There was a problem hiding this comment.
Pull request overview
This PR adds MyDot integration for license management and site registration, enabling users with license keys to connect their sites to MyDot for stats sharing and email reports. The implementation includes JWT token-based authentication for secure API access and a dedicated license management page in the WordPress admin.
Key Changes:
- New MyDot Connector class for license activation/deactivation and JWT-based site registration
- License management page integrated into plugin settings
- REST API endpoint permission callback updated to accept JWT tokens for external access
- Required PHP extensions (openssl, json) added to composer dependencies
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 25 comments.
Show a summary per file
| File | Description |
|---|---|
| includes/classes/MyDot/Connector.php | Core connector class handling license API calls, JWT token validation, site registration/unregistration, and cron-based license checks |
| admin/AdminPage/LicensePage.php | License settings page UI with forms for license activation and site registration |
| includes/classes/class-plugin.php | Initializes Connector instance during plugin bootstrap |
| includes/classes/class-rest-api.php | Updates /scans-stats endpoint permission callback to accept JWT tokens |
| composer.json | Adds ext-openssl and ext-json as required dependencies |
Comments suppressed due to low confidence (1)
includes/classes/MyDot/Connector.php:38
- The API endpoint is hardcoded to use HTTP instead of HTTPS. This is a security concern as license keys and JWT tokens will be transmitted in plain text over the network. Change the protocol to HTTPS to ensure secure communication with the MyDot API.
const API_ENDPOINT = 'http://my.equalizedigital.local';
This brings it in line to conform with RFC 8725 [PRO-286]
…pro display differences There is still more work to do to fully integrate with pro [PRO-286]
…nk utms [PRO-286]
…ndle-licence' of github.com:equalizedigital/accessibility-checker into william/pro-286-update-the-free-plugin-to-be-able-to-handle-licence
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In @admin/AdminPage/ConnectedServicesPage.php:
- Line 137: The heading string passed to esc_html_e is incorrect; replace the
text "Connected this site" with "Connect this site" in the esc_html_e call so
the line reads esc_html_e( 'Connect this site', 'accessibility-checker' ) to fix
the grammar in the <h2> heading.
In @includes/classes/MyDot/Connector.php:
- Around line 726-742: In validate_jwt_token_in_request_with_fallback, the
Authorization header is referenced but $parts is never defined; parse the header
first (e.g., trim the header and explode by space into $parts), ensure you
handle case-insensitive "Bearer" (or compare lowercased $parts[0] to 'bearer'),
and only call self::validate_jwt_token_with_fallback($parts[1]) when $parts
count is 2 and the scheme matches; otherwise return false.
- Around line 66-73: The cron hook was registered with the wrong prefix: replace
the Pro prefix `edacp_check_license_hook` with the correct
`edac_check_license_hook` wherever it’s referenced so the scheduled event fires;
update the add_action call that binds `periodic_check_license` (currently using
`edacp_check_license_hook`) and update `check_license_cron()` so the scheduled
event uses `edac_check_license_hook` (and any corresponding
wp_schedule_event/wp_clear_scheduled_hook calls) to match the `edac_` naming
convention.
- Around line 290-294: The check decodes the remote response into $license_data
but only calls update_option('edac_license_status', $license_data->license) when
the license is not 'valid', leaving a renewed/valid license unrecorded; change
the logic in the block that handles wp_remote_retrieve_body($response) and
json_decode(...) so that update_option('edac_license_status',
$license_data->license) is executed for all returned statuses (optionally
sanitizing $license_data->license first) instead of only when 'valid' !==
$license_data->license.
🧹 Nitpick comments (1)
admin/AdminPage/ConnectedServicesPage.php (1)
299-304: Inconsistent URL handling - consider usingedac_link_wrapper()for UTM tracking.The renewal link on line 301 uses a hardcoded URL with inline UTM parameters, while
render_page()uses\edac_link_wrapper()for consistent UTM handling. Consider updating for consistency:🔎 Suggested fix
case 'expired': return sprintf( /* translators: %1$s: item name, %2$s: item name */ - __( 'Your %1$s license has expired. <a href="https://my.equalizedigital.com/?utm_source=wpadmin" target="_blank">Please renew your license</a> to continue accessing additional features and services for %2$s.', 'accessibility-checker' ), + __( 'Your %1$s license has expired. <a href="%2$s" target="_blank">Please renew your license</a> to continue accessing additional features and services for %3$s.', 'accessibility-checker' ), + Connector::PRODUCT_NAME, + esc_url( \edac_link_wrapper( 'https://my.equalizedigital.com/', 'connected-services', 'renew', false ) ), - Connector::PRODUCT_NAME, Connector::PRODUCT_NAME );
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
composer.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
admin/AdminPage/ConnectedServicesPage.phpincludes/classes/MyDot/Connector.phpincludes/classes/class-rest-api.php
🧰 Additional context used
📓 Path-based instructions (5)
**/*.php
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.php: Follow WordPress Coding Standards (WPCS) in all PHP files
Class names use CamelCase (ClassNameConvention) for new classes
Use edac_ prefix for all custom action/filter hook names
Ensure PHP 7.4+ compatibility
Use type hints where appropriate (parameters, return types, properties)
Sanitize inputs, validate data, and escape outputs following WordPress security best practices; use nonces for forms/AJAX
Use the WordPress database API ($wpdb) for all database operations
Prefix functions and classes in the global namespace with edac_
Use WordPress transients for caching temporary data where appropriate
All user-facing text in PHP must be translatable using the accessibility-checker text domain
Use PHPDoc for all public classes, methods, and properties
Document all custom hooks (actions/filters) with docblocks including parameters and types
Files:
includes/classes/MyDot/Connector.phpadmin/AdminPage/ConnectedServicesPage.phpincludes/classes/class-rest-api.php
includes/**/*.php
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Place core (non-admin) functionality in the /includes directory
Files:
includes/classes/MyDot/Connector.phpincludes/classes/class-rest-api.php
{admin,includes}/**/[A-Z][A-Za-z0-9]*.php
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
New PHP class files must use CamelCase file names like ClassName.php
Files:
includes/classes/MyDot/Connector.phpadmin/AdminPage/ConnectedServicesPage.php
admin/**/*.php
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Place admin classes and admin-only PHP code in the /admin directory
Files:
admin/AdminPage/ConnectedServicesPage.php
**/class-*.php
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/class-*.php: Legacy PHP class files must use WordPress style naming class-class-name.php
Legacy class names use WordPress underscore style (Class_Name_Convention)
Files:
includes/classes/class-rest-api.php
🧠 Learnings (11)
📓 Common learnings
Learnt from: CR
Repo: equalizedigital/accessibility-checker PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-29T11:15:35.151Z
Learning: Applies to accessibility-checker.php : Implement proper plugin activation and deactivation hooks
📚 Learning: 2025-08-26T11:25:48.236Z
Learnt from: pattonwebz
Repo: equalizedigital/accessibility-checker PR: 1208
File: includes/options-page.php:401-410
Timestamp: 2025-08-26T11:25:48.236Z
Learning: In the Accessibility Checker plugin, the edac_is_pro() function uses a robust detection pattern by checking both EDACP_VERSION (which comes from the pro plugin) and EDAC_KEY_VALID existence before looking at its value. This prevents PHP notices and ensures reliable pro feature detection.
Applied to files:
includes/classes/MyDot/Connector.php
📚 Learning: 2025-08-29T11:15:35.151Z
Learnt from: CR
Repo: equalizedigital/accessibility-checker PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-29T11:15:35.151Z
Learning: Applies to **/*.php : Use edac_ prefix for all custom action/filter hook names
Applied to files:
includes/classes/MyDot/Connector.php
📚 Learning: 2025-07-28T17:02:44.387Z
Learnt from: pattonwebz
Repo: equalizedigital/accessibility-checker PR: 1108
File: admin/class-orphaned-issues-cleanup.php:49-52
Timestamp: 2025-07-28T17:02:44.387Z
Learning: The user pattonwebz prefers cron scheduling patterns where `wp_schedule_event()` is called in init_hooks() with conditional checks (like `wp_next_scheduled()`) rather than only in activation hooks, citing that performance impact is negligible due to caching and this is a familiar pattern for developers.
Applied to files:
includes/classes/MyDot/Connector.php
📚 Learning: 2025-08-29T11:15:35.151Z
Learnt from: CR
Repo: equalizedigital/accessibility-checker PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-29T11:15:35.151Z
Learning: Applies to accessibility-checker.php : Implement proper plugin activation and deactivation hooks
Applied to files:
includes/classes/MyDot/Connector.php
📚 Learning: 2025-08-26T13:24:26.788Z
Learnt from: pattonwebz
Repo: equalizedigital/accessibility-checker PR: 1208
File: includes/options-page.php:76-84
Timestamp: 2025-08-26T13:24:26.788Z
Learning: In WordPress, the admin_menu hook fires during the admin_init process, not after it, so remove_action calls placed inside functions hooked to admin_menu will execute at the appropriate time to affect other admin_init callbacks.
Applied to files:
includes/classes/MyDot/Connector.php
📚 Learning: 2025-08-26T13:24:26.788Z
Learnt from: pattonwebz
Repo: equalizedigital/accessibility-checker PR: 1208
File: includes/options-page.php:76-84
Timestamp: 2025-08-26T13:24:26.788Z
Learning: In WordPress, the admin_menu hook fires during the admin_init process, not after it. This means remove_action calls placed inside functions hooked to admin_menu will execute at the appropriate time to affect other admin_init callbacks. This timing works correctly for backwards compatibility scenarios where you need to remove Pro plugin hooks.
Applied to files:
includes/classes/MyDot/Connector.php
📚 Learning: 2025-08-26T13:24:26.788Z
Learnt from: pattonwebz
Repo: equalizedigital/accessibility-checker PR: 1208
File: includes/options-page.php:76-84
Timestamp: 2025-08-26T13:24:26.788Z
Learning: In WordPress, the admin_menu hook fires during the admin_init process, not after it. This means remove_action calls placed inside functions hooked to admin_menu will execute at the appropriate time to affect other admin_init callbacks. The timing works correctly for backwards compatibility scenarios where you need to remove Pro plugin hooks.
Applied to files:
includes/classes/MyDot/Connector.php
📚 Learning: 2025-08-29T11:15:35.151Z
Learnt from: CR
Repo: equalizedigital/accessibility-checker PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-29T11:15:35.151Z
Learning: Applies to **/*.php : Sanitize inputs, validate data, and escape outputs following WordPress security best practices; use nonces for forms/AJAX
Applied to files:
includes/classes/MyDot/Connector.php
📚 Learning: 2025-08-27T14:11:05.736Z
Learnt from: pattonwebz
Repo: equalizedigital/accessibility-checker PR: 1171
File: includes/helper-functions.php:808-826
Timestamp: 2025-08-27T14:11:05.736Z
Learning: In the Accessibility Checker Pro plugin, the class '\EqualizeDigital\AccessibilityCheckerPro\VirtualContent\PostType\VirtualPageType' was never released, so backward compatibility checks for this class name are not needed in edac_is_virtual_page() function.
Applied to files:
admin/AdminPage/ConnectedServicesPage.php
📚 Learning: 2025-08-29T11:15:35.151Z
Learnt from: CR
Repo: equalizedigital/accessibility-checker PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-29T11:15:35.151Z
Learning: Applies to composer.json : Use PSR-4 autoloading with the EqualizeDigital\AccessibilityChecker namespace
Applied to files:
includes/classes/class-rest-api.php
🧬 Code graph analysis (3)
includes/classes/MyDot/Connector.php (2)
admin/class-admin.php (1)
Admin(20-114)admin/AdminPage/ConnectedServicesPage.php (3)
ConnectedServicesPage(24-353)add_page(54-73)get_error_message(296-352)
admin/AdminPage/ConnectedServicesPage.php (1)
includes/classes/MyDot/Connector.php (1)
Connector(22-837)
includes/classes/class-rest-api.php (1)
includes/classes/MyDot/Connector.php (2)
Connector(22-837)validate_jwt_token_in_request_with_fallback(726-742)
🪛 PHPStan (2.1.33)
includes/classes/MyDot/Connector.php
734-734: Undefined variable: $parts
(variable.undefined)
734-734: Undefined variable: $parts
(variable.undefined)
736-736: Undefined variable: $parts
(variable.undefined)
⏰ 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). (2)
- GitHub Check: Integration Test: PHP 8.2 | WP latest (+ ms)
- GitHub Check: Integration Test: PHP 8.1 | WP latest (+ ms)
🔇 Additional comments (5)
includes/classes/class-rest-api.php (1)
107-113: Permission callback logic is appropriate for JWT-based external access.The fallback pattern (JWT validation → capability check) correctly enables external service access while maintaining WordPress authentication for admin users. However, note that
Connector::validate_jwt_token_in_request_with_fallback()has a critical bug (undefined$partsvariable) that will cause this to always returnfalse. Once that's fixed inConnector.php, this will work as intended.Ensure the scan statistics data exposed via this endpoint doesn't include sensitive information (e.g., user identifiers, emails) since JWT-authenticated requests bypass WordPress user authentication.
admin/AdminPage/ConnectedServicesPage.php (1)
127-260: Well-structured admin page with proper security practices.The implementation correctly:
- Uses
wp_nonce_field()for CSRF protection on all forms- Properly escapes output with
esc_html(),esc_attr(),esc_url()- Uses
wp_kses_post()for links containing HTML- Separates free plugin (license key) vs Pro (JWT-based) flows cleanly
includes/classes/MyDot/Connector.php (3)
627-689: JWT validation implements RFC 8725 best practices correctly.The implementation properly validates:
- Token structure and RS256 algorithm
- Signature using stored public key
- Expiration (
exp), issuer (iss), audience (aud), and not-before (nbf) claimsOne consideration: the
issandaudvalidations only trigger if those claims exist in the token. For maximum security per RFC 8725, ensure your issuer (MyDot API) always includes these claims in issued tokens.
56-73: Well-structured initialization with proper WordPress hook patterns.The
init()method correctly:
- Instantiates the admin page with capability parameter
- Registers settings via
admin_init- Uses
admin_post_*hooks for form handling (correct pattern)- Schedules cron for periodic checks
Based on learnings, the cron scheduling pattern with
wp_next_scheduled()check inadmin_initis an acceptable and familiar pattern for WordPress developers.
374-430: Admin notices won't display due to redirect timing.The
add_action('admin_notices', ...)calls inhandle_site_registration()won't work becausehandle_jwt_register_post()redirects immediately after calling this method. The notices are registered but never rendered because the page redirects before output.Consider using transients to persist notices across the redirect:
🔎 Suggested approach
// Instead of add_action(), store in transient: set_transient( 'edac_admin_notice', [ 'type' => 'success', 'message' => __( 'Site registered successfully...', 'accessibility-checker' ), ], 30 ); // Then in a separate method hooked to admin_notices: public function display_transient_notices() { $notice = get_transient( 'edac_admin_notice' ); if ( $notice ) { printf( '<div class="notice notice-%s is-dismissible"><p>%s</p></div>', esc_attr( $notice['type'] ), esc_html( $notice['message'] ) ); delete_transient( 'edac_admin_notice' ); } }Likely an incorrect or invalid review comment.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @admin/AdminPage/ConnectedServicesPage.php:
- Line 271: The code builds an admin link by concatenating get_bloginfo('url')
with a hard-coded /wp-admin path into $license_url; replace that construction to
use WordPress's admin_url() helper so the URL is correct in multisite and
filtered environments (update the assignment to $license_url in
ConnectedServicesPage:: — or wherever $license_url is set — to call
admin_url('admin.php?page=accessibility_checker_settings&tab=connected-services')
instead of string concatenation).
🧹 Nitpick comments (2)
admin/AdminPage/ConnectedServicesPage.php (2)
222-222: Replace inline styles with CSS classes.Inline
style="margin:0;"attributes are used on the forms. Consider using a CSS class instead for better maintainability and consistency with other styling recommendations.Also applies to: 233-233
301-301: Useedac_link_wrapper()for consistent UTM tracking.The URL at line 301 hardcodes
?utm_source=wpadmininstead of using theedac_link_wrapper()helper function that's consistently used elsewhere in this file (lines 132-135). This creates inconsistent UTM parameter handling.🔎 Proposed fix
-__( 'Your %1$s license has expired. <a href="https://my.equalizedigital.com/?utm_source=wpadmin" target="_blank">Please renew your license</a> to continue accessing additional features and services for %2$s.', 'accessibility-checker' ), +__( 'Your %1$s license has expired. <a href="' . esc_url( \edac_link_wrapper( 'https://my.equalizedigital.com/', 'license-error', 'renew', false ) ) . '" target="_blank">Please renew your license</a> to continue accessing additional features and services for %2$s.', 'accessibility-checker' ),Note: Since this is within a translatable string, you may need to refactor this to construct the link outside the translation string and use
sprintf()with a placeholder for better maintainability.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
admin/AdminPage/ConnectedServicesPage.php
🧰 Additional context used
📓 Path-based instructions (3)
**/*.php
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.php: Follow WordPress Coding Standards (WPCS) in all PHP files
Class names use CamelCase (ClassNameConvention) for new classes
Use edac_ prefix for all custom action/filter hook names
Ensure PHP 7.4+ compatibility
Use type hints where appropriate (parameters, return types, properties)
Sanitize inputs, validate data, and escape outputs following WordPress security best practices; use nonces for forms/AJAX
Use the WordPress database API ($wpdb) for all database operations
Prefix functions and classes in the global namespace with edac_
Use WordPress transients for caching temporary data where appropriate
All user-facing text in PHP must be translatable using the accessibility-checker text domain
Use PHPDoc for all public classes, methods, and properties
Document all custom hooks (actions/filters) with docblocks including parameters and types
Files:
admin/AdminPage/ConnectedServicesPage.php
admin/**/*.php
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Place admin classes and admin-only PHP code in the /admin directory
Files:
admin/AdminPage/ConnectedServicesPage.php
{admin,includes}/**/[A-Z][A-Za-z0-9]*.php
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
New PHP class files must use CamelCase file names like ClassName.php
Files:
admin/AdminPage/ConnectedServicesPage.php
🧠 Learnings (1)
📚 Learning: 2025-08-27T14:11:05.736Z
Learnt from: pattonwebz
Repo: equalizedigital/accessibility-checker PR: 1171
File: includes/helper-functions.php:808-826
Timestamp: 2025-08-27T14:11:05.736Z
Learning: In the Accessibility Checker Pro plugin, the class '\EqualizeDigital\AccessibilityCheckerPro\VirtualContent\PostType\VirtualPageType' was never released, so backward compatibility checks for this class name are not needed in edac_is_virtual_page() function.
Applied to files:
admin/AdminPage/ConnectedServicesPage.php
🧬 Code graph analysis (1)
admin/AdminPage/ConnectedServicesPage.php (1)
includes/classes/MyDot/Connector.php (1)
Connector(22-837)
⏰ 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). (1)
- GitHub Check: Integration Test: PHP 8.2 | WP latest (+ ms)
🔇 Additional comments (4)
admin/AdminPage/ConnectedServicesPage.php (4)
46-73: LGTM!Hook registration follows WordPress conventions and uses the proper
edac_prefix for custom hooks. The priority of 1001 forin_admin_headerensures notices display correctly.
128-137: LGTM!Option retrieval and link construction are secure. The use of
\edac_link_wrapper()for all external links ensures consistent UTM parameter tracking across the UI.
249-260: LGTM!Terms and privacy links are properly constructed with UTM tracking and correctly escaped. The i18n implementation with translator comments is appropriate.
22-22: Update @SInCE version tags to match the file header.Multiple docblocks throughout the class use
@since 1.xx.xas a placeholder, but the file header at line 10 specifies@since 1.34.0. Update all@sincetags to use the consistent version number1.34.0.Also applies to: 29-29, 38-38, 50-50, 78-78, 97-97, 112-112, 123-123, 265-265, 289-289
Likely an incorrect or invalid review comment.
…ndle-licence' of github.com:equalizedigital/accessibility-checker into william/pro-286-update-the-free-plugin-to-be-able-to-handle-licence
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
includes/classes/MyDot/Connector.php (1)
665-682: Consider requiringissandaudclaims rather than optional validation.RFC 8725 recommends that if the issuer issues JWTs for multiple relying parties, the
audclaim must be validated. Currently, these claims are only validated if present. If the MyDot issuer always includes these claims, consider making them required to strengthen security:// RFC 8725: Validate issuer claim to prevent token substitution attacks. - if ( isset( $payload['iss'] ) ) { - $expected_iss = self::get_jwt_issuer(); - if ( $payload['iss'] !== $expected_iss ) { - return false; - } + if ( ! isset( $payload['iss'] ) ) { + return false; // iss claim is required. + } + $expected_iss = self::get_jwt_issuer(); + if ( $payload['iss'] !== $expected_iss ) { + return false; }This prevents accepting tokens without issuer/audience claims, which could be security-relevant if the API is expected to always include them.
Please verify whether the MyDot API always includes
issandaudclaims in issued JWTs to determine if they should be required.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
includes/classes/MyDot/Connector.php
🧰 Additional context used
📓 Path-based instructions (3)
**/*.php
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.php: Follow WordPress Coding Standards (WPCS) in all PHP files
Class names use CamelCase (ClassNameConvention) for new classes
Use edac_ prefix for all custom action/filter hook names
Ensure PHP 7.4+ compatibility
Use type hints where appropriate (parameters, return types, properties)
Sanitize inputs, validate data, and escape outputs following WordPress security best practices; use nonces for forms/AJAX
Use the WordPress database API ($wpdb) for all database operations
Prefix functions and classes in the global namespace with edac_
Use WordPress transients for caching temporary data where appropriate
All user-facing text in PHP must be translatable using the accessibility-checker text domain
Use PHPDoc for all public classes, methods, and properties
Document all custom hooks (actions/filters) with docblocks including parameters and types
Files:
includes/classes/MyDot/Connector.php
includes/**/*.php
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Place core (non-admin) functionality in the /includes directory
Files:
includes/classes/MyDot/Connector.php
{admin,includes}/**/[A-Z][A-Za-z0-9]*.php
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
New PHP class files must use CamelCase file names like ClassName.php
Files:
includes/classes/MyDot/Connector.php
🧠 Learnings (8)
📚 Learning: 2025-08-26T11:25:48.236Z
Learnt from: pattonwebz
Repo: equalizedigital/accessibility-checker PR: 1208
File: includes/options-page.php:401-410
Timestamp: 2025-08-26T11:25:48.236Z
Learning: In the Accessibility Checker plugin, the edac_is_pro() function uses a robust detection pattern by checking both EDACP_VERSION (which comes from the pro plugin) and EDAC_KEY_VALID existence before looking at its value. This prevents PHP notices and ensures reliable pro feature detection.
Applied to files:
includes/classes/MyDot/Connector.php
📚 Learning: 2025-08-29T11:15:35.151Z
Learnt from: CR
Repo: equalizedigital/accessibility-checker PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-29T11:15:35.151Z
Learning: Applies to **/*.php : Use edac_ prefix for all custom action/filter hook names
Applied to files:
includes/classes/MyDot/Connector.php
📚 Learning: 2025-07-28T17:02:44.387Z
Learnt from: pattonwebz
Repo: equalizedigital/accessibility-checker PR: 1108
File: admin/class-orphaned-issues-cleanup.php:49-52
Timestamp: 2025-07-28T17:02:44.387Z
Learning: The user pattonwebz prefers cron scheduling patterns where `wp_schedule_event()` is called in init_hooks() with conditional checks (like `wp_next_scheduled()`) rather than only in activation hooks, citing that performance impact is negligible due to caching and this is a familiar pattern for developers.
Applied to files:
includes/classes/MyDot/Connector.php
📚 Learning: 2025-08-29T11:15:35.151Z
Learnt from: CR
Repo: equalizedigital/accessibility-checker PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-29T11:15:35.151Z
Learning: Applies to accessibility-checker.php : Implement proper plugin activation and deactivation hooks
Applied to files:
includes/classes/MyDot/Connector.php
📚 Learning: 2025-08-26T13:24:26.788Z
Learnt from: pattonwebz
Repo: equalizedigital/accessibility-checker PR: 1208
File: includes/options-page.php:76-84
Timestamp: 2025-08-26T13:24:26.788Z
Learning: In WordPress, the admin_menu hook fires during the admin_init process, not after it, so remove_action calls placed inside functions hooked to admin_menu will execute at the appropriate time to affect other admin_init callbacks.
Applied to files:
includes/classes/MyDot/Connector.php
📚 Learning: 2025-08-26T13:24:26.788Z
Learnt from: pattonwebz
Repo: equalizedigital/accessibility-checker PR: 1208
File: includes/options-page.php:76-84
Timestamp: 2025-08-26T13:24:26.788Z
Learning: In WordPress, the admin_menu hook fires during the admin_init process, not after it. This means remove_action calls placed inside functions hooked to admin_menu will execute at the appropriate time to affect other admin_init callbacks. This timing works correctly for backwards compatibility scenarios where you need to remove Pro plugin hooks.
Applied to files:
includes/classes/MyDot/Connector.php
📚 Learning: 2025-08-26T13:24:26.788Z
Learnt from: pattonwebz
Repo: equalizedigital/accessibility-checker PR: 1208
File: includes/options-page.php:76-84
Timestamp: 2025-08-26T13:24:26.788Z
Learning: In WordPress, the admin_menu hook fires during the admin_init process, not after it. This means remove_action calls placed inside functions hooked to admin_menu will execute at the appropriate time to affect other admin_init callbacks. The timing works correctly for backwards compatibility scenarios where you need to remove Pro plugin hooks.
Applied to files:
includes/classes/MyDot/Connector.php
📚 Learning: 2025-08-29T11:15:35.151Z
Learnt from: CR
Repo: equalizedigital/accessibility-checker PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-29T11:15:35.151Z
Learning: Applies to **/*.php : Sanitize inputs, validate data, and escape outputs following WordPress security best practices; use nonces for forms/AJAX
Applied to files:
includes/classes/MyDot/Connector.php
🧬 Code graph analysis (1)
includes/classes/MyDot/Connector.php (1)
admin/AdminPage/ConnectedServicesPage.php (3)
ConnectedServicesPage(24-353)add_page(54-73)get_error_message(296-352)
🔇 Additional comments (12)
includes/classes/MyDot/Connector.php (12)
1-50: Acknowledged: Development API endpoint will be updated before release.The constants and class structure look appropriate. The
API_ENDPOINTusing a local development URL has been acknowledged by the author as intentional for now.
82-109: LGTM!Settings registration follows WordPress best practices with proper type definitions and sanitization callbacks.
118-143: LGTM!The handler properly implements security best practices: capability check, nonce verification via
check_admin_referer(), input sanitization, and safe redirect.
152-199: LGTM!License activation flow handles errors gracefully and automatically triggers site registration on successful activation, which is good UX.
208-247: LGTM!Deactivation properly unregisters the site before deactivating the license and performs complete cleanup of stored options on success.
332-365: LGTM!Both JWT registration handlers implement proper security with capability checks, nonce verification, and safe redirects.
489-584: LGTM!Both
register_site()andunregister_site()are well-implemented API wrappers with proper parameter validation, error handling, and consistent request configuration.
593-608: LGTM!JWT issuer and audience getters are well-designed with filters for customization and protocol-agnostic comparison.
702-716: LGTM!The fallback pattern is well-designed - try validation, refresh key if needed, retry. This handles key rotation gracefully without user intervention.
726-743: LGTM!The
$partsvariable issue from previous reviews has been fixed. The Authorization header parsing and Bearer token extraction are correctly implemented.
757-812: Inconsistent API endpoints and response keys for public key retrieval.Two methods fetch the public key but use different endpoints and expect different response keys:
Method Endpoint Response Key verify_and_update_public_key()/public-keyjwt_public_keyrefresh_public_key_from_issuer()/get-public-keypublic_keyThis inconsistency could cause issues if one endpoint returns a different format or if the API consolidates these endpoints later.
Please verify whether both endpoints are intentional and if the response key difference (
jwt_public_keyvspublic_key) is correct. If they serve the same purpose, consider consolidating to a single endpoint/key name.
823-837: LGTM!The
safe_remote_get()wrapper properly handles VIP environment compatibility with appropriate fallback for non-VIP hosts.
…plugin-to-be-able-to-handle-licence' into william/pro-286-update-the-free-plugin-to-be-able-to-handle-licence
…ext collection date
When the Pro license recovers to valid status during periodic checks, the edacp_license_error option should be cleared to remove persistent error notices (expired, disabled, etc.) from the admin UI. This ensures both free and Pro license errors are cleared consistently when licenses recover to valid status. Addresses PR #491 review comment regarding missing error cleanup.
|
✅ Accessibility Checker build (primary only)
|
Adds an optional connection that users can establish if they enter a license key to connect their site to myDot for puposes of stats sharing.
Checklist
Summary by CodeRabbit
New Features
Chores