Problem/Motivation

Custom access checks can be added for operations in the layout builder either via custom code or via contrib modules like https://www.drupal.org/project/layout_builder_perms. In this way it is also possible to disallow moving blocks. In an advance use case it can be desirable to disallow blocks of a certain type into sections with a certain layout. This works fine but the layout builder UI does not respect the access on routes involved. This can lead to a situation in which the layout builder UI shows a move operation as successful (i.e. the block is move to another region in another section) but the ajax call to actually register the change resulted in a 403. The user will not notice this until reloading the page or saving the layout. Only then they will discover that the block was not moved.

Steps to reproduce

* Install Drupal, enable layout builder and create a layout builder enabled content type.
* Install Layout Builder Advanced Permissions (https://www.drupal.org/project/layout_builder_perms) and enable the Layout Builder Advanced Permissions Layout sub module.
* Create a custom role and configure permissions to allow the role to place blocks in sections with a layout of a certain type but not in sections with a layout of another type.
* Create a node of a layout builder enabled content type.
* Add a section (A) in the layout builder and configure it with a layout in which the custom role is allowed to place blocks.
* Add a section (B) in the layout builder and configure it with a layout in which the custom role is not allowed to place blocks.
* Add a block in section A.
* Save the layout.
* Go back to the layout builder and move the block from section A and section B. The layout builder will visually allow this and move the block to section B.
* Save the layout.
* See that the block is still in section A.

Proposed resolution

In core/modules/layout_builder/js/layout-builder.es6.js there is a behavior layoutBuilderBlockDrag. The Sortable that is created in this behavior can be extended with an onMove() callback. This callback is executed every time a block is dragged over a region. Returning false from this callback will disallow drop in that region. It will also prevent the region from being highlighted.

For this to work we will also need to add a URL that can be called via ajax and that only executes the access check for the layout_builder.move_block route and return the result (without actually executing the logic in the controller linked to the route).

Remaining tasks

- Create patch
- Review patch
- Commit patch

User interface changes

Block operations in layout builder that are not allowed will not be possible anymore in the layout builder UI.

API changes

N/A

Data model changes

N/A

Release notes snippet

TBD

Issue fork drupal-3213792

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

ricovandevin created an issue. See original summary.

ricovandevin’s picture

Status: Active » Needs review
StatusFileSize
new10.71 KB

Here is a patch. It seems to work fine but code might need grooming.

ricovandevin’s picture

Issue summary: View changes
gauravvvv’s picture

StatusFileSize
new10.71 KB
new1.53 KB

Re-rolled patch #3, added interdiff for same. Please review.

Version: 9.1.x-dev » 9.3.x-dev

Drupal 9.1.10 (June 4, 2021) and Drupal 9.2.10 (November 24, 2021) were the last bugfix releases of those minor version series. Drupal 9 bug reports should be targeted for the 9.3.x-dev branch from now on, and new development or disruptive changes should be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.15 was released on June 1st, 2022 and is the final full bugfix release for the Drupal 9.3.x series. Drupal 9.3.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.4.x-dev branch from now on, and new development or disruptive changes should be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

pooja saraah’s picture

StatusFileSize
new10.71 KB
new2.42 KB

Fixed failed commands on #4
Attached patch against Drupal 9.4.x

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.9 was released on December 7, 2022 and is the final full bugfix release for the Drupal 9.4.x series. Drupal 9.4.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.5.x-dev branch from now on, and new development or disruptive changes should be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

ricovandevin’s picture

StatusFileSize
new11.22 KB

Rerolled against 9.5.2.

pooja saraah’s picture

StatusFileSize
new10.88 KB
new5.1 KB

Fixed failed commands on #10
Attached patch against Drupal 9.5.x

smustgrave’s picture

Status: Needs review » Needs work
Issue tags: +Needs Review Queue Initiative, +Needs tests

#11 is failing CI builds.

make sure to run ./core/scripts/dev/commit-code-check.sh before uploading a patch to make sure there are no issues with code formatting. see https://www.drupal.org/docs/develop/development-tools/running-core-devel...

Tagging for tests to show this issue.

Did not attempt to test myself.

_utsavsharma’s picture

StatusFileSize
new4.42 KB
new10.69 KB

Fixed CCF for #10.

gauravvvv’s picture

Updating attributions

seanb’s picture

Status: Needs work » Needs review
StatusFileSize
new10.85 KB

Added missing use statement, which probably confirms we need tests.

smustgrave’s picture

Status: Needs review » Needs work

@seanB 100% correct. It's good to see that this change didn't break anything but could use a test case to make sure the issue being fixed here doesn't pop back up.

Also can we typehint public function allowed().

Not sure if you want to make it a D10 patch or both?

seanb’s picture

Version: 9.5.x-dev » 10.1.x-dev
StatusFileSize
new8.81 KB

Reroll for 10.1

heykarthikwithu’s picture

Status: Needs work » Needs review

Moving to Needs review, based on the #17 reroll.

(Pl move back if not correct)

smustgrave’s picture

Status: Needs review » Needs work

Was previously tagged for tests which still appear needed.

Thanks.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.