From ecf1d3790cfe7771103742ed1ebe5e6ed3cdccbc Mon Sep 17 00:00:00 2001 From: Luck Date: Sun, 5 Mar 2017 19:05:05 +0000 Subject: [PATCH] Fix issue where group nodes could be unset using the permission commands, and where users with per-server groups wouldn't be assigned to the default group globally --- .../generic/permission/PermissionUnset.java | 46 +++++++++++++------ .../permission/PermissionUnsetTemp.java | 45 ++++++++++++------ .../common/core/model/PermissionHolder.java | 23 ++++++++++ .../managers/impl/GenericUserManager.java | 4 ++ 4 files changed, 92 insertions(+), 26 deletions(-) diff --git a/common/src/main/java/me/lucko/luckperms/common/commands/generic/permission/PermissionUnset.java b/common/src/main/java/me/lucko/luckperms/common/commands/generic/permission/PermissionUnset.java index 4834fa556..8767ef72d 100644 --- a/common/src/main/java/me/lucko/luckperms/common/commands/generic/permission/PermissionUnset.java +++ b/common/src/main/java/me/lucko/luckperms/common/commands/generic/permission/PermissionUnset.java @@ -31,6 +31,7 @@ import me.lucko.luckperms.common.commands.utils.ArgumentUtils; import me.lucko.luckperms.common.commands.utils.ContextHelper; import me.lucko.luckperms.common.constants.Message; import me.lucko.luckperms.common.constants.Permission; +import me.lucko.luckperms.common.core.NodeFactory; import me.lucko.luckperms.common.core.model.PermissionHolder; import me.lucko.luckperms.common.data.LogEntry; import me.lucko.luckperms.common.plugin.LuckPermsPlugin; @@ -59,19 +60,38 @@ public class PermissionUnset extends SharedSubCommand { String world = ArgumentUtils.handleWorld(2, args); try { - switch (ContextHelper.determine(server, world)) { - case NONE: - holder.unsetPermission(node); - Message.UNSETPERMISSION_SUCCESS.send(sender, node, holder.getFriendlyName()); - break; - case SERVER: - holder.unsetPermission(node, server); - Message.UNSETPERMISSION_SERVER_SUCCESS.send(sender, node, holder.getFriendlyName(), server); - break; - case SERVER_AND_WORLD: - holder.unsetPermission(node, server, world); - Message.UNSETPERMISSION_SERVER_WORLD_SUCCESS.send(sender, node, holder.getFriendlyName(), server, world); - break; + // unset exact - with false value only + if (node.startsWith("group.")) { + switch (ContextHelper.determine(server, world)) { + case NONE: + holder.unsetPermissionExact(NodeFactory.make(node, false)); + Message.UNSETPERMISSION_SUCCESS.send(sender, node, holder.getFriendlyName()); + break; + case SERVER: + holder.unsetPermissionExact(NodeFactory.make(node, false, server)); + Message.UNSETPERMISSION_SERVER_SUCCESS.send(sender, node, holder.getFriendlyName(), server); + break; + case SERVER_AND_WORLD: + holder.unsetPermissionExact(NodeFactory.make(node, false, server, world)); + Message.UNSETPERMISSION_SERVER_WORLD_SUCCESS.send(sender, node, holder.getFriendlyName(), server, world); + break; + } + } else { + // standard unset + switch (ContextHelper.determine(server, world)) { + case NONE: + holder.unsetPermission(node); + Message.UNSETPERMISSION_SUCCESS.send(sender, node, holder.getFriendlyName()); + break; + case SERVER: + holder.unsetPermission(node, server); + Message.UNSETPERMISSION_SERVER_SUCCESS.send(sender, node, holder.getFriendlyName(), server); + break; + case SERVER_AND_WORLD: + holder.unsetPermission(node, server, world); + Message.UNSETPERMISSION_SERVER_WORLD_SUCCESS.send(sender, node, holder.getFriendlyName(), server, world); + break; + } } LogEntry.build().actor(sender).acted(holder) diff --git a/common/src/main/java/me/lucko/luckperms/common/commands/generic/permission/PermissionUnsetTemp.java b/common/src/main/java/me/lucko/luckperms/common/commands/generic/permission/PermissionUnsetTemp.java index 9e5b4ccd4..0b2337230 100644 --- a/common/src/main/java/me/lucko/luckperms/common/commands/generic/permission/PermissionUnsetTemp.java +++ b/common/src/main/java/me/lucko/luckperms/common/commands/generic/permission/PermissionUnsetTemp.java @@ -31,6 +31,7 @@ import me.lucko.luckperms.common.commands.utils.ArgumentUtils; import me.lucko.luckperms.common.commands.utils.ContextHelper; import me.lucko.luckperms.common.constants.Message; import me.lucko.luckperms.common.constants.Permission; +import me.lucko.luckperms.common.core.NodeFactory; import me.lucko.luckperms.common.core.model.PermissionHolder; import me.lucko.luckperms.common.data.LogEntry; import me.lucko.luckperms.common.plugin.LuckPermsPlugin; @@ -59,19 +60,37 @@ public class PermissionUnsetTemp extends SharedSubCommand { String world = ArgumentUtils.handleWorld(2, args); try { - switch (ContextHelper.determine(server, world)) { - case NONE: - holder.unsetPermission(node, true); - Message.UNSET_TEMP_PERMISSION_SUCCESS.send(sender, node, holder.getFriendlyName()); - break; - case SERVER: - holder.unsetPermission(node, server, true); - Message.UNSET_TEMP_PERMISSION_SERVER_SUCCESS.send(sender, node, holder.getFriendlyName(), server); - break; - case SERVER_AND_WORLD: - holder.unsetPermission(node, server, world, true); - Message.UNSET_TEMP_PERMISSION_SERVER_WORLD_SUCCESS.send(sender, node, holder.getFriendlyName(), server, world); - break; + // unset exact - with false value only + if (node.startsWith("group.")) { + switch (ContextHelper.determine(server, world)) { + case NONE: + holder.unsetPermissionExact(NodeFactory.make(node, false, true)); + Message.UNSET_TEMP_PERMISSION_SUCCESS.send(sender, node, holder.getFriendlyName()); + break; + case SERVER: + holder.unsetPermissionExact(NodeFactory.make(node, false, server, true)); + Message.UNSET_TEMP_PERMISSION_SERVER_SUCCESS.send(sender, node, holder.getFriendlyName(), server); + break; + case SERVER_AND_WORLD: + holder.unsetPermissionExact(NodeFactory.make(node, false, server, world, true)); + Message.UNSET_TEMP_PERMISSION_SERVER_WORLD_SUCCESS.send(sender, node, holder.getFriendlyName(), server, world); + break; + } + } else { + switch (ContextHelper.determine(server, world)) { + case NONE: + holder.unsetPermission(node, true); + Message.UNSET_TEMP_PERMISSION_SUCCESS.send(sender, node, holder.getFriendlyName()); + break; + case SERVER: + holder.unsetPermission(node, server, true); + Message.UNSET_TEMP_PERMISSION_SERVER_SUCCESS.send(sender, node, holder.getFriendlyName(), server); + break; + case SERVER_AND_WORLD: + holder.unsetPermission(node, server, world, true); + Message.UNSET_TEMP_PERMISSION_SERVER_WORLD_SUCCESS.send(sender, node, holder.getFriendlyName(), server, world); + break; + } } LogEntry.build().actor(sender).acted(holder) diff --git a/common/src/main/java/me/lucko/luckperms/common/core/model/PermissionHolder.java b/common/src/main/java/me/lucko/luckperms/common/core/model/PermissionHolder.java index 950678db5..6b396d684 100644 --- a/common/src/main/java/me/lucko/luckperms/common/core/model/PermissionHolder.java +++ b/common/src/main/java/me/lucko/luckperms/common/core/model/PermissionHolder.java @@ -875,6 +875,29 @@ public abstract class PermissionHolder { } catch (ObjectLacksException ignored) {} } + /** + * Unsets a permission node + * + * @param node the node to unset + * @throws ObjectLacksException if the holder doesn't have this node already + */ + public void unsetPermissionExact(Node node) throws ObjectLacksException { + ImmutableSet before = ImmutableSet.copyOf(getNodes()); + + synchronized (nodes) { + nodes.removeIf(e -> e.equals(node)); + } + invalidateCache(true); + + ImmutableSet after = ImmutableSet.copyOf(getNodes()); + + if (before.size() == after.size()) { + throw new ObjectLacksException(); + } + + plugin.getApiProvider().getEventFactory().handleNodeRemove(node, this, before, after); + } + /** * Unsets a transient permission node * diff --git a/common/src/main/java/me/lucko/luckperms/common/managers/impl/GenericUserManager.java b/common/src/main/java/me/lucko/luckperms/common/managers/impl/GenericUserManager.java index 8b53fb7b7..31910bf64 100644 --- a/common/src/main/java/me/lucko/luckperms/common/managers/impl/GenericUserManager.java +++ b/common/src/main/java/me/lucko/luckperms/common/managers/impl/GenericUserManager.java @@ -42,6 +42,10 @@ public class GenericUserManager extends AbstractManager im if (user.getPrimaryGroup() != null && !user.getPrimaryGroup().isEmpty()) { for (Node node : user.getPermissions(false)) { + if (node.isServerSpecific() || node.isWorldSpecific()) { + continue; + } + if (node.isGroupNode()) { hasGroup = true; break;