From 1c4b83c41dd955ca0ba7bebf7046a503b56f4ffd Mon Sep 17 00:00:00 2001 From: Joe Fusco Date: Fri, 31 Jul 2026 09:12:42 -0400 Subject: [PATCH] fix: filter presence read paths by per-post capability --- includes/admin-bar.php | 34 +++-- .../class-wp-rest-presence-controller.php | 6 +- .../class-wp-presence-widget-active-posts.php | 21 ++- tests/test-admin-bar.php | 120 ++++++++++++++++++ tests/widgets/test-widget-active-posts.php | 85 ++++++++++++- 5 files changed, 248 insertions(+), 18 deletions(-) create mode 100644 tests/test-admin-bar.php diff --git a/includes/admin-bar.php b/includes/admin-bar.php index 723dac0..ad49848 100644 --- a/includes/admin-bar.php +++ b/includes/admin-bar.php @@ -98,17 +98,35 @@ function ( $e ) use ( $current_uid ) { usort( $here, $sort_by_name ); usort( $elsewhere, $sort_by_name ); - // Build a map of user_id -> post_id for users currently editing a post. - $user_editing_post = array(); - $post_entries = wp_get_presence_by_room_prefix( 'postType/' ); - foreach ( $post_entries as $pe ) { + // Build a map of user_id -> post for users currently editing a post. + $editing = array(); + $post_ids = array(); + foreach ( wp_get_presence_by_room_prefix( 'postType/' ) as $pe ) { $parsed = wp_presence_parse_room( $pe->room ); - if ( $parsed ) { - $user_editing_post[ (int) $pe->user_id ] = $parsed['post_id']; + if ( ! $parsed ) { + continue; } + $editing[ (int) $pe->user_id ] = array( + 'room' => $pe->room, + 'post_id' => $parsed['post_id'], + ); + $post_ids[] = $parsed['post_id']; + } + + // The capability check below calls get_post() per room, so prime in one go. + // It reads neither the term nor the meta cache. + if ( ! empty( $post_ids ) ) { + _prime_post_caches( array_unique( $post_ids ), false, false ); } - if ( ! empty( $user_editing_post ) ) { - _prime_post_caches( array_unique( array_values( $user_editing_post ) ) ); + + // Drop the posts the current user cannot edit. Without this the menu gives + // the title and edit link of every post being worked on to anyone with + // `edit_posts`. Those entries keep the generic screen label instead. + $user_editing_post = array(); + foreach ( $editing as $user_id => $post ) { + if ( wp_can_access_presence_room( $post['room'], $current_uid ) ) { + $user_editing_post[ $user_id ] = $post['post_id']; + } } // "Here" count includes you. diff --git a/includes/class-wp-rest-presence-controller.php b/includes/class-wp-rest-presence-controller.php index 1f1a0cc..56456a9 100644 --- a/includes/class-wp-rest-presence-controller.php +++ b/includes/class-wp-rest-presence-controller.php @@ -560,7 +560,9 @@ public function get_rooms( $request ) { $rooms = wp_get_active_rooms(); $current_user_id = get_current_user_id(); - // Prime post caches to avoid N+1 queries during the wp_can_access_presence_room filtering loop. + // Prime post caches to avoid N+1 queries during the wp_can_access_presence_room + // filtering loop. The capability check reads neither the term nor the meta + // cache, so priming those is two queries spent on nothing. $post_ids = array(); foreach ( $rooms as $room ) { $parsed = wp_presence_parse_room( $room['room'] ); @@ -569,7 +571,7 @@ public function get_rooms( $request ) { } } if ( ! empty( $post_ids ) ) { - _prime_post_caches( array_unique( $post_ids ) ); + _prime_post_caches( array_unique( $post_ids ), false, false ); } $rooms = array_values( diff --git a/includes/widgets/class-wp-presence-widget-active-posts.php b/includes/widgets/class-wp-presence-widget-active-posts.php index 2c5e8c2..72e8104 100644 --- a/includes/widgets/class-wp-presence-widget-active-posts.php +++ b/includes/widgets/class-wp-presence-widget-active-posts.php @@ -274,13 +274,16 @@ public static function heartbeat_received( $response, $data, $screen_id ) { // p * @return array Array of post data with grouped editors. */ private static function build_active_posts_data() { - $entries = wp_get_presence_by_room_prefix( 'postType/' ); - $by_post = array(); - $now = time(); + $entries = wp_get_presence_by_room_prefix( 'postType/' ); + $by_post = array(); + $now = time(); + $current_user_id = get_current_user_id(); cache_users( wp_list_pluck( $entries, 'user_id' ) ); - // Prime post caches to avoid N+1 queries from get_post() in the loop. + // Prime post caches to avoid N+1 queries from get_post() and the + // capability check in the loop. Neither reads the term or meta cache, + // so priming those is two queries spent on nothing. $post_ids = array(); foreach ( $entries as $entry ) { $parsed = wp_presence_parse_room( $entry->room ); @@ -289,7 +292,7 @@ private static function build_active_posts_data() { } } if ( ! empty( $post_ids ) ) { - _prime_post_caches( array_unique( $post_ids ), true, true ); + _prime_post_caches( array_unique( $post_ids ), false, false ); } foreach ( $entries as $entry ) { @@ -313,6 +316,14 @@ private static function build_active_posts_data() { continue; } + // Rendering the widget only takes `edit_posts`, so without this a + // contributor would receive the title, edit link and editors of + // every post being worked on. Same check the REST controller + // applies to the room collection. + if ( ! wp_can_access_presence_room( $entry->room, $current_user_id ) ) { + continue; + } + $post = get_post( $post_id ); if ( ! $post ) { diff --git a/tests/test-admin-bar.php b/tests/test-admin-bar.php new file mode 100644 index 0000000..5b32483 --- /dev/null +++ b/tests/test-admin-bar.php @@ -0,0 +1,120 @@ +user->create( array( 'role' => 'editor' ) ); + self::$contributor_id = $factory->user->create( array( 'role' => 'contributor' ) ); + self::$post_id = $factory->post->create( + array( + 'post_title' => 'Secret Draft', + 'post_status' => 'draft', + 'post_author' => self::$editor_id, + ) + ); + } + + public function set_up() { + parent::set_up(); + // Only loaded on requests that actually render the bar. + require_once ABSPATH . WPINC . '/class-wp-admin-bar.php'; + } + + public function tear_down() { + global $wpdb; + // phpcs:ignore WordPress.DB.DirectDatabaseQuery + $wpdb->query( "TRUNCATE TABLE {$wpdb->presence}" ); + parent::tear_down(); + } + + /** + * Puts the editor online on the post editing screen for a given post. + * + * @param int $post_id The post the editor is working on. + */ + private function put_editor_on_post( $post_id ) { + wp_set_presence( + 'admin/online', + 'user-' . self::$editor_id, + array( 'screen' => 'post' ), + self::$editor_id + ); + wp_set_presence( + wp_presence_post_room( $post_id ), + 'lock-' . self::$editor_id, + array(), + self::$editor_id + ); + } + + /** + * Renders the node and returns every node title and link as one string. + * + * @return string Concatenated node titles and hrefs. + */ + private function render_node_markup() { + $bar = new WP_Admin_Bar(); + wp_presence_admin_bar_node( $bar ); + + $markup = ''; + foreach ( $bar->get_nodes() as $node ) { + $markup .= $node->title; + if ( is_string( $node->href ) ) { + $markup .= $node->href; + } + } + + return $markup; + } + + /** + * The menu labels each online user with the post they are editing. A + * contributor has `edit_posts`, which is all the node itself requires, but + * must not learn the title of a draft they cannot edit. + */ + public function test_hides_post_titles_the_user_cannot_edit() { + $this->put_editor_on_post( self::$post_id ); + + wp_set_current_user( self::$contributor_id ); + $markup = $this->render_node_markup(); + + $this->assertStringNotContainsString( 'Secret Draft', $markup ); + $this->assertStringNotContainsString( 'post=' . self::$post_id, $markup ); + } + + /** + * The user is still listed, only the post they are on is withheld. + */ + public function test_still_lists_the_user_without_the_post_title() { + $this->put_editor_on_post( self::$post_id ); + + wp_set_current_user( self::$contributor_id ); + $markup = $this->render_node_markup(); + + $editor = get_userdata( self::$editor_id ); + $this->assertStringContainsString( $editor->display_name, $markup ); + } + + /** + * A user who can edit the post still sees its title. + */ + public function test_shows_post_titles_the_user_can_edit() { + $this->put_editor_on_post( self::$post_id ); + + $other_editor_id = self::factory()->user->create( array( 'role' => 'editor' ) ); + wp_set_current_user( $other_editor_id ); + $markup = $this->render_node_markup(); + + $this->assertStringContainsString( 'Secret Draft', $markup ); + } +} diff --git a/tests/widgets/test-widget-active-posts.php b/tests/widgets/test-widget-active-posts.php index bae1bde..13f048e 100644 --- a/tests/widgets/test-widget-active-posts.php +++ b/tests/widgets/test-widget-active-posts.php @@ -10,12 +10,14 @@ class WP_Test_Presence_Widget_Active_Posts extends WP_UnitTestCase { private static $editor_id; private static $editor2_id; + private static $contributor_id; private static $post_id; public static function wpSetUpBeforeClass( WP_UnitTest_Factory $factory ) { - self::$editor_id = $factory->user->create( array( 'role' => 'editor' ) ); - self::$editor2_id = $factory->user->create( array( 'role' => 'editor' ) ); - self::$post_id = $factory->post->create( + self::$editor_id = $factory->user->create( array( 'role' => 'editor' ) ); + self::$editor2_id = $factory->user->create( array( 'role' => 'editor' ) ); + self::$contributor_id = $factory->user->create( array( 'role' => 'contributor' ) ); + self::$post_id = $factory->post->create( array( 'post_title' => 'Test Post', 'post_type' => 'post', @@ -162,4 +164,81 @@ public function test_excludes_non_post_rooms() { $this->assertCount( 0, $response['presence-active-posts'] ); } + + /** + * A contributor has `edit_posts`, which is all the widget itself requires, + * but cannot edit someone else's post and must not learn it is being + * worked on. + * + * @covers WP_Presence_Widget_Active_Posts::heartbeat_received + */ + public function test_heartbeat_excludes_posts_the_user_cannot_edit() { + $room = wp_presence_post_room( self::$post_id ); + wp_set_presence( $room, 'lock-' . self::$editor_id, array(), self::$editor_id ); + + wp_set_current_user( self::$contributor_id ); + + $response = WP_Presence_Widget_Active_Posts::heartbeat_received( + array(), + array( 'presence-active-posts-ping' => true ), + 'dashboard' + ); + + $this->assertCount( 0, $response['presence-active-posts'] ); + } + + /** + * @covers WP_Presence_Widget_Active_Posts::heartbeat_received + */ + public function test_heartbeat_includes_posts_the_user_can_edit() { + $draft_id = self::factory()->post->create( + array( + 'post_author' => self::$contributor_id, + 'post_status' => 'draft', + 'post_title' => 'Contributor Draft', + ) + ); + + $room = wp_presence_post_room( $draft_id ); + wp_set_presence( $room, 'lock-' . self::$contributor_id, array(), self::$contributor_id ); + + wp_set_current_user( self::$contributor_id ); + + $response = WP_Presence_Widget_Active_Posts::heartbeat_received( + array(), + array( 'presence-active-posts-ping' => true ), + 'dashboard' + ); + + $this->assertCount( 1, $response['presence-active-posts'] ); + $this->assertSame( $draft_id, $response['presence-active-posts'][0]['post_id'] ); + } + + /** + * Only the rooms the user cannot reach are dropped, not the whole response. + * + * @covers WP_Presence_Widget_Active_Posts::heartbeat_received + */ + public function test_heartbeat_filters_per_post_rather_than_all_or_nothing() { + $draft_id = self::factory()->post->create( + array( + 'post_author' => self::$contributor_id, + 'post_status' => 'draft', + ) + ); + + wp_set_presence( wp_presence_post_room( self::$post_id ), 'lock-a', array(), self::$editor_id ); + wp_set_presence( wp_presence_post_room( $draft_id ), 'lock-b', array(), self::$contributor_id ); + + wp_set_current_user( self::$contributor_id ); + + $response = WP_Presence_Widget_Active_Posts::heartbeat_received( + array(), + array( 'presence-active-posts-ping' => true ), + 'dashboard' + ); + + $this->assertCount( 1, $response['presence-active-posts'] ); + $this->assertSame( $draft_id, $response['presence-active-posts'][0]['post_id'] ); + } }