From 0af788fa1d6865dbec9ed71580e19925a11c3fb6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20S=C3=B6derberg?= Date: Mon, 12 Oct 2020 01:43:41 +0200 Subject: [PATCH] :bug: Fix issue with permissions Essentially, when there's an intermediary command and a child command with a variable leading argument, only the permission for the leading argument would be checked. In this case, that permission should only be considered if there's no more input. This fixes #46. --- .../cloud/commandframework/CommandTree.java | 3 +-- .../CommandPermissionTest.java | 23 ++++++++++++++++++- 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/cloud-core/src/main/java/cloud/commandframework/CommandTree.java b/cloud-core/src/main/java/cloud/commandframework/CommandTree.java index 070768af..21b6f161 100644 --- a/cloud-core/src/main/java/cloud/commandframework/CommandTree.java +++ b/cloud-core/src/main/java/cloud/commandframework/CommandTree.java @@ -271,7 +271,7 @@ public final class CommandTree { // The value has to be a variable final Node> child = children.get(0); permission = this.isPermitted(commandContext.getSender(), child); - if (permission != null) { + if (!commandQueue.isEmpty() && permission != null) { return Pair.of(null, new NoPermissionException( permission, commandContext.getSender(), @@ -289,7 +289,6 @@ public final class CommandTree { } else if (!child.getValue().isRequired()) { return Pair.of(this.cast(child.getValue().getOwningCommand()), null); } else if (child.isLeaf()) { - /* The child is not a leaf, but may have an intermediary executor, attempt to use it */ if (root.getValue() != null && root.getValue().getOwningCommand() != null) { final Command command = root.getValue().getOwningCommand(); if (!this.getCommandManager().hasPermission( diff --git a/cloud-core/src/test/java/cloud/commandframework/CommandPermissionTest.java b/cloud-core/src/test/java/cloud/commandframework/CommandPermissionTest.java index 6078956d..b94df642 100644 --- a/cloud-core/src/test/java/cloud/commandframework/CommandPermissionTest.java +++ b/cloud-core/src/test/java/cloud/commandframework/CommandPermissionTest.java @@ -23,6 +23,7 @@ // package cloud.commandframework; +import cloud.commandframework.arguments.standard.IntegerArgument; import cloud.commandframework.execution.CommandExecutionCoordinator; import cloud.commandframework.meta.CommandMeta; import cloud.commandframework.meta.SimpleCommandMeta; @@ -30,6 +31,8 @@ import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; +import java.util.concurrent.CompletionException; + class CommandPermissionTest { private final static CommandManager manager = new PermissionOutputtingCommandManager(); @@ -50,9 +53,21 @@ class CommandPermissionTest { Assertions.assertFalse(manager.suggest(new TestCommandSender(), "t").isEmpty()); } + @Test + void testComplexPermissions() { + manager.command(manager.commandBuilder("first").permission("first")); + manager.command(manager.commandBuilder("first").argument(IntegerArgument.of("second")).permission("second")); + manager.executeCommand(new TestCommandSender(), "first").join(); + Assertions.assertThrows( + CompletionException.class, + () -> manager.executeCommand(new TestCommandSender(), "first 10").join() + ); + } + + private static final class PermissionOutputtingCommandManager extends CommandManager { - public PermissionOutputtingCommandManager() { + private PermissionOutputtingCommandManager() { super(CommandExecutionCoordinator.simpleCoordinator(), cmd -> true); } @@ -61,6 +76,12 @@ class CommandPermissionTest { final TestCommandSender sender, final String permission ) { + if (permission.equalsIgnoreCase("first")) { + return true; + } + if (permission.equalsIgnoreCase("second")) { + return false; + } return acceptOne && permission.equalsIgnoreCase("test.permission.four"); }