From 82961f937e8e0f5bb12b0704ad6f73bdf9d74fe8 Mon Sep 17 00:00:00 2001 From: pattonwebz Date: Tue, 28 Jul 2026 18:39:17 +0100 Subject: [PATCH 01/23] Sync edac_ignore_issues as a real capability from edacp_ignore_user_roles 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 --- includes/options-page.php | 76 +++++++++++++++++++++++++++++++++++---- 1 file changed, 69 insertions(+), 7 deletions(-) diff --git a/includes/options-page.php b/includes/options-page.php index a0ac85a33..6f9850241 100644 --- a/includes/options-page.php +++ b/includes/options-page.php @@ -21,18 +21,80 @@ * @return bool */ function edac_user_can_ignore() { + return current_user_can( 'edac_ignore_issues' ); // phpcs:ignore WordPress.WP.Capabilities.Unknown -- This is a custom capability, synced from the edacp_ignore_user_roles setting. +} + +/** + * Let `manage_options` users always pass an `edac_ignore_issues` check, + * regardless of whether their role is currently in `edacp_ignore_user_roles`. + * Keeps `current_user_can( 'edac_ignore_issues' )` as the single check every + * call site (REST, AJAX, menu registration) can rely on. + * + * @param array $caps Required primitive capabilities. + * @param string $cap Capability being checked. + * @param int $user_id User ID. + * @return array + */ +function edac_map_ignore_capability( $caps, $cap, $user_id ) { + if ( 'edac_ignore_issues' === $cap && user_can( $user_id, 'manage_options' ) ) { + return []; + } + return $caps; +} +add_filter( 'map_meta_cap', 'edac_map_ignore_capability', 10, 3 ); - if ( current_user_can( 'manage_options' ) ) { - return true; +/** + * Sync the `edac_ignore_issues` capability onto exactly the roles allowed + * to dismiss/ignore issues, so `current_user_can( 'edac_ignore_issues' )` + * is always the source of truth rather than comparing against the option + * directly. + * + * @param array $roles Role slugs that should have the capability. + * @return void + */ +function edac_sync_ignore_capability( $roles ) { + $roles = is_array( $roles ) ? $roles : []; + + foreach ( wp_roles()->role_objects as $role_slug => $role ) { + if ( in_array( $role_slug, $roles, true ) ) { + $role->add_cap( 'edac_ignore_issues' ); + } else { + $role->remove_cap( 'edac_ignore_issues' ); + } } +} +add_action( + 'add_option_edacp_ignore_user_roles', + function ( $option, $value ) { + edac_sync_ignore_capability( $value ); + }, + 10, + 2 +); +add_action( + 'update_option_edacp_ignore_user_roles', + function ( $old_value, $value ) { + edac_sync_ignore_capability( $value ); + }, + 10, + 2 +); - $user = wp_get_current_user(); - $user_roles = ( isset( $user->roles ) ) ? $user->roles : []; - $ignore_user_roles = get_option( 'edacp_ignore_user_roles' ); - $interset = ( $user_roles && $ignore_user_roles ) ? array_intersect( $user_roles, $ignore_user_roles ) : false; +/** + * One-time migration: existing installs already have `edacp_ignore_user_roles` + * saved but never had it synced onto a real capability. Run once per site. + * + * @return void + */ +function edac_maybe_migrate_ignore_capability() { + if ( get_option( 'edac_ignore_cap_synced' ) ) { + return; + } - return ( $interset ); + edac_sync_ignore_capability( get_option( 'edacp_ignore_user_roles', [ 'administrator' ] ) ); + update_option( 'edac_ignore_cap_synced', 1 ); } +add_action( 'admin_init', 'edac_maybe_migrate_ignore_capability' ); /** * Add an options page under the Settings submenu From f89d86e7de55e9fb7a06f4d31597847862df7ec3 Mon Sep 17 00:00:00 2001 From: pattonwebz Date: Tue, 28 Jul 2026 18:39:26 +0100 Subject: [PATCH 02/23] Enforce edac_ignore_issues in the AJAX ignore handlers 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 --- admin/class-ajax.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/admin/class-ajax.php b/admin/class-ajax.php index 2bb38a891..91a37249d 100644 --- a/admin/class-ajax.php +++ b/admin/class-ajax.php @@ -323,7 +323,7 @@ function ( $a, $b ) { * * @allowed bool True if allowed, false if not */ - $ignore_permission = apply_filters( 'edac_ignore_permission', true ); + $ignore_permission = apply_filters( 'edac_ignore_permission', edac_user_can_ignore() ); $severity_map = [ 1 => [ @@ -826,6 +826,10 @@ public function add_ignore() { wp_send_json_error( new \WP_Error( '-1', __( 'Permission Denied', 'accessibility-checker' ) ) ); } + if ( ! edac_user_can_ignore() ) { + wp_send_json_error( new \WP_Error( '-5', __( 'Permission Denied', 'accessibility-checker' ) ) ); + } + global $wpdb; $table_name = $wpdb->prefix . 'accessibility_checker'; $raw_ids = isset( $_REQUEST['ids'] ) ? (array) wp_unslash( $_REQUEST['ids'] ) : []; // phpcs:ignore WordPress.Security.ValidatedSanitizedInput.InputNotSanitized -- Sanitization handled below. From 61f9589b9e489cdd3f7f0e898adb715ed4546274 Mon Sep 17 00:00:00 2001 From: pattonwebz Date: Tue, 28 Jul 2026 18:39:35 +0100 Subject: [PATCH 03/23] Enforce edac_ignore_issues on the REST dismiss-issue route 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 --- includes/classes/class-rest-api.php | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/includes/classes/class-rest-api.php b/includes/classes/class-rest-api.php index a1a11afde..8a553d2ab 100644 --- a/includes/classes/class-rest-api.php +++ b/includes/classes/class-rest-api.php @@ -340,6 +340,10 @@ function () use ( $ns, $version ) { return false; } + if ( ! edac_user_can_ignore() ) { + return false; + } + $table_name = edac_get_valid_table_name( $wpdb->prefix . 'accessibility_checker' ); if ( ! $table_name ) { return false; @@ -1204,6 +1208,14 @@ private function get_wcag_url_and_title_from_number( $wcag_number ) { public function dismiss_issue( $request ) { global $wpdb; + if ( ! edac_user_can_ignore() ) { + return new \WP_Error( + 'rest_forbidden', + __( 'Sorry, you are not allowed to dismiss issues.', 'accessibility-checker' ), + [ 'status' => rest_authorization_required_code() ] + ); + } + $issue_id = (int) $request['issue_id']; $action = $request->get_param( 'action' ); $reason = $request->get_param( 'reason' ) ?? ''; From ebc35c6af03be7161278c772c34d5aebe2802a1c Mon Sep 17 00:00:00 2001 From: pattonwebz Date: Tue, 28 Jul 2026 18:39:44 +0100 Subject: [PATCH 04/23] Localize canDismiss flag for the editor sidebar app 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 --- admin/class-enqueue-admin.php | 1 + 1 file changed, 1 insertion(+) diff --git a/admin/class-enqueue-admin.php b/admin/class-enqueue-admin.php index fcc888026..2a4837ca7 100644 --- a/admin/class-enqueue-admin.php +++ b/admin/class-enqueue-admin.php @@ -225,6 +225,7 @@ public static function maybe_enqueue_sidebar_script() { 'edacApiUrl' => esc_url_raw( rest_url() . 'accessibility-checker/v1' ), 'settingsUrl' => esc_url_raw( admin_url( 'admin.php?page=accessibility_checker_settings' ) ), 'canManageSettings' => current_user_can( apply_filters( 'edac_filter_settings_capability', 'manage_options' ) ), + 'canDismiss' => edac_user_can_ignore(), 'readabilityHelpUrl' => esc_url_raw( edac_link_wrapper( 'https://a11ychecker.com/help3265', 'wordpress-general', 'content-analysis-sidebar', false ) ), 'dismissReasons' => IgnoreUI::get_reasons(), 'simplifiedSummaryPrompt' => get_option( 'edac_simplified_summary_prompt', 'none' ), From c5c8c3174c974f69626a41e62fdbc43f3f340638 Mon Sep 17 00:00:00 2001 From: pattonwebz Date: Tue, 28 Jul 2026 18:42:29 +0100 Subject: [PATCH 05/23] Hide dismiss/reopen UI from users without canDismiss 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 --- src/issueModal/components/DismissPanel.js | 332 +++++++++--------- .../components/IssueDetailsModal.js | 1 + 2 files changed, 175 insertions(+), 158 deletions(-) diff --git a/src/issueModal/components/DismissPanel.js b/src/issueModal/components/DismissPanel.js index c51a835ce..0745665dc 100644 --- a/src/issueModal/components/DismissPanel.js +++ b/src/issueModal/components/DismissPanel.js @@ -26,6 +26,7 @@ import { getDismissReasonOptions } from '../../sidebar/utils/dismissHelpers'; * @param {Function} props.onCloseModal - Callback to close the parent modal. * @param {boolean} props.forceGlobal - When true, the primary dismiss action targets all pages (global dismiss). * @param {boolean} props.isPro - Whether the current UI is running in Pro. + * @param {boolean} props.canDismiss - Whether the current user is allowed to dismiss/reopen issues. */ const DismissPanel = ( { issue, @@ -35,6 +36,7 @@ const DismissPanel = ( { onCloseModal, forceGlobal = false, isPro = typeof window !== 'undefined' && ( window.edac_editor_app?.pro === '1' || window.edac_script_vars?.pro === '1' ), + canDismiss = true, } ) => { const panelRef = useRef( null ); const [ comment, setComment ] = useState( issue?.ignre_comment ? decodeEntities( issue.ignre_comment ) : '' ); @@ -122,6 +124,177 @@ const DismissPanel = ( { panelTitle = __( 'Dismiss Issue', 'accessibility-checker' ); } + let dismissBody; + if ( isIgnored ) { + dismissBody = ( + <> + { ( issue?.user || issue?.ignre_user_name || issue?.ignre_date || isGloballyDismissed ) && ( +
+ + { ( issue?.ignre_global === 1 || issue?.ignre_global === '1' ) && ( + <> +
{ __( 'Scope:', 'accessibility-checker' ) }
+
{ __( 'All pages', 'accessibility-checker' ) }
+ + ) } + { ( issue?.ignre_user_name || issue?.user ) && ( + <> +
{ __( 'By:', 'accessibility-checker' ) }
+
{ decodeEntities( issue.ignre_user_name || issue.user ) }
+ + ) } + { issue?.ignre_date && ( + <> +
{ __( 'On:', 'accessibility-checker' ) }
+
{ decodeEntities( issue.ignre_date ) }
+ + ) } +
+ ) } + { issue?.ignre_comment && ( +
+

+ { __( 'Reason for dismissal:', 'accessibility-checker' ) } +

+
+ { issue.ignre_comment } +
+
+ ) } + { canDismiss && isGloballyDismissed && ( +
+ +
+ ) } + { canDismiss && ! isGloballyDismissed && ( +
+ +
+ ) } + + ); + } else if ( canDismiss ) { + dismissBody = ( +
{ + e.preventDefault(); + handleToggleIgnore( true, dismissGlobally ); + } } + > + + +
+ + { canDismissGlobally && ( + ( + + +
+ ) } + /> + ) } + + + ); + } else { + dismissBody = ( + + { __( 'You do not have permission to dismiss issues.', 'accessibility-checker' ) } + + ); + } + return (
@@ -148,164 +321,7 @@ const DismissPanel = ( { { error } ) } - { isIgnored ? ( - <> - { ( issue?.user || issue?.ignre_user_name || issue?.ignre_date || isGloballyDismissed ) && ( -
- - { ( issue?.ignre_global === 1 || issue?.ignre_global === '1' ) && ( - <> -
{ __( 'Scope:', 'accessibility-checker' ) }
-
{ __( 'All pages', 'accessibility-checker' ) }
- - ) } - { ( issue?.ignre_user_name || issue?.user ) && ( - <> -
{ __( 'By:', 'accessibility-checker' ) }
-
{ decodeEntities( issue.ignre_user_name || issue.user ) }
- - ) } - { issue?.ignre_date && ( - <> -
{ __( 'On:', 'accessibility-checker' ) }
-
{ decodeEntities( issue.ignre_date ) }
- - ) } -
- ) } - { issue?.ignre_comment && ( -
-

- { __( 'Reason for dismissal:', 'accessibility-checker' ) } -

-
- { issue.ignre_comment } -
-
- ) } - { isGloballyDismissed ? ( -
- -
- ) : ( -
- -
- ) } - - ) : ( -
{ - e.preventDefault(); - handleToggleIgnore( true, dismissGlobally ); - } } - > - - -
- - { canDismissGlobally && ( - ( - - -
- ) } - /> - ) } -
- - ) } + { dismissBody } diff --git a/src/issueModal/components/IssueDetailsModal.js b/src/issueModal/components/IssueDetailsModal.js index 5c390c7be..be9df8d16 100644 --- a/src/issueModal/components/IssueDetailsModal.js +++ b/src/issueModal/components/IssueDetailsModal.js @@ -406,6 +406,7 @@ export const IssueDetailsModal = ( { issue, rule, onClose, isOpen, focusSection, onToggle={ () => setIsDismissPanelOpen( ! isDismissPanelOpen ) } onIgnore={ onIgnore } onCloseModal={ onClose } + canDismiss={ window.edac_sidebar_app?.canDismiss !== false } /> From 9ea08a87508ae8a3d116c805416adef446f49a48 Mon Sep 17 00:00:00 2001 From: pattonwebz Date: Tue, 28 Jul 2026 18:43:16 +0100 Subject: [PATCH 06/23] Add tests for edac_ignore_issues capability sync and enforcement 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 --- .../phpunit/includes/IgnoreCapabilityTest.php | 104 ++++++++++++++++++ .../includes/classes/RestApiEndpointsTest.php | 63 +++++++++++ 2 files changed, 167 insertions(+) create mode 100644 tests/phpunit/includes/IgnoreCapabilityTest.php diff --git a/tests/phpunit/includes/IgnoreCapabilityTest.php b/tests/phpunit/includes/IgnoreCapabilityTest.php new file mode 100644 index 000000000..1ba7701dd --- /dev/null +++ b/tests/phpunit/includes/IgnoreCapabilityTest.php @@ -0,0 +1,104 @@ +role_objects as $role ) { + $role->remove_cap( 'edac_ignore_issues' ); + } + parent::tearDown(); + } + + /** + * Syncing should add the capability only to the roles passed in, and + * remove it from roles not included. + * + * @return void + */ + public function test_sync_adds_and_removes_capability_by_role() { + wp_roles()->get_role( 'editor' )->add_cap( 'edac_ignore_issues' ); + + edac_sync_ignore_capability( [ 'author' ] ); + + $this->assertFalse( wp_roles()->get_role( 'editor' )->has_cap( 'edac_ignore_issues' ), 'Editor should have lost the capability.' ); + $this->assertTrue( wp_roles()->get_role( 'author' )->has_cap( 'edac_ignore_issues' ), 'Author should have gained the capability.' ); + } + + /** + * A user in a role that was granted the capability should pass + * edac_user_can_ignore(). + * + * @return void + */ + public function test_user_can_ignore_true_for_synced_role() { + edac_sync_ignore_capability( [ 'author' ] ); + + $user_id = self::factory()->user->create( [ 'role' => 'author' ] ); + wp_set_current_user( $user_id ); + + $this->assertTrue( edac_user_can_ignore() ); + } + + /** + * A user in a role that was not granted the capability should fail + * edac_user_can_ignore(). + * + * @return void + */ + public function test_user_can_ignore_false_for_unsynced_role() { + edac_sync_ignore_capability( [ 'author' ] ); + + $user_id = self::factory()->user->create( [ 'role' => 'editor' ] ); + wp_set_current_user( $user_id ); + + $this->assertFalse( edac_user_can_ignore() ); + } + + /** + * Manage_options users must always pass the check, even if their role + * was left out of edacp_ignore_user_roles (e.g. an admin restricted + * even the administrator role by mistake). + * + * @return void + */ + public function test_manage_options_user_always_can_ignore() { + edac_sync_ignore_capability( [ 'author' ] ); + + $user_id = self::factory()->user->create( [ 'role' => 'administrator' ] ); + wp_set_current_user( $user_id ); + + $this->assertFalse( wp_roles()->get_role( 'administrator' )->has_cap( 'edac_ignore_issues' ), 'Precondition: administrator role itself was not synced.' ); + $this->assertTrue( edac_user_can_ignore(), 'manage_options users must always be able to ignore/dismiss.' ); + } + + /** + * Saving the edacp_ignore_user_roles option should sync the capability + * automatically via the update_option/add_option hooks. + * + * @return void + */ + public function test_saving_option_triggers_sync() { + delete_option( 'edacp_ignore_user_roles' ); + add_option( 'edacp_ignore_user_roles', [ 'author' ] ); + + $this->assertTrue( wp_roles()->get_role( 'author' )->has_cap( 'edac_ignore_issues' ) ); + + update_option( 'edacp_ignore_user_roles', [ 'editor' ] ); + + $this->assertFalse( wp_roles()->get_role( 'author' )->has_cap( 'edac_ignore_issues' ) ); + $this->assertTrue( wp_roles()->get_role( 'editor' )->has_cap( 'edac_ignore_issues' ) ); + } +} diff --git a/tests/phpunit/includes/classes/RestApiEndpointsTest.php b/tests/phpunit/includes/classes/RestApiEndpointsTest.php index 98ed7bd8d..102fab835 100644 --- a/tests/phpunit/includes/classes/RestApiEndpointsTest.php +++ b/tests/phpunit/includes/classes/RestApiEndpointsTest.php @@ -89,6 +89,11 @@ public static function wpSetUpBeforeClass( $factory ) { // Give limited user edit_posts but not edit_others_posts so they cannot edit this post. $user = new WP_User( self::$limited_id ); $user->add_cap( 'edit_posts' ); + // Also grant edac_ignore_issues so dismiss tests exercise edit_post + // authorization specifically, independent of the ignore capability + // (covered separately in IgnoreCapabilityTest and + // test_single_issue_dismiss_forbidden_without_ignore_capability). + $user->add_cap( 'edac_ignore_issues' ); self::$post_id = $factory->post->create( [ @@ -589,6 +594,64 @@ public function test_single_issue_dismiss_authorized_user() { $this->assertSame( 'This is intentional', $updated_issue['ignre_comment'] ); } + /** + * Test: A user who can edit_post but lacks the edac_ignore_issues + * capability is forbidden from dismissing, even though edit_post alone + * used to be sufficient. + * + * @return void + */ + public function test_single_issue_dismiss_forbidden_without_ignore_capability() { + global $wpdb; + + $this->assertNotNull( $this->server ); + + // User can edit their own post, but was never granted edac_ignore_issues. + $user_id = self::factory()->user->create( [ 'role' => 'subscriber' ] ); + ( new WP_User( $user_id ) )->add_cap( 'edit_posts' ); + wp_set_current_user( $user_id ); + + $own_post_id = self::factory()->post->create( + [ + 'post_type' => 'post', + 'post_status' => 'draft', + 'post_author' => $user_id, + 'post_title' => 'Test Ignore Capability Post', + 'post_content' => 'Test content', + ] + ); + + $table_name = $wpdb->prefix . 'accessibility_checker'; + $site_id = get_current_blog_id(); + + // phpcs:disable WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching + $wpdb->insert( + $table_name, + [ + 'postid' => $own_post_id, + 'siteid' => $site_id, + 'type' => 'error', + 'rule' => 'single-no-ignore-cap-test', + 'ruletype' => 'error', + 'object' => 'single-no-ignore-cap-test', + 'recordcheck' => 1, + 'user' => $user_id, + 'ignre' => 0, + 'ignre_global' => 0, + ], + [ '%d', '%d', '%s', '%s', '%s', '%s', '%d', '%d', '%d', '%d' ] + ); + $issue_id = $wpdb->insert_id; + // phpcs:enable WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching + + $request = new \WP_REST_Request( 'POST', '/accessibility-checker/v1/dismiss-issue/' . $issue_id ); + $request->set_param( 'action', 'dismiss' ); + + $response = $this->server->dispatch( $request ); + + $this->assertSame( 403, $response->get_status(), 'A user without edac_ignore_issues should not be able to dismiss even their own editable post.' ); + } + /** * Test: Single issue dismissed by unauthorized user fails with 403. * From 95d82421d5ac338c805fc64aa06dfe4e7d1fbd90 Mon Sep 17 00:00:00 2001 From: pattonwebz Date: Tue, 28 Jul 2026 20:12:51 +0100 Subject: [PATCH 07/23] Add Synced_Capability, extracted from the edac_ignore_issues wiring MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .../Capabilities/Synced_Capability.php | 177 ++++++++++++++++++ 1 file changed, 177 insertions(+) create mode 100644 includes/classes/Capabilities/Synced_Capability.php diff --git a/includes/classes/Capabilities/Synced_Capability.php b/includes/classes/Capabilities/Synced_Capability.php new file mode 100644 index 000000000..b2b58b4cc --- /dev/null +++ b/includes/classes/Capabilities/Synced_Capability.php @@ -0,0 +1,177 @@ +capability = $capability; + $this->option_name = $option_name; + $this->default_roles = $default_roles; + $this->version = $version; + } + + /** + * Wire up the bypass filter, live sync on option save, and the + * version-gated migration. Call once, typically from plugin bootstrap. + * + * @return void + */ + public function register(): void { + add_filter( 'map_meta_cap', [ $this, 'bypass_for_admins' ], 10, 3 ); + + add_action( + "add_option_{$this->option_name}", + function ( $option, $value ) { + $this->sync( $value ); + }, + 10, + 2 + ); + add_action( + "update_option_{$this->option_name}", + function ( $old_value, $value ) { + $this->sync( $value ); + }, + 10, + 2 + ); + + add_action( 'admin_init', [ $this, 'maybe_migrate' ] ); + } + + /** + * Whether the current user has this capability. + * + * @return bool + */ + public function user_can(): bool { + return current_user_can( $this->capability ); // phpcs:ignore WordPress.WP.Capabilities.Unknown -- Custom capability, synced by this class. + } + + /** + * A REST route permission_callback closure for this capability, so + * routes can pass this directly instead of wrapping current_user_can() + * in their own inline closure. + * + * @return callable + */ + public function permission_callback(): callable { + return function () { + return $this->user_can(); + }; + } + + /** + * Map_meta_cap callback: manage_options users always pass a check + * against this capability, regardless of role sync. + * + * @param array $caps Required primitive capabilities. + * @param string $cap Capability being checked. + * @param int $user_id User ID. + * @return array + */ + public function bypass_for_admins( $caps, $cap, $user_id ) { + if ( $this->capability === $cap && user_can( $user_id, 'manage_options' ) ) { + return []; + } + return $caps; + } + + /** + * Add or remove this capability on every role so it matches exactly + * the role list passed in. + * + * @param mixed $roles Role slugs that should have the capability. + * @return void + */ + public function sync( $roles ): void { + $roles = is_array( $roles ) ? $roles : []; + + foreach ( wp_roles()->role_objects as $role_slug => $role ) { + if ( in_array( $role_slug, $roles, true ) ) { + $role->add_cap( $this->capability ); + } else { + $role->remove_cap( $this->capability ); + } + } + } + + /** + * Run the sync once per migration version. Covers two cases: a site + * that already had option_name set before this capability existed + * (needs an initial sync), and a site whose stored version predates a + * default_roles change (needs a re-sync even though it already ran an + * earlier version's migration once). + * + * @return void + */ + public function maybe_migrate(): void { + $version_option = "edac_capability_version_{$this->capability}"; + $stored_version = (int) get_option( $version_option, 0 ); + + if ( $stored_version >= $this->version ) { + return; + } + + $this->sync( get_option( $this->option_name, $this->default_roles ) ); + update_option( $version_option, $this->version ); + } +} From f12e49d521ced21d2b9782a34e2159664b2a1006 Mon Sep 17 00:00:00 2001 From: pattonwebz Date: Tue, 28 Jul 2026 20:13:00 +0100 Subject: [PATCH 08/23] Retrofit edac_ignore_issues onto Synced_Capability MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- includes/options-page.php | 93 +++++-------------- .../phpunit/includes/IgnoreCapabilityTest.php | 10 +- 2 files changed, 29 insertions(+), 74 deletions(-) diff --git a/includes/options-page.php b/includes/options-page.php index 6f9850241..c61ed969a 100644 --- a/includes/options-page.php +++ b/includes/options-page.php @@ -10,91 +10,46 @@ use EDAC\Admin\Settings; use EDAC\Inc\Accessibility_Statement; use EqualizeDigital\AccessibilityChecker\Admin\AdminPage\FixesPage; +use EqualizeDigital\AccessibilityChecker\Capabilities\Synced_Capability; if ( ! defined( 'ABSPATH' ) ) { exit; } /** - * Check if user can ignore or can manage options - * - * @return bool - */ -function edac_user_can_ignore() { - return current_user_can( 'edac_ignore_issues' ); // phpcs:ignore WordPress.WP.Capabilities.Unknown -- This is a custom capability, synced from the edacp_ignore_user_roles setting. -} - -/** - * Let `manage_options` users always pass an `edac_ignore_issues` check, - * regardless of whether their role is currently in `edacp_ignore_user_roles`. - * Keeps `current_user_can( 'edac_ignore_issues' )` as the single check every - * call site (REST, AJAX, menu registration) can rely on. + * The edac_ignore_issues capability, synced onto the roles listed in the + * edacp_ignore_user_roles option (set on the pro plugin's settings page), + * with a manage_options bypass. current_user_can( 'edac_ignore_issues' ) is + * the single check every call site (REST, AJAX, menu registration) relies + * on instead of comparing against the option directly. * - * @param array $caps Required primitive capabilities. - * @param string $cap Capability being checked. - * @param int $user_id User ID. - * @return array + * @return Synced_Capability */ -function edac_map_ignore_capability( $caps, $cap, $user_id ) { - if ( 'edac_ignore_issues' === $cap && user_can( $user_id, 'manage_options' ) ) { - return []; +function edac_ignore_capability(): Synced_Capability { + static $capability = null; + + if ( null === $capability ) { + $capability = new Synced_Capability( + 'edac_ignore_issues', + 'edacp_ignore_user_roles', + [ 'administrator' ], + 1 + ); + $capability->register(); } - return $caps; -} -add_filter( 'map_meta_cap', 'edac_map_ignore_capability', 10, 3 ); -/** - * Sync the `edac_ignore_issues` capability onto exactly the roles allowed - * to dismiss/ignore issues, so `current_user_can( 'edac_ignore_issues' )` - * is always the source of truth rather than comparing against the option - * directly. - * - * @param array $roles Role slugs that should have the capability. - * @return void - */ -function edac_sync_ignore_capability( $roles ) { - $roles = is_array( $roles ) ? $roles : []; - - foreach ( wp_roles()->role_objects as $role_slug => $role ) { - if ( in_array( $role_slug, $roles, true ) ) { - $role->add_cap( 'edac_ignore_issues' ); - } else { - $role->remove_cap( 'edac_ignore_issues' ); - } - } + return $capability; } -add_action( - 'add_option_edacp_ignore_user_roles', - function ( $option, $value ) { - edac_sync_ignore_capability( $value ); - }, - 10, - 2 -); -add_action( - 'update_option_edacp_ignore_user_roles', - function ( $old_value, $value ) { - edac_sync_ignore_capability( $value ); - }, - 10, - 2 -); +edac_ignore_capability(); /** - * One-time migration: existing installs already have `edacp_ignore_user_roles` - * saved but never had it synced onto a real capability. Run once per site. + * Check if user can ignore or can manage options * - * @return void + * @return bool */ -function edac_maybe_migrate_ignore_capability() { - if ( get_option( 'edac_ignore_cap_synced' ) ) { - return; - } - - edac_sync_ignore_capability( get_option( 'edacp_ignore_user_roles', [ 'administrator' ] ) ); - update_option( 'edac_ignore_cap_synced', 1 ); +function edac_user_can_ignore() { + return edac_ignore_capability()->user_can(); } -add_action( 'admin_init', 'edac_maybe_migrate_ignore_capability' ); /** * Add an options page under the Settings submenu diff --git a/tests/phpunit/includes/IgnoreCapabilityTest.php b/tests/phpunit/includes/IgnoreCapabilityTest.php index 1ba7701dd..291e84eca 100644 --- a/tests/phpunit/includes/IgnoreCapabilityTest.php +++ b/tests/phpunit/includes/IgnoreCapabilityTest.php @@ -6,7 +6,7 @@ */ /** - * Tests for edac_sync_ignore_capability(), edac_user_can_ignore(), and the + * Tests for the Synced_Capability-backed edac_user_can_ignore() and the * map_meta_cap manage_options override. */ class IgnoreCapabilityTest extends WP_UnitTestCase { @@ -31,7 +31,7 @@ public function tearDown(): void { public function test_sync_adds_and_removes_capability_by_role() { wp_roles()->get_role( 'editor' )->add_cap( 'edac_ignore_issues' ); - edac_sync_ignore_capability( [ 'author' ] ); + edac_ignore_capability()->sync( [ 'author' ] ); $this->assertFalse( wp_roles()->get_role( 'editor' )->has_cap( 'edac_ignore_issues' ), 'Editor should have lost the capability.' ); $this->assertTrue( wp_roles()->get_role( 'author' )->has_cap( 'edac_ignore_issues' ), 'Author should have gained the capability.' ); @@ -44,7 +44,7 @@ public function test_sync_adds_and_removes_capability_by_role() { * @return void */ public function test_user_can_ignore_true_for_synced_role() { - edac_sync_ignore_capability( [ 'author' ] ); + edac_ignore_capability()->sync( [ 'author' ] ); $user_id = self::factory()->user->create( [ 'role' => 'author' ] ); wp_set_current_user( $user_id ); @@ -59,7 +59,7 @@ public function test_user_can_ignore_true_for_synced_role() { * @return void */ public function test_user_can_ignore_false_for_unsynced_role() { - edac_sync_ignore_capability( [ 'author' ] ); + edac_ignore_capability()->sync( [ 'author' ] ); $user_id = self::factory()->user->create( [ 'role' => 'editor' ] ); wp_set_current_user( $user_id ); @@ -75,7 +75,7 @@ public function test_user_can_ignore_false_for_unsynced_role() { * @return void */ public function test_manage_options_user_always_can_ignore() { - edac_sync_ignore_capability( [ 'author' ] ); + edac_ignore_capability()->sync( [ 'author' ] ); $user_id = self::factory()->user->create( [ 'role' => 'administrator' ] ); wp_set_current_user( $user_id ); From 290778b979887cf205440e7e0d4e9c6d51cbd85d Mon Sep 17 00:00:00 2001 From: pattonwebz Date: Tue, 28 Jul 2026 20:13:08 +0100 Subject: [PATCH 09/23] Add standalone tests for Synced_Capability 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 --- .../Capabilities/SyncedCapabilityTest.php | 174 ++++++++++++++++++ 1 file changed, 174 insertions(+) create mode 100644 tests/phpunit/includes/classes/Capabilities/SyncedCapabilityTest.php diff --git a/tests/phpunit/includes/classes/Capabilities/SyncedCapabilityTest.php b/tests/phpunit/includes/classes/Capabilities/SyncedCapabilityTest.php new file mode 100644 index 000000000..2f1c22086 --- /dev/null +++ b/tests/phpunit/includes/classes/Capabilities/SyncedCapabilityTest.php @@ -0,0 +1,174 @@ +role_objects as $role ) { + $role->remove_cap( self::TEST_CAP ); + } + delete_option( self::TEST_OPTION ); + delete_option( 'edac_capability_version_' . self::TEST_CAP ); + wp_set_current_user( 0 ); + parent::tearDown(); + } + + /** + * Sync() should add the capability only to the roles passed in, and + * remove it from roles not included. + * + * @return void + */ + public function test_sync_adds_and_removes_capability_by_role() { + $capability = new Synced_Capability( self::TEST_CAP, self::TEST_OPTION ); + + wp_roles()->get_role( 'editor' )->add_cap( self::TEST_CAP ); + + $capability->sync( [ 'author' ] ); + + $this->assertFalse( wp_roles()->get_role( 'editor' )->has_cap( self::TEST_CAP ) ); + $this->assertTrue( wp_roles()->get_role( 'author' )->has_cap( self::TEST_CAP ) ); + } + + /** + * Register() should wire live sync to the option's add/update hooks. + * + * @return void + */ + public function test_register_syncs_on_option_save() { + $capability = new Synced_Capability( self::TEST_CAP, self::TEST_OPTION ); + $capability->register(); + + add_option( self::TEST_OPTION, [ 'author' ] ); + $this->assertTrue( wp_roles()->get_role( 'author' )->has_cap( self::TEST_CAP ) ); + + update_option( self::TEST_OPTION, [ 'editor' ] ); + $this->assertFalse( wp_roles()->get_role( 'author' )->has_cap( self::TEST_CAP ) ); + $this->assertTrue( wp_roles()->get_role( 'editor' )->has_cap( self::TEST_CAP ) ); + } + + /** + * Manage_options users must always pass user_can(), regardless of + * whether their role was synced. + * + * @return void + */ + public function test_manage_options_bypasses_sync() { + $capability = new Synced_Capability( self::TEST_CAP, self::TEST_OPTION ); + $capability->register(); + $capability->sync( [ 'author' ] ); + + $admin_id = self::factory()->user->create( [ 'role' => 'administrator' ] ); + wp_set_current_user( $admin_id ); + + $this->assertFalse( wp_roles()->get_role( 'administrator' )->has_cap( self::TEST_CAP ), 'Precondition: administrator role itself was not synced.' ); + $this->assertTrue( $capability->user_can() ); + } + + /** + * Permission_callback() should return a callable proxying user_can(), + * suitable for a REST route's permission_callback directly. + * + * @return void + */ + public function test_permission_callback_proxies_user_can() { + $capability = new Synced_Capability( self::TEST_CAP, self::TEST_OPTION ); + $capability->sync( [ 'author' ] ); + + $callback = $capability->permission_callback(); + $this->assertIsCallable( $callback ); + + $author_id = self::factory()->user->create( [ 'role' => 'author' ] ); + wp_set_current_user( $author_id ); + $this->assertTrue( $callback() ); + + $subscriber_id = self::factory()->user->create( [ 'role' => 'subscriber' ] ); + wp_set_current_user( $subscriber_id ); + $this->assertFalse( $callback() ); + } + + /** + * Maybe_migrate() should run the initial sync from default_roles when + * the option was never set and no migration has run yet. + * + * @return void + */ + public function test_migration_runs_once_for_unset_option() { + $capability = new Synced_Capability( self::TEST_CAP, self::TEST_OPTION, [ 'editor' ] ); + + $capability->maybe_migrate(); + + $this->assertTrue( wp_roles()->get_role( 'editor' )->has_cap( self::TEST_CAP ) ); + } + + /** + * Maybe_migrate() should not re-run (and shouldn't clobber roles synced + * some other way since) once it has already run for the current version. + * + * @return void + */ + public function test_migration_does_not_rerun_for_same_version() { + $capability = new Synced_Capability( self::TEST_CAP, self::TEST_OPTION, [ 'editor' ] ); + $capability->maybe_migrate(); + + // Simulate the site's config changing after the one-time migration ran. + $capability->sync( [ 'author' ] ); + + $capability->maybe_migrate(); + + $this->assertFalse( wp_roles()->get_role( 'editor' )->has_cap( self::TEST_CAP ), 'Migration should not have re-applied default_roles.' ); + $this->assertTrue( wp_roles()->get_role( 'author' )->has_cap( self::TEST_CAP ) ); + } + + /** + * Bumping the version should force maybe_migrate() to re-sync even + * though an earlier version's migration already ran once. + * + * @return void + */ + public function test_version_bump_forces_remigration() { + $v1 = new Synced_Capability( self::TEST_CAP, self::TEST_OPTION, [ 'editor' ], 1 ); + $v1->maybe_migrate(); + + // Site never saved the option, so it's still on default_roles from v1. + $this->assertTrue( wp_roles()->get_role( 'editor' )->has_cap( self::TEST_CAP ) ); + + $v2 = new Synced_Capability( self::TEST_CAP, self::TEST_OPTION, [ 'author' ], 2 ); + $v2->maybe_migrate(); + + $this->assertTrue( wp_roles()->get_role( 'author' )->has_cap( self::TEST_CAP ), 'v2 default_roles should have been applied.' ); + $this->assertFalse( wp_roles()->get_role( 'editor' )->has_cap( self::TEST_CAP ), 'v1 default_roles should no longer apply after the v2 re-sync.' ); + } +} From 4efe2e88700320d1b03efa1dc6fe63319a497644 Mon Sep 17 00:00:00 2001 From: pattonwebz Date: Tue, 28 Jul 2026 22:27:03 +0100 Subject: [PATCH 10/23] Add User_Capability_Grant for direct per-user capability grants MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .../Capabilities/User_Capability_Grant.php | 123 ++++++++++++++++++ 1 file changed, 123 insertions(+) create mode 100644 includes/classes/Capabilities/User_Capability_Grant.php diff --git a/includes/classes/Capabilities/User_Capability_Grant.php b/includes/classes/Capabilities/User_Capability_Grant.php new file mode 100644 index 000000000..4609a9a73 --- /dev/null +++ b/includes/classes/Capabilities/User_Capability_Grant.php @@ -0,0 +1,123 @@ +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(), + ] + ); + + return true; + } + + /** + * Revoke a capability that was granted directly to a user (does not + * affect a capability the user has via their role). + * + * @param int $user_id User to revoke the capability from. + * @param string $capability Capability string to revoke. + * @return bool True if the user was found and the revoke was applied. + */ + public static function revoke( int $user_id, string $capability ): bool { + $user = get_userdata( $user_id ); + if ( ! $user ) { + return false; + } + + $user->remove_cap( $capability ); + delete_user_meta( $user_id, self::META_PREFIX . $capability ); + + return true; + } + + /** + * Get attribution for a capability directly granted to a user via + * grant(), for display in an admin UI (e.g. "Granted by X on Y"). + * Returns null if the capability was never granted through this class + * (including if the user only has it via their role). + * + * @param int $user_id User to check. + * @param string $capability Capability string to check. + * @return array{granted_by: int, granted_at: int}|null + */ + public static function get_grant_info( int $user_id, string $capability ): ?array { + $meta = get_user_meta( $user_id, self::META_PREFIX . $capability, true ); + + return $meta ? $meta : null; + } + + /** + * Whether a capability was added directly to this user (as opposed to + * inherited from one of their roles). + * + * $user->caps holds only what was assigned directly to this user (role + * slugs plus any directly-added capabilities); $user->allcaps is the + * merged result including everything resolved from their roles. Using + * $user->caps here is what makes this "direct grant," not "has it at + * all," correctly distinct from user_can()/current_user_can(). + * + * @param int $user_id User to check. + * @param string $capability Capability string to check. + * @return bool + */ + public static function is_individually_granted( int $user_id, string $capability ): bool { + $user = get_userdata( $user_id ); + + return $user instanceof \WP_User && ! empty( $user->caps[ $capability ] ); + } +} From d81b817ab3a4a6006f0530ce97cab741fc64f778 Mon Sep 17 00:00:00 2001 From: pattonwebz Date: Tue, 28 Jul 2026 22:27:12 +0100 Subject: [PATCH 11/23] Add tests for User_Capability_Grant 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 --- .../Capabilities/UserCapabilityGrantTest.php | 190 ++++++++++++++++++ 1 file changed, 190 insertions(+) create mode 100644 tests/phpunit/includes/classes/Capabilities/UserCapabilityGrantTest.php diff --git a/tests/phpunit/includes/classes/Capabilities/UserCapabilityGrantTest.php b/tests/phpunit/includes/classes/Capabilities/UserCapabilityGrantTest.php new file mode 100644 index 000000000..01bca1b04 --- /dev/null +++ b/tests/phpunit/includes/classes/Capabilities/UserCapabilityGrantTest.php @@ -0,0 +1,190 @@ +role_objects as $role ) { + $role->remove_cap( self::TEST_CAP ); + } + wp_set_current_user( 0 ); + parent::tearDown(); + } + + /** + * Granting a capability to a user should make user_can() true for that + * user, without needing their role to have the capability at all. + * + * @return void + */ + public function test_grant_makes_user_can_true() { + $user_id = self::factory()->user->create( [ 'role' => 'subscriber' ] ); + + $this->assertFalse( user_can( $user_id, self::TEST_CAP ) ); + + User_Capability_Grant::grant( $user_id, self::TEST_CAP ); + + $this->assertTrue( user_can( $user_id, self::TEST_CAP ) ); + } + + /** + * Revoking a directly-granted capability should make user_can() false + * again. + * + * @return void + */ + public function test_revoke_removes_the_grant() { + $user_id = self::factory()->user->create( [ 'role' => 'subscriber' ] ); + + User_Capability_Grant::grant( $user_id, self::TEST_CAP ); + $this->assertTrue( user_can( $user_id, self::TEST_CAP ) ); + + User_Capability_Grant::revoke( $user_id, self::TEST_CAP ); + $this->assertFalse( user_can( $user_id, self::TEST_CAP ) ); + } + + /** + * Granting should record who granted it and roughly when. + * + * @return void + */ + public function test_grant_records_attribution() { + $granter_id = self::factory()->user->create( [ 'role' => 'administrator' ] ); + $user_id = self::factory()->user->create( [ 'role' => 'subscriber' ] ); + + User_Capability_Grant::grant( $user_id, self::TEST_CAP, $granter_id ); + + $info = User_Capability_Grant::get_grant_info( $user_id, self::TEST_CAP ); + + $this->assertIsArray( $info ); + $this->assertSame( $granter_id, $info['granted_by'] ); + $this->assertEqualsWithDelta( time(), $info['granted_at'], 5 ); + } + + /** + * With no explicit granter passed, attribution should default to the + * current user. + * + * @return void + */ + public function test_grant_defaults_attribution_to_current_user() { + $granter_id = self::factory()->user->create( [ 'role' => 'administrator' ] ); + $user_id = self::factory()->user->create( [ 'role' => 'subscriber' ] ); + + wp_set_current_user( $granter_id ); + User_Capability_Grant::grant( $user_id, self::TEST_CAP ); + + $info = User_Capability_Grant::get_grant_info( $user_id, self::TEST_CAP ); + + $this->assertSame( $granter_id, $info['granted_by'] ); + } + + /** + * Revoking should clear the attribution record, not just the WordPress + * capability itself. + * + * @return void + */ + public function test_revoke_clears_attribution() { + $user_id = self::factory()->user->create( [ 'role' => 'subscriber' ] ); + + User_Capability_Grant::grant( $user_id, self::TEST_CAP ); + User_Capability_Grant::revoke( $user_id, self::TEST_CAP ); + + $this->assertNull( User_Capability_Grant::get_grant_info( $user_id, self::TEST_CAP ) ); + } + + /** + * Get_grant_info() should be null for a user who was never individually + * granted the capability, even if they can() it via their role. + * + * @return void + */ + public function test_grant_info_null_when_capability_only_from_role() { + $user_id = self::factory()->user->create( [ 'role' => 'editor' ] ); + wp_roles()->get_role( 'editor' )->add_cap( self::TEST_CAP ); + + $this->assertTrue( user_can( $user_id, self::TEST_CAP ), 'Precondition: user has the capability via their role.' ); + $this->assertNull( User_Capability_Grant::get_grant_info( $user_id, self::TEST_CAP ) ); + } + + /** + * Is_individually_granted() should distinguish "granted directly to + * this user" from "has it via their role" — both should pass + * user_can(), but only the direct grant should read as individually + * granted. + * + * @return void + */ + public function test_is_individually_granted_distinguishes_from_role_capability() { + $role_user_id = self::factory()->user->create( [ 'role' => 'editor' ] ); + $granted_user_id = self::factory()->user->create( [ 'role' => 'subscriber' ] ); + + wp_roles()->get_role( 'editor' )->add_cap( self::TEST_CAP ); + User_Capability_Grant::grant( $granted_user_id, self::TEST_CAP ); + + $this->assertTrue( user_can( $role_user_id, self::TEST_CAP ) ); + $this->assertFalse( User_Capability_Grant::is_individually_granted( $role_user_id, self::TEST_CAP ), 'Role-derived capability is not an individual grant.' ); + + $this->assertTrue( user_can( $granted_user_id, self::TEST_CAP ) ); + $this->assertTrue( User_Capability_Grant::is_individually_granted( $granted_user_id, self::TEST_CAP ) ); + } + + /** + * A capability granted directly to a user must survive a role-level + * sync that removes the capability from that user's role — this is the + * core coexistence guarantee with Synced_Capability (or any other + * role-only sync), proven here without depending on that class. + * + * @return void + */ + public function test_direct_grant_survives_role_level_capability_removal() { + $user_id = self::factory()->user->create( [ 'role' => 'editor' ] ); + + wp_roles()->get_role( 'editor' )->add_cap( self::TEST_CAP ); + User_Capability_Grant::grant( $user_id, self::TEST_CAP ); + + // Simulate a role-level sync (like Synced_Capability::sync()) that + // decides 'editor' should no longer have this capability. + wp_roles()->get_role( 'editor' )->remove_cap( self::TEST_CAP ); + + $this->assertTrue( user_can( $user_id, self::TEST_CAP ), 'Direct grant should survive removal of the capability from the user\'s role.' ); + } + + /** + * Granting/revoking for a user ID that doesn't exist should fail + * gracefully rather than erroring. + * + * @return void + */ + public function test_grant_and_revoke_return_false_for_nonexistent_user() { + $bogus_id = 999999; + + $this->assertFalse( User_Capability_Grant::grant( $bogus_id, self::TEST_CAP ) ); + $this->assertFalse( User_Capability_Grant::revoke( $bogus_id, self::TEST_CAP ) ); + } +} From 5ade14608f9ad6dd96b0b09b6f9906b6b2c1f948 Mon Sep 17 00:00:00 2001 From: pattonwebz Date: Tue, 28 Jul 2026 23:12:41 +0100 Subject: [PATCH 12/23] Rename Synced_Capability -> SyncCapability, User_Capability_Grant -> UserCapabilityGrant MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- ...nced_Capability.php => SyncCapability.php} | 2 +- ...lity_Grant.php => UserCapabilityGrant.php} | 2 +- includes/options-page.php | 8 ++-- .../phpunit/includes/IgnoreCapabilityTest.php | 2 +- ...abilityTest.php => SyncCapabilityTest.php} | 22 +++++------ .../Capabilities/UserCapabilityGrantTest.php | 38 +++++++++---------- 6 files changed, 37 insertions(+), 37 deletions(-) rename includes/classes/Capabilities/{Synced_Capability.php => SyncCapability.php} (99%) rename includes/classes/Capabilities/{User_Capability_Grant.php => UserCapabilityGrant.php} (99%) rename tests/phpunit/includes/classes/Capabilities/{SyncedCapabilityTest.php => SyncCapabilityTest.php} (85%) diff --git a/includes/classes/Capabilities/Synced_Capability.php b/includes/classes/Capabilities/SyncCapability.php similarity index 99% rename from includes/classes/Capabilities/Synced_Capability.php rename to includes/classes/Capabilities/SyncCapability.php index b2b58b4cc..a8586876b 100644 --- a/includes/classes/Capabilities/Synced_Capability.php +++ b/includes/classes/Capabilities/SyncCapability.php @@ -18,7 +18,7 @@ * routes, future role-configurable features) can reuse the same pattern * instead of re-implementing the sync/bypass/migration trio each time. */ -class Synced_Capability { +class SyncCapability { /** * The capability string, e.g. 'edac_ignore_issues'. diff --git a/includes/classes/Capabilities/User_Capability_Grant.php b/includes/classes/Capabilities/UserCapabilityGrant.php similarity index 99% rename from includes/classes/Capabilities/User_Capability_Grant.php rename to includes/classes/Capabilities/UserCapabilityGrant.php index 4609a9a73..2309c9853 100644 --- a/includes/classes/Capabilities/User_Capability_Grant.php +++ b/includes/classes/Capabilities/UserCapabilityGrant.php @@ -27,7 +27,7 @@ * needs no new code. What WordPress has no concept of is *who* granted a * capability and *when*; that attribution is what this class adds. */ -class User_Capability_Grant { +class UserCapabilityGrant { /** * User meta key prefix under which grant attribution is stored, per diff --git a/includes/options-page.php b/includes/options-page.php index c61ed969a..2b3b98986 100644 --- a/includes/options-page.php +++ b/includes/options-page.php @@ -10,7 +10,7 @@ use EDAC\Admin\Settings; use EDAC\Inc\Accessibility_Statement; use EqualizeDigital\AccessibilityChecker\Admin\AdminPage\FixesPage; -use EqualizeDigital\AccessibilityChecker\Capabilities\Synced_Capability; +use EqualizeDigital\AccessibilityChecker\Capabilities\SyncCapability; if ( ! defined( 'ABSPATH' ) ) { exit; @@ -23,13 +23,13 @@ * the single check every call site (REST, AJAX, menu registration) relies * on instead of comparing against the option directly. * - * @return Synced_Capability + * @return SyncCapability */ -function edac_ignore_capability(): Synced_Capability { +function edac_ignore_capability(): SyncCapability { static $capability = null; if ( null === $capability ) { - $capability = new Synced_Capability( + $capability = new SyncCapability( 'edac_ignore_issues', 'edacp_ignore_user_roles', [ 'administrator' ], diff --git a/tests/phpunit/includes/IgnoreCapabilityTest.php b/tests/phpunit/includes/IgnoreCapabilityTest.php index 291e84eca..8d49c0af2 100644 --- a/tests/phpunit/includes/IgnoreCapabilityTest.php +++ b/tests/phpunit/includes/IgnoreCapabilityTest.php @@ -6,7 +6,7 @@ */ /** - * Tests for the Synced_Capability-backed edac_user_can_ignore() and the + * Tests for the SyncCapability-backed edac_user_can_ignore() and the * map_meta_cap manage_options override. */ class IgnoreCapabilityTest extends WP_UnitTestCase { diff --git a/tests/phpunit/includes/classes/Capabilities/SyncedCapabilityTest.php b/tests/phpunit/includes/classes/Capabilities/SyncCapabilityTest.php similarity index 85% rename from tests/phpunit/includes/classes/Capabilities/SyncedCapabilityTest.php rename to tests/phpunit/includes/classes/Capabilities/SyncCapabilityTest.php index 2f1c22086..4171bf0b8 100644 --- a/tests/phpunit/includes/classes/Capabilities/SyncedCapabilityTest.php +++ b/tests/phpunit/includes/classes/Capabilities/SyncCapabilityTest.php @@ -1,11 +1,11 @@ get_role( 'editor' )->add_cap( self::TEST_CAP ); @@ -68,7 +68,7 @@ public function test_sync_adds_and_removes_capability_by_role() { * @return void */ public function test_register_syncs_on_option_save() { - $capability = new Synced_Capability( self::TEST_CAP, self::TEST_OPTION ); + $capability = new SyncCapability( self::TEST_CAP, self::TEST_OPTION ); $capability->register(); add_option( self::TEST_OPTION, [ 'author' ] ); @@ -86,7 +86,7 @@ public function test_register_syncs_on_option_save() { * @return void */ public function test_manage_options_bypasses_sync() { - $capability = new Synced_Capability( self::TEST_CAP, self::TEST_OPTION ); + $capability = new SyncCapability( self::TEST_CAP, self::TEST_OPTION ); $capability->register(); $capability->sync( [ 'author' ] ); @@ -104,7 +104,7 @@ public function test_manage_options_bypasses_sync() { * @return void */ public function test_permission_callback_proxies_user_can() { - $capability = new Synced_Capability( self::TEST_CAP, self::TEST_OPTION ); + $capability = new SyncCapability( self::TEST_CAP, self::TEST_OPTION ); $capability->sync( [ 'author' ] ); $callback = $capability->permission_callback(); @@ -126,7 +126,7 @@ public function test_permission_callback_proxies_user_can() { * @return void */ public function test_migration_runs_once_for_unset_option() { - $capability = new Synced_Capability( self::TEST_CAP, self::TEST_OPTION, [ 'editor' ] ); + $capability = new SyncCapability( self::TEST_CAP, self::TEST_OPTION, [ 'editor' ] ); $capability->maybe_migrate(); @@ -140,7 +140,7 @@ public function test_migration_runs_once_for_unset_option() { * @return void */ public function test_migration_does_not_rerun_for_same_version() { - $capability = new Synced_Capability( self::TEST_CAP, self::TEST_OPTION, [ 'editor' ] ); + $capability = new SyncCapability( self::TEST_CAP, self::TEST_OPTION, [ 'editor' ] ); $capability->maybe_migrate(); // Simulate the site's config changing after the one-time migration ran. @@ -159,13 +159,13 @@ public function test_migration_does_not_rerun_for_same_version() { * @return void */ public function test_version_bump_forces_remigration() { - $v1 = new Synced_Capability( self::TEST_CAP, self::TEST_OPTION, [ 'editor' ], 1 ); + $v1 = new SyncCapability( self::TEST_CAP, self::TEST_OPTION, [ 'editor' ], 1 ); $v1->maybe_migrate(); // Site never saved the option, so it's still on default_roles from v1. $this->assertTrue( wp_roles()->get_role( 'editor' )->has_cap( self::TEST_CAP ) ); - $v2 = new Synced_Capability( self::TEST_CAP, self::TEST_OPTION, [ 'author' ], 2 ); + $v2 = new SyncCapability( self::TEST_CAP, self::TEST_OPTION, [ 'author' ], 2 ); $v2->maybe_migrate(); $this->assertTrue( wp_roles()->get_role( 'author' )->has_cap( self::TEST_CAP ), 'v2 default_roles should have been applied.' ); diff --git a/tests/phpunit/includes/classes/Capabilities/UserCapabilityGrantTest.php b/tests/phpunit/includes/classes/Capabilities/UserCapabilityGrantTest.php index 01bca1b04..a3fa973e8 100644 --- a/tests/phpunit/includes/classes/Capabilities/UserCapabilityGrantTest.php +++ b/tests/phpunit/includes/classes/Capabilities/UserCapabilityGrantTest.php @@ -1,11 +1,11 @@ assertFalse( user_can( $user_id, self::TEST_CAP ) ); - User_Capability_Grant::grant( $user_id, self::TEST_CAP ); + UserCapabilityGrant::grant( $user_id, self::TEST_CAP ); $this->assertTrue( user_can( $user_id, self::TEST_CAP ) ); } @@ -60,10 +60,10 @@ public function test_grant_makes_user_can_true() { public function test_revoke_removes_the_grant() { $user_id = self::factory()->user->create( [ 'role' => 'subscriber' ] ); - User_Capability_Grant::grant( $user_id, self::TEST_CAP ); + UserCapabilityGrant::grant( $user_id, self::TEST_CAP ); $this->assertTrue( user_can( $user_id, self::TEST_CAP ) ); - User_Capability_Grant::revoke( $user_id, self::TEST_CAP ); + UserCapabilityGrant::revoke( $user_id, self::TEST_CAP ); $this->assertFalse( user_can( $user_id, self::TEST_CAP ) ); } @@ -76,9 +76,9 @@ public function test_grant_records_attribution() { $granter_id = self::factory()->user->create( [ 'role' => 'administrator' ] ); $user_id = self::factory()->user->create( [ 'role' => 'subscriber' ] ); - User_Capability_Grant::grant( $user_id, self::TEST_CAP, $granter_id ); + UserCapabilityGrant::grant( $user_id, self::TEST_CAP, $granter_id ); - $info = User_Capability_Grant::get_grant_info( $user_id, self::TEST_CAP ); + $info = UserCapabilityGrant::get_grant_info( $user_id, self::TEST_CAP ); $this->assertIsArray( $info ); $this->assertSame( $granter_id, $info['granted_by'] ); @@ -96,9 +96,9 @@ public function test_grant_defaults_attribution_to_current_user() { $user_id = self::factory()->user->create( [ 'role' => 'subscriber' ] ); wp_set_current_user( $granter_id ); - User_Capability_Grant::grant( $user_id, self::TEST_CAP ); + UserCapabilityGrant::grant( $user_id, self::TEST_CAP ); - $info = User_Capability_Grant::get_grant_info( $user_id, self::TEST_CAP ); + $info = UserCapabilityGrant::get_grant_info( $user_id, self::TEST_CAP ); $this->assertSame( $granter_id, $info['granted_by'] ); } @@ -112,10 +112,10 @@ public function test_grant_defaults_attribution_to_current_user() { public function test_revoke_clears_attribution() { $user_id = self::factory()->user->create( [ 'role' => 'subscriber' ] ); - User_Capability_Grant::grant( $user_id, self::TEST_CAP ); - User_Capability_Grant::revoke( $user_id, self::TEST_CAP ); + UserCapabilityGrant::grant( $user_id, self::TEST_CAP ); + UserCapabilityGrant::revoke( $user_id, self::TEST_CAP ); - $this->assertNull( User_Capability_Grant::get_grant_info( $user_id, self::TEST_CAP ) ); + $this->assertNull( UserCapabilityGrant::get_grant_info( $user_id, self::TEST_CAP ) ); } /** @@ -129,7 +129,7 @@ public function test_grant_info_null_when_capability_only_from_role() { wp_roles()->get_role( 'editor' )->add_cap( self::TEST_CAP ); $this->assertTrue( user_can( $user_id, self::TEST_CAP ), 'Precondition: user has the capability via their role.' ); - $this->assertNull( User_Capability_Grant::get_grant_info( $user_id, self::TEST_CAP ) ); + $this->assertNull( UserCapabilityGrant::get_grant_info( $user_id, self::TEST_CAP ) ); } /** @@ -145,13 +145,13 @@ public function test_is_individually_granted_distinguishes_from_role_capability( $granted_user_id = self::factory()->user->create( [ 'role' => 'subscriber' ] ); wp_roles()->get_role( 'editor' )->add_cap( self::TEST_CAP ); - User_Capability_Grant::grant( $granted_user_id, self::TEST_CAP ); + UserCapabilityGrant::grant( $granted_user_id, self::TEST_CAP ); $this->assertTrue( user_can( $role_user_id, self::TEST_CAP ) ); - $this->assertFalse( User_Capability_Grant::is_individually_granted( $role_user_id, self::TEST_CAP ), 'Role-derived capability is not an individual grant.' ); + $this->assertFalse( UserCapabilityGrant::is_individually_granted( $role_user_id, self::TEST_CAP ), 'Role-derived capability is not an individual grant.' ); $this->assertTrue( user_can( $granted_user_id, self::TEST_CAP ) ); - $this->assertTrue( User_Capability_Grant::is_individually_granted( $granted_user_id, self::TEST_CAP ) ); + $this->assertTrue( UserCapabilityGrant::is_individually_granted( $granted_user_id, self::TEST_CAP ) ); } /** @@ -166,7 +166,7 @@ public function test_direct_grant_survives_role_level_capability_removal() { $user_id = self::factory()->user->create( [ 'role' => 'editor' ] ); wp_roles()->get_role( 'editor' )->add_cap( self::TEST_CAP ); - User_Capability_Grant::grant( $user_id, self::TEST_CAP ); + UserCapabilityGrant::grant( $user_id, self::TEST_CAP ); // Simulate a role-level sync (like Synced_Capability::sync()) that // decides 'editor' should no longer have this capability. @@ -184,7 +184,7 @@ public function test_direct_grant_survives_role_level_capability_removal() { public function test_grant_and_revoke_return_false_for_nonexistent_user() { $bogus_id = 999999; - $this->assertFalse( User_Capability_Grant::grant( $bogus_id, self::TEST_CAP ) ); - $this->assertFalse( User_Capability_Grant::revoke( $bogus_id, self::TEST_CAP ) ); + $this->assertFalse( UserCapabilityGrant::grant( $bogus_id, self::TEST_CAP ) ); + $this->assertFalse( UserCapabilityGrant::revoke( $bogus_id, self::TEST_CAP ) ); } } From a76aa195f38da3cfd4b0b90996bfe6facee92dd3 Mon Sep 17 00:00:00 2001 From: pattonwebz Date: Wed, 29 Jul 2026 15:51:18 +0100 Subject: [PATCH 13/23] Generalize SyncCapability to a (role, capability) bundle primitive 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 --- .../Capabilities/CapabilityChecker.php | 46 ++++++ .../classes/Capabilities/SyncCapability.php | 135 ++++++++++++------ .../Capabilities/CapabilityCheckerTest.php | 99 +++++++++++++ .../Capabilities/SyncCapabilityTest.php | 105 +++++++++++++- 4 files changed, 337 insertions(+), 48 deletions(-) create mode 100644 includes/classes/Capabilities/CapabilityChecker.php create mode 100644 tests/phpunit/includes/classes/Capabilities/CapabilityCheckerTest.php diff --git a/includes/classes/Capabilities/CapabilityChecker.php b/includes/classes/Capabilities/CapabilityChecker.php new file mode 100644 index 000000000..0138bd415 --- /dev/null +++ b/includes/classes/Capabilities/CapabilityChecker.php @@ -0,0 +1,46 @@ +capability = $capability; + public function __construct( $capabilities, string $option_name, array $default_roles = [], int $version = 1 ) { + $this->capabilities = is_array( $capabilities ) ? array_values( $capabilities ) : [ $capabilities ]; $this->option_name = $option_name; $this->default_roles = $default_roles; $this->version = $version; @@ -98,30 +109,35 @@ function ( $old_value, $value ) { } /** - * Whether the current user has this capability. + * Whether the current user has one of this instance's capabilities. + * Defaults to the first (or only) capability in the bundle so existing + * single-capability callers can keep calling user_can() with no argument. * + * @param string|null $capability Which capability to check; defaults to the first in the bundle. * @return bool */ - public function user_can(): bool { - return current_user_can( $this->capability ); // phpcs:ignore WordPress.WP.Capabilities.Unknown -- Custom capability, synced by this class. + public function user_can( ?string $capability = null ): bool { + // phpcs:ignore WordPress.WP.Capabilities.Unknown -- Custom capability, synced by this class. + return current_user_can( $capability ?? $this->capabilities[0] ); } /** - * A REST route permission_callback closure for this capability, so - * routes can pass this directly instead of wrapping current_user_can() - * in their own inline closure. + * A REST route permission_callback closure for one of this instance's + * capabilities, so routes can pass this directly instead of wrapping + * current_user_can() in their own inline closure. * + * @param string|null $capability Which capability to check; defaults to the first in the bundle. * @return callable */ - public function permission_callback(): callable { - return function () { - return $this->user_can(); + public function permission_callback( ?string $capability = null ): callable { + return function () use ( $capability ) { + return $this->user_can( $capability ); }; } /** * Map_meta_cap callback: manage_options users always pass a check - * against this capability, regardless of role sync. + * against any capability in this bundle, regardless of role sync. * * @param array $caps Required primitive capabilities. * @param string $cap Capability being checked. @@ -129,42 +145,69 @@ public function permission_callback(): callable { * @return array */ public function bypass_for_admins( $caps, $cap, $user_id ) { - if ( $this->capability === $cap && user_can( $user_id, 'manage_options' ) ) { + if ( in_array( $cap, $this->capabilities, true ) && user_can( $user_id, 'manage_options' ) ) { return []; } return $caps; } /** - * Add or remove this capability on every role so it matches exactly - * the role list passed in. + * Add or remove one capability on one role. The generic primitive + * everything else in this class is built on - safe to call directly for + * a single (role, capability) pair outside of the option-driven sync. + * + * @param string $role_slug Role slug, e.g. 'editor'. + * @param string $capability Capability string. + * @param bool $should_have Whether the role should have this capability. + * @return void + */ + public function sync_role_capability( string $role_slug, string $capability, bool $should_have ): void { + $role = wp_roles()->get_role( $role_slug ); + + if ( ! $role ) { + return; + } + + if ( $should_have ) { + $role->add_cap( $capability ); + } else { + $role->remove_cap( $capability ); + } + } + + /** + * Add or remove every capability in this bundle on every role so each + * capability matches exactly the role list passed in. * - * @param mixed $roles Role slugs that should have the capability. + * @param mixed $roles Role slugs that should have the capabilities. * @return void */ public function sync( $roles ): void { $roles = is_array( $roles ) ? $roles : []; - foreach ( wp_roles()->role_objects as $role_slug => $role ) { - if ( in_array( $role_slug, $roles, true ) ) { - $role->add_cap( $this->capability ); - } else { - $role->remove_cap( $this->capability ); + foreach ( array_keys( wp_roles()->role_objects ) as $role_slug ) { + $should_have = in_array( $role_slug, $roles, true ); + + foreach ( $this->capabilities as $capability ) { + $this->sync_role_capability( $role_slug, $capability, $should_have ); } } } /** * Run the sync once per migration version. Covers two cases: a site - * that already had option_name set before this capability existed - * (needs an initial sync), and a site whose stored version predates a - * default_roles change (needs a re-sync even though it already ran an - * earlier version's migration once). + * that already had option_name set before this bundle existed (needs an + * initial sync), and a site whose stored version predates a + * default_roles/capabilities change (needs a re-sync even though it + * already ran an earlier version's migration). + * + * Versioned per option (not per capability), since every capability in + * the bundle is always granted together and shares one migration. * * @return void */ public function maybe_migrate(): void { - $version_option = "edac_capability_version_{$this->capability}"; + $version_option = "edac_capability_version_{$this->option_name}"; $stored_version = (int) get_option( $version_option, 0 ); if ( $stored_version >= $this->version ) { diff --git a/tests/phpunit/includes/classes/Capabilities/CapabilityCheckerTest.php b/tests/phpunit/includes/classes/Capabilities/CapabilityCheckerTest.php new file mode 100644 index 000000000..650856f6a --- /dev/null +++ b/tests/phpunit/includes/classes/Capabilities/CapabilityCheckerTest.php @@ -0,0 +1,99 @@ +role_objects as $role ) { + $role->remove_cap( self::TEST_CAP ); + } + wp_set_current_user( 0 ); + parent::tearDown(); + } + + /** + * User_can() should reflect a plain role-level capability grant, with no + * SyncCapability instance involved at all. + * + * @return void + */ + public function test_user_can_reflects_role_grant() { + wp_roles()->get_role( 'editor' )->add_cap( self::TEST_CAP ); + + $user_id = self::factory()->user->create( [ 'role' => 'editor' ] ); + wp_set_current_user( $user_id ); + + $this->assertTrue( CapabilityChecker::user_can( self::TEST_CAP ) ); + } + + /** + * User_can() should return false for a user whose role lacks the + * capability. + * + * @return void + */ + public function test_user_can_false_without_grant() { + $user_id = self::factory()->user->create( [ 'role' => 'subscriber' ] ); + wp_set_current_user( $user_id ); + + $this->assertFalse( CapabilityChecker::user_can( self::TEST_CAP ) ); + } + + /** + * User_can() should accept an explicit user ID instead of relying on the + * current user. + * + * @return void + */ + public function test_user_can_checks_explicit_user_id() { + wp_roles()->get_role( 'editor' )->add_cap( self::TEST_CAP ); + $user_id = self::factory()->user->create( [ 'role' => 'editor' ] ); + + $this->assertTrue( CapabilityChecker::user_can( self::TEST_CAP, $user_id ) ); + } + + /** + * Permission_callback() should return a callable proxying user_can(), + * suitable for a REST route's permission_callback directly. + * + * @return void + */ + public function test_permission_callback_proxies_user_can() { + wp_roles()->get_role( 'editor' )->add_cap( self::TEST_CAP ); + + $callback = CapabilityChecker::permission_callback( self::TEST_CAP ); + $this->assertIsCallable( $callback ); + + $user_id = self::factory()->user->create( [ 'role' => 'editor' ] ); + wp_set_current_user( $user_id ); + $this->assertTrue( $callback() ); + + $subscriber_id = self::factory()->user->create( [ 'role' => 'subscriber' ] ); + wp_set_current_user( $subscriber_id ); + $this->assertFalse( $callback() ); + } +} diff --git a/tests/phpunit/includes/classes/Capabilities/SyncCapabilityTest.php b/tests/phpunit/includes/classes/Capabilities/SyncCapabilityTest.php index 4171bf0b8..719cf4823 100644 --- a/tests/phpunit/includes/classes/Capabilities/SyncCapabilityTest.php +++ b/tests/phpunit/includes/classes/Capabilities/SyncCapabilityTest.php @@ -22,6 +22,14 @@ class SyncCapabilityTest extends WP_UnitTestCase { */ private const TEST_CAP = 'edac_test_synced_capability'; + /** + * A second capability string, used only by the multi-capability bundle + * tests in this class. + * + * @var string + */ + private const TEST_CAP_2 = 'edac_test_synced_capability_two'; + /** * Option name used only by this test class. * @@ -30,7 +38,7 @@ class SyncCapabilityTest extends WP_UnitTestCase { private const TEST_OPTION = 'edac_test_synced_capability_roles'; /** - * Remove the capability from every role and the option/migration + * Remove the capabilities from every role and the option/migration * markers after each test so they don't leak into each other. * * @return void @@ -38,9 +46,10 @@ class SyncCapabilityTest extends WP_UnitTestCase { public function tearDown(): void { foreach ( wp_roles()->role_objects as $role ) { $role->remove_cap( self::TEST_CAP ); + $role->remove_cap( self::TEST_CAP_2 ); } delete_option( self::TEST_OPTION ); - delete_option( 'edac_capability_version_' . self::TEST_CAP ); + delete_option( 'edac_capability_version_' . self::TEST_OPTION ); wp_set_current_user( 0 ); parent::tearDown(); } @@ -171,4 +180,96 @@ public function test_version_bump_forces_remigration() { $this->assertTrue( wp_roles()->get_role( 'author' )->has_cap( self::TEST_CAP ), 'v2 default_roles should have been applied.' ); $this->assertFalse( wp_roles()->get_role( 'editor' )->has_cap( self::TEST_CAP ), 'v1 default_roles should no longer apply after the v2 re-sync.' ); } + + /** + * A single string capability (the pre-bundle constructor signature) + * must still work unchanged, since existing single-capability callers + * pass a string, not an array. + * + * @return void + */ + public function test_single_string_capability_still_works() { + $capability = new SyncCapability( self::TEST_CAP, self::TEST_OPTION ); + $capability->sync( [ 'author' ] ); + + $this->assertTrue( wp_roles()->get_role( 'author' )->has_cap( self::TEST_CAP ) ); + + $author_id = self::factory()->user->create( [ 'role' => 'author' ] ); + wp_set_current_user( $author_id ); + $this->assertTrue( $capability->user_can() ); + } + + /** + * Passing an array of capabilities should sync all of them together onto + * the same roles - the "bundle" case, e.g. ignore/global-ignore/explorer. + * + * @return void + */ + public function test_bundle_syncs_multiple_capabilities_together() { + $capability = new SyncCapability( [ self::TEST_CAP, self::TEST_CAP_2 ], self::TEST_OPTION ); + $capability->sync( [ 'author' ] ); + + $author = wp_roles()->get_role( 'author' ); + $this->assertTrue( $author->has_cap( self::TEST_CAP ) ); + $this->assertTrue( $author->has_cap( self::TEST_CAP_2 ) ); + + $this->assertFalse( wp_roles()->get_role( 'editor' )->has_cap( self::TEST_CAP ) ); + $this->assertFalse( wp_roles()->get_role( 'editor' )->has_cap( self::TEST_CAP_2 ) ); + } + + /** + * User_can()/permission_callback() must accept an explicit capability + * argument so bundle consumers can check one specific capability instead + * of only ever getting the first one in the bundle. + * + * @return void + */ + public function test_bundle_user_can_checks_the_capability_passed_in() { + $capability = new SyncCapability( [ self::TEST_CAP, self::TEST_CAP_2 ], self::TEST_OPTION ); + $capability->sync( [ 'author' ] ); + + $author_id = self::factory()->user->create( [ 'role' => 'author' ] ); + wp_set_current_user( $author_id ); + + $this->assertTrue( $capability->user_can( self::TEST_CAP ) ); + $this->assertTrue( $capability->user_can( self::TEST_CAP_2 ) ); + + $callback = $capability->permission_callback( self::TEST_CAP_2 ); + $this->assertTrue( $callback() ); + } + + /** + * Manage_options bypass must apply to every capability in the bundle, + * not just the first one. + * + * @return void + */ + public function test_manage_options_bypasses_every_capability_in_bundle() { + $capability = new SyncCapability( [ self::TEST_CAP, self::TEST_CAP_2 ], self::TEST_OPTION ); + $capability->register(); + $capability->sync( [ 'author' ] ); + + $admin_id = self::factory()->user->create( [ 'role' => 'administrator' ] ); + wp_set_current_user( $admin_id ); + + $this->assertTrue( $capability->user_can( self::TEST_CAP ) ); + $this->assertTrue( $capability->user_can( self::TEST_CAP_2 ) ); + } + + /** + * Sync_role_capability() is the generic (role, capability) primitive the + * rest of the class is built on - it should work as a standalone call, + * independent of any option-driven sync. + * + * @return void + */ + public function test_sync_role_capability_generic_primitive() { + $capability = new SyncCapability( self::TEST_CAP, self::TEST_OPTION ); + + $capability->sync_role_capability( 'editor', self::TEST_CAP, true ); + $this->assertTrue( wp_roles()->get_role( 'editor' )->has_cap( self::TEST_CAP ) ); + + $capability->sync_role_capability( 'editor', self::TEST_CAP, false ); + $this->assertFalse( wp_roles()->get_role( 'editor' )->has_cap( self::TEST_CAP ) ); + } } From bc1cff1bcac1dd917a306375450158d606614408 Mon Sep 17 00:00:00 2001 From: pattonwebz Date: Wed, 29 Jul 2026 15:51:31 +0100 Subject: [PATCH 14/23] Register ignore/global-ignore/explorer-access as one capability bundle 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 --- includes/options-page.php | 56 +++++++++++++--- .../phpunit/includes/IgnoreCapabilityTest.php | 67 +++++++++++++++++++ 2 files changed, 114 insertions(+), 9 deletions(-) diff --git a/includes/options-page.php b/includes/options-page.php index 2b3b98986..eda6d2e31 100644 --- a/includes/options-page.php +++ b/includes/options-page.php @@ -10,18 +10,33 @@ use EDAC\Admin\Settings; use EDAC\Inc\Accessibility_Statement; use EqualizeDigital\AccessibilityChecker\Admin\AdminPage\FixesPage; +use EqualizeDigital\AccessibilityChecker\Capabilities\CapabilityChecker; use EqualizeDigital\AccessibilityChecker\Capabilities\SyncCapability; if ( ! defined( 'ABSPATH' ) ) { exit; } +// The ignore-permissions capability bundle - all three are synced together +// onto whichever roles are configured in edacp_ignore_user_roles (set on the +// pro plugin's settings page). There is deliberately no separate settings +// control for the two newer capabilities; granting a role "can ignore +// issues" grants all three at once. +defined( 'EDAC_CAPABILITY_IGNORE_ISSUES' ) || define( 'EDAC_CAPABILITY_IGNORE_ISSUES', 'edac_ignore_issues' ); +// Larger blast radius than a per-post ignore: suppresses an issue across +// every post sharing a rule+object, not just the one being viewed. +defined( 'EDAC_CAPABILITY_IGNORE_ISSUES_GLOBALLY' ) || define( 'EDAC_CAPABILITY_IGNORE_ISSUES_GLOBALLY', 'edac_ignore_issues_globally' ); +// Gates getting into pro's Issues Explorer app at all, independent of +// whether the user can also ignore issues once inside it. +defined( 'EDAC_CAPABILITY_ISSUES_EXPLORER_ACCESS' ) || define( 'EDAC_CAPABILITY_ISSUES_EXPLORER_ACCESS', 'edac_issues_explorer_access' ); + /** - * The edac_ignore_issues capability, synced onto the roles listed in the - * edacp_ignore_user_roles option (set on the pro plugin's settings page), - * with a manage_options bypass. current_user_can( 'edac_ignore_issues' ) is - * the single check every call site (REST, AJAX, menu registration) relies - * on instead of comparing against the option directly. + * The ignore-permissions capability bundle (edac_ignore_issues, + * edac_ignore_issues_globally, edac_issues_explorer_access), synced onto the + * roles listed in the edacp_ignore_user_roles option, with a manage_options + * bypass. Consumers should check individual capabilities via + * CapabilityChecker (or the edac_user_can_*() helpers below) rather than + * calling into this instance directly. * * @return SyncCapability */ @@ -30,10 +45,14 @@ function edac_ignore_capability(): SyncCapability { if ( null === $capability ) { $capability = new SyncCapability( - 'edac_ignore_issues', + [ + EDAC_CAPABILITY_IGNORE_ISSUES, + EDAC_CAPABILITY_IGNORE_ISSUES_GLOBALLY, + EDAC_CAPABILITY_ISSUES_EXPLORER_ACCESS, + ], 'edacp_ignore_user_roles', [ 'administrator' ], - 1 + 2 // Bumped from 1: adds the two new bundled capabilities for roles already granted ignore access. ); $capability->register(); } @@ -43,12 +62,31 @@ function edac_ignore_capability(): SyncCapability { edac_ignore_capability(); /** - * Check if user can ignore or can manage options + * Check if user can ignore issues (per-post) or can manage options. * * @return bool */ function edac_user_can_ignore() { - return edac_ignore_capability()->user_can(); + return CapabilityChecker::user_can( EDAC_CAPABILITY_IGNORE_ISSUES ); +} + +/** + * Check if user can globally ignore an issue (suppress it across every post + * sharing a rule+object) or can manage options. + * + * @return bool + */ +function edac_user_can_ignore_globally() { + return CapabilityChecker::user_can( EDAC_CAPABILITY_IGNORE_ISSUES_GLOBALLY ); +} + +/** + * Check if user can access the (pro) Issues Explorer, or can manage options. + * + * @return bool + */ +function edac_user_can_access_issues_explorer() { + return CapabilityChecker::user_can( EDAC_CAPABILITY_ISSUES_EXPLORER_ACCESS ); } /** diff --git a/tests/phpunit/includes/IgnoreCapabilityTest.php b/tests/phpunit/includes/IgnoreCapabilityTest.php index 8d49c0af2..5e4bddf45 100644 --- a/tests/phpunit/includes/IgnoreCapabilityTest.php +++ b/tests/phpunit/includes/IgnoreCapabilityTest.php @@ -18,6 +18,8 @@ class IgnoreCapabilityTest extends WP_UnitTestCase { public function tearDown(): void { foreach ( wp_roles()->role_objects as $role ) { $role->remove_cap( 'edac_ignore_issues' ); + $role->remove_cap( 'edac_ignore_issues_globally' ); + $role->remove_cap( 'edac_issues_explorer_access' ); } parent::tearDown(); } @@ -101,4 +103,69 @@ public function test_saving_option_triggers_sync() { $this->assertFalse( wp_roles()->get_role( 'author' )->has_cap( 'edac_ignore_issues' ) ); $this->assertTrue( wp_roles()->get_role( 'editor' )->has_cap( 'edac_ignore_issues' ) ); } + + /** + * Syncing the ignore-roles option should grant all three bundled + * capabilities together - there is no separate setting for global-ignore + * or Issues Explorer access, so a role given ignore permission gets all + * three at once. + * + * @return void + */ + public function test_sync_grants_all_three_bundled_capabilities() { + edac_ignore_capability()->sync( [ 'author' ] ); + + $author = wp_roles()->get_role( 'author' ); + $this->assertTrue( $author->has_cap( 'edac_ignore_issues' ) ); + $this->assertTrue( $author->has_cap( 'edac_ignore_issues_globally' ) ); + $this->assertTrue( $author->has_cap( 'edac_issues_explorer_access' ) ); + } + + /** + * A user in a role that was granted the bundle should pass both new + * helper functions, mirroring edac_user_can_ignore(). + * + * @return void + */ + public function test_new_helper_functions_true_for_synced_role() { + edac_ignore_capability()->sync( [ 'author' ] ); + + $user_id = self::factory()->user->create( [ 'role' => 'author' ] ); + wp_set_current_user( $user_id ); + + $this->assertTrue( edac_user_can_ignore_globally() ); + $this->assertTrue( edac_user_can_access_issues_explorer() ); + } + + /** + * A user in a role that was not granted the bundle should fail both new + * helper functions. + * + * @return void + */ + public function test_new_helper_functions_false_for_unsynced_role() { + edac_ignore_capability()->sync( [ 'author' ] ); + + $user_id = self::factory()->user->create( [ 'role' => 'editor' ] ); + wp_set_current_user( $user_id ); + + $this->assertFalse( edac_user_can_ignore_globally() ); + $this->assertFalse( edac_user_can_access_issues_explorer() ); + } + + /** + * Manage_options users must always pass the new helper functions too, + * same as edac_user_can_ignore(). + * + * @return void + */ + public function test_manage_options_user_always_passes_new_helpers() { + edac_ignore_capability()->sync( [ 'author' ] ); + + $user_id = self::factory()->user->create( [ 'role' => 'administrator' ] ); + wp_set_current_user( $user_id ); + + $this->assertTrue( edac_user_can_ignore_globally() ); + $this->assertTrue( edac_user_can_access_issues_explorer() ); + } } From 665633578e643998235fe0a8a6d4e5dc5263032e Mon Sep 17 00:00:00 2001 From: pattonwebz Date: Wed, 29 Jul 2026 16:01:40 +0100 Subject: [PATCH 15/23] Harden SyncCapability against bundle-desync and version-key collision 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 --- .../classes/Capabilities/SyncCapability.php | 35 +++++++++++++++---- .../Capabilities/SyncCapabilityTest.php | 29 +++++++++++---- 2 files changed, 52 insertions(+), 12 deletions(-) diff --git a/includes/classes/Capabilities/SyncCapability.php b/includes/classes/Capabilities/SyncCapability.php index a4682bf93..1c5a4942c 100644 --- a/includes/classes/Capabilities/SyncCapability.php +++ b/includes/classes/Capabilities/SyncCapability.php @@ -153,15 +153,20 @@ public function bypass_for_admins( $caps, $cap, $user_id ) { /** * Add or remove one capability on one role. The generic primitive - * everything else in this class is built on - safe to call directly for - * a single (role, capability) pair outside of the option-driven sync. + * sync() is built on. Deliberately private: calling it directly for a + * single capability out of a multi-capability bundle would grant/revoke + * that one capability while leaving the rest of the bundle untouched + * for that role, breaking the "these capabilities always travel + * together" guarantee this class exists to provide. Always go through + * sync() (or the option it's wired to) so every capability in the + * bundle stays in lockstep. * * @param string $role_slug Role slug, e.g. 'editor'. * @param string $capability Capability string. * @param bool $should_have Whether the role should have this capability. * @return void */ - public function sync_role_capability( string $role_slug, string $capability, bool $should_have ): void { + private function sync_role_capability( string $role_slug, string $capability, bool $should_have ): void { $role = wp_roles()->get_role( $role_slug ); if ( ! $role ) { @@ -194,6 +199,23 @@ public function sync( $roles ): void { } } + /** + * Name of the option this bundle's migration-version marker is stored + * under. Includes a hash of the capability list, not just option_name, + * so two different SyncCapability instances that happen to point at the + * same option (e.g. a future feature layered onto an existing option) + * can never collide on one shared version counter and silently skip + * each other's migration. + * + * @return string + */ + private function version_option_name(): string { + $capabilities = $this->capabilities; + sort( $capabilities ); + + return 'edac_capability_version_' . $this->option_name . '_' . md5( implode( '|', $capabilities ) ); + } + /** * Run the sync once per migration version. Covers two cases: a site * that already had option_name set before this bundle existed (needs an @@ -201,13 +223,14 @@ public function sync( $roles ): void { * default_roles/capabilities change (needs a re-sync even though it * already ran an earlier version's migration). * - * Versioned per option (not per capability), since every capability in - * the bundle is always granted together and shares one migration. + * Versioned per (option, capability set) pair, not per capability, + * since every capability in the bundle is always granted together and + * shares one migration. * * @return void */ public function maybe_migrate(): void { - $version_option = "edac_capability_version_{$this->option_name}"; + $version_option = $this->version_option_name(); $stored_version = (int) get_option( $version_option, 0 ); if ( $stored_version >= $this->version ) { diff --git a/tests/phpunit/includes/classes/Capabilities/SyncCapabilityTest.php b/tests/phpunit/includes/classes/Capabilities/SyncCapabilityTest.php index 719cf4823..9c8dfa694 100644 --- a/tests/phpunit/includes/classes/Capabilities/SyncCapabilityTest.php +++ b/tests/phpunit/includes/classes/Capabilities/SyncCapabilityTest.php @@ -44,12 +44,23 @@ class SyncCapabilityTest extends WP_UnitTestCase { * @return void */ public function tearDown(): void { + global $wpdb; + foreach ( wp_roles()->role_objects as $role ) { $role->remove_cap( self::TEST_CAP ); $role->remove_cap( self::TEST_CAP_2 ); } delete_option( self::TEST_OPTION ); - delete_option( 'edac_capability_version_' . self::TEST_OPTION ); + // Version markers are keyed by option name + a hash of the capability + // set, so different tests in this file produce different keys - + // delete anything matching the option prefix rather than one exact key. + // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching + $wpdb->query( + $wpdb->prepare( + "DELETE FROM {$wpdb->options} WHERE option_name LIKE %s", + $wpdb->esc_like( 'edac_capability_version_' . self::TEST_OPTION ) . '%' + ) + ); wp_set_current_user( 0 ); parent::tearDown(); } @@ -257,19 +268,25 @@ public function test_manage_options_bypasses_every_capability_in_bundle() { } /** - * Sync_role_capability() is the generic (role, capability) primitive the - * rest of the class is built on - it should work as a standalone call, - * independent of any option-driven sync. + * Sync_role_capability() is the generic (role, capability) primitive + * sync() is built on. It's private (calling it directly for one + * capability out of a bundle would desync the rest of the bundle for + * that role), so this test reaches it via reflection rather than a + * public call - it's still worth covering in isolation from sync()'s + * roles-loop. * * @return void */ public function test_sync_role_capability_generic_primitive() { $capability = new SyncCapability( self::TEST_CAP, self::TEST_OPTION ); - $capability->sync_role_capability( 'editor', self::TEST_CAP, true ); + $method = new ReflectionMethod( SyncCapability::class, 'sync_role_capability' ); + $method->setAccessible( true ); + + $method->invoke( $capability, 'editor', self::TEST_CAP, true ); $this->assertTrue( wp_roles()->get_role( 'editor' )->has_cap( self::TEST_CAP ) ); - $capability->sync_role_capability( 'editor', self::TEST_CAP, false ); + $method->invoke( $capability, 'editor', self::TEST_CAP, false ); $this->assertFalse( wp_roles()->get_role( 'editor' )->has_cap( self::TEST_CAP ) ); } } From dd4a648ded9e0241e742c18b261c3e1250f2f419 Mon Sep 17 00:00:00 2001 From: pattonwebz Date: Wed, 29 Jul 2026 16:23:34 +0100 Subject: [PATCH 16/23] Require edac_ignore_issues_globally for largeBatch dismiss requests 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 --- includes/classes/class-rest-api.php | 13 +++ .../includes/classes/RestApiEndpointsTest.php | 79 +++++++++++++++++++ 2 files changed, 92 insertions(+) diff --git a/includes/classes/class-rest-api.php b/includes/classes/class-rest-api.php index 8a553d2ab..92dea7be6 100644 --- a/includes/classes/class-rest-api.php +++ b/includes/classes/class-rest-api.php @@ -1223,6 +1223,19 @@ public function dismiss_issue( $request ) { $ignore_global = $request->get_param( 'ignore_global' ) ?? 0; $large_batch = $request->get_param( 'largeBatch' ) ?? false; + // largeBatch is what actually performs the global action (updating every + // row that shares the object, not just $issue_id) - the per-post + // edit_post loop below only proves the user can edit each affected post, + // it doesn't prove they're allowed to take a global action at all. That + // requires the separate, larger-blast-radius capability. + if ( $large_batch && ! edac_user_can_ignore_globally() ) { + return new \WP_Error( + 'rest_forbidden', + __( 'Sorry, you are not allowed to dismiss issues globally.', 'accessibility-checker' ), + [ 'status' => rest_authorization_required_code() ] + ); + } + $table_name = $wpdb->prefix . 'accessibility_checker'; $site_id = get_current_blog_id(); diff --git a/tests/phpunit/includes/classes/RestApiEndpointsTest.php b/tests/phpunit/includes/classes/RestApiEndpointsTest.php index 102fab835..c0df8ed7b 100644 --- a/tests/phpunit/includes/classes/RestApiEndpointsTest.php +++ b/tests/phpunit/includes/classes/RestApiEndpointsTest.php @@ -94,6 +94,11 @@ public static function wpSetUpBeforeClass( $factory ) { // (covered separately in IgnoreCapabilityTest and // test_single_issue_dismiss_forbidden_without_ignore_capability). $user->add_cap( 'edac_ignore_issues' ); + // And edac_ignore_issues_globally, so the largeBatch tests below exercise + // the per-post edit_post authorization loop specifically, independent of + // the global-ignore capability gate (covered separately in + // test_large_batch_dismiss_forbidden_without_global_ignore_capability). + $user->add_cap( 'edac_ignore_issues_globally' ); self::$post_id = $factory->post->create( [ @@ -811,6 +816,80 @@ public function test_large_batch_dismiss_authorized_on_all() { } } + /** + * Test: A user who can edit_post on every affected post, and has + * edac_ignore_issues, but was never granted edac_ignore_issues_globally, + * must still be forbidden from a largeBatch dismiss - per-post edit + * permission is not a substitute for the global-ignore capability, since + * largeBatch updates every row sharing the object regardless of which + * single issue_id the request named. + * + * @return void + */ + public function test_large_batch_dismiss_forbidden_without_global_ignore_capability() { + global $wpdb; + + $this->assertNotNull( $this->server ); + + // User can edit their own post and has edac_ignore_issues, but was + // never granted edac_ignore_issues_globally. + $user_id = self::factory()->user->create( [ 'role' => 'subscriber' ] ); + $user = new WP_User( $user_id ); + $user->add_cap( 'edit_posts' ); + $user->add_cap( 'edac_ignore_issues' ); + + $post_id = self::factory()->post->create( + [ + 'post_type' => 'post', + 'post_status' => 'draft', + 'post_author' => $user_id, + 'post_title' => 'No Global Ignore Batch Post', + 'post_content' => 'No Global Ignore Batch Content', + ] + ); + + $table_name = $wpdb->prefix . 'accessibility_checker'; + $site_id = get_current_blog_id(); + $batch_object = 'batch-no-global-cap-test-' . wp_generate_uuid4(); + + // phpcs:disable WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching + $wpdb->insert( + $table_name, + [ + 'postid' => $post_id, + 'siteid' => $site_id, + 'type' => 'error', + 'rule' => 'batch-no-global-cap-test', + 'ruletype' => 'error', + 'object' => $batch_object, + 'recordcheck' => 1, + 'user' => $user_id, + 'ignre' => 0, + 'ignre_global' => 0, + ], + [ '%d', '%d', '%s', '%s', '%s', '%s', '%d', '%d', '%d', '%d' ] + ); + $issue_id = $wpdb->insert_id; + + wp_set_current_user( $user_id ); + + $request = new \WP_REST_Request( 'POST', '/accessibility-checker/v1/dismiss-issue/' . $issue_id ); + $request->set_param( 'action', 'dismiss' ); + $request->set_param( 'largeBatch', true ); + + $response = $this->server->dispatch( $request ); + + $this->assertSame( 403, $response->get_status(), 'largeBatch dismiss without edac_ignore_issues_globally should return 403 even when the user can edit every affected post.' ); + + $updated_issue = $wpdb->get_row( + $wpdb->prepare( 'SELECT ignre FROM %i WHERE id = %d', $table_name, $issue_id ), + ARRAY_A + ); + // phpcs:enable WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching + + $this->assertSame( '0', $updated_issue['ignre'], 'Issue should not have been dismissed.' ); + } + /** * Test: Large batch dismissed by user with partial authorization fails before bulk query. * From d9da482b8d89be8cad62ea23997b673367dfce42 Mon Sep 17 00:00:00 2001 From: pattonwebz Date: Wed, 29 Jul 2026 16:23:44 +0100 Subject: [PATCH 17/23] Gate global-dismiss UI controls on a real capability, not just isPro 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 --- admin/class-enqueue-admin.php | 1 + src/issueModal/components/DismissPanel.js | 31 +++++++++++-------- .../components/IssueDetailsModal.js | 1 + 3 files changed, 20 insertions(+), 13 deletions(-) diff --git a/admin/class-enqueue-admin.php b/admin/class-enqueue-admin.php index 2a4837ca7..e090a7079 100644 --- a/admin/class-enqueue-admin.php +++ b/admin/class-enqueue-admin.php @@ -226,6 +226,7 @@ public static function maybe_enqueue_sidebar_script() { 'settingsUrl' => esc_url_raw( admin_url( 'admin.php?page=accessibility_checker_settings' ) ), 'canManageSettings' => current_user_can( apply_filters( 'edac_filter_settings_capability', 'manage_options' ) ), 'canDismiss' => edac_user_can_ignore(), + 'canDismissGlobally' => edac_user_can_ignore_globally(), 'readabilityHelpUrl' => esc_url_raw( edac_link_wrapper( 'https://a11ychecker.com/help3265', 'wordpress-general', 'content-analysis-sidebar', false ) ), 'dismissReasons' => IgnoreUI::get_reasons(), 'simplifiedSummaryPrompt' => get_option( 'edac_simplified_summary_prompt', 'none' ), diff --git a/src/issueModal/components/DismissPanel.js b/src/issueModal/components/DismissPanel.js index 0745665dc..251ae2654 100644 --- a/src/issueModal/components/DismissPanel.js +++ b/src/issueModal/components/DismissPanel.js @@ -18,15 +18,19 @@ import { getDismissReasonOptions } from '../../sidebar/utils/dismissHelpers'; /** * Dismiss Panel Component * - * @param {Object} props - Component props. - * @param {Object} props.issue - The issue object. - * @param {boolean} props.isOpen - Whether the panel is open. - * @param {Function} props.onToggle - Callback when panel is toggled. - * @param {Function} props.onIgnore - Callback when issue is dismissed/restored. - * @param {Function} props.onCloseModal - Callback to close the parent modal. - * @param {boolean} props.forceGlobal - When true, the primary dismiss action targets all pages (global dismiss). - * @param {boolean} props.isPro - Whether the current UI is running in Pro. - * @param {boolean} props.canDismiss - Whether the current user is allowed to dismiss/reopen issues. + * @param {Object} props - Component props. + * @param {Object} props.issue - The issue object. + * @param {boolean} props.isOpen - Whether the panel is open. + * @param {Function} props.onToggle - Callback when panel is toggled. + * @param {Function} props.onIgnore - Callback when issue is dismissed/restored. + * @param {Function} props.onCloseModal - Callback to close the parent modal. + * @param {boolean} props.forceGlobal - When true, the primary dismiss action targets all pages (global dismiss). + * @param {boolean} props.isPro - Whether the current UI is running in Pro. + * @param {boolean} props.canDismiss - Whether the current user is allowed to dismiss/reopen issues. + * @param {boolean} props.canDismissGlobally - Whether the current user is allowed to dismiss/reopen an issue across every + * page it appears on. Must come from a real capability check (edac_user_can_ignore_globally()), + * not just isPro - showing this control to a user who lacks the capability + * only leads to the underlying REST call being rejected server-side. */ const DismissPanel = ( { issue, @@ -37,6 +41,7 @@ const DismissPanel = ( { forceGlobal = false, isPro = typeof window !== 'undefined' && ( window.edac_editor_app?.pro === '1' || window.edac_script_vars?.pro === '1' ), canDismiss = true, + canDismissGlobally = false, } ) => { const panelRef = useRef( null ); const [ comment, setComment ] = useState( issue?.ignre_comment ? decodeEntities( issue.ignre_comment ) : '' ); @@ -46,8 +51,8 @@ const DismissPanel = ( { const [ successNotice, setSuccessNotice ] = useState( null ); const [ isIgnored, setIsIgnored ] = useState( issue?.ignre === '1' || issue?.ignre === 1 ); const isGloballyDismissed = issue?.ignre_global === 1 || issue?.ignre_global === '1'; - const canDismissGlobally = isPro; - const dismissGlobally = canDismissGlobally && forceGlobal; + const canUseGlobalDismiss = isPro && canDismissGlobally; + const dismissGlobally = canUseGlobalDismiss && forceGlobal; const dismissReasonOptions = getDismissReasonOptions(); const dismissReasonLabel = dismissReasonOptions.find( ( option ) => option.value === issue?.ignre_reason )?.label; const handleToggleIgnore = async ( ignore, isGlobal = false ) => { @@ -161,7 +166,7 @@ const DismissPanel = ( { ) } - { canDismiss && isGloballyDismissed && ( + { canUseGlobalDismiss && isGloballyDismissed && (
- { canDismissGlobally && ( + { canUseGlobalDismiss && ( (
From 3816bc25aa5314be852515c513d4e62b301df6a5 Mon Sep 17 00:00:00 2001 From: pattonwebz Date: Wed, 29 Jul 2026 16:23:52 +0100 Subject: [PATCH 18/23] Migrate capabilities on init, not admin_init; reject empty bundles 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 --- .../classes/Capabilities/SyncCapability.php | 23 ++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/includes/classes/Capabilities/SyncCapability.php b/includes/classes/Capabilities/SyncCapability.php index 1c5a4942c..41d03e600 100644 --- a/includes/classes/Capabilities/SyncCapability.php +++ b/includes/classes/Capabilities/SyncCapability.php @@ -54,7 +54,7 @@ class SyncCapability { /** * Bumped when the default_roles (or the capability list) for this bundle * change; sites whose stored migration version is lower get re-synced - * once on their next admin_init, even if they already ran an earlier + * once on their next init, even if they already ran an earlier * version's migration. * * @var int @@ -71,9 +71,18 @@ class SyncCapability { * @param string $option_name Option holding the array of role slugs allowed these capabilities. * @param array $default_roles Roles to grant on first-ever sync (site had the option unset). * @param int $version Bump to re-run the migration when default_roles/capabilities change. + * + * @throws \InvalidArgumentException If $capabilities is an empty array - there is nothing for this + * instance to sync/check, and user_can()'s no-argument default + * would otherwise silently check an undefined (null) capability. */ public function __construct( $capabilities, string $option_name, array $default_roles = [], int $version = 1 ) { - $this->capabilities = is_array( $capabilities ) ? array_values( $capabilities ) : [ $capabilities ]; + $this->capabilities = is_array( $capabilities ) ? array_values( $capabilities ) : [ $capabilities ]; + + if ( [] === $this->capabilities ) { + throw new \InvalidArgumentException( 'SyncCapability requires at least one capability.' ); + } + $this->option_name = $option_name; $this->default_roles = $default_roles; $this->version = $version; @@ -105,7 +114,15 @@ function ( $old_value, $value ) { 2 ); - add_action( 'admin_init', [ $this, 'maybe_migrate' ] ); + // init, not admin_init: 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 would leave the very first request after a version bump + // building a menu, or serving a REST request, against pre-migration + // capabilities. init fires early enough on every request type - admin, + // front-end, REST, and cron alike - to have already run by the time any + // of those capability checks happen. + add_action( 'init', [ $this, 'maybe_migrate' ] ); } /** From 37958d4262b5d7aafa3cbfae576cf08e9622f03e Mon Sep 17 00:00:00 2001 From: pattonwebz Date: Wed, 29 Jul 2026 16:42:45 +0100 Subject: [PATCH 19/23] Skip the per-post edit_post loop when the user has global-ignore 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 --- includes/classes/class-rest-api.php | 27 +++++--- .../includes/classes/RestApiEndpointsTest.php | 66 +++++++++++-------- 2 files changed, 56 insertions(+), 37 deletions(-) diff --git a/includes/classes/class-rest-api.php b/includes/classes/class-rest-api.php index 92dea7be6..8dacb49e9 100644 --- a/includes/classes/class-rest-api.php +++ b/includes/classes/class-rest-api.php @@ -1228,7 +1228,8 @@ public function dismiss_issue( $request ) { // edit_post loop below only proves the user can edit each affected post, // it doesn't prove they're allowed to take a global action at all. That // requires the separate, larger-blast-radius capability. - if ( $large_batch && ! edac_user_can_ignore_globally() ) { + $can_ignore_globally = edac_user_can_ignore_globally(); + if ( $large_batch && ! $can_ignore_globally ) { return new \WP_Error( 'rest_forbidden', __( 'Sorry, you are not allowed to dismiss issues globally.', 'accessibility-checker' ), @@ -1288,14 +1289,22 @@ public function dismiss_issue( $request ) { ); } - foreach ( $issue_rows as $issue_row ) { - $post_id = isset( $issue_row['postid'] ) ? (int) $issue_row['postid'] : 0; - if ( $post_id <= 0 || ! current_user_can( 'edit_post', $post_id ) ) { - return new \WP_Error( - 'rest_forbidden', - __( 'Sorry, you are not allowed to dismiss one or more issues in this batch.', 'accessibility-checker' ), - [ 'status' => rest_authorization_required_code() ] - ); + // A user with edac_ignore_issues_globally is already trusted for + // this exact "affects posts you may not own" action (enforced + // above), so the per-post edit_post lookups below - one + // current_user_can() call per affected post - would be pure + // overhead for them. Kept as a fallback check for any caller that + // somehow reaches this branch without that capability. + if ( ! $can_ignore_globally ) { + foreach ( $issue_rows as $issue_row ) { + $post_id = isset( $issue_row['postid'] ) ? (int) $issue_row['postid'] : 0; + if ( $post_id <= 0 || ! current_user_can( 'edit_post', $post_id ) ) { + return new \WP_Error( + 'rest_forbidden', + __( 'Sorry, you are not allowed to dismiss one or more issues in this batch.', 'accessibility-checker' ), + [ 'status' => rest_authorization_required_code() ] + ); + } } } diff --git a/tests/phpunit/includes/classes/RestApiEndpointsTest.php b/tests/phpunit/includes/classes/RestApiEndpointsTest.php index c0df8ed7b..1d74a2fa8 100644 --- a/tests/phpunit/includes/classes/RestApiEndpointsTest.php +++ b/tests/phpunit/includes/classes/RestApiEndpointsTest.php @@ -94,11 +94,10 @@ public static function wpSetUpBeforeClass( $factory ) { // (covered separately in IgnoreCapabilityTest and // test_single_issue_dismiss_forbidden_without_ignore_capability). $user->add_cap( 'edac_ignore_issues' ); - // And edac_ignore_issues_globally, so the largeBatch tests below exercise - // the per-post edit_post authorization loop specifically, independent of - // the global-ignore capability gate (covered separately in - // test_large_batch_dismiss_forbidden_without_global_ignore_capability). - $user->add_cap( 'edac_ignore_issues_globally' ); + // Deliberately NOT edac_ignore_issues_globally here - it's granted + // per-test below where a largeBatch test specifically needs it, since + // holding it now bypasses the per-post edit_post loop entirely + // (see dismiss_issue()'s $can_ignore_globally short-circuit). self::$post_id = $factory->post->create( [ @@ -719,11 +718,11 @@ public function test_single_issue_dismiss_unauthorized_user() { } /** - * Test: Large batch dismissed by user with edit permission on all posts succeeds. + * Test: Large batch dismissed by a user with edac_ignore_issues_globally + * succeeds. * - * Verifies that when a user has edit_post capability for all posts - * in a large batch, the endpoint dismisses all issues with one bulk - * UPDATE query and returns success. + * Verifies that a user holding the global-ignore capability can dismiss + * an entire batch with one bulk UPDATE query and a 200 response. * * @return void */ @@ -732,6 +731,8 @@ public function test_large_batch_dismiss_authorized_on_all() { $this->assertNotNull( $this->server ); + ( new WP_User( self::$limited_id ) )->add_cap( 'edac_ignore_issues_globally' ); + // Create posts for batch test (limited user owns all). // Use 'draft' so the limited user (who only has edit_posts, not edit_published_posts) // can edit them — WordPress requires edit_published_posts for published posts. @@ -893,22 +894,25 @@ public function test_large_batch_dismiss_forbidden_without_global_ignore_capabil /** * Test: Large batch dismissed by user with partial authorization fails before bulk query. * - * Verifies that when a user can edit only SOME posts in a large batch, - * the endpoint returns rest_forbidden BEFORE executing the bulk UPDATE query, - * ensuring no data is modified when permission checks fail. + * Verifies that a user with edac_ignore_issues_globally can dismiss a + * batch that includes a post they do NOT personally have edit_post on - + * the whole point of the capability is to bypass that per-post + * ownership check, not just to unlock largeBatch requests in general. * * @return void */ - public function test_large_batch_dismiss_authorized_on_some() { + public function test_large_batch_dismiss_bypasses_per_post_check_with_global_capability() { global $wpdb; $this->assertNotNull( $this->server ); - // Create posts: one owned by limited_id, one by admin_id. - // Use 'draft' for the limited user's post so they can edit it with only edit_posts - // (WordPress requires edit_published_posts to edit published posts, which limited_id lacks). - // This correctly models partial authorization: the limited user CAN edit their draft post - // but CANNOT edit the admin-owned published post. + ( new WP_User( self::$limited_id ) )->add_cap( 'edac_ignore_issues_globally' ); + + // Create posts: one owned by limited_id, one by admin_id. limited_id + // only has edit_posts (not edit_others_posts), so without the + // global-ignore bypass they could edit_post on the first but not the + // second - that's exactly the distinction this test proves no longer + // matters once edac_ignore_issues_globally is granted. $limited_post = self::factory()->post->create( [ 'post_type' => 'post', @@ -934,7 +938,7 @@ public function test_large_batch_dismiss_authorized_on_some() { $batch_object = 'batch-partial-authorized-test-' . wp_generate_uuid4(); // phpcs:disable WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching - // Create first issue on limited_id's post (limited user CAN edit). + // Create first issue on limited_id's own post. $wpdb->insert( $table_name, [ @@ -953,7 +957,9 @@ public function test_large_batch_dismiss_authorized_on_some() { ); $first_issue_id = $wpdb->insert_id; - // Create second issue on admin_id's post (limited user CANNOT edit). + // Create second issue on admin_id's post - limited_id has no + // edit_post on this one, which is exactly what the global capability + // should bypass. $wpdb->insert( $table_name, [ @@ -981,26 +987,30 @@ public function test_large_batch_dismiss_authorized_on_some() { $response = $this->server->dispatch( $request ); - // Verify response is 403 Forbidden. - $this->assertSame( 403, $response->get_status(), 'Large batch dismiss with partial authorization should return 403.' ); + // Verify response is successful despite limited_id lacking edit_post + // on the admin-owned post - the global capability is the gate now. + $this->assertSame( 200, $response->get_status(), 'Large batch dismiss with edac_ignore_issues_globally should succeed even across posts the user cannot individually edit.' ); - // Verify NO issues were updated (permission check failed before bulk query). + // Verify BOTH issues were updated, including the one on the post + // limited_id doesn't own. $updated_issues = $wpdb->get_results( $wpdb->prepare( 'SELECT id, ignre FROM %i WHERE object = %s', $table_name, $batch_object ), ARRAY_A ); // phpcs:enable WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching + $this->assertCount( 2, $updated_issues, 'Both issues in the batch should be updated.' ); foreach ( $updated_issues as $issue ) { - $this->assertSame( '0', $issue['ignre'], 'No issues should be updated when permission check fails.' ); + $this->assertSame( '1', $issue['ignre'], 'Both issues should be dismissed, including the one on the post the user cannot individually edit.' ); } } /** - * Test: Large batch dismissed by user with no authorization fails. - * - * Verifies that when a user cannot edit ANY posts in a large batch, - * the endpoint returns rest_forbidden immediately and no data is modified. + * Test: Large batch dismissed by a user without edac_ignore_issues_globally + * fails, even though every affected post happens to belong to someone + * else (a scenario that would also fail the per-post edit_post loop, if + * that loop were ever reached - it isn't here, since the capability gate + * runs first for every largeBatch request). * * @return void */ From c96102f7f20d3198af0adb838a7b310e40224428 Mon Sep 17 00:00:00 2001 From: pattonwebz Date: Mon, 3 Aug 2026 20:03:43 +0100 Subject: [PATCH 20/23] Cover the init-migration and canDismissGlobally behavior added on this branch SyncCapabilityTest: the constructor rejecting an empty capabilities array and register() hooking maybe_migrate() to init (not admin_init) were both new behavior in 3816bc25 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 d9da482b. 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 --- tests/jest/issueModal/DismissPanel.test.js | 76 ++++++++++++++++++- .../Capabilities/SyncCapabilityTest.php | 35 +++++++++ 2 files changed, 108 insertions(+), 3 deletions(-) diff --git a/tests/jest/issueModal/DismissPanel.test.js b/tests/jest/issueModal/DismissPanel.test.js index 2d7320063..589edb000 100644 --- a/tests/jest/issueModal/DismissPanel.test.js +++ b/tests/jest/issueModal/DismissPanel.test.js @@ -94,9 +94,9 @@ describe( 'DismissPanel', () => { unmount(); } ); - test( 'keeps global undo available when an issue was globally dismissed', async () => { + test( 'keeps global undo available for a user who can globally dismiss', async () => { const { toggleIssueDismiss } = require( '../../../src/issueModal/api' ); - window.edac_editor_app.pro = '0'; + window.edac_editor_app.pro = '1'; const { container, unmount } = renderReact( { onToggle={ jest.fn() } onIgnore={ jest.fn() } onCloseModal={ jest.fn() } - isPro={ false } + isPro={ true } + canDismissGlobally={ true } />, ); @@ -129,4 +130,73 @@ describe( 'DismissPanel', () => { unmount(); } ); + + test( 'hides global dismiss controls for a Pro user without canDismissGlobally', () => { + window.edac_editor_app.pro = '1'; + + const { container, unmount } = renderReact( + , + ); + + expect( container.querySelector( 'button[aria-label="More dismiss options"]' ) ).toBeNull(); + expect( container.textContent ).not.toContain( 'Dismiss Globally' ); + + unmount(); + } ); + + test( 'hides the global undo for a Pro user without canDismissGlobally, even on an already-globally-dismissed issue', () => { + window.edac_editor_app.pro = '1'; + + const { container, unmount } = renderReact( + , + ); + + expect( container.textContent ).not.toContain( 'Remove Global Dismissal' ); + + unmount(); + } ); + + test( 'shows global dismiss controls for a Pro user with canDismissGlobally', () => { + window.edac_editor_app.pro = '1'; + + const { container, unmount } = renderReact( + , + ); + + expect( container.textContent ).toContain( 'Dismiss Globally' ); + + unmount(); + } ); } ); diff --git a/tests/phpunit/includes/classes/Capabilities/SyncCapabilityTest.php b/tests/phpunit/includes/classes/Capabilities/SyncCapabilityTest.php index 9c8dfa694..9096c6a1c 100644 --- a/tests/phpunit/includes/classes/Capabilities/SyncCapabilityTest.php +++ b/tests/phpunit/includes/classes/Capabilities/SyncCapabilityTest.php @@ -267,6 +267,41 @@ public function test_manage_options_bypasses_every_capability_in_bundle() { $this->assertTrue( $capability->user_can( self::TEST_CAP_2 ) ); } + /** + * The constructor should reject an empty capabilities array rather than + * silently falling through to user_can()'s no-argument default checking + * an undefined (null) capability. + * + * @return void + */ + public function test_constructor_throws_on_empty_capabilities_array() { + $this->expectException( \InvalidArgumentException::class ); + + new SyncCapability( [], self::TEST_OPTION ); + } + + /** + * Register() must hook maybe_migrate() to init, not admin_init - admin_menu + * and rest_api_init both fire before admin_init on their respective request + * types, so a migration gated on admin_init would miss the first request + * after a version bump for menu builds and REST-only requests entirely. + * + * @return void + */ + public function test_register_hooks_migration_to_init_not_admin_init() { + $capability = new SyncCapability( self::TEST_CAP, self::TEST_OPTION ); + $capability->register(); + + $this->assertNotFalse( + has_action( 'init', [ $capability, 'maybe_migrate' ] ), + 'maybe_migrate() should be hooked to init.' + ); + $this->assertFalse( + has_action( 'admin_init', [ $capability, 'maybe_migrate' ] ), + 'maybe_migrate() should not be hooked to admin_init.' + ); + } + /** * Sync_role_capability() is the generic (role, capability) primitive * sync() is built on. It's private (calling it directly for one From 3844a0a28179de524659c5fc0bae5526daaecc68 Mon Sep 17 00:00:00 2001 From: pattonwebz Date: Mon, 3 Aug 2026 20:28:29 +0100 Subject: [PATCH 21/23] Revoke synced capabilities when their backing option is deleted 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 --- .../classes/Capabilities/SyncCapability.php | 12 ++++++++++ .../Capabilities/SyncCapabilityTest.php | 24 +++++++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/includes/classes/Capabilities/SyncCapability.php b/includes/classes/Capabilities/SyncCapability.php index 41d03e600..2d1a93576 100644 --- a/includes/classes/Capabilities/SyncCapability.php +++ b/includes/classes/Capabilities/SyncCapability.php @@ -113,6 +113,18 @@ function ( $old_value, $value ) { 10, 2 ); + // Whatever deleted the option (typically an uninstall routine, gated + // behind the "delete data" preference) intends for the roles it + // granted to lose these capabilities too - without this, sync() + // would only ever run again on the next add_option/update_option, + // leaving the capabilities stuck on whichever roles had them at + // deletion time indefinitely. + add_action( + "delete_option_{$this->option_name}", + function () { + $this->sync( [] ); + } + ); // init, not admin_init: admin_menu (where menu capability checks happen) // and rest_api_init (where REST permission_callbacks are registered) both diff --git a/tests/phpunit/includes/classes/Capabilities/SyncCapabilityTest.php b/tests/phpunit/includes/classes/Capabilities/SyncCapabilityTest.php index 9096c6a1c..a247c4541 100644 --- a/tests/phpunit/includes/classes/Capabilities/SyncCapabilityTest.php +++ b/tests/phpunit/includes/classes/Capabilities/SyncCapabilityTest.php @@ -99,6 +99,30 @@ public function test_register_syncs_on_option_save() { $this->assertTrue( wp_roles()->get_role( 'editor' )->has_cap( self::TEST_CAP ) ); } + /** + * Deleting the option (e.g. an uninstall routine's opt-in data cleanup) + * should revoke the capability from every role it was synced onto - the + * option's own removal is the strongest possible signal that a stale + * grant shouldn't be left behind, and there is no other hook left that + * would ever catch this since add_option/update_option only fire again + * on a future save. + * + * @return void + */ + public function test_deleting_option_revokes_capability_from_all_roles() { + $capability = new SyncCapability( self::TEST_CAP, self::TEST_OPTION ); + $capability->register(); + + add_option( self::TEST_OPTION, [ 'editor', 'author' ] ); + $this->assertTrue( wp_roles()->get_role( 'editor' )->has_cap( self::TEST_CAP ) ); + $this->assertTrue( wp_roles()->get_role( 'author' )->has_cap( self::TEST_CAP ) ); + + delete_option( self::TEST_OPTION ); + + $this->assertFalse( wp_roles()->get_role( 'editor' )->has_cap( self::TEST_CAP ) ); + $this->assertFalse( wp_roles()->get_role( 'author' )->has_cap( self::TEST_CAP ) ); + } + /** * Manage_options users must always pass user_can(), regardless of * whether their role was synced. From 1d2f466c10127f37b2bd11b5db87d55babd17d55 Mon Sep 17 00:00:00 2001 From: pattonwebz Date: Mon, 3 Aug 2026 20:45:18 +0100 Subject: [PATCH 22/23] Remove unused UserCapabilityGrant 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 --- .../Capabilities/CapabilityChecker.php | 7 +- .../Capabilities/UserCapabilityGrant.php | 123 ------------ .../Capabilities/UserCapabilityGrantTest.php | 190 ------------------ 3 files changed, 4 insertions(+), 316 deletions(-) delete mode 100644 includes/classes/Capabilities/UserCapabilityGrant.php delete mode 100644 tests/phpunit/includes/classes/Capabilities/UserCapabilityGrantTest.php diff --git a/includes/classes/Capabilities/CapabilityChecker.php b/includes/classes/Capabilities/CapabilityChecker.php index 0138bd415..ae3cf1448 100644 --- a/includes/classes/Capabilities/CapabilityChecker.php +++ b/includes/classes/Capabilities/CapabilityChecker.php @@ -14,9 +14,10 @@ * * Deliberately has no knowledge of SyncCapability, option-backed bundles, or * how a capability came to be true for a given user - a role-level sync, a - * per-user UserCapabilityGrant, or a plain core WP capability all answer - * identically here. That's the point of the split: SyncCapability owns - * writing the role/capability relationship, this class only ever reads it. + * capability granted directly to one user (by a role-editor plugin, custom + * code, etc.), or a plain core WP capability all answer identically here. + * That's the point of the split: SyncCapability owns writing the + * role/capability relationship, this class only ever reads it. */ class CapabilityChecker { diff --git a/includes/classes/Capabilities/UserCapabilityGrant.php b/includes/classes/Capabilities/UserCapabilityGrant.php deleted file mode 100644 index 2309c9853..000000000 --- a/includes/classes/Capabilities/UserCapabilityGrant.php +++ /dev/null @@ -1,123 +0,0 @@ -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(), - ] - ); - - return true; - } - - /** - * Revoke a capability that was granted directly to a user (does not - * affect a capability the user has via their role). - * - * @param int $user_id User to revoke the capability from. - * @param string $capability Capability string to revoke. - * @return bool True if the user was found and the revoke was applied. - */ - public static function revoke( int $user_id, string $capability ): bool { - $user = get_userdata( $user_id ); - if ( ! $user ) { - return false; - } - - $user->remove_cap( $capability ); - delete_user_meta( $user_id, self::META_PREFIX . $capability ); - - return true; - } - - /** - * Get attribution for a capability directly granted to a user via - * grant(), for display in an admin UI (e.g. "Granted by X on Y"). - * Returns null if the capability was never granted through this class - * (including if the user only has it via their role). - * - * @param int $user_id User to check. - * @param string $capability Capability string to check. - * @return array{granted_by: int, granted_at: int}|null - */ - public static function get_grant_info( int $user_id, string $capability ): ?array { - $meta = get_user_meta( $user_id, self::META_PREFIX . $capability, true ); - - return $meta ? $meta : null; - } - - /** - * Whether a capability was added directly to this user (as opposed to - * inherited from one of their roles). - * - * $user->caps holds only what was assigned directly to this user (role - * slugs plus any directly-added capabilities); $user->allcaps is the - * merged result including everything resolved from their roles. Using - * $user->caps here is what makes this "direct grant," not "has it at - * all," correctly distinct from user_can()/current_user_can(). - * - * @param int $user_id User to check. - * @param string $capability Capability string to check. - * @return bool - */ - public static function is_individually_granted( int $user_id, string $capability ): bool { - $user = get_userdata( $user_id ); - - return $user instanceof \WP_User && ! empty( $user->caps[ $capability ] ); - } -} diff --git a/tests/phpunit/includes/classes/Capabilities/UserCapabilityGrantTest.php b/tests/phpunit/includes/classes/Capabilities/UserCapabilityGrantTest.php deleted file mode 100644 index a3fa973e8..000000000 --- a/tests/phpunit/includes/classes/Capabilities/UserCapabilityGrantTest.php +++ /dev/null @@ -1,190 +0,0 @@ -role_objects as $role ) { - $role->remove_cap( self::TEST_CAP ); - } - wp_set_current_user( 0 ); - parent::tearDown(); - } - - /** - * Granting a capability to a user should make user_can() true for that - * user, without needing their role to have the capability at all. - * - * @return void - */ - public function test_grant_makes_user_can_true() { - $user_id = self::factory()->user->create( [ 'role' => 'subscriber' ] ); - - $this->assertFalse( user_can( $user_id, self::TEST_CAP ) ); - - UserCapabilityGrant::grant( $user_id, self::TEST_CAP ); - - $this->assertTrue( user_can( $user_id, self::TEST_CAP ) ); - } - - /** - * Revoking a directly-granted capability should make user_can() false - * again. - * - * @return void - */ - public function test_revoke_removes_the_grant() { - $user_id = self::factory()->user->create( [ 'role' => 'subscriber' ] ); - - UserCapabilityGrant::grant( $user_id, self::TEST_CAP ); - $this->assertTrue( user_can( $user_id, self::TEST_CAP ) ); - - UserCapabilityGrant::revoke( $user_id, self::TEST_CAP ); - $this->assertFalse( user_can( $user_id, self::TEST_CAP ) ); - } - - /** - * Granting should record who granted it and roughly when. - * - * @return void - */ - public function test_grant_records_attribution() { - $granter_id = self::factory()->user->create( [ 'role' => 'administrator' ] ); - $user_id = self::factory()->user->create( [ 'role' => 'subscriber' ] ); - - UserCapabilityGrant::grant( $user_id, self::TEST_CAP, $granter_id ); - - $info = UserCapabilityGrant::get_grant_info( $user_id, self::TEST_CAP ); - - $this->assertIsArray( $info ); - $this->assertSame( $granter_id, $info['granted_by'] ); - $this->assertEqualsWithDelta( time(), $info['granted_at'], 5 ); - } - - /** - * With no explicit granter passed, attribution should default to the - * current user. - * - * @return void - */ - public function test_grant_defaults_attribution_to_current_user() { - $granter_id = self::factory()->user->create( [ 'role' => 'administrator' ] ); - $user_id = self::factory()->user->create( [ 'role' => 'subscriber' ] ); - - wp_set_current_user( $granter_id ); - UserCapabilityGrant::grant( $user_id, self::TEST_CAP ); - - $info = UserCapabilityGrant::get_grant_info( $user_id, self::TEST_CAP ); - - $this->assertSame( $granter_id, $info['granted_by'] ); - } - - /** - * Revoking should clear the attribution record, not just the WordPress - * capability itself. - * - * @return void - */ - public function test_revoke_clears_attribution() { - $user_id = self::factory()->user->create( [ 'role' => 'subscriber' ] ); - - UserCapabilityGrant::grant( $user_id, self::TEST_CAP ); - UserCapabilityGrant::revoke( $user_id, self::TEST_CAP ); - - $this->assertNull( UserCapabilityGrant::get_grant_info( $user_id, self::TEST_CAP ) ); - } - - /** - * Get_grant_info() should be null for a user who was never individually - * granted the capability, even if they can() it via their role. - * - * @return void - */ - public function test_grant_info_null_when_capability_only_from_role() { - $user_id = self::factory()->user->create( [ 'role' => 'editor' ] ); - wp_roles()->get_role( 'editor' )->add_cap( self::TEST_CAP ); - - $this->assertTrue( user_can( $user_id, self::TEST_CAP ), 'Precondition: user has the capability via their role.' ); - $this->assertNull( UserCapabilityGrant::get_grant_info( $user_id, self::TEST_CAP ) ); - } - - /** - * Is_individually_granted() should distinguish "granted directly to - * this user" from "has it via their role" — both should pass - * user_can(), but only the direct grant should read as individually - * granted. - * - * @return void - */ - public function test_is_individually_granted_distinguishes_from_role_capability() { - $role_user_id = self::factory()->user->create( [ 'role' => 'editor' ] ); - $granted_user_id = self::factory()->user->create( [ 'role' => 'subscriber' ] ); - - wp_roles()->get_role( 'editor' )->add_cap( self::TEST_CAP ); - UserCapabilityGrant::grant( $granted_user_id, self::TEST_CAP ); - - $this->assertTrue( user_can( $role_user_id, self::TEST_CAP ) ); - $this->assertFalse( UserCapabilityGrant::is_individually_granted( $role_user_id, self::TEST_CAP ), 'Role-derived capability is not an individual grant.' ); - - $this->assertTrue( user_can( $granted_user_id, self::TEST_CAP ) ); - $this->assertTrue( UserCapabilityGrant::is_individually_granted( $granted_user_id, self::TEST_CAP ) ); - } - - /** - * A capability granted directly to a user must survive a role-level - * sync that removes the capability from that user's role — this is the - * core coexistence guarantee with Synced_Capability (or any other - * role-only sync), proven here without depending on that class. - * - * @return void - */ - public function test_direct_grant_survives_role_level_capability_removal() { - $user_id = self::factory()->user->create( [ 'role' => 'editor' ] ); - - wp_roles()->get_role( 'editor' )->add_cap( self::TEST_CAP ); - UserCapabilityGrant::grant( $user_id, self::TEST_CAP ); - - // Simulate a role-level sync (like Synced_Capability::sync()) that - // decides 'editor' should no longer have this capability. - wp_roles()->get_role( 'editor' )->remove_cap( self::TEST_CAP ); - - $this->assertTrue( user_can( $user_id, self::TEST_CAP ), 'Direct grant should survive removal of the capability from the user\'s role.' ); - } - - /** - * Granting/revoking for a user ID that doesn't exist should fail - * gracefully rather than erroring. - * - * @return void - */ - public function test_grant_and_revoke_return_false_for_nonexistent_user() { - $bogus_id = 999999; - - $this->assertFalse( UserCapabilityGrant::grant( $bogus_id, self::TEST_CAP ) ); - $this->assertFalse( UserCapabilityGrant::revoke( $bogus_id, self::TEST_CAP ) ); - } -} From 248b9dce876d74017fa6ea57c1dcdb3e5ce522d0 Mon Sep 17 00:00:00 2001 From: pattonwebz Date: Mon, 3 Aug 2026 21:28:15 +0100 Subject: [PATCH 23/23] Let a largeBatch global-ignore bypass edit_post on the representative 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 --- includes/classes/class-rest-api.php | 11 +++ .../includes/classes/RestApiEndpointsTest.php | 86 +++++++++++++++++++ 2 files changed, 97 insertions(+) diff --git a/includes/classes/class-rest-api.php b/includes/classes/class-rest-api.php index 8dacb49e9..7c7d16a13 100644 --- a/includes/classes/class-rest-api.php +++ b/includes/classes/class-rest-api.php @@ -344,6 +344,17 @@ function () use ( $ns, $version ) { return false; } + // A largeBatch request from a user who can ignore globally doesn't + // need edit_post on the URL's representative issue - dismiss_issue() + // already re-verifies (or, for this exact capability, deliberately + // bypasses) per-post permission for every affected row once inside + // the handler. Gating on the one representative post here would + // block the very requests this capability exists to allow, whenever + // that post happens not to be one the user personally owns. + if ( $request->get_param( 'largeBatch' ) && edac_user_can_ignore_globally() ) { + return true; + } + $table_name = edac_get_valid_table_name( $wpdb->prefix . 'accessibility_checker' ); if ( ! $table_name ) { return false; diff --git a/tests/phpunit/includes/classes/RestApiEndpointsTest.php b/tests/phpunit/includes/classes/RestApiEndpointsTest.php index 1d74a2fa8..b960d4665 100644 --- a/tests/phpunit/includes/classes/RestApiEndpointsTest.php +++ b/tests/phpunit/includes/classes/RestApiEndpointsTest.php @@ -1003,6 +1003,92 @@ public function test_large_batch_dismiss_bypasses_per_post_check_with_global_cap foreach ( $updated_issues as $issue ) { $this->assertSame( '1', $issue['ignre'], 'Both issues should be dismissed, including the one on the post the user cannot individually edit.' ); } + + // The route's own permission_callback does a separate edit_post lookup + // against whichever post the URL's issue_id resolves to, before the + // handler above (and its per-post bypass) ever runs - $first_issue_id + // resolves to limited_id's own post, so this assertion alone can't + // prove that lookup also respects the global capability. See + // test_large_batch_dismiss_permission_callback_bypasses_edit_post_on_representative_post() + // for the case where the URL's own post isn't one the user can edit. + } + + /** + * Test: the dismiss-issue route's permission_callback must not require + * edit_post on the URL's own representative post when the request is a + * largeBatch global-ignore - that check runs before dismiss_issue() (and + * its per-post bypass) is ever reached, so gating it on ownership of one + * specific post would block exactly the requests edac_ignore_issues_globally + * exists to allow, any time that one post isn't personally owned by the + * caller. + * + * @return void + */ + public function test_large_batch_dismiss_permission_callback_bypasses_edit_post_on_representative_post() { + global $wpdb; + + $this->assertNotNull( $this->server ); + + ( new WP_User( self::$limited_id ) )->add_cap( 'edac_ignore_issues_globally' ); + + // Post owned by admin - limited_id has no edit_post on this one. + $admin_post = self::factory()->post->create( + [ + 'post_type' => 'post', + 'post_status' => 'publish', + 'post_author' => self::$admin_id, + 'post_title' => 'Admin-Only Representative Post', + 'post_content' => 'Admin-Only Representative Content', + ] + ); + + $table_name = $wpdb->prefix . 'accessibility_checker'; + $site_id = get_current_blog_id(); + $batch_object = 'batch-representative-post-test-' . wp_generate_uuid4(); + + // phpcs:disable WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching + $wpdb->insert( + $table_name, + [ + 'postid' => $admin_post, + 'siteid' => $site_id, + 'type' => 'error', + 'rule' => 'batch-representative-post', + 'ruletype' => 'error', + 'object' => $batch_object, + 'recordcheck' => 1, + 'user' => self::$admin_id, + 'ignre' => 0, + 'ignre_global' => 0, + ], + [ '%d', '%d', '%s', '%s', '%s', '%s', '%d', '%d', '%d', '%d' ] + ); + // The URL's issue_id resolves to $admin_post - the post limited_id + // cannot edit - which is exactly what the permission_callback's own + // edit_post lookup would otherwise gate on. + $issue_id = $wpdb->insert_id; + + wp_set_current_user( self::$limited_id ); + + $request = new \WP_REST_Request( 'POST', '/accessibility-checker/v1/dismiss-issue/' . $issue_id ); + $request->set_param( 'action', 'dismiss' ); + $request->set_param( 'largeBatch', true ); + + $response = $this->server->dispatch( $request ); + + $this->assertSame( + 200, + $response->get_status(), + 'largeBatch dismiss with edac_ignore_issues_globally should not be blocked by the permission_callback\'s edit_post check on the URL\'s own representative post.' + ); + + $updated_issue = $wpdb->get_row( + $wpdb->prepare( 'SELECT ignre FROM %i WHERE id = %d', $table_name, $issue_id ), + ARRAY_A + ); + // phpcs:enable WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching + + $this->assertSame( '1', $updated_issue['ignre'], 'Issue should have been dismissed.' ); } /**