Contrib.social

ValidReferenceConstraintValidator should not try to enforce data integrity on pre-existing references

Open on Drupal.org →
Created on 18 May 2018, almost 7 years ago
Updated 4 February 2023, about 2 years ago

Problem:

Nodes are not saving with inaccesible pre-existing referenced items.

Issue has been fixed once with the ticket: https://www.drupal.org/project/drupal/issues/2791269 → stating, "Allow saving pre-existing references to inaccessible items" means there should not be any validation check for the existing referenced items.

Still the node save was disallowed and throwing an exception : "This entity (node: 32) cannot be referenced."

On cheking deeper the solution contains a condition:

/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;
+          }

Accordingly, the clause !$existing_entities[$target_id]->access('view') is causing problem in my casue.

Steps to reproduce

  • Create a content type with an entity reference field, able to reference the same content type.
  • Give a test user access to edit all content of that type + "view own unpublished content".
  • Create an unpublished node using user "test user".
  • Create another node referencing the unpublished node. using user "test user"
  • As the test user, edit the second node. Notice the reference field says “- Restricted access - (1)”
  • Try saving the node, get an “This entity (node: 1) cannot be referenced.” error.

here, the condition is failing becasue the "test user" has "view access" for the referenced unpublished node ceated by himself.

It is not only the problem with "view own unpublished content" permisisons. When you have any other Node access module enabled like Domain Access or content modertion. The issue get more severe and requires a broader scope.

Becasue user will be editing a node shared on different domains consisting different doamins (assigned) entity references.

Required solutions

  1. The scope of the validation check should be defined.
    • When the items should be validated - pre-existing or new ?
    • Should validate existing items in case of deleted entites ?
    • What is overall requirement of the validations ?
  2. Remove the "view access" condition ? And add other conditions depending upon the anwser of above point.
🐛 Bug report
Status

Needs review

Version

10.1 ✨

Component
Entity reference  →

Last updated about 4 hours ago

No maintainer
Created by

🇮🇳India nghai Kochi

Live updates comments and jobs are added and updated live.
  • Needs subsystem maintainer review

    It is used to alert the maintainer(s) of a particular core subsystem that an issue significantly impacts their subsystem, and their signoff is needed (see the governance policy draft for more information). Also, if you use this tag, make sure the issue component is set to the correct subsystem. If an issue significantly impacts more than one subsystem, use needs framework manager review instead.

Sign in to follow issues

Merge Requests

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • Comment about 2 years ago →
    🇺🇸United States smustgrave

    Tagging for security implications just in case.

    Still needs subsystem maintainer review.

  • Status changed to Needs work about 2 years ago
  • Comment about 2 years ago →
    🇺🇸United States smustgrave

    Relooking at the issue summary and the proposed solutions have a number of questions? Do those need to be answered before reviewing? If they've been answered can the issue summary be updated please.

  • Comment over 1 year ago →
    🇮🇳India hemuvyas97

    Reroll of patch #25 for Drupal core 10.1.7

  • Comment about 1 year ago →
    🇮🇳India psingh10

    Rerolled the patch on Drupal 10.2.2

  • Comment 11 months ago →
    mrwhittaker

    Updated patch to prevent:
    TypeError: in_array(): Argument #2 ($haystack) must be of type array, null given in in_array() (regel 159 van /app/web/core/lib/Drupal/Core/Entity/Plugin/Validation/Constraint/ValidReferenceConstraintValidator.php).

  • Comment 9 months ago →
    🇺🇸United States pbabin

    I'm in Drupal 10.3.1 using Mercury Editor, Layout Paragraphs and Paragraphs.

    We are getting the following error when editing a node with the above setup with a particular paragraph type which is referencing an image (the image is not using a view to render - it is default).

    This entity (paragraph: [paragraph id #]) cannot be referenced.

    I found initially that I could clone the entity, delete the original, and then save the node. However, on the next edit the issue still remained.

    I tried the patch from 39 and could not apply it.

    I tried the patch from 38 and it addressed my specific issue.

  • Comment 8 months ago →
    🇺🇸United States nrogers

    Rerolled #38 for 10.3.x branch (it also applies to 11.x). There's something not quite right in #40 it's missing a variable name. I couldn't reproduce the TypeError either. I'll push this to an MR for 11.x as well.

  • Merge request !9210Close #2973863: Do not enforce data integrity on pre-existing entity references → (Open) created by nrogers
  • Pipeline finished with Success
    8 months ago
    Total: 541s
    #254013
  • Comment 6 months ago →
    🇬🇧United Kingdom Dubs

    There is an additional problem here - if we're working with TRANSLATED entities, the existing entity IDs need to be added from all the translations, otherwise the integrity violation still gets triggered. I've added another patch (apologies - haven't figured out how to contribute to an existing PR yet).

  • Comment 6 months ago →
    🇬🇧United Kingdom Dubs

    There is an additional problem here - if we're working with TRANSLATED entities, the existing entity IDs need to be added from all the translations, otherwise the integrity violation still gets triggered. I've added another patch (apologies - haven't figured out how to contribute to an existing PR yet).

  • Comment 2 months ago →
    🇩🇪Germany harkonn

    I tested the patch from #45 in drupal 10.4.1 and it works like desired.
    Non user-1 users can now successfully save published content that references entities that got unpublished in the lifecycle of the content without needing to unreference these items.

  • Comment about 2 months ago →
    🇩🇪Germany harkonn

    There was a bug with #45. Adding references to empty fields caused
    Warning: Undefined variable $existing_target_ids in Drupal\Core\Entity\Plugin\Validation\Constraint\ValidReferenceConstraintValidator->validate() (Line 140 in project\docroot\core\lib\Drupal\Core\Entity\Plugin\Validation\Constraint\ValidReferenceConstraintValidator.php)

    I fixed this bug by initialising the variable properly and i commited these changes to the issue fork / merge request.

  • Comment about 1 month ago →
    🇮🇳India manish-31

    Fixed the bug mentioned in #47 from the patch #45 (for 10.4)

contrib.social Blog FAQ Discussions
Production build 0.71.5 2024