Fix TeamSnap callback stalls

backend/app/routes/auth.py

@@ -2,6 +2,7 @@ from __future__ import annotations
 
 import secrets
 import time
+from urllib.parse import urlencode
 
 from fastapi import APIRouter, Depends, HTTPException, Query, Request, Response, status
 from fastapi.responses import JSONResponse, RedirectResponse
@@ -42,6 +43,10 @@ def normalize_return_to(return_to: str | None) -> str:
     return return_to
 
 
+def build_signin_error_redirect_url(message: str) -> str:
+    return f"/signin?{urlencode({'error': message})}"
+
+
 @router.get("/teamsnap/start")
 def teamsnap_start(return_to: str | None = Query(default="/")) -> Response:
     if not settings.teamsnap_client_id:
@@ -63,14 +68,31 @@ def teamsnap_start(return_to: str | None = Query(default="/")) -> Response:
 @router.get("/teamsnap/callback")
 async def teamsnap_callback(
     request: Request,
-    code: str = Query(...),
+    code: str | None = Query(default=None),
+    error: str | None = Query(default=None),
     db: Session = Depends(get_db),
 ) -> 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)
     session = UserSession(session_token=create_session_token(), provider="teamsnap")
     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.commit()
     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:
         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)
     if expires_soon and session.refresh_token:
         token_payload = await refresh_access_token(session.refresh_token)
         update_session_tokens(session, token_payload)
         if not session.external_user_id and 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.commit()
         db.refresh(session)

backend/tests/test_api.py

@@ -111,6 +111,89 @@ def test_teamsnap_token_returns_proxy_api_root() -> None:
     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:
     login = client.post("/auth/admin/login", json={"username": "admin", "password": "admin"})
     assert login.status_code == 200