From dd953532898d73ecf7dd1b36c11a3bafbb0e8332 Mon Sep 17 00:00:00 2001 From: Giorgio Gilestro Date: Tue, 28 Jul 2026 18:59:32 +0200 Subject: [PATCH 1/4] gitignore: ignore all of backup/, not just SQL dumps The backup/*.sql* patterns missed pre-change .env copies, which hold live Stripe and SMTP secrets and were committable. Co-Authored-By: Claude Opus 5 --- .gitignore | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/.gitignore b/.gitignore index 168165b..e9eb31d 100644 --- a/.gitignore +++ b/.gitignore @@ -9,8 +9,10 @@ __pycache__/ .ruff_cache/ .venv/ venv/ -backup/*.sql -backup/*.sql.gz +# Everything under backup/ is operational data, never source: DB dumps and +# pre-change .env copies (which hold live Stripe/SMTP secrets). The earlier +# backup/*.sql* patterns missed the .env copies — ignore the whole directory. +backup/ *.egg-info/ build/ dist/ From 4169a6767b323a1e4413d0be1e1e3ccfac0a311a Mon Sep 17 00:00:00 2001 From: Giorgio Gilestro Date: Tue, 28 Jul 2026 18:59:48 +0200 Subject: [PATCH 2/4] stripe: always bill in GBP, never geo-select a currency MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit /pricing renders "£7" and "£70" as static copy, but checkout sniffed CF-IPCountry / Accept-Language and passed a matching currency, so Stripe picked a currency_options rate. A US visitor was shown £7 and charged $9.99; a German one was charged €7. The page's "Prices in GBP" line was therefore untrue, and "two months free" only held in GBP and USD (the EUR annual is €80 against €84, a 4.8% saving). Displaying one price and billing another is what the UK CPRs and the EU price-indication rules prohibit, so drop the currency selection rather than patch the disclosure. Removes _sniff_currency and its country and locale tables, the currency field on CheckoutRequest, and the now-unused request parameter. The currency_options configured on the live Prices are left in place but unused. Reinstating geo-pricing requires making /pricing currency-aware first — copy, buttons and the currency-specific saving claim. Co-Authored-By: Claude Opus 5 --- app/routers/stripe_billing.py | 78 +++++++------------------ tests/test_stripe_billing.py | 107 +++++++++++++++++----------------- 2 files changed, 73 insertions(+), 112 deletions(-) diff --git a/app/routers/stripe_billing.py b/app/routers/stripe_billing.py index 169e5a1..77edb8c 100644 --- a/app/routers/stripe_billing.py +++ b/app/routers/stripe_billing.py @@ -19,7 +19,7 @@ from __future__ import annotations import asyncio import json -from typing import Any, Literal, Optional +from typing import Any, Literal import stripe from fastapi import APIRouter, Body, Depends, HTTPException, Request @@ -75,51 +75,21 @@ def _price_for(cadence: str) -> str: raise HTTPException(status_code=400, detail="cadence must be 'monthly' or 'annual'") -# Rough country → currency mapping. Covers the markets we have a stated -# rate for; everything else falls back to GBP (the home currency) and -# Stripe handles the FX at checkout. Configure the per-currency -# unit_amount on each Price's `currency_options` in the Stripe Dashboard -# — we just signal which option to use here. -_COUNTRY_CURRENCY: dict[str, str] = { - "US": "usd", "CA": "usd", - "GB": "gbp", "IM": "gbp", "JE": "gbp", "GG": "gbp", - **dict.fromkeys(( - "DE", "FR", "IT", "ES", "PT", "NL", "BE", "IE", "AT", "FI", - "GR", "LU", "MT", "CY", "EE", "LV", "LT", "SI", "SK", "HR", - ), "eur"), -} - -# Accept-Language locale → currency, used when CF-IPCountry is absent. -# Ambiguous locales (e.g. plain "fr" without region) get EUR because -# that's the majority outcome. -_LOCALE_CURRENCY: dict[str, str] = { - "en-gb": "gbp", "en": "gbp", - "en-us": "usd", "en-ca": "usd", - "fr": "eur", "de": "eur", "it": "eur", "es": "eur", - "pt": "eur", "nl": "eur", -} - - -def _sniff_currency(request: Request) -> str: - """Best-effort currency detection for new-customer checkouts. - - Order: explicit Cloudflare country header, then Accept-Language - (exact match then language-only). GBP as the final fallback. Only - consulted when the user has no Stripe customer record yet — Stripe - locks currency at customer creation, so an existing customer's - currency wins regardless of the request locale. - """ - cc = (request.headers.get("cf-ipcountry") or "").upper() - if cc in _COUNTRY_CURRENCY: - return _COUNTRY_CURRENCY[cc] - al = (request.headers.get("accept-language") or "").lower() - first = al.split(",", 1)[0].split(";", 1)[0].strip() - if first in _LOCALE_CURRENCY: - return _LOCALE_CURRENCY[first] - short = first.split("-", 1)[0] - if short in _LOCALE_CURRENCY: - return _LOCALE_CURRENCY[short] - return "gbp" +# NOTE: we deliberately never pass `currency` to Stripe, so every +# checkout bills the Price's base currency — GBP. An earlier version +# sniffed CF-IPCountry / Accept-Language and selected a matching +# `currency_options` entry, but /pricing renders £7 and £70 as static +# copy: a US visitor was shown £7 and charged $9.99. Showing one price +# and billing another is exactly what the UK CPRs and the EU +# price-indication rules prohibit, so the sniffing was removed rather +# than the disclosure patched. The `currency_options` still configured +# on the Prices in the Dashboard are simply unused. +# +# To reinstate geo-pricing, /pricing must render the matching currency +# in its copy, its buttons AND its annual-saving claim first (the claim +# is currency-specific: "two months free" is true at £70/£84 and +# $94.99/$119.88, but not at €80/€84). See git history for the removed +# _sniff_currency helper and its country/locale tables. def _stripe_client() -> stripe.StripeClient: @@ -136,10 +106,6 @@ def _stripe_client() -> stripe.StripeClient: class CheckoutRequest(BaseModel): cadence: Literal["monthly", "annual"] - # Optional override; when omitted we sniff from request headers. - # Honoured only for first-time checkouts (Stripe locks currency - # to the customer at creation). - currency: Optional[Literal["gbp", "usd", "eur"]] = None class CheckoutResponse(BaseModel): @@ -149,7 +115,6 @@ class CheckoutResponse(BaseModel): @router.post("/api/stripe/checkout", response_model=CheckoutResponse) async def create_checkout( body: CheckoutRequest, - request: Request, session: AsyncSession = Depends(get_session), cu: CurrentUser = Depends(require_auth), ) -> CheckoutResponse: @@ -178,13 +143,10 @@ async def create_checkout( # referral redemption flow ships. "allow_promotion_codes": True, } - # Multi-currency: for first-time buyers (no stripe_customer_id yet) - # we pass the detected/requested currency. Stripe picks the matching - # `currency_options` rate configured on the Price in the Dashboard, - # then locks that currency to the new customer record. Existing - # customers keep their original currency regardless. - if not user.stripe_customer_id: - create_kwargs["currency"] = body.currency or _sniff_currency(request) + # No `currency` kwarg — every checkout bills the Price's base + # currency (GBP), matching the static £7 / £70 copy on /pricing. + # See the note above _stripe_client() before reintroducing one. + # # Per-cadence cooling-off treatment: # # - Annual gets a 14-day free trial. No money moves during the diff --git a/tests/test_stripe_billing.py b/tests/test_stripe_billing.py index d231cd2..6feba1a 100644 --- a/tests/test_stripe_billing.py +++ b/tests/test_stripe_billing.py @@ -465,50 +465,69 @@ def test_checkout_endpoint_requires_login(tmp_path): assert r.status_code == 401, r.text -def test_checkout_passes_sniffed_currency_for_new_customer(tmp_path): - """First-time buyer (no stripe_customer_id yet) gets the currency - sniffed from the request. CF-IPCountry=US → 'usd', and Stripe will - look up the USD currency_option on the Price.""" +def test_checkout_never_passes_currency(tmp_path): + """Every checkout bills the Price's base currency (GBP), whatever the + visitor's geo headers say. + + /pricing renders "£7" and "£70" as static copy, so selecting a + `currency_options` rate would show one price and charge another — + a US visitor saw £7 and was billed $9.99. Regression guard: if + geo-pricing is ever reinstated, the pricing page must become + currency-aware in the same change. + """ + client, _, session_cookie = _build_app(tmp_path) + + seen = [] + + def asserter(params): + seen.append(params) + assert "currency" not in params, ( + "no currency may be sent — /pricing advertises GBP only" + ) + + for headers in ( + {"cf-ipcountry": "US"}, + {"cf-ipcountry": "DE"}, + {"accept-language": "en-US,en;q=0.5"}, + {}, + ): + with patch("app.routers.stripe_billing._stripe_client", + return_value=_fake_checkout_client(asserter)): + r = client.post( + "/api/stripe/checkout", + json={"cadence": "monthly"}, + cookies={"cassandra_session": session_cookie}, + headers=headers, + ) + assert r.status_code == 200, r.text + + assert len(seen) == 4 + + +def test_checkout_rejects_currency_in_body(tmp_path): + """The `currency` field is gone from CheckoutRequest. A client that + still sends one must not silently get GBP under a USD label — the + extra key is simply ignored by pydantic, so assert the call is still + currency-free rather than trusting the caller.""" client, _, session_cookie = _build_app(tmp_path) def asserter(params): - assert params["currency"] == "usd" + assert "currency" not in params with patch("app.routers.stripe_billing._stripe_client", return_value=_fake_checkout_client(asserter)): r = client.post( "/api/stripe/checkout", - json={"cadence": "monthly"}, + json={"cadence": "monthly", "currency": "usd"}, cookies={"cassandra_session": session_cookie}, - headers={"cf-ipcountry": "US"}, ) assert r.status_code == 200, r.text -def test_checkout_body_currency_overrides_sniff(tmp_path): - """Explicit `currency` in the request body beats header sniffing — - lets a UK-based buyer choose EUR if they want to.""" - client, _, session_cookie = _build_app(tmp_path) - - def asserter(params): - assert params["currency"] == "eur" - - with patch("app.routers.stripe_billing._stripe_client", - return_value=_fake_checkout_client(asserter)): - r = client.post( - "/api/stripe/checkout", - json={"cadence": "monthly", "currency": "eur"}, - cookies={"cassandra_session": session_cookie}, - headers={"cf-ipcountry": "GB"}, - ) - assert r.status_code == 200, r.text - - -def test_checkout_omits_currency_for_existing_customer(tmp_path): - """Existing customer: Stripe locked their currency at first - checkout, so passing `currency` again would error. Verify we omit - it (and also use the existing `customer` ref instead of - customer_email).""" +def test_checkout_uses_existing_customer_ref(tmp_path): + """Existing customer: use the stored `customer` ref rather than + `customer_email`, so repeat checkouts don't mint duplicate Stripe + customers.""" import asyncio from app.models import User @@ -524,36 +543,16 @@ def test_checkout_omits_currency_for_existing_customer(tmp_path): asyncio.run(_link()) def asserter(params): - assert "currency" not in params, ( - "currency must not be passed once a customer exists — " - "Stripe rejects mismatches against the locked customer currency" - ) + assert "currency" not in params assert params["customer"] == "cus_existing_xxxxxxxxxxxxxx" + assert "customer_email" not in params with patch("app.routers.stripe_billing._stripe_client", return_value=_fake_checkout_client(asserter)): r = client.post( "/api/stripe/checkout", - json={"cadence": "monthly", "currency": "usd"}, + json={"cadence": "monthly"}, cookies={"cassandra_session": session_cookie}, headers={"cf-ipcountry": "US"}, ) assert r.status_code == 200, r.text - - -def test_sniff_currency_fallback_chain(): - """Unit-test the header-sniffing helper: CF country wins, then - Accept-Language exact, then language-only, then GBP default.""" - from types import SimpleNamespace - - from app.routers.stripe_billing import _sniff_currency - - def _req(headers): - return SimpleNamespace(headers=headers) - - assert _sniff_currency(_req({"cf-ipcountry": "DE"})) == "eur" - assert _sniff_currency(_req({"cf-ipcountry": "us"})) == "usd" # case-insensitive - assert _sniff_currency(_req({"accept-language": "fr-FR,fr;q=0.9"})) == "eur" - assert _sniff_currency(_req({"accept-language": "en-US,en;q=0.5"})) == "usd" - assert _sniff_currency(_req({"accept-language": "ja,ja-JP;q=0.5"})) == "gbp" - assert _sniff_currency(_req({})) == "gbp" From 1305aa77ffa99c17560d1a9e6972e3bc6b5c7d91 Mon Sep 17 00:00:00 2001 From: Giorgio Gilestro Date: Wed, 29 Jul 2026 11:05:25 +0200 Subject: [PATCH 3/4] spec: currency-localised pricing design Restores multi-currency pricing that 4169a67 disabled, with the page and the charge reading the same source so they cannot drift apart. Co-Authored-By: Claude Opus 5 --- ...07-29-currency-localised-pricing-design.md | 222 ++++++++++++++++++ 1 file changed, 222 insertions(+) create mode 100644 docs/superpowers/specs/2026-07-29-currency-localised-pricing-design.md diff --git a/docs/superpowers/specs/2026-07-29-currency-localised-pricing-design.md b/docs/superpowers/specs/2026-07-29-currency-localised-pricing-design.md new file mode 100644 index 0000000..639a4c5 --- /dev/null +++ b/docs/superpowers/specs/2026-07-29-currency-localised-pricing-design.md @@ -0,0 +1,222 @@ +# Currency-localised pricing — Design Spec + +**Date:** 2026-07-29 +**Status:** Draft — pending implementation plan + +## Context + +`/pricing` hardcodes `£7` and `£70` in its copy and its buttons. Until +commit `4169a67`, checkout sniffed `CF-IPCountry` / `Accept-Language` +and passed a matching `currency` to Stripe, which then selected a +`currency_options` rate off the Price. A US visitor was shown £7 and +billed $9.99; a German visitor was billed €7. The page's "Prices in GBP" +line was untrue for two of the three currencies. + +`4169a67` fixed that by forcing GBP for everyone — correct, but it +gives up genuine multi-currency pricing that is already configured and +paid for on the Stripe side. This spec restores it properly: the page +displays the currency the customer will actually be charged. + +Live Prices today (both `livemode: true`, base currency GBP): + +| Price | Interval | GBP | EUR | USD | +|---|---|---|---|---| +| `price_1TbNshDLpLwvRJpKnuWjdU1x` | month | 700 | 700 | 999 | +| `price_1TbNtWDLpLwvRJpKze87qOJ4` | year | 7000 | 7000 | 9499 | + +The EUR annual was corrected from 8000 to 7000 on 2026-07-29, so the +"two months free" claim now holds in GBP and EUR (16.7%) and understates +USD (20.8%). + +## Goals + +- A visitor sees prices in the currency they will be charged, in the + page copy, the buttons, and the saving claim. +- The visitor can override the detected currency, and the choice sticks. +- The displayed amounts are structurally incapable of disagreeing with + what Stripe charges. +- The monthly cooling-off waiver is effective for non-UK customers. +- `/it/pricing` renders in Italian, matching how the landing page + already works. + +## Non-goals + +- Adding currencies beyond GBP/EUR/USD. Each would need + `currency_options` on both live Prices first. +- VAT calculation or Stripe Tax. `automatic_tax` is currently `false`; + see Open Questions. +- Changing the monthly/annual plan structure. Annual keeps its 14-day + trial, monthly keeps immediate billing with a waiver. +- Localising any public page other than `/pricing`. + +## Design + +### Two axes, both user-switchable + +| Axis | Values | Detection order | Cookie | +|---|---|---|---| +| Language | `en`, `it` | existing `detect_public_lang` | `rtm.lang` | +| Currency | `gbp`, `eur`, `usd` | cookie → country → Accept-Language → `gbp` | `rtm.ccy` | + +An earlier draft added a third, non-switchable `jurisdiction` axis to +select between UK Reg-36 and Italian art. 59 consent wording. It was +dropped: see "Consent wording" below. Nothing legally operative is +derived from IP geolocation. + +### Data flow + +``` +GET /pricing (or /it/pricing) + ├─ lang = detect_public_lang(cookie, accept-language, cf-country, user) + ├─ currency = detect_currency(cookie, cf-country, accept-language) + │ overridden by users.stripe_currency when set + └─ amounts = pricing_catalog.get(currency) + ↓ + render symbol + amounts + computed saving %, in `lang` + ↓ +POST /api/stripe/checkout {cadence, currency} + currency honoured only when the user has no stripe_customer_id +``` + +### Components + +**`app/services/pricing_catalog.py`** (new) + +Reads both Prices with `expand[]=currency_options`, caches the result in +memory for 1 hour, and exposes: + +```python +get(currency: str) -> PriceSet # monthly, annual, symbol, saving_pct +available() -> list[str] # currencies present on BOTH prices +``` + +`saving_pct` is computed as `1 - annual / (12 * monthly)` and rounded +down to a whole percent. It is never written by hand — this is what +structurally prevents a repeat of the €80-vs-€84 drift. + +Knows nothing about HTTP, requests, or templates. Takes a Stripe client +as a constructor argument so tests inject a fake. + +**Currency detection** — added to `app/services/locales.py` next to +`detect_public_lang`, reusing its country tables rather than starting a +parallel module. Pure function, no I/O: + +```python +detect_currency(cookie_ccy, cf_country, accept_language, allowed) -> str +``` + +Priority: an explicit cookie beats everything; then `CF-IPCountry`; +then the first `Accept-Language` tag; then `gbp`. `allowed` is passed in +by the caller from `pricing_catalog.available()` — the function stays +pure and does no I/O of its own; anything not in `allowed` falls through +to the next rule. + +The country table is carried over unchanged from the removed version, +including `CA -> usd`. No CAD price exists, so every choice for Canada is +a proxy; USD is the closest familiar one. Adding a real CAD +`currency_options` entry would be the actual fix, and is out of scope. + +**`/pricing` route** (`app/routers/public.py`) gains the currency in its +context and a sibling `/it/pricing` route. Copy moves into the existing +`app/locales/{en,it}.yaml` under a `pricing.` key, matching the landing +page. A `?ccy=` query parameter sets the cookie and redirects, so the +switcher works without JavaScript. + +**`/api/stripe/checkout`** restores the `currency` field on +`CheckoutRequest`, validated against `pricing_catalog.available()`, and +passes it only when `user.stripe_customer_id` is unset. This reverts the +mechanical part of `4169a67` while keeping its guarantee: the page and +the charge always agree, because both now read the same catalog. + +### Consent wording + +The monthly waiver currently cites *Regulation 36 of the Consumer +Contracts Regulations 2013*. That is UK law; for a customer resident +elsewhere the citation does not apply, and an ineffective waiver means a +monthly subscriber retains the 14-day refund right the checkbox was +meant to remove. + +UK Reg 36 and Italian `Codice del Consumo` art. 59 both implement +Directive 2011/83/EU art. 16(m). The waiver takes effect from its +substance — an express request for immediate performance plus an +acknowledgement that the cancellation right is lost — not from the +citation. Wording that states the substance and cites no statute is +therefore effective under both regimes, whereas citing the wrong one is +worse than citing none. + +New wording, in place of the current sentence: + +> I request that the service starts immediately, and I understand that +> once it has started I lose my right to cancel and get a refund. + +The Terms-of-Service agreement in the same checkbox is unchanged. The +Italian rendering of this sentence is a translation of substance, not of +a statutory reference, so it carries the same weight as the existing +`auth.ack` translations. + +This wording is subject to the legal sign-off already tracked on the +launch blocker list. It is not a lawyer-authored sentence. + +### Locked currency + +Stripe locks currency to the Customer at creation. A returning customer +whose subscription lapsed could otherwise be shown €7 and billed £7. + +Add `users.stripe_currency` (`String(3)`, nullable), populated in +`_grant_paid` from the subscription object. When set, `/pricing` renders +that currency and disables the switcher with a one-line explanation. +Requires a small Alembic migration. + +The simpler alternative — disable the switcher for anyone with a +`stripe_customer_id`, without storing the currency — is rejected because +it still shows a possibly-wrong currency; it only stops the user +changing it. + +### Failure modes + +| Condition | Behaviour | +|---|---| +| Stripe unreachable, warm cache | Serve stale cache indefinitely; log a warning | +| Stripe unreachable, cold cache | Static GBP amounts, switcher hidden — i.e. exactly today's page | +| Requested currency absent from a Price | Excluded from `available()`, so unreachable | +| `?ccy=` with an unknown value | Ignored, cookie untouched | + +The page never returns an error because of a pricing lookup. + +## Testing + +- `detect_currency` — table-driven unit tests over the priority chain, + including values outside `allowed` falling through to the next rule. +- `pricing_catalog` — fake Stripe client: happy path, `saving_pct` + arithmetic, currency missing from one Price but not the other, cold-cache + failure, stale-cache-on-failure. +- Route tests — `/pricing` and `/it/pricing` render expected symbols and + amounts per cookie/header combination; `?ccy=` sets the cookie. +- **Cross-check test:** for each currency, assert the amount rendered in + the page equals the amount Stripe would charge for the currency + checkout sends. This is the regression guard for the original bug and + is the most important test in the set. +- Locked-currency test — a user with `stripe_currency` set sees that + currency regardless of headers or cookie. + +## Open questions + +1. **EU VAT.** `automatic_tax` is `false`, so no VAT is charged. B2C + digital services sold into the EU have no VAT threshold — VAT is due + in the customer's member state from the first sale, normally via a + non-Union OSS registration. Displaying EUR does not create this + obligation, but selling to EU consumers does. Resolve before taking + EUR money. Registration decision, not a code change. +2. **`billing_address_collection`.** Currently unset, so Stripe defaults + to `auto` and may capture only a postal code. Setting it to + `required` puts a country on every Customer record — useful for the + VAT question above and for knowing where customers are. Recommended, + independent of this feature. + +## Out of scope / follow-ups + +- `customer.subscription.paused` and `.resumed` are subscribed at Stripe + but absent from `_HANDLERS`. Harmless while pause is disabled in the + portal configuration, but a live trap if it is ever enabled. +- Localising `/terms` and `/privacy`, which the Italian pricing page + will link to in English. From 78b2c2be288278b6d37f53e613f2b36fc9118f85 Mon Sep 17 00:00:00 2001 From: Giorgio Gilestro Date: Thu, 30 Jul 2026 12:13:47 +0200 Subject: [PATCH 4/4] stripe: collect billing address, handle paused subscriptions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two pre-launch gaps. 1. Checkout collected no address, so Stripe Customers carried no country. EU B2C digital-services VAT is due at the consumer's place of supply and we can't even scope that question without knowing where buyers are; the card's billing country is the evidence a tax authority accepts, an IP guess is not. It also gives AVS data to the fraud checks. Passing `customer` suppresses the write-back to the Customer record, so existing customers also need customer_update.address=auto — without it the country lands on the PaymentIntent and nowhere durable. 2. A paused subscription kept paid features while Stripe billed nothing. Both mechanisms were unhandled: status="paused" (trial ended with no usable card) had no entry in _HANDLERS at all, and `pause_collection` — what the customer portal's pause button uses — leaves status as "active", so the status check waved it through. Revoke on both, keeping stripe_subscription_id since the subscription still exists at Stripe and resumes under the same id. Pause is disabled in our live portal configuration, so (2) is latent rather than live — but it's a one-toggle mistake away from being real. Verified against live Stripe: a Checkout Session with the new params is accepted (gbp 700, billing_address_collection=required, livemode), and the live webhook endpoint already subscribes both paused and resumed. Co-Authored-By: Claude Opus 5 --- app/routers/stripe_billing.py | 50 ++++++++++- tests/test_stripe_billing.py | 153 ++++++++++++++++++++++++++++++++++ 2 files changed, 201 insertions(+), 2 deletions(-) diff --git a/app/routers/stripe_billing.py b/app/routers/stripe_billing.py index 77edb8c..30ab079 100644 --- a/app/routers/stripe_billing.py +++ b/app/routers/stripe_billing.py @@ -142,6 +142,14 @@ async def create_checkout( # Lets us paste in a referral coupon at checkout once the # referral redemption flow ships. "allow_promotion_codes": True, + # Collect a billing address on every checkout so each Stripe + # Customer carries a country. Two reasons: card-fraud checks get + # materially better with AVS data, and EU B2C digital-services + # VAT is charged at the *consumer's* place of supply — we can't + # answer the OSS question at all without knowing where buyers + # are. Note this is the address on the card, not an IP guess, + # which is the evidence a tax authority actually accepts. + "billing_address_collection": "required", } # No `currency` kwarg — every checkout bills the Price's base # currency (GBP), matching the static £7 / £70 copy on /pricing. @@ -165,6 +173,12 @@ async def create_checkout( create_kwargs["subscription_data"] = {"trial_period_days": 14} if user.stripe_customer_id: create_kwargs["customer"] = user.stripe_customer_id + # Required for billing_address_collection to actually persist: + # when `customer` is supplied, Stripe collects the address for + # the payment but leaves the Customer record untouched unless + # customer_update.address is "auto". Without this the country + # lands on the PaymentIntent and nowhere durable. + create_kwargs["customer_update"] = {"address": "auto"} else: create_kwargs["customer_email"] = user.email @@ -294,11 +308,15 @@ async def _grant_paid( await convert_referral(session, user) -async def _revoke_paid(user: User) -> None: +async def _revoke_paid(user: User, *, keep_subscription: bool = False) -> None: user.tier = "free" - user.stripe_subscription_id = None + if not keep_subscription: + user.stripe_subscription_id = None user.stripe_trial_end_at = None # Keep stripe_customer_id so a re-subscription matches this row. + # `keep_subscription` is for a pause: the subscription still exists + # at Stripe and will resume under the same id, so nulling our copy + # would lose the link while access is merely suspended. async def _handle_checkout_completed( @@ -336,6 +354,16 @@ async def _handle_subscription_event( customer_id=obj.get("customer")) return status = obj.get("status") + # `pause_collection` is a *different* mechanism from status="paused": + # the subscription stays `active` while Stripe simply stops invoicing. + # Unhandled, that leaves the customer on paid features indefinitely + # without paying, so treat any live pause as not-paid regardless of + # status. Pause is disabled in our live portal configuration, so in + # practice this only fires if someone re-enables it there or pauses + # from the Dashboard — which is exactly when we'd want it to work. + if obj.get("pause_collection"): + await _revoke_paid(user, keep_subscription=True) + return # Stripe statuses: trialing, active, past_due, canceled, unpaid, # incomplete, incomplete_expired, paused. Treat trialing/active as # paid; everything else holds tier the same until we get an explicit @@ -362,6 +390,22 @@ async def _handle_subscription_deleted( await _revoke_paid(user) +async def _handle_subscription_paused( + session: AsyncSession, event_type: str, obj: dict[str, Any], +) -> None: + """customer.subscription.paused — status flips to `paused` when a + trial ends with no usable payment method (trial_settings.end_behavior + .missing_payment_method = pause). No money is being collected, so + paid features come off. `.resumed` routes to the normal subscription + handler, which grants again on active/trialing.""" + user = await _find_user(session, customer_id=obj.get("customer")) + if user is None: + log.warning("stripe.user_not_found", event_type=event_type, + customer_id=obj.get("customer")) + return + await _revoke_paid(user, keep_subscription=True) + + async def _handle_audit_only( session: AsyncSession, event_type: str, obj: dict[str, Any], ) -> None: @@ -376,6 +420,8 @@ _HANDLERS = { "customer.subscription.created": _handle_subscription_event, "customer.subscription.updated": _handle_subscription_event, "customer.subscription.deleted": _handle_subscription_deleted, + "customer.subscription.paused": _handle_subscription_paused, + "customer.subscription.resumed": _handle_subscription_event, "invoice.paid": _handle_audit_only, "invoice.payment_failed": _handle_audit_only, "charge.refunded": _handle_audit_only, diff --git a/tests/test_stripe_billing.py b/tests/test_stripe_billing.py index 6feba1a..f592850 100644 --- a/tests/test_stripe_billing.py +++ b/tests/test_stripe_billing.py @@ -341,6 +341,131 @@ def test_subscription_active_grants_paid(tmp_path): assert asyncio.run(_check_tier()) == "paid" +# --- pause / resume -------------------------------------------------------- + + +def _activate(client, *, customer="cus_p", subscription="sub_p", evt="evt_a"): + """Link user 1 to a Stripe customer and put them on paid.""" + return _post_webhook(client, body={ + "id": evt, + "type": "checkout.session.completed", + "data": {"object": { + "client_reference_id": "1", + "customer": customer, + "subscription": subscription, + }}, + }) + + +def _tier_and_sub(factory): + async def _check(): + from sqlalchemy import select + from app.models import User + async with factory() as session: + u = (await session.execute( + select(User).where(User.id == 1) + )).scalar_one() + return u.tier, u.stripe_subscription_id + return asyncio.run(_check()) + + +def test_subscription_paused_drops_tier(tmp_path): + """status=paused means Stripe has stopped collecting (trial ended + with no usable card). Paid features must come off — otherwise the + customer keeps everything for free.""" + client, factory, _ = _build_app(tmp_path) + _activate(client) + assert _tier_and_sub(factory)[0] == "paid" + + r = _post_webhook(client, body={ + "id": "evt_paused", + "type": "customer.subscription.paused", + "data": {"object": { + "id": "sub_p", "customer": "cus_p", "status": "paused", + }}, + }) + assert r.status_code == 200, r.text + assert r.json()["status"] == "ok", "must not fall through to 'ignored'" + + tier, sub = _tier_and_sub(factory) + assert tier == "free" + # The subscription still exists at Stripe and resumes under the same + # id, so we keep our link to it. + assert sub == "sub_p" + + +def test_subscription_resumed_regrants_tier(tmp_path): + client, factory, _ = _build_app(tmp_path) + _activate(client) + _post_webhook(client, body={ + "id": "evt_paused2", + "type": "customer.subscription.paused", + "data": {"object": { + "id": "sub_p", "customer": "cus_p", "status": "paused", + }}, + }) + assert _tier_and_sub(factory)[0] == "free" + + r = _post_webhook(client, body={ + "id": "evt_resumed", + "type": "customer.subscription.resumed", + "data": {"object": { + "id": "sub_p", "customer": "cus_p", "status": "active", + }}, + }) + assert r.status_code == 200, r.text + assert r.json()["status"] == "ok" + assert _tier_and_sub(factory) == ("paid", "sub_p") + + +def test_pause_collection_drops_tier_despite_active_status(tmp_path): + """The portal's pause uses `pause_collection` and leaves status as + `active`, so the status check alone would keep the user on paid while + Stripe bills them nothing.""" + client, factory, _ = _build_app(tmp_path) + _activate(client) + assert _tier_and_sub(factory)[0] == "paid" + + r = _post_webhook(client, body={ + "id": "evt_pause_coll", + "type": "customer.subscription.updated", + "data": {"object": { + "id": "sub_p", + "customer": "cus_p", + "status": "active", + "pause_collection": {"behavior": "void"}, + }}, + }) + assert r.status_code == 200, r.text + assert _tier_and_sub(factory) == ("free", "sub_p") + + +def test_unpause_collection_regrants_tier(tmp_path): + """Resuming collection sends subscription.updated with + pause_collection cleared to null — that must grant paid back.""" + client, factory, _ = _build_app(tmp_path) + _activate(client) + _post_webhook(client, body={ + "id": "evt_pc_on", + "type": "customer.subscription.updated", + "data": {"object": { + "id": "sub_p", "customer": "cus_p", "status": "active", + "pause_collection": {"behavior": "void"}, + }}, + }) + assert _tier_and_sub(factory)[0] == "free" + + _post_webhook(client, body={ + "id": "evt_pc_off", + "type": "customer.subscription.updated", + "data": {"object": { + "id": "sub_p", "customer": "cus_p", "status": "active", + "pause_collection": None, + }}, + }) + assert _tier_and_sub(factory) == ("paid", "sub_p") + + # --- idempotency + unknown ------------------------------------------------ @@ -524,6 +649,30 @@ def test_checkout_rejects_currency_in_body(tmp_path): assert r.status_code == 200, r.text +def test_checkout_requires_billing_address(tmp_path): + """Every checkout must collect a billing address, so each Stripe + Customer ends up with a country. EU B2C digital-services VAT is due + at the consumer's place of supply, and the card's billing country is + the evidence for that — an IP guess is not.""" + client, _, session_cookie = _build_app(tmp_path) + + def asserter(params): + assert params["billing_address_collection"] == "required" + # New customer (no stored customer id): customer_update is only + # valid alongside `customer`, so it must be absent here. + assert "customer_update" not in params + + for cadence in ("monthly", "annual"): + with patch("app.routers.stripe_billing._stripe_client", + return_value=_fake_checkout_client(asserter)): + r = client.post( + "/api/stripe/checkout", + json={"cadence": cadence}, + cookies={"cassandra_session": session_cookie}, + ) + assert r.status_code == 200, r.text + + def test_checkout_uses_existing_customer_ref(tmp_path): """Existing customer: use the stored `customer` ref rather than `customer_email`, so repeat checkouts don't mint duplicate Stripe @@ -546,6 +695,10 @@ def test_checkout_uses_existing_customer_ref(tmp_path): assert "currency" not in params assert params["customer"] == "cus_existing_xxxxxxxxxxxxxx" assert "customer_email" not in params + # Without customer_update.address the collected address is + # attached to the payment only and the Customer record keeps a + # null address — i.e. still no country for the VAT question. + assert params["customer_update"] == {"address": "auto"} with patch("app.routers.stripe_billing._stripe_client", return_value=_fake_checkout_client(asserter)):