#601 Integrate plugin manager

- Encapsulate captcha functionality into a class instead of two public fields on the AuthMe main class(!)
   - Let CaptchaManager worry about whether it is enabled or not -> no need to check on the outside
- Implement full reloading support to enable/disable captchas + parameters
- Add unit tests
This commit is contained in:
ljacqu 2016-06-03 22:47:17 +02:00
parent 1f2a823f99
commit 12703d1613
8 changed files with 335 additions and 103 deletions

View File

@ -101,13 +101,10 @@ public class AuthMe extends JavaPlugin {
/*
* Maps and stuff
*/
// TODO #601: Integrate CaptchaManager
public final ConcurrentHashMap<String, BukkitTask> sessions = new ConcurrentHashMap<>();
public final ConcurrentHashMap<String, Integer> captcha = new ConcurrentHashMap<>();
public final ConcurrentHashMap<String, String> cap = new ConcurrentHashMap<>();
/*
* Public Instances
* Public instances
*/
public NewAPI api;
// TODO #655: Encapsulate mail

View File

@ -1,30 +1,40 @@
package fr.xephi.authme.cache;
import fr.xephi.authme.initialization.SettingsDependent;
import fr.xephi.authme.security.RandomString;
import fr.xephi.authme.settings.NewSetting;
import fr.xephi.authme.settings.properties.SecuritySettings;
import javax.inject.Inject;
import java.util.concurrent.ConcurrentHashMap;
/**
* Manager for the handling of captchas.
*/
public class CaptchaManager {
public class CaptchaManager implements SettingsDependent {
private final int threshold;
private final int captchaLength;
private final ConcurrentHashMap<String, Integer> playerCounts;
private final ConcurrentHashMap<String, String> captchaCodes;
public CaptchaManager(NewSetting settings) {
private boolean isEnabled;
private int threshold;
private int captchaLength;
@Inject
CaptchaManager(NewSetting settings) {
this.playerCounts = new ConcurrentHashMap<>();
this.captchaCodes = new ConcurrentHashMap<>();
this.threshold = settings.getProperty(SecuritySettings.MAX_LOGIN_TRIES_BEFORE_CAPTCHA);
this.captchaLength = settings.getProperty(SecuritySettings.CAPTCHA_LENGTH);
loadSettings(settings);
}
public void increaseCount(String player) {
String playerLower = player.toLowerCase();
/**
* Increases the failure count for the given player.
*
* @param name the player's name
*/
public void increaseCount(String name) {
if (isEnabled) {
String playerLower = name.toLowerCase();
Integer currentCount = playerCounts.get(playerLower);
if (currentCount == null) {
playerCounts.put(playerLower, 1);
@ -32,50 +42,86 @@ public class CaptchaManager {
playerCounts.put(playerLower, currentCount + 1);
}
}
}
/**
* Return whether the given player is required to solve a captcha.
* Returns whether the given player is required to solve a captcha.
*
* @param player The player to verify
* @return True if the player has to solve a captcha, false otherwise
* @param name the name of the player to verify
* @return true if the player has to solve a captcha, false otherwise
*/
public boolean isCaptchaRequired(String player) {
Integer count = playerCounts.get(player.toLowerCase());
public boolean isCaptchaRequired(String name) {
if (isEnabled) {
Integer count = playerCounts.get(name.toLowerCase());
return count != null && count >= threshold;
}
return false;
}
/**
* Return the captcha code for the player. Creates one if none present, so call only after
* checking with {@link #isCaptchaRequired}.
* Returns the stored captcha code for the player.
*
* @param player The player
* @return The code required for the player
* @param name the player's name
* @return the code the player is required to enter, or null if none registered
*/
public String getCaptchaCode(String player) {
String code = captchaCodes.get(player.toLowerCase());
if (code == null) {
code = RandomString.generate(captchaLength);
captchaCodes.put(player.toLowerCase(), code);
public String getCaptchaCode(String name) {
return captchaCodes.get(name.toLowerCase());
}
/**
* 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 = getCaptchaCode(name);
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 = RandomString.generate(captchaLength);
captchaCodes.put(name.toLowerCase(), code);
return code;
}
/**
* Return whether the supplied code is correct for the given player.
* Checks the given code against the existing one and resets the player's auth failure count upon success.
*
* @param player 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
* @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 player, String code) {
String savedCode = captchaCodes.get(player.toLowerCase());
public boolean checkCode(String name, String code) {
String savedCode = captchaCodes.get(name.toLowerCase());
if (savedCode == null) {
return true;
} else if (savedCode.equalsIgnoreCase(code)) {
captchaCodes.remove(player.toLowerCase());
captchaCodes.remove(name.toLowerCase());
playerCounts.remove(name.toLowerCase());
return true;
}
return false;
}
public void resetCounts(String name) {
if (isEnabled) {
captchaCodes.remove(name.toLowerCase());
playerCounts.remove(name.toLowerCase());
}
}
@Override
public void loadSettings(NewSetting settings) {
this.isEnabled = settings.getProperty(SecuritySettings.USE_CAPTCHA);
this.threshold = settings.getProperty(SecuritySettings.MAX_LOGIN_TRIES_BEFORE_CAPTCHA);
this.captchaLength = settings.getProperty(SecuritySettings.CAPTCHA_LENGTH);
}
}

View File

@ -1,12 +1,10 @@
package fr.xephi.authme.command.executable.captcha;
import fr.xephi.authme.AuthMe;
import fr.xephi.authme.cache.CaptchaManager;
import fr.xephi.authme.cache.auth.PlayerCache;
import fr.xephi.authme.command.CommandService;
import fr.xephi.authme.command.PlayerCommand;
import fr.xephi.authme.output.MessageKey;
import fr.xephi.authme.security.RandomString;
import fr.xephi.authme.settings.properties.SecuritySettings;
import org.bukkit.entity.Player;
import javax.inject.Inject;
@ -15,46 +13,32 @@ import java.util.List;
public class CaptchaCommand extends PlayerCommand {
@Inject
private AuthMe plugin;
private PlayerCache playerCache;
@Inject
private PlayerCache playerCache;
private CaptchaManager captchaManager;
@Override
public void runCommand(Player player, List<String> arguments, CommandService commandService) {
final String playerNameLowerCase = player.getName().toLowerCase();
final String captcha = arguments.get(0);
final String playerName = player.getName().toLowerCase();
// Command logic
if (playerCache.isAuthenticated(playerNameLowerCase)) {
if (playerCache.isAuthenticated(playerName)) {
commandService.send(player, MessageKey.ALREADY_LOGGED_IN_ERROR);
return;
}
if (!commandService.getProperty(SecuritySettings.USE_CAPTCHA)) {
} else if (!captchaManager.isCaptchaRequired(playerName)) {
commandService.send(player, MessageKey.USAGE_LOGIN);
return;
} else {
checkCaptcha(player, arguments.get(0), commandService);
}
}
if (!plugin.cap.containsKey(playerNameLowerCase)) {
commandService.send(player, MessageKey.USAGE_LOGIN);
return;
}
if (!captcha.equals(plugin.cap.get(playerNameLowerCase))) {
plugin.cap.remove(playerNameLowerCase);
int captchaLength = commandService.getProperty(SecuritySettings.CAPTCHA_LENGTH);
String randStr = RandomString.generate(captchaLength);
plugin.cap.put(playerNameLowerCase, randStr);
commandService.send(player, MessageKey.CAPTCHA_WRONG_ERROR, plugin.cap.get(playerNameLowerCase));
return;
}
plugin.captcha.remove(playerNameLowerCase);
plugin.cap.remove(playerNameLowerCase);
// Show a status message
commandService.send(player, MessageKey.CAPTCHA_SUCCESS);
commandService.send(player, MessageKey.LOGIN_MESSAGE);
private void checkCaptcha(Player player, String captchaCode, CommandService service) {
final boolean isCorrectCode = captchaManager.checkCode(player.getName(), captchaCode);
if (isCorrectCode) {
service.send(player, MessageKey.CAPTCHA_SUCCESS);
service.send(player, MessageKey.LOGIN_MESSAGE);
} else {
String newCode = captchaManager.generateCode(player.getName());
service.send(player, MessageKey.CAPTCHA_WRONG_ERROR, newCode);
}
}
}

View File

@ -2,6 +2,7 @@ package fr.xephi.authme.process.login;
import fr.xephi.authme.AuthMe;
import fr.xephi.authme.ConsoleLogger;
import fr.xephi.authme.cache.CaptchaManager;
import fr.xephi.authme.cache.auth.PlayerAuth;
import fr.xephi.authme.cache.auth.PlayerCache;
import fr.xephi.authme.cache.limbo.LimboCache;
@ -17,7 +18,6 @@ import fr.xephi.authme.process.AsynchronousProcess;
import fr.xephi.authme.process.ProcessService;
import fr.xephi.authme.process.SyncProcessManager;
import fr.xephi.authme.security.PasswordSecurity;
import fr.xephi.authme.security.RandomString;
import fr.xephi.authme.settings.Settings;
import fr.xephi.authme.settings.properties.DatabaseSettings;
import fr.xephi.authme.settings.properties.EmailSettings;
@ -66,25 +66,17 @@ public class AsynchronousLogin implements AsynchronousProcess {
@Inject
private PasswordSecurity passwordSecurity;
@Inject
private CaptchaManager captchaManager;
AsynchronousLogin() { }
private boolean needsCaptcha(Player player) {
final String name = player.getName().toLowerCase();
if (service.getProperty(SecuritySettings.USE_CAPTCHA)) {
if (plugin.captcha.containsKey(name)) {
int i = plugin.captcha.get(name) + 1;
plugin.captcha.remove(name);
plugin.captcha.putIfAbsent(name, i);
} else {
plugin.captcha.putIfAbsent(name, 1);
}
if (plugin.captcha.containsKey(name) && plugin.captcha.get(name) > Settings.maxLoginTry) {
plugin.cap.putIfAbsent(name, RandomString.generate(Settings.captchaLength));
service.send(player, MessageKey.USAGE_CAPTCHA, plugin.cap.get(name));
if (captchaManager.isCaptchaRequired(player.getName())) {
service.send(player, MessageKey.USAGE_CAPTCHA, captchaManager.getCaptchaCodeOrGenerateNew(player.getName()));
return true;
}
}
return false;
}
@ -124,7 +116,7 @@ public class AsynchronousLogin implements AsynchronousProcess {
}
final String ip = Utils.getPlayerIp(player);
if (Settings.getMaxLoginPerIp > 0
if (service.getProperty(RestrictionSettings.MAX_LOGIN_PER_IP) > 0
&& !permissionsManager.hasPermission(player, PlayerStatePermission.ALLOW_MULTIPLE_ACCOUNTS)
&& !"127.0.0.1".equalsIgnoreCase(ip) && !"localhost".equalsIgnoreCase(ip)) {
if (plugin.isLoggedIp(name, ip)) {
@ -168,16 +160,9 @@ public class AsynchronousLogin implements AsynchronousProcess {
.build();
database.updateSession(auth);
if (service.getProperty(SecuritySettings.USE_CAPTCHA)) {
if (plugin.captcha.containsKey(name)) {
plugin.captcha.remove(name);
}
if (plugin.cap.containsKey(name)) {
plugin.cap.remove(name);
}
}
captchaManager.resetCounts(name);
player.setNoDamageTicks(0);
if (!forceLogin)
service.send(player, MessageKey.LOGIN_SUCCESS);
@ -214,6 +199,7 @@ public class AsynchronousLogin implements AsynchronousProcess {
if (!service.getProperty(SecuritySettings.REMOVE_SPAM_FROM_CONSOLE)) {
ConsoleLogger.info(player.getName() + " used the wrong password");
}
captchaManager.increaseCount(name);
if (service.getProperty(RestrictionSettings.KICK_ON_WRONG_PASSWORD)) {
bukkitService.scheduleSyncDelayedTask(new Runnable() {
@Override

View File

@ -34,11 +34,11 @@ public class SecuritySettings implements SettingsClass {
public static final Property<Boolean> USE_LOGGING =
newProperty("Security.console.logConsole", true);
@Comment("Player need to put a captcha when he fails too lot the password")
@Comment("Enable captcha when a player uses wrong password too many times")
public static final Property<Boolean> USE_CAPTCHA =
newProperty("Security.captcha.useCaptcha", false);
@Comment("Max allowed tries before request a captcha")
@Comment("Max allowed tries before a captcha is required")
public static final Property<Integer> MAX_LOGIN_TRIES_BEFORE_CAPTCHA =
newProperty("Security.captcha.maxLoginTry", 5);

View File

@ -321,9 +321,9 @@ Security:
# Copy AuthMe log output in a separate file as well?
logConsole: true
captcha:
# Player need to put a captcha when he fails too lot the password
# Enable captcha when a player uses wrong password too many times
useCaptcha: false
# Max allowed tries before request a captcha
# Max allowed tries before a captcha is required
maxLoginTry: 5
# Captcha length
captchaLength: 5

View File

@ -1,10 +1,14 @@
package fr.xephi.authme.cache;
import fr.xephi.authme.ReflectionTestUtils;
import fr.xephi.authme.settings.NewSetting;
import fr.xephi.authme.settings.properties.SecuritySettings;
import org.junit.Test;
import java.util.Map;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.nullValue;
import static org.junit.Assert.assertThat;
import static org.mockito.BDDMockito.given;
import static org.mockito.Mockito.mock;
@ -39,7 +43,7 @@ public class CaptchaManagerTest {
String player = "Miner";
NewSetting settings = mockSettings(1, 4);
CaptchaManager manager = new CaptchaManager(settings);
String captchaCode = manager.getCaptchaCode(player);
String captchaCode = manager.getCaptchaCodeOrGenerateNew(player);
// when
boolean badResult = manager.checkCode(player, "wrong_code");
@ -53,11 +57,105 @@ public class CaptchaManagerTest {
assertThat(manager.checkCode(player, "bogus"), equalTo(true));
}
/**
* Tests {@link CaptchaManager#getCaptchaCode} and {@link CaptchaManager#getCaptchaCodeOrGenerateNew}.
* The former method should never change the code (and so return {@code null} for no code) while the latter should
* generate a new code if no code is yet present. If a code is saved, it should never generate a new one.
*/
@Test
public void shouldHaveSameCodeAfterGeneration() {
// given
String player = "Tester";
NewSetting settings = mockSettings(1, 5);
CaptchaManager manager = new CaptchaManager(settings);
// when
String code1 = manager.getCaptchaCode(player);
String code2 = manager.getCaptchaCodeOrGenerateNew(player);
String code3 = manager.getCaptchaCode(player);
String code4 = manager.getCaptchaCodeOrGenerateNew(player);
String code5 = manager.getCaptchaCode(player);
// then
assertThat(code1, nullValue());
assertThat(code2.length(), equalTo(5));
assertThat(code3, equalTo(code2));
assertThat(code4, equalTo(code2));
assertThat(code5, equalTo(code2));
}
@Test
public void shouldIncreaseAndResetCount() {
// given
String player = "plaYer";
NewSetting settings = mockSettings(2, 3);
CaptchaManager manager = new CaptchaManager(settings);
// when
manager.increaseCount(player);
manager.increaseCount(player);
// then
assertThat(manager.isCaptchaRequired(player), equalTo(true));
assertHasCount(manager, player, 2);
// when 2
manager.resetCounts(player);
// then 2
assertThat(manager.isCaptchaRequired(player), equalTo(false));
assertHasCount(manager, player, null);
}
@Test
public void shouldNotIncreaseCountForDisabledCaptcha() {
// given
String player = "someone_";
NewSetting settings = mockSettings(1, 3);
given(settings.getProperty(SecuritySettings.USE_CAPTCHA)).willReturn(false);
CaptchaManager manager = new CaptchaManager(settings);
// when
manager.increaseCount(player);
// then
assertThat(manager.isCaptchaRequired(player), equalTo(false));
assertHasCount(manager, player, null);
}
@Test
public void shouldNotCheckCountIfCaptchaIsDisabled() {
// given
String player = "Robert001";
NewSetting settings = mockSettings(1, 5);
CaptchaManager manager = new CaptchaManager(settings);
given(settings.getProperty(SecuritySettings.USE_CAPTCHA)).willReturn(false);
// when
manager.increaseCount(player);
// assumptions
assertThat(manager.isCaptchaRequired(player), equalTo(true));
assertHasCount(manager, player, 1);
// end assumptions
manager.loadSettings(settings);
boolean result = manager.isCaptchaRequired(player);
// then
assertThat(result, equalTo(false));
}
private static NewSetting mockSettings(int maxTries, int captchaLength) {
NewSetting settings = mock(NewSetting.class);
given(settings.getProperty(SecuritySettings.USE_CAPTCHA)).willReturn(true);
given(settings.getProperty(SecuritySettings.MAX_LOGIN_TRIES_BEFORE_CAPTCHA)).willReturn(maxTries);
given(settings.getProperty(SecuritySettings.CAPTCHA_LENGTH)).willReturn(captchaLength);
return settings;
}
private static void assertHasCount(CaptchaManager manager, String player, Integer count) {
@SuppressWarnings("unchecked")
Map<String, Integer> playerCounts = (Map<String, Integer>) ReflectionTestUtils
.getFieldValue(CaptchaManager.class, manager, "playerCounts");
assertThat(playerCounts.get(player.toLowerCase()), equalTo(count));
}
}

View File

@ -0,0 +1,121 @@
package fr.xephi.authme.command.executable.captcha;
import fr.xephi.authme.cache.CaptchaManager;
import fr.xephi.authme.cache.auth.PlayerCache;
import fr.xephi.authme.command.CommandService;
import fr.xephi.authme.output.MessageKey;
import org.bukkit.entity.Player;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.runners.MockitoJUnitRunner;
import java.util.Collections;
import static org.mockito.BDDMockito.given;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyNoMoreInteractions;
/**
* Test for {@link CaptchaCommand}.
*/
@RunWith(MockitoJUnitRunner.class)
public class CaptchaCommandTest {
@InjectMocks
private CaptchaCommand command;
@Mock
private CaptchaManager captchaManager;
@Mock
private PlayerCache playerCache;
@Mock
private CommandService commandService;
@Test
public void shouldDetectIfPlayerIsLoggedIn() {
// given
String name = "creeper011";
Player player = mockPlayerWithName(name);
given(playerCache.isAuthenticated(name)).willReturn(true);
// when
command.executeCommand(player, Collections.singletonList("123"), commandService);
// then
verify(commandService).send(player, MessageKey.ALREADY_LOGGED_IN_ERROR);
}
@Test
public void shouldShowLoginUsageIfCaptchaIsNotRequired() {
// given
String name = "bobby";
Player player = mockPlayerWithName(name);
given(playerCache.isAuthenticated(name)).willReturn(false);
given(captchaManager.isCaptchaRequired(name)).willReturn(false);
// when
command.executeCommand(player, Collections.singletonList("1234"), commandService);
// then
verify(commandService).send(player, MessageKey.USAGE_LOGIN);
verify(captchaManager).isCaptchaRequired(name);
verifyNoMoreInteractions(captchaManager);
}
@Test
public void shouldHandleCorrectCaptchaInput() {
// given
String name = "smith";
Player player = mockPlayerWithName(name);
given(playerCache.isAuthenticated(name)).willReturn(false);
given(captchaManager.isCaptchaRequired(name)).willReturn(true);
String captchaCode = "3991";
given(captchaManager.checkCode(name, captchaCode)).willReturn(true);
// when
command.executeCommand(player, Collections.singletonList(captchaCode), commandService);
// then
verify(captchaManager).isCaptchaRequired(name);
verify(captchaManager).checkCode(name, captchaCode);
verifyNoMoreInteractions(captchaManager);
verify(commandService).send(player, MessageKey.CAPTCHA_SUCCESS);
verify(commandService).send(player, MessageKey.LOGIN_MESSAGE);
verifyNoMoreInteractions(commandService);
}
@Test
public void shouldHandleWrongCaptchaInput() {
// given
String name = "smith";
Player player = mockPlayerWithName(name);
given(playerCache.isAuthenticated(name)).willReturn(false);
given(captchaManager.isCaptchaRequired(name)).willReturn(true);
String captchaCode = "2468";
given(captchaManager.checkCode(name, captchaCode)).willReturn(false);
String newCode = "1337";
given(captchaManager.generateCode(name)).willReturn(newCode);
// when
command.executeCommand(player, Collections.singletonList(captchaCode), commandService);
// then
verify(captchaManager).isCaptchaRequired(name);
verify(captchaManager).checkCode(name, captchaCode);
verify(captchaManager).generateCode(name);
verifyNoMoreInteractions(captchaManager);
verify(commandService).send(player, MessageKey.CAPTCHA_WRONG_ERROR, newCode);
verifyNoMoreInteractions(commandService);
}
private static Player mockPlayerWithName(String name) {
Player player = mock(Player.class);
given(player.getName()).willReturn(name);
return player;
}
}