search

WP-API / authentication

The home for design & development of a core WordPress REST API authentication solution
GNU General Public License v2.0
62 stars 2 forks source link

Scopes and Permissions #8

Open dshanske opened 4 years ago

dshanske commented 4 years ago

The way WordPress is designed, permissions are mapped to users and their roles. OAuth maps permissions to scopes.

Unless we want to give applications the same permissions as the users they are associated with, we need to map out a system to grant limited permissions to tokens, as well as all the administrative functionality around that.

aerych commented 4 years ago

Would it be reasonable for an initial implementation to assume the same permissions as the user and allow for more granular scoping in a later iteration?

Just thinking about the use case for the WordPress and WooCommerce mobile apps where its largely assumed the app can do whatever the user can do. We're keen to switch to the core REST API and move away from XML-RPC. I'd anticipate it would take much longer to implement the limited permissions tied to tokens as described above.

aaronpk commented 4 years ago

OAuth scopes are meant to limit what an access token can do within a user's existing permissions. OAuth scopes are definitely not a good way to build out the actual internal permissions system of the API.

A shortcut implementation is to completely ignore OAuth scopes, granting every access token the full permissions of what a user can do.

dshanske commented 4 years ago

@aerych I added this issue because I think we shouldn't take that shortcut if we can help it. It will be more painful later to change, especially with WordPress having a backward compatible philosophy. I would rather we build something that can be expanded on than start with allowing everything.

dshanske commented 4 years ago

@aaronpk We have a capabilities system, we just map capabilities to roles, and roles to users. It is just conceptualizing how to connect the pieces

dshanske commented 4 years ago

So, the basic proposal is this...scopes are essentially designed similar to roles, a role being a collection of capabilities. Scope would be a collection of capabilities, but adding the checks that they can't exceed the capabilities of the role of the user that is associated with the token.

dshanske commented 4 years ago

Then ensuring that when a token is used for authentication, that the current_user_can function returns the capabilities of the token not the user.

jkmassel commented 4 years ago

One thing to consider – what happens if a user's role changes?

In a situation in which a user authenticated as an editor and in the future sends a request as an administrator, would we continue to use the capabilities of the token instead of the user? That would be odd from a user's point of view, because I don't think it would be obvious that they need to reauthenticate in order to take advantage of their new capabilities.

In the reverse situation, in which a user authenticated as an editor and was later changed to a subscriber(perhaps by an administrator), it'd be important not to allow them the capabilities associated with their token.

One other consideration – currently, if a WordPress user changes the role in the UI for another user, it's expected that the changes will take effect immediately – a token overriding this breaks the mental model for user access, which seems like it'd be considered backward-incompatible.

One solution to resolve this might be to have current_user_can only return true for the common capabilities between the token and the user (probably using an early filter on user_has_cap to retrieve and calculate the common set of capabilities). This could potentially have implications for plugins (I'm thinking about pre-existing permissions management in particular, or anything scheduled by wp_cron, which would inherit a different set of permissions when run), but I don't know enough about these to be sure – just that we should be sure to consider them! :-)

dshanske commented 4 years ago

The scope should be a filter on the current user capabilities...so if the user loses them, the token would too. I implemented a version of this where it does an array comparison between the token capabilities and the user capabilities and what is the same on both is what is allowed...the alternative is invalidating tokens on user role change.

dshanske commented 4 years ago

map_meta_cap might work better than the user_has_cap filter.

TimothyBJacobs commented 4 years ago

Yeah, the scopes would only be able to limit existing capabilities. Probably using user_has_cap.

Cron shouldn't be an issue since the actual cron processing would be evaluated in a different request.

I think any general background tasks that might get triggered wouldn't be reliant on the current user's permissions. But if there was a cap check on a user that happened to be the current user, that could be an issue.

It probably wouldn't be a great idea for people to evaluate their background tasks during a REST API request anyways, since performance is obviously key.