Problem/Motivation

#2852116: Implement access control handler for library items is going to introduce an access control handler for paragraphs_library_item entity type. The basic idea is to forward the access check to the referenced entity/paragraph.
The problem with this approach appears in ParagraphAccessControllerHandler::checkAccess() as it tries to check the access of the parent entity. In this case, the parent entity is paragraphs_library_item.

Proposed resolution

As a (quick) fix, ParagraphAccessControllerHandler::checkAccess() should skip the parent access check for paragraphs_library_item entities and fallback to neutral access?

Remaining tasks

User interface changes

API changes

Data model changes

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mbovan created an issue. See original summary.

mbovan’s picture

Skipping parent's entity access check for library items.

Berdir’s picture

Status: Needs review » Needs work
+++ b/src/ParagraphAccessControlHandler.php
@@ -24,9 +24,15 @@ class ParagraphAccessControlHandler extends EntityAccessControlHandler {
+      $access_result = AccessResult::allowedIf($operation != 'view' || $paragraph->status->value);
+      // Library items have no support for parent's entity access check.
+      if ($paragraph->getParentEntity()->getEntityTypeId() == 'paragraphs_library_item') {
+        return $access_result;
+      }
+      else {
+        $parent_access = $paragraph->getParentEntity()->access($operation, $account, TRUE);
+      }
+      return $access_result->andIf($parent_access);
     }

the logic here is strange, in one case you return and then in else define an object that use after the if.

You can inver this with creating $access_result, if (not library) { call and add parent check} and then return.

Also, either the patch is drunk or we have a dead return line below that can be removed?

mbovan’s picture

Refactored the code.

Also, either the patch is drunk or we have a dead return line below that can be removed?

This applies when there is no parent entity.

Berdir’s picture

Status: Needs review » Reviewed & tested by the community
+++ b/src/ParagraphAccessControlHandler.php
@@ -20,15 +20,18 @@ class ParagraphAccessControlHandler extends EntityAccessControlHandler {
+      // Library items have no support for parent's entity access check.
+      if ($paragraph->getParentEntity()->getEntityTypeId() != 'paragraphs_library_item') {

I think that's just "for parent entity access check" (ing?). The parent is the entity, parent's entity sounds like the entity of the parent.

Can be fixed on commit.

mbovan’s picture

Status: Reviewed & tested by the community » Needs review
FileSize
1.53 KB
890 bytes

Right, fixed the comment in here.

RTBC can still apply, but changing to "Needs review" as I can't RTBC myself. :)

Berdir’s picture

Status: Needs review » Reviewed & tested by the community

  • miro_dietiker committed 88a5201 on 8.x-1.x authored by mbovan
    Issue #2854223 by mbovan: Do not check parent access for library items
    
miro_dietiker’s picture

Status: Reviewed & tested by the community » Fixed

Ugly, but committed. :-)

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.