From 5f6d8a990c965f86f25c4aeefa13150f349fba7e Mon Sep 17 00:00:00 2001 From: Pieter Frenssen Date: Wed, 29 Jul 2020 16:03:25 +0300 Subject: [PATCH 1/7] No need to inject the GroupAudienceHelper service, the GroupTypeManager also provides this information. --- og.services.yml | 2 +- src/Og.php | 4 +--- src/OgAccess.php | 15 ++------------- tests/src/Unit/OgAccessEntityTestBase.php | 6 +++--- tests/src/Unit/OgAccessTestBase.php | 14 +------------- 5 files changed, 8 insertions(+), 33 deletions(-) diff --git a/og.services.yml b/og.services.yml index 45e31dacf..be1821d89 100644 --- a/og.services.yml +++ b/og.services.yml @@ -21,7 +21,7 @@ services: - { name: 'cache.context'} og.access: class: Drupal\og\OgAccess - arguments: ['@config.factory', '@current_user', '@module_handler', '@og.group_type_manager', '@og.permission_manager', '@og.membership_manager', '@og.group_audience_helper'] + arguments: ['@config.factory', '@current_user', '@module_handler', '@og.group_type_manager', '@og.permission_manager', '@og.membership_manager'] og.context: class: Drupal\og\ContextProvider\OgContext arguments: ['@plugin.manager.og.group_resolver', '@config.factory'] diff --git a/src/Og.php b/src/Og.php index 1b7feb218..34043962a 100644 --- a/src/Og.php +++ b/src/Og.php @@ -260,8 +260,6 @@ public static function isGroup($entity_type_id, $bundle_id) { /** * Check if the given entity type and bundle is a group content. * - * This works by checking if the bundle has one or more group audience fields. - * * @param string $entity_type_id * The entity type. * @param string $bundle_id @@ -271,7 +269,7 @@ public static function isGroup($entity_type_id, $bundle_id) { * True or false if the given entity is group content. */ public static function isGroupContent($entity_type_id, $bundle_id) { - return \Drupal::service('og.group_audience_helper')->hasGroupAudienceField($entity_type_id, $bundle_id); + return \Drupal::service('og.group_type_manager')->isGroupContent($entity_type_id, $bundle_id); } /** diff --git a/src/OgAccess.php b/src/OgAccess.php index 7317f30c7..7439e192f 100644 --- a/src/OgAccess.php +++ b/src/OgAccess.php @@ -76,13 +76,6 @@ class OgAccess implements OgAccessInterface { */ protected $membershipManager; - /** - * The OG group audience helper. - * - * @var \Drupal\og\OgGroupAudienceHelperInterface - */ - protected $groupAudienceHelper; - /** * Constructs the OgAccess service. * @@ -98,17 +91,14 @@ class OgAccess implements OgAccessInterface { * The permission manager. * @param \Drupal\og\MembershipManagerInterface $membership_manager * The group membership manager. - * @param \Drupal\og\OgGroupAudienceHelperInterface $group_audience_helper - * The OG group audience helper. */ - public function __construct(ConfigFactoryInterface $config_factory, AccountProxyInterface $account_proxy, ModuleHandlerInterface $module_handler, GroupTypeManagerInterface $group_manager, PermissionManagerInterface $permission_manager, MembershipManagerInterface $membership_manager, OgGroupAudienceHelperInterface $group_audience_helper) { + public function __construct(ConfigFactoryInterface $config_factory, AccountProxyInterface $account_proxy, ModuleHandlerInterface $module_handler, GroupTypeManagerInterface $group_manager, PermissionManagerInterface $permission_manager, MembershipManagerInterface $membership_manager) { $this->configFactory = $config_factory; $this->accountProxy = $account_proxy; $this->moduleHandler = $module_handler; $this->groupTypeManager = $group_manager; $this->permissionManager = $permission_manager; $this->membershipManager = $membership_manager; - $this->groupAudienceHelper = $group_audience_helper; } /** @@ -230,8 +220,7 @@ public function userAccessEntity($operation, EntityInterface $entity, AccountInt } } - $is_group_content = $this->groupAudienceHelper->hasGroupAudienceField($entity_type_id, $bundle); - if ($is_group_content) { + if ($this->groupTypeManager->isGroupContent($entity_type_id, $bundle)) { $cache_tags = $entity_type->getListCacheTags(); // The entity might be a user or a non-user entity. diff --git a/tests/src/Unit/OgAccessEntityTestBase.php b/tests/src/Unit/OgAccessEntityTestBase.php index 4baf2dd78..8087db9e3 100644 --- a/tests/src/Unit/OgAccessEntityTestBase.php +++ b/tests/src/Unit/OgAccessEntityTestBase.php @@ -50,9 +50,9 @@ protected function setUp(): void { $this->groupContentEntity->getEntityTypeId()->willReturn($entity_type_id); $this->addCache($this->groupContentEntity); - // If the group audience helper is asked if the group content entity has any - // group audience fields, it is expected that this will return TRUE. - $this->groupAudienceHelper->hasGroupAudienceField($entity_type_id, $bundle) + // If the group type manager is asked if the group content entity is group + // content, it is expected that this will return TRUE. + $this->groupTypeManager->isGroupContent($entity_type_id, $bundle) ->willReturn(TRUE); // It is expected that a list of entity operation permissions is retrieved diff --git a/tests/src/Unit/OgAccessTestBase.php b/tests/src/Unit/OgAccessTestBase.php index 6e1a25728..79946f9ac 100644 --- a/tests/src/Unit/OgAccessTestBase.php +++ b/tests/src/Unit/OgAccessTestBase.php @@ -13,7 +13,6 @@ use Drupal\Core\Session\AccountInterface; use Drupal\Core\Session\AccountProxyInterface; use Drupal\og\MembershipManagerInterface; -use Drupal\og\OgGroupAudienceHelperInterface; use Drupal\og\OgMembershipInterface; use Drupal\Tests\UnitTestCase; use Drupal\og\GroupTypeManagerInterface; @@ -98,13 +97,6 @@ class OgAccessTestBase extends UnitTestCase { */ protected $membershipManager; - /** - * The OG group audience helper. - * - * @var \Drupal\og\OgGroupAudienceHelperInterface - */ - protected $groupAudienceHelper; - /** * The entity type manager service. * @@ -178,8 +170,6 @@ protected function setUp(): void { $this->membershipManager->getGroupCount(Argument::any())->willReturn(1); $this->membership->getRoles()->willReturn([$this->ogRole->reveal()]); - $this->groupAudienceHelper = $this->prophesize(OgGroupAudienceHelperInterface::class); - // @todo: Move to test. $this->ogRole->isAdmin()->willReturn(FALSE); $this->ogRole->getPermissions()->willReturn(['update group']); @@ -196,8 +186,7 @@ protected function setUp(): void { $module_handler->reveal(), $this->groupTypeManager->reveal(), $this->permissionManager->reveal(), - $this->membershipManager->reveal(), - $this->groupAudienceHelper->reveal() + $this->membershipManager->reveal() ); $container = new ContainerBuilder(); @@ -207,7 +196,6 @@ protected function setUp(): void { $container->set('module_handler', $this->prophesize(ModuleHandlerInterface::class)->reveal()); $container->set('og.group_type_manager', $this->groupTypeManager->reveal()); $container->set('og.membership_manager', $this->membershipManager->reveal()); - $container->set('og.group_audience_helper', $this->groupAudienceHelper->reveal()); // This is for caching purposes only. $container->set('current_user', $this->user->reveal()); From 6c3fe28960d39d1700a26e2ddb9245e827aefd28 Mon Sep 17 00:00:00 2001 From: Pieter Frenssen Date: Wed, 29 Jul 2020 16:58:34 +0300 Subject: [PATCH 2/7] Use correct terminology. ::userAccessEntity() deals with permissions rather than operations. --- src/OgAccess.php | 6 +++--- tests/src/Unit/OgAccessEntityTest.php | 14 +++++++------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/OgAccess.php b/src/OgAccess.php index 7439e192f..7c5ec3ab7 100644 --- a/src/OgAccess.php +++ b/src/OgAccess.php @@ -198,7 +198,7 @@ public function userAccess(EntityInterface $group, string $permission, AccountIn /** * {@inheritdoc} */ - public function userAccessEntity($operation, EntityInterface $entity, AccountInterface $user = NULL): AccessResultInterface { + public function userAccessEntity($permission, EntityInterface $entity, AccountInterface $user = NULL): AccessResultInterface { $result = AccessResult::neutral(); $entity_type = $entity->getEntityType(); @@ -206,7 +206,7 @@ public function userAccessEntity($operation, EntityInterface $entity, AccountInt $bundle = $entity->bundle(); if ($this->groupTypeManager->isGroup($entity_type_id, $bundle)) { - $user_access = $this->userAccess($entity, $operation, $user); + $user_access = $this->userAccess($entity, $permission, $user); if ($user_access->isAllowed()) { return $user_access; } @@ -240,7 +240,7 @@ public function userAccessEntity($operation, EntityInterface $entity, AccountInt // Check if the operation matches a group level operation such as // 'subscribe without approval'. - $user_access = $this->userAccess($group, $operation, $user); + $user_access = $this->userAccess($group, $permission, $user); if ($user_access->isAllowed()) { return $user_access->addCacheTags($cache_tags); } diff --git a/tests/src/Unit/OgAccessEntityTest.php b/tests/src/Unit/OgAccessEntityTest.php index 10df432f0..2ac1034e1 100644 --- a/tests/src/Unit/OgAccessEntityTest.php +++ b/tests/src/Unit/OgAccessEntityTest.php @@ -11,33 +11,33 @@ class OgAccessEntityTest extends OgAccessEntityTestBase { /** - * Tests access to an entity by different operations. + * Tests access to an entity by different permissions. * * @coversDefaultmethod ::userAccessEntity * @dataProvider permissionsProvider */ - public function testAccessByOperation($operation) { + public function testAccessByPermission($permission) { $this->membershipManager->getGroups($this->groupContentEntity->reveal())->willReturn([$this->entityTypeId => [$this->group]]); - $user_access = $this->ogAccess->userAccessEntity($operation, $this->groupContentEntity->reveal(), $this->user->reveal()); + $user_access = $this->ogAccess->userAccessEntity($permission, $this->groupContentEntity->reveal(), $this->user->reveal()); // We populate the allowed permissions cache in // OgAccessEntityTestBase::setup(). - $condition = $operation == 'update group' ? $user_access->isAllowed() : $user_access->isForbidden(); + $condition = $permission == 'update group' ? $user_access->isAllowed() : $user_access->isForbidden(); $this->assertTrue($condition); } /** - * Tests access to an entity by different operations, by an admin member. + * Tests access to an entity by different permissions, by an admin member. * * @coversDefaultmethod ::userAccessEntity * @dataProvider permissionsProvider */ - public function testAccessByOperationAdmin($operation) { + public function testAccessByPermissionAdmin($permission) { $this->membershipManager->getGroups($this->groupContentEntity->reveal())->willReturn([$this->entityTypeId => [$this->group]]); $this->user->hasPermission('administer organic groups')->willReturn(TRUE); - $user_entity_access = $this->ogAccess->userAccessEntity($operation, $this->groupContentEntity->reveal(), $this->user->reveal()); + $user_entity_access = $this->ogAccess->userAccessEntity($permission, $this->groupContentEntity->reveal(), $this->user->reveal()); $this->assertTrue($user_entity_access->isAllowed()); } From 74570b507135974e7752e1ec486a01861b0b0bd9 Mon Sep 17 00:00:00 2001 From: Pieter Frenssen Date: Wed, 29 Jul 2020 17:00:33 +0300 Subject: [PATCH 3/7] Adopt scalar type hints. --- src/OgAccess.php | 4 ++-- src/OgAccessInterface.php | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/OgAccess.php b/src/OgAccess.php index 7c5ec3ab7..86acfe476 100644 --- a/src/OgAccess.php +++ b/src/OgAccess.php @@ -198,7 +198,7 @@ public function userAccess(EntityInterface $group, string $permission, AccountIn /** * {@inheritdoc} */ - public function userAccessEntity($permission, EntityInterface $entity, AccountInterface $user = NULL): AccessResultInterface { + public function userAccessEntity(string $permission, EntityInterface $entity, AccountInterface $user = NULL): AccessResultInterface { $result = AccessResult::neutral(); $entity_type = $entity->getEntityType(); @@ -262,7 +262,7 @@ public function userAccessEntity($permission, EntityInterface $entity, AccountIn /** * {@inheritdoc} */ - public function userAccessGroupContentEntityOperation($operation, EntityInterface $group_entity, EntityInterface $group_content_entity, AccountInterface $user = NULL): AccessResultInterface { + public function userAccessGroupContentEntityOperation(string $operation, EntityInterface $group_entity, EntityInterface $group_content_entity, AccountInterface $user = NULL): AccessResultInterface { // Default to the current user. $user = $user ?: $this->accountProxy->getAccount(); diff --git a/src/OgAccessInterface.php b/src/OgAccessInterface.php index c15522261..3f3b6fca9 100644 --- a/src/OgAccessInterface.php +++ b/src/OgAccessInterface.php @@ -62,7 +62,7 @@ public function userAccess(EntityInterface $group, string $permission, AccountIn * @return \Drupal\Core\Access\AccessResultInterface * An access result object. */ - public function userAccessEntity($operation, EntityInterface $entity, AccountInterface $user = NULL): AccessResultInterface; + public function userAccessEntity(string $operation, EntityInterface $entity, AccountInterface $user = NULL): AccessResultInterface; /** * Checks access for entity operations on group content entities. @@ -89,7 +89,7 @@ public function userAccessEntity($operation, EntityInterface $entity, AccountInt * * @see \Drupal\og\PermissionManager::getEntityOperationPermissions() */ - public function userAccessGroupContentEntityOperation($operation, EntityInterface $group_entity, EntityInterface $group_content_entity, AccountInterface $user = NULL): AccessResultInterface; + public function userAccessGroupContentEntityOperation(string $operation, EntityInterface $group_entity, EntityInterface $group_content_entity, AccountInterface $user = NULL): AccessResultInterface; /** * Resets the static cache. From e27ef9980ef6ec7526eefef4f88130229115de0c Mon Sep 17 00:00:00 2001 From: Pieter Frenssen Date: Wed, 29 Jul 2020 17:02:38 +0300 Subject: [PATCH 4/7] Split up ::userAccessEntity() in two methods. This method was previously handling both user permissions and entity operations but this was confusing and problematic. It is now split into two separate methods with a clear scope: one is handling user permissions and the other handles entity operations. --- og.module | 2 +- src/OgAccess.php | 51 ++++++++++++++----- src/OgAccessInterface.php | 38 +++++++++++++- .../OgGroupContentOperationAccessTest.php | 2 +- 4 files changed, 77 insertions(+), 16 deletions(-) diff --git a/og.module b/og.module index 0dfda6370..cfb2791c4 100755 --- a/og.module +++ b/og.module @@ -154,7 +154,7 @@ function og_entity_access(EntityInterface $entity, $operation, AccountInterface } /** @var \Drupal\Core\Access\AccessResult $access */ - $access = \Drupal::service('og.access')->userAccessEntity($operation, $entity, $account); + $access = \Drupal::service('og.access')->userAccessEntityOperation($operation, $entity, $account); if ($access->isAllowed()) { return $access; diff --git a/src/OgAccess.php b/src/OgAccess.php index 86acfe476..0d23a4d41 100644 --- a/src/OgAccess.php +++ b/src/OgAccess.php @@ -230,16 +230,6 @@ public function userAccessEntity(string $permission, EntityInterface $entity, Ac $forbidden = AccessResult::forbidden()->addCacheTags($cache_tags); foreach ($groups as $entity_groups) { foreach ($entity_groups as $group) { - // Check if the operation matches a group content entity operation - // such as 'create article content'. - $operation_access = $this->userAccessGroupContentEntityOperation($operation, $group, $entity, $user); - - if ($operation_access->isAllowed()) { - return $operation_access->addCacheTags($cache_tags); - } - - // Check if the operation matches a group level operation such as - // 'subscribe without approval'. $user_access = $this->userAccess($group, $permission, $user); if ($user_access->isAllowed()) { return $user_access->addCacheTags($cache_tags); @@ -254,8 +244,45 @@ public function userAccessEntity(string $permission, EntityInterface $entity, Ac $result->addCacheTags($cache_tags); } - // Either the user didn't have permission, or the entity might be an - // orphaned group content. + // Either the user didn't have permission, or the entity might be orphaned + // group content. + return $result; + } + + /** + * {@inheritdoc} + */ + public function userAccessEntityOperation(string $operation, EntityInterface $entity, AccountInterface $user = NULL): AccessResultInterface { + $result = AccessResult::neutral(); + + $entity_type = $entity->getEntityType(); + $entity_type_id = $entity_type->id(); + $bundle = $entity->bundle(); + + if ($this->groupTypeManager->isGroupContent($entity_type_id, $bundle)) { + $cache_tags = $entity_type->getListCacheTags(); + + // The entity might be a user or a non-user entity. + $groups = $entity instanceof UserInterface ? $this->membershipManager->getUserGroups($entity->id()) : $this->membershipManager->getGroups($entity); + + if ($groups) { + $forbidden = AccessResult::forbidden()->addCacheTags($cache_tags); + foreach ($groups as $entity_groups) { + foreach ($entity_groups as $group) { + $operation_access = $this->userAccessGroupContentEntityOperation($operation, $group, $entity, $user); + if ($operation_access->isAllowed()) { + return $operation_access->addCacheTags($cache_tags); + } + } + } + return $forbidden; + } + + $result->addCacheTags($cache_tags); + } + + // Either the user didn't have permission, or the entity might be orphaned + // group content. return $result; } diff --git a/src/OgAccessInterface.php b/src/OgAccessInterface.php index 3f3b6fca9..88190fef4 100644 --- a/src/OgAccessInterface.php +++ b/src/OgAccessInterface.php @@ -50,7 +50,41 @@ interface OgAccessInterface { public function userAccess(EntityInterface $group, string $permission, AccountInterface $user = NULL, bool $skip_alter = FALSE): AccessResultInterface; /** - * Check if a user has access to a permission on a certain entity context. + * Determines whether a user has a group permission in a given entity. + * + * This does an exhaustive, but slow, check to discover whether access can be + * granted and works both on groups and group content. It will iterate over + * all groups that are associated with the entity and do a permission check on + * each group. If a passed in entity is both a group and group content, it + * will return a positive result if the user has the requested permission in + * either the entity itself or its parent group(s). + * + * In case you know the specific group you want to check access for then it is + * recommended to use the faster ::userAccess(). + * + * @param string $permission + * The name of the OG permission being checked. This includes both group + * level permissions such as 'subscribe without approval' and group content + * entity operation permissions such as 'edit own article content'. + * @param \Drupal\Core\Entity\EntityInterface $entity + * The entity object. This can be either a group or group content entity. + * @param \Drupal\Core\Session\AccountInterface $user + * (optional) The user object. If empty the current user will be used. + * + * @return \Drupal\Core\Access\AccessResultInterface + * An access result object. + */ + public function userAccessEntity(string $permission, EntityInterface $entity, AccountInterface $user = NULL): AccessResultInterface; + + /** + * Checks whether a user can perform an operation on a group content entity. + * + * This does an exhaustive, but slow, check to discover whether the operation + * can be performed. It will iterate over all groups that are associated with + * the group content entity and do an operation check on each group. + * + * In case you know the specific group you want to check access for then it is + * recommended to use the faster ::userAccessGroupContentEntityOperation(). * * @param string $operation * The operation to perform on the entity. @@ -62,7 +96,7 @@ public function userAccess(EntityInterface $group, string $permission, AccountIn * @return \Drupal\Core\Access\AccessResultInterface * An access result object. */ - public function userAccessEntity(string $operation, EntityInterface $entity, AccountInterface $user = NULL): AccessResultInterface; + public function userAccessEntityOperation(string $operation, EntityInterface $entity, AccountInterface $user = NULL): AccessResultInterface; /** * Checks access for entity operations on group content entities. diff --git a/tests/src/Kernel/Access/OgGroupContentOperationAccessTest.php b/tests/src/Kernel/Access/OgGroupContentOperationAccessTest.php index 79a5b4e5d..4c794cc3d 100644 --- a/tests/src/Kernel/Access/OgGroupContentOperationAccessTest.php +++ b/tests/src/Kernel/Access/OgGroupContentOperationAccessTest.php @@ -267,7 +267,7 @@ public function testAccess($group_content_bundle_id, $expected_access_matrix) { // group owner. $entity = $ownership === 'own' ? $this->groupContent[$group_content_bundle_id][$user_id] : $this->groupContent[$group_content_bundle_id]['group_owner']; $user = $this->users[$user_id]; - $this->assertEquals($expected_access, $og_access->userAccessEntity($operation, $entity, $user)->isAllowed(), "Operation: $operation, ownership: $ownership, user: $user_id, bundle: $group_content_bundle_id"); + $this->assertEquals($expected_access, $og_access->userAccessEntityOperation($operation, $entity, $user)->isAllowed(), "Operation: $operation, ownership: $ownership, user: $user_id, bundle: $group_content_bundle_id"); } } } From 489ca3610e5f9c6faaac82b362e1c5a6d189b041 Mon Sep 17 00:00:00 2001 From: Pieter Frenssen Date: Wed, 29 Jul 2020 17:05:03 +0300 Subject: [PATCH 5/7] Update documentation. --- src/OgAccessInterface.php | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/src/OgAccessInterface.php b/src/OgAccessInterface.php index 88190fef4..ba3e63c83 100644 --- a/src/OgAccessInterface.php +++ b/src/OgAccessInterface.php @@ -14,7 +14,7 @@ interface OgAccessInterface { /** - * Determines whether a user has a certain permission in a given group. + * Determines whether a user has a group permission in a given group. * * The following conditions will result in a positive result: * - The user is the global super user (UID 1). @@ -35,7 +35,9 @@ interface OgAccessInterface { * @param \Drupal\Core\Entity\EntityInterface $group * The group entity. * @param string $permission - * The permission being checked. + * The name of the OG permission being checked. This includes both group + * level permissions such as 'subscribe without approval' and group content + * entity operation permissions such as 'edit own article content'. * @param \Drupal\Core\Session\AccountInterface $user * (optional) The user to check. Defaults to the current user. * @param bool $skip_alter @@ -87,9 +89,9 @@ public function userAccessEntity(string $permission, EntityInterface $entity, Ac * recommended to use the faster ::userAccessGroupContentEntityOperation(). * * @param string $operation - * The operation to perform on the entity. + * The entity operation, such as "create", "update" or "delete". * @param \Drupal\Core\Entity\EntityInterface $entity - * The entity object. + * The group content entity. * @param \Drupal\Core\Session\AccountInterface $user * (optional) The user object. If empty the current user will be used. * @@ -99,16 +101,14 @@ public function userAccessEntity(string $permission, EntityInterface $entity, Ac public function userAccessEntityOperation(string $operation, EntityInterface $entity, AccountInterface $user = NULL): AccessResultInterface; /** - * Checks access for entity operations on group content entities. + * Checks access for entity operations on group content in a specific group. * * This checks if the user has permission to perform the requested operation * on the given group content entity according to the user's membership status - * in the given group. There is no formal support for access control on entity - * operations in core, so the mapping of permissions to operations is provided - * by PermissionManager::getEntityOperationPermissions(). + * in the given group. * * @param string $operation - * The entity operation. + * The entity operation, such as "create", "update" or "delete". * @param \Drupal\Core\Entity\EntityInterface $group_entity * The group entity, to retrieve the permissions from. * @param \Drupal\Core\Entity\EntityInterface $group_content_entity @@ -120,8 +120,6 @@ public function userAccessEntityOperation(string $operation, EntityInterface $en * * @return \Drupal\Core\Access\AccessResultInterface * The access result object. - * - * @see \Drupal\og\PermissionManager::getEntityOperationPermissions() */ public function userAccessGroupContentEntityOperation(string $operation, EntityInterface $group_entity, EntityInterface $group_content_entity, AccountInterface $user = NULL): AccessResultInterface; From a35ca50682e69e608ce7215049c41f961fbb45c1 Mon Sep 17 00:00:00 2001 From: Pieter Frenssen Date: Thu, 30 Jul 2020 21:39:10 +0300 Subject: [PATCH 6/7] Restore access checks on entity operations for both groups and group content. This benefits og_entity_access() which relies on this to work for any entity. --- src/EventSubscriber/OgEventSubscriber.php | 2 +- src/OgAccess.php | 28 +++++++++++++++++++++++ src/OgAccessInterface.php | 13 +++++++---- 3 files changed, 38 insertions(+), 5 deletions(-) diff --git a/src/EventSubscriber/OgEventSubscriber.php b/src/EventSubscriber/OgEventSubscriber.php index 08b51bac3..516f3570a 100644 --- a/src/EventSubscriber/OgEventSubscriber.php +++ b/src/EventSubscriber/OgEventSubscriber.php @@ -88,7 +88,7 @@ public function provideDefaultOgPermissions(PermissionEventInterface $event) { new GroupPermission([ 'name' => OgAccess::UPDATE_GROUP_PERMISSION, 'title' => $this->t('Edit group'), - 'description' => $this->t('Edit the group. Note: This permission controls only node entity type groups.'), + 'description' => $this->t('Edit the group entity.'), 'default roles' => [OgRoleInterface::ADMINISTRATOR], ]), new GroupPermission([ diff --git a/src/OgAccess.php b/src/OgAccess.php index 0d23a4d41..8f6cb68c8 100644 --- a/src/OgAccess.php +++ b/src/OgAccess.php @@ -34,6 +34,13 @@ class OgAccess implements OgAccessInterface { */ const UPDATE_GROUP_PERMISSION = 'update group'; + /** + * Maps entity operations performed on groups to group level permissions. + */ + const OPERATION_GROUP_PERMISSION_MAPPING = [ + 'update' => self::UPDATE_GROUP_PERMISSION, + ]; + /** * The config factory. * @@ -259,6 +266,27 @@ public function userAccessEntityOperation(string $operation, EntityInterface $en $entity_type_id = $entity_type->id(); $bundle = $entity->bundle(); + if ($this->groupTypeManager->isGroup($entity_type_id, $bundle)) { + // We are performing an entity operation on a group entity. Map the + // operation to the corresponding group level permission. + if (array_key_exists($operation, self::OPERATION_GROUP_PERMISSION_MAPPING)) { + $permission = self::OPERATION_GROUP_PERMISSION_MAPPING[$operation]; + + $user_access = $this->userAccess($entity, $permission, $user); + if ($user_access->isAllowed()) { + return $user_access; + } + else { + // An entity can be a group and group content in the same time. The + // group permission check didn't allow access, but the user still + // might have access to perform the operation in group content + // context. So instead of returning a deny here, we set the result, + // that might change if an access is found. + $result = AccessResult::forbidden()->inheritCacheability($user_access); + } + } + } + if ($this->groupTypeManager->isGroupContent($entity_type_id, $bundle)) { $cache_tags = $entity_type->getListCacheTags(); diff --git a/src/OgAccessInterface.php b/src/OgAccessInterface.php index ba3e63c83..c3a58c42c 100644 --- a/src/OgAccessInterface.php +++ b/src/OgAccessInterface.php @@ -79,11 +79,16 @@ public function userAccess(EntityInterface $group, string $permission, AccountIn public function userAccessEntity(string $permission, EntityInterface $entity, AccountInterface $user = NULL): AccessResultInterface; /** - * Checks whether a user can perform an operation on a group content entity. + * Checks whether a user can perform an operation on a given entity. * * This does an exhaustive, but slow, check to discover whether the operation - * can be performed. It will iterate over all groups that are associated with - * the group content entity and do an operation check on each group. + * can be performed. It works both with groups and group content entities. It + * will iterate over all groups that are associated with the entity and check + * if the user is allowed to perform the entity operation on the group content + * entity according to their role within each group. If a passed in entity is + * both a group and group content, it will return a positive result if the + * user has permission to perform the operation in either the entity itself or + * one of its parent group(s). * * In case you know the specific group you want to check access for then it is * recommended to use the faster ::userAccessGroupContentEntityOperation(). @@ -91,7 +96,7 @@ public function userAccessEntity(string $permission, EntityInterface $entity, Ac * @param string $operation * The entity operation, such as "create", "update" or "delete". * @param \Drupal\Core\Entity\EntityInterface $entity - * The group content entity. + * The entity object. This can be either a group or group content entity. * @param \Drupal\Core\Session\AccountInterface $user * (optional) The user object. If empty the current user will be used. * From 34a02f0c227cc90344a7455a4eeee1f29a3f812a Mon Sep 17 00:00:00 2001 From: Pieter Frenssen Date: Sun, 2 Aug 2020 18:32:57 +0300 Subject: [PATCH 7/7] Add @see links to both methods that refer to faster alternatives in their documentation. --- src/OgAccessInterface.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/OgAccessInterface.php b/src/OgAccessInterface.php index c3a58c42c..59fb7d557 100644 --- a/src/OgAccessInterface.php +++ b/src/OgAccessInterface.php @@ -75,6 +75,8 @@ public function userAccess(EntityInterface $group, string $permission, AccountIn * * @return \Drupal\Core\Access\AccessResultInterface * An access result object. + * + * @see \Drupal\og\userAccess(); */ public function userAccessEntity(string $permission, EntityInterface $entity, AccountInterface $user = NULL): AccessResultInterface; @@ -102,6 +104,8 @@ public function userAccessEntity(string $permission, EntityInterface $entity, Ac * * @return \Drupal\Core\Access\AccessResultInterface * An access result object. + * + * @see \Drupal\og\userAccessGroupContentEntityOperation(); */ public function userAccessEntityOperation(string $operation, EntityInterface $entity, AccountInterface $user = NULL): AccessResultInterface;