'; $content .= \esc_html__( 'This post does not add value to your website and solely exists to show what a post can look like. Therefore, "Hello World!" is not needed and should be deleted.', 'progress-planner' ); diff --git a/classes/suggested-tasks/providers/class-remove-terms-without-posts.php b/classes/suggested-tasks/providers/class-remove-terms-without-posts.php index 79c8cf9f2a..12bd8b1323 100644 --- a/classes/suggested-tasks/providers/class-remove-terms-without-posts.php +++ b/classes/suggested-tasks/providers/class-remove-terms-without-posts.php @@ -453,6 +453,25 @@ public function handle_interactive_task_submit() { \wp_send_json_error( [ 'message' => \esc_html__( 'Term not found.', 'progress-planner' ) ] ); } + // Only public taxonomies are tracked by this task, mirroring the check in + // maybe_remove_irrelevant_tasks(). + $taxonomy_object = \get_taxonomy( $taxonomy ); + if ( ! $taxonomy_object || ! $taxonomy_object->public ) { + \wp_send_json_error( [ 'message' => \esc_html__( 'You do not have permission to delete terms.', 'progress-planner' ) ] ); + } + + // Bind the request to a task that actually suggested this term, so this + // handler cannot be repurposed to delete arbitrary terms. + if ( ! $this->has_task_for_term( $term_id, $taxonomy ) ) { + \wp_send_json_error( [ 'message' => \esc_html__( 'You do not have permission to delete terms.', 'progress-planner' ) ] ); + } + + // Re-check the post count at deletion time: the term may have gained posts + // after the task was created. + if ( $term->count > self::MIN_POSTS ) { + \wp_send_json_error( [ 'message' => \esc_html__( 'This term is not empty and cannot be deleted.', 'progress-planner' ) ] ); + } + // Delete the term. $result = \wp_delete_term( $term_id, $taxonomy ); @@ -462,4 +481,24 @@ public function handle_interactive_task_submit() { \wp_send_json_success( [ 'message' => \esc_html__( 'Term deleted successfully.', 'progress-planner' ) ] ); } + + /** + * Check whether a task from this provider targets the given term. + * + * @param int $term_id The term ID. + * @param string $taxonomy The taxonomy. + * + * @return bool + */ + protected function has_task_for_term( $term_id, $taxonomy ) { + foreach ( \progress_planner()->get_suggested_tasks_db()->get_tasks_by( [ 'provider_id' => $this->get_provider_id() ] ) as $task ) { + if ( (int) $task->target_term_id === (int) $term_id + && (string) $task->target_taxonomy === (string) $taxonomy + ) { + return true; + } + } + + return false; + } } diff --git a/classes/suggested-tasks/providers/class-sample-page.php b/classes/suggested-tasks/providers/class-sample-page.php index e7ae313cf8..fef998f9fd 100644 --- a/classes/suggested-tasks/providers/class-sample-page.php +++ b/classes/suggested-tasks/providers/class-sample-page.php @@ -114,7 +114,7 @@ protected function get_description() { $content .= \sprintf( /* translators: %s: Link to the post. */ \esc_html__( 'On install, WordPress creates a "Sample Page" page. You can find yours at %s.', 'progress-planner' ), - '' . \esc_html( $sample_page_url ) . '', + '' . \esc_html( $sample_page_url ) . '', ); $content .= '
'; $content .= \esc_html__( 'This page does not add value to your website and solely exists to show what a page can look like. Therefore, "Sample Page" is not needed and should be deleted.', 'progress-planner' ); diff --git a/classes/suggested-tasks/providers/class-tasks.php b/classes/suggested-tasks/providers/class-tasks.php index 854bfebe99..94518540ef 100644 --- a/classes/suggested-tasks/providers/class-tasks.php +++ b/classes/suggested-tasks/providers/class-tasks.php @@ -736,7 +736,7 @@ public function get_task_actions( $data = [] ) { if ( $this->get_external_link_url() ) { $actions[] = [ 'priority' => 40, - 'html' => '' . \esc_html__( 'Why is this important?', 'progress-planner' ) . '', + 'html' => '' . \esc_html__( 'Why is this important?', 'progress-planner' ) . '', ]; } elseif ( isset( $data['content']['rendered'] ) && $data['content']['rendered'] !== '' && ! $this instanceof Tasks_Interactive ) { $actions[] = [ diff --git a/classes/suggested-tasks/providers/class-update-term-description.php b/classes/suggested-tasks/providers/class-update-term-description.php index 455a5675b7..fc27fef86f 100644 --- a/classes/suggested-tasks/providers/class-update-term-description.php +++ b/classes/suggested-tasks/providers/class-update-term-description.php @@ -445,6 +445,18 @@ public function handle_interactive_task_submit() { \wp_send_json_error( [ 'message' => \esc_html__( 'Term not found.', 'progress-planner' ) ] ); } + // Only public taxonomies are tracked by this task. + $taxonomy_object = \get_taxonomy( $taxonomy ); + if ( ! $taxonomy_object || ! $taxonomy_object->public ) { + \wp_send_json_error( [ 'message' => \esc_html__( 'You do not have permission to update terms.', 'progress-planner' ) ] ); + } + + // Bind the request to a task that actually suggested this term, so this + // handler cannot be repurposed to edit arbitrary terms. + if ( ! $this->has_task_for_term( $term_id, $taxonomy ) ) { + \wp_send_json_error( [ 'message' => \esc_html__( 'You do not have permission to update terms.', 'progress-planner' ) ] ); + } + // Update the term description. $result = \wp_update_term( $term_id, @@ -460,4 +472,24 @@ public function handle_interactive_task_submit() { \wp_send_json_success( [ 'message' => \esc_html__( 'Term description updated successfully.', 'progress-planner' ) ] ); } + + /** + * Check whether a task from this provider targets the given term. + * + * @param int $term_id The term ID. + * @param string $taxonomy The taxonomy. + * + * @return bool + */ + protected function has_task_for_term( $term_id, $taxonomy ) { + foreach ( \progress_planner()->get_suggested_tasks_db()->get_tasks_by( [ 'provider_id' => $this->get_provider_id() ] ) as $task ) { + if ( (int) $task->target_term_id === (int) $term_id + && (string) $task->target_taxonomy === (string) $taxonomy + ) { + return true; + } + } + + return false; + } } diff --git a/classes/suggested-tasks/providers/integrations/aioseo/class-archive-author.php b/classes/suggested-tasks/providers/integrations/aioseo/class-archive-author.php index 20cc606e49..4d5437cd6d 100644 --- a/classes/suggested-tasks/providers/integrations/aioseo/class-archive-author.php +++ b/classes/suggested-tasks/providers/integrations/aioseo/class-archive-author.php @@ -153,8 +153,7 @@ public function print_popover_form_contents() { * @return void */ public function handle_interactive_task_specific_submit() { - $this->verify_aioseo_active_or_fail(); - $this->verify_nonce_or_fail(); + $this->verify_aioseo_ajax_security(); \aioseo()->options->searchAppearance->archives->author->show = false; // @phpstan-ignore-line diff --git a/classes/suggested-tasks/providers/integrations/aioseo/class-archive-date.php b/classes/suggested-tasks/providers/integrations/aioseo/class-archive-date.php index 2b60d55fc6..e0b124128b 100644 --- a/classes/suggested-tasks/providers/integrations/aioseo/class-archive-date.php +++ b/classes/suggested-tasks/providers/integrations/aioseo/class-archive-date.php @@ -140,8 +140,7 @@ public function print_popover_form_contents() { * @return void */ public function handle_interactive_task_specific_submit() { - $this->verify_aioseo_active_or_fail(); - $this->verify_nonce_or_fail(); + $this->verify_aioseo_ajax_security(); \aioseo()->options->searchAppearance->archives->date->show = false; // @phpstan-ignore-line diff --git a/classes/suggested-tasks/providers/integrations/aioseo/class-crawl-settings-feed-authors.php b/classes/suggested-tasks/providers/integrations/aioseo/class-crawl-settings-feed-authors.php index 74297316e5..950eb99b44 100644 --- a/classes/suggested-tasks/providers/integrations/aioseo/class-crawl-settings-feed-authors.php +++ b/classes/suggested-tasks/providers/integrations/aioseo/class-crawl-settings-feed-authors.php @@ -150,8 +150,7 @@ public function print_popover_form_contents() { * @return void */ public function handle_interactive_task_specific_submit() { - $this->verify_aioseo_active_or_fail(); - $this->verify_nonce_or_fail(); + $this->verify_aioseo_ajax_security(); \aioseo()->options->searchAppearance->advanced->crawlCleanup->feeds->authors = false; // @phpstan-ignore-line diff --git a/classes/suggested-tasks/providers/integrations/aioseo/class-crawl-settings-feed-comments.php b/classes/suggested-tasks/providers/integrations/aioseo/class-crawl-settings-feed-comments.php index b942ac4a4c..528174c5aa 100644 --- a/classes/suggested-tasks/providers/integrations/aioseo/class-crawl-settings-feed-comments.php +++ b/classes/suggested-tasks/providers/integrations/aioseo/class-crawl-settings-feed-comments.php @@ -119,8 +119,7 @@ public function print_popover_form_contents() { * @return void */ public function handle_interactive_task_specific_submit() { - $this->verify_aioseo_active_or_fail(); - $this->verify_nonce_or_fail(); + $this->verify_aioseo_ajax_security(); // Global comment feed. if ( \aioseo()->options->searchAppearance->advanced->crawlCleanup->feeds->globalComments ) { // @phpstan-ignore-line diff --git a/classes/suggested-tasks/providers/integrations/aioseo/class-media-pages.php b/classes/suggested-tasks/providers/integrations/aioseo/class-media-pages.php index 93b22f6461..cc74ba42b1 100644 --- a/classes/suggested-tasks/providers/integrations/aioseo/class-media-pages.php +++ b/classes/suggested-tasks/providers/integrations/aioseo/class-media-pages.php @@ -126,8 +126,7 @@ public function print_popover_form_contents() { * @return void */ public function handle_interactive_task_specific_submit() { - $this->verify_aioseo_active_or_fail(); - $this->verify_nonce_or_fail(); + $this->verify_aioseo_ajax_security(); \aioseo()->dynamicOptions->searchAppearance->postTypes->attachment->redirectAttachmentUrls = 'attachment'; // @phpstan-ignore-line diff --git a/tests/phpunit/test-class-term-task-binding.php b/tests/phpunit/test-class-term-task-binding.php new file mode 100644 index 0000000000..b7c97af85f --- /dev/null +++ b/tests/phpunit/test-class-term-task-binding.php @@ -0,0 +1,241 @@ +_setRole( 'editor' ); + } + + /** + * Clean up. + */ + public function tear_down() { + unset( $_REQUEST['term_id'], $_REQUEST['taxonomy'], $_REQUEST['description'], $_REQUEST['nonce'] ); + $_POST = []; + parent::tear_down(); + } + + /** + * Populate the request with a signed payload. + * + * @param int $term_id The term ID. + * @param string $taxonomy The taxonomy. + * @param string $description Optional description. + * + * @return void + */ + private function set_request( $term_id, $taxonomy, $description = null ) { + $_REQUEST['term_id'] = $term_id; + $_REQUEST['taxonomy'] = $taxonomy; + $_REQUEST['nonce'] = \wp_create_nonce( 'progress_planner' ); + + if ( null !== $description ) { + $_REQUEST['description'] = $description; + } + + // The handlers read from $_POST; check_ajax_referer reads $_REQUEST. + $_POST = $_REQUEST; + } + + /** + * Run a handler and return the decoded JSON response. + * + * @param object $provider The task provider. + * + * @return array|null The decoded response. + */ + private function get_response( $provider ) { + // WordPress Core's _handleAjax() starts output buffering before calling + // AJAX actions. We do the same since we call the method directly. + \ini_set( 'implicit_flush', false ); // phpcs:ignore WordPress.PHP.IniSet.Risky + \ob_start(); + + try { + $provider->handle_interactive_task_submit(); + } catch ( \WPAjaxDieContinueException $e ) { // phpcs:ignore Generic.CodeAnalysis.EmptyStatement.DetectedCatch + // Expected: wp_send_json_* calls wp_die(). + } + + return \json_decode( $this->_last_response, true ); + } + + /** + * Create a task from a provider that targets a specific term. + * + * @param object $provider The task provider. + * @param int $term_id The term ID. + * @param string $taxonomy The taxonomy. + * + * @return void + */ + private function create_task_for_term( $provider, $term_id, $taxonomy ) { + \progress_planner()->get_suggested_tasks_db()->add( + [ + 'post_title' => 'Term task', + 'provider_id' => $provider->get_provider_id(), + 'target_term_id' => $term_id, + 'target_taxonomy' => $taxonomy, + ] + ); + } + + /** + * An Editor cannot delete an unrelated term that no task suggested. + * + * This is the privilege-escalation case: wp_delete_term() performs no + * capability check of its own, so without binding an Editor could delete + * any term in any taxonomy. + * + * @return void + */ + public function test_delete_rejects_term_without_matching_task() { + $term_id = $this->factory->term->create( [ 'taxonomy' => 'category' ] ); + + $this->set_request( $term_id, 'category' ); + $response = $this->get_response( new Remove_Terms_Without_Posts() ); + + $this->assertFalse( $response['success'], 'Deletion should be rejected.' ); + + $term = \get_term( $term_id, 'category' ); + $this->assertFalse( \is_wp_error( $term ), 'Term should not have been deleted.' ); + $this->assertNotNull( $term, 'Term should not have been deleted.' ); + } + + /** + * An Editor cannot rewrite the description of a term no task suggested. + * + * @return void + */ + public function test_update_description_rejects_term_without_matching_task() { + $term_id = $this->factory->term->create( + [ + 'taxonomy' => 'category', + 'description' => 'Original description.', + ] + ); + + $this->set_request( $term_id, 'category', 'Injected description.' ); + $response = $this->get_response( new Update_Term_Description() ); + + $this->assertFalse( $response['success'], 'Update should be rejected.' ); + + $term = \get_term( $term_id, 'category' ); + $this->assertSame( 'Original description.', $term->description, 'Description should be unchanged.' ); + } + + /** + * A non-public taxonomy is rejected even when a task exists for the term. + * + * @return void + */ + public function test_delete_rejects_non_public_taxonomy() { + \register_taxonomy( 'prpl_private_tax', 'post', [ 'public' => false ] ); + $term_id = $this->factory->term->create( [ 'taxonomy' => 'prpl_private_tax' ] ); + + $provider = new Remove_Terms_Without_Posts(); + $this->create_task_for_term( $provider, $term_id, 'prpl_private_tax' ); + + $this->set_request( $term_id, 'prpl_private_tax' ); + $response = $this->get_response( $provider ); + + $this->assertFalse( $response['success'], 'Non-public taxonomy should be rejected.' ); + $this->assertFalse( \is_wp_error( \get_term( $term_id, 'prpl_private_tax' ) ), 'Term should not have been deleted.' ); + + \unregister_taxonomy( 'prpl_private_tax' ); + } + + /** + * A term that a task legitimately suggested can still be deleted. + * + * Guards against the binding being too strict and breaking the feature. + * + * @return void + */ + public function test_delete_allows_term_suggested_by_a_task() { + $term_id = $this->factory->term->create( [ 'taxonomy' => 'category' ] ); + + $provider = new Remove_Terms_Without_Posts(); + $this->create_task_for_term( $provider, $term_id, 'category' ); + + $this->set_request( $term_id, 'category' ); + $response = $this->get_response( $provider ); + + $this->assertTrue( $response['success'], 'Deletion of a suggested term should succeed.' ); + + $term = \get_term( $term_id, 'category' ); + $this->assertTrue( null === $term || \is_wp_error( $term ), 'Term should have been deleted.' ); + } + + /** + * A description update for a legitimately suggested term still works. + * + * @return void + */ + public function test_update_description_allows_term_suggested_by_a_task() { + $term_id = $this->factory->term->create( + [ + 'taxonomy' => 'category', + 'description' => 'Original description.', + ] + ); + + $provider = new Update_Term_Description(); + $this->create_task_for_term( $provider, $term_id, 'category' ); + + $this->set_request( $term_id, 'category', 'A better description.' ); + $response = $this->get_response( $provider ); + + $this->assertTrue( $response['success'], 'Update of a suggested term should succeed.' ); + + $term = \get_term( $term_id, 'category' ); + $this->assertSame( 'A better description.', $term->description, 'Description should have been updated.' ); + } + + /** + * A suggested term that has since gained posts is not deleted. + * + * @return void + */ + public function test_delete_rejects_suggested_term_that_gained_posts() { + $term_id = $this->factory->term->create( [ 'taxonomy' => 'category' ] ); + + $provider = new Remove_Terms_Without_Posts(); + $this->create_task_for_term( $provider, $term_id, 'category' ); + + // Give the term enough posts to exceed MIN_POSTS. + foreach ( [ 1, 2 ] as $ignored ) { + $post_id = $this->factory->post->create(); + \wp_set_object_terms( $post_id, [ $term_id ], 'category', true ); + } + + $this->set_request( $term_id, 'category' ); + $response = $this->get_response( $provider ); + + $this->assertFalse( $response['success'], 'A term with posts should not be deleted.' ); + $this->assertFalse( \is_wp_error( \get_term( $term_id, 'category' ) ), 'Term should not have been deleted.' ); + } +}