diff --git a/api/src/main/java/edu/cornell/mannlib/vitro/webapp/beans/UserAccount.java b/api/src/main/java/edu/cornell/mannlib/vitro/webapp/beans/UserAccount.java index 7b9d49792..7b5581885 100644 --- a/api/src/main/java/edu/cornell/mannlib/vitro/webapp/beans/UserAccount.java +++ b/api/src/main/java/edu/cornell/mannlib/vitro/webapp/beans/UserAccount.java @@ -7,17 +7,11 @@ import java.util.Collections; import java.util.HashSet; import java.util.Set; -import edu.cornell.mannlib.vitro.webapp.controller.authenticate.Authenticator; +import org.apache.commons.lang3.RandomStringUtils; /** * Information about the account of a user. URI, email, password, etc. * - * The "password link expires hash" is just a string that is derived from the - * value in the passwordLinkExpires field. It doesn't have to be a hash, and - * there is no need for it to be cryptographic, but it seems embarrassing to - * just send the value as a clear string. There is no real need for security - * here, except that a brute force attack would allow someone to change the - * password on an account that they know has a password change pending. */ public class UserAccount { public static final int MIN_PASSWORD_LENGTH = 6; @@ -52,6 +46,7 @@ public class UserAccount { private String md5Password = ""; // Never null. private String oldPassword = ""; // Never null. private long passwordLinkExpires = 0L; // Never negative. + private String emailKey = ""; private boolean passwordChangeRequired = false; private int loginCount = 0; // Never negative. @@ -133,15 +128,27 @@ public class UserAccount { return passwordLinkExpires; } - public String getPasswordLinkExpiresHash() { - return limitStringLength(8, Authenticator.applyArgon2iEncoding(String - .valueOf(passwordLinkExpires))); - } - public void setPasswordLinkExpires(long passwordLinkExpires) { this.passwordLinkExpires = Math.max(0, passwordLinkExpires); } + public void generateEmailKey() { + boolean useLetters = true; + boolean useNumbers = true; + int length = 64; + emailKey = RandomStringUtils.random(length, useLetters, useNumbers); + } + + public void setEmailKey(String emailKey) { + if (emailKey != null) { + this.emailKey = emailKey; + } + } + + public String getEmailKey() { + return emailKey; + } + public boolean isPasswordChangeRequired() { return passwordChangeRequired; } @@ -247,6 +254,7 @@ public class UserAccount { + (", oldPassword=" + oldPassword) + (", argon2password=" + argon2Password) + (", passwordLinkExpires=" + passwordLinkExpires) + + (", emailKey =" + emailKey) + (", passwordChangeRequired=" + passwordChangeRequired) + (", externalAuthOnly=" + externalAuthOnly) + (", loginCount=" + loginCount) + (", status=" + status) diff --git a/api/src/main/java/edu/cornell/mannlib/vitro/webapp/controller/accounts/UserAccountsSelector.java b/api/src/main/java/edu/cornell/mannlib/vitro/webapp/controller/accounts/UserAccountsSelector.java index 6caadf24c..d878ae315 100644 --- a/api/src/main/java/edu/cornell/mannlib/vitro/webapp/controller/accounts/UserAccountsSelector.java +++ b/api/src/main/java/edu/cornell/mannlib/vitro/webapp/controller/accounts/UserAccountsSelector.java @@ -249,6 +249,7 @@ public class UserAccountsSelector { user.setMd5Password(ifLiteralPresent(solution, "md5pwd", "")); user.setArgon2Password(ifLiteralPresent(solution, "a2pwd", "")); user.setPasswordLinkExpires(ifLongPresent(solution, "expire", 0L)); + user.setEmailKey(ifLiteralPresent(solution, "emailKey", "")); user.setLoginCount(ifIntPresent(solution, "count", 0)); user.setLastLoginTime(ifLongPresent(solution, "lastLogin", 0)); user.setStatus(parseStatus(solution, "status", null)); diff --git a/api/src/main/java/edu/cornell/mannlib/vitro/webapp/controller/accounts/admin/UserAccountsAddPage.java b/api/src/main/java/edu/cornell/mannlib/vitro/webapp/controller/accounts/admin/UserAccountsAddPage.java index bdbc5dbce..7fc4181da 100644 --- a/api/src/main/java/edu/cornell/mannlib/vitro/webapp/controller/accounts/admin/UserAccountsAddPage.java +++ b/api/src/main/java/edu/cornell/mannlib/vitro/webapp/controller/accounts/admin/UserAccountsAddPage.java @@ -156,6 +156,7 @@ public class UserAccountsAddPage extends UserAccountsPage { u.setOldPassword(""); u.setPasswordChangeRequired(false); u.setPasswordLinkExpires(0); + u.setEmailKey(""); u.setLoginCount(0); u.setLastLoginTime(0L); u.setStatus(Status.INACTIVE); diff --git a/api/src/main/java/edu/cornell/mannlib/vitro/webapp/controller/accounts/admin/UserAccountsAddPageStrategy.java b/api/src/main/java/edu/cornell/mannlib/vitro/webapp/controller/accounts/admin/UserAccountsAddPageStrategy.java index 307ccf9f6..036d314e2 100644 --- a/api/src/main/java/edu/cornell/mannlib/vitro/webapp/controller/accounts/admin/UserAccountsAddPageStrategy.java +++ b/api/src/main/java/edu/cornell/mannlib/vitro/webapp/controller/accounts/admin/UserAccountsAddPageStrategy.java @@ -84,6 +84,7 @@ public abstract class UserAccountsAddPageStrategy extends UserAccountsPage { u.setStatus(Status.ACTIVE); } else { u.setPasswordLinkExpires(figureExpirationDate().getTime()); + u.generateEmailKey(); u.setStatus(Status.INACTIVE); } } @@ -119,10 +120,8 @@ public abstract class UserAccountsAddPageStrategy extends UserAccountsPage { private String buildCreatePasswordLink() { try { String email = page.getAddedAccount().getEmailAddress(); - String hash = page.getAddedAccount() - .getPasswordLinkExpiresHash(); - String relativeUrl = UrlBuilder.getUrl(CREATE_PASSWORD_URL, - "user", email, "key", hash); + String key = page.getAddedAccount().getEmailKey(); + String relativeUrl = UrlBuilder.getUrl(CREATE_PASSWORD_URL, "user", email, "key", key); URL context = new URL(vreq.getRequestURL().toString()); URL url = new URL(context, relativeUrl); diff --git a/api/src/main/java/edu/cornell/mannlib/vitro/webapp/controller/accounts/admin/UserAccountsEditPage.java b/api/src/main/java/edu/cornell/mannlib/vitro/webapp/controller/accounts/admin/UserAccountsEditPage.java index 42dd4102d..13e1ab0dd 100644 --- a/api/src/main/java/edu/cornell/mannlib/vitro/webapp/controller/accounts/admin/UserAccountsEditPage.java +++ b/api/src/main/java/edu/cornell/mannlib/vitro/webapp/controller/accounts/admin/UserAccountsEditPage.java @@ -274,6 +274,7 @@ public class UserAccountsEditPage extends UserAccountsPage { userAccount.setOldPassword(""); userAccount.setPasswordChangeRequired(false); userAccount.setPasswordLinkExpires(0L); + userAccount.setEmailKey(""); } if (isRootUser()) { diff --git a/api/src/main/java/edu/cornell/mannlib/vitro/webapp/controller/accounts/admin/UserAccountsEditPageStrategy.java b/api/src/main/java/edu/cornell/mannlib/vitro/webapp/controller/accounts/admin/UserAccountsEditPageStrategy.java index 708f11e66..d9ee8aa14 100644 --- a/api/src/main/java/edu/cornell/mannlib/vitro/webapp/controller/accounts/admin/UserAccountsEditPageStrategy.java +++ b/api/src/main/java/edu/cornell/mannlib/vitro/webapp/controller/accounts/admin/UserAccountsEditPageStrategy.java @@ -82,6 +82,7 @@ public abstract class UserAccountsEditPageStrategy extends UserAccountsPage { protected void setAdditionalProperties(UserAccount u) { if (resetPassword && !page.isExternalAuthOnly()) { u.setPasswordLinkExpires(figureExpirationDate().getTime()); + u.generateEmailKey(); } } @@ -121,10 +122,8 @@ public abstract class UserAccountsEditPageStrategy extends UserAccountsPage { private String buildResetPasswordLink() { try { String email = page.getUpdatedAccount().getEmailAddress(); - String hash = page.getUpdatedAccount() - .getPasswordLinkExpiresHash(); - String relativeUrl = UrlBuilder.getUrl(RESET_PASSWORD_URL, - "user", email, "key", hash); + String key = page.getUpdatedAccount().getEmailKey(); + String relativeUrl = UrlBuilder.getUrl(RESET_PASSWORD_URL, "user", email, "key", key); URL context = new URL(vreq.getRequestURL().toString()); URL url = new URL(context, relativeUrl); diff --git a/api/src/main/java/edu/cornell/mannlib/vitro/webapp/controller/accounts/user/UserAccountsCreatePasswordPage.java b/api/src/main/java/edu/cornell/mannlib/vitro/webapp/controller/accounts/user/UserAccountsCreatePasswordPage.java index b9515d4c7..68daa2d67 100644 --- a/api/src/main/java/edu/cornell/mannlib/vitro/webapp/controller/accounts/user/UserAccountsCreatePasswordPage.java +++ b/api/src/main/java/edu/cornell/mannlib/vitro/webapp/controller/accounts/user/UserAccountsCreatePasswordPage.java @@ -36,6 +36,7 @@ public class UserAccountsCreatePasswordPage extends userAccount.setArgon2Password(Authenticator.applyArgon2iEncoding(newPassword)); userAccount.setMd5Password(""); userAccount.setPasswordLinkExpires(0L); + userAccount.setEmailKey(""); userAccount.setPasswordChangeRequired(false); userAccount.setStatus(Status.ACTIVE); userAccountsDao.updateUserAccount(userAccount); @@ -53,6 +54,11 @@ public class UserAccountsCreatePasswordPage extends protected String passwordChangeNotPendingMessage() { return i18n.text("account_already_activated", userEmail); } + + @Override + protected String passwordChangeInavlidKeyMessage() { + return i18n.text("password_change_invalid_key", userEmail); + } @Override protected String templateName() { diff --git a/api/src/main/java/edu/cornell/mannlib/vitro/webapp/controller/accounts/user/UserAccountsFirstTimeExternalPage.java b/api/src/main/java/edu/cornell/mannlib/vitro/webapp/controller/accounts/user/UserAccountsFirstTimeExternalPage.java index fc11f665d..ffb34c754 100644 --- a/api/src/main/java/edu/cornell/mannlib/vitro/webapp/controller/accounts/user/UserAccountsFirstTimeExternalPage.java +++ b/api/src/main/java/edu/cornell/mannlib/vitro/webapp/controller/accounts/user/UserAccountsFirstTimeExternalPage.java @@ -195,6 +195,7 @@ public class UserAccountsFirstTimeExternalPage extends UserAccountsPage { u.setExternalAuthId(externalAuthId); u.setPasswordChangeRequired(false); u.setPasswordLinkExpires(0); + u.setEmailKey(""); u.setExternalAuthOnly(true); u.setLoginCount(0); u.setStatus(Status.ACTIVE); diff --git a/api/src/main/java/edu/cornell/mannlib/vitro/webapp/controller/accounts/user/UserAccountsMyAccountPageStrategy.java b/api/src/main/java/edu/cornell/mannlib/vitro/webapp/controller/accounts/user/UserAccountsMyAccountPageStrategy.java index 057098eea..ca895cab8 100644 --- a/api/src/main/java/edu/cornell/mannlib/vitro/webapp/controller/accounts/user/UserAccountsMyAccountPageStrategy.java +++ b/api/src/main/java/edu/cornell/mannlib/vitro/webapp/controller/accounts/user/UserAccountsMyAccountPageStrategy.java @@ -159,6 +159,7 @@ public abstract class UserAccountsMyAccountPageStrategy extends userAccount.setMd5Password(""); userAccount.setPasswordChangeRequired(false); userAccount.setPasswordLinkExpires(0L); + userAccount.setEmailKey(""); } } diff --git a/api/src/main/java/edu/cornell/mannlib/vitro/webapp/controller/accounts/user/UserAccountsPasswordBasePage.java b/api/src/main/java/edu/cornell/mannlib/vitro/webapp/controller/accounts/user/UserAccountsPasswordBasePage.java index 3923c17b2..d4cc56f03 100644 --- a/api/src/main/java/edu/cornell/mannlib/vitro/webapp/controller/accounts/user/UserAccountsPasswordBasePage.java +++ b/api/src/main/java/edu/cornell/mannlib/vitro/webapp/controller/accounts/user/UserAccountsPasswordBasePage.java @@ -103,12 +103,12 @@ public abstract class UserAccountsPasswordBasePage extends UserAccountsPage { return; } - String expectedKey = userAccount.getPasswordLinkExpiresHash(); - if (!key.equals(expectedKey)) { + String expectedKey = userAccount.getEmailKey(); + if (key.isEmpty() || !key.equals(expectedKey)) { log.warn("Password request for '" + userEmail + "' is bogus: key (" + key + ") doesn't match expected key (" + expectedKey + ")"); - bogusMessage = passwordChangeNotPendingMessage(); + bogusMessage = passwordChangeInavlidKeyMessage(); return; } @@ -153,7 +153,7 @@ public abstract class UserAccountsPasswordBasePage extends UserAccountsPage { body.put("minimumLength", UserAccount.MIN_PASSWORD_LENGTH); body.put("maximumLength", UserAccount.MAX_PASSWORD_LENGTH); body.put("userAccount", userAccount); - body.put("key", userAccount.getPasswordLinkExpiresHash()); + body.put("key", userAccount.getEmailKey()); body.put("newPassword", newPassword); body.put("confirmPassword", confirmPassword); body.put("formUrls", buildUrlsMap()); @@ -176,6 +176,8 @@ public abstract class UserAccountsPasswordBasePage extends UserAccountsPage { protected abstract String alreadyLoggedInMessage(String currentUserEmail); protected abstract String passwordChangeNotPendingMessage(); + + protected abstract String passwordChangeInavlidKeyMessage(); protected abstract String templateName(); } diff --git a/api/src/main/java/edu/cornell/mannlib/vitro/webapp/controller/accounts/user/UserAccountsResetPasswordPage.java b/api/src/main/java/edu/cornell/mannlib/vitro/webapp/controller/accounts/user/UserAccountsResetPasswordPage.java index 712df1b40..f865cbe94 100644 --- a/api/src/main/java/edu/cornell/mannlib/vitro/webapp/controller/accounts/user/UserAccountsResetPasswordPage.java +++ b/api/src/main/java/edu/cornell/mannlib/vitro/webapp/controller/accounts/user/UserAccountsResetPasswordPage.java @@ -55,6 +55,11 @@ public class UserAccountsResetPasswordPage extends UserAccountsPasswordBasePage protected String passwordChangeNotPendingMessage() { return i18n.text("password_change_not_pending", userEmail); } + + @Override + protected String passwordChangeInavlidKeyMessage() { + return i18n.text("password_change_invalid_key", userEmail); + } @Override protected String templateName() { diff --git a/api/src/main/java/edu/cornell/mannlib/vitro/webapp/controller/authenticate/BasicAuthenticator.java b/api/src/main/java/edu/cornell/mannlib/vitro/webapp/controller/authenticate/BasicAuthenticator.java index f6f95081b..34fc6a01d 100644 --- a/api/src/main/java/edu/cornell/mannlib/vitro/webapp/controller/authenticate/BasicAuthenticator.java +++ b/api/src/main/java/edu/cornell/mannlib/vitro/webapp/controller/authenticate/BasicAuthenticator.java @@ -134,6 +134,7 @@ public class BasicAuthenticator extends Authenticator { userAccount.setMd5Password(""); userAccount.setPasswordChangeRequired(false); userAccount.setPasswordLinkExpires(0L); + userAccount.setEmailKey(""); getUserAccountsDao().updateUserAccount(userAccount); } diff --git a/api/src/main/java/edu/cornell/mannlib/vitro/webapp/dao/VitroVocabulary.java b/api/src/main/java/edu/cornell/mannlib/vitro/webapp/dao/VitroVocabulary.java index fdd9387c0..fba900cad 100644 --- a/api/src/main/java/edu/cornell/mannlib/vitro/webapp/dao/VitroVocabulary.java +++ b/api/src/main/java/edu/cornell/mannlib/vitro/webapp/dao/VitroVocabulary.java @@ -155,6 +155,7 @@ public class VitroVocabulary { public static final String USERACCOUNT_LAST_LOGIN_TIME = VITRO_AUTH + "lastLoginTime"; public static final String USERACCOUNT_STATUS = VITRO_AUTH + "status"; public static final String USERACCOUNT_PASSWORD_LINK_EXPIRES = VITRO_AUTH + "passwordLinkExpires"; + public static final String USERACCOUNT_EMAIL_KEY = VITRO_AUTH + "emailKey"; public static final String USERACCOUNT_PASSWORD_CHANGE_REQUIRED = VITRO_AUTH + "passwordChangeRequired"; public static final String USERACCOUNT_EXTERNAL_AUTH_ID = VITRO_AUTH + "externalAuthId"; public static final String USERACCOUNT_EXTERNAL_AUTH_ONLY = VITRO_AUTH + "externalAuthOnly"; diff --git a/api/src/main/java/edu/cornell/mannlib/vitro/webapp/dao/jena/JenaBaseDaoCon.java b/api/src/main/java/edu/cornell/mannlib/vitro/webapp/dao/jena/JenaBaseDaoCon.java index d618ac804..71fd6447a 100644 --- a/api/src/main/java/edu/cornell/mannlib/vitro/webapp/dao/jena/JenaBaseDaoCon.java +++ b/api/src/main/java/edu/cornell/mannlib/vitro/webapp/dao/jena/JenaBaseDaoCon.java @@ -121,6 +121,7 @@ public class JenaBaseDaoCon { protected DatatypeProperty USERACCOUNT_LAST_LOGIN_TIME = _constModel.createDatatypeProperty(VitroVocabulary.USERACCOUNT_LAST_LOGIN_TIME); protected DatatypeProperty USERACCOUNT_STATUS = _constModel.createDatatypeProperty(VitroVocabulary.USERACCOUNT_STATUS); protected DatatypeProperty USERACCOUNT_PASSWORD_LINK_EXPIRES = _constModel.createDatatypeProperty(VitroVocabulary.USERACCOUNT_PASSWORD_LINK_EXPIRES); + protected DatatypeProperty USERACCOUNT_EMAIL_KEY = _constModel.createDatatypeProperty(VitroVocabulary.USERACCOUNT_EMAIL_KEY); protected DatatypeProperty USERACCOUNT_PASSWORD_CHANGE_REQUIRED = _constModel.createDatatypeProperty(VitroVocabulary.USERACCOUNT_PASSWORD_CHANGE_REQUIRED); protected DatatypeProperty USERACCOUNT_EXTERNAL_AUTH_ID = _constModel.createDatatypeProperty(VitroVocabulary.USERACCOUNT_EXTERNAL_AUTH_ID); protected DatatypeProperty USERACCOUNT_EXTERNAL_AUTH_ONLY = _constModel.createDatatypeProperty(VitroVocabulary.USERACCOUNT_EXTERNAL_AUTH_ONLY); diff --git a/api/src/main/java/edu/cornell/mannlib/vitro/webapp/dao/jena/UserAccountsDaoJena.java b/api/src/main/java/edu/cornell/mannlib/vitro/webapp/dao/jena/UserAccountsDaoJena.java index 73557fd5c..29c1d5a29 100644 --- a/api/src/main/java/edu/cornell/mannlib/vitro/webapp/dao/jena/UserAccountsDaoJena.java +++ b/api/src/main/java/edu/cornell/mannlib/vitro/webapp/dao/jena/UserAccountsDaoJena.java @@ -4,7 +4,6 @@ package edu.cornell.mannlib.vitro.webapp.dao.jena; import java.util.ArrayList; import java.util.Collection; -import java.util.Collections; import java.util.Comparator; import java.util.List; import java.util.Random; @@ -98,6 +97,8 @@ public class UserAccountsDaoJena extends JenaBaseDao implements UserAccountsDao u.setOldPassword(getPropertyStringValue(r, USERACCOUNT_OLD_PASSWORD)); u.setPasswordLinkExpires(getPropertyLongValue(r, USERACCOUNT_PASSWORD_LINK_EXPIRES)); + u.setEmailKey(getPropertyStringValue(r,USERACCOUNT_EMAIL_KEY)); + u.setPasswordChangeRequired(getPropertyBooleanValue(r, USERACCOUNT_PASSWORD_CHANGE_REQUIRED)); u.setExternalAuthOnly(getPropertyBooleanValue(r, @@ -240,6 +241,8 @@ public class UserAccountsDaoJena extends JenaBaseDao implements UserAccountsDao userAccount.getLoginCount(), model); addPropertyLongValue(res, USERACCOUNT_LAST_LOGIN_TIME, userAccount.getLastLoginTime(), model); + addPropertyStringValue(res, USERACCOUNT_EMAIL_KEY, + userAccount.getEmailKey(), model); if (userAccount.getStatus() != null) { addPropertyStringValue(res, USERACCOUNT_STATUS, userAccount .getStatus().toString(), model); @@ -306,6 +309,8 @@ public class UserAccountsDaoJena extends JenaBaseDao implements UserAccountsDao userAccount.getLoginCount(), model); updatePropertyLongValue(res, USERACCOUNT_LAST_LOGIN_TIME, userAccount.getLastLoginTime(), model); + updatePropertyStringValue(res, USERACCOUNT_EMAIL_KEY, + userAccount.getEmailKey(), model); if (userAccount.getStatus() == null) { updatePropertyStringValue(res, USERACCOUNT_STATUS, null, model); } else { diff --git a/webapp/src/main/webapp/templates/freemarker/body/accounts/userAccounts-createPassword.ftl b/webapp/src/main/webapp/templates/freemarker/body/accounts/userAccounts-createPassword.ftl index a3ba4273c..126a2626f 100644 --- a/webapp/src/main/webapp/templates/freemarker/body/accounts/userAccounts-createPassword.ftl +++ b/webapp/src/main/webapp/templates/freemarker/body/accounts/userAccounts-createPassword.ftl @@ -26,7 +26,7 @@
- + diff --git a/webapp/src/main/webapp/templates/freemarker/body/accounts/userAccounts-resetPassword.ftl b/webapp/src/main/webapp/templates/freemarker/body/accounts/userAccounts-resetPassword.ftl index cf11f0a72..645908854 100644 --- a/webapp/src/main/webapp/templates/freemarker/body/accounts/userAccounts-resetPassword.ftl +++ b/webapp/src/main/webapp/templates/freemarker/body/accounts/userAccounts-resetPassword.ftl @@ -26,7 +26,7 @@
- +