When calling node_access_rebuild() all nodes will return 403, no matter the configuration.
The problem exists because the function calls deleteGrants(), which removes permissions from all nodes, and afterwards uses entityQuery('node') to get all nodes that need their permissions rebuild. Because entityQuery has accessCheck set to default this will always return an empty array of nids.


$access_control_handler->deleteGrants();
...

$entity_query = \Drupal::entityQuery('node');
$entity_query->sort('nid', 'DESC');

This can easily be fixed by adding one line to the $entity_query:

$entity_query->accessCheck(false);

Comments

mgoedecke created an issue. See original summary.

mgoedecke’s picture

StatusFileSize
new560 bytes

following patch fixes the issue:

mgoedecke’s picture

Status: Active » Needs review
cilefen’s picture

Thank you for the report and the patch. Can you please check the bug priorities to be sure this is Critical?

mgoedecke’s picture

Thanks for your quick response. I think it depends on how you interpret "Render the system unusable and have no workaround.".

Currently if anyone uses a "node_access module" and rebuilds the permissions, all nodes will have access denied.
Without adding the applied patch I see no option to restore the permissions.

On the other hand, as it is already broken in 8.0.3 this issue would not prevent a new core release, so maybe it is rather a major than a critical?

cilefen’s picture

It could prevent a release as such. Let us see what others say about it.

mgoedecke’s picture

alright, feel free to change the prio if you see fit :-)

berdir’s picture

Status: Needs review » Needs work
Issue tags: -Node access, -node_access_rebuild +Needs tests

Seems legit, fix is correct.

But... how are there no tests failing in core? We definitely need test coverage for this.

Also, note that the issue tags field should only be used for specific, meaningful tags (e.g. various Needs something tags like the one I'm adding. Do not add anything if your not sure about it.

berdir’s picture

Status: Needs work » Needs review
StatusFileSize
new1.05 KB
new2.4 KB

Ok, confirmed, we are indeed missing test coverage.

Added that, also added a comment, also added the same to the batch query.

Note that the reason we're missing test coverage is that this only happens when the current user doesn't have the bypass node access permission, which I think is the common case for when this happens. Kind of surprised that you can even click on that link without that permission.

The last submitted patch, 9: node-access-rebuild-2671916-9-test-only.patch, failed testing.

jibran’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs tests

I think this is ready.

swentel’s picture

Yeah, that link has permission 'access administration pages' - not sure what or if anything else would be better though - could be a discussion, but not here.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

I've confirmed that in Drupal 7 the node access rebuild task also only requires 'access administration pages' and it is not a bug in Drupal 7 because node_access_rebuild() just queries the node table directly.

Committed dbf84c4 and pushed to 8.0.x and 8.1.x. Thanks!

  • alexpott committed d83a774 on 8.1.x
    Issue #2671916 by Berdir, mgoedecke: node_access_rebuild() will never...

  • alexpott committed dbf84c4 on 8.0.x
    Issue #2671916 by Berdir, mgoedecke: node_access_rebuild() will never...
mgoedecke’s picture

wow, that was fast! Thanks everybody

Status: Fixed » Closed (fixed)

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