victor34593993 commited on
Commit
ed45229
Β·
verified Β·
1 Parent(s): e74dda2

low-priority fixes: ADMIN_TOKEN hard boot guard + invoice.paid subscription check

Browse files
Files changed (4) hide show
  1. app/billing.py +12 -7
  2. app/main.py +8 -1
  3. tests/test_billing.py +24 -0
  4. tests/test_security.py +11 -0
app/billing.py CHANGED
@@ -319,19 +319,24 @@ async def handle_subscription_event(db: AsyncSession, event: dict) -> dict:
319
  log.info("stripe: subscription deleted -> chatbot OFF for %s", tenant.slug)
320
  return {"status": "disabled", "slug": tenant.slug}
321
  if etype == "invoice.paid":
 
 
 
 
 
 
 
 
 
 
 
 
322
  if not tenant.chatbot_enabled:
323
  if tenant.disabled_reason == "admin":
324
  # Manually paused by the owner β€” only the owner may re-enable.
325
  log.info("stripe: invoice paid but %s is admin-paused; staying OFF",
326
  tenant.slug)
327
  return {"status": "admin_paused", "slug": tenant.slug}
328
- sub_id = _invoice_subscription_id(obj)
329
- if not sub_id or sub_id != tenant.stripe_subscription_id:
330
- # Late/out-of-order invoice for a subscription that is no
331
- # longer this tenant's active one β€” do not resurrect.
332
- log.info("stripe: invoice paid for %s ignored (subscription %r "
333
- "does not match)", tenant.slug, sub_id)
334
- return {"status": "subscription_mismatch", "slug": tenant.slug}
335
  tenant.chatbot_enabled = True
336
  tenant.disabled_reason = ""
337
  await db.flush()
 
319
  log.info("stripe: subscription deleted -> chatbot OFF for %s", tenant.slug)
320
  return {"status": "disabled", "slug": tenant.slug}
321
  if etype == "invoice.paid":
322
+ # #28: check the subscription match FIRST, for both branches uniformly.
323
+ # Previously an ALREADY-enabled tenant returned "enabled" unconditionally
324
+ # without ever checking sub_id β€” harmless today (no mutation on that
325
+ # path), but the status was misleading and would silently mask a bug if
326
+ # that branch ever grew to mutate state on a stale/mismatched invoice.
327
+ sub_id = _invoice_subscription_id(obj)
328
+ if not sub_id or sub_id != tenant.stripe_subscription_id:
329
+ # Late/out-of-order invoice for a subscription that is no longer
330
+ # this tenant's active one β€” never act on it either way.
331
+ log.info("stripe: invoice paid for %s ignored (subscription %r "
332
+ "does not match)", tenant.slug, sub_id)
333
+ return {"status": "subscription_mismatch", "slug": tenant.slug}
334
  if not tenant.chatbot_enabled:
335
  if tenant.disabled_reason == "admin":
336
  # Manually paused by the owner β€” only the owner may re-enable.
337
  log.info("stripe: invoice paid but %s is admin-paused; staying OFF",
338
  tenant.slug)
339
  return {"status": "admin_paused", "slug": tenant.slug}
 
 
 
 
 
 
 
340
  tenant.chatbot_enabled = True
341
  tenant.disabled_reason = ""
342
  await db.flush()
app/main.py CHANGED
@@ -79,8 +79,15 @@ def _check_security(settings) -> None:
79
  """Fail fast on insecure production config. Skipped under testing."""
80
  if settings.testing:
81
  return
 
 
 
 
82
  if not settings.admin_token:
83
- log.warning("ADMIN_TOKEN is empty β€” the admin panel/API will reject all requests")
 
 
 
84
  # Every tenant's Shopify/WhatsApp secret is encrypted with secret_key, so a
85
  # public default would let anyone decrypt them. Crash loudly, never run insecure.
86
  if settings.secret_key == "dev-insecure-change-me":
 
79
  """Fail fast on insecure production config. Skipped under testing."""
80
  if settings.testing:
81
  return
82
+ # #27: an empty ADMIN_TOKEN previously only warned β€” the admin API/panel fails
83
+ # closed (never a security hole), but a misconfigured deploy would silently
84
+ # break the outreach machine's automation (it authenticates as admin) with no
85
+ # loud signal. Crash loudly instead, same as the SECRET_KEY guard below.
86
  if not settings.admin_token:
87
+ raise RuntimeError(
88
+ "ADMIN_TOKEN is empty. Set it as a Space/env secret before starting "
89
+ "(the admin panel/API and the outreach automation both need it)."
90
+ )
91
  # Every tenant's Shopify/WhatsApp secret is encrypted with secret_key, so a
92
  # public default would let anyone decrypt them. Crash loudly, never run insecure.
93
  if settings.secret_key == "dev-insecure-change-me":
tests/test_billing.py CHANGED
@@ -366,6 +366,30 @@ async def test_invoice_paid_matching_subscription_reenables(db_session):
366
  assert t.disabled_reason == "", slug
367
 
368
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
369
  async def test_invoice_paid_mismatched_or_missing_subscription_stays_off(db_session):
370
  t = await _disabled_tenant(
371
  db_session, slug="desfase", customer="cus_mis", sub="sub_actual", reason="billing"
 
366
  assert t.disabled_reason == "", slug
367
 
368
 
369
+ async def test_invoice_paid_for_already_enabled_tenant_checks_subscription_too(db_session):
370
+ """#28: an ALREADY-enabled tenant used to return 'enabled' unconditionally
371
+ for ANY invoice.paid on their customer id, without checking the invoice's
372
+ subscription matches their CURRENT one. A stale invoice for an old/replaced
373
+ subscription must report subscription_mismatch, same as the disabled path."""
374
+ t = Tenant(slug="yaactiva", name="yaactiva", stripe_customer_id="cus_ya",
375
+ stripe_subscription_id="sub_actual", chatbot_enabled=True, disabled_reason="")
376
+ db_session.add(t)
377
+ await db_session.flush()
378
+
379
+ r = await billing.handle_subscription_event(
380
+ db_session, _paid_event("cus_ya", {"subscription": "sub_VIEJA"})
381
+ )
382
+ assert r["status"] == "subscription_mismatch"
383
+ await db_session.refresh(t)
384
+ assert t.chatbot_enabled is True # untouched either way β€” just the status was misleading
385
+
386
+ # the matching subscription still reports "enabled" as before
387
+ r2 = await billing.handle_subscription_event(
388
+ db_session, _paid_event("cus_ya", {"subscription": "sub_actual"})
389
+ )
390
+ assert r2["status"] == "enabled"
391
+
392
+
393
  async def test_invoice_paid_mismatched_or_missing_subscription_stays_off(db_session):
394
  t = await _disabled_tenant(
395
  db_session, slug="desfase", customer="cus_mis", sub="sub_actual", reason="billing"
tests/test_security.py CHANGED
@@ -20,6 +20,17 @@ def test_default_secret_key_refuses_to_boot_in_prod():
20
  _check_security(_settings(secret_key="dev-insecure-change-me"))
21
 
22
 
 
 
 
 
 
 
 
 
 
 
 
23
  def test_strong_secret_key_boots():
24
  _check_security(_settings()) # no raise
25
 
 
20
  _check_security(_settings(secret_key="dev-insecure-change-me"))
21
 
22
 
23
+ def test_empty_admin_token_refuses_to_boot_in_prod():
24
+ """#27: was a silent warning β€” a misconfigured deploy would break the admin
25
+ API and the outreach automation (authenticates as admin) with no loud signal."""
26
+ with pytest.raises(RuntimeError, match="ADMIN_TOKEN"):
27
+ _check_security(_settings(admin_token=""))
28
+
29
+
30
+ def test_empty_admin_token_skipped_under_testing():
31
+ _check_security(_settings(testing=True, admin_token="")) # no raise
32
+
33
+
34
  def test_strong_secret_key_boots():
35
  _check_security(_settings()) # no raise
36