- Issue created by @mxr576
- Merge request !230Stop filtering by node status when a node access module is enabled → (Open) created by mxr576
- 🇭🇺Hungary mxr576 Hungary
Made a first stab on this, tried to use atomic and detailed commits because some initial refactoring was necessary.
- 🇭🇺Hungary mxr576 Hungary
I've set up a test environment to verify the behavior before and after applying the patch. The environment consists of:
- A test user (user1) who is the author of 2 published and 2 unpublished nodes (articles and pages)
- Important: user1 did not create these nodes - they were created by uid1 who set user1 as the author
- user1 has no special permissions beyond standard authenticated user permissions
- A simple Views view that lists content from a search index with only the Content Access processor enabled (similar to search_api_db_defaults setup)
- The search_api_test module and its replacement introduced in this MR (search_api_test_access) were enabled with the state flag that enables the node access featureBefore the fix
As shown in the screenshots, before applying the patch, user1 could not see the unpublished nodes they authored in the search results. Interestingly, they could see comments attached to these unpublished nodes - creating an inconsistent user experience.
After the fix
After applying the patch, user1 can now see the unpublished nodes they authored in the search results, which aligns with Drupal core's node access handling.
- 🇦🇹Austria drunken monkey Vienna, Austria
drunken monkey → made their first commit to this issue’s fork.
- 🇦🇹Austria drunken monkey Vienna, Austria
Thanks for reporting this issue!
I was not aware of this change, but definitely seems like something we want to support, indeed. I was always a bit peeved that node access was so inconsistent and giving users access to some unpublished nodes always required a bit of magic.
Seems it didn’t really get more consistent, but the system at least now has clearer rules and less magic, which is also nice.However, needless to say that this is a very sensitive area and we definitely do not want to introduce a bug that would show visitors content to which they do not have access. And since the change was only introcuced in Drupal 11.2 I wonder if we don’t need to also add a version check to make sure we don’t apply this new logic pre-maturely, i.e., in versions of Drupal Core that don’t apply it themselves yet.
It seems like our current test coverage would not catch this problem, but we should make very sure to not introduce any security problems, even in edge cases.
More worryingly, the tests even seem to fail against Drupal 11.2 currently, so please check that out, too.In any case, thanks again!