EntityAccessController documents the invocation of hook_entity_access(), however no hook is ever called.

A generic hook_entity_access should be added which is called for all entities, in the spirit of hook_entity_create/view/presave/load/delete etc.

core/lib/Drupal/Core/Entity/EntityAccessController.php

    // Invoke hook_entity_access(), hook results take precedence over overridden
    // implementations of EntityAccessController::checkAccess(). Entities
    // that have checks that need to be done before the hook is invoked should
    // do so by overridding this method.
????? nothing here ?????
    // We grant access to the entity if both of these conditions are met:
    // - No modules say to deny access.
    // - At least one module says to grant access.
    $access = module_invoke_all($entity->entityType() . '_access', $entity->getBCEntity(), $operation, $account, $langcode);

References

Proposal

Implement hook_entity_(create_)access() in addition to the existing hook_ENTITY_TYPE_access() and document hook_ENTITY_TYPE_ACCESS() because it is already being called.

API changes

None. Documentation will be fixed and two hooks will be added.

Issues blocked on this

#1839516: Introduce entity operation providers is blocked on this, because field_ui needs to hook into entity access control for any entity type and not just specific ones, in order to check access for the entity operations it defines. This is why a hook_entity_access() is required and hook_ENTITY_TYPE_access() can technically not be used.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dpi’s picture

Issue tags: +Entity Access

Tagging

clemens.tolboom’s picture

I couldn't find documentation about hook_entity_access on https://api.drupal.org/api/drupal/8/search/hook_entity_access while trying to work on tour_ui: #2031163: Main page is broken on 'admin/config/user-interface/tour'

Don't we need the API documentation first :)

Xano’s picture

Assigned: Unassigned » Xano
Xano’s picture

Assigned: Xano » Unassigned
Status: Active » Needs review
FileSize
7.11 KB
  1. I corrected the code comments.
  2. I added a real hook_entity_access() and hook_entity_create_access().
  3. I documented all four hooks in entity.api.php.
  4. I injected the module handler.
Xano’s picture

Title: Implement hook_entity_access() » Implement hook_entity_access() and hook_entity_create_access()

Status: Needs review » Needs work

The last submitted patch, drupal_2057377_4.patch, failed testing.

Xano’s picture

Status: Needs work » Needs review
FileSize
6.92 KB

Status: Needs review » Needs work

The last submitted patch, drupal_2057377_7.patch, failed testing.

Xano’s picture

Status: Needs work » Needs review
FileSize
9.89 KB
tim.plunkett’s picture

Status: Needs review » Needs work

The last submitted patch, drupal_2057377_9.patch, failed testing.

jibran’s picture

Xano’s picture

Status: Needs work » Needs review
FileSize
5.4 KB
jibran’s picture

  1. +++ b/core/includes/entity.api.php
    @@ -11,6 +11,74 @@
    + * @param \Drupal\Core\Entity\EntityInterface $entity
    + * @param string $operation
    + * @param \Drupal\Core\Session\AccountInterface $account
    + * @param string $langcode
    

    Little description will help.

  2. +++ b/core/includes/entity.api.php
    @@ -11,6 +11,74 @@
    + *
    ...
    + *
    

    desc missing here.

  3. +++ b/core/includes/entity.api.php
    @@ -11,6 +11,74 @@
    + * @see \Drupal\Core\Entity\EntityAccessController
    

    @see should be at the end of the documentation.

Status: Needs review » Needs work

The last submitted patch, drupal_2057377_12.patch, failed testing.

Xano’s picture

Status: Needs work » Needs review
FileSize
6.02 KB
4.55 KB

Status: Needs review » Needs work
Issue tags: -Entity Access

The last submitted patch, drupal_2057377_16.patch, failed testing.

Xano’s picture

Status: Needs work » Needs review

#16: drupal_2057377_16.patch queued for re-testing.

Status: Needs review » Needs work
Issue tags: +Entity Access

The last submitted patch, drupal_2057377_16.patch, failed testing.

Xano’s picture

Status: Needs work » Needs review
FileSize
6.31 KB
468 bytes
jibran’s picture

Issue tags: +Need tests

I think we need tests here.

Xano’s picture

Issue tags: -Need tests
FileSize
2.4 KB
9.01 KB
dawehner’s picture

  1. +++ b/core/includes/entity.api.php
    @@ -11,6 +11,86 @@
    + * @return bool|null
    ...
    +function hook_entity_access(\Drupal\Core\Entity\EntityInterface $entity, $operation, \Drupal\Core\Session\AccountInterface $account, $langcode) {
    +  return NULL;
    ...
    + * @return bool|null
    ...
    +  return NULL;
    

    Just wondering whether we should use AccessInterface::ALLOW DENY and KILL on the longrun for these values?

  2. +++ b/core/modules/system/lib/Drupal/system/Tests/Entity/EntityAccessTest.php
    @@ -136,4 +136,24 @@ function testEntityTranslationAccess() {
    +  protected function testHooks() {
    

    Oh since when do we use protected for test functions ... are they even called with that?

Xano’s picture

Just wondering whether we should use AccessInterface::ALLOW DENY and KILL on the longrun for these values?

AccessInterface is for routes. These are API hooks.

Oh since when do we use protected for test functions ... are they even called with that?

They're called from within the class and don't need to be public.

dawehner’s picture

Status: Needs review » Reviewed & tested by the community

AccessInterface is for routes. These are API hooks.

I know that it is for routes, though the interface is not part of the routing namespace, just saying ... and this might add some clarity

Regarding protected ... well let's just keep that, even it is inconsistent with like 99% of core.

dawehner’s picture

Issue summary: View changes

formatting

Xano’s picture

Issue summary: View changes

.

Xano’s picture

Issue summary: View changes

.

fago’s picture

Status: Reviewed & tested by the community » Needs work

The patch looks good to me, just the hook docs could use some improvement I think:

+++ b/core/includes/entity.api.php
@@ -11,6 +11,86 @@
+ * Checks entity operation access.

I don't think "Checks access" is a good description here. The hook does not check access for you, you can use the hook to do it yourself! I.e. you implement the hook to "control access" or "change access", not to check it.

hook_node_access() seems to go with "control access" so I'd suggest following that.

Xano’s picture

Status: Needs work » Needs review
FileSize
9.01 KB
1.03 KB
fago’s picture

Status: Needs review » Active

I think it should be "Check entity create access." - without the s. If you look at other hook documentations they are about you doing something, not about a function doing something for you.

Xano’s picture

Status: Active » Needs review
FileSize
9.01 KB
1.04 KB
fago’s picture

Status: Needs review » Reviewed & tested by the community

Thanks! Back to RTBC then.

alexpott’s picture

Title: Implement hook_entity_access() and hook_entity_create_access() » Change notice: Implement hook_entity_access() and hook_entity_create_access()
Priority: Normal » Major
Status: Reviewed & tested by the community » Active
Issue tags: +Needs change record

Committed 2891848 and pushed to 8.x. Thanks!

Xano’s picture

Assigned: Unassigned » Xano

@alexpott Thank you!

Xano’s picture

Title: Change notice: Implement hook_entity_access() and hook_entity_create_access() » Implement hook_entity_access() and hook_entity_create_access()
Assigned: Xano » Unassigned
Priority: Major » Normal
Status: Active » Fixed

The change notice is at https://drupal.org/node/2095227.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

xjm’s picture

Issue tags: -Needs change record

Please remove the tag when the change notification task is completed.

xjm’s picture

Issue summary: View changes

.