diff --git a/backend/src/api/admin_routes.py b/backend/src/api/admin_routes.py index 144dd742..904670cf 100644 --- a/backend/src/api/admin_routes.py +++ b/backend/src/api/admin_routes.py @@ -31,7 +31,7 @@ from src.rail import network as rail from src.servers import inventory as servers from src.servers import control as server_control -from src.skins.db import connect +from src import database from src.characters.creation_catalog import get_catalog from src.auth.users import session_user_in from src.scripts.util.dirs import validate_map @@ -261,11 +261,12 @@ def _server_write(request: Request, server: str, operation: str, body: ServerJob actor = None config = _config() token = request.cookies.get(session_cookie(config)) + users.require_accounts() try: - # Recheck role and session under the same write lock as the audit entry. - with connect() as conn: - conn.execute("BEGIN IMMEDIATE") - actor = session_user_in(conn, token) + # Recheck role and session in the audit entry's transaction, holding both rows until it + # commits: a demotion or revocation waits for the entry, or lands first and is seen here. + with database.connect() as conn: + actor = session_user_in(conn, token, lock=True) if actor is None: raise HTTPException(401, detail="not_signed_in") require_same_origin(request, config) @@ -283,7 +284,6 @@ def _server_write(request: Request, server: str, operation: str, body: ServerJob # Record the intent before crossing the process boundary. The host # persists the accepted job; the same id can safely be retried. audit.record(conn, actor=actor, action=action, outcome="requested", reason=reason, detail=detail) - conn.commit() result = server_control.request(operation, server=server, **{"plan_id" if operation == "start" else "job_id": body.id}, # backup_ref is unused, but helpers before the AMP backup require the key. diff --git a/backend/src/api/auth_routes.py b/backend/src/api/auth_routes.py index 1b782463..5d1fa7e3 100644 --- a/backend/src/api/auth_routes.py +++ b/backend/src/api/auth_routes.py @@ -173,6 +173,9 @@ def _patreon(user: dict) -> dict | None: @auth_router.get("/auth/discord/start") def discord_start(request: Request, return_to: str | None = None): config = _config() + # Without an accounts database (DATABASE_URL unset) sign-in answers 503 here, before + # anyone is sent to Discord or to the sign-in site. + users.require_accounts() if config.via_site: return _preview_start(request, config, return_to) state, url = users.start_sign_in(config, return_to) @@ -336,6 +339,8 @@ def preview_callback( config = _config() if not config.via_site: raise HTTPException(404, detail="Not Found") + # Before the ticket is spent with dev: without an accounts database there is nowhere to sign in. + users.require_accounts() cookie_state = request.cookies.get(state_cookie(config)) or "" status, token = "expired", None if ( diff --git a/backend/src/api/database_routes.py b/backend/src/api/database_routes.py index 3ba1d7c7..6f2ccda7 100644 --- a/backend/src/api/database_routes.py +++ b/backend/src/api/database_routes.py @@ -7,8 +7,8 @@ from src.api.auth_routes import _no_store from src.api.staff_access import require_staff_account from src.auth import audit +from src import database from src.dbstudio import studio -from src.skins.db import connect database_router = APIRouter(prefix="/admin/database", tags=["admin"]) @@ -55,11 +55,11 @@ def _audited(actor: dict, source_id: str, sql: str, write: bool, run, action: st def database_overview(request: Request, response: Response): actor = require_staff_account(request, "use_database") _no_store(response) - with connect() as conn: - counts = dict(conn.execute( - "SELECT json_extract(detail_json, '$.database'), count(*) FROM admin_audit " + with database.connect() as conn: + counts = {row["source"]: row["n"] for row in conn.execute( + "SELECT detail_json ->> 'database' AS source, count(*) AS n FROM admin_audit " "WHERE action IN ('database.read', 'database.write') AND outcome IN ('ok', 'failed') " - "GROUP BY 1").fetchall()) + "GROUP BY 1")} return _audited(actor, "*", "list databases", False, lambda: studio.overview(counts), "database.list") diff --git a/backend/src/auth/account_overview.py b/backend/src/auth/account_overview.py index daa196da..e083ec6d 100644 --- a/backend/src/auth/account_overview.py +++ b/backend/src/auth/account_overview.py @@ -69,6 +69,7 @@ def rank(player_uuid: str) -> str | None: if key is None: return None try: + # PG-MIGRATION: until the LuckPerms area converts, its mirror is in SQLite. with connect() as conn: if mirror.user_row(conn, key) is None: return None diff --git a/backend/src/auth/admin.py b/backend/src/auth/admin.py index 70d3b4e0..89004cac 100644 --- a/backend/src/auth/admin.py +++ b/backend/src/auth/admin.py @@ -1,23 +1,30 @@ """Staff operations on website accounts. -Every write reloads the actor's session and both roles inside one immediate -transaction, so a demotion or revocation that lands first always wins. The -change, the target's session revocation and the audit row commit together; -a refused attempt rolls back and is then logged on its own. +Every write locks the actor's and the target's account rows, then reloads the +actor's session and both roles inside that transaction, so a demotion or +revocation that lands first always wins. The change, the target's session +revocation and the audit row commit together; a refused attempt rolls back and +is then logged on its own. """ from __future__ import annotations -from src.skins.db import connect +import json + +from src import database +from src.skins import db as sqlite_db +from src.skins.codes import hash_secret from . import audit, roles -from .users import drop_view_leftovers, session_user_in +from .users import _TOKEN_MAX, PLACEHOLDERS_LOCK, api_time, drop_view_leftovers, require_accounts, session_user_in # A user's own sessions and the views they have open as players. -_REVOKE = "DELETE FROM user_sessions WHERE user_id = ? OR acting_user_id = ?" +_REVOKE = "DELETE FROM user_sessions WHERE user_id = %s OR acting_user_id = %s" REASON_MIN = 3 REASON_MAX = 500 _LOOKUP_MAX = 64 _LOOKUP_LIMIT = 5 +# Ids are bigint; a larger one from the URL names no account. +_ID_MAX = 2**63 - 1 _ACCOUNT_COLUMNS = ( "id AS user_id, discord_user_id, discord_username, discord_global_name, " @@ -42,39 +49,62 @@ def clean_reason(reason: str | None) -> str: return text +def _account_json(row) -> dict: + """An account row with its times in the API form.""" + account = dict(row) + for key in ("created_at", "last_login_at"): + account[key] = api_time(account[key]) + return account + + def _account(conn, user_id: int) -> dict | None: - row = conn.execute(f"SELECT {_ACCOUNT_COLUMNS} FROM users WHERE id = ? AND placeholder = 0", (user_id,)).fetchone() - return dict(row) if row else None + if not 0 < user_id <= _ID_MAX: + return None + row = conn.execute(f"SELECT {_ACCOUNT_COLUMNS} FROM users WHERE id = %s AND NOT placeholder", + (user_id,)).fetchone() + return _account_json(row) if row else None + + +def _lock_accounts(conn, token: str | None, target_id: int) -> None: + """Lock the actor's and the target's users rows, in id order, before anything reads them. + + Two staff acting on each other then run one after the other, and the second + sees the first's change. + """ + ids = {target_id} if 0 < target_id <= _ID_MAX else set() + if token and len(token) <= _TOKEN_MAX: + session = conn.execute("SELECT user_id FROM user_sessions WHERE token_hash = %s", + (hash_secret(token),)).fetchone() + if session is not None: + ids.add(session["user_id"]) + if ids: + conn.execute("SELECT id FROM users WHERE id = ANY(%s) ORDER BY id FOR UPDATE", (sorted(ids),)) def _staff_write(token: str, action: str, target_id: int, reason: str | None, apply) -> dict: """Run apply(conn, actor, target, reason) under the shared staff checks.""" + require_accounts() actor = target = None clean = None try: - with connect() as conn: - conn.execute("BEGIN IMMEDIATE") - try: - actor = session_user_in(conn, token) - if actor is None: - raise AdminError(401, "not_signed_in", "denied") - if not roles.is_staff(actor["role"]): - raise AdminError(403, "forbidden", "denied") - clean = clean_reason(reason) - target = _account(conn, target_id) - if target is None: - raise AdminError(404, "account_not_found", "not_found") - if actor["user_id"] == target["user_id"]: - raise AdminError(403, "cannot_act_on_self", "denied") - if not roles.outranks(actor["role"], target["role"]): - raise AdminError(403, "target_outranks_you", "denied") - detail = apply(conn, actor, target, clean) - audit.record(conn, actor=actor, action=action, outcome="ok", target=target, - reason=clean, detail=detail) - conn.commit() - except BaseException: - conn.rollback() - raise + with database.transaction(lock=PLACEHOLDERS_LOCK) as conn: + _lock_accounts(conn, token, target_id) + actor = session_user_in(conn, token, lock=True) + if actor is None: + raise AdminError(401, "not_signed_in", "denied") + if not roles.is_staff(actor["role"]): + raise AdminError(403, "forbidden", "denied") + clean = clean_reason(reason) + target = _account(conn, target_id) + if target is None: + raise AdminError(404, "account_not_found", "not_found") + if actor["user_id"] == target["user_id"]: + raise AdminError(403, "cannot_act_on_self", "denied") + if not roles.outranks(actor["role"], target["role"]): + raise AdminError(403, "target_outranks_you", "denied") + detail = apply(conn, actor, target, clean) + audit.record(conn, actor=actor, action=action, outcome="ok", target=target, + reason=clean, detail=detail) except AdminError as exc: if actor is not None: audit.record_refusal(actor=actor, action=action, outcome=exc.outcome, target=target, @@ -91,7 +121,7 @@ def apply(conn, actor, target, _reason): raise AdminError(403, "role_not_assignable", "denied") if new_role == target["role"]: raise AdminError(409, "role_unchanged", "conflict") - conn.execute("UPDATE users SET role = ? WHERE id = ?", (new_role, target["user_id"])) + conn.execute("UPDATE users SET role = %s WHERE id = %s", (new_role, target["user_id"])) # Live role checks already apply; signing out also stops open tabs # from carrying on with the old role's view. revoked = conn.execute(_REVOKE, (target["user_id"], target["user_id"])).rowcount @@ -112,22 +142,43 @@ def apply(conn, actor, target, _reason): return _staff_write(token, "account.sessions.revoke", target_id, reason, apply) -# Read-only view of the Discord link: listing never expires graces or queues notices. -_WITH_MINECRAFT = ( - "SELECT u.id AS user_id, u.discord_user_id, u.discord_username, u.discord_global_name, " - "u.discord_avatar, u.role, u.created_at, u.last_login_at, dl.minecraft_name " - "FROM users u LEFT JOIN discord_links dl ON dl.discord_user_id = u.discord_user_id" +_ACCOUNTS = ( + "SELECT id AS user_id, discord_user_id, discord_username, discord_global_name, " + "discord_avatar, role, created_at, last_login_at FROM users" ) +def _with_minecraft(rows) -> list[dict]: + """Accounts with the Minecraft name of their Discord link, if any. + + Read-only view of the link: listing never expires graces or queues notices. + """ + accounts = [_account_json(row) for row in rows] + # PG-MIGRATION: until the Linking area converts, discord_links is in SQLite. Its PR + # puts back the join: LEFT JOIN discord_links dl ON dl.discord_user_id = u.discord_user_id. + names = {} + ids = [a["discord_user_id"] for a in accounts] + if ids: + with sqlite_db.connect() as lite: + names = { + row["discord_user_id"]: row["minecraft_name"] + for row in lite.execute( + "SELECT discord_user_id, minecraft_name FROM discord_links " + "WHERE discord_user_id IN (SELECT value FROM json_each(?))", + (json.dumps(ids),), + ) + } + return [{**a, "minecraft_name": names.get(a["discord_user_id"])} for a in accounts] + + def list_staff() -> list[dict]: - with connect() as conn: + with database.connect() as conn: rows = conn.execute( - f"{_WITH_MINECRAFT} WHERE u.role IN ('mod', 'admin', 'root') " - "ORDER BY CASE u.role WHEN 'root' THEN 0 WHEN 'admin' THEN 1 ELSE 2 END, " - "lower(COALESCE(u.discord_username, u.discord_global_name, '')), u.id" + f"{_ACCOUNTS} WHERE role IN ('mod', 'admin', 'root') " + "ORDER BY CASE role WHEN 'root' THEN 0 WHEN 'admin' THEN 1 ELSE 2 END, " + "lower(COALESCE(discord_username, discord_global_name, '')), id" ).fetchall() - return [dict(row) for row in rows] + return _with_minecraft(rows) def lookup(query: str) -> list[dict]: @@ -135,14 +186,14 @@ def lookup(query: str) -> list[dict]: text = (query or "").strip().lstrip("@") if not text or len(text) > _LOOKUP_MAX: return [] - with connect() as conn: + with database.connect() as conn: rows = conn.execute( f""" - {_WITH_MINECRAFT} - WHERE u.placeholder = 0 AND (u.discord_user_id = ? OR lower(u.discord_username) = lower(?) - OR lower(u.discord_global_name) = lower(?)) - ORDER BY u.id LIMIT ? + {_ACCOUNTS} + WHERE NOT placeholder AND (discord_user_id = %s OR lower(discord_username) = lower(%s) + OR lower(discord_global_name) = lower(%s)) + ORDER BY id LIMIT %s """, (text, text, text, _LOOKUP_LIMIT), ).fetchall() - return [dict(row) for row in rows] + return _with_minecraft(rows) diff --git a/backend/src/auth/admin_cli.py b/backend/src/auth/admin_cli.py index 24efb988..6813f04b 100644 --- a/backend/src/auth/admin_cli.py +++ b/backend/src/auth/admin_cli.py @@ -15,56 +15,56 @@ import sys from pathlib import Path +from src import database from src.skins import db from . import audit from .admin import REASON_MAX, REASON_MIN, AdminError, clean_reason, list_staff -from .users import drop_view_leftovers +from .users import PLACEHOLDERS_LOCK, drop_view_leftovers def _set_root(discord_id: str, reason: str | None, *, grant: bool, demote_to: str = "player") -> str: clean = clean_reason(reason) - with db.connect() as conn: - conn.execute("BEGIN IMMEDIATE") - try: - row = conn.execute( - "SELECT id AS user_id, discord_user_id, discord_username, discord_global_name, role " - "FROM users WHERE discord_user_id = ? AND placeholder = 0", - (discord_id,), - ).fetchone() - if row is None: - raise SystemExit("No website account for that Discord id; sign in to the site once first.") - target = dict(row) - if grant: - if target["role"] == "root": - raise SystemExit("That account is already root.") - new_role = "root" - else: - if target["role"] != "root": - raise SystemExit("That account is not root.") - roots = conn.execute("SELECT COUNT(*) FROM users WHERE role = 'root'").fetchone()[0] - if roots <= 1: - raise SystemExit("Refusing to remove the last root account.") - new_role = demote_to - conn.execute("UPDATE users SET role = ? WHERE id = ?", (new_role, target["user_id"])) - revoked = conn.execute( - "DELETE FROM user_sessions WHERE user_id = ? OR acting_user_id = ?", - (target["user_id"], target["user_id"]), - ).rowcount - drop_view_leftovers(conn) - audit.record( - conn, actor=None, action="account.role.change", outcome="ok", target=target, reason=clean, - detail={"before": target["role"], "after": new_role, "sessions_revoked": revoked}, - ) - conn.commit() - except BaseException: - conn.rollback() - raise + with database.transaction(lock=PLACEHOLDERS_LOCK) as conn: + # Lock the target, and to take root away every root too, in id order, so two + # removals cannot each see the other root still there. + roots = "" if grant else " OR role = 'root'" + locked = conn.execute( + "SELECT id AS user_id, discord_user_id, discord_username, discord_global_name, role " + f"FROM users WHERE NOT placeholder AND (discord_user_id = %s{roots}) ORDER BY id FOR UPDATE", + (discord_id,), + ).fetchall() + target = next((dict(row) for row in locked if row["discord_user_id"] == discord_id), None) + if target is None: + raise SystemExit("No website account for that Discord id; sign in to the site once first.") + if grant: + if target["role"] == "root": + raise SystemExit("That account is already root.") + new_role = "root" + else: + if target["role"] != "root": + raise SystemExit("That account is not root.") + if sum(row["role"] == "root" for row in locked) <= 1: + raise SystemExit("Refusing to remove the last root account.") + new_role = demote_to + conn.execute("UPDATE users SET role = %s WHERE id = %s", (new_role, target["user_id"])) + revoked = conn.execute( + "DELETE FROM user_sessions WHERE user_id = %s OR acting_user_id = %s", + (target["user_id"], target["user_id"]), + ).rowcount + drop_view_leftovers(conn) + audit.record( + conn, actor=None, action="account.role.change", outcome="ok", target=target, reason=clean, + detail={"before": target["role"], "after": new_role, "sessions_revoked": revoked}, + ) return f"{audit.display_name(target)}: {target['role']} -> {new_role}" def main(argv: list[str] | None = None) -> int: parser = argparse.ArgumentParser(prog="python -m src.auth.admin_cli", description=__doc__.splitlines()[0]) + parser.add_argument("--database-url", help="the site's Postgres (default: DATABASE_URL)") + # PG-MIGRATION: until the Scripts area converts, Profile sessions and Discord links are + # still read from province.db (drop_view_leftovers, list-staff's Minecraft names). parser.add_argument("--db", type=Path, help="database file (default: the site's province.db)") sub = parser.add_subparsers(dest="command", required=True) for name in ("grant-root", "revoke-root"): @@ -84,6 +84,8 @@ def main(argv: list[str] | None = None) -> int: db.DRINKS_DIR = db.DATA_DIR / "drinks" print(f"Database: {db.DB_PATH}", file=sys.stderr) db.migrate() + if args.database_url: + database.open_pool(args.database_url, min_size=0, max_size=1) if args.command == "list-staff": for row in list_staff(): diff --git a/backend/src/auth/audit.py b/backend/src/auth/audit.py index 9fb82895..b81424e2 100644 --- a/backend/src/auth/audit.py +++ b/backend/src/auth/audit.py @@ -1,14 +1,10 @@ """Append-only staff audit log. Never store cookies, tokens or request bodies.""" from __future__ import annotations -import json +import sqlite3 from datetime import datetime, timezone -from src.skins.db import connect - - -def _now() -> str: - return datetime.now(timezone.utc).strftime("%Y-%m-%dT%H:%M:%SZ") +from src import database def display_name(user: dict | None) -> str | None: @@ -31,15 +27,22 @@ def record( actor is a signed-in user row (with role), or None for operator commands. """ + if isinstance(conn, sqlite3.Connection): + # PG-MIGRATION: until the LuckPerms area converts, luckperms/changes.py passes its + # SQLite connection. The audit log is in Postgres now, so this row commits on its + # own, just before the caller's. + with database.connect() as pg: + record(pg, actor=actor, action=action, outcome=outcome, target=target, reason=reason, detail=detail) + return conn.execute( """ INSERT INTO admin_audit ( created_at, actor_type, actor_user_id, actor_discord_id, actor_name, actor_role, action, outcome, target_user_id, target_discord_id, target_name, reason, detail_json - ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) + ) VALUES (%s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s) """, ( - _now(), + datetime.now(timezone.utc), "user" if actor else "system", actor.get("user_id") if actor else None, actor.get("discord_user_id") if actor else None, @@ -51,7 +54,7 @@ def record( target.get("discord_user_id") if target else None, display_name(target), reason, - json.dumps(detail or {}, sort_keys=True), + database.Jsonb(detail or {}), ), ) @@ -59,9 +62,8 @@ def record( def record_refusal(**kwargs) -> None: """Log a refused attempt in its own transaction. - Call only after the refused action's transaction has rolled back: - SQLite allows one writer, so this would otherwise wait on that lock. + Call only after the refused action's transaction has rolled back, so the + refusal is kept whatever that transaction did. """ - with connect() as conn: + with database.connect() as conn: record(conn, **kwargs) - conn.commit() diff --git a/backend/src/auth/discord_names.py b/backend/src/auth/discord_names.py index 6f3dffa6..40a1a15a 100644 --- a/backend/src/auth/discord_names.py +++ b/backend/src/auth/discord_names.py @@ -157,6 +157,7 @@ def store(discord_id: str, found: dict) -> None: def refresh_all(lookup: NameLookup) -> int: """Look up every linked player once; how many Discord answered for.""" + # PG-MIGRATION: until the Linking area converts, discord_links is in SQLite. with connect() as conn: ids = [row["discord_user_id"] for row in conn.execute("SELECT discord_user_id FROM discord_links")] answered = 0 diff --git a/backend/src/auth/microsoft_link.py b/backend/src/auth/microsoft_link.py index d197d540..50556466 100644 --- a/backend/src/auth/microsoft_link.py +++ b/backend/src/auth/microsoft_link.py @@ -4,6 +4,7 @@ import secrets from datetime import datetime, timedelta, timezone +from src import database from src.skins.codes import hash_secret from src.skins.db import connect @@ -92,5 +93,10 @@ def still_valid(conn, saved: dict) -> bool: "SELECT 1 FROM microsoft_link_states WHERE state_hash = ?", (saved["state_hash"],) ).fetchone() is None: return False - row = conn.execute("SELECT expires_at FROM user_sessions WHERE id = ?", (saved["session_id"],)).fetchone() - return row is not None and _parse_iso(row["expires_at"]) > _utcnow() + # PG-MIGRATION: until the Linking area converts, `conn` is SQLite and sessions are in + # Postgres, so the session is checked on its own connection. That reopens a narrow window + # between this check and the caller's commit (signing out in it no longer stops the link); + # the Linking PR reads the session on `conn` FOR SHARE, with the state row. + with database.connect() as pg: + row = pg.execute("SELECT expires_at FROM user_sessions WHERE id = %s", (saved["session_id"],)).fetchone() + return row is not None and row["expires_at"] > _utcnow() diff --git a/backend/src/auth/players.py b/backend/src/auth/players.py index 071c1f5e..3578f832 100644 --- a/backend/src/auth/players.py +++ b/backend/src/auth/players.py @@ -18,6 +18,7 @@ from src.coreprotect import activity, maps as co_maps, movement, sessions from src.coreprotect.cursors import BadCursor, decode as decode_cursor, encode as encode_cursor from src.coreprotect.reader import REQUEST_BUDGET_SECONDS, Budget, CoreProtectConfig, Reader, Unavailable +from src import database from src.skins.db import connect from src.characters.roster import list_roster from src.characters.wardrobe import active_face_png, active_figure_png, WardrobeError @@ -58,12 +59,17 @@ def _status(error: Unavailable | None) -> dict: # -------------------- _LINKS = """ - SELECT dl.player_uuid, dl.discord_user_id, dl.discord_username, dl.discord_nickname, dl.minecraft_name, - dl.linked_at, dl.left_guild_at, dl.grace_until, - u.id AS user_id, u.role, u.discord_username AS account_username, - u.discord_global_name, u.discord_avatar, u.created_at AS account_created_at, u.last_login_at - FROM discord_links dl LEFT JOIN users u ON u.discord_user_id = dl.discord_user_id AND u.placeholder = 0 + SELECT player_uuid, discord_user_id, discord_username, discord_nickname, minecraft_name, + linked_at, left_guild_at, grace_until + FROM discord_links """ +_ACCOUNTS = """ + SELECT discord_user_id, id AS user_id, role, discord_username AS account_username, + discord_global_name, discord_avatar, created_at AS account_created_at, last_login_at + FROM users WHERE NOT placeholder AND discord_user_id = ANY(%s) +""" +_NO_ACCOUNT = dict.fromkeys(("user_id", "role", "account_username", "discord_global_name", "discord_avatar", + "account_created_at", "last_login_at")) _ROSTER = """ SELECT player_uuid, realm_id, character_id, name, status, race, class, updated_at FROM character_roster @@ -72,15 +78,30 @@ def _status(error: Unavailable | None) -> dict: def _site_rows(key: str | None = None) -> tuple[list[dict], list[dict]]: """Discord links (with any website account) and characters, for one player or everyone.""" - with connect() as conn: + # PG-MIGRATION: until the Linking and Characters areas convert, discord_links and + # character_roster are in SQLite. Their PRs read them here with the accounts, as one + # query: discord_links LEFT JOIN users ON discord_user_id AND NOT placeholder. + with connect() as lite: if key is None: - links = conn.execute(_LINKS).fetchall() - roster = conn.execute(f"{_ROSTER} ORDER BY realm_id, lower(name)").fetchall() + links = lite.execute(_LINKS).fetchall() + roster = lite.execute(f"{_ROSTER} ORDER BY realm_id, lower(name)").fetchall() else: - links = conn.execute(f"{_LINKS} WHERE lower(dl.player_uuid) = ?", (key,)).fetchall() - roster = conn.execute(f"{_ROSTER} WHERE lower(player_uuid) = ? ORDER BY realm_id, lower(name)", + links = lite.execute(f"{_LINKS} WHERE lower(player_uuid) = ?", (key,)).fetchall() + roster = lite.execute(f"{_ROSTER} WHERE lower(player_uuid) = ? ORDER BY realm_id, lower(name)", (key,)).fetchall() - return [dict(r) for r in links], [dict(r) for r in roster] + links = [dict(r) for r in links] + accounts = {} + if links: + with database.connect() as conn: + for row in conn.execute(_ACCOUNTS, ([link["discord_user_id"] for link in links],)): + accounts[row["discord_user_id"]] = { + **row, "account_created_at": users.api_time(row["account_created_at"]), + "last_login_at": users.api_time(row["last_login_at"]), + } + for link in links: + link.update({k: v for k, v in accounts.get(link["discord_user_id"], _NO_ACCOUNT).items() + if k != "discord_user_id"}) + return links, [dict(r) for r in roster] def _discord(link: dict | None) -> dict | None: @@ -406,6 +427,7 @@ def profile(config: CoreProtectConfig, text: str) -> dict: def character_picture(text: str, realm: str, character_id: str, kind: str) -> bytes: key = _require(text) + # PG-MIGRATION: until the Characters area converts, character_roster is in SQLite. with connect() as conn: row = conn.execute( "SELECT player_uuid FROM character_roster " @@ -474,9 +496,8 @@ def _audit_messages(viewer: dict, config: CoreProtectConfig, key: str, kinds: tu "oldest": {"id": shown[-1]["id"], "time": shown[-1]["time"]}, } try: - with connect() as conn: + with database.connect() as conn: audit.record(conn, actor=viewer, action="player.messages.view", outcome="ok", detail=detail) - conn.commit() except Exception: raise PlayerError(503, "audit_unavailable") from None @@ -544,9 +565,8 @@ def _audit_movement(viewer: dict, config: CoreProtectConfig, subject: str, since if session is not None: detail["session"] = session try: - with connect() as conn: + with database.connect() as conn: audit.record(conn, actor=viewer, action="player.movement.view", outcome="ok", detail=detail) - conn.commit() except Exception: raise PlayerError(503, "audit_unavailable") from None diff --git a/backend/src/auth/preview_sign_in.py b/backend/src/auth/preview_sign_in.py index 88eb1285..00cd2f6a 100644 --- a/backend/src/auth/preview_sign_in.py +++ b/backend/src/auth/preview_sign_in.py @@ -21,8 +21,8 @@ import httpx +from src import database from src.skins.codes import hash_secret -from src.skins.db import connect from . import users from .config import SNOWFLAKE_MAX_LEN, AuthConfig @@ -71,42 +71,42 @@ 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),)) + with database.connect() as conn: + conn.execute("DELETE FROM preview_sign_in_tickets WHERE expires_at <= %s", (now,)) conn.execute( """ INSERT INTO preview_sign_in_tickets (ticket_hash, site, user_id, guild_member, guild_checked_at, expires_at) - VALUES (?, ?, ?, ?, ?, ?) + VALUES (%s, %s, %s, %s, %s, %s) """, ( - hash_secret(ticket), site, user["user_id"], 1 if user["guild_member"] else 0, - user["guild_checked_at"], users._iso(now + TICKET_TTL), + hash_secret(ticket), site, user["user_id"], bool(user["guild_member"]), + users._as_time(user["guild_checked_at"]), 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.""" + """Spend a ticket once; the player it carries, or None if unknown, expired or for another site. + + The answer is the preview hand-off: `guild_checked_at` stays `YYYY-MM-DDTHH:MM:SSZ`, + the only form a preview accepts (`_checked_at`). + """ if not valid_token(ticket) or not site: return None - digest = hash_secret(ticket) - with connect() as conn: - conn.execute("BEGIN IMMEDIATE") + # One DELETE both reads and spends the ticket, so two redeems cannot both get it. + with database.connect() as conn: 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 = ? + DELETE FROM preview_sign_in_tickets t USING users u + WHERE t.ticket_hash = %s AND u.id = t.user_id + RETURNING 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 """, - (digest,), + (hash_secret(ticket),), ).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(): + if row is None or row["site"] != site or row["expires_at"] <= users._utcnow(): return None return { "discord_user_id": row["discord_user_id"], @@ -115,7 +115,7 @@ def redeem(ticket: str | None, site: str | None) -> dict | None: "discord_avatar": row["discord_avatar"], "role": row["role"], "guild_member": bool(row["guild_member"]), - "guild_checked_at": row["guild_checked_at"], + "guild_checked_at": users.api_time(row["guild_checked_at"]), } diff --git a/backend/src/auth/test_admin.py b/backend/src/auth/test_admin.py index 94301de8..22031120 100644 --- a/backend/src/auth/test_admin.py +++ b/backend/src/auth/test_admin.py @@ -1,12 +1,14 @@ import json import sqlite3 +import psycopg import pytest from fastapi import FastAPI from fastapi.testclient import TestClient +from src import database from src.api import admin_routes, auth_routes -from src.auth import admin, admin_cli, audit, roles, users +from src.auth import admin, admin_cli, audit, roles, testing, users SITE = "https://www.tfminecraft.net" ORIGIN = {"Origin": SITE} @@ -14,7 +16,7 @@ @pytest.fixture -def env(database, monkeypatch): +def env(database, postgres, monkeypatch): monkeypatch.setenv("DISCORD_AUTH_ENABLED", "1") monkeypatch.setenv("DISCORD_CLIENT_ID", "client-id") monkeypatch.setenv("DISCORD_CLIENT_SECRET", "client-secret") @@ -41,11 +43,11 @@ def account(db, name, role="player"): "discord_avatar": None}, guild_member=True, ) - with db.connect() as conn: - conn.execute("UPDATE users SET role = ? WHERE discord_user_id = ?", (role, discord_id)) - conn.commit() - user_id = conn.execute("SELECT id FROM users WHERE discord_user_id = ?", (discord_id,)).fetchone()[0] - return user_id, token + return testing.set_role(discord_id, role), token + + +def discord_id_of(name): + return str(100000000000000000 + abs(hash(name)) % 10**15) def client(app, token=None): @@ -56,18 +58,16 @@ def client(app, token=None): def role_of(db, user_id): - with db.connect() as conn: - return conn.execute("SELECT role FROM users WHERE id = ?", (user_id,)).fetchone()[0] + return testing.value("SELECT role FROM users WHERE id = %s", (user_id,)) -def audit_rows(db): - with db.connect() as conn: - return [dict(r) for r in conn.execute("SELECT * FROM admin_audit ORDER BY id")] +def audit_rows(db=None): + """The audit log, oldest first; detail_json is the parsed object.""" + return testing.audit_rows() def sessions_of(db, user_id): - with db.connect() as conn: - return conn.execute("SELECT COUNT(*) FROM user_sessions WHERE user_id = ?", (user_id,)).fetchone()[0] + return testing.value("SELECT COUNT(*) FROM user_sessions WHERE user_id = %s", (user_id,)) # -------------------- @@ -126,8 +126,8 @@ def test_migration_adds_role_to_existing_users(tmp_path, monkeypatch): def test_fresh_database_rejects_unknown_roles(env): account(env, "alice") - with env.connect() as conn, pytest.raises(sqlite3.IntegrityError): - conn.execute("UPDATE users SET role = 'owner'") + with pytest.raises(database.CheckViolation): + testing.execute("UPDATE users SET role = 'owner'") # -------------------- @@ -135,19 +135,16 @@ def test_fresh_database_rejects_unknown_roles(env): # -------------------- def test_audit_rows_cannot_change(env): - with env.connect() as conn: - audit.record(conn, actor=None, action="test", outcome="ok") - conn.commit() - with pytest.raises(sqlite3.DatabaseError, match="append-only"): - conn.execute("UPDATE admin_audit SET outcome = 'x'") - with pytest.raises(sqlite3.DatabaseError, match="append-only"): - conn.execute("DELETE FROM admin_audit") - row_id = conn.execute("SELECT id FROM admin_audit").fetchone()[0] - with pytest.raises(sqlite3.DatabaseError, match="append-only"): - conn.execute( - "INSERT OR REPLACE INTO admin_audit (id, created_at, actor_type, action, outcome) " - "VALUES (?, 'x', 'system', 'forged', 'ok')", (row_id,)) - assert [r["action"] for r in audit_rows(env)] == ["test"] + with database.connect() as conn: + audit.record(conn, actor=None, action="test", outcome="ok", detail={"b": 1, "a": [2]}) + row_id = audit_rows(env)[0]["id"] + for statement in ("UPDATE admin_audit SET outcome = 'x'", "DELETE FROM admin_audit", + "INSERT INTO admin_audit (id, created_at, actor_type, action, outcome) " + f"VALUES ({row_id}, now(), 'system', 'forged', 'ok') ON CONFLICT (id) DO UPDATE SET outcome = 'x'"): + with pytest.raises(psycopg.errors.RaiseException, match="append-only"): + testing.execute(statement) + [row] = audit_rows(env) + assert (row["action"], row["actor_name"], row["detail_json"]) == ("test", "operator", {"a": [2], "b": 1}) # -------------------- @@ -178,10 +175,14 @@ def test_staff_list_and_lookup(app, env): pat_id, _ = account(env, "pat") with env.connect() as conn: conn.execute("INSERT INTO discord_links (player_uuid, discord_user_id, minecraft_name, linked_at) " - "SELECT 'u-pat', discord_user_id, 'PatMC', 'x' FROM users WHERE id = ?", (pat_id,)) + "VALUES ('u-pat', ?, 'PatMC', 'x')", (discord_id_of("pat"),)) conn.commit() c = client(app, root) - assert [s["discord_username"] for s in c.get("/admin/staff").json()["staff"]] == ["rory", "adam", "mona"] + staff = c.get("/admin/staff").json()["staff"] + assert [s["discord_username"] for s in staff] == ["rory", "adam", "mona"] + # Times keep the API form the panel has always had. + assert all(len(s["created_at"]) == 20 and s["created_at"].endswith("Z") for s in staff) + assert staff[0]["last_login_at"] == users._iso(users._parse_iso(staff[0]["last_login_at"])) found = c.get("/admin/accounts/lookup", params={"q": "@PAT"}).json()["accounts"] assert [(a["user_id"], a["minecraft_name"], a["role"]) for a in found] == [(pat_id, "PatMC", "player")] assert c.get("/admin/accounts/lookup", params={"q": "pa"}).json()["accounts"] == [] @@ -208,7 +209,7 @@ def test_root_can_make_admins_and_target_is_signed_out(app, env): assert (row["action"], row["outcome"], row["actor_user_id"], row["actor_role"], row["target_user_id"]) == ( "account.role.change", "ok", root_id, "root", pat_id) assert row["reason"] == "Promoted for events" and row["target_name"] == "pat" - assert json.loads(row["detail_json"]) == {"after": "admin", "before": "player", "sessions_revoked": 1} + assert row["detail_json"] == {"after": "admin", "before": "player", "sessions_revoked": 1} @pytest.mark.parametrize("actor_role,target_role,new_role,status,code", [ @@ -235,7 +236,7 @@ def test_role_change_rules(app, env, actor_role, target_role, new_role, status, if code: assert response.json()["detail"] == code assert role_of(env, target_id) == target_role - assert (row["outcome"], json.loads(row["detail_json"])["error"]) in { + assert (row["outcome"], row["detail_json"]["error"]) in { ("denied", code), ("conflict", code)} assert row["actor_role"] == actor_role else: @@ -263,6 +264,8 @@ def test_unknown_target(app, env): response = change(app, root, 9999, "mod") assert response.status_code == 404 assert audit_rows(env)[-1]["outcome"] == "not_found" + # Past bigint: no account, not a database error. + assert change(app, root, 2**70, "mod").status_code == 404 @pytest.mark.parametrize("headers", [{}, {"Origin": "https://evil.example"}, {"Origin": "https://dev.tfminecraft.net"}]) @@ -286,9 +289,7 @@ def test_demotion_that_lands_first_wins(app, env): """The actor's role is read inside the write, not from an earlier check.""" adam_id, adam = account(env, "adam", "admin") pat_id, _ = account(env, "pat") - with env.connect() as conn: - conn.execute("UPDATE users SET role = 'mod' WHERE id = ?", (adam_id,)) - conn.commit() + testing.execute("UPDATE users SET role = 'mod' WHERE id = %s", (adam_id,)) response = change(app, adam, pat_id, "mod") assert response.status_code == 403 and role_of(env, pat_id) == "player" @@ -296,9 +297,7 @@ def test_demotion_that_lands_first_wins(app, env): def test_revoked_actor_session_cannot_write(app, env): adam_id, adam = account(env, "adam", "admin") pat_id, _ = account(env, "pat") - with env.connect() as conn: - conn.execute("DELETE FROM user_sessions WHERE user_id = ?", (adam_id,)) - conn.commit() + testing.execute("DELETE FROM user_sessions WHERE user_id = %s", (adam_id,)) assert change(app, adam, pat_id, "mod").status_code == 401 assert role_of(env, pat_id) == "player" @@ -308,10 +307,10 @@ def test_failed_audit_rolls_back_the_change(app, env, monkeypatch): pat_id, _ = account(env, "pat") def broken(conn, **kwargs): - raise sqlite3.OperationalError("disk I/O error") + raise database.OperationalError("disk I/O error") monkeypatch.setattr(admin.audit, "record", broken) - with pytest.raises(sqlite3.OperationalError): + with pytest.raises(database.OperationalError): admin.change_role(client(app, root).cookies.get(SESSION), pat_id, "mod", "Promoted for events") assert role_of(env, pat_id) == "player" and sessions_of(env, pat_id) == 1 @@ -323,7 +322,7 @@ def broken(conn, **kwargs): def test_mod_can_sign_out_a_player_everywhere(app, env): _, mod = account(env, "mona", "mod") pat_id, _ = account(env, "pat") - users.sign_in({"discord_user_id": str(100000000000000000 + abs(hash("pat")) % 10**15), "discord_username": "pat", + users.sign_in({"discord_user_id": discord_id_of("pat"), "discord_username": "pat", "discord_global_name": None, "discord_avatar": None}, guild_member=True) response = client(app, mod).post(f"/admin/accounts/{pat_id}/sessions/revoke", json={"reason": "Reported shared login"}, headers=ORIGIN) @@ -347,8 +346,8 @@ def test_mod_cannot_sign_out_another_mod(app, env): def test_cli_grants_and_removes_root(env, capsys): rory_id, _ = account(env, "rory") sam_id, _ = account(env, "sam") - rory_discord = str(100000000000000000 + abs(hash("rory")) % 10**15) - sam_discord = str(100000000000000000 + abs(hash("sam")) % 10**15) + rory_discord = discord_id_of("rory") + sam_discord = discord_id_of("sam") assert admin_cli.main(["grant-root", "--discord-id", rory_discord, "--reason", "First owner"]) == 0 assert role_of(env, rory_id) == "root" and sessions_of(env, rory_id) == 0 @@ -384,8 +383,8 @@ def test_malformed_writes_are_audited_without_values(app, env, body, fields): assert response.status_code == 422 row = audit_rows(env)[-1] assert (row["action"], row["outcome"], row["actor_role"]) == ("account.role.change", "invalid", "root") - assert json.loads(row["detail_json"]) == {"error": "invalid_request", "fields": fields, "target_user_id": pat_id} - assert "x" * 50 not in row["detail_json"] and row["reason"] is None + assert row["detail_json"] == {"error": "invalid_request", "fields": fields, "target_user_id": pat_id} + assert "x" * 50 not in json.dumps(row["detail_json"]) and row["reason"] is None def test_malformed_writes_by_strangers_are_not_audited(app, env): @@ -398,4 +397,32 @@ def test_non_numeric_target_is_audited_without_an_id(app, env): _, root = account(env, "rory", "root") assert client(app, root).post("/admin/accounts/abc/sessions/revoke", json={"reason": "abc"}, headers=ORIGIN).status_code == 422 - assert json.loads(audit_rows(env)[-1]["detail_json"])["target_user_id"] is None + assert audit_rows(env)[-1]["detail_json"]["target_user_id"] is None + + +def test_staff_writes_without_an_accounts_database_are_503_and_change_nothing(app, env, monkeypatch): + from src.servers import control + + monkeypatch.setattr(control, "request", lambda *a, **k: pytest.fail("server helper called")) + _, root = account(env, "rory", "root") + pat_id, _ = account(env, "pat") + c = client(app, root) + staff_only = client(app) + staff_only.cookies.set("__Host-tfmc_staff_session", root, domain="testserver.local") + with testing.no_accounts_database(): + responses = { + "role": change(app, root, pat_id, "mod"), + "revoke": c.post(f"/admin/accounts/{pat_id}/sessions/revoke", json={"reason": "Shared login"}, + headers=ORIGIN), + "server": c.post("/admin/servers/dev/cancel", json={"id": "d" * 32, "reason": "scheduled restart"}, + headers=ORIGIN), + "view-as": c.post("/admin/players/0f6c5a52-3b7e-4c39-9d1d-2a8f0e7b9c11/view-as", headers=ORIGIN), + # The view cookie has lapsed; only the admin's own session is left to restore. + "account": staff_only.get("/account"), + "stop": staff_only.post("/auth/view-as/stop", headers=ORIGIN), + } + assert {name: (r.status_code, r.json()["detail"]) for name, r in responses.items()} == { + name: (503, "accounts_unavailable") for name in responses} + assert role_of(env, pat_id) == "player" and sessions_of(env, pat_id) == 1 + assert audit_rows(env) == [] + assert users.session_user(root)["role"] == "root" diff --git a/backend/src/auth/test_auth_routes.py b/backend/src/auth/test_auth_routes.py index e5215e23..260404af 100644 --- a/backend/src/auth/test_auth_routes.py +++ b/backend/src/auth/test_auth_routes.py @@ -1,4 +1,5 @@ import logging +import re import threading from datetime import timedelta from urllib.parse import parse_qs, urlsplit @@ -8,7 +9,7 @@ from fastapi.testclient import TestClient from src.api import auth_routes as routes -from src.auth import guild_check, users +from src.auth import guild_check, testing, users from src.auth.discord import DiscordError from src.skins import codes, discord_link @@ -22,6 +23,7 @@ OAUTH_CODE = "discord-oauth-code-should-not-leak" SESSION = "__Host-tfmc_session" STATE = "__Host-tfmc_discord_state" +API_TIME = r"\d{4}-\d\d-\d\dT\d\d:\d\d:\d\dZ" class Stub: @@ -55,7 +57,7 @@ def close(self): @pytest.fixture -def env(database, monkeypatch): +def env(database, postgres, monkeypatch): monkeypatch.setenv("DISCORD_AUTH_ENABLED", "1") monkeypatch.setenv("DISCORD_CLIENT_ID", "client-id") monkeypatch.setenv("DISCORD_CLIENT_SECRET", "client-secret") @@ -146,28 +148,65 @@ def test_callback_signs_in_and_returns_to_saved_path(api, monkeypatch, env, capl cookies = response.headers.get_list("set-cookie") assert any(c.startswith(SESSION + "=") and "Secure" in c and "HttpOnly" in c for c in cookies) assert any(c.startswith(STATE + '=""') or c.startswith(STATE + "=;") or (c.startswith(STATE) and "Max-Age=0" in c) for c in cookies) - with env.connect() as conn: - user = conn.execute("SELECT * FROM users").fetchone() - session = conn.execute("SELECT * FROM user_sessions").fetchone() - dump = "\n".join(str(tuple(r)) for t in ("users", "user_sessions", "discord_oauth_states") for r in conn.execute(f"SELECT * FROM {t}")) + [user] = testing.rows("SELECT * FROM users") + [session] = testing.rows("SELECT * FROM user_sessions") + dump = "\n".join(str(r) for t in ("users", "user_sessions", "discord_oauth_states") + for r in testing.rows(f"SELECT * FROM {t}")) assert user["discord_user_id"] == DISCORD_ID and user["discord_username"] == "steve_tfmc" - assert session["guild_member"] == 1 and session["user_id"] == user["id"] + assert session["guild_member"] is True and session["user_id"] == user["id"] + assert session["expires_at"] - session["created_at"] == users.SESSION_TTL assert ACCESS_TOKEN not in dump and OAUTH_CODE not in dump assert api.cookies.get(SESSION) not in dump logs = "\n".join(r.getMessage() for r in caplog.records if r.name == "auth") assert ACCESS_TOKEN not in logs and OAUTH_CODE not in logs +def test_session_times_keep_the_api_form(api, monkeypatch, env): + """Stored as timestamptz; cookies, /account and other areas still get YYYY-MM-DDTHH:MM:SSZ.""" + response, _ = sign_in(api, monkeypatch) + cookie = next(c for c in response.headers.get_list("set-cookie") if c.startswith(SESSION + "=")) + assert f"Max-Age={int(users.SESSION_TTL.total_seconds())}" in cookie + assert re.fullmatch(API_TIME, api.get("/account").json()["guild"]["checked_at"]) + user = users.session_user(api.cookies.get(SESSION)) + assert all(re.fullmatch(API_TIME, user[key]) for key in ("expires_at", "guild_checked_at", "created_at")) + assert users.SESSION_TTL.total_seconds() - 5 < routes.seconds_left(user["expires_at"]) <= users.SESSION_TTL.total_seconds() + assert user["guild_member"] is True + assert re.fullmatch(API_TIME, users.guild_state(user["session_id"])["guild_checked_at"]) + + +@pytest.fixture +def no_accounts(env, monkeypatch): + """No accounts database: DATABASE_URL unset and no pool, as on a preview of the postgres branch.""" + from src.database import connection + + monkeypatch.setattr(connection, "_pool", None) + monkeypatch.delenv("DATABASE_URL", raising=False) + + +def test_without_an_accounts_database_pages_work_signed_out_and_sign_in_is_503(api, no_accounts): + from src.auth import view_as + + assert api.get("/account").status_code == 401 + started = api.get("/auth/discord/start", follow_redirects=False) + assert (started.status_code, started.json()["detail"]) == (503, "accounts_unavailable") + assert users.session_user(None) is None and users.consume_state(None) is None + # A stale cookie gets a clear 503, never a 500; signing out still clears it. + api.cookies.set(SESSION, "stale-session-token", domain="testserver.local") + stale = api.get("/account") + assert (stale.status_code, stale.json()["detail"]) == (503, "accounts_unavailable") + assert api.post("/auth/logout", headers=ORIGIN).status_code == 200 + assert view_as.refuses_write(["stale-session-token"], None) is False + + def test_second_sign_in_updates_user_not_duplicate(api, monkeypatch, env): sign_in(api, monkeypatch) stub = Stub() stub.identity = lambda token: {"discord_user_id": DISCORD_ID, "discord_username": "renamed", "discord_global_name": None, "discord_avatar": None} sign_in(api, monkeypatch, stub) - with env.connect() as conn: - rows = conn.execute("SELECT discord_username FROM users").fetchall() - sessions = conn.execute("SELECT COUNT(*) FROM user_sessions").fetchone()[0] - assert [r[0] for r in rows] == ["renamed"] and sessions == 2 + rows = testing.rows("SELECT discord_username FROM users") + sessions = testing.value("SELECT COUNT(*) FROM user_sessions") + assert [r["discord_username"] for r in rows] == ["renamed"] and sessions == 2 def test_callback_without_matching_state_cookie_is_expired(api, monkeypatch, env): @@ -198,9 +237,7 @@ def test_expired_state_is_refused(api, monkeypatch, env): stub = Stub() monkeypatch.setattr(routes, "build_client", lambda config: stub) query, _ = start(api) - with env.connect() as conn: - conn.execute("UPDATE discord_oauth_states SET expires_at='2000-01-01T00:00:00Z'") - conn.commit() + testing.execute("UPDATE discord_oauth_states SET expires_at='2000-01-01T00:00:00Z'") response = api.get("/auth/discord/callback", params={"code": OAUTH_CODE, "state": query["state"][0]}, follow_redirects=False) assert response.headers["location"].endswith("signin=expired") and stub.exchanged == [] @@ -211,8 +248,7 @@ def test_denied_consent_creates_nothing(api, monkeypatch, env): query, _ = start(api) response = api.get("/auth/discord/callback", params={"error": "access_denied", "state": query["state"][0]}, follow_redirects=False) assert response.headers["location"].endswith("signin=denied") and stub.exchanged == [] - with env.connect() as conn: - assert conn.execute("SELECT COUNT(*) FROM users").fetchone()[0] == 0 + assert testing.value("SELECT COUNT(*) FROM users") == 0 @pytest.mark.parametrize("fail", ["exchange", "identity"]) @@ -220,8 +256,7 @@ def test_discord_failures_redirect_with_error(api, monkeypatch, env, fail): response, stub = sign_in(api, monkeypatch, Stub(fail=fail)) assert response.headers["location"] == SITE + "/account?signin=error" assert stub.closed - with env.connect() as conn: - assert conn.execute("SELECT COUNT(*) FROM user_sessions").fetchone()[0] == 0 + assert testing.value("SELECT COUNT(*) FROM user_sessions") == 0 def test_guild_check_failure_still_signs_in_without_membership(api, monkeypatch, env): @@ -240,9 +275,7 @@ def test_account_requires_session(api): def test_expired_session_is_refused(api, monkeypatch, env): sign_in(api, monkeypatch) - with env.connect() as conn: - conn.execute("UPDATE user_sessions SET expires_at='2000-01-01T00:00:00Z'") - conn.commit() + testing.execute("UPDATE user_sessions SET expires_at='2000-01-01T00:00:00Z'") assert api.get("/account").status_code == 401 @@ -299,9 +332,7 @@ def test_link_requires_guild_membership(api, monkeypatch): def test_link_requires_recent_guild_check(api, monkeypatch, env): sign_in(api, monkeypatch) old = users._iso(users._utcnow() - users.GUILD_CHECK_MAX_AGE - timedelta(minutes=1)) - with env.connect() as conn: - conn.execute("UPDATE user_sessions SET guild_checked_at=?", (old,)) - conn.commit() + testing.execute("UPDATE user_sessions SET guild_checked_at=%s", (old,)) response = api.post("/account/minecraft/link", json={"code": link_code()}, headers=ORIGIN) assert response.status_code == 403 and response.json()["detail"] == "guild_check_stale" assert api.get("/account").json()["guild"]["fresh"] is False @@ -602,9 +633,7 @@ def test_patreon_unlink_uses_signed_in_discord_account(api, monkeypatch): def make_stale(env, member=True): old = users._iso(users._utcnow() - users.GUILD_CHECK_MAX_AGE - timedelta(minutes=1)) - with env.connect() as conn: - conn.execute("UPDATE user_sessions SET guild_checked_at=?, guild_member=?", (old, 1 if member else 0)) - conn.commit() + testing.execute("UPDATE user_sessions SET guild_checked_at=%s, guild_member=%s", (old, member)) def bot_says(monkeypatch, answer, pause=0.0): diff --git a/backend/src/auth/test_locking.py b/backend/src/auth/test_locking.py new file mode 100644 index 00000000..fb8b3e82 --- /dev/null +++ b/backend/src/auth/test_locking.py @@ -0,0 +1,405 @@ +"""The accounts locking recipes, each raced on two connections against the real Postgres. + +One transaction holds a lock: either by hand on its own connection, or through a +Gate that pauses a path's transaction just after a chosen statement. The competing +path then runs on another thread. The test checks in pg_stat_activity that it is +waiting on a lock, and on which kind (`advisory` for the placeholders lock X3, a row +lock otherwise), then lets the holder commit and checks the waiter acted on the +holder's result instead of what it read before. + +Where a path takes the X3 advisory lock first, races between two whole calls of it +are serialised by X3, so tests that isolate a row-lock or constraint recipe hold +the competing row by hand instead. +""" +import sqlite3 +import threading +import time +from contextlib import contextmanager + +import pytest + +from src import database +from src.auth import admin, admin_cli, preview_sign_in, testing, users, view_as +from src.auth.test_admin import ORIGIN, account, app, client, discord_id_of, env # noqa: F401 +from src.auth.test_view_as import UUID, link_only +from src.skins.codes import hash_secret + +ADVISORY = {"advisory"} +ROW = {"transactionid", "tuple"} + + +class _Gated: + def __init__(self, conn, gate): + self._conn = conn + self._gate = gate + + def execute(self, query, params=None, **kwargs): + cursor = self._conn.execute(query, params, **kwargs) + self._gate.hit(str(query)) + return cursor + + def __getattr__(self, name): + return getattr(self._conn, name) + + +class Gate: + """Pauses the first transaction that runs a statement containing `pattern`, until released.""" + + def __init__(self, monkeypatch, pattern): + self.pattern = pattern + self.reached = threading.Event() + self.release = threading.Event() + self._armed = True + self._lock = threading.Lock() + real_connect, real_transaction = database.connect, database.transaction + + @contextmanager + def connect(**kwargs): + with real_connect(**kwargs) as conn: + yield _Gated(conn, self) + + @contextmanager + def transaction(**kwargs): + with real_transaction(**kwargs) as conn: + yield _Gated(conn, self) + + monkeypatch.setattr(database, "connect", connect) + monkeypatch.setattr(database, "transaction", transaction) + + def hit(self, query): + with self._lock: + if not self._armed or self.pattern not in query: + return + self._armed = False + self.reached.set() + assert self.release.wait(15), "gate never released" + + +class Run: + """fn() on its own thread; .value or .error once joined.""" + + def __init__(self, fn): + self.value = self.error = None + self.thread = threading.Thread(target=self._run, args=(fn,), daemon=True) + self.thread.start() + + def _run(self, fn): + try: + self.value = fn() + except BaseException as exc: # noqa: BLE001 - reported by the test + self.error = exc + + def join(self): + self.thread.join(25) + assert not self.thread.is_alive() + return self + + +def lock_wait(run: Run, kinds: set[str]) -> str: + """Wait until another backend of this database waits on a lock of one of `kinds`; return the kind. + + Fails if `run` finishes first: then it never waited. + """ + deadline = time.monotonic() + 10 + while time.monotonic() < deadline: + rows = testing.rows( + "SELECT wait_event FROM pg_stat_activity WHERE datname = current_database() " + "AND wait_event_type = 'Lock' AND pid <> pg_backend_pid()" + ) + if rows: + [row] = rows + assert row["wait_event"] in kinds, f"waiting on {row['wait_event']}, expected one of {kinds}" + return row["wait_event"] + if not run.thread.is_alive(): + pytest.fail(f"it finished without waiting on a lock ({run.error!r})") + time.sleep(0.02) + pytest.fail("no lock wait seen") + + +def race(gate, first, second, kinds): + """Run `first` up to the gate, start `second`, see it wait on `kinds`, then let both finish.""" + one = Run(first) + assert gate.reached.wait(10), one.error + two = Run(second) + lock_wait(two, kinds) + gate.release.set() + return one.join(), two.join() + + +def held(hold, fn, kinds): + """fn() while `hold(conn)` keeps its locks on another connection; it must wait on `kinds`.""" + with database.connect() as conn: + hold(conn) + run = Run(fn) + lock_wait(run, kinds) + return run.join() + + +def auth_config(): + from src.api import auth_routes + + return auth_routes._config() + + +# -------------------- +# B: compare-and-set +# -------------------- + +def test_a_discord_state_is_spent_once(env, monkeypatch): + state, _ = users.start_sign_in(auth_config(), "/map") + gate = Gate(monkeypatch, "DELETE FROM discord_oauth_states") + spend = lambda: users.consume_state(state) # noqa: E731 + one, two = race(gate, spend, spend, ROW) + assert (one.value, two.value) == ("/map", None) + + +def test_a_preview_ticket_is_redeemed_once(env, monkeypatch): + account(env, "steve") + token = users.sign_in({"discord_user_id": discord_id_of("steve"), "discord_username": "steve", + "discord_global_name": None, "discord_avatar": None}, guild_member=True) + site = "https://branch.tfminecraft.net" + ticket = preview_sign_in.issue(site, users.session_user(token)) + gate = Gate(monkeypatch, "DELETE FROM preview_sign_in_tickets t") + redeem = lambda: preview_sign_in.redeem(ticket, site) # noqa: E731 + one, two = race(gate, redeem, redeem, ROW) + assert one.value["discord_username"] == "steve" and two.value is None + + +# -------------------- +# C: constraints +# -------------------- + +def test_a_first_sign_in_joins_an_account_being_made(env): + """C on its own: the upsert waits on the uncommitted row with the same Discord id, then updates it.""" + discord_id = "123456789012345678" + identity = {"discord_user_id": discord_id, "discord_username": "new", "discord_global_name": None, + "discord_avatar": None} + run = held( + lambda conn: conn.execute( + "INSERT INTO users (discord_user_id, discord_username, created_at, updated_at, last_login_at) " + "VALUES (%s, 'other', now(), now(), now())", (discord_id,)), + lambda: users.sign_in(identity, guild_member=True), + ROW, + ) + assert run.error is None + assert testing.rows("SELECT discord_username FROM users") == [{"discord_username": "new"}] + assert testing.value("SELECT COUNT(*) FROM user_sessions") == 1 + + +def test_a_placeholder_never_collides_with_a_new_account(env): + """C on its own: a placeholder insert waits for a sign-in's insert of the same id and keeps it.""" + discord_id = link_only(env, "steve") + + def make(): + with database.connect() as conn: + return view_as._make_placeholder(conn, {"discord_user_id": discord_id, "discord_username": "steve"}) + + run = held( + lambda conn: conn.execute( + "INSERT INTO users (discord_user_id, discord_username, created_at, updated_at, last_login_at) " + "VALUES (%s, 'steve', now(), now(), now())", (discord_id,)), + make, + ROW, + ) + assert run.error is None and run.value["discord_user_id"] == discord_id + assert testing.value("SELECT placeholder FROM users WHERE discord_user_id = %s", (discord_id,)) is False + + +# -------------------- +# A: row locks +# -------------------- + +def test_a_role_change_waits_for_a_change_to_its_target(env): + """A in _staff_write: the target's row is locked before it is read, so a change landing + meanwhile is seen (here: the role is already what was asked) rather than overwritten.""" + _, rory = account(env, "rory", "root") + rhea_id, _ = account(env, "rhea", "root") + run = held( + lambda conn: conn.execute("UPDATE users SET role = 'admin' WHERE id = %s", (rhea_id,)), + lambda: admin.change_role(rory, rhea_id, "admin", "Stepping down"), + ROW, + ) + assert isinstance(run.error, admin.AdminError) and run.error.code == "role_unchanged" + + +def test_root_locks_wait_for_a_panel_demotion(env): + """A in _set_root: the root count waits for a demotion that holds a root's row.""" + rory_id, _ = account(env, "rory", "root") + account(env, "rhea", "root") + run = held( + lambda conn: conn.execute("UPDATE users SET role = 'admin' WHERE id = %s", (rory_id,)), + lambda: admin_cli._set_root(discord_id_of("rhea"), "Stepping down", grant=False), + ROW, + ) + assert isinstance(run.error, SystemExit) and "last root" in str(run.error) + assert testing.value("SELECT COUNT(*) FROM users WHERE role = 'root'") == 1 + + +# -------------------- +# X1: staff re-checks hold the session and user rows +# -------------------- + +@pytest.mark.parametrize("change", [ + "UPDATE users SET role = 'mod' WHERE id = %s", + "DELETE FROM user_sessions WHERE user_id = %s", +]) +def test_a_staff_recheck_holds_off_demotion_and_revocation(env, change): + admin_id, token = account(env, "adam", "admin") + with database.connect() as conn: + assert users.session_user_in(conn, token, lock=True)["role"] == "admin" + other = Run(lambda: testing.execute(change, (admin_id,))) + lock_wait(other, ROW) + other.join() + assert other.error is None and other.value == 1 + + +def test_an_ordinary_session_read_takes_no_lock(env): + admin_id, token = account(env, "adam", "admin") + with database.connect() as conn: + assert users.session_user_in(conn, token)["role"] == "admin" + assert testing.execute("UPDATE users SET role = 'mod' WHERE id = %s", (admin_id,)) == 1 + + +def test_a_lock_cannot_be_asked_of_the_sqlite_bridge(env): + _, token = account(env, "adam", "admin") + with pytest.raises(TypeError, match="Postgres"): + users.session_user_in(sqlite3.connect(":memory:"), token, lock=True) + + +def test_a_server_action_and_a_demotion_never_cross(app, env, monkeypatch): # noqa: F811 + from src.servers import control + + monkeypatch.setattr(control, "request", lambda op, **body: {"id": body["job_id"]}) + _, root = account(env, "rory", "root") + adam_id, adam = account(env, "adam", "admin") + # Paused once the action's audit row is written, still inside its transaction. + gate = Gate(monkeypatch, "INSERT INTO admin_audit") + body = {"id": "c" * 32, "reason": "scheduled restart"} + first, second = race( + gate, + lambda: client(app, adam).post("/admin/servers/dev/cancel", json=body, headers=ORIGIN), + lambda: admin.change_role(root, adam_id, "mod", "Stepping down"), + ROW, + ) + assert first.value.status_code == 200 and second.error is None + rows = [(r["action"], r["actor_role"], r["outcome"]) for r in testing.audit_rows()] + # The action was on record as the admin's before the demotion could commit. + assert rows[0] == ("server.cancel", "admin", "requested") + assert rows.index(("account.role.change", "root", "ok")) > 0 + + +# -------------------- +# X2: ending a view once +# -------------------- + +def _view(env): + _, admin_token = account(env, "boss", "admin") + link_only(env, "steve") + return view_as.start(admin_token, UUID)["token"] + + +def test_ending_a_view_locks_its_session_row(env): + """X2 on its own, without the placeholders lock: the second end finds nothing left to end.""" + view_token = _view(env) + where = ("s.token_hash = %s", (hash_secret(view_token),)) + + def end(): + with database.connect() as conn: + return view_as._end_where(conn, *where) + + run = held(lambda conn: view_as._end_where(conn, *where), end, ROW) + assert run.error is None and run.value == [] + assert [r["action"] for r in testing.audit_rows()].count("account.view_as.stop") == 1 + + +def test_a_view_still_open_check_holds_the_view_until_its_write_commits(env): + """view_still_open on a Postgres connection: ending the view waits for the check's transaction.""" + view_token = _view(env) + view = users.session_user(view_token) + with database.connect() as conn: + assert view_as.view_still_open(view["session_id"])(conn) is True + ending = Run(lambda: view_as.end(view_token)) + lock_wait(ending, ROW) + assert ending.join().error is None + assert users.session_user(view_token) is None + + +# -------------------- +# X3: placeholders and the views being committed +# -------------------- + +def test_two_requests_ending_one_view_wait_for_each_other(env, monkeypatch): + """Two whole end() calls: the placeholders lock serialises them, and the view is ended once.""" + view_token = _view(env) + gate = Gate(monkeypatch, "FOR UPDATE OF s") + end = lambda: view_as.end(view_token) # noqa: E731 + one, two = race(gate, end, end, ADVISORY) + assert one.error is None and two.error is None + assert [r["action"] for r in testing.audit_rows()].count("account.view_as.stop") == 1 + + +def test_a_view_being_opened_keeps_its_placeholder(env, monkeypatch): + """Ending one admin's view while another admin opens a view of the same placeholder.""" + _, boss = account(env, "boss", "admin") + _, other = account(env, "other", "admin") + link_only(env, "steve") + first_view = view_as.start(boss, UUID)["token"] + gate = Gate(monkeypatch, "INSERT INTO user_sessions") + opening, ending = race(gate, lambda: view_as.start(other, UUID), lambda: view_as.end(first_view), ADVISORY) + assert opening.error is None and ending.error is None + assert users.session_user(first_view) is None + second = users.session_user(opening.value["token"]) + assert second is not None and second["discord_username"] == "steve" + assert testing.value("SELECT COUNT(*) FROM users WHERE placeholder") == 1 + + +def test_two_roots_demoting_each_other_run_one_after_the_other(env, monkeypatch): + """Two whole role changes: the placeholders lock serialises them; the second's actor is gone.""" + one_id, one = account(env, "rory", "root") + two_id, two = account(env, "rhea", "root") + gate = Gate(monkeypatch, "FOR UPDATE") + first, second = race(gate, lambda: admin.change_role(one, two_id, "admin", "Stepping down"), + lambda: admin.change_role(two, one_id, "admin", "Stepping down"), ADVISORY) + assert first.error is None and first.value["after"] == "admin" + assert isinstance(second.error, admin.AdminError) and second.error.code == "not_signed_in" + assert testing.value("SELECT role FROM users WHERE id = %s", (one_id,)) == "root" + + +def test_two_cli_removals_cannot_leave_no_root(env, monkeypatch): + """Two whole revoke-root runs: the placeholders lock serialises them; the second sees one root.""" + account(env, "rory", "root") + account(env, "rhea", "root") + gate = Gate(monkeypatch, "FOR UPDATE") + first, second = race( + gate, + lambda: admin_cli._set_root(discord_id_of("rory"), "Stepping down", grant=False), + lambda: admin_cli._set_root(discord_id_of("rhea"), "Stepping down", grant=False), + ADVISORY, + ) + assert first.error is None + assert isinstance(second.error, SystemExit) and "last root" in str(second.error) + assert testing.value("SELECT COUNT(*) FROM users WHERE role = 'root'") == 1 + + +def test_every_leftover_sweep_takes_the_placeholders_lock_first(env): + _, boss = account(env, "boss", "admin") + pat_id, _ = account(env, "pat") + _, root = account(env, "rory", "root") + link_only(env, "steve") + identity = {"discord_user_id": "123456789012345678", "discord_username": "new", "discord_global_name": None, + "discord_avatar": None} + + def under_lock(fn): + run = held(lambda conn: database.advisory_lock(conn, users.PLACEHOLDERS_LOCK), fn, ADVISORY) + assert run.error is None, run.error + return run.value + + under_lock(lambda: users.sign_in(identity, guild_member=True)) + first = under_lock(lambda: view_as.start(boss, UUID))["token"] + under_lock(lambda: view_as.end(first)) + second = view_as.start(boss, UUID)["token"] + assert under_lock(lambda: view_as.stop(second, boss))["ended"] is True + under_lock(lambda: view_as.end_lapsed(boss)) + under_lock(lambda: admin.change_role(root, pat_id, "mod", "Helping out")) + under_lock(lambda: admin.revoke_sessions(root, pat_id, "Shared login")) + under_lock(lambda: admin_cli._set_root(discord_id_of("pat"), "Second owner", grant=True)) diff --git a/backend/src/auth/test_microsoft.py b/backend/src/auth/test_microsoft.py index 9cae6fb2..46c02066 100644 --- a/backend/src/auth/test_microsoft.py +++ b/backend/src/auth/test_microsoft.py @@ -10,7 +10,7 @@ import pytest from src.api import auth_routes as routes -from src.auth import microsoft, microsoft_link, users +from src.auth import microsoft, microsoft_link, testing, users from src.auth.microsoft import MicrosoftClient, MicrosoftConfig, MicrosoftError from src.auth.test_auth_routes import ( # noqa: F401 - fixtures DISCORD_ID, @@ -222,7 +222,8 @@ def test_microsoft_link_end_to_end(api, monkeypatch, ms_env, caplog): with ms_env.connect() as conn: assert conn.execute("SELECT COUNT(*) FROM microsoft_link_states").fetchone()[0] == 0 notice = conn.execute("SELECT type FROM plugin_notices WHERE player_uuid=?", (PLAYER,)).fetchone() - dump = "\n".join(str(tuple(r)) for t in ("users", "user_sessions", "discord_links") for r in conn.execute(f"SELECT * FROM {t}")) + dump = "\n".join(str(tuple(r)) for r in conn.execute("SELECT * FROM discord_links")) + dump += "\n".join(str(r) for t in ("users", "user_sessions") for r in testing.rows(f"SELECT * FROM {t}")) assert notice[0] == "link_success" logs = "\n".join(r.getMessage() for r in caplog.records if r.name == "auth") for secret in (MS_CODE, MS_TOKEN, MC_TOKEN, "xsts-token", "xbl-token"): @@ -286,9 +287,7 @@ def test_new_attempt_replaces_older_one(api, monkeypatch, ms_env): def age_guild_check(database): old = users._iso(users._utcnow() - users.GUILD_CHECK_MAX_AGE - timedelta(minutes=1)) - with database.connect() as conn: - conn.execute("UPDATE user_sessions SET guild_checked_at=?", (old,)) - conn.commit() + testing.execute("UPDATE user_sessions SET guild_checked_at=%s", (old,)) def test_callback_without_state_cookie_is_expired(api, monkeypatch, ms_env): @@ -387,9 +386,7 @@ def test_sign_out_during_the_chain_stops_the_link(api, monkeypatch, ms_env): sign_in(api, monkeypatch) def profile_after_logout(request): - with ms_env.connect() as conn: - conn.execute("DELETE FROM user_sessions") - conn.commit() + testing.execute("DELETE FROM user_sessions") return httpx.Response(200, json={"id": PROFILE_ID, "name": "SteveMC"}) use_chain(monkeypatch, {microsoft.MC_PROFILE_URL: profile_after_logout}) diff --git a/backend/src/auth/test_players.py b/backend/src/auth/test_players.py index 4a87cabe..0ea2d22d 100644 --- a/backend/src/auth/test_players.py +++ b/backend/src/auth/test_players.py @@ -1,10 +1,11 @@ import json +import re import pytest from fastapi import FastAPI from src.api import admin_routes, auth_routes -from src.auth import players +from src.auth import players, testing from src.auth.test_admin import account, client HAZEL = "0615a817-8cb4-4aef-95f7-f6c9bf7611b8" @@ -24,7 +25,7 @@ def app(env, coreprotect): @pytest.fixture -def env(database, monkeypatch): +def env(database, postgres, monkeypatch): monkeypatch.setenv("DISCORD_AUTH_ENABLED", "1") monkeypatch.setenv("DISCORD_CLIENT_ID", "client-id") monkeypatch.setenv("DISCORD_CLIENT_SECRET", "client-secret") @@ -332,12 +333,14 @@ def test_character_picture_without_a_skin_and_staff_catalog(app, env, world): def test_profile_shows_the_website_account(app, env, world): token = staff(env, "admin") + staff_discord_id = testing.value("SELECT discord_user_id FROM users LIMIT 1") with env.connect() as conn: - conn.execute("UPDATE discord_links SET discord_user_id = (SELECT discord_user_id FROM users LIMIT 1) " - "WHERE player_uuid = ?", (HAZEL,)) + conn.execute("UPDATE discord_links SET discord_user_id = ? WHERE player_uuid = ?", (staff_discord_id, HAZEL)) conn.commit() body = client(app, token).get(f"/admin/players/{HAZEL}").json() assert body["account"]["role"] == "admin" + assert re.fullmatch(r"\d{4}-\d\d-\d\dT\d\d:\d\d:\d\dZ", body["account"]["created_at"]) + assert re.fullmatch(r"\d{4}-\d\d-\d\dT\d\d:\d\d:\d\dZ", body["account"]["last_login_at"]) def test_profile_errors(app, env, world, monkeypatch): @@ -389,9 +392,7 @@ def test_unknown_player_sections_are_empty(app, env, world, monkeypatch): def message_audits(db): - with db.connect() as conn: - return [dict(r) for r in conn.execute( - "SELECT * FROM admin_audit WHERE action = 'player.messages.view' ORDER BY id")] + return testing.rows("SELECT * FROM admin_audit WHERE action = 'player.messages.view' ORDER BY id") def test_mods_never_see_messages(app, env, world, coreprotect): @@ -425,11 +426,11 @@ def test_admins_see_chat_and_whole_commands_and_it_is_audited(app, env, world, c assert rows[0]["actor_role"] == role assert rows[0]["outcome"] == "ok" assert rows[0]["reason"] is None - detail = json.loads(rows[0]["detail_json"]) + detail = rows[0]["detail_json"] assert detail["player_uuid"] == HAZEL and detail["rows"] == 3 and detail["paged"] is False assert detail["newest"]["time"] == 180 and detail["oldest"]["time"] == 160 # The audit records which rows were shown, never what they said. - assert "docks" not in rows[0]["detail_json"] and "private" not in rows[0]["detail_json"] + assert "docks" not in json.dumps(rows[0]["detail_json"]) and "private" not in json.dumps(rows[0]["detail_json"]) def test_channel_commands_are_chat_for_admins_and_a_channel_name_for_mods(app, env, world, coreprotect): @@ -445,7 +446,7 @@ def test_channel_commands_are_chat_for_admins_and_a_channel_name_for_mods(app, e assert [e["target"] for e in feed["entries"] if e["kind"] == "command"] == ["/home", "/msg Bob a private thing"] only_chat = client(app, token).get(f"/admin/players/{HAZEL}/activity", params={"kinds": "chat"}).json() assert [e["channel"] for e in only_chat["entries"]] == ["Emote", "FOOC", "LOOC"] - assert json.loads(message_audits(env)[0]["detail_json"])["rows"] == 5 + assert message_audits(env)[0]["detail_json"]["rows"] == 5 mod = client(app, staff(env, "mod")).get(f"/admin/players/{HAZEL}/activity").json() commands = [(e["target"], e["channel"]) for e in mod["entries"] if e["kind"] == "command"] @@ -483,9 +484,7 @@ def test_oversized_cursor_values_are_refused(app, env, world): def movement_audits(db): - with db.connect() as conn: - return [dict(r) for r in conn.execute( - "SELECT * FROM admin_audit WHERE action = 'player.movement.view' ORDER BY id")] + return testing.rows("SELECT * FROM admin_audit WHERE action = 'player.movement.view' ORDER BY id") @pytest.mark.parametrize("role", ["admin", "root"]) @@ -506,8 +505,8 @@ def test_movement_routes_are_audited(app, env, world, coreprotect, role): assert {p["minecraft_name"] for p in body["players"]} == {"MrEnzo99", "Quiet"} rows = movement_audits(env) - assert [json.loads(r["detail_json"])["player_uuid"] for r in rows] == [HAZEL, "everyone"] - detail = json.loads(rows[0]["detail_json"]) + assert [r["detail_json"]["player_uuid"] for r in rows] == [HAZEL, "everyone"] + detail = rows[0]["detail_json"] assert (detail["since"], detail["until"], detail["rows"]) == (50, 300, 3) # Which window and how much, never where. assert set(detail) == {"server", "player_uuid", "since", "until", "complete_from", "rows"} @@ -537,7 +536,7 @@ def test_session_movement(app, env, world, coreprotect): assert (body["since"], body["until"]) == (100, 200) assert [p[0] for p in body["points"]] == [100, 160, 200] assert body["as_of"] > 0 - detail = json.loads(movement_audits(env)[-1]["detail_json"]) + detail = movement_audits(env)[-1]["detail_json"] assert detail["session"] == sid and detail["rows"] == 3 # Another player's session id, a forged one, and a mod are all refused. diff --git a/backend/src/auth/test_preview_sign_in.py b/backend/src/auth/test_preview_sign_in.py index 9b328baa..61be7fec 100644 --- a/backend/src/auth/test_preview_sign_in.py +++ b/backend/src/auth/test_preview_sign_in.py @@ -1,3 +1,4 @@ +import re from datetime import timedelta from urllib.parse import parse_qs, urlsplit @@ -7,9 +8,8 @@ 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 import guild_check, preview_sign_in, testing, users from src.auth.config import AuthConfig -from src.skins.db import connect DEV = "https://dev.tfminecraft.net" PREVIEW = "https://my-branch.tfminecraft.net" @@ -27,7 +27,7 @@ def _app(base_url): @pytest.fixture -def dev(database, monkeypatch): +def dev(database, postgres, 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") @@ -44,7 +44,7 @@ def dev(database, monkeypatch): @pytest.fixture -def preview(database, monkeypatch): +def preview(database, postgres, 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"): @@ -191,6 +191,17 @@ def test_preview_callback_reports_a_refused_ticket(preview, monkeypatch): assert SESSION not in response.cookies +def test_preview_callback_without_an_accounts_database_keeps_the_ticket(preview, monkeypatch): + monkeypatch.setattr(preview_sign_in, "fetch_player", lambda *a: pytest.fail("ticket spent")) + preview.cookies.set(STATE, "s" * 43, domain="my-branch.tfminecraft.net") + with testing.no_accounts_database(): + response = preview.get(f"/auth/preview/callback?ticket={'t' * 43}&state={'s' * 43}", follow_redirects=False) + started = preview.get("/auth/discord/start", follow_redirects=False) + assert (response.status_code, response.json()["detail"]) == (503, "accounts_unavailable") + assert SESSION not in response.cookies + assert (started.status_code, started.json()["detail"]) == (503, "accounts_unavailable") + + def test_preview_has_no_discord_callback(preview): assert preview.get("/auth/discord/callback?code=x&state=y").status_code == 404 @@ -242,6 +253,9 @@ def test_dev_start_hands_a_signed_in_player_to_the_preview(dev): 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 + # The hand-off keeps the Z form previews parse strictly, so a preview accepts dev's answer. + assert re.fullmatch(r"\d{4}-\d\d-\d\dT\d\d:\d\d:\d\dZ", body["guild_checked_at"]) + assert preview_sign_in._player(body)["guild_checked_at"] == body["guild_checked_at"] assert dev.post("/auth/preview/redeem", json={"ticket": query["ticket"], "site": PREVIEW}).status_code == 410 @@ -264,9 +278,7 @@ def test_dev_start_refuses_a_malformed_site_without_failing(dev): 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() + testing.execute("UPDATE user_sessions SET guild_checked_at = %s", (stamp,)) return stamp @@ -338,8 +350,7 @@ def test_tickets_are_for_one_site_and_expire(dev, monkeypatch): 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")] + stored = [row["ticket_hash"] for row in testing.rows("SELECT ticket_hash FROM preview_sign_in_tickets")] assert stored and ticket not in stored diff --git a/backend/src/auth/test_staff_tools.py b/backend/src/auth/test_staff_tools.py index f9f5ffa2..46aa58fe 100644 --- a/backend/src/auth/test_staff_tools.py +++ b/backend/src/auth/test_staff_tools.py @@ -7,12 +7,12 @@ from src.api import auth_routes, precedent_routes, skins_routes from src.api.staff_access import staff_name -from src.auth import roles +from src.auth import roles, testing from src.auth.test_admin import ORIGIN, account, client @pytest.fixture -def app(database, monkeypatch): +def app(database, postgres, monkeypatch): monkeypatch.setenv("DISCORD_AUTH_ENABLED", "1") monkeypatch.setenv("DISCORD_CLIENT_ID", "client-id") monkeypatch.setenv("DISCORD_CLIENT_SECRET", "client-secret") @@ -42,8 +42,7 @@ def link(db, discord_user_id, uuid="player-1", name="StaffMC"): def discord_id(db, user_id): - with db.connect() as conn: - return conn.execute("SELECT discord_user_id FROM users WHERE id = ?", (user_id,)).fetchone()[0] + return testing.value("SELECT discord_user_id FROM users WHERE id = %s", (user_id,)) def test_moderators_have_both_tools(): diff --git a/backend/src/auth/test_view_as.py b/backend/src/auth/test_view_as.py index a2737461..e61dc920 100644 --- a/backend/src/auth/test_view_as.py +++ b/backend/src/auth/test_view_as.py @@ -1,3 +1,4 @@ +import re import sqlite3 import uuid @@ -7,7 +8,7 @@ from src.api import admin_routes, auth_routes from src.api.view_as_guard import ViewAsGuard -from src.auth import roles, users, view_as +from src.auth import roles, testing, users, view_as SITE = "https://www.tfminecraft.net" ORIGIN = {"Origin": SITE} @@ -18,7 +19,7 @@ @pytest.fixture -def env(database, monkeypatch): +def env(database, postgres, monkeypatch): monkeypatch.setenv("DISCORD_AUTH_ENABLED", "1") monkeypatch.setenv("DISCORD_CLIENT_ID", "client-id") monkeypatch.setenv("DISCORD_CLIENT_SECRET", "client-secret") @@ -52,16 +53,15 @@ def account(db, name, role="player", minecraft=None): "discord_avatar": None}, guild_member=True, ) - with db.connect() as conn: - conn.execute("UPDATE users SET role = ? WHERE discord_user_id = ?", (role, discord_id)) - if minecraft: + user_id = testing.set_role(discord_id, role) + if minecraft: + with db.connect() as conn: conn.execute( "INSERT INTO discord_links (player_uuid, discord_user_id, minecraft_name, discord_username, linked_at)" " VALUES (?, ?, ?, ?, '2026-10-01T00:00:00Z')", (UUID, discord_id, minecraft, name), ) - conn.commit() - user_id = conn.execute("SELECT id FROM users WHERE discord_user_id = ?", (discord_id,)).fetchone()[0] + conn.commit() return user_id, token @@ -77,14 +77,13 @@ def cookie(c, name): def audit_rows(db): - with db.connect() as conn: - return [dict(r) for r in conn.execute("SELECT * FROM admin_audit ORDER BY id")] + return testing.audit_rows() def linked_uuid(db, user_id): """The Minecraft UUID linked to this account, linking one first if it has none.""" + discord_id = testing.value("SELECT discord_user_id FROM users WHERE id = %s", (user_id,)) with db.connect() as conn: - discord_id = conn.execute("SELECT discord_user_id FROM users WHERE id = ?", (user_id,)).fetchone()[0] row = conn.execute("SELECT player_uuid FROM discord_links WHERE discord_user_id = ?", (discord_id,)).fetchone() if row: return row[0] @@ -113,8 +112,7 @@ def link_only(db, name, player_uuid=UUID): def placeholders(db): - with db.connect() as conn: - return conn.execute("SELECT COUNT(*) FROM users WHERE placeholder = 1").fetchone()[0] + return testing.value("SELECT COUNT(*) FROM users WHERE placeholder") def test_only_admins_may_view_as(): @@ -128,6 +126,9 @@ def test_admin_views_the_site_as_a_player(app, env): c, res = start(app, env, admin_token, player_id) assert res.status_code == 200 assert res.json()["return_to"] == "/profile" + assert re.fullmatch(r"\d{4}-\d\d-\d\dT\d\d:\d\d:\d\dZ", res.json()["expires_at"]) + view_cookie = next(h for h in res.headers.get_list("set-cookie") if h.startswith(SESSION + "=")) + assert re.search(r"Max-Age=(17\d\d|1800);", view_cookie) assert cookie(c, STAFF) == admin_token view_token = cookie(c, SESSION) assert view_token and view_token != admin_token @@ -251,9 +252,7 @@ def test_a_view_that_ran_out_hands_back_on_the_next_account_load(app, env): _, admin_token = account(env, "boss", "admin") player_id, _ = account(env, "steve") c, _ = start(app, env, admin_token, player_id) - with env.connect() as conn: - conn.execute("UPDATE user_sessions SET expires_at = '2000-01-01T00:00:00Z' WHERE acting_user_id IS NOT NULL") - conn.commit() + testing.expire_views() me = c.get("/account") assert me.status_code == 200 assert me.json()["user"]["discord_username"] == "boss" and me.json()["viewing_as"] is None @@ -294,9 +293,7 @@ def test_a_view_has_no_staff_rights_even_if_the_target_is_promoted(app, env): _, admin_token = account(env, "boss", "admin") player_id, _ = account(env, "steve") c, _ = start(app, env, admin_token, player_id) - with env.connect() as conn: - conn.execute("UPDATE users SET role = 'mod' WHERE id = ?", (player_id,)) - conn.commit() + testing.execute("UPDATE users SET role = 'mod' WHERE id = %s", (player_id,)) assert c.get("/admin/me").status_code == 403 @@ -337,9 +334,7 @@ def test_profile_tag_only_inside_a_view(): def _expire_views(db): - with db.connect() as conn: - conn.execute("UPDATE user_sessions SET expires_at = '2000-01-01T00:00:00Z' WHERE acting_user_id IS NOT NULL") - conn.commit() + testing.expire_views() def _view_profiles(db): @@ -359,10 +354,9 @@ def test_a_view_whose_cookie_the_browser_dropped_is_still_ended_and_audited(app, assert res.json() == {"restored": True, "return_to": f"/admin/players/{UUID}"} assert cookie(c, SESSION) == admin_token stopped = audit_rows(env)[-1] - assert stopped["action"] == "account.view_as.stop" and '"expired": true' in stopped["detail_json"] + assert stopped["action"] == "account.view_as.stop" and stopped["detail_json"]["expired"] is True assert _view_profiles(env) == 0 - with env.connect() as conn: - assert conn.execute("SELECT COUNT(*) FROM user_sessions WHERE acting_user_id IS NOT NULL").fetchone()[0] == 0 + assert testing.value("SELECT COUNT(*) FROM user_sessions WHERE acting_user_id IS NOT NULL") == 0 def test_leaving_twice_is_harmless(app, env): @@ -412,9 +406,7 @@ def test_profile_is_not_opened_for_a_view_that_ended_mid_request(app, env): player_id, _ = account(env, "steve", minecraft="Steve") c, _ = start(app, env, admin_token, player_id) view = users.session_user(cookie(c, SESSION)) - with env.connect() as conn: - conn.execute("DELETE FROM user_sessions WHERE id = ?", (view["session_id"],)) - conn.commit() + testing.execute("DELETE FROM user_sessions WHERE id = %s", (view["session_id"],)) with pytest.raises(codes.CodeError, match="session_ended"): codes.start_linked_profile_session(UUID, minted_via=view_as.profile_minted_via(view), expires_at=view["expires_at"], @@ -431,8 +423,7 @@ def test_signing_out_after_the_view_cookie_lapsed_still_ends_the_view(app, env): c.cookies.delete(SESSION, domain=DOMAIN) assert c.post("/auth/logout", headers=ORIGIN).status_code == 200 assert users.session_user(admin_token) is None - with env.connect() as conn: - assert conn.execute("SELECT COUNT(*) FROM user_sessions WHERE acting_user_id IS NOT NULL").fetchone()[0] == 0 + assert testing.value("SELECT COUNT(*) FROM user_sessions WHERE acting_user_id IS NOT NULL") == 0 assert audit_rows(env)[-1]["action"] == "account.view_as.stop" @@ -446,7 +437,7 @@ def test_starting_a_view_from_inside_one_is_refused_and_audited(app, env): assert (res.status_code, res.json()["detail"]) == (403, "viewing_as_player") row = audit_rows(env)[-1] assert (row["action"], row["outcome"], row["actor_name"]) == ("account.view_as.start", "conflict", "boss") - assert f'"player_uuid": "{other_uuid}"' in row["detail_json"] + assert row["detail_json"]["player_uuid"] == other_uuid def test_a_player_who_never_signed_in_is_viewed_through_a_placeholder(app, env): @@ -461,7 +452,7 @@ def test_a_player_who_never_signed_in_is_viewed_through_a_placeholder(app, env): assert c.post("/characters/anything", headers=ORIGIN).json()["detail"] == "viewing_as_player" started = audit_rows(env)[-1] assert (started["action"], started["target_name"]) == ("account.view_as.start", "steve") - assert '"placeholder": true' in started["detail_json"] + assert started["detail_json"]["placeholder"] is True # Staff still see them as having no site account. assert placeholders(env) == 1 @@ -470,8 +461,7 @@ def test_a_player_who_never_signed_in_is_viewed_through_a_placeholder(app, env): assert c.post("/auth/view-as/stop", headers=ORIGIN).json()["restored"] is True assert placeholders(env) == 0 - with env.connect() as conn: - assert conn.execute("SELECT COUNT(*) FROM users WHERE discord_user_id = ?", (discord_id,)).fetchone()[0] == 0 + assert testing.value("SELECT COUNT(*) FROM users WHERE discord_user_id = %s", (discord_id,)) == 0 def test_a_placeholder_whose_view_ran_out_goes_with_it(app, env): @@ -493,7 +483,6 @@ def test_signing_in_during_a_view_turns_the_placeholder_into_their_account(app, "discord_avatar": None}, guild_member=True) assert placeholders(env) == 0 c.post("/auth/view-as/stop", headers=ORIGIN) - with env.connect() as conn: - row = conn.execute("SELECT placeholder, discord_global_name FROM users WHERE discord_user_id = ?", - (discord_id,)).fetchone() - assert tuple(row) == (0, "Steve") + [row] = testing.rows("SELECT placeholder, discord_global_name FROM users WHERE discord_user_id = %s", + (discord_id,)) + assert row == {"placeholder": False, "discord_global_name": "Steve"} diff --git a/backend/src/auth/testing.py b/backend/src/auth/testing.py new file mode 100644 index 00000000..a3ed08d4 --- /dev/null +++ b/backend/src/auth/testing.py @@ -0,0 +1,53 @@ +"""Test helpers for accounts, which live in Postgres. Tests using them take the `postgres` fixture.""" +from __future__ import annotations + +from contextlib import contextmanager + +import pytest + +from src import database +from src.database import connection + + +def rows(sql: str, params: tuple = ()) -> list[dict]: + with database.connect() as conn: + return conn.execute(sql, params).fetchall() + + +def value(sql: str, params: tuple = ()): + """The first column of the first row, or None.""" + with database.connect() as conn: + row = conn.execute(sql, params).fetchone() + return None if row is None else next(iter(row.values())) + + +def execute(sql: str, params: tuple = ()) -> int: + """Run one statement in its own transaction; the number of rows it touched.""" + with database.connect() as conn: + return conn.execute(sql, params).rowcount + + +def audit_rows() -> list[dict]: + """The audit log, oldest first; detail_json is the parsed object.""" + return rows("SELECT * FROM admin_audit ORDER BY id") + + +def set_role(discord_id: str, role: str) -> int: + """Give the account a role; its id.""" + with database.connect() as conn: + return conn.execute("UPDATE users SET role = %s WHERE discord_user_id = %s RETURNING id", + (role, discord_id)).fetchone()["id"] + + +def expire_views() -> None: + execute("UPDATE user_sessions SET expires_at = '2000-01-01T00:00:00Z' WHERE acting_user_id IS NOT NULL") + + +@contextmanager +def no_accounts_database(): + """Inside the block there is no accounts database: DATABASE_URL unset and no pool, + as on a preview of the postgres branch. The test database is back afterwards.""" + with pytest.MonkeyPatch.context() as patch: + patch.setattr(connection, "_pool", None) + patch.delenv("DATABASE_URL", raising=False) + yield diff --git a/backend/src/auth/users.py b/backend/src/auth/users.py index 3fa3534f..c98f20f4 100644 --- a/backend/src/auth/users.py +++ b/backend/src/auth/users.py @@ -1,12 +1,22 @@ -"""Users, their cookie sessions and Discord OAuth states. Tokens are stored only as hashes.""" +"""Users, their cookie sessions and Discord OAuth states. Tokens are stored only as hashes. + +Times are timestamptz in Postgres. The session user dict carries them as the +`YYYY-MM-DDTHH:MM:SSZ` strings the site has always sent: cookie lifetimes, +/account, the preview hand-off and other areas parse that form. +""" from __future__ import annotations +import json import secrets +import sqlite3 from datetime import datetime, timedelta, timezone from urllib.parse import urlencode +from fastapi import HTTPException + +from src import database +from src.skins import db as sqlite_db from src.skins.codes import hash_secret -from src.skins.db import connect from src.skins.discord_link import remember_discord_nicknames, remember_discord_usernames from .config import AuthConfig @@ -23,22 +33,59 @@ # codes.minted_via for a Profile session opened during a view: this prefix and the view's session id. VIEW_PROFILE_PREFIX = "staff_view:" +# Every transaction that calls drop_view_leftovers takes this lock as its first +# statement, so it never deletes a placeholder whose view is being committed. +PLACEHOLDERS_LOCK = "auth:placeholders" + + +class AccountsUnavailable(HTTPException): + """No accounts database: DATABASE_URL is unset (a local run without Postgres, or a preview + before previews get their own database). Signed-out pages still work; sign-in answers 503.""" + + def __init__(self): + super().__init__(503, detail="accounts_unavailable") + + +def accounts_configured() -> bool: + try: + database.get_pool() + except database.NotConfiguredError: + return False + return True + + +def require_accounts() -> None: + if not accounts_configured(): + raise AccountsUnavailable() def drop_view_leftovers(conn) -> None: """Delete what views that no longer exist leave behind, however they ended: Profile - sessions opened during them, and placeholder accounts made for them.""" - conn.execute( - """ - DELETE FROM sessions WHERE code_id IN ( - SELECT id FROM codes - WHERE minted_via LIKE 'staff_view:%' - AND CAST(substr(minted_via, length('staff_view:') + 1) AS INTEGER) - NOT IN (SELECT id FROM user_sessions) + sessions opened during them, and placeholder accounts made for them. + + The caller's transaction must have opened with `database.transaction(lock=PLACEHOLDERS_LOCK)`. + """ + # PG-MIGRATION: until the Skins area converts, Profile sessions and their codes are in + # SQLite. This commits its SQLite delete in the middle of the Postgres transaction (it stays + # if that transaction rolls back) and takes SQLite's write lock while holding Postgres locks. + # Accepted until then; the Skins PR runs this on `conn` instead: + # DELETE FROM sessions WHERE code_id IN (SELECT id FROM codes WHERE minted_via LIKE 'staff_view:%' + # AND CAST(substr(minted_via, length('staff_view:') + 1) AS bigint) NOT IN (SELECT id FROM user_sessions)) + views = [row["id"] for row in conn.execute("SELECT id FROM user_sessions WHERE acting_user_id IS NOT NULL")] + with sqlite_db.connect() as lite: + lite.execute( + """ + DELETE FROM sessions WHERE code_id IN ( + SELECT id FROM codes + WHERE minted_via LIKE 'staff_view:%' + AND CAST(substr(minted_via, length('staff_view:') + 1) AS INTEGER) + NOT IN (SELECT value FROM json_each(?)) + ) + """, + (json.dumps(views),), ) - """ - ) - conn.execute("DELETE FROM users WHERE placeholder = 1 AND id NOT IN (SELECT user_id FROM user_sessions)") + lite.commit() + conn.execute("DELETE FROM users WHERE placeholder AND id NOT IN (SELECT user_id FROM user_sessions)") def _utcnow() -> datetime: @@ -53,6 +100,16 @@ def _parse_iso(value: str) -> datetime: return datetime.strptime(value, "%Y-%m-%dT%H:%M:%SZ").replace(tzinfo=timezone.utc) +def api_time(value: datetime | None) -> str | None: + """A stored time as the site's API form, `YYYY-MM-DDTHH:MM:SSZ`; None stays None.""" + return None if value is None else _iso(value) + + +def _as_time(value: datetime | str | None) -> datetime | None: + """A time to store: a datetime, or the API form a preview hands over.""" + return _parse_iso(value) if isinstance(value, str) else value + + def clean_return_to(value: str | None) -> str: """Only same-site paths: no scheme, host, protocol-relative or backslash forms.""" path = (value or "").strip() @@ -69,15 +126,15 @@ def clean_return_to(value: str | None) -> str: def start_sign_in(config: AuthConfig, return_to: str | None) -> tuple[str, str]: """Return (state, Discord authorise URL). The caller stores state in a cookie.""" + require_accounts() state = secrets.token_urlsafe(32) now = _utcnow() - with connect() as conn: - conn.execute("DELETE FROM discord_oauth_states WHERE expires_at <= ?", (_iso(now),)) + with database.connect() as conn: + conn.execute("DELETE FROM discord_oauth_states WHERE expires_at <= %s", (now,)) conn.execute( - "INSERT INTO discord_oauth_states (state_hash, return_to, expires_at) VALUES (?, ?, ?)", - (hash_secret(state), clean_return_to(return_to), _iso(now + STATE_TTL)), + "INSERT INTO discord_oauth_states (state_hash, return_to, expires_at) VALUES (%s, %s, %s)", + (hash_secret(state), clean_return_to(return_to), now + STATE_TTL), ) - conn.commit() query = urlencode([ ("response_type", "code"), ("client_id", config.client_id), @@ -93,16 +150,14 @@ def consume_state(state: str | None) -> str | None: """Spend a state once; return its saved path, or None if unknown or expired.""" if not state or len(state) > _TOKEN_MAX: return None - digest = hash_secret(state) - with connect() as conn: - conn.execute("BEGIN IMMEDIATE") + require_accounts() + # One DELETE both reads and spends the state, so two callbacks cannot both get it. + with database.connect() as conn: row = conn.execute( - "SELECT return_to, expires_at FROM discord_oauth_states WHERE state_hash = ?", - (digest,), + "DELETE FROM discord_oauth_states WHERE state_hash = %s RETURNING return_to, expires_at", + (hash_secret(state),), ).fetchone() - conn.execute("DELETE FROM discord_oauth_states WHERE state_hash = ?", (digest,)) - conn.commit() - if row is None or _parse_iso(row["expires_at"]) <= _utcnow(): + if row is None or row["expires_at"] <= _utcnow(): return None return row["return_to"] @@ -119,51 +174,47 @@ def sign_in( A preview passes the membership check and role dev already holds for the user. """ + require_accounts() token = secrets.token_urlsafe(32) now = _utcnow() - stamp = _iso(now) - with connect() as conn: - conn.execute("BEGIN IMMEDIATE") - conn.execute( + with database.transaction(lock=PLACEHOLDERS_LOCK) as conn: + user_id = conn.execute( """ INSERT INTO users ( discord_user_id, discord_username, discord_global_name, discord_avatar, created_at, updated_at, last_login_at - ) VALUES (?, ?, ?, ?, ?, ?, ?) - ON CONFLICT(discord_user_id) DO UPDATE SET + ) VALUES (%s, %s, %s, %s, %s, %s, %s) + ON CONFLICT (discord_user_id) DO UPDATE SET -- A placeholder made for a staff view becomes their account from now. - created_at = CASE WHEN users.placeholder = 1 THEN excluded.created_at ELSE users.created_at END, - placeholder = 0, + created_at = CASE WHEN users.placeholder THEN excluded.created_at ELSE users.created_at END, + placeholder = false, discord_username = excluded.discord_username, discord_global_name = excluded.discord_global_name, discord_avatar = excluded.discord_avatar, updated_at = excluded.updated_at, last_login_at = excluded.last_login_at + RETURNING id """, ( identity["discord_user_id"], identity["discord_username"], identity["discord_global_name"], - identity["discord_avatar"], stamp, stamp, stamp, + identity["discord_avatar"], now, now, now, ), - ) - 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("UPDATE users SET role = %s WHERE id = %s", (role, user_id)) # Views that ran out are left for src/auth/view_as.py to end and audit. - conn.execute("DELETE FROM user_sessions WHERE expires_at <= ? AND acting_user_id IS NULL", (stamp,)) + conn.execute("DELETE FROM user_sessions WHERE expires_at <= %s AND acting_user_id IS NULL", (now,)) drop_view_leftovers(conn) conn.execute( """ INSERT INTO user_sessions (token_hash, user_id, guild_member, guild_checked_at, created_at, expires_at) - VALUES (?, ?, ?, ?, ?, ?) + VALUES (%s, %s, %s, %s, %s, %s) """, ( - hash_secret(token), user_id, 1 if guild_member else 0, guild_checked_at or stamp, stamp, - _iso(now + SESSION_TTL), + hash_secret(token), user_id, bool(guild_member), _as_time(guild_checked_at) or now, now, + now + SESSION_TTL, ), ) - conn.commit() if identity["discord_username"]: # Keep the staff-facing name on an existing link current. remember_discord_usernames( @@ -179,29 +230,49 @@ def sign_in( def session_user(token: str | None) -> dict | None: - with connect() as conn: + """The signed-in user for a session token, or None. Without a token this never touches the + database, so signed-out pages work with no accounts database; with one it needs it (503).""" + if not token or len(token) > _TOKEN_MAX: + return None + require_accounts() + with database.connect() as conn: return session_user_in(conn, token) -def session_user_in(conn, token: str | None) -> dict | None: - """Session user on the caller's connection, so staff checks can run inside a write.""" +def session_user_in(conn, token: str | None, *, lock: bool = False) -> dict | None: + """Session user on the caller's connection, so staff checks can run inside a write. + + Staff writes pass `lock=True`: the session and user rows are held FOR SHARE until + the caller commits, so a demotion or revocation either waits for the write or + lands first and is seen by it. Ordinary reads take no lock. + """ if not token or len(token) > _TOKEN_MAX: return None + if isinstance(conn, sqlite3.Connection): + # PG-MIGRATION: until the LuckPerms area converts, luckperms/changes.py passes its + # SQLite connection. Sessions are in Postgres now, so read them there; nothing holds + # them for the caller's write meanwhile, so a lock cannot be honoured. + if lock: + raise TypeError("session_user_in(lock=True) needs a Postgres connection") + return session_user(token) row = conn.execute( - """ + f""" SELECT s.id AS session_id, s.expires_at, s.guild_member, s.guild_checked_at, s.acting_user_id, COALESCE(a.discord_username, a.discord_global_name) AS acting_name, u.id AS user_id, u.discord_user_id, u.discord_username, u.discord_global_name, u.discord_avatar, u.created_at, u.role FROM user_sessions s JOIN users u ON u.id = s.user_id LEFT JOIN users a ON a.id = s.acting_user_id - WHERE s.token_hash = ? + WHERE s.token_hash = %s + {"FOR SHARE OF s, u" if lock else ""} """, (hash_secret(token),), ).fetchone() - if row is None or _parse_iso(row["expires_at"]) <= _utcnow(): + if row is None or row["expires_at"] <= _utcnow(): return None user = dict(row) + for key in ("expires_at", "guild_checked_at", "created_at"): + user[key] = api_time(user[key]) if user["acting_user_id"] is not None: # Someone viewing the site as this user holds no staff rights, whatever their role. user["role"] = "player" @@ -210,23 +281,24 @@ def session_user_in(conn, token: str | None) -> dict | None: def guild_state(session_id: int) -> dict | None: """The session's stored membership check, as another request may have just updated it.""" - with connect() as conn: + require_accounts() + with database.connect() as conn: row = conn.execute( - "SELECT guild_member, guild_checked_at FROM user_sessions WHERE id = ?", (session_id,) + "SELECT guild_member, guild_checked_at FROM user_sessions WHERE id = %s", (session_id,) ).fetchone() - return dict(row) if row else None + return {"guild_member": row["guild_member"], "guild_checked_at": api_time(row["guild_checked_at"])} if row else None def record_guild_check(session_id: int, member: bool) -> str: """Store a membership check made after sign-in; returns its time.""" - stamp = _iso(_utcnow()) - with connect() as conn: + require_accounts() + now = _utcnow() + with database.connect() as conn: conn.execute( - "UPDATE user_sessions SET guild_member = ?, guild_checked_at = ? WHERE id = ?", - (1 if member else 0, stamp, session_id), + "UPDATE user_sessions SET guild_member = %s, guild_checked_at = %s WHERE id = %s", + (bool(member), now, session_id), ) - conn.commit() - return stamp + return _iso(now) def guild_check_recent(user: dict) -> bool: @@ -240,11 +312,11 @@ def guild_check_fresh(user: dict) -> bool: def revoke_session(token: str | None) -> None: - if not token or len(token) > _TOKEN_MAX: + if not token or len(token) > _TOKEN_MAX or not accounts_configured(): + # Without an accounts database there is no session to revoke; signing out still clears cookies. return - with connect() as conn: - conn.execute("DELETE FROM user_sessions WHERE token_hash = ?", (hash_secret(token),)) - conn.commit() + with database.connect() as conn: + conn.execute("DELETE FROM user_sessions WHERE token_hash = %s", (hash_secret(token),)) def avatar_url(user: dict) -> str: diff --git a/backend/src/auth/view_as.py b/backend/src/auth/view_as.py index 21e7738d..8b5cac08 100644 --- a/backend/src/auth/view_as.py +++ b/backend/src/auth/view_as.py @@ -7,16 +7,29 @@ """ from __future__ import annotations +import json import secrets +import sqlite3 from datetime import timedelta +from src import database +from src.skins import db as sqlite_db from src.skins.codes import hash_secret -from src.skins.db import connect from . import audit, roles from .admin import AdminError from .players import canonical_uuid -from .users import VIEW_PROFILE_PREFIX, _iso, _parse_iso, _utcnow, drop_view_leftovers, session_user_in +from .users import ( + PLACEHOLDERS_LOCK, + VIEW_PROFILE_PREFIX, + _iso, + _utcnow, + api_time, + drop_view_leftovers, + accounts_configured, + require_accounts, + session_user_in, +) VIEW_TTL = timedelta(minutes=30) _TOKEN_MAX = 256 @@ -28,44 +41,36 @@ def start(token: str | None, player_uuid: str) -> dict: A player who has never signed in gets a placeholder account for the view (users.placeholder), deleted once its views end. """ + require_accounts() actor = target = None key = canonical_uuid(player_uuid) try: - with connect() as conn: - conn.execute("BEGIN IMMEDIATE") - try: - actor = session_user_in(conn, token) - if actor is None: - raise AdminError(401, "not_signed_in", "denied") - if actor["acting_user_id"] is not None: - raise AdminError(409, "already_viewing", "conflict") - if not roles.can(actor["role"], "view_as_player"): - raise AdminError(403, "forbidden", "denied") - if key is None: - raise AdminError(400, "bad_uuid", "invalid") - link = conn.execute( - "SELECT discord_user_id, discord_username FROM discord_links WHERE lower(player_uuid) = ?", - (key,), - ).fetchone() - # The site knows a player through their Discord link; without one there is nothing to view. - if link is None: - raise AdminError(404, "not_linked", "not_found") - if actor["discord_user_id"] == link["discord_user_id"]: - raise AdminError(403, "cannot_act_on_self", "denied") - target = _account_for(conn, link["discord_user_id"]) - made = target is None - if made: - target = _make_placeholder(conn, link) - # Players only: staff accounts are never viewed. - if target["role"] != "player": - raise AdminError(403, "target_is_staff", "denied") - view_token, expires_at = _open(conn, actor, target) - audit.record(conn, actor=actor, action="account.view_as.start", outcome="ok", target=target, - detail={"expires_at": expires_at, "player_uuid": key, "placeholder": made}) - conn.commit() - except BaseException: - conn.rollback() - raise + with database.transaction(lock=PLACEHOLDERS_LOCK) as conn: + actor = session_user_in(conn, token, lock=True) + if actor is None: + raise AdminError(401, "not_signed_in", "denied") + if actor["acting_user_id"] is not None: + raise AdminError(409, "already_viewing", "conflict") + if not roles.can(actor["role"], "view_as_player"): + raise AdminError(403, "forbidden", "denied") + if key is None: + raise AdminError(400, "bad_uuid", "invalid") + link = _link_for_player(key) + # The site knows a player through their Discord link; without one there is nothing to view. + if link is None: + raise AdminError(404, "not_linked", "not_found") + if actor["discord_user_id"] == link["discord_user_id"]: + raise AdminError(403, "cannot_act_on_self", "denied") + target = _account_for(conn, link["discord_user_id"]) + made = target is None + if made: + target = _make_placeholder(conn, link) + # Players only: staff accounts are never viewed. + if target["role"] != "player": + raise AdminError(403, "target_is_staff", "denied") + view_token, expires_at = _open(conn, actor, target) + audit.record(conn, actor=actor, action="account.view_as.start", outcome="ok", target=target, + detail={"expires_at": expires_at, "player_uuid": key, "placeholder": made}) except AdminError as exc: if actor is not None: audit.record_refusal(actor=actor, action="account.view_as.start", outcome=exc.outcome, target=target, @@ -74,53 +79,64 @@ def start(token: str | None, player_uuid: str) -> dict: return {"token": view_token, "expires_at": expires_at, "staff_expires_at": actor["expires_at"]} +def _link_for_player(key: str) -> dict | None: + # PG-MIGRATION: until the Linking area converts, discord_links is in SQLite. Its PR + # reads it on the view's connection. + with sqlite_db.connect() as lite: + row = lite.execute( + "SELECT discord_user_id, discord_username FROM discord_links WHERE lower(player_uuid) = ?", (key,) + ).fetchone() + return dict(row) if row else None + + def _account_for(conn, discord_user_id: str) -> dict | None: row = conn.execute( "SELECT id AS user_id, discord_user_id, discord_username, discord_global_name, role " - "FROM users WHERE discord_user_id = ?", + "FROM users WHERE discord_user_id = %s", (discord_user_id,), ).fetchone() return dict(row) if row else None def _make_placeholder(conn, link) -> dict: - stamp = _iso(_utcnow()) + now = _utcnow() + # A sign-in may have made the account meanwhile: keep theirs. conn.execute( """ INSERT INTO users (discord_user_id, discord_username, created_at, updated_at, last_login_at, placeholder) - VALUES (?, ?, ?, ?, ?, 1) + VALUES (%s, %s, %s, %s, %s, true) + ON CONFLICT (discord_user_id) DO NOTHING """, - (link["discord_user_id"], link["discord_username"], stamp, stamp, stamp), + (link["discord_user_id"], link["discord_username"], now, now, now), ) return _account_for(conn, link["discord_user_id"]) def _open(conn, actor: dict, target: dict) -> tuple[str, str]: now = _utcnow() - stamp = _iso(now) - expires_at = _iso(now + VIEW_TTL) + expires = now + VIEW_TTL # The player's own last membership check, so the Account page shows what they would see. last = conn.execute( "SELECT guild_member, guild_checked_at FROM user_sessions " - "WHERE user_id = ? AND acting_user_id IS NULL ORDER BY id DESC LIMIT 1", + "WHERE user_id = %s AND acting_user_id IS NULL ORDER BY id DESC LIMIT 1", (target["user_id"],), ).fetchone() view_token = secrets.token_urlsafe(32) # users.sign_in skips views that ran out; end them here so each is audited. - _end_where(conn, "s.acting_user_id IS NOT NULL AND s.expires_at <= ?", (stamp,)) - conn.execute("DELETE FROM user_sessions WHERE expires_at <= ?", (stamp,)) + _end_where(conn, "s.acting_user_id IS NOT NULL AND s.expires_at <= %s", (now,)) + conn.execute("DELETE FROM user_sessions WHERE expires_at <= %s", (now,)) conn.execute( """ INSERT INTO user_sessions ( token_hash, user_id, guild_member, guild_checked_at, created_at, expires_at, acting_user_id - ) VALUES (?, ?, ?, ?, ?, ?, ?) + ) VALUES (%s, %s, %s, %s, %s, %s, %s) """, ( - hash_secret(view_token), target["user_id"], last["guild_member"] if last else 0, - last["guild_checked_at"] if last else None, stamp, expires_at, actor["user_id"], + hash_secret(view_token), target["user_id"], last["guild_member"] if last else False, + last["guild_checked_at"] if last else None, now, expires, actor["user_id"], ), ) - return view_token, expires_at + return view_token, _iso(expires) def stop(view_token: str | None, staff_token: str | None) -> dict: @@ -130,25 +146,22 @@ def stop(view_token: str | None, staff_token: str | None) -> dict: the admin who opened this view. The browser drops a view's cookie once it runs out, so without one this ends the admin's lapsed views instead. """ - with connect() as conn: - conn.execute("BEGIN IMMEDIATE") - try: - ended = _end_where(conn, "s.token_hash = ?", (hash_secret(view_token),)) if _valid(view_token) else [] - view = ended[0] if ended else None - staff = session_user_in(conn, staff_token) if _valid(staff_token) else None - if staff is not None and ( - staff["acting_user_id"] is not None - or (view is not None and staff["user_id"] != view["acting_user_id"]) - ): - staff = None - if staff is not None and view is None: - lapsed = _end_where(conn, "s.acting_user_id = ? AND s.expires_at <= ?", - (staff["user_id"], _iso(_utcnow()))) - view = lapsed[-1] if lapsed else None - conn.commit() - except BaseException: - conn.rollback() - raise + if not (_valid(view_token) or _valid(staff_token)): + return {"ended": False, "restored": False, "staff_expires_at": None, "player_uuid": None} + require_accounts() + with database.transaction(lock=PLACEHOLDERS_LOCK) as conn: + ended = _end_where(conn, "s.token_hash = %s", (hash_secret(view_token),)) if _valid(view_token) else [] + view = ended[0] if ended else None + staff = session_user_in(conn, staff_token) if _valid(staff_token) else None + if staff is not None and ( + staff["acting_user_id"] is not None + or (view is not None and staff["user_id"] != view["acting_user_id"]) + ): + staff = None + if staff is not None and view is None: + lapsed = _end_where(conn, "s.acting_user_id = %s AND s.expires_at <= %s", + (staff["user_id"], _utcnow())) + view = lapsed[-1] if lapsed else None return { "ended": view is not None, "restored": staff is not None, @@ -162,46 +175,62 @@ def _valid(token: str | None) -> bool: def _end_where(conn, where: str, params: tuple) -> list[dict]: - """Delete the matching views and their Profile sessions, auditing each; oldest first.""" + """Delete the matching views and their Profile sessions, auditing each; oldest first. + + The views are locked first, so two requests ending the same view end and audit it once. + The caller's transaction opened with the placeholders lock. + """ rows = conn.execute( f""" SELECT s.id, s.acting_user_id, s.expires_at, u.id AS user_id, u.discord_user_id, - u.discord_username, u.discord_global_name, dl.player_uuid + u.discord_username, u.discord_global_name FROM user_sessions s JOIN users u ON u.id = s.user_id - LEFT JOIN discord_links dl ON dl.discord_user_id = u.discord_user_id WHERE s.acting_user_id IS NOT NULL AND {where} ORDER BY s.id + FOR UPDATE OF s """, params, ).fetchall() now = _utcnow() views = [dict(row) for row in rows] + players = _players_for([view["discord_user_id"] for view in views]) for view in views: - conn.execute("DELETE FROM user_sessions WHERE id = ?", (view["id"],)) + view["player_uuid"] = players.get(view["discord_user_id"]) + conn.execute("DELETE FROM user_sessions WHERE id = %s", (view["id"],)) actor = conn.execute( "SELECT id AS user_id, discord_user_id, discord_username, discord_global_name, role " - "FROM users WHERE id = ?", + "FROM users WHERE id = %s", (view["acting_user_id"],), ).fetchone() audit.record(conn, actor=dict(actor) if actor else None, action="account.view_as.stop", outcome="ok", - target=view, detail={"expired": _parse_iso(view["expires_at"]) <= now}) + target=view, detail={"expired": view["expires_at"] <= now}) + view["expires_at"] = api_time(view["expires_at"]) if views: drop_view_leftovers(conn) return views +def _players_for(discord_ids: list[str]) -> dict[str, str]: + """The Minecraft UUID linked to each Discord account.""" + if not discord_ids: + return {} + # PG-MIGRATION: until the Linking area converts, discord_links is in SQLite. Its PR puts + # back the join: LEFT JOIN discord_links dl ON dl.discord_user_id = u.discord_user_id. + with sqlite_db.connect() as lite: + rows = lite.execute( + "SELECT discord_user_id, player_uuid FROM discord_links " + "WHERE discord_user_id IN (SELECT value FROM json_each(?))", + (json.dumps(discord_ids),), + ).fetchall() + return {row["discord_user_id"]: row["player_uuid"] for row in rows} + + def end(token: str | None) -> None: """End the view this session token belongs to, if it is one.""" - if not _valid(token): + if not _valid(token) or not accounts_configured(): return - with connect() as conn: - conn.execute("BEGIN IMMEDIATE") - try: - _end_where(conn, "s.token_hash = ?", (hash_secret(token),)) - conn.commit() - except BaseException: - conn.rollback() - raise + with database.transaction(lock=PLACEHOLDERS_LOCK) as conn: + _end_where(conn, "s.token_hash = %s", (hash_secret(token),)) def end_lapsed(staff_token: str | None) -> None: @@ -209,18 +238,12 @@ def end_lapsed(staff_token: str | None) -> None: For when the browser has already dropped the view's cookie. """ - if not _valid(staff_token): + if not _valid(staff_token) or not accounts_configured(): return - with connect() as conn: - conn.execute("BEGIN IMMEDIATE") - try: - staff = session_user_in(conn, staff_token) - if staff is not None and staff["acting_user_id"] is None: - _end_where(conn, "s.acting_user_id = ? AND s.expires_at <= ?", (staff["user_id"], _iso(_utcnow()))) - conn.commit() - except BaseException: - conn.rollback() - raise + with database.transaction(lock=PLACEHOLDERS_LOCK) as conn: + staff = session_user_in(conn, staff_token, lock=True) + if staff is not None and staff["acting_user_id"] is None: + _end_where(conn, "s.acting_user_id = %s AND s.expires_at <= %s", (staff["user_id"], _utcnow())) def audit_refused_start(cookie_tokens: list[str], player_uuid: str) -> None: @@ -228,10 +251,10 @@ def audit_refused_start(cookie_tokens: list[str], player_uuid: str) -> None: for token in cookie_tokens: if not _valid(token): continue - with connect() as conn: + with database.connect() as conn: row = conn.execute( "SELECT a.id AS user_id, a.discord_user_id, a.discord_username, a.discord_global_name, a.role " - "FROM user_sessions s JOIN users a ON a.id = s.acting_user_id WHERE s.token_hash = ?", + "FROM user_sessions s JOIN users a ON a.id = s.acting_user_id WHERE s.token_hash = %s", (hash_secret(token),), ).fetchone() if row is not None: @@ -248,31 +271,45 @@ def profile_minted_via(user: dict) -> str | None: def view_still_open(session_id: int): - """A check, run inside a write, that this view has not ended or run out.""" + """A check, run inside a write, that this view has not ended or run out. + + On a Postgres connection the view's row is held FOR SHARE until that write + commits, so ending the view waits for it or is seen by it. + """ def check(conn) -> bool: + if isinstance(conn, sqlite3.Connection): + # PG-MIGRATION: until the Skins area converts, start_linked_profile_session passes its + # SQLite connection. Views are in Postgres now, so check there. Nothing holds the view + # open for the caller's write meanwhile: a narrow window between this check and the + # caller's commit is open again until the Skins PR checks on its own connection. + with database.connect() as pg: + return check(pg) return conn.execute( - "SELECT 1 FROM user_sessions WHERE id = ? AND acting_user_id IS NOT NULL AND expires_at > ?", - (session_id, _iso(_utcnow())), + "SELECT 1 FROM user_sessions WHERE id = %s AND acting_user_id IS NOT NULL AND expires_at > %s " + "FOR SHARE", + (session_id, _utcnow()), ).fetchone() is not None return check def refuses_write(cookie_tokens: list[str], bearer_token: str | None) -> bool: """A view's session cookie, or a Profile session opened during a view, sent with a write.""" - with connect() as conn: - for token in cookie_tokens: - if not _valid(token): - continue - row = conn.execute( - "SELECT acting_user_id FROM user_sessions WHERE token_hash = ?", (hash_secret(token),) - ).fetchone() - if row is not None and row["acting_user_id"] is not None: + hashes = [hash_secret(token) for token in cookie_tokens if _valid(token)] + # Without an accounts database no view can exist. + if hashes and accounts_configured(): + with database.connect() as conn: + if conn.execute( + "SELECT 1 FROM user_sessions WHERE token_hash = ANY(%s) AND acting_user_id IS NOT NULL", (hashes,) + ).fetchone() is not None: return True - if _valid(bearer_token): - row = conn.execute( + if _valid(bearer_token): + # PG-MIGRATION: until the Skins area converts, Profile sessions and their codes are in + # SQLite. Its PR reads them on the connection above. + with sqlite_db.connect() as lite: + row = lite.execute( "SELECT c.minted_via FROM sessions s JOIN codes c ON c.id = s.code_id WHERE s.token_hash = ?", (hash_secret(bearer_token),), ).fetchone() - if row is not None and (row["minted_via"] or "").startswith(VIEW_PROFILE_PREFIX): - return True + if row is not None and (row["minted_via"] or "").startswith(VIEW_PROFILE_PREFIX): + return True return False diff --git a/backend/src/dbstudio/studio.py b/backend/src/dbstudio/studio.py index 19cf3ac8..ca160319 100644 --- a/backend/src/dbstudio/studio.py +++ b/backend/src/dbstudio/studio.py @@ -84,8 +84,12 @@ def json(self) -> dict: def sources() -> list[Source]: + # PG-MIGRATION: until the database studio area converts, "site" is province.db. Accounts and + # the audit log have moved to Postgres; their SQLite tables are stale leftovers. The studio's + # PR moves this source to Postgres under the province_studio role. found = [Source("site", "Site", "sqlite", True, - "province.db: accounts, characters, wardrobes, chronicle and ledger, audit log")] + "province.db: characters, wardrobes, skins, chronicle and ledger. Accounts and the " + "audit log are in Postgres; their tables here are stale")] config = coreprotect.CoreProtectConfig.from_env() if config.path: label = config.label or config.server.title() diff --git a/backend/src/dbstudio/test_studio.py b/backend/src/dbstudio/test_studio.py index 535e51ae..96871c9a 100644 --- a/backend/src/dbstudio/test_studio.py +++ b/backend/src/dbstudio/test_studio.py @@ -1,5 +1,4 @@ import contextlib -import json import os import sqlite3 from types import SimpleNamespace @@ -43,29 +42,41 @@ def test_only_root_can_use_the_studio(app, env): # noqa: F811 c = client(app, account(env, "rory", "root")[1]) overview = c.get("/admin/database") assert overview.status_code == 200 and overview.headers["cache-control"] == "no-store" - [site] = overview.json()["databases"] + # The accounts tests' Postgres (DATABASE_URL) is listed too. + site = next(d for d in overview.json()["databases"] if d["id"] == "site") assert site["id"] == "site" and site["writable"] and site["available"] and site["tables"] > 10 +def link(env, name): + # The studio's site source is still province.db; accounts are in Postgres. + with env.connect() as conn: + conn.execute("INSERT INTO discord_links (player_uuid, discord_user_id, minecraft_name, linked_at) " + "VALUES (?, ?, ?, '2026-10-01T00:00:00Z')", (f"uuid-{name}", f"id-{name}", name)) + conn.commit() + + def test_reads_cannot_write_and_are_audited(app, env): # noqa: F811 c = client(app, account(env, "rory", "root")[1]) - result = query(c, "SELECT id, role FROM users ORDER BY id") + link(env, "RoryMC") + result = query(c, "SELECT player_uuid, minecraft_name FROM discord_links ORDER BY player_uuid") assert result.status_code == 200 body = result.json() - assert body["columns"] == ["id", "role"] and body["rows"][0][1] == "root" and body["changes"] is None - refused = query(c, "UPDATE users SET role = 'player'") + assert body["columns"] == ["player_uuid", "minecraft_name"] and body["rows"][0][1] == "RoryMC" + assert body["changes"] is None + refused = query(c, "UPDATE discord_links SET minecraft_name = 'x'") assert refused.status_code == 400 and "readonly" in refused.json()["detail"] rows = [r for r in audit_rows(env) if r["action"].startswith("database.")] assert [(r["action"], r["outcome"]) for r in rows] == [("database.read", "ok"), ("database.read", "failed")] - assert json.loads(rows[0]["detail_json"])["sql"] == "SELECT id, role FROM users ORDER BY id" + assert rows[0]["detail_json"]["sql"] == "SELECT player_uuid, minecraft_name FROM discord_links ORDER BY player_uuid" assert c.get("/admin/database").json()["databases"][0]["queries"] == 2 def test_root_edits_are_audited_but_the_audit_log_is_protected(app, env): # noqa: F811 c = client(app, account(env, "rory", "root")[1]) - edited = query(c, "UPDATE users SET discord_global_name = 'Renamed' WHERE discord_username = 'rory'", write=True) + link(env, "RoryMC") + edited = query(c, "UPDATE discord_links SET minecraft_name = 'Renamed' WHERE minecraft_name = 'RoryMC'", write=True) assert edited.status_code == 200 and edited.json()["changes"] == 1 - assert query(c, "SELECT discord_global_name FROM users").json()["rows"] == [["Renamed"]] + assert query(c, "SELECT minecraft_name FROM discord_links").json()["rows"] == [["Renamed"]] for sql in ("DELETE FROM admin_audit", "UPDATE admin_audit SET outcome = 'x'", "DROP TABLE admin_audit", "DROP TRIGGER IF EXISTS admin_audit_no_update"): refused = query(c, sql, write=True) @@ -73,7 +84,7 @@ def test_root_edits_are_audited_but_the_audit_log_is_protected(app, env): # noq written = [r for r in audit_rows(env) if r["action"] == "database.write"] # Each edit is logged as requested before it runs, then with its outcome. assert [r["outcome"] for r in written] == ["requested", "ok"] + ["requested", "failed"] * 4 - assert "Renamed" in json.loads(written[0]["detail_json"])["sql"] + assert "Renamed" in written[0]["detail_json"]["sql"] def test_writes_need_the_sites_origin(app, env): # noqa: F811 @@ -91,7 +102,7 @@ def test_attach_is_refused(app, env, tmp_path): # noqa: F811 def test_secrets_are_masked_and_values_travel_safely(app, env): # noqa: F811 c = client(app, account(env, "rory", "root")[1]) - body = query(c, "SELECT token_hash, 9007199254740993 AS big, x'00ff' AS raw, 1.5 AS f FROM user_sessions").json() + body = query(c, "SELECT 'abc' AS token_hash, 9007199254740993 AS big, x'00ff' AS raw, 1.5 AS f").json() assert body["rows"][0] == [studio.MASK, "9007199254740993", {"blob": 2, "hex": "00ff"}, 1.5] @@ -148,7 +159,7 @@ def test_coreprotect_queries_have_a_short_budget(app, env, coreprotect_db, monke def test_unknown_database(app, env): # noqa: F811 c = client(app, account(env, "rory", "root")[1]) - assert c.get("/admin/database/postgres/schema").status_code == 404 + assert c.get("/admin/database/nowhere/schema").status_code == 404 assert query(c, "SELECT 1", "nope").status_code == 404 @@ -291,10 +302,10 @@ def test_semicolons_inside_strings_and_comments_are_fine(sql): def test_sqlite_edits_cannot_plant_triggers_or_unlock_the_schema(app, env): # noqa: F811 c = client(app, account(env, "rory", "root")[1]) for sql in ( - # Sol: a stored trigger would forge audit rows when the site later updates users. - "CREATE TRIGGER forge AFTER UPDATE ON users BEGIN INSERT INTO admin_audit (created_at, actor_type, action, " - "outcome) VALUES ('x', 'system', 'forged', 'ok'); END", - "CREATE TEMP TRIGGER t AFTER UPDATE ON users BEGIN SELECT 1; END", + # Sol: a stored trigger would forge audit rows when the site later updates a link. + "CREATE TRIGGER forge AFTER UPDATE ON discord_links BEGIN INSERT INTO admin_audit (created_at, actor_type, " + "action, outcome) VALUES ('x', 'system', 'forged', 'ok'); END", + "CREATE TEMP TRIGGER t AFTER UPDATE ON discord_links BEGIN SELECT 1; END", "PRAGMA writable_schema = ON", "PRAGMA foreign_keys = OFF", "VACUUM INTO '/tmp/copy.db'", @@ -306,7 +317,7 @@ def test_sqlite_edits_cannot_plant_triggers_or_unlock_the_schema(app, env): # n with env.connect() as conn: assert conn.execute("SELECT count(*) FROM sqlite_master WHERE type = 'trigger' AND name = 'forge'").fetchone()[0] == 0 # Schema pragmas that only describe still work in reads. - assert query(c, "PRAGMA table_info(users)").status_code == 200 + assert query(c, "PRAGMA table_info(discord_links)").status_code == 200 def test_edits_cannot_touch_the_autoincrement_counters(app, env): # noqa: F811 @@ -325,7 +336,8 @@ def test_edits_cannot_index_the_audit_log(app, env): # noqa: F811 # Sol: an index whose expression overflows would make every later audit insert fail. refused = query(c, "CREATE INDEX block_audit ON admin_audit(abs(-9223372036854775808)) WHERE id > 0", write=True) assert refused.status_code == 403 - assert query(c, "UPDATE users SET discord_global_name = 'Still logged'", write=True).status_code == 200 + link(env, "RoryMC") + assert query(c, "UPDATE discord_links SET minecraft_name = 'Still logged'", write=True).status_code == 200 assert [r["outcome"] for r in audit_rows(env) if r["action"] == "database.write"][-2:] == ["requested", "ok"] @@ -333,7 +345,7 @@ def test_schema_and_list_loads_are_audited(app, env): # noqa: F811 c = client(app, account(env, "rory", "root")[1]) assert c.get("/admin/database").status_code == 200 assert c.get("/admin/database/site/schema").status_code == 200 - assert c.get("/admin/database/postgres/schema").status_code == 404 + assert c.get("/admin/database/nowhere/schema").status_code == 404 rows = [(r["action"], r["outcome"]) for r in audit_rows(env) if r["action"].startswith("database.")] assert rows == [("database.list", "ok"), ("database.schema", "ok"), ("database.schema", "failed")] # Only reads and edits count as queries. diff --git a/backend/src/diagnostic/test_routes.py b/backend/src/diagnostic/test_routes.py index bcb6e479..2a4c5811 100644 --- a/backend/src/diagnostic/test_routes.py +++ b/backend/src/diagnostic/test_routes.py @@ -121,7 +121,7 @@ def test_speed_test_is_random_and_rate_limited(app): REPORT = {"findings": [{"level": "info", "title": "Built-in graphics"}, {"level": "fail", "title": "The resource pack download failed"}]} -def test_player_sends_a_report_and_staff_read_it(app, database): +def test_player_sends_a_report_and_staff_read_it(app, database, postgres): res = client(app).post("/diagnostic/reports", json={"report": REPORT, "minecraft_name": "Nijika_"}, headers={"CF-IPCountry": "PH"}) code = res.json()["code"] assert len(code) == reports.CODE_LENGTH @@ -134,7 +134,7 @@ def test_player_sends_a_report_and_staff_read_it(app, database): assert one["report"] == REPORT -def test_signed_in_player_is_named_on_the_report(app, database): +def test_signed_in_player_is_named_on_the_report(app, database, postgres): user_id, token = account(database, "player") with mock.patch.object(diagnostic_routes, "get_link_for_discord_id", return_value={"minecraft_name": "LinkedName"}): code = client(app, token).post("/diagnostic/reports", json={"report": REPORT, "minecraft_name": "Typed"}).json()["code"] @@ -142,7 +142,7 @@ def test_signed_in_player_is_named_on_the_report(app, database): assert saved["user_id"] == user_id and saved["minecraft_name"] == "LinkedName" -def test_reports_are_staff_only(app, database): +def test_reports_are_staff_only(app, database, postgres): assert client(app).get("/diagnostic/reports").status_code == 401 _, player = account(database, "player") assert client(app, player).get("/diagnostic/reports/ABCDEF").json() == {"detail": "forbidden"} diff --git a/backend/src/luckperms/changes.py b/backend/src/luckperms/changes.py index 7b851346..b97279db 100644 --- a/backend/src/luckperms/changes.py +++ b/backend/src/luckperms/changes.py @@ -16,6 +16,7 @@ import time from datetime import datetime, timezone +from src import database from src.auth import audit, roles from src.auth.admin import AdminError, clean_reason from src.auth.players import canonical_uuid @@ -244,11 +245,15 @@ def _linked_uuid(conn, discord_user_id: str | None) -> str | None: def _target_account_role(conn, uuid: str) -> str | None: - row = conn.execute( - "SELECT u.role FROM discord_links dl JOIN users u ON u.discord_user_id = dl.discord_user_id AND u.placeholder = 0 " - "WHERE lower(dl.player_uuid) = ?", - (uuid,), - ).fetchone() + link = conn.execute("SELECT discord_user_id FROM discord_links WHERE lower(player_uuid) = ?", (uuid,)).fetchone() + if link is None: + return None + # PG-MIGRATION: until the LuckPerms area converts, `conn` is SQLite and users are in + # Postgres. Its PR puts back the join: JOIN users u ON u.discord_user_id = dl.discord_user_id + # AND NOT u.placeholder. + with database.connect() as pg: + row = pg.execute("SELECT role FROM users WHERE discord_user_id = %s AND NOT placeholder", + (link["discord_user_id"],)).fetchone() return row["role"] if row else None @@ -443,8 +448,11 @@ def _actor_name(row) -> str: def _current_actor(conn, row) -> dict | None: """The actor's rights again at collection time: a demotion since queueing cancels the change.""" - actor = conn.execute("SELECT id AS user_id, discord_user_id, role FROM users WHERE id = ?", - (row["actor_user_id"],)).fetchone() + # PG-MIGRATION: until the LuckPerms area converts, `conn` is SQLite and users are in + # Postgres. Its PR reads the actor on `conn` FOR SHARE. + with database.connect() as pg: + actor = pg.execute("SELECT id AS user_id, discord_user_id, role FROM users WHERE id = %s", + (row["actor_user_id"],)).fetchone() if actor is None or not roles.can(actor["role"], "change_luckperms"): return None actor = dict(actor) diff --git a/backend/src/luckperms/test_luckperms.py b/backend/src/luckperms/test_luckperms.py index b75e2950..18a9ce34 100644 --- a/backend/src/luckperms/test_luckperms.py +++ b/backend/src/luckperms/test_luckperms.py @@ -1,4 +1,3 @@ -import json import time import pytest @@ -6,7 +5,7 @@ from fastapi.testclient import TestClient from src.api import admin_routes, auth_routes -from src.auth import users +from src.auth import testing, users from src.luckperms import changes, mirror, nodes, policy SITE = "https://www.tfminecraft.net" @@ -68,7 +67,7 @@ def snapshot(**overrides): @pytest.fixture -def env(database, monkeypatch): +def env(database, postgres, monkeypatch): monkeypatch.setenv("DISCORD_AUTH_ENABLED", "1") monkeypatch.setenv("DISCORD_CLIENT_ID", "client-id") monkeypatch.setenv("DISCORD_CLIENT_SECRET", "client-secret") @@ -97,8 +96,8 @@ def account(db, name, role="player", minecraft=None): "discord_avatar": None}, guild_member=True, ) + testing.set_role(discord_id, role) with db.connect() as conn: - conn.execute("UPDATE users SET role = ? WHERE discord_user_id = ?", (role, discord_id)) if minecraft: conn.execute( "INSERT INTO discord_links (player_uuid, discord_user_id, discord_username, minecraft_name, linked_at) " @@ -137,8 +136,7 @@ def submit(app, token, target_type, target, ops, reason="Event reward"): def audit_rows(db): - with db.connect() as conn: - return [dict(r) for r in conn.execute("SELECT * FROM admin_audit ORDER BY id")] + return testing.audit_rows() # -------------------- @@ -289,7 +287,7 @@ def test_admin_change_round_trip(app, bridge, staff, env): # And a snapshot built before the change cannot hide it again. assert bridge.put("/luckperms/plugin/snapshot", headers=PLUGIN, json=snapshot(revision=1500)).json()["stale"] rows = [r for r in audit_rows(env) if r["action"] == "luckperms.change"] - assert [(r["outcome"], json.loads(r["detail_json"]).get("error")) for r in rows] == [("ok", None), ("conflict", "change_pending")] + assert [(r["outcome"], r["detail_json"].get("error")) for r in rows] == [("ok", None), ("conflict", "change_pending")] @pytest.mark.parametrize("role,target,ops,status,code", [ @@ -388,9 +386,7 @@ def test_admins_cannot_touch_in_game_staff_even_unlinked(app, bridge, staff): def test_demotion_after_queueing_cancels_the_change(app, bridge, staff, env): change = submit(app, staff["admin"], "user", ALICE, [{"op": "add_node", "node": {"key": "tips.off"}}]).json() - with env.connect() as conn: - conn.execute("UPDATE users SET role = 'mod' WHERE discord_username = 'adam'") - conn.commit() + testing.execute("UPDATE users SET role = 'mod' WHERE discord_username = 'adam'") assert changes.fetch_for_bridge() == [] assert changes.get_change(change["id"])["error"] == "no_longer_allowed" @@ -452,9 +448,7 @@ def test_timeouts_settle_without_the_bridge(app, bridge, staff, monkeypatch): def test_guard_follows_the_actors_current_role(app, bridge, staff, env): submit(app, staff["root"], "user", ALICE, [{"op": "add_node", "node": {"key": "tips.off"}}]) - with env.connect() as conn: - conn.execute("UPDATE users SET role = 'admin' WHERE discord_username = 'rory'") - conn.commit() + testing.execute("UPDATE users SET role = 'admin' WHERE discord_username = 'rory'") sent = changes.fetch_for_bridge() assert len(sent) == 1 and "admin_groups" in sent[0]["guard"] diff --git a/backend/src/luckperms/views.py b/backend/src/luckperms/views.py index 5499dcd7..db69d21c 100644 --- a/backend/src/luckperms/views.py +++ b/backend/src/luckperms/views.py @@ -7,6 +7,7 @@ import time +from src import database from src.auth import roles from src.auth.admin import AdminError from src.auth.players import canonical_uuid @@ -118,14 +119,18 @@ def _pending(target_type: str, target: str) -> dict | None: def _account(conn, uuid: str) -> dict | None: row = conn.execute( - """ - SELECT dl.discord_user_id, dl.discord_username, u.id AS user_id, u.role - FROM discord_links dl LEFT JOIN users u ON u.discord_user_id = dl.discord_user_id AND u.placeholder = 0 - WHERE lower(dl.player_uuid) = ? - """, + "SELECT discord_user_id, discord_username FROM discord_links WHERE lower(player_uuid) = ?", (uuid,), ).fetchone() - return dict(row) if row else None + if row is None: + return None + # PG-MIGRATION: until the LuckPerms area converts, `conn` is SQLite and users are in + # Postgres. Its PR puts back the join: LEFT JOIN users u ON u.discord_user_id = + # dl.discord_user_id AND NOT u.placeholder. + with database.connect() as pg: + user = pg.execute("SELECT id AS user_id, role FROM users WHERE discord_user_id = %s AND NOT placeholder", + (row["discord_user_id"],)).fetchone() + return {**dict(row), **(user or {"user_id": None, "role": None})} def _track_moves(conn, uuid: str, tracks: list[dict], viewer: dict, allowed: bool, now: int) -> list[dict]: diff --git a/backend/src/rail/test_network.py b/backend/src/rail/test_network.py index 039e4bf5..9ce05b6b 100644 --- a/backend/src/rail/test_network.py +++ b/backend/src/rail/test_network.py @@ -133,7 +133,7 @@ def test_simplify_keeps_corners(): @pytest.fixture -def env(database, monkeypatch): +def env(database, postgres, monkeypatch): monkeypatch.setenv("DISCORD_AUTH_ENABLED", "1") monkeypatch.setenv("DISCORD_CLIENT_ID", "client-id") monkeypatch.setenv("DISCORD_CLIENT_SECRET", "client-secret") diff --git a/backend/src/servers/test_control.py b/backend/src/servers/test_control.py index 99a05285..9390ad94 100644 --- a/backend/src/servers/test_control.py +++ b/backend/src/servers/test_control.py @@ -7,6 +7,7 @@ from fastapi import FastAPI from src.servers import control, host, inventory +from src.auth import testing from src.auth.test_admin import account, client, audit_rows, ORIGIN @@ -197,7 +198,7 @@ def dispatch(self, body): @pytest.fixture -def app(database, monkeypatch): +def app(database, postgres, monkeypatch): from src.api import admin_routes, auth_routes monkeypatch.setenv("DISCORD_AUTH_ENABLED", "1") monkeypatch.setenv("DISCORD_CLIENT_ID", "client-id") @@ -234,14 +235,10 @@ def test_write_requires_admin_session_origin_reason_and_logs_actor(app, database def test_demoted_or_revoked_session_cannot_reach_helper(app, database, monkeypatch): monkeypatch.setattr(control, "request", lambda *_args, **_kwargs: pytest.fail("helper called")) user_id, token = account(database, "demoted", "admin") - with database.connect() as conn: - conn.execute("UPDATE users SET role='mod' WHERE id=?", (user_id,)) - conn.commit() + testing.execute("UPDATE users SET role='mod' WHERE id=%s", (user_id,)) body = {"id": "b" * 32, "reason": "testing role"} assert client(app, token).post("/admin/servers/dev/cancel", json=body, headers=ORIGIN).status_code == 403 - with database.connect() as conn: - conn.execute("DELETE FROM user_sessions WHERE user_id=?", (user_id,)) - conn.commit() + testing.execute("DELETE FROM user_sessions WHERE user_id=%s", (user_id,)) assert client(app, token).post("/admin/servers/dev/cancel", json=body, headers=ORIGIN).status_code == 401 @@ -418,7 +415,7 @@ def test_emergency_flag_is_strict_audited_and_forwarded(app, database, monkeypat body = {"id": "c"*32, "reason": "emergency plugin fix", "emergency_override": True} assert client(app, admin).post("/admin/servers/main/start", json=body, headers=ORIGIN).status_code == 200 assert calls[-1]["emergency_override"] is True and calls[-1]["backup_ref"] == "" - assert json.loads(audit_rows(database)[-1]["detail_json"])["emergency_override"] is True + assert audit_rows(database)[-1]["detail_json"]["emergency_override"] is True assert client(app, admin).post("/admin/servers/main/start", json={**body,"emergency_override":"false"}, headers=ORIGIN).status_code == 422 assert client(app, admin).post("/admin/servers/dev/start", json=body, headers=ORIGIN).status_code == 400 assert len(calls) == 1 @@ -635,7 +632,7 @@ def test_test_build_choice_is_start_only_and_forwarded(app, database, monkeypatc body = {"id": "d" * 32, "reason": "replace test builds", "replace_test_builds": ["Core"]} assert client(app, admin).post("/admin/servers/dev/start", json=body, headers=ORIGIN).status_code == 200 assert calls[-1]["replace_test_builds"] == ["Core"] - assert json.loads(audit_rows(database)[-1]["detail_json"])["replace_test_builds"] == ["Core"] + assert audit_rows(database)[-1]["detail_json"]["replace_test_builds"] == ["Core"] assert client(app, admin).post("/admin/servers/dev/start", json={**body, "replace_test_builds": []}, headers=ORIGIN).status_code == 200 assert "replace_test_builds" not in calls[-1] assert client(app, admin).post("/admin/servers/dev/cancel", json=body, headers=ORIGIN).status_code == 400 diff --git a/backend/src/servers/test_inventory.py b/backend/src/servers/test_inventory.py index 1ff875ed..9aa69ad7 100644 --- a/backend/src/servers/test_inventory.py +++ b/backend/src/servers/test_inventory.py @@ -469,7 +469,7 @@ def test_unreadable_jar_keeps_its_hash_without_a_version(tmp_path): assert jar.name is None and jar.version is None -def test_the_route_is_for_admins(database, monkeypatch, tmp_path): +def test_the_route_is_for_admins(database, postgres, monkeypatch, tmp_path): from fastapi import FastAPI from src.api import admin_routes, auth_routes diff --git a/backend/src/skins/discord_link.py b/backend/src/skins/discord_link.py index 2ab503b6..e4ad4bee 100644 --- a/backend/src/skins/discord_link.py +++ b/backend/src/skins/discord_link.py @@ -9,6 +9,7 @@ from .codes import generate_plaintext_code, hash_secret from .db import connect from .plugin_notices import enqueue_link_success, enqueue_plugin_notice +from src import database from src.text_validation import TextValidationError, assert_optional_display_name @@ -570,10 +571,16 @@ def record_guild_joined(discord_user_id: str) -> dict: def _cancel_microsoft_attempts(conn, discord_id: str) -> None: """Unlinking cancels Microsoft link attempts still open for that Discord account.""" - conn.execute( - "DELETE FROM microsoft_link_states WHERE user_id IN (SELECT id FROM users WHERE discord_user_id = ?)", - (discord_id,), - ) + # PG-MIGRATION: until the Linking area converts, users are in Postgres and these states in + # SQLite. Its PR puts back the subquery: user_id IN (SELECT id FROM users WHERE discord_user_id = %s). + pending = [row["user_id"] for row in conn.execute("SELECT DISTINCT user_id FROM microsoft_link_states")] + if not pending: + return + with database.connect() as pg: + user = pg.execute("SELECT id FROM users WHERE discord_user_id = %s AND id = ANY(%s)", + (discord_id, pending)).fetchone() + if user is not None: + conn.execute("DELETE FROM microsoft_link_states WHERE user_id = ?", (user["id"],)) def expire_due_graces() -> int: diff --git a/scripts/ps-preview.sh b/scripts/ps-preview.sh index 2d01c998..b4258c2b 100755 --- a/scripts/ps-preview.sh +++ b/scripts/ps-preview.sh @@ -97,6 +97,10 @@ create_data() { low_prio sqlite3 "$src/province.db" ".timeout 30000" ".backup '$tmp/data/province.db'" fi done + # PG-MIGRATION: host plumbing. Accounts (user_sessions, discord_oauth_states) live in + # Postgres on the postgres branch, so these two deletes hit stale SQLite tables and previews + # of that branch have no DATABASE_URL and cannot sign in. The host-plumbing PR gives each + # preview its own database and moves this scrub there. sqlite3 "$tmp/data/province.db" <<'SQL' .timeout 30000 DELETE FROM sessions;