+ * 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/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 // =========================================================================