diff --git a/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/command/PlayerCommandHandler.java b/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/command/PlayerCommandHandler.java index 4c7ca99029..64d55a9047 100644 --- a/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/command/PlayerCommandHandler.java +++ b/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/command/PlayerCommandHandler.java @@ -2,15 +2,22 @@ import org.bukkit.command.CommandSender; -import java.util.regex.Pattern; +import java.util.ArrayList; import java.util.Collections; +import java.util.LinkedHashSet; import java.util.List; +import java.util.Set; +import java.util.regex.Pattern; import com.bencodez.advancedcore.AdvancedCorePlugin; -public abstract class PlayerCommandHandler extends CommandHandler { - - private int playerArg = -1; +public abstract class PlayerCommandHandler extends CommandHandler { + + private static final String ALL_SELECTOR_SENTINEL = new String("__advancedcore_all_selector__"); + private int playerArg = -1; + private final Set allPermissionOverrides = new LinkedHashSet<>(); + private final Set legacyAllPermissionAliases = new LinkedHashSet<>(); + private final ThreadLocal legacyBulkDispatch = ThreadLocal.withInitial(() -> Boolean.FALSE); public PlayerCommandHandler(AdvancedCorePlugin plugin) { super(plugin); @@ -39,17 +46,47 @@ public PlayerCommandHandler(AdvancedCorePlugin plugin, String[] args, String per figureOutPlayerArg(); } - @Override - public void execute(CommandSender sender, String[] args) { - if (playerArg >= 0) { - if (args[playerArg].equalsIgnoreCase("all")) { - if (hasAllPermission(sender)) { - executeAll(sender, args); - } - return; - } - } - executeSinglePlayer(sender, args); + @Override + public boolean runCommand(CommandSender sender, String[] args) { + if (args == null) return false; + if (playerArg < 0 || playerArg >= args.length + || !"all".equalsIgnoreCase(args[playerArg])) return super.runCommand(sender, args); + String[] preservedArgs = args.clone(); + preservedArgs[playerArg] = ALL_SELECTOR_SENTINEL; + boolean legacyAlias = hasLegacyAllPermissionAlias(sender); + if (legacyAlias) legacyBulkDispatch.set(Boolean.TRUE); + try { + return super.runCommand(sender, preservedArgs); + } finally { + if (legacyAlias) legacyBulkDispatch.remove(); + } + } + + @Override + public boolean hasPerm(CommandSender sender) { + return Boolean.TRUE.equals(legacyBulkDispatch.get()) && hasLegacyAllPermissionAlias(sender) + || super.hasPerm(sender); + } + + @Override + public void execute(CommandSender sender, String[] args) { + String[] schema = getArgs(); + if (playerArg < 0 || schema == null || args == null || args.length < schema.length + || playerArg >= args.length) { + return; + } + if (args[playerArg] == null || args[playerArg].isBlank()) return; + if (args[playerArg].equalsIgnoreCase("all") || args[playerArg] == ALL_SELECTOR_SENTINEL) { + if (hasAllPermission(sender)) { + if (args[playerArg] == ALL_SELECTOR_SENTINEL) args[playerArg] = "all"; + executeAll(sender, args); + } else { + String noPermission = formatNoPerms(); + if (noPermission != null && !noPermission.isEmpty()) sendMessage(sender, noPermission); + } + return; + } + executeSinglePlayer(sender, args); } public abstract void executeAll(CommandSender sender, String[] args); @@ -57,29 +94,35 @@ public void execute(CommandSender sender, String[] args) { public abstract void executeSinglePlayer(CommandSender sender, String[] args); /** - * Checks the stronger permission required for the special {@code all} target. - * The first configured permission is treated as the granular command permission - * and receives an {@code .All} suffix. Any alternative permissions, such as an - * administrator permission, continue to act as overrides. + * Checks the complete authorization required for the special {@code all} target. + * Ordinary command authorization is required first, followed by either one of + * the dedicated bulk permissions or an explicitly configured administrator + * override. Overrides honor the handler's multiple-permission setting. * * @param sender command sender * @return whether bulk execution is authorized */ public boolean hasAllPermission(CommandSender sender) { - String permission = getPerm(); - if (permission == null || permission.isEmpty()) { - return false; + List configured = configuredPermissions(); + if (sender == null || configured.isEmpty()) return false; + if (hasLegacyAllPermissionAlias(sender)) return true; + if (!hasPerm(sender)) return false; + int limit = isAllowMultiplePermissions() ? configured.size() : 1; + for (int i = 0; i < limit; i++) { + String permission = configured.get(i); + // Keep the granular permission and its bulk node paired. A bulk node + // for one alternative must not combine with the base node of another. + if (!sender.hasPermission(permission)) continue; + if (allPermissionOverrides.contains(permission) + || sender.hasPermission(permission + ".All")) return true; } - String[] permissions = permission.split(Pattern.quote("|")); - if (sender.hasPermission(permissions[0] + ".All")) { - return true; - } - if (isAllowMultiplePermissions()) { - for (int i = 1; i < permissions.length; i++) { - if (sender.hasPermission(permissions[i])) { - return true; - } - } + return false; + } + + private boolean hasLegacyAllPermissionAlias(CommandSender sender) { + if (sender == null) return false; + for (String alias : legacyAllPermissionAliases) { + if (sender.hasPermission(alias)) return true; } return false; } @@ -89,24 +132,99 @@ public boolean hasAllPermission(CommandSender sender) { * command permission. Permission-listing commands can use this without granting * the bulk permission during ordinary command checks. * - * @return the dedicated permission for the {@code all} target + * @return dedicated permissions for the {@code all} target */ public List getAdditionalPermissions() { + List configured = configuredPermissions(); + if (configured.isEmpty()) return Collections.emptyList(); + ArrayList permissions = derivedAllPermissions(configured, isAllowMultiplePermissions()); + permissions.addAll(legacyAllPermissionAliases); + return Collections.unmodifiableList(permissions); + } + + private ArrayList derivedAllPermissions(List configured, boolean includeAlternatives) { + LinkedHashSet permissions = new LinkedHashSet<>(); + int limit = includeAlternatives ? configured.size() : 1; + for (int i = 0; i < limit; i++) { + String permission = configured.get(i); + if (!allPermissionOverrides.contains(permission)) permissions.add(permission + ".All"); + } + return new ArrayList<>(permissions); + } + + /** + * Marks configured permission alternatives that are full administrator + * overrides, rather than granular permissions which need their own + * {@code .All} node. Unknown or malformed values are ignored and can never + * grant bulk access. + * + * @param permissions configured administrator permission alternatives + * @return this handler + */ + public PlayerCommandHandler withAllPermissionOverrides(String... permissions) { + allPermissionOverrides.clear(); + List configured = configuredPermissions(); + if (permissions == null || configured.isEmpty()) return this; + for (String permission : permissions) { + if (permission != null && configured.contains(permission)) allPermissionOverrides.add(permission); + } + return this; + } + + /** + * Retains a previously published permission that represented the complete bulk + * operation by itself. Unlike a normal configured alternative, an alias never + * grants named-player execution. + */ + public PlayerCommandHandler withLegacyAllPermissionAliases(String... permissions) { + legacyAllPermissionAliases.clear(); + if (permissions == null) return this; + for (String permission : permissions) { + if (permission != null && !permission.isBlank() && !permission.endsWith(".All") + && permission.chars().noneMatch(Character::isWhitespace)) { + legacyAllPermissionAliases.add(permission); + } + } + return this; + } + + /** Returns the valid administrator alternatives used by bulk authorization. */ + public List getAllPermissionOverrides() { + return Collections.unmodifiableList(configuredAllPermissionOverrides()); + } + + private ArrayList configuredAllPermissionOverrides() { + ArrayList overrides = new ArrayList<>(); + for (String permission : configuredPermissions()) { + if (allPermissionOverrides.contains(permission)) overrides.add(permission); + } + return overrides; + } + + private List configuredPermissions() { String permission = getPerm(); - if (permission == null || permission.isEmpty()) { - return Collections.emptyList(); + if (permission == null || permission.isBlank()) return Collections.emptyList(); + String[] values = permission.split(Pattern.quote("|"), -1); + ArrayList permissions = new ArrayList<>(values.length); + for (String value : values) { + if (value.isBlank() || value.endsWith(".All") + || value.chars().anyMatch(Character::isWhitespace)) return Collections.emptyList(); + permissions.add(value); } - return Collections.singletonList(permission.split(Pattern.quote("|"))[0] + ".All"); + return permissions; } - - private void figureOutPlayerArg() { - for (int i = 0; i < getArgs().length; i++) { - if (getArgs()[i].equalsIgnoreCase("(player)")) { - playerArg = i; - return; - } - } - getPlugin().devDebug("Failed to figure out player arg number for: " + getArgs()); + + private void figureOutPlayerArg() { + playerArg = -1; + String[] args = getArgs(); + if (args == null) return; + for (int i = 0; i < args.length; i++) { + if ("(player)".equalsIgnoreCase(args[i])) { + playerArg = i; + return; + } + } + getPlugin().devDebug("Failed to figure out player arg number for: " + java.util.Arrays.toString(args)); } @Override diff --git a/AdvancedCore/src/main/java/com/bencodez/advancedcore/command/CommandLoader.java b/AdvancedCore/src/main/java/com/bencodez/advancedcore/command/CommandLoader.java index e6fd67a6be..79833f3a60 100644 --- a/AdvancedCore/src/main/java/com/bencodez/advancedcore/command/CommandLoader.java +++ b/AdvancedCore/src/main/java/com/bencodez/advancedcore/command/CommandLoader.java @@ -114,7 +114,7 @@ public void run() { }); }, null); } - }); + }); cmds.add(new CommandHandler(plugin, new String[] { "RunSQLQuery", "(List)" }, permPrefix + ".RunSQLQuery", "Execute sql query", true, true) { @@ -247,7 +247,7 @@ public void run() { } @Override - public void executeSinglePlayer(CommandSender sender, String[] args) { + public void executeSinglePlayer(CommandSender sender, String[] args) { AdvancedCoreUser user = plugin.getUserManager().getUser(args[1]); plugin.getRewardHandler().giveReward(user, args[3], new RewardOptions().setOnline(user.isOnline())); sender.sendMessage("&cGave " + args[1] + " the reward file " + args[3]); @@ -536,52 +536,27 @@ public void execute(CommandSender sender, String[] args) { }); } - cmds.add(new CommandHandler(plugin, new String[] { "User", "All", "SetData", "(text)", "(text)" }, - permPrefix + ".SetAllData", "Set all users data") { - - @Override - public void execute(CommandSender sender, String[] args) { - String data = args[4]; - if (data.equalsIgnoreCase("\"\"")) { - data = ""; - } - - final String key = args[3]; - final String value = data; - - plugin.getUserManager().forEachUserKeys((uuid, columns) -> { - AdvancedCoreUser user = plugin.getUserManager().getUser(uuid, false); - user.userDataFetechMode(UserDataFetchMode.NO_CACHE); - user.getData().setString(key, value); - }, (count) -> { - sender.sendMessage(MessageAPI.colorize("&cSet all users " + key + " to " + args[4])); - }); - } - }); - - cmds.add(new PlayerCommandHandler(plugin, new String[] { "User", "(player)", "SetData", "(text)", "(text)" }, + cmds.add(new PlayerCommandHandler(plugin, new String[] { "User", "(player)", "SetData", "(text)", "(text)" }, permPrefix + ".SetData", "Set user data") { - @Override - public void executeAll(CommandSender sender, String[] args) { - if (sender.hasPermission(permPrefix + ".SetAllData")) { - String data = args[4]; - if (data.equalsIgnoreCase("\"\"")) { - data = ""; - } - - final String key = args[3]; - final String value = data; - - plugin.getUserManager().forEachUserKeys((uuid, columns) -> { - AdvancedCoreUser user = plugin.getUserManager().getUser(uuid, false); - user.userDataFetechMode(UserDataFetchMode.NO_CACHE); - user.getData().setString(key, value); - }, (count) -> { - sender.sendMessage(MessageAPI.colorize("&cSet all users " + key + " to " + args[4])); - }); - } - } + @Override + public void executeAll(CommandSender sender, String[] args) { + String data = args[4]; + if (data.equalsIgnoreCase("\"\"")) { + data = ""; + } + + final String key = args[3]; + final String value = data; + + plugin.getUserManager().forEachUserKeys((uuid, columns) -> { + AdvancedCoreUser user = plugin.getUserManager().getUser(uuid, false); + user.userDataFetechMode(UserDataFetchMode.NO_CACHE); + user.getData().setString(key, value); + }, (count) -> { + sender.sendMessage(MessageAPI.colorize("&cSet all users " + key + " to " + args[4])); + }); + } @Override public void executeSinglePlayer(CommandSender sender, String[] args) { @@ -591,9 +566,9 @@ public void executeSinglePlayer(CommandSender sender, String[] args) { data = ""; } user.getData().setString(args[3], data); - sender.sendMessage(MessageAPI.colorize("&cSet " + args[3] + " for " + args[1] + " to " + args[4])); - } - }); + sender.sendMessage(MessageAPI.colorize("&cSet " + args[3] + " for " + args[1] + " to " + args[4])); + } + }.withLegacyAllPermissionAliases(permPrefix + ".SetAllData")); cmds.add(new CommandHandler(plugin, new String[] { "User", "(Player)", "ViewData" }, permPrefix + ".ViewData", "View playerdata") { diff --git a/AdvancedCore/src/test/java/com/bencodez/advancedcore/api/command/PlayerCommandHandlerTest.java b/AdvancedCore/src/test/java/com/bencodez/advancedcore/api/command/PlayerCommandHandlerTest.java index 9224d68e91..a9b02802a1 100644 --- a/AdvancedCore/src/test/java/com/bencodez/advancedcore/api/command/PlayerCommandHandlerTest.java +++ b/AdvancedCore/src/test/java/com/bencodez/advancedcore/api/command/PlayerCommandHandlerTest.java @@ -3,16 +3,32 @@ import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.mockStatic; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import org.bukkit.Bukkit; import org.bukkit.command.CommandSender; +import org.bukkit.entity.Player; +import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; +import org.mockito.ArgumentCaptor; +import org.mockito.MockedStatic; import com.bencodez.advancedcore.AdvancedCorePlugin; import com.bencodez.advancedcore.AdvancedCoreConfigOptions; +import com.bencodez.simpleapi.command.TabCompleteHandler; +import com.bencodez.simpleapi.scheduler.BukkitScheduler; class PlayerCommandHandlerTest { + @BeforeAll + static void registerPlayerArgument() { + TabCompleteHandler.getInstance().addTabCompleteOption("(player)", "all", "Alex"); + } @Test void forceConsoleConstructorForwardsFlag() { @@ -24,7 +40,7 @@ void forceConsoleConstructorForwardsFlag() { @Test void bulkTargetRequiresDedicatedPermission() { TestHandler handler = handler(false); - CommandSender sender = mock(CommandSender.class); + CommandSender sender = mock(Player.class); when(sender.hasPermission("example.command")).thenReturn(true); assertFalse(handler.hasAllPermission(sender)); @@ -35,8 +51,26 @@ void bulkTargetRequiresDedicatedPermission() { @Test void administratorAlternativeStillAuthorizesBulkTarget() { - TestHandler handler = handler(false); - CommandSender sender = mock(CommandSender.class); + TestHandler handler = handler(false).withOverrides("example.admin"); + CommandSender sender = mock(Player.class); + when(sender.hasPermission("example.admin")).thenReturn(true); + + assertTrue(handler.hasAllPermission(sender)); + } + + @Test + void administratorAlternativeHonorsMultiplePermissionSetting() { + TestHandler handler = handler(false, false).withOverrides("example.admin"); + CommandSender sender = mock(Player.class); + when(sender.hasPermission("example.admin")).thenReturn(true); + + assertFalse(handler.hasAllPermission(sender)); + } + + @Test + void primaryAdministratorOverrideStillAuthorizesWhenMultipleChecksDisabled() { + TestHandler handler = handler(false, false, "example.admin|example.command").withOverrides("example.admin"); + CommandSender sender = mock(Player.class); when(sender.hasPermission("example.admin")).thenReturn(true); assertTrue(handler.hasAllPermission(sender)); @@ -45,29 +79,316 @@ void administratorAlternativeStillAuthorizesBulkTarget() { @Test void exposesBulkPermissionForPermissionListings() { assertEquals(java.util.Collections.singletonList("example.command.All"), - handler(false).getAdditionalPermissions()); + handler(false).withOverrides("example.admin").getAdditionalPermissions()); + } + + @Test + void alternativeGranularPermissionNeedsItsOwnBulkPermission() { + TestHandler handler = handler(false, true, "example.command|example.alternate"); + CommandSender sender = mock(Player.class); + when(sender.hasPermission("example.alternate")).thenReturn(true); + + assertEquals(java.util.List.of("example.command.All", "example.alternate.All"), + handler.getAdditionalPermissions()); + assertFalse(handler.hasAllPermission(sender)); + + when(sender.hasPermission("example.alternate.All")).thenReturn(true); + assertTrue(handler.hasAllPermission(sender)); + } + + @Test + void bulkPermissionCannotCombineDifferentPermissionAlternatives() { + TestHandler handler = handler(false, true, "example.command|example.alternate"); + CommandSender sender = mock(Player.class); + when(sender.hasPermission("example.alternate")).thenReturn(true); + when(sender.hasPermission("example.command.All")).thenReturn(true); + + assertFalse(handler.hasAllPermission(sender)); + + when(sender.hasPermission("example.alternate.All")).thenReturn(true); + assertTrue(handler.hasAllPermission(sender)); + } + + @Test + void disabledMultipleChecksIgnoreSecondaryGranularBulkPermission() { + TestHandler handler = handler(false, false, "example.command|example.alternate"); + CommandSender sender = mock(Player.class); + when(sender.hasPermission("example.command")).thenReturn(true); + when(sender.hasPermission("example.alternate.All")).thenReturn(true); + + assertEquals(java.util.List.of("example.command.All"), handler.getAdditionalPermissions()); + assertFalse(handler.hasAllPermission(sender)); + } + + @Test + void soleConfiguredOverrideCanAuthorizeBulkWhenMultipleChecksEnabled() { + TestHandler handler = handler(false, true, "example.admin").withOverrides("example.admin"); + CommandSender sender = mock(Player.class); + when(sender.hasPermission("example.admin")).thenReturn(true); + + assertEquals(java.util.Collections.emptyList(), handler.getAdditionalPermissions()); + assertTrue(handler.hasAllPermission(sender)); + } + + @Test + void allPermissionWithoutBasePermissionDoesNotAuthorizeBulk() { + TestHandler handler = handler(false); + CommandSender sender = mock(Player.class); + when(sender.hasPermission("example.command.All")).thenReturn(true); + + assertFalse(handler.hasAllPermission(sender)); + } + + @Test + void legacyCombinedBulkAliasDoesNotGrantNamedPlayerAccess() { + TestHandler handler = handler(false).withLegacyAliases("example.legacyAll"); + CommandSender sender = mock(Player.class); + when(sender.hasPermission("example.legacyAll")).thenReturn(true); + + assertTrue(handler.hasAllPermission(sender)); + assertFalse(handler.hasPerm(sender)); + assertEquals(java.util.List.of("example.command.All", "example.admin.All", "example.legacyAll"), + handler.getAdditionalPermissions()); + } + + @Test + void dispatchRejectsAllPermissionWithoutBasePermission() { + TestContext context = context(true); + TestHandler handler = new TestHandler(context.plugin, false, "example.command|example.admin"); + CommandSender sender = mock(Player.class); + when(sender.hasPermission("example.command.All")).thenReturn(true); + + try (MockedStatic bukkit = mockStatic(Bukkit.class)) { + bukkit.when(() -> Bukkit.getPlayer("all")).thenReturn(null); + bukkit.when(Bukkit::getOnlinePlayers).thenReturn(java.util.Collections.emptySet()); + assertTrue(handler.runCommand(sender, new String[] { "user", "all" })); + } + + verify(context.scheduler, never()).runTaskAsynchronously(eq(context.plugin), org.mockito.ArgumentMatchers.any()); + verify(sender).sendMessage("§cDenied"); + assertEquals(0, handler.allExecutions); + assertEquals(0, handler.singleExecutions); + } + + @Test + void dispatchExecutesAuthorizedBulkExactlyOnce() { + TestContext context = context(true); + TestHandler handler = new TestHandler(context.plugin, false, "example.command|example.admin"); + CommandSender sender = mock(Player.class); + when(sender.hasPermission("example.command")).thenReturn(true); + when(sender.hasPermission("example.command.All")).thenReturn(true); + + try (MockedStatic bukkit = mockStatic(Bukkit.class)) { + bukkit.when(() -> Bukkit.getPlayer("all")).thenReturn(null); + bukkit.when(Bukkit::getOnlinePlayers).thenReturn(java.util.Collections.emptySet()); + assertTrue(handler.runCommand(sender, new String[] { "user", "all" })); + } + ArgumentCaptor task = ArgumentCaptor.forClass(Runnable.class); + verify(context.scheduler).runTaskAsynchronously(eq(context.plugin), task.capture()); + task.getValue().run(); + + assertEquals(1, handler.allExecutions); + assertEquals(0, handler.singleExecutions); + assertEquals("all", handler.lastBulkTarget); + } + + @Test + void dispatchPreservesAllBeforePartialPlayerNameNormalization() { + TestContext context = context(true); + TestHandler handler = new TestHandler(context.plugin, false, "example.command|example.admin"); + CommandSender sender = mock(Player.class); + Player partialMatch = mock(Player.class); + when(partialMatch.getName()).thenReturn("Sally"); + when(sender.hasPermission("example.command")).thenReturn(true); + when(sender.hasPermission("example.command.All")).thenReturn(true); + + try (MockedStatic bukkit = mockStatic(Bukkit.class)) { + bukkit.when(() -> Bukkit.getPlayer(anyString())).thenReturn(null); + bukkit.when(Bukkit::getOnlinePlayers).thenReturn(java.util.Set.of(partialMatch)); + assertTrue(handler.runCommand(sender, new String[] { "user", "all" })); + } + ArgumentCaptor task = ArgumentCaptor.forClass(Runnable.class); + verify(context.scheduler).runTaskAsynchronously(eq(context.plugin), task.capture()); + task.getValue().run(); + + assertEquals(1, handler.allExecutions); + assertEquals(0, handler.singleExecutions); + } + + @Test + void wrongCommandsAllPermissionDoesNotAuthorizeBulk() { + TestHandler handler = handler(false); + CommandSender sender = mock(Player.class); + when(sender.hasPermission("example.command")).thenReturn(true); + when(sender.hasPermission("other.command.All")).thenReturn(true); + + assertFalse(handler.hasAllPermission(sender)); + } + + @Test + void mixedCaseAllExecutesBulkExactlyOnce() { + TestHandler handler = handler(false); + CommandSender sender = mock(Player.class); + when(sender.hasPermission("example.command")).thenReturn(true); + when(sender.hasPermission("example.command.All")).thenReturn(true); + + handler.execute(sender, new String[] { "user", "ALL" }); + + assertEquals(1, handler.allExecutions); + assertEquals(0, handler.singleExecutions); + } + + @Test + void deniedBulkUsesConfiguredNoPermissionMessageWithoutSideEffects() { + TestContext context = context(true); + TestHandler handler = new TestHandler(context.plugin, false, "example.command|example.admin"); + CommandSender sender = mock(Player.class); + when(sender.hasPermission("example.command")).thenReturn(true); + + handler.execute(sender, new String[] { "user", "all" }); + + assertEquals(0, handler.allExecutions); + assertEquals(0, handler.singleExecutions); + verify(sender).sendMessage("§cDenied"); + } + + @Test + void namedPlayerExecutionIsUnchanged() { + TestHandler handler = handler(false); + CommandSender sender = mock(Player.class); + + handler.execute(sender, new String[] { "user", "Alex" }); + + assertEquals(0, handler.allExecutions); + assertEquals(1, handler.singleExecutions); + verify(sender, never()).sendMessage(anyString()); + } + + @Test + void shortArgumentsAndMissingPlayerSchemaFailClosed() { + TestHandler handler = handler(false); + CommandSender sender = mock(CommandSender.class); + + handler.execute(sender, new String[] { "user" }); + handler.execute(sender, new String[] { "user", null }); + handler.setArgs(new String[] { "user" }); + handler.execute(sender, new String[] { "user", "all" }); + + assertEquals(0, handler.allExecutions); + assertEquals(0, handler.singleExecutions); + } + + @Test + void directBulkExecutionRejectsArgumentsShorterThanSchema() { + TestContext context = context(true); + TestHandler handler = new TestHandler(context.plugin, false, "example.command", + new String[] { "user", "(player)", "mode", "(number)" }); + CommandSender sender = mock(CommandSender.class); + when(sender.hasPermission("example.command")).thenReturn(true); + when(sender.hasPermission("example.command.All")).thenReturn(true); + + handler.execute(sender, new String[] { "user", "all" }); + + assertEquals(0, handler.allExecutions); + assertEquals(0, handler.singleExecutions); + } + + @Test + void setArgsRecomputesPlayerIndexForNewSchema() { + TestHandler handler = handler(false); + CommandSender sender = mock(CommandSender.class); + when(sender.hasPermission("example.command")).thenReturn(true); + when(sender.hasPermission("example.command.All")).thenReturn(true); + + handler.setArgs(new String[] { "user", "mode", "(player)" }); + handler.execute(sender, new String[] { "user", "mode", "all" }); + + assertEquals(1, handler.allExecutions); + assertEquals(0, handler.singleExecutions); + } + + @Test + void invalidPermissionMetadataFailsClosed() { + TestHandler handler = handler(false, true, "example.command||example.admin") + .withOverrides("example.admin"); + CommandSender sender = mock(CommandSender.class); + when(sender.hasPermission(anyString())).thenReturn(true); + + assertEquals(java.util.Collections.emptyList(), handler.getAdditionalPermissions()); + assertEquals(java.util.Collections.emptyList(), handler.getAllPermissionOverrides()); + assertFalse(handler.hasAllPermission(sender)); + } + + @Test + void nullPermissionMetadataFailsClosedBeforeBaseAuthorization() { + TestHandler handler = handler(false); + handler.setPerm(null); + CommandSender sender = mock(CommandSender.class); + + assertFalse(handler.hasAllPermission(sender)); + verify(sender, never()).hasPermission(anyString()); } private TestHandler handler(boolean forceConsole) { + return handler(forceConsole, true); + } + + private TestHandler handler(boolean forceConsole, boolean multiplePermissions) { + return handler(forceConsole, multiplePermissions, "example.command|example.admin"); + } + + private TestHandler handler(boolean forceConsole, boolean multiplePermissions, String permission) { + TestContext context = context(multiplePermissions); + return new TestHandler(context.plugin, forceConsole, permission); + } + + private TestContext context(boolean multiplePermissions) { AdvancedCorePlugin plugin = mock(AdvancedCorePlugin.class); AdvancedCoreConfigOptions options = mock(AdvancedCoreConfigOptions.class); + BukkitScheduler scheduler = mock(BukkitScheduler.class); when(plugin.getOptions()).thenReturn(options); - when(options.isMultiplePermissionChecks()).thenReturn(true); - return new TestHandler(plugin, forceConsole); + when(plugin.getBukkitScheduler()).thenReturn(scheduler); + when(plugin.getLogger()).thenReturn(java.util.logging.Logger.getLogger("PlayerCommandHandlerTest")); + when(options.isMultiplePermissionChecks()).thenReturn(multiplePermissions); + when(options.getFormatNoPerms()).thenReturn("&cDenied"); + return new TestContext(plugin, scheduler); + } + + private record TestContext(AdvancedCorePlugin plugin, BukkitScheduler scheduler) { } private static final class TestHandler extends PlayerCommandHandler { - private TestHandler(AdvancedCorePlugin plugin, boolean forceConsole) { - super(plugin, new String[] { "user", "(player)" }, "example.command|example.admin", "help", true, - forceConsole); + private int allExecutions; + private int singleExecutions; + private String lastBulkTarget; + + private TestHandler(AdvancedCorePlugin plugin, boolean forceConsole, String permission) { + this(plugin, forceConsole, permission, new String[] { "user", "(player)" }); + } + + private TestHandler(AdvancedCorePlugin plugin, boolean forceConsole, String permission, String[] args) { + super(plugin, args, permission, "help", true, forceConsole); + } + + private TestHandler withOverrides(String... permissions) { + withAllPermissionOverrides(permissions); + return this; + } + + private TestHandler withLegacyAliases(String... permissions) { + withLegacyAllPermissionAliases(permissions); + return this; } @Override public void executeAll(CommandSender sender, String[] args) { + allExecutions++; + lastBulkTarget = args[1]; } @Override public void executeSinglePlayer(CommandSender sender, String[] args) { + singleExecutions++; } } } diff --git a/AdvancedCore/src/test/java/com/bencodez/advancedcore/command/CommandLoaderBulkPermissionTest.java b/AdvancedCore/src/test/java/com/bencodez/advancedcore/command/CommandLoaderBulkPermissionTest.java new file mode 100644 index 0000000000..45655b78e6 --- /dev/null +++ b/AdvancedCore/src/test/java/com/bencodez/advancedcore/command/CommandLoaderBulkPermissionTest.java @@ -0,0 +1,101 @@ +package com.bencodez.advancedcore.command; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.util.Arrays; + +import org.bukkit.entity.Player; +import org.junit.jupiter.api.Test; + +import com.bencodez.advancedcore.AdvancedCoreConfigOptions; +import com.bencodez.advancedcore.AdvancedCorePlugin; +import com.bencodez.advancedcore.api.command.CommandHandler; +import com.bencodez.advancedcore.api.command.PlayerCommandHandler; +import com.bencodez.advancedcore.api.user.UserManager; +import com.bencodez.simpleapi.scheduler.BukkitScheduler; + +class CommandLoaderBulkPermissionTest { + @Test + void setDataBulkUsesTheSharedBaseAndAllAuthorization() { + AdvancedCorePlugin plugin = mock(AdvancedCorePlugin.class); + AdvancedCoreConfigOptions options = mock(AdvancedCoreConfigOptions.class); + UserManager userManager = mock(UserManager.class); + when(plugin.getOptions()).thenReturn(options); + when(options.isMultiplePermissionChecks()).thenReturn(true); + when(plugin.getUserManager()).thenReturn(userManager); + BukkitScheduler scheduler = mock(BukkitScheduler.class); + when(plugin.getBukkitScheduler()).thenReturn(scheduler); + doAnswer(invocation -> { + invocation.getArgument(1).run(); + return null; + }).when(scheduler).runTaskAsynchronously(any(), any()); + Player sender = mock(Player.class); + when(sender.hasPermission("Example.SetData")).thenReturn(true); + when(sender.hasPermission("Example.SetData.All")).thenReturn(true); + + java.util.List commands = new CommandLoader(plugin).getBasicAdminCommands("Example"); + assertFalse(commands.stream().anyMatch(handler -> Arrays.equals(handler.getArgs(), + new String[] { "User", "All", "SetData", "(text)", "(text)" })), + "a literal all handler would bypass the shared PlayerCommandHandler gate"); + CommandHandler command = commands.stream() + .filter(handler -> Arrays.equals(handler.getArgs(), + new String[] { "User", "(player)", "SetData", "(text)", "(text)" })) + .findFirst().orElseThrow(); + assertTrue(command instanceof PlayerCommandHandler); + com.bencodez.simpleapi.command.TabCompleteHandler.getInstance() + .addTabCompleteOption("(player)"); + com.bencodez.simpleapi.command.TabCompleteHandler.getInstance().addTabCompleteOption("(text)"); + assertTrue(command.argsMatch("__advancedcore_all_selector__", 1)); + + try (org.mockito.MockedStatic bukkit = org.mockito.Mockito.mockStatic(org.bukkit.Bukkit.class)) { + bukkit.when(() -> org.bukkit.Bukkit.getPlayer(org.mockito.ArgumentMatchers.anyString())).thenReturn(null); + bukkit.when(org.bukkit.Bukkit::getOnlinePlayers).thenReturn(java.util.Collections.emptyList()); + assertTrue(command.runCommand(sender, + new String[] { "User", "all", "SetData", "rank", "trusted" })); + } + + verify(userManager).forEachUserKeys(any(), any()); + } + + @Test + void legacySetAllDataPermissionStillAuthorizesOnlyTheBulkTarget() { + AdvancedCorePlugin plugin = mock(AdvancedCorePlugin.class); + AdvancedCoreConfigOptions options = mock(AdvancedCoreConfigOptions.class); + UserManager userManager = mock(UserManager.class); + when(plugin.getOptions()).thenReturn(options); + when(options.isMultiplePermissionChecks()).thenReturn(true); + when(plugin.getUserManager()).thenReturn(userManager); + BukkitScheduler scheduler = mock(BukkitScheduler.class); + when(plugin.getBukkitScheduler()).thenReturn(scheduler); + doAnswer(invocation -> { + invocation.getArgument(1).run(); + return null; + }).when(scheduler).runTaskAsynchronously(any(), any()); + Player sender = mock(Player.class); + when(sender.hasPermission("Example.SetAllData")).thenReturn(true); + PlayerCommandHandler command = (PlayerCommandHandler) new CommandLoader(plugin).getBasicAdminCommands("Example") + .stream().filter(handler -> Arrays.equals(handler.getArgs(), + new String[] { "User", "(player)", "SetData", "(text)", "(text)" })) + .findFirst().orElseThrow(); + com.bencodez.simpleapi.command.TabCompleteHandler.getInstance() + .addTabCompleteOption("(player)"); + com.bencodez.simpleapi.command.TabCompleteHandler.getInstance().addTabCompleteOption("(text)"); + assertTrue(command.argsMatch("__advancedcore_all_selector__", 1)); + + try (org.mockito.MockedStatic bukkit = org.mockito.Mockito.mockStatic(org.bukkit.Bukkit.class)) { + bukkit.when(() -> org.bukkit.Bukkit.getPlayer(org.mockito.ArgumentMatchers.anyString())).thenReturn(null); + bukkit.when(org.bukkit.Bukkit::getOnlinePlayers).thenReturn(java.util.Collections.emptyList()); + assertTrue(command.runCommand(sender, + new String[] { "User", "all", "SetData", "rank", "trusted" })); + } + + verify(userManager).forEachUserKeys(any(), any()); + assertFalse(command.hasPerm(sender)); + } +}