Merge pull request #9678 from saschaszott/saschaszott-patch-2

LDAPAuthentication considers update of eperson's attributes

dspace-api/src/main/java/org/dspace/authenticate/LDAPAuthentication.java

@@ -17,6 +17,7 @@ import java.util.Collections;
 import java.util.Hashtable;
 import java.util.Iterator;
 import java.util.List;
+import java.util.Optional;
 import javax.naming.NamingEnumeration;
 import javax.naming.NamingException;
 import javax.naming.directory.Attribute;
@@ -68,12 +69,8 @@ import org.dspace.services.factory.DSpaceServicesFactory;
  * @author Ivan Masár
  * @author Michael Plate
  */
-public class LDAPAuthentication
+public class LDAPAuthentication implements AuthenticationMethod {
-    implements AuthenticationMethod {
 
-    /**
-     * log4j category
-     */
     private static final Logger log
             = org.apache.logging.log4j.LogManager.getLogger(LDAPAuthentication.class);
 
@@ -130,7 +127,7 @@ public class LDAPAuthentication
         return false;
     }
 
-    /*
+    /**
      * This is an explicit method.
      */
     @Override
@@ -138,7 +135,7 @@ public class LDAPAuthentication
         return false;
     }
 
-    /*
+    /**
      * Add authenticated users to the group defined in dspace.cfg by
      * the login.specialgroup key.
      */
@@ -177,7 +174,7 @@ public class LDAPAuthentication
         return Collections.EMPTY_LIST;
     }
 
-    /*
+    /**
      * Authenticate the given credentials.
      * This is the heart of the authentication method: test the
      * credentials for authenticity, and if accepted, attempt to match
@@ -187,7 +184,7 @@ public class LDAPAuthentication
      * @param context
      *  DSpace context, will be modified (ePerson set) upon success.
      *
-     * @param username
+     * @param netid
      *  Username (or email address) when method is explicit. Use null for
      *  implicit method.
      *
@@ -250,7 +247,7 @@ public class LDAPAuthentication
         }
 
         // Check a DN was found
-        if ((dn == null) || (dn.trim().equals(""))) {
+        if (StringUtils.isBlank(dn)) {
             log.info(LogHelper
                          .getHeader(context, "failed_login", "no DN found for user " + netid));
             return BAD_CREDENTIALS;
@@ -269,6 +266,18 @@ public class LDAPAuthentication
                 context.setCurrentUser(eperson);
                 request.setAttribute(LDAP_AUTHENTICATED, true);
 
+                // update eperson's attributes
+                context.turnOffAuthorisationSystem();
+                setEpersonAttributes(context, eperson, ldap, Optional.empty());
+                try {
+                    ePersonService.update(context, eperson);
+                    context.dispatchEvents();
+                } catch (AuthorizeException e) {
+                    log.warn("update of eperson " + eperson.getID()  + " failed", e);
+                } finally {
+                    context.restoreAuthSystemState();
+                }
+
                 // assign user to groups based on ldap dn
                 assignGroups(dn, ldap.ldapGroup, context);
 
@@ -313,14 +322,13 @@ public class LDAPAuthentication
                             log.info(LogHelper.getHeader(context,
                                                           "type=ldap-login", "type=ldap_but_already_email"));
                             context.turnOffAuthorisationSystem();
-                            eperson.setNetid(netid.toLowerCase());
+                            setEpersonAttributes(context, eperson, ldap, Optional.of(netid));
                             ePersonService.update(context, eperson);
                             context.dispatchEvents();
                             context.restoreAuthSystemState();
                             context.setCurrentUser(eperson);
                             request.setAttribute(LDAP_AUTHENTICATED, true);
 
-
                             // assign user to groups based on ldap dn
                             assignGroups(dn, ldap.ldapGroup, context);
 
@@ -331,20 +339,7 @@ public class LDAPAuthentication
                                 try {
                                     context.turnOffAuthorisationSystem();
                                     eperson = ePersonService.create(context);
-                                    if (StringUtils.isNotEmpty(email)) {
+                                    setEpersonAttributes(context, eperson, ldap, Optional.of(netid));
-                                        eperson.setEmail(email);
-                                    }
-                                    if (StringUtils.isNotEmpty(ldap.ldapGivenName)) {
-                                        eperson.setFirstName(context, ldap.ldapGivenName);
-                                    }
-                                    if (StringUtils.isNotEmpty(ldap.ldapSurname)) {
-                                        eperson.setLastName(context, ldap.ldapSurname);
-                                    }
-                                    if (StringUtils.isNotEmpty(ldap.ldapPhone)) {
-                                        ePersonService.setMetadataSingleValue(context, eperson,
-                                                MD_PHONE, ldap.ldapPhone, null);
-                                    }
-                                    eperson.setNetid(netid.toLowerCase());
                                     eperson.setCanLogIn(true);
                                     authenticationService.initEPerson(context, request, eperson);
                                     ePersonService.update(context, eperson);
@@ -382,6 +377,29 @@ public class LDAPAuthentication
         return BAD_ARGS;
     }
 
+    /**
+     * Update eperson's attributes
+     */
+    private void setEpersonAttributes(Context context, EPerson eperson, SpeakerToLDAP ldap, Optional<String> netid)
+        throws SQLException {
+
+        if (StringUtils.isNotEmpty(ldap.ldapEmail)) {
+            eperson.setEmail(ldap.ldapEmail);
+        }
+        if (StringUtils.isNotEmpty(ldap.ldapGivenName)) {
+            eperson.setFirstName(context, ldap.ldapGivenName);
+        }
+        if (StringUtils.isNotEmpty(ldap.ldapSurname)) {
+            eperson.setLastName(context, ldap.ldapSurname);
+        }
+        if (StringUtils.isNotEmpty(ldap.ldapPhone)) {
+            ePersonService.setMetadataSingleValue(context, eperson, MD_PHONE, ldap.ldapPhone, null);
+        }
+        if (netid.isPresent()) {
+            eperson.setNetid(netid.get().toLowerCase());
+        }
+    }
+
     /**
      * Internal class to manage LDAP query and results, mainly
      * because there are multiple values to return.
@@ -673,7 +691,7 @@ public class LDAPAuthentication
         }
     }
 
-    /*
+    /**
      * Returns the URL of an external login page which is not applicable for this authn method.
      *
      * Note: Prior to DSpace 7, this method return the page of login servlet.
@@ -701,7 +719,7 @@ public class LDAPAuthentication
         return "ldap";
     }
 
-    /*
+    /**
      * Add authenticated users to the group defined in dspace.cfg by
      * the authentication-ldap.login.groupmap.* key.
      *