Merge branch 'main' into task/main/DURACOM-288

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

@@ -17,6 +17,7 @@
 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 @@
  * @author Ivan Masár
  * @author Michael Plate
  */
-public class LDAPAuthentication
-    implements AuthenticationMethod {
+public class LDAPAuthentication implements AuthenticationMethod {
 
-    /**
-     * log4j category
-     */
     private static final Logger log
             = org.apache.logging.log4j.LogManager.getLogger(LDAPAuthentication.class);
 
@@ -130,15 +127,15 @@ public boolean allowSetPassword(Context context,
         return false;
     }
 
-    /*
+    /**
      * This is an explicit method.
      */
     @Override
     public boolean isImplicit() {
         return false;
     }
 
-    /*
+    /**
      * Add authenticated users to the group defined in dspace.cfg by
      * the login.specialgroup key.
      */
@@ -177,7 +174,7 @@ public List<Group> getSpecialGroups(Context context, HttpServletRequest request)
         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 List<Group> getSpecialGroups(Context context, HttpServletRequest request)
      * @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 int authenticate(Context context,
         }
 
         // 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 int authenticate(Context context,
                 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 int authenticate(Context context,
                             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 int authenticate(Context context,
                                 try {
                                     context.turnOffAuthorisationSystem();
                                     eperson = ePersonService.create(context);
-                                    if (StringUtils.isNotEmpty(email)) {
-                                        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());
+                                    setEpersonAttributes(context, eperson, ldap, Optional.of(netid));
                                     eperson.setCanLogIn(true);
                                     authenticationService.initEPerson(context, request, eperson);
                                     ePersonService.update(context, eperson);
@@ -382,6 +377,29 @@ public int authenticate(Context context,
         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.
@@ -503,6 +521,7 @@ protected String getDNOfUser(String adminUser, String adminPassword, Context con
                     } else {
                         searchName = ldap_provider_url + ldap_search_context;
                     }
+                    @SuppressWarnings("BanJNDI")
                     NamingEnumeration<SearchResult> answer = ctx.search(
                         searchName,
                         "(&({0}={1}))", new Object[] {ldap_id_field,
@@ -553,7 +572,7 @@ protected String getDNOfUser(String adminUser, String adminPassword, Context con
                             att = atts.get(attlist[4]);
                             if (att != null) {
                                 // loop through all groups returned by LDAP
-                                ldapGroup = new ArrayList<String>();
+                                ldapGroup = new ArrayList<>();
                                 for (NamingEnumeration val = att.getAll(); val.hasMoreElements(); )  {
                                     ldapGroup.add((String) val.next());
                                 }
@@ -633,7 +652,8 @@ protected boolean ldapAuthenticate(String netid, String password,
                         ctx.addToEnvironment(javax.naming.Context.AUTHORITATIVE, "true");
                         ctx.addToEnvironment(javax.naming.Context.REFERRAL, "follow");
                         // dummy operation to check if authentication has succeeded
-                        ctx.getAttributes("");
+                        @SuppressWarnings("BanJNDI")
+                        Attributes trash = ctx.getAttributes("");
                     } else if (!useTLS) {
                         // Authenticate
                         env.put(javax.naming.Context.SECURITY_AUTHENTICATION, "Simple");
@@ -671,7 +691,7 @@ protected boolean ldapAuthenticate(String netid, String password,
         }
     }
 
-    /*
+    /**
      * 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.
@@ -699,7 +719,7 @@ public String getName() {
         return "ldap";
     }
 
-    /*
+    /**
      * Add authenticated users to the group defined in dspace.cfg by
      * the authentication-ldap.login.groupmap.* key.
      *

dspace-api/src/main/java/org/dspace/curate/Curation.java

@@ -165,7 +165,7 @@ private long runQueue(TaskQueue queue, Curator curator) throws SQLException, Aut
      * End of curation script; logs script time if -v verbose is set
      *
      * @param timeRun Time script was started
-     * @throws SQLException If DSpace contextx can't complete
+     * @throws SQLException If DSpace context can't complete
      */
     private void endScript(long timeRun) throws SQLException {
         context.complete();
@@ -300,9 +300,17 @@ private void initGeneralLineOptionsAndCheckIfValid() {
         // scope
         if (this.commandLine.getOptionValue('s') != null) {
             this.scope = this.commandLine.getOptionValue('s');
-            if (this.scope != null && Curator.TxScope.valueOf(this.scope.toUpperCase()) == null) {
-                this.handler.logError("Bad transaction scope '" + this.scope + "': only 'object', 'curation' or " +
-                                      "'open' recognized");
+            boolean knownScope;
+            try {
+                Curator.TxScope.valueOf(this.scope.toUpperCase());
+                knownScope = true;
+            } catch (IllegalArgumentException | NullPointerException e) {
+                knownScope = false;
+            }
+            if (!knownScope) {
+                this.handler.logError("Bad transaction scope '"
+                        + this.scope
+                        + "': only 'object', 'curation' or 'open' recognized");
                 throw new IllegalArgumentException(
                     "Bad transaction scope '" + this.scope + "': only 'object', 'curation' or " +
                     "'open' recognized");

dspace-api/src/test/java/org/dspace/util/DSpaceKernelInitializer.java

@@ -83,6 +83,7 @@ public void initialize(final ConfigurableApplicationContext applicationContext)
      * Initially look for JNDI Resource called "java:/comp/env/dspace.dir".
      * If not found, use value provided in "dspace.dir" in Spring Environment
      */
+    @SuppressWarnings("BanJNDI")
     private String getDSpaceHome(ConfigurableEnvironment environment) {
         // Load the "dspace.dir" property from Spring's configuration.
         // This gives us the location of our DSpace configuration, which is

dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/DSpaceRestRepository.java

@@ -17,7 +17,6 @@
 
 import com.fasterxml.jackson.databind.JsonNode;
 import jakarta.servlet.http.HttpServletRequest;
-import org.apache.logging.log4j.Logger;
 import org.dspace.app.rest.exception.DSpaceBadRequestException;
 import org.dspace.app.rest.exception.RESTAuthorizationException;
 import org.dspace.app.rest.exception.RepositoryMethodNotImplementedException;
@@ -26,7 +25,6 @@
 import org.dspace.app.rest.model.RestAddressableModel;
 import org.dspace.app.rest.model.patch.Patch;
 import org.dspace.authorize.AuthorizeException;
-import org.dspace.content.service.MetadataFieldService;
 import org.dspace.core.Context;
 import org.springframework.beans.factory.BeanNameAware;
 import org.springframework.beans.factory.annotation.Autowired;
@@ -50,17 +48,12 @@ public abstract class DSpaceRestRepository<T extends RestAddressableModel, ID ex
     extends AbstractDSpaceRestRepository
     implements CrudRepository<T, ID>, PagingAndSortingRepository<T, ID>, BeanNameAware {
 
-    private static final Logger log = org.apache.logging.log4j.LogManager.getLogger(DSpaceRestRepository.class);
-
     private String thisRepositoryBeanName;
     private DSpaceRestRepository<T, ID> thisRepository;
 
     @Autowired
     private ApplicationContext applicationContext;
 
-    @Autowired
-    private MetadataFieldService metadataFieldService;
-
     /**
      * From BeanNameAware. Allows us to capture the name of the bean, so we can load it into thisRepository.
      * See getThisRepository() method.

dspace-server-webapp/src/main/java/org/dspace/app/rest/utils/DSpaceKernelInitializer.java

@@ -81,6 +81,7 @@ public void initialize(final ConfigurableApplicationContext applicationContext)
      * Initially look for JNDI Resource called "java:/comp/env/dspace.dir".
      * If not found, use value provided in "dspace.dir" in Spring Environment
      */
+    @SuppressWarnings("BanJNDI")
     private String getDSpaceHome(ConfigurableEnvironment environment) {
         // Load the "dspace.dir" property from Spring Boot's Configuration (application.properties)
         // This gives us the location of our DSpace configurations, necessary to start the kernel

dspace-server-webapp/src/test/java/org/dspace/app/rest/AuthorizationFeatureRestRepositoryIT.java

@@ -37,12 +37,12 @@ public class AuthorizationFeatureRestRepositoryIT extends AbstractControllerInte
     @Autowired
     private AuthorizationFeatureService authzFeatureService;
 
-    @Test
     /**
-     * All the features should be returned
+     * All the features should be returned.
      *
      * @throws Exception
      */
+    @Test
     public void findAllTest() throws Exception {
         int featuresNum = authzFeatureService.findAll().size();
         int expReturn = featuresNum > 20 ? 20 : featuresNum;
@@ -62,20 +62,20 @@ public void findAllTest() throws Exception {
 
     }
 
-    @Test
     /**
      * The feature endpoint must provide proper pagination. Unauthorized and
      * forbidden scenarios are managed in the findAllTest
      *
      * @throws Exception
      */
+    @Test
     public void findAllWithPaginationTest() throws Exception {
         int featuresNum = authzFeatureService.findAll().size();
 
         String adminToken = getAuthToken(admin.getEmail(), password);
-        List<String> featureIDs = new ArrayList<String>();
+        List<String> featureIDs = new ArrayList<>();
         for (int page = 0; page < featuresNum; page++) {
-            AtomicReference<String> idRef = new AtomicReference<String>();
+            AtomicReference<String> idRef = new AtomicReference<>();
 
             getClient(adminToken)
                 .perform(get("/api/authz/features").param("page", String.valueOf(page)).param("size", "1"))
@@ -101,12 +101,13 @@ public void findAllWithPaginationTest() throws Exception {
         }
     }
 
-    @Test
     /**
-     * The feature resource endpoint must expose the proper structure and be reserved to administrators
+     * The feature resource endpoint must expose the proper structure and be
+     * reserved to administrators.
      *
      * @throws Exception
      */
+    @Test
     public void findOneTest() throws Exception {
         getClient().perform(get("/api/authz/features/withdrawItem")).andExpect(status().isOk())
                    .andExpect(jsonPath("$.id", is("withdrawItem")))
@@ -121,12 +122,12 @@ public void findOneNotFoundTest() throws Exception {
 
     }
 
-    @Test
     /**
      * It should be possible to find features by resourcetype. The endpoint is only available to administrators
      *
      * @throws Exception
      */
+    @Test
     public void findByResourceTypeTest() throws Exception {
         AuthorizationFeature alwaysTrueFeature = authzFeatureService.find(AlwaysTrueFeature.NAME);
         String adminToken = getAuthToken(admin.getEmail(), password);