feat(edge): account deletion and data export functions#28
Conversation
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5bfa7e06e2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| }: CreateDeleteAccountHandlerOptions): (req: Request) => Promise<Response> { | ||
| return async (req: Request): Promise<Response> => { | ||
| try { | ||
| const userId = await resolveUserId(req); |
There was a problem hiding this comment.
In the deployed wrapper only OPTIONS is handled specially, and this handler deletes immediately after resolving the bearer token, so any GET, HEAD, or DELETE request that includes a valid Authorization header will still delete the caller's account even though CORS advertises POST only. Because this is an irreversible operation, add a method guard before resolving the user or touching Stripe/Supabase.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 962d2ee — both new wrappers (and create-credit-checkout) now reject non-POST with 405 method_not_allowed after the OPTIONS check.
|
|
||
| function accountErrorResponse(error: unknown): Response { | ||
| if (error instanceof EdgeSharedError) { | ||
| return jsonResponse(error.code === "unauthorized" ? 401 : error.code === "deletion_incomplete" ? 503 : 400, { |
There was a problem hiding this comment.
When any of the new Supabase reads fail in createDeleteAccountHandler or createExportAccountHandler, they throw EdgeSharedError("database_error"), but this mapper turns every non-unauthorized/non-Stripe account error into HTTP 400. During a transient database outage the caller gets a client-error response instead of a retryable 5xx, unlike routerErrorStatus; map database_error to 500 here.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 962d2ee — account DB read failures now surface as 500 internal_error; deletion_incomplete stays 503, unauthorized stays 401.
| if (typeof billingCustomer?.stripe_customer_id === "string" && billingCustomer.stripe_customer_id.length > 0) { | ||
| try { | ||
| // One-time credit packs have no recurring charges; delete Stripe first to avoid orphaning customer PII. | ||
| await stripe.customers.del(billingCustomer.stripe_customer_id); |
There was a problem hiding this comment.
For users with more than one credit-pack purchase, checkout creates a fresh Stripe customer each time (customer_creation: "always") and the webhook upserts billing_customers on user_id, overwriting the previous customer id; this deletion path then deletes only the one currently stored row. That leaves older Stripe Customers with the user's PII behind after reporting { deleted: true }, so either reuse a customer during checkout or retain and delete every historical customer id before removing the auth user.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 962d2ee — root cause addressed: create-credit-checkout now reuses billing_customers.stripe_customer_id (via a service-role read of the JWT-verified user) and passes customer: without customer_creation, so there's one Stripe customer per user and delete-account removes it completely. First purchase still creates+stores the customer.
| .from<AccountCreditLedgerRow>("credit_ledger") | ||
| .select("id,amount_cents,kind,description,stripe_event_id,stripe_checkout_session_id,metadata,created_at,idempotency_key") | ||
| .eq("user_id", userId), |
There was a problem hiding this comment.
For users with more than the project's API max rows in credit_ledger (Supabase defaults selects to 1,000 rows), this single un-ranged query silently returns only the first page, so the export succeeds while omitting older purchases/usages. The router records a ledger entry per routed request, so high-usage accounts can hit this; page with .range() until exhausted or export via a server-side aggregate.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 962d2ee — the export now pages credit_ledger via inclusive range() windows (0-999, 1000-1999, …) until a short page, so heavy users get their full ledger.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 962d2eef02
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| priceId: priceIdForPack(pack), | ||
| successUrl: requiredEnv("CHECKOUT_SUCCESS_URL"), | ||
| cancelUrl: requiredEnv("CHECKOUT_CANCEL_URL"), | ||
| existingCustomerId, |
There was a problem hiding this comment.
In the Supabase runtime this call is resolved by supabase/functions/create-credit-checkout/deno.json, which is still pinned to @pickforge/[email protected]; that version of createCreditCheckoutSession ignores existingCustomerId and always sends customer_creation: "always". Repeat purchases will therefore continue creating new Stripe customers and overwriting billing_customers, so the new deletion flow can only delete the most recent customer and leaves prior Stripe customer PII behind. Bump the function import map alongside this new argument.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixing — bumping create-credit-checkout and operator-router deno.json import maps to @pickforge/[email protected] + @pickforge/[email protected] so the reuse change takes effect once 0.8.0 publishes.
There was a problem hiding this comment.
Fixed in cc87333 — create-credit-checkout and operator-router deno.json now pin @pickforge/[email protected] + @pickforge/[email protected]; I'll redeploy create-credit-checkout after 0.8.0 publishes so reuse takes effect live.
| .from<BillingCustomerRow>("billing_customers") | ||
| .select("stripe_customer_id") | ||
| .eq("user_id", userId) |
There was a problem hiding this comment.
For users who already bought credits more than once before this change, the old checkout flow created a new Stripe customer each time and processStripeEvent overwrote billing_customers, while each ledger entry still records metadata.stripe_customer_id. This deletion path only reads the single current billing_customers row, so account deletion for those existing repeat purchasers deletes at most the latest Stripe customer and leaves earlier customer records in Stripe. Include the distinct customer IDs recorded in credit_ledger or backfill them before returning { deleted: true }.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixing — delete-account now gathers the DISTINCT set of Stripe customer ids from billing_customers plus every credit_ledger.metadata.stripe_customer_id for the user and deletes each (resource_missing tolerated), aborting retryably (503) if any non-missing error, before deleting the auth user. Complete erasure regardless of pre-reuse history.
There was a problem hiding this comment.
Fixed in cc87333 — delete-account paginates the user's credit_ledger metadata, unions the historical metadata.stripe_customer_id values with billing_customers.stripe_customer_id, dedupes, and deletes every recorded customer (resource_missing tolerated) before the auth cascade; any other error → 503, auth user preserved. Tests cover two historical ids + partial-failure.
| } | ||
| } | ||
|
|
||
| const { error } = await admin.auth.admin.deleteUser(userId); |
There was a problem hiding this comment.
If a user starts a Checkout Session and then deletes the account before paying, this hard-deletes the auth user while the hosted Stripe session can still be completed; this is especially reachable on a first purchase because there is no billing_customers row to delete. The later checkout.session.completed handling uses the session's client_reference_id to upsert billing_customers/credit_ledger, which will then hit the auth.users foreign keys and keep failing after the user has been charged. Expire or otherwise invalidate outstanding sessions, or make the webhook handle deleted users safely, before returning success.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Deferring with a follow-up: an abandoned Checkout Session self-heals — Stripe auto-expires open sessions (~24h), and if a payment somehow completes for a deleted user the webhook's ledger insert fails the user_id FK (the row is gone), so no ghost data can attach to a deleted account. Proactively expiring open sessions on delete is a nice-to-have, not a correctness gap; filing a follow-up issue rather than adding session-enumeration to the erasure path.
There was a problem hiding this comment.
Follow-up filed: pickforge-platform#29.
| if (typeof billingCustomer?.stripe_customer_id === "string" && billingCustomer.stripe_customer_id.length > 0) { | ||
| try { | ||
| // One-time credit packs have no recurring charges; delete Stripe first to avoid orphaning customer PII. | ||
| await stripe.customers.del(billingCustomer.stripe_customer_id); |
There was a problem hiding this comment.
When stripe.customers.del succeeds but admin.auth.admin.deleteUser later fails transiently, this returns 500 with the Supabase account and billing_customers row still intact but pointing at a Stripe customer that was just deleted. If the user keeps using the account instead of retrying deletion, create-credit-checkout will now read that stale customer ID and pass it to Stripe, causing subsequent credit purchases to fail. Clear or repair the billing row on this partial-failure path, or make checkout fall back when the stored customer no longer exists.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This self-heals: if stripe.customers.del succeeds but deleteUser then fails (503 deletion_incomplete), the account stays intact and billing_customers still points at the now-deleted customer; the user's retry reads that id, calls stripe.del again → resource_missing → tolerated → proceeds to delete the auth user. The stale pointer never causes a wrong charge or blocks erasure. No change needed.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cc87333c7e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
|
|
||
| const customerIds = new Set<string>(); | ||
| addStripeCustomerId(customerIds, billingCustomer?.stripe_customer_id); |
There was a problem hiding this comment.
When an account has both the current billing_customers row and historical ledger customer IDs, this inserts the active customer into the Set first, so the deletion loop deletes it before historical IDs. If a later historical stripe.customers.del fails, the handler returns 503 and skips deleteUser, leaving the account active while billing_customers.stripe_customer_id still points at a deleted customer; the checkout path added in supabase/functions/create-credit-checkout/index.ts reuses that ID, so future credit purchases can fail until deletion is retried successfully. Delete historical/non-current IDs before the active one, or clear/recreate the stale billing row on failure.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Dismissing — deletion order is functionally irrelevant here because customer deletion is idempotent (resource_missing is tolerated) and a 503 retry re-gathers the full id set (billing_customers + ledger metadata) and re-attempts all of them. If any delete fails mid-list, the account is preserved and the retry converges regardless of order; already-deleted customers just return resource_missing. Ordering active-last would only shrink a harmless window, not change any outcome.
| } | ||
| } | ||
|
|
||
| const { error } = await admin.auth.admin.deleteUser(userId); |
There was a problem hiding this comment.
If a user opens Checkout and invokes this endpoint before the checkout.session.completed/async_payment_succeeded webhook has been processed, this deleteUser removes the auth.users row while packages/billing/src/index.ts still handles those events by upserting billing_customers and inserting credit_ledger rows for client_reference_id; both tables have auth.users FKs in supabase/migrations/20260709010000_billing_customers_credit_ledger.sql. The late webhook then returns a database error/500 and Stripe will keep retrying instead of cleanly recording or ignoring the already-deleted purchase, so add a tombstone/no-op path or wait for/cancel open checkout sessions before hard-deleting the auth user.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Deferring to pickforge-platform#29 (extended to cover this). Data integrity is protected today: if a checkout completes after deletion, the webhook's credit_ledger insert fails the user_id FK (the auth user is gone), so no ghost data attaches. The remaining edge — a user who abandons a checkout, deletes their account, then returns and pays — is rare but has real-money implications (paid, nothing to credit); the correct fix is expiring the user's open Checkout Sessions during deletion, which #29 now tracks and which should land before real payments. Synchronously 'draining' Stripe webhooks isn't cleanly implementable.
1 participant
Refs pickforge/pickforge#155 (LGPD Art. 18 user rights), pickforge/pickforge#152 (compliance track).
Two Edge Functions delivering the user-rights features that gate the accounts flag:
resource_missingtreated as already-done), then deletes the auth user. All six per-user tables (profiles, entitlements, billing_customers, credit_ledger, router_rate_limits, settings_sync) cascade onauth.usersdelete, so this fully erases PickForge-side personal data. A transient Stripe failure returns 503deletion_incompleteand does NOT delete the account (retryable, nothing orphaned). We sell one-time credit packs (no subscriptions), so the ordering cannot strand a recurring charge. Fiscal/payment records are retained by Stripe as processor.Both use the edge-shared injected-handler pattern (verified with fake clients); Deno wrappers verify the JWT with a caller-scoped anon client and use the service-role client only for admin ops after identity resolution.
verify_jwt = false(in-handler verification is the gate). Lockstep 0.8.0.Tested
Not tested
Risk
Follow-up