From caf4f1b5666d3cd25fed9567784c71d94d26db6a Mon Sep 17 00:00:00 2001 From: "main()" Date: Sat, 13 Apr 2013 02:26:19 +0200 Subject: [PATCH] Bukkit perms are broken. Or at least they don't work the way they (in my opinion) should. Take a look at HierarchyPermission.java for a detailed explanation. --- .../permissions/HierarchyPermission.java | 52 +++++++++++++++++++ .../permissions/PermissionCollection.java | 38 ++++++++++++++ .../permissions/PermissionNode.java | 30 +++++++++++ .../permissions/Permissions.java | 21 ++++++++ .../permissions/package-info.java | 4 ++ .../MultiverseCore/utils/MVPermissions.java | 12 ++--- 6 files changed, 150 insertions(+), 7 deletions(-) create mode 100644 src/main/java/com/onarandombox/MultiverseCore/permissions/HierarchyPermission.java create mode 100644 src/main/java/com/onarandombox/MultiverseCore/permissions/PermissionCollection.java create mode 100644 src/main/java/com/onarandombox/MultiverseCore/permissions/PermissionNode.java create mode 100644 src/main/java/com/onarandombox/MultiverseCore/permissions/Permissions.java create mode 100644 src/main/java/com/onarandombox/MultiverseCore/permissions/package-info.java diff --git a/src/main/java/com/onarandombox/MultiverseCore/permissions/HierarchyPermission.java b/src/main/java/com/onarandombox/MultiverseCore/permissions/HierarchyPermission.java new file mode 100644 index 00000000..a767a9e5 --- /dev/null +++ b/src/main/java/com/onarandombox/MultiverseCore/permissions/HierarchyPermission.java @@ -0,0 +1,52 @@ +package com.onarandombox.MultiverseCore.permissions; + +import org.bukkit.Bukkit; +import org.bukkit.permissions.Permission; +import org.bukkit.permissions.Permissible; +import org.bukkit.permissions.PermissionDefault; + +/** + * Provides improved parent-child relationships for permissions. This is the standard implementation of {@link PermissionNode}. + *

+ * Apparently parents override their children in Bukkit: + * Let {@code permissionA} be a parent permission of {@code permissionB}, setting {@code permissionB} to {@code true}. + * Now a player has {@code permissionA=true} and {@code permissionB=false}. Result: Both are {@code true}. + * Another player has {@code permissionA=false} and {@code permissionB=true}. Result: Both are {@code false}. + *

+ * This is completely useless for things like MV's world access permissions where you could for example want to deny + * access to all worlds by default, with a few exceptions. + *

+ * This class takes over all the permission hierarchy checks and provides a way that makes sense. In the + * above examples, the effective permissions would be equal to the permissions that were set. + */ +public class HierarchyPermission implements PermissionNode { + private final PermissionNode parent; + private final Permission permission; + + public HierarchyPermission(final PermissionNode parent, final String permission, final String description) { + this.parent = parent; + Permission p = Bukkit.getServer().getPluginManager().getPermission(permission); + if (p == null) + p = new Permission(permission, description, PermissionDefault.FALSE); + else { + p.getChildren().clear(); + //p.recalculatePermissibles(); // Not necessary because it's done automatically after setting the default + p.setDefault(PermissionDefault.FALSE); + } + this.permission = p; + } + + /** + * Checks whether a given {@link Permissible} has this {@link HierarchyPermission}. + * + * @param permissible The {@link Permissible} that might have the permission. + * @return Whether it has the permission. + */ + public boolean has(Permissible permissible) { + if (permissible.isPermissionSet(permission)) + return permissible.hasPermission(permission); + + // ask the guy over there + return parent.has(permissible); + } +} diff --git a/src/main/java/com/onarandombox/MultiverseCore/permissions/PermissionCollection.java b/src/main/java/com/onarandombox/MultiverseCore/permissions/PermissionCollection.java new file mode 100644 index 00000000..090712ae --- /dev/null +++ b/src/main/java/com/onarandombox/MultiverseCore/permissions/PermissionCollection.java @@ -0,0 +1,38 @@ +package com.onarandombox.MultiverseCore.permissions; + +import org.bukkit.permissions.Permissible; + +import java.util.HashMap; +import java.util.Map; + +/** + * Represents a permission node with dynamically created children. + */ +public class PermissionCollection { + private final PermissionNode root; + private final Map elements; + private final String prefix; + private final String description; + + public PermissionCollection(PermissionNode parent, String prefix, String description) { + if (!prefix.endsWith(".")) + throw new IllegalArgumentException("The prefix must end with a dot!"); + this.root = new HierarchyPermission(parent, prefix + "*", description); + this.elements = new HashMap(); + this.prefix = prefix; + this.description = description; + } + + /** + * Checks whether a given {@link Permissible} has a specified element permission. + * + * @param p The {@link Permissible}. + * @param element The element. + * @return Whether the {@link Permissible} has the permission. + */ + public boolean has(Permissible p, String element) { + if (!this.elements.containsKey(element)) + this.elements.put(element, new HierarchyPermission(root, prefix + element, description)); + return this.elements.get(element).has(p); + } +} diff --git a/src/main/java/com/onarandombox/MultiverseCore/permissions/PermissionNode.java b/src/main/java/com/onarandombox/MultiverseCore/permissions/PermissionNode.java new file mode 100644 index 00000000..5f22b122 --- /dev/null +++ b/src/main/java/com/onarandombox/MultiverseCore/permissions/PermissionNode.java @@ -0,0 +1,30 @@ +package com.onarandombox.MultiverseCore.permissions; + +import org.bukkit.permissions.Permissible; + +/** + * A node in a permissions hierarchy. + * + * @see HierarchyPermission + */ +public interface PermissionNode { + static final class OperatorDefault implements PermissionNode { + @Override + public boolean has(final Permissible permissible) { + return permissible.isOp(); + } + } + + /** + * This {@link HierarchyPermission} depends on the operator state. Ops have it, non-ops don't. + */ + PermissionNode OP_DEFAULT = new OperatorDefault(); + + /** + * Checks whether a given {@link Permissible} has this node. + * + * @param permissible The {@link Permissible} that might have the permission. + * @return Whether it has the permission. + */ + boolean has(Permissible permissible); +} diff --git a/src/main/java/com/onarandombox/MultiverseCore/permissions/Permissions.java b/src/main/java/com/onarandombox/MultiverseCore/permissions/Permissions.java new file mode 100644 index 00000000..62eb5504 --- /dev/null +++ b/src/main/java/com/onarandombox/MultiverseCore/permissions/Permissions.java @@ -0,0 +1,21 @@ +package com.onarandombox.MultiverseCore.permissions; + +/** + * Permission nodes. + */ +public class Permissions { + protected Permissions() { + throw new UnsupportedOperationException(); + } + + /** + * The {@code multiverse.*} permission. + */ + public static final HierarchyPermission MV_ROOT = new HierarchyPermission(PermissionNode.OP_DEFAULT, + "multiverse.*", "Provides access to all Multiverse features."); + + /** + * The {@code multiverse.access.*} family of permissions. + */ + public static final PermissionCollection ACCESS = new PermissionCollection(MV_ROOT, "multiverse.access.", "World access"); +} diff --git a/src/main/java/com/onarandombox/MultiverseCore/permissions/package-info.java b/src/main/java/com/onarandombox/MultiverseCore/permissions/package-info.java new file mode 100644 index 00000000..19d1857f --- /dev/null +++ b/src/main/java/com/onarandombox/MultiverseCore/permissions/package-info.java @@ -0,0 +1,4 @@ +/** + * Contains permissions utility classes. + */ +package com.onarandombox.MultiverseCore.permissions; diff --git a/src/main/java/com/onarandombox/MultiverseCore/utils/MVPermissions.java b/src/main/java/com/onarandombox/MultiverseCore/utils/MVPermissions.java index 2ea79357..0199d46c 100644 --- a/src/main/java/com/onarandombox/MultiverseCore/utils/MVPermissions.java +++ b/src/main/java/com/onarandombox/MultiverseCore/utils/MVPermissions.java @@ -11,6 +11,7 @@ import com.onarandombox.MultiverseCore.MultiverseCore; import com.onarandombox.MultiverseCore.api.MVDestination; import com.onarandombox.MultiverseCore.api.MVWorldManager; import com.onarandombox.MultiverseCore.api.MultiverseWorld; +import com.onarandombox.MultiverseCore.permissions.Permissions; import com.pneumaticraft.commandhandler.PermissionsInterface; import org.bukkit.ChatColor; import org.bukkit.Location; @@ -103,18 +104,15 @@ public class MVPermissions implements PermissionsInterface { this.plugin.log(Level.FINEST, "EnforceAccess is OFF. Player was allowed in " + w.getAlias()); return true; } - return this.hasPermission(p, "multiverse.access." + w.getName(), false); + return Permissions.ACCESS.has(p, w.getName()); } private boolean canEnterLocation(Player p, Location l) { - if (l == null) { + if (l == null) return false; - } + String worldName = l.getWorld().getName(); - if (!this.plugin.getMVWorldManager().isMVWorld(worldName)) { - return false; - } - return this.hasPermission(p, "multiverse.access." + worldName, false); + return this.plugin.getMVWorldManager().isMVWorld(worldName) && Permissions.ACCESS.has(p, worldName); } /**