Give false default permissions priority over wildcards

I think this is a good compromise. It won't apply to registered permissions that are defaulted to 'op' (Bukkit) or 'undefined' (Sponge), only to those that are specifically set to false. The change is configurable, enabled by default for new installs of LP. Will hopefully go some way to resolving: - #2787 - https://v2.nucleuspowered.org/docs/nowildcard.html - NucleusPowered/Nucleus#1093 (and related) cc: @dualspiral @slipcor
LuckPerms · Dec 23, 2020 · 8167fbf · 8167fbf
1 parent 8dfeef9
commit 8167fbf
Show file tree

Hide file tree

Showing 19 changed files with 160 additions and 40 deletions.
diff --git a/bukkit/src/main/java/me/lucko/luckperms/bukkit/calculator/BukkitCalculatorFactory.java b/bukkit/src/main/java/me/lucko/luckperms/bukkit/calculator/BukkitCalculatorFactory.java
@@ -45,14 +45,8 @@
 public class BukkitCalculatorFactory implements CalculatorFactory {
     private final LPBukkitPlugin plugin;
 
-    private final DefaultsProcessor opDefaultsProcessor;
-    private final DefaultsProcessor nonOpDefaultsProcessor;
-
     public BukkitCalculatorFactory(LPBukkitPlugin plugin) {
         this.plugin = plugin;
-
-        this.opDefaultsProcessor = new DefaultsProcessor(this.plugin, true);
-        this.nonOpDefaultsProcessor = new DefaultsProcessor(this.plugin, false);
     }
 
     @Override
@@ -79,7 +73,8 @@ public PermissionCalculator build(QueryOptions queryOptions, CacheMetadata metad
 
         boolean op = queryOptions.option(BukkitContextManager.OP_OPTION).orElse(false);
         if (metadata.getHolderType() == HolderType.USER && this.plugin.getConfiguration().get(ConfigKeys.APPLY_BUKKIT_DEFAULT_PERMISSIONS)) {
-            processors.add(op ? this.opDefaultsProcessor : this.nonOpDefaultsProcessor);
+            boolean overrideWildcards = this.plugin.getConfiguration().get(ConfigKeys.APPLY_DEFAULT_NEGATIONS_BEFORE_WILDCARDS);
+            processors.add(new DefaultsProcessor(this.plugin, overrideWildcards, op));
         }
 
         if (op) {

diff --git a/bukkit/src/main/java/me/lucko/luckperms/bukkit/calculator/DefaultsProcessor.java b/bukkit/src/main/java/me/lucko/luckperms/bukkit/calculator/DefaultsProcessor.java
@@ -27,11 +27,14 @@
 
 import me.lucko.luckperms.bukkit.LPBukkitPlugin;
 import me.lucko.luckperms.common.calculator.processor.PermissionProcessor;
+import me.lucko.luckperms.common.calculator.processor.SpongeWildcardProcessor;
+import me.lucko.luckperms.common.calculator.processor.WildcardProcessor;
 import me.lucko.luckperms.common.calculator.result.TristateResult;
 
 import net.luckperms.api.util.Tristate;
 
 import org.bukkit.permissions.Permission;
+import org.bukkit.permissions.PermissionDefault;
 
 /**
  * Permission Processor for Bukkits "default" permission system.
@@ -41,15 +44,38 @@ public class DefaultsProcessor implements PermissionProcessor {
     private static final TristateResult.Factory PERMISSION_MAP_RESULT_FACTORY = new TristateResult.Factory(DefaultsProcessor.class, "permission map");
 
     private final LPBukkitPlugin plugin;
+    private final boolean overrideWildcards;
     private final boolean isOp;
 
-    public DefaultsProcessor(LPBukkitPlugin plugin, boolean isOp) {
+    public DefaultsProcessor(LPBukkitPlugin plugin, boolean overrideWildcards, boolean isOp) {
         this.plugin = plugin;
+        this.overrideWildcards = overrideWildcards;
         this.isOp = isOp;
     }
 
+    private boolean canOverrideWildcard(TristateResult prev) {
+        return this.overrideWildcards &&
+                (prev.processorClass() == WildcardProcessor.class || prev.processorClass() == SpongeWildcardProcessor.class) &&
+                prev.result() == Tristate.TRUE;
+    }
+
     @Override
-    public TristateResult hasPermission(String permission) {
+    public TristateResult hasPermission(TristateResult prev, String permission) {
+        if (prev != TristateResult.UNDEFINED) {
+            // Check to see if the result should be overridden
+            if (canOverrideWildcard(prev)) {
+                Permission defPerm = this.plugin.getPermissionMap().get(permission);
+                if (defPerm != null) {
+                    PermissionDefault def = defPerm.getDefault();
+                    if (def == PermissionDefault.FALSE || (this.isOp && def == PermissionDefault.NOT_OP)) {
+                        return PERMISSION_MAP_RESULT_FACTORY.result(Tristate.FALSE, "permission map (overriding wildcard): " + prev.cause());
+                    }
+                }
+            }
+
+            return prev;
+        }
+
         Tristate t = this.plugin.getDefaultPermissionMap().lookupDefaultPermission(permission, this.isOp);
         if (t != Tristate.UNDEFINED) {
             return DEFAULT_PERMISSION_MAP_RESULT_FACTORY.result(t);

diff --git a/bukkit/src/main/java/me/lucko/luckperms/bukkit/calculator/OpProcessor.java b/bukkit/src/main/java/me/lucko/luckperms/bukkit/calculator/OpProcessor.java
@@ -44,7 +44,10 @@ private OpProcessor() {
     }
 
     @Override
-    public TristateResult hasPermission(String permission) {
+    public TristateResult hasPermission(TristateResult prev, String permission) {
+        if (prev != TristateResult.UNDEFINED) {
+            return prev;
+        }
         return TRUE_RESULT;
     }
 }
diff --git a/bukkit/src/main/resources/config.yml b/bukkit/src/main/resources/config.yml
@@ -470,6 +470,17 @@ apply-wildcards: true
 #   and so on.
 apply-sponge-implicit-wildcards: false
 
+# If the plugin should apply negated Bukkit default permissions before it considers wildcard
+# assignments.
+#
+# - Plugin authors can define permissions which explicitly should not be given automatically to OPs.
+#   This is usually used for so called "anti-permissions" - permissions which, when granted, apply
+#   something negative.
+# - If this option is set to true, LuckPerms will consider any negated declarations made by
+#   plugins before it considers wildcards. (similar to the way the OP system works)
+# - If this option is set to false, LuckPerms will consider any wildcard assignments first.
+apply-default-negated-permissions-before-wildcards: true
+
 # If the plugin should parse regex permissions.
 #
 # - If set to true, LuckPerms will detect regex permissions, marked with "r=" at the start of the

diff --git a/common/src/main/java/me/lucko/luckperms/common/calculator/PermissionCalculator.java b/common/src/main/java/me/lucko/luckperms/common/calculator/PermissionCalculator.java
@@ -35,8 +35,6 @@
 import me.lucko.luckperms.common.plugin.LuckPermsPlugin;
 import me.lucko.luckperms.common.verbose.event.PermissionCheckEvent;
 
-import net.luckperms.api.util.Tristate;
-
 import org.checkerframework.checker.nullness.qual.NonNull;
 
 import java.util.List;
@@ -106,14 +104,11 @@ public TristateResult apply(@NonNull String permission) {
         // that this call is behind the cache.
         this.plugin.getPermissionRegistry().offer(permission);
 
+        TristateResult result = TristateResult.UNDEFINED;
         for (PermissionProcessor processor : this.processors) {
-            TristateResult result = processor.hasPermission(permission);
-            if (result.result() != Tristate.UNDEFINED) {
-                return result;
-            }
+            result = processor.hasPermission(result, permission);
         }
-
-        return TristateResult.UNDEFINED;
+        return result;
     }
 
     /**

diff --git a/...main/java/me/lucko/luckperms/common/calculator/processor/AbstractPermissionProcessor.java b/...main/java/me/lucko/luckperms/common/calculator/processor/AbstractPermissionProcessor.java
@@ -25,6 +25,8 @@
 
 package me.lucko.luckperms.common.calculator.processor;
 
+import me.lucko.luckperms.common.calculator.result.TristateResult;
+
 import java.util.Collections;
 import java.util.Map;
 
@@ -35,4 +37,14 @@ public abstract class AbstractPermissionProcessor implements PermissionProcessor
     public void setSource(Map<String, Boolean> sourceMap) {
         this.sourceMap = sourceMap;
     }
+
+    @Override
+    public TristateResult hasPermission(TristateResult prev, String permission) {
+        if (prev != TristateResult.UNDEFINED) {
+            return prev;
+        }
+        return hasPermission(permission);
+    }
+
+    public abstract TristateResult hasPermission(String permission);
 }
diff --git a/common/src/main/java/me/lucko/luckperms/common/calculator/processor/PermissionProcessor.java b/common/src/main/java/me/lucko/luckperms/common/calculator/processor/PermissionProcessor.java
@@ -41,10 +41,11 @@ public interface PermissionProcessor {
     /**
      * Returns the permission value determined by this calculator.
      *
+     * @param prev the result of the previous calculator in the chain
      * @param permission the permission
      * @return a tristate
      */
-    TristateResult hasPermission(String permission);
+    TristateResult hasPermission(TristateResult prev, String permission);
 
     /**
      * Sets the source permissions which should be used by this processor

diff --git a/common/src/main/java/me/lucko/luckperms/common/config/ConfigKeys.java b/common/src/main/java/me/lucko/luckperms/common/config/ConfigKeys.java
@@ -237,6 +237,11 @@ private ConfigKeys() {}
         return c.getBoolean("apply-sponge-implicit-wildcards", def);
     }));
 
+    /**
+     * If default negated permissions should be applied before wildcards.
+     */
+    public static final ConfigKey<Boolean> APPLY_DEFAULT_NEGATIONS_BEFORE_WILDCARDS = notReloadable(booleanKey("apply-default-negated-permissions-before-wildcards", false));
+
     /**
      * If regex permissions are being applied
      */

diff --git a/nukkit/src/main/java/me/lucko/luckperms/nukkit/calculator/DefaultsProcessor.java b/nukkit/src/main/java/me/lucko/luckperms/nukkit/calculator/DefaultsProcessor.java
@@ -26,6 +26,8 @@
 package me.lucko.luckperms.nukkit.calculator;
 
 import me.lucko.luckperms.common.calculator.processor.PermissionProcessor;
+import me.lucko.luckperms.common.calculator.processor.SpongeWildcardProcessor;
+import me.lucko.luckperms.common.calculator.processor.WildcardProcessor;
 import me.lucko.luckperms.common.calculator.result.TristateResult;
 import me.lucko.luckperms.nukkit.LPNukkitPlugin;
 import me.lucko.luckperms.nukkit.inject.PermissionDefault;
@@ -40,15 +42,37 @@ public class DefaultsProcessor implements PermissionProcessor {
     private static final TristateResult.Factory PERMISSION_MAP_RESULT_FACTORY = new TristateResult.Factory(DefaultsProcessor.class, "permission map");
 
     private final LPNukkitPlugin plugin;
+    private final boolean overrideWildcards;
     private final boolean isOp;
 
-    public DefaultsProcessor(LPNukkitPlugin plugin, boolean isOp) {
+    public DefaultsProcessor(LPNukkitPlugin plugin, boolean overrideWildcards, boolean isOp) {
         this.plugin = plugin;
+        this.overrideWildcards = overrideWildcards;
         this.isOp = isOp;
     }
 
+    private boolean canOverrideWildcard(TristateResult prev) {
+        return this.overrideWildcards &&
+                (prev.processorClass() == WildcardProcessor.class || prev.processorClass() == SpongeWildcardProcessor.class) &&
+                prev.result() == Tristate.TRUE;
+    }
+
     @Override
-    public TristateResult hasPermission(String permission) {
+    public TristateResult hasPermission(TristateResult prev, String permission) {
+        if (prev != TristateResult.UNDEFINED) {
+            // Check to see if the result should be overridden
+            if (canOverrideWildcard(prev)) {
+                PermissionDefault def = PermissionDefault.fromPermission(this.plugin.getPermissionMap().get(permission));
+                if (def != null) {
+                    if (def == PermissionDefault.FALSE || (this.isOp && def == PermissionDefault.NOT_OP)) {
+                        return PERMISSION_MAP_RESULT_FACTORY.result(Tristate.FALSE, "permission map (overriding wildcard): " + prev.cause());
+                    }
+                }
+            }
+
+            return prev;
+        }
+
         Tristate t = this.plugin.getDefaultPermissionMap().lookupDefaultPermission(permission, this.isOp);
         if (t != Tristate.UNDEFINED) {
             return DEFAULT_PERMISSION_MAP_RESULT_FACTORY.result(t);

diff --git a/nukkit/src/main/java/me/lucko/luckperms/nukkit/calculator/NukkitCalculatorFactory.java b/nukkit/src/main/java/me/lucko/luckperms/nukkit/calculator/NukkitCalculatorFactory.java
@@ -45,14 +45,8 @@
 public class NukkitCalculatorFactory implements CalculatorFactory {
     private final LPNukkitPlugin plugin;
 
-    private final DefaultsProcessor opDefaultsProcessor;
-    private final DefaultsProcessor nonOpDefaultsProcessor;
-
     public NukkitCalculatorFactory(LPNukkitPlugin plugin) {
         this.plugin = plugin;
-
-        this.opDefaultsProcessor = new DefaultsProcessor(this.plugin, true);
-        this.nonOpDefaultsProcessor = new DefaultsProcessor(this.plugin, false);
     }
 
     @Override
@@ -79,7 +73,8 @@ public PermissionCalculator build(QueryOptions queryOptions, CacheMetadata metad
 
         boolean op = queryOptions.option(NukkitContextManager.OP_OPTION).orElse(false);
         if (metadata.getHolderType() == HolderType.USER && this.plugin.getConfiguration().get(ConfigKeys.APPLY_NUKKIT_DEFAULT_PERMISSIONS)) {
-            processors.add(op ? this.opDefaultsProcessor : this.nonOpDefaultsProcessor);
+            boolean overrideWildcards = this.plugin.getConfiguration().get(ConfigKeys.APPLY_DEFAULT_NEGATIONS_BEFORE_WILDCARDS);
+            processors.add(new DefaultsProcessor(this.plugin, overrideWildcards, op));
         }
 
         if (op) {

diff --git a/nukkit/src/main/java/me/lucko/luckperms/nukkit/calculator/OpProcessor.java b/nukkit/src/main/java/me/lucko/luckperms/nukkit/calculator/OpProcessor.java
@@ -44,7 +44,10 @@ private OpProcessor() {
     }
 
     @Override
-    public TristateResult hasPermission(String permission) {
+    public TristateResult hasPermission(TristateResult prev, String permission) {
+        if (prev != TristateResult.UNDEFINED) {
+            return prev;
+        }
         return TRUE_RESULT;
     }
 }
diff --git a/nukkit/src/main/resources/config.yml b/nukkit/src/main/resources/config.yml
@@ -465,6 +465,17 @@ apply-wildcards: true
 #   and so on.
 apply-sponge-implicit-wildcards: false
 
+# If the plugin should apply negated Nukkit default permissions before it considers wildcard
+# assignments.
+#
+# - Plugin authors can define permissions which explicitly should not be given automatically to OPs.
+#   This is usually used for so called "anti-permissions" - permissions which, when granted, apply
+#   something negative.
+# - If this option is set to true, LuckPerms will consider any negated declarations made by
+#   plugins before it considers wildcards. (similar to the way the OP system works)
+# - If this option is set to false, LuckPerms will consider any wildcard assignments first.
+apply-default-negated-permissions-before-wildcards: true
+
 # If the plugin should parse regex permissions.
 #
 # - If set to true, LuckPerms will detect regex permissions, marked with "r=" at the start of the

diff --git a/sponge/src/main/java/me/lucko/luckperms/sponge/calculator/DefaultsProcessor.java b/sponge/src/main/java/me/lucko/luckperms/sponge/calculator/DefaultsProcessor.java
@@ -26,6 +26,8 @@
 package me.lucko.luckperms.sponge.calculator;
 
 import me.lucko.luckperms.common.calculator.processor.PermissionProcessor;
+import me.lucko.luckperms.common.calculator.processor.SpongeWildcardProcessor;
+import me.lucko.luckperms.common.calculator.processor.WildcardProcessor;
 import me.lucko.luckperms.common.calculator.result.TristateResult;
 import me.lucko.luckperms.sponge.service.model.LPPermissionService;
 import me.lucko.luckperms.sponge.service.model.LPSubject;
@@ -39,16 +41,41 @@ public abstract class DefaultsProcessor implements PermissionProcessor {
 
     protected final LPPermissionService service;
     private final QueryOptions queryOptions;
+    private final boolean overrideWildcards;
 
-    public DefaultsProcessor(LPPermissionService service, QueryOptions queryOptions) {
+    public DefaultsProcessor(LPPermissionService service, QueryOptions queryOptions, boolean overrideWildcards) {
         this.service = service;
         this.queryOptions = queryOptions;
+        this.overrideWildcards = overrideWildcards;
     }
 
     protected abstract LPSubject getTypeDefaults();
 
+    private boolean canOverrideWildcard(TristateResult prev) {
+        return this.overrideWildcards &&
+                (prev.processorClass() == WildcardProcessor.class || prev.processorClass() == SpongeWildcardProcessor.class) &&
+                prev.result() == Tristate.TRUE;
+    }
+
     @Override
-    public TristateResult hasPermission(String permission) {
+    public TristateResult hasPermission(TristateResult prev, String permission) {
+        if (prev != TristateResult.UNDEFINED) {
+            // Check to see if the result should be overridden
+            if (canOverrideWildcard(prev)) {
+                Tristate t = getTypeDefaults().getPermissionValue(this.queryOptions, permission);
+                if (t == Tristate.FALSE) {
+                    return TYPE_DEFAULTS_RESULT_FACTORY.result(Tristate.FALSE, "type defaults (overriding wildcard): " + prev.cause());
+                }
+
+                t = this.service.getRootDefaults().getPermissionValue(this.queryOptions, permission);
+                if (t == Tristate.FALSE) {
+                    return ROOT_DEFAULTS_RESULT_FACTORY.result(Tristate.FALSE, "root defaults (overriding wildcard): " + prev.cause());
+                }
+            }
+
+            return prev;
+        }
+
         Tristate t = getTypeDefaults().getPermissionValue(this.queryOptions, permission);
         if (t != Tristate.UNDEFINED) {
             return TYPE_DEFAULTS_RESULT_FACTORY.result(t);

diff --git a/sponge/src/main/java/me/lucko/luckperms/sponge/calculator/FixedDefaultsProcessor.java b/sponge/src/main/java/me/lucko/luckperms/sponge/calculator/FixedDefaultsProcessor.java
@@ -33,8 +33,8 @@
 public class FixedDefaultsProcessor extends DefaultsProcessor {
     private final LPSubject defaultsSubject;
 
-    public FixedDefaultsProcessor(LPPermissionService service, QueryOptions queryOptions, LPSubject defaultsSubject) {
-        super(service, queryOptions);
+    public FixedDefaultsProcessor(LPPermissionService service, QueryOptions queryOptions, LPSubject defaultsSubject, boolean overrideWildcards) {
+        super(service, queryOptions, overrideWildcards);
         this.defaultsSubject = defaultsSubject;
     }
 

diff --git a/sponge/src/main/java/me/lucko/luckperms/sponge/calculator/GroupDefaultsProcessor.java b/sponge/src/main/java/me/lucko/luckperms/sponge/calculator/GroupDefaultsProcessor.java
@@ -32,8 +32,8 @@
 import net.luckperms.api.query.QueryOptions;
 
 public class GroupDefaultsProcessor extends DefaultsProcessor implements PermissionProcessor {
-    public GroupDefaultsProcessor(LPPermissionService service, QueryOptions queryOptions) {
-        super(service, queryOptions);
+    public GroupDefaultsProcessor(LPPermissionService service, QueryOptions queryOptions, boolean overrideWildcards) {
+        super(service, queryOptions, overrideWildcards);
     }
 
     @Override

diff --git a/sponge/src/main/java/me/lucko/luckperms/sponge/calculator/SpongeCalculatorFactory.java b/sponge/src/main/java/me/lucko/luckperms/sponge/calculator/SpongeCalculatorFactory.java
@@ -67,10 +67,11 @@ public PermissionCalculator build(QueryOptions queryOptions, CacheMetadata metad
         }
 
         if (this.plugin.getConfiguration().get(ConfigKeys.APPLY_SPONGE_DEFAULT_SUBJECTS)) {
+            boolean overrideWildcards = this.plugin.getConfiguration().get(ConfigKeys.APPLY_DEFAULT_NEGATIONS_BEFORE_WILDCARDS);
             if (metadata.getHolderType() == HolderType.USER) {
-                processors.add(new UserDefaultsProcessor(this.plugin.getService(), queryOptions));
+                processors.add(new UserDefaultsProcessor(this.plugin.getService(), queryOptions, overrideWildcards));
             } else if (metadata.getHolderType() == HolderType.GROUP) {
-                processors.add(new GroupDefaultsProcessor(this.plugin.getService(), queryOptions));
+                processors.add(new GroupDefaultsProcessor(this.plugin.getService(), queryOptions, overrideWildcards));
             }
         }
 

diff --git a/sponge/src/main/java/me/lucko/luckperms/sponge/calculator/UserDefaultsProcessor.java b/sponge/src/main/java/me/lucko/luckperms/sponge/calculator/UserDefaultsProcessor.java
@@ -32,8 +32,8 @@
 import net.luckperms.api.query.QueryOptions;
 
 public class UserDefaultsProcessor extends DefaultsProcessor implements PermissionProcessor {
-    public UserDefaultsProcessor(LPPermissionService service, QueryOptions queryOptions) {
-        super(service, queryOptions);
+    public UserDefaultsProcessor(LPPermissionService service, QueryOptions queryOptions, boolean overrideWildcards) {
+        super(service, queryOptions, overrideWildcards);
     }
 
     @Override

diff --git a/...e/lucko/luckperms/sponge/service/model/calculated/CalculatedSubjectCachedDataManager.java b/...e/lucko/luckperms/sponge/service/model/calculated/CalculatedSubjectCachedDataManager.java
@@ -103,7 +103,7 @@ public PermissionCalculator build(QueryOptions queryOptions, CacheMetadata metad
         processors.add(new WildcardProcessor());
 
         if (!this.subject.getParentCollection().isDefaultsCollection()) {
-            processors.add(new FixedDefaultsProcessor(this.subject.getService(), queryOptions, this.subject.getDefaults()));
+            processors.add(new FixedDefaultsProcessor(this.subject.getService(), queryOptions, this.subject.getDefaults(), true));
         }
 
         return new PermissionCalculator(getPlugin(), metadata, processors.build());