Merge pull request #5783 from IQSS/3575-usernames

make usernames case insensitive #3575

doc/release-notes/3575-usernames.md

@@ -0,0 +1,4 @@
+* In an effort to prevent accidental duplicate accounts, user spoofing, or other username-based confusion, this release introduces a database constraint that no longer allows usernames that are exactly the same but use different capitalization, e.g. Bob11 vs. bob11. You may need to do some cleanup before upgrading to deal with existing usernames like this.
+* To check whether you have any usernames like this that need cleaning up, run the case insensitive duplicate queries from our [Useful Queries doc](https://docs.google.com/document/d/1-Y_iUduSxdDNeK1yiGUxe7t-Md7Fy965jp4o4m1XEoE/edit?usp=sharing "Useful Queries doc").
+* Once you identify the usernames that need cleaning up, you should use either [Merge User Accounts](http://guides.dataverse.org/en/latest/api/native-api.html#merge-user-accounts) (if it’s the same person) or [Change User Identifier](http://guides.dataverse.org/en/latest/api/native-api.html#change-user-identifier) (if they are different people).
+* After the cleanup you can safely upgrade without issue.

scripts/database/reference_data.sql

@@ -30,6 +30,7 @@ INSERT INTO guestbook(
 -- gets an answer. See also https://github.com/IQSS/dataverse/issues/2598#issuecomment-158219334
 CREATE UNIQUE INDEX dataverse_alias_unique_idx on dataverse (LOWER(alias));
 CREATE UNIQUE INDEX index_authenticateduser_lower_email ON authenticateduser (lower(email));
+CREATE UNIQUE INDEX index_authenticateduser_lower_useridentifier ON authenticateduser (lower(useridentifier));
 -- this field has been removed from builtinuser; CREATE UNIQUE INDEX index_builtinuser_lower_email ON builtinuser (lower(email));
 
 --Edit Dataset: Investigate and correct multiple draft issue: https://github.com/IQSS/dataverse/issues/2132

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

@@ -160,7 +160,7 @@ public String login() {
             logger.info("Credential list is null!");
             return null;
         }
-        for ( FilledCredential fc : filledCredentialsList ) {            
+        for ( FilledCredential fc : filledCredentialsList ) {       
             authReq.putCredential(fc.getCredential().getKey(), fc.getValue());
         }
         authReq.setIpAddress( dvRequestService.getDataverseRequest().getSourceAddress() );

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

@@ -123,9 +123,9 @@ private Response internalSave(BuiltinUser user, String password, String key) {
                 user.updateEncryptedPassword(PasswordEncryption.get().encrypt(password), PasswordEncryption.getLatestVersionNumber());
             }
 
-            // Make sure the identifier is unique
-            if ( (builtinUserSvc.findByUserName(user.getUserName()) != null)
-                    || ( authSvc.identifierExists(user.getUserName())) ) {
+            // Make sure the identifier is unique, case insensitive. "DATAVERSEADMIN" is not allowed to be created if "dataverseAdmin" exists.
+            if ((builtinUserSvc.findByUserName(user.getUserName()) != null)
+                    || (authSvc.identifierExists(user.getUserName()))) {
                 return error(Status.BAD_REQUEST, "username '" + user.getUserName() + "' already exists");
             }
             user = builtinUserSvc.save(user);
@@ -176,12 +176,13 @@ private Response internalSave(BuiltinUser user, String password, String key) {
 
         } catch ( EJBException ejbx ) {
             alr.setActionResult(ActionLogRecord.Result.InternalError);
-            alr.setInfo( alr.getInfo() + "// " + ejbx.getMessage());
+            String errorMessage = ejbx.getCausedByException().getLocalizedMessage();
+            alr.setInfo( alr.getInfo() + "// " + errorMessage);
             if ( ejbx.getCausedByException() instanceof IllegalArgumentException ) {
-                return error(Status.BAD_REQUEST, "Bad request: can't save user. " + ejbx.getCausedByException().getMessage());
+                return error(Status.BAD_REQUEST, "Bad request: can't save user. " + errorMessage);
             } else {
                 logger.log(Level.WARNING, "Error saving user: ", ejbx);
-                return error(Status.INTERNAL_SERVER_ERROR, "Can't save user: " + ejbx.getMessage());
+                return error(Status.INTERNAL_SERVER_ERROR, "Can't save user: " + errorMessage);
             }
 
         } catch (Exception e) {

src/main/java/edu/harvard/iq/dataverse/authorization/providers/builtin/BuiltinUser.java

@@ -27,7 +27,7 @@
 		@NamedQuery( name="BuiltinUser.findAll",
 				query = "SELECT u FROM BuiltinUser u ORDER BY u.userName"),
 		@NamedQuery( name="BuiltinUser.findByUserName",
-				query = "SELECT u FROM BuiltinUser u WHERE u.userName=:userName"),
+				query = "SELECT u FROM BuiltinUser u WHERE LOWER(u.userName)=LOWER(:userName)"),
 		@NamedQuery( name="BuiltinUser.listByUserNameLike",
 				query = "SELECT u FROM BuiltinUser u WHERE u.userName LIKE :userNameLike")
 })

src/main/java/edu/harvard/iq/dataverse/authorization/users/AuthenticatedUser.java

@@ -45,14 +45,14 @@
     @NamedQuery( name="AuthenticatedUser.findSuperUsers",
                 query="SELECT au FROM AuthenticatedUser au WHERE au.superuser = TRUE"),
     @NamedQuery( name="AuthenticatedUser.findByIdentifier",
-                query="select au from AuthenticatedUser au WHERE au.userIdentifier=:identifier"),
+                query="select au from AuthenticatedUser au WHERE LOWER(au.userIdentifier)=LOWER(:identifier)"),
     @NamedQuery( name="AuthenticatedUser.findByEmail",
                 query="select au from AuthenticatedUser au WHERE LOWER(au.email)=LOWER(:email)"),
     @NamedQuery( name="AuthenticatedUser.countOfIdentifier",
-                query="SELECT COUNT(a) FROM AuthenticatedUser a WHERE a.userIdentifier=:identifier"),
+                query="SELECT COUNT(a) FROM AuthenticatedUser a WHERE LOWER(a.userIdentifier)=LOWER(:identifier)"),
     @NamedQuery( name="AuthenticatedUser.filter",
                 query="select au from AuthenticatedUser au WHERE ("
-                        + "au.userIdentifier like :query OR "
+                        + "LOWER(au.userIdentifier) like LOWER(:query) OR "
                         + "lower(concat(au.firstName,' ',au.lastName)) like lower(:query))"),
     @NamedQuery( name="AuthenticatedUser.findAdminUser",
                 query="select au from AuthenticatedUser au WHERE "

src/main/java/edu/harvard/iq/dataverse/engine/command/impl/ChangeUserIdentifierCommand.java

@@ -49,7 +49,7 @@ public ChangeUserIdentifierCommand(DataverseRequest aRequest, AuthenticatedUser
     public void executeImpl(CommandContext ctxt) throws CommandException {  
 
         AuthenticatedUser authenticatedUserTestNewIdentifier = ctxt.authentication().getAuthenticatedUser(newIdentifier);
-        if (authenticatedUserTestNewIdentifier != null) {
+        if (authenticatedUserTestNewIdentifier != null && !authenticatedUserTestNewIdentifier.equals(au)) {
             String logMsg = " User " + newIdentifier + " already exists. Cannot use this as new identifier";
             throw new IllegalCommandException("Validation of submitted data failed. Details: " + logMsg, this);
         }

src/main/resources/db/migration/V4.13.0.1__3575-usernames.sql

@@ -0,0 +1 @@
+CREATE UNIQUE INDEX index_authenticateduser_lower_useridentifier ON authenticateduser (lower(useridentifier));

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

@@ -326,6 +326,34 @@ public void convertNonBcryptUserFromBuiltinToShib() {
 
     }
 
+    @Test
+    public void testUsernameCaseSensitivity() {
+        String randomUsername = UtilIT.getRandomIdentifier();
+        String lowercaseUsername = randomUsername.toLowerCase();
+        String uppercaseUsername = randomUsername.toUpperCase();
+        String randomEmailForLowercaseuser = UtilIT.getRandomIdentifier() + "@mailinator.com";
+        String randomEmailForUppercaseuser = UtilIT.getRandomIdentifier() + "@mailinator.com";
+
+        // Create first user (username all lower case).
+        Response createLowercaseUser = UtilIT.createUser(lowercaseUsername, randomEmailForLowercaseuser);
+        createLowercaseUser.prettyPrint();
+        createLowercaseUser.then().assertThat()
+                .statusCode(OK.getStatusCode());
+
+        // Attempt to create second user (same username but all UPPER CASE).
+        Response createUppercaseUser = UtilIT.createUser(uppercaseUsername, randomEmailForUppercaseuser);
+        createUppercaseUser.prettyPrint();
+        createUppercaseUser.then().assertThat()
+                .statusCode(BAD_REQUEST.getStatusCode())
+                /**
+                 * Technically, it's the lowercase version that exists but the
+                 * point gets across. There's currently no way to bubble up the
+                 * exact username it's in conflict with, even if we wanted to.
+                 */
+                .body("message", equalTo("username '" + uppercaseUsername + "' already exists"));
+        ;
+    }
+
     private Response convertUserFromBcryptToSha1(long idOfBcryptUserToConvert, String password) {
         JsonObjectBuilder data = Json.createObjectBuilder();
         data.add("builtinUserId", idOfBcryptUserToConvert);

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

@@ -103,6 +103,28 @@ public static Response createRandomUser() {
         return createRandomUser("user");
     }
 
+    public static Response createUser(String username, String email) {
+        logger.info("Creating user " + username);
+        String userAsJson = getUserAsJsonString(username, username, username, email);
+        String password = getPassword(userAsJson);
+        Response response = given()
+                .body(userAsJson)
+                .contentType(ContentType.JSON)
+                .post("/api/builtin-users?key=" + BUILTIN_USER_KEY + "&password=" + password);
+        return response;
+    }
+
+    private static String getUserAsJsonString(String username, String firstName, String lastName, String email) {
+        JsonObjectBuilder builder = Json.createObjectBuilder();
+        builder.add(USERNAME_KEY, username);
+        builder.add("firstName", firstName);
+        builder.add("lastName", lastName);
+        builder.add(EMAIL_KEY, email);
+        String userAsJson = builder.build().toString();
+        logger.fine("User to create: " + userAsJson);
+        return userAsJson;
+    }
+
     private static String getUserAsJsonString(String username, String firstName, String lastName) {
         JsonObjectBuilder builder = Json.createObjectBuilder();
         builder.add(USERNAME_KEY, username);