#472 Require recovery code before resetting password

- /email recovery generates recovery code and resets password only if recovery code is also given
- Change data source method to return email and recovery code
This commit is contained in:
ljacqu 2016-09-10 14:27:26 +02:00
parent 3b723bbbe9
commit c5f5c0d2fd
12 changed files with 224 additions and 68 deletions

View File

@ -0,0 +1,38 @@
package fr.xephi.authme.cache.auth;
/**
* Stored data for email recovery.
*/
public class EmailRecoveryData {
private final String email;
private final String recoveryCode;
/**
* Constructor.
*
* @param email the email address
* @param recoveryCode the recovery code, or null if not available
* @param codeExpiration
*/
public EmailRecoveryData(String email, String recoveryCode, Long codeExpiration) {
this.email = email;
this.recoveryCode = codeExpiration == null || System.currentTimeMillis() > codeExpiration
? null
: recoveryCode;
}
/**
* @return the email address
*/
public String getEmail() {
return email;
}
/**
* @return the recovery code, if available and not expired
*/
public String getRecoveryCode() {
return recoveryCode;
}
}

View File

@ -384,6 +384,7 @@ public class CommandInitializer {
.detailedDescription("Recover your account using an Email address by sending a mail containing " +
"a new password.")
.withArgument("email", "Email address", false)
.withArgument("code", "Recovery code", true)
.permission(PlayerPermission.RECOVER_EMAIL)
.executableCommand(RecoverEmailCommand.class)
.build();

View File

@ -1,7 +1,7 @@
package fr.xephi.authme.command.executable.email;
import fr.xephi.authme.ConsoleLogger;
import fr.xephi.authme.cache.auth.PlayerAuth;
import fr.xephi.authme.cache.auth.EmailRecoveryData;
import fr.xephi.authme.cache.auth.PlayerCache;
import fr.xephi.authme.command.CommandService;
import fr.xephi.authme.command.PlayerCommand;
@ -17,6 +17,9 @@ import org.bukkit.entity.Player;
import javax.inject.Inject;
import java.util.List;
/**
* Command for password recovery by email.
*/
public class RecoverEmailCommand extends PlayerCommand {
@Inject
@ -49,22 +52,47 @@ public class RecoverEmailCommand extends PlayerCommand {
return;
}
PlayerAuth auth = dataSource.getAuth(playerName);
if (auth == null) {
EmailRecoveryData recoveryData = dataSource.getEmailRecoveryData(playerName);
if (recoveryData == null) {
commandService.send(player, MessageKey.REGISTER_EMAIL_MESSAGE);
return;
}
if (!playerMail.equalsIgnoreCase(auth.getEmail()) || "your@email.com".equalsIgnoreCase(auth.getEmail())) {
final String email = recoveryData.getEmail();
if (email == null || !email.equalsIgnoreCase(playerMail) || "your@email.com".equalsIgnoreCase(email)) {
commandService.send(player, MessageKey.INVALID_EMAIL);
return;
}
if (arguments.size() == 1) {
// Process /email recover addr@example.com
createAndSendRecoveryCode(playerName, recoveryData);
} else {
// Process /email recover addr@example.com 12394
processRecoveryCode(player, arguments.get(1), recoveryData);
}
}
private void createAndSendRecoveryCode(String name, EmailRecoveryData recoveryData) {
// TODO #472: Add configurations
String recoveryCode = RandomString.generateHex(8);
long expiration = System.currentTimeMillis() + (3 * 60 * 60_000L); // 3 hours
dataSource.setRecoveryCode(name, recoveryCode, expiration);
sendMailSsl.sendRecoveryCode(recoveryData.getEmail(), recoveryCode);
}
private void processRecoveryCode(Player player, String code, EmailRecoveryData recoveryData) {
if (!code.equals(recoveryData.getRecoveryCode())) {
player.sendMessage("The recovery code is not correct! Use /email recovery [email] to generate a new one");
return;
}
final String name = player.getName();
String thePass = RandomString.generate(commandService.getProperty(EmailSettings.RECOVERY_PASSWORD_LENGTH));
HashedPassword hashNew = passwordSecurity.computeHash(thePass, playerName);
auth.setPassword(hashNew);
dataSource.updatePassword(auth);
sendMailSsl.sendPasswordMail(auth, thePass);
HashedPassword hashNew = passwordSecurity.computeHash(thePass, name);
dataSource.updatePassword(name, hashNew);
dataSource.removeRecoveryCode(name);
sendMailSsl.sendPasswordMail(name, recoveryData.getEmail(), thePass);
commandService.send(player, MessageKey.RECOVERY_EMAIL_SENT_MESSAGE);
}
}

View File

@ -10,6 +10,7 @@ import com.google.common.util.concurrent.MoreExecutors;
import com.google.common.util.concurrent.ThreadFactoryBuilder;
import fr.xephi.authme.ConsoleLogger;
import fr.xephi.authme.cache.auth.EmailRecoveryData;
import fr.xephi.authme.cache.auth.PlayerAuth;
import fr.xephi.authme.cache.auth.PlayerCache;
import fr.xephi.authme.security.crypts.HashedPassword;
@ -242,9 +243,8 @@ public class CacheDataSource implements DataSource {
}
@Override
public String getRecoveryCode(String name) {
// TODO #472: can probably get it from the cached Auth?
return source.getRecoveryCode(name);
public EmailRecoveryData getEmailRecoveryData(String name) {
return source.getEmailRecoveryData(name);
}
@Override

View File

@ -1,5 +1,6 @@
package fr.xephi.authme.datasource;
import fr.xephi.authme.cache.auth.EmailRecoveryData;
import fr.xephi.authme.cache.auth.PlayerAuth;
import fr.xephi.authme.initialization.Reloadable;
import fr.xephi.authme.security.crypts.HashedPassword;
@ -204,12 +205,12 @@ public interface DataSource extends Reloadable {
void setRecoveryCode(String name, String code, long expiration);
/**
* Get the recovery code of a user if available and not yet expired.
* Get the information necessary for performing a password recovery by email.
*
* @param name The name of the user
* @return The recovery code, or null if no current code available
* @return The data of the player, or null if player doesn't exist
*/
String getRecoveryCode(String name);
EmailRecoveryData getEmailRecoveryData(String name);
/**
* Remove the recovery code of a given user.

View File

@ -1,6 +1,7 @@
package fr.xephi.authme.datasource;
import fr.xephi.authme.ConsoleLogger;
import fr.xephi.authme.cache.auth.EmailRecoveryData;
import fr.xephi.authme.cache.auth.PlayerAuth;
import fr.xephi.authme.cache.auth.PlayerCache;
import fr.xephi.authme.security.crypts.HashedPassword;
@ -474,7 +475,7 @@ public class FlatFile implements DataSource {
}
@Override
public String getRecoveryCode(String name) {
public EmailRecoveryData getEmailRecoveryData(String name) {
throw new UnsupportedOperationException("Flat file no longer supported");
}

View File

@ -4,6 +4,7 @@ import com.google.common.annotations.VisibleForTesting;
import com.zaxxer.hikari.HikariDataSource;
import com.zaxxer.hikari.pool.HikariPool.PoolInitializationException;
import fr.xephi.authme.ConsoleLogger;
import fr.xephi.authme.cache.auth.EmailRecoveryData;
import fr.xephi.authme.cache.auth.PlayerAuth;
import fr.xephi.authme.security.HashAlgorithm;
import fr.xephi.authme.security.crypts.HashedPassword;
@ -881,15 +882,16 @@ public class MySQL implements DataSource {
}
@Override
public String getRecoveryCode(String name) {
String sql = "SELECT " + col.RECOVERY_CODE + " FROM " + tableName
+ " WHERE " + col.NAME + " = ? AND " + col.RECOVERY_EXPIRATION + " > ?;";
public EmailRecoveryData getEmailRecoveryData(String name) {
String sql = "SELECT " + col.EMAIL + ", " + col.RECOVERY_CODE + ", " + col.RECOVERY_EXPIRATION
+ " FROM " + tableName
+ " WHERE " + col.NAME + " = ?;";
try (Connection con = getConnection(); PreparedStatement pst = con.prepareStatement(sql)) {
pst.setString(1, name.toLowerCase());
pst.setLong(2, System.currentTimeMillis());
try (ResultSet rs = pst.executeQuery()) {
if (rs.next()) {
return rs.getString(1);
return new EmailRecoveryData(
rs.getString(col.EMAIL), rs.getString(col.RECOVERY_CODE), rs.getLong(col.RECOVERY_EXPIRATION));
}
}
} catch (SQLException e) {

View File

@ -2,6 +2,7 @@ package fr.xephi.authme.datasource;
import com.google.common.annotations.VisibleForTesting;
import fr.xephi.authme.ConsoleLogger;
import fr.xephi.authme.cache.auth.EmailRecoveryData;
import fr.xephi.authme.cache.auth.PlayerAuth;
import fr.xephi.authme.security.crypts.HashedPassword;
import fr.xephi.authme.settings.Settings;
@ -611,15 +612,16 @@ public class SQLite implements DataSource {
}
@Override
public String getRecoveryCode(String name) {
String sql = "SELECT " + col.RECOVERY_CODE + " FROM " + tableName
+ " WHERE " + col.NAME + " = ? AND " + col.RECOVERY_EXPIRATION + " > ?;";
public EmailRecoveryData getEmailRecoveryData(String name) {
String sql = "SELECT " + col.EMAIL + ", " + col.RECOVERY_CODE + ", " + col.RECOVERY_EXPIRATION
+ " FROM " + tableName
+ " WHERE " + col.NAME + " = ?;";
try (PreparedStatement pst = con.prepareStatement(sql)) {
pst.setString(1, name.toLowerCase());
pst.setLong(2, System.currentTimeMillis());
try (ResultSet rs = pst.executeQuery()) {
if (rs.next()) {
return rs.getString(1);
return new EmailRecoveryData(
rs.getString(col.EMAIL), rs.getString(col.RECOVERY_CODE), rs.getLong(col.RECOVERY_EXPIRATION));
}
}
} catch (SQLException e) {

View File

@ -2,7 +2,6 @@ package fr.xephi.authme.mail;
import fr.xephi.authme.AuthMe;
import fr.xephi.authme.ConsoleLogger;
import fr.xephi.authme.cache.auth.PlayerAuth;
import fr.xephi.authme.settings.Settings;
import fr.xephi.authme.settings.properties.EmailSettings;
import fr.xephi.authme.util.BukkitService;
@ -53,16 +52,17 @@ public class SendMailSSL {
/**
* Sends an email to the user with his new password.
*
* @param auth the player auth of the player
* @param name the name of the player
* @param mailAddress the player's email
* @param newPass the new password
*/
public void sendPasswordMail(final PlayerAuth auth, final String newPass) {
public void sendPasswordMail(String name, String mailAddress, String newPass) {
if (!hasAllInformation()) {
ConsoleLogger.warning("Cannot perform email registration: not all email settings are complete");
return;
}
final String mailText = replaceMailTags(settings.getEmailMessage(), auth, newPass);
final String mailText = replaceMailTags(settings.getEmailMessage(), name, newPass);
bukkitService.runTaskAsynchronously(new Runnable() {
@Override
@ -70,7 +70,7 @@ public class SendMailSSL {
Thread.currentThread().setContextClassLoader(this.getClass().getClassLoader());
HtmlEmail email;
try {
email = initializeMail(auth.getEmail());
email = initializeMail(mailAddress);
} catch (EmailException e) {
ConsoleLogger.logException("Failed to create email with the given settings:", e);
return;
@ -81,11 +81,11 @@ public class SendMailSSL {
File file = null;
if (settings.getProperty(EmailSettings.PASSWORD_AS_IMAGE)) {
try {
file = generateImage(auth.getNickname(), plugin, newPass);
file = generateImage(name, plugin, newPass);
content = embedImageIntoEmailContent(file, email, content);
} catch (IOException | EmailException e) {
ConsoleLogger.logException(
"Unable to send new password as image for email " + auth.getEmail() + ":", e);
"Unable to send new password as image for email " + mailAddress + ":", e);
}
}
@ -97,6 +97,20 @@ public class SendMailSSL {
});
}
public void sendRecoveryCode(String email, String code) {
// TODO #472: Create a configurable, more verbose message
String message = String.format("Use /email recovery %s %s to reset your password", email, code);
HtmlEmail htmlEmail;
try {
htmlEmail = initializeMail(email);
} catch (EmailException e) {
ConsoleLogger.logException("Failed to create email for recovery code:", e);
return;
}
sendEmail(message, htmlEmail);
}
private static File generateImage(String name, AuthMe plugin, String newPass) throws IOException {
ImageGenerator gen = new ImageGenerator(newPass);
File file = new File(plugin.getDataFolder(), name + "_new_pass.jpg");
@ -149,9 +163,9 @@ public class SendMailSSL {
}
}
private String replaceMailTags(String mailText, PlayerAuth auth, String newPass) {
private String replaceMailTags(String mailText, String name, String newPass) {
return mailText
.replace("<playername />", auth.getNickname())
.replace("<playername />", name)
.replace("<servername />", plugin.getServer().getServerName())
.replace("<generatedpass />", newPass);
}

View File

@ -149,7 +149,7 @@ public class AsyncRegister implements AsynchronousProcess {
}
database.updateEmail(auth);
database.updateSession(auth);
sendMailSsl.sendPasswordMail(auth, password);
sendMailSsl.sendPasswordMail(name, email, password);
syncProcessManager.processSyncEmailRegister(player);
}

View File

@ -1,7 +1,7 @@
package fr.xephi.authme.command.executable.email;
import fr.xephi.authme.TestHelper;
import fr.xephi.authme.cache.auth.PlayerAuth;
import fr.xephi.authme.cache.auth.EmailRecoveryData;
import fr.xephi.authme.cache.auth.PlayerCache;
import fr.xephi.authme.command.CommandService;
import fr.xephi.authme.datasource.DataSource;
@ -14,21 +14,25 @@ import org.bukkit.entity.Player;
import org.junit.BeforeClass;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.ArgumentCaptor;
import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.invocation.InvocationOnMock;
import org.mockito.runners.MockitoJUnitRunner;
import org.mockito.stubbing.Answer;
import java.util.Arrays;
import java.util.Collections;
import static fr.xephi.authme.AuthMeMatchers.stringWithLength;
import static org.hamcrest.Matchers.containsString;
import static org.junit.Assert.assertThat;
import static org.mockito.BDDMockito.given;
import static org.mockito.Matchers.any;
import static org.mockito.Matchers.anyLong;
import static org.mockito.Matchers.anyString;
import static org.mockito.Matchers.argThat;
import static org.mockito.Matchers.eq;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.only;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyNoMoreInteractions;
import static org.mockito.Mockito.verifyZeroInteractions;
@ -104,14 +108,14 @@ public class RecoverEmailCommandTest {
given(sender.getName()).willReturn(name);
given(sendMailSsl.hasAllInformation()).willReturn(true);
given(playerCache.isAuthenticated(name)).willReturn(false);
given(dataSource.getAuth(name)).willReturn(null);
given(dataSource.getEmailRecoveryData(name)).willReturn(null);
// when
command.executeCommand(sender, Collections.singletonList("someone@example.com"));
// then
verify(sendMailSsl).hasAllInformation();
verify(dataSource).getAuth(name);
verify(dataSource).getEmailRecoveryData(name);
verifyNoMoreInteractions(dataSource);
verify(commandService).send(sender, MessageKey.REGISTER_EMAIL_MESSAGE);
}
@ -124,14 +128,14 @@ public class RecoverEmailCommandTest {
given(sender.getName()).willReturn(name);
given(sendMailSsl.hasAllInformation()).willReturn(true);
given(playerCache.isAuthenticated(name)).willReturn(false);
given(dataSource.getAuth(name)).willReturn(authWithEmail(DEFAULT_EMAIL));
given(dataSource.getEmailRecoveryData(name)).willReturn(newEmailRecoveryData(DEFAULT_EMAIL));
// when
command.executeCommand(sender, Collections.singletonList(DEFAULT_EMAIL));
// then
verify(sendMailSsl).hasAllInformation();
verify(dataSource).getAuth(name);
verify(dataSource).getEmailRecoveryData(name);
verifyNoMoreInteractions(dataSource);
verify(commandService).send(sender, MessageKey.INVALID_EMAIL);
}
@ -144,18 +148,67 @@ public class RecoverEmailCommandTest {
given(sender.getName()).willReturn(name);
given(sendMailSsl.hasAllInformation()).willReturn(true);
given(playerCache.isAuthenticated(name)).willReturn(false);
given(dataSource.getAuth(name)).willReturn(authWithEmail("raptor@example.org"));
given(dataSource.getEmailRecoveryData(name)).willReturn(newEmailRecoveryData("raptor@example.org"));
// when
command.executeCommand(sender, Collections.singletonList("wrong-email@example.com"));
// then
verify(sendMailSsl).hasAllInformation();
verify(dataSource).getAuth(name);
verify(dataSource).getEmailRecoveryData(name);
verifyNoMoreInteractions(dataSource);
verify(commandService).send(sender, MessageKey.INVALID_EMAIL);
}
@Test
public void shouldGenerateRecoveryCode() {
// given
String name = "Vultur3";
Player sender = mock(Player.class);
given(sender.getName()).willReturn(name);
given(sendMailSsl.hasAllInformation()).willReturn(true);
given(playerCache.isAuthenticated(name)).willReturn(false);
String email = "v@example.com";
given(dataSource.getEmailRecoveryData(name)).willReturn(newEmailRecoveryData(email));
// when
command.executeCommand(sender, Collections.singletonList(email.toUpperCase()));
// then
verify(sendMailSsl).hasAllInformation();
verify(dataSource).getEmailRecoveryData(name);
ArgumentCaptor<String> codeCaptor = ArgumentCaptor.forClass(String.class);
verify(dataSource).setRecoveryCode(eq(name), codeCaptor.capture(), anyLong());
assertThat(codeCaptor.getValue(), stringWithLength(8));
verify(sendMailSsl).sendRecoveryCode(email, codeCaptor.getValue());
}
@Test
public void shouldSendErrorForInvalidRecoveryCode() {
// given
String name = "Vultur3";
Player sender = mock(Player.class);
given(sender.getName()).willReturn(name);
given(sendMailSsl.hasAllInformation()).willReturn(true);
given(playerCache.isAuthenticated(name)).willReturn(false);
String email = "vulture@example.com";
String code = "A6EF3AC8";
EmailRecoveryData recoveryData = newEmailRecoveryData(email, code);
given(dataSource.getEmailRecoveryData(name)).willReturn(recoveryData);
given(commandService.getProperty(EmailSettings.RECOVERY_PASSWORD_LENGTH)).willReturn(20);
given(passwordSecurity.computeHash(anyString(), eq(name)))
.willAnswer(invocation -> new HashedPassword((String) invocation.getArguments()[0]));
// when
command.executeCommand(sender, Arrays.asList(email, "bogus"));
// then
verify(sendMailSsl).hasAllInformation();
verify(dataSource, only()).getEmailRecoveryData(name);
verify(sender).sendMessage(argThat(containsString("The recovery code is not correct")));
verifyNoMoreInteractions(sendMailSsl);
}
@Test
public void shouldResetPasswordAndSendEmail() {
// given
@ -165,36 +218,36 @@ public class RecoverEmailCommandTest {
given(sendMailSsl.hasAllInformation()).willReturn(true);
given(playerCache.isAuthenticated(name)).willReturn(false);
String email = "vulture@example.com";
PlayerAuth auth = authWithEmail(email);
given(dataSource.getAuth(name)).willReturn(auth);
String code = "A6EF3AC8";
EmailRecoveryData recoveryData = newEmailRecoveryData(email, code);
given(dataSource.getEmailRecoveryData(name)).willReturn(recoveryData);
given(commandService.getProperty(EmailSettings.RECOVERY_PASSWORD_LENGTH)).willReturn(20);
given(passwordSecurity.computeHash(anyString(), eq(name)))
.willAnswer(new Answer<HashedPassword>() {
@Override
public HashedPassword answer(InvocationOnMock invocationOnMock) {
return new HashedPassword((String) invocationOnMock.getArguments()[0]);
}
});
.willAnswer(invocation -> new HashedPassword((String) invocation.getArguments()[0]));
// when
command.executeCommand(sender, Collections.singletonList(email.toUpperCase()));
command.executeCommand(sender, Arrays.asList(email, code));
// then
verify(sendMailSsl).hasAllInformation();
verify(dataSource).getAuth(name);
verify(passwordSecurity).computeHash(anyString(), eq(name));
verify(dataSource).updatePassword(auth);
assertThat(auth.getPassword().getHash(), stringWithLength(20));
verify(sendMailSsl).sendPasswordMail(eq(auth), argThat(stringWithLength(20)));
verify(dataSource).getEmailRecoveryData(name);
ArgumentCaptor<String> passwordCaptor = ArgumentCaptor.forClass(String.class);
verify(passwordSecurity).computeHash(passwordCaptor.capture(), eq(name));
String generatedPassword = passwordCaptor.getValue();
assertThat(generatedPassword, stringWithLength(20));
verify(dataSource).updatePassword(eq(name), any(HashedPassword.class));
verify(dataSource).removeRecoveryCode(name);
verify(sendMailSsl).sendPasswordMail(name, email, generatedPassword);
verify(commandService).send(sender, MessageKey.RECOVERY_EMAIL_SENT_MESSAGE);
}
private static PlayerAuth authWithEmail(String email) {
return PlayerAuth.builder()
.name("tester")
.email(email)
.build();
private static EmailRecoveryData newEmailRecoveryData(String email) {
return new EmailRecoveryData(email, null, 0L);
}
private static EmailRecoveryData newEmailRecoveryData(String email, String code) {
return new EmailRecoveryData(email, code, System.currentTimeMillis() + 10_000);
}
}

View File

@ -1,5 +1,6 @@
package fr.xephi.authme.datasource;
import fr.xephi.authme.cache.auth.EmailRecoveryData;
import fr.xephi.authme.cache.auth.PlayerAuth;
import fr.xephi.authme.security.crypts.HashedPassword;
import org.junit.Test;
@ -393,7 +394,7 @@ public abstract class AbstractDataSourceIntegrationTest {
dataSource.setRecoveryCode(name, code, System.currentTimeMillis() + 100_000L);
// then
assertThat(dataSource.getRecoveryCode(name), equalTo(code));
assertThat(dataSource.getEmailRecoveryData(name).getRecoveryCode(), equalTo(code));
}
@Test
@ -407,8 +408,10 @@ public abstract class AbstractDataSourceIntegrationTest {
dataSource.removeRecoveryCode(name);
// then
assertThat(dataSource.getRecoveryCode(name), nullValue());
assertThat(dataSource.getRecoveryCode("bobby"), nullValue());
EmailRecoveryData recoveryData = dataSource.getEmailRecoveryData(name);
assertThat(recoveryData.getRecoveryCode(), nullValue());
assertThat(recoveryData.getEmail(), equalTo("user@example.org"));
assertThat(dataSource.getEmailRecoveryData("bobby").getRecoveryCode(), nullValue());
}
@Test
@ -419,10 +422,23 @@ public abstract class AbstractDataSourceIntegrationTest {
dataSource.setRecoveryCode(name, "123456", System.currentTimeMillis() - 2_000L);
// when
String code = dataSource.getRecoveryCode(name);
EmailRecoveryData recoveryData = dataSource.getEmailRecoveryData(name);
// then
assertThat(code, nullValue());
assertThat(recoveryData.getEmail(), equalTo("user@example.org"));
assertThat(recoveryData.getRecoveryCode(), nullValue());
}
@Test
public void shouldReturnNullForNoAvailableUser() {
// given
DataSource dataSource = getDataSource();
// when
EmailRecoveryData result = dataSource.getEmailRecoveryData("does-not-exist");
// then
assertThat(result, nullValue());
}
}