Problem/Motivation

As of commit 44d92d7e, included in LDAP 8.x-4.0.0, the ldap_user submodule has an implicit dependency on ldap_authentication, not listed in ldap_user.info.yml, which can cause a fatal error.

In ldap_user/src/Plugin/Validation/Constraint/LdapProtectedUserFieldConstraintValidator.php (added in commit 44d92d7e), this ldap_user function relies on ldap_authentication existing:

public static function create(ContainerInterface $container): LdapProtectedUserFieldConstraintValidator {
  $plugin = parent::create($container);
  $plugin->setLoginValidator($container->get('ldap_authentication.login_validator'));
  return $plugin;
}

That requires the ldap_authentication.login_validator service exists, but in our situation it does not exist (we use ldap_user to sync some info to Drupal, but do not use LDAP for authentication). In this scenario, trying to edit a user results in a fatal error:
Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException: You have requested a non-existent service "ldap_authentication.login_validator". in Drupal\Component\DependencyInjection\Container->get()

Steps to reproduce

  1. Enable ldap_user, but do not enable ldap_authentication
  2. Edit a user, and click save

Proposed resolution

I do not know the intricacies of #2928091: Current password check incorrect in user edit form well enough so this is probably not the perfect fix, but I think ldap_user.module:ldap_user_entity_base_field_info_alter() should at least be modified to fence the new constraint with a \Drupal::service('module_handler')->moduleExists('ldap_authentication') check.

Remaining tasks

Hopefully someone knowledgeable about #2928091: Current password check incorrect in user edit form can weigh in on the fix.

Comments

jay.dansand created an issue. See original summary.

jay.dansand’s picture

Simple patch that fences the new constraint, allowing us to edit users again.

grahl’s picture

Status: Active » Needs review

  • grahl committed 08d3fff on 8.x-4.x authored by jay.dansand
    Issue #3239938 by jay.dansand: ldap_user added implicit dependency on...
grahl’s picture

Status: Needs review » Fixed

Given all other options this is probably the best solution for now, adding the patch.

There is probably an argument to be made to critically review if those classes shouldn't be part of ldap_authentication but there is also overlapping functionality in DrupalUserProcessor and the events being handled in ldap_user which should really go into ldap_authentication to have less coupling. The way we are reacting to login, user creation, etc. makes that not really easy to do.

Status: Fixed » Closed (fixed)

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