Come together with the global Drupal community in Rotterdam, 28 Sept – 1 Oct 2026. Sessions, contribution, connection, and Early Bird savings until 8 June.Problem/Motivation
Followup from #2461049: Node module permissions are broken if hook_node_grants is implemented.
The node grant/access record system has a behavior where if no hook_node_grant() implementations are defined and therefore no modules provide node access records for specific nodes, a global "view all grant" is written instead. Having this default behavior buried inside the storage is already confusing, but the documentation for it is already pretty thin. The NodeGrantDatabaseStorageInterface::writeDefault()method docs say:
Creates the default node access grant entry.
...So that's nice but what is the "default node access grant entry"?
There is also no documentation inside the main implementation of NodeGrantDatabaseStorage::writeDefault() about what the "default grant" is (i.e., fall back to a view access grant for all).
Also see
Furthermore, none of the classes and interfaces involved in node access belong to the node access documentation topic, so the default behavior is not discoverable or clear in the "big picture" there. This is important because "view access for all but no edit nor delete access" is only the default behavior when (a) the "bypass node access" permission is not granted (b) the "view published content" permission is granted (c) no hook_entity_access() or hook_node_access() implementations already allowed or denied access for the operation (d) the node is not unpublished.
Finally, this logic seems somewhat like it should be part of the node access control handler (as a conceptual default behavior), and the storage implementation should just be... a storage implementation. #2461049: Node module permissions are broken if hook_node_grants is implemented adds the logic as actual code, but then it's in two places. Maybe there should be a grantDefaultAccess() method or something that we factor out?
Proposed resolution
Improve the NodeGrantDatabaseStorage::writeDefault() method docblock and the NodeAccessControlHandlerInterface::writeDefaultGrant() documentation.Add inline documentation to- Not done, this would be repeating the docblock commentsNodeGrantDatabaseStorage::writeDefault()explaining what logic the storage implementation is implementing.- Clarify the default grant behavior in the node access group documentation.
Decide whether we should factor out the logic of the default behavior into its own method (or something).- Not done, let's leave it where it is. We already have weird duplication in the access control handler.
Related: #2473123: Add node grant classes and interfaces to the node access topic
Remaining tasks
TBD
User interface changes
N/A
API changes
TBD
Postponed until
#2461049: Node module permissions are broken if hook_node_grants is implemented
Issue fork drupal-2473093
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:
- 2473093-node-access-default
changes, plain diff MR !11918
Comments
Comment #1
xjmComment #2
xjmSeparated out #2473123: Add node grant classes and interfaces to the node access topic.
Comment #3
arla commentedAlso add
@ingroup node_accessto NodeGrantsDatabaseStorage(Interface), NodeAccessControlHandler (and more)?Edit: Nevermind, that is exactly what the related issue in the previous post is about.
Comment #4
xjmComment #11
moshe weitzman commentedI had to wade into this code recently. I agree with the diagnosis and proposal here.
Comment #12
bkosborneSame as #11
Comment #20
acbramley commentedStill seems to be applicable.
Comment #22
acbramley commentedAdded some basic context to the
NodeGrantDatabaseStorageInterface::writeDefaultdoc block, and linked to that fromNodeAccessControlHandlerInterface::writeDefaultGrant(personally I'd love to remove this duplication but that's for another issue).I don't think we need inline documentation as it should be covered by the doc block.
Last bit I'm not sure on is this:
Where is the node access group documentation? Maybe that no longer exists. We have quite a lot of docs at the top of https://git.drupalcode.org/project/drupal/-/blob/11.x/core/modules/node/..., maybe that's sufficient?
Comment #23
smustgrave commentedSeems pretty straight forward
Comment #24
alexpottCommitted and pushed e255c27dfcd to 11.x and 2adf694fd35 to 11.2.x. Thanks!
Backported to 11.2.x as docs only change.
Comment #27
xjmHOLY CRAP!
Amazing to see this fixed a decade later. Thanks everyone!