🐛 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.
This commit is contained in:
parent
674507fea6
commit
0af788fa1d
2 changed files with 23 additions and 3 deletions
|
|
@ -271,7 +271,7 @@ public final class CommandTree<C> {
|
||||||
// The value has to be a variable
|
// The value has to be a variable
|
||||||
final Node<CommandArgument<C, ?>> child = children.get(0);
|
final Node<CommandArgument<C, ?>> child = children.get(0);
|
||||||
permission = this.isPermitted(commandContext.getSender(), child);
|
permission = this.isPermitted(commandContext.getSender(), child);
|
||||||
if (permission != null) {
|
if (!commandQueue.isEmpty() && permission != null) {
|
||||||
return Pair.of(null, new NoPermissionException(
|
return Pair.of(null, new NoPermissionException(
|
||||||
permission,
|
permission,
|
||||||
commandContext.getSender(),
|
commandContext.getSender(),
|
||||||
|
|
@ -289,7 +289,6 @@ public final class CommandTree<C> {
|
||||||
} else if (!child.getValue().isRequired()) {
|
} else if (!child.getValue().isRequired()) {
|
||||||
return Pair.of(this.cast(child.getValue().getOwningCommand()), null);
|
return Pair.of(this.cast(child.getValue().getOwningCommand()), null);
|
||||||
} else if (child.isLeaf()) {
|
} 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) {
|
if (root.getValue() != null && root.getValue().getOwningCommand() != null) {
|
||||||
final Command<C> command = root.getValue().getOwningCommand();
|
final Command<C> command = root.getValue().getOwningCommand();
|
||||||
if (!this.getCommandManager().hasPermission(
|
if (!this.getCommandManager().hasPermission(
|
||||||
|
|
|
||||||
|
|
@ -23,6 +23,7 @@
|
||||||
//
|
//
|
||||||
package cloud.commandframework;
|
package cloud.commandframework;
|
||||||
|
|
||||||
|
import cloud.commandframework.arguments.standard.IntegerArgument;
|
||||||
import cloud.commandframework.execution.CommandExecutionCoordinator;
|
import cloud.commandframework.execution.CommandExecutionCoordinator;
|
||||||
import cloud.commandframework.meta.CommandMeta;
|
import cloud.commandframework.meta.CommandMeta;
|
||||||
import cloud.commandframework.meta.SimpleCommandMeta;
|
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.BeforeAll;
|
||||||
import org.junit.jupiter.api.Test;
|
import org.junit.jupiter.api.Test;
|
||||||
|
|
||||||
|
import java.util.concurrent.CompletionException;
|
||||||
|
|
||||||
class CommandPermissionTest {
|
class CommandPermissionTest {
|
||||||
|
|
||||||
private final static CommandManager<TestCommandSender> manager = new PermissionOutputtingCommandManager();
|
private final static CommandManager<TestCommandSender> manager = new PermissionOutputtingCommandManager();
|
||||||
|
|
@ -50,9 +53,21 @@ class CommandPermissionTest {
|
||||||
Assertions.assertFalse(manager.suggest(new TestCommandSender(), "t").isEmpty());
|
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<TestCommandSender> {
|
private static final class PermissionOutputtingCommandManager extends CommandManager<TestCommandSender> {
|
||||||
|
|
||||||
public PermissionOutputtingCommandManager() {
|
private PermissionOutputtingCommandManager() {
|
||||||
super(CommandExecutionCoordinator.simpleCoordinator(), cmd -> true);
|
super(CommandExecutionCoordinator.simpleCoordinator(), cmd -> true);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -61,6 +76,12 @@ class CommandPermissionTest {
|
||||||
final TestCommandSender sender,
|
final TestCommandSender sender,
|
||||||
final String permission
|
final String permission
|
||||||
) {
|
) {
|
||||||
|
if (permission.equalsIgnoreCase("first")) {
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
if (permission.equalsIgnoreCase("second")) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
return acceptOne && permission.equalsIgnoreCase("test.permission.four");
|
return acceptOne && permission.equalsIgnoreCase("test.permission.four");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue