P3: Defer billing object hardening until commercial enablement

[tomqwu]

Current Priority Decision

Owner direction, 2026-09-13: complete Church/Basketball business workflows and day-to-day operations for every role first; billing and platform readiness later.

Execution lane: phase: billing-later. Current business milestone: #289.
This section overrides older priority, start conditions, broad dependency order and launch estimates below. Keep the detailed technical recommendations where compatible. Execute only the NOW slice; do not expand a mixed issue into its whole production scope. Record completed slice receipts in #289; keep this issue open while retained later work remains.

NOW

Do not build checkout, plans, payment-method flows or billing-only ownership work for the business milestone. Preserve existing JWT-admin/object guards. #288 owns the minimal noncommercial/disabled-feature boundary; a still-reachable financial/tenant vulnerability is a current blocker, not postponed security.

LATER

Finish this ticket's original object-authorization/provider acceptance before any commercial exposure or enablement, after #289 business acceptance and an explicit owner restart.

Validation stays local. No CI checks, hosted reviewers or Ollama code review. Preserve tenant isolation, real member responses and atomic roster changes. A deferred feature is not permission to expose an unfixed vulnerability. No deployment, paid-provider activation or production sign-off is authorized by this reprioritization.

Before implementation, read #252 and #289; finish one role/work package with tests, local review and affected docs/assets. The earlier completion receipt below covers whole-issue closure, not a requirement to finish every deferred package before the business milestone.

---

Current Implementation Handoff

Prepared 2026-09-13 for a lower-cost builder at source 21a4a804aa57580451edded04736b1f51aef7e48.
No CI checks. All implementation validation and code review run locally.
Use the shared builder contract and this issue's work packages; no xhigh model or automatic model upgrade is required. This is a detailed recommendation, not a claim that a smaller model cannot make mistakes or that tests have passed.

Risk/review focus: High: financial object authorization.
Start condition: DEFERRED. Do not start implementation from this issue's older prompt or package list until the owner resumes this track after #289, except a confirmed still-reachable security regression.

Source of Truth and Current State

• api/dependencies.py
• api/routers/billing.py
• api/services/stripe_service.py
• api/services/billing_service.py
• api/utils/stripe_client.py
• api/models.py
• tests/api/test_billing_authorization.py

All mounted billing handlers inspected use get_current_admin_user. verify_admin_access is now an authenticated compatibility wrapper. Invoice scoping and detach/default payment ownership checks plus tests already exist. Remaining checkout-success handling ignores session_id and returns success from the actor's current subscription; inspect remaining object operations rather than redoing fixed guards.

This handoff supersedes stale implementation statements in the background below. Preserve existing successful behavior and tests. Recheck the current branch before editing; the baseline is a source pointer, not permission to discard newer changes.

Dependencies and Ownership

• Current execution prerequisite: owner reactivates this deferred track after P1: Accept complete Church and Basketball day-to-day business flows #289. Original provider/platform dependencies are retained as future acceptance, not blockers for the current business flow.
• Coordinate, but do not wait for entire tickets to close merely to inspect/write isolated tests: P0: Close tenant data exposure in scheduling APIs and exports #254, P2: Validate production configuration after business-flow acceptance #258, P3: Defer paid billing and SMS product acceptance #270, P1: Repair local test entry points, coverage and isolation #281.
• The final commands below require P1: Repair local test entry points, coverage and isolation #281's safe local environment or an equivalently verified isolated setup. A missing dependency blocks only the slice that needs it; document that state.
• Shared source/ORM/migration changes are serialized per the master roadmap. Do not start concurrent edits to the same ownership area.

Recommended Decisions

1. Preserve existing JWT-admin and invoice/customer ownership checks. Extend the existing operation matrix instead of writing a parallel auth implementation.
2. Checkout return is read-only: retrieve the requested checkout session using the provider adapter, verify customer and org mapping against the tenant-scoped Subscription, and return its real state. Only a validated webhook/reconciliation path may change paid entitlement.
3. A foreign or unknown provider object must not mutate anything. Auth denials must make zero provider calls; legitimate ownership verification may make a read-only retrieval, but never a foreign mutation.
4. Attachment must reject an already foreign-owned payment method; use trusted customer mapping and provider-supported setup/confirmation flow for unattached methods rather than claiming that an arbitrary method ID is ownership proof.

Small Work Packages

Each item is one reviewable slice, not permission for one giant PR. Add the failing regression first; finish code, tests and affected docs for that slice together. Leave this issue open until all packages and original acceptance criteria are satisfied or explicitly revised by the owner.

• 256.1: Run and extend tests/api/test_billing_authorization.py to capture current fixed behavior and uncovered object cases.
• 256.2: Repair checkout-success session validation and misleading success text with provider mocks; preserve webhook entitlement ownership under P3: Defer paid billing and SMS product acceptance #270.
• 256.3: Review attach/list/detach/default/invoice/portal/plan actions for tenant-object mapping and idempotent failure behavior.
• 256.4: Update current issue title/progress and billing/public docs so old identity-spoofing evidence is clearly historical.

Required Regression Cases

These are specifications for tests to add/retain, not claimed execution results. Each new negative case must assert unchanged unauthorized state and zero forbidden side effects.

• T256-01: Missing/invalid JWT or volunteer with person_id=admin -> existing auth denial before any service construction.
• T256-02: Own admin supplies foreign checkout customer/session -> generic denial, no subscription/payment state changes.
• T256-03: Own pending checkout -> pending state, never success/paid entitlement; completed own checkout -> verified read response, no duplicate billing action.
• T256-04: Foreign payment method attach/detach/default -> no foreign mutation; existing successful own-customer tests remain green.

Local Commands and Evidence

Existing targeted commands (paths checked against the audit source; run only after the stated safe preflight):

poetry run pytest tests/api/test_billing_authorization.py tests/api/test_billing_registered.py -q
poetry run pytest tests/web/test_billing.py -q

Also run the shared formatting/lint/touched-type/unit/full-suite and local review protocol from #252 for the final pushed revision. Add new targeted tests to these commands when implemented. Run API and browser tiers in separate processes. Native, PostgreSQL, image, provider and operator drills require their explicit environment; an unavailable tool/target is blocked/not run, never a pass.

Schema and Compatibility

No auth-schema rewrite is required. Provider-event idempotency storage belongs to #270; coordinate any migration there.

Stop Conditions

No live Stripe calls, charges or payment-method attachment during default tests. Pause if provider ownership cannot be established through the supported adapter; do not weaken validation to satisfy a mock.

After two failed focused repair attempts without new diagnostic evidence, stop the affected package and post the exact failure, commands, suspected boundary and needed decision. Do not silently broaden scope, weaken tests or upgrade models. A fresh local reviewer checks: Verify the distinction between authentication, read-only provider ownership verification and actual financial mutations; inspect checkout pending/success behavior.

Completion Receipt

• Work-package and regression IDs above map to changed files and actual results.
• Commands, versions, dates, pass/fail/skip/not-run counts, logs/screenshots and tested head/base SHAs are linked.
• A separate local review records findings and resolution; self-review is labeled if used and is not misrepresented as independent review.
• Affected docs/README/playbooks/screenshots and dependency/roadmap status are reconciled, not left as unnamed follow-ups.
• If implementation is authorized through PR/merge, GitHub reports mergeable and the shared local-evidence requirements are met; reviewer agents never merge.
• No hosted CI check, status attestation, Ollama reviewer, live provider action, deployment, real-data purge or store submission was introduced by implication.

Copyable Builder Prompt

First read this issue's Current Priority Decision and #289. Run only its NOW slice.
If this issue is deferred, report that state instead of starting the older package list.
Implement the next ready work package in tomqwu/SignUpFlow issue #256.
Read its Current Implementation Handoff and #252 Builder Handoff Contract first.
Inspect current source and preserve newer/unrelated changes. Start with the
package's failing regression, then complete code, local tests, local review and
affected docs/assets together. Do not skip acceptance or invent passing evidence.
No CI checks or Ollama code review. Do not deploy, activate providers, purge real
data or submit to stores. Stop and report unmet prerequisites or policy decisions.
Record the package/test IDs and exact reviewed/tested source SHAs before claiming done.

---

Earlier Audit and Acceptance Context

Current policy (2026-09-13)

No CI checks. Everything is validated locally. This includes code review,
formatting, lint, type checks, migrations, all test tiers, security scans,
artifact checks and mobile validation. Do not add hosted jobs, required CI
statuses, synthetic success checks or an Ollama reviewer. GitHub is for source,
PRs, issues and publication, not validation.

Record commands, environment, results, limitations and reviewed head/base SHAs.
Builders merge only with completed local evidence and GitHub mergeability;
reviewer agents never merge. Real staging/provider/device acceptance remains
required where applicable, driven by authorized local operator tools.
Historical evidence and older comments do not override this policy.

Parent roadmap: #252

Priority: P0, blocks core production pilot. Phase: A. Suggested owner: Backend/security. Original estimate (superseded; re-estimate remaining work): 2-3 engineering days.

Historical audit evidence (recheck against current source)

api/dependencies.py:56 verify_admin_access loads the person_id query parameter and checks that row's role without authenticating the caller. Billing mutations including api/routers/billing.py:891 use this dependency. A credential-free disposable request reached a mocked billing-portal provider and returned 200 when supplied a synthetic administrator ID. No Stripe request or charge was made.

Baseline: GitHub main 214e3f3f17a582d5f9b2063be6872ea2b1d25714, audited 2026-09-09. Findings concern synthetic reproduction or source inspection, not a claim of live exploitation.

Implementation plan

1. Preserve the already-implemented get_current_admin_user guards and authenticated verify_admin_access compatibility wrapper. Complete the remaining provider-object/checkout validation described in the current handoff.
2. Resolve local subscription/customer ownership inside the authenticated organization before provider access. A legitimate admin may use read-only provider retrieval to verify a checkout/payment object's ownership; reject unknown or foreign objects before any mutation.
3. Keep remaining compatibility callers explicitly authenticated; the old identity-spoofing helper is already fixed. Do not accept request identity or an arbitrary checkout ID as proof of permission.
4. Use provider spies to prove authentication/role denials happen before provider access, and object-ownership denials happen before any provider mutation, DB write or queued action. Read-only verification is permitted only for an authenticated authorized caller.

Acceptance criteria

• Anonymous and volunteer callers cannot invoke any billing admin operation; supplying another person's ID grants no access.
• Tenant A's admin cannot operate on tenant B's subscription, invoice, checkout or payment method.
• All successful operations have auditable authenticated actors and tenant-scoped lookups.
• Disabling/missing provider credentials does not count as an authorization control.

Dependencies

None; start immediately.

Validation

Use mocked Stripe only for unit/API authorization tests. Verify zero provider calls for authentication/role denials and zero mutations for object-ownership denials, test the web billing entry points, run make test-unit and make test-all. Live billing verification belongs to the separate optional billing task.

Whole-repository audit scope (2026-09-13)

Baseline: 21a4a804aa57580451edded04736b1f51aef7e48. This addendum assigns full-scope follow-through; it is not a new test pass or production sign-off. No CI checks; all review and validation runs locally.

Include every mounted billing read/write/portal/payment/invoice endpoint and its direct web caller in the auth matrix. Require provider-call spies for anonymous/volunteer authorization denials before provider access and foreign-object denial before any payment mutation. Read-only ownership retrieval for an authenticated tenant admin is explicitly distinguished. The default local browser harness inherits environment keys (#281); provider credentials being absent is not a security boundary. Keep sandbox billing acceptance separate in #270 and public claims in #278.

Keep evidence and disposition synchronized with master roadmap #252 and documentation ledger #277. Close only after the remaining acceptance criteria have linked local results; a planning/audit note is not completion.

[tomqwu]

Partial implementation in draft PR #272: all billing endpoints now require JWT-backed tenant-admin authorization; the legacy dependency no longer accepts a person_id as proof of identity; invoice queries include tenant scope; detach/default-payment operations verify Stripe customer ownership. 81 new real-auth/mocked-provider regression cases pass. Keep this issue open: checkout-session verification, attachment/SetupIntent ownership, complete authenticated audit coverage, and broader billing acceptance still need implementation and review. No live Stripe calls were made.