Skip to content
New issue

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

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

Already on GitHub? # to your account

Fix @commands.can_manage_channel always passing #6398

Merged
merged 2 commits into from
Jul 10, 2024

Conversation

Flame442
Copy link
Member

@Flame442 Flame442 commented Jul 9, 2024

Description of the changes

def can_manage_channel(*, allow_thread_owner: bool = False) -> Callable[[_T], _T]:
"""Restrict the command to users with permissions to manage channel.
This check properly resolves the permissions for `discord.Thread` as well.
This check can be overridden by rules.
Parameters
----------
allow_thread_owner: bool
If ``True``, the command will also be allowed to run if the author is a thread owner.
This can, for example, be useful to check if the author can edit a channel/thread's name
as that, in addition to members with manage channel/threads permission,
can also be done by the thread owner.
"""
return _can_manage_channel_deco(allow_thread_owner)

@commands.can_manage_channel passes the bool value allow_thread_owner as the first parameter to _can_manage_channel_deco

def _can_manage_channel_deco(
privilege_level: Optional[PrivilegeLevel] = None, allow_thread_owner: bool = False
) -> Callable[[_T], _T]:
async def predicate(ctx: "Context") -> bool:
if utils.can_user_manage_channel(
ctx.author, ctx.channel, allow_thread_owner=allow_thread_owner
):
return True
if privilege_level is not None:
if await PrivilegeLevel.from_ctx(ctx) >= privilege_level:
return True
return False
return permissions_check(predicate)

_can_manage_channel_deco expects the first parameter to be a PrivilegeLevel. When a bool is passed instead, the eventual comparison results in if await PrivilegeLevel.from_ctx(ctx) >= <bool>.

class PrivilegeLevel(enum.IntEnum):
"""Enumeration for special privileges."""
# Maintainer Note: do NOT re-order these.
# Each privilege level also implies access to the ones before it.
# Inserting new privilege levels at a later point is fine if that is considered.
NONE = enum.auto()
"""No special privilege level."""
MOD = enum.auto()
"""User has the mod role."""
ADMIN = enum.auto()
"""User has the admin role."""
GUILD_OWNER = enum.auto()
"""User is the guild level."""
BOT_OWNER = enum.auto()
"""User is a bot owner."""

Since enum.auto increments starting from 1, and bools are interpreted as an int of 0 or 1, the PrivilegeLevel requirement is always passed. As a result, this check always passes, regardless of the channel state or the permissions of the invoker. This is not the case for all other checks that use the _can_manage_channel_deco helper, as those checks explicitly pass a PrivilegeLevel.

Have the changes in this PR been tested?

Yes

@Flame442 Flame442 added Type: Bug Unexpected behavior, result, or exception. In case of PRs, it is a fix for the foregoing. Release Blocker This needs handling prior to the next non-hotfix release. labels Jul 9, 2024
@Flame442 Flame442 added this to the 3.5.10 milestone Jul 9, 2024
@github-actions github-actions bot added the Category: Core - API - Commands Package This is related to the `redbot.core.commands` package or `redbot.core.checks` module. label Jul 9, 2024
@Jackenmen Jackenmen modified the milestones: 3.5.11, 3.5.10 Jul 10, 2024
@Jackenmen Jackenmen added the Changelog Entry: Added Changelog entry for this PR has already been added to changelog PR. label Jul 10, 2024
Copy link
Member

@Jackenmen Jackenmen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Jackenmen Jackenmen merged commit 0b0b23b into Cog-Creators:V3/develop Jul 10, 2024
18 checks passed
@Flame442 Flame442 deleted the fix-can_manage_channel branch July 10, 2024 18:48
BenCos17 added a commit to JARVIS-discordbot/Red-DiscordBot-jarvis that referenced this pull request Jul 11, 2024
thanks Redbot team

* Docs: remove Atom from the list of recommended editors (Cog-Creators#6388)

* Fix info.json keys in approved CC guide (Cog-Creators#6382)

* Use YouTube source plugin over the deprecated built-in source (Cog-Creators#6373)

Co-authored-by: aikaterna <20862007+aikaterna@users.noreply.github.com>

* Bump d.py version to 2.4.0 (Cog-Creators#6401)

Co-authored-by: Ryan <yamikaitou@gmail.com>

* Fix `@commands.can_manage_channel` always passing (Cog-Creators#6398)

* Bump dependencies (Cog-Creators#6402)

* Red 3.5.10 - Changelog (Cog-Creators#6403)

* Automated Crowdin downstream (Cog-Creators#6405)

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* Version bump to 3.5.10 (Cog-Creators#6404)

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* Last-minute changelog fixes

* Version bump to 3.5.11.dev1 (Cog-Creators#6406)

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

---------

Co-authored-by: Seaswimmer <102361830+SeaswimmerTheFsh@users.noreply.github.com>
Co-authored-by: Michael Oliveira <34169552+Flame442@users.noreply.github.com>
Co-authored-by: Jakub Kuczys <me@jacken.men>
Co-authored-by: aikaterna <20862007+aikaterna@users.noreply.github.com>
Co-authored-by: Ryan <yamikaitou@gmail.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Dav-Git pushed a commit to Dav-Git/Red-DiscordBot that referenced this pull request Sep 8, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Category: Core - API - Commands Package This is related to the `redbot.core.commands` package or `redbot.core.checks` module. Changelog Entry: Added Changelog entry for this PR has already been added to changelog PR. Release Blocker This needs handling prior to the next non-hotfix release. Type: Bug Unexpected behavior, result, or exception. In case of PRs, it is a fix for the foregoing.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants