Merge pull request #3137 from Multiverse/ben/mv5/command-permissions

Implement more customisable way of doing permission checking

src/main/java/org/mvplugins/multiverse/core/commands/SpawnCommand.java

@@ -12,6 +12,7 @@
 import org.mvplugins.multiverse.core.commandtools.MVCommandIssuer;
 import org.mvplugins.multiverse.core.commandtools.MVCommandManager;
 import org.mvplugins.multiverse.core.commandtools.MultiverseCommand;
+import org.mvplugins.multiverse.core.permissions.CorePermissionsChecker;
 import org.mvplugins.multiverse.core.teleportation.AsyncSafetyTeleporter;
 import org.mvplugins.multiverse.core.utils.MVCorei18n;
 import org.mvplugins.multiverse.core.world.LoadedMultiverseWorld;
@@ -22,18 +23,22 @@
 class SpawnCommand extends CoreCommand {
     private final WorldManager worldManager;
     private final AsyncSafetyTeleporter safetyTeleporter;
+    private final CorePermissionsChecker permissionsChecker;
 
     @Inject
     SpawnCommand(@NotNull MVCommandManager commandManager,
                  @NotNull WorldManager worldManager,
-                 @NotNull AsyncSafetyTeleporter safetyTeleporter) {
+                 @NotNull AsyncSafetyTeleporter safetyTeleporter,
+                 @NotNull CorePermissionsChecker permissionsChecker) {
         super(commandManager);
         this.worldManager = worldManager;
         this.safetyTeleporter = safetyTeleporter;
+        this.permissionsChecker = permissionsChecker;
     }
 
     @CommandAlias("mvspawn")
     @Subcommand("spawn")
+    @CommandPermission("@mvspawn")
     @CommandCompletion("@players")
     @Syntax("[player]")
     @Description("{@@mv-core.spawn.description}")
@@ -44,20 +49,18 @@ void onSpawnTpCommand(
             @Syntax("[player]")
             @Description("{@@mv-core.spawn.player.description}")
             Player player) {
-        // TODO: Better handling of permission checking with CorePermissionsChecker
-        String permission = player.equals(issuer.getPlayer()) ? "multiverse.core.spawn.self" : "multiverse.core.spawn.other";
-        if (!issuer.hasPermission(permission)) {
-            issuer.sendMessage("You do not have permission to use this command!");
-            return;
-        }
-
         LoadedMultiverseWorld world = worldManager.getLoadedWorld(player.getWorld()).getOrNull();
         if (world == null) {
             issuer.sendMessage("The world the player you are trying to teleport is in, is not a multiverse world");
             return;
         }
 
-        // Teleport the player
+        if (!permissionsChecker.hasSpawnPermission(issuer.getIssuer(), player, world)) {
+            issuer.sendMessage("You do not have permission to use this command in this world!");
+            return;
+        }
+
+        // Teleport the player to spawn
         // TODO: Different message for teleporting self vs others
         safetyTeleporter.teleportSafely(issuer.getIssuer(), player, world.getSpawnLocation())
                 .onSuccess(() -> player.sendMessage(commandManager.formatMessage(
@@ -88,10 +91,4 @@ private String getTeleporterName(BukkitCommandIssuer issuer, Player teleportTo)
         }
         return issuer.getIssuer().getName();
     }
-
-    @Override
-    public boolean hasPermission(CommandIssuer issuer) {
-        // TODO: Fix autocomplete showing even if the player doesn't have permission
-        return issuer.hasPermission("multiverse.core.spawn.self") || issuer.hasPermission("multiverse.core.spawn.other");
-    }
 }

src/main/java/org/mvplugins/multiverse/core/commands/TeleportCommand.java

@@ -2,9 +2,9 @@
 
 import java.util.List;
 
-import co.aikar.commands.CommandIssuer;
 import co.aikar.commands.annotation.CommandAlias;
 import co.aikar.commands.annotation.CommandCompletion;
+import co.aikar.commands.annotation.CommandPermission;
 import co.aikar.commands.annotation.Description;
 import co.aikar.commands.annotation.Flags;
 import co.aikar.commands.annotation.Subcommand;
@@ -41,6 +41,7 @@ class TeleportCommand extends CoreCommand {
 
     @CommandAlias("mvtp")
     @Subcommand("teleport|tp")
+    @CommandPermission("@mvteleport")
     @CommandCompletion("@players|@mvworlds:playerOnly|@destinations:playerOnly @mvworlds|@destinations")
     @Syntax("[player] <destination>")
     @Description("{@@mv-core.teleport.description}")
@@ -77,9 +78,4 @@ void onTeleportCommand(
                             playerName, destination, throwable.getMessage());
                 });
     }
-
-    @Override
-    public boolean hasPermission(CommandIssuer issuer) {
-        return permissionsChecker.hasAnyTeleportPermission(issuer.getIssuer());
-    }
 }

src/main/java/org/mvplugins/multiverse/core/commandtools/MVCommandManager.java

@@ -2,11 +2,10 @@
 
 import java.util.List;
 
-import co.aikar.commands.BukkitCommandCompletionContext;
-import co.aikar.commands.BukkitCommandExecutionContext;
 import co.aikar.commands.CommandCompletions;
 import co.aikar.commands.CommandContexts;
 import co.aikar.commands.CommandHelp;
+import co.aikar.commands.CommandIssuer;
 import co.aikar.commands.HelpEntry;
 import co.aikar.commands.PaperCommandManager;
 import jakarta.inject.Inject;
@@ -32,6 +31,7 @@ public class MVCommandManager extends PaperCommandManager {
     private final CommandQueueManager commandQueueManager;
     private final Provider<MVCommandContexts> commandContextsProvider;
     private final Provider<MVCommandCompletions> commandCompletionsProvider;
+    private final MVCommandPermissions commandPermissions;
 
     @Inject
     MVCommandManager(
@@ -41,12 +41,14 @@ public class MVCommandManager extends PaperCommandManager {
             @NotNull Provider<MVCommandContexts> commandContextsProvider,
             @NotNull Provider<MVCommandCompletions> commandCompletionsProvider,
             @NotNull WorldManager worldManager,
-            @NotNull WorldNameChecker worldNameChecker) {
+            @NotNull WorldNameChecker worldNameChecker,
+            @NotNull MVCommandPermissions commandPermissions) {
         super(plugin);
         this.flagsManager = flagsManager;
         this.commandQueueManager = commandQueueManager;
         this.commandContextsProvider = commandContextsProvider;
         this.commandCompletionsProvider = commandCompletionsProvider;
+        this.commandPermissions = commandPermissions;
 
         MVCommandConditions.load(this, worldManager, worldNameChecker);
         this.enableUnstableAPI("help");
@@ -85,6 +87,10 @@ public PluginLocales getLocales() {
         return commandQueueManager;
     }
 
+    public synchronized @NotNull MVCommandPermissions getCommandPermissions() {
+        return commandPermissions;
+    }
+
     /**
      * Gets class responsible for parsing string args into objects.
      *
@@ -111,6 +117,11 @@ public PluginLocales getLocales() {
         return (MVCommandCompletions) this.completions;
     }
 
+    @Override
+    public boolean hasPermission(CommandIssuer issuer, String permission) {
+        return commandPermissions.hasPermission(issuer, permission);
+    }
+
     /**
      * Standardise usage command formatting for all mv modules.
      *

src/main/java/org/mvplugins/multiverse/core/commandtools/MVCommandPermissions.java

@@ -0,0 +1,57 @@
+package org.mvplugins.multiverse.core.commandtools;
+
+import co.aikar.commands.CommandIssuer;
+import co.aikar.commands.annotation.CommandPermission;
+import io.vavr.control.Option;
+import jakarta.inject.Inject;
+import org.jetbrains.annotations.NotNull;
+import org.jvnet.hk2.annotations.Service;
+import org.mvplugins.multiverse.core.permissions.CorePermissionsChecker;
+
+import java.util.HashMap;
+import java.util.Locale;
+import java.util.Map;
+import java.util.function.Predicate;
+
+/**
+ * Maps permission checking to custom logic for commands, to allow more complex permission checking.
+ */
+@Service
+public class MVCommandPermissions {
+    private final Map<String, Predicate<CommandIssuer>> permissionsCheckMap;
+
+    @Inject
+    MVCommandPermissions(@NotNull CorePermissionsChecker permissionsChecker) {
+        permissionsCheckMap = new HashMap<>();
+
+        registerPermissionChecker("mvteleport", issuer -> permissionsChecker.hasAnyTeleportPermission(issuer.getIssuer()));
+        registerPermissionChecker("mvspawn", issuer -> permissionsChecker.hasAnySpawnPermission(issuer.getIssuer()));
+    }
+
+    /**
+     * Registers a custom permission checker callback. Use `@id-name` in {@link CommandPermission} decorator to use
+     * the callback instead of the default permission string checking.
+     *
+     * @param id         The permission id
+     * @param checker    The permission checker callback
+     */
+    public void registerPermissionChecker(String id, Predicate<CommandIssuer> checker) {
+        permissionsCheckMap.put(prepareId(id), checker);
+    }
+
+    private static @NotNull String prepareId(String id) {
+        return (id.startsWith("@") ? "" : "@") + id.toLowerCase(Locale.ENGLISH);
+    }
+
+    /**
+     * Check if the issuer has the given permission.
+     * @param issuer        The issuer
+     * @param permission    The permission to check
+     * @return True if the issuer has the permission
+     */
+    boolean hasPermission(CommandIssuer issuer, String permission) {
+        return Option.of(permissionsCheckMap.get(permission))
+                .map(checker -> checker.test(issuer))
+                .getOrElse(() -> issuer.hasPermission(permission));
+    }
+}

src/main/java/org/mvplugins/multiverse/core/permissions/CorePermissions.java

@@ -26,6 +26,11 @@ final class CorePermissions {
      */
     static final String TELEPORT = "multiverse.teleport";
 
+    /**
+     * Permission to teleport to spawn in a world.
+     */
+    static final String SPAWN = "multiverse.core.spawn";
+
     private CorePermissions() {
         // Prevent instantiation as this is a static utility class
     }

src/main/java/org/mvplugins/multiverse/core/permissions/CorePermissionsChecker.java

@@ -39,6 +39,39 @@ public boolean hasGameModeBypassPermission(@NotNull CommandSender sender, @NotNu
         return hasPermission(sender, concatPermission(CorePermissions.GAMEMODE_BYPASS, world.getName()));
     }
 
+    /**
+     * Checks if the teleporter has permission to teleport the teleportee to the world's spawnpoint.
+     *
+     * @param teleporter    The teleporter.
+     * @param teleportee    The teleportee.
+     * @param world         The world.
+     * @return True if the teleporter has permission, false otherwise.
+     */
+    public boolean hasSpawnPermission(
+            @NotNull CommandSender teleporter,
+            @NotNull Entity teleportee,
+            @NotNull LoadedMultiverseWorld world) {
+        String permission = concatPermission(CorePermissions.SPAWN, teleportee.equals(teleporter) ? "self" : "other");
+        if (!hasPermission(teleporter, permission)) {
+            return false;
+        }
+        // TODO: Config whether to use finer permission
+        return hasPermission(teleporter, concatPermission(permission, world.getName()));
+    }
+
+    /**
+     * Check if the issuer has any base spawn permission of `multiverse.core.spawn.self` or `multiverse.core.spawn.other`
+     *
+     * @param sender    The sender that ran the command
+     * @return True if the sender has any base spawn permission.
+     */
+    public boolean hasAnySpawnPermission(@NotNull CommandSender sender) {
+        if (hasPermission(sender, concatPermission(CorePermissions.SPAWN, "self"))) {
+            return true;
+        }
+        return hasPermission(sender, concatPermission(CorePermissions.SPAWN, "other"));
+    }
+
     /**
      * Checks if the teleporter has permission to teleport the teleportee to the destination.
      *