cedaro / satispress

Expose installed WordPress plugins and themes as Composer packages.
500 stars 48 forks source link

"rest_cannot_read" when user tries to look at / add API keys under their profile #157

Open mihkeleidast opened 3 years ago

mihkeleidast commented 3 years ago

Our users have the "subscriber" role with the following capabilities added via the Members plugin: image

If a user with that role logs in to WordPress and navigates to their profile to look at / add API keys, nothing happens and the following errors are logged in the browser console. image image

From what I gather from the code, this method will return an error if the user can't manage options and it will never reach the edit_user check: https://github.com/cedaro/satispress/blob/develop/src/REST/ApiKeysController.php#L143-L160

bradyvercher commented 3 years ago

Permissions are primarily focused on the packages.json endpoint. Everything visible in the admin panel is geared toward site admins/users with the manage_options permission, so I haven't really thought through multi-user scenarios in the admin panel.

I can see how it'd be beneficial for users without manage_options to view their own API keys, but I'm not sure they should be able to create API keys unless a new custom permission was defined. In either case, that API key table probably needs to be removed or updated to account for users that can't create/revoke API keys.

mihkeleidast commented 3 years ago

This worked pre-1.0.0. We ran into issues after updating. We've rolled back to 0.7.2 for now, until this is solved.

bradyvercher commented 3 years ago

I thought I remembered it working that way at one point. The old interface was a mish-mash of JS templates, Backbone, and PHP code with data passed all sorts of ways. In some ways it was easier to manage what was visible. Let me see if there's a way to do capability checks in JS.

bradyvercher commented 3 years ago

I guess I'd forgotten it worked that way when I reworked the UI and thought only users with manage_options should be able to create and revoke API keys. Removing those manage_options capability checks in the REST controller would restore that behavior.

I think adding a new capability for managing API keys would be a good idea, but that can be implemented in a future release.