Separate group sync config for Discord -> Minecraft, Minecraft -> Discord
TomLewis opened this issue · 11 comments
This is more of a security vulnerability that needs to be patched.
With larger Discord servers, this requires having moderators to manage them, and there is only so much security you can put into Discord and trust.
DiscordSRV just accidentally gave someone a Donator rank, we have no idea how/where/why because DiscordSRV does not log when it sets a rank on people. #1125
This is a major security issue, that 1, its not logged, 2, nowhere/noway should DiscordSRV be able to hand out Donation ranks, there is only 1 role DiscordSRV should be able to set in-game and that's for when people Boost the Discord server, but I can only set a GLOBAL set of GroupRoleSynchronizationGroupsAndRolesToSync
that goes both ways MineCraft <-> Discord
.
Please consider strengthening the security by giving us a whitelist for each sync method:
There needs to be a separation between:
MineCraft -> Discord
whitelist GroupRoleSynchronizationGroupsAndRolesToSync
We need a separate whitelist for Discord -> Minecraft, something like:
Discord -> Minecraft
whitelist DiscordMineCraftGroupRoleSynchronizationGroupsAndRolesToSync
So then I can keep ranks in-game syncing to Discord, but if someone boosts the discord server to allow DiscordSRV to rank them to that role in-game, but without the scary inevitability of somehow giving someone any other rank that's in the whitelist for MineCraft -> Discord
. We still don't know how DiscordSRV did this, or if it was a moderator that accidentally gave them the role, which we wouldn't want synced in game!
I really, really need this.
I currently have twitch subs get ranks in-game, but those ranks are also purchasable.
There's no way for me to make it so that if someone purchases the rank, they'll keep it in-game. The rank is always automatically removed cause they don't have the Twitch Sub role in the Discord.
Turning on MinecraftIsAuthoratative
doesn't work and still has their rank automatically removed.
Discord -> MC OneWay
isn't an option as it still removes the purchased rank.
MC -> Discord OneWay
also isn't an option cause the Twitch Subs would then not get the rank in-game.
I think this is something to do with the Twitch Sub roles being managed roles.
I still require this also, and so does #1249.
We need a white-list for what discordSRV can set to Minecraft.
I really, really need this. I currently have twitch subs get ranks in-game, but those ranks are also purchasable. There's no way for me to make it so that if someone purchases the rank, they'll keep it in-game. The rank is always automatically removed cause they don't have the Twitch Sub role in the Discord. Turning on
MinecraftIsAuthoratative
doesn't work and still has their rank automatically removed.
Discord -> MC OneWay
isn't an option as it still removes the purchased rank.MC -> Discord OneWay
also isn't an option cause the Twitch Subs would then not get the rank in-game.I think this is something to do with the Twitch Sub roles being managed roles.
I've found a cheeky solution to this. I have a "bought" group for each rank (ironbought
, goldbought
, and diamondbought
). These groups will not sync at all. These are only given to people that purchase the rank. These groups inherit the permissions from their main group (iron
, gold
, and diamond
).
The Twitch Subs will get the main groups, and it's synced Discord --> MC one way.
This means that the people that purchase the ranks and the Twitch Subs will still get the "same" groups.
This is honestly a terrible way to do this since the players aren't directly being put into the groups, which could cause issues with other plugins (especially BungeeCord ones).
Just to clarify the severity of this security flaw, if anything, a person a bot a hacked account on Discord has the ability to change someones role on discord it is synced to the Minecraft server, which should only ever happen for 1 rank and 1 rank only; Discord boosters. Which is controlled by Discords automated systems. This is why a simple whitelist to say what is allowed to be synced from Discord -> Minecraft needs to be implemented and it boggles my mind why this wasn't in the initial design phase oh the sync system.
Unlikely to be implemented, very low priority
DiscordSRV syncs groups in both directions. If you don't want that, enable the one-way option.
So then I can keep ranks in-game syncing to Discord, but if someone boosts the discord server to allow DiscordSRV to rank them to that role in-game, but without the scary inevitability of somehow giving someone any other rank that's in the whitelist for
MineCraft -> Discord
.
LuckPerms contexts would accomplish this far better than group synchronization ever would. discordsrv:boosting=true
TIL that DiscordSRV adds contexts for Boosted, everyone would much prefer a rank, it's much cleaner that way. But this is a cheap workaround for now.
This will break all plugins reliant on checking for a players Rank, for example, Overhead Titles, Our Discord booster gets a custom overhead title in-game, which the plugin does formatting per group, not permission.
How are contexts saved? I've searched my database of users and no contexts are showing at all, they are all {}
SELECT * FROM luckperms_user_permissions WHERE contexts LIKE '%discordsrv%'
Shows nothing.
SELECT * FROM luckperms_user_permissions WHERE contexts <> "{}"
shows nothing.
Are contexts generated on the fly, stored in RAM?
How do I check who's been given these contexts?
Such a shame, it's only a whitelist and it's already party implemented. The second I clocked this untracked mistake I instantly set GroupRoleSynchronizationOneWay
.
At the very minimal of effort required, at least set GroupRoleSynchronizationOneWay
as a default setting to prevent any other servers from having this high priority security issue.
Both have the commands, and the LuckPerms article shows the web editor. If you'd like to know the format for the file, please ask LuckPerms' support
I tried, they don't know...
Why is nothing simple or just work.
Are contexts generated on the fly, stored in RAM?
Yes
How do I check who's been given these contexts?
/lp user <player name> info
shows the active contexts for a given player - so if you run that for a player that's online it will show their contexts
At the very minimal of effort required, at least set
GroupRoleSynchronizationOneWay
as a default setting to prevent any other servers from having this high priority security issue.
At no point should group synchronization be used in a way that it could turn into a security exploit. If someone sets up group synchronization that way, they did it on their own - there's no group synchronization by default. The comments above the options make it clear that synchronization is both ways
/lp user info shows the active contexts for a given player - so if you run that for a player that's online it will show their contexts
No Globally. I want to check all users, as you can see in my reply with the SQL queries trying to check all users to make sure contexts have been added to the correct people.
It's not being stored in the contexts database.
At no point should group synchronization be used in a way that it could turn into a security exploit. If someone sets up group synchronization that way, they did it on their own - there's no group synchronization by default. The comments above the options make it clear that synchronization is both ways
Exactly. Which is why I created this ticket to allow whitelists per sync because of the oversight of it being one global whitelist, so I'm forced to put in all the ranks I want to sync Minecraft -> Discord
as well as the 1 rank I want Discord -> Minecraft
So yes, you backed up the reason I created this ticket.
It creates a security flaw by forcing you to have 1 global whitelist.
DiscordSRV Contexts are not working.
Since both the Luckperms Contexts page AND the DiscordSRV Contexts page neither have examples of how to write these, I have had to guess, and this does not work.
- prefix.156.&a:
value: true
discordsrv: boosting
- suffix.156.&b »&r:
value: true
discordsrv: boosting
No Globally. I want to check all users, as you can see in my reply with the SQL queries trying to check all users to make sure contexts have been added to the correct people.
It's not being stored in the contexts database.
They aren't stored in the database, they change all the time. The context
column in luckperms_user_permissions
stores the contexts that are required for that permission to apply
Exactly. Which is why I created this ticket to allow whitelists per sync because of the oversight of it being one global whitelist, so I'm forced to put in all the ranks I want to sync Minecraft -> Discord as well as the 1 rank I want Discord -> Minecraft So yes, you backed up the reason I created this ticket.
It creates a security flaw by forcing you to have 1 global whitelist.
That's not a security flaw, that's a technical limitation.
Since both the Luckperms Contexts page AND the DiscordSRV Contexts page neither have examples of how to write these, I have had to guess, and this does not work.
Both have the commands, and the LuckPerms article shows the web editor. If you'd like to know the format for the file, please ask LuckPerms' support