stripe: collect billing address, handle paused subscriptions
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 <noreply@anthropic.com>
This commit is contained in:
parent
1305aa77ff
commit
78b2c2be28
2 changed files with 201 additions and 2 deletions
|
|
@ -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"
|
||||
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,
|
||||
|
|
|
|||
|
|
@ -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)):
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue