diff --git a/backend/api/src/routes/orderRoutes.js b/backend/api/src/routes/orderRoutes.js index ea81aaa6..560225d7 100644 --- a/backend/api/src/routes/orderRoutes.js +++ b/backend/api/src/routes/orderRoutes.js @@ -17,10 +17,10 @@ import { verifyDeliverySchema, predictDemandSchema } from '../validation/requestSchemas.js'; -import { changeDropSchema, cancelOrderSchema } from '../validation/requestSchemas.js'; import { awardReputationPoints } from '../services/reputation.js'; +import { changeDropSchema, cancelOrderSchema } from '../validation/requestSchemas.js'; import { buildDepositTx, recordDepositTx, escrowRelease, escrowRefund } from '../services/escrow.js'; -import { sendDeliveryOtpNotification } from '../services/notificationService.js'; +import { sendDeliveryOtpNotification, storeDeliveryOtp, getActiveDeliveryOtp, verifyDeliveryOtp, expireDeliveryOtps } from '../services/notificationService.js'; import { predictDemand, predictPrice } from '../services/ml.js'; import rateLimit from 'express-rate-limit'; import { z } from 'zod'; @@ -113,6 +113,16 @@ const verifyDeliveryLimiter = rateLimit({ message: { error: 'Too many delivery verification attempts. Please try again later.' }, }); +// Rate limiter for updating order milestones +const milestoneLimiter = rateLimit({ + windowMs: 60 * 1000, + max: process.env.NODE_ENV === 'test' ? 1000 : 5, + keyGenerator: (req) => req.user.id, + standardHeaders: true, + legacyHeaders: false, + message: { error: 'Too many milestone updates. Please slow down.' }, +}); + // Rate limiter for the predict-demand endpoint const predictDemandLimiter = rateLimit({ windowMs: 60 * 1000, @@ -377,10 +387,6 @@ router.get('/:id', authenticate, validateParams(paramIdSchema), async (req, res) } const responseOrder = { ...order }; - // Strip delivery OTP for drivers to prevent security bypass - if (req.user.role === 'driver' && responseOrder.delivery_otp) { - delete responseOrder.delivery_otp; - } const { data: timeline } = await supabase.from('order_timeline').select('milestone, milestone_time, completed, sort_order').eq('order_display_id', order.order_display_id).order('sort_order', { ascending: true }); @@ -711,7 +717,7 @@ router.post('/:id/bids/:bidId/accept', authenticate, requireRole(['customer']), // ============================================================================ // 12. UPDATE ORDER MILESTONE (ASSIGNED DRIVER) // ============================================================================ -router.put('/:id/milestones', authenticate, requireRole(['driver']), validateParams(paramIdSchema), validateBody(updateMilestoneSchema), async (req, res) => { +router.put('/:id/milestones', authenticate, requireRole(['driver']), milestoneLimiter, validateParams(paramIdSchema), validateBody(updateMilestoneSchema), async (req, res) => { const orderId = req.params.id; const { milestone } = req.body; @@ -735,11 +741,17 @@ router.put('/:id/milestones', authenticate, requireRole(['driver']), validatePar const updates = { status, updated_at: new Date().toISOString() }; let generatedOtp = null; - if (milestone === 'In Transit' && (!order.delivery_otp || isOtpExpired(order.otp_generated_at))) { - generatedOtp = crypto.randomInt(100000, 1000000).toString(); - updates.delivery_otp = generatedOtp; - updates.otp_generated_at = new Date().toISOString(); - await clearOtpState(orderId); + if (milestone === 'In Transit') { + const activeOtp = await getActiveDeliveryOtp(orderId); + if (!activeOtp) { + generatedOtp = crypto.randomInt(100000, 1000000).toString(); + const stored = await storeDeliveryOtp(orderId, generatedOtp, OTP_TTL_MINUTES); + if (stored) { + await clearOtpState(orderId); + } + } else { + logger.warn(`[OTP] Driver ${req.user.id} attempted OTP regeneration for order ${orderId}`); + } } const { data: updatedOrder, error: updateErr } = await supabase.from('orders').update(updates).eq('id', orderId).select('*').single(); @@ -752,13 +764,7 @@ router.put('/:id/milestones', authenticate, requireRole(['driver']), validatePar await sendDeliveryOtpNotification(order.customer_id, order.order_display_id, generatedOtp); } - // Strip delivery_otp from updatedOrder to prevent exposure to drivers - const responseOrder = { ...updatedOrder }; - if (responseOrder.delivery_otp) { - delete responseOrder.delivery_otp; - } - - const response = { message: 'Milestone updated successfully.', order: responseOrder, milestone, status }; + const response = { message: 'Milestone updated successfully.', order: updatedOrder, milestone, status }; res.json(response); } catch (err) { @@ -783,19 +789,19 @@ router.post('/:id/verify-delivery', authenticate, requireRole(['driver']), verif } try { - const { data: order, error: orderErr } = await supabase.from('orders').select('*').eq('id', orderId).maybeSingle(); + const { data: order, error: orderErr } = await supabase.from('orders').select('id, order_display_id, driver_id, customer_id, escrow_status, status').eq('id', orderId).maybeSingle(); if (orderErr || !order) return res.status(404).json({ error: 'Order not found.' }); if (order.driver_id !== req.user.id) return res.status(403).json({ error: 'Access Denied: You are not assigned to this order.' }); - if (!order.delivery_otp || order.otp_verified) return res.status(400).json({ error: 'OTP not available or already verified.' }); - // Check OTP expiry - if (isOtpExpired(order.otp_generated_at)) { + const otpRecord = await getActiveDeliveryOtp(orderId); + if (!otpRecord) { return res.status(400).json({ - error: 'OTP has expired. Please request a new delivery OTP.', + error: 'OTP not available or has expired. Please request a new delivery OTP.', }); } - if (order.delivery_otp !== String(otp)) { + const submittedHash = crypto.createHash('sha256').update(String(otp)).digest('hex'); + if (otpRecord.otp_hash !== submittedHash) { const count = await recordOtpFailure(orderId); const remaining = Math.max(0, OTP_MAX_FAILED_ATTEMPTS - count); const message = remaining > 0 @@ -807,15 +813,15 @@ router.post('/:id/verify-delivery', authenticate, requireRole(['driver']), verif // Successful verification — clear failure state await clearOtpState(orderId); + await verifyDeliveryOtp(orderId); const { data: preUpdatedOrder, error: updateErr } = await supabase.from('orders').update({ - otp_verified: true, updated_at: new Date().toISOString() + updated_at: new Date().toISOString() }) .eq('id', orderId) - .not('otp_verified', 'eq', true) .not('status', 'eq', 'cancelled') .not('status', 'eq', 'payment_released') - .select('*') + .select('id, order_display_id, status') .single(); if (updateErr) { @@ -832,20 +838,9 @@ router.post('/:id/verify-delivery', authenticate, requireRole(['driver']), verif return res.status(500).json({ error: 'Failed to complete trip and release payment.', details: rpcErr.message }); } - // Fetch the updated order directly from the database as the single source of truth - const { data: updatedOrder, error: fetchErr } = await supabase - .from('orders') - .select('*') - .eq('id', orderId) - .single(); - - if (fetchErr) { - logger.error('Failed to fetch updated order:', fetchErr.message); - return res.status(500).json({ error: 'Failed to retrieve completed order details.', details: fetchErr.message }); - } // Escrow: release funds to driver after successful delivery verification - if (updatedOrder.escrow_status === 'funded') { + if (order.escrow_status === 'funded') { try { const { txHash } = await escrowRelease(order.order_display_id); if (txHash) { @@ -859,16 +854,10 @@ router.post('/:id/verify-delivery', authenticate, requireRole(['driver']), verif logger.error('[escrow] Release failed for order', orderId, ':', releaseErr.message); } } else { - logger.info(`[escrow] Escrow not funded (status: ${updatedOrder.escrow_status}) — skipping on-chain release.`); + logger.info(`[escrow] Escrow not funded (status: ${order.escrow_status}) — skipping on-chain release.`); } - // Strip delivery_otp from updatedOrder to prevent exposure - const responseOrder = { ...updatedOrder }; - if (responseOrder.delivery_otp) { - delete responseOrder.delivery_otp; - } - - res.json({ message: 'Delivery verified successfully! Payment released to driver.', order: responseOrder }); + res.json({ message: 'Delivery verified successfully! Payment released to driver.' }); } catch (err) { res.status(500).json({ error: 'Internal Server Error' }); } @@ -961,10 +950,22 @@ router.post('/:id/cancel', authenticate, requireRole(['customer']), validatePara if (!order) return res.status(404).json({ error: 'Order not found.' }); if (order.customer_id !== req.user.id) return res.status(403).json({ error: 'Access Denied: You do not own this order.' }); + // Prevent cancellation if delivery OTP was already verified + const { data: otpCheck, error: otpCheckErr } = await supabase + .from('delivery_otps') + .select('id') + .eq('order_id', order.id) + .eq('verified', true) + .limit(1) + .maybeSingle(); + + if (!otpCheckErr && otpCheck) { + return res.status(409).json({ error: 'Cannot cancel: delivery OTP has already been verified.' }); + } + const { data: updatedOrder, error: updateErr } = await supabase.from('orders') .update({ status: 'cancelled', cancellation_reason: reason, updated_at: new Date().toISOString() }) .eq('order_display_id', orderId) - .not('otp_verified', 'eq', true) .not('status', 'in', '("delivered","payment_released","cancelled")') .select('cancellation_fee, order_display_id, status, cancellation_reason, escrow_status') .single(); diff --git a/backend/api/src/services/notificationService.js b/backend/api/src/services/notificationService.js index 5043dc58..894e25a0 100644 --- a/backend/api/src/services/notificationService.js +++ b/backend/api/src/services/notificationService.js @@ -1,5 +1,6 @@ import { supabase, firebaseAdmin } from '../config/db.js'; import logger from '../middleware/logger.js'; +import crypto from 'crypto'; /** * Fetch a user's FCM token from the profiles table. @@ -89,7 +90,107 @@ export async function sendFcmNotification(userId, notification, data = {}) { } /** - * Deliver the delivery OTP to the customer through a secure out-of-band channel. + * Persist a delivery OTP in the isolated delivery_otps table. + * Called when the order transitions to 'In Transit' and a fresh OTP is needed. + * + * @param {string} orderId - The order UUID. + * @param {string} otp - The 6-digit delivery OTP. + * @param {number} ttlMinutes - Time-to-live for the OTP (defaults to 15). + * @returns {Promise<{id: string} | null>} + */ +export async function storeDeliveryOtp(orderId, otp, ttlMinutes = 15) { + const expiresAt = new Date(Date.now() + ttlMinutes * 60 * 1000).toISOString(); + const otpHash = crypto.createHash('sha256').update(String(otp)).digest('hex'); + + const { data, error } = await supabase + .from('delivery_otps') + .insert({ + order_id: orderId, + otp_hash: otpHash, + expires_at: expiresAt, + verified: false, + }) + .select('id') + .single(); + + if (error) { + console.error('[NotificationService] Failed to store OTP:', error.message); + return null; + } + + console.log(`[NotificationService] OTP stored for order ${orderId}, expires at ${expiresAt}`); + return data; +} + +/** + * Retrieve the latest active (unexpired, unverified) OTP for an order. + * + * @param {string} orderId + * @returns {Promise<{id: string, otp_hash: string, expires_at: string} | null>} + */ +export async function getActiveDeliveryOtp(orderId) { + const { data, error } = await supabase + .from('delivery_otps') + .select('id, otp_hash, expires_at') + .eq('order_id', orderId) + .eq('verified', false) + .gte('expires_at', new Date().toISOString()) + .order('created_at', { ascending: false }) + .limit(1) + .maybeSingle(); + + if (error) { + console.error('[NotificationService] Failed to fetch active OTP:', error.message); + return null; + } + + return data; +} + +/** + * Mark a delivery OTP as verified. + * + * @param {string} orderId + * @returns {Promise} + */ +export async function verifyDeliveryOtp(orderId) { + const { error } = await supabase + .from('delivery_otps') + .update({ + verified: true, + verified_at: new Date().toISOString(), + }) + .eq('order_id', orderId) + .eq('verified', false); + + if (error) { + console.error('[NotificationService] Failed to verify OTP:', error.message); + return false; + } + + return true; +} + +/** + * Invalidate (expire) all active OTPs for an order. + * + * @param {string} orderId + * @returns {Promise} + */ +export async function expireDeliveryOtps(orderId) { + const { error } = await supabase + .from('delivery_otps') + .update({ expires_at: new Date().toISOString() }) + .eq('order_id', orderId) + .eq('verified', false); + + if (error) { + console.error('[NotificationService] Failed to expire OTPs:', error.message); + } +} + +/** + * Deliver the delivery OTP to the customer through out-of-band channels. * * @param {string} customerId - The customer's profile UUID. * @param {string} orderDisplayId - The display identifier of the order (e.g. #FFYYYYMMDDXXXX). @@ -163,7 +264,6 @@ export async function sendDeliveryOtpNotification(customerId, orderDisplayId, ot * @param {object} [metadata={}] - Optional metadata to persist. */ export async function sendPushNotification(userId, title, body, notifType, metadata = {}) { - // 1. Persist notification record if (supabase) { try { const { error } = await supabase @@ -178,6 +278,5 @@ export async function sendPushNotification(userId, title, body, notifType, metad } } - // 2. FCM delivery (fire-and-forget) void sendFcmNotification(userId, { title, body }, { notifType, ...metadata }); } diff --git a/backend/api/test/integration/orders.test.js b/backend/api/test/integration/orders.test.js index bbdcabfa..019d88d2 100644 --- a/backend/api/test/integration/orders.test.js +++ b/backend/api/test/integration/orders.test.js @@ -15,6 +15,7 @@ */ import { describe, it, expect, beforeEach, vi } from 'vitest'; import request from 'supertest'; +import crypto from 'crypto'; const routeEstimateMock = vi.fn(); @@ -710,18 +711,17 @@ describe('GET /api/orders/:id — order details', () => { expect(res.body.driver.name).toBe('Test Driver'); }); - it('exposes delivery_otp to customer but strips it for driver', async () => { + it('does not expose delivery_otp on order for any role (isolated table)', async () => { m.store.orders.push({ id: 'order-3', customer_id: 'customer-123', driver_id: 'driver-123', order_display_id: 'OD3', - delivery_otp: '654321', }); const app = buildApp(); - // 1. Customer request + // 1. Customer request — no delivery_otp on order object const customerRes = await request(app) .get('/api/orders/order-3') .set({ @@ -729,9 +729,9 @@ describe('GET /api/orders/:id — order details', () => { 'x-user-role': 'customer' }); expect(customerRes.status).toBe(200); - expect(customerRes.body.order.delivery_otp).toBe('654321'); + expect(customerRes.body.order).not.toHaveProperty('delivery_otp'); - // 2. Driver request + // 2. Driver request — also no delivery_otp const driverRes = await request(app) .get('/api/orders/order-3') .set({ @@ -970,14 +970,13 @@ describe('Delivery OTP Verification and Milestones', () => { expect(res.body.error).toBe('Cannot set Delivered milestone directly. Use /verify-delivery endpoint to confirm delivery.'); }); - it('generates OTP but does not return it in response when moving to In Transit milestone', async () => { + it('generates OTP in isolated table but does not return it in response when moving to In Transit milestone', async () => { m.store.orders = [{ id: 'order-1', customer_id: 'customer-456', driver_id: 'driver-123', order_display_id: 'ORD001', - status: 'picked_up', - otp_verified: false + status: 'picked_up' }]; m.store.order_timeline = [{ order_display_id: 'ORD001', @@ -985,6 +984,7 @@ describe('Delivery OTP Verification and Milestones', () => { completed: false }]; m.store.notifications = []; + m.store.delivery_otps = []; // ensure no stale OTP from prior tests const app = buildApp(); const res = await request(app) @@ -999,15 +999,18 @@ describe('Delivery OTP Verification and Milestones', () => { expect(res.body).not.toHaveProperty('otp'); expect(res.body.order).not.toHaveProperty('delivery_otp'); - const order = m.store.orders.find(o => o.id === 'order-1'); - expect(order.delivery_otp).toMatch(/^\d{6}$/); // 6-digit OTP - expect(order.otp_verified).toBe(false); - expect(order.otp_generated_at).toBeDefined(); + const otpRecord = m.store.delivery_otps.find(o => o.order_id === 'order-1'); + expect(otpRecord).toBeTruthy(); + expect(otpRecord.otp_hash).toMatch(/^[a-f0-9]{64}$/); // SHA-256 hash + expect(otpRecord.verified).toBe(false); + expect(otpRecord.expires_at).toBeDefined(); // Verify customer notification was created const notification = m.store.notifications.find(n => n.user_id === 'customer-456'); expect(notification).toBeTruthy(); - expect(notification.body).toContain(order.delivery_otp); + const otpInNotification = notification.body.match(/\b\d{6}\b/)[0]; + const expectedHash = crypto.createHash('sha256').update(otpInNotification).digest('hex'); + expect(otpRecord.otp_hash).toBe(expectedHash); expect(notification.notif_type).toBe('order_update'); }); @@ -1051,10 +1054,15 @@ describe('Delivery OTP Verification and Milestones', () => { m.store.orders = [{ id: 'order-1', driver_id: 'driver-123', - order_display_id: 'ORD001', - delivery_otp: '123456', - otp_verified: false, - otp_generated_at: new Date().toISOString() + order_display_id: 'ORD001' + }]; + m.store.delivery_otps = [{ + id: 'otp-1', + order_id: 'order-1', + otp_hash: crypto.createHash('sha256').update('123456').digest('hex'), + expires_at: new Date(Date.now() + 15 * 60 * 1000).toISOString(), + verified: false, + created_at: new Date().toISOString() }]; const app = buildApp(); @@ -1075,10 +1083,15 @@ describe('Delivery OTP Verification and Milestones', () => { id: 'order-1', driver_id: 'driver-123', order_display_id: 'ORD001', - delivery_otp: '123456', - otp_verified: false, - status: 'in_transit', - otp_generated_at: new Date().toISOString() + status: 'in_transit' + }]; + m.store.delivery_otps = [{ + id: 'otp-1', + order_id: 'order-1', + otp_hash: crypto.createHash('sha256').update('123456').digest('hex'), + expires_at: new Date(Date.now() + 15 * 60 * 1000).toISOString(), + verified: false, + created_at: new Date().toISOString() }]; m.store.order_timeline = [{ order_display_id: 'ORD001', @@ -1097,12 +1110,14 @@ describe('Delivery OTP Verification and Milestones', () => { expect(res.status).toBe(200); expect(res.body.message).toMatch(/Delivery verified successfully/i); - expect(res.body.order).not.toHaveProperty('delivery_otp'); const order = m.store.orders.find(o => o.id === 'order-1'); - expect(order.otp_verified).toBe(true); expect(order.status).toBe('payment_released'); + // OTP record should be marked as verified in isolated table + const otpRecord = m.store.delivery_otps.find(o => o.order_id === 'order-1'); + expect(otpRecord.verified).toBe(true); + const timeline = m.store.order_timeline.find(t => t.order_display_id === 'ORD001' && t.milestone === 'Delivered'); expect(timeline.completed).toBe(true); @@ -1116,10 +1131,15 @@ describe('Delivery OTP Verification and Milestones', () => { id: 'order-expired', driver_id: 'driver-123', order_display_id: 'ORD-EXP', - delivery_otp: '123456', - otp_verified: false, - status: 'in_transit', - otp_generated_at: new Date(Date.now() - 20 * 60 * 1000).toISOString() // 20 minutes ago (TTL is 15) + status: 'in_transit' + }]; + m.store.delivery_otps = [{ + id: 'otp-expired', + order_id: 'order-expired', + otp_hash: crypto.createHash('sha256').update('123456').digest('hex'), + expires_at: new Date(Date.now() - 1 * 60 * 1000).toISOString(), // expired 1 minute ago + verified: false, + created_at: new Date(Date.now() - 20 * 60 * 1000).toISOString() }]; const app = buildApp(); @@ -1141,10 +1161,15 @@ describe('Delivery OTP Verification and Milestones', () => { id: orderId, driver_id: 'driver-123', order_display_id: 'ORD-LOCK', - delivery_otp: '123456', - otp_verified: false, - status: 'in_transit', - otp_generated_at: new Date().toISOString() + status: 'in_transit' + }]; + m.store.delivery_otps = [{ + id: 'otp-lockout', + order_id: orderId, + otp_hash: crypto.createHash('sha256').update('123456').digest('hex'), + expires_at: new Date(Date.now() + 15 * 60 * 1000).toISOString(), + verified: false, + created_at: new Date().toISOString() }]; const app = buildApp(); @@ -1188,8 +1213,8 @@ describe('Delivery OTP Verification and Milestones', () => { const originalNow = Date.now; Date.now = () => originalNow() + 31 * 60 * 1000; - // Update the generated time so the OTP itself isn't expired - m.store.orders.find(o => o.id === orderId).otp_generated_at = new Date(Date.now()).toISOString(); + // Update expires_at so the OTP itself isn't expired after lockout bypass + m.store.delivery_otps[0].expires_at = new Date(Date.now() + 15 * 60 * 1000).toISOString(); try { // Correct OTP should now succeed @@ -1213,10 +1238,15 @@ describe('Delivery OTP Verification and Milestones', () => { id: orderId, driver_id: 'driver-123', order_display_id: 'ORD-CLEAR', - delivery_otp: '123456', - otp_verified: false, - status: 'in_transit', - otp_generated_at: new Date().toISOString() + status: 'in_transit' + }]; + m.store.delivery_otps = [{ + id: 'otp-clear-state', + order_id: orderId, + otp_hash: crypto.createHash('sha256').update('123456').digest('hex'), + expires_at: new Date(Date.now() + 15 * 60 * 1000).toISOString(), + verified: false, + created_at: new Date().toISOString() }]; const app = buildApp(); @@ -1232,7 +1262,7 @@ describe('Delivery OTP Verification and Milestones', () => { .send({ otp: '000000' }); } - // Succeed + // Succeed — this calls verifyDeliveryOtp which marks the OTP as verified const resSuccess = await request(app) .post(`/api/orders/${orderId}/verify-delivery`) .set({ @@ -1242,9 +1272,10 @@ describe('Delivery OTP Verification and Milestones', () => { .send({ otp: '123456' }); expect(resSuccess.status).toBe(200); - // Reset verification status manually in the mock store to test lockout reset - const order = m.store.orders.find(o => o.id === orderId); - order.otp_verified = false; + // Reset OTP record in isolated table so subsequent verifications can find it + const otpRecord = m.store.delivery_otps.find(o => o.order_id === orderId); + otpRecord.verified = false; + delete otpRecord.verified_at; // Fail 4 more times (if state wasn't cleared, this would lockout since total failures would be 7) for (let i = 1; i <= 4; i++) { @@ -1260,17 +1291,22 @@ describe('Delivery OTP Verification and Milestones', () => { } }); - it('regenerates OTP but does not return it in response when milestone In Transit is called and existing OTP has expired', async () => { + it('regenerates OTP in isolated table when milestone In Transit is called and existing OTP has expired', async () => { const orderId = 'order-regen'; m.store.orders = [{ id: orderId, customer_id: 'customer-456', driver_id: 'driver-123', order_display_id: 'ORD-REGEN', - delivery_otp: '123456', - otp_verified: false, - status: 'in_transit', - otp_generated_at: new Date(Date.now() - 20 * 60 * 1000).toISOString() + status: 'in_transit' + }]; + m.store.delivery_otps = [{ + id: 'otp-regen-old', + order_id: orderId, + otp_hash: crypto.createHash('sha256').update('123456').digest('hex'), + expires_at: new Date(Date.now() - 1 * 60 * 1000).toISOString(), // expired + verified: false, + created_at: new Date(Date.now() - 20 * 60 * 1000).toISOString() }]; m.store.order_timeline = [{ order_display_id: 'ORD-REGEN', @@ -1292,15 +1328,25 @@ describe('Delivery OTP Verification and Milestones', () => { expect(res.body).not.toHaveProperty('otp'); expect(res.body.order).not.toHaveProperty('delivery_otp'); - const order = m.store.orders.find(o => o.id === orderId); - expect(order.delivery_otp).not.toBe('123456'); - expect(order.delivery_otp).toMatch(/^\d{6}$/); - expect(new Date(order.otp_generated_at).getTime()).toBeGreaterThan(Date.now() - 5000); + // The old OTP should still be in the table + const oldOtp = m.store.delivery_otps.find(o => o.id === 'otp-regen-old'); + expect(oldOtp).toBeTruthy(); - // Verify customer notification was created + // A new OTP should have been created + const expectedOldHash = crypto.createHash('sha256').update('123456').digest('hex'); + const newOtps = m.store.delivery_otps.filter(o => o.order_id === orderId && o.otp_hash !== expectedOldHash); + expect(newOtps.length).toBeGreaterThan(0); + const newOtp = newOtps[0]; + expect(newOtp.otp_hash).toMatch(/^[a-f0-9]{64}$/); + expect(newOtp.verified).toBe(false); + expect(newOtp.expires_at).toBeDefined(); + + // Verify customer notification was created with the new OTP const notification = m.store.notifications.find(n => n.user_id === 'customer-456'); expect(notification).toBeTruthy(); - expect(notification.body).toContain(order.delivery_otp); + const otpInNotification = notification.body.match(/\b\d{6}\b/)[0]; + const expectedNewHash = crypto.createHash('sha256').update(otpInNotification).digest('hex'); + expect(newOtp.otp_hash).toBe(expectedNewHash); }); describe('Redis-based rate limiting & error fallback resilience', () => { @@ -1360,10 +1406,15 @@ describe('Delivery OTP Verification and Milestones', () => { id: 'order-redis-active', driver_id: 'driver-123', customer_id: 'customer-456', - order_display_id: 'ORD-REDIS', - delivery_otp: '123456', - otp_verified: false, - otp_generated_at: new Date().toISOString() + order_display_id: 'ORD-REDIS' + }]; + m.store.delivery_otps = [{ + id: 'otp-redis-active', + order_id: 'order-redis-active', + otp_hash: crypto.createHash('sha256').update('123456').digest('hex'), + expires_at: new Date(Date.now() + 15 * 60 * 1000).toISOString(), + verified: false, + created_at: new Date().toISOString() }]; const app = buildApp(); @@ -1396,10 +1447,10 @@ describe('Delivery OTP Verification and Milestones', () => { expect(res6.status).toBe(429); expect(res6.body.error).toContain('Too many failed OTP attempts'); - // Expire the OTP now, so that the In Transit milestone update triggers regeneration and clear - m.store.orders[0].otp_generated_at = new Date(Date.now() - 20 * 60 * 1000).toISOString(); + // Expire the OTP in the isolated table so the milestone triggers regeneration + m.store.delivery_otps[0].expires_at = new Date(Date.now() - 1 * 60 * 1000).toISOString(); - // Milestone change clears lockout in Redis + // Milestone change clears lockout in Redis and generates new OTP await request(app) .put('/api/orders/order-redis-active/milestones') .set({ 'x-user-id': 'driver-123', 'x-user-role': 'driver' }) @@ -1415,10 +1466,15 @@ describe('Delivery OTP Verification and Milestones', () => { id: 'order-redis-failing', driver_id: 'driver-123', customer_id: 'customer-456', - order_display_id: 'ORD-FAIL-REDIS', - delivery_otp: '123456', - otp_verified: false, - otp_generated_at: new Date().toISOString() + order_display_id: 'ORD-FAIL-REDIS' + }]; + m.store.delivery_otps = [{ + id: 'otp-redis-failing', + order_id: 'order-redis-failing', + otp_hash: crypto.createHash('sha256').update('123456').digest('hex'), + expires_at: new Date(Date.now() + 15 * 60 * 1000).toISOString(), + verified: false, + created_at: new Date().toISOString() }]; const app = buildApp(); diff --git a/docs/migration_add_delivery_otp.sql b/docs/migration_add_delivery_otp.sql index 68d8dd3c..5c805cb7 100644 --- a/docs/migration_add_delivery_otp.sql +++ b/docs/migration_add_delivery_otp.sql @@ -1,56 +1,52 @@ --- Migration: Add delivery OTP functionality to orders table --- Add columns for delivery OTP -ALTER TABLE orders ADD COLUMN IF NOT EXISTS delivery_otp TEXT; -ALTER TABLE orders ADD COLUMN IF NOT EXISTS otp_verified BOOLEAN DEFAULT false; -ALTER TABLE orders ADD COLUMN IF NOT EXISTS otp_generated_at TIMESTAMPTZ; - --- RPC Function: Complete trip and release payment to driver (by order ID) -CREATE OR REPLACE FUNCTION complete_trip_tx(p_order_id UUID) -RETURNS void -LANGUAGE plpgsql -SECURITY DEFINER -AS $$ -DECLARE - v_order RECORD; -BEGIN - -- Get the order details - SELECT * INTO v_order FROM orders WHERE id = p_order_id; - - IF NOT FOUND THEN - RAISE EXCEPTION 'Order not found'; - END IF; - - IF v_order.driver_id IS NULL THEN - RAISE EXCEPTION 'No driver assigned to this order'; - END IF; - - -- Update driver's wallet - UPDATE driver_details - SET - total_trips = total_trips + 1, - wallet_confirmed = wallet_confirmed + v_order.total_amount, - wallet_total = wallet_total + v_order.total_amount, - updated_at = NOW() - WHERE user_id = v_order.driver_id; - - -- Log wallet transaction - INSERT INTO wallet_transactions ( - driver_id, order_display_id, amount, txn_type, status, description - ) VALUES ( - v_order.driver_id, - v_order.order_display_id, - v_order.total_amount, - 'credit', - 'confirmed', - 'Payout for Order ' || v_order.order_display_id +-- Migration: Isolate delivery OTP into a dedicated table with RLS +-- Removes OTP columns from orders to prevent leak via broad RLS policies. + +-- 1. Create the isolated delivery_otps table +CREATE TABLE IF NOT EXISTS delivery_otps ( + id UUID PRIMARY KEY DEFAULT gen_random_uuid(), + order_id UUID NOT NULL REFERENCES orders(id) ON DELETE CASCADE, + otp_hash TEXT NOT NULL, + expires_at TIMESTAMPTZ NOT NULL, + verified BOOLEAN NOT NULL DEFAULT false, + verified_at TIMESTAMPTZ, + created_at TIMESTAMPTZ NOT NULL DEFAULT NOW() +); + +CREATE INDEX IF NOT EXISTS idx_delivery_otps_order_id ON delivery_otps(order_id); +CREATE INDEX IF NOT EXISTS idx_delivery_otps_expires_at ON delivery_otps(expires_at); + +-- 2. Enable RLS +ALTER TABLE delivery_otps ENABLE ROW LEVEL SECURITY; + +-- 3. RLS policies + +-- Customers can read their own delivery OTPs (needed for verification flow) +CREATE POLICY customer_select_delivery_otp ON delivery_otps + FOR SELECT + USING ( + order_id IN ( + SELECT id FROM orders WHERE customer_id = auth.uid() + ) ); - - -- Update daily earnings summary - INSERT INTO earnings_daily (driver_id, day_date, amount, trip_count) - VALUES (v_order.driver_id, CURRENT_DATE, v_order.total_amount, 1) - ON CONFLICT (driver_id, day_date) - DO UPDATE SET - amount = earnings_daily.amount + EXCLUDED.amount, - trip_count = earnings_daily.trip_count + 1; -END; -$$; + +-- Drivers cannot select delivery OTPs at all +CREATE POLICY no_driver_select_delivery_otp ON delivery_otps + FOR SELECT + USING (false); + +-- Only the service role (backend) can insert / update delivery OTPs +CREATE POLICY service_insert_delivery_otp ON delivery_otps + FOR INSERT + WITH CHECK (auth.role() = 'service_role'); + +CREATE POLICY service_update_delivery_otp ON delivery_otps + FOR UPDATE + USING (auth.role() = 'service_role') + WITH CHECK (auth.role() = 'service_role'); + +-- 4. Drop old OTP columns from orders table +ALTER TABLE orders DROP COLUMN IF EXISTS delivery_otp; +ALTER TABLE orders DROP COLUMN IF EXISTS otp_verified; +ALTER TABLE orders DROP COLUMN IF EXISTS otp_generated_at; + +-- 5. Update complete_trip_tx RPC (no OTP changes needed — it uses orders.total_amount only)