Harden media and gameday access control

This commit is contained in:
Codex
2026-04-24 08:09:31 -05:00
parent cc241d4ae7
commit 22d4f8c017
4 changed files with 333 additions and 70 deletions

View File

@@ -26,6 +26,15 @@
- Auth and session responses remain `no-store` so cached data is limited to app-owned clip state.
- TeamSnap read queries now use cached-first stale-while-revalidate behavior on the client.
## Completed V1 Hardening
- Media and gameday mutations now stay within the authenticated session's selected team and player scope.
- Upload and clip-creation failures now clean up orphaned files before bubbling errors back to the client.
## Completed Asset Source Cleanup
- Editable artwork sources now live in `frontend/assets/design/`, while the exported web-ready images remain in `frontend/public/`.
- The splash artwork in `frontend/public/splash-art.svg` still serves as the editable vector source for the startup images in `frontend/public/`.
- The existing `frontend/public/icon.svg` already covers the app icon artwork, so no separate raster-to-vector conversion was needed there.
## Storage Status
- Backend media persists in the `backend-media` named Docker volume.

View File

@@ -20,6 +20,14 @@ from ..schemas import (
router = APIRouter(prefix="/games", tags=["games"])
def resolve_session_player_id(session: UserSession, requested_player_id: str | None = None) -> str:
if not session.external_team_id or not session.external_player_id:
raise HTTPException(status_code=422, detail="Select a team and player before using gameday")
if requested_player_id and requested_player_id != session.external_player_id:
raise HTTPException(status_code=403, detail="This player does not match your selected session")
return session.external_player_id
def assignment_to_response(assignment: GameAssignment) -> GameAssignmentResponse:
normalized_url = f"/media/files/{assignment.clip.normalized_path}" if assignment.clip.normalized_path else None
return GameAssignmentResponse(
@@ -57,9 +65,7 @@ def list_pins(
session: UserSession = Depends(require_session),
db: Session = Depends(get_db),
) -> list[GameAssignmentResponse]:
player_id = external_player_id or session.external_player_id
if not player_id or not session.external_team_id:
raise HTTPException(status_code=422, detail="Provide a player to list pins")
player_id = resolve_session_player_id(session, external_player_id)
query = select(GameAssignment).join(GameAssignment.clip).where(
GameAssignment.external_team_id == session.external_team_id,
@@ -80,15 +86,15 @@ def list_assignments(
response: Response,
external_game_id: str,
external_player_id: str | None = Query(default=None),
_: UserSession = Depends(require_session),
session: UserSession = Depends(require_session),
db: Session = Depends(get_db),
) -> list[GameAssignmentResponse]:
resolve_session_player_id(session, external_player_id)
query = select(GameAssignment).join(GameAssignment.clip).where(
GameAssignment.external_team_id == session.external_team_id,
GameAssignment.external_game_id == external_game_id,
AudioClip.hidden.is_(False),
)
if external_player_id:
query = query.where(GameAssignment.external_player_id == external_player_id)
assignments = db.scalars(query.order_by(AudioClip.sort_order.asc(), GameAssignment.updated_at.desc())).all()
payload = [assignment_to_response(assignment) for assignment in assignments]
etag, not_modified = prepare_conditional_response(request, payload)
@@ -102,38 +108,42 @@ def list_assignments(
def create_assignment(
external_game_id: str,
payload: GameAssignmentCreate,
_: UserSession = Depends(require_session),
session: UserSession = Depends(require_session),
db: Session = Depends(get_db),
) -> GameAssignmentResponse:
player_id = resolve_session_player_id(session, payload.external_player_id)
if payload.external_team_id != session.external_team_id:
raise HTTPException(status_code=403, detail="This team does not match your selected session")
clip = db.get(AudioClip, payload.clip_id)
if clip is None or clip.normalization_status != "ready":
raise HTTPException(status_code=422, detail="Clip is not ready")
if clip.hidden:
raise HTTPException(status_code=404, detail="Clip not found")
if clip.asset.external_team_id != payload.external_team_id:
if clip.asset.external_team_id != session.external_team_id:
raise HTTPException(status_code=422, detail="Clip does not belong to this team")
if clip.asset.owner_external_player_id != payload.external_player_id:
if clip.asset.owner_external_player_id != player_id:
raise HTTPException(status_code=403, detail="You can only pin clips owned by that player")
assignment = db.scalar(
select(GameAssignment).where(
GameAssignment.external_game_id == external_game_id,
GameAssignment.clip_id == payload.clip_id,
GameAssignment.external_team_id == session.external_team_id,
)
)
if assignment is None:
assignment = GameAssignment(
external_team_id=payload.external_team_id,
external_team_id=session.external_team_id,
external_game_id=external_game_id,
external_player_id=payload.external_player_id,
external_player_id=player_id,
clip_id=payload.clip_id,
batting_slot=payload.batting_slot,
status=payload.status,
)
db.add(assignment)
else:
assignment.external_team_id = payload.external_team_id
assignment.external_player_id = payload.external_player_id
assignment.external_team_id = session.external_team_id
assignment.external_player_id = player_id
assignment.clip_id = payload.clip_id
assignment.batting_slot = payload.batting_slot
assignment.status = payload.status
@@ -147,14 +157,15 @@ def delete_assignment(
external_game_id: str,
assignment_id: int,
external_player_id: str | None = Query(default=None),
_: UserSession = Depends(require_session),
session: UserSession = Depends(require_session),
db: Session = Depends(get_db),
) -> None:
player_id = resolve_session_player_id(session, external_player_id)
assignment = db.get(GameAssignment, assignment_id)
if assignment is None or assignment.external_game_id != external_game_id:
raise HTTPException(status_code=404, detail="Pin not found")
if external_player_id is not None and assignment.external_player_id != external_player_id:
raise HTTPException(status_code=403, detail="Pin does not belong to that player")
if assignment.external_team_id != session.external_team_id or assignment.external_player_id != player_id:
raise HTTPException(status_code=403, detail="Pin does not belong to your selected session")
db.delete(assignment)
db.commit()
@@ -165,13 +176,18 @@ def prepare_game(
request: Request,
response: Response,
external_game_id: str,
_: UserSession = Depends(require_session),
session: UserSession = Depends(require_session),
db: Session = Depends(get_db),
) -> GamePrepResponse:
resolve_session_player_id(session, None)
assignments = db.scalars(
select(GameAssignment)
.join(GameAssignment.clip)
.where(GameAssignment.external_game_id == external_game_id, AudioClip.hidden.is_(False))
.where(
GameAssignment.external_team_id == session.external_team_id,
GameAssignment.external_game_id == external_game_id,
AudioClip.hidden.is_(False),
)
.order_by(AudioClip.sort_order.asc(), GameAssignment.updated_at.desc())
).all()
external_team_id = assignments[0].external_team_id if assignments else ""

View File

@@ -29,6 +29,31 @@ router = APIRouter(prefix="/media", tags=["media"])
DEFAULT_CLIP_LENGTH_MS = 30_000
def resolve_media_scope(
session: UserSession,
*,
requested_team_id: str | None = None,
requested_player_id: str | None = None,
require_player: bool = False,
) -> tuple[str, str | None]:
if session.is_admin:
team_id = requested_team_id or session.external_team_id
player_id = requested_player_id if requested_player_id is not None else session.external_player_id
if not team_id:
raise HTTPException(status_code=422, detail="Select a team before managing media")
if require_player and not player_id:
raise HTTPException(status_code=422, detail="Select a player before managing media")
return team_id, player_id
if not session.external_team_id or not session.external_player_id:
raise HTTPException(status_code=422, detail="Select a team and player before managing media")
if requested_team_id and requested_team_id != session.external_team_id:
raise HTTPException(status_code=403, detail="This team does not match your selected session")
if requested_player_id and requested_player_id != session.external_player_id:
raise HTTPException(status_code=403, detail="This player does not match your selected session")
return session.external_team_id, session.external_player_id
def clip_to_response(clip: AudioClip) -> AudioClipResponse:
normalized_url = f"/media/files/{clip.normalized_path}" if clip.normalized_path else None
waveform = storage.load_or_generate_waveform(clip.asset.storage_path)
@@ -61,10 +86,15 @@ def prepare_conditional_response(
return etag, is_matching_etag(request, etag)
def can_manage_asset(session: UserSession, asset: AudioAsset, owner_external_player_id: str | None = None) -> bool:
if session.is_admin or asset.uploaded_by_session_id == session.id:
def can_manage_asset(session: UserSession, asset: AudioAsset) -> bool:
if session.is_admin:
return True
return owner_external_player_id is not None and asset.owner_external_player_id == owner_external_player_id
return (
session.external_team_id is not None
and session.external_player_id is not None
and asset.external_team_id == session.external_team_id
and asset.owner_external_player_id == session.external_player_id
)
def next_clip_sort_order(db: Session, *, external_team_id: str, owner_external_player_id: str) -> int:
@@ -91,42 +121,52 @@ def create_asset_with_default_clip(
size_bytes: int,
storage_path: str,
) -> AudioAssetResponse:
asset = AudioAsset(
external_team_id=external_team_id,
owner_external_player_id=owner_external_player_id,
uploaded_by_session_id=session.id,
title=title,
original_filename=original_filename,
mime_type=mime_type,
size_bytes=size_bytes,
storage_path=storage_path,
)
db.add(asset)
db.flush()
clip = AudioClip(
asset_id=asset.id,
label=asset.title,
start_ms=0,
end_ms=DEFAULT_CLIP_LENGTH_MS,
sort_order=next_clip_sort_order(
db,
normalized_path: str | None = None
try:
asset = AudioAsset(
external_team_id=external_team_id,
owner_external_player_id=owner_external_player_id,
),
normalization_status="processing",
)
db.add(clip)
db.flush()
uploaded_by_session_id=session.id,
title=title,
original_filename=original_filename,
mime_type=mime_type,
size_bytes=size_bytes,
storage_path=storage_path,
)
db.add(asset)
db.flush()
normalized_name = f"clip-{clip.id}-{secrets.token_hex(6)}{Path(asset.storage_path).suffix or '.bin'}"
clip.normalized_path = storage.normalize_clip(asset.storage_path, normalized_name)
clip.normalization_status = "ready"
storage.generate_waveform(asset.storage_path)
clip = AudioClip(
asset_id=asset.id,
label=asset.title,
start_ms=0,
end_ms=DEFAULT_CLIP_LENGTH_MS,
sort_order=next_clip_sort_order(
db,
external_team_id=external_team_id,
owner_external_player_id=owner_external_player_id,
),
normalization_status="processing",
)
db.add(clip)
db.flush()
db.commit()
db.refresh(asset)
return AudioAssetResponse.model_validate(asset, from_attributes=True)
normalized_name = f"clip-{clip.id}-{secrets.token_hex(6)}{Path(asset.storage_path).suffix or '.bin'}"
normalized_path = str(Path("normalized") / normalized_name)
normalized_path = storage.normalize_clip(asset.storage_path, normalized_name)
clip.normalized_path = normalized_path
clip.normalization_status = "ready"
storage.generate_waveform(asset.storage_path)
db.commit()
db.refresh(asset)
return AudioAssetResponse.model_validate(asset, from_attributes=True)
except Exception:
db.rollback()
if normalized_path:
storage.delete_relative_path(normalized_path)
storage.delete_relative_path(storage_path)
raise
def download_media_to_storage(url: str) -> tuple[str, int, str, str]:
@@ -180,6 +220,12 @@ async def upload_audio(
session: UserSession = Depends(require_session),
db: Session = Depends(get_db),
) -> AudioAssetResponse:
external_team_id, owner_external_player_id = resolve_media_scope(
session,
requested_team_id=external_team_id,
requested_player_id=owner_external_player_id,
require_player=True,
)
extension = Path(file.filename or "upload.bin").suffix or ".bin"
storage_name = f"{secrets.token_hex(16)}{extension}"
relative_path, size = storage.save_upload(file, storage_name)
@@ -202,6 +248,12 @@ def import_audio(
session: UserSession = Depends(require_session),
db: Session = Depends(get_db),
) -> AudioAssetResponse:
external_team_id, owner_external_player_id = resolve_media_scope(
session,
requested_team_id=payload.external_team_id,
requested_player_id=payload.owner_external_player_id,
require_player=True,
)
relative_path, size_bytes, original_filename, source_title = download_media_to_storage(payload.url)
title = payload.title.strip() if payload.title else ""
if not title:
@@ -226,9 +278,14 @@ def list_assets(
response: Response,
external_team_id: str,
owner_external_player_id: str | None = None,
_: UserSession = Depends(require_session),
session: UserSession = Depends(require_session),
db: Session = Depends(get_db),
) -> list[AudioAssetResponse]:
external_team_id, owner_external_player_id = resolve_media_scope(
session,
requested_team_id=external_team_id,
requested_player_id=owner_external_player_id,
)
query = select(AudioAsset).where(AudioAsset.external_team_id == external_team_id)
if owner_external_player_id:
query = query.where(AudioAsset.owner_external_player_id == owner_external_player_id)
@@ -251,7 +308,7 @@ def delete_asset(
asset = db.get(AudioAsset, asset_id)
if asset is None:
raise HTTPException(status_code=404, detail="Asset not found")
if not can_manage_asset(session, asset, owner_external_player_id):
if not can_manage_asset(session, asset):
raise HTTPException(status_code=403, detail="You can only delete your own uploads")
clips = db.scalars(select(AudioClip).where(AudioClip.asset_id == asset.id)).all()
@@ -279,7 +336,7 @@ def update_asset(
asset = db.get(AudioAsset, asset_id)
if asset is None:
raise HTTPException(status_code=404, detail="Asset not found")
if not can_manage_asset(session, asset, owner_external_player_id):
if not can_manage_asset(session, asset):
raise HTTPException(status_code=403, detail="You can only update your own uploads")
title = payload.title.strip()
@@ -295,15 +352,21 @@ def update_asset(
@router.post("/clips", response_model=AudioClipResponse)
def create_clip(
payload: AudioClipCreate,
_: UserSession = Depends(require_session),
session: UserSession = Depends(require_session),
db: Session = Depends(get_db),
) -> AudioClipResponse:
external_team_id, owner_external_player_id = resolve_media_scope(
session,
requested_team_id=payload.external_team_id,
requested_player_id=payload.owner_external_player_id,
require_player=True,
)
asset = db.get(AudioAsset, payload.asset_id)
if asset is None:
raise HTTPException(status_code=404, detail="Asset not found")
if asset.external_team_id != payload.external_team_id:
if asset.external_team_id != external_team_id:
raise HTTPException(status_code=422, detail="Clip does not belong to this team")
if asset.owner_external_player_id != payload.owner_external_player_id:
if asset.owner_external_player_id != owner_external_player_id:
raise HTTPException(status_code=403, detail="You can only create clips for that player")
if payload.end_ms <= payload.start_ms:
raise HTTPException(status_code=422, detail="Clip end must be greater than start")
@@ -315,8 +378,8 @@ def create_clip(
end_ms=payload.end_ms,
sort_order=next_clip_sort_order(
db,
external_team_id=payload.external_team_id,
owner_external_player_id=payload.owner_external_player_id,
external_team_id=external_team_id,
owner_external_player_id=owner_external_player_id,
),
normalization_status="processing",
)
@@ -342,7 +405,7 @@ def update_clip(
clip = db.get(AudioClip, clip_id)
if clip is None:
raise HTTPException(status_code=404, detail="Clip not found")
if not can_manage_asset(session, clip.asset, owner_external_player_id):
if not can_manage_asset(session, clip.asset):
raise HTTPException(status_code=403, detail="You can only update clips from your own uploads")
if payload.end_ms <= payload.start_ms:
raise HTTPException(status_code=422, detail="Clip end must be greater than start")
@@ -369,7 +432,7 @@ def delete_clip(
clip = db.get(AudioClip, clip_id)
if clip is None:
raise HTTPException(status_code=404, detail="Clip not found")
if not can_manage_asset(session, clip.asset, owner_external_player_id):
if not can_manage_asset(session, clip.asset):
raise HTTPException(status_code=403, detail="You can only delete clips from your own uploads")
db.execute(delete(GameAssignment).where(GameAssignment.clip_id == clip.id))
@@ -386,9 +449,14 @@ def list_clips(
external_team_id: str,
owner_external_player_id: str | None = None,
include_hidden: bool = False,
_: UserSession = Depends(require_session),
session: UserSession = Depends(require_session),
db: Session = Depends(get_db),
) -> list[AudioClipResponse]:
external_team_id, owner_external_player_id = resolve_media_scope(
session,
requested_team_id=external_team_id,
requested_player_id=owner_external_player_id,
)
query = (
select(AudioClip)
.join(AudioClip.asset)
@@ -414,15 +482,19 @@ def reorder_clips(
session: UserSession = Depends(require_session),
db: Session = Depends(get_db),
) -> None:
if not session.is_admin and session.external_team_id != payload.external_team_id:
raise HTTPException(status_code=403, detail="You can only reorder clips for your selected team")
external_team_id, owner_external_player_id = resolve_media_scope(
session,
requested_team_id=payload.external_team_id,
requested_player_id=payload.owner_external_player_id,
require_player=True,
)
clips = db.scalars(
select(AudioClip)
.join(AudioClip.asset)
.where(
AudioAsset.external_team_id == payload.external_team_id,
AudioAsset.owner_external_player_id == payload.owner_external_player_id,
AudioAsset.external_team_id == external_team_id,
AudioAsset.owner_external_player_id == owner_external_player_id,
)
).all()
clips_by_id = {clip.id: clip for clip in clips}

View File

@@ -11,7 +11,7 @@ from fastapi.testclient import TestClient
from app.config import settings
from app.database import Base, SessionLocal, engine
from app.main import app
from app.models import AudioAsset, AudioClip, UserSession
from app.models import AudioAsset, AudioClip, GameAssignment, UserSession
from app.routes.teamsnap import rewrite_teamsnap_urls
@@ -344,6 +344,63 @@ def test_player_can_pin_a_clip_to_multiple_games_independently() -> None:
assert [item["clip_id"] for item in pins.json()] == [first_clip.id]
def test_player_cannot_pin_another_players_clip() -> None:
db = SessionLocal()
owner_session = UserSession(
session_token="owner-session",
provider="teamsnap",
external_team_id="team-1",
external_player_id="player-1",
)
attacker_session = UserSession(
session_token="attacker-session",
provider="teamsnap",
external_team_id="team-1",
external_player_id="player-2",
)
db.add_all([owner_session, attacker_session])
db.flush()
asset = AudioAsset(
external_team_id="team-1",
owner_external_player_id="player-1",
uploaded_by_session_id=owner_session.id,
title="Song",
original_filename="song.mp3",
mime_type="audio/mpeg",
size_bytes=123,
storage_path="uploads/song.mp3",
)
db.add(asset)
db.flush()
clip = AudioClip(
asset_id=asset.id,
label="Intro",
start_ms=0,
end_ms=10000,
normalization_status="ready",
normalized_path="clips/intro.mp3",
)
db.add(clip)
db.commit()
db.refresh(clip)
db.close()
client.cookies.set(settings.session_cookie_name, "attacker-session")
response = client.post(
"/games/game-1/assignments",
json={
"external_team_id": "team-1",
"external_player_id": "player-1",
"clip_id": clip.id,
"batting_slot": 1,
"status": "ready",
},
)
assert response.status_code == 403
def test_upload_creates_default_clip_and_clip_ranges_can_be_updated() -> None:
login = client.post("/auth/admin/login", json={"username": "admin", "password": "admin"})
assert login.status_code == 200
@@ -552,7 +609,12 @@ def test_hidden_clips_are_removed_from_gameday_views_but_remain_pinnable() -> No
def test_clip_updates_can_use_player_scoped_authorization() -> None:
uploader_session = UserSession(session_token="uploader-session", provider="teamsnap")
editor_session = UserSession(session_token="editor-session", provider="teamsnap")
editor_session = UserSession(
session_token="editor-session",
provider="teamsnap",
external_team_id="team-3",
external_player_id="player-3",
)
db = SessionLocal()
db.add_all([uploader_session, editor_session])
@@ -597,6 +659,59 @@ def test_clip_updates_can_use_player_scoped_authorization() -> None:
assert updated_clip["label"] == "Player clip"
def test_clip_updates_reject_cross_player_scopes() -> None:
uploader_session = UserSession(
session_token="uploader-session",
provider="teamsnap",
external_team_id="team-3",
external_player_id="player-3",
)
editor_session = UserSession(
session_token="editor-session",
provider="teamsnap",
external_team_id="team-3",
external_player_id="player-4",
)
db = SessionLocal()
db.add_all([uploader_session, editor_session])
db.flush()
asset = AudioAsset(
external_team_id="team-3",
owner_external_player_id="player-3",
uploaded_by_session_id=uploader_session.id,
title="Player track",
original_filename="player-track.mp3",
mime_type="audio/mpeg",
size_bytes=123,
storage_path="uploads/player-track.mp3",
)
db.add(asset)
db.flush()
clip = AudioClip(
asset_id=asset.id,
label="Player clip",
start_ms=0,
end_ms=30000,
normalization_status="ready",
normalized_path="clips/player-clip.mp3",
)
db.add(clip)
db.commit()
db.refresh(clip)
db.close()
client.cookies.set(settings.session_cookie_name, "editor-session")
update = client.patch(
f"/media/clips/{clip.id}",
params={"owner_external_player_id": "player-3"},
json={"start_ms": 1500, "end_ms": 9000},
)
assert update.status_code == 403
def test_create_clip_uses_team_and_player_scope() -> None:
login = client.post("/auth/admin/login", json={"username": "admin", "password": "admin"})
assert login.status_code == 200
@@ -634,6 +749,57 @@ def test_create_clip_uses_team_and_player_scope() -> None:
assert clip["end_ms"] == 6000
def test_create_asset_with_default_clip_cleans_up_files_on_failure(monkeypatch: pytest.MonkeyPatch) -> None:
from app.routes import media as media_routes
db = SessionLocal()
session = UserSession(
session_token="cleanup-session",
provider="teamsnap",
external_team_id="team-clean",
external_player_id="player-clean",
)
db.add(session)
db.commit()
db.refresh(session)
source_relative_path = "uploads/cleanup-source.wav"
source_path = settings.media_root / source_relative_path
source_path.parent.mkdir(parents=True, exist_ok=True)
source_path.write_bytes(make_test_wav_bytes())
normalized_path = settings.media_root / "normalized" / "cleanup-copy.wav"
def fake_normalize_clip(source_relative_path_arg: str, clip_name: str) -> str:
normalized_path.parent.mkdir(parents=True, exist_ok=True)
normalized_path.write_bytes((settings.media_root / source_relative_path_arg).read_bytes())
return "normalized/cleanup-copy.wav"
def fake_generate_waveform(source_relative_path_arg: str, bins: int = media_routes.WAVEFORM_PEAK_COUNT) -> dict[str, int | list[int]]:
raise RuntimeError("waveform failed")
monkeypatch.setattr(media_routes.storage, "normalize_clip", fake_normalize_clip)
monkeypatch.setattr(media_routes.storage, "generate_waveform", fake_generate_waveform)
with pytest.raises(RuntimeError):
media_routes.create_asset_with_default_clip(
db=db,
session=session,
external_team_id="team-clean",
owner_external_player_id="player-clean",
title="Cleanup track",
original_filename="cleanup-source.wav",
mime_type="audio/wav",
size_bytes=source_path.stat().st_size,
storage_path=source_relative_path,
)
assert not source_path.exists()
assert not normalized_path.exists()
assert db.query(AudioAsset).count() == 0
assert db.query(AudioClip).count() == 0
db.close()
def test_asset_title_can_be_edited() -> None:
login = client.post("/auth/admin/login", json={"username": "admin", "password": "admin"})
assert login.status_code == 200