diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml
index 40622d7..1ee2a69 100644
--- a/.github/workflows/publish.yml
+++ b/.github/workflows/publish.yml
@@ -18,7 +18,7 @@ on:
jobs:
publish:
- uses: bentoboxworld/.github/.github/workflows/publish-platforms.yml@fe4b1f03c19f4fd4212020a06a07a7097923adec # master
+ uses: bentoboxworld/.github/.github/workflows/publish-platforms.yml@ca2dcd167e8db4e0f671a976080744dda43801a6 # master
with:
use_release_asset: "true" # publish the jar attached to the release; do not rebuild
hangar_slug: "AOneBlock" # blank = skip Hangar
diff --git a/pom.xml b/pom.xml
index 13548cd..65440e6 100644
--- a/pom.xml
+++ b/pom.xml
@@ -67,7 +67,7 @@
+ * The full magic-block processing runs at {@link EventPriority#HIGHEST} so that + * other protection plugins get a chance to cancel first. However, reward-granting + * plugins such as Jobs Reborn also listen at {@code HIGHEST} with + * {@code ignoreCancelled = true}. Within a single priority the execution order is + * just plugin-registration order, so Jobs could pay out before our + * {@code HIGHEST} handler cancels the break. Because the magic block respawns when + * the break is cancelled, that let players mine it endlessly for infinite rewards. + *
+ * Cancelling the denied break here, at {@link EventPriority#LOWEST}, guarantees it + * happens before any {@code ignoreCancelled = true} handler at a later priority, so + * those plugins are skipped and no reward is granted. + * + * @param e The BlockBreakEvent. + * @see Issue #534 + */ + @EventHandler(priority = EventPriority.LOWEST, ignoreCancelled = true) + public void onBlockBreakDeny(final BlockBreakEvent e) { + if (!addon.inWorld(e.getBlock().getWorld())) { + return; + } + Location l = e.getBlock().getLocation(); + // checkIsland cancels the event and sends the protection message if the player + // is not allowed to break the magic block. + addon.getIslands().getIslandAt(l).filter(i -> l.equals(i.getCenter())) + .ifPresent(i -> checkIsland(e, e.getPlayer(), i.getCenter(), addon.MAGIC_BLOCK)); + } + /** * Handles the breaking of the magic block by a player. * @param e The BlockBreakEvent. diff --git a/src/main/java/world/bentobox/aoneblock/listeners/BossBarListener.java b/src/main/java/world/bentobox/aoneblock/listeners/BossBarListener.java index 966d8d8..ece5aa2 100644 --- a/src/main/java/world/bentobox/aoneblock/listeners/BossBarListener.java +++ b/src/main/java/world/bentobox/aoneblock/listeners/BossBarListener.java @@ -91,6 +91,11 @@ public static Component bukkitToAdventure(String legacyString) { } private void tryToShowActionBar(UUID uuid, Island island) { + // The listener is registered whenever either the boss bar or action bar flag is + // registered, so the global setting must be checked here as well as the flag + if (!addon.getSettings().isActionBar()) { + return; + } User user = User.getInstance(uuid); Player player = Bukkit.getPlayer(uuid); @@ -125,6 +130,11 @@ private void tryToShowActionBar(UUID uuid, Island island) { * @param island island they are on */ private void tryToShowBossBar(UUID uuid, Island island) { + // The listener is registered whenever either the boss bar or action bar flag is + // registered, so the global setting must be checked here as well as the flag + if (!addon.getSettings().isBossBar()) { + return; + } User user = User.getInstance(uuid); // Only show if enabled for island diff --git a/src/test/java/world/bentobox/aoneblock/listeners/BlockListenerTest2.java b/src/test/java/world/bentobox/aoneblock/listeners/BlockListenerTest2.java index 275908f..0062193 100644 --- a/src/test/java/world/bentobox/aoneblock/listeners/BlockListenerTest2.java +++ b/src/test/java/world/bentobox/aoneblock/listeners/BlockListenerTest2.java @@ -13,8 +13,11 @@ import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.atLeastOnce; +import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; +import static org.mockito.Mockito.spy; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -42,6 +45,8 @@ import org.bukkit.block.data.Brushable; import org.bukkit.entity.EntityType; import org.bukkit.entity.Item; +import org.bukkit.event.EventHandler; +import org.bukkit.event.EventPriority; import org.bukkit.event.block.Action; import org.bukkit.event.block.BlockBreakEvent; import org.bukkit.event.entity.EntityInteractEvent; @@ -1050,6 +1055,107 @@ void testOnBlockBreakByMinionNotInWorld() { verify(im, never()).getIslandAt(any()); } + // ========================================================================= + // onBlockBreakDeny — early cancel to stop reward exploits (issue #534) + // ========================================================================= + + /** + * The early-deny handler must run at {@link EventPriority#LOWEST} with + * {@code ignoreCancelled = true}. Reward-granting plugins such as Jobs Reborn + * listen for the {@link BlockBreakEvent} at {@code HIGHEST} with + * {@code ignoreCancelled = true}; cancelling here, before them, guarantees they + * are skipped when the player lacks the magic-block permission. + * See https://github.com/BentoBoxWorld/AOneBlock/issues/534 + */ + @Test + void testOnBlockBreakDenyRegisteredAtLowestPriority() throws NoSuchMethodException { + EventHandler eh = BlockListener.class.getMethod("onBlockBreakDeny", BlockBreakEvent.class) + .getAnnotation(EventHandler.class); + assertNotNull(eh); + assertEquals(EventPriority.LOWEST, eh.priority()); + assertTrue(eh.ignoreCancelled()); + } + + /** + * Test method for + * {@link world.bentobox.aoneblock.listeners.BlockListener#onBlockBreakDeny(BlockBreakEvent)} + * When the player lacks the MAGIC_BLOCK permission the break is cancelled at this + * early stage (via checkIsland), so later reward plugins are skipped. Regression + * test for https://github.com/BentoBoxWorld/AOneBlock/issues/534 + */ + @Test + void testOnBlockBreakDenyCancelsWhenNotAllowed() { + BlockListener spyBl = spy(bl); + // Emulate FlagListener.checkIsland's deny behaviour: cancel the event and return false. + doAnswer(inv -> { + ((BlockBreakEvent) inv.getArgument(0)).setCancelled(true); + return false; + }).when(spyBl).checkIsland(any(), any(), any(), any()); + + BlockBreakEvent e = new BlockBreakEvent(magicBlock, mockPlayer); + spyBl.onBlockBreakDeny(e); + + assertTrue(e.isCancelled()); + // The flag was checked against the island centre for the breaking player. + verify(spyBl).checkIsland(any(), eq(mockPlayer), eq(location), any()); + } + + /** + * Test method for + * {@link world.bentobox.aoneblock.listeners.BlockListener#onBlockBreakDeny(BlockBreakEvent)} + * When the player is allowed, the early handler leaves the event untouched so that + * normal magic-block processing and legitimate rewards proceed. + */ + @Test + void testOnBlockBreakDenyAllowsWhenPermitted() { + BlockListener spyBl = spy(bl); + doReturn(true).when(spyBl).checkIsland(any(), any(), any(), any()); + + BlockBreakEvent e = new BlockBreakEvent(magicBlock, mockPlayer); + spyBl.onBlockBreakDeny(e); + + assertFalse(e.isCancelled()); + } + + /** + * Test method for + * {@link world.bentobox.aoneblock.listeners.BlockListener#onBlockBreakDeny(BlockBreakEvent)} + * Not in an addon world → early return, the flag is never checked. + */ + @Test + void testOnBlockBreakDenyNotInWorld() { + when(addon.inWorld(world)).thenReturn(false); + BlockListener spyBl = spy(bl); + + BlockBreakEvent e = new BlockBreakEvent(magicBlock, mockPlayer); + spyBl.onBlockBreakDeny(e); + + assertFalse(e.isCancelled()); + verify(spyBl, never()).checkIsland(any(), any(), any(), any()); + } + + /** + * Test method for + * {@link world.bentobox.aoneblock.listeners.BlockListener#onBlockBreakDeny(BlockBreakEvent)} + * Block is in world but is not the island centre (magic block) → the flag is never + * checked, so ordinary block breaking elsewhere on the island is unaffected. + */ + @Test + void testOnBlockBreakDenyNotCenterBlock() { + Block other = mock(Block.class); + when(other.getWorld()).thenReturn(world); + Location otherLoc = mock(Location.class); + when(other.getLocation()).thenReturn(otherLoc); + when(im.getIslandAt(otherLoc)).thenReturn(Optional.of(island)); + BlockListener spyBl = spy(bl); + + BlockBreakEvent e = new BlockBreakEvent(other, mockPlayer); + spyBl.onBlockBreakDeny(e); + + assertFalse(e.isCancelled()); + verify(spyBl, never()).checkIsland(any(), any(), any(), any()); + } + // ========================================================================= // onBlockBreak(PlayerBucketFillEvent) guard tests // ========================================================================= diff --git a/src/test/java/world/bentobox/aoneblock/listeners/BossBarListenerTest.java b/src/test/java/world/bentobox/aoneblock/listeners/BossBarListenerTest.java new file mode 100644 index 0000000..8bc4bec --- /dev/null +++ b/src/test/java/world/bentobox/aoneblock/listeners/BossBarListenerTest.java @@ -0,0 +1,144 @@ +package world.bentobox.aoneblock.listeners; + +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.doNothing; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.util.Collections; + +import org.bukkit.Bukkit; +import org.bukkit.Material; +import org.bukkit.block.Block; +import org.bukkit.boss.BossBar; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.mockito.Mock; + +import net.kyori.adventure.text.Component; +import world.bentobox.aoneblock.AOneBlock; +import world.bentobox.aoneblock.CommonTestSetup; +import world.bentobox.aoneblock.Settings; +import world.bentobox.aoneblock.dataobjects.OneBlockIslands; +import world.bentobox.aoneblock.events.MagicBlockEvent; +import world.bentobox.aoneblock.oneblocks.OneBlocksManager; + +/** + * Tests the display of the boss bar and action bar, and in particular that the + * config.yml settings turn them off (https://github.com/BentoBoxWorld/AOneBlock/issues/537). + * @author tastybento + */ +public class BossBarListenerTest extends CommonTestSetup { + + private AOneBlock addon; + private Settings settings; + private BossBarListener bbl; + @Mock + private OneBlocksManager obm; + @Mock + private OneBlockIslands obi; + @Mock + private BossBar bossBar; + @Mock + private Block block; + + /** + */ + @Override + @BeforeEach + public void setUp() throws Exception { + super.setUp(); + + addon = spy(new AOneBlock()); + settings = new Settings(); + addon.setSettings(settings); + doNothing().when(addon).logError(anyString()); + doReturn(obm).when(addon).getOneBlockManager(); + doReturn(obi).when(addon).getOneBlocksIsland(any()); + + // Phase progress + when(obi.getPhaseName()).thenReturn("Plains"); + when(obm.getNextPhaseBlocks(obi)).thenReturn(100); + when(obm.getPhaseBlocks(obi)).thenReturn(500); + when(obm.getPercentageDone(obi)).thenReturn(80D); + + // Bukkit + mockedBukkit.when(() -> Bukkit.getPlayer(uuid)).thenReturn(mockPlayer); + mockedBukkit.when(() -> Bukkit.createBossBar(anyString(), any(), any())).thenReturn(bossBar); + when(bossBar.getPlayers()).thenReturn(Collections.emptyList()); + + bbl = new BossBarListener(addon); + } + + /** + */ + @Override + @AfterEach + public void tearDown() throws Exception { + super.tearDown(); + } + + private void fireMagicBlockEvent() { + bbl.onBreakBlockEvent(new MagicBlockEvent(island, uuid, null, block, Material.STONE)); + } + + /** + * Test that the action bar is shown when enabled in the config and allowed on the island. + */ + @Test + void testActionBarShownWhenEnabled() { + when(island.isAllowed(addon.ONEBLOCK_ACTIONBAR)).thenReturn(true); + fireMagicBlockEvent(); + verify(mockPlayer).sendActionBar(any(Component.class)); + } + + /** + * Test for https://github.com/BentoBoxWorld/AOneBlock/issues/537 - the action bar + * must not be shown when disabled in config.yml, even though the boss bar flag has + * registered the listener. + */ + @Test + void testActionBarNotShownWhenDisabledInConfig() { + settings.setActionBar(false); + when(island.isAllowed(addon.ONEBLOCK_ACTIONBAR)).thenReturn(true); + fireMagicBlockEvent(); + verify(mockPlayer, never()).sendActionBar(any(Component.class)); + } + + /** + * Test that the action bar is not shown when the island flag denies it. + */ + @Test + void testActionBarNotShownWhenFlagDenied() { + // island.isAllowed is false by default in CommonTestSetup + fireMagicBlockEvent(); + verify(mockPlayer, never()).sendActionBar(any(Component.class)); + } + + /** + * Test that the boss bar is shown when enabled in the config and allowed on the island. + */ + @Test + void testBossBarShownWhenEnabled() { + when(island.isAllowed(addon.ONEBLOCK_BOSSBAR)).thenReturn(true); + fireMagicBlockEvent(); + verify(bossBar).addPlayer(mockPlayer); + } + + /** + * Test that the boss bar is not shown when disabled in config.yml. + */ + @Test + void testBossBarNotShownWhenDisabledInConfig() { + settings.setBossBar(false); + when(island.isAllowed(addon.ONEBLOCK_BOSSBAR)).thenReturn(true); + fireMagicBlockEvent(); + mockedBukkit.verify(() -> Bukkit.createBossBar(anyString(), any(), any()), never()); + verify(bossBar, never()).addPlayer(any()); + } +}