From 180bbbf0bed4e2e2493bbd1feb6e2b418bffc89c Mon Sep 17 00:00:00 2001 From: ljacqu Date: Fri, 5 Jan 2018 01:26:25 +0100 Subject: [PATCH] #930 Refactor captcha managers to have a crude captcha storage class instead of inheritance - Remove abstract captcha manager in favor of a primitive captcha code storage (composition over inheritance) - Supply player when checking captcha code for further usage (fixes open point from previous commit) --- .../executable/captcha/CaptchaCommand.java | 8 +- .../executable/register/RegisterCommand.java | 2 +- .../authme/data/AbstractCaptchaManager.java | 126 ------------------ .../authme/data/LoginCaptchaManager.java | 80 ----------- .../data/RegistrationCaptchaManager.java | 71 ---------- .../data/captcha/CaptchaCodeStorage.java | 90 +++++++++++++ .../authme/data/captcha/CaptchaManager.java | 44 ++++++ .../data/captcha/LoginCaptchaManager.java | 99 ++++++++++++++ .../captcha/RegistrationCaptchaManager.java | 83 ++++++++++++ .../data/limbo/LimboPlayerTaskManager.java | 2 +- .../process/login/AsynchronousLogin.java | 2 +- .../authme/AuthMeInitializationTest.java | 2 +- .../xephi/authme/ClassesConsistencyTest.java | 6 +- .../captcha/CaptchaCommandTest.java | 20 +-- .../register/RegisterCommandTest.java | 2 +- .../LoginCaptchaManagerTest.java | 9 +- .../RegistrationCaptchaManagerTest.java | 21 ++- .../limbo/LimboPlayerTaskManagerTest.java | 2 +- 18 files changed, 361 insertions(+), 308 deletions(-) delete mode 100644 src/main/java/fr/xephi/authme/data/AbstractCaptchaManager.java delete mode 100644 src/main/java/fr/xephi/authme/data/LoginCaptchaManager.java delete mode 100644 src/main/java/fr/xephi/authme/data/RegistrationCaptchaManager.java create mode 100644 src/main/java/fr/xephi/authme/data/captcha/CaptchaCodeStorage.java create mode 100644 src/main/java/fr/xephi/authme/data/captcha/CaptchaManager.java create mode 100644 src/main/java/fr/xephi/authme/data/captcha/LoginCaptchaManager.java create mode 100644 src/main/java/fr/xephi/authme/data/captcha/RegistrationCaptchaManager.java rename src/test/java/fr/xephi/authme/data/{ => captcha}/LoginCaptchaManagerTest.java (96%) rename src/test/java/fr/xephi/authme/data/{ => captcha}/RegistrationCaptchaManagerTest.java (77%) diff --git a/src/main/java/fr/xephi/authme/command/executable/captcha/CaptchaCommand.java b/src/main/java/fr/xephi/authme/command/executable/captcha/CaptchaCommand.java index ab1746567..b568f305f 100644 --- a/src/main/java/fr/xephi/authme/command/executable/captcha/CaptchaCommand.java +++ b/src/main/java/fr/xephi/authme/command/executable/captcha/CaptchaCommand.java @@ -1,9 +1,9 @@ package fr.xephi.authme.command.executable.captcha; import fr.xephi.authme.command.PlayerCommand; -import fr.xephi.authme.data.LoginCaptchaManager; -import fr.xephi.authme.data.RegistrationCaptchaManager; import fr.xephi.authme.data.auth.PlayerCache; +import fr.xephi.authme.data.captcha.LoginCaptchaManager; +import fr.xephi.authme.data.captcha.RegistrationCaptchaManager; import fr.xephi.authme.data.limbo.LimboService; import fr.xephi.authme.message.MessageKey; import fr.xephi.authme.service.CommonService; @@ -47,7 +47,7 @@ public class CaptchaCommand extends PlayerCommand { } private void checkLoginCaptcha(Player player, String captchaCode) { - final boolean isCorrectCode = loginCaptchaManager.checkCode(player.getName(), captchaCode); + final boolean isCorrectCode = loginCaptchaManager.checkCode(player, captchaCode); if (isCorrectCode) { commonService.send(player, MessageKey.CAPTCHA_SUCCESS); commonService.send(player, MessageKey.LOGIN_MESSAGE); @@ -59,7 +59,7 @@ public class CaptchaCommand extends PlayerCommand { } private void checkRegisterCaptcha(Player player, String captchaCode) { - final boolean isCorrectCode = registrationCaptchaManager.checkCode(player.getName(), captchaCode); + final boolean isCorrectCode = registrationCaptchaManager.checkCode(player, captchaCode); if (isCorrectCode) { commonService.send(player, MessageKey.CAPTCHA_SUCCESS); commonService.send(player, MessageKey.REGISTER_MESSAGE); diff --git a/src/main/java/fr/xephi/authme/command/executable/register/RegisterCommand.java b/src/main/java/fr/xephi/authme/command/executable/register/RegisterCommand.java index f10d26f7f..a67076487 100644 --- a/src/main/java/fr/xephi/authme/command/executable/register/RegisterCommand.java +++ b/src/main/java/fr/xephi/authme/command/executable/register/RegisterCommand.java @@ -2,7 +2,7 @@ package fr.xephi.authme.command.executable.register; import fr.xephi.authme.ConsoleLogger; import fr.xephi.authme.command.PlayerCommand; -import fr.xephi.authme.data.RegistrationCaptchaManager; +import fr.xephi.authme.data.captcha.RegistrationCaptchaManager; import fr.xephi.authme.mail.EmailService; import fr.xephi.authme.message.MessageKey; import fr.xephi.authme.process.Management; diff --git a/src/main/java/fr/xephi/authme/data/AbstractCaptchaManager.java b/src/main/java/fr/xephi/authme/data/AbstractCaptchaManager.java deleted file mode 100644 index ce6766775..000000000 --- a/src/main/java/fr/xephi/authme/data/AbstractCaptchaManager.java +++ /dev/null @@ -1,126 +0,0 @@ -package fr.xephi.authme.data; - -import fr.xephi.authme.initialization.HasCleanup; -import fr.xephi.authme.initialization.SettingsDependent; -import fr.xephi.authme.settings.Settings; -import fr.xephi.authme.settings.properties.SecuritySettings; -import fr.xephi.authme.util.RandomStringUtils; -import fr.xephi.authme.util.expiring.ExpiringMap; - -import java.util.concurrent.TimeUnit; - -/** - * Manages captcha codes. - */ -public abstract class AbstractCaptchaManager implements SettingsDependent, HasCleanup { - - // Note: Proper expiration is set in reload(), which is also called on initialization - private final ExpiringMap captchaCodes = new ExpiringMap<>(0, TimeUnit.MINUTES); - private int captchaLength; - - /** - * Constructor. - * - * @param settings the settings instance - */ - public AbstractCaptchaManager(Settings settings) { - initialize(settings); - } - - /** - * Returns whether the given player is required to solve a captcha. - * - * @param name the name of the player to verify - * @return true if the player has to solve a captcha, false otherwise - */ - public abstract boolean isCaptchaRequired(String name); - - /** - * Returns the stored captcha for the player or generates and saves a new one. - * - * @param name the player's name - * @return the code the player is required to enter - */ - public String getCaptchaCodeOrGenerateNew(String name) { - String code = captchaCodes.get(name.toLowerCase()); - return code == null ? generateCode(name) : code; - } - - /** - * Generates a code for the player and returns it. - * - * @param name the name of the player to generate a code for - * @return the generated code - */ - public String generateCode(String name) { - String code = RandomStringUtils.generate(captchaLength); - captchaCodes.put(name.toLowerCase(), code); - return code; - } - - /** - * Checks the given code against the existing one and resets the player's auth failure count upon success. - * - * @param name the name of the player to check - * @param code the supplied code - * @return true if the code matches or if no captcha is required for the player, false otherwise - */ - public boolean checkCode(String name, String code) { - final String nameLowerCase = name.toLowerCase(); - String savedCode = captchaCodes.get(nameLowerCase); - if (savedCode != null && savedCode.equalsIgnoreCase(code)) { - captchaCodes.remove(nameLowerCase); - processSuccessfulCode(nameLowerCase); - return true; - } else { - processUnsuccessfulCode(nameLowerCase); - } - return false; - } - - private void initialize(Settings settings) { - captchaLength = settings.getProperty(SecuritySettings.CAPTCHA_LENGTH); - captchaCodes.setExpiration(minutesBeforeCodeExpires(settings), TimeUnit.MINUTES); - } - - /** - * Called on initialization and on reload. - * - * @param settings the settings instance - */ - @Override - public void reload(Settings settings) { - // Note ljacqu 20171201: Use initialize() as an in-between method so that we can call it in the constructor - // without causing any trouble to a child that may extend reload -> at the point of calling, the child's fields - // would not yet be initialized. - initialize(settings); - } - - @Override - public void performCleanup() { - captchaCodes.removeExpiredEntries(); - } - - /** - * Called when a player has successfully solved the captcha. - * - * @param nameLower the player's name (all lowercase) - */ - protected abstract void processSuccessfulCode(String nameLower); - - /** - * Called when a player has failed the captcha code. - * - * @param nameLower the player's name (all lowercase) - */ - protected void processUnsuccessfulCode(String nameLower) { - } - - /** - * Returns the number of minutes a generated captcha code should live for before it may expire. - * - * @param settings the settings instance - * @return number of minutes that the code is valid for - */ - protected abstract int minutesBeforeCodeExpires(Settings settings); -} diff --git a/src/main/java/fr/xephi/authme/data/LoginCaptchaManager.java b/src/main/java/fr/xephi/authme/data/LoginCaptchaManager.java deleted file mode 100644 index e45e951f9..000000000 --- a/src/main/java/fr/xephi/authme/data/LoginCaptchaManager.java +++ /dev/null @@ -1,80 +0,0 @@ -package fr.xephi.authme.data; - -import fr.xephi.authme.settings.Settings; -import fr.xephi.authme.settings.properties.SecuritySettings; -import fr.xephi.authme.util.expiring.TimedCounter; - -import javax.inject.Inject; -import java.util.concurrent.TimeUnit; - -/** - * Manager for the handling of captchas after too many failed login attempts. - */ -public class LoginCaptchaManager extends AbstractCaptchaManager { - - // Note: proper expiration is set in reload(), which is also called on initialization by the parent - private final TimedCounter playerCounts = new TimedCounter<>(0, TimeUnit.MINUTES); - - private boolean isEnabled; - private int threshold; - - @Inject - LoginCaptchaManager(Settings settings) { - super(settings); - reload(settings); - } - - /** - * Increases the failure count for the given player. - * - * @param name the player's name - */ - public void increaseLoginFailureCount(String name) { - if (isEnabled) { - String playerLower = name.toLowerCase(); - playerCounts.increment(playerLower); - } - } - - @Override - public boolean isCaptchaRequired(String playerName) { - return isEnabled && playerCounts.get(playerName.toLowerCase()) >= threshold; - } - - /** - * Resets the login count of the given player to 0. - * - * @param name the player's name - */ - public void resetLoginFailureCount(String name) { - if (isEnabled) { - playerCounts.remove(name.toLowerCase()); - } - } - - @Override - public void reload(Settings settings) { - super.reload(settings); - - this.isEnabled = settings.getProperty(SecuritySettings.ENABLE_LOGIN_FAILURE_CAPTCHA); - this.threshold = settings.getProperty(SecuritySettings.MAX_LOGIN_TRIES_BEFORE_CAPTCHA); - long countTimeout = settings.getProperty(SecuritySettings.CAPTCHA_COUNT_MINUTES_BEFORE_RESET); - playerCounts.setExpiration(countTimeout, TimeUnit.MINUTES); - } - - @Override - public void performCleanup() { - super.performCleanup(); - playerCounts.removeExpiredEntries(); - } - - @Override - protected void processSuccessfulCode(String nameLower) { - playerCounts.remove(nameLower); - } - - @Override - protected int minutesBeforeCodeExpires(Settings settings) { - return settings.getProperty(SecuritySettings.CAPTCHA_COUNT_MINUTES_BEFORE_RESET); - } -} diff --git a/src/main/java/fr/xephi/authme/data/RegistrationCaptchaManager.java b/src/main/java/fr/xephi/authme/data/RegistrationCaptchaManager.java deleted file mode 100644 index 7022356d1..000000000 --- a/src/main/java/fr/xephi/authme/data/RegistrationCaptchaManager.java +++ /dev/null @@ -1,71 +0,0 @@ -package fr.xephi.authme.data; - -import fr.xephi.authme.data.limbo.LimboService; -import fr.xephi.authme.initialization.circulardependency.HasCircularDependency; -import fr.xephi.authme.initialization.circulardependency.InjectAfterInitialization; -import fr.xephi.authme.settings.Settings; -import fr.xephi.authme.settings.properties.SecuritySettings; -import fr.xephi.authme.util.expiring.ExpiringSet; -import org.bukkit.Bukkit; -import org.bukkit.entity.Player; - -import javax.inject.Inject; -import java.util.concurrent.TimeUnit; - -/** - * Captcha manager for registration. - */ -public class RegistrationCaptchaManager extends AbstractCaptchaManager implements HasCircularDependency { - - private static final int MINUTES_VALID_FOR_REGISTRATION = 30; - - private LimboService limboService; - - private final ExpiringSet verifiedNamesForRegistration = - new ExpiringSet<>(MINUTES_VALID_FOR_REGISTRATION, TimeUnit.MINUTES); - private boolean isEnabled; - - @Inject - RegistrationCaptchaManager(Settings settings) { - super(settings); - reload(settings); - } - - @Override - public boolean isCaptchaRequired(String name) { - return isEnabled && !verifiedNamesForRegistration.contains(name.toLowerCase()); - } - - @Override - public void reload(Settings settings) { - super.reload(settings); - this.isEnabled = settings.getProperty(SecuritySettings.ENABLE_CAPTCHA_FOR_REGISTRATION); - } - - @Override - public void performCleanup() { - super.performCleanup(); - verifiedNamesForRegistration.removeExpiredEntries(); - } - - @Override - protected void processSuccessfulCode(String nameLower) { - verifiedNamesForRegistration.add(nameLower); - } - - @Override - protected void processUnsuccessfulCode(String nameLower) { - final Player player = Bukkit.getPlayerExact(nameLower); // TODO #930: Pass in player! - limboService.resetMessageTask(player, false); - } - - @Override - protected int minutesBeforeCodeExpires(Settings settings) { - return MINUTES_VALID_FOR_REGISTRATION; - } - - @InjectAfterInitialization - public void setLimboService(LimboService limboService) { - this.limboService = limboService; - } -} diff --git a/src/main/java/fr/xephi/authme/data/captcha/CaptchaCodeStorage.java b/src/main/java/fr/xephi/authme/data/captcha/CaptchaCodeStorage.java new file mode 100644 index 000000000..ba4ec470f --- /dev/null +++ b/src/main/java/fr/xephi/authme/data/captcha/CaptchaCodeStorage.java @@ -0,0 +1,90 @@ +package fr.xephi.authme.data.captcha; + +import fr.xephi.authme.util.RandomStringUtils; +import fr.xephi.authme.util.expiring.ExpiringMap; + +import java.util.concurrent.TimeUnit; + +/** + * Primitive service for storing captcha codes. + */ +public class CaptchaCodeStorage { + + /** Map of captcha codes (with player name as key, case-insensitive). */ + private ExpiringMap captchaCodes; + /** Number of characters newly generated captcha codes should have. */ + private int captchaLength; + + /** + * Constructor. + * + * @param expirationInMinutes minutes after which a saved captcha code expires + * @param captchaLength the number of characters a captcha code should have + */ + public CaptchaCodeStorage(long expirationInMinutes, int captchaLength) { + this.captchaCodes = new ExpiringMap<>(expirationInMinutes, TimeUnit.MINUTES); + this.captchaLength = captchaLength; + } + + /** + * Sets the expiration of captcha codes. + * + * @param expirationInMinutes minutes after which a saved captcha code expires + */ + public void setExpirationInMinutes(long expirationInMinutes) { + captchaCodes.setExpiration(expirationInMinutes, TimeUnit.MINUTES); + } + + /** + * Sets the captcha length. + * + * @param captchaLength number of characters a captcha code should have + */ + public void setCaptchaLength(int captchaLength) { + this.captchaLength = captchaLength; + } + + /** + * Returns the stored captcha for the player or generates and saves a new one. + * + * @param name the player's name + * @return the code the player is required to enter + */ + public String getCodeOrGenerateNew(String name) { + String code = captchaCodes.get(name.toLowerCase()); + return code == null ? generateCode(name) : code; + } + + /** + * Generates a code for the player and returns it. + * + * @param name the name of the player to generate a code for + * @return the generated code + */ + public String generateCode(String name) { + String code = RandomStringUtils.generate(captchaLength); + captchaCodes.put(name.toLowerCase(), code); + return code; + } + + /** + * Checks the given code against the existing one. Upon success, the saved captcha code is removed from storage. + * + * @param name the name of the player to check + * @param code the supplied code + * @return true if the code matches, false otherwise + */ + public boolean checkCode(String name, String code) { + String nameLowerCase = name.toLowerCase(); + String savedCode = captchaCodes.get(nameLowerCase); + if (savedCode != null && savedCode.equalsIgnoreCase(code)) { + captchaCodes.remove(nameLowerCase); + return true; + } + return false; + } + + public void removeExpiredEntries() { + captchaCodes.removeExpiredEntries(); + } +} diff --git a/src/main/java/fr/xephi/authme/data/captcha/CaptchaManager.java b/src/main/java/fr/xephi/authme/data/captcha/CaptchaManager.java new file mode 100644 index 000000000..b5b01d850 --- /dev/null +++ b/src/main/java/fr/xephi/authme/data/captcha/CaptchaManager.java @@ -0,0 +1,44 @@ +package fr.xephi.authme.data.captcha; + +import org.bukkit.entity.Player; + +/** + * Manages captcha codes. + */ +public interface CaptchaManager { + + /** + * Returns whether the given player is required to solve a captcha. + * + * @param name the name of the player to verify + * @return true if the player has to solve a captcha, false otherwise + */ + boolean isCaptchaRequired(String name); + + /** + * Returns the stored captcha for the player or generates and saves a new one. + * + * @param name the player's name + * @return the code the player is required to enter + */ + String getCaptchaCodeOrGenerateNew(String name); + + /** + * Generates a code for the player and returns it. + * + * @param name the name of the player to generate a code for + * @return the generated code + */ + String generateCode(String name); + + /** + * Checks the given code against the existing one. This method may perform additional state changes + * on success or failure, such as modifying some counter or setting a player as verified. + * + * @param player the player to check + * @param code the supplied code + * @return true if the code matches, false otherwise + */ + boolean checkCode(Player player, String code); + +} diff --git a/src/main/java/fr/xephi/authme/data/captcha/LoginCaptchaManager.java b/src/main/java/fr/xephi/authme/data/captcha/LoginCaptchaManager.java new file mode 100644 index 000000000..8bf21441a --- /dev/null +++ b/src/main/java/fr/xephi/authme/data/captcha/LoginCaptchaManager.java @@ -0,0 +1,99 @@ +package fr.xephi.authme.data.captcha; + +import fr.xephi.authme.initialization.HasCleanup; +import fr.xephi.authme.initialization.SettingsDependent; +import fr.xephi.authme.settings.Settings; +import fr.xephi.authme.settings.properties.SecuritySettings; +import fr.xephi.authme.util.expiring.TimedCounter; +import org.bukkit.entity.Player; + +import javax.inject.Inject; +import java.util.concurrent.TimeUnit; + +/** + * Manager for the handling of captchas after too many failed login attempts. + */ +public class LoginCaptchaManager implements CaptchaManager, SettingsDependent, HasCleanup { + + private final TimedCounter playerCounts; + private final CaptchaCodeStorage captchaCodeStorage; + + private boolean isEnabled; + private int threshold; + + @Inject + LoginCaptchaManager(Settings settings) { + // Note: Proper values are set in reload() + this.captchaCodeStorage = new CaptchaCodeStorage(30, 4); + this.playerCounts = new TimedCounter<>(9, TimeUnit.MINUTES); + reload(settings); + } + + /** + * Increases the failure count for the given player. + * + * @param name the player's name + */ + public void increaseLoginFailureCount(String name) { + if (isEnabled) { + String playerLower = name.toLowerCase(); + playerCounts.increment(playerLower); + } + } + + @Override + public boolean isCaptchaRequired(String playerName) { + return isEnabled && playerCounts.get(playerName.toLowerCase()) >= threshold; + } + + @Override + public String getCaptchaCodeOrGenerateNew(String name) { + return captchaCodeStorage.getCodeOrGenerateNew(name); + } + + @Override + public String generateCode(String name) { + return captchaCodeStorage.generateCode(name); + } + + @Override + public boolean checkCode(Player player, String code) { + String nameLower = player.getName().toLowerCase(); + boolean isCodeCorrect = captchaCodeStorage.checkCode(nameLower, code); + if (isCodeCorrect) { + playerCounts.remove(nameLower); + } + return isCodeCorrect; + } + + /** + * Resets the login count of the given player to 0. + * + * @param name the player's name + */ + public void resetLoginFailureCount(String name) { + if (isEnabled) { + playerCounts.remove(name.toLowerCase()); + } + } + + @Override + public void reload(Settings settings) { + int expirationInMinutes = settings.getProperty(SecuritySettings.CAPTCHA_COUNT_MINUTES_BEFORE_RESET); + captchaCodeStorage.setExpirationInMinutes(expirationInMinutes); + int captchaLength = settings.getProperty(SecuritySettings.CAPTCHA_LENGTH); + captchaCodeStorage.setCaptchaLength(captchaLength); + + int countTimeout = settings.getProperty(SecuritySettings.CAPTCHA_COUNT_MINUTES_BEFORE_RESET); + playerCounts.setExpiration(countTimeout, TimeUnit.MINUTES); + + isEnabled = settings.getProperty(SecuritySettings.ENABLE_LOGIN_FAILURE_CAPTCHA); + threshold = settings.getProperty(SecuritySettings.MAX_LOGIN_TRIES_BEFORE_CAPTCHA); + } + + @Override + public void performCleanup() { + playerCounts.removeExpiredEntries(); + captchaCodeStorage.removeExpiredEntries(); + } +} diff --git a/src/main/java/fr/xephi/authme/data/captcha/RegistrationCaptchaManager.java b/src/main/java/fr/xephi/authme/data/captcha/RegistrationCaptchaManager.java new file mode 100644 index 000000000..a5961241c --- /dev/null +++ b/src/main/java/fr/xephi/authme/data/captcha/RegistrationCaptchaManager.java @@ -0,0 +1,83 @@ +package fr.xephi.authme.data.captcha; + +import fr.xephi.authme.data.limbo.LimboService; +import fr.xephi.authme.initialization.HasCleanup; +import fr.xephi.authme.initialization.SettingsDependent; +import fr.xephi.authme.initialization.circulardependency.HasCircularDependency; +import fr.xephi.authme.initialization.circulardependency.InjectAfterInitialization; +import fr.xephi.authme.settings.Settings; +import fr.xephi.authme.settings.properties.SecuritySettings; +import fr.xephi.authme.util.expiring.ExpiringSet; +import org.bukkit.entity.Player; + +import javax.inject.Inject; +import java.util.concurrent.TimeUnit; + +/** + * Captcha manager for registration. + */ +public class RegistrationCaptchaManager + implements CaptchaManager, SettingsDependent, HasCleanup, HasCircularDependency { + + private static final int MINUTES_VALID_FOR_REGISTRATION = 30; + + private LimboService limboService; + + private final ExpiringSet verifiedNamesForRegistration; + private final CaptchaCodeStorage captchaCodeStorage; + private boolean isEnabled; + + @Inject + RegistrationCaptchaManager(Settings settings) { + // NOTE: proper captcha length is set in reload() + this.captchaCodeStorage = new CaptchaCodeStorage(MINUTES_VALID_FOR_REGISTRATION, 4); + this.verifiedNamesForRegistration = new ExpiringSet<>(MINUTES_VALID_FOR_REGISTRATION, TimeUnit.MINUTES); + reload(settings); + } + + @Override + public boolean isCaptchaRequired(String name) { + return isEnabled && !verifiedNamesForRegistration.contains(name.toLowerCase()); + } + + @Override + public String getCaptchaCodeOrGenerateNew(String name) { + return captchaCodeStorage.getCodeOrGenerateNew(name); + } + + @Override + public String generateCode(String name) { + return captchaCodeStorage.generateCode(name); + } + + @Override + public boolean checkCode(Player player, String code) { + String nameLower = player.getName().toLowerCase(); + boolean isCodeCorrect = captchaCodeStorage.checkCode(nameLower, code); + if (isCodeCorrect) { + verifiedNamesForRegistration.add(nameLower); + } else { + limboService.resetMessageTask(player, false); + } + return isCodeCorrect; + } + + @Override + public void reload(Settings settings) { + int captchaLength = settings.getProperty(SecuritySettings.CAPTCHA_LENGTH); + captchaCodeStorage.setCaptchaLength(captchaLength); + + isEnabled = settings.getProperty(SecuritySettings.ENABLE_CAPTCHA_FOR_REGISTRATION); + } + + @Override + public void performCleanup() { + verifiedNamesForRegistration.removeExpiredEntries(); + captchaCodeStorage.removeExpiredEntries(); + } + + @InjectAfterInitialization + public void setLimboService(LimboService limboService) { + this.limboService = limboService; + } +} diff --git a/src/main/java/fr/xephi/authme/data/limbo/LimboPlayerTaskManager.java b/src/main/java/fr/xephi/authme/data/limbo/LimboPlayerTaskManager.java index 0ad2b793a..9b9373e61 100644 --- a/src/main/java/fr/xephi/authme/data/limbo/LimboPlayerTaskManager.java +++ b/src/main/java/fr/xephi/authme/data/limbo/LimboPlayerTaskManager.java @@ -1,7 +1,7 @@ package fr.xephi.authme.data.limbo; -import fr.xephi.authme.data.RegistrationCaptchaManager; import fr.xephi.authme.data.auth.PlayerCache; +import fr.xephi.authme.data.captcha.RegistrationCaptchaManager; import fr.xephi.authme.message.MessageKey; import fr.xephi.authme.message.Messages; import fr.xephi.authme.service.BukkitService; diff --git a/src/main/java/fr/xephi/authme/process/login/AsynchronousLogin.java b/src/main/java/fr/xephi/authme/process/login/AsynchronousLogin.java index 9159e83a4..5dacba5a4 100644 --- a/src/main/java/fr/xephi/authme/process/login/AsynchronousLogin.java +++ b/src/main/java/fr/xephi/authme/process/login/AsynchronousLogin.java @@ -2,7 +2,7 @@ package fr.xephi.authme.process.login; import com.google.common.annotations.VisibleForTesting; import fr.xephi.authme.ConsoleLogger; -import fr.xephi.authme.data.LoginCaptchaManager; +import fr.xephi.authme.data.captcha.LoginCaptchaManager; import fr.xephi.authme.data.TempbanManager; import fr.xephi.authme.data.auth.PlayerAuth; import fr.xephi.authme.data.auth.PlayerCache; diff --git a/src/test/java/fr/xephi/authme/AuthMeInitializationTest.java b/src/test/java/fr/xephi/authme/AuthMeInitializationTest.java index 236cc57c4..64cbfa41f 100644 --- a/src/test/java/fr/xephi/authme/AuthMeInitializationTest.java +++ b/src/test/java/fr/xephi/authme/AuthMeInitializationTest.java @@ -6,7 +6,7 @@ import ch.jalu.injector.InjectorBuilder; import com.google.common.io.Files; import fr.xephi.authme.api.v3.AuthMeApi; import fr.xephi.authme.command.CommandHandler; -import fr.xephi.authme.data.RegistrationCaptchaManager; +import fr.xephi.authme.data.captcha.RegistrationCaptchaManager; import fr.xephi.authme.data.limbo.LimboService; import fr.xephi.authme.datasource.DataSource; import fr.xephi.authme.initialization.DataFolder; diff --git a/src/test/java/fr/xephi/authme/ClassesConsistencyTest.java b/src/test/java/fr/xephi/authme/ClassesConsistencyTest.java index 651295164..38b2fbf43 100644 --- a/src/test/java/fr/xephi/authme/ClassesConsistencyTest.java +++ b/src/test/java/fr/xephi/authme/ClassesConsistencyTest.java @@ -4,6 +4,7 @@ import ch.jalu.configme.properties.Property; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; +import fr.xephi.authme.data.captcha.CaptchaCodeStorage; import fr.xephi.authme.datasource.Columns; import fr.xephi.authme.datasource.mysqlextensions.MySqlExtension; import fr.xephi.authme.initialization.HasCleanup; @@ -42,7 +43,7 @@ public class ClassesConsistencyTest { /** Expiring structure types. */ private static final Set> EXPIRING_STRUCTURES = ImmutableSet.of( - ExpiringSet.class, ExpiringMap.class, TimedCounter.class); + ExpiringSet.class, ExpiringMap.class, TimedCounter.class, CaptchaCodeStorage.class); /** Immutable types, which are allowed to be used in non-private constants. */ private static final Set> IMMUTABLE_TYPES = ImmutableSet.of( @@ -157,10 +158,9 @@ public class ClassesConsistencyTest { public void shouldImplementHasCleanup() { // given / when / then for (Class clazz : ALL_CLASSES) { - if (hasExpiringCollectionAsField(clazz)) { + if (hasExpiringCollectionAsField(clazz) && !EXPIRING_STRUCTURES.contains(clazz)) { assertThat("Class '" + clazz.getSimpleName() + "' has expiring collections, should implement HasCleanup", HasCleanup.class.isAssignableFrom(clazz), equalTo(true)); - // System.out.println("Successful check for " + clazz); } } } diff --git a/src/test/java/fr/xephi/authme/command/executable/captcha/CaptchaCommandTest.java b/src/test/java/fr/xephi/authme/command/executable/captcha/CaptchaCommandTest.java index 830f24317..7ec74411c 100644 --- a/src/test/java/fr/xephi/authme/command/executable/captcha/CaptchaCommandTest.java +++ b/src/test/java/fr/xephi/authme/command/executable/captcha/CaptchaCommandTest.java @@ -1,8 +1,8 @@ package fr.xephi.authme.command.executable.captcha; -import fr.xephi.authme.data.LoginCaptchaManager; -import fr.xephi.authme.data.RegistrationCaptchaManager; import fr.xephi.authme.data.auth.PlayerCache; +import fr.xephi.authme.data.captcha.LoginCaptchaManager; +import fr.xephi.authme.data.captcha.RegistrationCaptchaManager; import fr.xephi.authme.data.limbo.LimboService; import fr.xephi.authme.message.MessageKey; import fr.xephi.authme.service.CommonService; @@ -86,14 +86,14 @@ public class CaptchaCommandTest { given(playerCache.isAuthenticated(name)).willReturn(false); given(loginCaptchaManager.isCaptchaRequired(name)).willReturn(true); String captchaCode = "3991"; - given(loginCaptchaManager.checkCode(name, captchaCode)).willReturn(true); + given(loginCaptchaManager.checkCode(player, captchaCode)).willReturn(true); // when command.executeCommand(player, Collections.singletonList(captchaCode)); // then verify(loginCaptchaManager).isCaptchaRequired(name); - verify(loginCaptchaManager).checkCode(name, captchaCode); + verify(loginCaptchaManager).checkCode(player, captchaCode); verifyNoMoreInteractions(loginCaptchaManager); verify(commonService).send(player, MessageKey.CAPTCHA_SUCCESS); verify(commonService).send(player, MessageKey.LOGIN_MESSAGE); @@ -109,7 +109,7 @@ public class CaptchaCommandTest { given(playerCache.isAuthenticated(name)).willReturn(false); given(loginCaptchaManager.isCaptchaRequired(name)).willReturn(true); String captchaCode = "2468"; - given(loginCaptchaManager.checkCode(name, captchaCode)).willReturn(false); + given(loginCaptchaManager.checkCode(player, captchaCode)).willReturn(false); String newCode = "1337"; given(loginCaptchaManager.generateCode(name)).willReturn(newCode); @@ -118,7 +118,7 @@ public class CaptchaCommandTest { // then verify(loginCaptchaManager).isCaptchaRequired(name); - verify(loginCaptchaManager).checkCode(name, captchaCode); + verify(loginCaptchaManager).checkCode(player, captchaCode); verify(loginCaptchaManager).generateCode(name); verifyNoMoreInteractions(loginCaptchaManager); verify(commonService).send(player, MessageKey.CAPTCHA_WRONG_ERROR, newCode); @@ -133,13 +133,13 @@ public class CaptchaCommandTest { given(loginCaptchaManager.isCaptchaRequired(name)).willReturn(false); given(registrationCaptchaManager.isCaptchaRequired(name)).willReturn(true); String captchaCode = "A89Y3"; - given(registrationCaptchaManager.checkCode(name, captchaCode)).willReturn(true); + given(registrationCaptchaManager.checkCode(player, captchaCode)).willReturn(true); // when command.executeCommand(player, Collections.singletonList(captchaCode)); // then - verify(registrationCaptchaManager).checkCode(name, captchaCode); + verify(registrationCaptchaManager).checkCode(player, captchaCode); verify(loginCaptchaManager, only()).isCaptchaRequired(name); verify(commonService).send(player, MessageKey.CAPTCHA_SUCCESS); verify(commonService).send(player, MessageKey.REGISTER_MESSAGE); @@ -152,14 +152,14 @@ public class CaptchaCommandTest { Player player = mockPlayerWithName(name); given(registrationCaptchaManager.isCaptchaRequired(name)).willReturn(true); String captchaCode = "SFL3"; - given(registrationCaptchaManager.checkCode(name, captchaCode)).willReturn(false); + given(registrationCaptchaManager.checkCode(player, captchaCode)).willReturn(false); given(registrationCaptchaManager.generateCode(name)).willReturn("new code"); // when command.executeCommand(player, Collections.singletonList(captchaCode)); // then - verify(registrationCaptchaManager).checkCode(name, captchaCode); + verify(registrationCaptchaManager).checkCode(player, captchaCode); verify(registrationCaptchaManager).generateCode(name); verify(commonService).send(player, MessageKey.CAPTCHA_WRONG_ERROR, "new code"); } diff --git a/src/test/java/fr/xephi/authme/command/executable/register/RegisterCommandTest.java b/src/test/java/fr/xephi/authme/command/executable/register/RegisterCommandTest.java index f421fa27c..a4d8f7aa7 100644 --- a/src/test/java/fr/xephi/authme/command/executable/register/RegisterCommandTest.java +++ b/src/test/java/fr/xephi/authme/command/executable/register/RegisterCommandTest.java @@ -1,7 +1,7 @@ package fr.xephi.authme.command.executable.register; import fr.xephi.authme.TestHelper; -import fr.xephi.authme.data.RegistrationCaptchaManager; +import fr.xephi.authme.data.captcha.RegistrationCaptchaManager; import fr.xephi.authme.mail.EmailService; import fr.xephi.authme.message.MessageKey; import fr.xephi.authme.process.Management; diff --git a/src/test/java/fr/xephi/authme/data/LoginCaptchaManagerTest.java b/src/test/java/fr/xephi/authme/data/captcha/LoginCaptchaManagerTest.java similarity index 96% rename from src/test/java/fr/xephi/authme/data/LoginCaptchaManagerTest.java rename to src/test/java/fr/xephi/authme/data/captcha/LoginCaptchaManagerTest.java index 6d530b66e..7fdd109f6 100644 --- a/src/test/java/fr/xephi/authme/data/LoginCaptchaManagerTest.java +++ b/src/test/java/fr/xephi/authme/data/captcha/LoginCaptchaManagerTest.java @@ -1,9 +1,10 @@ -package fr.xephi.authme.data; +package fr.xephi.authme.data.captcha; import fr.xephi.authme.ReflectionTestUtils; import fr.xephi.authme.settings.Settings; import fr.xephi.authme.settings.properties.SecuritySettings; import fr.xephi.authme.util.expiring.TimedCounter; +import org.bukkit.entity.Player; import org.junit.Test; import static org.hamcrest.Matchers.equalTo; @@ -38,10 +39,12 @@ public class LoginCaptchaManagerTest { @Test public void shouldCreateAndCheckCaptcha() { // given - String player = "Miner"; + String name = "Miner"; + Player player = mock(Player.class); + given(player.getName()).willReturn(name); Settings settings = mockSettings(1, 4); LoginCaptchaManager manager = new LoginCaptchaManager(settings); - String captchaCode = manager.getCaptchaCodeOrGenerateNew(player); + String captchaCode = manager.getCaptchaCodeOrGenerateNew(name); // when boolean badResult = manager.checkCode(player, "wrong_code"); diff --git a/src/test/java/fr/xephi/authme/data/RegistrationCaptchaManagerTest.java b/src/test/java/fr/xephi/authme/data/captcha/RegistrationCaptchaManagerTest.java similarity index 77% rename from src/test/java/fr/xephi/authme/data/RegistrationCaptchaManagerTest.java rename to src/test/java/fr/xephi/authme/data/captcha/RegistrationCaptchaManagerTest.java index 59f8dde8f..4a71ab2ca 100644 --- a/src/test/java/fr/xephi/authme/data/RegistrationCaptchaManagerTest.java +++ b/src/test/java/fr/xephi/authme/data/captcha/RegistrationCaptchaManagerTest.java @@ -1,9 +1,10 @@ -package fr.xephi.authme.data; +package fr.xephi.authme.data.captcha; import fr.xephi.authme.ReflectionTestUtils; import fr.xephi.authme.settings.Settings; import fr.xephi.authme.settings.properties.SecuritySettings; import fr.xephi.authme.util.expiring.ExpiringMap; +import org.bukkit.entity.Player; import org.junit.Test; import static fr.xephi.authme.AuthMeMatchers.stringWithLength; @@ -46,8 +47,11 @@ public class RegistrationCaptchaManagerTest { RegistrationCaptchaManager captchaManager = new RegistrationCaptchaManager(settings); getCodeMap(captchaManager).put("test", captcha); + Player player = mock(Player.class); + given(player.getName()).willReturn("TeSt"); + // when - boolean isSuccessful = captchaManager.checkCode("TeSt", captcha); + boolean isSuccessful = captchaManager.checkCode(player, captcha); // then assertThat(isSuccessful, equalTo(true)); @@ -72,11 +76,18 @@ public class RegistrationCaptchaManagerTest { assertThat(captcha1, equalTo(captcha2)); assertThat(captcha1, stringWithLength(captchaLength)); + // given (2) + Player player = mock(Player.class); + given(player.getName()).willReturn("toast"); + // when (2) / then (2) - assertThat(captchaManager.checkCode("toast", captcha1), equalTo(true)); + assertThat(captchaManager.checkCode(player, captcha1), equalTo(true)); } - private static ExpiringMap getCodeMap(AbstractCaptchaManager captchaManager) { - return ReflectionTestUtils.getFieldValue(AbstractCaptchaManager.class, captchaManager, "captchaCodes"); + @SuppressWarnings("unchecked") + private static ExpiringMap getCodeMap(RegistrationCaptchaManager captchaManager) { + CaptchaCodeStorage captchaStorage = ReflectionTestUtils.getFieldValue( + RegistrationCaptchaManager.class, captchaManager, "captchaCodeStorage"); + return ReflectionTestUtils.getFieldValue(CaptchaCodeStorage.class, captchaStorage, "captchaCodes"); } } diff --git a/src/test/java/fr/xephi/authme/data/limbo/LimboPlayerTaskManagerTest.java b/src/test/java/fr/xephi/authme/data/limbo/LimboPlayerTaskManagerTest.java index e09aadc71..9a8d876ca 100644 --- a/src/test/java/fr/xephi/authme/data/limbo/LimboPlayerTaskManagerTest.java +++ b/src/test/java/fr/xephi/authme/data/limbo/LimboPlayerTaskManagerTest.java @@ -1,8 +1,8 @@ package fr.xephi.authme.data.limbo; import fr.xephi.authme.TestHelper; -import fr.xephi.authme.data.RegistrationCaptchaManager; import fr.xephi.authme.data.auth.PlayerCache; +import fr.xephi.authme.data.captcha.RegistrationCaptchaManager; import fr.xephi.authme.message.MessageKey; import fr.xephi.authme.message.Messages; import fr.xephi.authme.service.BukkitService;