diff --git a/doc/release-notes/3575-usernames.md b/doc/release-notes/3575-usernames.md new file mode 100644 index 00000000000..896e9910817 --- /dev/null +++ b/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. diff --git a/scripts/database/reference_data.sql b/scripts/database/reference_data.sql index cb12db7c882..41370a22013 100644 --- a/scripts/database/reference_data.sql +++ b/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 diff --git a/src/main/java/edu/harvard/iq/dataverse/LoginPage.java b/src/main/java/edu/harvard/iq/dataverse/LoginPage.java index d5c57311008..b8f2abadbad 100644 --- a/src/main/java/edu/harvard/iq/dataverse/LoginPage.java +++ b/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() ); diff --git a/src/main/java/edu/harvard/iq/dataverse/api/BuiltinUsers.java b/src/main/java/edu/harvard/iq/dataverse/api/BuiltinUsers.java index e85f4454b24..515184e50b8 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/BuiltinUsers.java +++ b/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) { diff --git a/src/main/java/edu/harvard/iq/dataverse/authorization/providers/builtin/BuiltinUser.java b/src/main/java/edu/harvard/iq/dataverse/authorization/providers/builtin/BuiltinUser.java index 142ca23052d..fd7231e827c 100644 --- a/src/main/java/edu/harvard/iq/dataverse/authorization/providers/builtin/BuiltinUser.java +++ b/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") }) diff --git a/src/main/java/edu/harvard/iq/dataverse/authorization/users/AuthenticatedUser.java b/src/main/java/edu/harvard/iq/dataverse/authorization/users/AuthenticatedUser.java index 287cce366b6..449ca56e89e 100644 --- a/src/main/java/edu/harvard/iq/dataverse/authorization/users/AuthenticatedUser.java +++ b/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 " diff --git a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/ChangeUserIdentifierCommand.java b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/ChangeUserIdentifierCommand.java index 81acc62bc9a..4a5998aea00 100644 --- a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/ChangeUserIdentifierCommand.java +++ b/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); } diff --git a/src/main/resources/db/migration/V4.13.0.1__3575-usernames.sql b/src/main/resources/db/migration/V4.13.0.1__3575-usernames.sql new file mode 100644 index 00000000000..0b1804bdfc4 --- /dev/null +++ b/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)); diff --git a/src/test/java/edu/harvard/iq/dataverse/api/UsersIT.java b/src/test/java/edu/harvard/iq/dataverse/api/UsersIT.java index 4c356e2ef44..522528ddec6 100644 --- a/src/test/java/edu/harvard/iq/dataverse/api/UsersIT.java +++ b/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); diff --git a/src/test/java/edu/harvard/iq/dataverse/api/UtilIT.java b/src/test/java/edu/harvard/iq/dataverse/api/UtilIT.java index 6eac493c5f2..4487a0553ae 100644 --- a/src/test/java/edu/harvard/iq/dataverse/api/UtilIT.java +++ b/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);