Restructure node access grant behavior into the node access handler [#2473041] | Drupal.org

Restructure node access grant behavior into the node access handler [#2473041] | Drupal.org
Skip to search
Can we use first and third party cookies and web beacons to
understand our audience, and to tailor promotions you see
Restructure node access grant behavior into the node access handler
Fixed
Project:
Drupal core
Version:
11.x-dev
Component:
node system
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Issue tags:
Node access
D8 Accelerate Dev Days
Reporter:
xjm
Created:
17 Apr 2015 at 09:05 UTC
Updated:
23 Apr 2026 at 03:13 UTC
Jump to comment:
Most recent
Problem/Motivation
There is a good bit of spaghetti code happening in the node access API. In particular, the declaration of grants within realms and node access record implementations is tightly coupled -- i.e. node access module usually will need both
hook_node_grants()
and
hook_node_access_records()
. However, the actual functionality is still scattered between
node.module
(from D7 and before) and the Drupal 8 node and entity access control API implementation in
NodeAccessControlHandler
. In particular, the invocation of
hook_node_grants()
is in one place (in
node_access_grants()
) while the invocation of
hook_node_access_records()
is in the other.
This leads to a situation where it's difficult to find everything a developer needs to "know" to understand the node access system in D8.
Proposed resolution
Move the
node_access_grants
to the
NodeAccessControlHandler
and deprecate the function call.
As a followup, check in on the initiative to make a general 'entity access API' and see if it would be appropriate to rework the node access API into a plugin based API rather than using hooks before that initiative completes.
Remaining tasks
Write the code change and the change record.
User interface changes
None.
API changes
TBD.
Issue fork
drupal-2473041
Show commands
Start within a Git clone of the project using the
version control instructions
Add & fetch this issue fork’s repository
Or,
if you do not have
SSH keys set up on git.drupalcode.org
Add & fetch this issue fork’s repository
2 hidden branches
2473041-11.x
plain diff
MR
!15537
Check out this branch for the first time
Check out existing branch, if you already have it locally
2473041-restructure-node-access
plain diff
MR
!14702
Check out this branch for the first time
Check out existing branch, if you already have it locally
About issue forks
Comments
Comment
#1
xjm
she/her
commented
17 April 2015 at 10:49
Issue tags:
D8 Accelerate Dev Days
or
to post comments
Comment
#2
dpi
Perth, Australia
commented
28 January 2016 at 21:15
Component:
number.module
» node system
or
to post comments
Comment
#3
xjm
she/her
commented
25 February 2016 at 15:51
Status:
Postponed
» Active
Thanks @dpi.
or
to post comments
Comment
#4
25 February 2016 at 15:51
Version:
8.1.x-dev
» 8.2.x-dev
Drupal 8.1.0-beta1
was released on March 2, 2016, which means new developments and disruptive changes should now 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
or
to post comments
Comment
#5
25 February 2016 at 15:51
Version:
8.2.x-dev
» 8.3.x-dev
Drupal 8.2.0-beta1
was released on August 3, 2016, which means new developments and disruptive changes should now 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
or
to post comments
Comment
#6
25 February 2016 at 15:51
Version:
8.3.x-dev
» 8.4.x-dev
Drupal 8.3.0-alpha1
will be released the week of January 30, 2017, which means new developments and disruptive changes should now 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
or
to post comments
Comment
#7
25 February 2016 at 15:51
Version:
8.4.x-dev
» 8.5.x-dev
Drupal 8.4.0-alpha1
will be released the week of July 31, 2017, which means new developments and disruptive changes should now 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
or
to post comments
Comment
#8
25 February 2016 at 15:51
Version:
8.5.x-dev
» 8.6.x-dev
Drupal 8.5.0-alpha1
will be released the week of January 17, 2018, which means new developments and disruptive changes should now 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
or
to post comments
Comment
#9
25 February 2016 at 15:51
Version:
8.6.x-dev
» 8.7.x-dev
Drupal 8.6.0-alpha1
will be released the week of July 16, 2018, which means new developments and disruptive changes should now 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
or
to post comments
Comment
#10
25 February 2016 at 15:51
Version:
8.7.x-dev
» 8.8.x-dev
Drupal 8.7.0-alpha1
will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the
Drupal 8 minor version schedule
and the
Allowed changes during the Drupal 8 release cycle
or
to post comments
Comment
#11
25 February 2016 at 15:51
Version:
8.8.x-dev
» 8.9.x-dev
Drupal 8.8.0-alpha1
will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the
Drupal 8 and 9 minor version schedule
and the
Allowed changes during the Drupal 8 and 9 release cycles
or
to post comments
Comment
#12
25 February 2016 at 15:51
Version:
8.9.x-dev
» 9.1.x-dev
Drupal 8.9.0-beta1
was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now 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
or
to post comments
Comment
#13
25 February 2016 at 15:51
Version:
9.1.x-dev
» 9.2.x-dev
Drupal 9.1.0-alpha1
will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the
Drupal 9 minor version schedule
and the
Allowed changes during the Drupal 9 release cycle
or
to post comments
Comment
#14
25 February 2016 at 15:51
Version:
9.2.x-dev
» 9.3.x-dev
Drupal 9.2.0-alpha1
will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the
Drupal core minor version schedule
and the
Allowed changes during the Drupal core release cycle
or
to post comments
Comment
#15
25 February 2016 at 15:51
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
or
to post comments
Comment
#16
25 February 2016 at 15:51
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
or
to post comments
Comment
#17
joey-santiago
commented
19 August 2022 at 11:58
It would be so nice having this going forward. The hook_node_grants is giving me so much pain these days.
Maybe i should open a new issue complaining about this bug i found out, but to be honest, i don't have ideas on how that should be solved and at the moment i think this could be the proper place where to make sure this bug is taken care of.
Bug Explanation:
install drupal and translation modules, make the status field translatable on a certain content type
create a new node of that type, leaving the original node unpublished. Translate it in one language, publish the translation
visit the nodes as anonymous user. You'll be able to access the published translation and you'll get a 403 on the unpublished original translation
install a custom module which implements the hook_node_grants hook. It can be completely empty.
the permissions will be re-generated
now enjoy the bug: as anonymous you'll be able to access both nodes when you save the published translation. If you save the original node (unpublished), then you won't be able to access neither of the nodes
This happens because
NodeGrantDatabaseStorage::write
is put to work whenever a module implements the node_grants hook and at that point it doesn't take at all the status of the translation into account.
Is anyone pushing this forward? Should i try writing a patch for what i think is a problem in 9.4, or should i create a different issue for this?
or
to post comments
Comment
#18
19 August 2022 at 11:58
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
or
to post comments
Comment
#19
19 August 2022 at 11:58
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
or
to post comments
Comment
#20
acbramley
commented
20 March 2025 at 04:03
Triaged and this is still relevant.
or
to post comments
Comment
#21
acbramley
commented
25 June 2025 at 01:49
Parent issue:
#3532197: [meta] Deprecate node.module functions
or
to post comments
Comment
#22
acbramley
commented
30 June 2025 at 22:43
Title:
Deprecate node_access_grants() and move its functionality to NodeAccessControlHandler
» Deprecate node_access_grants()
Issue summary:
View changes
Status:
Active
» Postponed
Postponed on
#3038908: Deprecate node_access_view_all_nodes()
or
to post comments
Comment
#23
acbramley
commented
2 July 2025 at 01:57
Title:
Deprecate node_access_grants()
» Restructure node access API
Issue summary:
View changes
Issue tags:
Needs issue summary update
Parent issue:
#3532197: [meta] Deprecate node.module functions
Re-titling this back to a ticket about the node access API in general.
node_access_grants
the function is being deprecated in
#3038908: Deprecate node_access_view_all_nodes()
or
to post comments
Comment
#24
xjm
she/her
commented
3 July 2025 at 09:29
This rescope is too broad BTW. There is already an entire initiative to create an Entity Access API that should hopefully eventually replace the legacy, SQL-coupled node access grant and access record system.
This issue should remain scoped to refactoring the functionality of whatever replaces
node_access_grants()
into the access control handler.
@joey-santiago, if in any case you are still following for this, you should either search for or file a dedicated bug report. Thanks!
or
to post comments
Comment
#25
xjm
she/her
commented
3 July 2025 at 09:47
Title:
Restructure node access API
» Restructure node access grant behavior into the node access handler
or
to post comments
Comment
#26
xjm
she/her
commented
3 July 2025 at 09:56
Issue tags:
Node access
or
to post comments
Comment
#27
acbramley
commented
3 July 2025 at 22:27
This is being rescoped again based on
or
to post comments
Comment
#28
alexpott
he/they
🇪🇺🌍
commented
8 July 2025 at 13:55
Another thing to consider here is that @catch has pointed out there is a lot of complexity in \Drupal\node\Cache\NodeAccessGrantsCacheContext() that probably belongs in the API
or
to post comments
Comment
#29
acbramley
commented
6 October 2025 at 23:49
Status:
Postponed
» Active
The other issue is in, so this isn't postponed anymore.
or
to post comments
Comment
#30
6 October 2025 at 23:49
Version:
11.x-dev
» main
Drupal core is now using the
main
branch as the primary development branch. New developments and disruptive changes should now be targeted to the
main
branch.
Read more in the announcement
or
to post comments
Comment
#31
steven jones
commented
6 February 2026 at 10:19
Issue summary:
View changes
Issue tags:
D8 Accelerate Dev Days
, -
Needs issue summary update
I've cleaned up the issue summary, because actually
hook_node_grants()
does return
grants
within realms, and thus it's not an 'info hook'.
I've really narrowed the scope of the issue for the specific cleanup of essentially being able to make the ideological change from procedural code in the .module file to the exact same code, in a class. Which makes this task pretty straightforward, assuming that the
NodeAccessControlHandler
is the right place to put that code.
I've then punted further change into a follow-up, since there's no real point in changing how the node access API works if it's all going to get replaced with a generic entity access API...talking of which, does anyone know where that initiative is?
or
to post comments
Comment
#32
nicxvan
commented
7 February 2026 at 03:36
Issue tags:
D8 Accelerate Dev Days
Thank you for taking this! I'm interested in this as part of the module file elimination initiative.
Adding back the tag since we normally keeping historical tags like that.
or
to post comments
Comment
#33
acbramley
commented
8 February 2026 at 23:40
I feel like this issue should stay as the "restructure..." and we should just create a new issue to deprecate
node_access_grants
so things don't get confused.
or
to post comments
Comment
#34
steven jones
commented
9 February 2026 at 21:10
Assigned:
Unassigned
steven jones
Hmmm...but the original IS:
was actually mostly about moving the code and deprecating
node_access_grants
, with then massive bit for essentially changing the two hooks for gathering records and grants into a plugin. So I'd argue this
is
is the right issue to do the deprecation in, and then actually keep it laser focused on that, because it's not going to be trivial...
I took a stab at doing the deprecation, because the function is only called in 5 places in core, so how hard can it be, eh? Well two of the usages are easy to solve, but three of them are in the
NodeGrantDatabaseStorage
class...which a dependency of
NodeAccessControlHandler
which does make sense because they were two tightly coupled systems that were teased apart a little.
Logically it does make sense for the
node_access_grants
to get moved to
NodeAccessControlHandler
as proposed in the IS, as it's a high-level thing, and not concerned with the actual storage mechanisms etc.
I'm not sure how best to resolve this dependency cycle, seems like the storage is actually doing a decent amount more than simply storage. I suspect that those methods involved will need to accept an additional argument that then the high-level handler can pass in.
or
to post comments
Comment
#35
9 February 2026 at 21:27
steven jones
opened
merge request !14702
or
to post comments
Comment
#36
steven jones
commented
9 February 2026 at 21:44
Assigned:
steven jones
» Unassigned
Status:
Active
» Needs work
I've opened a draft PR to get the ball rolling on this one...I'm now wondering if the resolution on the dependencies is to introduce a third service, which could then be required by these two, and that service would actually do the hook invocation-ing, and thus its internals could eventually get replaced by a plugin based service etc. That might neatly tie everything up, as you'd have:
NodeAccessControlHandler
Helps integrate the node access control system into nodes
NodeGrantDatabaseStorage
Stores the access records / grants in the database and has helpers for altering queries etc.
NodeAccessHookBasedRecordsAndGrantsImplementationHandler
The new service that provides the single 'API' for node access control modules, as in, they really only need to implement a couple of hooks, nicely documented by this class, and they are done. They very specifically do not need to implement anything else. This is the class that in the future could switch to collecting plugins and calling methods on them.
Splitting things up might help them to be more maintainable and make it clearer to developers what a record, realm and grant are etc.
Anyway, I shall un-assign for now and I shall move it along if I have some more time.
or
to post comments
Comment
#37
acbramley
commented
9 February 2026 at 22:38
Hmmm...but the original IS:
was actually mostly about moving the code and deprecating node_access_grants
That was 11 years ago.
The main driver from the recent comments here is to get this method out of the .module file, and not get hung up on rearchitecture discussions. See this slack thread
. That is why I'm suggesting splitting the deprecation into a new issue.
I think a 3rd service is our only way forward here, we can't change the API of NodeGrantDatabaseStorage like you've got in that MR without massive BC issues, and as you say we'll have circular dependency issues if we add the method to the access control handler.
That was what we originally had in
#3038908: Deprecate node_access_view_all_nodes()
which went through several rescopes.
or
to post comments
Comment
#38
steven jones
commented
10 February 2026 at 09:20
Okay, so you want this issue to be about some sort of 'restructure' and then we spin out another ticket for the 'deprecation only'.
That's fine, happy to go with the flow.
But to be clear, the 'deprecation only' work will have to introduce a new service as mentioned above. You're happy with that, and it literally would only have the single method?
or
to post comments
Comment
#39
steven jones
commented
10 February 2026 at 21:50
Also...I did the refactor to introduce another service in the draft PR. Went for minimal changes rather than trying to dial in the right names for things, as obviously that can come later.
I'll leave it as Needs work while waiting on clarity of direction for this ticket and any spin-off.
or
to post comments
Comment
#40
nicxvan
commented
7 March 2026 at 04:33
This actually looks pretty close, I have a suggestion to convert to camel case.
We also do not need an interface for this, the service name can just be the class.
We also need a rebase due to the deprecation removals in main.
or
to post comments
Comment
#41
steven jones
commented
9 March 2026 at 16:16
Status:
Needs work
» Needs review
@nicxvan made those changes as requested. So setting to Needs review.
We'd need a CR too, and to update the code with the correct link too I assume?
There's still my question from
#38
where I'm asking about if continuing in this ticket is the right thing to do etc.?
or
to post comments
Comment
#42
nicxvan
commented
9 March 2026 at 16:48
I added the CR and some suggestions to update the CR links in the deprecation notices.
I think this is ready once those are applied and it's green.
I think this is fine to do here like this, then the follow up can address the api questions.
or
to post comments
Comment
#43
acbramley
commented
9 March 2026 at 23:42
Added some more minor feedback, I'm also not a huge fan of the name of the class/method NodeGrantsHelper::nodeAccessGrants is a bit strange IMO. I don't have any better suggestions at this time though so won't hold the issue up on it.
or
to post comments
Comment
#44
nicxvan
commented
9 March 2026 at 23:48
How about NodeGrants::grantAccess
Or NodeGrants::accessGrants
I added some suggestions, but if we change the name they will all need to be updated.
or
to post comments
Comment
#45
nicxvan
commented
13 March 2026 at 01:07
Status:
Needs review
» Needs work
Needs work for the suggestions that need applying, then I think it's ready.
or
to post comments
Comment
#46
steven jones
commented
13 March 2026 at 14:35
Status:
Needs work
» Needs review
Incorporated all the suggested changes, so moving back to Needs review.
or
to post comments
Comment
#47
nicxvan
commented
13 March 2026 at 15:00
Status:
Needs review
» Reviewed & tested by the community
I think this is ready.
I rebased due to the service autowire issue, the only change I made was:
Drupal\node\NodeGrantsHelper:
autowire: true
to
Drupal\node\NodeGrantsHelper: ~
So I think it's still ok to RTBC.
All feedback has been addressed.
We need some followups created for refactoring.
or
to post comments
Comment
#48
godotislate
he/him
commented
16 March 2026 at 19:25
Status:
Reviewed & tested by the community
» Needs work
Couple small suggetions on the MR.
or
to post comments
Comment
#49
acbramley
commented
16 March 2026 at 22:14
Status:
Needs work
» Reviewed & tested by the community
Applied all suggestions.
or
to post comments
Comment
#50
godotislate
he/him
commented
20 April 2026 at 23:57
Status:
Reviewed & tested by the community
» Needs work
I bumped the removal version of
node_access_grants()
to D13, since it is a "public" function and search_api at the very least uses it. I would go ahead and commit that, but I also just spotted that we're using property promotion on the new
NodeGrantsHelper
argument for the
NodeGrantDatabaseStorage
and
NodeAccessGrantsCacheContext
constructors. @berdir has pointed out in other similar issues that doing so makes the class property type nullable, and then making it not nullable later is a hassle, so let's assign the property the old way.
or
to post comments
Comment
#51
acbramley
commented
21 April 2026 at 00:02
Status:
Needs work
» Reviewed & tested by the community
making it not nullable later is a hassle, so let's assign the property the old way.
We've done this countless times, where is it coming up as a hassle?
I've made the changes, but IMO it would be a shame if we have to do this for every new constructor prop going forward.
or
to post comments
Comment
#52
godotislate
he/him
commented
22 April 2026 at 22:54
Version:
main
» 11.x-dev
Status:
Reviewed & tested by the community
» Patch (to be ported)
Committed
c21604f
and pushed to main. Thanks!
Needs a separate MR for 11.x, so setting to patch to be ported.
or
to post comments
Comment
#53
22 April 2026 at 22:57
godotislate
closed
merge request !14702
or
to post comments
Comment
#54
22 April 2026 at 23:00
godotislate
committed
c21604f8
on
main
refactor: #2473041 Restructure node access grant behavior into the node...
or
to post comments
Comment
#55
22 April 2026 at 23:06
acbramley
opened
merge request !15537
or
to post comments
Comment
#56
acbramley
commented
22 April 2026 at 23:06
Status:
Patch (to be ported)
» Needs review
cherry picked onto 11.x
or
to post comments
Comment
#57
godotislate
he/him
commented
23 April 2026 at 02:33
Status:
Needs review
» Reviewed & tested by the community
Added a couple changes to the service definitions in the node.services.yml because they aren't autowired by default in 11.x, but I think they're minor enough for me to RTBC.
If someone else can RTBC +1, I can commit.
or
to post comments
Comment
#58
acbramley
commented
23 April 2026 at 02:39
Thanks @godotislate those changes look good
or
to post comments
Comment
#59
nicxvan
commented
23 April 2026 at 02:44
+1 just compared them only difference is the services.
or
to post comments
Comment
#60
23 April 2026 at 03:01
godotislate
closed
merge request !15537
or
to post comments
Comment
#61
23 April 2026 at 03:01
godotislate
committed
26f41796
on
11.x
refactor: #2473041 Restructure node access grant behavior into the node...
or
to post comments
Comment
#62
godotislate
he/him
commented
23 April 2026 at 03:01
Status:
Reviewed & tested by the community
» Fixed
Committed
26f4179
and pushed to 11.x. Thanks!
or
to post comments
Comment
#63
23 April 2026 at 03:01
Now that this issue is closed,
review the
contribution record
As a contributor, attribute any organization that helped you, or if you volunteered your own time.
Maintainers, credit people who helped resolve this issue.
or
to post comments
Comment
#64
nicxvan
commented
23 April 2026 at 03:13
Related issues:
#3566536: [meta] eliminate core .module files
Thanks!
or
to post comments
Contribution record
Change record drafts
node_access_grants has been deprecated
Add child issue
clone issue
Related issues
Referenced by
#2461049: Node module permissions are broken if hook_node_grants is implemented
#3038908: Deprecate node_access_view_all_nodes()
Infrastructure management for Drupal.org provided by
Need a Drupal 7 extended support partner? Consider Tag1.
News items
News
Planet Drupal
Social media
Sign up for Drupal news
Security advisories
Jobs
Our community
Community
Services
Training
Hosting
Contributor guide
Groups & meetups
DrupalCon
Code of conduct
Documentation
Documentation
Drupal Guide
Drupal User Guide
Developer docs
API.Drupal.org
Drupal code base
Download & Extend
Drupal core
Modules
Themes
Distributions
Governance of community
About
Web accessibility
Drupal Association
About Drupal.org
Drupal is a
registered trademark
of
Dries Buytaert