diff --git a/src/main/java/fr/xephi/authme/cache/SessionManager.java b/src/main/java/fr/xephi/authme/cache/SessionManager.java index 95b6d8db5..33d3f9a63 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 f2d9cc5ba..832c650d1 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/command/CommandInitializer.java b/src/main/java/fr/xephi/authme/command/CommandInitializer.java index 0d5c6da07..b29fd2150 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 8965c96fa..26b0e2f9c 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 @@ -11,12 +11,17 @@ 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.EmailSettings.RECOVERY_PASSWORD_LENGTH; + +/** + * Command for password recovery by email. + */ public class RecoverEmailCommand extends PlayerCommand { @Inject @@ -34,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); @@ -49,22 +57,55 @@ public class RecoverEmailCommand extends PlayerCommand { return; } - PlayerAuth auth = dataSource.getAuth(playerName); + PlayerAuth auth = dataSource.getAuth(playerName); // TODO: Create method to get email only if (auth == null) { commandService.send(player, MessageKey.REGISTER_EMAIL_MESSAGE); return; } - if (!playerMail.equalsIgnoreCase(auth.getEmail()) || "your@email.com".equalsIgnoreCase(auth.getEmail())) { + final String email = auth.getEmail(); + if (email == null || !email.equalsIgnoreCase(playerMail) || "your@email.com".equalsIgnoreCase(email)) { commandService.send(player, MessageKey.INVALID_EMAIL); return; } - 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); + if (recoveryCodeManager.isRecoveryCodeNeeded()) { + // Process /email recovery addr@example.com + if (arguments.size() == 1) { + createAndSendRecoveryCode(player, email); + } else { + // Process /email recovery addr@example.com 12394 + processRecoveryCode(player, arguments.get(1), email); + } + } else { + generateAndSendNewPassword(player, email); + } + } + + 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)) { + commandService.send(player, MessageKey.INCORRECT_RECOVERY_CODE); + return; + } + + 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); + 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 a79cb5805..bab51daf1 100644 --- a/src/main/java/fr/xephi/authme/datasource/CacheDataSource.java +++ b/src/main/java/fr/xephi/authme/datasource/CacheDataSource.java @@ -8,7 +8,6 @@ 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.PlayerAuth; import fr.xephi.authme.cache.auth.PlayerCache; diff --git a/src/main/java/fr/xephi/authme/mail/SendMailSSL.java b/src/main/java/fr/xephi/authme/mail/SendMailSSL.java index 6088a5bca..74dc6db36 100644 --- a/src/main/java/fr/xephi/authme/mail/SendMailSSL.java +++ b/src/main/java/fr/xephi/authme/mail/SendMailSSL.java @@ -2,9 +2,9 @@ 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.settings.properties.SecuritySettings; import fr.xephi.authme.util.BukkitService; import fr.xephi.authme.util.StringUtils; import org.apache.commons.mail.EmailConstants; @@ -53,16 +53,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 = replaceTagsForPasswordMail(settings.getPasswordEmailMessage(), name, newPass); bukkitService.runTaskAsynchronously(new Runnable() { @Override @@ -70,7 +71,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 +82,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 +98,20 @@ public class SendMailSSL { }); } + 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 { + 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,13 +164,21 @@ public class SendMailSSL { } } - private String replaceMailTags(String mailText, PlayerAuth auth, String newPass) { + private String replaceTagsForPasswordMail(String mailText, String name, String newPass) { return mailText - .replace("", auth.getNickname()) + .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/output/MessageKey.java b/src/main/java/fr/xephi/authme/output/MessageKey.java index c445b9a2d..c5a7cd368 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/process/register/AsyncRegister.java b/src/main/java/fr/xephi/authme/process/register/AsyncRegister.java index 57390a54a..a7cf57949 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/main/java/fr/xephi/authme/service/RecoveryCodeManager.java b/src/main/java/fr/xephi/authme/service/RecoveryCodeManager.java new file mode 100644 index 000000000..e37d22733 --- /dev/null +++ b/src/main/java/fr/xephi/authme/service/RecoveryCodeManager.java @@ -0,0 +1,99 @@ +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; +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); + } + + /** + * @return whether recovery codes are enabled or not + */ + public boolean isRecoveryCodeNeeded() { + 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 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) { + 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); + } + + @Override + public void reload(Settings settings) { + recoveryCodeLength = settings.getProperty(SecuritySettings.RECOVERY_CODE_LENGTH); + recoveryCodeExpirationMillis = settings.getProperty(RECOVERY_CODE_HOURS_VALID) * MILLIS_PER_HOUR; + } + + /** + * Entry with an expiration. + */ + @VisibleForTesting + static final class ExpiringEntry { + + private final String code; + private final long expiration; + + ExpiringEntry(String code, long expiration) { + this.code = code; + this.expiration = expiration; + } + + String getCode() { + return System.currentTimeMillis() < expiration ? code : null; + } + } + +} diff --git a/src/main/java/fr/xephi/authme/settings/Settings.java b/src/main/java/fr/xephi/authme/settings/Settings.java index 855c14df5..9a1cb5fa5 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 aa3f3783d..7d91f8183 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 (0 to disable)") + 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 c2122ee3e..3a7040d17 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 07f3c3c7e..3221913b1 100644 --- a/src/main/resources/config.yml +++ b/src/main/resources/config.yml @@ -338,6 +338,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 (0 to disable) + length: 8 + # How many hours is a recovery code valid for? + validForHours: 4 Converter: Rakamak: # Rakamak file name diff --git a/src/main/resources/messages/messages_en.yml b/src/main/resources/messages/messages_en.yml index f96db6044..a942fe85d 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/main/resources/recovery_code_email.html b/src/main/resources/recovery_code_email.html new file mode 100644 index 000000000..e5614f4f4 --- /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/TestHelper.java b/src/test/java/fr/xephi/authme/TestHelper.java index e6435b5a9..f0f9da8d1 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/command/executable/email/RecoverEmailCommandTest.java b/src/test/java/fr/xephi/authme/command/executable/email/RecoverEmailCommandTest.java index 2f57443ae..14d2cc62b 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 @@ -9,26 +9,29 @@ 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; 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.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; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.verifyZeroInteractions; @@ -58,6 +61,9 @@ public class RecoverEmailCommandTest { @Mock private SendMailSSL sendMailSsl; + + @Mock + private RecoveryCodeManager recoveryCodeManager; @BeforeClass public static void initLogger() { @@ -124,7 +130,7 @@ 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.getAuth(name)).willReturn(newAuthWithEmail(DEFAULT_EMAIL)); // when command.executeCommand(sender, Collections.singletonList(DEFAULT_EMAIL)); @@ -144,7 +150,7 @@ 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.getAuth(name)).willReturn(newAuthWithEmail("raptor@example.org")); // when command.executeCommand(sender, Collections.singletonList("wrong-email@example.com")); @@ -156,6 +162,60 @@ public class RecoverEmailCommandTest { 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.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).getAuth(name); + verify(recoveryCodeManager).generateCode(name); + verify(commandService).send(sender, MessageKey.RECOVERY_CODE_SENT); + verify(sendMailSsl).sendRecoveryCode(name, email, code); + } + + @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"; + PlayerAuth auth = newAuthWithEmail(email); + given(dataSource.getAuth(name)).willReturn(auth); + given(commandService.getProperty(EmailSettings.RECOVERY_PASSWORD_LENGTH)).willReturn(20); + 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()).getAuth(name); + verify(commandService).send(sender, MessageKey.INCORRECT_RECOVERY_CODE); + verifyNoMoreInteractions(sendMailSsl); + } + @Test public void shouldResetPasswordAndSendEmail() { // given @@ -165,36 +225,67 @@ public class RecoverEmailCommandTest { given(sendMailSsl.hasAllInformation()).willReturn(true); given(playerCache.isAuthenticated(name)).willReturn(false); String email = "vulture@example.com"; - PlayerAuth auth = authWithEmail(email); + String code = "A6EF3AC8"; + 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(new Answer() { - @Override - public HashedPassword answer(InvocationOnMock invocationOnMock) { - return new HashedPassword((String) invocationOnMock.getArguments()[0]); - } - }); + .willAnswer(invocation -> new HashedPassword((String) invocation.getArguments()[0])); + given(recoveryCodeManager.isRecoveryCodeNeeded()).willReturn(true); + given(recoveryCodeManager.isCodeValid(name, code)).willReturn(true); // 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))); + 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(recoveryCodeManager).removeCode(name); + verify(sendMailSsl).sendPasswordMail(name, email, generatedPassword); + 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 authWithEmail(String email) { + private static PlayerAuth newAuthWithEmail(String email) { return PlayerAuth.builder() - .name("tester") + .name("name") .email(email) .build(); } - } diff --git a/src/test/java/fr/xephi/authme/datasource/AbstractDataSourceIntegrationTest.java b/src/test/java/fr/xephi/authme/datasource/AbstractDataSourceIntegrationTest.java index 7fbccdd4a..1223130ca 100644 --- a/src/test/java/fr/xephi/authme/datasource/AbstractDataSourceIntegrationTest.java +++ b/src/test/java/fr/xephi/authme/datasource/AbstractDataSourceIntegrationTest.java @@ -381,5 +381,4 @@ public abstract class AbstractDataSourceIntegrationTest { // then assertThat(dataSource.getAllAuths(), empty()); } - } diff --git a/src/test/java/fr/xephi/authme/datasource/MySqlIntegrationTest.java b/src/test/java/fr/xephi/authme/datasource/MySqlIntegrationTest.java index ca8089216..93d8a3998 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 e0eb3eca8..4836b198d 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/java/fr/xephi/authme/service/RecoveryCodeManagerTest.java b/src/test/java/fr/xephi/authme/service/RecoveryCodeManagerTest.java new file mode 100644 index 000000000..c606a743a --- /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)); + } +} diff --git a/src/test/java/fr/xephi/authme/settings/SettingsTest.java b/src/test/java/fr/xephi/authme/settings/SettingsTest.java index 5f4bac9b3..3cde1f2d9 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)); 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 5dea47d51..48891c747 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) );