Implement externalauth module for ldap_authentication

Some minor remarks/comments - happy for all of these to be follow up tasks

1. +++ b/ldap_user/ldap_user.install
   @@ -127,3 +77,30 @@ function ldap_user_update_8001() {
   +    drupal_set_message("Please install the <a href='https://drupal.org/project/externalauth'>External Auth</a> module.", "error");
   

   We should wrap this in t()

2. +++ b/ldap_user/ldap_user.install
   @@ -127,3 +77,30 @@ function ldap_user_update_8001() {
   +    // @TODO Throw an \Drupal\Core\Utility\UpdateException
   

   We should do that here too.

3. +++ b/ldap_user/ldap_user.install
   @@ -127,3 +77,30 @@ function ldap_user_update_8001() {
   +  foreach ( db_query("SELECT aid, uid, identifier FROM {ldap_user_identities}")->fetchAll() as $identity ) {
   +    // Load the user as the service expects an account.
   +    $account = user_load($identity->uid);
   +    $authmap = \Drupal::service('externalauth.authmap');
   +    $authmap->save($account, 'ldap_user', $identity->identifier);
   +
   +    // Delete the row if successful.
   +    db_query("DELETE FROM {ldap_user_identities} WHERE aid = :aid", array(':aid' => $identity->aid));
   +  }
   

   Should we do this in a batch (using the passed $sandbox argument) in case there are some sites with 1000s of users?

4. +++ b/ldap_user/ldap_user.install
   @@ -127,3 +77,30 @@ function ldap_user_update_8001() {
   +  // Drop the table if successful.
   +  db_query("DROP TABLE {ldap_user_identities}");
   

   I think we should do this in a separate update hook, just for clean separation - this is something @andypost taught me.

There is at least one functional error in patch #9 in ldap_user_get_identifier_from_map(), where $authname is declared but $authmap used.

+++ b/ldap_user/ldap_user.module
@@ -568,19 +568,21 @@ function ldap_user_grab_password_validate($form, FormState &$form_state) {
+  $authname = \Drupal::service('externalauth.authmap')->getAuthdata($uid, 'ldap_user');
+  return $authmap->authname;

This looks wrong

Sorry to reopen this but I think something obvious is still missing: ldap_user should have externalauth as a dependency so that this gets enabled automatically. The trivial patch for that is attached.

Additional patch needed since ldap_user_get_identifier_from_map() still gets called with uid instead of id().

Sorry, one more update but ldap_user_get_identifier_from_map() was calling its service incorrectly. Weird since all other functions did it correctly.

thanks

It looks like Provisioning Drupal accounts from LDAP is not functioning. A browse of the source makes me think this might be intentional, or at least somewhat known. Is there a plan/timeline for enabling this ?

ldap_user_get_identifier_from_map is not properly returning the authname.

see patch

LGTM