diff --git a/src/Entity/GroupMembershipTrait.php b/src/Entity/GroupMembershipTrait.php index be918c65c093449f2ea354157b3d33b784b8b521..ad0339768ed59e7c8dc01bfb88730f715a0be8b4 100644 --- a/src/Entity/GroupMembershipTrait.php +++ b/src/Entity/GroupMembershipTrait.php @@ -4,14 +4,34 @@ namespace Drupal\group\Entity; use Drupal\Core\Cache\CacheBackendInterface; use Drupal\Core\Cache\CacheableMetadata; +use Drupal\Core\Entity\EntityMalformedException; +use Drupal\Core\Entity\EntityStorageInterface; use Drupal\Core\Session\AccountInterface; use Drupal\group\Entity\Storage\GroupRoleStorageInterface; +use Drupal\group\Plugin\Validation\Constraint\GroupMembershipRoles; /** * Functionality trait for a group_membership bundle class. */ trait GroupMembershipTrait { + /** + * {@inheritdoc} + */ + public function preSave(EntityStorageInterface $storage) { + parent::preSave($storage); + + // @todo 4.x.x Validate all constraints in parent preSave()?. + $violations = $this->validate(); + foreach ($violations as $violation) { + // To not break BC we only throw exceptions for our constraint. + if (!$violation->getConstraint() instanceof GroupMembershipRoles) { + continue; + } + throw new EntityMalformedException($violation->getMessage()); + } + } + /** * {@inheritdoc} */ @@ -32,16 +52,6 @@ trait GroupMembershipTrait { } } - // @todo Add the below two checks to a preSave() hook. - $storage = \Drupal::entityTypeManager()->getStorage('group_role'); - if (!$group_role = $storage->load($role_id)) { - throw new \InvalidArgumentException(sprintf('Could not add role with ID %s, role does not exist.', $role_id)); - } - assert($group_role instanceof GroupRoleInterface); - if ($group_role->getGroupTypeId() !== $this->getGroupTypeId()) { - throw new \InvalidArgumentException(sprintf('Could not add role with ID %s, role belongs to a different group type.', $role_id)); - } - $this->group_roles[] = $role_id; $this->save(); } diff --git a/src/Entity/GroupRelationship.php b/src/Entity/GroupRelationship.php index b77a601da357fc256a4716e77c5894b0865c0d86..06338494006f4fe268c0a88b7428e981d0be1663 100644 --- a/src/Entity/GroupRelationship.php +++ b/src/Entity/GroupRelationship.php @@ -70,7 +70,8 @@ use Drupal\user\EntityOwnerTrait; * field_ui_base_route = "entity.group_relationship_type.edit_form", * permission_granularity = "bundle", * constraints = { - * "GroupRelationshipCardinality" = {} + * "GroupRelationshipCardinality" = {}, + * "GroupMembershipRoles" = {} * } * ) */ diff --git a/src/Plugin/Validation/Constraint/GroupMembershipRoles.php b/src/Plugin/Validation/Constraint/GroupMembershipRoles.php new file mode 100644 index 0000000000000000000000000000000000000000..4bb544f5c9b568eddda9c142bcf18c3b98e68c00 --- /dev/null +++ b/src/Plugin/Validation/Constraint/GroupMembershipRoles.php @@ -0,0 +1,39 @@ +<?php + +namespace Drupal\group\Plugin\Validation\Constraint; + +use Symfony\Component\Validator\Constraint; + +/** + * Checks the roles assigned to a group membership. + * + * @Constraint( + * id = "GroupMembershipRoles", + * label = @Translation("Group membership roles check", context = "Validation"), + * type = "entity:group_relationship" + * ) + */ +class GroupMembershipRoles extends Constraint { + + /** + * When a role does not exist. + * + * @var string + */ + public $roleDoesNotExistMessage = 'Could not assign role with ID @role_id: Role does not exist.'; + + /** + * When a role belongs to different group type. + * + * @var string + */ + public $roleDifferentGroupTypeMessage = 'Could not assign role with ID @role_id: Role belongs to a different group type.'; + + /** + * When a role belongs to a non-individual scope. + * + * @var string + */ + public $roleNonIndividualScopeMessage = 'Could not assign role with ID @role_id: Role does not belong to the individual scope.'; + +} diff --git a/src/Plugin/Validation/Constraint/GroupMembershipRolesValidator.php b/src/Plugin/Validation/Constraint/GroupMembershipRolesValidator.php new file mode 100644 index 0000000000000000000000000000000000000000..291f128cd59b905ad46bf1f872a2cfa69250b883 --- /dev/null +++ b/src/Plugin/Validation/Constraint/GroupMembershipRolesValidator.php @@ -0,0 +1,82 @@ +<?php + +namespace Drupal\group\Plugin\Validation\Constraint; + +use Drupal\Core\DependencyInjection\ContainerInjectionInterface; +use Drupal\Core\Entity\EntityTypeManagerInterface; +use Drupal\Core\Field\Plugin\Field\FieldType\EntityReferenceItem; +use Drupal\group\Entity\GroupRelationshipInterface; +use Drupal\group\Entity\GroupRoleInterface; +use Drupal\group\Entity\Storage\GroupRoleStorageInterface; +use Drupal\group\PermissionScopeInterface; +use Symfony\Component\DependencyInjection\ContainerInterface; +use Symfony\Component\Validator\Constraint; +use Symfony\Component\Validator\ConstraintValidator; + +/** + * Checks whether a group membership's roles are valid. + */ +class GroupMembershipRolesValidator extends ConstraintValidator implements ContainerInjectionInterface { + + public function __construct(protected EntityTypeManagerInterface $entityTypeManager) {} + + /** + * {@inheritdoc} + */ + public static function create(ContainerInterface $container) { + return new static($container->get('entity_type.manager')); + } + + /** + * {@inheritdoc} + */ + public function validate(mixed $value, Constraint $constraint): void { + $group_relationship = $value; + assert($group_relationship instanceof GroupRelationshipInterface); + assert($constraint instanceof GroupMembershipRoles); + + // This constraint only applies to memberships. + if ($group_relationship->getPluginId() !== 'group_membership') { + return; + } + + // If someone for some reason removed the roles field, we can't do anything. + if (!$group_relationship->hasField('group_roles')) { + return; + } + + $group_role_storage = $this->entityTypeManager->getStorage('group_role'); + assert($group_role_storage instanceof GroupRoleStorageInterface); + + $existing_roles = $group_role_storage->loadMultiple(); + foreach ($group_relationship->get('group_roles') as $group_role_ref) { + assert($group_role_ref instanceof EntityReferenceItem); + $group_role_id = $group_role_ref->target_id; + + if (!array_key_exists($group_role_id, $existing_roles)) { + $this->context->buildViolation($constraint->roleDoesNotExistMessage) + ->setParameter('@role_id', $group_role_id) + ->atPath('group_roles') + ->addViolation(); + } + else { + assert($existing_roles[$group_role_id] instanceof GroupRoleInterface); + // @todo 4.x.x: This overlaps with ValidReferenceConstraint, remove when + // we validate all constraints on a group relationship in preSave(). + if ($existing_roles[$group_role_id]->getGroupTypeId() !== $group_relationship->getGroupTypeId()) { + $this->context->buildViolation($constraint->roleDifferentGroupTypeMessage) + ->setParameter('@role_id', $group_role_id) + ->atPath('group_roles') + ->addViolation(); + } + if ($existing_roles[$group_role_id]->getScope() !== PermissionScopeInterface::INDIVIDUAL_ID) { + $this->context->buildViolation($constraint->roleNonIndividualScopeMessage) + ->setParameter('@role_id', $group_role_id) + ->atPath('group_roles') + ->addViolation(); + } + } + } + } + +} diff --git a/tests/src/Functional/GroupTypeFormTest.php b/tests/src/Functional/GroupTypeFormTest.php index 9e39bce5317af9ec1ead60711e0467193ec0d1d6..0e8174a73430e5c578a610f5ab0c302e315f058d 100644 --- a/tests/src/Functional/GroupTypeFormTest.php +++ b/tests/src/Functional/GroupTypeFormTest.php @@ -2,6 +2,7 @@ namespace Drupal\Tests\group\Functional; +use Drupal\Core\Entity\EntityTypeBundleInfoInterface; use Drupal\group\GroupMembership; /** @@ -107,6 +108,14 @@ class GroupTypeFormTest extends GroupBrowserTestBase { ] + $this->commonValues; $this->submitForm($edit, $this->setUpAddFormAndGetSubmitButton()); + // The group creation below will trigger a new membership, which gets + // validated on pre-save. Because we saved the new group type in another + // request, the bundle info is out of date and aforementioned validation + // will fail. + $bundle_info = \Drupal::service('entity_type.bundle.info'); + assert($bundle_info instanceof EntityTypeBundleInfoInterface); + $bundle_info->clearCachedBundles(); + $group = $this->createGroup(['type' => $this->groupTypeId]); $this->assertEquals(\Drupal::currentUser()->id(), $group->getOwnerId()); $this->assertInstanceOf(GroupMembership::class, $group->getMember(\Drupal::currentUser())); @@ -175,6 +184,14 @@ class GroupTypeFormTest extends GroupBrowserTestBase { ] + $this->commonValues; $this->submitForm($edit, $this->setUpAddFormAndGetSubmitButton()); + // The group creation below will trigger a new membership, which gets + // validated on pre-save. Because we saved the new group type in another + // request, the bundle info is out of date and aforementioned validation + // will fail. + $bundle_info = \Drupal::service('entity_type.bundle.info'); + assert($bundle_info instanceof EntityTypeBundleInfoInterface); + $bundle_info->clearCachedBundles(); + $membership = $this->createGroup(['type' => $this->groupTypeId])->getMember(\Drupal::currentUser()); $ids = array_column($membership->getGroupRelationship()->get('group_roles')->getValue(), 'target_id'); $this->assertNotContains($this->groupTypeId . '-admin', $ids); @@ -195,6 +212,14 @@ class GroupTypeFormTest extends GroupBrowserTestBase { ] + $this->commonValues; $this->submitForm($edit, $this->setUpAddFormAndGetSubmitButton()); + // The group creation below will trigger a new membership, which gets + // validated on pre-save. Because we saved the new group type in another + // request, the bundle info is out of date and aforementioned validation + // will fail. + $bundle_info = \Drupal::service('entity_type.bundle.info'); + assert($bundle_info instanceof EntityTypeBundleInfoInterface); + $bundle_info->clearCachedBundles(); + $membership = $this->createGroup(['type' => $this->groupTypeId])->getMember(\Drupal::currentUser()); $ids = array_column($membership->getGroupRelationship()->get('group_roles')->getValue(), 'target_id'); $this->assertContains($this->groupTypeId . '-admin', $ids); diff --git a/tests/src/Kernel/GroupMembershipRolesTest.php b/tests/src/Kernel/GroupMembershipRolesTest.php new file mode 100644 index 0000000000000000000000000000000000000000..9a869618a4b1f2e1fe997ce57e733eda3f5f441a --- /dev/null +++ b/tests/src/Kernel/GroupMembershipRolesTest.php @@ -0,0 +1,100 @@ +<?php + +namespace Drupal\Tests\group\Kernel; + +use Drupal\Core\Entity\EntityConstraintViolationListInterface; +use Drupal\Core\StringTranslation\TranslatableMarkup; +use Drupal\group\Entity\Storage\GroupRelationshipStorageInterface; +use Drupal\group\Entity\Storage\GroupRelationshipTypeStorageInterface; +use Drupal\group\PermissionScopeInterface; +use Drupal\group\Plugin\Validation\Constraint\GroupMembershipRoles; +use Drupal\user\RoleInterface; + +/** + * Tests for the GroupMembershipRoles constraint. + * + * @group group + * + * @coversDefaultClass \Drupal\group\Plugin\Validation\Constraint\GroupMembershipRoles + */ +class GroupMembershipRolesTest extends GroupKernelTestBase { + + /** + * Tests the role validation. + * + * @covers ::validate + */ + public function testValidate(): void { + $grt_storage = $this->entityTypeManager->getStorage('group_relationship_type'); + assert($grt_storage instanceof GroupRelationshipTypeStorageInterface); + + $gr_storage = $this->entityTypeManager()->getStorage('group_relationship'); + assert($gr_storage instanceof GroupRelationshipStorageInterface); + + $group_type = $this->createGroupType(); + $group_type_id = $group_type->id(); + + $group_relationship = $gr_storage->create([ + 'type' => $grt_storage->getRelationshipTypeId($group_type_id, 'group_membership'), + 'gid' => $this->createGroup(['type' => $group_type_id])->id(), + 'entity_id' => $this->createUser()->id(), + 'group_roles' => [], + ]); + + // Verify the absence of violations in a newly created membership. + $violations = $group_relationship->validate(); + $this->filterViolations($violations); + $this->assertCount(0, $violations, 'A freshly created group membership has no violations.'); + + // Try to add a non-existent role to the relationship. + $group_relationship->set('group_roles', ['banana']); + + $violations = $group_relationship->validate(); + $this->filterViolations($violations); + $this->assertCount(1, $violations, 'Adding a non-existent role triggers a violation.'); + $message = new TranslatableMarkup('Could not assign role with ID banana: Role does not exist.'); + $this->assertEquals((string) $message, (string) $violations->get(0)->getMessage()); + + // Try to add a role from another group type. + $group_role_id = $this->createGroupRole([ + 'group_type' => $this->createGroupType()->id(), + 'scope' => PermissionScopeInterface::INDIVIDUAL_ID, + ])->id(); + $group_relationship->set('group_roles', [$group_role_id]); + + $violations = $group_relationship->validate(); + $this->filterViolations($violations); + $this->assertCount(1, $violations, 'Adding a role from another group type triggers a violation.'); + $message = new TranslatableMarkup('Could not assign role with ID @role_id: Role belongs to a different group type.', ['@role_id' => $group_role_id]); + $this->assertEquals((string) $message, (string) $violations->get(0)->getMessage()); + + // Try to add an insider role. + $group_role_id = $this->createGroupRole([ + 'group_type' => $group_type_id, + 'scope' => PermissionScopeInterface::INSIDER_ID, + 'global_role' => RoleInterface::AUTHENTICATED_ID, + ])->id(); + $group_relationship->set('group_roles', [$group_role_id]); + + $violations = $group_relationship->validate(); + $this->filterViolations($violations); + $this->assertCount(1, $violations, 'Adding an insider role triggers a violation.'); + $message = new TranslatableMarkup('Could not assign role with ID @role_id: Role does not belong to the individual scope.', ['@role_id' => $group_role_id]); + $this->assertEquals((string) $message, (string) $violations->get(0)->getMessage()); + } + + /** + * Filters out violations that did not come from our subject constraint. + * + * @param \Drupal\Core\Entity\EntityConstraintViolationListInterface $violations + * The list of violations to filter. + */ + protected function filterViolations(EntityConstraintViolationListInterface $violations): void { + foreach ($violations as $offset => $violation) { + if (!$violation->getConstraint() instanceof GroupMembershipRoles) { + $violations->offsetUnset($offset); + } + } + } + +} diff --git a/tests/src/Kernel/GroupRoleAssignedTest.php b/tests/src/Kernel/GroupRoleAssignedTest.php index 3e303706b3c3383e2967566e658236b7d268a632..660fd46ef749ae07ad5ad3fbfccea66a0e9fc38b 100644 --- a/tests/src/Kernel/GroupRoleAssignedTest.php +++ b/tests/src/Kernel/GroupRoleAssignedTest.php @@ -56,6 +56,8 @@ class GroupRoleAssignedTest extends GroupKernelTestBase { public function testSynchronizedRole(string $scope): void { $group_type_id_a = $this->createGroupType()->id(); $group_type_id_b = $this->createGroupType()->id(); + $group_type_id_c = $this->createGroupType()->id(); + $individual_role = ['scope' => PermissionScopeInterface::INDIVIDUAL_ID]; $synchronized_role = ['scope' => $scope, 'global_role' => RoleInterface::AUTHENTICATED_ID]; // Verify the absence of violations in a newly created role. @@ -63,8 +65,11 @@ class GroupRoleAssignedTest extends GroupKernelTestBase { $violations = $group_role->getTypedData()->validate(); $this->assertCount(0, $violations, 'A freshly created role has no violations.'); - // Assign the role to a member of a group. - $this->createGroup(['type' => $group_type_id_a])->addMember($this->createUser(), ['group_roles' => [$group_role->id()]]); + // Assign an individual role to a member and then change its scope. + $group_role = $this->createGroupRole($individual_role + ['group_type' => $group_type_id_b]); + $this->createGroup(['type' => $group_type_id_b])->addMember($this->createUser(), ['group_roles' => [$group_role->id()]]); + $group_role->set('scope', $scope); + $group_role->set('global_role', RoleInterface::AUTHENTICATED_ID); // Verify that there is now a violation on the group role. $violations = $group_role->getTypedData()->validate(); @@ -76,7 +81,7 @@ class GroupRoleAssignedTest extends GroupKernelTestBase { $this->assertEquals((string) $message, (string) $violations->get(0)->getMessage()); // Create another role and verify that it has no violations. - $violations = $this->createGroupRole($synchronized_role + ['group_type' => $group_type_id_b])->getTypedData()->validate(); + $violations = $this->createGroupRole($synchronized_role + ['group_type' => $group_type_id_c])->getTypedData()->validate(); $this->assertCount(0, $violations, 'Another freshly created role has no violations.'); }