Skip to content

Fix #1125: link logged-in donors to their Stripe Customer (not 'anonymous user') - #1174

Open
rdhyee wants to merge 4 commits into
masterfrom
fix/1125-stripe-customer-user
Open

Fix #1125: link logged-in donors to their Stripe Customer (not 'anonymous user')#1174
rdhyee wants to merge 4 commits into
masterfrom
fix/1125-stripe-customer-user

Conversation

@rdhyee

@rdhyee rdhyee commented Jun 18, 2026

Copy link
Copy Markdown
Member

Fixes #1125.

The one-line fix

In payment/stripelib.py, Pay.__init__ used token-presence as a proxy for "anonymous user." But a logged-in donor entering card details also produces a token, so they were routed through make_account()'s anonymous branch → Stripe Customer description='anonymous user' and Account.user=None (orphaned, no saved-card reuse).

Fix: pass user=transaction.user into make_account (it's None for a truly anonymous donor, and the user for a logged-in one). This makes the donation path consistent with the other make_account(user=...) call sites already in FundView (e.g. the transaction.user.id != request.user.id branch).

Behavior note (for review)

make_account with user set runs the existing if user and user.profile.account: path (deactivate old account → save new → recharge_failed_transactions()). That path is already exercised for logged-in users elsewhere, so this isn't new behavior — it just extends it to the campaign/THANKS donation path. Worth a reviewer eye on the recharge_failed_transactions() side-effect for repeat donors.

Verification

  • Bug reproduced on live txns 16674 (2026-06-18, the cutover-validation payment) and 15493 (2026-04-16) — both showed "anonymous user" for a logged-in donor.
  • ⚠️ Not yet run against Stripe test-mode — should be charge-tested before merge (small change, but payment flow).

Scope

Minimal targeted fix. The broader Stripe modernization (13-yr-old API version, missing name/receipt_email/metadata) is tracked separately in #1142.


Provenance (deliberate disclosure): rbotyee+Claude diagnosis + fix; rbotyee+Codex review (in progress → LGTM); curated by @rdhyee, not yet human/test-verified.

🤖 Generated with Claude Code

rdhyee and others added 4 commits June 18, 2026 08:59
…ous)

Pay.__init__ used token-presence as a proxy for "anonymous", but logged-in
donors also submit a card token, so they were sent down make_account()'s
anonymous branch -> Customer description='anonymous user', Account.user=None.
Pass user=transaction.user (None for true anonymous), matching the other
make_account(user=...) call sites. Verified the bug reproduced on live txns
16674 (2026-06-18) and 15493 (2026-04-16).

Co-authored-by: Claude (rbotyee+Claude) <noreply@anthropic.com>
Co-authored-by: Codex (rbotyee+Codex) <noreply@openai.com>
Codex-flagged regression from the make_account(user=...) change: an anonymous
POST to a logged-in user's no-campaign transaction would mutate the owner's
Stripe account. Reject anonymous requests on user-owned transactions.

Co-authored-by: Claude (rbotyee+Claude) <noreply@anthropic.com>
Co-authored-by: Codex (rbotyee+Codex) <noreply@openai.com>
Guard now rejects BOTH anonymous and authenticated-different-user attempts to
pay a no-campaign transaction owned by another user (Codex round 2). Adds
FundViewOwnershipTest covering both cases (asserts charge() is not called).

NOTE: test authored from frontend/tests.py patterns but NOT executed in this
environment (no Django test DB/Stripe here) -- run before merge.

Co-authored-by: Claude (rbotyee+Claude) <noreply@anthropic.com>
Co-authored-by: Codex (rbotyee+Codex) <noreply@openai.com>
INSTALLED_APPS uses 'regluit.frontend'/'regluit.payment'; the patch target must
be regluit.frontend.views.PaymentManager to intercept the loaded view. Verified
against existing frontend/tests.py + settings/common.py conventions.

Co-authored-by: Claude (rbotyee+Claude) <noreply@anthropic.com>
Co-authored-by: Codex (rbotyee+Codex) <noreply@openai.com>
@rdhyee

rdhyee commented Jun 18, 2026

Copy link
Copy Markdown
Member Author

Codex collaboration → converged (rbotyee+Claude × rbotyee+Codex)

3 review rounds, each caught something real:

  1. R1: passing user=transaction.user let an anonymous request mutate an owner's Stripe account → added ownership guard.
  2. R2: an authenticated different user could still take over the txn → consolidated guard rejects both; added FundViewOwnershipTest.
  3. R3: guard confirmed correct (closes both vectors; owner / anonymous-unowned paths still proceed). Test import + @patch targets needed the regluit. prefix → fixed (verified against repo convention).

Status: guard logic Codex-LGTM'd; tests corrected per repo convention but not yet executed (no Django test env here). Will run manage.py test frontend on the rebuilt test.unglue.it (4.2) before marking ready.

@rdhyee

rdhyee commented Jun 18, 2026

Copy link
Copy Markdown
Member Author

✅ Tests PASS on real Django 4.2 (test.unglue.it)

Ran on the freshly-rebuilt test.unglue.it (Django 4.2.21, py3.12, prod-snapshot DB) — the regression test the Codex loop produced now executes green:

test_anonymous_cannot_pay_owned_transaction ... ok
test_other_user_cannot_pay_owned_transaction ... ok
Ran 2 tests in 2.319s
OK

Test DB built + migrated cleanly; both ownership-guard cases confirmed (anonymous + authenticated-mismatch rejected before charge()). manage.py check surfaced only pre-existing warnings (django-ckeditor CVE notice → tracked in #1142; MySQL strict-mode hint). Moving out of draft — guard is Codex-LGTM'd and the regression suite is green on real 4.2.

@rdhyee
rdhyee marked this pull request as ready for review June 18, 2026 16:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Logged-in donations create anonymous Stripe Customer due to token-presence check

1 participant