Problem/Motivation
The Drupal\ldap_authentication\Controller::processLogin() method is triggering multiple binds, (5+) during a login attempt.
This was compared side-by-side with the Drupal 7 implementation, which only triggers 3 binds. One from admin, then from the user, then from admin.
The issue with several binds is that it causes redundancy and unnecessary connections to the LDAP setup, which could potentially have negative affect on server load and availability.
Debug Report
For testing:
- I set up a quick LDAP kubernetes pod/service on a kube-adm environment.
- Generated 100 dummy users with passwords and loaded them into LDAP.
- Port-forwarded to the LDAP service and connected the instance to Drupal 7 and Drupal 10 environments.
- Hit LDAP on Drupal 7 and observed 3 binds consistently on each successful login attempt.
- Hit LDAP on Drupal 10 and observed multiple binds (5+) consistently on each successful login attempt. Breakpointed and stepped through the Drupal\ldap_authentication\Controller::processLogin() method and observed that the Drupal\ldap_servers\LdapBridge::bind() method gets called repeatedly on each step, causing unnecessary duplicate bind attempts.
Proposed resolution
Created a patch that uses drupal_static to store previous attempts with a set of credentials, and also stores the ldap object for later use, containing the successful (or unsuccessful) bind connection, for later use.
With this change in place, only 2 binds are made to the LDAP server on login. One pre-flight bind by admin for the record lookup, and a second for the user credential check bind.
Issue fork ldap-3527944
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
Comment #5
tgaugesI created a new branch 3527944-2-prevent-unnecessary-duplicate-bind-attempts with a new solution because the solution in 3527944-prevent-unnecessary-duplicate-bind-attempts would break when re-binding without changing the server. This happens when using
CredentialsStorageto bind with user credentials.Please take a look at the merge request :)
Comment #6
tgaugesI was wrong: An LDAP BIND is valid for the whole connection. It does not matter if another BIND happens in the same connection. So my merge request can be seen as alternative solution, but not really different in functionality.