Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Permission Overwrites resolving incorrectly. #1253

Closed
bdistin opened this issue Mar 8, 2017 · 3 comments
Closed

Permission Overwrites resolving incorrectly. #1253

bdistin opened this issue Mar 8, 2017 · 3 comments

Comments

@bdistin
Copy link
Contributor

bdistin commented Mar 8, 2017

Tested on latest Master.

The bug is in this particular code:

for (const overwrite of overwrites.role.concat(overwrites.member)) { permissions &= ~overwrite.deny; allow |= overwrite.allow; } permissions |= allow;

This means any allows, no matter where the overwrite is positioned in the overwrites, will super-cede any denys. Which is a problem, because this is not how discord resolves permissions. Discord takes the overwrites for each permission at it's highest position of overwrites, and that is law.

Given a member with 2 roles, the higher role has deny send_messages and the lower has allow send_messages; d.js will resolve allow send_messages when in-fact discord will deny send_messages.

@meew0
Copy link
Contributor

meew0 commented Mar 8, 2017

That's not true at all, on the same level of overwrites an allow always takes precedence over a deny. Discord even confirms this on their permissions help page. I reproduced the setup you described in the last paragraph on a server with no other roles or overwrites and got the result that the member with the two roles was in fact able to send messages.

@bdistin
Copy link
Contributor Author

bdistin commented Mar 8, 2017

My apologies, ignore the op.. I must have been tired last night because this morning I discovered I didn't check the box on the lower role I was testing... Testing that specific setup again this morning provided results as you expected... However, if @ everyone has allow, and the specific user has deny: or separately if @ everyone has allow, and the specific user has a role that has deny:

@bdistin bdistin changed the title Role Positions are not being accounted for, when resolving permission overwrites. @ everyone role is overwriting specific role and user denys, when resolving permission overwrites. Mar 8, 2017
@bdistin
Copy link
Contributor Author

bdistin commented Mar 8, 2017

That link made me think of one more test, that also failed. Non-everyone Roles overwrite Member Specific overwrites

@bdistin bdistin changed the title @ everyone role is overwriting specific role and user denys, when resolving permission overwrites. Permission Overwrites resolving incorrectly. Mar 8, 2017
@iCrawl iCrawl closed this as completed in 8729ee6 Mar 15, 2017
devsnek pushed a commit to devsnek/discord.js that referenced this issue May 14, 2017
* Fix discordjs#1253

* apparently @ everyone role can be undefined

* Fix oops

* Fixes possible mutiple roles named '@​everyone'

* Clean up order/logic
devsnek pushed a commit to devsnek/discord.js that referenced this issue May 14, 2017
* Fix discordjs#1253

* apparently @ everyone role can be undefined

* Fix oops

* Fixes possible mutiple roles named '@​everyone'

* Clean up order/logic
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 9, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants