Merge branch 'develop' into 3055-get-more-groups #3055

src/main/java/edu/harvard/iq/dataverse/EditDatafilesPage.java

@@ -1047,7 +1047,12 @@ public boolean isDuplicate(FileMetadata fileMetadata) {
         // more than a certain number of files... Still, needs to be revisited
         // before the final 4.0. 
         // -- L.A. 4.0
-        Iterator<FileMetadata> fmIt = workingVersion.getFileMetadatas().iterator();
+
+        // make a "defensive copy" to avoid java.util.ConcurrentModificationException from being thrown
+        // when uploading 100+ files
+        List<FileMetadata> wvCopy = new ArrayList<>(workingVersion.getFileMetadatas());
+        Iterator<FileMetadata> fmIt = wvCopy.iterator();
+
         while (fmIt.hasNext()) {
             FileMetadata fm = fmIt.next();
             String md5 = fm.getDataFile().getmd5();
@@ -1116,7 +1121,7 @@ private InputStream getDropBoxInputStream(String fileLink, GetMethod dropBoxMeth
      * Using information from the DropBox choose, ingest the chosen files
      *  https://www.dropbox.com/developers/dropins/chooser/js
      * 
-     * @param e 
+     * @param event
      */
     public void handleDropBoxUpload(ActionEvent event) {
 

src/main/java/edu/harvard/iq/dataverse/api/Admin.java

@@ -6,14 +6,12 @@
 import static edu.harvard.iq.dataverse.api.AbstractApiBean.errorResponse;
 import edu.harvard.iq.dataverse.api.dto.RoleDTO;
 import edu.harvard.iq.dataverse.authorization.AuthenticatedUserDisplayInfo;
-import edu.harvard.iq.dataverse.authorization.AuthenticatedUserLookup;
 import edu.harvard.iq.dataverse.authorization.AuthenticationProvider;
 import edu.harvard.iq.dataverse.authorization.UserIdentifier;
 import edu.harvard.iq.dataverse.authorization.exceptions.AuthenticationProviderFactoryNotFoundException;
 import edu.harvard.iq.dataverse.authorization.exceptions.AuthorizationSetupException;
 import edu.harvard.iq.dataverse.authorization.providers.AuthenticationProviderFactory;
 import edu.harvard.iq.dataverse.authorization.providers.AuthenticationProviderRow;
-import edu.harvard.iq.dataverse.authorization.providers.builtin.BuiltinAuthenticationProvider;
 import edu.harvard.iq.dataverse.authorization.providers.builtin.BuiltinUser;
 import edu.harvard.iq.dataverse.authorization.providers.builtin.BuiltinUserServiceBean;
 import edu.harvard.iq.dataverse.authorization.providers.shib.ShibAuthenticationProvider;
@@ -35,7 +33,6 @@
 
 import static edu.harvard.iq.dataverse.util.json.NullSafeJsonBuilder.jsonObjectBuilder;
 import static edu.harvard.iq.dataverse.util.json.JsonPrinter.*;
-import java.sql.SQLException;
 import java.util.Map;
 import java.util.logging.Level;
 import java.util.logging.Logger;
@@ -266,9 +263,6 @@ public Response listAuthenticatedUsers() {
     }
 
     /**
-     * @todo Refactor more of this business logic into ShibServiceBean in case
-     * we want to build a superuser GUI around this some day.
-     *
      * curl -X PUT -d "shib@mailinator.com"
      * http://localhost:8080/api/admin/authenticatedUsers/id/11/convertShibToBuiltIn
      */
@@ -283,78 +277,26 @@ public Response convertShibUserToBuiltin(@PathParam("id") Long id, String newEma
         } catch (WrappedResponse ex) {
             return errorResponse(Response.Status.FORBIDDEN, "Superusers only.");
         }
-        AuthenticatedUser userToConvert = authSvc.findByID(id);
-        if (userToConvert == null) {
-            return errorResponse(Response.Status.BAD_REQUEST, "User id " + id + " not found.");
-        }
-        AuthenticatedUserLookup lookup = userToConvert.getAuthenticatedUserLookup();
-        if (lookup == null) {
-            return errorResponse(Response.Status.BAD_REQUEST, "User id " + id + " does not have an 'authenticateduserlookup' row");
-        }
-        String providerId = lookup.getAuthenticationProviderId();
-        if (providerId == null) {
-            return errorResponse(Response.Status.BAD_REQUEST, "User id " + id + " provider id is null.");
-        }
-        String shibProviderId = ShibAuthenticationProvider.PROVIDER_ID;
-        if (!providerId.equals(shibProviderId)) {
-            return errorResponse(Response.Status.BAD_REQUEST, "User id " + id + " cannot be converted because current provider id is '" + providerId + "' rather than '" + shibProviderId + "'.");
-        }
-        BuiltinUser builtinUser = null;
         try {
-            /**
-             * @todo Refactor more of the logic and error checking into this
-             * convertShibToBuiltIn method.
-             */
-            builtinUser = authSvc.convertShibToBuiltIn(userToConvert, newEmailAddress);
-        } catch (Throwable ex) {
-            while (ex.getCause() != null) {
-                ex = ex.getCause();
+            BuiltinUser builtinUser = authSvc.convertShibToBuiltIn(id, newEmailAddress);
+            if (builtinUser == null) {
+                return errorResponse(Response.Status.BAD_REQUEST, "User id " + id + " could not be converted from Shibboleth to BuiltIn. An Exception was not thrown.");
             }
-            if (ex instanceof ConstraintViolationException) {
-                ConstraintViolationException constraintViolationException = (ConstraintViolationException) ex;
-                StringBuilder userMsg = new StringBuilder();
-                StringBuilder logMsg = new StringBuilder();
-                logMsg.append("User id " + id + " cannot be converted from Shibboleth to BuiltIn. ");
-                for (ConstraintViolation<?> violation : constraintViolationException.getConstraintViolations()) {
-                    logMsg.append(" Invalid value: <<<").append(violation.getInvalidValue()).append(">>> for ").append(violation.getPropertyPath()).append(" at ").append(violation.getLeafBean()).append(" - ").append(violation.getMessage());
-                    userMsg.append(" Invalid value: <<<").append(violation.getInvalidValue()).append(">>> for ").append(violation.getPropertyPath()).append(" - ").append(violation.getMessage());
-                }
-                logger.warning(logMsg.toString());
-                return errorResponse(Response.Status.BAD_REQUEST, "User id " + id + " could not be converted from Shibboleth to BuiltIn: " + userMsg.toString());
-            } else {
-                return errorResponse(Response.Status.INTERNAL_SERVER_ERROR, "User id " + id + " cannot be converted due to unexpected exception: " + ex);
-            }
-        }
-        if (builtinUser == null) {
-            return errorResponse(Response.Status.BAD_REQUEST, "User id " + id + " could not be converted from Shibboleth to BuiltIn");
-        }
-        try {
-            /**
-             * @todo Should this logic be moved to the
-             * authSvc.convertShibToBuiltIn() method?
-             */
-            lookup.setAuthenticationProviderId(BuiltinAuthenticationProvider.PROVIDER_ID);
-            lookup.setPersistentUserId(userToConvert.getUserIdentifier());
-            em.persist(lookup);
-            userToConvert.setEmail(newEmailAddress);
-            em.persist(userToConvert);
-            em.flush();
+            JsonObjectBuilder output = Json.createObjectBuilder();
+            output.add("email", builtinUser.getEmail());
+            output.add("username", builtinUser.getUserName());
+            return okResponse(output);
         } catch (Throwable ex) {
+            StringBuilder sb = new StringBuilder();
+            sb.append(ex + " ");
             while (ex.getCause() != null) {
                 ex = ex.getCause();
+                sb.append(ex + " ");
             }
-            if (ex instanceof SQLException) {
-                String msg = "User id " + id + " only half converted from Shibboleth to BuiltIn and may not be able to log in. Manual changes may be necessary on 'authenticationproviderid' and 'authenticationproviderid' on 'authenticateduserlookup' table and 'email' on 'authenticateduser' table.";
-                logger.warning(msg);
-                return errorResponse(Response.Status.BAD_REQUEST, msg);
-            } else {
-                return errorResponse(Response.Status.INTERNAL_SERVER_ERROR, "User id " + id + " only half converted from Shibboleth to BuiltIn and may not be able to log in due to unexpected exception: " + ex.getClass().getName());
-            }
+            String msg = "User id " + id + " could not be converted from Shibboleth to BuiltIn. Details from Exception: " + sb;
+            logger.info(msg);
+            return errorResponse(Response.Status.BAD_REQUEST, msg);
         }
-        JsonObjectBuilder output = Json.createObjectBuilder();
-        output.add("email", builtinUser.getEmail());
-        output.add("username", builtinUser.getUserName());
-        return okResponse(output);
     }
 
     /**

src/main/java/edu/harvard/iq/dataverse/api/BuiltinUsers.java

@@ -8,6 +8,7 @@
 import edu.harvard.iq.dataverse.authorization.providers.builtin.PasswordEncryption;
 import edu.harvard.iq.dataverse.authorization.users.ApiToken;
 import edu.harvard.iq.dataverse.authorization.users.AuthenticatedUser;
+import edu.harvard.iq.dataverse.util.json.JsonPrinter;
 import java.sql.Timestamp;
 import java.util.Calendar;
 import java.util.logging.Level;
@@ -24,6 +25,7 @@
 import javax.ws.rs.core.Response;
 import javax.ws.rs.core.Response.Status;
 import static edu.harvard.iq.dataverse.util.json.JsonPrinter.json;
+import static edu.harvard.iq.dataverse.util.json.JsonPrinter.jsonForAuthUser;
 
 /**
  * REST API bean for managing {@link BuiltinUser}s.
@@ -134,6 +136,7 @@ private Response internalSave(BuiltinUser user, String password, String key) {
 
             JsonObjectBuilder resp = Json.createObjectBuilder();
             resp.add("user", json(user));
+            resp.add("authenticatedUser", jsonForAuthUser(au));
             resp.add("apiToken", token.getTokenString());
 
             alr.setInfo("builtinUser:" + user.getUserName() + " authenticatedUser:" + au.getIdentifier() );

src/main/java/edu/harvard/iq/dataverse/authorization/AuthenticationServiceBean.java

@@ -18,6 +18,7 @@
 import edu.harvard.iq.dataverse.authorization.providers.shib.ShibAuthenticationProvider;
 import edu.harvard.iq.dataverse.authorization.users.ApiToken;
 import edu.harvard.iq.dataverse.authorization.users.AuthenticatedUser;
+import java.sql.SQLException;
 import java.sql.Timestamp;
 import java.util.Calendar;
 import java.util.Date;
@@ -504,30 +505,63 @@ public AuthenticatedUser convertBuiltInToShib(AuthenticatedUser builtInUserToCon
     }
 
     /**
-     * @param authenticatedUser The AuthenticatedUser (Shibboleth user) to
-     * convert to a BuiltinUser.
+     * @param idOfAuthUserToConvert The id of the AuthenticatedUser (Shibboleth
+     * user) to convert to a BuiltinUser.
      * @param newEmailAddress The new email address that will be used instead of
      * the user's old email address from the institution that they have left.
      * @return BuiltinUser
-     * @throws java.lang.Exception You must catch a potential
-     * ConstraintViolationException due to Bean Validation (non-null, etc.) on
-     * the entities. Report these back to the superuser.
+     * @throws java.lang.Exception You must catch and report back to the user (a
+     * superuser) any Exceptions.
      */
-    public BuiltinUser convertShibToBuiltIn(AuthenticatedUser authenticatedUser, String newEmailAddress) throws Exception {
+    public BuiltinUser convertShibToBuiltIn(Long idOfAuthUserToConvert, String newEmailAddress) throws Exception {
+        AuthenticatedUser authenticatedUser = findByID(idOfAuthUserToConvert);
+        if (authenticatedUser == null) {
+            throw new Exception("User id " + idOfAuthUserToConvert + " not found.");
+        }
+        AuthenticatedUser existingUserWithSameEmail = getAuthenticatedUserByEmail(newEmailAddress);
+        if (existingUserWithSameEmail != null) {
+            throw new Exception("User id " + idOfAuthUserToConvert + " (" + authenticatedUser.getIdentifier() + ") cannot be converted from Shibboleth to BuiltIn because the email address " + newEmailAddress + " is already in use by user id " + existingUserWithSameEmail.getId() + " (" + existingUserWithSameEmail.getIdentifier() + ").");
+        }
         BuiltinUser builtinUser = new BuiltinUser();
         builtinUser.setUserName(authenticatedUser.getUserIdentifier());
         builtinUser.setFirstName(authenticatedUser.getFirstName());
         builtinUser.setLastName(authenticatedUser.getLastName());
+        // Bean Validation will check for null and invalid email addresses
         builtinUser.setEmail(newEmailAddress);
-        /**
-         * @todo If there are violations, don't even try to persist the user.
-         * Report the violations. Write tests around this.
-         */
         ValidatorFactory factory = Validation.buildDefaultValidatorFactory();
         Validator validator = factory.getValidator();
         Set<ConstraintViolation<BuiltinUser>> violations = validator.validate(builtinUser);
-        logger.fine("constraint violation count: " + violations.size());
-        builtinUser = builtinUserServiceBean.save(builtinUser);
+        int numViolations = violations.size();
+        if (numViolations > 0) {
+            StringBuilder logMsg = new StringBuilder();
+            for (ConstraintViolation<?> violation : violations) {
+                logMsg.append(" Invalid value: <<<").append(violation.getInvalidValue()).append(">>> for ").append(violation.getPropertyPath()).append(" at ").append(violation.getLeafBean()).append(" - ").append(violation.getMessage());
+            }
+            throw new Exception("User id " + idOfAuthUserToConvert + " cannot be converted from Shibboleth to BuiltIn because of constraint violations on the BuiltIn user that would be created: " + numViolations + ". Details: " + logMsg);
+        }
+        try {
+            builtinUser = builtinUserServiceBean.save(builtinUser);
+        } catch (IllegalArgumentException ex) {
+            throw new Exception("User id " + idOfAuthUserToConvert + " cannot be converted from Shibboleth to BuiltIn because of an IllegalArgumentException creating the row in the builtinuser table: " + ex);
+        }
+        AuthenticatedUserLookup lookup = authenticatedUser.getAuthenticatedUserLookup();
+        if (lookup == null) {
+            throw new Exception("User id " + idOfAuthUserToConvert + " does not have an 'authenticateduserlookup' row");
+        }
+        String providerId = lookup.getAuthenticationProviderId();
+        if (providerId == null) {
+            throw new Exception("User id " + idOfAuthUserToConvert + " provider id is null.");
+        }
+        String shibProviderId = ShibAuthenticationProvider.PROVIDER_ID;
+        if (!providerId.equals(shibProviderId)) {
+            throw new Exception("User id " + idOfAuthUserToConvert + " cannot be converted from Shibboleth to BuiltIn because current provider id is '" + providerId + "' rather than '" + shibProviderId + "'.");
+        }
+        lookup.setAuthenticationProviderId(BuiltinAuthenticationProvider.PROVIDER_ID);
+        lookup.setPersistentUserId(authenticatedUser.getUserIdentifier());
+        em.persist(lookup);
+        authenticatedUser.setEmail(newEmailAddress);
+        em.persist(authenticatedUser);
+        em.flush();
         return builtinUser;
     }
 

src/test/java/edu/harvard/iq/dataverse/api/AdminIT.java

@@ -36,7 +36,7 @@ public static void setUp() {
         Response createUserToConvert = UtilIT.createRandomUser();
         createUserToConvert.prettyPrint();
 
-        idOfUserToConvert = createUserToConvert.body().jsonPath().getLong("data.user.id");
+        idOfUserToConvert = createUserToConvert.body().jsonPath().getLong("data.authenticatedUser.id");
         emailOfUserToConvert = createUserToConvert.body().jsonPath().getString("data.user.email");
         usernameOfUserToConvert = UtilIT.getUsernameFromResponse(createUserToConvert);
 
@@ -97,12 +97,6 @@ public void testConvertShibUserToBuiltin() throws Exception {
 
         Response infoOfUserToConvert = UtilIT.getAuthenticatedUser(usernameOfUserToConvert, superuserApiToken);
         infoOfUserToConvert.prettyPrint();
-        /**
-         * https://github.com/IQSS/dataverse/issues/2418 is cropping up again in
-         * that if we try to reuse this ID below it's the wrong ID (off by one).
-         * Weird and troubling.
-         */
-        idOfUserToConvert = infoOfUserToConvert.body().jsonPath().getLong("data.id");
 
         String invalidEmailAddress = "invalidEmailAddress";
         Response invalidEmailFail = UtilIT.migrateShibToBuiltin(idOfUserToConvert, invalidEmailAddress, superuserApiToken);

src/test/java/edu/harvard/iq/dataverse/api/BuiltinUsersIT.java

@@ -28,7 +28,8 @@ public void testUserId() {
         assertEquals(200, createUserResponse.getStatusCode());
 
         JsonPath createdUser = JsonPath.from(createUserResponse.body().asString());
-        int userIdFromJsonCreateResponse = createdUser.getInt("data.user." + idKey);
+        int builtInUserIdFromJsonCreateResponse = createdUser.getInt("data.user." + idKey);
+        int authenticatedUserIdFromJsonCreateResponse = createdUser.getInt("data.authenticatedUser." + idKey);
         String username = createdUser.getString("data.user." + usernameKey);
 
         Response getUserResponse = getUserFromDatabase(username);
@@ -43,14 +44,9 @@ public void testUserId() {
         deleteUserResponse.prettyPrint();
 
         System.out.println(userIdFromDatabase + " was the id from the database");
-        System.out.println(userIdFromJsonCreateResponse + " was the id from JSON response on create");
-        /**
-         * This test is expected to pass on a clean, fresh database but for an
-         * unknown reason it fails when you load it up with a production
-         * database from dataverse.harvard.edu. Why? This is what
-         * https://github.com/IQSS/dataverse/issues/2418 is about.
-         */
-        assertEquals(userIdFromDatabase, userIdFromJsonCreateResponse);
+        System.out.println(builtInUserIdFromJsonCreateResponse + " was the id of the BuiltinUser from JSON response on create");
+        System.out.println(authenticatedUserIdFromJsonCreateResponse + " was the id of the AuthenticatedUser from JSON response on create");
+        assertEquals(userIdFromDatabase, authenticatedUserIdFromJsonCreateResponse);
     }
 
     @Test