From 0aac8928afe6efcffd7251518b9fcd2a54a63bc2 Mon Sep 17 00:00:00 2001 From: ljacqu Date: Sat, 10 Sep 2016 09:13:17 +0200 Subject: [PATCH 1/7] #472 Create recovery code/expiration columns and methods in data source --- .../authme/datasource/CacheDataSource.java | 18 ++++++ .../fr/xephi/authme/datasource/Columns.java | 4 ++ .../xephi/authme/datasource/DataSource.java | 24 ++++++++ .../fr/xephi/authme/datasource/FlatFile.java | 15 +++++ .../fr/xephi/authme/datasource/MySQL.java | 56 +++++++++++++++++++ .../fr/xephi/authme/datasource/SQLite.java | 56 +++++++++++++++++++ .../settings/properties/DatabaseSettings.java | 8 +++ src/main/resources/config.yml | 4 ++ src/test/java/fr/xephi/authme/TestHelper.java | 11 ++++ .../AbstractDataSourceIntegrationTest.java | 43 ++++++++++++++ .../datasource/MySqlIntegrationTest.java | 2 +- .../datasource/SQLiteIntegrationTest.java | 2 +- .../authme/datasource/sql-initialize.sql | 2 + 13 files changed, 243 insertions(+), 2 deletions(-) diff --git a/src/main/java/fr/xephi/authme/datasource/CacheDataSource.java b/src/main/java/fr/xephi/authme/datasource/CacheDataSource.java index a79cb580..36372430 100644 --- a/src/main/java/fr/xephi/authme/datasource/CacheDataSource.java +++ b/src/main/java/fr/xephi/authme/datasource/CacheDataSource.java @@ -235,6 +235,24 @@ public class CacheDataSource implements DataSource { return source.getAllAuths(); } + @Override + public void setRecoveryCode(String name, String code, long expiration) { + source.setRecoveryCode(name, code, expiration); + cachedAuths.refresh(name); + } + + @Override + public String getRecoveryCode(String name) { + // TODO #472: can probably get it from the cached Auth? + return source.getRecoveryCode(name); + } + + @Override + public void removeRecoveryCode(String name) { + source.removeRecoveryCode(name); + cachedAuths.refresh(name); + } + @Override public List getLoggedPlayers() { return new ArrayList<>(PlayerCache.getInstance().getCache().values()); diff --git a/src/main/java/fr/xephi/authme/datasource/Columns.java b/src/main/java/fr/xephi/authme/datasource/Columns.java index b6d732cd..d56aae72 100644 --- a/src/main/java/fr/xephi/authme/datasource/Columns.java +++ b/src/main/java/fr/xephi/authme/datasource/Columns.java @@ -22,6 +22,8 @@ public final class Columns { public final String EMAIL; public final String ID; public final String IS_LOGGED; + public final String RECOVERY_CODE; + public final String RECOVERY_EXPIRATION; public Columns(Settings settings) { NAME = settings.getProperty(DatabaseSettings.MYSQL_COL_NAME); @@ -38,6 +40,8 @@ public final class Columns { EMAIL = settings.getProperty(DatabaseSettings.MYSQL_COL_EMAIL); ID = settings.getProperty(DatabaseSettings.MYSQL_COL_ID); IS_LOGGED = settings.getProperty(DatabaseSettings.MYSQL_COL_ISLOGGED); + RECOVERY_CODE = settings.getProperty(DatabaseSettings.MYSQL_COL_RECOVERY_CODE); + RECOVERY_EXPIRATION = settings.getProperty(DatabaseSettings.MYSQL_COL_RECOVERY_EXPIRATION); } } diff --git a/src/main/java/fr/xephi/authme/datasource/DataSource.java b/src/main/java/fr/xephi/authme/datasource/DataSource.java index 5b25fde4..96788d83 100644 --- a/src/main/java/fr/xephi/authme/datasource/DataSource.java +++ b/src/main/java/fr/xephi/authme/datasource/DataSource.java @@ -194,6 +194,30 @@ public interface DataSource extends Reloadable { */ List getAllAuths(); + /** + * Set the password recovery code for a user. + * + * @param name The name of the user + * @param code The recovery code + * @param expiration Recovery code expiration (milliseconds timestamp) + */ + void setRecoveryCode(String name, String code, long expiration); + + /** + * Get the recovery code of a user if available and not yet expired. + * + * @param name The name of the user + * @return The recovery code, or null if no current code available + */ + String getRecoveryCode(String name); + + /** + * Remove the recovery code of a given user. + * + * @param name The name of the user + */ + void removeRecoveryCode(String name); + /** * Reload the data source. */ diff --git a/src/main/java/fr/xephi/authme/datasource/FlatFile.java b/src/main/java/fr/xephi/authme/datasource/FlatFile.java index f3f18014..d600a3e6 100644 --- a/src/main/java/fr/xephi/authme/datasource/FlatFile.java +++ b/src/main/java/fr/xephi/authme/datasource/FlatFile.java @@ -468,6 +468,21 @@ public class FlatFile implements DataSource { throw new UnsupportedOperationException("Flat file no longer supported"); } + @Override + public void setRecoveryCode(String name, String code, long expiration) { + throw new UnsupportedOperationException("Flat file no longer supported"); + } + + @Override + public String getRecoveryCode(String name) { + throw new UnsupportedOperationException("Flat file no longer supported"); + } + + @Override + public void removeRecoveryCode(String name) { + throw new UnsupportedOperationException("Flat file no longer supported"); + } + private static PlayerAuth buildAuthFromArray(String[] args) { // Format allows 2, 3, 4, 7, 8, 9 fields. Anything else is unknown if (args.length >= 2 && args.length <= 9 && args.length != 5 && args.length != 6) { diff --git a/src/main/java/fr/xephi/authme/datasource/MySQL.java b/src/main/java/fr/xephi/authme/datasource/MySQL.java index dc08d7b4..5b944044 100644 --- a/src/main/java/fr/xephi/authme/datasource/MySQL.java +++ b/src/main/java/fr/xephi/authme/datasource/MySQL.java @@ -208,6 +208,14 @@ public class MySQL implements DataSource { st.executeUpdate("ALTER TABLE " + tableName + " ADD COLUMN " + col.IS_LOGGED + " SMALLINT NOT NULL DEFAULT '0' AFTER " + col.EMAIL); } + + if (isColumnMissing(md, col.RECOVERY_CODE)) { + st.executeUpdate("ALTER TABLE " + tableName + " ADD COLUMN " + col.RECOVERY_CODE + " VARCHAR(20);"); + } + + if (isColumnMissing(md, col.RECOVERY_EXPIRATION)) { + st.executeUpdate("ALTER TABLE " + tableName + " ADD COLUMN " + col.RECOVERY_EXPIRATION + " BIGINT;"); + } } ConsoleLogger.info("MySQL setup finished"); } @@ -856,6 +864,54 @@ public class MySQL implements DataSource { return auths; } + @Override + public void setRecoveryCode(String name, String code, long expiration) { + String sql = "UPDATE " + tableName + + " SET " + col.RECOVERY_CODE + " = ?, " + + col.RECOVERY_EXPIRATION + " = ?" + + " WHERE " + col.NAME + " = ?;"; + try (Connection con = getConnection(); PreparedStatement pst = con.prepareStatement(sql)) { + pst.setString(1, code); + pst.setLong(2, expiration); + pst.setString(3, name.toLowerCase()); + pst.executeUpdate(); + } catch (SQLException e) { + logSqlException(e); + } + } + + @Override + public String getRecoveryCode(String name) { + String sql = "SELECT " + col.RECOVERY_CODE + " FROM " + tableName + + " WHERE " + col.NAME + " = ? AND " + col.RECOVERY_EXPIRATION + " > ?;"; + 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); + } + } + } catch (SQLException e) { + logSqlException(e); + } + return null; + } + + @Override + public void removeRecoveryCode(String name) { + String sql = "UPDATE " + tableName + + " SET " + col.RECOVERY_CODE + " = NULL" + + " AND " + col.RECOVERY_EXPIRATION + " = NULL" + + " WHERE " + col.NAME + " = ?;"; + try (Connection con = getConnection(); PreparedStatement pst = con.prepareStatement(sql)) { + pst.setString(1, name.toLowerCase()); + pst.executeUpdate(); + } catch (SQLException e) { + logSqlException(e); + } + } + private PlayerAuth buildAuthFromResultSet(ResultSet row) throws SQLException { String salt = col.SALT.isEmpty() ? null : row.getString(col.SALT); int group = col.GROUP.isEmpty() ? -1 : row.getInt(col.GROUP); diff --git a/src/main/java/fr/xephi/authme/datasource/SQLite.java b/src/main/java/fr/xephi/authme/datasource/SQLite.java index 8aae94c5..df90d2eb 100644 --- a/src/main/java/fr/xephi/authme/datasource/SQLite.java +++ b/src/main/java/fr/xephi/authme/datasource/SQLite.java @@ -127,6 +127,14 @@ public class SQLite implements DataSource { if (isColumnMissing(md, col.IS_LOGGED)) { st.executeUpdate("ALTER TABLE " + tableName + " ADD COLUMN " + col.IS_LOGGED + " INT DEFAULT '0';"); } + + if (isColumnMissing(md, col.RECOVERY_CODE)) { + st.executeUpdate("ALTER TABLE " + tableName + " ADD COLUMN " + col.RECOVERY_CODE + " VARCHAR(20);"); + } + + if (isColumnMissing(md, col.RECOVERY_EXPIRATION)) { + st.executeUpdate("ALTER TABLE " + tableName + " ADD COLUMN " + col.RECOVERY_EXPIRATION + " BIGINT;"); + } } ConsoleLogger.info("SQLite Setup finished"); } @@ -586,6 +594,54 @@ public class SQLite implements DataSource { return auths; } + @Override + public void setRecoveryCode(String name, String code, long expiration) { + String sql = "UPDATE " + tableName + + " SET " + col.RECOVERY_CODE + " = ?, " + + col.RECOVERY_EXPIRATION + " = ?" + + " WHERE " + col.NAME + " = ?;"; + try (PreparedStatement pst = con.prepareStatement(sql)) { + pst.setString(1, code); + pst.setLong(2, expiration); + pst.setString(3, name.toLowerCase()); + pst.executeUpdate(); + } catch (SQLException e) { + logSqlException(e); + } + } + + @Override + public String getRecoveryCode(String name) { + String sql = "SELECT " + col.RECOVERY_CODE + " FROM " + tableName + + " WHERE " + col.NAME + " = ? AND " + col.RECOVERY_EXPIRATION + " > ?;"; + 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); + } + } + } catch (SQLException e) { + logSqlException(e); + } + return null; + } + + @Override + public void removeRecoveryCode(String name) { + String sql = "UPDATE " + tableName + + " SET " + col.RECOVERY_CODE + " = NULL" + + " AND " + col.RECOVERY_EXPIRATION + " = NULL" + + " WHERE " + col.NAME + " = ?;"; + try (PreparedStatement pst = con.prepareStatement(sql)) { + pst.setString(1, name.toLowerCase()); + pst.executeUpdate(); + } catch (SQLException e) { + logSqlException(e); + } + } + private PlayerAuth buildAuthFromResultSet(ResultSet row) throws SQLException { String salt = !col.SALT.isEmpty() ? row.getString(col.SALT) : null; diff --git a/src/main/java/fr/xephi/authme/settings/properties/DatabaseSettings.java b/src/main/java/fr/xephi/authme/settings/properties/DatabaseSettings.java index 2dc769ec..9b76c09c 100644 --- a/src/main/java/fr/xephi/authme/settings/properties/DatabaseSettings.java +++ b/src/main/java/fr/xephi/authme/settings/properties/DatabaseSettings.java @@ -98,6 +98,14 @@ public class DatabaseSettings implements SettingsHolder { public static final Property MYSQL_COL_GROUP = newProperty("ExternalBoardOptions.mySQLColumnGroup", ""); + @Comment("Column for storing recovery code (when password lost)") + public static final Property MYSQL_COL_RECOVERY_CODE = + newProperty("DataSource.mySQLrecoveryCode", "recoverycode"); + + @Comment("Column for storing recovery code expiration") + public static final Property MYSQL_COL_RECOVERY_EXPIRATION = + newProperty("DataSource.mySQLrecoveryExpiration", "recoveryexpiration"); + private DatabaseSettings() { } diff --git a/src/main/resources/config.yml b/src/main/resources/config.yml index 07f3c3c7..a744f43c 100644 --- a/src/main/resources/config.yml +++ b/src/main/resources/config.yml @@ -40,6 +40,10 @@ DataSource: mySQLlastlocWorld: world # Column for RealName mySQLRealName: realname + # Column for storing recovery code (when password lost) + mySQLrecoveryCode: recoverycode + # Column for storing recovery code expiration + mySQLrecoveryExpiration: recoveryexpiration settings: # The name shown in the help messages. helpHeader: AuthMeReloaded diff --git a/src/test/java/fr/xephi/authme/TestHelper.java b/src/test/java/fr/xephi/authme/TestHelper.java index e6435b5a..f0f9da8d 100644 --- a/src/test/java/fr/xephi/authme/TestHelper.java +++ b/src/test/java/fr/xephi/authme/TestHelper.java @@ -126,6 +126,17 @@ public final class TestHelper { return logger; } + /** + * Set ConsoleLogger to use a new real logger. + * + * @return The real logger used by ConsoleLogger + */ + public static Logger setRealLogger() { + Logger logger = Logger.getAnonymousLogger(); + ConsoleLogger.setLogger(logger); + return logger; + } + /** * Check that a class only has a hidden, zero-argument constructor, preventing the * instantiation of such classes (utility classes). Invokes the hidden constructor diff --git a/src/test/java/fr/xephi/authme/datasource/AbstractDataSourceIntegrationTest.java b/src/test/java/fr/xephi/authme/datasource/AbstractDataSourceIntegrationTest.java index 7fbccdd4..9c417f62 100644 --- a/src/test/java/fr/xephi/authme/datasource/AbstractDataSourceIntegrationTest.java +++ b/src/test/java/fr/xephi/authme/datasource/AbstractDataSourceIntegrationTest.java @@ -382,4 +382,47 @@ public abstract class AbstractDataSourceIntegrationTest { assertThat(dataSource.getAllAuths(), empty()); } + @Test + public void shouldSetRecoveryCode() { + // given + DataSource dataSource = getDataSource(); + String name = "Bobby"; + String code = "A123BC"; + + // when + dataSource.setRecoveryCode(name, code, System.currentTimeMillis() + 100_000L); + + // then + assertThat(dataSource.getRecoveryCode(name), equalTo(code)); + } + + @Test + public void shouldRemoveRecoveryCode() { + // given + String name = "User"; + DataSource dataSource = getDataSource(); + dataSource.setRecoveryCode(name, "code", System.currentTimeMillis() + 20_000L); + + // when + dataSource.removeRecoveryCode(name); + + // then + assertThat(dataSource.getRecoveryCode(name), nullValue()); + assertThat(dataSource.getRecoveryCode("bobby"), nullValue()); + } + + @Test + public void shouldNotReturnRecoveryCodeIfExpired() { + // given + String name = "user"; + DataSource dataSource = getDataSource(); + dataSource.setRecoveryCode(name, "123456", System.currentTimeMillis() - 2_000L); + + // when + String code = dataSource.getRecoveryCode(name); + + // then + assertThat(code, nullValue()); + } + } diff --git a/src/test/java/fr/xephi/authme/datasource/MySqlIntegrationTest.java b/src/test/java/fr/xephi/authme/datasource/MySqlIntegrationTest.java index ca808921..93d8a399 100644 --- a/src/test/java/fr/xephi/authme/datasource/MySqlIntegrationTest.java +++ b/src/test/java/fr/xephi/authme/datasource/MySqlIntegrationTest.java @@ -53,7 +53,7 @@ public class MySqlIntegrationTest extends AbstractDataSourceIntegrationTest { }); set(DatabaseSettings.MYSQL_DATABASE, "h2_test"); set(DatabaseSettings.MYSQL_TABLE, "authme"); - TestHelper.setupLogger(); + TestHelper.setRealLogger(); Path sqlInitFile = TestHelper.getJarPath(TestHelper.PROJECT_ROOT + "datasource/sql-initialize.sql"); sqlInitialize = new String(Files.readAllBytes(sqlInitFile)); diff --git a/src/test/java/fr/xephi/authme/datasource/SQLiteIntegrationTest.java b/src/test/java/fr/xephi/authme/datasource/SQLiteIntegrationTest.java index e0eb3eca..4836b198 100644 --- a/src/test/java/fr/xephi/authme/datasource/SQLiteIntegrationTest.java +++ b/src/test/java/fr/xephi/authme/datasource/SQLiteIntegrationTest.java @@ -56,7 +56,7 @@ public class SQLiteIntegrationTest extends AbstractDataSourceIntegrationTest { }); set(DatabaseSettings.MYSQL_DATABASE, "sqlite-test"); set(DatabaseSettings.MYSQL_TABLE, "authme"); - TestHelper.setupLogger(); + TestHelper.setRealLogger(); Path sqlInitFile = TestHelper.getJarPath(TestHelper.PROJECT_ROOT + "datasource/sql-initialize.sql"); // Note ljacqu 20160221: It appears that we can only run one statement per Statement.execute() so we split diff --git a/src/test/resources/fr/xephi/authme/datasource/sql-initialize.sql b/src/test/resources/fr/xephi/authme/datasource/sql-initialize.sql index 5dea47d5..48891c74 100644 --- a/src/test/resources/fr/xephi/authme/datasource/sql-initialize.sql +++ b/src/test/resources/fr/xephi/authme/datasource/sql-initialize.sql @@ -13,6 +13,8 @@ CREATE TABLE authme ( email VARCHAR(255) DEFAULT 'your@email.com', isLogged INT DEFAULT '0', realname VARCHAR(255) NOT NULL DEFAULT 'Player', salt varchar(255), + recoverycode VARCHAR(20), + recoveryexpiration BIGINT, CONSTRAINT table_const_prim PRIMARY KEY (id) ); From 3b723bbbe98c7373752807443ef2c5290ab2ab52 Mon Sep 17 00:00:00 2001 From: ljacqu Date: Sat, 10 Sep 2016 09:29:34 +0200 Subject: [PATCH 2/7] Fix removal of recovery code --- src/main/java/fr/xephi/authme/datasource/MySQL.java | 4 ++-- src/main/java/fr/xephi/authme/datasource/SQLite.java | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/main/java/fr/xephi/authme/datasource/MySQL.java b/src/main/java/fr/xephi/authme/datasource/MySQL.java index 5b944044..0e9c6466 100644 --- a/src/main/java/fr/xephi/authme/datasource/MySQL.java +++ b/src/main/java/fr/xephi/authme/datasource/MySQL.java @@ -901,8 +901,8 @@ public class MySQL implements DataSource { @Override public void removeRecoveryCode(String name) { String sql = "UPDATE " + tableName - + " SET " + col.RECOVERY_CODE + " = NULL" - + " AND " + col.RECOVERY_EXPIRATION + " = NULL" + + " SET " + col.RECOVERY_CODE + " = NULL, " + + col.RECOVERY_EXPIRATION + " = NULL" + " WHERE " + col.NAME + " = ?;"; try (Connection con = getConnection(); PreparedStatement pst = con.prepareStatement(sql)) { pst.setString(1, name.toLowerCase()); diff --git a/src/main/java/fr/xephi/authme/datasource/SQLite.java b/src/main/java/fr/xephi/authme/datasource/SQLite.java index df90d2eb..5f08e076 100644 --- a/src/main/java/fr/xephi/authme/datasource/SQLite.java +++ b/src/main/java/fr/xephi/authme/datasource/SQLite.java @@ -631,8 +631,8 @@ public class SQLite implements DataSource { @Override public void removeRecoveryCode(String name) { String sql = "UPDATE " + tableName - + " SET " + col.RECOVERY_CODE + " = NULL" - + " AND " + col.RECOVERY_EXPIRATION + " = NULL" + + " SET " + col.RECOVERY_CODE + " = NULL, " + + col.RECOVERY_EXPIRATION + " = NULL" + " WHERE " + col.NAME + " = ?;"; try (PreparedStatement pst = con.prepareStatement(sql)) { pst.setString(1, name.toLowerCase()); From c5f5c0d2fd1e5c837ee431c9597c3c0f5d256bb5 Mon Sep 17 00:00:00 2001 From: ljacqu Date: Sat, 10 Sep 2016 14:27:26 +0200 Subject: [PATCH 3/7] #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 --- .../authme/cache/auth/EmailRecoveryData.java | 38 ++++++ .../authme/command/CommandInitializer.java | 1 + .../executable/email/RecoverEmailCommand.java | 44 +++++-- .../authme/datasource/CacheDataSource.java | 6 +- .../xephi/authme/datasource/DataSource.java | 7 +- .../fr/xephi/authme/datasource/FlatFile.java | 3 +- .../fr/xephi/authme/datasource/MySQL.java | 12 +- .../fr/xephi/authme/datasource/SQLite.java | 12 +- .../fr/xephi/authme/mail/SendMailSSL.java | 32 +++-- .../process/register/AsyncRegister.java | 2 +- .../email/RecoverEmailCommandTest.java | 109 +++++++++++++----- .../AbstractDataSourceIntegrationTest.java | 26 ++++- 12 files changed, 224 insertions(+), 68 deletions(-) create mode 100644 src/main/java/fr/xephi/authme/cache/auth/EmailRecoveryData.java diff --git a/src/main/java/fr/xephi/authme/cache/auth/EmailRecoveryData.java b/src/main/java/fr/xephi/authme/cache/auth/EmailRecoveryData.java new file mode 100644 index 00000000..392b14c6 --- /dev/null +++ b/src/main/java/fr/xephi/authme/cache/auth/EmailRecoveryData.java @@ -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; + } +} diff --git a/src/main/java/fr/xephi/authme/command/CommandInitializer.java b/src/main/java/fr/xephi/authme/command/CommandInitializer.java index 0d5c6da0..b29fd215 100644 --- a/src/main/java/fr/xephi/authme/command/CommandInitializer.java +++ b/src/main/java/fr/xephi/authme/command/CommandInitializer.java @@ -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(); diff --git a/src/main/java/fr/xephi/authme/command/executable/email/RecoverEmailCommand.java b/src/main/java/fr/xephi/authme/command/executable/email/RecoverEmailCommand.java index 8965c96f..671a31bd 100644 --- a/src/main/java/fr/xephi/authme/command/executable/email/RecoverEmailCommand.java +++ b/src/main/java/fr/xephi/authme/command/executable/email/RecoverEmailCommand.java @@ -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); } } diff --git a/src/main/java/fr/xephi/authme/datasource/CacheDataSource.java b/src/main/java/fr/xephi/authme/datasource/CacheDataSource.java index 36372430..a4f642d0 100644 --- a/src/main/java/fr/xephi/authme/datasource/CacheDataSource.java +++ b/src/main/java/fr/xephi/authme/datasource/CacheDataSource.java @@ -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 diff --git a/src/main/java/fr/xephi/authme/datasource/DataSource.java b/src/main/java/fr/xephi/authme/datasource/DataSource.java index 96788d83..b069cf4b 100644 --- a/src/main/java/fr/xephi/authme/datasource/DataSource.java +++ b/src/main/java/fr/xephi/authme/datasource/DataSource.java @@ -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. diff --git a/src/main/java/fr/xephi/authme/datasource/FlatFile.java b/src/main/java/fr/xephi/authme/datasource/FlatFile.java index d600a3e6..f2490a84 100644 --- a/src/main/java/fr/xephi/authme/datasource/FlatFile.java +++ b/src/main/java/fr/xephi/authme/datasource/FlatFile.java @@ -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"); } diff --git a/src/main/java/fr/xephi/authme/datasource/MySQL.java b/src/main/java/fr/xephi/authme/datasource/MySQL.java index 0e9c6466..ff771bb2 100644 --- a/src/main/java/fr/xephi/authme/datasource/MySQL.java +++ b/src/main/java/fr/xephi/authme/datasource/MySQL.java @@ -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) { diff --git a/src/main/java/fr/xephi/authme/datasource/SQLite.java b/src/main/java/fr/xephi/authme/datasource/SQLite.java index 5f08e076..f6ae8f90 100644 --- a/src/main/java/fr/xephi/authme/datasource/SQLite.java +++ b/src/main/java/fr/xephi/authme/datasource/SQLite.java @@ -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) { diff --git a/src/main/java/fr/xephi/authme/mail/SendMailSSL.java b/src/main/java/fr/xephi/authme/mail/SendMailSSL.java index 6088a5bc..8b8f8b7e 100644 --- a/src/main/java/fr/xephi/authme/mail/SendMailSSL.java +++ b/src/main/java/fr/xephi/authme/mail/SendMailSSL.java @@ -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("", auth.getNickname()) + .replace("", name) .replace("", plugin.getServer().getServerName()) .replace("", newPass); } diff --git a/src/main/java/fr/xephi/authme/process/register/AsyncRegister.java b/src/main/java/fr/xephi/authme/process/register/AsyncRegister.java index 57390a54..a7cf5794 100644 --- a/src/main/java/fr/xephi/authme/process/register/AsyncRegister.java +++ b/src/main/java/fr/xephi/authme/process/register/AsyncRegister.java @@ -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); } diff --git a/src/test/java/fr/xephi/authme/command/executable/email/RecoverEmailCommandTest.java b/src/test/java/fr/xephi/authme/command/executable/email/RecoverEmailCommandTest.java index 2f57443a..4d60877e 100644 --- a/src/test/java/fr/xephi/authme/command/executable/email/RecoverEmailCommandTest.java +++ b/src/test/java/fr/xephi/authme/command/executable/email/RecoverEmailCommandTest.java @@ -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 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() { - @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 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); } } diff --git a/src/test/java/fr/xephi/authme/datasource/AbstractDataSourceIntegrationTest.java b/src/test/java/fr/xephi/authme/datasource/AbstractDataSourceIntegrationTest.java index 9c417f62..59c6ce22 100644 --- a/src/test/java/fr/xephi/authme/datasource/AbstractDataSourceIntegrationTest.java +++ b/src/test/java/fr/xephi/authme/datasource/AbstractDataSourceIntegrationTest.java @@ -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()); } } From bff344ba8f8ee58305944bdede7fb67f73255693 Mon Sep 17 00:00:00 2001 From: ljacqu Date: Sat, 10 Sep 2016 16:39:35 +0200 Subject: [PATCH 4/7] #472 Recovery code: allow to configure length, expiration and email --- .../fr/xephi/authme/cache/SessionManager.java | 5 +- .../fr/xephi/authme/cache/TempbanManager.java | 7 +-- .../authme/cache/auth/EmailRecoveryData.java | 11 ++-- .../executable/email/RecoverEmailCommand.java | 13 ++-- .../fr/xephi/authme/mail/SendMailSSL.java | 19 ++++-- .../fr/xephi/authme/settings/Settings.java | 63 ++++++++++--------- .../settings/properties/SecuritySettings.java | 8 +++ src/main/java/fr/xephi/authme/util/Utils.java | 5 ++ src/main/resources/config.yml | 5 ++ src/main/resources/recovery_code_email.html | 9 +++ .../email/RecoverEmailCommandTest.java | 20 ++++-- .../xephi/authme/settings/SettingsTest.java | 12 ++-- 12 files changed, 117 insertions(+), 60 deletions(-) create mode 100644 src/main/resources/recovery_code_email.html diff --git a/src/main/java/fr/xephi/authme/cache/SessionManager.java b/src/main/java/fr/xephi/authme/cache/SessionManager.java index 95b6d8db..33d3f9a6 100644 --- a/src/main/java/fr/xephi/authme/cache/SessionManager.java +++ b/src/main/java/fr/xephi/authme/cache/SessionManager.java @@ -11,13 +11,14 @@ import java.util.Iterator; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; +import static fr.xephi.authme.util.Utils.MILLIS_PER_MINUTE; + /** * Manages sessions, allowing players to be automatically logged in if they join again * within a configurable amount of time. */ public class SessionManager implements SettingsDependent, HasCleanup { - private static final int MINUTE_IN_MILLIS = 60_000; // Player -> expiration of session in milliseconds private final Map sessions = new ConcurrentHashMap<>(); @@ -52,7 +53,7 @@ public class SessionManager implements SettingsDependent, HasCleanup { */ public void addSession(String name) { if (enabled) { - long timeout = System.currentTimeMillis() + timeoutInMinutes * MINUTE_IN_MILLIS; + long timeout = System.currentTimeMillis() + timeoutInMinutes * MILLIS_PER_MINUTE; sessions.put(name.toLowerCase(), timeout); } } diff --git a/src/main/java/fr/xephi/authme/cache/TempbanManager.java b/src/main/java/fr/xephi/authme/cache/TempbanManager.java index f2d9cc5b..832c650d 100644 --- a/src/main/java/fr/xephi/authme/cache/TempbanManager.java +++ b/src/main/java/fr/xephi/authme/cache/TempbanManager.java @@ -18,14 +18,13 @@ import java.util.Map; import java.util.concurrent.ConcurrentHashMap; import static fr.xephi.authme.settings.properties.SecuritySettings.TEMPBAN_MINUTES_BEFORE_RESET; +import static fr.xephi.authme.util.Utils.MILLIS_PER_MINUTE; /** * Manager for handling temporary bans. */ public class TempbanManager implements SettingsDependent, HasCleanup { - private static final long MINUTE_IN_MILLISECONDS = 60_000; - private final Map> ipLoginFailureCounts; private final BukkitService bukkitService; private final Messages messages; @@ -113,7 +112,7 @@ public class TempbanManager implements SettingsDependent, HasCleanup { final String reason = messages.retrieveSingle(MessageKey.TEMPBAN_MAX_LOGINS); final Date expires = new Date(); - long newTime = expires.getTime() + (length * MINUTE_IN_MILLISECONDS); + long newTime = expires.getTime() + (length * MILLIS_PER_MINUTE); expires.setTime(newTime); bukkitService.scheduleSyncDelayedTask(new Runnable() { @@ -133,7 +132,7 @@ public class TempbanManager implements SettingsDependent, HasCleanup { this.isEnabled = settings.getProperty(SecuritySettings.TEMPBAN_ON_MAX_LOGINS); this.threshold = settings.getProperty(SecuritySettings.MAX_LOGIN_TEMPBAN); this.length = settings.getProperty(SecuritySettings.TEMPBAN_LENGTH); - this.resetThreshold = settings.getProperty(TEMPBAN_MINUTES_BEFORE_RESET) * MINUTE_IN_MILLISECONDS; + this.resetThreshold = settings.getProperty(TEMPBAN_MINUTES_BEFORE_RESET) * MILLIS_PER_MINUTE; } @Override diff --git a/src/main/java/fr/xephi/authme/cache/auth/EmailRecoveryData.java b/src/main/java/fr/xephi/authme/cache/auth/EmailRecoveryData.java index 392b14c6..3cc02577 100644 --- a/src/main/java/fr/xephi/authme/cache/auth/EmailRecoveryData.java +++ b/src/main/java/fr/xephi/authme/cache/auth/EmailRecoveryData.java @@ -13,13 +13,16 @@ public class EmailRecoveryData { * * @param email the email address * @param recoveryCode the recovery code, or null if not available - * @param codeExpiration + * @param codeExpiration expiration timestamp of the recovery code */ public EmailRecoveryData(String email, String recoveryCode, Long codeExpiration) { this.email = email; - this.recoveryCode = codeExpiration == null || System.currentTimeMillis() > codeExpiration - ? null - : recoveryCode; + + if (codeExpiration == null || System.currentTimeMillis() > codeExpiration) { + this.recoveryCode = null; + } else { + this.recoveryCode = recoveryCode; + } } /** diff --git a/src/main/java/fr/xephi/authme/command/executable/email/RecoverEmailCommand.java b/src/main/java/fr/xephi/authme/command/executable/email/RecoverEmailCommand.java index 671a31bd..cf9c3d4f 100644 --- a/src/main/java/fr/xephi/authme/command/executable/email/RecoverEmailCommand.java +++ b/src/main/java/fr/xephi/authme/command/executable/email/RecoverEmailCommand.java @@ -17,6 +17,10 @@ import org.bukkit.entity.Player; import javax.inject.Inject; import java.util.List; +import static fr.xephi.authme.settings.properties.SecuritySettings.RECOVERY_CODE_HOURS_VALID; +import static fr.xephi.authme.settings.properties.SecuritySettings.RECOVERY_CODE_LENGTH; +import static fr.xephi.authme.util.Utils.MILLIS_PER_HOUR; + /** * Command for password recovery by email. */ @@ -74,11 +78,12 @@ public class RecoverEmailCommand extends PlayerCommand { } 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 + String recoveryCode = RandomString.generateHex(commandService.getProperty(RECOVERY_CODE_LENGTH)); + long expiration = System.currentTimeMillis() + + commandService.getProperty(RECOVERY_CODE_HOURS_VALID) * MILLIS_PER_HOUR; + dataSource.setRecoveryCode(name, recoveryCode, expiration); - sendMailSsl.sendRecoveryCode(recoveryData.getEmail(), recoveryCode); + sendMailSsl.sendRecoveryCode(name, recoveryData.getEmail(), recoveryCode); } private void processRecoveryCode(Player player, String code, EmailRecoveryData recoveryData) { diff --git a/src/main/java/fr/xephi/authme/mail/SendMailSSL.java b/src/main/java/fr/xephi/authme/mail/SendMailSSL.java index 8b8f8b7e..74dc6db3 100644 --- a/src/main/java/fr/xephi/authme/mail/SendMailSSL.java +++ b/src/main/java/fr/xephi/authme/mail/SendMailSSL.java @@ -4,6 +4,7 @@ import fr.xephi.authme.AuthMe; import fr.xephi.authme.ConsoleLogger; import fr.xephi.authme.settings.Settings; import fr.xephi.authme.settings.properties.EmailSettings; +import fr.xephi.authme.settings.properties.SecuritySettings; import fr.xephi.authme.util.BukkitService; import fr.xephi.authme.util.StringUtils; import org.apache.commons.mail.EmailConstants; @@ -62,7 +63,7 @@ public class SendMailSSL { return; } - final String mailText = replaceMailTags(settings.getEmailMessage(), name, newPass); + final String mailText = replaceTagsForPasswordMail(settings.getPasswordEmailMessage(), name, newPass); bukkitService.runTaskAsynchronously(new Runnable() { @Override @@ -97,9 +98,9 @@ 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); + public void sendRecoveryCode(String name, String email, String code) { + String message = replaceTagsForRecoveryCodeMail(settings.getRecoveryCodeEmailMessage(), + name, code, settings.getProperty(SecuritySettings.RECOVERY_CODE_HOURS_VALID)); HtmlEmail htmlEmail; try { @@ -163,13 +164,21 @@ public class SendMailSSL { } } - private String replaceMailTags(String mailText, String name, String newPass) { + private String replaceTagsForPasswordMail(String mailText, String name, String newPass) { return mailText .replace("", name) .replace("", plugin.getServer().getServerName()) .replace("", newPass); } + private String replaceTagsForRecoveryCodeMail(String mailText, String name, String code, int hoursValid) { + return mailText + .replace("", name) + .replace("", plugin.getServer().getServerName()) + .replace("", code) + .replace("", String.valueOf(hoursValid)); + } + private void setPropertiesForPort(HtmlEmail email, int port) throws EmailException { switch (port) { case 587: diff --git a/src/main/java/fr/xephi/authme/settings/Settings.java b/src/main/java/fr/xephi/authme/settings/Settings.java index 855c14df..9a1cb5fa 100644 --- a/src/main/java/fr/xephi/authme/settings/Settings.java +++ b/src/main/java/fr/xephi/authme/settings/Settings.java @@ -4,16 +4,14 @@ import com.github.authme.configme.SettingsManager; import com.github.authme.configme.knownproperties.PropertyEntry; import com.github.authme.configme.migration.MigrationService; import com.github.authme.configme.resource.PropertyResource; +import com.google.common.base.Charsets; import com.google.common.io.Files; import fr.xephi.authme.ConsoleLogger; import fr.xephi.authme.settings.properties.PluginSettings; -import fr.xephi.authme.settings.properties.RegistrationSettings; import fr.xephi.authme.util.StringUtils; import java.io.File; import java.io.IOException; -import java.nio.charset.Charset; -import java.util.ArrayList; import java.util.List; import static fr.xephi.authme.util.FileUtils.copyFileFromResource; @@ -26,8 +24,9 @@ public class Settings extends SettingsManager { private final File pluginFolder; /** The file with the localized messages based on {@link PluginSettings#MESSAGES_LANGUAGE}. */ private File messagesFile; - private List welcomeMessage; - private String emailMessage; + private String[] welcomeMessage; + private String passwordEmailMessage; + private String recoveryCodeEmailMessage; /** * Constructor. @@ -67,8 +66,17 @@ public class Settings extends SettingsManager { * * @return The email message */ - public String getEmailMessage() { - return emailMessage; + public String getPasswordEmailMessage() { + return passwordEmailMessage; + } + + /** + * Return the text to use when someone requests to receive a recovery code. + * + * @return The email message + */ + public String getRecoveryCodeEmailMessage() { + return recoveryCodeEmailMessage; } /** @@ -76,14 +84,15 @@ public class Settings extends SettingsManager { * * @return The welcome message */ - public List getWelcomeMessage() { + public String[] getWelcomeMessage() { return welcomeMessage; } private void loadSettingsFromFiles() { messagesFile = buildMessagesFile(); - welcomeMessage = readWelcomeMessage(); - emailMessage = readEmailMessage(); + passwordEmailMessage = readFile("email.html"); + recoveryCodeEmailMessage = readFile("recovery_code_email.html"); + welcomeMessage = readFile("welcome.txt").split("\n"); } @Override @@ -114,30 +123,22 @@ public class Settings extends SettingsManager { return StringUtils.makePath("messages", "messages_" + language + ".yml"); } - private List readWelcomeMessage() { - if (getProperty(RegistrationSettings.USE_WELCOME_MESSAGE)) { - final File welcomeFile = new File(pluginFolder, "welcome.txt"); - final Charset charset = Charset.forName("UTF-8"); - if (copyFileFromResource(welcomeFile, "welcome.txt")) { - try { - return Files.readLines(welcomeFile, charset); - } catch (IOException e) { - ConsoleLogger.logException("Failed to read file '" + welcomeFile.getPath() + "':", e); - } - } - } - return new ArrayList<>(0); - } - - private String readEmailMessage() { - final File emailFile = new File(pluginFolder, "email.html"); - final Charset charset = Charset.forName("UTF-8"); - if (copyFileFromResource(emailFile, "email.html")) { + /** + * Reads a file from the plugin folder or copies it from the JAR to the plugin folder. + * + * @param filename the file to read + * @return the file's contents + */ + private String readFile(String filename) { + final File file = new File(pluginFolder, filename); + if (copyFileFromResource(file, filename)) { try { - return Files.toString(emailFile, charset); + return Files.toString(file, Charsets.UTF_8); } catch (IOException e) { - ConsoleLogger.logException("Failed to read file '" + emailFile.getPath() + "':", e); + ConsoleLogger.logException("Failed to read file '" + filename + "':", e); } + } else { + ConsoleLogger.warning("Failed to copy file '" + filename + "' from JAR"); } return ""; } diff --git a/src/main/java/fr/xephi/authme/settings/properties/SecuritySettings.java b/src/main/java/fr/xephi/authme/settings/properties/SecuritySettings.java index aa3f3783..fb1cf258 100644 --- a/src/main/java/fr/xephi/authme/settings/properties/SecuritySettings.java +++ b/src/main/java/fr/xephi/authme/settings/properties/SecuritySettings.java @@ -109,6 +109,14 @@ public class SecuritySettings implements SettingsHolder { public static final Property TEMPBAN_MINUTES_BEFORE_RESET = newProperty("Security.tempban.minutesBeforeCounterReset", 480); + @Comment("Number of characters a recovery code should have") + public static final Property RECOVERY_CODE_LENGTH = + newProperty("Security.recoveryCode.length", 8); + + @Comment("How many hours is a recovery code valid for?") + public static final Property RECOVERY_CODE_HOURS_VALID = + newProperty("Security.recoveryCode.validForHours", 4); + private SecuritySettings() { } diff --git a/src/main/java/fr/xephi/authme/util/Utils.java b/src/main/java/fr/xephi/authme/util/Utils.java index c2122ee3..3a7040d1 100644 --- a/src/main/java/fr/xephi/authme/util/Utils.java +++ b/src/main/java/fr/xephi/authme/util/Utils.java @@ -11,6 +11,11 @@ import java.util.regex.Pattern; */ public final class Utils { + /** Number of milliseconds in a minute. */ + public static final long MILLIS_PER_MINUTE = 60_000L; + /** Number of milliseconds in an hour. */ + public static final long MILLIS_PER_HOUR = 60 * MILLIS_PER_MINUTE; + private Utils() { } diff --git a/src/main/resources/config.yml b/src/main/resources/config.yml index a744f43c..18721925 100644 --- a/src/main/resources/config.yml +++ b/src/main/resources/config.yml @@ -342,6 +342,11 @@ Security: # How many minutes before resetting the count for failed logins by IP and username # Default: 480 minutes (8 hours) minutesBeforeCounterReset: 480 + recoveryCode: + # Number of characters a recovery code should have + length: 8 + # How many hours is a recovery code valid for? + validForHours: 4 Converter: Rakamak: # Rakamak file name diff --git a/src/main/resources/recovery_code_email.html b/src/main/resources/recovery_code_email.html new file mode 100644 index 00000000..e5614f4f --- /dev/null +++ b/src/main/resources/recovery_code_email.html @@ -0,0 +1,9 @@ +

Dear ,

+ +

+ You have requested to reset your password on . To reset it, + please use the recovery code : /email recover [email] . +

+

+ The code expires in hours. +

diff --git a/src/test/java/fr/xephi/authme/command/executable/email/RecoverEmailCommandTest.java b/src/test/java/fr/xephi/authme/command/executable/email/RecoverEmailCommandTest.java index 4d60877e..732e0777 100644 --- a/src/test/java/fr/xephi/authme/command/executable/email/RecoverEmailCommandTest.java +++ b/src/test/java/fr/xephi/authme/command/executable/email/RecoverEmailCommandTest.java @@ -10,6 +10,7 @@ import fr.xephi.authme.output.MessageKey; import fr.xephi.authme.security.PasswordSecurity; import fr.xephi.authme.security.crypts.HashedPassword; import fr.xephi.authme.settings.properties.EmailSettings; +import fr.xephi.authme.settings.properties.SecuritySettings; import org.bukkit.entity.Player; import org.junit.BeforeClass; import org.junit.Test; @@ -23,11 +24,14 @@ import java.util.Arrays; import java.util.Collections; import static fr.xephi.authme.AuthMeMatchers.stringWithLength; +import static fr.xephi.authme.util.Utils.MILLIS_PER_HOUR; +import static org.hamcrest.Matchers.allOf; import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.greaterThan; +import static org.hamcrest.Matchers.lessThan; 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; @@ -170,6 +174,10 @@ public class RecoverEmailCommandTest { given(playerCache.isAuthenticated(name)).willReturn(false); String email = "v@example.com"; given(dataSource.getEmailRecoveryData(name)).willReturn(newEmailRecoveryData(email)); + int codeLength = 7; + given(commandService.getProperty(SecuritySettings.RECOVERY_CODE_LENGTH)).willReturn(codeLength); + int hoursValid = 12; + given(commandService.getProperty(SecuritySettings.RECOVERY_CODE_HOURS_VALID)).willReturn(hoursValid); // when command.executeCommand(sender, Collections.singletonList(email.toUpperCase())); @@ -178,9 +186,13 @@ public class RecoverEmailCommandTest { verify(sendMailSsl).hasAllInformation(); verify(dataSource).getEmailRecoveryData(name); ArgumentCaptor codeCaptor = ArgumentCaptor.forClass(String.class); - verify(dataSource).setRecoveryCode(eq(name), codeCaptor.capture(), anyLong()); - assertThat(codeCaptor.getValue(), stringWithLength(8)); - verify(sendMailSsl).sendRecoveryCode(email, codeCaptor.getValue()); + ArgumentCaptor expirationCaptor = ArgumentCaptor.forClass(Long.class); + verify(dataSource).setRecoveryCode(eq(name), codeCaptor.capture(), expirationCaptor.capture()); + assertThat(codeCaptor.getValue(), stringWithLength(codeLength)); + // Check expiration with a tolerance + assertThat(expirationCaptor.getValue() - System.currentTimeMillis(), + allOf(lessThan(12L * MILLIS_PER_HOUR), greaterThan((long) (11.9 * MILLIS_PER_HOUR)))); + verify(sendMailSsl).sendRecoveryCode(name, email, codeCaptor.getValue()); } @Test diff --git a/src/test/java/fr/xephi/authme/settings/SettingsTest.java b/src/test/java/fr/xephi/authme/settings/SettingsTest.java index 5f4bac9b..3cde1f2d 100644 --- a/src/test/java/fr/xephi/authme/settings/SettingsTest.java +++ b/src/test/java/fr/xephi/authme/settings/SettingsTest.java @@ -23,9 +23,10 @@ import java.util.List; import static fr.xephi.authme.settings.properties.PluginSettings.MESSAGES_LANGUAGE; import static fr.xephi.authme.util.StringUtils.makePath; +import static org.hamcrest.Matchers.arrayContaining; +import static org.hamcrest.Matchers.arrayWithSize; import static org.hamcrest.Matchers.endsWith; import static org.hamcrest.Matchers.equalTo; -import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.nullValue; import static org.junit.Assert.assertThat; @@ -127,12 +128,11 @@ public class SettingsTest { TestSettingsMigrationServices.alwaysFulfilled(), knownProperties); // when - List result = settings.getWelcomeMessage(); + String[] result = settings.getWelcomeMessage(); // then - assertThat(result, hasSize(2)); - assertThat(result.get(0), equalTo(welcomeMessage.split("\\n")[0])); - assertThat(result.get(1), equalTo(welcomeMessage.split("\\n")[1])); + assertThat(result, arrayWithSize(2)); + assertThat(result, arrayContaining(welcomeMessage.split("\\n"))); } @Test @@ -148,7 +148,7 @@ public class SettingsTest { TestSettingsMigrationServices.alwaysFulfilled(), knownProperties); // when - String result = settings.getEmailMessage(); + String result = settings.getPasswordEmailMessage(); // then assertThat(result, equalTo(emailMessage)); From e30d7220bd275ab46d01f1da62784ab8d404ebff Mon Sep 17 00:00:00 2001 From: ljacqu Date: Fri, 16 Sep 2016 21:42:16 +0200 Subject: [PATCH 5/7] #472 Store recovery codes in memory instead of in data source --- .../authme/cache/auth/EmailRecoveryData.java | 41 ----------- .../executable/email/RecoverEmailCommand.java | 59 ++++++++------- .../authme/datasource/CacheDataSource.java | 19 ----- .../fr/xephi/authme/datasource/Columns.java | 4 - .../xephi/authme/datasource/DataSource.java | 25 ------- .../fr/xephi/authme/datasource/FlatFile.java | 16 ---- .../fr/xephi/authme/datasource/MySQL.java | 58 --------------- .../fr/xephi/authme/datasource/SQLite.java | 58 --------------- .../authme/service/RecoveryCodeManager.java | 73 +++++++++++++++++++ .../settings/properties/DatabaseSettings.java | 8 -- src/main/resources/config.yml | 4 - .../email/RecoverEmailCommandTest.java | 72 +++++++++--------- .../AbstractDataSourceIntegrationTest.java | 60 --------------- 13 files changed, 140 insertions(+), 357 deletions(-) delete mode 100644 src/main/java/fr/xephi/authme/cache/auth/EmailRecoveryData.java create mode 100644 src/main/java/fr/xephi/authme/service/RecoveryCodeManager.java diff --git a/src/main/java/fr/xephi/authme/cache/auth/EmailRecoveryData.java b/src/main/java/fr/xephi/authme/cache/auth/EmailRecoveryData.java deleted file mode 100644 index 3cc02577..00000000 --- a/src/main/java/fr/xephi/authme/cache/auth/EmailRecoveryData.java +++ /dev/null @@ -1,41 +0,0 @@ -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 expiration timestamp of the recovery code - */ - public EmailRecoveryData(String email, String recoveryCode, Long codeExpiration) { - this.email = email; - - if (codeExpiration == null || System.currentTimeMillis() > codeExpiration) { - this.recoveryCode = null; - } else { - this.recoveryCode = recoveryCode; - } - } - - /** - * @return the email address - */ - public String getEmail() { - return email; - } - - /** - * @return the recovery code, if available and not expired - */ - public String getRecoveryCode() { - return recoveryCode; - } -} diff --git a/src/main/java/fr/xephi/authme/command/executable/email/RecoverEmailCommand.java b/src/main/java/fr/xephi/authme/command/executable/email/RecoverEmailCommand.java index cf9c3d4f..6f7923d7 100644 --- a/src/main/java/fr/xephi/authme/command/executable/email/RecoverEmailCommand.java +++ b/src/main/java/fr/xephi/authme/command/executable/email/RecoverEmailCommand.java @@ -1,7 +1,7 @@ package fr.xephi.authme.command.executable.email; 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.command.CommandService; import fr.xephi.authme.command.PlayerCommand; @@ -11,15 +11,13 @@ import fr.xephi.authme.output.MessageKey; import fr.xephi.authme.security.PasswordSecurity; import fr.xephi.authme.security.RandomString; import fr.xephi.authme.security.crypts.HashedPassword; -import fr.xephi.authme.settings.properties.EmailSettings; +import fr.xephi.authme.service.RecoveryCodeManager; import org.bukkit.entity.Player; import javax.inject.Inject; import java.util.List; -import static fr.xephi.authme.settings.properties.SecuritySettings.RECOVERY_CODE_HOURS_VALID; -import static fr.xephi.authme.settings.properties.SecuritySettings.RECOVERY_CODE_LENGTH; -import static fr.xephi.authme.util.Utils.MILLIS_PER_HOUR; +import static fr.xephi.authme.settings.properties.EmailSettings.RECOVERY_PASSWORD_LENGTH; /** * Command for password recovery by email. @@ -41,6 +39,9 @@ public class RecoverEmailCommand extends PlayerCommand { @Inject private SendMailSSL sendMailSsl; + @Inject + private RecoveryCodeManager recoveryCodeManager; + @Override public void runCommand(Player player, List arguments) { final String playerMail = arguments.get(0); @@ -56,48 +57,54 @@ public class RecoverEmailCommand extends PlayerCommand { return; } - EmailRecoveryData recoveryData = dataSource.getEmailRecoveryData(playerName); - if (recoveryData == null) { + PlayerAuth auth = dataSource.getAuth(playerName); // TODO: Create method to get email only + if (auth == null) { commandService.send(player, MessageKey.REGISTER_EMAIL_MESSAGE); return; } - final String email = recoveryData.getEmail(); + final String email = auth.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); + if (recoveryCodeManager.isRecoveryCodeNeeded()) { + // Process /email recovery addr@example.com + if (arguments.size() == 1) { + createAndSendRecoveryCode(playerName, email); + } else { + // Process /email recovery addr@example.com 12394 + processRecoveryCode(player, arguments.get(1), email); + } } else { - // Process /email recover addr@example.com 12394 - processRecoveryCode(player, arguments.get(1), recoveryData); + generateAndSendNewPassword(player, email); } } - private void createAndSendRecoveryCode(String name, EmailRecoveryData recoveryData) { - String recoveryCode = RandomString.generateHex(commandService.getProperty(RECOVERY_CODE_LENGTH)); - long expiration = System.currentTimeMillis() - + commandService.getProperty(RECOVERY_CODE_HOURS_VALID) * MILLIS_PER_HOUR; - - dataSource.setRecoveryCode(name, recoveryCode, expiration); - sendMailSsl.sendRecoveryCode(name, recoveryData.getEmail(), recoveryCode); + private void createAndSendRecoveryCode(String name, String email) { + String recoveryCode = recoveryCodeManager.generateCode(name); + sendMailSsl.sendRecoveryCode(name, email, recoveryCode); } - private void processRecoveryCode(Player player, String code, EmailRecoveryData recoveryData) { - if (!code.equals(recoveryData.getRecoveryCode())) { + private void processRecoveryCode(Player player, String code, String email) { + final String name = player.getName(); + if (!recoveryCodeManager.isCodeValid(name, code)) { 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)); + generateAndSendNewPassword(player, email); + recoveryCodeManager.removeCode(name); + } + + private void generateAndSendNewPassword(Player player, String email) { + String name = player.getName(); + String thePass = RandomString.generate(commandService.getProperty(RECOVERY_PASSWORD_LENGTH)); HashedPassword hashNew = passwordSecurity.computeHash(thePass, name); + dataSource.updatePassword(name, hashNew); - dataSource.removeRecoveryCode(name); - sendMailSsl.sendPasswordMail(name, recoveryData.getEmail(), thePass); + sendMailSsl.sendPasswordMail(name, email, thePass); commandService.send(player, MessageKey.RECOVERY_EMAIL_SENT_MESSAGE); } } diff --git a/src/main/java/fr/xephi/authme/datasource/CacheDataSource.java b/src/main/java/fr/xephi/authme/datasource/CacheDataSource.java index a4f642d0..bab51daf 100644 --- a/src/main/java/fr/xephi/authme/datasource/CacheDataSource.java +++ b/src/main/java/fr/xephi/authme/datasource/CacheDataSource.java @@ -8,9 +8,7 @@ import com.google.common.util.concurrent.ListenableFuture; import com.google.common.util.concurrent.ListeningExecutorService; 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; @@ -236,23 +234,6 @@ public class CacheDataSource implements DataSource { return source.getAllAuths(); } - @Override - public void setRecoveryCode(String name, String code, long expiration) { - source.setRecoveryCode(name, code, expiration); - cachedAuths.refresh(name); - } - - @Override - public EmailRecoveryData getEmailRecoveryData(String name) { - return source.getEmailRecoveryData(name); - } - - @Override - public void removeRecoveryCode(String name) { - source.removeRecoveryCode(name); - cachedAuths.refresh(name); - } - @Override public List getLoggedPlayers() { return new ArrayList<>(PlayerCache.getInstance().getCache().values()); diff --git a/src/main/java/fr/xephi/authme/datasource/Columns.java b/src/main/java/fr/xephi/authme/datasource/Columns.java index d56aae72..b6d732cd 100644 --- a/src/main/java/fr/xephi/authme/datasource/Columns.java +++ b/src/main/java/fr/xephi/authme/datasource/Columns.java @@ -22,8 +22,6 @@ public final class Columns { public final String EMAIL; public final String ID; public final String IS_LOGGED; - public final String RECOVERY_CODE; - public final String RECOVERY_EXPIRATION; public Columns(Settings settings) { NAME = settings.getProperty(DatabaseSettings.MYSQL_COL_NAME); @@ -40,8 +38,6 @@ public final class Columns { EMAIL = settings.getProperty(DatabaseSettings.MYSQL_COL_EMAIL); ID = settings.getProperty(DatabaseSettings.MYSQL_COL_ID); IS_LOGGED = settings.getProperty(DatabaseSettings.MYSQL_COL_ISLOGGED); - RECOVERY_CODE = settings.getProperty(DatabaseSettings.MYSQL_COL_RECOVERY_CODE); - RECOVERY_EXPIRATION = settings.getProperty(DatabaseSettings.MYSQL_COL_RECOVERY_EXPIRATION); } } diff --git a/src/main/java/fr/xephi/authme/datasource/DataSource.java b/src/main/java/fr/xephi/authme/datasource/DataSource.java index b069cf4b..5b25fde4 100644 --- a/src/main/java/fr/xephi/authme/datasource/DataSource.java +++ b/src/main/java/fr/xephi/authme/datasource/DataSource.java @@ -1,6 +1,5 @@ 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; @@ -195,30 +194,6 @@ public interface DataSource extends Reloadable { */ List getAllAuths(); - /** - * Set the password recovery code for a user. - * - * @param name The name of the user - * @param code The recovery code - * @param expiration Recovery code expiration (milliseconds timestamp) - */ - void setRecoveryCode(String name, String code, long expiration); - - /** - * Get the information necessary for performing a password recovery by email. - * - * @param name The name of the user - * @return The data of the player, or null if player doesn't exist - */ - EmailRecoveryData getEmailRecoveryData(String name); - - /** - * Remove the recovery code of a given user. - * - * @param name The name of the user - */ - void removeRecoveryCode(String name); - /** * Reload the data source. */ diff --git a/src/main/java/fr/xephi/authme/datasource/FlatFile.java b/src/main/java/fr/xephi/authme/datasource/FlatFile.java index f2490a84..f3f18014 100644 --- a/src/main/java/fr/xephi/authme/datasource/FlatFile.java +++ b/src/main/java/fr/xephi/authme/datasource/FlatFile.java @@ -1,7 +1,6 @@ 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; @@ -469,21 +468,6 @@ public class FlatFile implements DataSource { throw new UnsupportedOperationException("Flat file no longer supported"); } - @Override - public void setRecoveryCode(String name, String code, long expiration) { - throw new UnsupportedOperationException("Flat file no longer supported"); - } - - @Override - public EmailRecoveryData getEmailRecoveryData(String name) { - throw new UnsupportedOperationException("Flat file no longer supported"); - } - - @Override - public void removeRecoveryCode(String name) { - throw new UnsupportedOperationException("Flat file no longer supported"); - } - private static PlayerAuth buildAuthFromArray(String[] args) { // Format allows 2, 3, 4, 7, 8, 9 fields. Anything else is unknown if (args.length >= 2 && args.length <= 9 && args.length != 5 && args.length != 6) { diff --git a/src/main/java/fr/xephi/authme/datasource/MySQL.java b/src/main/java/fr/xephi/authme/datasource/MySQL.java index ff771bb2..dc08d7b4 100644 --- a/src/main/java/fr/xephi/authme/datasource/MySQL.java +++ b/src/main/java/fr/xephi/authme/datasource/MySQL.java @@ -4,7 +4,6 @@ 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; @@ -209,14 +208,6 @@ public class MySQL implements DataSource { st.executeUpdate("ALTER TABLE " + tableName + " ADD COLUMN " + col.IS_LOGGED + " SMALLINT NOT NULL DEFAULT '0' AFTER " + col.EMAIL); } - - if (isColumnMissing(md, col.RECOVERY_CODE)) { - st.executeUpdate("ALTER TABLE " + tableName + " ADD COLUMN " + col.RECOVERY_CODE + " VARCHAR(20);"); - } - - if (isColumnMissing(md, col.RECOVERY_EXPIRATION)) { - st.executeUpdate("ALTER TABLE " + tableName + " ADD COLUMN " + col.RECOVERY_EXPIRATION + " BIGINT;"); - } } ConsoleLogger.info("MySQL setup finished"); } @@ -865,55 +856,6 @@ public class MySQL implements DataSource { return auths; } - @Override - public void setRecoveryCode(String name, String code, long expiration) { - String sql = "UPDATE " + tableName - + " SET " + col.RECOVERY_CODE + " = ?, " - + col.RECOVERY_EXPIRATION + " = ?" - + " WHERE " + col.NAME + " = ?;"; - try (Connection con = getConnection(); PreparedStatement pst = con.prepareStatement(sql)) { - pst.setString(1, code); - pst.setLong(2, expiration); - pst.setString(3, name.toLowerCase()); - pst.executeUpdate(); - } catch (SQLException e) { - logSqlException(e); - } - } - - @Override - 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()); - try (ResultSet rs = pst.executeQuery()) { - if (rs.next()) { - return new EmailRecoveryData( - rs.getString(col.EMAIL), rs.getString(col.RECOVERY_CODE), rs.getLong(col.RECOVERY_EXPIRATION)); - } - } - } catch (SQLException e) { - logSqlException(e); - } - return null; - } - - @Override - public void removeRecoveryCode(String name) { - String sql = "UPDATE " + tableName - + " SET " + col.RECOVERY_CODE + " = NULL, " - + col.RECOVERY_EXPIRATION + " = NULL" - + " WHERE " + col.NAME + " = ?;"; - try (Connection con = getConnection(); PreparedStatement pst = con.prepareStatement(sql)) { - pst.setString(1, name.toLowerCase()); - pst.executeUpdate(); - } catch (SQLException e) { - logSqlException(e); - } - } - private PlayerAuth buildAuthFromResultSet(ResultSet row) throws SQLException { String salt = col.SALT.isEmpty() ? null : row.getString(col.SALT); int group = col.GROUP.isEmpty() ? -1 : row.getInt(col.GROUP); diff --git a/src/main/java/fr/xephi/authme/datasource/SQLite.java b/src/main/java/fr/xephi/authme/datasource/SQLite.java index f6ae8f90..8aae94c5 100644 --- a/src/main/java/fr/xephi/authme/datasource/SQLite.java +++ b/src/main/java/fr/xephi/authme/datasource/SQLite.java @@ -2,7 +2,6 @@ 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; @@ -128,14 +127,6 @@ public class SQLite implements DataSource { if (isColumnMissing(md, col.IS_LOGGED)) { st.executeUpdate("ALTER TABLE " + tableName + " ADD COLUMN " + col.IS_LOGGED + " INT DEFAULT '0';"); } - - if (isColumnMissing(md, col.RECOVERY_CODE)) { - st.executeUpdate("ALTER TABLE " + tableName + " ADD COLUMN " + col.RECOVERY_CODE + " VARCHAR(20);"); - } - - if (isColumnMissing(md, col.RECOVERY_EXPIRATION)) { - st.executeUpdate("ALTER TABLE " + tableName + " ADD COLUMN " + col.RECOVERY_EXPIRATION + " BIGINT;"); - } } ConsoleLogger.info("SQLite Setup finished"); } @@ -595,55 +586,6 @@ public class SQLite implements DataSource { return auths; } - @Override - public void setRecoveryCode(String name, String code, long expiration) { - String sql = "UPDATE " + tableName - + " SET " + col.RECOVERY_CODE + " = ?, " - + col.RECOVERY_EXPIRATION + " = ?" - + " WHERE " + col.NAME + " = ?;"; - try (PreparedStatement pst = con.prepareStatement(sql)) { - pst.setString(1, code); - pst.setLong(2, expiration); - pst.setString(3, name.toLowerCase()); - pst.executeUpdate(); - } catch (SQLException e) { - logSqlException(e); - } - } - - @Override - 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()); - try (ResultSet rs = pst.executeQuery()) { - if (rs.next()) { - return new EmailRecoveryData( - rs.getString(col.EMAIL), rs.getString(col.RECOVERY_CODE), rs.getLong(col.RECOVERY_EXPIRATION)); - } - } - } catch (SQLException e) { - logSqlException(e); - } - return null; - } - - @Override - public void removeRecoveryCode(String name) { - String sql = "UPDATE " + tableName - + " SET " + col.RECOVERY_CODE + " = NULL, " - + col.RECOVERY_EXPIRATION + " = NULL" - + " WHERE " + col.NAME + " = ?;"; - try (PreparedStatement pst = con.prepareStatement(sql)) { - pst.setString(1, name.toLowerCase()); - pst.executeUpdate(); - } catch (SQLException e) { - logSqlException(e); - } - } - private PlayerAuth buildAuthFromResultSet(ResultSet row) throws SQLException { String salt = !col.SALT.isEmpty() ? row.getString(col.SALT) : null; diff --git a/src/main/java/fr/xephi/authme/service/RecoveryCodeManager.java b/src/main/java/fr/xephi/authme/service/RecoveryCodeManager.java new file mode 100644 index 00000000..bca8dd51 --- /dev/null +++ b/src/main/java/fr/xephi/authme/service/RecoveryCodeManager.java @@ -0,0 +1,73 @@ +package fr.xephi.authme.service; + +import fr.xephi.authme.initialization.SettingsDependent; +import fr.xephi.authme.security.RandomString; +import fr.xephi.authme.settings.Settings; +import fr.xephi.authme.settings.properties.SecuritySettings; + +import javax.inject.Inject; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; + +import static fr.xephi.authme.settings.properties.SecuritySettings.RECOVERY_CODE_HOURS_VALID; +import static fr.xephi.authme.util.Utils.MILLIS_PER_HOUR; + +/** + * Manager for recovery codes. + */ +public class RecoveryCodeManager implements SettingsDependent { + + private Map recoveryCodes = new ConcurrentHashMap<>(); + + private int recoveryCodeLength; + private long recoveryCodeExpirationMillis; + + @Inject + RecoveryCodeManager(Settings settings) { + reload(settings); + } + + public boolean isRecoveryCodeNeeded() { + return recoveryCodeExpirationMillis > 0; + } + + public String generateCode(String player) { + String code = RandomString.generateHex(recoveryCodeLength); + recoveryCodes.put(player, new TimedEntry(code, System.currentTimeMillis() + recoveryCodeExpirationMillis)); + return code; + } + + public boolean isCodeValid(String player, String code) { + TimedEntry entry = recoveryCodes.get(player); + if (entry != null) { + return code != null && code.equals(entry.getCode()); + } + return false; + } + + public void removeCode(String player) { + recoveryCodes.remove(player); + } + + @Override + public void reload(Settings settings) { + recoveryCodeLength = settings.getProperty(SecuritySettings.RECOVERY_CODE_LENGTH); + recoveryCodeExpirationMillis = settings.getProperty(RECOVERY_CODE_HOURS_VALID) * MILLIS_PER_HOUR; + } + + private static final class TimedEntry { + + private final String code; + private final long expiration; + + TimedEntry(String code, long expiration) { + this.code = code; + this.expiration = expiration; + } + + public String getCode() { + return System.currentTimeMillis() < expiration ? code : null; + } + } + +} diff --git a/src/main/java/fr/xephi/authme/settings/properties/DatabaseSettings.java b/src/main/java/fr/xephi/authme/settings/properties/DatabaseSettings.java index 9b76c09c..2dc769ec 100644 --- a/src/main/java/fr/xephi/authme/settings/properties/DatabaseSettings.java +++ b/src/main/java/fr/xephi/authme/settings/properties/DatabaseSettings.java @@ -98,14 +98,6 @@ public class DatabaseSettings implements SettingsHolder { public static final Property MYSQL_COL_GROUP = newProperty("ExternalBoardOptions.mySQLColumnGroup", ""); - @Comment("Column for storing recovery code (when password lost)") - public static final Property MYSQL_COL_RECOVERY_CODE = - newProperty("DataSource.mySQLrecoveryCode", "recoverycode"); - - @Comment("Column for storing recovery code expiration") - public static final Property MYSQL_COL_RECOVERY_EXPIRATION = - newProperty("DataSource.mySQLrecoveryExpiration", "recoveryexpiration"); - private DatabaseSettings() { } diff --git a/src/main/resources/config.yml b/src/main/resources/config.yml index 18721925..6cf97f9f 100644 --- a/src/main/resources/config.yml +++ b/src/main/resources/config.yml @@ -40,10 +40,6 @@ DataSource: mySQLlastlocWorld: world # Column for RealName mySQLRealName: realname - # Column for storing recovery code (when password lost) - mySQLrecoveryCode: recoverycode - # Column for storing recovery code expiration - mySQLrecoveryExpiration: recoveryexpiration settings: # The name shown in the help messages. helpHeader: AuthMeReloaded diff --git a/src/test/java/fr/xephi/authme/command/executable/email/RecoverEmailCommandTest.java b/src/test/java/fr/xephi/authme/command/executable/email/RecoverEmailCommandTest.java index 732e0777..6185d0b2 100644 --- a/src/test/java/fr/xephi/authme/command/executable/email/RecoverEmailCommandTest.java +++ b/src/test/java/fr/xephi/authme/command/executable/email/RecoverEmailCommandTest.java @@ -1,7 +1,7 @@ package fr.xephi.authme.command.executable.email; import fr.xephi.authme.TestHelper; -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.command.CommandService; import fr.xephi.authme.datasource.DataSource; @@ -9,6 +9,7 @@ import fr.xephi.authme.mail.SendMailSSL; import fr.xephi.authme.output.MessageKey; import fr.xephi.authme.security.PasswordSecurity; import fr.xephi.authme.security.crypts.HashedPassword; +import fr.xephi.authme.service.RecoveryCodeManager; import fr.xephi.authme.settings.properties.EmailSettings; import fr.xephi.authme.settings.properties.SecuritySettings; import org.bukkit.entity.Player; @@ -24,11 +25,7 @@ import java.util.Arrays; import java.util.Collections; import static fr.xephi.authme.AuthMeMatchers.stringWithLength; -import static fr.xephi.authme.util.Utils.MILLIS_PER_HOUR; -import static org.hamcrest.Matchers.allOf; import static org.hamcrest.Matchers.containsString; -import static org.hamcrest.Matchers.greaterThan; -import static org.hamcrest.Matchers.lessThan; import static org.junit.Assert.assertThat; import static org.mockito.BDDMockito.given; import static org.mockito.Matchers.any; @@ -66,6 +63,9 @@ public class RecoverEmailCommandTest { @Mock private SendMailSSL sendMailSsl; + + @Mock + private RecoveryCodeManager recoveryCodeManager; @BeforeClass public static void initLogger() { @@ -112,14 +112,14 @@ public class RecoverEmailCommandTest { given(sender.getName()).willReturn(name); given(sendMailSsl.hasAllInformation()).willReturn(true); given(playerCache.isAuthenticated(name)).willReturn(false); - given(dataSource.getEmailRecoveryData(name)).willReturn(null); + given(dataSource.getAuth(name)).willReturn(null); // when command.executeCommand(sender, Collections.singletonList("someone@example.com")); // then verify(sendMailSsl).hasAllInformation(); - verify(dataSource).getEmailRecoveryData(name); + verify(dataSource).getAuth(name); verifyNoMoreInteractions(dataSource); verify(commandService).send(sender, MessageKey.REGISTER_EMAIL_MESSAGE); } @@ -132,14 +132,14 @@ public class RecoverEmailCommandTest { given(sender.getName()).willReturn(name); given(sendMailSsl.hasAllInformation()).willReturn(true); given(playerCache.isAuthenticated(name)).willReturn(false); - given(dataSource.getEmailRecoveryData(name)).willReturn(newEmailRecoveryData(DEFAULT_EMAIL)); + given(dataSource.getAuth(name)).willReturn(newAuthWithEmail(DEFAULT_EMAIL)); // when command.executeCommand(sender, Collections.singletonList(DEFAULT_EMAIL)); // then verify(sendMailSsl).hasAllInformation(); - verify(dataSource).getEmailRecoveryData(name); + verify(dataSource).getAuth(name); verifyNoMoreInteractions(dataSource); verify(commandService).send(sender, MessageKey.INVALID_EMAIL); } @@ -152,14 +152,14 @@ public class RecoverEmailCommandTest { given(sender.getName()).willReturn(name); given(sendMailSsl.hasAllInformation()).willReturn(true); given(playerCache.isAuthenticated(name)).willReturn(false); - given(dataSource.getEmailRecoveryData(name)).willReturn(newEmailRecoveryData("raptor@example.org")); + given(dataSource.getAuth(name)).willReturn(newAuthWithEmail("raptor@example.org")); // when command.executeCommand(sender, Collections.singletonList("wrong-email@example.com")); // then verify(sendMailSsl).hasAllInformation(); - verify(dataSource).getEmailRecoveryData(name); + verify(dataSource).getAuth(name); verifyNoMoreInteractions(dataSource); verify(commandService).send(sender, MessageKey.INVALID_EMAIL); } @@ -173,26 +173,23 @@ public class RecoverEmailCommandTest { given(sendMailSsl.hasAllInformation()).willReturn(true); given(playerCache.isAuthenticated(name)).willReturn(false); String email = "v@example.com"; - given(dataSource.getEmailRecoveryData(name)).willReturn(newEmailRecoveryData(email)); + given(dataSource.getAuth(name)).willReturn(newAuthWithEmail(email)); int codeLength = 7; given(commandService.getProperty(SecuritySettings.RECOVERY_CODE_LENGTH)).willReturn(codeLength); int hoursValid = 12; given(commandService.getProperty(SecuritySettings.RECOVERY_CODE_HOURS_VALID)).willReturn(hoursValid); + String code = "a94f37"; + given(recoveryCodeManager.isRecoveryCodeNeeded()).willReturn(true); + given(recoveryCodeManager.generateCode(name)).willReturn(code); // when command.executeCommand(sender, Collections.singletonList(email.toUpperCase())); // then verify(sendMailSsl).hasAllInformation(); - verify(dataSource).getEmailRecoveryData(name); - ArgumentCaptor codeCaptor = ArgumentCaptor.forClass(String.class); - ArgumentCaptor expirationCaptor = ArgumentCaptor.forClass(Long.class); - verify(dataSource).setRecoveryCode(eq(name), codeCaptor.capture(), expirationCaptor.capture()); - assertThat(codeCaptor.getValue(), stringWithLength(codeLength)); - // Check expiration with a tolerance - assertThat(expirationCaptor.getValue() - System.currentTimeMillis(), - allOf(lessThan(12L * MILLIS_PER_HOUR), greaterThan((long) (11.9 * MILLIS_PER_HOUR)))); - verify(sendMailSsl).sendRecoveryCode(name, email, codeCaptor.getValue()); + verify(dataSource).getAuth(name); + verify(recoveryCodeManager).generateCode(name); + verify(sendMailSsl).sendRecoveryCode(name, email, code); } @Test @@ -204,19 +201,18 @@ public class RecoverEmailCommandTest { 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); + PlayerAuth auth = newAuthWithEmail(email); + given(dataSource.getAuth(name)).willReturn(auth); given(commandService.getProperty(EmailSettings.RECOVERY_PASSWORD_LENGTH)).willReturn(20); - given(passwordSecurity.computeHash(anyString(), eq(name))) - .willAnswer(invocation -> new HashedPassword((String) invocation.getArguments()[0])); + given(recoveryCodeManager.isRecoveryCodeNeeded()).willReturn(true); + given(recoveryCodeManager.isCodeValid(name, "bogus")).willReturn(false); // when command.executeCommand(sender, Arrays.asList(email, "bogus")); // then verify(sendMailSsl).hasAllInformation(); - verify(dataSource, only()).getEmailRecoveryData(name); + verify(dataSource, only()).getAuth(name); verify(sender).sendMessage(argThat(containsString("The recovery code is not correct"))); verifyNoMoreInteractions(sendMailSsl); } @@ -231,35 +227,35 @@ public class RecoverEmailCommandTest { 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); + PlayerAuth auth = newAuthWithEmail(email); + given(dataSource.getAuth(name)).willReturn(auth); given(commandService.getProperty(EmailSettings.RECOVERY_PASSWORD_LENGTH)).willReturn(20); given(passwordSecurity.computeHash(anyString(), eq(name))) .willAnswer(invocation -> new HashedPassword((String) invocation.getArguments()[0])); + given(recoveryCodeManager.isRecoveryCodeNeeded()).willReturn(true); + given(recoveryCodeManager.isCodeValid(name, code)).willReturn(true); // when command.executeCommand(sender, Arrays.asList(email, code)); // then verify(sendMailSsl).hasAllInformation(); - verify(dataSource).getEmailRecoveryData(name); + verify(dataSource).getAuth(name); ArgumentCaptor 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(recoveryCodeManager).removeCode(name); verify(sendMailSsl).sendPasswordMail(name, email, generatedPassword); verify(commandService).send(sender, MessageKey.RECOVERY_EMAIL_SENT_MESSAGE); } - private static EmailRecoveryData newEmailRecoveryData(String email) { - return new EmailRecoveryData(email, null, 0L); + private static PlayerAuth newAuthWithEmail(String email) { + return PlayerAuth.builder() + .name("name") + .email(email) + .build(); } - - private static EmailRecoveryData newEmailRecoveryData(String email, String code) { - return new EmailRecoveryData(email, code, System.currentTimeMillis() + 10_000); - } - } diff --git a/src/test/java/fr/xephi/authme/datasource/AbstractDataSourceIntegrationTest.java b/src/test/java/fr/xephi/authme/datasource/AbstractDataSourceIntegrationTest.java index 59c6ce22..1223130c 100644 --- a/src/test/java/fr/xephi/authme/datasource/AbstractDataSourceIntegrationTest.java +++ b/src/test/java/fr/xephi/authme/datasource/AbstractDataSourceIntegrationTest.java @@ -1,6 +1,5 @@ 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; @@ -382,63 +381,4 @@ public abstract class AbstractDataSourceIntegrationTest { // then assertThat(dataSource.getAllAuths(), empty()); } - - @Test - public void shouldSetRecoveryCode() { - // given - DataSource dataSource = getDataSource(); - String name = "Bobby"; - String code = "A123BC"; - - // when - dataSource.setRecoveryCode(name, code, System.currentTimeMillis() + 100_000L); - - // then - assertThat(dataSource.getEmailRecoveryData(name).getRecoveryCode(), equalTo(code)); - } - - @Test - public void shouldRemoveRecoveryCode() { - // given - String name = "User"; - DataSource dataSource = getDataSource(); - dataSource.setRecoveryCode(name, "code", System.currentTimeMillis() + 20_000L); - - // when - dataSource.removeRecoveryCode(name); - - // then - EmailRecoveryData recoveryData = dataSource.getEmailRecoveryData(name); - assertThat(recoveryData.getRecoveryCode(), nullValue()); - assertThat(recoveryData.getEmail(), equalTo("user@example.org")); - assertThat(dataSource.getEmailRecoveryData("bobby").getRecoveryCode(), nullValue()); - } - - @Test - public void shouldNotReturnRecoveryCodeIfExpired() { - // given - String name = "user"; - DataSource dataSource = getDataSource(); - dataSource.setRecoveryCode(name, "123456", System.currentTimeMillis() - 2_000L); - - // when - EmailRecoveryData recoveryData = dataSource.getEmailRecoveryData(name); - - // then - 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()); - } - } From c78acee6e0c026c5a821a3c1b57a95048b30d215 Mon Sep 17 00:00:00 2001 From: ljacqu Date: Fri, 16 Sep 2016 23:18:32 +0200 Subject: [PATCH 6/7] #472 Add translatable messages and unit tests --- .../executable/email/RecoverEmailCommand.java | 11 +- .../fr/xephi/authme/output/MessageKey.java | 6 +- .../authme/service/RecoveryCodeManager.java | 40 ++++-- .../settings/properties/SecuritySettings.java | 2 +- src/main/resources/config.yml | 2 +- src/main/resources/messages/messages_en.yml | 2 + .../email/RecoverEmailCommandTest.java | 31 +++++ .../service/RecoveryCodeManagerTest.java | 117 ++++++++++++++++++ 8 files changed, 196 insertions(+), 15 deletions(-) create mode 100644 src/test/java/fr/xephi/authme/service/RecoveryCodeManagerTest.java diff --git a/src/main/java/fr/xephi/authme/command/executable/email/RecoverEmailCommand.java b/src/main/java/fr/xephi/authme/command/executable/email/RecoverEmailCommand.java index 6f7923d7..26b0e2f9 100644 --- a/src/main/java/fr/xephi/authme/command/executable/email/RecoverEmailCommand.java +++ b/src/main/java/fr/xephi/authme/command/executable/email/RecoverEmailCommand.java @@ -72,7 +72,7 @@ public class RecoverEmailCommand extends PlayerCommand { if (recoveryCodeManager.isRecoveryCodeNeeded()) { // Process /email recovery addr@example.com if (arguments.size() == 1) { - createAndSendRecoveryCode(playerName, email); + createAndSendRecoveryCode(player, email); } else { // Process /email recovery addr@example.com 12394 processRecoveryCode(player, arguments.get(1), email); @@ -82,15 +82,16 @@ public class RecoverEmailCommand extends PlayerCommand { } } - private void createAndSendRecoveryCode(String name, String email) { - String recoveryCode = recoveryCodeManager.generateCode(name); - sendMailSsl.sendRecoveryCode(name, email, recoveryCode); + private void createAndSendRecoveryCode(Player player, String email) { + String recoveryCode = recoveryCodeManager.generateCode(player.getName()); + sendMailSsl.sendRecoveryCode(player.getName(), email, recoveryCode); + commandService.send(player, MessageKey.RECOVERY_CODE_SENT); } private void processRecoveryCode(Player player, String code, String email) { final String name = player.getName(); if (!recoveryCodeManager.isCodeValid(name, code)) { - player.sendMessage("The recovery code is not correct! Use /email recovery [email] to generate a new one"); + commandService.send(player, MessageKey.INCORRECT_RECOVERY_CODE); return; } diff --git a/src/main/java/fr/xephi/authme/output/MessageKey.java b/src/main/java/fr/xephi/authme/output/MessageKey.java index c445b9a2..c5a7cd36 100644 --- a/src/main/java/fr/xephi/authme/output/MessageKey.java +++ b/src/main/java/fr/xephi/authme/output/MessageKey.java @@ -147,7 +147,11 @@ public enum MessageKey { KICK_FOR_ADMIN_REGISTER("kicked_admin_registered"), - INCOMPLETE_EMAIL_SETTINGS("incomplete_email_settings"); + INCOMPLETE_EMAIL_SETTINGS("incomplete_email_settings"), + + RECOVERY_CODE_SENT("recovery_code_sent"), + + INCORRECT_RECOVERY_CODE("recovery_code_incorrect"); private String key; private String[] tags; diff --git a/src/main/java/fr/xephi/authme/service/RecoveryCodeManager.java b/src/main/java/fr/xephi/authme/service/RecoveryCodeManager.java index bca8dd51..e37d2273 100644 --- a/src/main/java/fr/xephi/authme/service/RecoveryCodeManager.java +++ b/src/main/java/fr/xephi/authme/service/RecoveryCodeManager.java @@ -1,5 +1,6 @@ package fr.xephi.authme.service; +import com.google.common.annotations.VisibleForTesting; import fr.xephi.authme.initialization.SettingsDependent; import fr.xephi.authme.security.RandomString; import fr.xephi.authme.settings.Settings; @@ -17,7 +18,7 @@ import static fr.xephi.authme.util.Utils.MILLIS_PER_HOUR; */ public class RecoveryCodeManager implements SettingsDependent { - private Map recoveryCodes = new ConcurrentHashMap<>(); + private Map recoveryCodes = new ConcurrentHashMap<>(); private int recoveryCodeLength; private long recoveryCodeExpirationMillis; @@ -27,24 +28,45 @@ public class RecoveryCodeManager implements SettingsDependent { reload(settings); } + /** + * @return whether recovery codes are enabled or not + */ public boolean isRecoveryCodeNeeded() { - return recoveryCodeExpirationMillis > 0; + return recoveryCodeLength > 0 && recoveryCodeExpirationMillis > 0; } + /** + * Generates the recovery code for the given player. + * + * @param player the player to generate a code for + * @return the generated code + */ public String generateCode(String player) { String code = RandomString.generateHex(recoveryCodeLength); - recoveryCodes.put(player, new TimedEntry(code, System.currentTimeMillis() + recoveryCodeExpirationMillis)); + recoveryCodes.put(player, new ExpiringEntry(code, System.currentTimeMillis() + recoveryCodeExpirationMillis)); return code; } + /** + * Checks whether the supplied code is valid for the given player. + * + * @param player the player to check for + * @param code the code to check + * @return true if the code matches and has not expired, false otherwise + */ public boolean isCodeValid(String player, String code) { - TimedEntry entry = recoveryCodes.get(player); + ExpiringEntry entry = recoveryCodes.get(player); if (entry != null) { return code != null && code.equals(entry.getCode()); } return false; } + /** + * Removes the player's recovery code if present. + * + * @param player the player + */ public void removeCode(String player) { recoveryCodes.remove(player); } @@ -55,17 +77,21 @@ public class RecoveryCodeManager implements SettingsDependent { recoveryCodeExpirationMillis = settings.getProperty(RECOVERY_CODE_HOURS_VALID) * MILLIS_PER_HOUR; } - private static final class TimedEntry { + /** + * Entry with an expiration. + */ + @VisibleForTesting + static final class ExpiringEntry { private final String code; private final long expiration; - TimedEntry(String code, long expiration) { + ExpiringEntry(String code, long expiration) { this.code = code; this.expiration = expiration; } - public String getCode() { + String getCode() { return System.currentTimeMillis() < expiration ? code : null; } } diff --git a/src/main/java/fr/xephi/authme/settings/properties/SecuritySettings.java b/src/main/java/fr/xephi/authme/settings/properties/SecuritySettings.java index fb1cf258..7d91f818 100644 --- a/src/main/java/fr/xephi/authme/settings/properties/SecuritySettings.java +++ b/src/main/java/fr/xephi/authme/settings/properties/SecuritySettings.java @@ -109,7 +109,7 @@ public class SecuritySettings implements SettingsHolder { public static final Property TEMPBAN_MINUTES_BEFORE_RESET = newProperty("Security.tempban.minutesBeforeCounterReset", 480); - @Comment("Number of characters a recovery code should have") + @Comment("Number of characters a recovery code should have (0 to disable)") public static final Property RECOVERY_CODE_LENGTH = newProperty("Security.recoveryCode.length", 8); diff --git a/src/main/resources/config.yml b/src/main/resources/config.yml index 6cf97f9f..3221913b 100644 --- a/src/main/resources/config.yml +++ b/src/main/resources/config.yml @@ -339,7 +339,7 @@ Security: # Default: 480 minutes (8 hours) minutesBeforeCounterReset: 480 recoveryCode: - # Number of characters a recovery code should have + # Number of characters a recovery code should have (0 to disable) length: 8 # How many hours is a recovery code valid for? validForHours: 4 diff --git a/src/main/resources/messages/messages_en.yml b/src/main/resources/messages/messages_en.yml index f96db604..a942fe85 100644 --- a/src/main/resources/messages/messages_en.yml +++ b/src/main/resources/messages/messages_en.yml @@ -70,3 +70,5 @@ accounts_owned_self: 'You own %count accounts:' accounts_owned_other: 'The player %name has %count accounts:' kicked_admin_registered: 'An admin just registered you; please log in again' incomplete_email_settings: 'Error: not all required settings are set for sending emails. Please contact an admin.' +recovery_code_sent: 'A recovery code to reset your password has been sent to your email.' +recovery_code_incorrect: 'The recovery code is not correct! Use /email recovery [email] to generate a new one' diff --git a/src/test/java/fr/xephi/authme/command/executable/email/RecoverEmailCommandTest.java b/src/test/java/fr/xephi/authme/command/executable/email/RecoverEmailCommandTest.java index 6185d0b2..d57693cf 100644 --- a/src/test/java/fr/xephi/authme/command/executable/email/RecoverEmailCommandTest.java +++ b/src/test/java/fr/xephi/authme/command/executable/email/RecoverEmailCommandTest.java @@ -251,6 +251,37 @@ public class RecoverEmailCommandTest { verify(commandService).send(sender, MessageKey.RECOVERY_EMAIL_SENT_MESSAGE); } + @Test + public void shouldGenerateNewPasswordWithoutRecoveryCode() { + // given + String name = "sh4rK"; + Player sender = mock(Player.class); + given(sender.getName()).willReturn(name); + given(sendMailSsl.hasAllInformation()).willReturn(true); + given(playerCache.isAuthenticated(name)).willReturn(false); + String email = "shark@example.org"; + PlayerAuth auth = newAuthWithEmail(email); + given(dataSource.getAuth(name)).willReturn(auth); + given(commandService.getProperty(EmailSettings.RECOVERY_PASSWORD_LENGTH)).willReturn(20); + given(passwordSecurity.computeHash(anyString(), eq(name))) + .willAnswer(invocation -> new HashedPassword((String) invocation.getArguments()[0])); + given(recoveryCodeManager.isRecoveryCodeNeeded()).willReturn(false); + + // when + command.executeCommand(sender, Collections.singletonList(email)); + + // then + verify(sendMailSsl).hasAllInformation(); + verify(dataSource).getAuth(name); + ArgumentCaptor 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(sendMailSsl).sendPasswordMail(name, email, generatedPassword); + verify(commandService).send(sender, MessageKey.RECOVERY_EMAIL_SENT_MESSAGE); + } + private static PlayerAuth newAuthWithEmail(String email) { return PlayerAuth.builder() diff --git a/src/test/java/fr/xephi/authme/service/RecoveryCodeManagerTest.java b/src/test/java/fr/xephi/authme/service/RecoveryCodeManagerTest.java new file mode 100644 index 00000000..c606a743 --- /dev/null +++ b/src/test/java/fr/xephi/authme/service/RecoveryCodeManagerTest.java @@ -0,0 +1,117 @@ +package fr.xephi.authme.service; + +import ch.jalu.injector.testing.BeforeInjecting; +import ch.jalu.injector.testing.DelayedInjectionRunner; +import ch.jalu.injector.testing.InjectDelayed; +import fr.xephi.authme.ReflectionTestUtils; +import fr.xephi.authme.service.RecoveryCodeManager.ExpiringEntry; +import fr.xephi.authme.settings.Settings; +import fr.xephi.authme.settings.properties.SecuritySettings; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; + +import java.util.Map; + +import static fr.xephi.authme.AuthMeMatchers.stringWithLength; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.nullValue; +import static org.junit.Assert.assertThat; +import static org.mockito.BDDMockito.given; + +/** + * Test for {@link RecoveryCodeManager}. + */ +@RunWith(DelayedInjectionRunner.class) +public class RecoveryCodeManagerTest { + + @InjectDelayed + private RecoveryCodeManager recoveryCodeManager; + + @Mock + private Settings settings; + + @BeforeInjecting + public void initSettings() { + given(settings.getProperty(SecuritySettings.RECOVERY_CODE_HOURS_VALID)).willReturn(4); + given(settings.getProperty(SecuritySettings.RECOVERY_CODE_LENGTH)).willReturn(5); + } + + @Test + public void shouldBeDisabledForNonPositiveLength() { + assertThat(recoveryCodeManager.isRecoveryCodeNeeded(), equalTo(true)); + + // given + given(settings.getProperty(SecuritySettings.RECOVERY_CODE_LENGTH)).willReturn(0); + + // when + recoveryCodeManager.reload(settings); + + // then + assertThat(recoveryCodeManager.isRecoveryCodeNeeded(), equalTo(false)); + } + + @Test + public void shouldGenerateAndStoreCode() { + // given + String name = "Bobbers"; + + // when + recoveryCodeManager.generateCode(name); + + // then + ExpiringEntry entry = getCodeMap().get(name); + assertThat(entry.getCode(), stringWithLength(5)); + } + + @Test + public void shouldNotConsiderExpiredCode() { + // given + String player = "Cat"; + String code = "11F235"; + setCodeInMap(player, code, System.currentTimeMillis() - 500); + + // when + boolean result = recoveryCodeManager.isCodeValid(player, code); + + // then + assertThat(result, equalTo(false)); + } + + @Test + public void shouldRecognizeCorrectCode() { + // given + String player = "dragon"; + String code = recoveryCodeManager.generateCode(player); + + // when + boolean result = recoveryCodeManager.isCodeValid(player, code); + + // then + assertThat(result, equalTo(true)); + } + + @Test + public void shouldRemoveCode() { + // given + String player = "Tester"; + String code = recoveryCodeManager.generateCode(player); + + // when + recoveryCodeManager.removeCode(player); + + // then + assertThat(recoveryCodeManager.isCodeValid(player, code), equalTo(false)); + assertThat(getCodeMap().get(player), nullValue()); + } + + + private Map getCodeMap() { + return ReflectionTestUtils.getFieldValue(RecoveryCodeManager.class, recoveryCodeManager, "recoveryCodes"); + } + + private void setCodeInMap(String player, String code, long expiration) { + Map map = getCodeMap(); + map.put(player, new ExpiringEntry(code, expiration)); + } +} From d55ede5dab9c291cdbf6c199e527b8cc76003a76 Mon Sep 17 00:00:00 2001 From: ljacqu Date: Fri, 16 Sep 2016 23:45:40 +0200 Subject: [PATCH 7/7] Fix failing test --- .../command/executable/email/RecoverEmailCommandTest.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/test/java/fr/xephi/authme/command/executable/email/RecoverEmailCommandTest.java b/src/test/java/fr/xephi/authme/command/executable/email/RecoverEmailCommandTest.java index d57693cf..14d2cc62 100644 --- a/src/test/java/fr/xephi/authme/command/executable/email/RecoverEmailCommandTest.java +++ b/src/test/java/fr/xephi/authme/command/executable/email/RecoverEmailCommandTest.java @@ -25,12 +25,10 @@ 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.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; @@ -189,6 +187,7 @@ public class RecoverEmailCommandTest { verify(sendMailSsl).hasAllInformation(); verify(dataSource).getAuth(name); verify(recoveryCodeManager).generateCode(name); + verify(commandService).send(sender, MessageKey.RECOVERY_CODE_SENT); verify(sendMailSsl).sendRecoveryCode(name, email, code); } @@ -213,7 +212,7 @@ public class RecoverEmailCommandTest { // then verify(sendMailSsl).hasAllInformation(); verify(dataSource, only()).getAuth(name); - verify(sender).sendMessage(argThat(containsString("The recovery code is not correct"))); + verify(commandService).send(sender, MessageKey.INCORRECT_RECOVERY_CODE); verifyNoMoreInteractions(sendMailSsl); }