permission_callback should be called before validate_callback in REST API

[donjajo]

I am building a REST endpoint that goes like this users/articles/{article_id} while {article_id} can be validated with validate_callback on the $args parameter level.

In my validation, I check for author of the article to match the current logged in user. If not, it should return WP_Error of 404 status, assuming the user is logged in.

I expect 403 permission error to come up for guest, from permission_callback, but it does not. Rather, it throws 404 from validate_callback since the user is not logged in, and the ID does not match the author. Hence, permission_callback is not run.
Most of the validate_callback on the endpoint does some user authentication validations which expects permission_callback to already have handled.

I believe a better design will be permission_callback called first on each route before even running validations.

I am building a REST endpoint that goes like this users/articles/{article_id} while {article_id} can be validated with validate_callback on the $args parameter level.

In my validation, I check for the author of the article to match the currently logged-in user. If not, it should return a WP_Error of 404 status, assuming the user is logged in.

I expected a 403 permission error to come up for the guest from permission_callback, but it did not. Rather, it throws a 404 from validate_callback since the user is not logged in, and the ID does not match the author. Hence, permission_callback is not run.

Most of the validate_callback on the endpoint does some user authentication validations which expect permission_callback to have already been handled, but it is not

A better design will be permission_callback called first on each route before even running validations.

[dd32]

I believe what you're seeing is correct.

validate_callback shouldn't be used to validate whether it's correct for a user, rather, it should validate that the value presented is correct for the field.

For example:

validate_callback: Validate that the ID input is numeric. This should only fail for always invalid inputs.
ie. is_numeric( $input )

permission_callback: Verify that the user is logged in, and has access to the referenced item
ie. current_user_can( 'edit_post', $post_id )

callback: Fetch data, output
ie. return get_post( $post_id )->post_name;

It would be best practice to also include capability checks in the callback itself, rather than purely relying upon the permission_callback, as capability checks should be relatively cheap computation wise.

[donjajo]

Replying to dd32:

I believe what you're seeing is correct.

validate_callback shouldn't be used to validate whether it's correct for a user, rather, it should validate that the value presented is correct for the field.

For example:

validate_callback: Validate that the ID input is numeric. This should only fail for always invalid inputs.
ie. is_numeric( $input )

permission_callback: Verify that the user is logged in, and has access to the referenced item
ie. current_user_can( 'edit_post', $post_id )

callback: Fetch data, output
ie. return get_post( $post_id )->post_name;

It would be best practice to also include capability checks in the callback itself, rather than purely relying upon the permission_callback, as capability checks should be relatively cheap computation wise.

Your explanation is valid. What I am concerned about is the order of execution. I think permission check should come first before the validation check.

For example:

• permission_callback: Checks if user is logged in and is in "writers role"
• validation_callback: Checks if the current logged-in user has access to their URL resource which specifies the ID. This is also a valid use-case of validation_callback, because, if the user does not, it is an invalid value. (e.g. trying to access a post that you did not create users/articles/{article_id})

We require permissions to pass before validation.

[dd32]

I disagree personally, as in your example the validation callback is doing a permission check rather than validation.

The intention is that only valid inputs then proceed to check if the user has access (ie. Can't determine if the user can edit the post, if no valid post ID is provided), and only if the validation and permission checks pass does it pass to the callback.

While I understand your opinion of the order of operations, as you've noted, that's not something that can be changed due to back-compat and existing plugins. So even if the current behaviour was wrong (which I don't believe it is) it couldn't be changed.

If you don't agree with the behaviour of the callbacks, you can skip using validation_callback and/or permission_callback and instead perform all validation and permission checking within the callback.