cal/major-domo-v2
1
0
Fork 0
Code Issues 6 Pull Requests 2 Actions Packages Projects Releases Wiki Activity

feat: add is_admin() helper to utils/permissions.py (#55)

Browse Source 
Add centralized `is_admin(interaction)` helper that includes the
`isinstance(interaction.user, discord.Member)` guard, preventing
AttributeError in DM contexts.

Use it in `can_edit_player_image()` which previously accessed
`guild_permissions.administrator` directly without the isinstance
guard. Update the corresponding test to mock the user with
`spec=discord.Member` so the isinstance check passes.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
Cal Corum 2026-03-04 22:33:31 -06:00
parent ee324693b1
commit c41166d53c
3 changed files with 66 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
commands/profile/images.py
View File

@ -22,6 +22,7 @@ from utils.decorators import logged_command
from views.embeds import EmbedColors, EmbedTemplate
from views.base import BaseView
from models.player import Player
from utils.permissions import is_admin
# URL Validation Functions
@ -117,7 +118,7 @@ async def can_edit_player_image(
If has permission, error_message is empty string
"""
# Admins can edit anyone
if interaction.user.guild_permissions.administrator:
if is_admin(interaction):
logger.debug("User is admin, granting permission", user_id=interaction.user.id)
return True, ""

27
tests/test_commands_profile_images.py
View File

@ -3,17 +3,19 @@ Tests for player image management commands.
Covers URL validation, permission checking, and command execution.
"""
import pytest
import asyncio
from unittest.mock import MagicMock, patch
import aiohttp
import discord
from aioresponses import aioresponses
from commands.profile.images import (
validate_url_format,
check_url_accessibility,
can_edit_player_image,
ImageCommands
ImageCommands,
)
from tests.factories import PlayerFactory, TeamFactory
@ -94,7 +96,7 @@ class TestURLAccessibility:
url = "https://example.com/image.jpg"
with aioresponses() as m:
m.head(url, status=200, headers={'content-type': 'image/jpeg'})
m.head(url, status=200, headers={"content-type": "image/jpeg"})
is_accessible, error = await check_url_accessibility(url)
@ -118,7 +120,7 @@ class TestURLAccessibility:
url = "https://example.com/page.html"
with aioresponses() as m:
m.head(url, status=200, headers={'content-type': 'text/html'})
m.head(url, status=200, headers={"content-type": "text/html"})
is_accessible, error = await check_url_accessibility(url)
@ -157,6 +159,7 @@ class TestPermissionChecking:
async def test_admin_can_edit_any_player(self):
"""Test administrator can edit any player's images."""
mock_interaction = MagicMock()
mock_interaction.user = MagicMock(spec=discord.Member)
mock_interaction.user.id = 12345
mock_interaction.user.guild_permissions.administrator = True
@ -186,7 +189,9 @@ class TestPermissionChecking:
mock_logger = MagicMock()
with patch('commands.profile.images.team_service.get_teams_by_owner') as mock_get_teams:
with patch(
"commands.profile.images.team_service.get_teams_by_owner"
) as mock_get_teams:
mock_get_teams.return_value = [user_team]
has_permission, error = await can_edit_player_image(
@ -211,7 +216,9 @@ class TestPermissionChecking:
mock_logger = MagicMock()
with patch('commands.profile.images.team_service.get_teams_by_owner') as mock_get_teams:
with patch(
"commands.profile.images.team_service.get_teams_by_owner"
) as mock_get_teams:
mock_get_teams.return_value = [user_team]
has_permission, error = await can_edit_player_image(
@ -236,7 +243,9 @@ class TestPermissionChecking:
mock_logger = MagicMock()
with patch('commands.profile.images.team_service.get_teams_by_owner') as mock_get_teams:
with patch(
"commands.profile.images.team_service.get_teams_by_owner"
) as mock_get_teams:
mock_get_teams.return_value = [user_team]
has_permission, error = await can_edit_player_image(
@ -258,7 +267,9 @@ class TestPermissionChecking:
mock_logger = MagicMock()
with patch('commands.profile.images.team_service.get_teams_by_owner') as mock_get_teams:
with patch(
"commands.profile.images.team_service.get_teams_by_owner"
) as mock_get_teams:
mock_get_teams.return_value = []
has_permission, error = await can_edit_player_image(
@ -299,7 +310,7 @@ class TestImageCommandsIntegration:
async def test_set_image_command_structure(self, commands_cog):
"""Test that set_image command is properly configured."""
assert hasattr(commands_cog, 'set_image')
assert hasattr(commands_cog, "set_image")
assert commands_cog.set_image.name == "set-image"
async def test_fancy_card_updates_vanity_card_field(self, commands_cog):

64
utils/permissions.py
View File

@ -7,6 +7,7 @@ servers and user types:
- @league_only: Only available in the league server
- @requires_team: User must have a team (works with global commands)
"""
import logging
from functools import wraps
from typing import Optional, Callable
@ -21,6 +22,7 @@ logger = logging.getLogger(__name__)
class PermissionError(Exception):
"""Raised when a user doesn't have permission to use a command."""
pass
@ -54,17 +56,16 @@ async def get_user_team(user_id: int) -> Optional[dict]:
# This call is automatically cached by TeamService
config = get_config()
team = await team_service.get_team_by_owner(
owner_id=user_id,
season=config.sba_season
owner_id=user_id, season=config.sba_season
)
if team:
logger.debug(f"User {user_id} has team: {team.lname}")
return {
'id': team.id,
'name': team.lname,
'abbrev': team.abbrev,
'season': team.season
"id": team.id,
"name": team.lname,
"abbrev": team.abbrev,
"season": team.season,
}
logger.debug(f"User {user_id} does not have a team")
@ -77,6 +78,18 @@ def is_league_server(guild_id: int) -> bool:
return guild_id == config.guild_id
def is_admin(interaction: discord.Interaction) -> bool:
"""Check if the interaction user is a server administrator.
Includes an isinstance guard for discord.Member so this is safe
to call from DM contexts (where guild_permissions is unavailable).
"""
return (
isinstance(interaction.user, discord.Member)
and interaction.user.guild_permissions.administrator
)
def league_only():
"""
Decorator to restrict a command to the league server only.
@ -87,14 +100,14 @@ def league_only():
async def team_command(self, interaction: discord.Interaction):
# Only executes in league server
"""
def decorator(func: Callable) -> Callable:
@wraps(func)
async def wrapper(self, interaction: discord.Interaction, *args, **kwargs):
# Check if in a guild
if not interaction.guild:
await interaction.response.send_message(
"❌ This command can only be used in a server.",
ephemeral=True
"❌ This command can only be used in a server.", ephemeral=True
)
return
@ -102,13 +115,14 @@ def league_only():
if not is_league_server(interaction.guild.id):
await interaction.response.send_message(
"❌ This command is only available in the SBa league server.",
ephemeral=True
ephemeral=True,
)
return
return await func(self, interaction, *args, **kwargs)
return wrapper
return decorator
@ -123,6 +137,7 @@ def requires_team():
async def mymoves_command(self, interaction: discord.Interaction):
# Only executes if user has a team
"""
def decorator(func: Callable) -> Callable:
@wraps(func)
async def wrapper(self, interaction: discord.Interaction, *args, **kwargs):
@ -133,29 +148,33 @@ def requires_team():
if team is None:
await interaction.response.send_message(
"❌ This command requires you to have a team in the SBa league. Contact an admin if you believe this is an error.",
ephemeral=True
ephemeral=True,
)
return
# Store team info in interaction for command to use
# This allows commands to access the team without another lookup
interaction.extras['user_team'] = team
interaction.extras["user_team"] = team
return await func(self, interaction, *args, **kwargs)
except Exception as e:
# Log the error for debugging
logger.error(f"Error checking team ownership for user {interaction.user.id}: {e}", exc_info=True)
logger.error(
f"Error checking team ownership for user {interaction.user.id}: {e}",
exc_info=True,
)
# Provide helpful error message to user
await interaction.response.send_message(
"❌ Unable to verify team ownership due to a temporary error. Please try again in a moment. "
"If this persists, contact an admin.",
ephemeral=True
ephemeral=True,
)
return
return wrapper
return decorator
@ -170,12 +189,14 @@ def global_command():
async def roll_command(self, interaction: discord.Interaction):
# Available in all servers
"""
def decorator(func: Callable) -> Callable:
@wraps(func)
async def wrapper(self, interaction: discord.Interaction, *args, **kwargs):
return await func(self, interaction, *args, **kwargs)
return wrapper
return decorator
@ -190,35 +211,35 @@ def admin_only():
async def sync_command(self, interaction: discord.Interaction):
# Only executes for admins
"""
def decorator(func: Callable) -> Callable:
@wraps(func)
async def wrapper(self, interaction: discord.Interaction, *args, **kwargs):
# Check if user is guild admin
if not interaction.guild:
await interaction.response.send_message(
"❌ This command can only be used in a server.",
ephemeral=True
"❌ This command can only be used in a server.", ephemeral=True
)
return
# Check if user has admin permissions
if not isinstance(interaction.user, discord.Member):
await interaction.response.send_message(
"❌ Unable to verify permissions.",
ephemeral=True
"❌ Unable to verify permissions.", ephemeral=True
)
return
if not interaction.user.guild_permissions.administrator:
await interaction.response.send_message(
"❌ This command requires administrator permissions.",
ephemeral=True
ephemeral=True,
)
return
return await func(self, interaction, *args, **kwargs)
return wrapper
return decorator
@ -241,6 +262,7 @@ def league_admin_only():
async def admin_sync_prefix(self, ctx: commands.Context):
# Only league server admins can use this
"""
def decorator(func: Callable) -> Callable:
@wraps(func)
async def wrapper(self, ctx_or_interaction, *args, **kwargs):
@ -254,6 +276,7 @@ def league_admin_only():
async def send_error(msg: str):
await ctx.send(msg)
else:
interaction = ctx_or_interaction
guild = interaction.guild
@ -269,7 +292,9 @@ def league_admin_only():
# Check if league server
if not is_league_server(guild.id):
await send_error("❌ This command is only available in the SBa league server.")
await send_error(
"❌ This command is only available in the SBa league server."
)
return
# Check admin permissions
@ -284,4 +309,5 @@ def league_admin_only():
return await func(self, ctx_or_interaction, *args, **kwargs)
return wrapper
return decorator