diff --git a/commands/injuries/management.py b/commands/injuries/management.py index cf030dd..d8ef483 100644 --- a/commands/injuries/management.py +++ b/commands/injuries/management.py @@ -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) diff --git a/tests/test_injury_ownership.py b/tests/test_injury_ownership.py new file mode 100644 index 0000000..7256c15 --- /dev/null +++ b/tests/test_injury_ownership.py @@ -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