Compare commits

...

4 Commits

Author SHA1 Message Date
Codex
867090ac05 Allow same-team clip reads in gameday 2026-04-24 10:16:02 -05:00
Codex
6e93a03b0f Exclude API routes from PWA navigation fallback 2026-04-24 10:01:27 -05:00
Codex
022609191f Log TeamSnap token exchange failures 2026-04-24 09:52:29 -05:00
Codex
2574dc52c5 Fix TeamSnap callback stalls 2026-04-24 09:28:58 -05:00
6 changed files with 243 additions and 30 deletions

View File

@@ -1,5 +1,6 @@
from __future__ import annotations from __future__ import annotations
import logging
import secrets import secrets
from datetime import datetime, timedelta, timezone from datetime import datetime, timedelta, timezone
from urllib.parse import urlencode from urllib.parse import urlencode
@@ -13,6 +14,8 @@ from .config import settings
from .database import get_db from .database import get_db
from .models import UserSession from .models import UserSession
logger = logging.getLogger(__name__)
def utcnow() -> datetime: def utcnow() -> datetime:
return datetime.now(timezone.utc) return datetime.now(timezone.utc)
@@ -119,36 +122,57 @@ async def fetch_teamsnap_user_id(access_token: str) -> str | None:
async def exchange_code_for_token(code: str) -> dict: async def exchange_code_for_token(code: str) -> dict:
async with httpx.AsyncClient(timeout=15.0) as client: try:
response = await client.post( async with httpx.AsyncClient(timeout=15.0) as client:
settings.teamsnap_token_url, response = await client.post(
data={ settings.teamsnap_token_url,
"grant_type": "authorization_code", data={
"code": code, "grant_type": "authorization_code",
"redirect_uri": settings.teamsnap_redirect_uri, "code": code,
"client_id": settings.teamsnap_client_id, "redirect_uri": settings.teamsnap_redirect_uri,
"client_secret": settings.teamsnap_client_secret, "client_id": settings.teamsnap_client_id,
}, "client_secret": settings.teamsnap_client_secret,
headers={"Accept": "application/json"}, },
) headers={"Accept": "application/json"},
)
except httpx.HTTPError as exc:
logger.exception("TeamSnap token exchange request failed")
raise HTTPException(status_code=status.HTTP_502_BAD_GATEWAY, detail="TeamSnap token exchange failed") from exc
if response.status_code >= 400: if response.status_code >= 400:
logger.error(
"TeamSnap token exchange rejected: status=%s body=%s redirect_uri=%s client_id_suffix=%s",
response.status_code,
response.text,
settings.teamsnap_redirect_uri,
settings.teamsnap_client_id[-6:] if settings.teamsnap_client_id else "",
)
raise HTTPException(status_code=status.HTTP_502_BAD_GATEWAY, detail="TeamSnap token exchange failed") raise HTTPException(status_code=status.HTTP_502_BAD_GATEWAY, detail="TeamSnap token exchange failed")
return response.json() return response.json()
async def refresh_access_token(refresh_token: str) -> dict: async def refresh_access_token(refresh_token: str) -> dict:
async with httpx.AsyncClient(timeout=15.0) as client: try:
response = await client.post( async with httpx.AsyncClient(timeout=15.0) as client:
settings.teamsnap_token_url, response = await client.post(
data={ settings.teamsnap_token_url,
"grant_type": "refresh_token", data={
"refresh_token": refresh_token, "grant_type": "refresh_token",
"client_id": settings.teamsnap_client_id, "refresh_token": refresh_token,
"client_secret": settings.teamsnap_client_secret, "client_id": settings.teamsnap_client_id,
}, "client_secret": settings.teamsnap_client_secret,
headers={"Accept": "application/json"}, },
) headers={"Accept": "application/json"},
)
except httpx.HTTPError as exc:
logger.exception("TeamSnap token refresh request failed")
raise HTTPException(status_code=status.HTTP_502_BAD_GATEWAY, detail="TeamSnap token refresh failed") from exc
if response.status_code >= 400: if response.status_code >= 400:
logger.error(
"TeamSnap token refresh rejected: status=%s body=%s client_id_suffix=%s",
response.status_code,
response.text,
settings.teamsnap_client_id[-6:] if settings.teamsnap_client_id else "",
)
raise HTTPException(status_code=status.HTTP_502_BAD_GATEWAY, detail="TeamSnap token refresh failed") raise HTTPException(status_code=status.HTTP_502_BAD_GATEWAY, detail="TeamSnap token refresh failed")
return response.json() return response.json()

View File

@@ -2,6 +2,7 @@ from __future__ import annotations
import secrets import secrets
import time import time
from urllib.parse import urlencode
from fastapi import APIRouter, Depends, HTTPException, Query, Request, Response, status from fastapi import APIRouter, Depends, HTTPException, Query, Request, Response, status
from fastapi.responses import JSONResponse, RedirectResponse from fastapi.responses import JSONResponse, RedirectResponse
@@ -42,6 +43,10 @@ def normalize_return_to(return_to: str | None) -> str:
return return_to return return_to
def build_signin_error_redirect_url(message: str) -> str:
return f"/signin?{urlencode({'error': message})}"
@router.get("/teamsnap/start") @router.get("/teamsnap/start")
def teamsnap_start(return_to: str | None = Query(default="/")) -> Response: def teamsnap_start(return_to: str | None = Query(default="/")) -> Response:
if not settings.teamsnap_client_id: if not settings.teamsnap_client_id:
@@ -63,14 +68,31 @@ def teamsnap_start(return_to: str | None = Query(default="/")) -> Response:
@router.get("/teamsnap/callback") @router.get("/teamsnap/callback")
async def teamsnap_callback( async def teamsnap_callback(
request: Request, request: Request,
code: str = Query(...), code: str | None = Query(default=None),
error: str | None = Query(default=None),
db: Session = Depends(get_db), db: Session = Depends(get_db),
) -> Response: ) -> Response:
if error:
redirect = RedirectResponse(
url=build_signin_error_redirect_url(f"TeamSnap sign-in failed: {error}"),
status_code=status.HTTP_303_SEE_OTHER,
)
set_no_store(redirect)
redirect.delete_cookie(settings.auth_return_cookie_name)
return redirect
if not code:
redirect = RedirectResponse(
url=build_signin_error_redirect_url("TeamSnap sign-in did not return an authorization code."),
status_code=status.HTTP_303_SEE_OTHER,
)
set_no_store(redirect)
redirect.delete_cookie(settings.auth_return_cookie_name)
return redirect
token_payload = await exchange_code_for_token(code) token_payload = await exchange_code_for_token(code)
session = UserSession(session_token=create_session_token(), provider="teamsnap") session = UserSession(session_token=create_session_token(), provider="teamsnap")
update_session_tokens(session, token_payload) update_session_tokens(session, token_payload)
if session.access_token:
session.external_user_id = await fetch_teamsnap_user_id(session.access_token)
db.add(session) db.add(session)
db.commit() db.commit()
redirect_target = normalize_return_to(request.cookies.get(settings.auth_return_cookie_name)) redirect_target = normalize_return_to(request.cookies.get(settings.auth_return_cookie_name))
@@ -113,12 +135,21 @@ async def teamsnap_token(
if not session.access_token: if not session.access_token:
raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED, detail="Missing TeamSnap access token") raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED, detail="Missing TeamSnap access token")
session_updated = False
if not session.external_user_id:
session.external_user_id = await fetch_teamsnap_user_id(session.access_token)
session_updated = session.external_user_id is not None
expires_soon = session.token_expires_at is None or session.token_expires_at.timestamp() <= (time.time() + 60) expires_soon = session.token_expires_at is None or session.token_expires_at.timestamp() <= (time.time() + 60)
if expires_soon and session.refresh_token: if expires_soon and session.refresh_token:
token_payload = await refresh_access_token(session.refresh_token) token_payload = await refresh_access_token(session.refresh_token)
update_session_tokens(session, token_payload) update_session_tokens(session, token_payload)
if not session.external_user_id and session.access_token: if not session.external_user_id and session.access_token:
session.external_user_id = await fetch_teamsnap_user_id(session.access_token) session.external_user_id = await fetch_teamsnap_user_id(session.access_token)
session_updated = True
if session_updated or expires_soon:
db.add(session) db.add(session)
db.commit() db.commit()
db.refresh(session) db.refresh(session)

View File

@@ -54,6 +54,26 @@ def resolve_media_scope(
return session.external_team_id, session.external_player_id return session.external_team_id, session.external_player_id
def resolve_media_read_scope(
session: UserSession,
*,
requested_team_id: str | None = None,
requested_player_id: str | None = None,
) -> 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 viewing media")
return team_id, player_id
if not session.external_team_id:
raise HTTPException(status_code=422, detail="Select a team before viewing 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")
return session.external_team_id, requested_player_id
def clip_to_response(clip: AudioClip) -> AudioClipResponse: def clip_to_response(clip: AudioClip) -> AudioClipResponse:
normalized_url = f"/media/files/{clip.normalized_path}" if clip.normalized_path else None normalized_url = f"/media/files/{clip.normalized_path}" if clip.normalized_path else None
waveform = storage.load_or_generate_waveform(clip.asset.storage_path) waveform = storage.load_or_generate_waveform(clip.asset.storage_path)
@@ -452,7 +472,7 @@ def list_clips(
session: UserSession = Depends(require_session), session: UserSession = Depends(require_session),
db: Session = Depends(get_db), db: Session = Depends(get_db),
) -> list[AudioClipResponse]: ) -> list[AudioClipResponse]:
external_team_id, owner_external_player_id = resolve_media_scope( external_team_id, owner_external_player_id = resolve_media_read_scope(
session, session,
requested_team_id=external_team_id, requested_team_id=external_team_id,
requested_player_id=owner_external_player_id, requested_player_id=owner_external_player_id,

View File

@@ -111,6 +111,89 @@ def test_teamsnap_token_returns_proxy_api_root() -> None:
assert response.headers["cache-control"] == "no-store" assert response.headers["cache-control"] == "no-store"
def test_teamsnap_callback_redirects_without_waiting_for_user_lookup(monkeypatch: pytest.MonkeyPatch) -> None:
from app.routes import auth as auth_routes
async def fake_exchange_code_for_token(code: str) -> dict:
assert code == "test-code"
return {
"access_token": "callback-access-token",
"refresh_token": "callback-refresh-token",
"expires_in": 3600,
}
async def fail_fetch_teamsnap_user_id(_: str) -> str | None:
raise AssertionError("callback should not fetch the TeamSnap user profile before redirecting")
monkeypatch.setattr(auth_routes, "exchange_code_for_token", fake_exchange_code_for_token)
monkeypatch.setattr(auth_routes, "fetch_teamsnap_user_id", fail_fetch_teamsnap_user_id)
client.cookies.set(settings.auth_return_cookie_name, "/library")
response = client.get("/auth/teamsnap/callback", params={"code": "test-code"}, follow_redirects=False)
assert response.status_code == 303
assert response.headers["location"] == "/library"
assert response.headers["cache-control"] == "no-store"
assert settings.session_cookie_name in response.cookies
db = SessionLocal()
session = db.query(UserSession).filter_by(session_token=response.cookies[settings.session_cookie_name]).one()
assert session.provider == "teamsnap"
assert session.access_token == "callback-access-token"
assert session.refresh_token == "callback-refresh-token"
assert session.external_user_id is None
db.close()
def test_teamsnap_callback_redirects_to_signin_when_code_is_blank() -> None:
client.cookies.set(settings.auth_return_cookie_name, "/library")
response = client.get("/auth/teamsnap/callback", params={"code": ""}, follow_redirects=False)
assert response.status_code == 303
assert response.headers["location"] == "/signin?error=TeamSnap+sign-in+did+not+return+an+authorization+code."
assert settings.auth_return_cookie_name not in response.cookies
def test_teamsnap_callback_redirects_to_signin_when_teamsnap_returns_error() -> None:
client.cookies.set(settings.auth_return_cookie_name, "/library")
response = client.get("/auth/teamsnap/callback", params={"error": "access_denied"}, follow_redirects=False)
assert response.status_code == 303
assert response.headers["location"] == "/signin?error=TeamSnap+sign-in+failed%3A+access_denied"
assert settings.auth_return_cookie_name not in response.cookies
def test_teamsnap_token_backfills_external_user_id(monkeypatch: pytest.MonkeyPatch) -> None:
from app.routes import auth as auth_routes
async def fake_fetch_teamsnap_user_id(access_token: str) -> str | None:
assert access_token == "token-value"
return "user-42"
monkeypatch.setattr(auth_routes, "fetch_teamsnap_user_id", fake_fetch_teamsnap_user_id)
db = SessionLocal()
session = UserSession(session_token="teamsnap-session", provider="teamsnap", access_token="token-value")
db.add(session)
db.commit()
db.close()
client.cookies.set(settings.session_cookie_name, "teamsnap-session")
response = client.post(
"/auth/teamsnap/token",
headers={"host": "kif.local.ascorrea.com", "x-forwarded-proto": "https"},
)
assert response.status_code == 200
db = SessionLocal()
refreshed_session = db.query(UserSession).filter_by(session_token="teamsnap-session").one()
assert refreshed_session.external_user_id == "user-42"
db.close()
def test_session_and_clip_reads_use_cache_validators() -> None: def test_session_and_clip_reads_use_cache_validators() -> None:
login = client.post("/auth/admin/login", json={"username": "admin", "password": "admin"}) login = client.post("/auth/admin/login", json={"username": "admin", "password": "admin"})
assert login.status_code == 200 assert login.status_code == 200
@@ -607,6 +690,58 @@ def test_hidden_clips_are_removed_from_gameday_views_but_remain_pinnable() -> No
assert [item["clip_id"] for item in pins.json()] == [clip.id] assert [item["clip_id"] for item in pins.json()] == [clip.id]
def test_same_team_player_can_read_another_players_clips() -> None:
db = SessionLocal()
owner_session = UserSession(
session_token="owner-library-session",
provider="teamsnap",
external_team_id="team-share",
external_player_id="player-owner",
)
viewer_session = UserSession(
session_token="viewer-library-session",
provider="teamsnap",
external_team_id="team-share",
external_player_id="player-viewer",
)
db.add_all([owner_session, viewer_session])
db.flush()
asset = AudioAsset(
external_team_id="team-share",
owner_external_player_id="player-owner",
uploaded_by_session_id=owner_session.id,
title="Shared song",
original_filename="shared-song.mp3",
mime_type="audio/mpeg",
size_bytes=123,
storage_path="uploads/shared-song.mp3",
)
db.add(asset)
db.flush()
clip = AudioClip(
asset_id=asset.id,
label="Shared clip",
start_ms=0,
end_ms=10000,
normalization_status="ready",
normalized_path="normalized/shared-clip.mp3",
)
db.add(clip)
db.commit()
db.close()
client.cookies.set(settings.session_cookie_name, "viewer-library-session")
response = client.get(
"/media/clips",
params={"external_team_id": "team-share", "owner_external_player_id": "player-owner"},
)
assert response.status_code == 200
assert [item["id"] for item in response.json()] == [clip.id]
assert response.json()[0]["owner_external_player_id"] == "player-owner"
def test_clip_updates_can_use_player_scoped_authorization() -> None: def test_clip_updates_can_use_player_scoped_authorization() -> None:
uploader_session = UserSession(session_token="uploader-session", provider="teamsnap") uploader_session = UserSession(session_token="uploader-session", provider="teamsnap")
editor_session = UserSession( editor_session = UserSession(

View File

@@ -1,12 +1,14 @@
import { useState } from "react"; import { useState } from "react";
import { useLocation, useNavigate } from "react-router-dom"; import { useLocation, useNavigate, useSearchParams } from "react-router-dom";
import { api } from "../api/client"; import { api } from "../api/client";
export function SignInPage() { export function SignInPage() {
const navigate = useNavigate(); const navigate = useNavigate();
const location = useLocation(); const location = useLocation();
const [searchParams] = useSearchParams();
const [error, setError] = useState<string | null>(null); const [error, setError] = useState<string | null>(null);
const callbackError = searchParams.get("error");
async function handleTeamSnapStart() { async function handleTeamSnapStart() {
try { try {
@@ -46,10 +48,10 @@ export function SignInPage() {
</div> </div>
</div> </div>
</div> </div>
{error ? ( {error || callbackError ? (
<div className="col-12 col-md-8 col-lg-5 col-xl-4"> <div className="col-12 col-md-8 col-lg-5 col-xl-4">
<div className="alert alert-danger mb-0" role="alert"> <div className="alert alert-danger mb-0" role="alert">
{error} {error ?? callbackError}
</div> </div>
</div> </div>
) : null} ) : null}

View File

@@ -23,6 +23,7 @@ export default defineConfig(({ mode }) => {
"apple-splash-1290x2796.png", "apple-splash-1290x2796.png",
], ],
workbox: { workbox: {
navigateFallbackDenylist: [/^\/api\//],
runtimeCaching: [ runtimeCaching: [
{ {
urlPattern: ({ url }) => url.pathname.startsWith("/api/media/files/"), urlPattern: ({ url }) => url.pathname.startsWith("/api/media/files/"),