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 eb188f790..b3fb3b62b 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 @@ -5,24 +5,31 @@ import fr.xephi.authme.command.PlayerCommand; import fr.xephi.authme.data.auth.PlayerAuth; import fr.xephi.authme.data.auth.PlayerCache; import fr.xephi.authme.datasource.DataSource; +import fr.xephi.authme.initialization.Reloadable; import fr.xephi.authme.mail.EmailService; import fr.xephi.authme.message.MessageKey; +import fr.xephi.authme.message.Messages; import fr.xephi.authme.security.PasswordSecurity; import fr.xephi.authme.security.crypts.HashedPassword; import fr.xephi.authme.service.CommonService; import fr.xephi.authme.service.RecoveryCodeService; +import fr.xephi.authme.settings.properties.SecuritySettings; import fr.xephi.authme.util.RandomStringUtils; +import fr.xephi.authme.util.expiring.Duration; +import fr.xephi.authme.util.expiring.ExpiringSet; import org.bukkit.entity.Player; +import javax.annotation.PostConstruct; import javax.inject.Inject; import java.util.List; +import java.util.concurrent.TimeUnit; import static fr.xephi.authme.settings.properties.EmailSettings.RECOVERY_PASSWORD_LENGTH; /** * Command for password recovery by email. */ -public class RecoverEmailCommand extends PlayerCommand { +public class RecoverEmailCommand extends PlayerCommand implements Reloadable { @Inject private PasswordSecurity passwordSecurity; @@ -42,8 +49,19 @@ public class RecoverEmailCommand extends PlayerCommand { @Inject private RecoveryCodeService recoveryCodeService; + @Inject + private Messages messages; + + private ExpiringSet emailCooldown; + + @PostConstruct + private void initEmailCooldownSet() { + emailCooldown = new ExpiringSet<>( + commonService.getProperty(SecuritySettings.EMAIL_RECOVERY_COOLDOWN_SECONDS), TimeUnit.SECONDS); + } + @Override - public void runCommand(Player player, List arguments) { + protected void runCommand(Player player, List arguments) { final String playerMail = arguments.get(0); final String playerName = player.getName(); @@ -78,15 +96,29 @@ public class RecoverEmailCommand extends PlayerCommand { processRecoveryCode(player, arguments.get(1), email); } } else { - generateAndSendNewPassword(player, email); + boolean maySendMail = checkEmailCooldown(player); + if (maySendMail) { + generateAndSendNewPassword(player, email); + } } } + @Override + public void reload() { + emailCooldown.setExpiration( + commonService.getProperty(SecuritySettings.EMAIL_RECOVERY_COOLDOWN_SECONDS), TimeUnit.SECONDS); + } + private void createAndSendRecoveryCode(Player player, String email) { + if (!checkEmailCooldown(player)) { + return; + } + String recoveryCode = recoveryCodeService.generateCode(player.getName()); boolean couldSendMail = emailService.sendRecoveryCode(player.getName(), email, recoveryCode); if (couldSendMail) { commonService.send(player, MessageKey.RECOVERY_CODE_SENT); + emailCooldown.add(player.getName().toLowerCase()); } else { commonService.send(player, MessageKey.EMAIL_SEND_FAILURE); } @@ -111,8 +143,19 @@ public class RecoverEmailCommand extends PlayerCommand { boolean couldSendMail = emailService.sendPasswordMail(name, email, thePass); if (couldSendMail) { commonService.send(player, MessageKey.RECOVERY_EMAIL_SENT_MESSAGE); + emailCooldown.add(player.getName().toLowerCase()); } else { commonService.send(player, MessageKey.EMAIL_SEND_FAILURE); } } + + private boolean checkEmailCooldown(Player player) { + Duration waitDuration = emailCooldown.getExpiration(player.getName().toLowerCase()); + if (waitDuration.getDuration() > 0) { + String durationText = messages.formatDuration(waitDuration); + messages.send(player, MessageKey.EMAIL_COOLDOWN_ERROR, durationText); + return false; + } + return true; + } } diff --git a/src/main/java/fr/xephi/authme/message/MessageKey.java b/src/main/java/fr/xephi/authme/message/MessageKey.java index 7546a89c0..e8c3939a9 100644 --- a/src/main/java/fr/xephi/authme/message/MessageKey.java +++ b/src/main/java/fr/xephi/authme/message/MessageKey.java @@ -225,7 +225,35 @@ public enum MessageKey { RECOVERY_CODE_SENT("recovery_code_sent"), /** The recovery code is not correct! Use "/email recovery [email]" to generate a new one */ - INCORRECT_RECOVERY_CODE("recovery_code_incorrect"); + INCORRECT_RECOVERY_CODE("recovery_code_incorrect"), + + /** An email was already sent recently. You must wait %time before you can send a new one. */ + EMAIL_COOLDOWN_ERROR("email_cooldown_error", "%time"), + + /** second */ + SECOND("second"), + + /** seconds */ + SECONDS("seconds"), + + /** minute */ + MINUTE("minute"), + + /** minutes */ + MINUTES("minutes"), + + /** hour */ + HOUR("hour"), + + /** hours */ + HOURS("hours"), + + /** day */ + DAY("day"), + + /** days */ + DAYS("days"); + private String key; private String[] tags; diff --git a/src/main/java/fr/xephi/authme/message/Messages.java b/src/main/java/fr/xephi/authme/message/Messages.java index 5d777ab83..d7fdcec89 100644 --- a/src/main/java/fr/xephi/authme/message/Messages.java +++ b/src/main/java/fr/xephi/authme/message/Messages.java @@ -1,11 +1,15 @@ package fr.xephi.authme.message; +import com.google.common.collect.ImmutableMap; import fr.xephi.authme.ConsoleLogger; import fr.xephi.authme.initialization.Reloadable; +import fr.xephi.authme.util.expiring.Duration; import org.bukkit.ChatColor; import org.bukkit.command.CommandSender; import javax.inject.Inject; +import java.util.Map; +import java.util.concurrent.TimeUnit; /** * Class for retrieving and sending translatable messages to players. @@ -15,6 +19,20 @@ public class Messages implements Reloadable { // Custom Authme tag replaced to new line private static final String NEWLINE_TAG = "%nl%"; + /** Contains the keys of the singular messages for time units. */ + private static final Map TIME_UNIT_SINGULARS = ImmutableMap.builder() + .put(TimeUnit.SECONDS, MessageKey.SECOND) + .put(TimeUnit.MINUTES, MessageKey.MINUTE) + .put(TimeUnit.HOURS, MessageKey.HOUR) + .put(TimeUnit.DAYS, MessageKey.DAY).build(); + + /** Contains the keys of the plural messages for time units. */ + private static final Map TIME_UNIT_PLURALS = ImmutableMap.builder() + .put(TimeUnit.SECONDS, MessageKey.SECONDS) + .put(TimeUnit.MINUTES, MessageKey.MINUTES) + .put(TimeUnit.HOURS, MessageKey.HOURS) + .put(TimeUnit.DAYS, MessageKey.DAYS).build(); + private final MessageFileHandlerProvider messageFileHandlerProvider; private MessageFileHandler messageFileHandler; @@ -71,6 +89,22 @@ public class Messages implements Reloadable { return message.split("\n"); } + /** + * Returns the textual representation for the given duration. + * Note that this class only supports the time units days, hour, minutes and seconds. + * + * @param duration the duration to build a text of + * @return text of the duration + */ + public String formatDuration(Duration duration) { + long value = duration.getDuration(); + MessageKey timeUnitKey = value == 1 + ? TIME_UNIT_SINGULARS.get(duration.getTimeUnit()) + : TIME_UNIT_PLURALS.get(duration.getTimeUnit()); + + return value + " " + retrieveMessage(timeUnitKey); + } + /** * Retrieve the message from the text file. * 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 daed1a8c9..496fb3b2b 100644 --- a/src/main/java/fr/xephi/authme/settings/properties/SecuritySettings.java +++ b/src/main/java/fr/xephi/authme/settings/properties/SecuritySettings.java @@ -114,6 +114,13 @@ public class SecuritySettings implements SettingsHolder { public static final Property RECOVERY_CODE_HOURS_VALID = newProperty("Security.recoveryCode.validForHours", 4); + @Comment({ + "Seconds a user has to wait for before a password recovery mail may be sent again", + "This prevents an attacker from abusing AuthMe's email feature." + }) + public static final Property EMAIL_RECOVERY_COOLDOWN_SECONDS = + newProperty("Security.emailRecovery.cooldown", 60); + 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 e7ec1657a..b7628147b 100644 --- a/src/main/java/fr/xephi/authme/util/Utils.java +++ b/src/main/java/fr/xephi/authme/util/Utils.java @@ -3,7 +3,6 @@ package fr.xephi.authme.util; import fr.xephi.authme.ConsoleLogger; import java.util.Collection; -import java.util.concurrent.TimeUnit; import java.util.regex.Pattern; /** @@ -71,36 +70,4 @@ public final class Utils { return Runtime.getRuntime().availableProcessors(); } - public static Duration convertMillisToSuitableUnit(long duration) { - TimeUnit targetUnit; - if (duration > 1000L * 60L * 60L * 24L) { - targetUnit = TimeUnit.DAYS; - } else if (duration > 1000L * 60L * 60L) { - targetUnit = TimeUnit.HOURS; - } else if (duration > 1000L * 60L) { - targetUnit = TimeUnit.MINUTES; - } else if (duration > 1000L) { - targetUnit = TimeUnit.SECONDS; - } else { - targetUnit = TimeUnit.MILLISECONDS; - } - - return new Duration(targetUnit, duration); - } - - public static final class Duration { - - private final long duration; - private final TimeUnit unit; - - Duration(TimeUnit targetUnit, long durationMillis) { - this(targetUnit, durationMillis, TimeUnit.MILLISECONDS); - } - - Duration(TimeUnit targetUnit, long sourceDuration, TimeUnit sourceUnit) { - this.duration = targetUnit.convert(sourceDuration, sourceUnit); - this.unit = targetUnit; - } - } - } diff --git a/src/main/java/fr/xephi/authme/util/expiring/ExpiringMap.java b/src/main/java/fr/xephi/authme/util/expiring/ExpiringMap.java index 894b6486e..e920805c2 100644 --- a/src/main/java/fr/xephi/authme/util/expiring/ExpiringMap.java +++ b/src/main/java/fr/xephi/authme/util/expiring/ExpiringMap.java @@ -46,7 +46,13 @@ public class ExpiringMap { */ public V get(K key) { ExpiringEntry value = entries.get(key); - return value == null ? null : value.getValue(); + if (value == null) { + return null; + } else if (System.currentTimeMillis() > value.getExpiration()) { + entries.remove(key); + return null; + } + return value.getValue(); } /** @@ -115,7 +121,7 @@ public class ExpiringMap { } V getValue() { - return System.currentTimeMillis() > expiration ? null : value; + return value; } long getExpiration() { diff --git a/src/main/java/fr/xephi/authme/util/expiring/ExpiringSet.java b/src/main/java/fr/xephi/authme/util/expiring/ExpiringSet.java index 7e711673e..fea8fb313 100644 --- a/src/main/java/fr/xephi/authme/util/expiring/ExpiringSet.java +++ b/src/main/java/fr/xephi/authme/util/expiring/ExpiringSet.java @@ -83,23 +83,22 @@ public class ExpiringSet { /** * Returns the duration of the entry until it expires (provided it is not removed or re-added). - * If the entry does not exist, -1 is returned. + * If the entry does not exist, a duration of -1 seconds is returned. * * @param entry the entry whose duration before it expires should be returned - * @param unit the unit in which to return the duration * @return duration the entry will remain in the set (if there are not modifications) */ - public long getExpiration(E entry, TimeUnit unit) { + public Duration getExpiration(E entry) { Long expiration = entries.get(entry); if (expiration == null) { - return -1; + return new Duration(-1, TimeUnit.SECONDS); } long stillPresentMillis = expiration - System.currentTimeMillis(); if (stillPresentMillis < 0) { entries.remove(entry); - return -1; + return new Duration(-1, TimeUnit.SECONDS); } - return unit.convert(stillPresentMillis, TimeUnit.MILLISECONDS); + return Duration.createWithSuitableUnit(stillPresentMillis, TimeUnit.MILLISECONDS); } /** diff --git a/src/main/java/fr/xephi/authme/util/expiring/TimedCounter.java b/src/main/java/fr/xephi/authme/util/expiring/TimedCounter.java index 67839294c..c3ae908cd 100644 --- a/src/main/java/fr/xephi/authme/util/expiring/TimedCounter.java +++ b/src/main/java/fr/xephi/authme/util/expiring/TimedCounter.java @@ -1,6 +1,5 @@ package fr.xephi.authme.util.expiring; -import java.util.Objects; import java.util.concurrent.TimeUnit; /** @@ -42,9 +41,10 @@ public class TimedCounter extends ExpiringMap { * @return the total of all valid entries */ public int total() { + long currentTime = System.currentTimeMillis(); return entries.values().stream() + .filter(entry -> currentTime <= entry.getExpiration()) .map(ExpiringEntry::getValue) - .filter(Objects::nonNull) .reduce(0, Integer::sum); } } 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 556a10331..5b1020780 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,29 +1,39 @@ package fr.xephi.authme.command.executable.email; +import ch.jalu.injector.testing.BeforeInjecting; +import ch.jalu.injector.testing.DelayedInjectionRunner; +import ch.jalu.injector.testing.InjectDelayed; import fr.xephi.authme.TestHelper; import fr.xephi.authme.data.auth.PlayerAuth; import fr.xephi.authme.data.auth.PlayerCache; import fr.xephi.authme.datasource.DataSource; import fr.xephi.authme.mail.EmailService; import fr.xephi.authme.message.MessageKey; +import fr.xephi.authme.message.Messages; import fr.xephi.authme.security.PasswordSecurity; import fr.xephi.authme.security.crypts.HashedPassword; import fr.xephi.authme.service.CommonService; import fr.xephi.authme.service.RecoveryCodeService; import fr.xephi.authme.settings.properties.EmailSettings; +import fr.xephi.authme.settings.properties.SecuritySettings; +import fr.xephi.authme.util.expiring.Duration; 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.junit.MockitoJUnitRunner; +import org.mockito.Mockito; import java.util.Arrays; import java.util.Collections; +import java.util.concurrent.TimeUnit; import static fr.xephi.authme.AuthMeMatchers.stringWithLength; +import static org.hamcrest.Matchers.both; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.greaterThan; +import static org.hamcrest.Matchers.lessThan; import static org.junit.Assert.assertThat; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyString; @@ -38,19 +48,19 @@ import static org.mockito.Mockito.verifyZeroInteractions; /** * Test for {@link RecoverEmailCommand}. */ -@RunWith(MockitoJUnitRunner.class) +@RunWith(DelayedInjectionRunner.class) public class RecoverEmailCommandTest { private static final String DEFAULT_EMAIL = "your@email.com"; - @InjectMocks + @InjectDelayed private RecoverEmailCommand command; @Mock private PasswordSecurity passwordSecurity; @Mock - private CommonService commandService; + private CommonService commonService; @Mock private DataSource dataSource; @@ -64,11 +74,19 @@ public class RecoverEmailCommandTest { @Mock private RecoveryCodeService recoveryCodeService; + @Mock + private Messages messages; + @BeforeClass public static void initLogger() { TestHelper.setupLogger(); } + @BeforeInjecting + public void initSettings() { + given(commonService.getProperty(SecuritySettings.EMAIL_RECOVERY_COOLDOWN_SECONDS)).willReturn(40); + } + @Test public void shouldHandleMissingMailProperties() { // given @@ -79,7 +97,7 @@ public class RecoverEmailCommandTest { command.executeCommand(sender, Collections.singletonList("some@email.tld")); // then - verify(commandService).send(sender, MessageKey.INCOMPLETE_EMAIL_SETTINGS); + verify(commonService).send(sender, MessageKey.INCOMPLETE_EMAIL_SETTINGS); verifyZeroInteractions(dataSource, passwordSecurity); } @@ -98,7 +116,7 @@ public class RecoverEmailCommandTest { // then verify(emailService).hasAllInformation(); verifyZeroInteractions(dataSource); - verify(commandService).send(sender, MessageKey.ALREADY_LOGGED_IN_ERROR); + verify(commonService).send(sender, MessageKey.ALREADY_LOGGED_IN_ERROR); } @Test @@ -118,7 +136,7 @@ public class RecoverEmailCommandTest { verify(emailService).hasAllInformation(); verify(dataSource).getAuth(name); verifyNoMoreInteractions(dataSource); - verify(commandService).send(sender, MessageKey.USAGE_REGISTER); + verify(commonService).send(sender, MessageKey.USAGE_REGISTER); } @Test @@ -138,7 +156,7 @@ public class RecoverEmailCommandTest { verify(emailService).hasAllInformation(); verify(dataSource).getAuth(name); verifyNoMoreInteractions(dataSource); - verify(commandService).send(sender, MessageKey.INVALID_EMAIL); + verify(commonService).send(sender, MessageKey.INVALID_EMAIL); } @Test @@ -158,7 +176,7 @@ public class RecoverEmailCommandTest { verify(emailService).hasAllInformation(); verify(dataSource).getAuth(name); verifyNoMoreInteractions(dataSource); - verify(commandService).send(sender, MessageKey.INVALID_EMAIL); + verify(commonService).send(sender, MessageKey.INVALID_EMAIL); } @Test @@ -183,7 +201,7 @@ public class RecoverEmailCommandTest { verify(emailService).hasAllInformation(); verify(dataSource).getAuth(name); verify(recoveryCodeService).generateCode(name); - verify(commandService).send(sender, MessageKey.RECOVERY_CODE_SENT); + verify(commonService).send(sender, MessageKey.RECOVERY_CODE_SENT); verify(emailService).sendRecoveryCode(name, email, code); } @@ -207,7 +225,7 @@ public class RecoverEmailCommandTest { // then verify(emailService).hasAllInformation(); verify(dataSource, only()).getAuth(name); - verify(commandService).send(sender, MessageKey.INCORRECT_RECOVERY_CODE); + verify(commonService).send(sender, MessageKey.INCORRECT_RECOVERY_CODE); verifyNoMoreInteractions(emailService); } @@ -224,7 +242,7 @@ public class RecoverEmailCommandTest { String code = "A6EF3AC8"; PlayerAuth auth = newAuthWithEmail(email); given(dataSource.getAuth(name)).willReturn(auth); - given(commandService.getProperty(EmailSettings.RECOVERY_PASSWORD_LENGTH)).willReturn(20); + given(commonService.getProperty(EmailSettings.RECOVERY_PASSWORD_LENGTH)).willReturn(20); given(passwordSecurity.computeHash(anyString(), eq(name))) .willAnswer(invocation -> new HashedPassword(invocation.getArgument(0))); given(recoveryCodeService.isRecoveryCodeNeeded()).willReturn(true); @@ -243,7 +261,7 @@ public class RecoverEmailCommandTest { verify(dataSource).updatePassword(eq(name), any(HashedPassword.class)); verify(recoveryCodeService).removeCode(name); verify(emailService).sendPasswordMail(name, email, generatedPassword); - verify(commandService).send(sender, MessageKey.RECOVERY_EMAIL_SENT_MESSAGE); + verify(commonService).send(sender, MessageKey.RECOVERY_EMAIL_SENT_MESSAGE); } @Test @@ -258,7 +276,7 @@ public class RecoverEmailCommandTest { 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(commonService.getProperty(EmailSettings.RECOVERY_PASSWORD_LENGTH)).willReturn(20); given(passwordSecurity.computeHash(anyString(), eq(name))) .willAnswer(invocation -> new HashedPassword(invocation.getArgument(0))); given(recoveryCodeService.isRecoveryCodeNeeded()).willReturn(false); @@ -275,7 +293,40 @@ public class RecoverEmailCommandTest { assertThat(generatedPassword, stringWithLength(20)); verify(dataSource).updatePassword(eq(name), any(HashedPassword.class)); verify(emailService).sendPasswordMail(name, email, generatedPassword); - verify(commandService).send(sender, MessageKey.RECOVERY_EMAIL_SENT_MESSAGE); + verify(commonService).send(sender, MessageKey.RECOVERY_EMAIL_SENT_MESSAGE); + } + + @Test + public void shouldNotSendEmailIfCooldownCheckFails() { + // given + String name = "feverRay"; + Player sender = mock(Player.class); + given(sender.getName()).willReturn(name); + given(emailService.hasAllInformation()).willReturn(true); + given(emailService.sendRecoveryCode(anyString(), anyString(), anyString())).willReturn(true); + given(playerCache.isAuthenticated(name)).willReturn(false); + String email = "mymail@example.org"; + PlayerAuth auth = newAuthWithEmail(email); + given(dataSource.getAuth(name)).willReturn(auth); + given(recoveryCodeService.isRecoveryCodeNeeded()).willReturn(true); + given(recoveryCodeService.generateCode(anyString())).willReturn("Code"); + // Trigger sending of recovery code + command.executeCommand(sender, Collections.singletonList(email)); + + Mockito.reset(emailService, commonService); + given(emailService.hasAllInformation()).willReturn(true); + given(messages.formatDuration(any(Duration.class))).willReturn("8 minutes"); + + // when + command.executeCommand(sender, Collections.singletonList(email)); + + // then + verify(emailService, only()).hasAllInformation(); + ArgumentCaptor durationCaptor = ArgumentCaptor.forClass(Duration.class); + verify(messages).formatDuration(durationCaptor.capture()); + assertThat(durationCaptor.getValue().getDuration(), both(lessThan(41L)).and(greaterThan(36L))); + assertThat(durationCaptor.getValue().getTimeUnit(), equalTo(TimeUnit.SECONDS)); + verify(messages).send(sender, MessageKey.EMAIL_COOLDOWN_ERROR, "8 minutes"); } diff --git a/src/test/java/fr/xephi/authme/message/MessagesIntegrationTest.java b/src/test/java/fr/xephi/authme/message/MessagesIntegrationTest.java index 94fe9a0df..8f48fffec 100644 --- a/src/test/java/fr/xephi/authme/message/MessagesIntegrationTest.java +++ b/src/test/java/fr/xephi/authme/message/MessagesIntegrationTest.java @@ -1,7 +1,9 @@ package fr.xephi.authme.message; +import com.google.common.collect.ImmutableMap; import fr.xephi.authme.ConsoleLogger; import fr.xephi.authme.TestHelper; +import fr.xephi.authme.util.expiring.Duration; import org.bukkit.command.CommandSender; import org.bukkit.entity.Player; import org.junit.Before; @@ -11,6 +13,8 @@ import org.mockito.ArgumentCaptor; import org.mockito.Mockito; import java.io.File; +import java.util.Map; +import java.util.concurrent.TimeUnit; import java.util.function.Function; import java.util.logging.Logger; @@ -230,6 +234,26 @@ public class MessagesIntegrationTest { assertThat(result, equalTo("Use /captcha 24680 to solve the captcha")); } + @Test + public void shouldFormatDurationObjects() { + // given + Map expectedTexts = ImmutableMap.builder() + .put(new Duration(1, TimeUnit.SECONDS), "1 second") + .put(new Duration(12, TimeUnit.SECONDS), "12 seconds") + .put(new Duration(1, TimeUnit.MINUTES), "1 minute") + .put(new Duration(0, TimeUnit.MINUTES), "0 minutes") + .put(new Duration(1, TimeUnit.HOURS), "1 hour") + .put(new Duration(-4, TimeUnit.HOURS), "-4 hours") + .put(new Duration(1, TimeUnit.DAYS), "1 day") + .put(new Duration(44, TimeUnit.DAYS), "44 days") + .build(); + + // when / then + for (Map.Entry entry : expectedTexts.entrySet()) { + assertThat(messages.formatDuration(entry.getKey()), equalTo(entry.getValue())); + } + } + @SuppressWarnings("unchecked") private static MessageFileHandlerProvider providerReturning(File file, String defaultFile) { MessageFileHandlerProvider handler = mock(MessageFileHandlerProvider.class); diff --git a/src/test/java/fr/xephi/authme/util/expiring/ExpiringSetTest.java b/src/test/java/fr/xephi/authme/util/expiring/ExpiringSetTest.java index 15a55aa09..42180ab26 100644 --- a/src/test/java/fr/xephi/authme/util/expiring/ExpiringSetTest.java +++ b/src/test/java/fr/xephi/authme/util/expiring/ExpiringSetTest.java @@ -4,7 +4,6 @@ import org.junit.Test; import java.util.concurrent.TimeUnit; -import static org.hamcrest.Matchers.either; import static org.hamcrest.Matchers.equalTo; import static org.junit.Assert.assertThat; @@ -97,14 +96,31 @@ public class ExpiringSetTest { set.add("my entry"); // when - long expiresInHours = set.getExpiration("my entry", TimeUnit.HOURS); - long expiresInMinutes = set.getExpiration("my entry", TimeUnit.MINUTES); - long unknownExpires = set.getExpiration("bogus", TimeUnit.SECONDS); + Duration expiration = set.getExpiration("my entry"); + Duration unknownExpiration = set.getExpiration("bogus"); // then - assertThat(expiresInHours, equalTo(2L)); - assertThat(expiresInMinutes, either(equalTo(122L)).or(equalTo(123L))); - assertThat(unknownExpires, equalTo(-1L)); + assertIsDuration(expiration, 2, TimeUnit.HOURS); + assertIsDuration(unknownExpiration, -1, TimeUnit.SECONDS); + } + + @Test + public void shouldReturnExpirationInSuitableUnits() { + // given + ExpiringSet set = new ExpiringSet<>(601, TimeUnit.SECONDS); + set.add(12); + set.setExpiration(49, TimeUnit.HOURS); + set.add(23); + + // when + Duration expiration12 = set.getExpiration(12); + Duration expiration23 = set.getExpiration(23); + Duration expectedUnknown = set.getExpiration(-100); + + // then + assertIsDuration(expiration12, 10, TimeUnit.MINUTES); + assertIsDuration(expiration23, 2, TimeUnit.DAYS); + assertIsDuration(expectedUnknown, -1, TimeUnit.SECONDS); } @Test @@ -114,9 +130,14 @@ public class ExpiringSetTest { set.add(23); // when - long expiresInSeconds = set.getExpiration(23, TimeUnit.SECONDS); + Duration expiration = set.getExpiration(23); // then - assertThat(expiresInSeconds, equalTo(-1L)); + assertIsDuration(expiration, -1, TimeUnit.SECONDS); + } + + private static void assertIsDuration(Duration duration, long expectedDuration, TimeUnit expectedUnit) { + assertThat(duration.getTimeUnit(), equalTo(expectedUnit)); + assertThat(duration.getDuration(), equalTo(expectedDuration)); } }