Cog-Creators / Red-DiscordBot

A multi-function Discord bot
https://docs.discord.red
GNU General Public License v3.0
4.63k stars 2.28k forks source link

[Permissions] Document COG deny edge case #6395

Open cool-aid-man opened 2 weeks ago

cool-aid-man commented 2 weeks ago

https://github.com/Cog-Creators/Red-DiscordBot/blob/ad1e1aa2ba804cceae29757b48cce0c0c0ae4703/redbot/core/commands/commands.py#L451-L458 The current logic for processing Permissions overrides entirely resolves the COG permissions, then exits early if the resolved state is to deny, and only then resolves COMMAND permissions. Because the process of resolving COG and COMMAND permissions is split up like this, there is no way to maintain the order of operations from [p]permissions explain between these two categories. The output of [p]permissions explain should be modified to clarify this limitation explicitly.

The original issue is maintained below for reproduction steps.


Original issue ### What Red version are you using? 3.5.9 ### What were you trying to do? ## I've got a few scenarios where the `permissions` don't seem to be working as intended. 1st Scene. Where if a user has **both** the roles (1255859061547728966 & 1255859041620721727) they can **Only** use the `hug` command and not the rest of the sub-commands under the same cog. Which they should be able to ```yaml COG: Perform: default: false 1255859061547728966: true COMMAND: hug: 1255859041620721727: true ``` > - Both are `role IDs` (where `1255859041620721727` role is placed higher in the hierarchy than `1255859061547728966`). > - As you can see, there's **No conflict** (as both the rules are stated to - `true`) for the above rule. **Explanation for this `permissions` settings -** - By default its set to `false` for everyone - on cog level (Which means at this point **no one can use ANY commands from the cog `Perform`**) - Rule has override for people with `1255859061547728966` - Which means only `1255859061547728966` can use ALL the commands (including the `hug`) from the cog. - People having `1255859041620721727` role also has an override **allowing** them to use **just the** `hug` command. (This means people having just the `1255859041620721727` role can **only** use `hug` but people with the `1255859061547728966` role can use **every commands** including hug.) The issue comes when the user has **both** the roles (`1255859061547728966` & `1255859041620721727`) - where the cog **stops** the user from running **every** other command **beside** `hug`. Which is weird and shouldn't be happening as the permissions rule for the `1255859041620721727` role, **is not** conflicting with the cog's rule by any means. So, in this case - the cog should be able to let the user run every single command that the cog offers. That includes `hug` & other commands. ----- **Scene 2** Now, at this point if you `deny` the rule for the `1255859041620721727` role on `hug` command with - > - `

permissions addserverrule deny hug 1255859041620721727` Then the cog starts to act very weird - Either it will let the user (**having both the roles**) run every command beside `hug` - **Intended behavior** as you've set the rule to deny - Or, it will **stop** the user (**having both the roles**) from running every command including `hug` - **Not intended** behavior. The cog starts to work once you `restart` the bot (reloading doesn't solve it). Work in the sense of - by respecting this `deny` rule. - @Flame442 was managed to replicate the issue [here](https://discord.com/channels/133049272517001216/387398816317440000/1255970680277831810). ### What did you expect to happen? **Expected Behavior:** When there's **NO conflict** (cog & command both states to `true`) between the cog & its commands, in the permissions rule settings - the `permissions cog` should allow the user (**having both the roles**) run all the commands **regardless** of the roles positions in the hierarchy. By taking **both** the rules from cog & command into account. Because if you just switch the **ruleset** by allowing the `1255859041620721727` role to run cog-wide & overriding the `1255859061547728966` role to `deny` the `hug` command then the cog do take both the rules into account. ```yaml COG: Perform: default: false 1255859041620721727: true COMMAND: hug: 1255859061547728966: false ``` > - Both are `role IDs` (where the `1255859041620721727` role is placed higher in the hierarchy than the `1255859061547728966`). > - As you can see, there's `denied` `hug` permission for the `1255859061547728966` role. In this case, the cog takes both the rule-sets into account and lets the user (having both the roles) to run every other command beside `hug` - which is intended behavior. ### What actually happened? Cog did not let the user (**having both the roles**) run the commands - when there's no conflict. ### How can we reproduce this error? 1. Steps are included above. ^^^ ### Anything else? N/A

Flame442 commented 2 weeks ago

https://github.com/Cog-Creators/Red-DiscordBot/blob/ad1e1aa2ba804cceae29757b48cce0c0c0ae4703/redbot/core/commands/commands.py#L451-L458 The current logic for processing Permissions overrides entirely resolves the COG permissions, then exits early if the resolved state is to deny, and only then resolves COMMAND permissions. Because the process of resolving COG and COMMAND permissions is split up like this, there is no way to maintain the order of operations from [p]permissions explain between these two categories. Fixing this would require entirely rewriting how COG level permissions have been implemented, and is out of scope of what we are able to do.

I am modifying this issue to make the goal be documenting this behavior in [p]permissions explain. ([ref] for org members)