Make WordPress Core

Opened 6 years ago

Closed 5 years ago

#45423 closed defect (bug) (wontfix)

Use custom capabilities for the block editor's reusable blocks.

Reported by: peterwilsoncc's profile peterwilsoncc Owned by:
Milestone: Priority: normal
Severity: normal Version: 5.0
Component: Role/Capability Keywords:
Focuses: Cc:

Description

The new wp_block post type for reusable blocks is currently mapped to the post post types capabilities, including some work-arounds in map_meta_cap() for the purpose.

To allow for custom roles limiting users to editing/reading certain post types (for example a "Page Editor" or "Photo manager") these ought to have their own capabilities, consistent with other built in post types.

This will also allow web site owners to customize existing roles; for example to allow authors to edit another user's reusable blocks or for contributors to create them.

Follow up from #45098.

Attachments (2)

45423.diff (4.6 KB) - added by peterwilsoncc 6 years ago.
45423.2.diff (23.8 KB) - added by peterwilsoncc 6 years ago.

Download all attachments as: .zip

Change History (24)

@peterwilsoncc
6 years ago

#1 @matveb
6 years ago

  • Milestone changed from Awaiting Review to 5.0.1

#2 @pento
6 years ago

  • Milestone changed from 5.0.1 to 5.0.2

#3 @pento
6 years ago

  • Milestone changed from 5.0.2 to 5.1

This one is going to need some more thinking time. Moving it to 5.1, but we can move it back to a 5.0.x later if there's time.

#4 @peterwilsoncc
6 years ago

@pento Is there anything in particular you need me to take a look at to help move this forward?

#5 @desrosj
6 years ago

Related bug: #45373.

#6 @peterwilsoncc
6 years ago

  • Milestone changed from 5.1 to Future Release

There are a few things I see needed for this to go forward:

  • Hiding the appropriate UI element for users failing the create_post or edit_post meta caps
  • Removing the option to insert shared blocks for users blocked from reading them (in the dropdown, via a slash command and anywhere else I have forgotten)
  • Removing the menu should be handled by core already
  • Decisions about which core roles should be able to publish a shared post if that needs to be different to the rules for posts and pages
  • What happens for users who can create posts but not publish them when they attempt to create a post from within the authoring UI. IE, is the content duplicated, an unpublished post inserted into their current post or 🤷🏻‍♂️

It's the eleventh hour for the 5.1 release. As there's a fair amount to consider, I'm moving this to future release.

This ticket was mentioned in Slack in #core-editor by desrosj. View the logs.


6 years ago

#8 @peterwilsoncc
6 years ago

  • Keywords has-unit-tests added
  • Milestone changed from Future Release to 5.1
  • Owner set to pento
  • Status changed from new to assigned

In 45423.2.diff

  • replaces the primitive caps used by blocks with *_blocks
  • The read and create_posts post type caps use an atypical mapping, the caps are not needed for the roles
  • updates the block permissions tests to use edit_post, create_post and delete_post meta caps
  • adds the new caps to roles in an update routine
  • adds the new caps to the existing unit tests
  • the UI for authors and contributors remains visible on the add page/post screens, however this isn't a regression.

This fixes the white screen issues identified in #45373.

#9 @pento
6 years ago

Given that this approach has been working in the Gutenberg plugin, I'm inclined to run with it.

One important thing for the dev note, sites that delay running the upgrade routines (eg, multisites) must do it before upgrading, or the block editor will throw errors when you try to use or create reusable blocks.

#10 @flixos90
6 years ago

  • Keywords needs-patch needs-unit-tests added; has-patch has-unit-tests removed
  • Milestone changed from 5.1 to 5.2

I recommend against moving forward with this approach. We haven't introduced new primitive capabilities since WordPress 3.0, and that is for a reason. Migrating databases like that should not be required and can easily cause issues, specifically with larger multisites. There have been numerous capabilities introduced since 3.0, but these have always used either the map_meta_cap or user_has_cap filter, granting them dynamically.

I think we should introduce these capabilities as dynamic capabilities using the user_has_cap filter, making them what I have so far called "fake-primitive" capabilities (only for the lack of a better word). See #44468 for more information on that.

Since this ticket has been moved from 5.1 to "Future Release" before, it seems to not be super-critical for 5.1. Let's tackle #44468 and this one as an enhancement for 5.2, to have sufficient time for a proper implementation. I'll move the two tickets to that milestone now, and I'm happy to work on it. For the unit tests, this ticket is blocked by #44468.

If I was mistaken and this ticket is more critical, I suggest moving both of them to 5.1.x.

#11 @johnbillion
6 years ago

I concur with Felix. Introducing new primitive capabilities is impossible due to the use of custom roles that also potentially need to receive the new caps. Beyond the default roles in WordPress, a custom role could be given the edit_posts capability but would never receive the edit_blocks capability with this upgrade routine.

These caps should be granted dynamically via the map_meta_cap or user_has_cap filter.

#12 @pento
6 years ago

  • Owner pento deleted

@flixos90, @johnbillion: That's basically what Core is already doing, except it's happening slightly before the user_has_cap filter, in map_meta_cap().

The issue isn't in granting the capabilities, the issue appears to be in how custom post types interact with these "fake primitive" capabilities, particularly when trying to map them to other fake primitives: they don't seem to map correctly.

I'll commit the fix in #45373 for now, and we can see what feedback we get during beta, but we may need to revisit this ticket before 5.1. I don't really want to add caps to the database (hence why we didn't in 5.0), so I'd appreciate y'all making some time to dig into this further.

This ticket was mentioned in Slack in #core by audrasjb. View the logs.


6 years ago

#14 @audrasjb
6 years ago

  • Milestone changed from 5.2 to 5.3

As per today's bug scrub, we are going to move this one to the next Release since beta 3 and RC are approaching.

This ticket was mentioned in Slack in #core by david.baumwald. View the logs.


5 years ago

#16 follow-up: @davidbaumwald
5 years ago

  • Keywords early added
  • Milestone changed from 5.3 to 5.4

This was discussed during today's bug scrub for 5.3, and it was decided that this should move to 5.4 early in the cycle. If someone can shepherd this quickly, feel free to move back up accordingly.

#17 in reply to: ↑ 16 @peterwilsoncc
5 years ago

Replying to davidbaumwald:

This was discussed during today's bug scrub for 5.3, and it was decided that this should move to 5.4 early in the cycle. If someone can shepherd this quickly, feel free to move back up accordingly.

100% agree this is an early ticket.

After 12 months I think the window for moving the faux primitives out of map_meta_cap has been missed, sites with custom roles may have introduced their own filters to fix the issues above.

That said, I'm not sure the block capabilities are working 100% as intended so I will consider a way forward with regard to that.

This ticket was mentioned in Slack in #core-editor by skorasaurus. View the logs.


5 years ago

#19 @skorasaurus
5 years ago

I'm running into this right now as we have users who we wish to access reusable blocks on custom post types. We want to simplify the editing experience for our internal staff by only showing them the content that they work on; (some custom post types) and not all content of the post type 'posts' which is the current behavior as mentioned in this ticket.

While developing, I had mistakenly assumed that the read_blocks permission would give them permission to access the blocks (both core and reusable)
(and this wasn't documented anywhere, to my knowledge, until I delved into the code and then found this ticket).

As a result, I am guessing as a work-around users/agencies creating their own plugins with their own blocks instead of using reusable blocks and I may end up taking the same approach.

Last edited 5 years ago by skorasaurus (previous) (diff)

#20 @audrasjb
5 years ago

  • Keywords needs-dev-note added

This ticket was mentioned in Slack in #core by david.baumwald. View the logs.


5 years ago

#22 @peterwilsoncc
5 years ago

  • Keywords needs-patch needs-unit-tests early needs-dev-note removed
  • Milestone 5.4 deleted
  • Resolution set to wontfix
  • Status changed from assigned to closed

As discussed in a recent bug scrub, closing this of due to the length of time it's been in Core. Changing it now would be a bit of a backward compatibility nightmare.

Theme and plugin developers can work around this through the use of filters and changing the behaviour in Core could break that for for anyone who has done so.

Note: See TracTickets for help on using tickets.