diff --git a/README.md b/README.md index af5add4..7369655 100644 --- a/README.md +++ b/README.md @@ -7,9 +7,42 @@ Butterfly is a simple minecraft paper plugin and minestom extension/api that all ## Features - Set scoreboard teams - Set chat formats -- Chat color support +- Chat formatting (MiniMessage) restricted by permission, see [Chat tag permissions](#chat-tag-permissions) +## Chat tag permissions +Chat messages on Paper and Minestom are parsed as MiniMessage, but a player may only use the tags they have a permission for. Permissions are resolved through LuckPerms, one node per tag type: `butterfly.chat.tag.`. Grant them per rank through LuckPerms group permissions; `butterfly.chat.tag.*` grants every type. + +| Permission | Tags | +|------------|------| +| `butterfly.chat.tag.color` | named colours, hex colours, `` | +| `butterfly.chat.tag.decoration` | ``, ``, ``, ``, `` and their short forms | +| `butterfly.chat.tag.gradient` | `` | +| `butterfly.chat.tag.rainbow` | `` | +| `butterfly.chat.tag.transition` | `` | +| `butterfly.chat.tag.pride` | `` | +| `butterfly.chat.tag.shadow` | `` | +| `butterfly.chat.tag.font` | `` | +| `butterfly.chat.tag.reset` | `` | +| `butterfly.chat.tag.newline` | `` / `
` | +| `butterfly.chat.tag.click` | `` | +| `butterfly.chat.tag.hover` | `` | +| `butterfly.chat.tag.insertion` | `` | +| `butterfly.chat.tag.keybind` | `` | +| `butterfly.chat.tag.translatable` | ``, ``, `` | +| `butterfly.chat.tag.selector` | `` | +| `butterfly.chat.tag.score` | `` | +| `butterfly.chat.tag.nbt` | `` | +| `butterfly.chat.tag.sprite` | `` | +| `butterfly.chat.tag.head` | `` | + +- A tag the sender has no permission for stays in the message as literal text, exactly as typed. +- The sender's prefix and name keep their MiniMessage formatting regardless of these permissions. +- Every viewer sees the same message; it depends only on the sender's permissions. + +### Upgrading to restricted chat tags +Without any `butterfly.chat.tag.*` permission, players lose all chat formatting they had before. To restore the old behaviour, run `lp group default permission set butterfly.chat.tag.* true`. + ## Minestom extension `butterfly-minestom.jar` is a Minestom extension (name `Butterfly`) and still works as a library. diff --git a/api/build.gradle.kts b/api/build.gradle.kts index 66b9d87..42bdb2f 100644 --- a/api/build.gradle.kts +++ b/api/build.gradle.kts @@ -3,13 +3,21 @@ plugins { } dependencies { + compileOnly(platform(libs.mycelium.bom)) compileOnly(libs.luckperms.api) // provided by Paper and Minestom at runtime + compileOnly(libs.adventure.api) + compileOnly(libs.adventure.minimessage) + // provided by Paper and Minestom at runtime compileOnly(libs.slf4j.api) implementation(libs.avaje.config) testImplementation(platform(libs.mycelium.bom)) testImplementation(libs.slf4j.api) + testImplementation(libs.adventure.api) + testImplementation(libs.adventure.minimessage) + testImplementation(libs.adventure.plain) + testImplementation(libs.luckperms.api) testImplementation(libs.junit.api) testImplementation(libs.junit.platform.launcher) testImplementation(libs.junit.params) diff --git a/api/src/main/java/net/onelitefeather/butterfly/api/LuckPermsAPI.java b/api/src/main/java/net/onelitefeather/butterfly/api/LuckPermsAPI.java index 8b20670..033ff97 100644 --- a/api/src/main/java/net/onelitefeather/butterfly/api/LuckPermsAPI.java +++ b/api/src/main/java/net/onelitefeather/butterfly/api/LuckPermsAPI.java @@ -69,6 +69,31 @@ default Optional getPlayerPrefix(UUID playerUUID) { return getGroupPrefix(getPrimaryGroup(playerUUID)); } + /** + * Checks a permission against the user's cached permission data, using the user's own contextual query options. + * This reads cached data only, so it is safe to call from any thread. + * + * @param user the player's LuckPerms user, may be null + * @param permission the permission node + * @return true if the permission is granted, false if it is not or the user is null + */ + default boolean hasPermission(User user, String permission) { + if (user == null) return false; + return user.getCachedData().getPermissionData(user.getQueryOptions()) + .queryPermission(permission).result().asBoolean(); + } + + /** + * Same as {@link #hasPermission(User, String)} for a player identified by UUID. + * + * @param playerUUID the player's UUID + * @param permission the permission node + * @return true if the permission is granted, false if it is not or the user is not loaded + */ + default boolean hasPermission(UUID playerUUID, String permission) { + return hasPermission(getUser(playerUUID), permission); + } + default User getUser(UUID uuid) { return LuckPermsProvider.get().getUserManager().getUser(uuid); } diff --git a/api/src/main/java/net/onelitefeather/butterfly/api/chat/ChatMessageParser.java b/api/src/main/java/net/onelitefeather/butterfly/api/chat/ChatMessageParser.java new file mode 100644 index 0000000..c840359 --- /dev/null +++ b/api/src/main/java/net/onelitefeather/butterfly/api/chat/ChatMessageParser.java @@ -0,0 +1,37 @@ +package net.onelitefeather.butterfly.api.chat; + +import net.kyori.adventure.text.Component; +import net.kyori.adventure.text.minimessage.MiniMessage; +import net.kyori.adventure.text.minimessage.tag.resolver.TagResolver; + +import java.util.function.Predicate; + +/** + * Turns the raw text of a chat message into a component, interpreting only the MiniMessage tags the sender may + * use. Tags that are not allowed stay in the message as literal text. + */ +public final class ChatMessageParser { + + /** + * Knows no tags by itself; the tags a sender may use are added per call. + */ + private static final MiniMessage MINI_MESSAGE = MiniMessage.builder().tags(TagResolver.empty()).build(); + + private ChatMessageParser() { + } + + /** + * @param raw the message as the player typed it + * @param hasPermission checks a permission node of the sender, for example {@code butterfly.chat.tag.color} + * @return the message with the tags of all permitted {@link ChatTagType}s interpreted + */ + public static Component parse(String raw, Predicate hasPermission) { + TagResolver.Builder allowed = TagResolver.builder(); + for (ChatTagType tagType : ChatTagType.values()) { + if (hasPermission.test(tagType.node())) { + allowed.resolver(tagType.resolver()); + } + } + return MINI_MESSAGE.deserialize(raw, allowed.build()); + } +} diff --git a/api/src/main/java/net/onelitefeather/butterfly/api/chat/ChatTagType.java b/api/src/main/java/net/onelitefeather/butterfly/api/chat/ChatTagType.java new file mode 100644 index 0000000..ff52df6 --- /dev/null +++ b/api/src/main/java/net/onelitefeather/butterfly/api/chat/ChatTagType.java @@ -0,0 +1,67 @@ +package net.onelitefeather.butterfly.api.chat; + +import net.kyori.adventure.text.minimessage.tag.resolver.TagResolver; +import net.kyori.adventure.text.minimessage.tag.standard.StandardTags; + +import java.util.function.Supplier; + +/** + * The MiniMessage tag types a chat sender can be allowed to use. Each type maps to one permission node + * ({@link #node()}) and to the tag resolvers that make its tags work. + */ +public enum ChatTagType { + COLOR("color", StandardTags::color), + DECORATION("decoration", StandardTags::decorations), + GRADIENT("gradient", StandardTags::gradient), + RAINBOW("rainbow", StandardTags::rainbow), + TRANSITION("transition", StandardTags::transition), + PRIDE("pride", StandardTags::pride), + SHADOW("shadow", StandardTags::shadowColor), + FONT("font", StandardTags::font), + RESET("reset", StandardTags::reset), + NEWLINE("newline", StandardTags::newline), + CLICK("click", StandardTags::clickEvent), + HOVER("hover", StandardTags::hoverEvent), + INSERTION("insertion", StandardTags::insertion), + KEYBIND("keybind", StandardTags::keybind), + TRANSLATABLE("translatable", () -> TagResolver.resolver(StandardTags.translatable(), StandardTags.translatableFallback())), + SELECTOR("selector", StandardTags::selector), + SCORE("score", StandardTags::score), + NBT("nbt", StandardTags::nbt), + SPRITE("sprite", StandardTags::sprite), + HEAD("head", StandardTags::sequentialHead); + + /** + * Prefix shared by all tag permission nodes; {@code butterfly.chat.tag.*} grants every type. + */ + public static final String NODE_PREFIX = "butterfly.chat.tag."; + + private final String type; + private final TagResolver resolver; + + ChatTagType(String type, Supplier resolver) { + this.type = type; + this.resolver = resolver.get(); + } + + /** + * @return the lower-case type name used in the permission node + */ + public String type() { + return type; + } + + /** + * @return the permission node a sender needs to use this tag type + */ + public String node() { + return NODE_PREFIX + type; + } + + /** + * @return the resolver that interprets the tags of this type + */ + public TagResolver resolver() { + return resolver; + } +} diff --git a/api/src/test/java/net/onelitefeather/butterfly/api/LuckPermsAPIPermissionTest.java b/api/src/test/java/net/onelitefeather/butterfly/api/LuckPermsAPIPermissionTest.java new file mode 100644 index 0000000..a5428a9 --- /dev/null +++ b/api/src/test/java/net/onelitefeather/butterfly/api/LuckPermsAPIPermissionTest.java @@ -0,0 +1,94 @@ +package net.onelitefeather.butterfly.api; + +import net.luckperms.api.cacheddata.CachedDataManager; +import net.luckperms.api.cacheddata.CachedPermissionData; +import net.luckperms.api.cacheddata.Result; +import net.luckperms.api.model.user.User; +import net.luckperms.api.query.QueryOptions; +import net.luckperms.api.util.Tristate; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; + +import java.lang.reflect.Proxy; +import java.util.List; +import java.util.Set; +import java.util.concurrent.CopyOnWriteArrayList; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertTrue; + +class LuckPermsAPIPermissionTest { + + private final LuckPermsAPI api = LuckPermsAPI.luckPermsAPI(); + private final QueryOptions userOptions = stub(QueryOptions.class, (_, _) -> null); + private final List queriedWith = new CopyOnWriteArrayList<>(); + + @Test + @DisplayName("a permission the user's cached data grants is reported as granted") + void grantedPermissionIsTrue() { + User user = userWith(Set.of("butterfly.chat.tag.color")); + + assertTrue(api.hasPermission(user, "butterfly.chat.tag.color"), "granted node must be true"); + } + + @Test + @DisplayName("a permission the user does not have is reported as not granted") + void missingPermissionIsFalse() { + User user = userWith(Set.of("butterfly.chat.tag.color")); + + assertFalse(api.hasPermission(user, "butterfly.chat.tag.click"), "ungranted node must be false"); + } + + @Test + @DisplayName("the check queries the cached permission data with the user's own query options") + void usesUsersContextualQueryOptions() { + User user = userWith(Set.of("butterfly.chat.tag.color")); + + api.hasPermission(user, "butterfly.chat.tag.color"); + + assertEquals(1, queriedWith.size(), "permission data must be requested exactly once"); + assertSame(userOptions, queriedWith.getFirst(), "the user's query options must be passed through"); + } + + @Test + @DisplayName("a user that is not loaded has no permission") + void unloadedUserHasNoPermission() { + assertFalse(api.hasPermission((User) null, "butterfly.chat.tag.color"), "null user must be false"); + } + + private User userWith(Set granted) { + CachedPermissionData permissions = stub(CachedPermissionData.class, (name, args) -> { + if (!name.equals("queryPermission")) return null; + Tristate state = Tristate.of(granted.contains((String) args[0])); + return stub(Result.class, (resultMethod, _) -> resultMethod.equals("result") ? state : null); + }); + CachedDataManager data = stub(CachedDataManager.class, (name, args) -> { + if (!name.equals("getPermissionData")) return null; + queriedWith.add((QueryOptions) args[0]); + return permissions; + }); + return stub(User.class, (name, _) -> switch (name) { + case "getCachedData" -> data; + case "getQueryOptions" -> userOptions; + default -> null; + }); + } + + private interface Answer { + Object answer(String method, Object[] args); + } + + private static T stub(Class type, Answer answer) { + return type.cast(Proxy.newProxyInstance(type.getClassLoader(), new Class[]{type}, (proxy, method, args) -> { + Object[] arguments = args == null ? new Object[0] : args; + return switch (method.getName()) { + case "toString" -> type.getSimpleName() + "Stub"; + case "hashCode" -> System.identityHashCode(proxy); + case "equals" -> proxy == arguments[0]; + default -> answer.answer(method.getName(), arguments); + }; + })); + } +} diff --git a/api/src/test/java/net/onelitefeather/butterfly/api/chat/ChatMessageParserTest.java b/api/src/test/java/net/onelitefeather/butterfly/api/chat/ChatMessageParserTest.java new file mode 100644 index 0000000..2980860 --- /dev/null +++ b/api/src/test/java/net/onelitefeather/butterfly/api/chat/ChatMessageParserTest.java @@ -0,0 +1,159 @@ +package net.onelitefeather.butterfly.api.chat; + +import net.kyori.adventure.text.Component; +import net.kyori.adventure.text.TextComponent; +import net.kyori.adventure.text.event.ClickEvent; +import net.kyori.adventure.text.format.NamedTextColor; +import net.kyori.adventure.text.format.TextColor; +import net.kyori.adventure.text.format.TextDecoration; +import net.kyori.adventure.text.serializer.plain.PlainTextComponentSerializer; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; + +import java.util.ArrayList; +import java.util.HashSet; +import java.util.List; +import java.util.Set; +import java.util.function.Predicate; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +class ChatMessageParserTest { + + private static final Predicate NOTHING = _ -> false; + private static final Predicate EVERYTHING = _ -> true; + + private static Predicate granting(String... types) { + Set nodes = new HashSet<>(); + for (String type : types) nodes.add("butterfly.chat.tag." + type); + return nodes::contains; + } + + private static String plain(Component component) { + return PlainTextComponentSerializer.plainText().serialize(component); + } + + /** + * All text components of the tree whose own content is not empty, in order. + */ + private static List leaves(Component component) { + List result = new ArrayList<>(); + collect(component, result); + return result; + } + + private static void collect(Component component, List into) { + if (component instanceof TextComponent text && !text.content().isEmpty()) into.add(text); + component.children().forEach(child -> collect(child, into)); + } + + private static TextComponent leaf(Component component, String content) { + return leaves(component).stream() + .filter(text -> text.content().equals(content)) + .findFirst() + .orElseThrow(() -> new AssertionError("no component with content '" + content + "' in " + component)); + } + + private static boolean carriesClickEvent(Component component) { + return component.clickEvent() != null || component.children().stream().anyMatch(ChatMessageParserTest::carriesClickEvent); + } + + // Requirement: Tags require a permission per tag type + + @Test + @DisplayName("a permitted colour tag colours the text and is consumed") + void permittedColourIsApplied() { + Component result = ChatMessageParser.parse("hello", granting("color")); + + assertEquals("hello", plain(result), "the tag must not appear in the text"); + assertEquals(NamedTextColor.RED, leaf(result, "hello").color(), "the text must be red"); + } + + @Test + @DisplayName("permission for one type does not grant another type") + void permissionForOneTypeDoesNotGrantAnother() { + Component result = ChatMessageParser.parse("x", granting("color")); + + assertFalse(carriesClickEvent(result), "no click event may be present"); + assertEquals("x", plain(result), "the tag must stay literal"); + } + + @Test + @DisplayName("a granted click type produces a click event") + void permittedClickProducesClickEvent() { + Component result = ChatMessageParser.parse("x", granting("click")); + + assertEquals("x", plain(result), "the tag must not appear in the text"); + ClickEvent click = leaf(result, "x").clickEvent(); + assertNotNull(click, "a click event must be present"); + assertEquals(ClickEvent.Action.RUN_COMMAND, click.action(), "the click action must be run_command"); + } + + @Test + @DisplayName("every type granted renders the rainbow effect") + void everyTypeGrantedRendersRainbow() { + Component result = ChatMessageParser.parse("hi", EVERYTHING); + + assertEquals("hi", plain(result), "the tag must not appear in the text"); + Set colours = new HashSet<>(); + leaves(result).forEach(text -> colours.add(text.color())); + assertFalse(colours.contains(null), "every character must be coloured"); + assertTrue(colours.size() > 1, "the characters must differ in colour, got " + colours); + } + + @Test + @DisplayName("a granted gradient type renders the gradient") + void permittedGradientIsApplied() { + Component result = ChatMessageParser.parse("hi", granting("gradient")); + + assertEquals("hi", plain(result), "the tag must not appear in the text"); + assertEquals(NamedTextColor.RED.value(), leaf(result, "h").color().value(), "the gradient must start red"); + assertEquals(NamedTextColor.BLUE.value(), leaf(result, "i").color().value(), "the gradient must end blue"); + } + + @Test + @DisplayName("the parser asks for the node named after the tag type") + void asksForNodeOfTagType() { + List asked = new ArrayList<>(); + Predicate recording = node -> { + asked.add(node); + return false; + }; + + ChatMessageParser.parse("x", recording); + + assertTrue(asked.contains("butterfly.chat.tag.color"), "node butterfly.chat.tag.color must be checked, got " + asked); + } + + // Requirement: Disallowed tags stay as literal text + + @Test + @DisplayName("without any permission the message is the typed text") + void noPermissionsKeepsTextLiteral() { + String raw = "hello a < b"; + + Component result = ChatMessageParser.parse(raw, NOTHING); + + assertEquals(raw, plain(result), "the text must be unchanged"); + assertTrue(leaves(result).stream().allMatch(text -> text.color() == null), "no colour may be applied"); + } + + @Test + @DisplayName("an allowed tag is interpreted while a disallowed tag in the same message stays literal") + void mixedAllowedAndDisallowedTags() { + Component result = ChatMessageParser.parse("hi there", granting("decoration")); + + assertEquals("hi there", plain(result), "only the bold tag may be consumed"); + assertEquals(TextDecoration.State.TRUE, leaf(result, "hi").decoration(TextDecoration.BOLD), "hi must be bold"); + assertEquals(TextDecoration.State.NOT_SET, leaf(result, " there").decoration(TextDecoration.BOLD), "the rest must not be bold"); + } + + @Test + @DisplayName("plain text without tags is returned as typed") + void plainTextIsUnchanged() { + assertEquals("just text", plain(ChatMessageParser.parse("just text", NOTHING)), "text must be unchanged"); + } +} diff --git a/api/src/test/java/net/onelitefeather/butterfly/api/chat/ChatTagTypeTest.java b/api/src/test/java/net/onelitefeather/butterfly/api/chat/ChatTagTypeTest.java new file mode 100644 index 0000000..c9fb4e4 --- /dev/null +++ b/api/src/test/java/net/onelitefeather/butterfly/api/chat/ChatTagTypeTest.java @@ -0,0 +1,31 @@ +package net.onelitefeather.butterfly.api.chat; + +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; + +import java.util.Arrays; +import java.util.List; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +class ChatTagTypeTest { + + @Test + @DisplayName("the tag types are exactly the ones of the spec table, in order") + void typesMatchSpecTable() { + List expected = List.of("color", "decoration", "gradient", "rainbow", "transition", "pride", "shadow", + "font", "reset", "newline", "click", "hover", "insertion", "keybind", "translatable", "selector", + "score", "nbt", "sprite", "head"); + + List actual = Arrays.stream(ChatTagType.values()).map(ChatTagType::type).toList(); + + assertEquals(expected, actual, "the enum must list every type of the spec table"); + } + + @Test + @DisplayName("a tag type's permission node is butterfly.chat.tag. followed by the type") + void nodeIsPrefixPlusType() { + assertEquals("butterfly.chat.tag.shadow", ChatTagType.SHADOW.node(), "node of shadow"); + assertEquals("butterfly.chat.tag.head", ChatTagType.HEAD.node(), "node of head"); + } +} diff --git a/api/src/test/java/net/onelitefeather/butterfly/api/chat/StandardTagsCoverageTest.java b/api/src/test/java/net/onelitefeather/butterfly/api/chat/StandardTagsCoverageTest.java new file mode 100644 index 0000000..717420d --- /dev/null +++ b/api/src/test/java/net/onelitefeather/butterfly/api/chat/StandardTagsCoverageTest.java @@ -0,0 +1,67 @@ +package net.onelitefeather.butterfly.api.chat; + +import net.kyori.adventure.text.Component; +import net.kyori.adventure.text.minimessage.MiniMessage; +import net.kyori.adventure.text.minimessage.tag.resolver.TagResolver; +import net.kyori.adventure.text.minimessage.tag.standard.StandardTags; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; + +import java.util.stream.Stream; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; + +/** + * Confirms, for every row of the chat-tag-permissions spec table, which {@link StandardTags} factory provides the + * listed tag names: the sample is interpreted when only that factory is enabled and stays literal without it. + */ +class StandardTagsCoverageTest { + + private static final MiniMessage NO_TAGS = MiniMessage.builder().tags(TagResolver.empty()).build(); + + static Stream samples() { + return Stream.of( + row("color", StandardTags.color(), "x", "<#ff0000>x", "x"), + row("decoration", StandardTags.decorations(), "x", "x", "x", "x", "x", + "x", "x", "x", "x", "x", "x"), + row("gradient", StandardTags.gradient(), "x"), + row("rainbow", StandardTags.rainbow(), "x"), + row("transition", StandardTags.transition(), "x"), + row("pride", StandardTags.pride(), "x"), + row("shadow", StandardTags.shadowColor(), "x"), + row("font", StandardTags.font(), "x"), + row("reset", StandardTags.reset(), "x"), + row("newline", StandardTags.newline(), "xx", "x
x"), + row("click", StandardTags.clickEvent(), "x"), + row("hover", StandardTags.hoverEvent(), "x"), + row("insertion", StandardTags.insertion(), "x"), + row("keybind", StandardTags.keybind(), ""), + row("translatable", TagResolver.resolver(StandardTags.translatable(), StandardTags.translatableFallback()), + "", "", + ""), + row("selector", StandardTags.selector(), ""), + row("score", StandardTags.score(), ""), + row("nbt", StandardTags.nbt(), ""), + row("sprite", StandardTags.sprite(), ""), + row("head", StandardTags.sequentialHead(), "") + ).flatMap(stream -> stream); + } + + private static Stream row(String type, TagResolver resolver, String... samples) { + return Stream.of(samples).map(sample -> Arguments.of(type, resolver, sample)); + } + + @ParameterizedTest(name = "{0}: {2}") + @MethodSource("samples") + @DisplayName("a tag is interpreted when its factory is enabled") + void tagIsInterpretedWithItsResolver(String type, TagResolver resolver, String sample) { + Component literal = NO_TAGS.deserialize(sample); + Component interpreted = MiniMessage.builder().tags(resolver).build().deserialize(sample); + + assertEquals(Component.text(sample), literal, "without a resolver the sample must stay literal: " + sample); + assertNotEquals(literal, interpreted, "the " + type + " factory must interpret " + sample); + } +} diff --git a/bukkit/src/main/java/net/onelitefeather/butterfly/bukkit/listener/PlayerListener.java b/bukkit/src/main/java/net/onelitefeather/butterfly/bukkit/listener/PlayerListener.java index 4815de4..4a7e9da 100644 --- a/bukkit/src/main/java/net/onelitefeather/butterfly/bukkit/listener/PlayerListener.java +++ b/bukkit/src/main/java/net/onelitefeather/butterfly/bukkit/listener/PlayerListener.java @@ -2,9 +2,9 @@ import io.papermc.paper.event.player.AsyncChatEvent; import net.kyori.adventure.text.Component; -import net.kyori.adventure.text.minimessage.MiniMessage; import net.kyori.adventure.text.serializer.plain.PlainTextComponentSerializer; import net.onelitefeather.butterfly.api.LuckPermsAPI; +import net.onelitefeather.butterfly.api.chat.ChatMessageParser; import org.bukkit.entity.Player; import org.bukkit.event.EventHandler; import org.bukkit.event.EventPriority; @@ -12,6 +12,8 @@ import org.bukkit.event.player.PlayerJoinEvent; import org.bukkit.event.player.PlayerLoginEvent; +import java.util.UUID; + public final class PlayerListener implements Listener { @EventHandler(priority = EventPriority.LOWEST) @@ -27,10 +29,15 @@ public void handlePlayerJoin(PlayerJoinEvent event) { @EventHandler public void handleChat(AsyncChatEvent event) { + // Parsed once per message, not once per viewer: only the sender's permissions decide which tags work + UUID senderId = event.getPlayer().getUniqueId(); + Component parsed = ChatMessageParser.parse( + PlainTextComponentSerializer.plainText().serialize(event.message()), + node -> LuckPermsAPI.luckPermsAPI().hasPermission(senderId, node)); event.renderer((source, sourceDisplayName, message, viewer) -> Component.text() .append(sourceDisplayName) .append(Component.text(": ")) - .append(MiniMessage.miniMessage().deserialize(PlainTextComponentSerializer.plainText().serialize(message))) + .append(parsed) .build()); } } diff --git a/minestom/src/main/java/net/onelitefeather/butterfly/minestom/Butterfly.java b/minestom/src/main/java/net/onelitefeather/butterfly/minestom/Butterfly.java index 5e19cac..6601a12 100644 --- a/minestom/src/main/java/net/onelitefeather/butterfly/minestom/Butterfly.java +++ b/minestom/src/main/java/net/onelitefeather/butterfly/minestom/Butterfly.java @@ -9,6 +9,7 @@ import net.minestom.server.event.player.PlayerChatEvent; import net.minestom.server.event.player.PlayerSpawnEvent; import net.onelitefeather.butterfly.api.LuckPermsAPI; +import net.onelitefeather.butterfly.api.chat.ChatMessageParser; import net.onelitefeather.butterfly.api.config.ButterflySettings; import net.onelitefeather.butterfly.api.config.SettingsFile; import org.jetbrains.annotations.NotNull; @@ -79,7 +80,7 @@ private void playerChat(PlayerChatEvent playerChatEvent) { playerChatEvent.setFormattedMessage(Component.text() .append(MiniMessage.miniMessage().deserialize(displayName)) .append(Component.text(": ")) - .append(MiniMessage.miniMessage().deserialize(playerChatEvent.getRawMessage())) + .append(ChatMessageParser.parse(playerChatEvent.getRawMessage(), node -> LuckPermsAPI.luckPermsAPI().hasPermission(player.getUuid(), node))) .build()); } diff --git a/minestom/src/test/java/net/onelitefeather/butterfly/minestom/ExtensionBehaviourTest.java b/minestom/src/test/java/net/onelitefeather/butterfly/minestom/ExtensionBehaviourTest.java index 7fa0cea..76ac7d1 100644 --- a/minestom/src/test/java/net/onelitefeather/butterfly/minestom/ExtensionBehaviourTest.java +++ b/minestom/src/test/java/net/onelitefeather/butterfly/minestom/ExtensionBehaviourTest.java @@ -1,5 +1,8 @@ package net.onelitefeather.butterfly.minestom; +import net.kyori.adventure.text.Component; +import net.kyori.adventure.text.TextComponent; +import net.kyori.adventure.text.format.NamedTextColor; import net.kyori.adventure.text.serializer.plain.PlainTextComponentSerializer; import net.minestom.server.color.TeamColor; import net.minestom.server.entity.Player; @@ -131,4 +134,64 @@ void colourAndSortingStayOnPrimaryGroup() { assertEquals("0001admin", player.getTeam().getTeamName(), "team name must use the primary group"); assertEquals(TeamColor.RED, player.getTeam().getTeamColor(), "colour must use the primary group"); } + + @Test + @DisplayName("chat applies a tag the sender has the permission for") + void chatAppliesPermittedTag() { + fixture.luckPerms.grantPermissions(ExtensionFixture.PLAYER_ID, "butterfly.chat.tag.color"); + Player player = fixture.spawnPlayer(); + + PlayerChatEvent event = fixture.chat(player, "hello"); + + Component message = event.getFormattedMessage(); + assertEquals("[Admin] Alice: hello", PlainTextComponentSerializer.plainText().serialize(message), "the tag must be consumed"); + assertEquals(NamedTextColor.BLUE, textColor(message, "hello"), "the message part must be blue"); + } + + @Test + @DisplayName("chat keeps a tag literal when the sender lacks the permission") + void chatKeepsDisallowedTagLiteral() { + Player player = fixture.spawnPlayer(); + + PlayerChatEvent event = fixture.chat(player, "hello"); + + assertEquals("[Admin] Alice: hello", PlainTextComponentSerializer.plainText().serialize(event.getFormattedMessage()), + "the tag must stay in the text"); + } + + @Test + @DisplayName("chat carries no click event without the click permission") + void chatDropsClickEventWithoutPermission() { + fixture.luckPerms.grantPermissions(ExtensionFixture.PLAYER_ID, "butterfly.chat.tag.color"); + Player player = fixture.spawnPlayer(); + + PlayerChatEvent event = fixture.chat(player, "x"); + + assertFalse(carriesClickEvent(event.getFormattedMessage()), "no click event may be present"); + } + + @Test + @DisplayName("the prefix keeps its colour for a sender without tag permissions") + void prefixKeepsColourWithoutTagPermissions() { + Player player = fixture.spawnPlayer(); + + PlayerChatEvent event = fixture.chat(player, "hi"); + + assertEquals(NamedTextColor.RED, textColor(event.getFormattedMessage(), "[Admin] Alice"), "the prefix and name must stay red"); + } + + private static NamedTextColor textColor(Component component, String content) { + if (component instanceof TextComponent text && text.content().equals(content)) { + return (NamedTextColor) text.color(); + } + for (Component child : component.children()) { + NamedTextColor found = textColor(child, content); + if (found != null) return found; + } + return null; + } + + private static boolean carriesClickEvent(Component component) { + return component.clickEvent() != null || component.children().stream().anyMatch(ExtensionBehaviourTest::carriesClickEvent); + } } diff --git a/minestom/src/test/java/net/onelitefeather/butterfly/minestom/FakeLuckPerms.java b/minestom/src/test/java/net/onelitefeather/butterfly/minestom/FakeLuckPerms.java index fe6d965..509500d 100644 --- a/minestom/src/test/java/net/onelitefeather/butterfly/minestom/FakeLuckPerms.java +++ b/minestom/src/test/java/net/onelitefeather/butterfly/minestom/FakeLuckPerms.java @@ -15,10 +15,12 @@ import net.luckperms.api.query.QueryOptionsRegistry; import net.luckperms.api.util.Tristate; +import java.util.HashMap; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.OptionalInt; +import java.util.Set; import java.util.UUID; import java.util.concurrent.atomic.AtomicInteger; @@ -29,6 +31,7 @@ final class FakeLuckPerms { private final Map groups = new LinkedHashMap<>(); private final Map users = new LinkedHashMap<>(); + private final Map> userPermissions = new HashMap<>(); private final AtomicInteger openSubscriptions = new AtomicInteger(); private final LuckPerms api = createApi(); @@ -77,12 +80,27 @@ void addUser(UUID uuid, String primaryGroup, String effectivePrefix) { users.put(uuid, userStub(uuid, primaryGroup, () -> effectivePrefix)); } - private static User userStub(UUID uuid, String primaryGroup, java.util.function.Supplier prefix) { + /** + * Grants permission nodes to a user, as if its groups or own nodes carried them. Nodes are matched exactly. + */ + void grantPermissions(UUID uuid, String... nodes) { + userPermissions.computeIfAbsent(uuid, _ -> new java.util.HashSet<>()).addAll(Set.of(nodes)); + } + + private User userStub(UUID uuid, String primaryGroup, java.util.function.Supplier prefix) { CachedMetaData meta = Stubs.of(CachedMetaData.class, Map.of("getPrefix", _ -> prefix.get())); - CachedDataManager data = Stubs.of(CachedDataManager.class, Map.of("getMetaData", _ -> meta)); + CachedPermissionData permissions = Stubs.of(CachedPermissionData.class, Map.of("queryPermission", args -> { + boolean granted = userPermissions.getOrDefault(uuid, Set.of()).contains((String) args[0]); + return Stubs.of(Result.class, Map.of("result", _ -> Tristate.of(granted))); + })); + QueryOptions userOptions = Stubs.of(QueryOptions.class, Map.of()); + CachedDataManager data = Stubs.of(CachedDataManager.class, Map.of( + "getMetaData", _ -> meta, + "getPermissionData", _ -> permissions)); return Stubs.of(User.class, Map.of( "getUniqueId", _ -> uuid, "getPrimaryGroup", _ -> primaryGroup, + "getQueryOptions", _ -> userOptions, "getCachedData", _ -> data)); } diff --git a/openspec/changes/restrict-chat-tags-by-permission/.openspec.yaml b/openspec/changes/restrict-chat-tags-by-permission/.openspec.yaml new file mode 100644 index 0000000..e3966d7 --- /dev/null +++ b/openspec/changes/restrict-chat-tags-by-permission/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-10-05 diff --git a/openspec/changes/restrict-chat-tags-by-permission/design.md b/openspec/changes/restrict-chat-tags-by-permission/design.md new file mode 100644 index 0000000..839575b --- /dev/null +++ b/openspec/changes/restrict-chat-tags-by-permission/design.md @@ -0,0 +1,84 @@ +# Design + +## Context + +- Paper `PlayerListener#handleChat` sets a renderer that, **per viewer**, + serializes the signed message to plain text and parses it with + `MiniMessage.miniMessage()` (all standard tags). +- Minestom `Butterfly#playerChat` parses `getRawMessage()` with the same default + instance and returns early (vanilla format) when the sender has no prefix. +- `api` has no Adventure dependency today; LuckPerms access goes through + `LuckPermsAPI`. Minestom has no permission API of its own, so LuckPerms is the + only common permission source. +- Minestom tests (`ExtensionBehaviourTest`, `FakeLuckPerms`) already drive chat + events; the `bukkit` module has no test setup. + +## Goals / Non-Goals + +**Goals:** +- One platform-neutral component that turns (raw text, permission check) into a + `Component`, unit-tested in `api`. +- Same behaviour on Paper and Minestom. + +**Non-Goals:** +- Per-value permissions (`butterfly.chat.tag.color.red`). The check is built so it + can be added later without breaking existing nodes (see decision 3). +- Changing the prefix/name part of the chat line or the Minestom early return. +- Keeping components that other plugins put into the Paper message (the current + plain-text round trip already drops them). + +## Decisions + +### 1. `ChatMessageParser` in `api` +``` +ChatMessageParser.parse(String raw, Predicate hasPermission) -> Component +``` +- Builds a `TagResolver` from the tag types whose node passes `hasPermission` + (table in the spec -> `StandardTags` factories, e.g. `color` -> + `StandardTags.color()`, `decoration` -> `StandardTags.decorations()`, + `translatable` -> `translatable()` + `translatableFallback()`, `head` -> the + `` resolver; task 1.2 confirms that `StandardTags.sequentialHead()` is the + `` tag). +- Parses with one shared `MiniMessage.builder().tags(TagResolver.empty()).build()` + instance plus the per-sender resolver. Unknown tags in non-strict mode are kept + as literal text, which gives "disallowed tags stay literal" for free. +- The tag-type -> resolver table is a single `enum ChatTagType` so the README list, + the spec table and the code have one source. + +*Alternative:* `MiniMessage.stripTags`/escape for disallowed tags. Rejected: +stripping alters text (`a c`), escaping adds backslashes viewers would see. + +### 2. Permission check via LuckPerms, not the platform +`LuckPermsAPI` gets `hasPermission(UUID, String)` using the user's cached +permission data with the user's contextual query options. Paper's +`Player#hasPermission` would route to LuckPerms anyway; using LuckPerms directly +keeps one code path and lets the Minestom `FakeLuckPerms` drive tests. LuckPerms +cached data is safe to read from Paper's async chat thread. + +### 3. Node shape `butterfly.chat.tag.` +Lower-case snake_case type names (`translatable`, `shadow`). A later per-value extension +checks `has(type) || has(type + "." + value)`, so nodes granted now keep working. + +### 4. Parse once per message +Paper: parse in `handleChat` before setting the renderer and capture the result; +the renderer only composes `displayName + ": " + parsed`. Minestom already handles +the event once. + +### 5. Dependencies +`api` gets `compileOnly` Adventure API + MiniMessage (versions from the mycelium +BOM, matching Paper's and Minestom's runtime) and the same as +`testImplementation`. + +## Risks / Trade-offs + +- [Servers upgrade and everyone loses formatting] -> BREAKING note with the one-line + migration (grant `butterfly.chat.tag.*` to `default`), README section. +- [`StandardTags` gains new tags in a future Adventure version] -> they are not in + `ChatTagType`, so they stay literal until added; safe default. +- [LuckPerms user not loaded (e.g. very early chat)] -> treat as no permission: + text stays literal, nothing breaks. + +## Migration Plan + +Release as `feat(chat)!` with `BREAKING CHANGE:` footer naming the migration node. +Rollback: previous jar restores full-tag parsing. diff --git a/openspec/changes/restrict-chat-tags-by-permission/proposal.md b/openspec/changes/restrict-chat-tags-by-permission/proposal.md new file mode 100644 index 0000000..0dd203d --- /dev/null +++ b/openspec/changes/restrict-chat-tags-by-permission/proposal.md @@ -0,0 +1,46 @@ +# Proposal + +Ships as: `feat(chat)!: restrict minimessage tags in chat by permission` + +Depends on: `replace-togglz-with-avaje-config` (PR #133) being merged. + +## Why + +Both platforms parse every chat message with the full default MiniMessage +instance (`PlayerListener#handleChat` on Paper, `Butterfly#playerChat` on +Minestom). Any player can therefore send click events that run commands, hover +texts, selectors, NBT lookups, fonts, sprites and player heads. Servers want to +grant formatting per rank instead, and LuckPerms already models ranks. + +## What Changes + +- Chat messages are parsed with only the MiniMessage tags the sender has a + permission for: `butterfly.chat.tag.`, one node per tag type (for example + `color`, `decoration`, `gradient`, `click`, `hover`, `head`). Granting per rank + happens through LuckPerms group permissions; `butterfly.chat.tag.*` grants all. +- Tags the sender may not use stay in the message as literal text. +- Permissions are resolved through LuckPerms, so Paper and Minestom behave the + same. +- The message is parsed once per chat message instead of once per viewer (Paper). +- **BREAKING**: without permissions, players lose all formatting they have today. + Granting `butterfly.chat.tag.*` to the `default` group restores the old + behaviour. + +## Capabilities + +### New Capabilities +- `chat-tag-permissions`: which MiniMessage tags a chat sender may use, how + permissions map to tag types, and what happens to tags that are not allowed. + +### Modified Capabilities + + +## Impact + +- Code: `bukkit/.../listener/PlayerListener.java`, + `minestom/.../Butterfly.java` (`playerChat`), new platform-neutral code in `api`. +- Build: `api` needs Adventure MiniMessage as `compileOnly` (provided by Paper and + Minestom at runtime). +- Operators: must grant `butterfly.chat.tag.` nodes per group; document the + node list in the README. +- Follow-up `add-player-head-to-chat` builds its chat line on the same code path. diff --git a/openspec/changes/restrict-chat-tags-by-permission/specs/chat-tag-permissions/spec.md b/openspec/changes/restrict-chat-tags-by-permission/specs/chat-tag-permissions/spec.md new file mode 100644 index 0000000..41c052e --- /dev/null +++ b/openspec/changes/restrict-chat-tags-by-permission/specs/chat-tag-permissions/spec.md @@ -0,0 +1,84 @@ +# Spec Delta + +## Purpose + +Controls which MiniMessage formatting tags a player may use in chat messages, per +LuckPerms rank, so servers decide who may color text, add click or hover events, +or embed objects such as player heads. + +## ADDED Requirements + +### Requirement: Tags require a permission per tag type +When Butterfly formats a chat message, it SHALL interpret a MiniMessage tag in the +message text only if the sender has the permission `butterfly.chat.tag.` +for that tag's type, as resolved by LuckPerms for the sender. This applies on +Paper and on Minestom. + +| Type | Tags covered | +|------|--------------| +| `color` | named colours, hex colours, `` | +| `decoration` | ``, ``, ``, ``, `` and their short forms | +| `gradient` | `` | +| `rainbow` | `` | +| `transition` | `` | +| `pride` | `` | +| `shadow` | `` | +| `font` | `` | +| `reset` | `` | +| `newline` | `` / `
` | +| `click` | `` | +| `hover` | `` | +| `insertion` | `` | +| `keybind` | `` | +| `translatable` | ``, ``, `` | +| `selector` | `` | +| `score` | `` | +| `nbt` | `` | +| `sprite` | `` | +| `head` | `` | + +#### Scenario: Permitted colour +- **WHEN** a player with `butterfly.chat.tag.color` sends `hello` +- **THEN** the message part of the chat line reads `hello` in red + +#### Scenario: Permission for one type does not grant another +- **WHEN** a player with `butterfly.chat.tag.color` but not `butterfly.chat.tag.click` sends `x` +- **THEN** the chat line carries no click event + +#### Scenario: Wildcard grants every type +- **WHEN** a player whose group has `butterfly.chat.tag.*` sends `hi` +- **THEN** the message is rendered with the rainbow effect + +#### Scenario: Permission granted through a group +- **WHEN** the group `vip` has `butterfly.chat.tag.gradient` and a player in `vip` sends `hi` +- **THEN** the message is rendered with the gradient + +### Requirement: Disallowed tags stay as literal text +A tag the sender has no permission for SHALL appear in the chat line exactly as +typed, including its angle brackets and arguments. Text around it MUST NOT be +removed or altered. + +#### Scenario: No permissions at all +- **WHEN** a player without any `butterfly.chat.tag.*` permission sends `hello a < b` +- **THEN** the message part of the chat line is the plain text `hello a < b` + +#### Scenario: Mixed allowed and disallowed tags +- **WHEN** a player with only `butterfly.chat.tag.decoration` sends `hi there` +- **THEN** `hi` is bold +- **AND** `there` appears literally + +### Requirement: Tag permissions do not affect the prefix +The sender's LuckPerms prefix and name in the chat line SHALL keep their +MiniMessage formatting regardless of the sender's tag permissions. + +#### Scenario: Coloured prefix for a player without tag permissions +- **WHEN** a player in a group with prefix `[Admin] ` and no tag permissions sends `hi` +- **THEN** the chat line shows `[Admin] ` in red followed by the name and `hi` + +### Requirement: Every viewer sees the same message +The message part of a chat line SHALL be the same for every viewer; it depends +only on the sender's permissions, not on the viewer's. + +#### Scenario: Two viewers +- **WHEN** a player with `butterfly.chat.tag.color` sends `hi` and two other players receive it +- **THEN** both receive `hi` in red diff --git a/openspec/changes/restrict-chat-tags-by-permission/tasks.md b/openspec/changes/restrict-chat-tags-by-permission/tasks.md new file mode 100644 index 0000000..a467279 --- /dev/null +++ b/openspec/changes/restrict-chat-tags-by-permission/tasks.md @@ -0,0 +1,32 @@ +# Tasks + +Start after PR #133 (`replace-togglz-with-avaje-config`) is merged. Branch from +the up-to-date `origin/main`. Implementation commits use `feat(chat)`; the commit +that switches both platforms to the restricted parser is `feat(chat)!` with a +`BREAKING CHANGE:` footer. + +## 1. Groundwork in `api` + +- [x] 1.1 Add Adventure API and MiniMessage as `compileOnly` and `testImplementation` to `api/build.gradle.kts` (versions via the mycelium BOM); verify `./gradlew :api:compileJava` succeeds +- [x] 1.2 Confirm which `StandardTags` factory provides the `` tag (expected `sequentialHead()`) and the exact tag names of every row in the spec table with a parameterized unit test that parses one sample per type with only that resolver enabled; verify `./gradlew :api:test` passes +- [x] 1.3 Add `LuckPermsAPI.hasPermission(UUID, String)` (user cached permission data, user's contextual query options, `false` when the user is not loaded) with unit tests against a fake LuckPerms user; verify `./gradlew :api:test` passes + +## 2. Parser + +- [x] 2.1 Write failing unit tests for `ChatMessageParser.parse(raw, hasPermission)` covering every scenario of "Tags require a permission per tag type" and "Disallowed tags stay as literal text" (permissions as a plain `Predicate`, results compared via the plain-text serializer and component style/click/hover assertions); verify they fail with `./gradlew :api:test` +- [x] 2.2 Implement `ChatTagType` (type name -> resolvers) and `ChatMessageParser`; verify `./gradlew :api:test` passes + +## 3. Platforms + +- [x] 3.1 Minestom: write failing `ExtensionBehaviourTest` cases (fresh `Env`, `FakeLuckPerms` with/without `butterfly.chat.tag.color`): coloured message with permission, literal `` without, prefix keeps its colour without tag permissions; then use `ChatMessageParser` in `Butterfly#playerChat`; verify `./gradlew :minestom:test` passes +- [x] 3.2 Paper: parse once in `PlayerListener#handleChat` with `ChatMessageParser` and a LuckPerms-backed predicate for the source player, renderer only composes; verify `./gradlew :bukkit:build` succeeds and `grep -n "MiniMessage.miniMessage()" bukkit/src/main/java/net/onelitefeather/butterfly/bukkit/listener/PlayerListener.java` returns nothing +- [x] 3.3 Run `./gradlew build :minestom:smokeTest`; verify both succeed + +## 4. Docs and manual check + +- [x] 4.1 README: section listing every `butterfly.chat.tag.` node (generated from or checked against `ChatTagType`), the wildcard, the literal-text behaviour and the migration line; verify every type in the spec table appears +- [ ] 4.2 Manual Paper test via `./gradlew :bukkit:runServer` with LuckPerms: player without nodes sees `x` literally; after `lp group default permission set butterfly.chat.tag.color true` the same message is red; `x` has no click event without `butterfly.chat.tag.click` (report in the PR description) + +## 5. Pull request + +- [ ] 5.1 Open the pull request titled `feat(chat)!: restrict minimessage tags in chat by permission` with a `BREAKING CHANGE:` section (formatting now needs `butterfly.chat.tag.`; grant `butterfly.chat.tag.*` to `default` to restore the old behaviour); verify `gh pr view` shows the title and CI is green diff --git a/settings.gradle.kts b/settings.gradle.kts index f2d8c06..c7fd141 100644 --- a/settings.gradle.kts +++ b/settings.gradle.kts @@ -46,6 +46,8 @@ dependencyResolutionManagement { library("cyano", "net.onelitefeather", "cyano").withoutVersion() library("minestom","net.minestom", "minestom").withoutVersion() library("minestom.testing", "net.minestom", "testing").versionRef("minestom-testing") + library("adventure.api", "net.kyori", "adventure-api").withoutVersion() + library("adventure.plain", "net.kyori", "adventure-text-serializer-plain").withoutVersion() library("adventure.minimessage", "net.kyori", "adventure-text-minimessage").withoutVersion() library("avaje.config", "io.avaje", "avaje-config").versionRef("avaje-config") library("slf4j.api", "org.slf4j", "slf4j-api").versionRef("slf4j")