diff --git a/app.py b/app.py index fac6317..c109543 100644 --- a/app.py +++ b/app.py @@ -22,6 +22,7 @@ import climate import content import digest import discord_interactions as discord_interactions_mod +import discord_link import db import grid import metrics @@ -714,6 +715,8 @@ app.include_router( ) # Subscriptions + notifications (authed, user-scoped). app.include_router(api_accounts.router, prefix=f"{BASE}/api/v2") +# Discord account linking (OAuth2, authed). +app.include_router(discord_link.router, prefix=f"{BASE}/api/v2") # --- static frontend (served under BASE) ----------------------------------- diff --git a/discord_link.py b/discord_link.py new file mode 100644 index 0000000..2177727 --- /dev/null +++ b/discord_link.py @@ -0,0 +1,168 @@ +"""Link a Thermograph account to a Discord account via OAuth2 (identify scope). + +The Discord user id we store here is the durable key a later feature (DM alerts) +uses to reach the person. The flow is the standard authorization-code grant: + + /link/start -> redirect the logged-in user to Discord's consent screen + /link/callback -> exchange the code, read the Discord user id, store it + /unlink -> forget it + +`state` is signed with the app's auth secret (stdlib hmac, no new dependency) and +carries the Thermograph user id, so the callback can't be replayed or bound to a +different account. Everything requires an active session, so linking always acts on +the person who is actually logged in. +""" +from __future__ import annotations + +import base64 +import hashlib +import hmac +import json +import os +import time +import urllib.parse + +import httpx +from fastapi import APIRouter, Depends, Request +from fastapi.responses import RedirectResponse +from sqlalchemy import update +from sqlalchemy.ext.asyncio import AsyncSession + +from db import get_async_session +from models import User +from users import SECRET, current_active_user + +router = APIRouter(tags=["discord"]) + +CLIENT_ID = os.environ.get("THERMOGRAPH_DISCORD_APP_ID", "").strip() +CLIENT_SECRET = os.environ.get("THERMOGRAPH_DISCORD_CLIENT_SECRET", "").strip() +BASE = os.environ.get("THERMOGRAPH_BASE", "/thermograph").rstrip("/") + +_AUTHORIZE = "https://discord.com/api/oauth2/authorize" +_TOKEN = "https://discord.com/api/oauth2/token" +_ME = "https://discord.com/api/users/@me" + +# How long a start->callback round trip may take before the signed state expires. +_STATE_TTL = 600 +_TIMEOUT = httpx.Timeout(10.0) + + +def enabled() -> bool: + return bool(CLIENT_ID and CLIENT_SECRET) + + +# --- signed state ------------------------------------------------------------ +def _b64(raw: bytes) -> str: + return base64.urlsafe_b64encode(raw).decode().rstrip("=") + + +def _unb64(s: str) -> bytes: + return base64.urlsafe_b64decode(s + "=" * (-len(s) % 4)) + + +def _sign_state(uid: str) -> str: + payload = _b64(json.dumps({"u": uid, "t": int(time.time())}).encode()) + mac = hmac.new(SECRET.encode(), payload.encode(), hashlib.sha256).hexdigest()[:32] + return f"{payload}.{mac}" + + +def _verify_state(state: str) -> str | None: + """The Thermograph user id the state was minted for, or None if it's missing, + tampered, or older than the TTL.""" + try: + payload, mac = state.split(".", 1) + except (ValueError, AttributeError): + return None + expect = hmac.new(SECRET.encode(), payload.encode(), hashlib.sha256).hexdigest()[:32] + if not hmac.compare_digest(mac, expect): + return None + try: + data = json.loads(_unb64(payload)) + except (ValueError, json.JSONDecodeError): + return None + if time.time() - float(data.get("t", 0)) > _STATE_TTL: + return None + return data.get("u") + + +def _redirect_uri(request: Request) -> str: + """The callback URL, matched to how the app is actually reached (scheme/host + + base path). Must equal the redirect registered in the Discord portal.""" + proto = request.headers.get("x-forwarded-proto") or request.url.scheme + host = request.headers.get("host") or request.url.netloc + return f"{proto}://{host}{BASE}/api/v2/discord/link/callback" + + +def _account_redirect(status: str) -> RedirectResponse: + # Back to the alerts page, which surfaces the link state; 303 so the browser + # issues a GET after the callback. + return RedirectResponse(url=f"{BASE}/subscriptions?discord={status}", status_code=303) + + +# --- routes ------------------------------------------------------------------ +@router.get("/discord/link/start") +async def link_start(request: Request, user=Depends(current_active_user)): + if not enabled(): + return _account_redirect("unavailable") + params = { + "client_id": CLIENT_ID, + "redirect_uri": _redirect_uri(request), + "response_type": "code", + "scope": "identify", + "state": _sign_state(str(user.id)), + "prompt": "consent", + } + return RedirectResponse(url=f"{_AUTHORIZE}?{urllib.parse.urlencode(params)}", status_code=303) + + +@router.get("/discord/link/callback") +async def link_callback( + request: Request, + user=Depends(current_active_user), + session: AsyncSession = Depends(get_async_session), +): + if not enabled(): + return _account_redirect("unavailable") + # User declined on Discord's screen, or the state doesn't check out. + code = request.query_params.get("code") + state = request.query_params.get("state", "") + if not code: + return _account_redirect("cancelled") + if _verify_state(state) != str(user.id): + return _account_redirect("error") + try: + async with httpx.AsyncClient(timeout=_TIMEOUT) as client: + tok = await client.post(_TOKEN, data={ + "client_id": CLIENT_ID, + "client_secret": CLIENT_SECRET, + "grant_type": "authorization_code", + "code": code, + "redirect_uri": _redirect_uri(request), + }, headers={"Content-Type": "application/x-www-form-urlencoded"}) + if tok.status_code >= 300: + return _account_redirect("error") + access = tok.json().get("access_token") + me = await client.get(_ME, headers={"Authorization": f"Bearer {access}"}) + if me.status_code >= 300: + return _account_redirect("error") + discord_id = str(me.json().get("id") or "") + except Exception: # noqa: BLE001 - any network/parse failure is a graceful error + return _account_redirect("error") + if not discord_id: + return _account_redirect("error") + await session.execute( + update(User).where(User.id == user.id).values(discord_id=discord_id) + ) + await session.commit() + return _account_redirect("linked") + + +@router.post("/discord/unlink", status_code=204) +async def unlink( + user=Depends(current_active_user), + session: AsyncSession = Depends(get_async_session), +): + await session.execute( + update(User).where(User.id == user.id).values(discord_id=None) + ) + await session.commit() diff --git a/models.py b/models.py index 6168356..2fb4cde 100644 --- a/models.py +++ b/models.py @@ -30,8 +30,12 @@ from db import Base class User(SQLAlchemyBaseUserTableUUID, Base): # Inherits id (UUID), email (unique), hashed_password, is_active, - # is_superuser, is_verified. One optional extra: + # is_superuser, is_verified. Optional extras: display_name: Mapped[str | None] = mapped_column(String(120), nullable=True) + # Linked Discord account id (OAuth2 identify) — the key DM alerts reach the user + # by. NB: create_all only makes this on a fresh DB; an existing prod accounts.db + # needs the manual migration in deploy/migrations/ (no Alembic in this project). + discord_id: Mapped[str | None] = mapped_column(String(32), unique=True, nullable=True) class AccessToken(SQLAlchemyBaseAccessTokenTableUUID, Base): diff --git a/schemas.py b/schemas.py index 1ba3d75..dae5ac4 100644 --- a/schemas.py +++ b/schemas.py @@ -20,6 +20,9 @@ DEFAULT_METRICS = ["tmax", "feels", "precip"] # --- users (fastapi-users) --------------------------------------------------- class UserRead(schemas.BaseUser[uuid.UUID]): display_name: str | None = None + # Present so the frontend can show linked/unlinked state; set via the OAuth + # flow (discord_link.py), not editable through the profile. + discord_id: str | None = None class UserCreate(schemas.BaseUserCreate): diff --git a/tests/test_discord_link.py b/tests/test_discord_link.py new file mode 100644 index 0000000..66efb8f --- /dev/null +++ b/tests/test_discord_link.py @@ -0,0 +1,125 @@ +"""Discord account linking: signed-state integrity, OAuth callback (Discord HTTP +mocked), unlink, and auth gating. Reuses the throwaway accounts DB from conftest.""" +import pytest +from fastapi.testclient import TestClient + +import app as appmod +import db +import discord_link as dl + +V2 = "/thermograph/api/v2" +PW = "supersecret123" + + +@pytest.fixture(scope="module", autouse=True) +def _tables(): + db.Base.metadata.create_all(db.sync_engine) + + +def _login(client, email): + r = client.post(f"{V2}/auth/register", json={"email": email, "password": PW}) + assert r.status_code in (201, 400) + assert client.post(f"{V2}/auth/login", data={"username": email, "password": PW}).status_code == 204 + + +# --- signed state ------------------------------------------------------------ + +def test_state_round_trips_and_rejects_tampering(): + s = dl._sign_state("user-123") + assert dl._verify_state(s) == "user-123" + payload, mac = s.split(".", 1) + assert dl._verify_state(f"{payload}.{'0' * len(mac)}") is None # bad mac + assert dl._verify_state(payload) is None # no mac + assert dl._verify_state("garbage") is None + + +def test_state_expires(monkeypatch): + monkeypatch.setattr(dl, "_STATE_TTL", -1) # already expired + assert dl._verify_state(dl._sign_state("u")) is None + + +def test_state_is_secret_dependent(monkeypatch): + s = dl._sign_state("u") + monkeypatch.setattr(dl, "SECRET", dl.SECRET + "-different") + assert dl._verify_state(s) is None # signed under the old secret + + +# --- routes ------------------------------------------------------------------ + +def test_link_requires_auth(): + c = TestClient(appmod.app) + assert c.get(f"{V2}/discord/link/start", follow_redirects=False).status_code == 401 + assert c.post(f"{V2}/discord/unlink").status_code == 401 + + +def test_start_redirects_to_discord(monkeypatch): + monkeypatch.setattr(dl, "CLIENT_ID", "app123") + monkeypatch.setattr(dl, "CLIENT_SECRET", "secret456") + c = TestClient(appmod.app) + _login(c, "start@example.com") + r = c.get(f"{V2}/discord/link/start", follow_redirects=False) + assert r.status_code == 303 + loc = r.headers["location"] + assert loc.startswith("https://discord.com/api/oauth2/authorize") + assert "client_id=app123" in loc and "scope=identify" in loc and "state=" in loc + + +def test_start_when_unconfigured_bounces_back(monkeypatch): + monkeypatch.setattr(dl, "CLIENT_ID", "") + monkeypatch.setattr(dl, "CLIENT_SECRET", "") + c = TestClient(appmod.app) + _login(c, "noconf@example.com") + r = c.get(f"{V2}/discord/link/start", follow_redirects=False) + assert r.status_code == 303 and "discord=unavailable" in r.headers["location"] + + +class _Resp: + def __init__(self, code, data): self.status_code = code; self._data = data + def json(self): return self._data + + +class _MockClient: + """Stands in for httpx.AsyncClient: token exchange then /users/@me.""" + def __init__(self, *a, **k): pass + async def __aenter__(self): return self + async def __aexit__(self, *a): return False + async def post(self, url, **k): return _Resp(200, {"access_token": "tok"}) + async def get(self, url, **k): return _Resp(200, {"id": "discord-999", "username": "someone"}) + + +def test_callback_links_the_account(monkeypatch): + monkeypatch.setattr(dl, "CLIENT_ID", "app123") + monkeypatch.setattr(dl, "CLIENT_SECRET", "secret456") + monkeypatch.setattr(dl.httpx, "AsyncClient", _MockClient) + c = TestClient(appmod.app) + _login(c, "callback@example.com") + uid = c.get(f"{V2}/users/me").json()["id"] + state = dl._sign_state(uid) + r = c.get(f"{V2}/discord/link/callback?code=abc&state={state}", follow_redirects=False) + assert r.status_code == 303 and "discord=linked" in r.headers["location"] + # /users/me now reports the linked id, and unlink clears it. + assert c.get(f"{V2}/users/me").json()["discord_id"] == "discord-999" + assert c.post(f"{V2}/discord/unlink").status_code == 204 + assert c.get(f"{V2}/users/me").json()["discord_id"] is None + + +def test_callback_rejects_a_forged_state(monkeypatch): + monkeypatch.setattr(dl, "CLIENT_ID", "app123") + monkeypatch.setattr(dl, "CLIENT_SECRET", "secret456") + monkeypatch.setattr(dl.httpx, "AsyncClient", _MockClient) + c = TestClient(appmod.app) + _login(c, "forged@example.com") + # State signed for a different user id must not link this session. + r = c.get(f"{V2}/discord/link/callback?code=abc&state={dl._sign_state('someone-else')}", + follow_redirects=False) + assert r.status_code == 303 and "discord=error" in r.headers["location"] + assert c.get(f"{V2}/users/me").json()["discord_id"] is None + + +def test_callback_without_code_is_cancelled(monkeypatch): + monkeypatch.setattr(dl, "CLIENT_ID", "app123") + monkeypatch.setattr(dl, "CLIENT_SECRET", "secret456") + c = TestClient(appmod.app) + _login(c, "cancel@example.com") + r = c.get(f"{V2}/discord/link/callback?error=access_denied", follow_redirects=False) + assert r.status_code == 303 and "discord=cancelled" in r.headers["location"]