Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure permisson_callback is present or warned in REST-API endpoints #75

Closed
mattyrob opened this issue May 8, 2021 · 4 comments · Fixed by ClassicPress/ClassicPress#732

Comments

@mattyrob
Copy link
Contributor

mattyrob commented May 8, 2021

Expected behavior

REST-API endpoints should enforce a permission_callback even for public endpoints. By enforcing this and reporting an error via _doing_it_wrong when the callback is missing the security of the REST-API is improved.

All endpoints must be registered with such a callback and the omission of the callback with create an error.

Current behavior

Currently, a permission_callback is not required and as such this can result in some endpoints unintentionally being made public. The current behaviour also does not make is an easy task when reading code to ensure that such a callback is present as a simple spelling error can be missed for example permissions_callback (not the plurality of permissons)

Possible solution

There is a backport option as this has been fixed upstream.

Steps to reproduce (for bugs)

  1. Register a REST-API Endpoint without a permission_callback
  2. Note who it is publicly available

Context

This has been seen upstream according to the original ticket and several CP contributors have discussed this in slack

@bahiirwa
Copy link
Contributor

bahiirwa commented May 8, 2021

The fix is in

https://core.trac.wordpress.org/changeset/48526
https://core.trac.wordpress.org/changeset/48571,

will you take on this?

@mattyrob
Copy link
Contributor Author

mattyrob commented May 8, 2021

@bahiirwa - yep, working on a PR now.

@bahiirwa
Copy link
Contributor

@mattyrob @striebwj Punting this to 1.4.0 considering the 1.3.0 roadmap right now.

@ClassyBot
Copy link
Collaborator

This issue has been mentioned on ClassicPress Forums. There might be relevant details there:

https://forums.classicpress.net/t/work-towards-release-1-4-0/3305/1

@viktorix viktorix transferred this issue from ClassicPress/ClassicPress Nov 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants