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

feat: add team ownership verification to /injury set-new and /injury clear (closes #18)

Browse Source 
Prevents users from managing injuries for players not on their team.
Admins bypass the check; org affiliates (MiL/IL) are recognized.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Cal Corum 2026-03-01 16:18:30 -06:00
parent 98d47a1262
commit 99489a3c42
2 changed files with 298 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

57
commands/injuries/management.py
View File

@ -42,6 +42,52 @@ class InjuryGroup(app_commands.Group):
self.logger = get_contextual_logger(f"{__name__}.InjuryGroup")
self.logger.info("InjuryGroup initialized")
async def _verify_team_ownership(
self, interaction: discord.Interaction, player: "Player"
) -> bool:
"""
Verify the invoking user owns the team the player is on.
Returns True if ownership is confirmed, False if denied (sends error embed).
Admins bypass the check.
"""
# Admins can manage any team's injuries
if (
isinstance(interaction.user, discord.Member)
and interaction.user.guild_permissions.administrator
):
return True
if not player.team_id:
return True # Can't verify without team data, allow through
from services.team_service import team_service
config = get_config()
user_team = await team_service.get_team_by_owner(
owner_id=interaction.user.id,
season=config.sba_season,
)
if user_team is None:
embed = EmbedTemplate.error(
title="No Team Found",
description="You don't appear to own a team this season.",
)
await interaction.followup.send(embed=embed, ephemeral=True)
return False
player_team = player.team
if player_team is None or not user_team.is_same_organization(player_team):
embed = EmbedTemplate.error(
title="Not Your Player",
description=f"**{player.name}** is not on your team. You can only manage injuries for your own players.",
)
await interaction.followup.send(embed=embed, ephemeral=True)
return False
return True
def has_player_role(self, interaction: discord.Interaction) -> bool:
"""Check if user has the SBA Players role."""
# Cast to Member to access roles (User doesn't have roles attribute)
@ -511,10 +557,9 @@ class InjuryGroup(app_commands.Group):
player.team = await team_service.get_team(player.team_id)
# Check if player is on user's team
# Note: This assumes you have a function to get team by owner
# For now, we'll skip this check - you can add it if needed
# TODO: Add team ownership verification
# Verify the invoking user owns this player's team
if not await self._verify_team_ownership(interaction, player):
return
# Check if player already has an active injury
existing_injury = await injury_service.get_active_injury(
@ -701,6 +746,10 @@ class InjuryGroup(app_commands.Group):
player.team = await team_service.get_team(player.team_id)
# Verify the invoking user owns this player's team
if not await self._verify_team_ownership(interaction, player):
return
# Get active injury
injury = await injury_service.get_active_injury(player.id, current.season)

245
tests/test_injury_ownership.py Normal file
View File

@ -0,0 +1,245 @@
"""Tests for injury command team ownership verification (issue #18).
Ensures /injury set-new and /injury clear only allow users to manage
injuries for players on their own team (or organizational affiliates).
Admins bypass the check.
"""
import discord
import pytest
from unittest.mock import AsyncMock, MagicMock, patch
from commands.injuries.management import InjuryGroup
from models.player import Player
from models.team import Team
def _make_team(team_id: int, abbrev: str, sname: str | None = None) -> Team:
"""Create a Team via model_construct to skip validation.
For MiL teams (e.g. PORMIL), pass sname explicitly to avoid the IL
disambiguation logic in _get_base_abbrev treating them as IL teams.
"""
return Team.model_construct(
id=team_id,
abbrev=abbrev,
sname=sname or abbrev,
lname=f"Team {abbrev}",
season=13,
)
def _make_player(player_id: int, name: str, team: Team) -> Player:
"""Create a Player via model_construct to skip validation."""
return Player.model_construct(
id=player_id,
name=name,
wara=2.0,
season=13,
team_id=team.id,
team=team,
)
def _make_interaction(is_admin: bool = False) -> MagicMock:
"""Create a mock Discord interaction with configurable admin status."""
interaction = MagicMock()
interaction.user = MagicMock()
interaction.user.id = 12345
interaction.user.guild_permissions = MagicMock()
interaction.user.guild_permissions.administrator = is_admin
# Make isinstance(interaction.user, discord.Member) return True
interaction.user.__class__ = discord.Member
interaction.followup = MagicMock()
interaction.followup.send = AsyncMock()
return interaction
@pytest.fixture
def injury_group():
return InjuryGroup()
class TestVerifyTeamOwnership:
"""Tests for InjuryGroup._verify_team_ownership (issue #18)."""
@pytest.mark.asyncio
async def test_admin_bypasses_check(self, injury_group):
"""Admins should always pass the ownership check."""
interaction = _make_interaction(is_admin=True)
por_team = _make_team(1, "POR")
player = _make_player(100, "Mike Trout", por_team)
result = await injury_group._verify_team_ownership(interaction, player)
assert result is True
@pytest.mark.asyncio
async def test_owner_passes_check(self, injury_group):
"""User who owns the player's team should pass."""
interaction = _make_interaction(is_admin=False)
por_team = _make_team(1, "POR")
player = _make_player(100, "Mike Trout", por_team)
with patch("services.team_service.team_service") as mock_ts, patch(
"commands.injuries.management.get_config"
) as mock_config:
mock_config.return_value.sba_season = 13
mock_ts.get_team_by_owner = AsyncMock(return_value=por_team)
result = await injury_group._verify_team_ownership(interaction, player)
assert result is True
@pytest.mark.asyncio
async def test_org_affiliate_passes_check(self, injury_group):
"""User who owns the ML team should pass for MiL/IL affiliate players."""
interaction = _make_interaction(is_admin=False)
por_ml = _make_team(1, "POR")
por_mil = _make_team(2, "PORMIL", sname="POR MiL")
player = _make_player(100, "Minor Leaguer", por_mil)
with patch("services.team_service.team_service") as mock_ts, patch(
"commands.injuries.management.get_config"
) as mock_config:
mock_config.return_value.sba_season = 13
mock_ts.get_team_by_owner = AsyncMock(return_value=por_ml)
mock_ts.get_team = AsyncMock(return_value=por_mil)
result = await injury_group._verify_team_ownership(interaction, player)
assert result is True
@pytest.mark.asyncio
async def test_different_team_fails(self, injury_group):
"""User who owns a different team should be denied."""
interaction = _make_interaction(is_admin=False)
por_team = _make_team(1, "POR")
nyy_team = _make_team(2, "NYY")
player = _make_player(100, "Mike Trout", nyy_team)
with patch("services.team_service.team_service") as mock_ts, patch(
"commands.injuries.management.get_config"
) as mock_config:
mock_config.return_value.sba_season = 13
mock_ts.get_team_by_owner = AsyncMock(return_value=por_team)
mock_ts.get_team = AsyncMock(return_value=nyy_team)
result = await injury_group._verify_team_ownership(interaction, player)
assert result is False
interaction.followup.send.assert_called_once()
call_kwargs = interaction.followup.send.call_args
embed = call_kwargs.kwargs.get("embed") or call_kwargs.args[0]
assert "Not Your Player" in embed.title
@pytest.mark.asyncio
async def test_no_team_owned_fails(self, injury_group):
"""User who owns no team should be denied."""
interaction = _make_interaction(is_admin=False)
nyy_team = _make_team(2, "NYY")
player = _make_player(100, "Mike Trout", nyy_team)
with patch("services.team_service.team_service") as mock_ts, patch(
"commands.injuries.management.get_config"
) as mock_config:
mock_config.return_value.sba_season = 13
mock_ts.get_team_by_owner = AsyncMock(return_value=None)
result = await injury_group._verify_team_ownership(interaction, player)
assert result is False
interaction.followup.send.assert_called_once()
call_kwargs = interaction.followup.send.call_args
embed = call_kwargs.kwargs.get("embed") or call_kwargs.args[0]
assert "No Team Found" in embed.title
@pytest.mark.asyncio
async def test_il_affiliate_passes_check(self, injury_group):
"""User who owns the ML team should pass for IL (injured list) players."""
interaction = _make_interaction(is_admin=False)
por_ml = _make_team(1, "POR")
por_il = _make_team(3, "PORIL", sname="POR IL")
player = _make_player(100, "IL Stash", por_il)
with patch("services.team_service.team_service") as mock_ts, patch(
"commands.injuries.management.get_config"
) as mock_config:
mock_config.return_value.sba_season = 13
mock_ts.get_team_by_owner = AsyncMock(return_value=por_ml)
result = await injury_group._verify_team_ownership(interaction, player)
assert result is True
@pytest.mark.asyncio
async def test_player_team_not_populated_fails(self, injury_group):
"""Player with team_id but unpopulated team object should be denied.
Callers are expected to populate player.team before calling
_verify_team_ownership. If they don't, the method treats the missing
team as a failed check rather than silently allowing access.
"""
interaction = _make_interaction(is_admin=False)
por_team = _make_team(1, "POR")
player = Player.model_construct(
id=100,
name="Orphan Player",
wara=2.0,
season=13,
team_id=99,
team=None,
)
with patch("services.team_service.team_service") as mock_ts, patch(
"commands.injuries.management.get_config"
) as mock_config:
mock_config.return_value.sba_season = 13
mock_ts.get_team_by_owner = AsyncMock(return_value=por_team)
result = await injury_group._verify_team_ownership(interaction, player)
assert result is False
@pytest.mark.asyncio
async def test_error_embeds_are_ephemeral(self, injury_group):
"""Error responses should be ephemeral so only the invoking user sees them."""
interaction = _make_interaction(is_admin=False)
nyy_team = _make_team(2, "NYY")
player = _make_player(100, "Mike Trout", nyy_team)
with patch("services.team_service.team_service") as mock_ts, patch(
"commands.injuries.management.get_config"
) as mock_config:
mock_config.return_value.sba_season = 13
# Test "No Team Found" path
mock_ts.get_team_by_owner = AsyncMock(return_value=None)
await injury_group._verify_team_ownership(interaction, player)
call_kwargs = interaction.followup.send.call_args
assert call_kwargs.kwargs.get("ephemeral") is True
# Reset and test "Not Your Player" path
interaction = _make_interaction(is_admin=False)
por_team = _make_team(1, "POR")
with patch("services.team_service.team_service") as mock_ts, patch(
"commands.injuries.management.get_config"
) as mock_config:
mock_config.return_value.sba_season = 13
mock_ts.get_team_by_owner = AsyncMock(return_value=por_team)
await injury_group._verify_team_ownership(interaction, player)
call_kwargs = interaction.followup.send.call_args
assert call_kwargs.kwargs.get("ephemeral") is True
@pytest.mark.asyncio
async def test_player_without_team_id_passes(self, injury_group):
"""Players with no team_id should pass (can't verify, allow through)."""
interaction = _make_interaction(is_admin=False)
player = Player.model_construct(
id=100,
name="Free Agent",
wara=0.0,
season=13,
team_id=None,
team=None,
)
result = await injury_group._verify_team_ownership(interaction, player)
assert result is True