PRO-1239: Enforce ignore/dismiss permission with a real capability - #1855
PRO-1239: Enforce ignore/dismiss permission with a real capability#1855pattonwebz wants to merge 24 commits into
Conversation
…oles
edac_user_can_ignore() previously recomputed a role-array intersection
on every call and returned it directly (array|false, not bool). Replace
it with current_user_can('edac_ignore_issues'), a capability synced
onto exactly the allowed roles whenever edacp_ignore_user_roles is
saved, plus a map_meta_cap bypass for manage_options and a one-time
migration for sites that already have the option set. Gives every
future call site (REST, AJAX, menu registration) one consistent,
strictly-boolean check instead of duplicating the role logic.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The Fast-Track quick-ignore filter defaulted edac_ignore_permission to true instead of checking the current user, and the bulk-ignore AJAX handler had no ignore-capability check at all (only the edit_post check on individual posts). Both now gate on edac_user_can_ignore(). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Add the capability check to both the route's permission_callback and dismiss_issue() itself, so a user with edit_post on the target post but no ignore permission gets a 403 rather than being allowed through on edit_post alone. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Expose edac_user_can_ignore() to the sidebar's JS config so the dismiss UI can react to the current user's permission instead of assuming every logged-in user can dismiss. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
DismissPanel now reads a canDismiss prop (default true, so existing usages are unaffected) and shows a "You do not have permission to dismiss issues" notice instead of the dismiss form/actions when false. IssueDetailsModal wires it from window.edac_sidebar_app.canDismiss. Extracted the isIgnored/canDismiss branching into a dismissBody variable (matching the existing panelTitle pattern) instead of a three-way ternary, since ESLint's no-nested-ternary rule flags nested ternaries even when parenthesized. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
IgnoreCapabilityTest covers the role add/remove sync behavior when edacp_ignore_user_roles is saved. RestApiEndpointsTest gains a case proving a user with edit_post but no edac_ignore_issues still gets a 403 on dismiss-issue, and the existing edit_post-authorization test fixture is updated to grant edac_ignore_issues explicitly so it keeps testing edit_post authorization in isolation from the new check. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Companion PR: equalizedigital/accessibility-checker-pro#832 |
|
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:
📝 WalkthroughWalkthroughIgnore permissions now use synchronized capabilities, with support for direct user grants. AJAX, REST, and sidebar dismissal flows enforce individual and global permissions. PHPUnit tests cover synchronization, migration, authorization, and capability grants. ChangesIgnore capability authorization
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant DismissEndpoint
participant CapabilityChecker
participant IssueStore
Caller->>DismissEndpoint: submit dismiss request
DismissEndpoint->>CapabilityChecker: check ignore capability
CapabilityChecker-->>DismissEndpoint: permission result
DismissEndpoint->>IssueStore: update issue when permitted
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Generalizes the sync/manage_options-bypass/version-gated-migration trio that edac_ignore_issues hand-rolled in options-page.php, so future role-configurable features (and the pro plugin's REST routes that currently copy-paste inline current_user_can() closures) can reuse it instead of re-implementing the same pattern. Deliberately not generalized further than the one real use case demands: the role source is a plain option name (not an injected callable) and the admin bypass is hardcoded to manage_options (not a constructor parameter) — both would be solving hypothetical second cases that don't exist yet. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Replaces the hand-rolled sync/bypass/migration functions with a single Synced_Capability instance behind an edac_ignore_capability() accessor; edac_user_can_ignore() now just delegates to it. No behavior change — same capability name, option name, default role, and migration semantics as before, just de-duplicated. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Uses a throwaway capability/option pair, independent of edac_ignore_issues, to cover sync/register/manage_options-bypass/ permission_callback plus the migration behavior that had no direct test coverage before this class existed: initial migration on an unset option, no re-run at the same version, and a version bump forcing re-sync. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Pushed 3 more commits adding `Synced_Capability` — a small reusable class extracted from this PR's own `edac_ignore_issues` wiring (sync onto roles from an option + `manage_options` bypass + version-gated migration), so future role-configurable features and the pro plugin's REST routes (currently full of copy-pasted inline `current_user_can()` closures) can reuse the pattern instead of hand-rolling it again. Deliberately kept narrow: the role source is a plain option name rather than an injected callable, and the admin bypass is hardcoded to `manage_options` rather than a constructor parameter — both would be generalizing for a hypothetical second case that doesn't exist yet given there's exactly one real caller today. `edac_ignore_issues` itself now just instantiates this class — no behavior change, same capability/option/default-role/migration semantics as before. Added standalone tests for the class independent of that one feature, plus fixed up `IgnoreCapabilityTest` to call through the new API. Full suite: 887 tests passing, 15 pre-existing skips. 🤖 Generated with Claude Code |
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@includes/classes/Capabilities/Synced_Capability.php`:
- Around line 8-21: Rename the Synced_Capability class and its file to
SyncedCapability.php, following the project’s CamelCase convention. Update all
references, including the use statements in options-page.php and
SyncedCapabilityTest.php, while preserving the class behavior and namespace.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 021f446a-6aba-4fb8-bee0-e1d7d08b50e8
📒 Files selected for processing (4)
includes/classes/Capabilities/Synced_Capability.phpincludes/options-page.phptests/phpunit/includes/IgnoreCapabilityTest.phptests/phpunit/includes/classes/Capabilities/SyncedCapabilityTest.php
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/phpunit/includes/IgnoreCapabilityTest.php
| namespace EqualizeDigital\AccessibilityChecker\Capabilities; | ||
|
|
||
| /** | ||
| * Registers a real WordPress capability, kept in sync with the role list | ||
| * stored in a plugin option, with a manage_options bypass and a | ||
| * version-gated migration for sites that already had the option set | ||
| * before the capability existed. | ||
| * | ||
| * Extracted from the hand-rolled edac_ignore_issues wiring in | ||
| * includes/options-page.php so other gated features (Explorer REST | ||
| * routes, future role-configurable features) can reuse the same pattern | ||
| * instead of re-implementing the sync/bypass/migration trio each time. | ||
| */ | ||
| class Synced_Capability { |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Class name doesn't follow the project's CamelCase convention.
Synced_Capability uses underscore separation, but the existing codebase pattern (e.g. FixesPage used alongside this in includes/options-page.php) and the coding guideline both call for CamelCase class names. Since this class is explicitly designed for reuse by future gated features, renaming now (before more consumers adopt it) avoids inconsistency later. This would require updating the use statements in includes/options-page.php and tests/phpunit/includes/classes/Capabilities/SyncedCapabilityTest.php as well.
As per coding guidelines, "Use CamelCase naming convention for new class names (e.g., ClassName.php)".
♻️ Proposed rename
-class Synced_Capability {
+class SyncedCapability {Also rename the file to SyncedCapability.php and update the two consuming use statements.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@includes/classes/Capabilities/Synced_Capability.php` around lines 8 - 21,
Rename the Synced_Capability class and its file to SyncedCapability.php,
following the project’s CamelCase convention. Update all references, including
the use statements in options-page.php and SyncedCapabilityTest.php, while
preserving the class behavior and namespace.
Source: Coding guidelines
William asked whether a capability could be granted to an individual
user (not just via role), and whether an admin could see who granted
it. Synced_Capability doesn't need to change for this: its sync() only
touches role objects (wp_user_roles option), while a capability added
directly to a user via $user->add_cap() lives in that user's own
wp_capabilities meta — separate storage that current_user_can()/
user_can() already merges. A role-level sync can never clobber an
individual grant.
What WordPress doesn't provide is attribution — no concept of who
granted a capability or when. User_Capability_Grant adds that as a
thin layer: grant()/revoke() wrap add_cap()/remove_cap() and record
{granted_by, granted_at} in user meta; get_grant_info() surfaces it for
a future admin UI; is_individually_granted() distinguishes "granted
directly to this user" from "has it via their role" (both pass
user_can() identically, only the former reads $user->caps rather than
the role-merged $user->allcaps).
Works with any capability string, not coupled to edac_ignore_issues or
Synced_Capability. No admin UI yet — this is the underlying mechanism
a future grant/revoke screen would call.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Covers grant/revoke, attribution recording and defaulting to the current user, revoke clearing attribution, is_individually_granted() distinguishing a direct grant from a role-derived capability, graceful handling of a nonexistent user ID, and the key coexistence guarantee: a direct grant survives a role-level capability removal (proving Synced_Capability-style role sync can never clobber it, without depending on that class). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Pushed 2 more commits adding `User_Capability_Grant` — a small class for granting a capability directly to an individual user (independent of their role) with attribution (who granted it, when). This came out of a design question during review: does `Synced_Capability` need to change to support per-user grants? Turns out no — its `sync()` only ever touches role objects (`wp_user_roles` option), while a capability added directly to a user via `$user->add_cap()` lives in that user's own `wp_capabilities` meta, completely separate storage that `current_user_can()`/`user_can()` already merges automatically. A role-level sync can never clobber an individual grant — proven directly in the new test suite (`test_direct_grant_survives_role_level_capability_removal`). What WordPress genuinely lacks is attribution — no concept of who granted a capability or when. `User_Capability_Grant::grant()/revoke()/get_grant_info()/is_individually_granted()` adds that as a thin, capability-agnostic layer (works with any capability string, not coupled to `edac_ignore_issues`). No admin UI yet — this is the underlying mechanism a future grant/revoke screen would call. Full suite: 896 tests passing, 15 pre-existing skips. 🤖 Generated with Claude Code |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@includes/classes/Capabilities/User_Capability_Grant.php`:
- Around line 48-63: Update User_Capability_Grant::grant() and revoke() to track
whether this class added the capability, rather than attributing every
pre-existing capability assignment to itself. Persist that ownership flag with
the grant metadata, and make revoke() call remove_cap() only when the flag
indicates this class added it; preserve metadata cleanup and add a regression
test covering a pre-existing direct capability that remains after grant/revoke.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 79665a82-9e00-443e-8ced-e03b8572ace2
📒 Files selected for processing (2)
includes/classes/Capabilities/User_Capability_Grant.phptests/phpunit/includes/classes/Capabilities/UserCapabilityGrantTest.php
| public static function grant( int $user_id, string $capability, int $granted_by = 0 ): bool { | ||
| $user = get_userdata( $user_id ); | ||
| if ( ! $user ) { | ||
| return false; | ||
| } | ||
|
|
||
| $user->add_cap( $capability ); | ||
|
|
||
| update_user_meta( | ||
| $user_id, | ||
| self::META_PREFIX . $capability, | ||
| [ | ||
| 'granted_by' => $granted_by ? $granted_by : get_current_user_id(), | ||
| 'granted_at' => time(), | ||
| ] | ||
| ); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Do not revoke a pre-existing direct capability.
grant() records attribution even when another plugin already assigned the capability directly, while revoke() always removes it. Granting and then revoking through this class can therefore strip an unrelated direct permission. Record whether this class actually added the capability, and only call remove_cap() when that flag is true; add a regression test for this case.
Proposed direction
+ $was_directly_granted = ! empty( $user->caps[ $capability ] );
$user->add_cap( $capability );
update_user_meta(
$user_id,
self::META_PREFIX . $capability,
[
'granted_by' => $granted_by ? $granted_by : get_current_user_id(),
'granted_at' => time(),
+ 'added_by_this_grant' => ! $was_directly_granted,
]
);
+ $grant_info = self::get_grant_info( $user_id, $capability );
- $user->remove_cap( $capability );
+ if ( ! empty( $grant_info['added_by_this_grant'] ) ) {
+ $user->remove_cap( $capability );
+ }
delete_user_meta( $user_id, self::META_PREFIX . $capability );Also applies to: 82-83
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@includes/classes/Capabilities/User_Capability_Grant.php` around lines 48 -
63, Update User_Capability_Grant::grant() and revoke() to track whether this
class added the capability, rather than attributing every pre-existing
capability assignment to itself. Persist that ownership flag with the grant
metadata, and make revoke() call remove_cap() only when the flag indicates this
class added it; preserve metadata cleanup and add a regression test covering a
pre-existing direct capability that remains after grant/revoke.
…UserCapabilityGrant Match the codebase's PSR-4 class-naming convention (PascalCase, no underscores — see FixesManager, Connector) rather than the legacy underscore-separated style used by the older EDAC\Admin\* classes. Renames files, class names, and every reference (includes/options-page.php, both test suites), no behavior change. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Pushed a rename: `Synced_Capability` → `SyncCapability`, `User_Capability_Grant` → `UserCapabilityGrant` (files, class names, and every reference — no behavior change). Matches this codebase's PSR-4 class-naming convention (PascalCase, no underscores — same as `FixesManager`, `Connector`) rather than the legacy underscore style. `CAPABILITY_MAP.md` updated to match throughout. Full suite: 896 tests passing, 15 pre-existing skips. Also filed 7 Linear cards (PRO-1254–PRO-1260) for the capability-addition requests from William's review — Settings/Fixes capability, frontend highlighter capability, Full Site Scan + save-results capabilities, simplified-summary capability (low priority), the license REST route normalization onto `AbstractRestRoute`, multisite/export sync with `edac_ignore_issues`, and an Export-plugin capability. None of those are code changes in this PR — future work, linked back to PRO-1239 for context. 🤖 Generated with Claude Code |
Splits SyncCapability into a pure writer that can sync N capabilities from one option (via a generic sync_role_capability() primitive) and a new agnostic CapabilityChecker reader class, so the upcoming ignore/global-ignore/explorer-access capability split can share one option and one migration instead of three independent instances. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Bundles edac_ignore_issues with two new capabilities - edac_ignore_issues_globally and edac_issues_explorer_access - onto the existing edacp_ignore_user_roles option/roles, rather than adding a separate settings control for each. Adds edac_user_can_ignore_globally() and edac_user_can_access_issues_explorer() helpers alongside the existing edac_user_can_ignore(), all routed through CapabilityChecker. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
sync_role_capability() is now private - calling it directly for one capability out of a bundle would grant/revoke that one while leaving the rest of the bundle out of sync for that role, contradicting the class's own guarantee that bundled capabilities always travel together. The migration-version marker is now keyed by option_name plus a hash of the capability set, not option_name alone, so two SyncCapability instances that ever point at the same option can't silently share one version counter and skip each other's migration. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
dismiss_issue()'s largeBatch path is what actually performs a global ignore (updating every row sharing an object across every post), but was only gated by the ordinary per-post edac_ignore_issues capability plus a per-post edit_post loop - neither of which represents "allowed to take a global action" specifically. A role granted edac_ignore_issues and edac_issues_explorer_access but not edac_ignore_issues_globally could still perform the real global suppression through this endpoint, since pro's separate /global-ignore route (correctly gated on the new capability) only syncs a persistence table, not the actual issue rows. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
DismissPanel showed its "Dismiss Globally" / "Remove Global Dismissal" controls whenever isPro was true, regardless of whether the current user actually has edac_ignore_issues_globally - the only consequence was the underlying REST call failing after the new server-side check, but the control shouldn't be offered at all to a user who can't use it. Adds a canDismissGlobally prop (sourced from edac_user_can_ignore_globally() via wp_localize_script) that both the free plugin's per-post issue modal and pro's Issues Explorer now pass through. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
admin_menu (where menu capability checks happen) and rest_api_init (where REST permission_callbacks are registered) both fire before admin_init on their respective request types, so migrating on admin_init left the first request after a version bump building a menu, or serving a REST request, against pre-migration capabilities - and REST-only requests never fire admin_init at all. init fires early enough on every request type to have already run by the time any of those checks happen. Also throws if SyncCapability is ever constructed with zero capabilities, rather than silently passing null to current_user_can() from user_can()'s no-argument default. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
A largeBatch request already requires edac_ignore_issues_globally to reach this code at all - re-checking edit_post per affected post for a user who already holds that capability is redundant work (and, more importantly, blocks the capability from doing what it's for: letting a trusted role act on posts they don't personally own). The loop stays as a fallback for any caller that somehow reaches this branch without the capability. Updates the largeBatch tests accordingly: what was "authorized on some" now proves the bypass covers posts the user doesn't own, and the previously-shared edac_ignore_issues_globally grant on the test fixture user moves to per-test so tests that still want the fallback loop exercised (partial-post-ownership blocked) aren't accidentally short-circuited by it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…s branch SyncCapabilityTest: the constructor rejecting an empty capabilities array and register() hooking maybe_migrate() to init (not admin_init) were both new behavior in 3816bc2 with no direct test coverage. DismissPanel: the existing 'keeps global undo available' test predates the canDismissGlobally prop and was asserting on the old isPro-only gate, which now fails - a Pro user's isPro flag alone no longer implies they can see the global-dismiss/undo controls after d9da482. Updated it to pass canDismissGlobally, and added tests for the capability actually gating both the initiate and undo controls independent of isPro. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
includes/classes/class-rest-api.php (1)
336-358: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winGlobal-ignore bypass is incomplete: the permission_callback still gates on
edit_postfor the URL's representative issue, and no test catches it. The root cause is in the permission_callback; the test gap is a direct consequence of never exercising a representative issue on a post the actor can't edit.
includes/classes/class-rest-api.php#L336-L358: add a bypass so that whenlargeBatchis true andedac_user_can_ignore_globally()is true, the permission_callback returnstruewithout requiringcurrent_user_can( 'edit_post', $post_id )on the representative issue's post (see the diff in the per-site comment above).tests/phpunit/includes/classes/RestApiEndpointsTest.php#L904-L1006: extendtest_large_batch_dismiss_bypasses_per_post_check_with_global_capability(or add a new test) so the request's URLissue_idis the one on$admin_post(a post the acting user cannot edit), not$limited_post, so the test actually exercises the permission_callback's global bypass path once fixed.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@includes/classes/class-rest-api.php` around lines 336 - 358, The permission_callback in includes/classes/class-rest-api.php lines 336-358 must return true for largeBatch requests when edac_user_can_ignore_globally() is true, without requiring edit_post on the representative issue; retain existing validation for other requests. Update tests/phpunit/includes/classes/RestApiEndpointsTest.php lines 904-1006 by extending test_large_batch_dismiss_bypasses_per_post_check_with_global_capability (or adding a test) so the request uses the issue_id from $admin_post, exercising the global bypass with a post the actor cannot edit.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@includes/classes/class-rest-api.php`:
- Around line 336-358: The permission_callback in
includes/classes/class-rest-api.php lines 336-358 must return true for
largeBatch requests when edac_user_can_ignore_globally() is true, without
requiring edit_post on the representative issue; retain existing validation for
other requests. Update tests/phpunit/includes/classes/RestApiEndpointsTest.php
lines 904-1006 by extending
test_large_batch_dismiss_bypasses_per_post_check_with_global_capability (or
adding a test) so the request uses the issue_id from $admin_post, exercising the
global bypass with a post the actor cannot edit.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7aa3648a-e2ad-4277-a6f3-59abf5bc4a48
📒 Files selected for processing (11)
admin/class-enqueue-admin.phpincludes/classes/Capabilities/CapabilityChecker.phpincludes/classes/Capabilities/SyncCapability.phpincludes/classes/class-rest-api.phpincludes/options-page.phpsrc/issueModal/components/DismissPanel.jssrc/issueModal/components/IssueDetailsModal.jstests/phpunit/includes/IgnoreCapabilityTest.phptests/phpunit/includes/classes/Capabilities/CapabilityCheckerTest.phptests/phpunit/includes/classes/Capabilities/SyncCapabilityTest.phptests/phpunit/includes/classes/RestApiEndpointsTest.php
🚧 Files skipped from review as they are similar to previous changes (3)
- src/issueModal/components/IssueDetailsModal.js
- admin/class-enqueue-admin.php
- src/issueModal/components/DismissPanel.js
register() only hooked add_option/update_option, so deleting the
option (e.g. Pro's uninstall routine, gated behind the "delete data"
preference) left every role that had been granted the bundle stuck
with it indefinitely - the old array_intersect()-against-get_option()
check re-read the option live on every request, so this is a real
behavior change from before: deleting the option used to revoke
access instantly for everyone but admins, and now it wouldn't at all.
Hooking delete_option_{$option_name} to sync([]) closes that gap.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
No call site in either plugin ever used grant()/revoke()/get_grant_info() - per-user capability overrides aren't a planned feature, and site owners who want that already have WordPress's own add_cap()/remove_cap() or a role-editor plugin. Speculative infrastructure with no consumer and no near-term plan isn't worth the maintenance surface. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… post
The dismiss-issue route's permission_callback unconditionally required
current_user_can('edit_post', $post_id) against whichever post the
URL's issue_id resolved to - dismiss_issue() itself already bypasses
the per-post edit_post loop for a user with edac_ignore_issues_globally,
but that bypass never got a chance to run if the one representative
post in the URL wasn't personally editable by the caller, since the
permission_callback rejected the request first.
Flagged by CodeRabbit on PR #1855; the existing largeBatch-bypass test
didn't catch it because its URL issue_id happened to resolve to a post
the actor could already edit. Added a test that puts the URL's issue_id
on a post only the global capability - not personal ownership - can
unlock.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…-develop # Conflicts: # admin/class-ajax.php # tests/phpunit/includes/classes/RestApiEndpointsTest.php
1 participant
Summary
Introduces a real, synced WordPress capability (
edac_ignore_issues) as the single source of truth for who can dismiss/ignore accessibility issues, replacing the ad-hocedac_user_can_ignore()role-intersection check that wasn't consistently enforced everywhere.edac_user_can_ignore()now just checkscurrent_user_can('edac_ignore_issues')edacp_ignore_user_roleswhenever that option is saved, with amap_meta_capbypass somanage_optionsusers always pass, plus a one-time migration for existing installstrue), the bulk-ignore AJAX handler (had no ignore-capability check), and the RESTdismiss-issueroute (permission callback + handler)canDismissconfig flagRelated
Test plan
edacp_ignore_user_rolesto a role, confirm that role gainsedac_ignore_issuesand can dismiss; confirm a role left out cannot (gets a notice in the UI, 403 from REST/AJAX)manage_optionsusers can always dismiss regardless of role configadmin_initwithout needing to re-save the settingnpm run lint, PHPUnit🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Tests