Follow-up for #1801304: Add Entity reference field
Currently we check only node_access()

CommentFileSizeAuthor
#11 1847600-11.patch1.92 KBamateescu
#1 1847600.patch1.68 KBamateescu
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

amateescu’s picture

Title: Use entity_access() in Entity-reference formatter » Use proper entity 'view' access checks in entity reference
Component: field system » entity_reference.module
Assigned: amitaibu » Unassigned
Status: Active » Postponed
FileSize
1.68 KB

This change is really small, but unfortunately postponed on #1862750: Implement entity access API for nodes.

Berdir’s picture

+++ b/core/modules/entity_reference/entity_reference.moduleundefined
@@ -509,9 +509,7 @@ function entity_reference_autocomplete_callback_get_matches($type, $field, $inst
-    if (!$entity || !$entity_access) {
+    if (!$entity || !$entity->access('view')) {
       return MENU_ACCESS_DENIED;

Not related to this but MENU_ACCESS_DENIED does not exist anymore this should throw an AccessDeniedException().

Damien Tournoud’s picture

Category: task » bug
amateescu’s picture

Status: Postponed » Needs review

The node access patch has been committed, now should be a good time to do this.

Berdir’s picture

#1: 1847600.patch queued for re-testing.

Berdir’s picture

Issue tags: +Entity Access

Tagging.

amateescu’s picture

Not related to this but MENU_ACCESS_DENIED does not exist anymore this should throw an AccessDeniedException().

That's being fixed in #1801356: Entity reference autocomplete using routes.

Other than that, this patch is a no-brainer.. RTBC anyone? :)

Berdir’s picture

comments have not been converted to entity access API, so they would get broken by this, in case someone would attempt to do something that crazy ;)

Also, if that other issue fixes that, it will conflict with this change. Will RTBC once that one is in and this re-rolled.

Berdir’s picture

Issue tags: -Entity Access

#1: 1847600.patch queued for re-testing.

Status: Needs review » Needs work
Issue tags: +Entity Access

The last submitted patch, 1847600.patch, failed testing.

amateescu’s picture

Status: Needs work » Needs review
FileSize
1.92 KB

Rerolled.

Berdir’s picture

Status: Needs review » Reviewed & tested by the community

Yes, patch context looks better now :)

With the current code, the access defaults to TRUE if they are not nodes. The default entity access controller however defaults to FALSE.

This means that references to entity types that do not provide an entity access controller will no longer work. Which right now in core means comments and files.

That might or might not block this from being committed right now, but the code changes here are fine and will not require additional changes, so setting to RTBC.

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Well, that seems simple enough.

Committed and pushed to 8.x. Thanks!

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