From 9889b4e41a9afb2686c93e13c805b6d62d1a338c Mon Sep 17 00:00:00 2001 From: Kristiaan Van den Eynde Date: Fri, 27 Oct 2017 15:02:08 +0200 Subject: [PATCH] Issue #2888588 by kevin.dutra, kristiaanvandeneynde, seanB: Performance issues with node access control --- modules/gnode/gnode.module | 152 ++++++++++++++++++++++++++++++++++++--------- 1 file changed, 124 insertions(+), 28 deletions(-) diff --git a/modules/gnode/gnode.module b/modules/gnode/gnode.module index 0efb6b1..fef8015 100644 --- a/modules/gnode/gnode.module +++ b/modules/gnode/gnode.module @@ -8,6 +8,7 @@ use Drupal\Core\Access\AccessResult; use Drupal\Core\Session\AccountInterface; use Drupal\group\Entity\GroupContentType; +use Drupal\group\Entity\GroupRoleInterface; use Drupal\node\NodeInterface; use Drupal\node\NodeTypeInterface; @@ -22,6 +23,37 @@ function gnode_node_type_insert(NodeTypeInterface $node_type) { } /** + * Implements hook_ENTITY_TYPE_insert(). + */ +function gnode_group_role_insert(GroupRoleInterface $group_role) { + // Because we optimize for anonymous users, it also means we need to rebuild + // the node grants table when an anonymous role is added. + // See: gnode_node_access_records() + if ($group_role->isAnonymous()) { + node_access_needs_rebuild(TRUE); + } +} + +/** + * Implements hook_ENTITY_TYPE_update(). + */ +function gnode_group_role_update(GroupRoleInterface $group_role) { + // Because we optimize for anonymous users, it also means we need to rebuild + // the node grants table when an anonymous role has its permissions changed. + // See: gnode_node_access_records() + if ($group_role->isAnonymous()) { + $new = array_unique($group_role->getPermissions()); + $old = array_unique($group_role->original->getPermissions()); + sort($new); + sort($old); + + if ($new != $old) { + node_access_needs_rebuild(TRUE); + } + } +} + +/** * Implements hook_node_access(). * * When trying to view, update or delete a node it suffices to have the right to @@ -114,6 +146,7 @@ function gnode_node_access(NodeInterface $node, $op, AccountInterface $account) * - 'gnode:NODE_TYPE': Grants view and update or delete any access to nodes. * - 'gnode_unpublished:NODE_TYPE': Grants view access to unpublished nodes. * - 'gnode_author:UID:NODE_TYPE': Grants update or delete access to authors. + * - 'gnode_anonymous': Given to anonymous users. * - 'gnode_bypass': Given to anyone with the 'bypass group access' permission. * * @see gnode_node_access_records() @@ -124,6 +157,12 @@ function gnode_node_grants(AccountInterface $account, $op) { return ['gnode_bypass' => [GNODE_MASTER_GRANT_ID]]; } + // Anonymous users get the anonymous grant. See the implementation in the + // gnode_node_access_records() function as to why that is. + if ($account->isAnonymous()) { + return ['gnode_anonymous' => [GNODE_MASTER_GRANT_ID]]; + } + // Gather the machine names of all node types. $entity_type_manager = \Drupal::entityTypeManager(); $node_type_ids = $entity_type_manager @@ -131,8 +170,8 @@ function gnode_node_grants(AccountInterface $account, $op) { ->getQuery() ->execute(); - // Initialize a grant array for members and one for anonymous/outsider users. - $grants_m = $grants_ao = []; + // Initialize a grant array for members and one for outsider users. + $grants_m = $grants_o = []; // If the user could not bypass group access, we need to check their access // for every single group. Because loading every group would incur a massive @@ -174,8 +213,8 @@ function gnode_node_grants(AccountInterface $account, $op) { } // All other groups have the benefit of sharing the same permission set among - // all anonymous or authenticated users per group type. We can therefore know - // the user's permissions for all groups of the same type they aren't part of. + // all authenticated users per group type. We can therefore know the user's + // permissions for all groups of the same group type they aren't part of. /** @var \Drupal\group\Entity\GroupTypeInterface[] $group_types */ $group_types = $entity_type_manager->getStorage('group_type')->loadMultiple(); foreach ($group_types as $group_type) { @@ -192,11 +231,8 @@ function gnode_node_grants(AccountInterface $account, $op) { continue; } - // Grab the anonymous or outsider role for the group type depending on the - // user's account status (anonymous or authenticated). - $group_role = $account->isAnonymous() - ? $group_type->getAnonymousRole() - : $group_type->getOutsiderRole(); + // Grab the outsider role for the group type. + $group_role = $group_type->getOutsiderRole(); foreach ($node_type_ids as $node_type_id) { $plugin_id = "group_node:$node_type_id"; @@ -210,10 +246,10 @@ function gnode_node_grants(AccountInterface $account, $op) { switch ($op) { case 'view': if ($group_role->hasPermission("view $plugin_id entity")) { - $grants_ao["gnode:$node_type_id"][] = $gids; + $grants_o["gnode:$node_type_id"][] = $gids; } if ($group_role->hasPermission("view unpublished $plugin_id entity")) { - $grants_ao["gnode_unpublished:$node_type_id"][] = $gids; + $grants_o["gnode_unpublished:$node_type_id"][] = $gids; } break; @@ -221,26 +257,26 @@ function gnode_node_grants(AccountInterface $account, $op) { case 'delete': // If you can act on any node, there's no need for the author grant. if ($group_role->hasPermission("$op any $plugin_id entity")) { - $grants_ao["gnode:$node_type_id"][] = $gids; + $grants_o["gnode:$node_type_id"][] = $gids; } elseif ($group_role->hasPermission("$op own $plugin_id entity")) { $uid = $account->id(); - $grants_ao["gnode_author:$uid:$node_type_id"][] = $gids; + $grants_o["gnode_author:$uid:$node_type_id"][] = $gids; } break; } } } - // The code above populated the anonymous/outsider grants by adding the group - // IDs per group type. We need to combine this into one big list of group IDs - // per entry in the $grants_ao array. - foreach ($grants_ao as $key => $gids_per_group_type) { - $grants_ao[$key] = array_reduce($gids_per_group_type, 'array_merge', []); + // The code above populated the outsider grants by adding the group IDs per + // group type. We need to combine this into one big list of group IDs per + // entry in the $grants_o array. + foreach ($grants_o as $key => $gids_per_group_type) { + $grants_o[$key] = array_reduce($gids_per_group_type, 'array_merge', []); } - // Recursively merge the member grants with the anonymous/outsider grants. - return array_merge_recursive($grants_m, $grants_ao); + // Recursively merge the member grants with the outsider grants. + return array_merge_recursive($grants_m, $grants_o); } /** @@ -249,11 +285,12 @@ function gnode_node_grants(AccountInterface $account, $op) { * @see gnode_node_grants() */ function gnode_node_access_records(NodeInterface $node) { - $records = []; - $type = $node->bundle(); + $records = $group_types = []; + $node_type_id = $node->bundle(); + $plugin_id = "group_node:$node_type_id"; // Only act if there are group content types for this node type. - $group_content_types = GroupContentType::loadByContentPluginId("group_node:$type"); + $group_content_types = GroupContentType::loadByContentPluginId($plugin_id); if (empty($group_content_types)) { return $records; } @@ -281,21 +318,80 @@ function gnode_node_access_records(NodeInterface $node) { ]; // Set records for every group the node belongs to. + $uid = $node->getOwnerId(); + $prefix = $node->isPublished() ? 'gnode' : 'gnode_unpublished'; foreach ($group_contents as $group_content) { /** @var \Drupal\group\Entity\GroupContentInterface $group_content */ - $gid = $group_content->getGroup()->id(); + $group = $group_content->getGroup(); + $group_type_id = $group_content->bundle(); + + // Gather all group types the node belongs to. + if (!isset($group_types[$group_type_id])) { + $group_types[$group_type_id] = $group->getGroupType(); + } + + // We use the group ID as the realm ID. + $gid = $group->id(); // Add the non-author record for viewing nodes. - $prefix = $node->isPublished() ? 'gnode' : 'gnode_unpublished'; - $records[] = ['gid' => $gid, 'realm' => "$prefix:$type"] + $base; + $records[] = ['gid' => $gid, 'realm' => "$prefix:$node_type_id"] + $base; // Add the author record for updating or deleting. - $uid = $node->getOwnerId(); - $records[] = ['gid' => $gid, 'realm' => "gnode_author:$uid:$type"] + $base; + $records[] = ['gid' => $gid, 'realm' => "gnode_author:$uid:$node_type_id"] + $base; } // Add the general access bypass record. $records[] = ['gid' => GNODE_MASTER_GRANT_ID, 'realm' => 'gnode_bypass'] + $base; + // For anonymous users we actually build the access record based on the groups + // the node belongs to. After all: If you're anonymous to one group, you're + // anonymous to all groups. Meaning that if one of the node's groups allows + // anonymous users to view the node, all anonymous users can view it. We can + // use this to our advantage by assigning a special access record that we can + // provide a grant for in gnode_node_grants(). + $anonymous_record = [ + 'realm' => 'gnode_anonymous', + 'gid' => GNODE_MASTER_GRANT_ID, + 'grant_view' => 0, + 'grant_update' => 0, + 'grant_delete' => 0, + 'priority' => 0, + ]; + + // Get references to the grants for faster and more readable loops below. + $can_view = &$anonymous_record['grant_view']; + $can_update = &$anonymous_record['grant_update']; + $can_delete = &$anonymous_record['grant_delete']; + + $view_permission = $node->isPublished() + ? "view $plugin_id entity" + : "view unpublished $plugin_id entity"; + + foreach ($group_types as $group_type) { + /** @var \Drupal\group\Entity\GroupTypeInterface $group_type */ + $group_role = $group_type->getAnonymousRole(); + + if (!$can_view && $group_role->hasPermission($view_permission)) { + $can_view = 1; + } + if (!$can_update && $group_role->hasPermission("update any $plugin_id entity")) { + $can_update = 1; + } + if (!$can_delete && $group_role->hasPermission("delete any $plugin_id entity")) { + $can_delete = 1; + } + + // If the node is owned by anonymous, we also need to check for the author + // permissions following the pattern "$op own $plugin_id entity". + if ($uid == 0) { + if (!$can_update && $group_role->hasPermission("update own $plugin_id entity")) { + $can_update = 1; + } + if (!$can_delete && $group_role->hasPermission("delete own $plugin_id entity")) { + $can_delete = 1; + } + } + } + return $records; } -- 2.8.1