Problem/Motivation

The "Upgrade Status"-module says the following for several places within the LDAP submodules:

Relying on entity queries to check access by default is deprecated in drupal:9.2.0 and an error will be thrown from drupal:10.0.0. Call \Drupal\Core\Entity\Query\QueryInterface::accessCheck() with TRUE or FALSE to specify whether access should be checked.

Steps to reproduce

Validate the LDAP-module with "Upgrade Status" and read the report.

Proposed resolution

Add explicit accessCheck()-calls to the entity queries. The following places were detected:

  1. ldap_authentication/src/AuthenticationServers.php 64
  2. ldap_authentication/src/Controller/LoginValidatorBase.php 548
  3. ldap_authentication/src/Form/LdapAuthenticationAdminForm.php 87
  4. ldap_authorization/src/Plugin/authorization/Provider/LDAPAuthorizationProvider.php 117
  5. ldap_query/src/Form/QueryEntityForm.php 51
  6. ldap_servers/ldap_servers.install 61
  7. ldap_servers/ldap_servers.install 98
  8. ldap_servers/ldap_servers.install 147
  9. ldap_servers/src/Form/DebuggingReviewForm.php 150
  10. ldap_servers/src/Form/DebuggingReviewForm.php 165
  11. ldap_servers/src/Form/DebuggingReviewForm.php 180
  12. ldap_servers/src/LdapUserManager.php 164
  13. ldap_user/src/Form/LdapUserAdminForm.php 77
  14. ldap_user/src/Form/LdapUserAdminForm.php 230
  15. ldap_user/src/Processor/OrphanProcessor.php 136

Remaining tasks

  1. Add accessCheck() everywhere.
  2. Review the changes.

User interface changes

none

API changes

none

Data model changes

none

CommentFileSizeAuthor
#4 accessCheck-3380653-4.patch10.01 KBlars.stiebenz
#2 accessCheck-3380653-2.patch8.28 KBdineshkumarbollu

Issue fork ldap-3380653

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:

About issue forks

Comments

lars.stiebenz created an issue. See original summary.

dineshkumarbollu’s picture

dineshkumarbollu commented
StatusFileSize
new accessCheck-3380653-2.patch8.28 KB

provided patch for this please review.

lars.stiebenz opened merge request !40

lars.stiebenz’s picture

lars.stiebenz commented
Issue summary: View changes
Status: Active » Needs review
StatusFileSize
new accessCheck-3380653-4.patch10.01 KB

My approach is basically the same as #3380653-2: Entity queries need an explicit accessCheck()-call in Drupal 10: The accessChecks are all set to FALSE since all the queries are used either on admin pages or in background processes.

Additionally I removed some queries entirely. At several places the query was used to get the IDs of all entities (mostly server configurations) and the result was only used as a parameter for loadMultiple(). Since loadMultiple() without a parameter loads all entities too, the query before should be unnecessary.

Or in code:
Old:

$servers = $storage->getQuery()->execute();
foreach ($storage->loadMultiple($servers) as $server) {
  // ...
}

New:

foreach ($storage->loadMultiple() as $server) {
  // ...
}

"New" should give the same result as "Old" but with 1 query less.

bluegeek9 made their first commit to this issue’s fork.

bluegeek9 opened merge request !43

  • bluegeek9 committed f3929e9f on 8.x-4.x 
    Issue #3380653 by lars.stiebenz: Entity queries need an explicit...
bluegeek9’s picture

bluegeek9 commented
Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.