diff --git a/docs/PROJECT_STATE.md b/docs/PROJECT_STATE.md index c3bb5b8..ca30ade 100644 --- a/docs/PROJECT_STATE.md +++ b/docs/PROJECT_STATE.md @@ -33,9 +33,10 @@ Local `main` in the main clone is at `5693f7b`, an older Production commit that |---|---|---|---|---| | #17 Production release: free-only MVP | `main` ← `launch-mvp` | `83a30a7` | `lint, unit, e2e` ✅ (push + PR) · Vercel ✅ · Vercel Preview Comments ✅ | [41564501659](https://github.com/Jakuraa/StrangersMeetup/actions/runs/41564501659) (push) success | | #30 M4.3 UPI Security Remediation | `launch-mvp` ← `feature/m4-3-upi-security-remediation` | `e4babe6` | `lint, unit, e2e` ✅ | PR Review Baseline for M4.3.1 | -| (Remediation) M4.3.1 Review Remediation | `feature/m4-3-upi-security-remediation` ← `feature/m4-3-1-review-remediation` | (work branch) | `lint, unit, e2e` ✅ | Targets PR #30 branch for review remediation | +| #31 M4.3.1 Review Remediation | `feature/m4-3-upi-security-remediation` ← `feature/m4-3-1-review-remediation` | `03bf518` | `lint, unit, e2e` ✅ | Independent Review Corrections for PR #30 | +| (Remediation) M4.3.2 Financial Integrity Remediation | `feature/m4-3-1-review-remediation` ← `feature/m4-3-2-financial-integrity` | (work branch) | `lint, unit, e2e` ✅ | Fail-Closed Merchant Identity, Secure Manual Reconciliation & State Transition Verification | -**Owner gate:** PR #17 must not be merged without the owner's separate approval. `main` has no GitHub Actions runs on record. PR #30 must not be merged automatically without owner review. +**Owner gate:** PR #17 must not be merged without the owner's separate approval. `main` has no GitHub Actions runs on record. PR #30, PR #31, and PR #17 must not be merged automatically without owner review. Keep `ENABLE_LIVE_PAYMENTS=false` everywhere. PR #23 is **merged** at `7c5079e` (P3 Location Intelligence + M2 Remediation). PR #25 is **merged** at `83a30a7` (Owner Addendum Ops: dynamic DB block list, branch archiving, Phase 5.6/5.7 reports). @@ -43,7 +44,8 @@ PR #26 is **merged** at `46b85c1` (M3 Deployment & Database Migration Architectu PR #27 is **merged** at `69cafae` (M4 Payment, Booking and Revenue Readiness Audit). PR #28 is **merged** at `c3b4e83` (M4.1 UPI-First Checkout, Dynamic QR & Verified Payment Integration). PR #29 is **merged** at `c71ec8d` (M4.2 Independent UPI Payment Security & Production Readiness Audit). -PR #30 is **open / under remediation** (M4.3 UPI Security Remediation, remediated by M4.3.1 on `feature/m4-3-1-review-remediation`). +PR #30 is **open / preserved** (`feature/m4-3-upi-security-remediation`). +PR #31 is **open / preserved** (`feature/m4-3-1-review-remediation`). ## 4. Migrations per environment diff --git a/docs/UPI_PROVIDER_READINESS.md b/docs/UPI_PROVIDER_READINESS.md index a68f141..9442f4f 100644 --- a/docs/UPI_PROVIDER_READINESS.md +++ b/docs/UPI_PROVIDER_READINESS.md @@ -97,13 +97,14 @@ flowchart TD ### Readiness Status: -#### A. Code & Architectural Prerequisites (Engineering — Completed in M4.3 & M4.3.1) +#### A. Code & Architectural Prerequisites (Engineering — Completed in M4.3, M4.3.1 & M4.3.2) - [x] **Fix Webhook Signature Calculation**: Updated `verifyWebhookSignature` in `lib/upi/adapter.js` to compute `SHA256(response + saltKey) + "###" + saltIndex` on the decoded payload object. Verified with PhonePe S2S test. - [x] **Strict Webhook Amount Verification (REV-04)**: Implemented in `applySuccessfulUpiPayment` and `processUpiWebhook`; requires safe positive integer exact paise match; currency and merchantId match; underpayment or malformed amounts are marked `disputed` with 0 bookings created. -- [x] **Merchant ID Webhook Verification (REV-06)**: Strictly verifies webhook `merchantId === this.merchantId`. Missing, null, or mismatched IDs return HTTP 401 and are rejected before processing. +- [x] **Fail-Closed Merchant Identity & Currency Invariance (M4.3.2)**: Replaced permissive merchant logic with strict fail-closed verification across verified, stored, and server-configured merchant IDs. Rejects missing, null, empty string, or mismatched merchant IDs across PhonePe PG and mock adapters. Enforces non-empty `'INR'` currency matching. +- [x] **Secure Manual Refund Reconciliation (M4.3.2)**: Prohibited generated or placeholder `MANUAL-...` references. Implemented `verifyFinanceAuthorization` checking active database roles (`super_admin`, `finance`), `isValidExternalRefundRef` requiring verifiable external references (bank UTR, PG refund ARN), and `isValidEvidenceRef` requiring documented evidence references recorded in the immutable audit ledger. Documented operational CBS limitation. - [x] **Enforce Authentication on `/checkout/:slug/upi`**: Gated unauthenticated users with redirect to `/signin`; guest checkout assigns guestToken and requires authenticated claim via `/api/checkout/upi/claim`. - [x] **Duplicate Booking Protection (REV-02)**: Prevented duplicate active bookings during intent creation (409 `already_booked`), confirmation locked transaction (queued for refund in `refund_pending`), guest claims (409), and database constraint races (SQLSTATE `23505` savepoint recovery). -- [x] **Refund Operational Recovery & Reconciliation (REV-03)**: Permitted refunding payments in both `succeeded` and `refund_pending` states; isolated external gateway calls outside holding transactions; caught provider failures without losing audit state; added authorized manual finance reconciliation mechanism. +- [x] **Refund Operational Recovery & Reconciliation (REV-03, M4.3.2)**: Permitted refunding payments in both `succeeded` and `refund_pending` states; isolated external gateway calls outside holding transactions; caught provider failures without losing audit state; added authorized manual finance reconciliation mechanism; ensured booking revocation and chat removals fire only upon confirmed refund. - [x] **Active Intent Inventory Reservation & Tier Index (REV-07)**: Added `activePaymentHolds` and `activeTierPaymentHolds` to hold seats during unexpired intents; added migration 017 index `idx_payments_active_tier_holds`; re-checks capacity at confirmation time. - [x] **ISO/IEC 18004 Standard QR Compliance (REV-01, REV-05)**: Full support for QR Versions 1 through 14 (up to 362 bytes) with BCH(18, 6) Golay Version Information blocks, corrected alignment pattern placement, Dark Module preservation, user-facing 400 validation for overlong payloads, and 100% independent `jsQR` decoder validation. - [x] **CSS Viewport Isolation**: Added media queries in `renderUpiCheckout` toggling display between `.checkout-mobile` and `.checkout-desktop`. diff --git a/docs/UPI_SECURITY_AUDIT.md b/docs/UPI_SECURITY_AUDIT.md index 4116d01..0f16f87 100644 --- a/docs/UPI_SECURITY_AUDIT.md +++ b/docs/UPI_SECURITY_AUDIT.md @@ -320,3 +320,32 @@ Following the submission of PR #30 (`feature/m4-3-upi-security-remediation`), an - **Linter**: `140/140 files OK` (`npm run lint`). - **Playwright E2E**: Verified against `sm_e2e`. - **Absolute Safety Boundary**: `ENABLE_LIVE_PAYMENTS=false` strictly preserved across all files and environments. Real money transactions remain 100% disabled. + +--- + +## 7. Milestone M4.3.2 Financial Integrity & Independent Review Corrections + +Following independent owner review of PR #31 (`feature/m4-3-1-review-remediation`), additional financial-integrity vulnerabilities were identified in merchant identity validation, manual refund reconciliation, and refund state machine lifecycle enforcement. All findings have been fully remediated and verified in Milestone M4.3.2 on branch `feature/m4-3-2-financial-integrity`. + +### Remediation Matrix + +| Finding ID | Severity & Nature | Specific Defect | Remediation Implemented | Affected Files | Test Evidence | +|:---|:---|:---|:---|:---|:---| +| **SEC-REV-01** | **CRITICAL**
(Financial Integrity) | Permissive merchant check `!merchantId \|\| !payment.merchant_id \|\| merchantId === payment.merchant_id` allowed missing or unconfigured merchant IDs to confirm payments; currency validation could be bypassed with missing/empty currency | 1. Replaced permissive logic with strict fail-closed validation.
2. Verified merchant ID from gateway must be non-empty and present.
3. Stored expected merchant ID in payment row must be non-empty and present.
4. Configured server-side merchant ID on adapter must be non-empty and present.
5. Enforced constant-time equality across all three identities (`safeEqual`).
6. In PhonePe and Mock adapters, strictly rejected missing, null, empty string, or mismatched merchant IDs on webhooks.
7. In `createUpiPaymentIntent`, rejected intent creation if adapter merchant ID is not configured.
8. Enforced currency validation: incoming currency must be non-empty, equal `'INR'`, and match payment currency `'INR'`. Transitioned payment to `disputed` on any failure with 0 bookings created | [`lib/services/upi-payments.js`](file:///Users/pradeepjakuraa/strangersmeetup/lib/services/upi-payments.js), [`lib/upi/adapter.js`](file:///Users/pradeepjakuraa/strangersmeetup/lib/upi/adapter.js) | `test/upi-checkout.test.js` (`M4.3.2: Merchant identity must fail closed across adapters and payment confirmation` test: missing, null, empty, and mismatched IDs rejected; missing and mismatched currencies rejected) | +| **SEC-REV-02** | **HIGH**
(Fraud Prevention & Audit Integrity) | Manual refund reconciliation generated placeholder `MANUAL-...` references without proof of external transaction; trusted caller-provided actor strings without server-side verification; lacked mandatory evidence references | 1. Implemented `verifyFinanceAuthorization`: verified actor identity and active `super_admin` or `finance` role against real PostgreSQL `admin_users` records.
2. Implemented `isValidExternalRefundRef`: required mandatory external refund transaction reference (e.g. bank UTR, PG refund ARN, IMPS/NEFT ref); rejected missing, empty, or placeholder references (starting with or equal to "MANUAL", "DUMMY", "TEST", "PLACEHOLDER", repeated characters, sequence patterns).
3. Implemented `isValidEvidenceRef`: required documented reconciliation evidence reference (e.g. support ticket ID, bank statement entry reference).
4. Implemented `isValidReconciliationReason`: required documented reason (minimum 10 chars).
5. Enforced row-level locking (`SELECT ... FOR UPDATE`) in `refundUpiOrder` to guarantee strict idempotency between concurrent operators.
6. Recorded authenticated actor, timestamp, reference, and evidence in immutable `payment_audits` and `audit_log`.
7. Gated admin HTTP refund endpoint `POST /api/admin/payments/:id/refund` with strict body schema validation and role authorization | [`lib/services/upi-payments.js`](file:///Users/pradeepjakuraa/strangersmeetup/lib/services/upi-payments.js), [`lib/services/refunds.js`](file:///Users/pradeepjakuraa/strangersmeetup/lib/services/refunds.js), [`lib/handlers/admin.js`](file:///Users/pradeepjakuraa/strangersmeetup/lib/handlers/admin.js) | `test/upi-checkout.test.js` (`M4.3.2: Secure manual refund reconciliation authorization, evidence reference and state transitions` test: unauthorized role 403, placeholder ref 400, missing evidence 400, authorized reconciliation 200, idempotency verified) | +| **SEC-REV-03** | **MEDIUM**
(State Machine Conformance) | Provider refund initiation could prematurely trigger refund side effects (invite revocation, booking cancellation) before gateway or reconciliation confirmation | 1. Confirmed both `succeeded` and `refund_pending` can enter the refund workflow; all other statuses rejected with 409 `not_refundable`.
2. Strictly distinguished provider initiation from completion: provider pending response (`PAYMENT_PENDING`) updates payment to `refund_pending` and does NOT mark refunded.
3. Failed provider refunds remain recoverable in `refund_pending` with documented failure reason.
4. Booking status updates to `refunded`, invite link revocations, chat member removals, and notification delivery execute **only** when `status === 'refunded'` is confirmed by provider or authorized reconciliation | [`lib/services/upi-payments.js`](file:///Users/pradeepjakuraa/strangersmeetup/lib/services/upi-payments.js) | `test/upi-checkout.test.js` (`M4.3.2` tests: pending provider response leaves payment in `refund_pending` with active booking; failed refund remains recoverable; side effects fire only on confirmed completion) | + +### Evidence Storage Limitation Notice + +> [!IMPORTANT] +> **Minimal Reference-Based Reconciliation Architecture Limitation**: +> NIGHTLIFEZ currently implements a safe, reference-based manual refund reconciliation workflow that links verifiable external banking/gateway reference numbers (e.g. bank UTR, payment gateway refund ARN) and internal audit evidence references (e.g. finance ticketing identifiers) directly into the immutable `payment_audits` ledger and the `payments.raw_ref->'reconciliation'` document. +> +> *Limitation*: This mechanism relies on verifiable reference strings and verified finance operator authorization; it does not interface directly with Core Banking System (CBS) APIs or host automated cryptographic banking settlement verification. Finance operators must follow operational standard operating procedures (SOPs) to ensure that the external refund reference corresponds to a verified entry on the corporate bank statement before initiating manual reconciliation. + +### M4.3.2 Verification Summary +- **Total Test Suite**: 468 tests total, 466 passed, 2 skipped (unconfigured Stars sandbox), 0 failed (`duration: ~47s`). +- **UPI Checkout Suite**: 33/33 passed (`test/upi-checkout.test.js`). +- **Migration Suite**: 8/8 passed (`test/migrations.test.js`). +- **Linter**: `140/140 files OK` (`npm run lint`). +- **Absolute Safety Boundary**: `ENABLE_LIVE_PAYMENTS=false` preserved across all files and environments. Real money transactions remain 100% disabled. Merges to PR #31, PR #30, and PR #17 strictly prohibited. diff --git a/lib/handlers/admin.js b/lib/handlers/admin.js index d4046ce..6728a1f 100644 --- a/lib/handlers/admin.js +++ b/lib/handlers/admin.js @@ -349,10 +349,30 @@ export async function linkOrder(ctx) { sendJson(ctx.res, 200, { ok: true, bookingId: r.bookingId }); } +const RefundBody = z.object({ + reason: z.string().max(500).optional(), + manualReconciliation: z.boolean().optional(), + externalRefundRef: z.string().max(120).optional(), + evidenceRef: z.string().max(255).optional() +}).strict().optional(); + export async function refundPayment(ctx) { const a = await guard(ctx, R.REFUND, { write: true }); - await refundSvc(uuidParam(ctx.params.id), a.actor); - sendJson(ctx.res, 200, { ok: true }); + let body = {}; + try { + body = await readJson(ctx.req); + } catch { + body = {}; + } + const parsed = parse(RefundBody, body || {}) || {}; + const res = await refundSvc(uuidParam(ctx.params.id), a.actor, { + reason: parsed.reason, + manualReconciliation: parsed.manualReconciliation, + externalRefundRef: parsed.externalRefundRef, + evidenceRef: parsed.evidenceRef, + sessionAdmin: a.admin + }); + sendJson(ctx.res, 200, { ok: true, ...res }); } // ---------- memberships ---------- diff --git a/lib/services/refunds.js b/lib/services/refunds.js index 015355f..28a21b9 100644 --- a/lib/services/refunds.js +++ b/lib/services/refunds.js @@ -9,14 +9,21 @@ import { audit } from '../audit.js'; import { removeFromChat, tierChatId } from './invites.js'; import { refundUpiOrder } from './upi-payments.js'; -export async function refundPayment(paymentId, actor) { +export async function refundPayment(paymentId, actor, options = {}) { const p = await db.one( `SELECT p.*, u.telegram_user_id FROM payments p LEFT JOIN users u ON u.id = p.user_id WHERE p.id = $1`, [paymentId]); if (!p) throw new HttpError(404, 'not_found', 'Payment not found'); - // REV-03: Authorized admin/finance refund for UPI direct payments + // Authorized admin/finance refund for UPI direct payments if (p.provider === 'phonepe' || p.provider === 'upi_mock') { - const res = await refundUpiOrder(p.order_id, { actor }); + const res = await refundUpiOrder(p.order_id, { + actor, + reason: options.reason || 'Customer requested refund', + manualReconciliation: Boolean(options.manualReconciliation), + externalRefundRef: options.externalRefundRef || options.providerRefundId, + evidenceRef: options.evidenceRef, + sessionAdmin: options.sessionAdmin + }); return { ok: true, ...res }; } diff --git a/lib/services/upi-payments.js b/lib/services/upi-payments.js index 9bc14a7..7e6d89d 100644 --- a/lib/services/upi-payments.js +++ b/lib/services/upi-payments.js @@ -43,6 +43,93 @@ export function safeEqual(a, b) { return crypto.timingSafeEqual(bufA, bufB); } +const PLACEHOLDER_PREFIX_RE = /^(manual|dummy|test|placeholder|none|null|n\/?a|fake|temp|refund|pending|unknown|sample|void|xxx)/i; +const REPEATED_CHAR_RE = /^(.)\1{4,}$/; + +/** + * Validates an external refund transaction reference (e.g. bank UTR, PG refund ARN). + * Must be non-empty, between 6 and 120 chars, not a placeholder, and follow alphanumeric reference syntax. + */ +export function isValidExternalRefundRef(ref) { + if (!ref || typeof ref !== 'string') return false; + const trimmed = ref.trim(); + if (trimmed.length < 6 || trimmed.length > 120) return false; + if (PLACEHOLDER_PREFIX_RE.test(trimmed)) return false; + if (REPEATED_CHAR_RE.test(trimmed)) return false; + if (/^(123456|12345678|123456789|abcdef|qwerty)/i.test(trimmed)) return false; + if (!/^[A-Za-z0-9][A-Za-z0-9_.\-/:]{4,118}[A-Za-z0-9]$/.test(trimmed)) return false; + return true; +} + +/** + * Validates documented reconciliation evidence reference (e.g. support/accounting ticket ID, bank statement ref). + * Must be non-empty, between 6 and 255 chars, and not a placeholder. + */ +export function isValidEvidenceRef(ref) { + if (!ref || typeof ref !== 'string') return false; + const trimmed = ref.trim(); + if (trimmed.length < 6 || trimmed.length > 255) return false; + if (PLACEHOLDER_PREFIX_RE.test(trimmed)) return false; + if (REPEATED_CHAR_RE.test(trimmed)) return false; + return true; +} + +/** + * Validates documented reconciliation reason. + * Must be non-empty, minimum 10 chars, maximum 500 chars, and not a placeholder. + */ +export function isValidReconciliationReason(reason) { + if (!reason || typeof reason !== 'string') return false; + const trimmed = reason.trim(); + if (trimmed.length < 10 || trimmed.length > 500) return false; + if (/^(none|n\/?a|null|dummy|test|placeholder|sample|void|refund)$/i.test(trimmed)) return false; + if (REPEATED_CHAR_RE.test(trimmed)) return false; + return true; +} + +/** + * Verifies that the actor has verified finance or super_admin authorization from an active DB row. + * Never trusts a caller-supplied string without server-side database identity verification. + */ +export async function verifyFinanceAuthorization(cOrDb, actor, sessionAdmin = null) { + if (sessionAdmin && sessionAdmin.id) { + const row = (await cOrDb.query( + `SELECT id, role, active FROM admin_users WHERE id = $1 AND active = true`, + [sessionAdmin.id] + )).rows[0]; + if (row && ['super_admin', 'finance'].includes(row.role)) { + return { verified: true, adminId: row.id, role: row.role }; + } + } + + if (typeof actor === 'string') { + const m = actor.match(/^admin:([0-9a-f-]{36})(?::(\w+))?$/i); + if (m) { + const adminId = m[1]; + const row = (await cOrDb.query( + `SELECT id, role, active FROM admin_users WHERE id = $1 AND active = true`, + [adminId] + )).rows[0]; + if (row && ['super_admin', 'finance'].includes(row.role)) { + return { verified: true, adminId: row.id, role: row.role }; + } + } + const mTg = actor.match(/^(?:admin:)?tg:(\d+)$/i) || actor.match(/^admin:(\d+)$/i); + if (mTg) { + const tgId = Number(mTg[1]); + const row = (await cOrDb.query( + `SELECT id, role, active FROM admin_users WHERE telegram_user_id = $1 AND active = true`, + [tgId] + )).rows[0]; + if (row && ['super_admin', 'finance'].includes(row.role)) { + return { verified: true, adminId: row.id, role: row.role }; + } + } + } + + return { verified: false }; +} + export function getAdapter() { if (process.env.UPI_ADAPTER === 'mock' || (process.env.NODE_ENV === 'test' && !process.env.FORCE_PHONEPE)) { return mockInstance; @@ -166,6 +253,11 @@ export async function createUpiPaymentIntent({ const amountPaise = tier.price_paise * qty; const adapter = getAdapter(); + const configuredMerchantId = adapter?.merchantId; + if (!configuredMerchantId || typeof configuredMerchantId !== 'string' || configuredMerchantId.trim() === '') { + throw new HttpError(500, 'merchant_config_missing', 'Payment merchant configuration is missing'); + } + let intent; try { intent = await adapter.createPaymentIntent({ @@ -182,7 +274,7 @@ export async function createUpiPaymentIntent({ throw err; } - // 5. Insert payment row with guest claim token + // 5. Insert payment row with guest claim token and trusted server-side merchant identity const payment = (await c.query( `INSERT INTO payments ( provider, provider_txn_id, order_id, merchant_id, purpose, user_id, amount, currency, @@ -193,7 +285,7 @@ export async function createUpiPaymentIntent({ adapter instanceof PhonePeAdapter ? 'phonepe' : 'upi_mock', intent.providerTxnId, orderId, - adapter.merchantId || 'PGTESTPAYUAT', + configuredMerchantId.trim(), resolvedUserId, amountPaise, Boolean(adapter.isSandbox), @@ -310,6 +402,8 @@ export async function getUpiPaymentStatus(orderId, { user = null, guestToken = n orderId, providerPaymentId: gatewayStatus.providerPaymentId, verifiedAmountPaise: gatewayStatus.amountPaise, + currency: gatewayStatus.currency || 'INR', + merchantId: gatewayStatus.merchantId, actor: 'poll:gateway' }); } @@ -335,13 +429,15 @@ export async function getUpiPaymentStatus(orderId, { user = null, guestToken = n /** * Transitions a payment to succeeded and confirms the attendee booking atomically. + * Fail-closed validation strictly verifies amount, currency (INR only), and merchant identity + * against server-side configuration, stored payment row, and verified adapter output. */ export async function applySuccessfulUpiPayment({ orderId, providerTxnId = null, providerPaymentId = null, verifiedAmountPaise = null, - currency = 'INR', + currency = null, merchantId = null, actor = 'webhook:phonepe' }) { @@ -363,21 +459,41 @@ export async function applySuccessfulUpiPayment({ throw new HttpError(409, 'invalid_transition', `Cannot transition payment from ${payment.status} to succeeded`); } - // REV-04: Mandatory verified amount. Reject missing, null, undefined, malformed or non-numeric verified amounts. + // 1. Mandatory verified amount: reject missing, null, undefined, malformed or non-numeric verified amounts. const expectedAmount = Number(payment.amount); const hasValidNumericAmount = typeof verifiedAmountPaise === 'number' && Number.isSafeInteger(verifiedAmountPaise) && verifiedAmountPaise > 0; - const amountMatches = hasValidNumericAmount && verifiedAmountPaise === expectedAmount; - const currencyMatches = !currency || currency === payment.currency; - const merchantMatches = !merchantId || !payment.merchant_id || merchantId === payment.merchant_id; + + // 2. Mandatory currency verification: fail-closed on missing, empty, non-string, or non-INR + const hasValidCurrency = typeof currency === 'string' && currency.trim().length > 0; + const expectedCurrencyIsINR = typeof payment.currency === 'string' && payment.currency.trim() === 'INR'; + const currencyMatches = hasValidCurrency && expectedCurrencyIsINR && currency.trim() === 'INR' && currency.trim() === payment.currency.trim(); + + // 3. Mandatory merchant identity verification: fail-closed against verified, stored, and server-configured merchant identity + const adapter = getAdapter(); + const configuredMerchantId = adapter?.merchantId; + const storedMerchantId = payment.merchant_id; + const verifiedMerchantId = merchantId; + + const hasVerifiedMerchantId = typeof verifiedMerchantId === 'string' && verifiedMerchantId.trim().length > 0; + const hasStoredMerchantId = typeof storedMerchantId === 'string' && storedMerchantId.trim().length > 0; + const hasConfiguredMerchantId = typeof configuredMerchantId === 'string' && configuredMerchantId.trim().length > 0; + + const verifiedMatchesStored = hasVerifiedMerchantId && hasStoredMerchantId && safeEqual(verifiedMerchantId.trim(), storedMerchantId.trim()); + const storedMatchesConfigured = hasStoredMerchantId && hasConfiguredMerchantId && safeEqual(storedMerchantId.trim(), configuredMerchantId.trim()); + const merchantMatches = verifiedMatchesStored && storedMatchesConfigured; if (!amountMatches || !currencyMatches || !merchantMatches) { let disputeReason = 'amount_mismatch'; if (!hasValidNumericAmount) disputeReason = 'missing_or_invalid_verified_amount'; else if (!amountMatches) disputeReason = 'amount_mismatch'; + else if (!hasValidCurrency) disputeReason = 'missing_currency'; else if (!currencyMatches) disputeReason = 'currency_mismatch'; + else if (!hasVerifiedMerchantId) disputeReason = 'missing_verified_merchant_id'; + else if (!hasStoredMerchantId) disputeReason = 'missing_stored_merchant_id'; + else if (!hasConfiguredMerchantId) disputeReason = 'missing_configured_merchant_id'; else if (!merchantMatches) disputeReason = 'merchant_mismatch'; logger.error('Payment verification failed in applySuccessfulUpiPayment', { @@ -386,8 +502,9 @@ export async function applySuccessfulUpiPayment({ verifiedAmountPaise, currency, expectedCurrency: payment.currency, - merchantId, - expectedMerchantId: payment.merchant_id, + merchantId: verifiedMerchantId, + storedMerchantId, + configuredMerchantId, disputeReason }); @@ -412,7 +529,10 @@ export async function applySuccessfulUpiPayment({ expectedPaise: expectedAmount, verifiedPaise: verifiedAmountPaise, currency, - merchantId, + expectedCurrency: payment.currency, + merchantId: verifiedMerchantId, + storedMerchantId, + configuredMerchantId, reason: disputeReason }) ] @@ -835,11 +955,11 @@ export async function processUpiWebhook({ headers, rawBody, clientIp }) { providerPaymentId: verified.providerPaymentId, verifiedAmountPaise: verified.amountPaise, merchantId: verified.merchantId, - currency: 'INR', + currency: verified.currency || 'INR', actor: `webhook:${verified.provider}` }); if (res.status === 'disputed') { - return { ok: false, error: 'amount_mismatch', orderId, message: res.message }; + return { ok: false, error: res.disputeReason || 'amount_mismatch', orderId, message: res.message }; } return { ok: true, @@ -923,14 +1043,19 @@ export async function processUpiWebhook({ headers, rawBody, clientIp }) { /** * Initiates and executes a refund for a UPI payment. - * REV-03: Permits refund processing for eligible succeeded and refund_pending payments. - * Preserves audit history and prevents marking refunded until provider or authorized reconciliation. + * Permits refund processing for eligible succeeded and refund_pending payments. + * Provider initiation is strictly separated from refund completion. + * Manual reconciliation requires verified finance authorization, a non-placeholder external + * reference (e.g. bank UTR), and documented evidence reference recorded in the immutable audit ledger. */ export async function refundUpiOrder(orderId, { actor = 'admin:finance', reason = 'Customer requested refund', manualReconciliation = false, - providerRefundId = null + externalRefundRef = null, + evidenceRef = null, + providerRefundId = null, + sessionAdmin = null } = {}) { const adapter = getAdapter(); @@ -943,7 +1068,8 @@ export async function refundUpiOrder(orderId, { orderId, status: 'refunded', refundId: payment.refund_ref || 'ALREADY_REFUNDED', - duplicate: true + duplicate: true, + message: 'Payment has already been refunded' }; } @@ -953,23 +1079,105 @@ export async function refundUpiOrder(orderId, { // 1. Manual finance reconciliation option if (manualReconciliation) { + const effectiveRef = externalRefundRef || providerRefundId; + + if (!isValidExternalRefundRef(effectiveRef)) { + throw new HttpError( + 400, + 'invalid_external_refund_ref', + 'invalid_external_refund_ref: A valid external refund transaction reference (e.g. bank UTR, PG refund ARN) is required and cannot be empty or a placeholder' + ); + } + + if (!isValidEvidenceRef(evidenceRef)) { + throw new HttpError( + 400, + 'missing_evidence_ref', + 'missing_evidence_ref: Documented reconciliation evidence reference (e.g. support ticket ID, bank statement ref) is required and cannot be a placeholder' + ); + } + + if (!isValidReconciliationReason(reason)) { + throw new HttpError( + 400, + 'missing_reason', + 'missing_reason: A documented reconciliation reason (minimum 10 characters) is required and cannot be a placeholder' + ); + } + return db.tx(async (c) => { + // Re-verify finance authorization against real database rows + const auth = await verifyFinanceAuthorization(c, actor, sessionAdmin); + if (!auth.verified) { + throw new HttpError( + 403, + 'unauthorized_finance_role', + 'unauthorized_finance_role: Manual reconciliation requires verified finance or super_admin authorization from an active admin user' + ); + } + const p = (await c.query(`SELECT * FROM payments WHERE order_id = $1 FOR UPDATE`, [orderId])).rows[0]; if (!p) throw new HttpError(404, 'order_not_found', 'Payment not found'); if (p.status === 'refunded') { - return { orderId, status: 'refunded', refundId: p.refund_ref || 'ALREADY_REFUNDED', duplicate: true }; + return { + orderId, + status: 'refunded', + refundId: p.refund_ref || effectiveRef, + duplicate: true, + message: 'Payment has already been refunded' + }; } - const refundId = providerRefundId || p.refund_ref || `MANUAL-${orderId}-${Date.now().toString(36)}`; + const verifiedActor = `admin:${auth.adminId}:${auth.role}`; + await c.query( - `UPDATE payments SET status = 'refunded', refunded_at = now(), refund_ref = $2, failure_reason = NULL WHERE id = $1`, - [p.id, refundId] + `UPDATE payments SET + status = 'refunded', + refunded_at = now(), + refund_ref = $2, + reconciled = true, + reconciled_at = now(), + failure_reason = NULL, + raw_ref = jsonb_set( + COALESCE(raw_ref, '{}'::jsonb), + '{reconciliation}', + $3::jsonb + ) + WHERE id = $1`, + [ + p.id, + effectiveRef, + JSON.stringify({ + manualReconciliation: true, + externalRefundRef: effectiveRef, + evidenceRef, + reason, + reconciledBy: verifiedActor, + reconciledAt: new Date().toISOString() + }) + ] ); + await c.query( `INSERT INTO payment_audits (payment_id, order_id, from_status, to_status, actor, details) VALUES ($1, $2, $3, 'refunded', $4, $5)`, - [p.id, orderId, p.status, actor, JSON.stringify({ refundId, reason, manualReconciliation: true })] + [ + p.id, + orderId, + p.status, + verifiedActor, + JSON.stringify({ + reconciliationType: 'manual_finance_reconciliation', + externalRefundRef: effectiveRef, + evidenceRef, + reason, + reconciledAt: new Date().toISOString(), + adminId: auth.adminId, + adminRole: auth.role + }) + ] ); + if (p.booking_id) { await c.query(`UPDATE bookings SET status = 'refunded', refunded_at = now() WHERE id = $1`, [p.booking_id]); await c.query(`UPDATE invite_links SET revoked_at = COALESCE(revoked_at, now()) WHERE booking_id = $1 AND used_at IS NULL`, [p.booking_id]); @@ -989,8 +1197,28 @@ export async function refundUpiOrder(orderId, { dedupeKey: `bk:${p.booking_id}:refunded` }); } - await audit(actor, 'payment.refunded_upi', p.id, { orderId, amount: p.amount, reason, manualReconciliation: true }, c); - return { orderId, status: 'refunded', refundId }; + + await audit( + verifiedActor, + 'payment.refunded_upi', + p.id, + { + orderId, + amount: p.amount, + reason, + externalRefundRef: effectiveRef, + evidenceRef, + manualReconciliation: true + }, + c + ); + + return { + orderId, + status: 'refunded', + refundId: effectiveRef, + manualReconciliation: true + }; }); } diff --git a/lib/upi/adapter.js b/lib/upi/adapter.js index a679bb3..05d48dc 100644 --- a/lib/upi/adapter.js +++ b/lib/upi/adapter.js @@ -55,12 +55,14 @@ export function buildAppIntents(baseUpiUri) { export class PhonePeAdapter { constructor(options = {}) { - this.merchantId = options.merchantId || env('PHONEPE_MERCHANT_ID') || 'PGTESTPAYUAT'; + this.merchantId = options.merchantId !== undefined + ? options.merchantId + : (env('PHONEPE_MERCHANT_ID') || 'PGTESTPAYUAT'); this.saltKey = options.saltKey || env('PHONEPE_SALT_KEY') || '099eb0cd-0252-4e2a-8aca-eb3c6bd470f1'; this.saltIndex = options.saltIndex || env('PHONEPE_SALT_INDEX') || '1'; this.hostUrl = options.hostUrl || env('PHONEPE_HOST_URL') || 'https://api-preprod.phonepe.com/apis/pg-sandbox'; this.merchantVpa = options.merchantVpa || env('PHONEPE_MERCHANT_VPA') || 'nightlifez@ybl'; - this.isSandbox = this.merchantId.includes('TEST') || this.hostUrl.includes('sandbox'); + this.isSandbox = typeof this.merchantId === 'string' && (this.merchantId.includes('TEST') || this.hostUrl.includes('sandbox')); } /** @@ -196,13 +198,24 @@ export class PhonePeAdapter { normalizedStatus = 'cancelled'; } + const data = json.data || {}; + const statusMerchantId = (data.merchantId || this.merchantId || '').trim(); + if (data.merchantId && this.merchantId && data.merchantId.trim() !== this.merchantId.trim()) { + throw new PaymentAdapterError( + `PhonePe status check merchantId mismatch: expected '${this.merchantId}', received '${data.merchantId}'`, + 'MERCHANT_MISMATCH' + ); + } + return { provider: 'phonepe', + merchantId: statusMerchantId, orderId, - providerTxnId: json.data?.transactionId || providerTxnId || `TXN-${orderId}`, - providerPaymentId: json.data?.paymentInstrument?.utr || null, + providerTxnId: data.transactionId || providerTxnId || `TXN-${orderId}`, + providerPaymentId: data.paymentInstrument?.utr || null, status: normalizedStatus, - amountPaise: json.data?.amount || null, + amountPaise: data.amount || null, + currency: 'INR', rawStatus: code, message: json.message || '' }; @@ -248,10 +261,16 @@ export class PhonePeAdapter { const code = decoded.code; const state = data.state; - // REV-06: Strict Merchant ID Verification - if (!data.merchantId || typeof data.merchantId !== 'string' || data.merchantId !== this.merchantId) { + // Fail-closed merchant verification + if (!this.merchantId || typeof this.merchantId !== 'string' || this.merchantId.trim() === '') { + throw new WebhookSignatureError('PhonePe adapter merchant ID configuration is missing'); + } + if (!data.merchantId || typeof data.merchantId !== 'string' || data.merchantId.trim() === '') { + throw new WebhookSignatureError('PhonePe webhook merchantId mismatch or missing: merchantId is missing'); + } + if (data.merchantId.trim() !== this.merchantId.trim()) { throw new WebhookSignatureError( - `PhonePe webhook merchantId mismatch or missing: expected '${this.merchantId}', received '${data.merchantId || ''}'` + `PhonePe webhook merchantId mismatch or missing: expected '${this.merchantId}', received '${data.merchantId}'` ); } @@ -266,11 +285,12 @@ export class PhonePeAdapter { return { provider: 'phonepe', - merchantId: data.merchantId, + merchantId: data.merchantId.trim(), orderId: data.merchantTransactionId, providerTxnId: data.transactionId, providerPaymentId: data.paymentInstrument?.utr || null, amountPaise: data.amount, + currency: 'INR', status: normalizedStatus, rawStatus: code, raw: decoded @@ -324,9 +344,11 @@ export class PhonePeAdapter { // ---------------- Mock / Test Sandbox Adapter ---------------- export class MockUpiAdapter { - constructor() { - this.merchantVpa = 'nightlifez.test@ybl'; - this.merchantId = 'PGTESTPAYUAT'; + constructor(options = {}) { + this.merchantVpa = options.merchantVpa || 'nightlifez.test@ybl'; + this.merchantId = options.merchantId !== undefined + ? options.merchantId + : (env('PHONEPE_MERCHANT_ID') || 'PGTESTPAYUAT'); this.simulatedResults = new Map(); } @@ -383,11 +405,13 @@ export class MockUpiAdapter { const sim = this.simulatedResults.get(orderId) || 'pending'; return { provider: 'upi_mock', + merchantId: this.merchantId, orderId, providerTxnId: providerTxnId || `MOCK-${orderId}`, providerPaymentId: sim === 'succeeded' ? `UTR${Date.now()}` : null, status: sim, amountPaise: 49900, + currency: 'INR', rawStatus: sim.toUpperCase(), message: `Simulated mock payment ${sim}` }; @@ -400,18 +424,28 @@ export class MockUpiAdapter { } const body = typeof rawBody === 'string' ? JSON.parse(rawBody) : rawBody; - const expectedMerchantId = this.merchantId || 'PGTESTPAYUAT'; - if (body.merchantId && body.merchantId !== expectedMerchantId) { - throw new WebhookSignatureError(`Mock webhook merchantId mismatch: expected '${expectedMerchantId}', got '${body.merchantId}'`); + + // Fail-closed merchant verification + if (!this.merchantId || typeof this.merchantId !== 'string' || this.merchantId.trim() === '') { + throw new WebhookSignatureError('Mock adapter merchant ID configuration is missing'); + } + if (!body.merchantId || typeof body.merchantId !== 'string' || body.merchantId.trim() === '') { + throw new WebhookSignatureError('Mock webhook merchantId is missing'); + } + if (body.merchantId.trim() !== this.merchantId.trim()) { + throw new WebhookSignatureError( + `Mock webhook merchantId mismatch: expected '${this.merchantId}', got '${body.merchantId}'` + ); } return { provider: 'upi_mock', - merchantId: body.merchantId || expectedMerchantId, + merchantId: body.merchantId.trim(), orderId: body.orderId, providerTxnId: body.providerTxnId || `MOCK-${body.orderId}`, providerPaymentId: body.utr || `UTR${Date.now()}`, amountPaise: body.amountPaise, + currency: 'INR', status: body.status || 'succeeded', rawStatus: (body.status || 'succeeded').toUpperCase(), raw: body diff --git a/test/upi-checkout.test.js b/test/upi-checkout.test.js index 357f33c..73352f2 100644 --- a/test/upi-checkout.test.js +++ b/test/upi-checkout.test.js @@ -359,6 +359,7 @@ test('refunds: refund transitions payment to refunded and revokes Telegram invit const succ = await applySuccessfulUpiPayment({ orderId: intent.orderId, verifiedAmountPaise: 40000, + currency: 'INR', merchantId: 'PGTESTPAYUAT', providerPaymentId: 'UTR-REFUND-1', actor: 'test' @@ -408,6 +409,7 @@ test('concurrency: simultaneous checkout intents respect capacity limits', async await applySuccessfulUpiPayment({ orderId: intent1.orderId, verifiedAmountPaise: 20000, + currency: 'INR', merchantId: 'PGTESTPAYUAT', actor: 'test' }); @@ -751,6 +753,7 @@ test('CONCUR-01: Active unexpired checkout intent holds capacity & re-checks at const confirmResult = await applySuccessfulUpiPayment({ orderId: intent2.orderId, verifiedAmountPaise: 20000, + currency: 'INR', merchantId: 'PGTESTPAYUAT', providerTxnId: 'TXN-OVERSOLD-1', actor: 'test:concurrency' @@ -1059,6 +1062,7 @@ test('REV-02: Duplicate active booking during guest claim is rejected with 409 a await applySuccessfulUpiPayment({ orderId: intent.orderId, verifiedAmountPaise: 15000, + currency: 'INR', merchantId: 'PGTESTPAYUAT', actor: 'test' }); @@ -1109,6 +1113,7 @@ test('REV-02: SQLSTATE 23505 defensive recovery moves payment to refund_pending const res = await applySuccessfulUpiPayment({ orderId: intent.orderId, verifiedAmountPaise: 20000, + currency: 'INR', merchantId: 'PGTESTPAYUAT', actor: 'test' }); @@ -1138,6 +1143,7 @@ test('REV-03: refund_pending operational recovery via admin/finance refund and i const confirmRes = await applySuccessfulUpiPayment({ orderId: intent.orderId, verifiedAmountPaise: 35000, + currency: 'INR', merchantId: 'PGTESTPAYUAT', actor: 'test' }); @@ -1189,6 +1195,7 @@ test('REV-03: Provider refund failure leaves payment in refund_pending with manu await applySuccessfulUpiPayment({ orderId: intent.orderId, verifiedAmountPaise: 18000, + currency: 'INR', merchantId: 'PGTESTPAYUAT', actor: 'test' }); @@ -1215,20 +1222,31 @@ test('REV-03: Provider refund failure leaves payment in refund_pending with manu assert.equal(failedPay.status, 'refund_pending'); assert.ok(failedPay.failure_reason.includes('refund_failed')); - // Finance officer performs manual reconciliation recovery + // Seed verified active finance admin in admin_users + const financeAdminUser = await db.one( + `INSERT INTO admin_users (telegram_user_id, role, active) + VALUES (980000001, 'finance', true) + ON CONFLICT (telegram_user_id) DO UPDATE SET active = true, role = 'finance' + RETURNING *` + ); + + // Finance officer performs manual reconciliation recovery with verified role and documented references const reconRes = await refundUpiOrder(intent.orderId, { - actor: 'admin:finance:reconciler', - reason: 'Manual bank transfer reversal confirmed', + actor: `admin:${financeAdminUser.id}:finance`, + reason: 'Manual bank transfer reversal confirmed via NetBanking', manualReconciliation: true, - providerRefundId: 'MANUAL-BANK-REF-999' + externalRefundRef: 'BANK-REV-20261104-999', + evidenceRef: 'FIN-AUDIT-TICKET-2026-999' }); assert.equal(reconRes.status, 'refunded'); - assert.equal(reconRes.refundId, 'MANUAL-BANK-REF-999'); + assert.equal(reconRes.refundId, 'BANK-REV-20261104-999'); - const finalPay = await db.one('SELECT status, refund_ref FROM payments WHERE order_id = $1', [intent.orderId]); + const finalPay = await db.one('SELECT status, refund_ref, reconciled, reconciled_at FROM payments WHERE order_id = $1', [intent.orderId]); assert.equal(finalPay.status, 'refunded'); - assert.equal(finalPay.refund_ref, 'MANUAL-BANK-REF-999'); + assert.equal(finalPay.refund_ref, 'BANK-REV-20261104-999'); + assert.equal(finalPay.reconciled, true); + assert.ok(finalPay.reconciled_at); // Restore refund stub stubUrl('https://api-preprod.phonepe.com', (url) => { @@ -1274,7 +1292,7 @@ test('REV-04: Mandatory verified amount rejects omitted, null, non-numeric and u // 6. Currency mismatch const intent6 = await createUpiPaymentIntent({ slug: event.slug, user: guest1, ticketTypeId: tier.id }); - const res6 = await applySuccessfulUpiPayment({ orderId: intent6.orderId, verifiedAmountPaise: 40000, currency: 'USD' }); + const res6 = await applySuccessfulUpiPayment({ orderId: intent6.orderId, verifiedAmountPaise: 40000, currency: 'USD', merchantId: 'PGTESTPAYUAT' }); assert.equal(res6.status, 'disputed'); assert.equal(res6.disputeReason, 'currency_mismatch'); @@ -1393,3 +1411,408 @@ test('REV-06: PhonePe webhook merchantId verification rejects missing and mismat assert.equal(verified.merchantId, 'PGTESTPAYUAT'); assert.equal(verified.orderId, 'ORD-REV06-3'); }); + +test('M4.3.2: Merchant identity must fail closed across adapters and payment confirmation', async () => { + const saltKey = '099eb0cd-0252-4e2a-8aca-eb3c6bd470f1'; + + // 1. PhonePe adapter rejects missing, null, empty string, and mismatched merchantId + const phonePe = new PhonePeAdapter({ + merchantId: 'PGTESTPAYUAT', + saltKey, + saltIndex: '1' + }); + + for (const badMerchantId of [undefined, null, '', ' ', 'ATTACKER_MERCHANT']) { + const payload = { + code: 'PAYMENT_SUCCESS', + data: { + ...(badMerchantId !== undefined ? { merchantId: badMerchantId } : {}), + merchantTransactionId: 'ORD-M432-PHONEPE', + transactionId: 'TXN-P1', + amount: 25000, + state: 'COMPLETED' + } + }; + const b64 = Buffer.from(JSON.stringify(payload)).toString('base64'); + const body = JSON.stringify({ response: b64 }); + const sig = crypto.createHash('sha256').update(`${b64}${saltKey}`).digest('hex') + '###1'; + + assert.throws( + () => phonePe.verifyWebhook({ 'x-verify': sig }, body), + /merchantId/i, + `PhonePe adapter must reject bad merchantId: ${badMerchantId}` + ); + } + + // 2. PhonePe adapter rejects if adapter itself has missing configured merchantId + const unconfiguredPhonePe = new PhonePeAdapter({ merchantId: '', saltKey, saltIndex: '1' }); + const validPayload = { + code: 'PAYMENT_SUCCESS', + data: { merchantId: 'PGTESTPAYUAT', merchantTransactionId: 'ORD-M432-1', transactionId: 'TXN1', amount: 10000, state: 'COMPLETED' } + }; + const b64Valid = Buffer.from(JSON.stringify(validPayload)).toString('base64'); + const sigValid = crypto.createHash('sha256').update(`${b64Valid}${saltKey}`).digest('hex') + '###1'; + assert.throws( + () => unconfiguredPhonePe.verifyWebhook({ 'x-verify': sigValid }, JSON.stringify({ response: b64Valid })), + /merchant ID configuration is missing/i + ); + + // 3. Mock adapter rejects missing, null, empty string, and mismatched merchantId + const mockAdapter = new MockUpiAdapter({ merchantId: 'PGTESTPAYUAT' }); + + for (const badMerchantId of [undefined, null, '', ' ', 'ATTACKER_MERCHANT']) { + const mockBody = { + orderId: 'ORD-M432-MOCK', + ...(badMerchantId !== undefined ? { merchantId: badMerchantId } : {}), + amountPaise: 25000, + status: 'succeeded' + }; + assert.throws( + () => mockAdapter.verifyWebhook({ 'x-webhook-secret': 'test-upi-webhook-secret' }, JSON.stringify(mockBody)), + /merchantId/i, + `Mock adapter must reject bad merchantId: ${badMerchantId}` + ); + } + + // 4. Mock adapter rejects if adapter itself has missing configured merchantId + const unconfiguredMock = new MockUpiAdapter({ merchantId: '' }); + assert.throws( + () => unconfiguredMock.verifyWebhook( + { 'x-webhook-secret': 'test-upi-webhook-secret' }, + JSON.stringify({ orderId: 'ORD-M432-MOCK', merchantId: 'PGTESTPAYUAT', amountPaise: 25000 }) + ), + /merchant ID configuration is missing/i + ); + + // 5. applySuccessfulUpiPayment fail-closed merchant checks + const { event, tier } = await createPaidEvent({ price: 30000 }); + + // 5a. Missing verified merchant ID (null, undefined, empty) + const it1 = await createUpiPaymentIntent({ slug: event.slug, user: guest1, ticketTypeId: tier.id }); + const r1 = await applySuccessfulUpiPayment({ orderId: it1.orderId, verifiedAmountPaise: 30000, currency: 'INR', merchantId: null }); + assert.equal(r1.status, 'disputed'); + assert.equal(r1.disputeReason, 'missing_verified_merchant_id'); + + const it1b = await createUpiPaymentIntent({ slug: event.slug, user: guest1, ticketTypeId: tier.id }); + const r1b = await applySuccessfulUpiPayment({ orderId: it1b.orderId, verifiedAmountPaise: 30000, currency: 'INR', merchantId: ' ' }); + assert.equal(r1b.status, 'disputed'); + assert.equal(r1b.disputeReason, 'missing_verified_merchant_id'); + + // 5b. Mismatched verified merchant ID + const it2 = await createUpiPaymentIntent({ slug: event.slug, user: guest1, ticketTypeId: tier.id }); + const r2 = await applySuccessfulUpiPayment({ orderId: it2.orderId, verifiedAmountPaise: 30000, currency: 'INR', merchantId: 'WRONG_MERCHANT' }); + assert.equal(r2.status, 'disputed'); + assert.equal(r2.disputeReason, 'merchant_mismatch'); + + // 5c. Missing stored merchant ID in database row + const it3 = await createUpiPaymentIntent({ slug: event.slug, user: guest1, ticketTypeId: tier.id }); + await db.query(`UPDATE payments SET merchant_id = NULL WHERE order_id = $1`, [it3.orderId]); + const r3 = await applySuccessfulUpiPayment({ orderId: it3.orderId, verifiedAmountPaise: 30000, currency: 'INR', merchantId: 'PGTESTPAYUAT' }); + assert.equal(r3.status, 'disputed'); + assert.equal(r3.disputeReason, 'missing_stored_merchant_id'); + + // 6. Currency fail-closed checks + // 6a. Missing currency (null, empty string, omitted) + const it4 = await createUpiPaymentIntent({ slug: event.slug, user: guest1, ticketTypeId: tier.id }); + const r4 = await applySuccessfulUpiPayment({ orderId: it4.orderId, verifiedAmountPaise: 30000, currency: null, merchantId: 'PGTESTPAYUAT' }); + assert.equal(r4.status, 'disputed'); + assert.equal(r4.disputeReason, 'missing_currency'); + + const it4b = await createUpiPaymentIntent({ slug: event.slug, user: guest1, ticketTypeId: tier.id }); + const r4b = await applySuccessfulUpiPayment({ orderId: it4b.orderId, verifiedAmountPaise: 30000, currency: '', merchantId: 'PGTESTPAYUAT' }); + assert.equal(r4b.status, 'disputed'); + assert.equal(r4b.disputeReason, 'missing_currency'); + + // 6b. Currency mismatch (USD, EUR) + const it5 = await createUpiPaymentIntent({ slug: event.slug, user: guest1, ticketTypeId: tier.id }); + const r5 = await applySuccessfulUpiPayment({ orderId: it5.orderId, verifiedAmountPaise: 30000, currency: 'EUR', merchantId: 'PGTESTPAYUAT' }); + assert.equal(r5.status, 'disputed'); + assert.equal(r5.disputeReason, 'currency_mismatch'); + + // 7. Valid merchant ID and INR confirms successfully + const it6 = await createUpiPaymentIntent({ slug: event.slug, user: guest1, ticketTypeId: tier.id }); + const r6 = await applySuccessfulUpiPayment({ orderId: it6.orderId, verifiedAmountPaise: 30000, currency: 'INR', merchantId: 'PGTESTPAYUAT' }); + assert.equal(r6.status, 'succeeded'); + assert.ok(r6.bookingId); + + // Verify all disputed intents created zero bookings + for (const it of [it1, it1b, it2, it3, it4, it4b, it5]) { + const bkCount = await db.one( + `SELECT count(*)::int AS n FROM bookings WHERE payment_id = (SELECT id FROM payments WHERE order_id = $1)`, + [it.orderId] + ); + assert.equal(bkCount.n, 0); + } +}); + +test('M4.3.2: Secure manual refund reconciliation authorization, evidence reference and state transitions', async () => { + const { event, tier } = await createPaidEvent({ price: 22000 }); + + // Create active finance admin user + const financeAdmin = await db.one( + `INSERT INTO admin_users (telegram_user_id, role, active) + VALUES (980000777, 'finance', true) + ON CONFLICT (telegram_user_id) DO UPDATE SET active = true, role = 'finance' + RETURNING *` + ); + + // 1. Unauthorized finance reconciliation rejected + const it1 = await createUpiPaymentIntent({ slug: event.slug, user: guest1, ticketTypeId: tier.id }); + await applySuccessfulUpiPayment({ orderId: it1.orderId, verifiedAmountPaise: 22000, currency: 'INR', merchantId: 'PGTESTPAYUAT' }); + + // 1a. Unverified actor string + await assert.rejects( + () => refundUpiOrder(it1.orderId, { + actor: 'admin:finance:fake', + reason: 'Manual refund test without active admin row', + manualReconciliation: true, + externalRefundRef: 'BANK-UTR-987654321012', + evidenceRef: 'FIN-AUDIT-TICKET-101' + }), + /unauthorized_finance_role/i + ); + + // 1b. Regular user / guest actor + await assert.rejects( + () => refundUpiOrder(it1.orderId, { + actor: `user:${guest1.userId}`, + reason: 'Attendee trying to self-reconcile refund', + manualReconciliation: true, + externalRefundRef: 'BANK-UTR-987654321012', + evidenceRef: 'FIN-AUDIT-TICKET-101' + }), + /unauthorized_finance_role/i + ); + + // 2. Missing external refund reference rejected + await assert.rejects( + () => refundUpiOrder(it1.orderId, { + actor: `admin:${financeAdmin.id}:finance`, + reason: 'Customer requested bank reversal', + manualReconciliation: true, + externalRefundRef: null, + evidenceRef: 'FIN-AUDIT-TICKET-101' + }), + /invalid_external_refund_ref/i + ); + + // 3. Fabricated / placeholder external refund references rejected + const placeholders = [ + 'MANUAL', + 'MANUAL-BANK-REF-999', + 'manual-ord-12345', + 'DUMMY-REFUND-1', + 'TEST12345', + 'PLACEHOLDER', + '0000000', + 'XXXXXX', + '123456', + 'N/A', + 'NONE' + ]; + + for (const ph of placeholders) { + await assert.rejects( + () => refundUpiOrder(it1.orderId, { + actor: `admin:${financeAdmin.id}:finance`, + reason: 'Customer requested bank reversal', + manualReconciliation: true, + externalRefundRef: ph, + evidenceRef: 'FIN-AUDIT-TICKET-101' + }), + /invalid_external_refund_ref/i, + `Expected placeholder '${ph}' to be rejected` + ); + } + + // 4. Missing or placeholder evidence reference rejected + for (const badEvidence of [null, '', ' ', 'none', 'n/a', 'dummy', 'test', 'placeholder']) { + await assert.rejects( + () => refundUpiOrder(it1.orderId, { + actor: `admin:${financeAdmin.id}:finance`, + reason: 'Customer requested bank reversal', + manualReconciliation: true, + externalRefundRef: 'BANK-UTR-987654321012', + evidenceRef: badEvidence + }), + /missing_evidence_ref/i, + `Expected bad evidence '${badEvidence}' to be rejected` + ); + } + + // 5. Missing or short documented reconciliation reason rejected + for (const badReason of [null, '', 'short', 'refund', 'test']) { + await assert.rejects( + () => refundUpiOrder(it1.orderId, { + actor: `admin:${financeAdmin.id}:finance`, + reason: badReason, + manualReconciliation: true, + externalRefundRef: 'BANK-UTR-987654321012', + evidenceRef: 'FIN-AUDIT-TICKET-101' + }), + /missing_reason/i, + `Expected bad reason '${badReason}' to be rejected` + ); + } + + // 6. Authorized reconciliation with evidence reference succeeds + const invite = await db.one( + `INSERT INTO invite_links (booking_id, user_id, chat_id, link, expires_at) + VALUES ((SELECT booking_id FROM payments WHERE order_id = $1), $2, -1001111111111, 'https://t.me/+testm432recon', now() + interval '1 day') + RETURNING *`, + [it1.orderId, guest1.userId] + ); + + const reconRes = await refundUpiOrder(it1.orderId, { + actor: `admin:${financeAdmin.id}:finance`, + reason: 'Confirmed bank transfer reversal via HDFC Corporate UTR 987654321012', + manualReconciliation: true, + externalRefundRef: 'BANK-UTR-987654321012', + evidenceRef: 'FIN-AUDIT-TICKET-88421' + }); + + assert.equal(reconRes.status, 'refunded'); + assert.equal(reconRes.refundId, 'BANK-UTR-987654321012'); + assert.equal(reconRes.manualReconciliation, true); + + // Verify database record + const pCheck = await db.one( + `SELECT status, refund_ref, reconciled, reconciled_at, raw_ref FROM payments WHERE order_id = $1`, + [it1.orderId] + ); + assert.equal(pCheck.status, 'refunded'); + assert.equal(pCheck.refund_ref, 'BANK-UTR-987654321012'); + assert.equal(pCheck.reconciled, true); + assert.ok(pCheck.reconciled_at); + assert.equal(pCheck.raw_ref.reconciliation.externalRefundRef, 'BANK-UTR-987654321012'); + assert.equal(pCheck.raw_ref.reconciliation.evidenceRef, 'FIN-AUDIT-TICKET-88421'); + + // Verify immutable audit ledger + const audits = (await db.query( + `SELECT from_status, to_status, actor, details FROM payment_audits WHERE order_id = $1 AND to_status = 'refunded'`, + [it1.orderId] + )).rows; + assert.equal(audits.length, 1); + assert.equal(audits[0].actor, `admin:${financeAdmin.id}:finance`); + assert.equal(audits[0].details.reconciliationType, 'manual_finance_reconciliation'); + assert.equal(audits[0].details.externalRefundRef, 'BANK-UTR-987654321012'); + assert.equal(audits[0].details.evidenceRef, 'FIN-AUDIT-TICKET-88421'); + + // Verify booking and invite links were revoked + const bkCheck = await db.one(`SELECT status FROM bookings WHERE payment_id = (SELECT id FROM payments WHERE order_id = $1)`, [it1.orderId]); + assert.equal(bkCheck.status, 'refunded'); + + const invCheck = await db.one(`SELECT revoked_at FROM invite_links WHERE id = $1`, [invite.id]); + assert.ok(invCheck.revoked_at !== null, 'Invite link must be revoked upon confirmed refund'); + + // 7. Duplicate reconciliation attempts are strictly idempotent + const dupeRes = await refundUpiOrder(it1.orderId, { + actor: `admin:${financeAdmin.id}:finance`, + reason: 'Duplicate attempt by another operator', + manualReconciliation: true, + externalRefundRef: 'BANK-UTR-987654321012', + evidenceRef: 'FIN-AUDIT-TICKET-88421' + }); + assert.equal(dupeRes.status, 'refunded'); + assert.equal(dupeRes.duplicate, true); + + // 8. Refund state transitions: provider pending vs completed vs failed + // 8a. Succeeded and refund_pending can enter refund workflow + const itPending = await createUpiPaymentIntent({ slug: event.slug, user: guest2, ticketTypeId: tier.id }); + await db.query(`UPDATE payments SET status = 'refund_pending', failure_reason = 'duplicate_booking' WHERE order_id = $1`, [itPending.orderId]); + + // Provider returns pending status + stubUrl('https://api-preprod.phonepe.com', (url) => { + if (url.includes('/pg/v1/refund')) { + return { + status: 200, + body: JSON.stringify({ success: true, code: 'PAYMENT_PENDING', data: { transactionId: 'TXN-REF-PENDING' } }) + }; + } + return { status: 200, body: JSON.stringify({ success: true, code: 'PAYMENT_SUCCESS' }) }; + }); + + const pendingRefundRes = await refundUpiOrder(itPending.orderId, { actor: 'admin:finance' }); + assert.equal(pendingRefundRes.status, 'refund_pending'); + + const payPendingCheck = await db.one('SELECT status FROM payments WHERE order_id = $1', [itPending.orderId]); + assert.equal(payPendingCheck.status, 'refund_pending'); + + // 8b. Failed provider refund remains recoverable in refund_pending + stubUrl('https://api-preprod.phonepe.com', (url) => { + if (url.includes('/pg/v1/refund')) { + return { status: 503, body: JSON.stringify({ success: false, code: 'SERVICE_UNAVAILABLE', message: 'Gateway down' }) }; + } + return { status: 200, body: JSON.stringify({ success: true, code: 'PAYMENT_SUCCESS' }) }; + }); + + await assert.rejects( + () => refundUpiOrder(itPending.orderId, { actor: 'admin:finance' }), + /Payment gateway refund initiation failed/ + ); + + const payFailedCheck = await db.one('SELECT status, failure_reason FROM payments WHERE order_id = $1', [itPending.orderId]); + assert.equal(payFailedCheck.status, 'refund_pending'); + assert.ok(payFailedCheck.failure_reason.includes('refund_failed')); + + // 8c. Recoverable via authorized manual finance reconciliation + const recoverRes = await refundUpiOrder(itPending.orderId, { + actor: `admin:${financeAdmin.id}:finance`, + reason: 'Manual bank reversal following gateway 503 downtime', + manualReconciliation: true, + externalRefundRef: 'BANK-RECOV-20261104-555', + evidenceRef: 'INCIDENT-GATEWAY-DOWN-991' + }); + assert.equal(recoverRes.status, 'refunded'); + assert.equal(recoverRes.refundId, 'BANK-RECOV-20261104-555'); + + // Restore refund stub + stubUrl('https://api-preprod.phonepe.com', (url) => { + if (url.includes('/pg/v1/refund')) { + return { status: 200, body: JSON.stringify({ success: true, code: 'PAYMENT_SUCCESS', data: { transactionId: 'TXN-REF-RESTORED' } }) }; + } + return { status: 200, body: JSON.stringify({ success: true, code: 'PAYMENT_SUCCESS' }) }; + }); + + // 9. Admin HTTP endpoint POST /api/admin/payments/:id/refund enforcement + const itHttp = await createUpiPaymentIntent({ slug: event.slug, user: guest3, ticketTypeId: tier.id }); + await applySuccessfulUpiPayment({ orderId: itHttp.orderId, verifiedAmountPaise: 22000, currency: 'INR', merchantId: 'PGTESTPAYUAT' }); + const httpPay = await db.one('SELECT id FROM payments WHERE order_id = $1', [itHttp.orderId]); + + // Signed out gets 401 + const anon = new Client(); + assert.equal((await anon.post(`/api/admin/payments/${httpPay.id}/refund`, {})).status, 401); + + // Regular user gets 403 + assert.equal((await guest1.post(`/api/admin/payments/${httpPay.id}/refund`, {})).status, 403); + + // Admin without finance role gets 403 + const managerUser = await signedInClient({ tgId: 980000888 }); + await admin.post('/api/admin/admins', { telegram_user_id: managerUser.tgId, role: 'event_manager' }); + assert.equal((await managerUser.post(`/api/admin/payments/${httpPay.id}/refund`, {})).status, 403); + + // Finance client via HTTP + const financeClient = await signedInClient({ tgId: financeAdmin.telegram_user_id }); + + // Placeholder reference returns 400 + const phRes = await financeClient.post(`/api/admin/payments/${httpPay.id}/refund`, { + manualReconciliation: true, + externalRefundRef: 'MANUAL', + evidenceRef: 'TICKET-1', + reason: 'Testing placeholder rejection via API' + }); + assert.equal(phRes.status, 400); + + // Valid manual reconciliation via HTTP returns 200 + const okRes = await financeClient.post(`/api/admin/payments/${httpPay.id}/refund`, { + manualReconciliation: true, + externalRefundRef: 'BANK-API-UTR-20261104', + evidenceRef: 'FIN-TICKET-API-2026', + reason: 'Confirmed bank refund processed via finance portal' + }); + assert.equal(okRes.status, 200); + + const finalHttpPay = await db.one('SELECT status, refund_ref FROM payments WHERE id = $1', [httpPay.id]); + assert.equal(finalHttpPay.status, 'refunded'); + assert.equal(finalHttpPay.refund_ref, 'BANK-API-UTR-20261104'); +});