fix(payment): correlate PaymentIntent/Charge-referencing webhooks to the right transaction
Real Stripe TEST traffic (Phase 9.1) exposed a genuine defect the deterministic TestProvider never could: payment_intent.payment_failed and charge.refunded events reference Stripe's PaymentIntent/Charge object, never the Checkout Session, but payment_transactions. provider_reference only ever stored the Session id. A real failure or refund webhook would have silently failed to match any transaction (UNKNOWN_TRANSACTION). - payment-provider.js: session creation now also sets payment_intent_data.metadata (the same metadata already carrying our transaction_id), so it propagates onto the PaymentIntent Stripe creates. normalizeEvent() (now async) reads transaction_id back from the PaymentIntent's own metadata directly for payment_intent.payment_failed, and via one extra paymentIntents.retrieve call for charge.refunded (Charges don't inherit PaymentIntent metadata automatically). - commerce.js: the transaction lookup now matches provider_reference OR transaction_id — one additional OR clause, no schema/migration change. - webhook-routes.js: added the now-required await on verifyAndParseWebhook (TestProvider's synchronous return is unaffected). - docker-compose.dev.yml: wires PAYMENT_PROVIDER/STRIPE_SECRET_KEY/STRIPE_WEBHOOK_SECRET through to the api container (previously entirely absent — Phase 9 never needed it against the deterministic provider), empty/unset by default matching EMAIL_PROVIDER's own convention. Verified fixed against a real Stripe TEST refund (Phase 9.1); e2e-phase9.js 20/20 and the Winter Break commerce rehearsal 13/13 rerun clean after the fix.
This commit is contained in:
@@ -85,7 +85,13 @@ async function processWebhookEvent(pool, studentsAuth, provider, normalizedEvent
|
||||
return { result: 'ignored' };
|
||||
}
|
||||
|
||||
const { rows: txRows } = await pool.query(`SELECT * FROM payment_transactions WHERE provider_reference = $1`, [providerReference]);
|
||||
// Phase 9.1 real-Stripe-TEST fix — providerReference is usually the Checkout Session id, but
|
||||
// for payment_intent.payment_failed/charge.refunded StripeProvider.normalizeEvent() instead
|
||||
// returns OUR OWN transaction_id (there is no other reliable Stripe-side reference for those
|
||||
// event types back to the Session-keyed row) — match either column, never a schema change.
|
||||
const { rows: txRows } = await pool.query(
|
||||
`SELECT * FROM payment_transactions WHERE provider_reference = $1 OR transaction_id::text = $1`, [providerReference]
|
||||
);
|
||||
const transaction = txRows[0];
|
||||
if (!transaction) {
|
||||
await pool.query(`UPDATE payment_webhook_events SET result = 'error', processed_at = now() WHERE webhook_event_id = $1`, [eventId]);
|
||||
|
||||
@@ -42,13 +42,22 @@ class StripeProvider {
|
||||
success_url: successUrl,
|
||||
cancel_url: cancelUrl,
|
||||
metadata,
|
||||
// Phase 9.1 real-Stripe-TEST fix — a later payment_intent.payment_failed or
|
||||
// charge.refunded event references the PaymentIntent/Charge, never the Checkout Session
|
||||
// (a genuinely different Stripe object id, only discovered by testing against real
|
||||
// Stripe — the deterministic TestProvider always reused one caller-supplied reference for
|
||||
// every event type, which never exposed this). The Session's own `metadata` above does
|
||||
// NOT propagate to its PaymentIntent automatically; `payment_intent_data.metadata` does —
|
||||
// this is what lets normalizeEvent() below correlate those later events back to OUR OWN
|
||||
// transaction_id, not Stripe's session id.
|
||||
payment_intent_data: { metadata },
|
||||
});
|
||||
return { checkoutUrl: session.url, providerReference: session.id };
|
||||
}
|
||||
|
||||
// Real signature verification via Stripe's own SDK helper — never a hand-rolled HMAC compare.
|
||||
// Throws on a bad/forged signature; the webhook route treats that as a clean 400, never a 500.
|
||||
verifyAndParseWebhook(rawBody, signatureHeader) {
|
||||
async verifyAndParseWebhook(rawBody, signatureHeader) {
|
||||
if (!this.client || !this.webhookSecret) throw Object.assign(new Error('Stripe webhook verification is not configured.'), { code: 'PROVIDER_NOT_CONFIGURED' });
|
||||
const event = this.client.webhooks.constructEvent(rawBody, signatureHeader, this.webhookSecret);
|
||||
return this.normalizeEvent(event);
|
||||
@@ -58,14 +67,33 @@ class StripeProvider {
|
||||
// types this integration actually acts on are mapped to a real status; everything else comes
|
||||
// back with normalizedStatus: null, which webhook-routes.js records (for audit completeness)
|
||||
// but never acts on.
|
||||
normalizeEvent(event) {
|
||||
//
|
||||
// providerReference is usually the Checkout Session id (matches payment_transactions.
|
||||
// provider_reference, set at checkout creation) — but payment_intent.payment_failed and
|
||||
// charge.refunded reference the PaymentIntent/Charge instead, which was never stored anywhere
|
||||
// as a lookup key. Both are correlated back via OUR OWN transaction_id, echoed back through
|
||||
// payment_intent_data.metadata at session creation (see createCheckoutSession above);
|
||||
// commerce.js's transaction lookup matches on either provider_reference OR transaction_id.
|
||||
async normalizeEvent(event) {
|
||||
const obj = event.data.object;
|
||||
let providerReference = obj.id;
|
||||
let normalizedStatus = null;
|
||||
if (event.type === 'checkout.session.completed') { normalizedStatus = 'paid'; providerReference = obj.id; }
|
||||
else if (event.type === 'checkout.session.expired') { normalizedStatus = 'cancelled'; providerReference = obj.id; }
|
||||
else if (event.type === 'payment_intent.payment_failed') { normalizedStatus = 'failed'; providerReference = obj.metadata?.checkout_session_id || obj.id; }
|
||||
else if (event.type === 'charge.refunded') { normalizedStatus = obj.amount_refunded < obj.amount ? 'partially_refunded' : 'refunded'; providerReference = obj.payment_intent || obj.id; }
|
||||
else if (event.type === 'payment_intent.payment_failed') {
|
||||
// obj IS the PaymentIntent — its own metadata already carries our transaction_id.
|
||||
normalizedStatus = 'failed'; providerReference = obj.metadata?.transaction_id || obj.id;
|
||||
} else if (event.type === 'charge.refunded') {
|
||||
normalizedStatus = obj.amount_refunded < obj.amount ? 'partially_refunded' : 'refunded';
|
||||
// obj is a Charge — Charges do not inherit their PaymentIntent's metadata, so retrieve
|
||||
// the PaymentIntent (one extra call, only on the rare refund path) to read ours back.
|
||||
let piMetadataRef = null;
|
||||
if (obj.payment_intent) {
|
||||
try { const pi = await this.client.paymentIntents.retrieve(obj.payment_intent); piMetadataRef = pi.metadata?.transaction_id || null; }
|
||||
catch { /* fall through to the raw PaymentIntent id below */ }
|
||||
}
|
||||
providerReference = piMetadataRef || obj.payment_intent || obj.id;
|
||||
}
|
||||
return { eventId: event.id, eventType: event.type, providerReference, normalizedStatus };
|
||||
}
|
||||
}
|
||||
|
||||
@@ -20,7 +20,7 @@ module.exports = function registerWebhookRoutes(app, deps) {
|
||||
const signature = req.get('Stripe-Signature') || req.get('X-Test-Payment-Signature');
|
||||
let normalizedEvent;
|
||||
try {
|
||||
normalizedEvent = provider.verifyAndParseWebhook(req.rawBody, signature);
|
||||
normalizedEvent = await provider.verifyAndParseWebhook(req.rawBody, signature);
|
||||
} catch (e) {
|
||||
// A forged/invalid signature is a clean, quiet 400 — never a 500, never a hint about
|
||||
// WHY verification failed (timing/oracle hygiene, matching this codebase's existing
|
||||
|
||||
Reference in New Issue
Block a user