diff --git a/docs/architecture/authentication-and-authorization.md b/docs/architecture/authentication-and-authorization.md index 855e82eb7..abe0680d2 100644 --- a/docs/architecture/authentication-and-authorization.md +++ b/docs/architecture/authentication-and-authorization.md @@ -135,10 +135,13 @@ deliberate context-policy role check, not a permission, because the override PIN person acting in an owner/manager capacity regardless of what their individual permission grants say. To prevent event loop starvation and transaction lock contention during CPU-intensive bcrypt evaluations, manager PINs are verified asynchronously via `verifyPinAsync` outside the transaction -(with client-IP rate limits checked first). The pre-verified approver is then atomically re-validated -inside `withTxn` against current SQLite state (active status, owner/manager role, required permissions, -and matching `pin_hash`). The check cannot be hoisted into middleware without changing the read the -decision is based on, and it is invisible to anything that inspects the route's middleware chain. +(with client-IP rate limits checked first). Candidate filtering during pre-verification enforces the +role and permissions required by the transition (including `payments.take` when completion is +blocked by unsettled payment), ensuring fallback PIN scanning only matches eligible approvers. The +pre-verified approver is then atomically re-validated inside `withTxn` against current SQLite state +(active status, owner/manager role, required permissions, and matching `pin_hash`). The check cannot +be hoisted into middleware without changing the read the decision is based on, and it is invisible to +anything that inspects the route's middleware chain. **KDS WebSocket authorization.** The KDS socket authenticates with the same bearer token but authorizes per message. See below. diff --git a/docs/reference/api.md b/docs/reference/api.md index c7fbe370b..fad943bc1 100644 --- a/docs/reference/api.md +++ b/docs/reference/api.md @@ -329,7 +329,9 @@ one. Failures without a registered tuple may retain the `{ error }` response. separate operation via `/api/refunds`) and restores product, variant, add-on, and recipe stock only for items still in `pending` status. - When `approver_id` (or `manager_id`, `user_id`) is supplied on an overridden transition, PIN verification is - restricted to that active user without falling back to scanning other managers. Every overridden `status_changed` audit log entry records + restricted to that active user without falling back to scanning other managers. When omitted, candidate managers + are evaluated, skipping users who lack the required role or permissions (including `payments.take` when completion + is blocked by unsettled payment). Every overridden `status_changed` audit log entry records `actor_user_id`, `approved_by`, `overridden_safeguards`, `bill_ids`, `outstanding_amount`, `outstanding_amounts`, and `collected_amount` (plus `undelivered_items` when overriding kitchen delivery), and never records the PIN. diff --git a/main/routes/orders.ts b/main/routes/orders.ts index 8be52f317..090a6450c 100644 --- a/main/routes/orders.ts +++ b/main/routes/orders.ts @@ -2302,23 +2302,69 @@ router.post( }, ); -interface OrderStatusBillEvaluationRow { - id?: number | string; - total?: number | string | null; - round_off?: number | string | null; - paid_amount?: number | string | null; - refunded_cents?: number | string | null; - balance?: number | string | null; - payment_status?: string | null; - payment_details?: unknown; -} +function evaluateOrderStatusPaymentBlock( + db: ReturnType, + orderId: string | number | unknown, + orderTotal: unknown, + orderRoundOff: unknown, +): { + minorFactor: number; + roundCurrency: (amount: number) => number; + evaluatedBills: Array<{ + id: string; + paymentStatus: string; + totalMinor: number; + unroundedTotalMinor: number; + recordedPaidMinor: number; + effectiveCollectedMinor: number; + hasCollectedFunds: boolean; + isPayable: boolean; + unpaidBalanceMinor: number; + isSettled: boolean; + }>; + orderTotalMinor: number; + payableBills: Array<{ + id: string; + paymentStatus: string; + totalMinor: number; + unroundedTotalMinor: number; + recordedPaidMinor: number; + effectiveCollectedMinor: number; + hasCollectedFunds: boolean; + isPayable: boolean; + unpaidBalanceMinor: number; + isSettled: boolean; + }>; + totalOutstandingMinor: number; + totalCollectedMinor: number; + paymentBlocksCompletion: boolean; +} { + const tenantCurrency = getTenantCurrency(); + const minorFactor = getCurrencyMinorUnitFactor(tenantCurrency); + const fractionDigits = getCurrencyFractionDigits(tenantCurrency); + const roundCurrency = (amount: number): number => + fractionDigits === 0 ? Math.round(amount) : Number(amount.toFixed(fractionDigits)); -function evaluateOrderPaymentCompletion( - order: { total?: number | string | null; round_off?: number | string | null } | null | undefined, - bills: OrderStatusBillEvaluationRow[], - minorFactor = 100, -) { - const evaluatedBills = bills.map((b) => { + const billRows = db + .prepare(` + SELECT b.*, + COALESCE((SELECT SUM(r.amount_cents) FROM refunds r WHERE r.bill_id = b.id), 0) AS refunded_cents + FROM bills b + WHERE b.order_id = ? + ORDER BY b.id ASC + `) + .all(String(orderId)) as Array<{ + id: string; + total?: number | null; + round_off?: number | null; + paid_amount?: number | null; + refunded_cents?: number | null; + balance?: number | null; + payment_details?: unknown; + payment_status?: string | null; + }>; + + const evaluatedBills = billRows.map((b) => { const totalMinor = Math.max(0, Math.round(Number(b.total || 0) * minorFactor)); const roundOffMinor = Math.round(Number(b.round_off || 0) * minorFactor); const unroundedTotalMinor = Math.max(0, totalMinor - roundOffMinor); @@ -2378,8 +2424,8 @@ function evaluateOrderPaymentCompletion( }; }); - const orderTotalMinor = Math.max(0, Math.round(Number(order?.total || 0) * minorFactor)); - const orderRoundOffMinor = Math.round(Number(order?.round_off || 0) * minorFactor); + const orderTotalMinor = Math.max(0, Math.round(Number(orderTotal || 0) * minorFactor)); + const orderRoundOffMinor = Math.round(Number(orderRoundOff || 0) * minorFactor); const orderUnroundedTotalMinor = Math.max(0, orderTotalMinor - orderRoundOffMinor); const effectiveOrderTargetMinor = Math.min(orderTotalMinor, orderUnroundedTotalMinor); const payableBills = evaluatedBills.filter((b) => b.isPayable); @@ -2395,22 +2441,20 @@ function evaluateOrderPaymentCompletion( payableBills.reduce((sum, b) => sum + b.unpaidBalanceMinor, 0) + unbilledMinor; const totalCollectedMinor = evaluatedBills.reduce((sum, b) => sum + b.effectiveCollectedMinor, 0); - const requiresPaymentOverride = + const paymentBlocksCompletion = totalOutstandingMinor > 0 || (orderTotalMinor > 0 && payableBills.length === 0) || payableBills.some((b) => !b.isSettled); return { + minorFactor, + roundCurrency, evaluatedBills, - payableBills, orderTotalMinor, - orderRoundOffMinor, - orderUnroundedTotalMinor, - effectiveOrderTargetMinor, - unbilledMinor, + payableBills, totalOutstandingMinor, totalCollectedMinor, - requiresPaymentOverride, + paymentBlocksCompletion, }; } @@ -2455,6 +2499,15 @@ router.patch( return res.status(429).json({ error: 'Too many PIN attempts. Try again in 15 minutes.' }); } if (/^\d{4,6}$/.test(override_pin)) { + const preVerificationRequiresPayment = + status === 'completed' && + evaluateOrderStatusPaymentBlock( + db, + req.params.id, + (order as any).total, + (order as any).round_off, + ).paymentBlocksCompletion; + const explicitApproverId = req.body.approver_id || req.body.manager_id || req.body.user_id; const candidateUsers = explicitApproverId @@ -2473,7 +2526,8 @@ router.patch( if ( hasRole(candidate.role, ROLE_ACCESS.ownerManager) && hasPermission(candidate.id, 'orders.status.update') && - (status !== 'cancelled' || hasPermission(candidate.id, 'orders.item.cancel')) + (status !== 'cancelled' || hasPermission(candidate.id, 'orders.item.cancel')) && + (!preVerificationRequiresPayment || hasPermission(candidate.id, 'payments.take')) ) { if (await verifyPinAsync(candidate.pin_hash, override_pin)) { preVerifiedStatusApprover = { @@ -2555,41 +2609,22 @@ router.patch( const statusOrder = ['pending', 'preparing', 'ready', 'served', 'completed']; const currentStatusIndex = statusOrder.indexOf(currentOrder.status); - const tenantCurrency = getTenantCurrency(); - const minorFactor = getCurrencyMinorUnitFactor(tenantCurrency); - const fractionDigits = getCurrencyFractionDigits(tenantCurrency); - const roundCurrency = (amount: number): number => - fractionDigits === 0 ? Math.round(amount) : Number(amount.toFixed(fractionDigits)); - - const billRows = db - .prepare(` - SELECT b.*, - COALESCE((SELECT SUM(r.amount_cents) FROM refunds r WHERE r.bill_id = b.id), 0) AS refunded_cents - FROM bills b - WHERE b.order_id = ? - ORDER BY b.id ASC - `) - .all(req.params.id) as Array<{ - id: string; - total?: number | null; - round_off?: number | null; - paid_amount?: number | null; - refunded_cents?: number | null; - balance?: number | null; - payment_details?: unknown; - payment_status?: string | null; - }>; - const { + minorFactor, + roundCurrency, evaluatedBills, - payableBills, orderTotalMinor, + payableBills, totalOutstandingMinor, totalCollectedMinor, - requiresPaymentOverride, - } = evaluateOrderPaymentCompletion(currentOrder, billRows, minorFactor); - - const paymentBlocksCompletion = status === 'completed' && requiresPaymentOverride; + paymentBlocksCompletion: rawPaymentBlocksCompletion, + } = evaluateOrderStatusPaymentBlock( + db, + req.params.id, + currentOrder.total, + currentOrder.round_off, + ); + const paymentBlocksCompletion = status === 'completed' && rawPaymentBlocksCompletion; const requireKitchenDelivered = db .prepare( diff --git a/tests/manager-pin-verification.test.ts b/tests/manager-pin-verification.test.ts index 86501898b..1b7ba18a3 100644 --- a/tests/manager-pin-verification.test.ts +++ b/tests/manager-pin-verification.test.ts @@ -316,6 +316,74 @@ async function run() { 'PIN-only void records matching manager 1 in approved_by', ); + // Test 11: Completing unpaid order with shared PIN requires payment permission during pre-verification + // Seed two managers sharing PIN 5555: + // mgr-shared-nopay comes first alphabetically, but has payments.take denied + // mgr-shared-pay has default manager permissions including payments.take + db.prepare(` + INSERT INTO users (id, name, email, password, role, pin_hash, is_active, created_at, updated_at) + VALUES ('mgr-shared-nopay', 'Manager No Pay', 'mgrnopay@test.local', 'Pass1234!', 'manager', ?, 1, ?, ?) + `).run(bcrypt.hashSync('5555', 10), now(), now()); + db.prepare(` + INSERT INTO user_permission_overrides (user_id, permission_id, effect, updated_by, created_at, updated_at) + VALUES ('mgr-shared-nopay', 'payments.take', 'deny', 'mgr-1', ?, ?) + `).run(now(), now()); + + db.prepare(` + INSERT INTO users (id, name, email, password, role, pin_hash, is_active, created_at, updated_at) + VALUES ('mgr-shared-pay', 'Manager With Pay', 'mgrpay@test.local', 'Pass1234!', 'manager', ?, 1, ?, ?) + `).run(bcrypt.hashSync('5555', 10), now(), now()); + + const unpaidOrderRes = db + .prepare(` + INSERT INTO orders (order_number, type, subtotal, tax_amount, total, status, created_at, updated_at) + VALUES ('ORD-UNPAID-001', 'dine_in', 100, 0, 100, 'pending', ?, ?) + `) + .run(now(), now()); + const unpaidOrderId = unpaidOrderRes.lastInsertRowid; + + db.prepare(` + INSERT INTO bills (order_id, bill_number, subtotal, tax_amount, total, paid_amount, balance, payment_status, created_at, updated_at) + VALUES (?, 'BILL-UNPAID-001', 100, 0, 100, 0, 100, 'unpaid', ?, ?) + `).run(unpaidOrderId, now(), now()); + + // Test 11a: Explicit approver who lacks payments.take fails with 403 + const res11a = await request(app) + .patch(`/api/orders/${unpaidOrderId}/status`) + .set(cashierAuth) + .send({ status: 'completed', override_pin: '5555', approver_id: 'mgr-shared-nopay' }); + assertEqual(res11a.status, 403, 'Explicit approver lacking payments.take is rejected with 403'); + resetOrderPinRateLimit(); + + // Test 11b: Fallback PIN scan skips ineligible mgr-shared-nopay and authorizes mgr-shared-pay + const res11b = await request(app) + .patch(`/api/orders/${unpaidOrderId}/status`) + .set(cashierAuth) + .send({ status: 'completed', override_pin: '5555' }); + assertEqual( + res11b.status, + 200, + 'Fallback PIN scan authorizes eligible manager with payments.take', + ); + resetOrderPinRateLimit(); + + const completedOrder = db + .prepare('SELECT status FROM orders WHERE id = ?') + .get(unpaidOrderId) as any; + assertEqual(completedOrder.status, 'completed', 'Order status updated to completed'); + + const auditLog11 = db + .prepare(` + SELECT * FROM order_audit_log WHERE order_id = ? ORDER BY id DESC LIMIT 1 + `) + .get(unpaidOrderId) as any; + assert(!!auditLog11, 'Audit log entry created for completed unpaid order'); + assertEqual( + JSON.parse(auditLog11.details_json || '{}').approved_by, + 'mgr-shared-pay', + 'Audit log attributes approval to eligible mgr-shared-pay', + ); + const results = getResults(); if (results.failed > 0) { throw new Error(`${results.failed} test assertions failed`);