Adjust Layout Builder permission checking for inline blocks once more granular block permissions exist

Created on 18 March 2019, about 5 years ago
Updated 22 February 2024, 3 months ago

Problem/Motivation

We currently have granular inline block permissions in Drupal, allowing to precisely set which custom block bundles can be created / edited by which user role. Unfortunately, restricting access to certain custom block types doesn't work in Layout Builder that allows to add / edit all or nothing within a layout by design.

Proposed resolution

  1. Add a new permission create and edit accessible custom blocks.
  2. Restrict custom blocks list in LB to only those accessible ones for users having only that permission and not create and edit custom blocks
  3. Alter LB block add, edit and delete route access basing on custom block permissions for the current user and the old / new permission.
  4. (Of course) Add automated test coverage.
  5. Create a follow-up to remove the old permission and make LB always respect block access logic or at least mark the old permission with restrict access: true

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

📌 Task
Status

Needs work

Version

11.0 🔥

Component
Layout builder 

Last updated about 12 hours ago

Created by

🇺🇸United States tim.plunkett Philadelphia

Live updates comments and jobs are added and updated live.
  • Blocks-Layouts

    Blocks and Layouts Initiative. See the #2811175 Add layouts to Drupal issue.

Sign in to follow issues

Merge Requests

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • 🇺🇸United States benjifisher Boston area

    I think it would be disruptive to change things so that the current LB permission is no longer enough to create and edit inline blocks (of any type). If we were going to do that, we should have done it when the new block permissions were added, in 10.1.0. Now I think the right thing to do is to add a new permission that needs one of the more granular block permissions to be useful.

    Currently, in layout_builder.permissions.yml:

    create and edit custom blocks:
      title: 'Create and edit content blocks'
      description: 'Manage the single-use blocks within the Layout Builder'
    

    I think we can tweak the description: change "the" to "all".

    The new permission would be something like

    create and edit certain custom blocks:
      title: 'Create and edit certain content blocks'
      description: 'Use Layout Builder to manage single-use blocks that the user can add or edit'
    

    In the description, I am trying to be consistent with the labels of some existing LB permissions, like "Content - Article: Configure layout overrides for content items that the user can edit".

  • Status changed to Needs review 7 months ago
  • 🇮🇳India _utsavsharma

    Tried to address the #10.

  • last update 7 months ago
    Build Successful
  • Status changed to Needs work 7 months ago
  • 🇺🇸United States benjifisher Boston area

    @_utsavsharma :

    Thanks for starting this issue.

    We should keep the old permission and add a new one. The patch in #11 replaces the old permission with a new one.

    Once we have the permission, we will have to update the access controls so that the permission does what it says. That will be the hard part of this issue.

    We also need to expand the issue summary. So far, it is just a stub.

  • 🇵🇱Poland Graber

    No point having 2 permissions ultimately since the old one will allow bypassing access of granular block permissions. I was thinking maybe a follow-up with an update hook in the next major to leave only 1 permission so site owners will not end up not having access unexpectedly until they update to that next major with a CR of course?

  • Assigned to Graber
  • 🇵🇱Poland Graber

    Also.. I'll work on it.

  • Pipeline finished with Failed
    5 months ago
    #75685
  • 🇵🇱Poland Graber

    Added some initial work on this. Definitely needs more work.

  • Pipeline finished with Canceled
    5 months ago
    #76186
  • Pipeline finished with Failed
    5 months ago
    #76188
  • Pipeline finished with Success
    5 months ago
    #76193
  • Pipeline finished with Failed
    5 months ago
    #76281
  • Pipeline finished with Failed
    5 months ago
    #76291
  • Pipeline finished with Failed
    5 months ago
    #76297
  • Pipeline finished with Failed
    4 months ago
    #79854
  • Pipeline finished with Failed
    4 months ago
    #80068
  • Pipeline finished with Failed
    4 months ago
    #80072
  • Pipeline finished with Failed
    4 months ago
    #80074
  • Pipeline finished with Failed
    4 months ago
    #80078
  • Pipeline finished with Failed
    4 months ago
    #80085
  • Pipeline finished with Failed
    4 months ago
    #80663
  • Pipeline finished with Failed
    4 months ago
    #80812
  • Pipeline finished with Failed
    4 months ago
    #80814
  • Pipeline finished with Success
    4 months ago
    #80847
  • Issue was unassigned.
  • Status changed to Needs review 4 months ago
  • 🇵🇱Poland Graber

    Passing to the first review and testing.

  • 🇵🇱Poland Graber

    Thank you for checking this Alec! Included your remarks except those about catching exceptions. I think if any of those exceptions happen, it indicates a serious issue with the project that should be caught on a local / dev environment or as early as possible in any case (invalid custom code or data corruption) and not allowed to stay there much longer and eventually be noticed resulting in a completely different bug.

  • Pipeline finished with Success
    4 months ago
    #81763
  • 🇵🇱Poland Graber

    Single test fail completely unrelated: Drupal\Tests\ckeditor5\FunctionalJavascript\MediaLinkabilityTest.

  • Pipeline finished with Running
    4 months ago
    #81773
  • 🇮🇹Italy plach Venezia

    @Graber

    Could you please add some details to the IS Problems/Motivations section?

  • Pipeline finished with Running
    4 months ago
    #82256
  • 🇵🇱Poland Graber

    Thank you for checking this @plach, all resolved, if you have concerns about the Kernel test, please provide more details, I don't think we need to go for a functional in such a simple case and at a big performance cost. Layout Builder tests are very heavy already :(

  • 🇮🇹Italy plach Venezia

    Looks good to me, I'll manually test this for a while and then report back.

  • Pipeline finished with Success
    4 months ago
    #82788
  • 🇺🇸United States smustgrave

    Issue summary appears to have been updated previously.

    Tested locally the new permission and an editor role. Things appear to be working as expected but will leave in review for more eyes +1

  • 🇸🇮Slovenia alecsmrekar

    Most entity types rely on an "update" operation instead of the "edit" operation. Paragraphs will check the "update" operation on it's parent entity. If the parent entity is a block, we'll need to convert "update" to "edit".

    Here's a proposal for changing getBundlePermission:

      public static function getBundlePermission(string $block_bundle, string $operation): string {
        if ($operation === 'create') {
          return 'create ' . $block_bundle . ' block content';
        }
        elseif ($operation === 'update') {
          $operation = 'edit';
        }
        return $operation . ' any ' . $block_bundle . ' block content';
      }
  • Status changed to Needs work 3 months ago
  • 🇺🇸United States smustgrave

    To try out #26

  • 🇵🇱Poland Graber

    @alecsmrekar feel free to commit that, it'll be your change after all :)

Production build 0.69.0 2024