diff --git a/php/context/class-fieldmanager-context-term.php b/php/context/class-fieldmanager-context-term.php index cd6c777a80..6f9ce6a5ec 100644 --- a/php/context/class-fieldmanager-context-term.php +++ b/php/context/class-fieldmanager-context-term.php @@ -80,6 +80,15 @@ class Fieldmanager_Context_Term extends Fieldmanager_Context_Storable { */ private $current_taxonomy; + /** + * Store data for inserted terms to ensure, as much as possible, that FM + * only stores data to the term being created and not any other term created + * as a side effect. + * + * @var array|null + */ + protected $inserting_term_data; + /** * Instantiate this context. You can either pass an array of all args * (preferred), or pass them individually (deprecated). @@ -161,6 +170,7 @@ public function __construct( $args, $taxonomies = array(), $show_on_add = true, foreach ( $this->taxonomies as $taxonomy ) { if ( $this->show_on_add ) { add_action( $taxonomy . '_add_form_fields', array( $this, 'add_term_fields' ), 10, 1 ); + add_filter( 'wp_insert_term_data', array( $this, 'verify_term_data_on_insert' ), PHP_INT_MAX, 3 ); add_action( 'created_term', array( $this, 'save_term_fields' ), 10, 3 ); } @@ -288,6 +298,37 @@ public function term_fields( $html_template, $taxonomy, $term = null ) { * @param string $taxonomy The term taxonomy. */ public function save_term_fields( $term_id, $tt_id, $taxonomy ) { + // phpcs:disable WordPress.Security.NonceVerification.Missing + // Ensure that the term being created/updated is the term in the request. + if ( ! empty( $_POST['tag_ID'] ) && $term_id !== (int) $_POST['tag_ID'] ) { + return; + } elseif ( empty( $_POST['tag_ID'] ) ) { + // Ensure this was the created_term action. + if ( ! doing_action( 'created_term' ) ) { + return; + } + + if ( empty( $this->inserting_term_data ) ) { + // Something has gone awry, this shouldn't happen, so bail. + return; + } + + // Core expects terms to have a unique combination of [taxonomy, name, parent]. + // Therefore, this verifies that data against what was recorded prior to insert. + $term = get_term( $term_id ); + if ( + $term->name !== $this->inserting_term_data['name'] + || $term->parent !== $this->inserting_term_data['parent'] + || $taxonomy !== $this->inserting_term_data['taxonomy'] + ) { + return; + } + + // This term appears to check out. Remove the recorded insert data. + unset( $this->inserting_term_data ); + } + // phpcs:enable WordPress.Security.NonceVerification.Missing + // Make sure this field is attached to the taxonomy being saved and this is the appropriate action. // phpcs:ignore WordPress.PHP.StrictInArray.MissingTrueStrict -- baseline if ( ! in_array( $taxonomy, $this->taxonomies ) ) { @@ -310,6 +351,44 @@ public function save_term_fields( $term_id, $tt_id, $taxonomy ) { $this->save_to_term_meta( $term_id, $taxonomy ); } + /** + * Verify that the term being inserted is the term that Fieldmanager data + * should be added to. + * + * This method serves to protect against a plugin or theme creating one or + * more additional terms as a side effect of a term created from the form. + * In order to protect against that, it's necessary to check the inserting + * term's [name, taxonomy, parent], as WordPress requires the combination of + * these fields to be unique. In order to guard against a plugin or theme + * changing this data, it is recorded immediately prior to the term being + * insert, and then checked in `save_term_fields()`. + * + * @param array $data { + * Term data to be inserted. + * + * @type string $name Term name. + * @type string $slug Term slug. + * @type int $term_group Term group. + * } + * @param string $taxonomy Taxonomy slug. + * @param array $args Arguments passed to wp_insert_term(), which is + * raw $_POST data. + * @return array Unmodified `$data`. + */ + public function verify_term_data_on_insert( $data, $taxonomy, $args ) { + // Confirm that this data has the FM nonce for this field. + if ( ! empty( $args[ $this->nonce_key() ] ) ) { + // Append the data to the queued insert. + $this->inserting_term_data = array( + 'name' => $data['name'], + 'taxonomy' => $taxonomy, + 'parent' => ! empty( $args['parent'] ) ? max( (int) $args['parent'], 0 ) : 0, + ); + } + + return $data; + } + /** * Helper to save an array of data to term meta. * diff --git a/php/context/class-fieldmanager-context.php b/php/context/class-fieldmanager-context.php index c4763467a0..eb46dcdd7a 100644 --- a/php/context/class-fieldmanager-context.php +++ b/php/context/class-fieldmanager-context.php @@ -41,6 +41,15 @@ public function __construct() { add_filter( 'wp_refresh_nonces', array( $this, 'refresh_nonce' ) ); } + /** + * Get the nonce key for this field. + * + * @return string + */ + protected function nonce_key() { + return 'fieldmanager-' . $this->fm->name . '-nonce'; + } + /** * Include a fresh nonce for this field in a response with refreshed nonces. * @@ -50,7 +59,7 @@ public function __construct() { * @return array Updated response data. */ public function refresh_nonce( $response ) { - $response['fieldmanager_refresh_nonces']['replace'][ 'fieldmanager-' . $this->fm->name . '-nonce' ] = wp_create_nonce( 'fieldmanager-save-' . $this->fm->name ); + $response['fieldmanager_refresh_nonces']['replace'][ $this->nonce_key() ] = wp_create_nonce( 'fieldmanager-save-' . $this->fm->name ); return $response; } @@ -62,12 +71,12 @@ public function refresh_nonce( $response ) { * @return bool */ protected function is_valid_nonce() { - if ( empty( $_POST[ 'fieldmanager-' . $this->fm->name . '-nonce' ] ) ) { + if ( empty( $_POST[ $this->nonce_key() ] ) ) { return false; } // phpcs:ignore WordPress.Security.ValidatedSanitizedInput.InputNotSanitized -- baseline - if ( ! wp_verify_nonce( $_POST[ 'fieldmanager-' . $this->fm->name . '-nonce' ], 'fieldmanager-save-' . $this->fm->name ) ) { + if ( ! wp_verify_nonce( $_POST[ $this->nonce_key() ], 'fieldmanager-save-' . $this->fm->name ) ) { $this->fm->_unauthorized_access( __( 'Nonce validation failed', 'fieldmanager' ) ); } @@ -113,7 +122,7 @@ protected function render_field( $args = array() ) { $data = array_key_exists( 'data', $args ) ? $args['data'] : null; $echo = isset( $args['echo'] ) ? $args['echo'] : true; - $nonce = wp_nonce_field( 'fieldmanager-save-' . $this->fm->name, 'fieldmanager-' . $this->fm->name . '-nonce', true, false ); + $nonce = wp_nonce_field( 'fieldmanager-save-' . $this->fm->name, $this->nonce_key(), true, false ); $field = $this->fm->element_markup( $data ); if ( $echo ) { // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped -- baseline @@ -122,5 +131,4 @@ protected function render_field( $args = array() ) { return $nonce . $field; } } - } diff --git a/tests/php/test-fieldmanager-context-term.php b/tests/php/test-fieldmanager-context-term.php index d944f85c9b..2af9622d3b 100644 --- a/tests/php/test-fieldmanager-context-term.php +++ b/tests/php/test-fieldmanager-context-term.php @@ -408,4 +408,161 @@ public function test_unserialize_data_mixed_depth() { $this->assertStringContainsString('name="base_group[test_group][deep_text]"', $html ); $this->assertStringContainsString( '>' . $data['test_group']['deep_text'] . '', $html ); } + + /** + * @see https://github.com/alleyinteractive/wordpress-fieldmanager/issues/831 + */ + public function test_term_saving_side_effects_on_term_update() { + $name = 'side-effect'; + $value = 'Fieldmanager was here'; + + // Register a Fieldmanager Field and add the term context. + $fm = new Fieldmanager_TextField( compact( 'name' ) ); + $fm->add_term_meta_box( 'Testing Side Effects', [ $this->taxonomy ] ); + + // Set the POST data. + $_POST = [ + 'tag_ID' => $this->term_id, + 'taxonomy' => $this->taxonomy, + 'name' => 'News', + 'slug' => 'news', + 'description' => 'General news', + 'parent' => '-1', + "fieldmanager-{$name}-nonce" => wp_create_nonce( "fieldmanager-save-{$name}" ), + $name => $value, + ]; + + // Trigger the intended save. + do_action( + 'edited_term', + $this->term_id, + $this->tt_id, + $this->taxonomy + ); + + // Fake a side effect. + $side_effect_term = self::factory()->term->create_and_get( + [ + 'taxonomy' => $this->taxonomy, + ] + ); + do_action( + 'edited_term', + $side_effect_term->term_id, + $side_effect_term->term_taxonomy_id, + $this->taxonomy + ); + + $this->assertSame( $value, get_term_meta( $this->term_id, $name, true ) ); + $this->assertSame( [], get_term_meta( $side_effect_term->term_id, $name ) ); + } + + /** + * @see https://github.com/alleyinteractive/wordpress-fieldmanager/issues/831 + */ + public function test_term_saving_side_effects_on_term_create() { + $name = 'side-effect'; + $value = 'Fieldmanager was here'; + $new_term_name = 'New Term'; + + // Register a Fieldmanager Field and add the term context. + $fm = new Fieldmanager_TextField( compact( 'name' ) ); + $fm->add_term_meta_box( 'Testing Side Effects', [ $this->taxonomy ] ); + + // Set the POST data. + $_POST = [ + 'taxonomy' => $this->taxonomy, + 'tag-name' => $new_term_name, + 'slug' => '', + 'description' => '', + 'parent' => '-1', + "fieldmanager-{$name}-nonce" => wp_create_nonce( "fieldmanager-save-{$name}" ), + $name => $value, + ]; + + // Trigger the intended save. + $new_term = wp_insert_term( $new_term_name, $this->taxonomy, $_POST ); + + // Fake a side effect. + $side_effect_term = self::factory()->term->create_and_get( + [ + 'taxonomy' => $this->taxonomy, + ] + ); + do_action( + 'edited_term', + $side_effect_term->term_id, + $side_effect_term->term_taxonomy_id, + $this->taxonomy + ); + + $this->assertSame( $value, get_term_meta( $new_term['term_id'], $name, true ) ); + $this->assertSame( [], get_term_meta( $side_effect_term->term_id, $name ) ); + } + + /** + * @see https://github.com/alleyinteractive/wordpress-fieldmanager/issues/831 + */ + public function test_term_meta_saving_on_term_create_when_a_filter_alters_the_term_name() { + $name = 'testing'; + $value = 'Fieldmanager was here'; + $new_term_name = 'New Term'; + + // Register a Fieldmanager Field and add the term context. + $fm = new Fieldmanager_TextField( compact( 'name' ) ); + $fm->add_term_meta_box( 'Testing', [ $this->taxonomy ] ); + + // Set the POST data. + $_POST = [ + 'taxonomy' => $this->taxonomy, + 'tag-name' => $new_term_name, + 'slug' => '', + 'description' => '', + 'parent' => '-1', + "fieldmanager-{$name}-nonce" => wp_create_nonce( "fieldmanager-save-{$name}" ), + $name => $value, + ]; + + // Manipualte the term name prior to insert. + add_filter( + 'pre_insert_term', + function( $term_name ) { + return "Edited: {$term_name}"; + } + ); + + // Insert the term. + $term = wp_insert_term( $new_term_name, $this->taxonomy, $_POST ); + + $this->assertSame( $value, get_term_meta( $term['term_id'], $name, true ) ); + } + + /** + * @see https://github.com/alleyinteractive/wordpress-fieldmanager/issues/831 + */ + public function test_term_meta_saving_on_term_create_when_term_name_has_special_characters() { + $name = 'testing'; + $value = 'Fieldmanager was here'; + $new_term_name = 'Aprés & Mañana™'; + + // Register a Fieldmanager Field and add the term context. + $fm = new Fieldmanager_TextField( compact( 'name' ) ); + $fm->add_term_meta_box( 'Testing', [ $this->taxonomy ] ); + + // Set the POST data. + $_POST = [ + 'taxonomy' => $this->taxonomy, + 'tag-name' => $new_term_name, + 'slug' => '', + 'description' => '', + 'parent' => '-1', + "fieldmanager-{$name}-nonce" => wp_create_nonce( "fieldmanager-save-{$name}" ), + $name => $value, + ]; + + // Insert the term. + $term = wp_insert_term( $new_term_name, $this->taxonomy, $_POST ); + + $this->assertSame( $value, get_term_meta( $term['term_id'], $name, true ) ); + } }