Problem/Motivation

By not doing so, the filter prevents users from viewing content that they should be able to view on a site with no additional node access modules installed beyond core.

Steps to reproduce

Install standard profile
Add Content access: Access filter to admin/content view
Add user with Content editor role
Login as content editor and visit admin/content
Notice no content is shown

Proposed resolution

Check for implementations of hook_node_access before joining on access tables

Remaining tasks

Review
Commit

User interface changes

None

Introduced terminology

None

API changes

None

Data model changes

None

Release notes snippet

N/A

CommentFileSizeAuthor
#28 1822440-28-test-only.patch2.22 KBAnkit.Gupta
#17 1822440-17-test-only.patch9.11 KBmohit_aghera
#1 core-1822440-1.patch737 bytesdawehner
views-content-access-node-grants.patch712 bytesezra-g

Issue fork drupal-1822440

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

dawehner’s picture

dawehner
Primary language German
commented
Project: Views (for Drupal 7) » Drupal core
Version: 7.x-3.x-dev » 8.x-dev
Component: Code » node.module
Issue tags: +VDC
StatusFileSize
new core-1822440-1.patch737 bytes

This looks great.

I'm not sure what the proper way of getting patches from contrib to core would be, but i think for really simple patches it might make sense to forward-port them.

smira’s picture

smira commented

this applies cleanly, honestly not sure how to test what approach might be better ;)

jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented
Issue tags: +Needs tests

It is a bug it needs some test to show the bug.

jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented
Status: Needs review » Needs work

As per #3.

xjm’s picture

xjm
she/her
Primary language English
commented
Component: node.module » node system
Issue summary: View changes

(Merging "node system" and "node.module" components for 8.x; disregard.)

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

Bug reports should be targeted against the 8.6.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.8.x-dev

Drupal 8.6.x will not receive any further development aside from security fixes. Bug reports should be targeted against the 8.8.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.9.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

Bug reports should be targeted against the 8.9.x-dev branch from now on, and new development or disruptive changes should be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

catch’s picture

catch
he/him
Primary language English
commented
Issue tags: +Bug Smash Initiative

This is still valid.

For adding a test, it would probably need a test view with the filter applied + some nodes, and then check the generated query to make sure it doesn't join on the node_access table.

mohit_aghera’s picture

mohit_aghera commented
Assigned: Unassigned » mohit_aghera
mohit_aghera’s picture

mohit_aghera commented
Version: 8.9.x-dev » 9.3.x-dev
mohit_aghera’s picture

mohit_aghera commented
StatusFileSize
new 1822440-17-test-only.patch9.11 KB

Adding test-only patch.

mohit_aghera opened merge request !982

mohit_aghera’s picture

mohit_aghera commented
Assigned: mohit_aghera » Unassigned
Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 17: 1822440-17-test-only.patch, failed testing. View results

mohit_aghera’s picture

mohit_aghera commented
Status: Needs work » Needs review
catch’s picture

catch
he/him
Primary language English
commented

Test coverage looks good, couple of nits:

  1. +++ b/core/modules/views/tests/src/Functional/Plugin/AccessTest.php
@@ -64,6 +64,11 @@ protected function setUp($import_test_views = TRUE): void {
+
+    // Create Article node type.
+    $this->drupalCreateContentType(['type' => 'page', 'name' => 'Page']);
+    $this->drupalCreateNode();

    Article vs. page mismatch between the comment and the code. 

  2. +++ b/core/modules/views/tests/src/Functional/Plugin/AccessTest.php
@@ -110,4 +115,32 @@ public function testStaticAccessPlugin() {
+  /**
+   * Tests that node_access table is joined when node_grant hook is implemented.
+   */

    Maybe 'hook_node_grants()' instead of 'node_grant hook'.

mohit_aghera’s picture

mohit_aghera commented

Resolved the feedback on the PR.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

acbramley’s picture

acbramley commented
Status: Needs review » Needs work
Issue tags: -Needs tests +Needs reroll

Tests have been added and initial feedback addressed, however the MR is still against 9.3.x so will need to be rebased against 10.1.x (is there a tag for rebase vs reroll?)

The issue summary could use a bit of sprucing up but not a deal breaker.

Ankit.Gupta’s picture

Ankit.Gupta commented
Status: Needs work » Needs review
Issue tags: -Needs reroll
StatusFileSize
new 1822440-28-test-only.patch2.22 KB

Reroll the patch #17 with Drupal 10.1.x

mohit_aghera’s picture

mohit_aghera commented

Hi @Ankit.Gupta
We don't need to upload the test-only patch.
Current MR already has the patch and the test case.

Hi @acbramley
I have updated the destination branch for the PR
Will trigger the build to see how it goes.

Status: Needs review » Needs work

The last submitted patch, 28: 1822440-28-test-only.patch, failed testing. View results

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

acbramley’s picture

acbramley commented
Status: Needs work » Needs review

Rebased onto 11.x and tidied up the test a bit.

smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work

Can we add an issue summary?

acbramley’s picture

acbramley commented
Issue summary: View changes
Status: Needs work » Needs review
smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community
Issue tags: +Needs Review Queue Initiative 
1) Drupal\Tests\views\Functional\Plugin\AccessTest::testContentAccessFilter
Failed asserting that an array does not contain 'node_access'.
/builds/issue/drupal-1822440/core/modules/views/tests/src/Functional/Plugin/AccessTest.php:133
FAILURES!

Shows test coverage
Actual change seems pretty straight forward, no issue and pipeline is green. I did rebase because it was 200+ commits

  • catch committed cf7c02a6 on 11.2.x 
    Issue #1822440 by mohit_aghera, acbramley, smustgrave, dawehner, ezra-g...

  • catch committed ec2569fd on 11.x 
    Issue #1822440 by mohit_aghera, acbramley, smustgrave, dawehner, ezra-g...
catch’s picture

catch
he/him
Primary language English
commented
Version: 11.x-dev » 11.2.x-dev
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 11.x and cherry-picked to 11.2.x, thanks!

catch closed merge request !982

Status: Fixed » Closed (fixed)

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