If a module implementing node grants is enabled, and a non-standard operation is passed to check access to a node, then a database exception is thrown.

Problem/Motivation

The node grant system is not activated unless there is an installed module which implements hook_node_grants. Drupal does not come with any non-test modules implementing this hook.

Once the grant system is enabled, \Drupal\node\NodeGrantDatabaseStorage->access() constructs a database conditions' column by concatenating 'grant' . $operation. However only 'grant_view', 'grant_update', 'grant_delete' columns exist. The code does not do any pre-checks for column existence.

This code was uncovered due to a non-standard operation implemented by RNG, combined with a hook_node_grants implementer in content_access.

Steps to reproduce:

  1. Enable content_access module, or any module implementing node grants.
  2. Rebuild node access.
  3. Create a test script, executing
      $node->access('a_random_operation');
  4. As a limited user, run the test script.

Proposed resolution

The patch ensures the operation passed is one of the three grant columns in node_access database table.

Remaining tasks

N/A

User interface changes

None

API changes

None

Data model changes

None

References

- RNG issue: Possible problem with RNG and CONTENT ACCESS #71
- Content Access issue: #2653252: "The website encountered an unexpected error" upon setting Access Control for individual node

CommentFileSizeAuthor
#3 node-grant-operation-2659078.patch1.53 KBdpi
#3 node-grant-operation-2659078-testonly.patch708 bytesdpi
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dpi created an issue. See original summary.

dpi’s picture

dpi
Location Perth, Australia
CreditAttribution: dpi as a volunteer commented
Issue summary: View changes
dpi’s picture

dpi
Location Perth, Australia
CreditAttribution: dpi as a volunteer commented
Status: Active » Needs review
FileSize
node-grant-operation-2659078-testonly.patch708 bytes
node-grant-operation-2659078.patch1.53 KB
swentel’s picture

swentel CreditAttribution: swentel commented 
+++ b/core/modules/node/src/NodeGrantDatabaseStorage.php
@@ -65,6 +65,11 @@ public function __construct(Connection $database, ModuleHandlerInterface $module
+      return AccessResult::neutral();

Maybe we should be harder here and throw an exception ?

dpi’s picture

dpi
Location Perth, Australia
CreditAttribution: dpi as a volunteer commented

Maybe we should be harder here and throw an exception ?

Non-CRUD operation strings are permitted in Drupal, its just the grant system is not equipped to handle them.

I dont think this should fail hard, control should be returned to the entity access system on failure.

swentel’s picture

swentel CreditAttribution: swentel commented

Oh, ok, carry on then :)

The last submitted patch, 3: node-grant-operation-2659078-testonly.patch, failed testing.

platinum1’s picture

platinum1 CreditAttribution: platinum1 commented

The patch seems to do the trick. Thank you

platinum1’s picture

platinum1 CreditAttribution: platinum1 commented
Status: Needs review » Reviewed & tested by the community
catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.1.x and cherry-picked to 8.0.x. Thanks!

  • catch committed 7daedb3 on 8.1.x 
    Issue #2659078 by dpi: Unexpected access operation throws exception when...

  • catch committed 6b711d9 on 
    Issue #2659078 by dpi: Unexpected access operation throws exception when...

Status: Fixed » Closed (fixed)

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

dpi’s picture

dpi
Location Perth, Australia
CreditAttribution: dpi as a volunteer commented
Assigned: dpi » Unassigned