#17-fix-permissions. #18
Conversation
Fixes WP-API#17. Currently the permissions do not match what core uses to restric plugin access. This does not account for multisite, which will be added in a separate commit on a separate PR. Tests are updated to reflect unauthenticated and authenticated users without permissions.
Fixing the permissions to support multisite. I was going to do this in a separate PR but I forgot that the CI runs both tests at once. This handles priviledges for site admins as well when plugin management is activated in the settings panel.
|
I forgot our CI runs both multisite and single site tests in one go. The second commit adds in multisite permissions checks for plugins; matching WordPress core. |
| if ( ! current_user_can( 'manage_options' ) ) { // TODO: Something related to plugins. activate_plugin capability seems to not be available for multi-site superadmin (?) | ||
| return new WP_Error( 'rest_forbidden', __( 'Sorry, you cannot view the list of plugins' ), array( 'status' => rest_authorization_required_code() ) ); | ||
| if ( is_multisite() ) { | ||
| $menu_permissions = get_site_option( 'menu_items' ); |
There was a problem hiding this comment.
Hmm I'm not sure if I understand the rational behind using this option value, maybe I'm misunderstanding but it looks like you're looking at what is present in the menu to decide what they have permissions to?
There was a problem hiding this comment.
On a multisite install, regular site admins (non super admins), can view and manage plugins specific to their site, when this option is enabled. I wanted to reflect that core capability in the permissions check here. I will work on making the conditionals more clear. If it is not apparent then it is bad code. I will try to make the intent more clear. Should I maybe add in a doc block explaining all of that or should we not support this?
There was a problem hiding this comment.
Ahh I see, menu_items is a site level option, one of which is whether admins are allowed to manage plugins on their site? In that case, let's just add some inline docs explaining what that is. I think it's a good thing to support. Also, if it's possible to break the conditionals down more to be more readable, I'd be +1 on that
This is the proper permissions check for multisite and single site installs for WordPress.
|
This is good to merge the failing tests are unrelated. This fix is in here #23. |
Fixes #17. Currently the permissions do not match what core uses to
restric plugin access. This does not account for multisite, which will
be added in a separate commit on a separate PR. Tests are updated to
reflect unauthenticated and authenticated users without permissions.