Allow saving pre-existing references to inaccessible items

There are good reasons why saving a form with an entity reference to an entity that the current user doesn't have access to is not allowed, so I'd recommend against making this the default behaviour.

I think a reasonable implementation would be to add a specific permission to grant access to doing this, since you are giving the user an elevated access. Also, if we explicitly add a permission for this, it means that we can leave the referenced entity visible for the user (with the current "restricted access" text) and just alter the permissions check to allow the form submission.

Ah, that's completely reasonable. Disregard my previous comment.

We've hit this problem in Entityqueue 7.x as well and we had to create a workaround for it: #2383903: Cannot save queues with entities user doesn't have access to

I would argue that this is actually a bug in Entity reference, which shouldn't try to validate pre-existing items. Here's a patch with a test.

Some tests don't bother to create the parent entity object but that's easy to fix, we just need an additional check.

This is related to #2807437: Anonymous user cannot upload private file, although that issue is not about existing references, but a valid (IMHO) use case for selectively allowing new references to inaccessible entities.

Needs a re-roll because of the change to short array syntax in core #2776975: March 3, 2017: Convert core to array syntax coding standards for Drupal 8.3.x RC phase. Here's a new patch and a diff.

Interdiff doesn't seem to be able to cope with comparing the old with the new array syntax, so I've had to make do with a simple diff.

Sure thing, that's a good idea :)

+++ b/core/lib/Drupal/Core/Entity/Plugin/Validation/Constraint/ValidReferenceConstraintValidator.php
@@ -117,6 +117,20 @@ public function validate($value, Constraint $constraint) {
+      if ($value->getParent() && ($entity = $value->getEntity()) && !$entity->isNew()) {
+        $existing_entity = $this->entityTypeManager->getStorage($entity->getEntityTypeId())->loadUnchanged($entity->id());
+        $referenced_entities = $existing_entity->{$value->getFieldDefinition()->getName()}->referencedEntities();
+
+        foreach ($referenced_entities as $referenced_entity) {
+          if (($key = array_search($referenced_entity->id(), $target_ids)) !== FALSE) {
+            unset($target_ids[$key]);
+          }
+        }

Wondering if we should make this specific to entities that exist but you as a user don't have access to?

Not quite sure how, to be honest, but this logic I think would also pass validation for use cases such as:

* No longer allowed based on the configuration, e.g. wrong bundle
* entity that was deleted (not quite sure what the current behavior there is)

@Berdir, you are totally right!

I wrote new test cases for both of those scenarios, reference no longer allowed based on the configuration and entity that was deleted, and the test-only patch is the code from #13 with that additional test coverage.

+++ b/core/lib/Drupal/Core/Entity/Plugin/Validation/Constraint/ValidReferenceConstraintValidator.php
@@ -119,32 +119,31 @@ public function validate($value, Constraint $constraint) {
+          // Check if any of the invalid existing references are simply not
+          // accessible by the user, in which case they need to be excluded from
+          // validation
+          if (isset($previously_referenced_ids[$target_id]) && isset($existing_entities[$target_id]) && !$existing_entities[$target_id]->access('view')) {
+            continue;
+          }

Not sure how view vs view label operations apply here exactly. We currently use query tag based access for those entities, where we so far never talked about how that works combined with view label. Just wondering if there's possibly a weird/special case here, what happens when you have a user reference field and you don't have permission to view user profiles? Is there a case where that could result in the wrong result?

Just wondering if there's possibly a weird/special case here, what happens when you have a user reference field and you don't have permission to view user profiles? Is there a case where that could result in the wrong result?

Well, we have to keep in mind that all this is about letting the user save *pre-existing* inaccessible references, so I chose the 'view' operation especially because its scope is broader than 'view label'.

The expectation is that you are allowed to save a pre-existing reference to a user when you don't have access to view user profiles.

Edit: fixed typo.

Fair enough, lets do this, I think this is well tested.

+++ b/core/tests/Drupal/KernelTests/Core/Entity/ValidReferenceConstraintValidatorTest.php
@@ -66,4 +83,152 @@ public function testValidation() {
+    // of that bundle ca not be referenced anymore.

s/ca/can.

Fixed that on commit. Committed/pushed to 8.5.x and cherry-picked to 8.4.x. Thanks!

Dear @Berdir, Dear @amateescu

caught some issues related to the current solution for the requirement "Allow saving pre-existing references to inaccessible items"

have opened and reported the whole scenario in this ticket https://www.drupal.org/project/drupal/issues/2973863?

Could you please review once ?

Thanks

See also #3008943: Clean up todo in InlineBlockEntityOperations::handleEntityDelete(), introduced by this issue.

This patch doesn't solve the following case:
1-Create a user reference field ALLOWING ONLY users with a given role
2-Add a user (having the requested role) to this field and save the node
3-block the user
4-As a non admin, edit the node and try to save it. It will be impossible (error: this entity (user:xxx) can not be referenced)