diff --git a/docs/3.registering-abilities.md b/docs/3.registering-abilities.md index 9acc5ced..c7881751 100644 --- a/docs/3.registering-abilities.md +++ b/docs/3.registering-abilities.md @@ -23,10 +23,10 @@ The `$args` array accepts the following keys: - `execute_callback` (`callable`, **Required**): The PHP function or method to execute when this ability is called. - The callback receives one optional argument: it can have any type as defined in the input schema (e.g., `array`, `object`, `string`, etc.). - The callback should return the result of the ability's operation or return a `WP_Error` object on failure. -- `permission_callback` (`callable`|`null`, **Optional**): A callback function to check if the current user has permission to execute this ability. - - Should return `true` if the user has permission, or a `WP_Error` object otherwise. - - If not provided, the ability will only validate input parameters before execution. - - This defaults to `true` if not set. +- `permission_callback` (`callable`, **Required**): A callback function to check if the current user has permission to execute this ability. + - The callback receives one optional argument: it can have any type as defined in the input schema (e.g., `array`, `object`, `string`, etc.). + - The callback should return a boolean (`true` if the user has permission, `false` otherwise), or a `WP_Error` object on failure. + - If the input does not validate against the input schema, the permission callback will not be called, and a `WP_Error` will be returned instead. - `meta` (`array`, **Optional**): An associative array for storing arbitrary additional metadata about the ability. **Ability ID Convention** diff --git a/includes/abilities-api.php b/includes/abilities-api.php index bb1ad5fe..f898ac00 100644 --- a/includes/abilities-api.php +++ b/includes/abilities-api.php @@ -31,10 +31,10 @@ * @phpstan-param array{ * label?: string, * description?: string, - * input_schema?: array, - * output_schema?: array, * execute_callback?: callable( mixed $input= ): (mixed|\WP_Error), * permission_callback?: callable( mixed $input= ): (bool|\WP_Error), + * input_schema?: array, + * output_schema?: array, * meta?: array, * ability_class?: class-string<\WP_Ability>, * ... diff --git a/includes/abilities-api/class-wp-abilities-registry.php b/includes/abilities-api/class-wp-abilities-registry.php index dd507bd3..c303ee15 100644 --- a/includes/abilities-api/class-wp-abilities-registry.php +++ b/includes/abilities-api/class-wp-abilities-registry.php @@ -46,18 +46,17 @@ final class WP_Abilities_Registry { * @param string $name The name of the ability. The name must be a string containing a namespace * prefix, i.e. `my-plugin/my-ability`. It can only contain lowercase * alphanumeric characters, dashes and the forward slash. - * @param array $args An associative array of arguments for the ability. This should include - * `label`, `description`, `input_schema`, `output_schema`, - * `execute_callback`, `permission_callback`, `meta`, and ability_class. + * @param array $args An associative array of arguments for the ability. See wp_register_ability() for + * details. * @return ?\WP_Ability The registered ability instance on success, null on failure. * * @phpstan-param array{ * label?: string, * description?: string, + * execute_callback?: callable( mixed $input= ): (mixed|\WP_Error), + * permission_callback?: callable( mixed $input= ): (bool|\WP_Error), * input_schema?: array, * output_schema?: array, - * execute_callback?: callable( mixed $input= ): (mixed|\WP_Error), - * permission_callback?: ?callable( mixed $input= ): (bool|\WP_Error), * meta?: array, * ability_class?: class-string<\WP_Ability>, * ... diff --git a/includes/abilities-api/class-wp-ability.php b/includes/abilities-api/class-wp-ability.php index a95cc84f..267c8767 100644 --- a/includes/abilities-api/class-wp-ability.php +++ b/includes/abilities-api/class-wp-ability.php @@ -73,9 +73,9 @@ class WP_Ability { * The optional ability permission callback. * * @since 0.1.0 - * @var ?callable( mixed $input= ): (bool|\WP_Error) + * @var callable( mixed $input= ): (bool|\WP_Error) */ - protected $permission_callback = null; + protected $permission_callback; /** * The optional ability metadata. @@ -143,15 +143,16 @@ public function __construct( string $name, array $args ) { * @phpstan-return array{ * label: string, * description: string, + * execute_callback: callable( mixed $input= ): (mixed|\WP_Error), + * permission_callback: callable( mixed $input= ): (bool|\WP_Error), * input_schema?: array, * output_schema?: array, - * execute_callback: callable( mixed $input= ): (mixed|\WP_Error), - * permission_callback?: ?callable( mixed $input= ): (bool|\WP_Error), * meta?: array, * ..., * } $args */ protected function prepare_properties( array $args ): array { + // Required args must be present and of the correct type. if ( empty( $args['label'] ) || ! is_string( $args['label'] ) ) { throw new \InvalidArgumentException( esc_html__( 'The ability properties must contain a `label` string.' ) @@ -164,27 +165,28 @@ protected function prepare_properties( array $args ): array { ); } - if ( isset( $args['input_schema'] ) && ! is_array( $args['input_schema'] ) ) { + if ( empty( $args['execute_callback'] ) || ! is_callable( $args['execute_callback'] ) ) { throw new \InvalidArgumentException( - esc_html__( 'The ability properties should provide a valid `input_schema` definition.' ) + esc_html__( 'The ability properties must contain a valid `execute_callback` function.' ) ); } - if ( isset( $args['output_schema'] ) && ! is_array( $args['output_schema'] ) ) { + if ( empty( $args['permission_callback'] ) || ! is_callable( $args['permission_callback'] ) ) { throw new \InvalidArgumentException( - esc_html__( 'The ability properties should provide a valid `output_schema` definition.' ) + esc_html__( 'The ability properties must provide a valid `permission_callback` function.' ) ); } - if ( empty( $args['execute_callback'] ) || ! is_callable( $args['execute_callback'] ) ) { + // Optional args only need to be of the correct type if they are present. + if ( isset( $args['input_schema'] ) && ! is_array( $args['input_schema'] ) ) { throw new \InvalidArgumentException( - esc_html__( 'The ability properties must contain a valid `execute_callback` function.' ) + esc_html__( 'The ability properties should provide a valid `input_schema` definition.' ) ); } - if ( isset( $args['permission_callback'] ) && ! is_callable( $args['permission_callback'] ) ) { + if ( isset( $args['output_schema'] ) && ! is_array( $args['output_schema'] ) ) { throw new \InvalidArgumentException( - esc_html__( 'The ability properties should provide a valid `permission_callback` function.' ) + esc_html__( 'The ability properties should provide a valid `output_schema` definition.' ) ); } @@ -278,6 +280,7 @@ protected function validate_input( $input = null ) { if ( null === $input ) { return true; } + return new \WP_Error( 'ability_missing_input_schema', sprintf( @@ -306,8 +309,8 @@ protected function validate_input( $input = null ) { /** * Checks whether the ability has the necessary permissions. - * If the permission callback is not set, the default behavior is to allow access - * when the input provided passes validation. + * + * The input is validated against the input schema before it is passed to to permission callback. * * @since 0.1.0 * @@ -320,10 +323,6 @@ public function has_permission( $input = null ) { return $is_valid; } - if ( ! is_callable( $this->permission_callback ) ) { - return true; - } - if ( empty( $this->get_input_schema() ) ) { return call_user_func( $this->permission_callback ); } diff --git a/tests/unit/abilities-api/wpAbilitiesRegistry.php b/tests/unit/abilities-api/wpAbilitiesRegistry.php index 1564a840..8e63924e 100644 --- a/tests/unit/abilities-api/wpAbilitiesRegistry.php +++ b/tests/unit/abilities-api/wpAbilitiesRegistry.php @@ -112,6 +112,7 @@ public function test_register_invalid_uppercase_characters_in_name() { * Should reject ability registration without a label. * * @covers WP_Abilities_Registry::register + * @covers WP_Ability::prepare_properties * * @expectedIncorrectUsage WP_Abilities_Registry::register */ @@ -127,6 +128,7 @@ public function test_register_invalid_missing_label() { * Should reject ability registration with invalid label type. * * @covers WP_Abilities_Registry::register + * @covers WP_Ability::prepare_properties * * @expectedIncorrectUsage WP_Abilities_Registry::register */ @@ -141,6 +143,7 @@ public function test_register_invalid_label_type() { * Should reject ability registration without a description. * * @covers WP_Abilities_Registry::register + * @covers WP_Ability::prepare_properties * * @expectedIncorrectUsage WP_Abilities_Registry::register */ @@ -156,6 +159,7 @@ public function test_register_invalid_missing_description() { * Should reject ability registration with invalid description type. * * @covers WP_Abilities_Registry::register + * @covers WP_Ability::prepare_properties * * @expectedIncorrectUsage WP_Abilities_Registry::register */ @@ -170,6 +174,7 @@ public function test_register_invalid_description_type() { * Should reject ability registration without an execute callback. * * @covers WP_Abilities_Registry::register + * @covers WP_Ability::prepare_properties * * @expectedIncorrectUsage WP_Abilities_Registry::register */ @@ -185,6 +190,7 @@ public function test_register_invalid_missing_execute_callback() { * Should reject ability registration if the execute callback is not a callable. * * @covers WP_Abilities_Registry::register + * @covers WP_Ability::prepare_properties * * @expectedIncorrectUsage WP_Abilities_Registry::register */ @@ -195,10 +201,27 @@ public function test_register_incorrect_execute_callback_type() { $this->assertNull( $result ); } + /** + * Should reject ability registration without an execute callback. + * + * @covers WP_Abilities_Registry::register + * @covers WP_Ability::prepare_properties + * + * @expectedIncorrectUsage WP_Abilities_Registry::register + */ + public function test_register_invalid_missing_permission_callback() { + // Remove the permission_callback from the args. + unset( self::$test_ability_args['permission_callback'] ); + + $result = $this->registry->register( self::$test_ability_name, self::$test_ability_args ); + $this->assertNull( $result ); + } + /** * Should reject ability registration if the permission callback is not a callable. * * @covers WP_Abilities_Registry::register + * @covers WP_Ability::prepare_properties * * @expectedIncorrectUsage WP_Abilities_Registry::register */ @@ -213,6 +236,7 @@ public function test_register_incorrect_permission_callback_type() { * Should reject ability registration if the input schema is not an array. * * @covers WP_Abilities_Registry::register + * @covers WP_Ability::prepare_properties * * @expectedIncorrectUsage WP_Abilities_Registry::register */ @@ -227,6 +251,7 @@ public function test_register_incorrect_input_schema_type() { * Should reject ability registration if the output schema is not an array. * * @covers WP_Abilities_Registry::register + * @covers WP_Ability::prepare_properties * * @expectedIncorrectUsage WP_Abilities_Registry::register */ @@ -241,6 +266,7 @@ public function test_register_incorrect_output_schema_type() { * Should reject ability registration with invalid meta type. * * @covers WP_Abilities_Registry::register + * @covers WP_Ability::prepare_properties * * @expectedIncorrectUsage WP_Abilities_Registry::register */ diff --git a/tests/unit/abilities-api/wpRegisterAbility.php b/tests/unit/abilities-api/wpRegisterAbility.php index 7bfb983a..22330b23 100644 --- a/tests/unit/abilities-api/wpRegisterAbility.php +++ b/tests/unit/abilities-api/wpRegisterAbility.php @@ -225,7 +225,6 @@ public function test_register_ability_custom_ability_class(): void { ); } - /** * Tests executing an ability with input not matching schema. */ diff --git a/tests/unit/rest-api/wpRestAbilitiesRunController.php b/tests/unit/rest-api/wpRestAbilitiesRunController.php index 74683cbf..f7ae2195 100644 --- a/tests/unit/rest-api/wpRestAbilitiesRunController.php +++ b/tests/unit/rest-api/wpRestAbilitiesRunController.php @@ -703,38 +703,6 @@ public function test_ability_without_type_defaults_to_tool(): void { $this->assertEquals( 200, $post_response->get_status() ); } - /** - * Test permission check with null permission callback. - */ - public function test_permission_check_passes_when_callback_not_set(): void { - // Register ability without permission callback. - wp_register_ability( - 'test/no-permission-callback', - array( - 'label' => 'No Permission Callback', - 'description' => 'Ability without permission callback', - 'execute_callback' => static function () { - return array( 'executed' => true ); - }, - 'meta' => array( 'type' => 'tool' ), - // No permission_callback set - ) - ); - - wp_set_current_user( 0 ); // Not logged in - - $request = new WP_REST_Request( 'POST', '/wp/v2/abilities/test/no-permission-callback/run' ); - $request->set_header( 'Content-Type', 'application/json' ); - - $response = $this->server->dispatch( $request ); - - // Should succeed when no permission callback is set - $this->assertEquals( 200, $response->get_status() ); - - // Restore user for other tests - wp_set_current_user( self::$user_id ); - } - /** * Test edge case with empty input for both GET and POST. */