From aa8d81c7f20dd6d05318a75f65bdced4e5653fad Mon Sep 17 00:00:00 2001 From: Alex Pott Date: Tue, 28 Jul 2020 09:27:29 +0100 Subject: [PATCH] Issue #2348203 by agentrickard, govind.maloo, mohit_aghera, dagmar, init90, Berdir, alexpott, chx, catch, xjm: hook_node_access() no longer fires for the 'create' operation --- core/lib/Drupal/Core/Entity/entity.api.php | 22 ++++++- core/modules/node/node.api.php | 74 ---------------------- core/modules/node/node.module | 57 +++++++++-------- 3 files changed, 51 insertions(+), 102 deletions(-) diff --git a/core/lib/Drupal/Core/Entity/entity.api.php b/core/lib/Drupal/Core/Entity/entity.api.php index 7e1277fdd03..0b163dc3a0c 100644 --- a/core/lib/Drupal/Core/Entity/entity.api.php +++ b/core/lib/Drupal/Core/Entity/entity.api.php @@ -616,6 +616,18 @@ use Drupal\node\Entity\NodeType; * are invoked are hook_entity_create_access() and * hook_ENTITY_TYPE_create_access() instead. * + * The access to an entity can be influenced in several ways: + * - To explicitly allow access, return an AccessResultInterface object with + * isAllowed() returning TRUE. Other modules can override this access by + * returning TRUE for isForbidden(). + * - To explicitly forbid access, return an AccessResultInterface object with + * isForbidden() returning TRUE. Access will be forbidden even if your module + * (or another module) also returns TRUE for isNeutral() or isAllowed(). + * - To neither allow nor explicitly forbid access, return an + * AccessResultInterface object with isNeutral() returning TRUE. + * - If your module does not return an AccessResultInterface object, neutral + * access will be assumed. + * * The Node entity type has a complex system for determining access, which * developers can interact with. This is described in the * @link node_access Node access topic. @endlink @@ -643,7 +655,10 @@ use Drupal\node\Entity\NodeType; * @param string $operation * The operation that is to be performed on $entity. * @param \Drupal\Core\Session\AccountInterface $account - * The account trying to access the entity. + * The account trying to access the entity. Usually one of: + * - "view" + * - "update" + * - "delete" * * @return \Drupal\Core\Access\AccessResultInterface * The access result. The final result is calculated by using @@ -677,7 +692,10 @@ function hook_entity_access(\Drupal\Core\Entity\EntityInterface $entity, $operat * @param \Drupal\Core\Entity\EntityInterface $entity * The entity to check access to. * @param string $operation - * The operation that is to be performed on $entity. + * The operation that is to be performed on $entity. Usually one of: + * - "view" + * - "update" + * - "delete" * @param \Drupal\Core\Session\AccountInterface $account * The account trying to access the entity. * diff --git a/core/modules/node/node.api.php b/core/modules/node/node.api.php index 54013ffdcbd..8461d91139f 100644 --- a/core/modules/node/node.api.php +++ b/core/modules/node/node.api.php @@ -8,7 +8,6 @@ use Drupal\node\NodeInterface; use Drupal\Component\Utility\Html; use Drupal\Component\Utility\Xss; -use Drupal\Core\Access\AccessResult; /** * @addtogroup hooks @@ -286,79 +285,6 @@ function hook_node_grants_alter(&$grants, \Drupal\Core\Session\AccountInterface } } -/** - * Controls access to a node. - * - * Modules may implement this hook if they want to have a say in whether or not - * a given user has access to perform a given operation on a node. - * - * The administrative account (user ID #1) always passes any access check, so - * this hook is not called in that case. Users with the "bypass node access" - * permission may always view and edit content through the administrative - * interface. - * - * The access to a node can be influenced in several ways: - * - To explicitly allow access, return an AccessResultInterface object with - * isAllowed() returning TRUE. Other modules can override this access by - * returning TRUE for isForbidden(). - * - To explicitly forbid access, return an AccessResultInterface object with - * isForbidden() returning TRUE. Access will be forbidden even if your module - * (or another module) also returns TRUE for isNeutral() or isAllowed(). - * - To neither allow nor explicitly forbid access, return an - * AccessResultInterface object with isNeutral() returning TRUE. - * - If your module does not return an AccessResultInterface object, neutral - * access will be assumed. - * - * Also note that this function isn't called for node listings (e.g., RSS feeds, - * the default home page at path 'node', a recent content block, etc.) See - * @link node_access Node access rights @endlink for a full explanation. - * - * @param \Drupal\node\NodeInterface|string $node - * Either a node entity or the machine name of the content type on which to - * perform the access check. - * @param string $op - * The operation to be performed. Possible values: - * - "create" - * - "delete" - * - "update" - * - "view" - * @param \Drupal\Core\Session\AccountInterface $account - * The user object to perform the access check operation on. - * - * @return \Drupal\Core\Access\AccessResultInterface - * The access result. - * - * @ingroup node_access - */ -function hook_node_access(\Drupal\node\NodeInterface $node, $op, \Drupal\Core\Session\AccountInterface $account) { - $type = $node->bundle(); - - switch ($op) { - case 'create': - return AccessResult::allowedIfHasPermission($account, 'create ' . $type . ' content'); - - case 'update': - if ($account->hasPermission('edit any ' . $type . ' content')) { - return AccessResult::allowed()->cachePerPermissions(); - } - else { - return AccessResult::allowedIf($account->hasPermission('edit own ' . $type . ' content') && ($account->id() == $node->getOwnerId()))->cachePerPermissions()->cachePerUser()->addCacheableDependency($node); - } - - case 'delete': - if ($account->hasPermission('delete any ' . $type . ' content')) { - return AccessResult::allowed()->cachePerPermissions(); - } - else { - return AccessResult::allowedIf($account->hasPermission('delete own ' . $type . ' content') && ($account->id() == $node->getOwnerId()))->cachePerPermissions()->cachePerUser()->addCacheableDependency($node); - } - - default: - // No opinion. - return AccessResult::neutral(); - } -} - /** * Act on a node being displayed as a search result. * diff --git a/core/modules/node/node.module b/core/modules/node/node.module index eae4365c798..31b1143372d 100644 --- a/core/modules/node/node.module +++ b/core/modules/node/node.module @@ -746,16 +746,16 @@ function node_form_system_themes_admin_form_submit($form, FormStateInterface $fo * @{ * The node access system determines who can do what to which nodes. * - * In determining access rights for a node, \Drupal\node\NodeAccessControlHandler - * first checks whether the user has the "bypass node access" permission. Such - * users have unrestricted access to all nodes. user 1 will always pass this - * check. + * In determining access rights for an existing node, + * \Drupal\node\NodeAccessControlHandler first checks whether the user has the + * "bypass node access" permission. Such users have unrestricted access to all + * nodes. user 1 will always pass this check. * - * Next, all implementations of hook_node_access() will be called. Each - * implementation may explicitly allow, explicitly forbid, or ignore the access - * request. If at least one module says to forbid the request, it will be - * rejected. If no modules deny the request and at least one says to allow it, - * the request will be permitted. + * Next, all implementations of hook_ENTITY_TYPE_access() for node will + * be called. Each implementation may explicitly allow, explicitly forbid, or + * ignore the access request. If at least one module says to forbid the request, + * it will be rejected. If no modules deny the request and at least one says to + * allow it, the request will be permitted. * * If all modules ignore the access request, then the node_access table is used * to determine access. All node access modules are queried using @@ -768,40 +768,42 @@ function node_form_system_themes_admin_form_submit($form, FormStateInterface $fo * * In node listings (lists of nodes generated from a select query, such as the * default home page at path 'node', an RSS feed, a recent content block, etc.), - * the process above is followed except that hook_node_access() is not called on - * each node for performance reasons and for proper functioning of the pager - * system. When adding a node listing to your module, be sure to use an entity - * query, which will add a tag of "node_access". This will allow modules dealing - * with node access to ensure only nodes to which the user has access are - * retrieved, through the use of hook_query_TAG_alter(). See the + * the process above is followed except that hook_ENTITY_TYPE_access() is not + * called on each node for performance reasons and for proper functioning of + * the pager system. When adding a node listing to your module, be sure to use + * an entity query, which will add a tag of "node_access". This will allow + * modules dealing with node access to ensure only nodes to which the user has + * access are retrieved, through the use of hook_query_TAG_alter(). See the * @link entity_api Entity API topic @endlink for more information on entity * queries. Tagging a query with "node_access" does not check the * published/unpublished status of nodes, so the base query is responsible * for ensuring that unpublished nodes are not displayed to inappropriate users. * * Note: Even a single module returning an AccessResultInterface object from - * hook_node_access() whose isForbidden() method equals TRUE will block access - * to the node. Therefore, implementers should take care to not deny access - * unless they really intend to. Unless a module wishes to actively forbid - * access it should return an AccessResultInterface object whose isAllowed() nor - * isForbidden() methods return TRUE, to allow other modules or the node_access - * table to control access. + * hook_ENTITY_TYPE_access() whose isForbidden() method equals TRUE will block + * access to the node. Therefore, implementers should take care to not deny + * access unless they really intend to. Unless a module wishes to actively + * forbid access it should return an AccessResultInterface object whose + * isAllowed() nor isForbidden() methods return TRUE, to allow other modules or + * the node_access table to control access. + * + * Note also that access to create nodes is handled by + * hook_ENTITY_TYPE_create_access(). * * To see how to write a node access module of your own, see * node_access_example.module. + * + * @see \Drupal\node\NodeAccessControlHandler */ /** - * Implements hook_node_access(). + * Implements hook_ENTITY_TYPE_access(). */ function node_node_access(NodeInterface $node, $op, AccountInterface $account) { $type = $node->bundle(); - $access = AccessResult::neutral(); + // Note create access is handled by hook_ENTITY_TYPE_create_access(). switch ($op) { - case 'create': - $access = AccessResult::allowedIfHasPermission($account, 'create ' . $type . ' content'); - case 'update': $access = AccessResult::allowedIfHasPermission($account, 'edit any ' . $type . ' content'); if (!$access->isAllowed() && $account->hasPermission('edit own ' . $type . ' content')) { @@ -815,6 +817,9 @@ function node_node_access(NodeInterface $node, $op, AccountInterface $account) { $access = $access->orIf(AccessResult::allowedIf($account->id() == $node->getOwnerId()))->cachePerUser()->addCacheableDependency($node); } break; + + default: + $access = AccessResult::neutral(); } return $access;