[BUG] Discord role-based `minecraftCommands` permissions break for users with overlapping roles

[Zeklandia]

Per the "Combined Example" in the docs, I have multiple role-based permissions sets for using Minecraft commands in Discord. However, the basic permissions are associated with a role that all verified players have, and this is preventing anyone who additionally has the moderator or administrator role from using commands privileged to those roles. I have tried rearranging the order in the configuration file to no effect.

    [[minecraftCommands.permissions]]
        role = "[verified players role ID]"
        commands = ["nick", "nickname", "styled-nicknames", "list"]
        permissionLevel = 1

    [[minecraftCommands.permissions]]
        role = "[moderator role ID]"
        commands = ["kick", "ban", "ban-ip", "afkplus"]
        permissionLevel = 3

    [[minecraftCommands.permissions]]
        role = "[admin role ID]"
        commands = []
        permissionLevel = 4

For example, I have the verified player role and the administrator role. I can run any of the commands available to the verified player role, but I am prevented from running any other commands. If I remove the configuration for the verified player role, then I am able to run any command, as the configuration for the administrator role specifies.

[hypherionmc]

This is intentional.

In the past, we checked the permissions for any role that the user has, which resulted in users being able to use commands, that aren't specifically configured for them.

To fix the issue, order your entries from highest permission, to lowest, because it will always use the first entry it finds, for the highest role of the user. Basically, the permissions work the same way as discord role permissions work

[Zeklandia]

Even with the order reversed, I was still not able to run any commands other than the ones the verified players role is allowed to run. I tried reloading the cache and restarting the bot, and it still prevented me from using other commands. But after reading your words more carefully, I added permissions for a role I have on Discord that is ordered before the role with admin permissions. That worked.

So, it is not enough to have a role with a larger permissions scope, your highest Discord role must have a larger permissions scope.

[Zeklandia]

I discovered another issue while testing this out. Running ?dynmap stats yields only this as output:

Tile Render Statistics:

Can multi-line outputs not be returned when running Minecraft commands in Discord?

[hypherionmc]

So, it is not enough to have a role with a larger permissions scope, your highest Discord role must have a larger permissions scope.

Hmm that doesn't sound right. The permission check is supposed to work like this:

1) User ID 2) Highest discord role that matches a configured role.

Meaning, if I have a permissions block with my discord user ID as the role, that would always be my command permissions. Next, it loops over the roles, from highest to lowest, until it finds a permission block that matches a role.

Once either of these occur, it should stop looking for more permissions, and continue with the command execution.

What exactly is the error you are getting when trying to run a command?

Can multi-line outputs not be returned when running Minecraft commands in Discord?

If it outputs the information as command output, it should. Some mods however, like to return data as "chat" messages, or "system" messages when a command is executed, so the command output can't correctly return it. Quick look at the source code, confirms this. It sends the output as a message to the user that executed the command, instead of sending it as a command reply. Hmmmm

[Zeklandia]

It would just tell me I didn't have permission to run that command. I have three roles: Discord Admin, Minecraft Admin, and Verified Player. My configuration above has permissions specified for only Minecraft Admin and Verified Player. Even after reordering the configuration per your recommendation, I was still not able to use the permissions granted to Minecraft Admin, so I duplicated its permissions configuration and used the ID for Discord Admin, putting it first in the config file. Then, I was able to run any command.

It sends the output as a message to the user that executed the command, instead of sending it as a command reply. Hmmmm

Dynmap doesn't play nice with other mods on Fabric. I'm still waiting for it to support LuckPerms.

[hypherionmc]

Interesting. Wonder if there's a bug in that system that no one encountered, or did encounter and just didn't bother to report it. I'll run some tests along with your "not ready" issue later on to see what's going on there

Dynmap doesn't play nice with other mods on Fabric

Sadly this is true for most mods that also have plugin versions, or that started as plugins, and then added modded versions as well. They design a lot of custom systems, to make development easier, but then it doesn't play nice with other mods, or some forms of compat cannot be added due to their system limitations

[Zeklandia]

There's something else going on here. I was able to get my permissions fixed by copying the Minecraft Admin entry for the Discord Admin role. But other users whose highest role is Minecraft Admin are still not able to use any commands other than what Verified Player is allowed. The role IDs are correct. They are in descending order of access in the configuration file, with identical entries. Minecraft Admin and Verified Player are these users' two highest roles.

[hypherionmc]

What is the ordering of these roles in your discord?

I'm struggling to find people to help me test these issue, because everyone that normally helps are busy with other things, and testing this solo doesn't work out

[Zeklandia]

The order is the same in the config file and on the Discord:

• Discord Admin
• Minecraft Admin
• Minecraft Mod
• Verified Player

[hypherionmc]

That should be correct. I wonder if the bot is somehow loading the roles out of order in your discord, instead of highest to lowest 🤔

Would you be willing to run a "test" build, that dumps the name of the roles the person executing the command has to the log, so that we can see if that's the case?

[Zeklandia]

Sure thing. I don't think the order is backwards, as then it shouldn't work for any of us. I do wonder if it's somehow skipping over the first entry or otherwise off by one when parsing the config file. Let's find out.

On Mon, Nov 11, 2024 at 1:50 AM HypherionSA @.***> wrote:

That should be correct. I wonder if the bot is somehow loading the roles out of order in your discord, instead of highest to lowest 🤔

Would you be willing to run a "test" build, that dumps the name of the roles the person executing the command has to the log, so that we can see if that's the case?

— Reply to this email directly, view it on GitHub https://github.com/hypherionmc/sdlink/issues/130#issuecomment-2467368214, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAW374NVSZYNHVOD47XSCD32ABHRRAVCNFSM6AAAAABRNEOZG6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDINRXGM3DQMRRGQ . You are receiving this because you authored the thread.Message ID: @.***>

[hypherionmc]

I'll do one that dumps both the roles, and how it matched with the config, so we have a sort of "internal" view of what the mod is doing.

Will send it you in a bit

[hypherionmc]

Here you go.

https://cdn.nightbloom.cc/debug/SimpleDiscordLink-Universal-3.2.1+debug.0.jar

When someone runs a command it will dump a block of text to the console (make sure debugging is set to enabled in the config). Role ID's are not private (since anyone in your discord server can easily access them), so you are fine to put the output here. If you still want to be safe, just remove a couple of characters from the role ids

[hypherionmc]

:raised_eyebrow: It should look like this:

[07:31:49] [pool-2-thread-1/ERROR]: *************************** SDLINK COMMAND DEBUG ****************************
Discord User H
Discord ID: 354707828298088459
Role: 1125506959848771745 (1125506959848771745)
No Allowed Commands Found
Role: 1130906319084343356 (1130906319084343356)
No Allowed Commands Found
Using command block 0: weather clear, help, list

Also, See I didn't dump the role names, but dumped the ID's twice.....

[hypherionmc]

Updated build that actually dumps the names, and not the ID twice....

https://cdn.nightbloom.cc/debug/SimpleDiscordLink-Universal-3.2.1+debug.1.jar

[Zeklandia]

I realized I had debugging = false in the config file. Here's the output from when I successfully ran a command with the first build (I have added some newlines where I deemed it appropriate):

*************************** SDLINK COMMAND DEBUG ****************************
Discord User [Zeklandia]
Discord ID: [ID]
Role: [Discord Admin] ([Discord Admin])
Allowed Commands:
Fallback Permission Level: 4

Role: [Verified Player] ([Verified Player])
Allowed Commands: nick, nickname, styled-nicknames, list
Fallback Permission Level: 1

Role: [Minecraft Admin] ([Minecraft Admin])
Allowed Commands:
Fallback Permission Level: 4

Role: [Verified Discord] ([Verified Discord])
No Allowed Commands Found
Using command block [Discord Admin]:

And here's the config for reference:

    #Discord Admin
    [[minecraftCommands.permissions]]
        role = "[Discord Admin]"
        commands = []
        permissionLevel = 4

    #Minecraft Admin
    [[minecraftCommands.permissions]]
        role = "[Minecraft Admin]"
        commands = []
        permissionLevel = 4

    #Minecraft Mod
    [[minecraftCommands.permissions]]
        role = "[Minecraft Mod]"
        commands = ["kick", "ban", "ban-ip", "afkplus"]
        permissionLevel = 3

    #Verified Player
    [[minecraftCommands.permissions]]
        role = "[Verified Player]"
        commands = ["nick", "nickname", "styled-nicknames", "list"]
        permissionLevel = 1

I have a hunch that the roles are getting sorted by their role ID. At least, when I sort them by their ID, I get the same ordering as the debug output:

1. Discord Admin
2. Verified Player
3. Minecraft Admin
4. Minecraft Mod
5. Verified Discord

[hypherionmc]

So I also confirmed again how the system works now during testing (I kinda forgot, because it's been a while).

So we look for commands in this order:

1) Your Discord user ID 2) Your Highest role, that has a command block configured 3) Everyone command block (block with ID as "0").

If none of these match, you will see the "sorry, you are not allowed" message.

So in your specific case:

1) Is there a command block matching your discord user id? No 2) Is there a command block matching your Highest role (Discord ADMIN in your case): Yes, we use this to allow your commands

Now, if you were to remove your discord admin role, it should now use the next role, which is Verified Player.

I have confirmed in code, that the order of the command blocks in the config has not affect on what commands the player is allowed. It matches them purely based on the role ids. So even if your Discord Admin block was last in the config, it would still be the first one picked, because your highest role is discord admin

---

Edit: This is the config that I tested with, and my output of the debug:

#Execute Minecraft commands in Discord
[minecraftCommands]
    #Allow executing Minecraft commands from Discord
    enabled = true
    #Command Prefix. For example ?weather clear
    prefix = "?"
    #Should command replies be deleted automatically or not
    keepReplies = false
    #You can leave this empty, or enter the channel ID's (surrounded by "") of channels where linked commands can be used
    allowedChannels = []

    #List of command permissions
    [[minecraftCommands.permissions]]
        role = "0"
        commands = ["weather clear", "help", "list"]
        permissionLevel = 1

    [[minecraftCommands.permissions]]
        role = "1130906319084343356"
        commands = ["say"]
        permissionLevel = 1

    [[minecraftCommands.permissions]]
        role = "1125506959848771745"
        commands = ["whitelist"]
        permissionLevel = 1

[07:42:15] [pool-8-thread-2/ERROR]: *************************** SDLINK COMMAND DEBUG ****************************
Discord User H
Discord ID: 354707828298088459
Role: First Dark Team (1125506959848771745)
Allowed Commands: whitelist
Fallback Permission Level: 1
Role: mc staff (1130906319084343356)
Allowed Commands: say
Fallback Permission Level: 1
Using command block 1125506959848771745: whitelist

[Zeklandia]

For testing purposes, I removed the Discord Admin role from my account. This means I have the same roles as the other admins who are unable to run commands. Here's the output:

*************************** SDLINK COMMAND DEBUG ****************************
Discord User [Zeklandia]
Discord ID: [ID]
Role: Verified Player ([Verified Player])
Allowed Commands: nick, nickname, styled-nicknames, list
Fallback Permission Level: 1

Role: Minecraft Admin ([Minecraft Admin])
Allowed Commands:
Fallback Permission Level: 4

Role: Verified Discord ([Verified Discord])
No Allowed Commands Found
Using command block [Verified Player]: nick, nickname, styled-nicknames, list

For clarification, here is the ordering of these roles in Discord:

1. Discord Admin
2. Minecraft Admin
3. Minecraft Mod
4. Verified Player
5. Verified Discord

[hypherionmc]

Ah ha, you see. There it is. The roles are completely out of order (in the mod, not your fault).

In the command output, it's first loading your Verified player role, then your admin role, and finally your discord role, where it's supposed to be, Minecraft Admin -> Verified Player -> Verified Discord

[Zeklandia]

I think I see what the issue is. It's this line. It's sorting the output of this, which should be the numerical role IDs as strings. This matches what I figured out earlier about the way the role IDs sorted matching the mod's ordering of them.

[hypherionmc]

I think I see what the issue is. It's this line. It's sorting the output of this, which should be the numerical role IDs as strings. This matches what I figured out earlier about the way the role IDs sorted matching the mod's ordering of them.

Those two functions are unrelated. The second one you linked is the cache used for roles used throughout the mod. It has no affect on the commands system, since it takes the roles directly from the user, bypassing the cache. However, you are on to something with the first one. The sorting is not working like it should.

This is why I say that:

*************************** SDLINK COMMAND DEBUG ****************************
Discord User H
Discord ID: 354707828298088459
Role: First Dark Team (1125506959848771745) 15
Allowed Commands: whitelist
Fallback Permission Level: 1

Role: Mod Developer (1131308819302072320) 9
No Allowed Commands Found

Role: mc staff (1130906319084343356) 7
Allowed Commands: say
Fallback Permission Level: 1

Role: Collaborators (1125507107974819940) 13
No Allowed Commands Found

Using command block 1125506959848771745: whitelist

(15, 9, 7, 13 are the "position" of the role inside my discord). I am expecting it to sort according to 15, 13, 9, 7, which it is not.

[hypherionmc]

https://cdn.nightbloom.cc/debug/SimpleDiscordLink-Universal-3.2.1+debug.2.jar

Here we go. This should fix the issue. It was indeed the first piece of sorting code. Because I was collecting it as a set, it did not preserve the order at all, so that sorting code was useless

[Zeklandia]

Ah. GitHub can only connect the dots so well. I am confused by these data structures, as they are unfamiliar to me.

So we look for commands in this order:

1. Your Discord user ID

2. Your Highest role, that has a command block configured

3. Everyone command block (block with ID as "0").

Doesn't step 2 make for a broader issue that anyone with a lower-ranked Discord role that has additional permissions gets overlooked? It seems it should iterate over each role and 'OR' the results to allow the command if any role has the permission to do it.

[Zeklandia]

The latest build does indeed order the roles and validate permissions correctly.

[hypherionmc]

It seems it should iterate over each role

We did that early on in the system, and it actually had the complete opposite effect. It opened up a major security bug, where if the server admin configured the command blocks wrong, people could get access to OP commands, even when they shouldn't.

I don't remember the exact case, but it had something to do with an empty list of commands, and the permission level. If I remember correctly, it was a case of there were two blocks configured with limited commands, but the one block had a higher permission level than the first one, which allowed the user to abuse commands with that permission level.

It had something to do with how the permission levels work. If a "commands" list is empty, it uses the permissionLevel to determine what commands the user is allowed to run, which would override the other blocks that actually have limited commands

[Zeklandia]

That sounds like there were two different things going on. If a list of commands is specified, then the permission level should be moot, right? If that's allowing someone who doesn't have blanket command privileges to run additional commands, then that is a problem. But if someone has a role with blanket command privileges and a permission level greater than a role that has a restricted command list, then I would say that's merely an oversight in configuration.

[hypherionmc]

Yeah. It was mostly a case of user error, but because it's such a simple mistake to make, we had to change it to force "first found, only used" configs.

I am planning another revision of the entire commands system, but it's pretty low on the to-do list right now, since for most part the system works (except now this stupid bug lol), but it will be before the end of the year hopefully. No user facing changes, but how it works internally.

[Zeklandia]

I bet it would be a lot of work, but if users could associate a LuckPerms group with a role and have SDLink use LuckPerms to figure out the permissions, it would make it a lot easier for users to manage the configuration.

Thank you for resolving this issue. SDLink is a very impressive and helpful mod, and I'm grateful that you've taken the time to make it work for us.

[hypherionmc]

You're welcome, and thank you for your help in testing too.

As for LuckPerms, we are actually planning LuckPerms and FTB support to sync in-game ranks with discord roles (which is hopefully going to be included in the next release, but no promised yet).

Not sure yet how we will tie it in with the commands system, but something to keep in mind