fix: wrong permission check logic

WiIIiam278 · Mar 20, 2024 · 8a6d6b4 · 8a6d6b4
1 parent 3dc9292
commit 8a6d6b4
Show file tree

Hide file tree

Showing 25 changed files with 242 additions and 206 deletions.
diff --git a/bukkit/src/main/java/net/william278/huskchat/command/BukkitCommand.java b/bukkit/src/main/java/net/william278/huskchat/command/BukkitCommand.java
@@ -25,6 +25,7 @@
 import org.bukkit.command.Command;
 import org.bukkit.command.CommandSender;
 import org.bukkit.entity.Player;
+import org.bukkit.permissions.Permission;
 import org.jetbrains.annotations.NotNull;
 
 import java.util.Arrays;
@@ -39,11 +40,17 @@ public class BukkitCommand extends Command {
 
     public BukkitCommand(@NotNull CommandBase command, @NotNull BukkitHuskChat plugin) {
         super(command.getName(), command.getUsage(), command.getUsage(), command.getAliases());
+        this.setPermission(command.getPermission());
         this.command = command;
         this.plugin = plugin;
 
-        // Register with bukkit
+        // Register command & permission with Bukkit
         plugin.getCommandMap().register("huskchat", this);
+        if (getPermission() != null) {
+            plugin.getServer().getPluginManager().addPermission(new Permission(
+                    getPermission(), getUsage(), Permission.DEFAULT_PERMISSION)
+            );
+        }
     }
 
     @Override
@@ -60,12 +67,23 @@ public boolean execute(@NotNull CommandSender sender, @NotNull String commandLab
     @Override
     public List<String> tabComplete(@NotNull CommandSender sender, @NotNull String alias,
                                     @NotNull String[] args) throws IllegalArgumentException {
-        if (sender instanceof Player player && player.hasPermission(command.getPermission())) {
-            return command.onTabComplete(BukkitUser.adapt(player, plugin), args);
+        if (!(sender instanceof Player player)) {
+            return command.onTabComplete(plugin.getConsoleUser(), args);
+        }
+        final BukkitUser user = BukkitUser.adapt(player, plugin);
+        if (!user.hasPermission(command.getPermission(), !command.isOperatorOnly())) {
+            return List.of();
         }
-        return List.of();
+        return command.onTabComplete(user, args);
     }
 
+    @Override
+    public boolean testPermissionSilent(@NotNull CommandSender target) {
+        if (target instanceof Player player) {
+            return BukkitUser.adapt(player, plugin).hasPermission(command.getPermission(), !command.isOperatorOnly());
+        }
+        return true;
+    }
 
     public enum Type {
         HUSKCHAT((plugin) -> Optional.of(new BukkitCommand(new HuskChatCommand(plugin), plugin))),

diff --git a/bukkit/src/main/java/net/william278/huskchat/user/BukkitUser.java b/bukkit/src/main/java/net/william278/huskchat/user/BukkitUser.java
@@ -22,6 +22,7 @@
 import net.william278.huskchat.HuskChat;
 import org.bukkit.entity.Player;
 import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
 
 public class BukkitUser extends OnlineUser {
     private final Player player;
@@ -53,8 +54,12 @@ public int getPlayersOnServer() {
     }
 
     @Override
-    public boolean hasPermission(@NotNull String node) {
-        return player.hasPermission(node);
+    public boolean hasPermission(@Nullable String node, boolean allowByDefault) {
+        if (node != null && player.isPermissionSet(node)) {
+            return player.hasPermission(node);
+        } else {
+            return allowByDefault;
+        }
     }
 
     @NotNull

diff --git a/bungee/src/main/java/net/william278/huskchat/command/BungeeCommand.java b/bungee/src/main/java/net/william278/huskchat/command/BungeeCommand.java
@@ -50,18 +50,27 @@ public BungeeCommand(@NotNull CommandBase command, @NotNull BungeeHuskChat plugi
     @Override
     public void execute(CommandSender sender, String[] args) {
         if (sender instanceof ProxiedPlayer player) {
-            command.onExecute(BungeeUser.adapt(player, plugin), args);
+            final BungeeUser user = BungeeUser.adapt(player, plugin);
+            if (!user.hasPermission(command.getPermission(), !command.isOperatorOnly())) {
+                plugin.getLocales().sendMessage(user, "error_no_permission");
+                return;
+            }
+            command.onExecute(user, args);
         } else {
             command.onExecute(ConsoleUser.wrap(plugin), args);
         }
     }
 
     @Override
     public Iterable<String> onTabComplete(CommandSender sender, String[] args) {
-        if (sender instanceof ProxiedPlayer player && player.hasPermission(command.getPermission())) {
-            return command.onTabComplete(BungeeUser.adapt(player, plugin), args);
+        if (!(sender instanceof ProxiedPlayer player)) {
+            return command.onTabComplete(plugin.getConsoleUser(), args);
         }
-        return List.of();
+        final BungeeUser user = BungeeUser.adapt(player, plugin);
+        if (!user.hasPermission(command.getPermission(), !command.isOperatorOnly())) {
+            return List.of();
+        }
+        return command.onTabComplete(user, args);
     }
 
     public enum Type {

diff --git a/bungee/src/main/java/net/william278/huskchat/user/BungeeUser.java b/bungee/src/main/java/net/william278/huskchat/user/BungeeUser.java
@@ -22,6 +22,7 @@
 import net.md_5.bungee.api.connection.ProxiedPlayer;
 import net.william278.huskchat.HuskChat;
 import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
 
 /**
  * Bungee implementation of a cross-platform {@link OnlineUser}
@@ -63,8 +64,12 @@ public int getPlayersOnServer() {
     }
 
     @Override
-    public boolean hasPermission(String s) {
-        return player.hasPermission(s);
+    public boolean hasPermission(@Nullable String node, boolean allowByDefault) {
+        if (node != null && player.getPermissions().contains(node)) {
+            return player.hasPermission(node);
+        } else {
+            return allowByDefault;
+        }
     }
 
     @NotNull

diff --git a/common/src/main/java/net/william278/huskchat/channel/Channel.java b/common/src/main/java/net/william278/huskchat/channel/Channel.java
@@ -140,11 +140,11 @@ public boolean isServerRestricted(@NotNull String server) {
     }
 
     public boolean canUserSend(@NotNull OnlineUser user) {
-        return permissions.getSend().isEmpty() || user.hasPermission(permissions.getSend().get());
+        return permissions.getSend().map(node -> user.hasPermission(node, false)).orElse(true);
     }
 
     public boolean canUserReceive(@NotNull OnlineUser user) {
-        return permissions.getReceive().isEmpty() || user.hasPermission(permissions.getReceive().get());
+        return permissions.getReceive().map(node -> user.hasPermission(node, false)).orElse(true);
     }
 
 }
diff --git a/common/src/main/java/net/william278/huskchat/command/BroadcastCommand.java b/common/src/main/java/net/william278/huskchat/command/BroadcastCommand.java
@@ -31,22 +31,19 @@ public class BroadcastCommand extends CommandBase {
 
     public BroadcastCommand(@NotNull HuskChat plugin) {
         super(plugin.getSettings().getBroadcastCommand().getBroadcastAliases(), "<message>", plugin);
+        this.operatorOnly = true;
     }
 
     @Override
     public void onExecute(@NotNull OnlineUser player, @NotNull String[] args) {
-        if (player.hasPermission(getPermission())) {
-            if (args.length >= 1) {
-                StringJoiner message = new StringJoiner(" ");
-                for (String argument : args) {
-                    message.add(argument);
-                }
-                new BroadcastMessage(player, message.toString(), plugin).dispatch();
-            } else {
-                plugin.getLocales().sendMessage(player, "error_invalid_syntax", getUsage());
+        if (args.length >= 1) {
+            StringJoiner message = new StringJoiner(" ");
+            for (String argument : args) {
+                message.add(argument);
             }
+            new BroadcastMessage(player, message.toString(), plugin).dispatch();
         } else {
-            plugin.getLocales().sendMessage(player, "error_no_permission");
+            plugin.getLocales().sendMessage(player, "error_invalid_syntax", getUsage());
         }
     }
 

diff --git a/common/src/main/java/net/william278/huskchat/command/ChannelCommand.java b/common/src/main/java/net/william278/huskchat/command/ChannelCommand.java
@@ -42,22 +42,15 @@ public void onExecute(@NotNull OnlineUser player, @NotNull String[] args) {
             plugin.getLocales().sendMessage(player, "error_in_game_only");
             return;
         }
-        if (player.hasPermission(getPermission())) {
-            if (args.length == 1) {
-                plugin.getUserCache().switchPlayerChannel(player, args[0], plugin);
-            } else {
-                plugin.getLocales().sendMessage(player, "error_invalid_syntax", getUsage());
-            }
+        if (args.length == 1) {
+            plugin.getUserCache().switchPlayerChannel(player, args[0], plugin);
         } else {
-            plugin.getLocales().sendMessage(player, "error_no_permission");
+            plugin.getLocales().sendMessage(player, "error_invalid_syntax", getUsage());
         }
     }
 
     @Override
     public List<String> onTabComplete(@NotNull OnlineUser player, @NotNull String[] args) {
-        if (!player.hasPermission(getPermission())) {
-            return List.of();
-        }
         if (args.length <= 1) {
             return getUsableChannels(player).stream()
                     .filter(val -> val.toLowerCase().startsWith((args.length == 1) ? args[0].toLowerCase() : ""))
@@ -70,7 +63,7 @@ public List<String> onTabComplete(@NotNull OnlineUser player, @NotNull String[]
     @Unmodifiable
     public Set<String> getUsableChannels(@NotNull OnlineUser player) {
         return plugin.getChannels().getChannels().stream()
-                .filter(c -> c.getPermissions().getSend().map(player::hasPermission).orElse(true))
+                .filter(c -> c.canUserSend(player))
                 .map(Channel::getId)
                 .collect(Collectors.toSet());
     }

diff --git a/common/src/main/java/net/william278/huskchat/command/CommandBase.java b/common/src/main/java/net/william278/huskchat/command/CommandBase.java
@@ -19,9 +19,11 @@
 
 package net.william278.huskchat.command;
 
+import lombok.Getter;
 import net.william278.huskchat.HuskChat;
 import net.william278.huskchat.user.OnlineUser;
 import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
 
 import java.util.List;
 import java.util.Locale;
@@ -31,9 +33,11 @@
  */
 public abstract class CommandBase {
 
+    protected final HuskChat plugin;
     protected final List<String> aliases;
     protected final String usage;
-    protected final HuskChat plugin;
+    @Getter
+    protected boolean operatorOnly = false;
 
     public CommandBase(@NotNull List<String> aliases, @NotNull String usage, @NotNull HuskChat plugin) {
         if (aliases.isEmpty()) {
@@ -85,9 +89,12 @@ public String getUsage() {
     /**
      * Command permission node
      */
-    @NotNull
-    public String getPermission() {
-        return "huskchat.command." + getName();
+    @Nullable
+    public String getPermission(@NotNull String... children) {
+        return String.join(".",
+                "huskchat", "command", getName(),
+                String.join(".", children)
+        );
     }
 
 }
diff --git a/common/src/main/java/net/william278/huskchat/command/HuskChatCommand.java b/common/src/main/java/net/william278/huskchat/command/HuskChatCommand.java
@@ -63,15 +63,11 @@ public HuskChatCommand(@NotNull HuskChat plugin) {
                         AboutMenu.Link.of("https://github.com/WiIIiam278/HuskChat/issues").text("Issues").icon("❌").color(TextColor.color(0xff9f0f)),
                         AboutMenu.Link.of("https://discord.gg/tVYhJfyDWG").text("Discord").icon("⭐").color(TextColor.color(0x6773f5)))
                 .build();
+        this.operatorOnly = true;
     }
 
     @Override
     public void onExecute(@NotNull OnlineUser player, @NotNull String[] args) {
-        if (!player.hasPermission(getPermission())) {
-            plugin.getLocales().sendMessage(player, "error_no_permission");
-            return;
-        }
-
         if (args.length >= 1) {
             switch (args[0].toLowerCase(Locale.ROOT)) {
                 case "about", "info" -> player.sendMessage(aboutMenu.toComponent());
@@ -97,9 +93,6 @@ public void onExecute(@NotNull OnlineUser player, @NotNull String[] args) {
 
     @Override
     public List<String> onTabComplete(@NotNull OnlineUser player, @NotNull String[] args) {
-        if (!player.hasPermission(getPermission())) {
-            return List.of();
-        }
         if (args.length <= 1) {
             return Arrays.stream(COMMAND_TAB_ARGUMENTS)
                     .filter(i -> i.toLowerCase().startsWith((args.length == 1) ? args[0].toLowerCase() : ""))

diff --git a/common/src/main/java/net/william278/huskchat/command/LocalSpyCommand.java b/common/src/main/java/net/william278/huskchat/command/LocalSpyCommand.java
@@ -34,6 +34,7 @@ public class LocalSpyCommand extends CommandBase {
 
     public LocalSpyCommand(@NotNull HuskChat plugin) {
         super(plugin.getSettings().getLocalSpy().getLocalspyAliases(), "[color]", plugin);
+        this.operatorOnly = true;
     }
 
     @Override
@@ -42,39 +43,35 @@ public void onExecute(@NotNull OnlineUser player, @NotNull String[] args) {
             plugin.getLocales().sendMessage(player, "error_in_game_only");
             return;
         }
-        if (player.hasPermission(getPermission())) {
-            if (args.length == 1) {
-                UserCache.SpyColor color;
-                Optional<UserCache.SpyColor> selectedColor = UserCache.SpyColor.getColor(args[0]);
-                if (selectedColor.isPresent()) {
-                    try {
-                        color = selectedColor.get();
-                        plugin.getUserCache().setLocalSpy(player, color);
-                        plugin.getLocales().sendMessage(player, "local_spy_toggled_on_color",
-                                color.colorCode, color.name().toLowerCase().replaceAll("_", " "));
-                    } catch (IOException e) {
-                        plugin.log(Level.SEVERE, "Failed to save local spy state to spies file");
-                    }
-                    return;
-                }
-            }
-            if (!plugin.getUserCache().isLocalSpying(player)) {
+        if (args.length == 1) {
+            UserCache.SpyColor color;
+            Optional<UserCache.SpyColor> selectedColor = UserCache.SpyColor.getColor(args[0]);
+            if (selectedColor.isPresent()) {
                 try {
-                    plugin.getUserCache().setLocalSpy(player);
-                    plugin.getLocales().sendMessage(player, "local_spy_toggled_on");
-                } catch (IOException e) {
-                    plugin.log(Level.SEVERE, "Failed to save local spy state to spies file");
-                }
-            } else {
-                try {
-                    plugin.getUserCache().removeLocalSpy(player);
-                    plugin.getLocales().sendMessage(player, "local_spy_toggled_off");
+                    color = selectedColor.get();
+                    plugin.getUserCache().setLocalSpy(player, color);
+                    plugin.getLocales().sendMessage(player, "local_spy_toggled_on_color",
+                            color.colorCode, color.name().toLowerCase().replaceAll("_", " "));
                 } catch (IOException e) {
                     plugin.log(Level.SEVERE, "Failed to save local spy state to spies file");
                 }
+                return;
+            }
+        }
+        if (!plugin.getUserCache().isLocalSpying(player)) {
+            try {
+                plugin.getUserCache().setLocalSpy(player);
+                plugin.getLocales().sendMessage(player, "local_spy_toggled_on");
+            } catch (IOException e) {
+                plugin.log(Level.SEVERE, "Failed to save local spy state to spies file");
             }
         } else {
-            plugin.getLocales().sendMessage(player, "error_no_permission");
+            try {
+                plugin.getUserCache().removeLocalSpy(player);
+                plugin.getLocales().sendMessage(player, "local_spy_toggled_off");
+            } catch (IOException e) {
+                plugin.log(Level.SEVERE, "Failed to save local spy state to spies file");
+            }
         }
     }
 

diff --git a/common/src/main/java/net/william278/huskchat/command/MessageCommand.java b/common/src/main/java/net/william278/huskchat/command/MessageCommand.java
@@ -39,24 +39,20 @@ public MessageCommand(@NotNull HuskChat plugin) {
 
     @Override
     public void onExecute(@NotNull OnlineUser player, @NotNull String[] args) {
-        if (player.hasPermission(getPermission())) {
-            if (args.length >= 2) {
-                StringJoiner message = new StringJoiner(" ");
-                int messageWordCount = 0;
-                for (String arg : args) {
-                    if (messageWordCount >= 1) {
-                        message.add(arg);
-                    }
-                    messageWordCount++;
+        if (args.length >= 2) {
+            StringJoiner message = new StringJoiner(" ");
+            int messageWordCount = 0;
+            for (String arg : args) {
+                if (messageWordCount >= 1) {
+                    message.add(arg);
                 }
-                final List<String> targetPlayers = getTargetPlayers(args[0]);
-                final String messageToSend = message.toString();
-                new PrivateMessage(player, targetPlayers, messageToSend, plugin).dispatch();
-            } else {
-                plugin.getLocales().sendMessage(player, "error_invalid_syntax", getUsage());
+                messageWordCount++;
             }
+            final List<String> targetPlayers = getTargetPlayers(args[0]);
+            final String messageToSend = message.toString();
+            new PrivateMessage(player, targetPlayers, messageToSend, plugin).dispatch();
         } else {
-            plugin.getLocales().sendMessage(player, "error_no_permission");
+            plugin.getLocales().sendMessage(player, "error_invalid_syntax", getUsage());
         }
     }