From b1a9abc019c80a582b5a45eb05d87f0215da6b50 Mon Sep 17 00:00:00 2001 From: games647 Date: Sat, 19 Jun 2021 17:47:30 +0200 Subject: [PATCH] Perform all restriction checks only on cache Cache registration status for online players. The cache stays valid until the player leaves the server or logs manual in. --- .../xephi/authme/data/auth/PlayerCache.java | 39 ++++++++++++++++- .../authme/datasource/CacheDataSource.java | 2 +- .../authme/listener/ListenerService.java | 43 +++++++++++-------- .../protocollib/InventoryPacketAdapter.java | 18 +++----- .../protocollib/ProtocolLibService.java | 39 +++-------------- .../protocollib/TabCompletePacketAdapter.java | 9 ++-- .../authme/process/join/AsynchronousJoin.java | 8 +++- .../authme/service/TeleportationService.java | 3 +- .../authme/listener/ListenerServiceTest.java | 10 +++-- 9 files changed, 97 insertions(+), 74 deletions(-) diff --git a/src/main/java/fr/xephi/authme/data/auth/PlayerCache.java b/src/main/java/fr/xephi/authme/data/auth/PlayerCache.java index 46fafbf77..7c21ee4b3 100644 --- a/src/main/java/fr/xephi/authme/data/auth/PlayerCache.java +++ b/src/main/java/fr/xephi/authme/data/auth/PlayerCache.java @@ -10,6 +10,7 @@ import java.util.concurrent.ConcurrentHashMap; public class PlayerCache { private final Map cache = new ConcurrentHashMap<>(); + private final Map registeredCache = new ConcurrentHashMap<>(); PlayerCache() { } @@ -20,6 +21,7 @@ public class PlayerCache { * @param auth the player auth object to save */ public void updatePlayer(PlayerAuth auth) { + registeredCache.put(auth.getNickname().toLowerCase(), RegistrationStatus.REGISTERED); cache.put(auth.getNickname().toLowerCase(), auth); } @@ -30,6 +32,7 @@ public class PlayerCache { */ public void removePlayer(String user) { cache.remove(user.toLowerCase()); + registeredCache.remove(user.toLowerCase()); } /** @@ -43,6 +46,35 @@ public class PlayerCache { return cache.containsKey(user.toLowerCase()); } + /** + * Add a registration entry to the cache for active use later like the player active playing. + * + * @param user player name + * @param status registration status + */ + public void addRegistrationStatus(String user, RegistrationStatus status) { + registeredCache.put(user.toLowerCase(), status); + } + + /** + * Update the status for existing entries like currently active users + * @param user player name + * @param status newest query result + */ + public void updateRegistrationStatus(String user, RegistrationStatus status) { + registeredCache.replace(user, status); + } + + /** + * Checks if there is cached result with the player having an account. + * Warning: This shouldn't be used for authentication, because the result could be outdated. + * @param user player name + * @return Cached result about being registered or unregistered and UNKNOWN if there is no cache entry + */ + public RegistrationStatus getRegistrationStatus(String user) { + return registeredCache.getOrDefault(user.toLowerCase(), RegistrationStatus.UNKNOWN); + } + /** * Returns the PlayerAuth associated with the given user, if available. * @@ -66,8 +98,13 @@ public class PlayerCache { * * @return all player auths inside the player cache */ - public Map getCache() { + public Map getAuthCache() { return this.cache; } + public enum RegistrationStatus { + REGISTERED, + UNREGISTERED, + UNKNOWN + } } diff --git a/src/main/java/fr/xephi/authme/datasource/CacheDataSource.java b/src/main/java/fr/xephi/authme/datasource/CacheDataSource.java index e1418dcaf..9b4fad54a 100644 --- a/src/main/java/fr/xephi/authme/datasource/CacheDataSource.java +++ b/src/main/java/fr/xephi/authme/datasource/CacheDataSource.java @@ -267,7 +267,7 @@ public class CacheDataSource implements DataSource { @Override public List getLoggedPlayersWithEmptyMail() { - return playerCache.getCache().values().stream() + return playerCache.getAuthCache().values().stream() .filter(auth -> Utils.isEmailEmpty(auth.getEmail())) .map(PlayerAuth::getRealName) .collect(Collectors.toList()); diff --git a/src/main/java/fr/xephi/authme/listener/ListenerService.java b/src/main/java/fr/xephi/authme/listener/ListenerService.java index d283f3e4e..4502a9e31 100644 --- a/src/main/java/fr/xephi/authme/listener/ListenerService.java +++ b/src/main/java/fr/xephi/authme/listener/ListenerService.java @@ -1,6 +1,7 @@ package fr.xephi.authme.listener; import fr.xephi.authme.data.auth.PlayerCache; +import fr.xephi.authme.data.auth.PlayerCache.RegistrationStatus; import fr.xephi.authme.datasource.DataSource; import fr.xephi.authme.initialization.SettingsDependent; import fr.xephi.authme.service.ValidationService; @@ -17,7 +18,7 @@ import javax.inject.Inject; /** * Service class for the AuthMe listeners to determine whether an event should be canceled. */ -class ListenerService implements SettingsDependent { +public class ListenerService implements SettingsDependent { private final DataSource dataSource; private final PlayerCache playerCache; @@ -77,28 +78,32 @@ class ListenerService implements SettingsDependent { * @return true if the associated event should be canceled, false otherwise */ public boolean shouldCancelEvent(Player player) { - return player != null && !checkAuth(player.getName()) && !PlayerUtils.isNpc(player); + return player != null && !PlayerUtils.isNpc(player) && shouldRestrictPlayer(player.getName()); + } + + /** + * Check if restriction are required for the given player name. The check will be performed against the local + * cache. This means changes from other sources like web services will have a delay to it. + * + * @param name player name + * @return true if the player needs to be restricted + */ + public boolean shouldRestrictPlayer(String name) { + if (validationService.isUnrestricted(name) || playerCache.isAuthenticated(name)) { + return false; + } + + if (isRegistrationForced) { + // registration always required to play - so restrict everything + return true; + } + + // registration not enforced, but registered players needs to be restricted if not logged in + return playerCache.getRegistrationStatus(name) == RegistrationStatus.REGISTERED; } @Override public void reload(Settings settings) { isRegistrationForced = settings.getProperty(RegistrationSettings.FORCE); } - - /** - * Checks whether the player is allowed to perform actions (i.e. whether he is logged in - * or if other settings permit playing). - * - * @param name the name of the player to verify - * @return true if the player may play, false otherwise - */ - private boolean checkAuth(String name) { - if (validationService.isUnrestricted(name) || playerCache.isAuthenticated(name)) { - return true; - } - if (!isRegistrationForced && !dataSource.isAuthAvailable(name)) { - return true; - } - return false; - } } diff --git a/src/main/java/fr/xephi/authme/listener/protocollib/InventoryPacketAdapter.java b/src/main/java/fr/xephi/authme/listener/protocollib/InventoryPacketAdapter.java index ec256d9b8..eded77c5b 100644 --- a/src/main/java/fr/xephi/authme/listener/protocollib/InventoryPacketAdapter.java +++ b/src/main/java/fr/xephi/authme/listener/protocollib/InventoryPacketAdapter.java @@ -27,8 +27,7 @@ import com.comphenix.protocol.reflect.StructureModifier; import fr.xephi.authme.AuthMe; import fr.xephi.authme.ConsoleLogger; -import fr.xephi.authme.data.auth.PlayerCache; -import fr.xephi.authme.datasource.DataSource; +import fr.xephi.authme.listener.ListenerService; import fr.xephi.authme.output.ConsoleLoggerFactory; import fr.xephi.authme.service.BukkitService; @@ -51,17 +50,12 @@ class InventoryPacketAdapter extends PacketAdapter { private static final int HOTBAR_SIZE = 9; private final ConsoleLogger logger = ConsoleLoggerFactory.get(InventoryPacketAdapter.class); - private final PlayerCache playerCache; - private final DataSource dataSource; - private final ProtocolLibService protocolLibService; + private final ListenerService listenerService; - InventoryPacketAdapter(AuthMe plugin, PlayerCache playerCache, DataSource dataSource, - ProtocolLibService protocolLibService) { + InventoryPacketAdapter(AuthMe plugin, ListenerService listenerService) { super(plugin, PacketType.Play.Server.SET_SLOT, PacketType.Play.Server.WINDOW_ITEMS); - this.playerCache = playerCache; - this.dataSource = dataSource; - this.protocolLibService = protocolLibService; + this.listenerService = listenerService; } @Override @@ -70,7 +64,7 @@ class InventoryPacketAdapter extends PacketAdapter { PacketContainer packet = packetEvent.getPacket(); int windowId = packet.getIntegers().read(0); - if (windowId == PLAYER_INVENTORY && protocolLibService.shouldRestrictPlayer(player.getName())) { + if (windowId == PLAYER_INVENTORY && listenerService.shouldRestrictPlayer(player.getName())) { packetEvent.setCancelled(true); } } @@ -84,7 +78,7 @@ class InventoryPacketAdapter extends PacketAdapter { ProtocolLibrary.getProtocolManager().addPacketListener(this); bukkitService.getOnlinePlayers().stream() - .filter(player -> protocolLibService.shouldRestrictPlayer(player.getName())) + .filter(player -> listenerService.shouldRestrictPlayer(player.getName())) .forEach(this::sendBlankInventoryPacket); } diff --git a/src/main/java/fr/xephi/authme/listener/protocollib/ProtocolLibService.java b/src/main/java/fr/xephi/authme/listener/protocollib/ProtocolLibService.java index 20695e97b..01d682ab3 100644 --- a/src/main/java/fr/xephi/authme/listener/protocollib/ProtocolLibService.java +++ b/src/main/java/fr/xephi/authme/listener/protocollib/ProtocolLibService.java @@ -6,10 +6,10 @@ import fr.xephi.authme.ConsoleLogger; import fr.xephi.authme.data.auth.PlayerCache; import fr.xephi.authme.datasource.DataSource; import fr.xephi.authme.initialization.SettingsDependent; +import fr.xephi.authme.listener.ListenerService; import fr.xephi.authme.output.ConsoleLoggerFactory; import fr.xephi.authme.service.BukkitService; import fr.xephi.authme.settings.Settings; -import fr.xephi.authme.settings.properties.RegistrationSettings; import fr.xephi.authme.settings.properties.RestrictionSettings; import org.bukkit.entity.Player; @@ -27,22 +27,21 @@ public class ProtocolLibService implements SettingsDependent { /* Settings */ private boolean protectInvBeforeLogin; private boolean denyTabCompleteBeforeLogin; - private boolean isRegistrationForced; /* Service */ private boolean isEnabled; private final AuthMe plugin; private final BukkitService bukkitService; + private final ListenerService listenerService; private final PlayerCache playerCache; - private final DataSource dataSource; @Inject - ProtocolLibService(AuthMe plugin, Settings settings, BukkitService bukkitService, PlayerCache playerCache, - DataSource dataSource) { + ProtocolLibService(AuthMe plugin, Settings settings, BukkitService bukkitService, ListenerService listenerService, + PlayerCache playerCache) { this.plugin = plugin; this.bukkitService = bukkitService; + this.listenerService = listenerService; this.playerCache = playerCache; - this.dataSource = dataSource; reload(settings); } @@ -68,7 +67,7 @@ public class ProtocolLibService implements SettingsDependent { if (protectInvBeforeLogin) { if (inventoryPacketAdapter == null) { // register the packet listener and start hiding it for all already online players (reload) - inventoryPacketAdapter = new InventoryPacketAdapter(plugin, playerCache, dataSource, this); + inventoryPacketAdapter = new InventoryPacketAdapter(plugin, listenerService); inventoryPacketAdapter.register(bukkitService); } } else if (inventoryPacketAdapter != null) { @@ -78,7 +77,7 @@ public class ProtocolLibService implements SettingsDependent { if (denyTabCompleteBeforeLogin) { if (tabCompletePacketAdapter == null) { - tabCompletePacketAdapter = new TabCompletePacketAdapter(plugin, this); + tabCompletePacketAdapter = new TabCompletePacketAdapter(plugin, listenerService); tabCompletePacketAdapter.register(); } } else if (tabCompletePacketAdapter != null) { @@ -116,34 +115,10 @@ public class ProtocolLibService implements SettingsDependent { } } - /** - * Should the given player need to be restricted - * - * @param playerName player that is about to prevented to do or see something - * @return true if restriction is necessary - */ - protected boolean shouldRestrictPlayer(String playerName) { - if (playerCache.isAuthenticated(playerName)) { - // fully logged in - no need to protect it - return false; - } - - if (dataSource.isCached()) { - // load from cache or only request once - return dataSource.isAuthAvailable(playerName); - } - - // data source is not cached - this means queries would run blocking - // If registration is enforced: **assume** player is registered to prevent any information leak - // If not, players could play even without a registration, so there is no need for protection - return isRegistrationForced; - } - @Override public void reload(Settings settings) { boolean oldProtectInventory = this.protectInvBeforeLogin; - this.isRegistrationForced = settings.getProperty(RegistrationSettings.FORCE); this.denyTabCompleteBeforeLogin = settings.getProperty(RestrictionSettings.DENY_TABCOMPLETE_BEFORE_LOGIN); this.protectInvBeforeLogin = settings.getProperty(RestrictionSettings.PROTECT_INVENTORY_BEFORE_LOGIN); diff --git a/src/main/java/fr/xephi/authme/listener/protocollib/TabCompletePacketAdapter.java b/src/main/java/fr/xephi/authme/listener/protocollib/TabCompletePacketAdapter.java index f36d06b9c..4a67f550e 100644 --- a/src/main/java/fr/xephi/authme/listener/protocollib/TabCompletePacketAdapter.java +++ b/src/main/java/fr/xephi/authme/listener/protocollib/TabCompletePacketAdapter.java @@ -8,17 +8,18 @@ import com.comphenix.protocol.events.PacketEvent; import com.comphenix.protocol.reflect.FieldAccessException; import fr.xephi.authme.AuthMe; import fr.xephi.authme.ConsoleLogger; +import fr.xephi.authme.listener.ListenerService; import fr.xephi.authme.output.ConsoleLoggerFactory; class TabCompletePacketAdapter extends PacketAdapter { private final ConsoleLogger logger = ConsoleLoggerFactory.get(TabCompletePacketAdapter.class); - private final ProtocolLibService protocolLibService; + private final ListenerService listenerService; - TabCompletePacketAdapter(AuthMe plugin, ProtocolLibService protocolLibService) { + TabCompletePacketAdapter(AuthMe plugin, ListenerService listenerService) { super(plugin, ListenerPriority.NORMAL, PacketType.Play.Client.TAB_COMPLETE); - this.protocolLibService = protocolLibService; + this.listenerService = listenerService; } @Override @@ -26,7 +27,7 @@ class TabCompletePacketAdapter extends PacketAdapter { if (event.getPacketType() == PacketType.Play.Client.TAB_COMPLETE) { try { String playerName = event.getPlayer().getName(); - if (protocolLibService.shouldRestrictPlayer(playerName)) { + if (listenerService.shouldRestrictPlayer(playerName)) { event.setCancelled(true); } } catch (FieldAccessException e) { diff --git a/src/main/java/fr/xephi/authme/process/join/AsynchronousJoin.java b/src/main/java/fr/xephi/authme/process/join/AsynchronousJoin.java index 7b16e564a..d715759cf 100644 --- a/src/main/java/fr/xephi/authme/process/join/AsynchronousJoin.java +++ b/src/main/java/fr/xephi/authme/process/join/AsynchronousJoin.java @@ -2,6 +2,8 @@ package fr.xephi.authme.process.join; import fr.xephi.authme.ConsoleLogger; import fr.xephi.authme.data.ProxySessionManager; +import fr.xephi.authme.data.auth.PlayerCache; +import fr.xephi.authme.data.auth.PlayerCache.RegistrationStatus; import fr.xephi.authme.data.limbo.LimboService; import fr.xephi.authme.datasource.DataSource; import fr.xephi.authme.events.ProtectInventoryEvent; @@ -73,6 +75,9 @@ public class AsynchronousJoin implements AsynchronousProcess { @Inject private SessionService sessionService; + @Inject + private PlayerCache playerCache; + @Inject private ProxySessionManager proxySessionManager; @@ -112,7 +117,8 @@ public class AsynchronousJoin implements AsynchronousProcess { } final boolean isAuthAvailable = database.isAuthAvailable(name); - + RegistrationStatus status = RegistrationStatus.UNREGISTERED; + playerCache.addRegistrationStatus(name, isAuthAvailable ? RegistrationStatus.REGISTERED : status); if (isAuthAvailable) { // Protect inventory if (service.getProperty(PROTECT_INVENTORY_BEFORE_LOGIN)) { diff --git a/src/main/java/fr/xephi/authme/service/TeleportationService.java b/src/main/java/fr/xephi/authme/service/TeleportationService.java index f0eb7f851..ef84f2a41 100644 --- a/src/main/java/fr/xephi/authme/service/TeleportationService.java +++ b/src/main/java/fr/xephi/authme/service/TeleportationService.java @@ -3,6 +3,7 @@ package fr.xephi.authme.service; import fr.xephi.authme.ConsoleLogger; import fr.xephi.authme.data.auth.PlayerAuth; import fr.xephi.authme.data.auth.PlayerCache; +import fr.xephi.authme.data.auth.PlayerCache.RegistrationStatus; import fr.xephi.authme.data.limbo.LimboPlayer; import fr.xephi.authme.datasource.DataSource; import fr.xephi.authme.events.AbstractTeleportEvent; @@ -112,7 +113,7 @@ public class TeleportationService implements Reloadable { return; } - if (!player.hasPlayedBefore() || !dataSource.isAuthAvailable(player.getName())) { + if (!player.hasPlayedBefore() || playerCache.getRegistrationStatus(player.getName()) == RegistrationStatus.UNREGISTERED) { logger.debug("Attempting to teleport player `{0}` to first spawn", player.getName()); performTeleportation(player, new FirstSpawnTeleportEvent(player, firstSpawn)); } diff --git a/src/test/java/fr/xephi/authme/listener/ListenerServiceTest.java b/src/test/java/fr/xephi/authme/listener/ListenerServiceTest.java index 76877fcac..6e71ab63d 100644 --- a/src/test/java/fr/xephi/authme/listener/ListenerServiceTest.java +++ b/src/test/java/fr/xephi/authme/listener/ListenerServiceTest.java @@ -130,7 +130,7 @@ public class ListenerServiceTest { // then assertThat(result, equalTo(false)); verify(playerCache).isAuthenticated(playerName); - verify(dataSource).isAuthAvailable(playerName); + verify(playerCache).getRegistrationStatus(playerName); } @Test @@ -154,10 +154,9 @@ public class ListenerServiceTest { public void shouldAllowNpcPlayer() { // given String playerName = "other_npc"; - Player player = mockPlayerWithName(playerName); + Player player = mockPlayerWithName(playerName, true); EntityEvent event = mock(EntityEvent.class); given(event.getEntity()).willReturn(player); - given(player.hasMetadata("NPC")).willReturn(true); // when boolean result = listenerService.shouldCancelEvent(event); @@ -214,8 +213,13 @@ public class ListenerServiceTest { } private static Player mockPlayerWithName(String name) { + return mockPlayerWithName(name,false); + } + + private static Player mockPlayerWithName(String name, boolean npc) { Player player = mock(Player.class); given(player.getName()).willReturn(name); + given(player.hasMetadata("NPC")).willReturn(npc); return player; }