Merge pull request #1396 from AuthMe/1350-optional-async-register-checks

#1350 Add option to force using the sync PlayerLoginEvent
This commit is contained in:
Gabriele C 2017-11-02 11:04:05 +01:00 committed by GitHub
commit cb8b2cbbad
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 180 additions and 55 deletions

View File

@ -1,5 +1,5 @@
<!-- AUTO-GENERATED FILE! Do not edit this directly -->
<!-- File auto-generated on Sat Oct 28 10:39:36 CEST 2017. See docs/config/config.tpl.md -->
<!-- File auto-generated on Tue Oct 31 15:56:59 CET 2017. See docs/config/config.tpl.md -->
## AuthMe Configuration
The first time you run AuthMe it will create a config.yml file in the plugins/AuthMe folder,
@ -110,6 +110,8 @@ settings:
# Message language, available languages:
# https://github.com/AuthMe/AuthMeReloaded/blob/master/docs/translations.md
messagesLanguage: 'en'
# Forces authme to hook into Vault instead of a specific permission handler system.
forceVaultHook: false
# Log level: INFO, FINE, DEBUG. Use INFO for general messages,
# FINE for some additional detailed ones (like password failed),
# and DEBUG for debugging
@ -117,6 +119,10 @@ settings:
# By default we schedule async tasks when talking to the database. If you want
# typical communication with the database to happen synchronously, set this to false
useAsyncTasks: true
# By default we handle the AsyncPlayerPreLoginEvent which makes the plugin faster
# but it is incompatible with any permission plugin not included in our compatibility list.
# If you have issues with permission checks on player join please disable this option.
useAsyncPreLoginEvent: true
restrictions:
# Can not authenticated players chat?
# Keep in mind that this feature also blocks all commands not
@ -554,4 +560,4 @@ To change settings on a running server, save your changes to config.yml and use
---
This page was automatically generated on the [AuthMe/AuthMeReloaded repository](https://github.com/AuthMe/AuthMeReloaded/tree/master/docs/) on Sat Oct 28 10:39:36 CEST 2017
This page was automatically generated on the [AuthMe/AuthMeReloaded repository](https://github.com/AuthMe/AuthMeReloaded/tree/master/docs/) on Tue Oct 31 15:56:59 CET 2017

View File

@ -0,0 +1,66 @@
package fr.xephi.authme.listener;
import fr.xephi.authme.permission.PermissionNode;
import fr.xephi.authme.permission.PermissionsManager;
import org.bukkit.entity.Player;
import java.util.function.BiFunction;
/**
* Represents a player joining the server, which depending on the available
* information may be his name or the actual Player object.
*/
public final class JoiningPlayer {
private final String name;
private final BiFunction<PermissionsManager, PermissionNode, Boolean> permissionLookupFunction;
/**
* Hidden constructor.
*
* @param name the player's name
* @param permFunction the function to use for permission lookups
*/
private JoiningPlayer(String name, BiFunction<PermissionsManager, PermissionNode, Boolean> permFunction) {
this.name = name;
this.permissionLookupFunction = permFunction;
}
/**
* Creates a {@link JoiningPlayer} instance from the given name.
*
* @param name the player's name
* @return the created instance
*/
public static JoiningPlayer fromName(String name) {
return new JoiningPlayer(name, (manager, perm) -> manager.hasPermissionOffline(name, perm));
}
/**
* Creates a {@link JoiningPlayer} instance from the given Player object.
*
* @param player the player
* @return the created instance
*/
public static JoiningPlayer fromPlayerObject(Player player) {
return new JoiningPlayer(player.getName(), (manager, perm) -> manager.hasPermission(player, perm));
}
/**
* @return the player's name
*/
public String getName() {
return name;
}
/**
* Returns the function to use for permission lookups. Takes two arguments: the PermissionsManager instance,
* and the permission node to look up. The result is a boolean indicating whether or not this joining player
* has permission.
*
* @return the permissions lookup function to use
*/
public BiFunction<PermissionsManager, PermissionNode, Boolean> getPermissionLookupFunction() {
return permissionLookupFunction;
}
}

View File

@ -64,16 +64,16 @@ public class OnJoinVerifier implements Reloadable {
/**
* Checks if Antibot is enabled.
*
* @param name the player name
* @param joiningPlayer the joining player to check
* @param isAuthAvailable whether or not the player is registered
* @throws FailedVerificationException if the verification fails
*/
public void checkAntibot(String name, boolean isAuthAvailable) throws FailedVerificationException {
if (isAuthAvailable || permissionsManager.hasPermissionOffline(name, PlayerStatePermission.BYPASS_ANTIBOT)) {
public void checkAntibot(JoiningPlayer joiningPlayer, boolean isAuthAvailable) throws FailedVerificationException {
if (isAuthAvailable || permissionsManager.hasPermission(joiningPlayer, PlayerStatePermission.BYPASS_ANTIBOT)) {
return;
}
if (antiBotService.shouldKick()) {
antiBotService.addPlayerKick(name);
antiBotService.addPlayerKick(joiningPlayer.getName());
throw new FailedVerificationException(MessageKey.KICK_ANTIBOT);
}
}
@ -167,15 +167,16 @@ public class OnJoinVerifier implements Reloadable {
/**
* Checks that the player's country is admitted.
*
* @param name the player name
* @param joiningPlayer the joining player to verify
* @param address the player address
* @param isAuthAvailable whether or not the user is registered
* @throws FailedVerificationException if the verification fails
*/
public void checkPlayerCountry(String name, String address, boolean isAuthAvailable) throws FailedVerificationException {
public void checkPlayerCountry(JoiningPlayer joiningPlayer, String address,
boolean isAuthAvailable) throws FailedVerificationException {
if ((!isAuthAvailable || settings.getProperty(ProtectionSettings.ENABLE_PROTECTION_REGISTERED))
&& settings.getProperty(ProtectionSettings.ENABLE_PROTECTION)
&& !permissionsManager.hasPermissionOffline(name, PlayerStatePermission.BYPASS_COUNTRY_CHECK)
&& !permissionsManager.hasPermission(joiningPlayer, PlayerStatePermission.BYPASS_COUNTRY_CHECK)
&& !validationService.isCountryAdmitted(address)) {
throw new FailedVerificationException(MessageKey.COUNTRY_BANNED_ERROR);
}

View File

@ -14,6 +14,7 @@ import fr.xephi.authme.service.ValidationService;
import fr.xephi.authme.settings.Settings;
import fr.xephi.authme.settings.SpawnLoader;
import fr.xephi.authme.settings.properties.HooksSettings;
import fr.xephi.authme.settings.properties.PluginSettings;
import fr.xephi.authme.settings.properties.RegistrationSettings;
import fr.xephi.authme.settings.properties.RestrictionSettings;
import org.bukkit.ChatColor;
@ -83,7 +84,7 @@ public class PlayerListener implements Listener {
@Inject
private PermissionsManager permissionsManager;
private static boolean isAsyncPlayerPreLoginEventCalled = false;
private boolean isAsyncPlayerPreLoginEventCalled = false;
@EventHandler(ignoreCancelled = true, priority = EventPriority.LOWEST)
public void onPlayerCommandPreprocess(PlayerCommandPreprocessEvent event) {
@ -206,8 +207,9 @@ public class PlayerListener implements Listener {
teleportationService.teleportNewPlayerToFirstSpawn(player);
}
private void runOnJoinChecks(String name, String ip) throws FailedVerificationException {
private void runOnJoinChecks(JoiningPlayer joiningPlayer, String ip) throws FailedVerificationException {
// Fast stuff
final String name = joiningPlayer.getName();
onJoinVerifier.checkSingleSession(name);
onJoinVerifier.checkIsValidName(name);
@ -216,9 +218,9 @@ public class PlayerListener implements Listener {
final PlayerAuth auth = dataSource.getAuth(name);
final boolean isAuthAvailable = auth != null;
onJoinVerifier.checkKickNonRegistered(isAuthAvailable);
onJoinVerifier.checkAntibot(name, isAuthAvailable);
onJoinVerifier.checkAntibot(joiningPlayer, isAuthAvailable);
onJoinVerifier.checkNameCasing(name, auth);
onJoinVerifier.checkPlayerCountry(name, ip, isAuthAvailable);
onJoinVerifier.checkPlayerCountry(joiningPlayer, ip, isAuthAvailable);
}
// Note #831: AsyncPlayerPreLoginEvent is not fired by all servers in offline mode
@ -230,6 +232,9 @@ public class PlayerListener implements Listener {
@EventHandler(priority = EventPriority.HIGHEST)
public void onAsyncPlayerPreLoginEvent(AsyncPlayerPreLoginEvent event) {
isAsyncPlayerPreLoginEventCalled = true;
if (!settings.getProperty(PluginSettings.USE_ASYNC_PRE_LOGIN_EVENT)) {
return;
}
final String name = event.getName();
@ -245,7 +250,7 @@ public class PlayerListener implements Listener {
}
try {
runOnJoinChecks(name, event.getAddress().getHostAddress());
runOnJoinChecks(JoiningPlayer.fromName(name), event.getAddress().getHostAddress());
} catch (FailedVerificationException e) {
event.setKickMessage(m.retrieveSingle(e.getReason(), e.getArgs()));
event.setLoginResult(AsyncPlayerPreLoginEvent.Result.KICK_OTHER);
@ -271,9 +276,9 @@ public class PlayerListener implements Listener {
return;
}
if (!isAsyncPlayerPreLoginEventCalled) {
if (!isAsyncPlayerPreLoginEventCalled || !settings.getProperty(PluginSettings.USE_ASYNC_PRE_LOGIN_EVENT)) {
try {
runOnJoinChecks(name, event.getAddress().getHostAddress());
runOnJoinChecks(JoiningPlayer.fromPlayerObject(player), event.getAddress().getHostAddress());
} catch (FailedVerificationException e) {
event.setKickMessage(m.retrieveSingle(e.getReason(), e.getArgs()));
event.setResult(PlayerLoginEvent.Result.KICK_OTHER);

View File

@ -2,6 +2,7 @@ package fr.xephi.authme.permission;
import fr.xephi.authme.ConsoleLogger;
import fr.xephi.authme.initialization.Reloadable;
import fr.xephi.authme.listener.JoiningPlayer;
import fr.xephi.authme.permission.handlers.BPermissionsHandler;
import fr.xephi.authme.permission.handlers.LuckPermsHandler;
import fr.xephi.authme.permission.handlers.PermissionHandler;
@ -86,7 +87,7 @@ public class PermissionsManager implements Reloadable {
return;
}
} catch (PermissionHandlerException e) {
e.printStackTrace();
ConsoleLogger.logException("Failed to create Vault hook (forced):", e);
}
} else {
// Loop through all the available permissions system types
@ -229,6 +230,17 @@ public class PermissionsManager implements Reloadable {
return player.hasPermission(permissionNode.getNode());
}
/**
* Check if the given player has permission for the given permission node.
*
* @param joiningPlayer The player to check
* @param permissionNode The permission node to verify
* @return true if the player has permission, false otherwise
*/
public boolean hasPermission(JoiningPlayer joiningPlayer, PermissionNode permissionNode) {
return joiningPlayer.getPermissionLookupFunction().apply(this, permissionNode);
}
/**
* Check if a player has permission for the given permission node. This is for offline player checks.
* If no permissions system is used, then the player will not have permission.

View File

@ -82,6 +82,14 @@ public final class PluginSettings implements SettingsHolder {
public static final Property<Boolean> USE_ASYNC_TASKS =
newProperty("settings.useAsyncTasks", true);
@Comment({
"By default we handle the AsyncPlayerPreLoginEvent which makes the plugin faster",
"but it is incompatible with any permission plugin not included in our compatibility list.",
"If you have issues with permission checks on player join please disable this option."
})
public static final Property<Boolean> USE_ASYNC_PRE_LOGIN_EVENT =
newProperty("settings.useAsyncPreLoginEvent", true);
private PluginSettings() {
}

View File

@ -29,7 +29,6 @@ import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.junit.MockitoJUnitRunner;
import java.net.InetSocketAddress;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
@ -376,27 +375,27 @@ public class OnJoinVerifierTest {
@Test
public void shouldAllowUser() throws FailedVerificationException {
// given
String name = "Bobby";
JoiningPlayer joiningPlayer = JoiningPlayer.fromName("Bobby");
boolean isAuthAvailable = false;
given(permissionsManager.hasPermissionOffline(name, PlayerStatePermission.BYPASS_ANTIBOT)).willReturn(false);
given(permissionsManager.hasPermission(joiningPlayer, PlayerStatePermission.BYPASS_ANTIBOT)).willReturn(false);
given(antiBotService.shouldKick()).willReturn(false);
// when
onJoinVerifier.checkAntibot(name, isAuthAvailable);
onJoinVerifier.checkAntibot(joiningPlayer, isAuthAvailable);
// then
verify(permissionsManager).hasPermissionOffline(name, PlayerStatePermission.BYPASS_ANTIBOT);
verify(permissionsManager).hasPermission(joiningPlayer, PlayerStatePermission.BYPASS_ANTIBOT);
verify(antiBotService).shouldKick();
}
@Test
public void shouldAllowUserWithAuth() throws FailedVerificationException {
// given
String name = "Lacey";
JoiningPlayer joiningPlayer = JoiningPlayer.fromName("Lacey");
boolean isAuthAvailable = true;
// when
onJoinVerifier.checkAntibot(name, isAuthAvailable);
onJoinVerifier.checkAntibot(joiningPlayer, isAuthAvailable);
// then
verifyZeroInteractions(permissionsManager, antiBotService);
@ -405,32 +404,32 @@ public class OnJoinVerifierTest {
@Test
public void shouldAllowUserWithBypassPermission() throws FailedVerificationException {
// given
String name = "Steward";
JoiningPlayer joiningPlayer = JoiningPlayer.fromName("Steward");
boolean isAuthAvailable = false;
given(permissionsManager.hasPermissionOffline(name, PlayerStatePermission.BYPASS_ANTIBOT)).willReturn(true);
given(permissionsManager.hasPermission(joiningPlayer, PlayerStatePermission.BYPASS_ANTIBOT)).willReturn(true);
// when
onJoinVerifier.checkAntibot(name, isAuthAvailable);
onJoinVerifier.checkAntibot(joiningPlayer, isAuthAvailable);
// then
verify(permissionsManager).hasPermissionOffline(name, PlayerStatePermission.BYPASS_ANTIBOT);
verify(permissionsManager).hasPermission(joiningPlayer, PlayerStatePermission.BYPASS_ANTIBOT);
verifyZeroInteractions(antiBotService);
}
@Test
public void shouldKickUserForFailedAntibotCheck() throws FailedVerificationException {
// given
String name = "D3";
JoiningPlayer joiningPlayer = JoiningPlayer.fromName("D3");
boolean isAuthAvailable = false;
given(permissionsManager.hasPermissionOffline(name, PlayerStatePermission.BYPASS_ANTIBOT)).willReturn(false);
given(permissionsManager.hasPermission(joiningPlayer, PlayerStatePermission.BYPASS_ANTIBOT)).willReturn(false);
given(antiBotService.shouldKick()).willReturn(true);
// when / then
try {
onJoinVerifier.checkAntibot(name, isAuthAvailable);
onJoinVerifier.checkAntibot(joiningPlayer, isAuthAvailable);
fail("Expected exception to be thrown");
} catch (FailedVerificationException e) {
verify(permissionsManager).hasPermissionOffline(name, PlayerStatePermission.BYPASS_ANTIBOT);
verify(permissionsManager).hasPermission(joiningPlayer, PlayerStatePermission.BYPASS_ANTIBOT);
verify(antiBotService).shouldKick();
}
@ -441,18 +440,18 @@ public class OnJoinVerifierTest {
*/
@Test
public void shouldNotCheckCountry() throws FailedVerificationException {
String name = "david";
JoiningPlayer joiningPlayer = JoiningPlayer.fromName("david");
String ip = "127.0.0.1";
// protection setting disabled
given(settings.getProperty(ProtectionSettings.ENABLE_PROTECTION)).willReturn(false);
given(settings.getProperty(ProtectionSettings.ENABLE_PROTECTION_REGISTERED)).willReturn(true);
onJoinVerifier.checkPlayerCountry(name, ip, false);
onJoinVerifier.checkPlayerCountry(joiningPlayer, ip, false);
verifyZeroInteractions(validationService);
// protection for registered players disabled
given(settings.getProperty(ProtectionSettings.ENABLE_PROTECTION_REGISTERED)).willReturn(false);
onJoinVerifier.checkPlayerCountry(name, ip, true);
onJoinVerifier.checkPlayerCountry(joiningPlayer, ip, true);
verifyZeroInteractions(validationService);
}
@ -460,12 +459,12 @@ public class OnJoinVerifierTest {
public void shouldCheckAndAcceptUnregisteredPlayerCountry() throws FailedVerificationException {
// given
String ip = "192.168.0.1";
String name = "lucas";
JoiningPlayer joiningPlayer = JoiningPlayer.fromName("lucas");
given(settings.getProperty(ProtectionSettings.ENABLE_PROTECTION)).willReturn(true);
given(validationService.isCountryAdmitted(ip)).willReturn(true);
// when
onJoinVerifier.checkPlayerCountry(name, ip, false);
onJoinVerifier.checkPlayerCountry(joiningPlayer, ip, false);
// then
verify(validationService).isCountryAdmitted(ip);
@ -475,13 +474,13 @@ public class OnJoinVerifierTest {
public void shouldCheckAndAcceptRegisteredPlayerCountry() throws FailedVerificationException {
// given
String ip = "192.168.10.24";
String name = "gabriel";
JoiningPlayer joiningPlayer = JoiningPlayer.fromName("gabriel");
given(settings.getProperty(ProtectionSettings.ENABLE_PROTECTION)).willReturn(true);
given(settings.getProperty(ProtectionSettings.ENABLE_PROTECTION_REGISTERED)).willReturn(true);
given(validationService.isCountryAdmitted(ip)).willReturn(true);
// when
onJoinVerifier.checkPlayerCountry(name, ip, true);
onJoinVerifier.checkPlayerCountry(joiningPlayer, ip, true);
// then
verify(validationService).isCountryAdmitted(ip);
@ -491,7 +490,7 @@ public class OnJoinVerifierTest {
public void shouldThrowForBannedCountry() throws FailedVerificationException {
// given
String ip = "192.168.40.0";
String name = "bob";
JoiningPlayer joiningPlayer = JoiningPlayer.fromName("bob");
given(settings.getProperty(ProtectionSettings.ENABLE_PROTECTION)).willReturn(true);
given(validationService.isCountryAdmitted(ip)).willReturn(false);
@ -499,7 +498,7 @@ public class OnJoinVerifierTest {
expectValidationExceptionWith(MessageKey.COUNTRY_BANNED_ERROR);
// when
onJoinVerifier.checkPlayerCountry(name, ip, false);
onJoinVerifier.checkPlayerCountry(joiningPlayer, ip, false);
}
@SuppressWarnings({ "unchecked", "rawtypes" })

View File

@ -48,7 +48,6 @@ import org.mockito.Mock;
import org.mockito.junit.MockitoJUnitRunner;
import java.net.InetAddress;
import java.net.UnknownHostException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
@ -64,6 +63,7 @@ import static org.junit.Assert.assertThat;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyBoolean;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.BDDMockito.given;
import static org.mockito.Mockito.atLeast;
import static org.mockito.Mockito.doThrow;
@ -555,7 +555,7 @@ public class PlayerListenerTest {
}
@Test
public void shouldPerformAllJoinVerificationsSuccessfully() throws FailedVerificationException, UnknownHostException {
public void shouldPerformAllJoinVerificationsSuccessfully() throws FailedVerificationException {
// given
String name = "someone";
Player player = mockPlayerWithName(name);
@ -575,10 +575,10 @@ public class PlayerListenerTest {
verify(onJoinVerifier).refusePlayerForFullServer(event);
verify(onJoinVerifier).checkSingleSession(name);
verify(onJoinVerifier).checkIsValidName(name);
verify(onJoinVerifier).checkAntibot(name, true);
verify(onJoinVerifier).checkAntibot(any(JoiningPlayer.class), eq(true));
verify(onJoinVerifier).checkKickNonRegistered(true);
verify(onJoinVerifier).checkNameCasing(name, auth);
verify(onJoinVerifier).checkPlayerCountry(name, ip, true);
verify(onJoinVerifier).checkPlayerCountry(any(JoiningPlayer.class), eq(ip), eq(true));
verifyNoModifyingCalls(event);
}

View File

@ -1,5 +1,6 @@
package fr.xephi.authme.permission;
import fr.xephi.authme.listener.JoiningPlayer;
import org.bukkit.Server;
import org.bukkit.command.CommandSender;
import org.bukkit.entity.Player;
@ -12,8 +13,13 @@ import org.mockito.junit.MockitoJUnitRunner;
import static org.hamcrest.Matchers.equalTo;
import static org.junit.Assert.assertThat;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.BDDMockito.given;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.verify;
/**
* Test for {@link PermissionsManager}.
@ -148,4 +154,26 @@ public class PermissionsManagerTest {
// then
assertThat(result, equalTo(true));
}
@Test
public void shouldHandleJoiningPlayerPermissionChecksWithProperMethod() {
// given
Player player = mock(Player.class);
JoiningPlayer fromPlayer = JoiningPlayer.fromPlayerObject(player);
JoiningPlayer fromName = JoiningPlayer.fromName("Chris");
PermissionsManager permManagerSpy = spy(permissionsManager);
given(permManagerSpy.hasPermission(any(Player.class), eq(PlayerPermission.LOGIN))).willReturn(true);
given(permManagerSpy.hasPermissionOffline(anyString(), eq(PlayerPermission.UNREGISTER))).willReturn(true);
// when
boolean resultFromPlayer = permManagerSpy.hasPermission(fromPlayer, PlayerPermission.LOGIN);
boolean resultFromName = permManagerSpy.hasPermission(fromName, PlayerPermission.UNREGISTER);
// then
assertThat(resultFromPlayer, equalTo(true));
assertThat(resultFromName, equalTo(true));
verify(permManagerSpy).hasPermission(player, PlayerPermission.LOGIN);
verify(permManagerSpy).hasPermissionOffline(fromName.getName(), PlayerPermission.UNREGISTER);
}
}