Skip to content

Commit

Permalink
Fixed a bug causing internal errors when tab completing. Improved aut…
Browse files Browse the repository at this point in the history
…h communication
  • Loading branch information
LielAmar committed Mar 9, 2022
1 parent f090180 commit 0c8ffb0
Show file tree
Hide file tree
Showing 21 changed files with 87 additions and 35 deletions.
4 changes: 2 additions & 2 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

<groupId>com.lielamar</groupId>
<artifactId>2fa</artifactId>
<version>1.6.0</version>
<version>1.6.1</version>

<properties>
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
Expand Down Expand Up @@ -132,7 +132,7 @@
<dependency>
<groupId>com.lielamar</groupId>
<artifactId>LielsUtils</artifactId>
<version>1.6.27</version>
<version>1.6.28</version>
<scope>compile</scope>
</dependency>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ public void setupAuth() {
this.storageHandler = StorageHandler.loadStorageHandler(this.configHandler, getDataFolder().getAbsolutePath());

AuthCommunicationHandler authCommunicationHandler;

{
if(this.configHandler.getCommunicationMethod() == CommunicationMethod.PROXY) {
authCommunicationHandler = new ProxyAuthCommunication(this);
Expand All @@ -106,7 +107,7 @@ public void setupAuth() {
}
}

this.authHandler = new AuthHandler(this, storageHandler, authCommunicationHandler);
this.authHandler = new AuthHandler(this, storageHandler, authCommunicationHandler, new BasicAuthCommunication(this));
this.authTracker = new AuthTracker();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public boolean runCommand(@NotNull CommandSender commandSender, @NotNull String[
}

@Override
public List<String> tabOptions(@NotNull CommandSender commandSender, @NotNull String[] strings) {
public List<String> tabOptions(@NotNull CommandSender commandSender, @NotNull String[] args) {
return new ArrayList<>();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,8 @@ public boolean runCommand(@NotNull CommandSender commandSender, @NotNull String[
}

@Override
public List<String> tabOptions(@NotNull CommandSender commandSender, @NotNull String[] strings) {
return new TabOptionsBuilder().player().build(new String[0]);
public List<String> tabOptions(@NotNull CommandSender commandSender, @NotNull String[] args) {
return new TabOptionsBuilder().player().build(args);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,8 @@ public boolean runCommand(@NotNull CommandSender commandSender, @NotNull String[
}

@Override
public List<String> tabOptions(@NotNull CommandSender commandSender, @NotNull String[] strings) {
return new TabOptionsBuilder().player().build(new String[0]);
public List<String> tabOptions(@NotNull CommandSender commandSender, @NotNull String[] args) {
return new TabOptionsBuilder().player().build(args);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,8 @@ public boolean runCommand(@NotNull CommandSender commandSender, @NotNull String[
}

@Override
public List<String> tabOptions(@NotNull CommandSender commandSender, @NotNull String[] strings) {
return new TabOptionsBuilder().player().build(new String[0]);
public List<String> tabOptions(@NotNull CommandSender commandSender, @NotNull String[] args) {
return new TabOptionsBuilder().player().build(args);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ public boolean runCommand(@NotNull CommandSender commandSender, @NotNull String[
}

@Override
public List<String> tabOptions(@NotNull CommandSender commandSender, @NotNull String[] strings) {
public List<String> tabOptions(@NotNull CommandSender commandSender, @NotNull String[] args) {
return new ArrayList<>();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ public boolean runCommand(@NotNull CommandSender commandSender, @NotNull String[
}

@Override
public List<String> tabOptions(@NotNull CommandSender commandSender, @NotNull String[] strings) {
public List<String> tabOptions(@NotNull CommandSender commandSender, @NotNull String[] args) {
return new ArrayList<>();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ public boolean runCommand(@NotNull CommandSender commandSender, @NotNull String[
}

@Override
public List<String> tabOptions(@NotNull CommandSender commandSender, @NotNull String[] strings) {
public List<String> tabOptions(@NotNull CommandSender commandSender, @NotNull String[] args) {
return new ArrayList<>();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ public boolean runCommand(@NotNull CommandSender commandSender, @NotNull String[
}

@Override
public List<String> tabOptions(@NotNull CommandSender commandSender, @NotNull String[] strings) {
public List<String> tabOptions(@NotNull CommandSender commandSender, @NotNull String[] args) {
return new ArrayList<>();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ public boolean runCommand(@NotNull CommandSender commandSender, @NotNull String[
}

@Override
public List<String> tabOptions(@NotNull CommandSender commandSender, @NotNull String[] strings) {
public List<String> tabOptions(@NotNull CommandSender commandSender, @NotNull String[] args) {
return new ArrayList<>();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,9 @@ public BasicAuthCommunication(TwoFactorAuthentication plugin) {
long timeout = this.plugin.getConfigHandler().getCommunicationTimeout();

// Timeouts all callbacks that were set more than ${timeout} seconds ago using #onTimeout
Bukkit.getScheduler().runTaskTimerAsynchronously(plugin, () -> {
Bukkit.getScheduler().scheduleSyncRepeatingTask(plugin, () -> {
long currentTimestamp = System.currentTimeMillis();

super.callbacks.entrySet().removeIf(entry -> {
if(((currentTimestamp - entry.getValue().getExecutionStamp()) / 1000) > (timeout / 20)) {
entry.getValue().onTimeout();
Expand Down
44 changes: 33 additions & 11 deletions src/main/java/com/lielamar/auth/bukkit/handlers/AuthHandler.java
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@
import java.util.Arrays;
import java.util.HashMap;
import java.util.Map;
import java.util.Objects;
import java.util.UUID;
import java.util.stream.Collectors;

public class AuthHandler extends com.lielamar.auth.shared.handlers.AuthHandler {

Expand All @@ -48,9 +48,11 @@ public class AuthHandler extends com.lielamar.auth.shared.handlers.AuthHandler {

protected Hash hash;

private int callbackTimeouts;

public AuthHandler(@NotNull TwoFactorAuthentication plugin, @Nullable StorageHandler storageHandler,
@Nullable AuthCommunicationHandler authCommunicationHandler) {
super(storageHandler, authCommunicationHandler);
@Nullable AuthCommunicationHandler authCommunicationHandler, @Nullable AuthCommunicationHandler fallbackCommunicationHandler) {
super(storageHandler, authCommunicationHandler, fallbackCommunicationHandler);

this.plugin = plugin;

Expand All @@ -63,6 +65,8 @@ public AuthHandler(@NotNull TwoFactorAuthentication plugin, @Nullable StorageHan
if(hashType.equalsIgnoreCase("SHA256")) this.hash = new SHA256();
else if(hashType.equalsIgnoreCase("SHA512")) this.hash = new SHA512();
else this.hash = new NoHash();

this.callbackTimeouts = 0;
}

public void reloadOnlinePlayers() {
Expand Down Expand Up @@ -139,8 +143,22 @@ public void playerJoin(@NotNull UUID uuid) {
if(super.authCommunicationHandler == null)
super.authCommunicationHandler = new BasicAuthCommunication(this.plugin);

// Checking whether we need to retrieve data on the user
Player player = Bukkit.getPlayer(uuid);

if(player == null || !player.isOnline())
return;

if(this.getStorageHandler() == null)
return;

if(!player.hasPermission("2fa.use") || this.getStorageHandler().getKey(uuid) == null) {
this.changeState(uuid, AuthState.DISABLED);
return;
}

// Setting the initial state so players can't abuse the brief moment without 2fa protection
changeState(uuid, AuthState.PENDING_LOGIN);
this.changeState(uuid, AuthState.PENDING_LOGIN);

// Asking communication handler to load the player state and execute LoadAuthCallback when a result is given
super.authCommunicationHandler.loadPlayerState(uuid, new LoadAuthCallback(uuid));
Expand Down Expand Up @@ -335,16 +353,11 @@ public void execute(AuthState authState) {
if(player == null || !player.isOnline())
return;

if(!player.hasPermission("2fa.use")) {
changeState(playerUUID, AuthState.DISABLED);
return;
}

if(getStorageHandler() == null)
return;

// If AuthCommunication's result returned that the player is already authenticated, we don't need to continue
if(authState == AuthState.AUTHENTICATED) {
if(authState == AuthState.AUTHENTICATED || authState == AuthState.NONE) {
changeState(this.playerUUID, authState);
return;
}
Expand All @@ -370,7 +383,16 @@ public void execute(AuthState authState) {

@Override
public void onTimeout() {
execute(AuthState.NONE);
callbackTimeouts++;
if(callbackTimeouts > 10) {
callbackTimeouts = 0;

Bukkit.getOnlinePlayers().stream().filter(pl -> pl.hasPermission("2fa.alerts")).forEach(pl -> {
plugin.getMessageHandler().sendMessage(pl, MessageHandler.TwoFAMessages.COMMUNICATION_METHOD_NOT_CORRECT);
});
}

fallbackCommunicationHandler.loadPlayerState(playerUUID, this);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import org.bukkit.event.block.BlockPlaceEvent;
import org.bukkit.event.entity.EntityDamageByEntityEvent;
import org.bukkit.event.entity.EntityDamageEvent;
import org.bukkit.event.entity.PlayerDeathEvent;
import org.bukkit.event.inventory.InventoryClickEvent;
import org.bukkit.event.inventory.InventoryMoveItemEvent;
import org.bukkit.event.player.*;
Expand Down Expand Up @@ -51,7 +52,7 @@ public ConfigHandler(FileManager fileManager) {
protected Location tpAfterAuthLocation = null;

protected CommunicationMethod communicationMethod = CommunicationMethod.PROXY;
protected int communicationTimeout = 100;
protected int communicationTimeout = 30;


public boolean shouldCheckForUpdates() { return this.checkForUpdates; }
Expand Down Expand Up @@ -409,7 +410,8 @@ protected enum ShorterEvents {
CHANGE_SLOT(PlayerItemHeldEvent.class),
COMMANDS(PlayerCommandPreprocessEvent.class),
MOVE_ITEM(InventoryMoveItemEvent.class),
INTERACT_WITH_FRAMES(PlayerInteractEntityEvent.class);
INTERACT_WITH_FRAMES(PlayerInteractEntityEvent.class),
DEATH(PlayerDeathEvent.class);

private final Class<? extends Event> matchingEvent;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import org.bukkit.event.block.BlockPlaceEvent;
import org.bukkit.event.entity.EntityDamageByEntityEvent;
import org.bukkit.event.entity.EntityDamageEvent;
import org.bukkit.event.entity.EntityPickupItemEvent;
import org.bukkit.event.entity.PlayerDeathEvent;
import org.bukkit.event.inventory.InventoryClickEvent;
import org.bukkit.event.inventory.InventoryMoveItemEvent;
Expand Down Expand Up @@ -91,6 +92,19 @@ public void onItemPickup(PlayerPickupItemEvent event) {
}
}

@EventHandler (priority = EventPriority.HIGHEST)
public void onItemPickup(EntityPickupItemEvent event) {
if(!(event.getEntity() instanceof Player)) return;

if(!this.plugin.getConfigHandler().getDisabledEvents().getOrDefault(event.getClass(), true)) return;

if(this.plugin.getAuthHandler().needsToAuthenticate(event.getEntity().getUniqueId())) {
event.setCancelled(true);
} else if(this.plugin.getAuthHandler().isQRCodeItem(event.getItem().getItemStack())) {
event.getItem().remove();
}
}

@EventHandler (priority = EventPriority.HIGHEST)
public void onEntityDamage(EntityDamageEvent event) {
if(!this.plugin.getConfigHandler().getDisabledEvents().getOrDefault(event.getClass(), true)) return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ public abstract class AuthHandler {
private final Map<UUID, Integer> failedAttempts;

protected StorageHandler storageHandler;
protected AuthCommunicationHandler authCommunicationHandler;
protected AuthCommunicationHandler authCommunicationHandler, fallbackCommunicationHandler;
protected HashMap<UUID, AuthState> authStates;


Expand All @@ -26,11 +26,17 @@ public AuthHandler() {
}

public AuthHandler(@Nullable StorageHandler storageHandler, @Nullable AuthCommunicationHandler authCommunicationHandler) {
this(storageHandler, authCommunicationHandler, null);
}

public AuthHandler(@Nullable StorageHandler storageHandler, @Nullable AuthCommunicationHandler authCommunicationHandler,
@Nullable AuthCommunicationHandler fallbackCommunicationHandler) {
this.pendingKeys = new HashMap<>();
this.failedAttempts = new HashMap<>();

this.storageHandler = storageHandler;
this.authCommunicationHandler = authCommunicationHandler;
this.fallbackCommunicationHandler = fallbackCommunicationHandler;

this.authStates = new HashMap<>();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,8 @@ public enum TwoFAMessages {
RELOADED_CONFIG("&aConfig was reloaded"),
FAILED_AUTHENTICATION_ALERT("&c%name% failed to authenticate %times% times"),
SOMETHING_WENT_WRONG("&cSomething went wrong. Please contact a Staff Member!"),
COMMUNICATION_METHOD_NOT_CORRECT("&cWe have noticed that a lot of the requests 2FA make between servers are being timed out." +
"\nPlease make sure Communication-Method is set to the best option for your server in the configuration!"),
KEYWORD_ENABLED("Enabled"),
KEYWORD_DISABLED("Disabled"),
KEYWORD_REQUIRED("Required"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@

import java.nio.file.Path;

@Plugin(id = "twofa", name = "2FA", version = "1.6.0", description = "Add another layer of protection to your server", authors = { "Liel Amar", "SadGhost"})
@Plugin(id = "twofa", name = "2FA", version = "1.6.1", description = "Add another layer of protection to your server", authors = { "Liel Amar", "SadGhost"})
public class TwoFactorAuthentication implements TwoFactorAuthenticationPlugin {

private final ProxyServer proxy;
Expand Down
2 changes: 1 addition & 1 deletion src/main/resources/bungee.yml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
name: 2FA
version: "1.6.0"
version: "1.6.1"
author: "LielAmar"
main: com.lielamar.auth.bungee.TwoFactorAuthentication
description: Add another layer of protection to your server
3 changes: 2 additions & 1 deletion src/main/resources/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ tp-after-auth:

# Possible methods for the plugin to communicate between servers
#
# - NONE (Only use NONE if you have a single server)
# - PROXY
# - REDIS (NOT SUPPORTED YET)
# - RABBITMQ (NOT SUPPORTED YET)
Expand All @@ -111,7 +112,7 @@ communication-method: PROXY
communication-data:
# How long (in ticks) should the plugin wait before timing out communication messages
# 1 second = 20 ticks
timeout: 100
timeout: 30

# Possible methods for the plugin to store data
#
Expand Down
7 changes: 5 additions & 2 deletions src/main/resources/plugin.yml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
name: 2FA
version: "1.6.0"
version: "1.6.1"
api-version: 1.13
authors: [LielAmar, SadGhost]
main: com.lielamar.auth.bukkit.TwoFactorAuthentication
Expand Down Expand Up @@ -32,6 +32,7 @@ permissions:
2fa.remove: true
2fa.remove.others: true
2fa.reload: true
2fa.alerts: true
2fa.use:
description: Permissions to use the /2FA command
default: op
Expand All @@ -49,4 +50,6 @@ permissions:
default: op
2fa.demand:
description: A player with this permission must have 2FA linked to thier account
default: false
default: false
2fa.alerts:
description: Admin permissions to get alerts on critical issues with 2FA configuration

0 comments on commit 0c8ffb0

Please sign in to comment.