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/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/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/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..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. * @@ -76,13 +83,6 @@ class OgAccess implements OgAccessInterface { */ protected $membershipManager; - /** - * The OG group audience helper. - * - * @var \Drupal\og\OgGroupAudienceHelperInterface - */ - protected $groupAudienceHelper; - /** * Constructs the OgAccess service. * @@ -98,17 +98,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; } /** @@ -208,7 +205,7 @@ public function userAccess(EntityInterface $group, string $permission, AccountIn /** * {@inheritdoc} */ - public function userAccessEntity($operation, EntityInterface $entity, AccountInterface $user = NULL): AccessResultInterface { + public function userAccessEntity(string $permission, EntityInterface $entity, AccountInterface $user = NULL): AccessResultInterface { $result = AccessResult::neutral(); $entity_type = $entity->getEntityType(); @@ -216,7 +213,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; } @@ -230,8 +227,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. @@ -241,17 +237,7 @@ public function userAccessEntity($operation, EntityInterface $entity, AccountInt $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, $operation, $user); + $user_access = $this->userAccess($group, $permission, $user); if ($user_access->isAllowed()) { return $user_access->addCacheTags($cache_tags); } @@ -265,15 +251,73 @@ public function userAccessEntity($operation, EntityInterface $entity, AccountInt $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->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(); + + // 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; } /** * {@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..59fb7d557 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 @@ -50,31 +52,72 @@ 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. + * + * @see \Drupal\og\userAccess(); + */ + public function userAccessEntity(string $permission, EntityInterface $entity, AccountInterface $user = NULL): AccessResultInterface; + + /** + * 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 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(). * * @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 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. + * + * @see \Drupal\og\userAccessGroupContentEntityOperation(); */ - public function userAccessEntity($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. + * 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 @@ -86,10 +129,8 @@ public function userAccessEntity($operation, EntityInterface $entity, AccountInt * * @return \Drupal\Core\Access\AccessResultInterface * The access result object. - * - * @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. 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"); } } } 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()); } 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());