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);
| Comment | File | Size | Author |
|---|---|---|---|
| #9 | node-access-rebuild-2671916-9.patch | 2.4 KB | berdir |
| #9 | node-access-rebuild-2671916-9-test-only.patch | 1.05 KB | berdir |
| #2 | fix_node_access_rebuild-2671916-1.patch | 560 bytes | mgoedecke |
Comments
Comment #2
mgoedecke commentedfollowing patch fixes the issue:
Comment #3
mgoedecke commentedComment #4
cilefen commentedThank you for the report and the patch. Can you please check the bug priorities to be sure this is Critical?
Comment #5
mgoedecke commentedThanks 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?
Comment #6
cilefen commentedIt could prevent a release as such. Let us see what others say about it.
Comment #7
mgoedecke commentedalright, feel free to change the prio if you see fit :-)
Comment #8
berdirSeems 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.
Comment #9
berdirOk, 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.
Comment #11
jibranI think this is ready.
Comment #12
swentel commentedYeah, 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.
Comment #13
alexpottI'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 becausenode_access_rebuild()just queries the node table directly.Committed dbf84c4 and pushed to 8.0.x and 8.1.x. Thanks!
Comment #16
mgoedecke commentedwow, that was fast! Thanks everybody