Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

make usernames case insensitive #3575 #5783

Merged
merged 15 commits into from
May 15, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions doc/release-notes/3575-usernames.md
Original file line number Diff line number Diff line change
@@ -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.
1 change: 1 addition & 0 deletions scripts/database/reference_data.sql
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/edu/harvard/iq/dataverse/LoginPage.java
Original file line number Diff line number Diff line change
Expand Up @@ -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() );
Expand Down
13 changes: 7 additions & 6 deletions src/main/java/edu/harvard/iq/dataverse/api/BuiltinUsers.java
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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")
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 "
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
CREATE UNIQUE INDEX index_authenticateduser_lower_useridentifier ON authenticateduser (lower(useridentifier));
28 changes: 28 additions & 0 deletions src/test/java/edu/harvard/iq/dataverse/api/UsersIT.java
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
22 changes: 22 additions & 0 deletions src/test/java/edu/harvard/iq/dataverse/api/UtilIT.java
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down