diff --git a/.github/workflows/preview.yml b/.github/workflows/preview.yml index 6458aa9f..22ea8e88 100644 --- a/.github/workflows/preview.yml +++ b/.github/workflows/preview.yml @@ -57,7 +57,8 @@ jobs: push: ${{ env.IS_MAIN != 'true' }} provenance: false # The browser calls the preview's own /api, which the router sends to its backend. - build-args: ${{ matrix.service == 'frontend' && format('NEXT_PUBLIC_API_URL={0}/api', steps.slug.outputs.url) || '' }} + # The backend signs players in through dev, as Discord only returns them to registered sites. + build-args: ${{ matrix.service == 'frontend' && format('NEXT_PUBLIC_API_URL={0}/api', steps.slug.outputs.url) || 'SIGN_IN_SITE=https://dev.tfminecraft.net' }} tags: ${{ env.IS_MAIN != 'true' && format('{0}-{1}:{2}', env.IMAGE_PREFIX, matrix.service, steps.slug.outputs.slug) || '' }} labels: org.opencontainers.image.source=${{ github.server_url }}/${{ github.repository }} # Branches cannot read each other's Actions cache, so main keeps a registry cache they all share. @@ -100,7 +101,7 @@ jobs: echo "### Preview: $URL" echo echo "Built from \`${GITHUB_SHA::7}\`. It stops 90 minutes after this deploy; push again or re-run this workflow to restart it." - echo "Discord sign-in is off on previews." + echo "Discord sign-in goes through dev and keeps your dev role." } >> "$GITHUB_STEP_SUMMARY" teardown: diff --git a/backend/Dockerfile b/backend/Dockerfile index 451c374d..550bc329 100644 --- a/backend/Dockerfile +++ b/backend/Dockerfile @@ -44,6 +44,10 @@ RUN npm run build WORKDIR /app +# Previews sign in through dev (src/auth/preview_sign_in.py); empty everywhere else. +ARG SIGN_IN_SITE="" +ENV SIGN_IN_SITE=${SIGN_IN_SITE} + EXPOSE 8000 CMD ["python3", "-m", "uvicorn", "server:app", "--host", "0.0.0.0", "--port", "8000"] diff --git a/backend/server.py b/backend/server.py index 57220a0b..e62d0314 100644 --- a/backend/server.py +++ b/backend/server.py @@ -26,9 +26,12 @@ class _OAuthQueryFilter(logging.Filter): - """Drop OAuth codes and states from access log lines for callback routes.""" + """Drop OAuth codes, states and preview tickets from access log lines for sign-in routes.""" - PATHS = ("/auth/discord/callback", "/auth/microsoft/callback", "/patreon/oauth/callback") + PATHS = ( + "/auth/discord/callback", "/auth/microsoft/callback", "/patreon/oauth/callback", + "/auth/preview/start", "/auth/preview/callback", + ) def filter(self, record: logging.LogRecord) -> bool: args = record.args diff --git a/backend/src/api/auth_routes.py b/backend/src/api/auth_routes.py index 3092c034..aeb6ed0e 100644 --- a/backend/src/api/auth_routes.py +++ b/backend/src/api/auth_routes.py @@ -10,6 +10,7 @@ import logging import os import re +import secrets import time from collections import defaultdict from urllib.parse import urlencode, urlsplit @@ -19,7 +20,7 @@ from pydantic import BaseModel, Field from src.api.prod_guard import is_production -from src.auth import account_overview, guild_check, users +from src.auth import account_overview, guild_check, preview_sign_in, users from src.auth.config import AuthConfig from src.auth import microsoft_link from src.auth.discord import DiscordClient, DiscordError @@ -71,7 +72,7 @@ def build_microsoft_client(config: MicrosoftConfig) -> MicrosoftClient: def _config() -> AuthConfig: config = AuthConfig.from_env() - if not config.enabled or config.problems(): + if not (config.enabled or config.via_site) or config.problems(): raise HTTPException(503, detail="discord_auth_disabled") return config @@ -161,8 +162,10 @@ def _patreon(user: dict) -> dict | None: # -------------------- @auth_router.get("/auth/discord/start") -def discord_start(return_to: str | None = None): +def discord_start(request: Request, return_to: str | None = None): config = _config() + if config.via_site: + return _preview_start(request, config, return_to) state, url = users.start_sign_in(config, return_to) response = _no_store(RedirectResponse(url, status_code=302)) _set_cookie(response, config, state_cookie(config), state, int(users.STATE_TTL.total_seconds())) @@ -172,6 +175,8 @@ def discord_start(return_to: str | None = None): @auth_router.get("/auth/discord/callback") def discord_callback(request: Request, code: str | None = None, state: str | None = None, error: str | None = None): config = _config() + if config.via_site: + raise HTTPException(404, detail="Not Found") cookie_state = request.cookies.get(state_cookie(config)) or "" status, return_to, token = "error", users.RETURN_DEFAULT, None if state and cookie_state and hmac.compare_digest(cookie_state, state): @@ -223,6 +228,115 @@ def _finish_sign_in(config: AuthConfig, code: str | None) -> str | None: return users.sign_in(identity, guild_member=guild_member, member=member) +# -------------------- +# Branch previews sign in through dev (src/auth/preview_sign_in.py) +# -------------------- + +PREVIEW_START_PATH = "/api/auth/preview/start" +PREVIEW_CALLBACK_PATH = "/api/auth/preview/callback" + + +def _preview_start(request: Request, config: AuthConfig, return_to: str | None) -> Response: + """On a preview: send the browser to the sign-in site, which returns it with a ticket. + + Signing in again while signed in is "I've joined, check again", so dev must not hand + back the check it already has. + """ + state = secrets.token_urlsafe(32) + params = {"site": config.site_origin, "state": state, "return_to": users.clean_return_to(return_to)} + if users.session_user(request.cookies.get(session_cookie(config))) is not None: + params["recheck"] = "1" + query = urlencode(params) + response = _no_store(RedirectResponse(f"{config.sign_in_site}{PREVIEW_START_PATH}?{query}", status_code=302)) + _set_cookie(response, config, state_cookie(config), state, int(users.STATE_TTL.total_seconds())) + return response + + +@auth_router.get("/auth/preview/start") +def preview_start( + request: Request, + site: str | None = None, + state: str | None = None, + return_to: str | None = None, + recheck: str | None = None, +): + """On dev: hand the signed-in player to a preview, signing them in here first if needed.""" + config = _config() + target = preview_sign_in.preview_site(site) + if config.via_site or target is None or not preview_sign_in.valid_token(state): + raise HTTPException(404, detail="Not Found") + back = users.clean_return_to(return_to) + # Discord's callback returns the browser here, now signed in and freshly checked. + again = PREVIEW_START_PATH + "?" + urlencode({"site": target, "state": state, "return_to": back}) + if users.clean_return_to(again) != again: + again = PREVIEW_START_PATH + "?" + urlencode({"site": target, "state": state}) + user = users.session_user(request.cookies.get(session_cookie(config))) + if user is None: + return discord_start(request, again) + # Linking on the preview needs a recent check, and only dev can make one: with the + # bot, or else by signing in again. `again` drops recheck, so this ends. + user, answered = guild_check.check(user) + if not answered and (recheck == "1" or not users.guild_check_recent(user)): + return discord_start(request, again) + ticket = preview_sign_in.issue(target, user) + logger.info("Preview sign-in ticket user_id=%s site=%s", user["user_id"], target) + query = urlencode({"ticket": ticket, "state": state, "return_to": back}) + response = _no_store(RedirectResponse(f"{target}{PREVIEW_CALLBACK_PATH}?{query}", status_code=302)) + response.headers["Referrer-Policy"] = "no-referrer" + return response + + +class RedeemBody(BaseModel): + ticket: str = Field(..., max_length=256) + site: str = Field(..., max_length=256) + + +@auth_router.post("/auth/preview/redeem") +def preview_redeem(body: RedeemBody, response: Response): + """On dev: the preview's backend trades a ticket for the player it carries.""" + config = _config() + site = preview_sign_in.preview_site(body.site) + if config.via_site or site is None: + raise HTTPException(404, detail="Not Found") + player = preview_sign_in.redeem(body.ticket, site) + if player is None: + raise HTTPException(410, detail="ticket_invalid") + _no_store(response) + return player + + +@auth_router.get("/auth/preview/callback") +def preview_callback( + request: Request, ticket: str | None = None, state: str | None = None, return_to: str | None = None +): + """On a preview: open a session for the player dev's ticket carries.""" + config = _config() + if not config.via_site: + raise HTTPException(404, detail="Not Found") + cookie_state = request.cookies.get(state_cookie(config)) or "" + status, token = "expired", None + if ( + preview_sign_in.valid_token(state) + and preview_sign_in.valid_token(cookie_state) + and hmac.compare_digest(cookie_state, state) + and preview_sign_in.valid_token(ticket) + ): + player = preview_sign_in.fetch_player(config, ticket) + token = preview_sign_in.sign_in(player) if player else None + status = "ok" if token else "error" + logger.info("Preview sign-in callback status=%s", status) + response = ( + _site_redirect(config, users.clean_return_to(return_to)) if token + else _site_redirect(config, users.RETURN_DEFAULT, signin=status) + ) + # The ticket is in this URL; keep it out of the next page's Referer. + response.headers["Referrer-Policy"] = "no-referrer" + _clear_cookie(response, config, state_cookie(config)) + if token: + _set_cookie(response, config, session_cookie(config), token, int(users.SESSION_TTL.total_seconds())) + return response + + @auth_router.post("/auth/logout") def logout(request: Request): config = _config() diff --git a/backend/src/auth/config.py b/backend/src/auth/config.py index 2397ce8d..0746816c 100644 --- a/backend/src/auth/config.py +++ b/backend/src/auth/config.py @@ -30,6 +30,8 @@ class AuthConfig: guild_id: str site_url: str api_base: str + # A branch preview signs in through this site instead of with Discord; see preview_sign_in.py. + sign_in_site: str = "" @classmethod def from_env(cls) -> AuthConfig: @@ -44,8 +46,14 @@ def from_env(cls) -> AuthConfig: guild_id=os.getenv("DISCORD_GUILD_ID", "").strip(), site_url=site, api_base=(os.getenv("DISCORD_API_BASE", API_BASE_DEFAULT).strip() or API_BASE_DEFAULT).rstrip("/"), + sign_in_site=os.getenv("SIGN_IN_SITE", "").strip().rstrip("/"), ) + @property + def via_site(self) -> bool: + """True on a branch preview, which has no Discord settings of its own.""" + return bool(self.sign_in_site) + @property def site_origin(self) -> str: # problems() refuses an invalid site, so routes never see this fallback. @@ -59,15 +67,19 @@ def secure_cookies(self) -> bool: def problems(self) -> list[str]: """Settings that stop sign-in from working; empty when usable.""" errors = [] + site = _origin(self.site_url) + if site is None or urlsplit(self.site_url).path not in {"", "/"}: + errors.append("SITE_PUBLIC_URL") + if self.via_site: + if _origin(self.sign_in_site) != self.sign_in_site or self.sign_in_site == site: + errors.append("SIGN_IN_SITE") + return errors if not self.client_id or any(ch.isspace() for ch in self.client_id): errors.append("DISCORD_CLIENT_ID") if not self.client_secret: errors.append("DISCORD_CLIENT_SECRET") if not self.guild_id.isdigit() or len(self.guild_id) > SNOWFLAKE_MAX_LEN: errors.append("DISCORD_GUILD_ID") - site = _origin(self.site_url) - if site is None or urlsplit(self.site_url).path not in {"", "/"}: - errors.append("SITE_PUBLIC_URL") if not _callback_matches_site(self.redirect_uri, site) or not urlsplit(self.redirect_uri).path.endswith( CALLBACK_ROUTE ): diff --git a/backend/src/auth/preview_sign_in.py b/backend/src/auth/preview_sign_in.py new file mode 100644 index 00000000..88eb1285 --- /dev/null +++ b/backend/src/auth/preview_sign_in.py @@ -0,0 +1,183 @@ +"""Discord sign-in for branch previews, through the dev site. + +Discord only returns players to addresses registered with it, so a preview at +https://.tfminecraft.net cannot run Discord sign-in itself. Dev does it +for them: the preview sends the browser to dev, dev signs the player in (or +uses the session it already has) and sends them back with a single-use ticket. +The preview redeems the ticket with dev for the player's identity, role and +membership check, and opens its own session. Previews never hold the Discord +secret. + +Dev sets PREVIEW_SIGN_IN_DOMAIN and hands tickets only to preview hosts under +it. A preview sets SIGN_IN_SITE to dev's address. +""" +from __future__ import annotations + +import os +import re +import secrets +from datetime import timedelta +from urllib.parse import urlsplit + +import httpx + +from src.skins.codes import hash_secret +from src.skins.db import connect + +from . import users +from .config import SNOWFLAKE_MAX_LEN, AuthConfig +from .roles import ROLES + +TICKET_TTL = timedelta(seconds=120) +TIMEOUT_SECONDS = 10.0 +REDEEM_PATH = "/api/auth/preview/redeem" +_TOKEN = re.compile(r"[A-Za-z0-9_-]{16,256}") +# Matches ps-preview and .github/scripts/preview_slug.py. +_SLUG = re.compile(r"[a-z0-9](?:[a-z0-9-]{0,38}[a-z0-9])?") +_RESERVED = {"www", "dev", "api", "mail", "map", "admin", "staff", "play", "mc", "status", "preview", "t3"} +_TEXT_MAX = 64 + + +def valid_token(value: str | None) -> bool: + return bool(value) and _TOKEN.fullmatch(value) is not None + + +def preview_site(value: str | None) -> str | None: + """The preview origin dev may send a ticket to, or None.""" + domain = os.getenv("PREVIEW_SIGN_IN_DOMAIN", "").strip().lower() + if not domain or not value: + return None + try: + parts = urlsplit(value) + host = parts.hostname or "" + except ValueError: + return None + slug, _, rest = host.partition(".") + if ( + parts.scheme != "https" + or parts.netloc != host + or parts.path + or parts.query + or parts.fragment + or rest != domain + or not _SLUG.fullmatch(slug) + or slug in _RESERVED + ): + return None + return f"https://{host}" + + +def issue(site: str, user: dict) -> str: + """A ticket for the signed-in dev user, redeemable once by that preview.""" + ticket = secrets.token_urlsafe(32) + now = users._utcnow() + with connect() as conn: + conn.execute("DELETE FROM preview_sign_in_tickets WHERE expires_at <= ?", (users._iso(now),)) + conn.execute( + """ + INSERT INTO preview_sign_in_tickets + (ticket_hash, site, user_id, guild_member, guild_checked_at, expires_at) + VALUES (?, ?, ?, ?, ?, ?) + """, + ( + hash_secret(ticket), site, user["user_id"], 1 if user["guild_member"] else 0, + user["guild_checked_at"], users._iso(now + TICKET_TTL), + ), + ) + conn.commit() + return ticket + + +def redeem(ticket: str | None, site: str | None) -> dict | None: + """Spend a ticket once; the player it carries, or None if unknown, expired or for another site.""" + if not valid_token(ticket) or not site: + return None + digest = hash_secret(ticket) + with connect() as conn: + conn.execute("BEGIN IMMEDIATE") + row = conn.execute( + """ + SELECT t.site, t.guild_member, t.guild_checked_at, t.expires_at, + u.discord_user_id, u.discord_username, u.discord_global_name, u.discord_avatar, u.role + FROM preview_sign_in_tickets t JOIN users u ON u.id = t.user_id + WHERE t.ticket_hash = ? + """, + (digest,), + ).fetchone() + conn.execute("DELETE FROM preview_sign_in_tickets WHERE ticket_hash = ?", (digest,)) + conn.commit() + if row is None or row["site"] != site or users._parse_iso(row["expires_at"]) <= users._utcnow(): + return None + return { + "discord_user_id": row["discord_user_id"], + "discord_username": row["discord_username"], + "discord_global_name": row["discord_global_name"], + "discord_avatar": row["discord_avatar"], + "role": row["role"], + "guild_member": bool(row["guild_member"]), + "guild_checked_at": row["guild_checked_at"], + } + + +def _text(value) -> str | None: + return value[:_TEXT_MAX] if isinstance(value, str) and value else None + + +def _checked_at(value) -> str | None: + if not isinstance(value, str): + return None + try: + users._parse_iso(value) + except ValueError: + return None + return value + + +def _player(body) -> dict | None: + """Dev's answer, checked field by field; None if anything is off.""" + if not isinstance(body, dict): + return None + discord_id = body.get("discord_user_id") + if not isinstance(discord_id, str) or not discord_id.isdigit() or len(discord_id) > SNOWFLAKE_MAX_LEN: + return None + checked_at = _checked_at(body.get("guild_checked_at")) + if body.get("role") not in ROLES or not isinstance(body.get("guild_member"), bool) or checked_at is None: + return None + return { + "discord_user_id": discord_id, + "discord_username": _text(body.get("discord_username")), + "discord_global_name": _text(body.get("discord_global_name")), + "discord_avatar": _text(body.get("discord_avatar")), + "role": body["role"], + "guild_member": body["guild_member"], + "guild_checked_at": checked_at, + } + + +def fetch_player(config: AuthConfig, ticket: str, http: httpx.Client | None = None) -> dict | None: + """Redeem a ticket with the sign-in site; the player, or None when refused or unreachable.""" + client = http or httpx.Client(timeout=TIMEOUT_SECONDS) + try: + response = client.post( + config.sign_in_site + REDEEM_PATH, + json={"ticket": ticket, "site": config.site_origin}, + headers={"Accept": "application/json"}, + ) + if response.status_code != 200: + return None + return _player(response.json()) + except (httpx.HTTPError, ValueError): + return None + finally: + if http is None: + client.close() + + +def sign_in(player: dict) -> str: + """Open a preview session for the player dev vouched for.""" + return users.sign_in( + player, + guild_member=player["guild_member"], + guild_checked_at=player["guild_checked_at"], + role=player["role"], + ) diff --git a/backend/src/auth/test_discord_client.py b/backend/src/auth/test_discord_client.py index 96ae4266..e3e84624 100644 --- a/backend/src/auth/test_discord_client.py +++ b/backend/src/auth/test_discord_client.py @@ -118,4 +118,6 @@ def record(path): assert "secret" not in record("/auth/discord/callback?code=secret&state=s") assert "secret" not in record("/patreon/oauth/callback?code=secret") + assert "secret" not in record("/auth/preview/start?site=x&state=secret") + assert "secret" not in record("/auth/preview/callback?ticket=secret&state=s") assert "?q=keep" in record("/maps?q=keep") diff --git a/backend/src/auth/test_preview_sign_in.py b/backend/src/auth/test_preview_sign_in.py new file mode 100644 index 00000000..9b328baa --- /dev/null +++ b/backend/src/auth/test_preview_sign_in.py @@ -0,0 +1,389 @@ +from datetime import timedelta +from urllib.parse import parse_qs, urlsplit + +import httpx +import pytest +from fastapi import FastAPI +from fastapi.testclient import TestClient + +from src.api import auth_routes as routes +from src.auth import guild_check, preview_sign_in, users +from src.auth.config import AuthConfig +from src.skins.db import connect + +DEV = "https://dev.tfminecraft.net" +PREVIEW = "https://my-branch.tfminecraft.net" +DISCORD_ID = "123456789012345678" +SESSION = "__Host-tfmc_session" +STATE = "__Host-tfmc_discord_state" +IDENTITY = {"discord_user_id": DISCORD_ID, "discord_username": "steve_tfmc", + "discord_global_name": "Steve", "discord_avatar": "a_1234abcd"} + + +def _app(base_url): + app = FastAPI() + app.include_router(routes.auth_router) + return TestClient(app, base_url=base_url) + + +@pytest.fixture +def dev(database, monkeypatch): + """Dev: Discord sign-in, and tickets for previews under tfminecraft.net.""" + monkeypatch.setenv("DISCORD_AUTH_ENABLED", "1") + monkeypatch.setenv("DISCORD_CLIENT_ID", "client-id") + monkeypatch.setenv("DISCORD_CLIENT_SECRET", "client-secret") + monkeypatch.setenv("DISCORD_GUILD_ID", "999999999999999999") + monkeypatch.setenv("SITE_PUBLIC_URL", DEV) + monkeypatch.setenv("PREVIEW_SIGN_IN_DOMAIN", "tfminecraft.net") + monkeypatch.setenv("PATREON_ENABLED", "0") + monkeypatch.delenv("DISCORD_REDIRECT_URI", raising=False) + monkeypatch.delenv("SIGN_IN_SITE", raising=False) + monkeypatch.delenv("DISCORD_BOT_TOKEN", raising=False) + guild_check.clear() + return _app(DEV) + + +@pytest.fixture +def preview(database, monkeypatch): + """A preview: no Discord settings, signs in through dev.""" + for name in ("DISCORD_AUTH_ENABLED", "DISCORD_CLIENT_ID", "DISCORD_CLIENT_SECRET", "DISCORD_GUILD_ID", + "DISCORD_REDIRECT_URI", "PREVIEW_SIGN_IN_DOMAIN", "DISCORD_BOT_TOKEN"): + monkeypatch.delenv(name, raising=False) + monkeypatch.setenv("SITE_PUBLIC_URL", PREVIEW) + monkeypatch.setenv("SIGN_IN_SITE", DEV) + monkeypatch.setenv("PATREON_ENABLED", "0") + guild_check.clear() + return _app(PREVIEW) + + +def _dev_session(dev, guild_member=True): + token = users.sign_in(IDENTITY, guild_member=guild_member) + dev.cookies.set(SESSION, token, domain="dev.tfminecraft.net") + return token + + +def _query(response): + return {k: v[0] for k, v in parse_qs(urlsplit(response.headers["location"]).query).items()} + + +def _player(**overrides): + return {**IDENTITY, "role": "admin", "guild_member": True, + "guild_checked_at": "2026-10-10T12:00:00Z", **overrides} + + +# -------------------- +# Which sites may receive a ticket +# -------------------- + +@pytest.mark.parametrize("site", [ + "https://my-branch.tfminecraft.net", + "https://a.tfminecraft.net", + "https://codex-new-map.tfminecraft.net", +]) +def test_preview_sites(site, monkeypatch): + monkeypatch.setenv("PREVIEW_SIGN_IN_DOMAIN", "tfminecraft.net") + assert preview_sign_in.preview_site(site) == site + + +@pytest.mark.parametrize("site", [ + None, "", + "https://www.tfminecraft.net", "https://dev.tfminecraft.net", "https://api.tfminecraft.net", + "https://tfminecraft.net", "https://a.b.tfminecraft.net", "https://evil.com", + "https://my-branch.tfminecraft.net.evil.com", "http://my-branch.tfminecraft.net", + "https://my-branch.tfminecraft.net:8443", "https://my-branch.tfminecraft.net/x", + "https://my-branch.tfminecraft.net?x=1", "https://user@my-branch.tfminecraft.net", + "https://-branch.tfminecraft.net", "https://MY-BRANCH.tfminecraft.net", + "https://" + "a" * 41 + ".tfminecraft.net", "https://my-branch.tfminecraft.net.", + "https://[", "https://a\uff03b.tfminecraft.net", "https://my-br\u0430nch.tfminecraft.net", +]) +def test_other_sites_are_refused(site, monkeypatch): + monkeypatch.setenv("PREVIEW_SIGN_IN_DOMAIN", "tfminecraft.net") + assert preview_sign_in.preview_site(site) is None + + +def test_no_preview_sites_without_the_setting(monkeypatch): + monkeypatch.delenv("PREVIEW_SIGN_IN_DOMAIN", raising=False) + assert preview_sign_in.preview_site(PREVIEW) is None + + +# -------------------- +# Settings +# -------------------- + +def test_preview_config_needs_no_discord_settings(preview): + config = AuthConfig.from_env() + assert config.via_site and config.problems() == [] + + +@pytest.mark.parametrize("value", ["dev.tfminecraft.net", "https://dev.tfminecraft.net/api", PREVIEW]) +def test_preview_config_refuses_a_bad_sign_in_site(preview, monkeypatch, value): + monkeypatch.setenv("SIGN_IN_SITE", value) + assert AuthConfig.from_env().problems() == ["SIGN_IN_SITE"] + + +# -------------------- +# Preview: start and callback +# -------------------- + +def test_preview_start_sends_the_browser_to_dev(preview): + response = preview.get("/auth/discord/start?return_to=/account?tab=x", follow_redirects=False) + assert response.status_code == 302 + assert response.headers["location"].startswith(DEV + "/api/auth/preview/start?") + query = _query(response) + assert query["site"] == PREVIEW and query["return_to"] == "/account?tab=x" + assert response.cookies[STATE] == query["state"] + + +def test_preview_start_asks_for_a_recheck_when_signed_in(preview): + assert "recheck" not in _query(preview.get("/auth/discord/start", follow_redirects=False)) + token = preview_sign_in.sign_in(_player()) + preview.cookies.set(SESSION, token, domain="my-branch.tfminecraft.net") + assert _query(preview.get("/auth/discord/start", follow_redirects=False))["recheck"] == "1" + + +def test_preview_start_drops_an_offsite_return(preview): + response = preview.get("/auth/discord/start?return_to=//evil.com", follow_redirects=False) + assert _query(response)["return_to"] == users.RETURN_DEFAULT + + +def test_preview_callback_signs_in_with_dev_role_and_check(preview, monkeypatch): + seen = [] + + def fetch(config, ticket): + seen.append((config.site_origin, ticket)) + return _player() + + monkeypatch.setattr(preview_sign_in, "fetch_player", fetch) + state = "s" * 43 + preview.cookies.set(STATE, state, domain="my-branch.tfminecraft.net") + response = preview.get( + f"/auth/preview/callback?ticket={'t' * 43}&state={state}&return_to=/admin", follow_redirects=False + ) + assert response.status_code == 302 + assert response.headers["location"] == PREVIEW + "/admin" + assert response.headers["referrer-policy"] == "no-referrer" + assert seen == [(PREVIEW, "t" * 43)] + user = users.session_user(response.cookies[SESSION]) + assert user["role"] == "admin" and user["guild_member"] == 1 + assert user["guild_checked_at"] == "2026-10-10T12:00:00Z" + + preview.cookies.set(SESSION, response.cookies[SESSION], domain="my-branch.tfminecraft.net") + account = preview.get("/account") + assert account.status_code == 200 + assert account.json()["user"]["role"] == "admin" + + +@pytest.mark.parametrize("cookie", [None, "x" * 43]) +def test_preview_callback_needs_the_browser_that_started(preview, monkeypatch, cookie): + monkeypatch.setattr(preview_sign_in, "fetch_player", lambda *a: pytest.fail("ticket redeemed")) + if cookie: + preview.cookies.set(STATE, cookie, domain="my-branch.tfminecraft.net") + response = preview.get(f"/auth/preview/callback?ticket={'t' * 43}&state={'s' * 43}", follow_redirects=False) + assert response.headers["location"] == PREVIEW + "/account?signin=expired" + assert SESSION not in response.cookies + + +def test_preview_callback_reports_a_refused_ticket(preview, monkeypatch): + monkeypatch.setattr(preview_sign_in, "fetch_player", lambda *a: None) + preview.cookies.set(STATE, "s" * 43, domain="my-branch.tfminecraft.net") + response = preview.get(f"/auth/preview/callback?ticket={'t' * 43}&state={'s' * 43}", follow_redirects=False) + assert response.headers["location"] == PREVIEW + "/account?signin=error" + assert SESSION not in response.cookies + + +def test_preview_has_no_discord_callback(preview): + assert preview.get("/auth/discord/callback?code=x&state=y").status_code == 404 + + +def test_dev_has_no_preview_callback(dev): + assert dev.get(f"/auth/preview/callback?ticket={'t' * 43}&state={'s' * 43}").status_code == 404 + + +# -------------------- +# Dev: start and redeem +# -------------------- + +def test_dev_start_signs_in_first_then_returns_here(dev): + state = "s" * 43 + response = dev.get( + "/auth/preview/start", params={"site": PREVIEW, "state": state, "return_to": "/admin"}, + follow_redirects=False, + ) + assert response.status_code == 302 + assert response.headers["location"].startswith("https://discord.com/oauth2/authorize?") + discord_state = _query(response)["state"] + saved = users.consume_state(discord_state) + assert saved == routes.PREVIEW_START_PATH + f"?site=https%3A%2F%2Fmy-branch.tfminecraft.net&state={state}&return_to=%2Fadmin" + + +def test_dev_start_drops_a_return_too_long_to_save(dev): + response = dev.get( + "/auth/preview/start", params={"site": PREVIEW, "state": "s" * 43, "return_to": "/" + "a" * 480}, + follow_redirects=False, + ) + saved = users.consume_state(_query(response)["state"]) + assert saved.startswith(routes.PREVIEW_START_PATH) and "return_to" not in saved + + +def test_dev_start_hands_a_signed_in_player_to_the_preview(dev): + _dev_session(dev) + state = "s" * 43 + response = dev.get( + "/auth/preview/start", params={"site": PREVIEW, "state": state, "return_to": "/admin"}, + follow_redirects=False, + ) + assert response.status_code == 302 + assert response.headers["location"].startswith(PREVIEW + "/api/auth/preview/callback?") + assert response.headers["referrer-policy"] == "no-referrer" + query = _query(response) + assert query["state"] == state and query["return_to"] == "/admin" + + redeemed = dev.post("/auth/preview/redeem", json={"ticket": query["ticket"], "site": PREVIEW}) + assert redeemed.status_code == 200 + body = redeemed.json() + assert body["discord_user_id"] == DISCORD_ID and body["role"] == "player" and body["guild_member"] is True + assert dev.post("/auth/preview/redeem", json={"ticket": query["ticket"], "site": PREVIEW}).status_code == 410 + + +@pytest.mark.parametrize("params", [ + {"site": "https://www.tfminecraft.net", "state": "s" * 43}, + {"site": "https://evil.com", "state": "s" * 43}, + {"site": PREVIEW, "state": "short"}, + {"site": PREVIEW}, +]) +def test_dev_start_refuses_other_sites_and_bad_states(dev, params): + _dev_session(dev) + assert dev.get("/auth/preview/start", params=params, follow_redirects=False).status_code == 404 + + +def test_dev_start_refuses_a_malformed_site_without_failing(dev): + response = dev.get("/auth/preview/start", params={"site": "https://[", "state": "s" * 43}, follow_redirects=False) + assert response.status_code == 404 + assert dev.post("/auth/preview/redeem", json={"ticket": "t" * 43, "site": "https://["}).status_code == 404 + + +def _age_check(minutes): + stamp = users._iso(users._utcnow() - timedelta(minutes=minutes)) + with connect() as conn: + conn.execute("UPDATE user_sessions SET guild_checked_at = ?", (stamp,)) + conn.commit() + return stamp + + +def _start(dev): + return dev.get("/auth/preview/start", params={"site": PREVIEW, "state": "s" * 43}, follow_redirects=False) + + +def test_dev_start_signs_in_again_when_a_stale_check_cannot_be_refreshed(dev): + _dev_session(dev) + _age_check(60) + response = _start(dev) + assert response.headers["location"].startswith("https://discord.com/oauth2/authorize?") + assert users.consume_state(_query(response)["state"]).startswith(routes.PREVIEW_START_PATH) + + +def test_dev_start_refreshes_a_stale_check_with_the_bot(dev, monkeypatch): + monkeypatch.setattr(guild_check, "_ask", lambda discord_id, http: (True, 0.0)) + _dev_session(dev) + old = _age_check(60) + response = _start(dev) + assert response.headers["location"].startswith(PREVIEW + "/api/auth/preview/callback?") + player = preview_sign_in.redeem(_query(response)["ticket"], PREVIEW) + assert player["guild_member"] is True and player["guild_checked_at"] > old + + +def test_dev_start_rechecks_a_recent_non_member_once(dev): + _dev_session(dev, guild_member=False) + params = {"site": PREVIEW, "state": "s" * 43, "recheck": "1"} + response = dev.get("/auth/preview/start", params=params, follow_redirects=False) + assert response.headers["location"].startswith("https://discord.com/oauth2/authorize?") + saved = users.consume_state(_query(response)["state"]) + assert saved.startswith(routes.PREVIEW_START_PATH) and "recheck" not in saved + + +def test_dev_start_trusts_the_bot_on_a_recheck(dev, monkeypatch): + monkeypatch.setattr(guild_check, "_ask", lambda discord_id, http: (True, 0.0)) + _dev_session(dev, guild_member=False) + params = {"site": PREVIEW, "state": "s" * 43, "recheck": "1"} + response = dev.get("/auth/preview/start", params=params, follow_redirects=False) + assert preview_sign_in.redeem(_query(response)["ticket"], PREVIEW)["guild_member"] is True + + +def test_dev_start_hands_over_a_recent_non_member_without_looping(dev): + _dev_session(dev, guild_member=False) + response = _start(dev) + assert response.headers["location"].startswith(PREVIEW + "/api/auth/preview/callback?") + assert preview_sign_in.redeem(_query(response)["ticket"], PREVIEW)["guild_member"] is False + + +def test_dev_start_is_off_without_the_setting(dev, monkeypatch): + monkeypatch.delenv("PREVIEW_SIGN_IN_DOMAIN") + _dev_session(dev) + response = dev.get("/auth/preview/start", params={"site": PREVIEW, "state": "s" * 43}, follow_redirects=False) + assert response.status_code == 404 + + +def test_tickets_are_for_one_site_and_expire(dev, monkeypatch): + _dev_session(dev) + user = users.session_user(dev.cookies.get(SESSION)) + other = "https://other.tfminecraft.net" + assert preview_sign_in.redeem(preview_sign_in.issue(PREVIEW, user), other) is None + + ticket = preview_sign_in.issue(PREVIEW, user) + later = users._utcnow() + preview_sign_in.TICKET_TTL + timedelta(seconds=1) + monkeypatch.setattr(users, "_utcnow", lambda: later) + assert preview_sign_in.redeem(ticket, PREVIEW) is None + + +def test_tickets_are_stored_hashed(dev): + _dev_session(dev) + ticket = preview_sign_in.issue(PREVIEW, users.session_user(dev.cookies.get(SESSION))) + with connect() as conn: + stored = [row[0] for row in conn.execute("SELECT ticket_hash FROM preview_sign_in_tickets")] + assert stored and ticket not in stored + + +def test_redeem_refuses_other_sites(dev): + assert dev.post("/auth/preview/redeem", json={"ticket": "t" * 43, "site": DEV}).status_code == 404 + + +# -------------------- +# Preview asks dev +# -------------------- + +def _client(handler): + return httpx.Client(transport=httpx.MockTransport(handler)) + + +def test_fetch_player_posts_the_ticket_and_site(preview): + seen = [] + + def handler(request): + seen.append((str(request.url), request.read())) + return httpx.Response(200, json=_player()) + + player = preview_sign_in.fetch_player(AuthConfig.from_env(), "t" * 43, _client(handler)) + assert player == _player() + assert seen[0][0] == DEV + "/api/auth/preview/redeem" + assert b'"site":"https://my-branch.tfminecraft.net"' in seen[0][1].replace(b" ", b"") + + +@pytest.mark.parametrize("answer", [ + httpx.Response(410, json={"detail": "ticket_invalid"}), + httpx.Response(200, text="not json"), + httpx.Response(200, json=[]), + httpx.Response(200, json=_player(discord_user_id="12ab")), + httpx.Response(200, json=_player(role="owner")), + httpx.Response(200, json=_player(guild_member="yes")), + httpx.Response(200, json=_player(guild_checked_at=None)), + httpx.Response(200, json=_player(guild_checked_at="yesterday")), +]) +def test_fetch_player_refuses_odd_answers(preview, answer): + assert preview_sign_in.fetch_player(AuthConfig.from_env(), "t" * 43, _client(lambda r: answer)) is None + + +def test_fetch_player_survives_dev_being_down(preview): + def handler(request): + raise httpx.ConnectError("down") + + assert preview_sign_in.fetch_player(AuthConfig.from_env(), "t" * 43, _client(handler)) is None diff --git a/backend/src/auth/users.py b/backend/src/auth/users.py index 79261d19..ebd6f903 100644 --- a/backend/src/auth/users.py +++ b/backend/src/auth/users.py @@ -87,8 +87,18 @@ def consume_state(state: str | None) -> str | None: return row["return_to"] -def sign_in(identity: dict, *, guild_member: bool, member: dict | None = None) -> str: - """Upsert the user, open a session and return its plaintext token.""" +def sign_in( + identity: dict, + *, + guild_member: bool, + member: dict | None = None, + guild_checked_at: str | None = None, + role: str | None = None, +) -> str: + """Upsert the user, open a session and return its plaintext token. + + A preview passes the membership check and role dev already holds for the user. + """ token = secrets.token_urlsafe(32) now = _utcnow() stamp = _iso(now) @@ -115,13 +125,18 @@ def sign_in(identity: dict, *, guild_member: bool, member: dict | None = None) - user_id = conn.execute( "SELECT id FROM users WHERE discord_user_id = ?", (identity["discord_user_id"],) ).fetchone()["id"] + if role is not None: + conn.execute("UPDATE users SET role = ? WHERE id = ?", (role, user_id)) conn.execute("DELETE FROM user_sessions WHERE expires_at <= ?", (stamp,)) conn.execute( """ INSERT INTO user_sessions (token_hash, user_id, guild_member, guild_checked_at, created_at, expires_at) VALUES (?, ?, ?, ?, ?, ?) """, - (hash_secret(token), user_id, 1 if guild_member else 0, stamp, stamp, _iso(now + SESSION_TTL)), + ( + hash_secret(token), user_id, 1 if guild_member else 0, guild_checked_at or stamp, stamp, + _iso(now + SESSION_TTL), + ), ) conn.commit() if identity["discord_username"]: @@ -183,11 +198,14 @@ def record_guild_check(session_id: int, member: bool) -> str: return stamp -def guild_check_fresh(user: dict) -> bool: +def guild_check_recent(user: dict) -> bool: + """The session's membership check, member or not, is recent enough to act on.""" checked = user.get("guild_checked_at") - if not user.get("guild_member") or not checked: - return False - return _utcnow() - _parse_iso(checked) <= GUILD_CHECK_MAX_AGE + return bool(checked) and _utcnow() - _parse_iso(checked) <= GUILD_CHECK_MAX_AGE + + +def guild_check_fresh(user: dict) -> bool: + return bool(user.get("guild_member")) and guild_check_recent(user) def revoke_session(token: str | None) -> None: diff --git a/backend/src/skins/schema.sql b/backend/src/skins/schema.sql index 9cf58e5d..0ddc8be2 100644 --- a/backend/src/skins/schema.sql +++ b/backend/src/skins/schema.sql @@ -680,6 +680,17 @@ CREATE TABLE IF NOT EXISTS discord_oauth_states ( CREATE INDEX IF NOT EXISTS idx_discord_link_codes_player ON discord_link_codes(player_uuid); +-- Single-use tickets that carry a dev sign-in to a branch preview, which +-- redeems one with dev within two minutes (src/auth/preview_sign_in.py). +CREATE TABLE IF NOT EXISTS preview_sign_in_tickets ( + ticket_hash TEXT PRIMARY KEY, + site TEXT NOT NULL, + user_id INTEGER NOT NULL REFERENCES users(id) ON DELETE CASCADE, + guild_member INTEGER NOT NULL, + guild_checked_at TEXT, + expires_at TEXT NOT NULL +); + -- Single-use Microsoft link attempts, tied to the site session that started -- them. The callback marks a row used and deletes it once done; unlinking -- deletes the user's rows, which cancels any attempt still in flight.