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"