Problem/Motivation

On Drupal 10.0, executing node_get_recent(1); anywhere in your codebase returns a fatal error:

The website encountered an unexpected error. Please try again later.

Drupal\Core\Entity\Query\QueryException: Entity queries must explicitly set whether the query should be access checked or not. See Drupal\Core\Entity\Query\QueryInterface::accessCheck(). in Drupal\Core\Entity\Query\Sql\Query->prepare() (line 141 of core/lib/Drupal/Core/Entity/Query/Sql/Query.php).
Drupal\Core\Entity\Query\Sql\Query->execute() (Line: 713)
node_get_recent(1)

This is due to CR #3201242: Access checking must be explicitly specified on content entity queries, which is now enforced in Drupal 10 by throwing an exception if ::accessCheck() is not explicitly included in any content entity queries. To be honest, this is no surprise because issue #3356516: Deprecate node_get_recent() explains the situation succinctly:

node_get_recent() is unused and untested in core.

The fix in Drupal core would technically be a one-liner and might still make sense to try to get in (without tests?) while progress is made on #3356516 to remove the function completely in Drupal 11.

function node_get_recent($number = 10) {
  $account = \Drupal::currentUser();
  $query = \Drupal::entityQuery('node');
+  $query->accessCheck(FALSE);
  if (!$account->hasPermission('bypass node access')) {
    // If the user is able to view their own unpublished nodes, allow them
    // to see these in addition to published nodes. Check that they actually

Steps to reproduce

Proposed resolution

Add a call to ::accessCheck() to avoid the error. The person who authored the CR (mentioned above) seemed to suggest that ::accessCheck(FALSE) is what the default behavior implies. If you review the code of node_get_recent() it includes its own additional permissions logic, so I think FALSE is safe.

Remaining tasks

Write tests
Review

User interface changes

N/a

API changes

N/a

Data model changes

N/a

Release notes snippet

Issue fork drupal-3357368

Command icon 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:

Comments

jwilson3 created an issue. See original summary.

jwilson3’s picture

jwilson3’s picture

Title: Calls to node_get_recent() cause a fatal error and WSOD » EntityQuery Access Check in node_get_recent()
Related issues: +#2785449: It's too easy to write entity queries with access checks that must not have them

jwilson3’s picture

Status: Active » Needs review
jwilson3’s picture

Issue summary: View changes
smustgrave’s picture

Issue summary: View changes
Status: Needs review » Needs work
Issue tags: +Needs Review Queue Initiative, +Needs tests

Thank you for reporting.

Next step would be to write a test showing this issue.

catch’s picture

I think this might have been found and fixed already via phpstan in #3330981: Fix PHPStan L1 error "Relying on entity queries to check access by default is deprecated...". Please check on 10.1?

jwilson3’s picture

Status: Needs work » Closed (duplicate)

@catch yes. It appears you're correct that this has been fixed on D10.1 with commit from #3330981:

https://git.drupalcode.org/project/drupal/-/commit/c49ae937eab4115b6c3d6...

The actual fix was a 2-line code change and is slightly different than the 1-liner approach I'd made here, but logically equivalent since both work on the same $query variable. I speculate this was in order to pass the phpstan automated detections.

Also it looks like the decision was to go with ::accessCheck(TRUE), instead of FALSE that I suggested here. AFAICT this is fine.

Given there are no tests to prove how it works for different user perms one way or another, someone with a deeper use case than my own would have to appear and show that the logic is not working correctly.