Updated: Comment #0
Problem/Motivation
After custom block is placed in some region with "node condition" there's no way to remove or reconfigure one.
That's caused by EntityListController::getOperations()
code
if ($entity->access('update')) {
$operations['edit'] = array(
'title' => $this->t('Edit'),
'href' => $uri['path'] . '/edit',
'options' => $uri['options'],
'weight' => 10,
);
}
User could not have access to update/delete a custom block because node_block_access()
fires first and blocks execution of entity access callback in EntityAccessController::access()
$access = array_merge(
$this->moduleHandler->invokeAll('entity_access', array($entity, $operation, $account, $langcode)),
$this->moduleHandler->invokeAll($entity->entityType() . '_access', array($entity, $operation, $account, $langcode))
);
if (($return = $this->processAccessHookResults($access)) === NULL) {
// No module had an opinion about the access, so let's the access
// controller check create access.
$return = (bool) $this->checkAccess($entity, $operation, $langcode, $account);
}
return $this->setCache($return, $entity->uuid(), $operation, $langcode, $account);
The comment makes sense so ...
Proposed resolution
Use BlockListController::access()
method to fire it before standard controller
Remaining tasks
tbd
User interface changes
UI should work
API changes
not sure
Comment | File | Size | Author |
---|---|---|---|
#3 | interdiff.txt | 3.88 KB | dawehner |
#3 | block-2139141.patch | 4.24 KB | dawehner |
#2 | 2139141-block-access-2.patch | 1.42 KB | andypost |
block-access.patch | 832 bytes | andypost | |
Comments
Comment #2
andypostAnother approach, do not allow to fire hooks for operations other then 'view'
Comment #3
dawehnerI think it is wrong to not fire those hooks on other cases than 'view'.
Let's use the proper interfaces, if we touch all the instances anyway.
Comment #4
andypostRe-title, actually we need to think more about block access
Agree here, so seems that first patch was right - we need to override
access()
method and move visibility logic to special method to make it extendable in contrib.Currently access to view block is hardcoded in (page-url, role) but node-type access check is implemented via
node_block_access()
and overlay removes unneeded blocks via the same hookComment #6
larowlanWe should be testing that the links are visible
Comment #7
olli CreditAttribution: olli commentedrelated #2099503: No operations for some blocks at Block layout page in overlay and #2124599: node_block_access affects access to editing blocks Removes "operations" links for Block layout page.
Comment #8
andypostProbably better to refactor the way like visibility works, related #1880274: Reimplement block visibility settings
Suppose current implementation is weird and will bring a lot of troubles in contrib because each contrib implementation have to add check for
$operation == 'view'
inhook_block_access()
So I'd prefer #0 approach to override
BlockAccessController::access()
to execute hooks afterBlockBase::access()
to allow override visibility of the blockComment #9
tim.plunkettThis actually causes a bug in overlay, see #2099503: No operations for some blocks at Block layout page in overlay
If that is marked a duplicate, the tests should be moved here.
Comment #10
tim.plunkettSee also #2124599: node_block_access affects access to editing blocks Removes "operations" links for Block layout page.
Comment #11
andypostOverlay hunk could be skipped now, maybe better to postpone the issue on #2124599: node_block_access affects access to editing blocks Removes "operations" links for Block layout page. or just mark as duplicate?
Comment #12
andypostComment #13
mr.baileysWorking on this as part of the Drupal Developer Days sprint in Szeged.
Comment #14
mr.baileysSeems to me this should be marked as duplicate of #2124599: node_block_access affects access to editing blocks Removes "operations" links for Block layout page..