Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions docs/abilities.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
5 changes: 4 additions & 1 deletion docs/hooks-reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

---

Expand Down
9 changes: 8 additions & 1 deletion includes/abilities.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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',
Expand Down
31 changes: 31 additions & 0 deletions includes/post-types.php
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down
22 changes: 15 additions & 7 deletions includes/rest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
46 changes: 44 additions & 2 deletions tests/phpunit/tests/abilities.php
Original file line number Diff line number Diff line change
Expand Up @@ -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' );
Expand Down Expand Up @@ -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 );
}
}
61 changes: 53 additions & 8 deletions tests/phpunit/tests/entries.php
Original file line number Diff line number Diff line change
Expand Up @@ -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' ) ) );
Expand All @@ -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'] );

Expand All @@ -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 */

/**
Expand Down
Loading