From 5fe55afc681dbc30efd4ae08e25bef3a27ea25b6 Mon Sep 17 00:00:00 2001 From: Svenja Reissaus Date: Thu, 2 Aug 2018 14:55:21 -0300 Subject: [PATCH] Explicit permission checks and more admin bypasses --- .../com/massivecraft/factions/cmd/CmdBan.java | 14 +++---- .../massivecraft/factions/cmd/CmdChest.java | 11 +++-- .../massivecraft/factions/cmd/CmdClaim.java | 7 ++-- .../factions/cmd/CmdDeinvite.java | 7 ++++ .../massivecraft/factions/cmd/CmdDisband.java | 11 ++--- .../massivecraft/factions/cmd/CmdFWarp.java | 14 +++---- .../com/massivecraft/factions/cmd/CmdFly.java | 1 - .../massivecraft/factions/cmd/CmdHome.java | 2 +- .../massivecraft/factions/cmd/CmdInvite.java | 5 ++- .../factions/cmd/CmdSetFWarp.java | 10 +++-- .../massivecraft/factions/cmd/CmdSethome.java | 21 +++------- .../com/massivecraft/factions/cmd/CmdTnt.java | 11 ++--- .../massivecraft/factions/cmd/CmdTntFill.java | 10 +++-- .../massivecraft/factions/cmd/CmdUnban.java | 19 ++------- .../massivecraft/factions/cmd/CmdUnclaim.java | 8 ++-- .../factions/cmd/FPromoteCommand.java | 40 +++++++++++++------ .../listeners/FactionsBlockListener.java | 11 ++--- .../listeners/FactionsPlayerListener.java | 4 -- 18 files changed, 105 insertions(+), 101 deletions(-) diff --git a/src/main/java/com/massivecraft/factions/cmd/CmdBan.java b/src/main/java/com/massivecraft/factions/cmd/CmdBan.java index 10b351c7..19cfa071 100644 --- a/src/main/java/com/massivecraft/factions/cmd/CmdBan.java +++ b/src/main/java/com/massivecraft/factions/cmd/CmdBan.java @@ -39,18 +39,16 @@ public class CmdBan extends FCommand { return; } - // Can the player ban for this faction? - // Check for ALLOW access as well before we check for role. - if (access != Access.ALLOW) { - if (!Permission.BAN.has(sender, true) || !assertMinRole(Role.MODERATOR)) { - return; - } - } else { - if (!Permission.BAN.has(sender, true)) { + // Adds bypass to admins and clean permission check + if (!fme.isAdminBypassing()) { + Access access = myFaction.getAccess(fme, PermissableAction.BAN); + if (access != Access.ALLOW && fme.getRole() != Role.ADMIN) { + fme.msg(TL.GENERIC_FPERM_NOPERMISSION, "ban"); return; } } + // Good on permission checks. Now lets just ban the player. FPlayer target = argAsFPlayer(0); if (target == null) { diff --git a/src/main/java/com/massivecraft/factions/cmd/CmdChest.java b/src/main/java/com/massivecraft/factions/cmd/CmdChest.java index c8ad67b3..9f90454a 100644 --- a/src/main/java/com/massivecraft/factions/cmd/CmdChest.java +++ b/src/main/java/com/massivecraft/factions/cmd/CmdChest.java @@ -31,11 +31,14 @@ public class CmdChest extends FCommand { return; } // This permission check is way too explicit but it's clean - Access access = myFaction.getAccess(fme, PermissableAction.CHEST); - if (access != Access.ALLOW && fme.getRole() != Role.ADMIN) { - fme.msg(TL.GENERIC_NOPERMISSION, "chest"); - return; + if (!fme.isAdminBypassing()) { + Access access = myFaction.getAccess(fme, PermissableAction.CHEST); + if (access != Access.ALLOW && fme.getRole() != Role.ADMIN) { + fme.msg(TL.GENERIC_FPERM_NOPERMISSION, "access chest"); + return; + } } + me.openInventory(fme.getFaction().getChest()); diff --git a/src/main/java/com/massivecraft/factions/cmd/CmdClaim.java b/src/main/java/com/massivecraft/factions/cmd/CmdClaim.java index d70628ea..a729236a 100644 --- a/src/main/java/com/massivecraft/factions/cmd/CmdClaim.java +++ b/src/main/java/com/massivecraft/factions/cmd/CmdClaim.java @@ -37,14 +37,15 @@ public class CmdClaim extends FCommand { final Faction forFaction = this.argAsFaction(1, myFaction); // Default to own if (!fme.isAdminBypassing()) { - Access access = forFaction.getAccess(fme, PermissableAction.TERRITORY); - if (access == Access.DENY) { - fme.msg(TL.GENERIC_NOPERMISSION, "change faction territory!"); + Access access = myFaction.getAccess(fme, PermissableAction.TERRITORY); + if (access != Access.ALLOW && fme.getRole() != Role.ADMIN) { + fme.msg(TL.GENERIC_FPERM_NOPERMISSION, "change faction territory"); return; } } + if (radius < 1) { msg(TL.COMMAND_CLAIM_INVALIDRADIUS); return; diff --git a/src/main/java/com/massivecraft/factions/cmd/CmdDeinvite.java b/src/main/java/com/massivecraft/factions/cmd/CmdDeinvite.java index d2fef309..24c3777d 100644 --- a/src/main/java/com/massivecraft/factions/cmd/CmdDeinvite.java +++ b/src/main/java/com/massivecraft/factions/cmd/CmdDeinvite.java @@ -31,6 +31,13 @@ public class CmdDeinvite extends FCommand { @Override public void perform() { FPlayer you = this.argAsBestFPlayerMatch(0); + if (!fme.isAdminBypassing()) { + Access access = myFaction.getAccess(fme, PermissableAction.INVITE); + if (access != Access.ALLOW && fme.getRole() != Role.ADMIN) { + fme.msg(TL.GENERIC_FPERM_NOPERMISSION, "manage invites"); + return; + } + } if (you == null) { FancyMessage msg = new FancyMessage(TL.COMMAND_DEINVITE_CANDEINVITE.toString()).color(ChatColor.GOLD); for (String id : myFaction.getInvites()) { diff --git a/src/main/java/com/massivecraft/factions/cmd/CmdDisband.java b/src/main/java/com/massivecraft/factions/cmd/CmdDisband.java index 78aead41..ab4975a0 100644 --- a/src/main/java/com/massivecraft/factions/cmd/CmdDisband.java +++ b/src/main/java/com/massivecraft/factions/cmd/CmdDisband.java @@ -44,16 +44,13 @@ public class CmdDisband extends FCommand { boolean isMyFaction = fme != null && faction == myFaction; - if (isMyFaction) { - if (!assertMinRole(Role.ADMIN)) { - return; - } - } else { - if (!Permission.DISBAND_ANY.has(sender, true)) { + if (!fme.isAdminBypassing()) { + Access access = myFaction.getAccess(fme, PermissableAction.DISBAND); + if (access != Access.ALLOW && fme.getRole() != Role.ADMIN) { + fme.msg(TL.GENERIC_FPERM_NOPERMISSION, "disband faction"); return; } } - if (!faction.isNormal()) { msg(TL.COMMAND_DISBAND_IMMUTABLE.toString()); return; diff --git a/src/main/java/com/massivecraft/factions/cmd/CmdFWarp.java b/src/main/java/com/massivecraft/factions/cmd/CmdFWarp.java index 8f1a1cf9..eae26dd5 100644 --- a/src/main/java/com/massivecraft/factions/cmd/CmdFWarp.java +++ b/src/main/java/com/massivecraft/factions/cmd/CmdFWarp.java @@ -30,15 +30,15 @@ public class CmdFWarp extends FCommand { @Override public void perform() { //TODO: check if in combat. - - // Check for access first. - Access access = myFaction.getAccess(fme, PermissableAction.WARP); - - if (access == Access.DENY) { - fme.msg(TL.GENERIC_NOPERMISSION, "warp"); - return; + if (!fme.isAdminBypassing()) { + Access access = myFaction.getAccess(fme, PermissableAction.WARP); + if (access != Access.ALLOW && fme.getRole() != Role.ADMIN) { + fme.msg(TL.GENERIC_FPERM_NOPERMISSION, "use warps"); + return; + } } + if (args.size() == 0) { WarpGUI warpGUI = new WarpGUI(fme); warpGUI.build(); diff --git a/src/main/java/com/massivecraft/factions/cmd/CmdFly.java b/src/main/java/com/massivecraft/factions/cmd/CmdFly.java index 924df7b5..b01c049f 100644 --- a/src/main/java/com/massivecraft/factions/cmd/CmdFly.java +++ b/src/main/java/com/massivecraft/factions/cmd/CmdFly.java @@ -55,7 +55,6 @@ public class CmdFly extends FCommand { } } - if (FPlayers.getInstance().getByPlayer(player).isVanished()) { // Actually, vanished players (such as admins) should not display particles to prevent others from knowing their vanished assistance for moderation. // But we can keep it as a config. diff --git a/src/main/java/com/massivecraft/factions/cmd/CmdHome.java b/src/main/java/com/massivecraft/factions/cmd/CmdHome.java index d710e89a..1f4ffc6b 100644 --- a/src/main/java/com/massivecraft/factions/cmd/CmdHome.java +++ b/src/main/java/com/massivecraft/factions/cmd/CmdHome.java @@ -47,7 +47,7 @@ public class CmdHome extends FCommand { fme.msg(TL.COMMAND_HOME_TELEPORTDISABLED); return; } - f (!fme.isAdminBypassing()) { + if (!fme.isAdminBypassing()) { Access access = myFaction.getAccess(fme, PermissableAction.HOME); if (access != Access.ALLOW && fme.getRole() != Role.ADMIN) { fme.msg(TL.GENERIC_FPERM_NOPERMISSION, "teleport home"); diff --git a/src/main/java/com/massivecraft/factions/cmd/CmdInvite.java b/src/main/java/com/massivecraft/factions/cmd/CmdInvite.java index 2d6075e9..95f28619 100644 --- a/src/main/java/com/massivecraft/factions/cmd/CmdInvite.java +++ b/src/main/java/com/massivecraft/factions/cmd/CmdInvite.java @@ -50,11 +50,12 @@ public class CmdInvite extends FCommand { if (!fme.isAdminBypassing()) { Access access = myFaction.getAccess(fme, PermissableAction.INVITE); - if (access == Access.DENY || (access == Access.UNDEFINED && !assertMinRole(Role.MODERATOR))) { - fme.msg(TL.GENERIC_FPERM_NOPERMISSION, "invite"); + if (access != Access.ALLOW && fme.getRole() != Role.ADMIN) { + fme.msg(TL.GENERIC_FPERM_NOPERMISSION, "manage invites"); return; } } + if (myFaction.isInvited(target)) { fme.msg(TL.COMMAND_INVITE_ALREADYINVITED, target.getName()); return; diff --git a/src/main/java/com/massivecraft/factions/cmd/CmdSetFWarp.java b/src/main/java/com/massivecraft/factions/cmd/CmdSetFWarp.java index 9f066019..b0e57380 100644 --- a/src/main/java/com/massivecraft/factions/cmd/CmdSetFWarp.java +++ b/src/main/java/com/massivecraft/factions/cmd/CmdSetFWarp.java @@ -38,11 +38,15 @@ public class CmdSetFWarp extends FCommand { Access access = myFaction.getAccess(fme, PermissableAction.SETWARP); // This statement allows us to check if they've specifically denied it, or default to // the old setting of allowing moderators to set warps. - if (access == Access.DENY || (access == Access.UNDEFINED && !assertMinRole(Role.MODERATOR))) { - fme.msg(TL.GENERIC_NOPERMISSION, "set warp"); - return; + if (!fme.isAdminBypassing()) { + Access access = myFaction.getAccess(fme, PermissableAction.SETWARP); + if (access != Access.ALLOW && fme.getRole() != Role.ADMIN) { + fme.msg(TL.GENERIC_FPERM_NOPERMISSION, "set warps"); + return; + } } + int maxWarps = P.p.getConfig().getInt("max-warps", 5); if (maxWarps <= myFaction.getWarps().size()) { fme.msg(TL.COMMAND_SETFWARP_LIMIT, maxWarps); diff --git a/src/main/java/com/massivecraft/factions/cmd/CmdSethome.java b/src/main/java/com/massivecraft/factions/cmd/CmdSethome.java index f4289b66..6ad7ecfb 100644 --- a/src/main/java/com/massivecraft/factions/cmd/CmdSethome.java +++ b/src/main/java/com/massivecraft/factions/cmd/CmdSethome.java @@ -40,22 +40,11 @@ public class CmdSethome extends FCommand { return; } - Access access = faction.getAccess(fme, PermissableAction.SETHOME); - if (access == Access.DENY) { - fme.msg(TL.GENERIC_NOPERMISSION, "sethome"); - return; - } - - // If player does not have allow run extra permission checks - if (access != Access.ALLOW) { - if (faction == myFaction) { - if (!assertMinRole(Role.MODERATOR)) { - return; - } - } else { - if (!Permission.SETHOME_ANY.has(sender, true)) { - return; - } + if (!fme.isAdminBypassing()) { + Access access = myFaction.getAccess(fme, PermissableAction.SETHOME); + if (access != Access.ALLOW && fme.getRole() != Role.ADMIN && !Permission.SETHOME_ANY.has(sender, true)) { + fme.msg(TL.GENERIC_FPERM_NOPERMISSION, "set home"); + return; } } diff --git a/src/main/java/com/massivecraft/factions/cmd/CmdTnt.java b/src/main/java/com/massivecraft/factions/cmd/CmdTnt.java index 53abe28a..399f4835 100644 --- a/src/main/java/com/massivecraft/factions/cmd/CmdTnt.java +++ b/src/main/java/com/massivecraft/factions/cmd/CmdTnt.java @@ -36,13 +36,14 @@ public class CmdTnt extends FCommand { return; } - Access access = fme.getFaction().getAccess(fme, PermissableAction.TNTBANK); - if (access != Access.ALLOW && fme.getRole() != Role.ADMIN) { - fme.msg(TL.GENERIC_NOPERMISSION, "tntbank"); - return; + if (!fme.isAdminBypassing()) { + Access access = myFaction.getAccess(fme, PermissableAction.TNTBANK); + if (access != Access.ALLOW && fme.getRole() != Role.ADMIN) { + fme.msg(TL.GENERIC_FPERM_NOPERMISSION, "use tnt bank"); + return; + } } - if (args.size() == 2) { if (args.get(0).equalsIgnoreCase("add") || args.get(0).equalsIgnoreCase("a")) { int testNumber = -1; diff --git a/src/main/java/com/massivecraft/factions/cmd/CmdTntFill.java b/src/main/java/com/massivecraft/factions/cmd/CmdTntFill.java index ef59ef0c..ee226b11 100644 --- a/src/main/java/com/massivecraft/factions/cmd/CmdTntFill.java +++ b/src/main/java/com/massivecraft/factions/cmd/CmdTntFill.java @@ -34,12 +34,16 @@ public class CmdTntFill extends FCommand { @Override public void perform() { - Access access = fme.getFaction().getAccess(fme, PermissableAction.TNTFILL); - if (access.equals(Access.DENY)) { - fme.msg(TL.GENERIC_NOPERMISSION, "tntfill"); + if (!fme.isAdminBypassing()) { + Access access = myFaction.getAccess(fme, PermissableAction.TNTFILL); + if (access != Access.ALLOW && fme.getRole() != Role.ADMIN) { + fme.msg(TL.GENERIC_FPERM_NOPERMISSION, "use tnt fill"); + return; + } } + msg(TL.COMMAND_TNTFILL_HEADER); int radius = argAsInt(0, 16); int amount = argAsInt(1, 16); diff --git a/src/main/java/com/massivecraft/factions/cmd/CmdUnban.java b/src/main/java/com/massivecraft/factions/cmd/CmdUnban.java index 2060a43b..2af076ac 100644 --- a/src/main/java/com/massivecraft/factions/cmd/CmdUnban.java +++ b/src/main/java/com/massivecraft/factions/cmd/CmdUnban.java @@ -26,21 +26,10 @@ public class CmdUnban extends FCommand { @Override public void perform() { - Access access = myFaction.getAccess(fme, PermissableAction.BAN); - if (access == Access.DENY) { - fme.msg(TL.GENERIC_NOPERMISSION, "ban"); - return; - } - - // Can the player set the home for this faction? - // Check for ALLOW access as well before we check for role. - // TODO: no more duplicate code :( - if (access != Access.ALLOW) { - if (!Permission.BAN.has(sender) && !(assertMinRole(Role.MODERATOR))) { - return; - } - } else { - if (!Permission.BAN.has(sender, true)) { + if (!fme.isAdminBypassing()) { + Access access = myFaction.getAccess(fme, PermissableAction.BAN); + if (access != Access.ALLOW && fme.getRole() != Role.ADMIN && !Permission.BAN.has(sender, true)) { + fme.msg(TL.GENERIC_FPERM_NOPERMISSION, "manage bans"); return; } } diff --git a/src/main/java/com/massivecraft/factions/cmd/CmdUnclaim.java b/src/main/java/com/massivecraft/factions/cmd/CmdUnclaim.java index 6be7cbae..2ebf8d58 100644 --- a/src/main/java/com/massivecraft/factions/cmd/CmdUnclaim.java +++ b/src/main/java/com/massivecraft/factions/cmd/CmdUnclaim.java @@ -36,15 +36,13 @@ public class CmdUnclaim extends FCommand { final Faction forFaction = this.argAsFaction(1, myFaction); // Default to own if (!fme.isAdminBypassing()) { - Access access = forFaction.getAccess(fme, PermissableAction.TERRITORY); - if (access == Access.DENY) { - fme.msg(TL.GENERIC_NOPERMISSION, "change faction territory!"); + Access access = myFaction.getAccess(fme, PermissableAction.TERRITORY); + if (access != Access.ALLOW && fme.getRole() != Role.ADMIN) { + fme.msg(TL.GENERIC_FPERM_NOPERMISSION, "manage faction territory"); return; } } - - if (radius < 1) { msg(TL.COMMAND_CLAIM_INVALIDRADIUS); return; diff --git a/src/main/java/com/massivecraft/factions/cmd/FPromoteCommand.java b/src/main/java/com/massivecraft/factions/cmd/FPromoteCommand.java index cc9eb38f..947dc90d 100644 --- a/src/main/java/com/massivecraft/factions/cmd/FPromoteCommand.java +++ b/src/main/java/com/massivecraft/factions/cmd/FPromoteCommand.java @@ -38,21 +38,37 @@ public class FPromoteCommand extends FCommand { return; } - Access access = myFaction.getAccess(fme.getRole(), PermissableAction.PROMOTE); - - // Well this is messy. - if (access == null || access == Access.UNDEFINED) { - if (!assertMinRole(Role.MODERATOR)) { - return; - } - } else if (access == Access.DENY) { - msg(TL.COMMAND_NOACCESS); - return; - } - Role current = target.getRole(); Role promotion = Role.getRelative(current, +relative); + // Now it ain't that messy + if (!fme.isAdminBypassing()) { + Access access = myFaction.getAccess(fme, PermissableAction.PROMOTE); + if (access != Access.ALLOW && fme.getRole() != Role.ADMIN) { + fme.msg(TL.GENERIC_NOPERMISSION, "manage ranks"); + return; + } + if (target == fme) { + fme.msg(TL.COMMAND_PROMOTE_NOTSELF); + return; + } + // Don't allow people to manage role of their same rank + if (fme.getRole() == current) { + fme.msg(TL.COMMAND_PROMOTE_NOT_SAME); + return; + } + // Don't allow people to promote people to their same or higher rank. + if (fme.getRole().value <= promotion.value) { + fme.msg(TL.COMMAND_PROMOTE_NOT_ALLOWED); + return; + } + } + + if (promotion == null) { + fme.msg(TL.COMMAND_PROMOTE_NOTTHATPLAYER); + return; + } + if (promotion == null) { fme.msg(TL.COMMAND_PROMOTE_NOTTHATPLAYER); return; diff --git a/src/main/java/com/massivecraft/factions/listeners/FactionsBlockListener.java b/src/main/java/com/massivecraft/factions/listeners/FactionsBlockListener.java index ff18946d..b4517828 100644 --- a/src/main/java/com/massivecraft/factions/listeners/FactionsBlockListener.java +++ b/src/main/java/com/massivecraft/factions/listeners/FactionsBlockListener.java @@ -79,11 +79,12 @@ public class FactionsBlockListener implements Listener { return; } if (event.getBlock().getType() == Material.LEGACY_MOB_SPAWNER) { - Access access = fme.getFaction().getAccess(fme, PermissableAction.SPAWNER); - if (access.equals(Access.DENY)) { - fme.msg(TL.GENERIC_NOPERMISSION, "mine spawners"); - event.setCancelled(true); - + if (!fme.isAdminBypassing()) { + Access access = myFaction.getAccess(fme, PermissableAction.SPAWNER); + if (access != Access.ALLOW && fme.getRole() != Role.ADMIN) { + fme.msg(TL.GENERIC_FPERM_NOPERMISSION, "mine spawners"); + return; + } } } } diff --git a/src/main/java/com/massivecraft/factions/listeners/FactionsPlayerListener.java b/src/main/java/com/massivecraft/factions/listeners/FactionsPlayerListener.java index 27300352..d4b22add 100644 --- a/src/main/java/com/massivecraft/factions/listeners/FactionsPlayerListener.java +++ b/src/main/java/com/massivecraft/factions/listeners/FactionsPlayerListener.java @@ -360,10 +360,6 @@ public class FactionsPlayerListener implements Listener { return false; } - - - - @EventHandler(priority = EventPriority.NORMAL) public void onPlayerJoin(PlayerJoinEvent event) { initPlayer(event.getPlayer());