Be more explicit with who can save and clear issues for a given post - #1202
Be more explicit with who can save and clear issues for a given post#1202pattonwebz wants to merge 13 commits into
Conversation
…ighter This is used alongside the other logged in check to determine if we show a button to rescan or clear issues to a given user
… the post in the id passed through request
…sues or clearing issues
…sults and issue clearing
|
Note Other AI code review bot(s) detectedCodeRabbit 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. WalkthroughAdds a frontend localization flag Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Browser as User (Browser)
participant FE as Frontend Highlighter
participant WP as WP REST Router
participant API as REST_Api
participant Cap as current_user_can
rect rgba(230,240,255,0.6)
note right of FE: Uses localized edacFrontendHighlighterApp.userCanEdit & loggedIn
Browser->>FE: Load page
FE->>FE: Read edacFrontendHighlighterApp.userCanEdit & loggedIn
alt userCanEdit && loggedIn
FE->>Browser: Show "Clear Issues" / "Rescan"
else
FE->>Browser: Hide controls
end
end
rect rgba(240,255,230,0.6)
note right of WP: REST request flow with per-post permission check
Browser->>WP: POST /post-scan-results/{id}
WP->>API: permission_callback($request)
API->>API: Ensure id present (400 if missing)
API->>Cap: current_user_can('edit_post', id)
Cap-->>API: true / false
API-->>WP: allow / deny
alt Allowed
WP-->>Browser: 200 OK
else Denied
WP-->>Browser: 403 or 400
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @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 refines the permission system for accessibility scan features, ensuring that only users with explicit editing rights for a specific post can save scan results or clear issues. It updates both backend REST API endpoints and frontend UI elements to enforce more granular access control, enhancing security and data integrity.
Highlights
- Enhanced REST API Permissions: Replaced broad 'edit_posts' capability checks with a new, specific 'user_can_edit_passed_post_id' method for accessibility scan-related REST API endpoints, ensuring users can only modify posts they are authorized to edit.
- Dynamic Frontend UI Control: Updated the frontend highlighter panel to conditionally display 'Clear Issues' and 'Rescan This Page' buttons, making them visible only to users who are logged in and possess the specific capability to edit the current post.
- Comprehensive Permission Testing: Introduced a new PHPUnit test class, 'RestApiEndpointsTest', to rigorously verify that the updated REST API permission checks function correctly for both administrative and limited users, preventing unauthorized actions.
Using Gemini Code Assist
The 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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.
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
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request does a great job of improving permission handling for the REST API endpoints and frontend UI. The change to use a more granular permission check for editing specific posts is a solid improvement for security and correctness. The new tests are comprehensive and verify the behavior correctly.
I have one suggestion for improvement in class-rest-api.php regarding the implementation of the new permission callback function. The current implementation returns a WP_REST_Response object on failure, which is not standard for permission callbacks. It should return a WP_Error object to be more robust and compliant with WordPress REST API standards. Other than that, the changes look excellent.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/frontendHighlighterApp/index.js (1)
397-405: Simplify boolean gating expression (minor)Optional chaining already guards access. Coerce to a strict boolean to avoid accidental truthy values.
Apply:
-const userCanEdit = edacFrontendHighlighterApp && edacFrontendHighlighterApp?.userCanEdit && edacFrontendHighlighterApp?.loggedIn; +const userCanEdit = Boolean( edacFrontendHighlighterApp?.userCanEdit && edacFrontendHighlighterApp?.loggedIn );tests/phpunit/includes/classes/RestApiEndpointsTest.php (1)
138-162: Add a positive case for limited users editing their own posts (optional)To fully prove per-post semantics, add a test where the limited user creates a post and is allowed to save/clear for that post.
Example:
public function test_limited_user_can_manage_own_post() { wp_set_current_user( self::$limited_id ); $own_post_id = self::factory()->post->create( [ 'post_type' => 'post', 'post_status' => 'publish', 'post_author' => self::$limited_id, ] ); // Save scan results. $req1 = new WP_REST_Request( 'POST', '/accessibility-checker/v1/post-scan-results/' . $own_post_id ); $req1->set_param( 'id', $own_post_id ); $req1->set_param( 'violations', [ [ 'ruleId' => 'image-alt', 'html' => '<img>' ] ] ); $resp1 = $this->server->dispatch( $req1 ); $this->assertSame( 200, $resp1->get_status() ); // Clear issues. $req2 = new WP_REST_Request( 'POST', '/accessibility-checker/v1/clear-issues/' . $own_post_id ); $req2->set_param( 'id', $own_post_id ); $req2->set_body( wp_json_encode( [ 'flush' => true ] ) ); $req2->set_header( 'Content-Type', 'application/json' ); $resp2 = $this->server->dispatch( $req2 ); $this->assertSame( 200, $resp2->get_status() ); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
includes/classes/class-enqueue-frontend.php(1 hunks)includes/classes/class-rest-api.php(3 hunks)src/frontendHighlighterApp/index.js(2 hunks)tests/phpunit/includes/classes/RestApiEndpointsTest.php(1 hunks)
🧰 Additional context used
📓 Path-based instructions (10)
**/*.php
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
**/*.php: Follow WordPress coding standards (WPCS) in all PHP files
Use PSR-4 autoloading with EqualizeDigital\AccessibilityChecker namespace for new classes
Use WordPress hooks and filters appropriately
Use edac_ prefix for custom hooks and filters
Minimum PHP 7.4 compatibility
Use type hints where appropriate in PHP code
Follow WordPress security best practices (sanitization, validation, nonces)
Use WordPress database API (wpdb) for database operations
Prefix all functions and classes with edac_ when in global namespace
Use WordPress hooks (actions/filters) for extensibility
Use WordPress transients for caching
Sanitize all user inputs
Validate and escape all outputs
Use WordPress nonces for form submissions
Implement proper capability checks
Provide appropriate hooks for extensibility when adding new functionality
Use descriptive hook names with edac_ prefix
Document all custom hooks in docblocks
Document custom hooks and filters with clear descriptions and parameter types
Use PHPDoc for all public classes, methods, and properties
Escape all output, especially in admin screens and user-generated content
Use WordPress error handling functions (e.g., WP_Error) for PHP errors
Files:
includes/classes/class-enqueue-frontend.phptests/phpunit/includes/classes/RestApiEndpointsTest.phpincludes/classes/class-rest-api.php
**/class-*.php
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
**/class-*.php: Legacy files are autoloaded using EDAC namespace
Legacy class names use WordPress style (Class_Name_Convention)
Legacy file names use class-class-name.php (WordPress style)
Legacy Classes: class-class-name.php
Deprecate legacy code with clear docblocks and migration notes
Files:
includes/classes/class-enqueue-frontend.phpincludes/classes/class-rest-api.php
**/*.{php,js}
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
**/*.{php,js}: Follow WordPress internationalization (i18n) practices and use accessibility-checker text domain
All user-facing text must be translatable
Add inline comments for complex accessibility logic or non-obvious code
Use semantic HTML structure
ARIA attributes used correctly
Keyboard navigation supported
Images have descriptive alt text
Heading hierarchy logical
Screen reader compatibility
Forms are accessible and labeled
Files:
includes/classes/class-enqueue-frontend.phptests/phpunit/includes/classes/RestApiEndpointsTest.phpsrc/frontendHighlighterApp/index.jsincludes/classes/class-rest-api.php
includes/**/*.php
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
includes/**/*.php: Validate all AJAX requests and REST endpoints with nonces and capability checks
Avoid blocking queries in PHP, especially during scans
Files:
includes/classes/class-enqueue-frontend.phpincludes/classes/class-rest-api.php
**/[A-Z]*.php
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
**/[A-Z]*.php: Class names use CamelCase (ClassNameConvention) for new classes
File names use ClassNameConvention.php for new classes (CamelCase)
Classes: ClassName.php
Files:
tests/phpunit/includes/classes/RestApiEndpointsTest.php
tests/phpunit/**/*.php
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
tests/phpunit/**/*.php: Write unit tests for new PHP functions and classes
Add integration tests for major features and accessibility rules
Files:
tests/phpunit/includes/classes/RestApiEndpointsTest.php
**/*.js
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
**/*.js: Strings in JavaScript also need translation support using wp.i18n functions
Gracefully handle JavaScript errors to avoid breaking accessibility features
Files:
src/frontendHighlighterApp/index.js
**/*.{css,js}
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
Support RTL languages where applicable
Files:
src/frontendHighlighterApp/index.js
src/**/*.js
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
src/**/*.js: Focus management implemented
Minimize DOM operations in JavaScript for frontend scanning
Files:
src/frontendHighlighterApp/index.js
**/[a-z]*.js
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
JavaScript Utilities: utilityName.js
Files:
src/frontendHighlighterApp/index.js
🧠 Learnings (1)
📚 Learning: 2025-08-04T16:46:23.515Z
Learnt from: CR
PR: equalizedigital/accessibility-checker#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-04T16:46:23.515Z
Learning: Applies to tests/phpunit/**/*.php : Add integration tests for major features and accessibility rules
Applied to files:
tests/phpunit/includes/classes/RestApiEndpointsTest.php
🧬 Code Graph Analysis (2)
tests/phpunit/includes/classes/RestApiEndpointsTest.php (2)
admin/class-admin.php (1)
Admin(20-99)admin/class-update-database.php (2)
Update_Database(16-82)edac_update_database(36-81)
src/frontendHighlighterApp/index.js (1)
src/editorApp/index.js (1)
rescanButton(83-83)
🔇 Additional comments (5)
includes/classes/class-enqueue-frontend.php (1)
104-104: LGTM: userCanEdit is correctly derived from per-post capabilityUsing current_user_can('edit_post', $post_id) aligns frontend gating with REST enforcement. No issues spotted.
src/frontendHighlighterApp/index.js (1)
419-419: UI container class logic reads cleaner after gating changeSwitching to !userCanEdit for single_button styling matches the new permission concept. Looks good.
includes/classes/class-rest-api.php (2)
87-89: Correctly passes request to permission_callbackHanding the WP_REST_Request into the permission check enables per-post evaluation. Nice.
179-181: Correctly reuses permission helper for clear-issuesPermission callback parity between endpoints is good; keeps logic centralized.
tests/phpunit/includes/classes/RestApiEndpointsTest.php (1)
101-131: Good coverage: validates per-post permissions for saving scan resultsThe admin/limited user scenarios accurately exercise the new permission_callback behavior on POST /post-scan-results/{id}.
This test verifies that a limited user can save scan results and clear issues for their own post. [PRO-165]
Enhance the API by ensuring that the 'id' argument is required and properly sanitized.
There was a problem hiding this comment.
Pull Request Overview
This PR enhances permission handling for REST API endpoints to ensure only users who can edit specific posts are allowed to save scan results or clear issues. The implementation moves from generic edit_posts capability checks to post-specific edit_post checks.
Key changes:
- Replaced generic permission callbacks with a new
user_can_edit_passed_post_idmethod that validates edit permissions for the specific post ID - Updated frontend UI to show action buttons only to users who can edit the current post
- Added comprehensive test coverage for the new permission logic
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| includes/classes/class-rest-api.php | Implements post-specific permission checks and adds new validation method |
| includes/classes/class-enqueue-frontend.php | Adds userCanEdit property to frontend app data |
| src/frontendHighlighterApp/index.js | Updates UI logic to respect post-specific edit permissions |
| tests/phpunit/includes/classes/RestApiEndpointsTest.php | Adds comprehensive test coverage for permission enforcement |
| 'validate_callback' => function ( $param ) { | ||
| return is_numeric( $param ); | ||
| }, | ||
| return is_numeric( $param ); }, |
There was a problem hiding this comment.
The validate callback function should be properly formatted on separate lines for better readability and consistency with WordPress coding standards.
| 'validate_callback' => function ( $param ) { | ||
| return is_numeric( $param ); | ||
| }, | ||
| return is_numeric( $param ); }, |
There was a problem hiding this comment.
The validate callback function should be properly formatted on separate lines for better readability and consistency with WordPress coding standards.
| 'post_type' => 'post', | ||
| 'post_status' => 'draft', | ||
| 'post_author' => self::$limited_id, | ||
| ] |
There was a problem hiding this comment.
Trailing whitespace after the closing bracket should be removed.
| ] | |
| ] |
| 'ruleId' => 'image-alt', | ||
| 'html' => '<img>', | ||
| ], | ||
| ] |
There was a problem hiding this comment.
Trailing whitespace after the closing bracket should be removed.
| ] | |
| ] |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
includes/classes/class-rest-api.php (1)
208-225: Type-hint the request and use a more standard error code for missing params.PHPDoc is correct, but the method parameter isn’t type-hinted. Also, WordPress commonly uses
rest_missing_callback_paramfor missing required params.- public function user_can_edit_passed_post_id( $request ) { - if ( ! isset( $request['id'] ) ) { - return new \WP_Error( 'rest_post_invalid_id', __( 'A required parameter is missing.', 'accessibility-checker' ), [ 'status' => 400 ] ); - } + public function user_can_edit_passed_post_id( \WP_REST_Request $request ) { + if ( ! isset( $request['id'] ) ) { + return new \WP_Error( + 'rest_missing_callback_param', + __( 'A required parameter is missing: id.', 'accessibility-checker' ), + [ 'status' => 400 ] + ); + } $post_id = (int) $request['id']; return current_user_can( 'edit_post', $post_id ); // able to edit the post. }
🧹 Nitpick comments (8)
includes/classes/class-rest-api.php (5)
82-86: Nit: simplify validate callback (and it's redundant with the regex route param).Given the route already uses
(?P<id>\d+),validate_callbackis redundant. If you keep it, prefer a simple callable for readability.Apply:
'args' => [ 'id' => [ 'required' => true, - 'validate_callback' => function ( $param ) { - return is_numeric( $param ); }, + 'validate_callback' => 'is_numeric', 'sanitize_callback' => 'absint', ], ],
88-90: Use method reference for permission callback to reduce closure noise.Directly referencing the method is cleaner and avoids an unnecessary closure.
- 'permission_callback' => function ( $request ) { - return $this->user_can_edit_passed_post_id( $request ); - }, + 'permission_callback' => [ $this, 'user_can_edit_passed_post_id' ],
175-179: Nit: duplicateidarg schema — consider a small helper.The schema for
idis duplicated across endpoints. Extracting it into a helper improves maintainability.If you keep the inline schema, same readability tweak applies as above:
'args' => [ 'id' => [ 'required' => true, - 'validate_callback' => function ( $param ) { - return is_numeric( $param ); }, + 'validate_callback' => 'is_numeric', 'sanitize_callback' => 'absint', ], ],Optionally, add this helper to the class (outside the shown range):
private function id_arg_schema(): array { return [ 'required' => true, 'validate_callback' => 'is_numeric', 'sanitize_callback' => 'absint', ]; }Then use it:
'args' => [ 'id' => $this->id_arg_schema() ],
181-183: Use method reference for permission callback (same as other endpoint).Keep it consistent and concise.
- 'permission_callback' => function ( $request ) { - return $this->user_can_edit_passed_post_id( $request ); - }, + 'permission_callback' => [ $this, 'user_can_edit_passed_post_id' ],
230-236: Type-hint the request param for consistency with other handlers.Other handlers (e.g., get_site_summary) type-hint
\WP_REST_Request. Mirror that here.- public function clear_issues_for_post( $request ) { + public function clear_issues_for_post( \WP_REST_Request $request ) {tests/phpunit/includes/classes/RestApiEndpointsTest.php (3)
101-131: Strengthen assertions to validate response payload.You already assert status; also assert payload structure/values to catch regressions.
$response = $this->server->dispatch( $request ); $this->assertSame( 200, $response->get_status(), 'Admin should be allowed to save scan results.' ); $data = $response->get_data(); $this->assertIsArray( $data ); + $this->assertArrayHasKey( 'success', $data ); + $this->assertTrue( $data['success'] ); + $this->assertArrayHasKey( 'id', $data ); + $this->assertSame( self::$post_id, $data['id'] );
138-162: Add assertions for response body fields on success and keep a small check for 403 payload.Validating
idandflushedhelps ensure the endpoint semantics don’t drift.$resp1 = $this->server->dispatch( $r1 ); $this->assertSame( 200, $resp1->get_status(), 'Admin should be allowed to clear issues.' ); $body1 = $resp1->get_data(); $this->assertIsArray( $body1 ); $this->assertArrayHasKey( 'success', $body1 ); $this->assertTrue( $body1['success'] ); + $this->assertArrayHasKey( 'id', $body1 ); + $this->assertSame( self::$post_id, $body1['id'] ); + $this->assertArrayHasKey( 'flushed', $body1 ); + $this->assertTrue( $body1['flushed'] );
169-201: Optionally assert payload structure for “own post” success paths.Minor, but helps ensure both endpoints return the expected success envelope for non-admins acting on their own content.
$resp1 = $this->server->dispatch( $req1 ); $this->assertSame( 200, $resp1->get_status() ); + $data1 = $resp1->get_data(); + $this->assertIsArray( $data1 ); + $this->assertArrayHasKey( 'success', $data1 ); + $this->assertTrue( $data1['success'] ); // Clear issues. $req2 = new WP_REST_Request( 'POST', '/accessibility-checker/v1/clear-issues/' . $own_post_id ); $req2->set_param( 'id', $own_post_id ); $req2->set_body( wp_json_encode( [ 'flush' => true ] ) ); $req2->set_header( 'Content-Type', 'application/json' ); $resp2 = $this->server->dispatch( $req2 ); $this->assertSame( 200, $resp2->get_status() ); + $data2 = $resp2->get_data(); + $this->assertIsArray( $data2 ); + $this->assertArrayHasKey( 'success', $data2 ); + $this->assertTrue( $data2['success'] );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
includes/classes/class-rest-api.php(3 hunks)tests/phpunit/includes/classes/RestApiEndpointsTest.php(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.php
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
**/*.php: Follow WordPress coding standards (WPCS) in all PHP files
Use PSR-4 autoloading with EqualizeDigital\AccessibilityChecker namespace for new classes
Use WordPress hooks and filters appropriately
Use edac_ prefix for custom hooks and filters
Minimum PHP 7.4 compatibility
Use type hints where appropriate in PHP code
Follow WordPress security best practices (sanitization, validation, nonces)
Use WordPress database API (wpdb) for database operations
Prefix all functions and classes with edac_ when in global namespace
Use WordPress hooks (actions/filters) for extensibility
Use WordPress transients for caching
Sanitize all user inputs
Validate and escape all outputs
Use WordPress nonces for form submissions
Implement proper capability checks
Provide appropriate hooks for extensibility when adding new functionality
Use descriptive hook names with edac_ prefix
Document all custom hooks in docblocks
Document custom hooks and filters with clear descriptions and parameter types
Use PHPDoc for all public classes, methods, and properties
Escape all output, especially in admin screens and user-generated content
Use WordPress error handling functions (e.g., WP_Error) for PHP errors
Files:
includes/classes/class-rest-api.phptests/phpunit/includes/classes/RestApiEndpointsTest.php
**/class-*.php
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
**/class-*.php: Legacy files are autoloaded using EDAC namespace
Legacy class names use WordPress style (Class_Name_Convention)
Legacy file names use class-class-name.php (WordPress style)
Legacy Classes: class-class-name.php
Deprecate legacy code with clear docblocks and migration notes
Files:
includes/classes/class-rest-api.php
**/*.{php,js}
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
**/*.{php,js}: Follow WordPress internationalization (i18n) practices and use accessibility-checker text domain
All user-facing text must be translatable
Add inline comments for complex accessibility logic or non-obvious code
Use semantic HTML structure
ARIA attributes used correctly
Keyboard navigation supported
Images have descriptive alt text
Heading hierarchy logical
Screen reader compatibility
Forms are accessible and labeled
Files:
includes/classes/class-rest-api.phptests/phpunit/includes/classes/RestApiEndpointsTest.php
includes/**/*.php
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
includes/**/*.php: Validate all AJAX requests and REST endpoints with nonces and capability checks
Avoid blocking queries in PHP, especially during scans
Files:
includes/classes/class-rest-api.php
**/[A-Z]*.php
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
**/[A-Z]*.php: Class names use CamelCase (ClassNameConvention) for new classes
File names use ClassNameConvention.php for new classes (CamelCase)
Classes: ClassName.php
Files:
tests/phpunit/includes/classes/RestApiEndpointsTest.php
tests/phpunit/**/*.php
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
tests/phpunit/**/*.php: Write unit tests for new PHP functions and classes
Add integration tests for major features and accessibility rules
Files:
tests/phpunit/includes/classes/RestApiEndpointsTest.php
🧠 Learnings (1)
📚 Learning: 2025-08-04T16:46:23.515Z
Learnt from: CR
PR: equalizedigital/accessibility-checker#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-04T16:46:23.515Z
Learning: Applies to tests/phpunit/**/*.php : Add integration tests for major features and accessibility rules
Applied to files:
tests/phpunit/includes/classes/RestApiEndpointsTest.php
🧬 Code Graph Analysis (1)
tests/phpunit/includes/classes/RestApiEndpointsTest.php (1)
admin/class-update-database.php (2)
Update_Database(16-82)edac_update_database(36-81)
⏰ 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). (5)
- GitHub Check: Integration Test: PHP 8.2 | WP latest (+ ms)
- GitHub Check: Integration Test: PHP 8.1 | WP latest
- GitHub Check: Integration Test: PHP 8.1 | WP latest (+ ms)
- GitHub Check: Integration Test: PHP 8.1 | WP 6.2
- GitHub Check: Integration Test: PHP 8.2 | WP latest (+ ms)
🔇 Additional comments (3)
tests/phpunit/includes/classes/RestApiEndpointsTest.php (3)
47-53: LGTM: route init and server bootstrap are handled correctly.Triggering
initandrest_api_initthen cachingrest_get_server()makes the tests deterministic across runs.
72-78: Ensure the Update_Database class is autoloaded in the test environment.If the plugin’s autoloader isn’t active in the test bootstrap, this direct instantiation could fatal. Confirm the test bootstrap loads the plugin (and autoloader) before this runs.
If needed, add a guard:
if ( class_exists( '\EDAC\Admin\Update_Database' ) ) { ( new \EDAC\Admin\Update_Database() )->edac_update_database(); }
13-20: Nice coverage: admin vs limited user vs own-post.The matrix of permission scenarios is well covered and aligns with the new per-post capability checks.
Ensure consistent code style by aligning closing braces for validation callbacks in the REST API.
Added assertions to check for 'success', 'id', and 'flushed' keys in the API response data for various test cases.
…ccessing scan stats
…accessing scan stats
…accessing scan stats
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
includes/classes/class-rest-api.php (4)
80-91: Per-post permission callback and arg schema: good; consider validating ID > 0Nice move to gate
/post-scan-results/{id}on a per-post capability and to add arg schema. Minor nit: validate that the ID is a positive integer to avoid0(which won’t map to a real post). Note that in WP REST,permission_callbackruns before arg validation/sanitization, so your explicit(int)cast inside the callback is still the source of truth.Apply:
'args' => [ 'id' => [ - 'required' => true, - 'validate_callback' => function ( $param ) { - return is_numeric( $param ); - }, + 'required' => true, + 'validate_callback' => function ( $param ) { + return is_numeric( $param ) && (int) $param > 0; + }, 'sanitize_callback' => 'absint', ], ], 'permission_callback' => function ( $request ) { return $this->user_can_edit_passed_post_id( $request ); },
107-108: Centralize capability for stats endpoints behind a filterAll four stats endpoints are guarded with
edit_posts. That’s reasonable, but sites often want a different cap (e.g.,manage_options) or a custom role. Recommend introducing anedac_filter so integrators can override without code edits. Same filter can be reused across these endpoints.Apply:
'permission_callback' => function () { - return current_user_can( 'edit_posts' ); + $cap = apply_filters( 'edac_required_capability_for_scans_stats', 'edit_posts' ); + return current_user_can( $cap ); },Repeat the same change for:
- scans-stats (Lines 107-108)
- clear-cached-scans-stats (Lines 124-125)
- scans-stats-by-post-type (Lines 141-142)
- scans-stats-by-post-types (Lines 158-159)
Please also add a short PHPDoc for the new filter, following WPCS and your guidelines (text domain accessibility-checker).
Also applies to: 124-125, 141-142, 158-159
174-185: Clear-issues route aligns with per-post capability; also validate ID > 0Good parity with the save route. Same small nit: ensure the ID is positive.
Apply:
'args' => [ 'id' => [ - 'required' => true, - 'validate_callback' => function ( $param ) { - return is_numeric( $param ); - }, + 'required' => true, + 'validate_callback' => function ( $param ) { + return is_numeric( $param ) && (int) $param > 0; + }, 'sanitize_callback' => 'absint', ], ], 'permission_callback' => function ( $request ) { return $this->user_can_edit_passed_post_id( $request ); },
210-227: Permission callback helper: add param type-hint; refine error code/messageGreat fix returning WP_Error instead of WP_REST_Response, and docblock looks good. Two small improvements:
- Add a parameter type-hint for WP_REST_Request (7.4-safe).
- Use the core-style code for missing params:
rest_missing_callback_paramand specify which param is missing.Apply:
-/** - * Check if the user can edit a post. - * - * This is a permission callback to replace several places where we check if the user can edit a post. - * - * @since 1.31.0 - * - * @param \WP_REST_Request $request The request object passed from the REST call. This should contain the 'id' of the post to check permissions for. - * - * @return bool|\WP_Error - */ -public function user_can_edit_passed_post_id( $request ) { - if ( ! isset( $request['id'] ) ) { - return new \WP_Error( 'rest_post_invalid_id', __( 'A required parameter is missing.', 'accessibility-checker' ), [ 'status' => 400 ] ); - } - $post_id = (int) $request['id']; - return current_user_can( 'edit_post', $post_id ); // able to edit the post. -} +/** + * Check if the user can edit a post. + * + * Permission callback used where per-post edit checks are required. + * + * @since 1.31.0 + * + * @param \WP_REST_Request $request The request object. Must contain 'id' of the post to check. + * @return bool|\WP_Error + */ +public function user_can_edit_passed_post_id( \WP_REST_Request $request ) { + if ( ! isset( $request['id'] ) ) { + return new \WP_Error( + 'rest_missing_callback_param', + __( 'A required parameter is missing: id.', 'accessibility-checker' ), + [ 'status' => 400 ] + ); + } + $post_id = (int) $request['id']; + return current_user_can( 'edit_post', $post_id ); +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- 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/class-rest-api.php(7 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.php
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
**/*.php: Follow WordPress coding standards (WPCS) in all PHP files
Use PSR-4 autoloading with EqualizeDigital\AccessibilityChecker namespace for new classes
Use WordPress hooks and filters appropriately
Use edac_ prefix for custom hooks and filters
Minimum PHP 7.4 compatibility
Use type hints where appropriate in PHP code
Follow WordPress security best practices (sanitization, validation, nonces)
Use WordPress database API (wpdb) for database operations
Prefix all functions and classes with edac_ when in global namespace
Use WordPress hooks (actions/filters) for extensibility
Use WordPress transients for caching
Sanitize all user inputs
Validate and escape all outputs
Use WordPress nonces for form submissions
Implement proper capability checks
Provide appropriate hooks for extensibility when adding new functionality
Use descriptive hook names with edac_ prefix
Document all custom hooks in docblocks
Document custom hooks and filters with clear descriptions and parameter types
Use PHPDoc for all public classes, methods, and properties
Escape all output, especially in admin screens and user-generated content
Use WordPress error handling functions (e.g., WP_Error) for PHP errors
Files:
includes/classes/class-rest-api.php
**/class-*.php
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
**/class-*.php: Legacy files are autoloaded using EDAC namespace
Legacy class names use WordPress style (Class_Name_Convention)
Legacy file names use class-class-name.php (WordPress style)
Legacy Classes: class-class-name.php
Deprecate legacy code with clear docblocks and migration notes
Files:
includes/classes/class-rest-api.php
**/*.{php,js}
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
**/*.{php,js}: Follow WordPress internationalization (i18n) practices and use accessibility-checker text domain
All user-facing text must be translatable
Add inline comments for complex accessibility logic or non-obvious code
Use semantic HTML structure
ARIA attributes used correctly
Keyboard navigation supported
Images have descriptive alt text
Heading hierarchy logical
Screen reader compatibility
Forms are accessible and labeled
Files:
includes/classes/class-rest-api.php
includes/**/*.php
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
includes/**/*.php: Validate all AJAX requests and REST endpoints with nonces and capability checks
Avoid blocking queries in PHP, especially during scans
Files:
includes/classes/class-rest-api.php
🔇 Additional comments (1)
includes/classes/class-rest-api.php (1)
232-232: Docblock param type correctedDocblock now uses \WP_REST_Request for the request param. Looks good.
…r clearing cached scan stats
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
admin/class-welcome-page.php (1)
269-273: Consider conditioning the explanatory copy when the button is hidden.For users without publish_posts, the text says they can click “Update Counts” but the button won’t be visible. Either wrap this paragraph in the same capability check or adjust the copy for non-privileged users.
Example:
<?php if ( current_user_can( 'publish_posts' ) ) : ?> <p><?php esc_html_e( 'This summary is automatically updated every 24 hours, or any time a full site scan is completed. You can also manually update these results by clicking the Update Counts button.', 'accessibility-checker' ); ?></p> <?php else : ?> <p><?php esc_html_e( 'This summary is automatically updated every 24 hours, or any time a full site scan is completed.', 'accessibility-checker' ); ?></p> <?php endif; ?>includes/classes/class-rest-api.php (4)
81-91: Minor cleanup: use direct callback and native validator (no closures).Trim some boilerplate for readability and consistency.
Apply:
'args' => [ 'id' => [ - 'required' => true, - 'validate_callback' => function ( $param ) { - return is_numeric( $param ); - }, + 'required' => true, + 'validate_callback' => 'is_numeric', 'sanitize_callback' => 'absint', ], ], - 'permission_callback' => function ( $request ) { - return $this->user_can_edit_passed_post_id( $request ); - }, + 'permission_callback' => [ $this, 'user_can_edit_passed_post_id' ],
138-143: Permission hardening is fine; optionally add explicit args for slug.While you’re touching this route, consider declaring and sanitizing the slug param explicitly for clarity (the path regex already constrains it).
Example:
'args' => [ 'slug' => [ 'required' => true, 'sanitize_callback' => 'sanitize_key', 'validate_callback' => function( $param ) { return (bool) preg_match( '/^[a-zA-Z0-9_-]+$/', $param ); }, ], ],
176-185: Minor cleanup: remove closures as above.Mirror the simplifications suggested for post-scan-results.
Apply:
'args' => [ 'id' => [ - 'required' => true, - 'validate_callback' => function ( $param ) { - return is_numeric( $param ); - }, + 'required' => true, + 'validate_callback' => 'is_numeric', 'sanitize_callback' => 'absint', ], ], - 'permission_callback' => function ( $request ) { - return $this->user_can_edit_passed_post_id( $request ); - }, + 'permission_callback' => [ $this, 'user_can_edit_passed_post_id' ],
210-227: Type-hint the request parameter and prefer a core error code.
- Add a \WP_REST_Request type-hint to the helper (PHP 7.4-safe).
- Use rest_missing_callback_param to align with core error semantics (optional).
Apply:
- public function user_can_edit_passed_post_id( $request ) { - if ( ! isset( $request['id'] ) ) { - return new \WP_Error( 'rest_post_invalid_id', __( 'A required parameter is missing.', 'accessibility-checker' ), [ 'status' => 400 ] ); - } + public function user_can_edit_passed_post_id( \WP_REST_Request $request ) { + if ( ! isset( $request['id'] ) ) { + return new \WP_Error( + 'rest_missing_callback_param', + __( 'A required parameter is missing.', 'accessibility-checker' ), + [ 'status' => 400 ] + ); + } $post_id = (int) $request['id']; return current_user_can( 'edit_post', $post_id ); // able to edit the post. }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
admin/class-welcome-page.php(1 hunks)includes/classes/class-rest-api.php(7 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.php
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
**/*.php: Follow WordPress coding standards (WPCS) in all PHP files
Use PSR-4 autoloading with EqualizeDigital\AccessibilityChecker namespace for new classes
Use WordPress hooks and filters appropriately
Use edac_ prefix for custom hooks and filters
Minimum PHP 7.4 compatibility
Use type hints where appropriate in PHP code
Follow WordPress security best practices (sanitization, validation, nonces)
Use WordPress database API (wpdb) for database operations
Prefix all functions and classes with edac_ when in global namespace
Use WordPress hooks (actions/filters) for extensibility
Use WordPress transients for caching
Sanitize all user inputs
Validate and escape all outputs
Use WordPress nonces for form submissions
Implement proper capability checks
Provide appropriate hooks for extensibility when adding new functionality
Use descriptive hook names with edac_ prefix
Document all custom hooks in docblocks
Document custom hooks and filters with clear descriptions and parameter types
Use PHPDoc for all public classes, methods, and properties
Escape all output, especially in admin screens and user-generated content
Use WordPress error handling functions (e.g., WP_Error) for PHP errors
Files:
admin/class-welcome-page.phpincludes/classes/class-rest-api.php
**/class-*.php
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
**/class-*.php: Legacy files are autoloaded using EDAC namespace
Legacy class names use WordPress style (Class_Name_Convention)
Legacy file names use class-class-name.php (WordPress style)
Legacy Classes: class-class-name.php
Deprecate legacy code with clear docblocks and migration notes
Files:
admin/class-welcome-page.phpincludes/classes/class-rest-api.php
**/*.{php,js}
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
**/*.{php,js}: Follow WordPress internationalization (i18n) practices and use accessibility-checker text domain
All user-facing text must be translatable
Add inline comments for complex accessibility logic or non-obvious code
Use semantic HTML structure
ARIA attributes used correctly
Keyboard navigation supported
Images have descriptive alt text
Heading hierarchy logical
Screen reader compatibility
Forms are accessible and labeled
Files:
admin/class-welcome-page.phpincludes/classes/class-rest-api.php
includes/**/*.php
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
includes/**/*.php: Validate all AJAX requests and REST endpoints with nonces and capability checks
Avoid blocking queries in PHP, especially during scans
Files:
includes/classes/class-rest-api.php
⏰ 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 (6)
admin/class-welcome-page.php (1)
43-47: Gating “Update Counts” behind publish_posts is correct and aligned with REST permission.Matches the /clear-cached-scans-stats permission_callback (publish_posts) and prevents exposing the control to users who can’t perform the action anyway. LGTM.
includes/classes/class-rest-api.php (5)
81-91: Per-post ID args + permission callback are solid.
- args: id is required, validated, and sanitized (absint).
- permission: moves from blanket edit_posts to per-post edit via helper.
This is exactly the right tightening for write operations.
106-108: Restricting scans-stats to edit_posts looks good.These admin-focused stats shouldn’t be exposed to general readers. LGTM.
124-125: Correct capability for cache clearing.Requiring publish_posts for POST /clear-cached-scans-stats aligns with the admin UI and the state-changing nature of the action.
158-159: LGTM – consistent permissioning.edit_posts for the multi-type stats endpoint matches the other stats routes.
176-185: ID arg handling + per-post permission for clear-issues is correct.Good combination of required+sanitized route param and a permission check bound to that post.
|
A rebased version of this was merged into main. |
This pull request improves permission handling for REST API endpoints and frontend UI elements to ensure that only users with the appropriate capabilities can perform actions related to post accessibility scans. It introduces a more granular permission check for editing posts, updates the frontend logic to match these checks, and adds comprehensive tests to verify correct behavior.
Permission handling improvements
edit_postscapability checks in REST API endpoint registration with a new method,user_can_edit_passed_post_id, which checks if the current user can edit the specific post ID provided in the request. This change is applied to both the post scan results and clear issues endpoints inclass-rest-api.php. [1] [2] [3]userCanEditproperty to the frontend highlighter app's initialization data, ensuring the UI can accurately reflect the user's ability to edit the current post.Frontend UI logic updates
index.jsto show the "Clear Issues" and "Rescan This Page" buttons only to users who can both edit the post and are logged in, aligning the UI with the new permission checks. [1] [2]Testing
RestApiEndpointsTest, to verify that only users with edit permissions for a specific post can save scan results or clear issues via the REST API. This includes tests for both admin and limited users.Checklist
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Tests
Chores