Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 50 additions & 0 deletions app/routers/auth_providers.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,28 @@ def _frontend_complete_url(provider_name: str, *, associated: bool = False) -> s
return f"{settings.frontend_url}/callback/{provider_name}-{suffix}"


def _frontend_associate_error_url(provider_name: str, code: str) -> str:
"""Where to drop the user when the associate flow fails.

The browser lands on the profile page (where they started), with the
error code in the query string. Otherwise FastAPI would render raw
JSON to the user in prod, where Steam/Google redirect straight to
this API rather than through the SPA's callback page.
"""
return (
f"{settings.frontend_url}/profile?associate_error={code}&associate_provider={provider_name}"
)


def _oauth_error_code(detail: Any) -> str | None:
"""Pull the ``code`` string out of an HTTPException's structured detail."""
if isinstance(detail, dict):
code = detail.get("code")
if isinstance(code, str):
return code
return None


# --- State (CSRF + purpose binding) ---------------------------------------


Expand Down Expand Up @@ -390,6 +412,34 @@ async def associate_callback(
request: Request,
user: User = Depends(current_active_user),
session: AsyncSession = Depends(get_async_session),
):
# In prod the provider redirects the browser directly here, so an
# HTTPException would render as raw JSON to the user. Convert known
# error codes into a 302 back to /profile with the code in the query
# string — the SPA renders a proper alert from there.
try:
return await _associate_callback_impl(
provider_name=provider_name,
request=request,
user=user,
session=session,
)
except HTTPException as exc:
code = _oauth_error_code(exc.detail)
if code is None:
raise
return RedirectResponse(
url=_frontend_associate_error_url(provider_name, code),
status_code=302,
)


async def _associate_callback_impl(
*,
provider_name: str,
request: Request,
user: User,
session: AsyncSession,
):
provider = _resolve_provider(provider_name)
state = request.query_params.get("state")
Expand Down
57 changes: 40 additions & 17 deletions tests/test_provider_associate.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,17 @@
Steam-first user can link Google. The OAuthAccount table doesn't
care which provider came first.
2. Conflict detection. Linking a provider identity that's already
attached to another user returns 409.
attached to another user redirects with ``oauth_account_already_linked``.
3. Idempotency. Linking the same provider identity to the same user
twice doesn't error or duplicate.
4. State validation. Wrong purpose, wrong user, or a missing CSRF
cookie all yield 400.
cookie all redirect to /profile with the error code in the URL.
5. ``GET /auth/me/connections`` returns the linked-provider list.
"""

from __future__ import annotations

from urllib.parse import parse_qs, urlparse
from uuid import uuid4

import pytest
Expand All @@ -24,6 +25,7 @@
from sqlalchemy.ext.asyncio import AsyncSession

from app.auth import current_active_user
from app.config import settings
from app.main import app
from app.models.oauth_account import OAuthAccount
from app.models.user import User
Expand All @@ -37,6 +39,21 @@
_generate_state,
)


def _assert_associate_error(resp, *, expected_code: str, expected_provider: str) -> None:
"""Errors in the associate callback now 302 to /profile with the
failure code in the query string so the SPA can render an alert."""
assert resp.status_code == 302, resp.text
parsed = urlparse(resp.headers["location"])
assert parsed.path == "/profile"
expected_base = urlparse(settings.frontend_url)
if expected_base.netloc:
assert parsed.netloc == expected_base.netloc
params = parse_qs(parsed.query)
assert params.get("associate_error") == [expected_code]
assert params.get("associate_provider") == [expected_provider]


# --- helpers --------------------------------------------------------------


Expand Down Expand Up @@ -262,9 +279,11 @@ async def test_link_steam_already_on_other_user_returns_409(
finally:
_clear_user_override()

assert resp.status_code == 409, resp.text
body = resp.json()
assert body["detail"]["code"] == "oauth_account_already_linked"
_assert_associate_error(
resp,
expected_code="oauth_account_already_linked",
expected_provider="steam",
)

# No new row was created; the Steam ID remains tied to the original user.
rows = (
Expand Down Expand Up @@ -333,7 +352,7 @@ async def test_relinking_same_provider_identity_is_noop(


class TestStateValidation:
async def test_missing_state_returns_400(
async def test_missing_state_redirects_to_profile_with_error(
self,
client: AsyncClient,
linked_user: User,
Expand All @@ -345,10 +364,11 @@ async def test_missing_state_returns_400(
resp = await client.get("/auth/steam/associate/callback", follow_redirects=False)
finally:
_clear_user_override()
assert resp.status_code == 400, resp.text
assert resp.json()["detail"]["code"] == "oauth_state_missing"
_assert_associate_error(
resp, expected_code="oauth_state_missing", expected_provider="steam"
)

async def test_purpose_mismatch_returns_400(
async def test_purpose_mismatch_redirects_to_profile_with_error(
self,
client: AsyncClient,
linked_user: User,
Expand All @@ -367,10 +387,11 @@ async def test_purpose_mismatch_returns_400(
)
finally:
_clear_user_override()
assert resp.status_code == 400, resp.text
assert resp.json()["detail"]["code"] == "oauth_state_wrong_purpose"
_assert_associate_error(
resp, expected_code="oauth_state_wrong_purpose", expected_provider="steam"
)

async def test_user_mismatch_returns_400(
async def test_user_mismatch_redirects_to_profile_with_error(
self,
client: AsyncClient,
linked_user: User,
Expand All @@ -392,10 +413,11 @@ async def test_user_mismatch_returns_400(
)
finally:
_clear_user_override()
assert resp.status_code == 400, resp.text
assert resp.json()["detail"]["code"] == "oauth_state_user_mismatch"
_assert_associate_error(
resp, expected_code="oauth_state_user_mismatch", expected_provider="steam"
)

async def test_missing_csrf_cookie_returns_400(
async def test_missing_csrf_cookie_redirects_to_profile_with_error(
self,
client: AsyncClient,
linked_user: User,
Expand All @@ -413,8 +435,9 @@ async def test_missing_csrf_cookie_returns_400(
)
finally:
_clear_user_override()
assert resp.status_code == 400, resp.text
assert resp.json()["detail"]["code"] == "oauth_csrf_mismatch"
_assert_associate_error(
resp, expected_code="oauth_csrf_mismatch", expected_provider="steam"
)


# --- /auth/me/connections -------------------------------------------------
Expand Down