diff --git a/docs/abilities.md b/docs/abilities.md index 89d7840..dd23210 100644 --- a/docs/abilities.md +++ b/docs/abilities.md @@ -25,14 +25,17 @@ note. | `create-form` | Builds a form from a title and a loose field list (`{ type, label, required?, hint?, placeholder?, choices? }`); ids are minted, defaults seeded, shortcode returned. Fields go through the same normaliser a JSON import does. | edit forms | | `set-form-theme` | Applies an installed theme to a form; refuses invented slugs rather than falling back. | edit forms | | `list-entries` | Queries submissions with search, date range, status, starred and pagination; answers come back raw (by field id) and readable (by label). | read entries **for that form** | -| `get-entry` | One submission in full: label, raw value and formatted value per question. | read entries (checked against the entry's form) | +| `get-entry` | One submission in full: label, raw value and formatted value per question. | read entries **for the entry's form** | | `submit-form` | Submits through the visitor pipeline — availability, validation, anti-spam, storage, notifications. Refusals return per-field errors and store nothing. | any authenticated user | | `form-report` | The analytics report as data: counts, rates, timeline, distributions, NPS, optional `group_by` breakdown. Prefer it over fetching every entry when the job is summarising. | read entries **for that form** | Where a permission says **for that form**, the `alltfo_can_read_entries` filter is asked with the `form_id` from the ability's input — so a site that confines a user to a department's forms through that filter confines these -abilities the same way. +abilities the same way. `get-entry` names an entry rather than a form, so its +gate resolves the entry to the form it belongs to first (`alltfo_can_read_entry()`) +and asks the filter about that form — the refusal happens at the permission +check, before the ability runs. ## The one honest liberty diff --git a/docs/hooks-reference.md b/docs/hooks-reference.md index 3feca0f..81bc0bb 100644 --- a/docs/hooks-reference.md +++ b/docs/hooks-reference.md @@ -1084,7 +1084,10 @@ entries of the forms it owns by returning false for every other id. The abilities honour it with the form in hand: `get-form`, `list-entries` and `form-report` ask this filter with the `form_id` from their input, and -`list-forms` asks it per form before listing one. +`list-forms` asks it per form before listing one. `get-entry` and the +`/entries/{id}` REST route name an entry, not a form, so their permission gates +go through `alltfo_can_read_entry( $entry_id )`, which resolves the entry to its +form and asks this filter about that form. --- diff --git a/includes/abilities.php b/includes/abilities.php index ea290c3..7ab7d87 100644 --- a/includes/abilities.php +++ b/includes/abilities.php @@ -144,6 +144,13 @@ function alltfo_register_abilities() { return alltfo_can_read_entries( absint( $input['form_id'] ?? 0 ) ); }; + // An ability that names an *entry* asks about the form that entry belongs + // to -- the gate itself, not only the data path behind it, honours the + // per-form filter. + $read_entry = static function ( $input ) { + return alltfo_can_read_entry( absint( $input['entry_id'] ?? 0 ) ); + }; + // Submitting is what visitors do, so no capability is asked for — but the // ability *is* held to an authenticated user, because it mints the time-trap // signature a rendered form would carry, and that liberty is only honest on @@ -341,7 +348,7 @@ function alltfo_register_abilities() { 'label' => __( 'Get an entry', 'allterrain-forms' ), 'description' => __( 'Reads one submission in full: every question with its label, the raw stored value and a formatted human-readable value, plus the submission date and status.', 'allterrain-forms' ), 'category' => $category, - 'permission_callback' => $read_entries, + 'permission_callback' => $read_entry, 'execute_callback' => 'alltfo_ability_get_entry', 'input_schema' => array( 'type' => 'object', diff --git a/includes/post-types.php b/includes/post-types.php index 72849fb..1ca4b56 100644 --- a/includes/post-types.php +++ b/includes/post-types.php @@ -414,6 +414,37 @@ function alltfo_can_read_entries( $form_id = 0 ) { return (bool) apply_filters( 'alltfo_can_read_entries', $can, (int) $form_id ); } +/** + * Whether the current user may read one particular entry. + * + * An entry belongs to a form, and the per-form `alltfo_can_read_entries` + * filter is the seam a site uses to confine a reader to some forms and not + * others -- so a gate that names an entry must ask about *that* entry's form, + * not the blanket "any form at all". This is what the `get-entry` ability and + * the `/entries/{id}` REST route ask before their callbacks run; the data + * path asks it again in `alltfo_prepare_entry()`, so the gate and the door + * agree. + * + * An id that is not an entry falls back to the blanket question, so a caller + * holding the capability still reaches its own "that entry does not exist" + * answer instead of an opaque refusal. + * + * @since 0.1.0 + * + * @param int $entry_id The entry. + * @return bool + */ +function alltfo_can_read_entry( $entry_id ) { + $entry_id = absint( $entry_id ); + $post = $entry_id ? get_post( $entry_id ) : null; + + if ( ! $post || ALLTFO_ENTRY_TYPE !== $post->post_type ) { + return alltfo_can_read_entries(); + } + + return alltfo_can_read_entries( (int) get_post_meta( $post->ID, ALLTFO_META_FORM, true ) ); +} + /** * Every status an entry can hold. * diff --git a/includes/rest.php b/includes/rest.php index 7ba3d59..4af7f0f 100644 --- a/includes/rest.php +++ b/includes/rest.php @@ -369,17 +369,25 @@ function alltfo_rest_can_edit() { * @return true|WP_Error */ function alltfo_rest_can_read_entries( $request ) { + $route = (string) $request->get_route(); $form_id = (int) $request->get_param( 'form_id' ); + $id = (int) $request->get_param( 'id' ); + + // What `id` names depends on the route. On `/entries/{id}` it is an + // *entry*, and the per-form question is about the form that entry belongs + // to. On `/forms/{id}/analytics` it is the form itself. Treating one as + // the other would ask the per-form filter about the wrong thing entirely. + if ( ! $form_id && $id && false !== strpos( $route, '/entries/' ) ) { + $allowed = alltfo_can_read_entry( $id ); + } else { + if ( ! $form_id && false !== strpos( $route, '/forms/' ) ) { + $form_id = $id; + } - // The analytics route names its form parameter `id`, because it lives at - // `/forms/{id}/analytics`. Only routes under `/forms/` may read it that - // way -- on `/entries/{id}` the `id` is an *entry*, and treating it as a - // form would ask the per-form filter about the wrong thing entirely. - if ( ! $form_id && false !== strpos( (string) $request->get_route(), '/forms/' ) ) { - $form_id = (int) $request->get_param( 'id' ); + $allowed = alltfo_can_read_entries( $form_id ); } - if ( alltfo_can_read_entries( $form_id ) ) { + if ( $allowed ) { return true; } diff --git a/tests/phpunit/tests/abilities.php b/tests/phpunit/tests/abilities.php index ff26cb3..9f0764b 100644 --- a/tests/phpunit/tests/abilities.php +++ b/tests/phpunit/tests/abilities.php @@ -342,20 +342,48 @@ public function test_a_subscriber_can_submit() { $this->assertSame( 1, alltfo_count_entries( $form_id ) ); } + /** + * Submits one answer to a form and returns the entry id. + * + * @param int $form_id The form. + * @return int The entry id. + */ + protected function make_entry( $form_id ) { + $form = $this->run_ability( 'allterrain-forms/get-form', array( 'form_id' => $form_id ) ); + $result = $this->run_ability( + 'allterrain-forms/submit-form', + array( + 'form_id' => $form_id, + 'values' => array( $form['fields'][0]['id'] => 'Tomas Berg' ), + ) + ); + + $this->assertTrue( $result['success'] ); + + return (int) $result['entry_id']; + } + /** * The per-form filter confines every reading ability, id in hand. * * A user the `alltfo_can_read_entries` filter restricts to one form gets - * that form and no other — from get-form, from form-report, and from the - * list itself. + * that form and no other — from get-form, from form-report, from get-entry, + * and from the list itself. For get-entry the refusal must come from the + * *permission gate*, not only from the data path behind it: an ability's + * declared gate is what an MCP client and the REST channel consult before + * running anything. * * @covers ::alltfo_register_abilities * @covers ::alltfo_ability_list_forms + * @covers ::alltfo_can_read_entry */ public function test_per_form_filter_confines_the_read_abilities() { $allowed = $this->make_form(); $other = $this->make_form(); + $my_entry = $this->make_entry( $allowed ); + $their_entry = $this->make_entry( $other ); + $reader = self::factory()->user->create( array( 'role' => 'subscriber' ) ); ( new WP_User( $reader ) )->add_cap( 'alltfo_read_entries' ); @@ -385,6 +413,20 @@ public function test_per_form_filter_confines_the_read_abilities() { $this->assertContains( $allowed, $ids ); $this->assertNotContains( $other, $ids, 'The list is confined the same way.' ); + $get_entry = wp_get_ability( 'allterrain-forms/get-entry' ); + + $this->assertTrue( $get_entry->check_permissions( array( 'entry_id' => $my_entry ) ) ); + $this->assertFalse( + $get_entry->check_permissions( array( 'entry_id' => $their_entry ) ), + 'The gate itself refuses an entry of a form outside the filter.' + ); + + $this->assertIsArray( $get_entry->execute( array( 'entry_id' => $my_entry ) ) ); + $this->assertWPError( + $get_entry->execute( array( 'entry_id' => $their_entry ) ), + 'An entry of a form outside the filter is not readable.' + ); + remove_filter( 'alltfo_can_read_entries', $filter, 10 ); } } diff --git a/tests/phpunit/tests/entries.php b/tests/phpunit/tests/entries.php index 5ba98f4..9cf2fcb 100644 --- a/tests/phpunit/tests/entries.php +++ b/tests/phpunit/tests/entries.php @@ -180,14 +180,18 @@ public function test_starring_requires_an_entry() { } /** - * The analytics route's form id reaches the per-form read filter. + * The analytics and entry routes ask the per-form read filter about the + * right form. * - * The route names its parameter `id`, because it lives at - * `/forms/{id}/analytics` -- and the permission callback used to read only - * `form_id`, so the documented `alltfo_can_read_entries` seam was always - * asked about form 0 rather than the form whose numbers were being read. + * Both name their parameter `id`, and it means a different thing on each: + * the form itself on `/forms/{id}/analytics`, an entry on `/entries/{id}`. + * The permission callback used to read only `form_id`, so the documented + * `alltfo_can_read_entries` seam was asked about form 0 on both -- and an + * entry route must resolve its entry to the form it belongs to rather than + * mistake the entry id for a form id. * * @covers ::alltfo_rest_can_read_entries + * @covers ::alltfo_can_read_entry */ public function test_analytics_permission_asks_about_the_right_form() { wp_set_current_user( self::factory()->user->create( array( 'role' => 'administrator' ) ) ); @@ -209,8 +213,8 @@ public function test_analytics_permission_asks_about_the_right_form() { $result = alltfo_rest_can_read_entries( $request ); - // An entry route's `id` is an entry, not a form, and must stay out of - // the per-form question. + // An entry route's `id` is an entry, not a form: the per-form question + // is about the form the entry belongs to. $entry_request = new WP_REST_Request( 'GET', '/' . ALLTFO_REST_NAMESPACE . '/entries/' . $seeded['entry_id'] ); $entry_request->set_param( 'id', $seeded['entry_id'] ); @@ -221,10 +225,51 @@ public function test_analytics_permission_asks_about_the_right_form() { $this->assertWPError( $result, 'Denying the form must deny its analytics.' ); $this->assertContains( $seeded['form_id'], $asked, 'The filter must be asked about the form being read.' ); - $this->assertTrue( $entry_result ); + $this->assertWPError( $entry_result, 'Denying the form must deny each of its entries at the gate.' ); $this->assertNotContains( $seeded['entry_id'], $asked, 'An entry id must never be mistaken for a form id.' ); } + /** + * Asking about an entry asks about its form; asking about no entry asks + * the blanket question. + * + * @covers ::alltfo_can_read_entry + */ + public function test_reading_one_entry_asks_about_its_form() { + wp_set_current_user( self::factory()->user->create( array( 'role' => 'administrator' ) ) ); + alltfo_add_capabilities(); + + $seeded = $this->seed(); + $asked = array(); + + $filter = static function ( $can, $form_id ) use ( &$asked ) { + $asked[] = $form_id; + + return $can; + }; + + add_filter( 'alltfo_can_read_entries', $filter, 10, 2 ); + + $this->assertTrue( alltfo_can_read_entry( $seeded['entry_id'] ) ); + $this->assertSame( array( $seeded['form_id'] ), $asked ); + + $asked = array(); + + // A form id, a page id, an invented id: none is an entry, so the + // question falls back to "any form at all" and the caller keeps its + // own "does not exist" answer. + $this->assertTrue( alltfo_can_read_entry( $seeded['form_id'] ) ); + $this->assertTrue( alltfo_can_read_entry( 987654 ) ); + $this->assertSame( array( 0, 0 ), $asked ); + + remove_filter( 'alltfo_can_read_entries', $filter ); + + wp_set_current_user( self::factory()->user->create( array( 'role' => 'subscriber' ) ) ); + + $this->assertFalse( alltfo_can_read_entry( $seeded['entry_id'] ), 'Without the capability nothing is readable.' ); + $this->assertFalse( alltfo_can_read_entry( 987654 ) ); + } + /* ---------------------------------------------------------------- Export */ /**