diff --git a/api/main.py b/api/main.py index 114ab3acb..d1dbee35a 100644 --- a/api/main.py +++ b/api/main.py @@ -345,7 +345,17 @@ def get_optional_user( async def ensure_profile(conn: asyncpg.Connection, user_id: str, email: str | None = None) -> None: - """Create/update profile row, claim beta premium if slots remain, link pending invitations.""" + """Create/update profile row, claim beta premium if slots remain, link pending invitations. + + The beta grant must also fire on the ON CONFLICT branch, not only on a fresh + INSERT. Registration handlers in auth.py create the profiles row first (with + no premium/beta), so by the time ensure_profile runs (on /me/profile, + /me/results, /results) the row already exists and the INSERT branch never + wins. Without granting on conflict, the first ~500 beta users were silently + left on premium = false and sent to the paywall. The grant condition mirrors + the INSERT: a slot remains AND the row is not already premium or beta. We + never revoke an existing grant or a paid premium (OR with the current value), + and a paid user is never relabelled beta (the grant requires NOT premium).""" if email: await conn.execute( """ @@ -353,7 +363,14 @@ async def ensure_profile(conn: asyncpg.Connection, user_id: str, email: str | No SELECT $1, $2, (SELECT COUNT(*) < $3 FROM profiles WHERE is_beta = TRUE), (SELECT COUNT(*) < $3 FROM profiles WHERE is_beta = TRUE) - ON CONFLICT (id) DO UPDATE SET email = EXCLUDED.email + ON CONFLICT (id) DO UPDATE SET + email = EXCLUDED.email, + premium = profiles.premium OR ( + NOT profiles.premium AND NOT profiles.is_beta + AND (SELECT COUNT(*) < $3 FROM profiles WHERE is_beta = TRUE)), + is_beta = profiles.is_beta OR ( + NOT profiles.premium AND NOT profiles.is_beta + AND (SELECT COUNT(*) < $3 FROM profiles WHERE is_beta = TRUE)) """, user_id, email.lower(), BETA_TOTAL, ) @@ -372,7 +389,13 @@ async def ensure_profile(conn: asyncpg.Connection, user_id: str, email: str | No SELECT $1, (SELECT COUNT(*) < $2 FROM profiles WHERE is_beta = TRUE), (SELECT COUNT(*) < $2 FROM profiles WHERE is_beta = TRUE) - ON CONFLICT (id) DO NOTHING + ON CONFLICT (id) DO UPDATE SET + premium = profiles.premium OR ( + NOT profiles.premium AND NOT profiles.is_beta + AND (SELECT COUNT(*) < $2 FROM profiles WHERE is_beta = TRUE)), + is_beta = profiles.is_beta OR ( + NOT profiles.premium AND NOT profiles.is_beta + AND (SELECT COUNT(*) < $2 FROM profiles WHERE is_beta = TRUE)) """, user_id, BETA_TOTAL, ) diff --git a/api/tests/test_beta_grant.py b/api/tests/test_beta_grant.py new file mode 100644 index 000000000..3a5ceaeac --- /dev/null +++ b/api/tests/test_beta_grant.py @@ -0,0 +1,73 @@ +""" +Beta premium auto-grant in ensure_profile (first ~500 users free). + +ensure_profile() must grant premium/is_beta on the ON CONFLICT branch, not only +on a fresh INSERT: registration handlers (auth.py) create the profiles row +first, so the row already exists by the time ensure_profile runs and the INSERT +branch never wins. Before the fix, the grant silently never applied and beta +users were sent to the paywall. + +There is no database in CI, so we verify two things without one: + 1. the executed upsert grants premium AND is_beta on conflict (regression + guard against reverting to "DO UPDATE SET email" only); and + 2. the grant's boolean truth table, replicated in Python from the SQL + expression, behaves correctly across all states. +""" + +from __future__ import annotations + +import asyncio +import os +import sys + +sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..")) + +os.environ.setdefault("JWT_SECRET", "x" * 48) + +import main as main_module # noqa: E402 + + +class RecordingConn: + def __init__(self): + self.executed: list[str] = [] + + async def execute(self, query, *args): + self.executed.append(query) + return "INSERT 0 1" + + +def _run(coro): + return asyncio.get_event_loop().run_until_complete(coro) + + +def test_conflict_branch_grants_premium_and_beta(): + conn = RecordingConn() + _run(main_module.ensure_profile(conn, "user-1", "person@example.com")) + upsert = next(q for q in conn.executed if "INSERT INTO profiles" in q) + # The conflict branch must touch premium and is_beta, not just email. + conflict = upsert.split("ON CONFLICT", 1)[1] + assert "premium" in conflict + assert "is_beta" in conflict + # Never silently revoke: each column ORs with its current value. + assert "profiles.premium OR" in conflict + assert "profiles.is_beta OR" in conflict + # The cap is enforced by the remaining-slots subquery. + assert "COUNT(*) <" in conflict + + +def _granted(premium: bool, is_beta: bool, slots_remain: bool) -> tuple[bool, bool]: + """Python mirror of the SQL grant expression for premium and is_beta.""" + grant = (not premium) and (not is_beta) and slots_remain + return (premium or grant, is_beta or grant) + + +def test_grant_truth_table(): + # Defectively-denied beta user, slots remain -> granted both. + assert _granted(False, False, True) == (True, True) + # Slots exhausted -> stays paywalled. + assert _granted(False, False, False) == (False, False) + # Already a beta user -> unchanged, never double-touched. + assert _granted(True, True, True) == (True, True) + # Paid customer (premium, not beta): keeps premium, never relabelled beta. + assert _granted(True, False, True) == (True, False) + assert _granted(True, False, False) == (True, False) diff --git a/db/migrations/029_profiles_is_beta.sql b/db/migrations/029_profiles_is_beta.sql new file mode 100644 index 000000000..6ca5f4890 --- /dev/null +++ b/db/migrations/029_profiles_is_beta.sql @@ -0,0 +1,13 @@ +-- Migration 029: record the profiles.is_beta column in version control +-- +-- is_beta marks a profile that claimed one of the first BETA_TOTAL (500) free +-- premium slots. The column is read and written by ensure_profile() and the +-- /beta endpoint (api/main.py) and referenced by ADR 0012, but it was added +-- directly on the production database during the beta launch and never had a +-- migration. This closes that gap so the schema is reproducible from migrations. +-- +-- Idempotent and a no-op on production (the column already exists there). +-- Mirrors the shape of premium (003_premium.sql): boolean, not null, default false. + +alter table public.profiles + add column if not exists is_beta boolean not null default false; diff --git a/docs/decisions/0012-paywall-backend-enforcement.md b/docs/decisions/0012-paywall-backend-enforcement.md index 9fc34f680..8425672ef 100644 --- a/docs/decisions/0012-paywall-backend-enforcement.md +++ b/docs/decisions/0012-paywall-backend-enforcement.md @@ -16,10 +16,23 @@ authenticated user. Premium is determined by a single `premium` boolean column on `profiles`. It is set in two places: -- **Beta auto-grant** (`api/main.py:344-377`): on profile creation, `premium` and - `is_beta` are both set to `(SELECT COUNT(*) < BETA_TOTAL FROM profiles WHERE - is_beta = TRUE)` with `BETA_TOTAL = 500` (`api/main.py:344`). So the first ~500 - users are silently granted premium for free. +- **Beta auto-grant** (`api/main.py:344-377`): `premium` and `is_beta` are set to + `(SELECT COUNT(*) < BETA_TOTAL FROM profiles WHERE is_beta = TRUE)` with + `BETA_TOTAL = 500` (`api/main.py:344`). The intent: the first ~500 users get + premium for free. + + **Correction (2026-06-17):** this grant was in fact *never firing*. The grant + ran only on `ensure_profile`'s fresh-INSERT branch, but the registration + handlers in `api/auth.py` create the `profiles` row first, so by the time + `ensure_profile` runs the row already exists and only the `ON CONFLICT DO + UPDATE SET email` branch executed — leaving every user on `premium = false` + and sent to the paywall. Fixed by granting on the conflict branch too (slot + remains AND not already premium/beta), which also retro-grants the users who + were defectively denied, on their next authenticated request. Note this shifts + the semantics from "first 500 to **register**" to "first 500 to **make an + authenticated request** after the fix", because `is_beta` was ~0 in production + (the grant had never run). The fate of the grant (Open questions 2 and 3) is + unchanged by the fix and still needs a decision. - **Stripe webhook** (`api/main.py:674,690`): on `checkout.session.completed`, `UPDATE profiles SET premium = true`. Checkout is created at `/checkout` (`api/main.py:658`) from `STRIPE_PRICE_ID` (a single line item — apparent @@ -98,10 +111,10 @@ Open questions being answered. becomes real, not cosmetic. - The dependency reads `premium` per request (one indexed PK lookup), same cost as `require_admin` — negligible. -- **Interaction with the beta grant:** because the first ~500 users were - auto-granted `premium = true`, enabling enforcement changes nothing for them — - they pass. Enforcement only starts biting once the beta window closes or the - grant is revoked (see Open questions). +- **Interaction with the beta grant:** now that the grant actually fires (see the + 2026-06-17 correction in Context), beta users hold `premium = true`, so enabling + enforcement changes nothing for them — they pass. Enforcement only starts biting + once the beta window closes or the grant is revoked (see Open questions). - Every gated endpoint must be tested for both the premium-pass and non-premium-block paths before this ships.