Permalink
Browse files

Improve permission resolution of RootCommand's

ACF's permission tree can go more complex where a single root command
may have multiple dependent perm nodes.

So essentially ACF does not assign permission nodes to root in bukkit
and the such in a reasonable manner.

With this commit, we try to identify a single unique permission node,
and assign that permission node as the node to use where applicable.

In Bukkit/Sponge, we implement testPermission instead, which does a smarter
look up of all potential commands that root command might execute for the
given issuer, and if they have permission to any of them, then pass as true.

This is much more accurate, so if the issuer has access to no subcommand
then the root command should not be revealed anymore in Bukkit or Sponge.

In bungee, we are best guess at the unique perm node, and if there is
any ambiguity, it will be null and seen by everyone (but still enforces
permission checks)
  • Loading branch information...
aikar committed Feb 5, 2019
1 parent 227052f commit 999c803091e0f099abd78623bb61e3079b28e638
@@ -63,14 +63,19 @@ public boolean execute(CommandSender sender, String commandLabel, String[] args)
return true;
}

@Override
public boolean testPermissionSilent(CommandSender target) {
return hasAnyPermission(manager.getCommandIssuer(target));
}

public void addChild(BaseCommand command) {
if (this.defCommand == null || !command.subCommands.get(BaseCommand.DEFAULT).isEmpty()) {
this.defCommand = command;
this.setPermission(command.permission);
//this.setDescription(command.getDescription());
//this.setUsage(command.getUsage());
}
addChildShared(this.children, this.subCommands, command);
setPermission(getUniquePermission());
}

@Override
@@ -89,7 +94,7 @@ public CommandManager getManager() {
}

@Override
public BaseCommand getDefCommand(){
public BaseCommand getDefCommand() {
return defCommand;
}

@@ -40,6 +40,7 @@
private SetMultimap<String, RegisteredCommand> subCommands = HashMultimap.create();
private List<BaseCommand> children = new ArrayList<>();
boolean isRegistered = false;
private String uniquePermission;

BungeeRootCommand(BungeeCommandManager manager, String name) {
super(name);
@@ -59,6 +60,7 @@ public void addChild(BaseCommand command) {

}
this.addChildShared(this.children, this.subCommands, command);
this.uniquePermission = getUniquePermission();
}

@Override
@@ -81,6 +83,11 @@ public void execute(CommandSender sender, String[] args) {
execute(manager.getCommandIssuer(sender), getName(), args);
}

@Override
public String getPermission() {
return uniquePermission;
}

@Override
public Iterable<String> onTabComplete(CommandSender commandSender, String[] strings) {
return getTabCompletions(manager.getCommandIssuer(commandSender), getName(), strings);
@@ -901,16 +901,20 @@ public boolean hasPermission(CommandIssuer issuer) {
return permission == null || permission.isEmpty() || (manager.hasPermission(issuer, permission) && (parentCommand == null || parentCommand.hasPermission(issuer)));
}


public Set<String> getRequiredPermissions() {
if (this.permission == null || this.permission.isEmpty()) {
return Collections.emptySet();
Set<String> permissions = new HashSet<>();
if (this.permission != null && !this.permission.isEmpty()) {
permissions.addAll(Arrays.asList(ACFPatterns.COMMA.split(this.permission)));
}
if (parentCommand != null) {
permissions.addAll(parentCommand.getRequiredPermissions());
}
return new HashSet<>(Arrays.asList(ACFPatterns.COMMA.split(this.permission)));

return permissions;
}

public boolean requiresPermission(String permission) {
return getRequiredPermissions().contains(permission) || this.parentCommand != null && parentCommand.requiresPermission(permission);
return getRequiredPermissions().contains(permission);
}

public String getName() {
@@ -25,17 +25,41 @@

import co.aikar.commands.apachecommonslang.ApacheCommonsLangUtil;

import java.util.Collections;
import java.util.List;
import java.util.Set;

public class ForwardingCommand extends BaseCommand {
private final BaseCommand command;
private final String[] baseArgs;
private final RegisteredCommand regCommand;

ForwardingCommand(BaseCommand command, String[] baseArgs) {
this.commandName = command.commandName;
this.command = command;
ForwardingCommand(BaseCommand baseCommand, RegisteredCommand regCommand, String[] baseArgs) {
this.regCommand = regCommand;
this.commandName = baseCommand.commandName;
this.command = baseCommand;
this.baseArgs = baseArgs;
this.manager = command.manager;
this.manager = baseCommand.manager;
}

@Override
public List<RegisteredCommand> getRegisteredCommands() {
return Collections.singletonList(regCommand);
}

@Override
public Set<String> getRequiredPermissions() {
return command.getRequiredPermissions();
}

@Override
public boolean hasPermission(Object issuer) {
return command.hasPermission(issuer);
}

@Override
public boolean requiresPermission(String permission) {
return command.requiresPermission(permission);
}

@Override
@@ -194,6 +194,7 @@ void handleException(CommandIssuer sender, List<String> args, Exception e) {
Map<String, Object> resolveContexts(CommandIssuer sender, List<String> args) throws InvalidCommandArgument {
return resolveContexts(sender, args, parameters.length);
}

@Nullable
Map<String, Object> resolveContexts(CommandIssuer sender, List<String> args, int argLimit) throws InvalidCommandArgument {
args = new ArrayList<>(args);
@@ -282,14 +283,15 @@ public String getPermission() {
}

public Set<String> getRequiredPermissions() {
if (this.permission == null || this.permission.isEmpty()) {
return Collections.emptySet();
Set<String> permissions = scope.getRequiredPermissions();
if (this.permission != null && !this.permission.isEmpty()) {
permissions.addAll(Arrays.asList(ACFPatterns.COMMA.split(this.permission)));
}
return new HashSet<>(Arrays.asList(ACFPatterns.COMMA.split(this.permission)));
return permissions;
}

public boolean requiresPermission(String permission) {
return getRequiredPermissions().contains(permission) || scope.requiresPermission(permission);
return getRequiredPermissions().contains(permission);
}

public String getPrefSubCommand() {
@@ -299,11 +301,11 @@ public String getPrefSubCommand() {
public String getSyntaxText() {
return syntaxText;
}

public String getHelpText() {
return helpText != null ? helpText : "";
}

public boolean isPrivate() {
return isPrivate;
}
@@ -315,6 +317,7 @@ public String getCommand() {
public void addSubcommand(String cmd) {
this.registeredSubcommands.add(cmd);
}

public void addSubcommands(Collection<String> cmd) {
this.registeredSubcommands.addAll(cmd);
}
@@ -33,12 +33,15 @@

public interface RootCommand {
void addChild(BaseCommand command);

CommandManager getManager();

SetMultimap<String, RegisteredCommand> getSubCommands();

List<BaseCommand> getChildren();

String getCommandName();

default void addChildShared(List<BaseCommand> children, SetMultimap<String, RegisteredCommand> subCommands, BaseCommand command) {
command.subCommands.entries().forEach(e -> {
String key = e.getKey();
@@ -61,6 +64,43 @@ default void addChildShared(List<BaseCommand> children, SetMultimap<String, Regi
children.add(command);
}

/**
* @return If this root command can be summarized to a single required permission node to use it, returns that value. If any RegisteredCommand is permission-less, or has multiple required permission nodes, null is returned.
*/
default String getUniquePermission() {
Set<String> permissions = new HashSet<>();
for (BaseCommand child : getChildren()) {
for (RegisteredCommand<?> value : child.subCommands.values()) {
Set<String> requiredPermissions = value.getRequiredPermissions();
if (requiredPermissions.isEmpty()) {
return null;
} else {
permissions.addAll(requiredPermissions);
}
}
}
return permissions.size() == 1 ? permissions.iterator().next() : null;
}

default boolean hasAnyPermission(CommandIssuer issuer) {
List<BaseCommand> children = getChildren();
if (children.isEmpty()) {
return true;
}

for (BaseCommand child : children) {
if (!child.hasPermission(issuer)) {
continue;
}
for (RegisteredCommand value : child.getRegisteredCommands()) {
if (value.hasPermission(issuer)) {
return true;
}
}
}
return false;
}

default BaseCommand execute(CommandIssuer sender, String commandLabel, String[] args) {
BaseCommand command = getBaseCommand(args);

@@ -109,7 +149,7 @@ default RegisteredCommand getDefaultRegisteredCommand() {
return null;
}

default BaseCommand getDefCommand(){
default BaseCommand getDefCommand() {
return null;
}

@@ -72,7 +72,7 @@ public CommandResult process(@NotNull CommandSource source, @NotNull String argu

@Override
public boolean testPermission(@NotNull CommandSource source) {
return this.defCommand.hasPermission(source);
return this.hasAnyPermission(manager.getCommandIssuer(source));
}

@Override
@@ -105,7 +105,7 @@ public void addChild(BaseCommand command) {
}

@Override
public BaseCommand getDefCommand(){
public BaseCommand getDefCommand() {
return defCommand;
}

0 comments on commit 999c803

Please sign in to comment.