discord-jda / JDA

Java wrapper for the popular chat & VOIP service: Discord https://discord.com
Apache License 2.0
4.35k stars 734 forks source link

Discord Channel Permissions Inconsistencies #414

Open schnapster opened 7 years ago

schnapster commented 7 years ago

I'm getting a lot of these:

net.dv8tion.jda.core.exceptions.ErrorResponseException: 50013: Missing Permissions
    at net.dv8tion.jda.core.exceptions.ErrorResponseException.create(ErrorResponseException.java:145)
    at net.dv8tion.jda.core.requests.Request.onFailure(Request.java:83)
    at net.dv8tion.jda.core.entities.impl.TextChannelImpl$4.handleResponse(TextChannelImpl.java:360)
    at net.dv8tion.jda.core.requests.Request.handleResponse(Request.java:166)
    at net.dv8tion.jda.core.requests.Requester.execute(Requester.java:186)
    at net.dv8tion.jda.core.requests.Requester.execute(Requester.java:86)
    at net.dv8tion.jda.core.requests.ratelimit.BotRateLimiter$Bucket.run(BotRateLimiter.java:310)
    at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
    at java.util.concurrent.FutureTask.run(FutureTask.java:266)
    at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.access$201(ScheduledThreadPoolExecutor.java:180)
    at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:293)
    at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
    at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
    at java.lang.Thread.run(Thread.java:748)

To reproduce: Give the bot the "Manage Roles" permission, and try to do something like clearing Emojis from a message. The problem here is, that I can't even check this in front of, because JDA will tell me that yes, it does have permissions to Manage Messages only to then error out when queue()ing that action. @MinnDevelopment explained it to me like this:

MANAGE_ROLES = MANAGE_PERMISSIONS MANAGE_PERMISSIONS -> ALL_TEXT_PERMISSIONS ALL_TEXT_PERMISSIONS -> MANAGE_MESSAGE

and that this was an inconsistency in handling permissions on Discords side.

So how can I know in this case in advance that I am missing a permission?

MinnDevelopment commented 7 years ago

You can get permissions without the special cases by using PermissionUtil.getExplicitPermissions(...)

schnapster commented 7 years ago

Thanks, I shall use that then. Wasn't aware this was in JDA since I never really looked at PermissionUtil due to it being a JDA internal class. Guess these methods aren't about to change significantly?

MinnDevelopment commented 7 years ago

PermissionUtil will likely not be updated until a new permission change is made by discord. This is likely to happen with the release of channel categories.

MinnDevelopment commented 6 years ago

Since we've been waiting half a year for a change in channel permissions, I updated the handling for now to properly respect the current (bad) channel permission system. This is available in build 333 and will be in the next release unless something in the permissions on discord's end changes first (who am I kidding).

This issue will stay open to keep track of this problem as it is still an issue on discord's end which needs to be fixed.

MinnDevelopment commented 6 years ago

This has been resolved in release 3.5.1 I will keep this issue open until discord fixes their permissions

CanIGetaPR commented 3 years ago

This can be closed now, Discord fixed the permission leak by not allowing users with manage roles to grant permissions they don't already possess.

Example: Screenshot of trying to edit a role's permissions while using a role with only manage roles permission activated. (And some basic permissions inherited from @everyone)

image

DV8FromTheWorld commented 3 years ago

If this is indeed the case then PermissionUtil will need to be re-evaluated to see if the situation has changed and if we need to modify our functionality to match the now-fixed functionality.

V-Play-Games commented 2 years ago

I think this issue can now be closed, no?