Files
edr-platform/docs/SOLUTIONS.md

9.3 KiB
Raw Permalink Blame History

EDR Passenger — Solutions

Concrete fixes for the confirmed findings in docs/ISSUES.md. Ordered by priority. Each references the exact site and the intended change. Code sketches are illustrative, not drop-in patches.

Test coverage backing these: 28 automated tests (25 API jest + 3 UI Playwright) reproduce the findings. Fix a finding → its 🔴 test flips from "bug present" to failing; update the test to assert the corrected behavior.


P0 — deploy blockers (money creation/theft, broken booking)

C-10 — Authenticated POST /bookings is broken

bookings.service.ts:773 resolve unconditionally; delete the UUID-format gate:

// BEFORE: resolves only when passengerId is NOT a UUID (never fires for real IAM ids)
if (dto.passengerId && !dto.passengerId.match(/^[0-9a-f-]{36}$/i)) {  }
// AFTER: always translate the authenticated identity → the Passenger.id
if (dto.passengerId) {
  const passenger = await this.prisma.passenger.findUnique({
    where: { iamUserId: dto.passengerId }, select: { id: true },
  });
  if (passenger) dto = { ...dto, passengerId: passenger.id };
  // else: leave as-is only if it already IS a Passenger.id (guest/admin paths)
}

Keep the controller's "don't trust the body" intent (bookings.controller.ts:528) — it's correct to take identity from the JWT; the service just has to map iamUserId → Passenger.id. Regression test: test/authed-booking-passengerid.e2e-spec.ts.

C-1 — Booking total is client-controlled

bookings.service.ts:863-899: stop trusting reviewedTotalMinor/seatFareMinor. Recompute the fare server-side and reject a mismatch:

const server = fareCalculation.totalMinor;
if (dto.reviewedTotalMinor != null && Math.abs(dto.reviewedTotalMinor - server) > 1) {
  throw new BadRequestException('Price changed — please review the updated fare');
}
resolvedTotalMinor = server; // never persist a client amount as the charge basis

Apply the same to guest-booking.service.ts:206-245.

C-2 — Loyalty redemption unbounded + never deducted

bookings.service.ts:1705 (and :1028/:1249/:1451): validate + debit inside the booking transaction:

const acct = await tx.loyaltyAccount.findUnique({ where: { passengerId } });
const pts = Math.min(dto.loyaltyRedemptionPoints ?? 0, acct?.pointsBalance ?? 0, MAX_REDEEM);
const loyaltyMinor = pts * POINTS_TO_MINOR;
await tx.loyaltyLedgerEntry.create({ data: { accountId: acct.id, delta: -pts, reason: 'REDEEMED', balanceAfter: acct.pointsBalance - pts } });
await tx.loyaltyAccount.update({ where: { id: acct.id }, data: { pointsBalance: { decrement: pts } } });

C-3 — Wallet top-up: no ownership, no backing

  • wallet.controller.ts:34: enforce req.user owns :passengerId (or is admin) before top-up.
  • wallet.service.ts:50: only credit after a confirmed PaymentIntent (a top-up is a purchase).
  • wallet.controller.ts:23-24: remove @IsPublic() from GET /wallet/accounts.

C-4 — Payment amount never validated

  • intents.service.ts:541-548: on confirmedAmountMinor !== intent.amountMinor, do NOT mark SUCCEEDED — set a AMOUNT_MISMATCH state and alert. Populate confirmedAmountMinor in each webhook handler (e.g. waafi-webhook.service.ts:63).
  • payments.service.ts:809 finalizePaymentSuccess: assert the settled amount equals booking.totalMinor before confirming.

C-5 — Late webhook resurrects an expired/cancelled booking

payments.service.ts:809 finalizePaymentSuccess: refuse to confirm unless the booking is still PENDING_PAYMENT; route a late success to the refund/again-available flow:

if (booking.status !== 'PENDING_PAYMENT') { await this.refundLatePayment(intent); return { alreadyFinalized: true }; }

C-6 — Wallet double-spend (no row lock)

payments.service.ts:461-484: lock the row or use an atomic conditional update:

const res = await tx.$executeRaw`UPDATE passenger."WalletAccount"
  SET "balanceMinor" = "balanceMinor" - ${total}
  WHERE "passengerId" = ${booking.passengerId} AND "balanceMinor" >= ${total}`;
if (res === 0) return { success: false }; // insufficient / lost the race

Regression test: critical-repro.e2e-spec.ts (C-6, barrier-forced interleave).

C-7 — Refund computed but never disbursed

bookings.service.ts:2017-2027: on booking.cancelled, actually disburse — credit the wallet or call the provider refund — and drive refundStatus PENDING → PROCESSING → COMPLETED. Add a reconciliation sweep for stuck PENDING rows.

C-8 — Exchange-rate writes missing the ADMIN check (any passenger can write FX)

fare-engine/currency.controller.ts:25,32: add @PassengerAdmin() (+ @ApiBearerAuth) to the PUT and PATCH handlers, matching the already-guarded DELETE. (Not unauthenticated — the global JwtGuard requires a token; the gap is the missing authorization. Verified live: passenger → 200.)

C-9 — @Roles('ADMIN') is dead

Register the guard globally so @Roles is enforced:

// app.module.ts providers
{ provide: APP_GUARD, useClass: RolesGuard }

…or convert configurable-fare / segment-fare / system-config controllers to the working @PassengerAdmin()/@PassengerStaff() guards.


P1 — pricing correctness (HIGH)

  • H-1 promo → negative total (fare-engine.service.ts:192): totalEtbMinor = Math.max(0, subtotal - discount); DTO @Min(0) @Max(100) on percentOff, @Min(0) on amountOffMinor (promos.dto.ts:20,26).
  • H-2 missing FX → 1.0 (currency.service.ts:142): remove the silent return 1.0 — throw / block the quote so it fails closed.
  • H-3/H-4 FX divergence & unit confusion (currency.service.ts): collapse the 4 routines into one convert(fromMinor, from, to): {minor|major} with one rounding + one fallback policy; give it a branded Minor/Major return type and audit every amountMinor assignment across the payment boundary.
  • H-5 percentOff:0 treated as FIXED (fare-engine.service.ts:185): use promo.percentOff != null ? … : promo.amountOffMinor.
  • H-6 insuranceFeeMinor dual meaning (fare-engine.service.ts:130,154,167): split into insuranceMultiplierBps and insuranceFeeMinor; use one consistently.
  • H-7 domestic ETB fare × USD→ETB rate (fare-engine.service.ts:157): don't apply a currency conversion to a domestic base fare — separate the minor-unit scaling from FX.
  • H-8 baggage ignores seat class (excess-baggage.service.ts:53): findFirst({ where: { seatClassId } }).
  • H-9 baggage/supp skip conversion + DJF rounding (excess-baggage.service.ts:166, supplementary-charges.service.ts:132): route through convertMinorToChargeMajor.
  • H-10 future-dated FX applied now (currency.service.ts:88,137): add effectiveDate: { lte: new Date() } to the rate lookups.
  • H-11 non-deterministic fare resolution (fare-engine.service.ts:84,298): add orderBy: { validFrom: 'desc' }; include validFrom in SegmentFareRule's unique key (schema.prisma:1113) to allow dated versions.
  • H-12 divergent free-child rules (fare-engine.service.ts:172 vs bookings.service.ts:1690 vs :1622): extract ONE computeFare() used by quote, booking, and payment.
  • H-13 promo silently dropped → overcharge (review/page.tsx:587, search.service.ts:940-970): book the breakdown's discounted totalMinor, not the client-summed undiscounted per-pax fares — or return discounted per-pax fares. Fold into the C-1 fix (server recomputes the authoritative total incl. promo).

P2 — config validation & safety (MEDIUM) / UI (LOW)

  • M-1 negative fares: add @Min(0) to baseFareMinor (schedules.dto.ts:85,95) and basePrice (seat-classes.dto.ts:29).
  • M-2 promo bounds/date: @Min(0) @Max(100) on percentOff, @IsDateString() on validUntil (promos.dto.ts); add min-spend / usage-limit / max-cap columns.
  • M-3 PATCH /config arbitrary: replace the raw body with a whitelisted, typed DTO with per-key range checks (system-config.controller.ts:34).
  • M-4 past-date / double-booked schedules (schedules.service.ts:105,124): reject past departureAt; widen the overlap check to the train across all routes.
  • M-5 deletes ignore references (stations/seat-classes/currencies/schedules services): add referential guards before delete/disable; wrap the schedule cascade (schedules.service.ts:438-485) in a transaction.
  • M-6 non-atomic booking write (bookings.service.ts:883-926): wrap create + confirmSeats + tier increment in one $transaction.
  • L-1 DJF shown with 2 decimals (portal/src/utils/format.ts:22): format per CHARGE_CURRENCY_DECIMALS (DJF = 0).
  • L-2 client-side fare math (portal/src/utils/fare-utils.ts, review/page.tsx): render only server-computed amounts; never submit a client-derived total (ties to C-1).
  • L-3 loyalty accrual currency (payments.service.ts:1062): accrue from the actual charged amount/currency, not ETB minor.

Suggested sequencing

  1. C-10, C-3, C-8, C-9 — quickest high-impact (a few lines each): unblock authenticated booking, stop free wallet credit, guard FX writes, enforce roles.
  2. C-1, C-2, C-4, C-5, C-6, C-7 — the money-integrity core (needs transactions + validation).
  3. H-2, H-3, H-4, H-7 — the FX/units foundation others compound on.
  4. H-1, H-5, H-8..H-12, M-1, M-2 — pricing correctness + validation.
  5. M-3..M-6, L-1..L-3 — config safety + UI consistency.