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/cache/auth/EmailRecoveryData.java b/src/main/java/fr/xephi/authme/cache/auth/EmailRecoveryData.java index 392b14c61..3cc025776 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 671a31bde..cf9c3d4fb 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 8b8f8b7e4..74dc6db36 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 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..fb1cf2584 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 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 a744f43cd..187219257 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 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/command/executable/email/RecoverEmailCommandTest.java b/src/test/java/fr/xephi/authme/command/executable/email/RecoverEmailCommandTest.java index 4d60877e1..732e07774 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 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));