Make WordPress Core

Opened 8 years ago

Last modified 7 years ago

#36940 assigned enhancement

Break `manage_sites` capability up into more targeted caps

Reported by: johnjamesjacoby's profile johnjamesjacoby Owned by: johnjamesjacoby's profile johnjamesjacoby
Milestone: Future Release Priority: normal
Severity: normal Version: 3.0
Component: Networks and Sites Keywords: needs-patch granular-capabilities
Focuses: multisite Cc:

Description

The manage_sites capability is currently used as a blanket, to cover all needs when it comes to editing a site in a multisite installation.

Started in #15800 (and having chatted with @jeremyfelt at length) we'd like to break manage_sites up into new capabilities that more acutely convey what part of a site someone is allowed to edit.

The goal is to allow users with /network admin access to have more fine-grained control over what parts of a site they can edit. For example: manage_site_settings = false so a user can modify site themes & users, but not have access to the raw option data.

We are imagining they would look something like:

  • manage_site_info (site-info.php)
  • manage_site_settings (site-settings.php)
  • manage_site_themes (site-themes.php)
  • manage_site_users (site-users.php)

In addition:

  • We would pass the site ID through current_user_can() to provide additional context
  • Switch to using create_sites in site-new.php (vs. manage_sites which was likely a copy-paste assumption that the capabilities across these similar files should match)

More paraphrasing of our past 4 months of chat in #core-multisite:

  • Because only WordPress Super Admins can access any of these by default, renaming these capabilities should be considered a backwards compatible change
  • We /could/ go as far as mapping all of these new caps to manage_sites to maintain compatibility, but for plugin authors wishing to take advantage of this, it would require an additional map_meta_cap override that we would like to try and avoid
  • This should not result in much code churn, and will only change multisite files, and a handful of functions that special-case multisite using the manage_sites capability check
  • We would still keep manage_sites in a few higher-level places (as a site equivalent to edit_posts) to ensure that a user has access to certain UI elements that allow them entry to more detailed site editing

Change History (10)

This ticket was mentioned in Slack in #core-multisite by richardtape. View the logs.


8 years ago

#2 follow-up: @jeremyfelt
8 years ago

In general, +1, though a few thoughts after discussion during bug scrub today:

  • Are there any downsides to using meta caps instead? It seems like the right scenario for it.
  • If we introduce these, are there other places in core where they may apply?
  • Is manage_site_ the right name when thinking about future compatibility for other caps at the site level?
  • A plugin can already alter the caps required for individual tabs.

#3 in reply to: ↑ 2 @johnjamesjacoby
8 years ago

Replying to jeremyfelt:

Are there any downsides to using meta caps instead? It seems like the right scenario for it.

We still need to update the individual files to check for the newly mapped caps.

The reason to use map_meta_cap is to make assumptions that one capability conditionally maps to another capability. In this case, can manage_site_settings map to manage_sites? Yes. But if we need to rely on map_meta_cap internally for this, overriding that behavior is more difficult than a user not having the cap, because no users ever have these caps directly, which means allowing or denying from the global $wp_roles array will no longer work. Instead, one would need to also hook into map_meta_cap to override the previous map_meta_cap conclusion, and that is not terribly straight forward.

What would we win? Is it worth the cost of complexity since there is no use-case inside of WordPress proper where a user would have manage_sites and not any of the others?

If we introduce these, are there other places in core where they may apply?

site-new.php is the only other direct change that I found. It uses create_sites in one place, and manage_sites in another, so you can't actually create a new site unless you can also manage_sites which, I guess maybe makes sense, but one cap shouldn't be a prerequisite for another (that's what roles are for.)

Any other places where manage_sites is used, is as a general assumption that the current user can manage "all" sites, though not necessarily a specific one.

Is manage_site_ the right name when thinking about future compatibility for other caps at the site level?

Probably... The manage_ prefix is used in many places, and I think _site_ is a good next-level namespace to identify that it's a network-area site-specific capability check. edit_ vs. manage_ might work too, but is the ability to manage something is different than the ability to edit something?

If we wanted parity with Posts, for example, then we'd opt for edit_site and then toss that into map_meta_cap() or some other new multisite specific equivalent. Right now, we don't map any network area caps that I can think of, so we're starting to rabbit hole ourselves. :)

A plugin can already alter the caps required for individual tabs.

A plugin can only alter the caps that are checked to make the tabs appear. The URL for the files each tab links to can still be accessed by any user that has the manage_sites capability and hits the URL directly.

---

In summary:

  • Each file needs its own unique capability check at the top.
  • If we decide to use map_meta_cap to check the $pagenow instead, that will work, but be less efficient and require more hacks to work-around. And we'd likely want to use the Post object and it's single-caps as a guide.
  • map_meta_cap filtering seems unnecessary in core, since no Super Admin would ever not have any of the caps being mapped to manage_sites.

This ticket was mentioned in Slack in #core-multisite by jeremyfelt. View the logs.


8 years ago

#5 @jeremyfelt
8 years ago

  • Keywords needs-patch added; has-patch 2nd-opinion removed
  • Owner set to johnjamesjacoby
  • Status changed from new to assigned

We had a good discussion covering this in Slack today.

To paraphrase:

  • manage_site_* is a good name. manage_ matches what we do with manage_sites already. _site_ will never be used in the context of a single site as we'll use the object directly. (e.g. manage_options).
  • manage_site_* can be used to decide whether or not to show the "tabs". These capabilities are filterable by plugins.
  • At the top of each site, we'll need to check manage_sites and manage_site_* to retain max-compat with any plugins currently denying access to these pages.

#6 @ocean90
8 years ago

Because only WordPress Super Admins can access any of these by default, renaming these capabilities should be considered a backwards compatible change

That statement is wrong. You can create a role which only has the read, manage_network, and manage_sites caps and assign it to a user. The user will have access to wp-admin/network/index.php, wp-admin/network/upgrade.php and wp-admin/network/site*.php.

#7 @jeremyfelt
8 years ago

  • Milestone changed from 4.6 to Future Release
  • Type changed from defect (bug) to enhancement

This should be considered an enhancement as it's introducing new capabilities. We can work to get it in 4.7 early once a patch is ready.

#8 @flixos90
8 years ago

A similar ticket, however with a slightly different approach, was (accidentally) opened in #39156. One of these might probably be closed.

This ticket was mentioned in Slack in #core-multisite by flixos90. View the logs.


8 years ago

#10 @johnbillion
7 years ago

  • Keywords granular-capabilities added
Note: See TracTickets for help on using tickets.