litvinovg/vitro
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Use unique key in account activation link and reset password link (#234)

Browse source 
* Use unique key for email activation and password reset

* Renamed old variable from hash to key

* Check for null before setting email key for backward compatibility. Removed comment about old behaviour.

* Send password_change_invalid_key message instead of password_change_not_pending on key mismatch.
This commit is contained in:
Georgy Litvinov 2021-06-03 14:59:29 +02:00
parent 0df4231b41
commit f16eef642a
17 changed files with 59 additions and 27 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

32
api/src/main/java/edu/cornell/mannlib/vitro/webapp/beans/UserAccount.java
View file

@ -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)

1
api/src/main/java/edu/cornell/mannlib/vitro/webapp/controller/accounts/UserAccountsSelector.java
View file

@ -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));

1
api/src/main/java/edu/cornell/mannlib/vitro/webapp/controller/accounts/admin/UserAccountsAddPage.java
View file

@ -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);

7
api/src/main/java/edu/cornell/mannlib/vitro/webapp/controller/accounts/admin/UserAccountsAddPageStrategy.java
View file

@ -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);

1
api/src/main/java/edu/cornell/mannlib/vitro/webapp/controller/accounts/admin/UserAccountsEditPage.java
View file

@ -274,6 +274,7 @@ public class UserAccountsEditPage extends UserAccountsPage {
userAccount.setOldPassword("");
userAccount.setPasswordChangeRequired(false);
userAccount.setPasswordLinkExpires(0L);
userAccount.setEmailKey("");
}
if (isRootUser()) {

7
api/src/main/java/edu/cornell/mannlib/vitro/webapp/controller/accounts/admin/UserAccountsEditPageStrategy.java
View file

@ -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);

6
api/src/main/java/edu/cornell/mannlib/vitro/webapp/controller/accounts/user/UserAccountsCreatePasswordPage.java
View file

@ -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() {

1
api/src/main/java/edu/cornell/mannlib/vitro/webapp/controller/accounts/user/UserAccountsFirstTimeExternalPage.java
View file

@ -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);

1
api/src/main/java/edu/cornell/mannlib/vitro/webapp/controller/accounts/user/UserAccountsMyAccountPageStrategy.java
View file

@ -159,6 +159,7 @@ public abstract class UserAccountsMyAccountPageStrategy extends
userAccount.setMd5Password("");
userAccount.setPasswordChangeRequired(false);
userAccount.setPasswordLinkExpires(0L);
userAccount.setEmailKey("");
}
}

10
api/src/main/java/edu/cornell/mannlib/vitro/webapp/controller/accounts/user/UserAccountsPasswordBasePage.java
View file

@ -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();
}

5
api/src/main/java/edu/cornell/mannlib/vitro/webapp/controller/accounts/user/UserAccountsResetPasswordPage.java
View file

@ -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() {

1
api/src/main/java/edu/cornell/mannlib/vitro/webapp/controller/authenticate/BasicAuthenticator.java
View file

@ -134,6 +134,7 @@ public class BasicAuthenticator extends Authenticator {
userAccount.setMd5Password("");
userAccount.setPasswordChangeRequired(false);
userAccount.setPasswordLinkExpires(0L);
userAccount.setEmailKey("");
getUserAccountsDao().updateUserAccount(userAccount);
}

1
api/src/main/java/edu/cornell/mannlib/vitro/webapp/dao/VitroVocabulary.java
View file

@ -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";

1
api/src/main/java/edu/cornell/mannlib/vitro/webapp/dao/jena/JenaBaseDaoCon.java
View file

@ -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);

7
api/src/main/java/edu/cornell/mannlib/vitro/webapp/dao/jena/UserAccountsDaoJena.java
View file

@ -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 {

2
webapp/src/main/webapp/templates/freemarker/body/accounts/userAccounts-createPassword.ftl
View file

@ -26,7 +26,7 @@
<form method="POST" action="${formUrls.createPassword}" class="customForm" role="create password">
<input type="hidden" name="user" value="${userAccount.emailAddress}" role="input" />
<input type="hidden" name="key" value="${userAccount.passwordLinkExpiresHash}" role="input" />
<input type="hidden" name="key" value="${userAccount.emailKey}" role="input" />
<label for="new-password">${strings.new_password}<span class="requiredHint"> *</span></label>
<input type="password" name="newPassword" value="${newPassword}" id="new-password" role="input" />

2
webapp/src/main/webapp/templates/freemarker/body/accounts/userAccounts-resetPassword.ftl
View file

@ -26,7 +26,7 @@
<section id="reset-password" role="region">
<form method="POST" action="${formUrls.resetPassword}" class="customForm" role="create password">
<input type="hidden" name="user" value="${userAccount.emailAddress}" />
<input type="hidden" name="key" value="${userAccount.passwordLinkExpiresHash}" />
<input type="hidden" name="key" value="${userAccount.emailKey}" />
<label for="new-password">${strings.new_password}<span class="requiredHint"> *</span></label>
<input type="password" name="newPassword" value="${newPassword}" id="new-password" role="input" />