Skip to content
This repository was archived by the owner on Sep 24, 2018. It is now read-only.

Introduce rest_authorization_required_code() #1808

Merged
rmccue merged 2 commits into from
Dec 9, 2015
Merged

Introduce rest_authorization_required_code() #1808

rmccue merged 2 commits into from
Dec 9, 2015

Conversation

@danielbachhuber
Copy link
Member

@danielbachhuber danielbachhuber commented Dec 8, 2015

This function produces a contextually-specific HTTP error code based on
whether the user is logged in. It should be used when producing an error
from a failed current_user_can() check, because it's helpful to the
client to be able to distinguish between 401 and 403

Fixes #757
Fixes #1665

@danielbachhuber
Introduce rest_authorization_required_code()
7ba0ae6 
This function produces a contextually-specific HTTP error code based on
whether the user is logged in. It should be used when producing an error
from a failed `current_user_can()` check, because it's helpful to the
client to be able to distinguish between 401 and 403
@danielbachhuber danielbachhuber added this to the 2.0 Beta 9 milestone Dec 8, 2015
@danielbachhuber
Copy link
Member Author

danielbachhuber commented Dec 8, 2015

For the historical artifact, here's all of the places our tests currently fail:

There were 15 failures:

1) WP_Test_REST_Attachments_Controller::test_create_item_invalid_edit_permissions
Failed asserting that 403 matches expected 401.

/srv/www/wordpress-develop.dev/tests/phpunit/includes/testcase-rest-api.php:16
/srv/www/wordpress-develop.dev/src/wp-content/plugins/wp-api/tests/test-rest-attachments-controller.php:230

2) WP_Test_REST_Comments_Controller::test_get_items_no_permission
Failed asserting that 401 matches expected 403.

/srv/www/wordpress-develop.dev/tests/phpunit/includes/testcase-rest-api.php:16
/srv/www/wordpress-develop.dev/src/wp-content/plugins/wp-api/tests/test-rest-comments-controller.php:81

3) WP_Test_REST_Comments_Controller::test_get_comment_invalid_context
Failed asserting that 401 matches expected 403.

/srv/www/wordpress-develop.dev/tests/phpunit/includes/testcase-rest-api.php:16
/srv/www/wordpress-develop.dev/src/wp-content/plugins/wp-api/tests/test-rest-comments-controller.php:273

4) WP_Test_REST_Comments_Controller::test_get_comment_not_approved
Failed asserting that 401 matches expected 403.

/srv/www/wordpress-develop.dev/tests/phpunit/includes/testcase-rest-api.php:16
/srv/www/wordpress-develop.dev/src/wp-content/plugins/wp-api/tests/test-rest-comments-controller.php:293

5) WP_Test_REST_Comments_Controller::test_update_comment_invalid_permission
Failed asserting that 401 matches expected 403.

/srv/www/wordpress-develop.dev/tests/phpunit/includes/testcase-rest-api.php:16
/srv/www/wordpress-develop.dev/src/wp-content/plugins/wp-api/tests/test-rest-comments-controller.php:776

6) WP_Test_REST_Meta_Posts_Controller::test_get_item_unauthenticated
Failed asserting that 401 matches expected 403.

/srv/www/wordpress-develop.dev/tests/phpunit/includes/testcase-rest-api.php:16
/srv/www/wordpress-develop.dev/src/wp-content/plugins/wp-api/tests/test-rest-meta-posts-controller.php:199

7) WP_Test_REST_Meta_Posts_Controller::test_create_item_unauthenticated
Failed asserting that 401 matches expected 403.

/srv/www/wordpress-develop.dev/tests/phpunit/includes/testcase-rest-api.php:16
/srv/www/wordpress-develop.dev/src/wp-content/plugins/wp-api/tests/test-rest-meta-posts-controller.php:374

8) WP_Test_REST_Meta_Posts_Controller::test_update_meta_unauthenticated
Failed asserting that 401 matches expected 403.

/srv/www/wordpress-develop.dev/tests/phpunit/includes/testcase-rest-api.php:16
/srv/www/wordpress-develop.dev/src/wp-content/plugins/wp-api/tests/test-rest-meta-posts-controller.php:677

9) WP_Test_REST_Meta_Posts_Controller::test_delete_item_unauthenticated
Failed asserting that 401 matches expected 403.

/srv/www/wordpress-develop.dev/tests/phpunit/includes/testcase-rest-api.php:16
/srv/www/wordpress-develop.dev/src/wp-content/plugins/wp-api/tests/test-rest-meta-posts-controller.php:960

10) WP_Test_REST_Post_Statuses_Controller::test_get_item_invalid_access
Failed asserting that 401 matches expected 403.

/srv/www/wordpress-develop.dev/tests/phpunit/includes/testcase-rest-api.php:16
/srv/www/wordpress-develop.dev/src/wp-content/plugins/wp-api/tests/test-rest-post-statuses-controller.php:56

11) WP_Test_REST_Posts_Controller::test_get_post_list_context_without_permission
Failed asserting that 401 matches expected 403.

/srv/www/wordpress-develop.dev/tests/phpunit/includes/testcase-rest-api.php:16
/srv/www/wordpress-develop.dev/src/wp-content/plugins/wp-api/tests/test-rest-posts-controller.php:316

12) WP_Test_REST_Posts_Controller::test_get_post_context_without_permission
Failed asserting that 401 matches expected 403.

/srv/www/wordpress-develop.dev/tests/phpunit/includes/testcase-rest-api.php:16
/srv/www/wordpress-develop.dev/src/wp-content/plugins/wp-api/tests/test-rest-posts-controller.php:326

13) WP_Test_REST_Revisions_Controller::test_get_items_no_permission
Failed asserting that 401 matches expected 403.

/srv/www/wordpress-develop.dev/tests/phpunit/includes/testcase-rest-api.php:16
/srv/www/wordpress-develop.dev/src/wp-content/plugins/wp-api/tests/test-rest-revisions-controller.php:61

14) WP_Test_REST_Revisions_Controller::test_get_item_no_permission
Failed asserting that 401 matches expected 403.

/srv/www/wordpress-develop.dev/tests/phpunit/includes/testcase-rest-api.php:16
/srv/www/wordpress-develop.dev/src/wp-content/plugins/wp-api/tests/test-rest-revisions-controller.php:94

15) WP_Test_REST_Users_Controller::test_get_item_published_author_wrong_context
Failed asserting that 401 matches expected 403.

/srv/www/wordpress-develop.dev/tests/phpunit/includes/testcase-rest-api.php:16
/srv/www/wordpress-develop.dev/src/wp-content/plugins/wp-api/tests/test-rest-users-controller.php:301

@danielbachhuber
Copy link
Member Author

danielbachhuber commented Dec 8, 2015

@WP-API/amigos #reviewmerge

@rmccue
Copy link
Member

rmccue commented Dec 9, 2015

I've been thinking about adding either constants for a function to help for a while in a more generic way too, similar to how Symfony does it. Thoughts @WP-API/amigos?

rmccue added a commit that referenced this pull request Dec 9, 2015
@rmccue
Merge pull request #1808 from WP-API/757-authorization-error
28b77bd 
Introduce `rest_authorization_required_code()`
@rmccue rmccue merged commit 28b77bd into develop Dec 9, 2015
@rmccue rmccue deleted the 757-authorization-error branch December 9, 2015 04:55
@danielbachhuber
Copy link
Member Author

danielbachhuber commented Dec 9, 2015

Could. I don't have a strong opinion either way. While I like that HTTP codes are a clear language, using constants instead could help prevent typos from causing egregious bugs.

@dave-green-uk dave-green-uk mentioned this pull request Jun 26, 2016
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

Review

Projects

None yet

Milestone

2.0 Beta 9

Development

Successfully merging this pull request may close these issues.

3 participants

@danielbachhuber @rmccue