From 83ec5ecd36618054ace4121e87117e8eb465af1a Mon Sep 17 00:00:00 2001 From: denisireri Date: Mon, 12 Oct 2026 11:52:35 +0300 Subject: [PATCH] Enforce tenant isolation in the data layer instead of per route Every route had to remember to add the business filter to every query; nothing stopped one that forgot. The audit for this change found several that had: the M-Pesa status re-check, the stock ledger's duplicate checks, the daily-inventory and branch-price upserts, a cash-sale replay lookup, several load-by-id lookups, and every populate(). - tenantGuard: a Mongoose plugin on all ten business-scoped models. Any find/update/delete/count/distinct, or an aggregate whose first stage is not a $match, that does not pin exactly one businessId throws TenantScopeError before reaching the database ($in, $exists and other operators do not count). - crossTenant("reason", fn): the explicit, documented exemption for the queries that must span businesses - login's email lookup, refresh and invite-token lookups, the M-Pesa callback, the platform console and the maintenance scripts. git grep crossTenant lists them all. - scopedPopulate: populated branch, product and staff names are also restricted to the business. - Every flagged query site now names the business (or is exempted with a reason). - Tests: a check that every model with a businessId field carries the guard; the guard refusing unscoped queries of every kind; and a route sweep where business A sends business B's ids to every read route (nothing of B's comes back) and every write route (all refused, B's data unchanged). Test-side database inspections go through db(), an explicit test exemption. Removing one route's business filter makes the suite fail. Co-Authored-By: Claude Opus 5.5 --- docs/PILOT_READINESS.md | 27 ++- scripts/migrate-multitenant.ts | 3 +- scripts/migrate-per-business-identity.ts | 3 +- scripts/seed.ts | 3 +- src/app/api/auth/login/route.ts | 7 +- src/app/api/auth/me/route.ts | 4 +- src/app/api/auth/refresh/route.ts | 5 +- src/app/api/auth/signup/route.ts | 2 +- src/app/api/branch-prices/route.ts | 2 +- src/app/api/branches/route.ts | 2 +- src/app/api/expenses/route.ts | 5 +- src/app/api/inventory/dispatch/route.ts | 2 +- src/app/api/inventory/evening-count/route.ts | 2 +- src/app/api/invites/[token]/route.ts | 5 +- src/app/api/mpesa/callback/route.ts | 5 +- .../api/mpesa/status/[transactionId]/route.ts | 2 +- src/app/api/platform/businesses/route.ts | 28 +-- src/app/api/purchases/route.ts | 5 +- src/app/api/stock/adjustments/route.ts | 2 +- src/app/api/stock/movements/route.ts | 7 +- src/app/api/stock/transfers/route.ts | 9 +- src/app/api/transactions/cash/route.ts | 6 +- src/app/api/transactions/route.ts | 3 +- src/lib/server/models/Branch.ts | 3 + src/lib/server/models/DailyInventory.ts | 3 + src/lib/server/models/Expense.ts | 3 + src/lib/server/models/Product.ts | 5 + src/lib/server/models/Purchase.ts | 3 + src/lib/server/models/StockMovement.ts | 3 + src/lib/server/models/StockTransfer.ts | 3 + src/lib/server/models/Transaction.ts | 3 + src/lib/server/models/User.ts | 3 + src/lib/server/platform.ts | 4 +- src/lib/server/stock.ts | 10 +- src/lib/server/tenantGuard.ts | 114 ++++++++++++ tests/access.test.ts | 8 +- tests/branchPrices.test.ts | 6 +- tests/concurrency.test.ts | 4 +- tests/followups.test.ts | 14 +- tests/helpers.ts | 6 + tests/identity.test.ts | 4 +- tests/isolation.test.ts | 172 ++++++++++++++++++ tests/sync.test.ts | 14 +- 43 files changed, 447 insertions(+), 77 deletions(-) create mode 100644 src/lib/server/tenantGuard.ts create mode 100644 tests/isolation.test.ts diff --git a/docs/PILOT_READINESS.md b/docs/PILOT_READINESS.md index 0807a4a..286d436 100644 --- a/docs/PILOT_READINESS.md +++ b/docs/PILOT_READINESS.md @@ -157,10 +157,31 @@ platform-wide unique indexes for per-business ones. Until it runs, operators cannot open the Platform console and a second business cannot reuse an email. It is safe to re-run. +## Tenant isolation is enforced + +Every model that holds one business's data carries a guard +(`src/lib/server/tenantGuard.ts`). A query that does not name exactly one +business (`businessId: `) throws `TenantScopeError` before it reaches +the database, so a route that forgets the business filter fails loudly in +development and in the tests instead of reading or changing another +business's data. + +- Guarded: User, Branch, Product, BranchPricing, Transaction, + DailyInventory, Expense, Purchase, StockMovement, StockTransfer. A test + fails if a new model with a `businessId` field is added without the guard. +- Queries that genuinely span businesses must be wrapped in + `crossTenant("reason", ...)`: login's email lookup, refresh-session and + invite-token lookups, the M-Pesa callback, the platform console, and the + maintenance scripts. `git grep crossTenant` lists every one. +- Related records are loaded with `scopedPopulate`, so a populated branch, + product or staff name is also restricted to the business. +- `tests/isolation.test.ts` sweeps every read route with another business's + ids (nothing of theirs comes back) and every write route (all refused, + their data unchanged). +- Not guarded, by design: Business (the tenant itself, always looked up by + the session's id), UserSession, MpesaCallbackLog and StockLock. + ## Still open -- **Isolation by convention.** Tenant filters are still written per route. - The tests cover the sales, stock, transfer and access paths, not every - route. - **Cost of goods is an average.** It uses each product's average buying price across the business, not first-in-first-out per branch. Units sold with no purchase on record are counted and flagged, but have no cost. diff --git a/scripts/migrate-multitenant.ts b/scripts/migrate-multitenant.ts index 741981d..2aa9b4d 100644 --- a/scripts/migrate-multitenant.ts +++ b/scripts/migrate-multitenant.ts @@ -13,6 +13,7 @@ import { Expense } from "../src/lib/server/models/Expense"; import { Purchase } from "../src/lib/server/models/Purchase"; import { Business } from "../src/lib/server/models/Business"; import { DEFAULT_TIER, TRIAL_DAYS } from "../src/lib/tiers"; +import { crossTenant } from "../src/lib/server/tenantGuard"; async function migrate() { await connectDb(); @@ -75,7 +76,7 @@ async function migrate() { process.exit(0); } -migrate().catch((err) => { +crossTenant("one-off migration of data from before businesses existed", migrate).catch((err) => { console.error("migration failed:", err.message); process.exit(1); }); \ No newline at end of file diff --git a/scripts/migrate-per-business-identity.ts b/scripts/migrate-per-business-identity.ts index d3ecff7..42beddf 100644 --- a/scripts/migrate-per-business-identity.ts +++ b/scripts/migrate-per-business-identity.ts @@ -6,6 +6,7 @@ import mongoose from "mongoose"; import { connectDb } from "../src/lib/server/db"; import { User } from "../src/lib/server/models/User"; import { platformAdminEmails } from "../src/lib/server/platform"; +import { crossTenant } from "../src/lib/server/tenantGuard"; /** * One-off, safe to re-run. Moves user identity from platform-wide to @@ -43,7 +44,7 @@ async function main() { await mongoose.disconnect(); } -main().catch((err) => { +crossTenant("identity migration spans every business", main).catch((err) => { console.error("migration failed:", err.message); process.exit(1); }); diff --git a/scripts/seed.ts b/scripts/seed.ts index 3b677c2..9a9ba26 100644 --- a/scripts/seed.ts +++ b/scripts/seed.ts @@ -14,6 +14,7 @@ import { User } from "../src/lib/server/models/User"; import { Product } from "../src/lib/server/models/Product"; import { Business } from "../src/lib/server/models/Business"; import { isPlatformAdminEmail } from "../src/lib/server/platform"; +import { crossTenant } from "../src/lib/server/tenantGuard"; /** * Run once against a fresh database: `npm run seed`. @@ -107,7 +108,7 @@ async function seed() { process.exit(0); } -seed().catch((err) => { +crossTenant("seeding may find a legacy admin outside any business", seed).catch((err) => { console.error("seed failed:", err.message); process.exit(1); }); diff --git a/src/app/api/auth/login/route.ts b/src/app/api/auth/login/route.ts index c2cf544..b6da50b 100644 --- a/src/app/api/auth/login/route.ts +++ b/src/app/api/auth/login/route.ts @@ -9,6 +9,7 @@ import { signAccessToken, generateRefreshToken, hashRefreshToken, refreshTtlMs } import { ACCESS_COOKIE, REFRESH_COOKIE, accessCookieOptions, refreshCookieOptions } from "@/lib/server/auth"; import { isPlatformAdmin } from "@/lib/server/platform"; import { objectId } from "@/lib/server/validation"; +import { crossTenant } from "@/lib/server/tenantGuard"; import { getEnv } from "@/lib/server/env"; import { tenantAccessError } from "@/lib/server/subscription"; @@ -54,7 +55,9 @@ export async function POST(req: NextRequest) { // several businesses, each with its own password. const filter: Record = { email: email.toLowerCase(), isActive: true }; if (businessId) filter.businessId = businessId; - const candidates = await User.find(filter).select("+passwordHash"); + const candidates = await crossTenant("login looks an email up in every business", () => + User.find(filter).select("+passwordHash").exec() + ); if (candidates.length === 0) { return NextResponse.json({ error: "Incorrect email or password" }, { status: 401 }); } @@ -149,7 +152,7 @@ export async function POST(req: NextRequest) { deviceId, }); - const branch = user.branchId ? await Branch.findById(user.branchId).lean() : null; + const branch = user.branchId ? await Branch.findOne({ _id: user.branchId, businessId: user.businessId }).lean() : null; const business = await Business.findById(user.businessId).lean(); const res = NextResponse.json({ diff --git a/src/app/api/auth/me/route.ts b/src/app/api/auth/me/route.ts index da965d7..28373f6 100644 --- a/src/app/api/auth/me/route.ts +++ b/src/app/api/auth/me/route.ts @@ -16,11 +16,11 @@ export async function GET(req: NextRequest) { const auth = getAuth(req); if (!auth) return NextResponse.json({ error: "Not authenticated" }, { status: 401 }); - const user = await User.findById(auth.sub).lean(); + const user = await User.findOne({ _id: auth.sub, businessId: auth.businessId }).lean(); if (!user || !user.isActive) { return NextResponse.json({ error: "Not authenticated" }, { status: 401 }); } - const branch = user.branchId ? await Branch.findById(user.branchId).lean() : null; + const branch = user.branchId ? await Branch.findOne({ _id: user.branchId, businessId: user.businessId }).lean() : null; const business = await Business.findById(user.businessId).lean(); const tier = (business?.tier as TierId) ?? "STARTER"; diff --git a/src/app/api/auth/refresh/route.ts b/src/app/api/auth/refresh/route.ts index 56c2d15..356f2d1 100644 --- a/src/app/api/auth/refresh/route.ts +++ b/src/app/api/auth/refresh/route.ts @@ -4,6 +4,7 @@ import { User } from "@/lib/server/models/User"; import { UserSession } from "@/lib/server/models/UserSession"; import { signAccessToken, generateRefreshToken, hashRefreshToken, refreshTtlMs } from "@/lib/server/jwt"; import { tenantAccessError } from "@/lib/server/subscription"; +import { crossTenant } from "@/lib/server/tenantGuard"; import { ACCESS_COOKIE, REFRESH_COOKIE, accessCookieOptions, refreshCookieOptions } from "@/lib/server/auth"; export const runtime = "nodejs"; @@ -24,7 +25,9 @@ export async function POST(req: NextRequest) { return NextResponse.json({ error: "Session expired. Please log in again." }, { status: 401 }); } - const user = await User.findById(session.userId); + const user = await crossTenant("a refresh session identifies its user before the business is known", () => + User.findById(session.userId).exec() + ); if (!user || !user.isActive) { await UserSession.deleteOne({ _id: session._id }); return NextResponse.json({ error: "Account no longer active" }, { status: 401 }); diff --git a/src/app/api/auth/signup/route.ts b/src/app/api/auth/signup/route.ts index db150a1..fec787c 100644 --- a/src/app/api/auth/signup/route.ts +++ b/src/app/api/auth/signup/route.ts @@ -94,7 +94,7 @@ export async function POST(req: NextRequest) { await owner.save(); } catch (e) { await Business.deleteOne({ _id: businessId }).catch(() => {}); - await User.deleteOne({ _id: ownerId }).catch(() => {}); + await User.deleteOne({ _id: ownerId, businessId }).catch(() => {}); const msg = (e as Error).message || ""; if (msg.includes("duplicate key")) { return NextResponse.json({ error: "An account with this email or phone already exists." }, { status: 409 }); diff --git a/src/app/api/branch-prices/route.ts b/src/app/api/branch-prices/route.ts index fe9549b..f6d738c 100644 --- a/src/app/api/branch-prices/route.ts +++ b/src/app/api/branch-prices/route.ts @@ -106,7 +106,7 @@ export async function PUT(req: NextRequest) { } const row = await BranchPricing.findOneAndUpdate( - { branchId, productId }, + { businessId: auth.businessId, branchId, productId }, { businessId: auth.businessId, unitPrice, updatedBy: auth.sub }, { upsert: true, new: true, setDefaultsOnInsert: true } ).lean(); diff --git a/src/app/api/branches/route.ts b/src/app/api/branches/route.ts index 770b2b7..77476b9 100644 --- a/src/app/api/branches/route.ts +++ b/src/app/api/branches/route.ts @@ -106,7 +106,7 @@ export async function POST(req: NextRequest) { { status: 201 } ); } catch (err: any) { - await Branch.deleteOne({ _id: branch._id }).catch(() => {}); + await Branch.deleteOne({ _id: branch._id, businessId }).catch(() => {}); if (err?.code === 11000) { return NextResponse.json({ error: "Someone in your business already uses this phone number or email." }, { status: 409 }); } diff --git a/src/app/api/expenses/route.ts b/src/app/api/expenses/route.ts index dffbefb..a9b92e8 100644 --- a/src/app/api/expenses/route.ts +++ b/src/app/api/expenses/route.ts @@ -5,6 +5,7 @@ import { Expense, EXPENSE_CATEGORIES } from "@/lib/server/models/Expense"; import { Branch } from "@/lib/server/models/Branch"; import { objectId } from "@/lib/server/validation"; import { authorize, branchParam, MANAGERS } from "@/lib/server/auth"; +import { scopedPopulate } from "@/lib/server/tenantGuard"; export const runtime = "nodejs"; export const dynamic = "force-dynamic"; @@ -36,8 +37,8 @@ export async function GET(req: NextRequest) { } const expenses = await Expense.find(filter) - .populate("branchId", "branchName") - .populate("recordedBy", "name") + .populate(scopedPopulate("branchId", "branchName", auth.businessId)) + .populate(scopedPopulate("recordedBy", "name", auth.businessId)) .sort({ createdAt: -1 }) .limit(500) .lean(); diff --git a/src/app/api/inventory/dispatch/route.ts b/src/app/api/inventory/dispatch/route.ts index 843b0c0..df95643 100644 --- a/src/app/api/inventory/dispatch/route.ts +++ b/src/app/api/inventory/dispatch/route.ts @@ -43,7 +43,7 @@ export async function POST(req: NextRequest) { const date = parsed.data.date ?? todayNairobi(); const row = await DailyInventory.findOneAndUpdate( - { branchId, productId: parsed.data.productId, date }, + { businessId: auth.businessId, branchId, productId: parsed.data.productId, date }, { morningDispatched: parsed.data.morningDispatched, recordedBy: auth.sub, businessId: auth.businessId }, { upsert: true, new: true, setDefaultsOnInsert: true } ); diff --git a/src/app/api/inventory/evening-count/route.ts b/src/app/api/inventory/evening-count/route.ts index 7c80057..cf2b829 100644 --- a/src/app/api/inventory/evening-count/route.ts +++ b/src/app/api/inventory/evening-count/route.ts @@ -42,7 +42,7 @@ export async function POST(req: NextRequest) { const date = parsed.data.date ?? todayNairobi(); const row = await DailyInventory.findOneAndUpdate( - { branchId, productId: parsed.data.productId, date }, + { businessId: auth.businessId, branchId, productId: parsed.data.productId, date }, { eveningPhysicalCount: parsed.data.eveningPhysicalCount, recordedBy: auth.sub, businessId: auth.businessId }, { upsert: true, new: true, setDefaultsOnInsert: true } ); diff --git a/src/app/api/invites/[token]/route.ts b/src/app/api/invites/[token]/route.ts index 5a2d2ea..07aaf9d 100644 --- a/src/app/api/invites/[token]/route.ts +++ b/src/app/api/invites/[token]/route.ts @@ -5,13 +5,16 @@ import { connectDb } from "@/lib/server/db"; import { User } from "@/lib/server/models/User"; import { Business } from "@/lib/server/models/Business"; import { hashInviteToken } from "@/lib/server/invites"; +import { crossTenant } from "@/lib/server/tenantGuard"; export const runtime = "nodejs"; export const dynamic = "force-dynamic"; async function findInvitee(token: string) { if (!token || token.length > 100) return null; - const user = await User.findOne({ inviteTokenHash: hashInviteToken(token), isActive: true }); + const user = await crossTenant("an invite token identifies its account before sign-in", () => + User.findOne({ inviteTokenHash: hashInviteToken(token), isActive: true }).exec() + ); if (!user || !user.inviteExpiresAt || user.inviteExpiresAt.getTime() < Date.now()) return null; return user; } diff --git a/src/app/api/mpesa/callback/route.ts b/src/app/api/mpesa/callback/route.ts index c7a84c0..359bd9f 100644 --- a/src/app/api/mpesa/callback/route.ts +++ b/src/app/api/mpesa/callback/route.ts @@ -3,6 +3,7 @@ import { connectDb } from "@/lib/server/db"; import { isMpesaEnabled } from "@/lib/server/env"; import { Transaction } from "@/lib/server/models/Transaction"; import { recordSaleMovement } from "@/lib/server/stock"; +import { crossTenant } from "@/lib/server/tenantGuard"; import { MpesaCallbackLog } from "@/lib/server/models/MpesaCallbackLog"; export const runtime = "nodejs"; @@ -69,7 +70,9 @@ export async function POST(req: NextRequest) { mpesaReceiptNumber = receiptItem ? String(receiptItem.Value) : null; } - const txn = await Transaction.findOne({ "mpesaDetails.checkoutRequestId": checkoutRequestId }); + const txn = await crossTenant("the payment provider's callback is matched by checkout id", () => + Transaction.findOne({ "mpesaDetails.checkoutRequestId": checkoutRequestId }).exec() + ); if (!txn) { await MpesaCallbackLog.create({ diff --git a/src/app/api/mpesa/status/[transactionId]/route.ts b/src/app/api/mpesa/status/[transactionId]/route.ts index a04ec25..30ed7d8 100644 --- a/src/app/api/mpesa/status/[transactionId]/route.ts +++ b/src/app/api/mpesa/status/[transactionId]/route.ts @@ -26,7 +26,7 @@ export async function GET(req: NextRequest, { params }: { params: { transactionI const result = await queryStkStatus(txn.mpesaDetails.checkoutRequestId); const code = Number(result.ResultCode); if (!Number.isNaN(code) && code !== 1032) { - const fresh = await Transaction.findOne({ _id: txn._id, paymentStatus: "PENDING" }); + const fresh = await Transaction.findOne({ _id: txn._id, businessId: auth.businessId, paymentStatus: "PENDING" }); if (fresh) { fresh.paymentStatus = code === 0 ? "SUCCESS" : "FAILED"; fresh.mpesaDetails.resultCode = code; diff --git a/src/app/api/platform/businesses/route.ts b/src/app/api/platform/businesses/route.ts index d55ea93..4ebb564 100644 --- a/src/app/api/platform/businesses/route.ts +++ b/src/app/api/platform/businesses/route.ts @@ -4,6 +4,7 @@ import { Business } from "@/lib/server/models/Business"; import { User } from "@/lib/server/models/User"; import { Branch } from "@/lib/server/models/Branch"; import { requirePlatformAdmin } from "@/lib/server/platform"; +import { crossTenant } from "@/lib/server/tenantGuard"; import { TIERS, effectiveMaxBranches, type TierId, type SubscriptionStatus } from "@/lib/tiers"; export const runtime = "nodejs"; @@ -23,23 +24,26 @@ export async function GET(req: NextRequest) { const businesses = await Business.find({}).sort({ createdAt: -1 }).lean(); // Usage counts per business in two grouped queries instead of N+1. - const [userCounts, branchCounts] = await Promise.all([ - User.aggregate<{ _id: unknown; count: number }>([ - { $group: { _id: "$businessId", count: { $sum: 1 } } }, - ]), - Branch.aggregate<{ _id: unknown; count: number }>([ - { $match: { isActive: { $ne: false } } }, - { $group: { _id: "$businessId", count: { $sum: 1 } } }, - ]), - ]); + const [userCounts, branchCounts] = await crossTenant("the platform console counts every business", () => + Promise.all([ + User.aggregate<{ _id: unknown; count: number }>([{ $group: { _id: "$businessId", count: { $sum: 1 } } }]).exec(), + Branch.aggregate<{ _id: unknown; count: number }>([ + { $match: { isActive: { $ne: false } } }, + { $group: { _id: "$businessId", count: { $sum: 1 } } }, + ]).exec(), + ]) + ); const userMap = new Map(userCounts.map((u) => [String(u._id), u.count])); const branchMap = new Map(branchCounts.map((b) => [String(b._id), b.count])); // Owner contact lookup. const ownerIds = businesses.map((b) => b.ownerId); - const owners = await User.find({ _id: { $in: ownerIds } }) - .select("name email phoneNumber") - .lean(); + const owners = await crossTenant("the platform console lists every business's owner", () => + User.find({ _id: { $in: ownerIds } }) + .select("name email phoneNumber") + .lean() + .exec() + ); const ownerMap = new Map(owners.map((o) => [String(o._id), o])); const rows = businesses.map((b) => { diff --git a/src/app/api/purchases/route.ts b/src/app/api/purchases/route.ts index 5b6826f..16323eb 100644 --- a/src/app/api/purchases/route.ts +++ b/src/app/api/purchases/route.ts @@ -7,6 +7,7 @@ import { ACTIVE_BRANCH, Branch } from "@/lib/server/models/Branch"; import { objectId } from "@/lib/server/validation"; import { recordMovement } from "@/lib/server/stock"; import { authorize, branchParam, MANAGERS } from "@/lib/server/auth"; +import { scopedPopulate } from "@/lib/server/tenantGuard"; export const runtime = "nodejs"; export const dynamic = "force-dynamic"; @@ -36,8 +37,8 @@ export async function GET(req: NextRequest) { } const purchases = await Purchase.find(filter) - .populate("branchId", "branchName") - .populate("recordedBy", "name") + .populate(scopedPopulate("branchId", "branchName", auth.businessId)) + .populate(scopedPopulate("recordedBy", "name", auth.businessId)) .sort({ createdAt: -1 }) .limit(500) .lean(); diff --git a/src/app/api/stock/adjustments/route.ts b/src/app/api/stock/adjustments/route.ts index c375d0c..d81bbff 100644 --- a/src/app/api/stock/adjustments/route.ts +++ b/src/app/api/stock/adjustments/route.ts @@ -64,7 +64,7 @@ export async function POST(req: NextRequest) { const lockedBranch = branchId; try { const quantity = await withStockLock(auth.businessId, lockedBranch, data.productId, async () => { - const already = await StockMovement.exists({ type: "ADJUSTMENT", refId }); + const already = await StockMovement.exists({ type: "ADJUSTMENT", refId, businessId: auth.businessId }); if (!already) { const before = await stockOf(auth.businessId, lockedBranch, data.productId); const delta = data.mode === "COUNT" ? data.quantity - before : data.quantity; diff --git a/src/app/api/stock/movements/route.ts b/src/app/api/stock/movements/route.ts index 5baa387..4580365 100644 --- a/src/app/api/stock/movements/route.ts +++ b/src/app/api/stock/movements/route.ts @@ -2,6 +2,7 @@ import { NextRequest, NextResponse } from "next/server"; import { connectDb } from "@/lib/server/db"; import { StockMovement } from "@/lib/server/models/StockMovement"; import { authorize, branchParam, MANAGERS } from "@/lib/server/auth"; +import { scopedPopulate } from "@/lib/server/tenantGuard"; export const runtime = "nodejs"; export const dynamic = "force-dynamic"; @@ -23,9 +24,9 @@ export async function GET(req: NextRequest) { const movements = await StockMovement.find(filter) .sort({ createdAt: -1 }) .limit(limit) - .populate("branchId", "branchName") - .populate("productId", "label unit") - .populate("recordedBy", "name") + .populate(scopedPopulate("branchId", "branchName", auth.businessId)) + .populate(scopedPopulate("productId", "label unit", auth.businessId)) + .populate(scopedPopulate("recordedBy", "name", auth.businessId)) .lean(); return NextResponse.json({ movements }); } diff --git a/src/app/api/stock/transfers/route.ts b/src/app/api/stock/transfers/route.ts index 1a0f806..dea13e7 100644 --- a/src/app/api/stock/transfers/route.ts +++ b/src/app/api/stock/transfers/route.ts @@ -5,6 +5,7 @@ import { ACTIVE_BRANCH, Branch } from "@/lib/server/models/Branch"; import { Product } from "@/lib/server/models/Product"; import { StockTransfer, IStockTransfer } from "@/lib/server/models/StockTransfer"; import { authorize, branchParam, MANAGERS } from "@/lib/server/auth"; +import { scopedPopulate } from "@/lib/server/tenantGuard"; import { objectId } from "@/lib/server/validation"; import { recordMovement, stockOf, StockBusyError, withStockLock } from "@/lib/server/stock"; @@ -27,10 +28,10 @@ export async function GET(req: NextRequest) { const transfers = await StockTransfer.find(filter) .sort({ createdAt: -1 }) .limit(100) - .populate("fromBranchId", "branchName") - .populate("toBranchId", "branchName") - .populate("productId", "label unit") - .populate("recordedBy", "name") + .populate(scopedPopulate("fromBranchId", "branchName", auth.businessId)) + .populate(scopedPopulate("toBranchId", "branchName", auth.businessId)) + .populate(scopedPopulate("productId", "label unit", auth.businessId)) + .populate(scopedPopulate("recordedBy", "name", auth.businessId)) .lean(); return NextResponse.json({ transfers }); } diff --git a/src/app/api/transactions/cash/route.ts b/src/app/api/transactions/cash/route.ts index b87db58..dc97d2b 100644 --- a/src/app/api/transactions/cash/route.ts +++ b/src/app/api/transactions/cash/route.ts @@ -167,10 +167,8 @@ export async function POST(req: NextRequest) { } catch (err: unknown) { if ((err as { code?: number })?.code !== 11000) throw err; // Already recorded by an earlier attempt: reuse it. - const existing = await Transaction.findOne({ clientUuid: `${sale.clientUuid}:${index}` }); - if (!existing || existing.businessId.toString() !== auth.businessId) { - return reject("This sale id is already in use."); - } + const existing = await Transaction.findOne({ clientUuid: `${sale.clientUuid}:${index}`, businessId: auth.businessId }); + if (!existing) return reject("This sale id is already in use."); txn = existing; } // Idempotent, and also repairs a line whose stock movement was not diff --git a/src/app/api/transactions/route.ts b/src/app/api/transactions/route.ts index 7b8348f..ceda6c6 100644 --- a/src/app/api/transactions/route.ts +++ b/src/app/api/transactions/route.ts @@ -2,6 +2,7 @@ import { NextRequest, NextResponse } from "next/server"; import { connectDb } from "@/lib/server/db"; import { Transaction } from "@/lib/server/models/Transaction"; import { authorize, branchParam } from "@/lib/server/auth"; +import { scopedPopulate } from "@/lib/server/tenantGuard"; import { startOfTodayNairobi } from "@/lib/server/businessDay"; export const runtime = "nodejs"; @@ -26,7 +27,7 @@ export async function GET(req: NextRequest) { const transactions = await Transaction.find(filter) .sort({ createdAt: -1 }) .limit(limit) - .populate("branchId", "branchName") + .populate(scopedPopulate("branchId", "branchName", auth.businessId)) .lean(); return NextResponse.json({ transactions }); diff --git a/src/lib/server/models/Branch.ts b/src/lib/server/models/Branch.ts index 1803fe5..65d81d9 100644 --- a/src/lib/server/models/Branch.ts +++ b/src/lib/server/models/Branch.ts @@ -1,3 +1,4 @@ +import { tenantGuard } from "@/lib/server/tenantGuard"; import { Schema, model, models, Model, Types, Document } from "mongoose"; export interface IBranch extends Document { @@ -30,5 +31,7 @@ const branchSchema = new Schema( /** Filter for branches that are in use. */ export const ACTIVE_BRANCH = { isActive: { $ne: false } } as const; +branchSchema.plugin(tenantGuard); + export const Branch = (models.Branch as Model) || model("Branch", branchSchema); diff --git a/src/lib/server/models/DailyInventory.ts b/src/lib/server/models/DailyInventory.ts index 96cbac5..51a1178 100644 --- a/src/lib/server/models/DailyInventory.ts +++ b/src/lib/server/models/DailyInventory.ts @@ -1,3 +1,4 @@ +import { tenantGuard } from "@/lib/server/tenantGuard"; import { Schema, model, models, Model, Types, Document } from "mongoose"; export interface IDailyInventory extends Document { @@ -40,6 +41,8 @@ dailyInventorySchema.index({ branchId: 1, date: 1 }); // One ledger row per branch+product+day. dailyInventorySchema.index({ branchId: 1, productId: 1, date: 1 }, { unique: true }); +dailyInventorySchema.plugin(tenantGuard); + export const DailyInventory = (models.DailyInventory as Model) || model("DailyInventory", dailyInventorySchema); diff --git a/src/lib/server/models/Expense.ts b/src/lib/server/models/Expense.ts index 6192a51..cc7c639 100644 --- a/src/lib/server/models/Expense.ts +++ b/src/lib/server/models/Expense.ts @@ -1,3 +1,4 @@ +import { tenantGuard } from "@/lib/server/tenantGuard"; import { Schema, model, models, Model, Types, Document } from "mongoose"; // Business expense categories. Kept as a constrained string set so the UI @@ -46,5 +47,7 @@ expenseSchema.index({ category: 1 }); // `models.Expense` guard avoids OverwriteModelError on serverless warm // re-invocations where the module is evaluated more than once. +expenseSchema.plugin(tenantGuard); + export const Expense = (models.Expense as Model) || model("Expense", expenseSchema); diff --git a/src/lib/server/models/Product.ts b/src/lib/server/models/Product.ts index ea2225f..591d8ec 100644 --- a/src/lib/server/models/Product.ts +++ b/src/lib/server/models/Product.ts @@ -1,3 +1,4 @@ +import { tenantGuard } from "@/lib/server/tenantGuard"; import { Schema, model, models, Model, Types, Document } from "mongoose"; export interface IProduct extends Document { @@ -32,6 +33,8 @@ const productSchema = new Schema( { timestamps: true } ); +productSchema.plugin(tenantGuard); + export const Product = (models.Product as Model) || model("Product", productSchema); @@ -57,6 +60,8 @@ const branchPricingSchema = new Schema( ); branchPricingSchema.index({ branchId: 1, productId: 1 }, { unique: true }); +branchPricingSchema.plugin(tenantGuard); + export const BranchPricing = (models.BranchPricing as Model) || model("BranchPricing", branchPricingSchema); diff --git a/src/lib/server/models/Purchase.ts b/src/lib/server/models/Purchase.ts index 23c034d..98e4480 100644 --- a/src/lib/server/models/Purchase.ts +++ b/src/lib/server/models/Purchase.ts @@ -1,3 +1,4 @@ +import { tenantGuard } from "@/lib/server/tenantGuard"; import { Schema, model, models, Model, Types, Document } from "mongoose"; // A recorded stock purchase from a supplier. Completing a purchase is what @@ -40,5 +41,7 @@ const purchaseSchema = new Schema( purchaseSchema.index({ branchId: 1, createdAt: -1 }); purchaseSchema.index({ productId: 1 }); +purchaseSchema.plugin(tenantGuard); + export const Purchase = (models.Purchase as Model) || model("Purchase", purchaseSchema); diff --git a/src/lib/server/models/StockMovement.ts b/src/lib/server/models/StockMovement.ts index 52b3977..8365100 100644 --- a/src/lib/server/models/StockMovement.ts +++ b/src/lib/server/models/StockMovement.ts @@ -1,3 +1,4 @@ +import { tenantGuard } from "@/lib/server/tenantGuard"; import { Schema, model, models, Model, Types, Document } from "mongoose"; export const MOVEMENT_TYPES = ["SALE", "PURCHASE", "TRANSFER_OUT", "TRANSFER_IN", "ADJUSTMENT"] as const; @@ -46,6 +47,8 @@ stockMovementSchema.index({ type: 1, refId: 1 }, { unique: true }); stockMovementSchema.index({ businessId: 1, branchId: 1, productId: 1 }); stockMovementSchema.index({ businessId: 1, branchId: 1, createdAt: -1 }); +stockMovementSchema.plugin(tenantGuard); + export const StockMovement = (models.StockMovement as Model) || model("StockMovement", stockMovementSchema); diff --git a/src/lib/server/models/StockTransfer.ts b/src/lib/server/models/StockTransfer.ts index 14856a2..a89364e 100644 --- a/src/lib/server/models/StockTransfer.ts +++ b/src/lib/server/models/StockTransfer.ts @@ -1,3 +1,4 @@ +import { tenantGuard } from "@/lib/server/tenantGuard"; import { Schema, model, models, Model, Types, Document } from "mongoose"; export interface IStockTransfer extends Document { @@ -35,6 +36,8 @@ const stockTransferSchema = new Schema( stockTransferSchema.index({ businessId: 1, clientUuid: 1 }, { unique: true }); stockTransferSchema.index({ businessId: 1, createdAt: -1 }); +stockTransferSchema.plugin(tenantGuard); + export const StockTransfer = (models.StockTransfer as Model) || model("StockTransfer", stockTransferSchema); diff --git a/src/lib/server/models/Transaction.ts b/src/lib/server/models/Transaction.ts index ae15738..bc9fe02 100644 --- a/src/lib/server/models/Transaction.ts +++ b/src/lib/server/models/Transaction.ts @@ -1,3 +1,4 @@ +import { tenantGuard } from "@/lib/server/tenantGuard"; import { Schema, model, models, Model, Types, Document } from "mongoose"; export type PaymentMethod = "CASH" | "MPESA"; @@ -105,6 +106,8 @@ transactionSchema.index({ businessId: 1, createdAt: -1 }); transactionSchema.index({ paymentStatus: 1 }); transactionSchema.index({ businessId: 1, saleId: 1 }); +transactionSchema.plugin(tenantGuard); + export const Transaction = (models.Transaction as Model) || model("Transaction", transactionSchema); diff --git a/src/lib/server/models/User.ts b/src/lib/server/models/User.ts index f93ff91..8f9baf7 100644 --- a/src/lib/server/models/User.ts +++ b/src/lib/server/models/User.ts @@ -1,3 +1,4 @@ +import { tenantGuard } from "@/lib/server/tenantGuard"; import { Schema, model, models, Model, Types, Document } from "mongoose"; import bcrypt from "bcryptjs"; @@ -80,5 +81,7 @@ userSchema.methods.comparePassword = async function (password: string): Promise< return bcrypt.compare(password, this.passwordHash); }; +userSchema.plugin(tenantGuard); + export const User = (models.User as Model) || model("User", userSchema); diff --git a/src/lib/server/platform.ts b/src/lib/server/platform.ts index 29df84c..a416e4a 100644 --- a/src/lib/server/platform.ts +++ b/src/lib/server/platform.ts @@ -48,7 +48,9 @@ export async function requirePlatformAdmin( ): Promise<{ userId: string; email: string } | null> { const auth = getAuth(req); if (!auth) return null; - const user = await User.findById(auth.sub).select("email isActive platformAdmin").lean(); + const user = await User.findOne({ _id: auth.sub, businessId: auth.businessId }) + .select("email isActive platformAdmin") + .lean(); if (!user || !user.isActive) return null; if (!isPlatformAdmin(user)) return null; return { userId: auth.sub, email: user.email }; diff --git a/src/lib/server/stock.ts b/src/lib/server/stock.ts index 454c108..df08820 100644 --- a/src/lib/server/stock.ts +++ b/src/lib/server/stock.ts @@ -4,6 +4,7 @@ import { ACTIVE_BRANCH, Branch } from "@/lib/server/models/Branch"; import { Product } from "@/lib/server/models/Product"; import { Purchase } from "@/lib/server/models/Purchase"; import { StockLock } from "@/lib/server/models/StockLock"; +import { crossTenant } from "@/lib/server/tenantGuard"; import type { ITransaction } from "@/lib/server/models/Transaction"; type Id = Types.ObjectId | string; @@ -26,7 +27,7 @@ export interface MovementInput { */ export async function recordMovement(m: MovementInput): Promise { await StockMovement.updateOne( - { type: m.type, refId: m.refId }, + { type: m.type, refId: m.refId, businessId: m.businessId }, { $setOnInsert: { businessId: m.businessId, @@ -155,11 +156,14 @@ export async function getStockLevels(businessId: Id, branchId?: Id | null): Prom export async function backfillPurchaseMovements(businessId?: Id): Promise { const filter: Record = { productId: { $ne: null } }; if (businessId) filter.businessId = new mongoose.Types.ObjectId(businessId.toString()); + const purchases = businessId + ? await Purchase.find(filter).lean() + : await crossTenant("the backfill runs over every business", () => Purchase.find(filter).lean().exec()); let added = 0; - for await (const p of Purchase.find(filter).lean().cursor()) { + for (const p of purchases) { const refId = p._id.toString(); - if (await StockMovement.exists({ type: "PURCHASE", refId })) continue; + if (await StockMovement.exists({ type: "PURCHASE", refId, businessId: p.businessId })) continue; await recordMovement({ businessId: p.businessId, branchId: p.branchId, diff --git a/src/lib/server/tenantGuard.ts b/src/lib/server/tenantGuard.ts new file mode 100644 index 0000000..f34b639 --- /dev/null +++ b/src/lib/server/tenantGuard.ts @@ -0,0 +1,114 @@ +import { AsyncLocalStorage } from "async_hooks"; +import { Types, type MongooseQueryMiddleware, type Schema } from "mongoose"; + +/** + * Tenant isolation, enforced instead of remembered. + * + * Every model that holds one business's data is guarded: a query against + * it that does not name exactly one business (`businessId: `) throws + * TenantScopeError before it reaches the database. A route that forgets + * the business filter therefore fails loudly in development and in the + * test suite, instead of quietly reading or changing another business's + * data in production. + * + * The few queries that legitimately span businesses (login looking an + * email up everywhere, an invite token, the payment provider's callback, + * the platform console, maintenance scripts) must say so explicitly: + * + * await crossTenant("login looks the email up in every business", () => User.find(...)); + * + * Guarded: find, findOne (and findById / exists), findOneAndUpdate / + * Delete / Replace, updateOne / updateMany, replaceOne, deleteOne / + * deleteMany, countDocuments, distinct, and aggregate (its first stage + * must be a $match naming the business). populate() runs a find on the + * referenced model, so populate through `scopedPopulate` below. + * + * Not guarded, by design: Business (the tenant itself, always looked up by + * the id from the session), UserSession (keyed by the signed-in user), + * MpesaCallbackLog (provider audit log), StockLock (key already contains + * the business). Document saves and inserts are covered by `businessId` + * being a required field. + */ + +export class TenantScopeError extends Error { + constructor(model: string, op: string) { + super( + `Tenant isolation: ${model}.${op} must filter by a single businessId ` + + `(or run inside crossTenant("why", ...) if it really spans businesses).` + ); + this.name = "TenantScopeError"; + } +} + +const crossTenantReason = new AsyncLocalStorage(); + +/** + * Run `fn`, whose queries deliberately span businesses. `reason` documents + * why. The result is awaited inside the scope, so a lazy Mongoose query + * returned by `fn` executes while the exemption applies. + */ +export function crossTenant(reason: string, fn: () => PromiseLike): Promise { + if (!reason.trim()) throw new Error("crossTenant needs a reason"); + return crossTenantReason.run(reason, async () => await fn()); +} + +/** True when the filter pins exactly one business. */ +export function namesOneBusiness(filter: unknown): boolean { + if (!filter || typeof filter !== "object") return false; + const f = filter as Record; + const v = f.businessId; + if (typeof v === "string" || v instanceof Types.ObjectId) return true; + if (v && typeof v === "object" && "$eq" in (v as object)) { + const eq = (v as { $eq: unknown }).$eq; + return typeof eq === "string" || eq instanceof Types.ObjectId; + } + return Array.isArray(f.$and) && f.$and.some(namesOneBusiness); +} + +const QUERY_OPS: MongooseQueryMiddleware[] = [ + "find", + "findOne", + "findOneAndUpdate", + "findOneAndDelete", + "findOneAndReplace", + "updateOne", + "updateMany", + "replaceOne", + "deleteOne", + "deleteMany", + "countDocuments", + "distinct", +]; + +const guardedSchemas = new WeakSet(); + +/** Whether a schema has the guard (the test suite checks every business-scoped model does). */ +export function isTenantGuarded(schema: Schema): boolean { + return guardedSchemas.has(schema); +} + +/** Mongoose plugin: apply to every schema with a `businessId` path. */ +export function tenantGuard(schema: Schema): void { + guardedSchemas.add(schema); + + schema.pre(QUERY_OPS, function (this: any) { + if (crossTenantReason.getStore()) return; + if (!namesOneBusiness(this.getFilter())) throw new TenantScopeError(this.model.modelName, this.op); + }); + + schema.pre("aggregate", function (this: any) { + if (crossTenantReason.getStore()) return; + const first = this.pipeline()[0]; + if (!first || !namesOneBusiness(first.$match)) { + throw new TenantScopeError(this._model?.modelName ?? "Model", "aggregate"); + } + }); +} + +/** + * populate() options that keep the referenced documents inside the + * business too (populate queries the referenced model, which is guarded). + */ +export function scopedPopulate(path: string, select: string, businessId: string | Types.ObjectId) { + return { path, select, match: { businessId } }; +} diff --git a/tests/access.test.ts b/tests/access.test.ts index a1e7f82..a6c2b0a 100644 --- a/tests/access.test.ts +++ b/tests/access.test.ts @@ -15,7 +15,7 @@ import * as login from "@/app/api/auth/login/route"; import * as signup from "@/app/api/auth/signup/route"; import * as stkpush from "@/app/api/mpesa/stkpush/route"; import * as callback from "@/app/api/mpesa/callback/route"; -import { json, makeTenant, params, req, uuid } from "./helpers"; +import { json, makeTenant, params, req, uuid, db } from "./helpers"; describe("role-based access on the backend", () => { it("blocks cashiers from owner-level money endpoints with 403", async () => { @@ -78,7 +78,7 @@ describe("user deactivation", () => { params({ id: t.cashierA.id }) ); expect(res.status).toBe(200); - expect((await User.findById(t.cashierA.id))!.isActive).toBe(false); + expect((await db(() => User.findById(t.cashierA.id)))!.isActive).toBe(false); expect(await UserSession.countDocuments({ userId: t.cashierA.id })).toBe(0); }); @@ -105,7 +105,7 @@ describe("tenant status", () => { it("refuses login for a suspended business", async () => { const t = await makeTenant(); const passwordHash = await bcrypt.hash("correct-horse", 4); - await User.updateOne({ _id: t.cashierA.id }, { passwordHash }); + await db(() => User.updateOne({ _id: t.cashierA.id }, { passwordHash })); await Business.updateOne({ _id: t.businessId }, { subscriptionStatus: "SUSPENDED" }); const res = await login.POST( @@ -119,7 +119,7 @@ describe("tenant status", () => { it("still allows login for an active business", async () => { const t = await makeTenant(); const passwordHash = await bcrypt.hash("correct-horse", 4); - await User.updateOne({ _id: t.managerA.id }, { passwordHash }); + await db(() => User.updateOne({ _id: t.managerA.id }, { passwordHash })); const res = await login.POST( req("POST", "/api/auth/login", { body: { email: t.managerA.email, password: "correct-horse", deviceId: uuid() }, diff --git a/tests/branchPrices.test.ts b/tests/branchPrices.test.ts index 3ead51a..6ce2a28 100644 --- a/tests/branchPrices.test.ts +++ b/tests/branchPrices.test.ts @@ -4,7 +4,7 @@ import { BranchPricing } from "@/lib/server/models/Product"; import * as prices from "@/app/api/branch-prices/route"; import * as products from "@/app/api/products/route"; import * as cash from "@/app/api/transactions/cash/route"; -import { json, makeTenant, req, uuid, type Actor, type Tenant } from "./helpers"; +import { json, makeTenant, req, uuid, type Actor, type Tenant, db } from "./helpers"; const setPrice = (as: Actor, branchId: string, productId: string, unitPrice: number) => prices.PUT(req("PUT", "/api/branch-prices", { as, body: { branchId, productId, unitPrice } })); @@ -44,7 +44,7 @@ describe("branch prices", () => { const t = await makeTenant(); await setPrice(t.owner, t.branchA, t.water20, 180); await setPrice(t.owner, t.branchA, t.water20, 200); - expect(await BranchPricing.countDocuments({})).toBe(0); + expect(await db(() => BranchPricing.countDocuments({}))).toBe(0); await setPrice(t.owner, t.branchA, t.water5, 55); const reset = await prices.DELETE( @@ -85,6 +85,6 @@ describe("branch prices", () => { expect((await setPrice(t.owner, other.branchA, t.water20, 150)).status).toBe(404); expect((await setPrice(t.owner, t.branchA, other.water20, 150)).status).toBe(404); expect((await prices.GET(req("GET", "/api/branch-prices?branchId=" + other.branchA, { as: t.owner }))).status).toBe(404); - expect(await BranchPricing.countDocuments({})).toBe(0); + expect(await db(() => BranchPricing.countDocuments({}))).toBe(0); }); }); diff --git a/tests/concurrency.test.ts b/tests/concurrency.test.ts index b14525e..1006d5c 100644 --- a/tests/concurrency.test.ts +++ b/tests/concurrency.test.ts @@ -4,7 +4,7 @@ import { StockTransfer } from "@/lib/server/models/StockTransfer"; import { StockLock } from "@/lib/server/models/StockLock"; import * as adjustments from "@/app/api/stock/adjustments/route"; import * as transfers from "@/app/api/stock/transfers/route"; -import { makeTenant, req, uuid, type Tenant } from "./helpers"; +import { makeTenant, req, uuid, type Tenant, db } from "./helpers"; async function openingStock(t: Tenant, quantity: number) { await adjustments.POST( @@ -35,7 +35,7 @@ describe("concurrent stock changes", () => { expect(statuses.filter((s) => s === 409)).toHaveLength(3); expect(await stockOf(t.businessId, t.branchA, t.water20)).toBe(2); expect(await stockOf(t.businessId, t.branchB, t.water20)).toBe(8); - expect(await StockTransfer.countDocuments({})).toBe(2); + expect(await db(() => StockTransfer.countDocuments({}))).toBe(2); }); it("holds across repeated races, not just once", async () => { diff --git a/tests/followups.test.ts b/tests/followups.test.ts index e4bb869..e841c09 100644 --- a/tests/followups.test.ts +++ b/tests/followups.test.ts @@ -19,7 +19,7 @@ import * as profit from "@/app/api/reports/profit/route"; import * as cash from "@/app/api/transactions/cash/route"; import * as adjustments from "@/app/api/stock/adjustments/route"; import * as transfers from "@/app/api/stock/transfers/route"; -import { json, makeTenant, params, req, uuid, type Tenant } from "./helpers"; +import { json, makeTenant, params, req, uuid, type Tenant, db } from "./helpers"; async function setPlan(t: Tenant, tier: string, subscriptionStatus: string) { await Business.updateOne({ _id: t.businessId }, { tier, subscriptionStatus }); @@ -39,7 +39,7 @@ describe("F-01 trial branch allowance", () => { it("counts only active branches, and a paid Starter plan allows one", async () => { const t = await makeTenant(); await setPlan(t, "STARTER", "TRIALING"); - await Branch.updateOne({ _id: t.branchB }, { isActive: false }); + await db(() => Branch.updateOne({ _id: t.branchB }, { isActive: false })); expect((await branches.POST(req("POST", "/x", { as: t.owner, body: newBranch("C") }))).status).toBe(201); await setPlan(t, "STARTER", "ACTIVE"); @@ -70,7 +70,7 @@ describe("F-06 branch deactivation", () => { expect(withStaff.status).toBe(409); expect((await withStaff.json()).error).toMatch(/active staff/); - await User.updateOne({ _id: t.cashierB.id }, { isActive: false }); + await db(() => User.updateOne({ _id: t.cashierB.id }, { isActive: false })); expect((await deactivate()).status).toBe(200); // Hidden from pickers, kept on the Branches page. @@ -82,7 +82,7 @@ describe("F-06 branch deactivation", () => { it("will not reactivate a branch past the plan's limit", async () => { const t = await makeTenant(); - await Branch.updateOne({ _id: t.branchB }, { isActive: false }); + await db(() => Branch.updateOne({ _id: t.branchB }, { isActive: false })); await setPlan(t, "STARTER", "ACTIVE"); const res = await branchById.PATCH(req("PATCH", "/x", { as: t.owner, body: { isActive: true } }), params({ id: t.branchB })); expect(res.status).toBe(403); @@ -152,7 +152,7 @@ describe("F-04 / F-05 / CAT-12 catalogue", () => { await adjustments.POST( req("POST", "/x", { as: t.owner, body: { clientUuid: uuid(), branchId: t.branchA, productId: t.water5, mode: "DELTA", quantity: 7 } }) ); - await Product.updateOne({ _id: t.water5 }, { isActive: false }); + await db(() => Product.updateOne({ _id: t.water5 }, { isActive: false })); const forManager = await json(await products.GET(req("GET", "/api/products?includeInactive=1", { as: t.managerA }))); const forCashier = await json(await products.GET(req("GET", "/api/products?includeInactive=1", { as: t.cashierA }))); @@ -228,13 +228,13 @@ describe("STA-04 staff invites", () => { expect((await invites.GET(req("GET", "/x"), params({ token }))).status).toBe(404); expect((await invites.GET(req("GET", "/x"), params({ token: fresh }))).status).toBe(200); - await User.updateOne({ _id: new mongoose.Types.ObjectId(userId) }, { inviteExpiresAt: new Date(Date.now() - 1000) }); + await db(() => User.updateOne({ _id: new mongoose.Types.ObjectId(userId) }, { inviteExpiresAt: new Date(Date.now() - 1000) })); expect((await invites.GET(req("GET", "/x"), params({ token: fresh }))).status).toBe(404); }); it("does not let a manager re-invite another branch's staff", async () => { const t = await makeTenant(); - await User.updateOne({ _id: t.cashierB.id }, { inviteExpiresAt: new Date(Date.now() + 60_000) }); + await db(() => User.updateOne({ _id: t.cashierB.id }, { inviteExpiresAt: new Date(Date.now() + 60_000) })); const res = await reinvite.POST(req("POST", "/x", { as: t.managerA }), params({ id: t.cashierB.id })); expect(res.status).toBe(403); }); diff --git a/tests/helpers.ts b/tests/helpers.ts index b52c9cf..6e673cc 100644 --- a/tests/helpers.ts +++ b/tests/helpers.ts @@ -7,6 +7,7 @@ import { User, UserRole } from "@/lib/server/models/User"; import { Product } from "@/lib/server/models/Product"; import { signAccessToken } from "@/lib/server/jwt"; import { ACCESS_COOKIE } from "@/lib/server/auth"; +import { crossTenant } from "@/lib/server/tenantGuard"; let phoneSeq = 0; function nextPhone(): string { @@ -131,3 +132,8 @@ export async function json(res: Response): Promise<{ status: number; bo export function uuid(): string { return randomUUID(); } + +/** Tests may inspect and arrange any business's data directly. */ +export function db(fn: () => PromiseLike): Promise { + return crossTenant("test inspects the database directly", fn); +} diff --git a/tests/identity.test.ts b/tests/identity.test.ts index 841b435..1f76dfc 100644 --- a/tests/identity.test.ts +++ b/tests/identity.test.ts @@ -6,7 +6,7 @@ import * as login from "@/app/api/auth/login/route"; import * as me from "@/app/api/auth/me/route"; import * as platformBusinesses from "@/app/api/platform/businesses/route"; import { ACCESS_COOKIE } from "@/lib/server/auth"; -import { json, makeTenant, req, uuid, type Actor, type Tenant } from "./helpers"; +import { json, makeTenant, req, uuid, type Actor, type Tenant, db } from "./helpers"; const PERSON = { name: "Wanjiru", email: "wanjiru@aquaflow.test", phoneNumber: "0722000111" }; const OPERATOR = "operator@aquaflow.test"; // listed in PLATFORM_ADMIN_EMAILS by tests/setup.ts @@ -35,7 +35,7 @@ describe("email and phone are unique per business", () => { const b = await makeTenant("Beta"); expect((await hire(a, a.owner)).status).toBe(201); expect((await hire(b, b.owner)).status).toBe(201); - expect(await User.countDocuments({ email: PERSON.email })).toBe(2); + expect(await db(() => User.countDocuments({ email: PERSON.email }))).toBe(2); }); it("still refuses a duplicate inside the same business", async () => { diff --git a/tests/isolation.test.ts b/tests/isolation.test.ts new file mode 100644 index 0000000..0e0e1a1 --- /dev/null +++ b/tests/isolation.test.ts @@ -0,0 +1,172 @@ +import { describe, expect, it } from "vitest"; +import mongoose from "mongoose"; +import { readdirSync } from "fs"; +import path from "path"; +import { crossTenant, isTenantGuarded, TenantScopeError } from "@/lib/server/tenantGuard"; +import { User } from "@/lib/server/models/User"; +import { Transaction } from "@/lib/server/models/Transaction"; +import { Expense } from "@/lib/server/models/Expense"; +import { StockMovement } from "@/lib/server/models/StockMovement"; +import { stockOf } from "@/lib/server/stock"; +import * as cash from "@/app/api/transactions/cash/route"; +import * as transactions from "@/app/api/transactions/route"; +import * as expenses from "@/app/api/expenses/route"; +import * as purchases from "@/app/api/purchases/route"; +import * as stock from "@/app/api/stock/route"; +import * as movements from "@/app/api/stock/movements/route"; +import * as transfers from "@/app/api/stock/transfers/route"; +import * as adjustments from "@/app/api/stock/adjustments/route"; +import * as summary from "@/app/api/dashboard/summary/route"; +import * as reconciliation from "@/app/api/dashboard/reconciliation/route"; +import * as profit from "@/app/api/reports/profit/route"; +import * as users from "@/app/api/users/route"; +import * as userById from "@/app/api/users/[id]/route"; +import * as reinvite from "@/app/api/users/[id]/invite/route"; +import * as branchById from "@/app/api/branches/[id]/route"; +import * as productById from "@/app/api/products/[id]/route"; +import * as branchPrices from "@/app/api/branch-prices/route"; +import * as dispatch from "@/app/api/inventory/dispatch/route"; +import * as eveningCount from "@/app/api/inventory/evening-count/route"; +import { db, json, makeTenant, params, req, uuid, type Tenant } from "./helpers"; + +// Models that hold one business's data but are deliberately unguarded (see tenantGuard.ts). +const UNGUARDED = new Set(["Business", "UserSession", "MpesaCallbackLog", "StockLock"]); + +describe("the tenant guard", () => { + it("is on every model that stores a businessId", async () => { + // Load every model file so none can be skipped by accident. + const dir = path.resolve(__dirname, "../src/lib/server/models"); + for (const file of readdirSync(dir)) await import(path.join(dir, file)); + + const scoped = Object.values(mongoose.models).filter((m) => m.schema.path("businessId") && !UNGUARDED.has(m.modelName)); + expect(scoped.length).toBeGreaterThanOrEqual(10); + const unguarded = scoped.filter((m) => !isTenantGuarded(m.schema)).map((m) => m.modelName); + expect(unguarded).toEqual([]); + }); + + it("refuses queries that do not name exactly one business", async () => { + const t = await makeTenant(); + const attempts: [string, () => PromiseLike][] = [ + ["find", () => Transaction.find({ branchId: t.branchA })], + ["findById", () => User.findById(t.cashierA.id)], + ["exists", () => StockMovement.exists({ type: "SALE" })], + ["updateMany", () => Expense.updateMany({}, { amount: 0 })], + ["deleteMany", () => Transaction.deleteMany({ paymentMethod: "CASH" })], + ["countDocuments", () => User.countDocuments({})], + ["$in over businesses", () => User.find({ businessId: { $in: [t.businessId, new mongoose.Types.ObjectId()] } })], + ["$exists", () => User.find({ businessId: { $exists: true } })], + ["aggregate without $match", () => Transaction.aggregate([{ $group: { _id: "$businessId" } }])], + ["aggregate matching later", () => Transaction.aggregate([{ $sort: { createdAt: -1 } }, { $match: { businessId: t.businessId } }])], + ["unscoped populate", () => Expense.find({ businessId: t.businessId }).populate("branchId")], + ]; + await Expense.create({ businessId: t.businessId, branchId: t.branchA, category: "Rent", amount: 1, description: "x", recordedBy: t.owner.id }); + for (const [label, run] of attempts) { + await expect(Promise.resolve().then(run), label).rejects.toBeInstanceOf(TenantScopeError); + } + }); + + it("allows scoped queries, and cross-business ones only with a stated reason", async () => { + const t = await makeTenant(); + expect(await User.countDocuments({ businessId: t.businessId })).toBe(4); + expect(await User.countDocuments({ $and: [{ businessId: t.businessId }, { role: "DELIVERY" }] })).toBe(2); + expect(await crossTenant("test", () => User.countDocuments({}))).toBe(4); + expect(() => crossTenant(" ", () => User.countDocuments({}))).toThrow(/reason/); + // The exemption ends with the callback. + await expect(Promise.resolve().then(() => User.countDocuments({}))).rejects.toBeInstanceOf(TenantScopeError); + }); +}); + +/** Give tenant B one of everything, so a leak into tenant A's view would show. */ +async function seedVictim(b: Tenant) { + await adjustments.POST( + req("POST", "/x", { as: b.owner, body: { clientUuid: uuid(), branchId: b.branchA, productId: b.water20, mode: "DELTA", quantity: 50 } }) + ); + await cash.POST( + req("POST", "/x", { as: b.cashierA, body: { sales: [{ clientUuid: uuid(), lines: [{ productId: b.water20, quantity: 3 }] }] } }) + ); + await expenses.POST(req("POST", "/x", { as: b.owner, body: { branchId: b.branchA, category: "Rent", amount: 7777, description: "B rent" } })); + await purchases.POST( + req("POST", "/x", { as: b.owner, body: { branchId: b.branchA, supplierName: "B supplier", productId: b.water20, quantity: 5, unitCost: 99 } }) + ); + await transfers.POST( + req("POST", "/x", { as: b.owner, body: { clientUuid: uuid(), fromBranchId: b.branchA, toBranchId: b.branchB, productId: b.water20, quantity: 2 } }) + ); +} + +describe("route sweep: another business's ids get nothing", () => { + it("every read route returns none of business B's data to business A, whatever ids A sends", async () => { + const a = await makeTenant("Alpha"); + const b = await makeTenant("Beta"); + await seedVictim(b); + + const reads: [string, Response][] = []; + const get = async (label: string, handler: (r: any) => Promise, url: string) => + reads.push([label, await handler(req("GET", url, { as: a.owner }))]); + const q = "?branchId=" + b.branchA; + await get("transactions", transactions.GET, "/api/transactions" + q); + await get("expenses", expenses.GET, "/api/expenses" + q); + await get("purchases", purchases.GET, "/api/purchases" + q); + await get("stock", stock.GET, "/api/stock" + q); + await get("movements", movements.GET, "/api/stock/movements" + q); + await get("transfers", transfers.GET, "/api/stock/transfers" + q); + await get("summary", summary.GET, "/api/dashboard/summary" + q); + await get("reconciliation", reconciliation.GET, "/api/dashboard/reconciliation" + q); + await get("profit", profit.GET, "/api/reports/profit" + q); + await get("users", users.GET, "/api/users" + q); + await get("branch prices", branchPrices.GET, "/api/branch-prices" + q); + + const bIds = [b.businessId, b.branchA, b.branchB, b.water20, b.owner.id, b.cashierA.id]; + for (const [label, res] of reads) { + const text = await res.text(); + for (const id of bIds) expect(text.includes(id), `${label} leaked ${id}`).toBe(false); + expect(text.includes("7777") || text.includes("B supplier"), `${label} leaked B's figures`).toBe(false); + } + // Totals A sees are A's own (zero), not B's sale. + const totals = JSON.parse(await (await summary.GET(req("GET", "/api/dashboard/summary" + q, { as: a.owner }))).text()); + expect(totals.totals.cashTotal).toBe(0); + }); + + it("every write route refuses business B's ids and leaves B's data untouched", async () => { + const a = await makeTenant("Alpha"); + const b = await makeTenant("Beta"); + await seedVictim(b); + const before = { + stockA: await stockOf(b.businessId, b.branchA, b.water20), + cashier: await db(() => User.findById(b.cashierA.id).lean()), + sales: await Transaction.countDocuments({ businessId: b.businessId }), + }; + + const as = a.owner; + const writes: [string, Response][] = [ + ["deactivate B's cashier", await userById.PATCH(req("PATCH", "/x", { as, body: { isActive: false } }), params({ id: b.cashierA.id }))], + ["move B's cashier", await userById.PATCH(req("PATCH", "/x", { as, body: { branchId: a.branchA } }), params({ id: b.cashierA.id }))], + ["re-invite B's cashier", await reinvite.POST(req("POST", "/x", { as }), params({ id: b.cashierA.id }))], + ["rename B's branch", await branchById.PATCH(req("PATCH", "/x", { as, body: { branchName: "Pwned" } }), params({ id: b.branchA }))], + ["reprice B's product", await productById.PATCH(req("PATCH", "/x", { as, body: { unitPrice: 1 } }), params({ id: b.water20 }))], + ["branch price on B", await branchPrices.PUT(req("PUT", "/x", { as, body: { branchId: b.branchA, productId: b.water20, unitPrice: 1 } }))], + ["adjust B's stock", await adjustments.POST(req("POST", "/x", { as, body: { clientUuid: uuid(), branchId: b.branchA, productId: b.water20, mode: "COUNT", quantity: 0 } }))], + ["transfer out of B", await transfers.POST(req("POST", "/x", { as, body: { clientUuid: uuid(), fromBranchId: b.branchA, toBranchId: a.branchA, productId: b.water20, quantity: 1 } }))], + ["transfer into B", await transfers.POST(req("POST", "/x", { as, body: { clientUuid: uuid(), fromBranchId: a.branchA, toBranchId: b.branchA, productId: a.water20, quantity: 1 } }))], + ["expense on B", await expenses.POST(req("POST", "/x", { as, body: { branchId: b.branchA, category: "Rent", amount: 1, description: "x" } }))], + ["purchase into B", await purchases.POST(req("POST", "/x", { as, body: { branchId: b.branchA, supplierName: "x", productId: b.water20, quantity: 1, unitCost: 1 } }))], + ["dispatch at B", await dispatch.POST(req("POST", "/x", { as, body: { branchId: b.branchA, productId: b.water20, morningDispatched: 1 } }))], + ["count at B", await eveningCount.POST(req("POST", "/x", { as, body: { branchId: b.branchA, productId: b.water20, eveningPhysicalCount: 1 } }))], + ["sale of B's product", await cash.POST(req("POST", "/x", { as: a.cashierA, body: { sales: [{ clientUuid: uuid(), lines: [{ productId: b.water20, quantity: 1 }] }] } }))], + ]; + for (const [label, res] of writes) { + if (label.startsWith("sale")) { + expect((await json(res)).body.results[0].status, label).toBe("rejected"); + } else { + expect(res.status, label).toBeGreaterThanOrEqual(400); + } + } + + expect(await stockOf(b.businessId, b.branchA, b.water20)).toBe(before.stockA); + expect(await db(() => User.findById(b.cashierA.id).lean())).toMatchObject({ + isActive: before.cashier!.isActive, + branchId: before.cashier!.branchId, + }); + expect(await Transaction.countDocuments({ businessId: b.businessId })).toBe(before.sales); + expect(await Expense.countDocuments({ businessId: b.businessId })).toBe(1); + }); +}); diff --git a/tests/sync.test.ts b/tests/sync.test.ts index dc0c19b..39ec7f4 100644 --- a/tests/sync.test.ts +++ b/tests/sync.test.ts @@ -4,7 +4,7 @@ import { Product, BranchPricing } from "@/lib/server/models/Product"; import { stockOf } from "@/lib/server/stock"; import * as cash from "@/app/api/transactions/cash/route"; import type { SaleResult } from "@/lib/syncProtocol"; -import { json, makeTenant, req, uuid, type Actor, type Tenant } from "./helpers"; +import { json, makeTenant, req, uuid, type Actor, type Tenant, db } from "./helpers"; function sale(t: Tenant, cashier: Actor, lines: { productId: string; quantity: number; unitPrice?: number }[], extra = {}) { return { @@ -31,7 +31,7 @@ describe("offline sale sync", () => { const { status, body } = await post(t.cashierA, [s]); expect(status).toBe(200); expect(body.results).toEqual([{ clientUuid: s.clientUuid, status: "saved", amountTotal: 580, priceMismatch: false }]); - expect(await Transaction.countDocuments({ saleId: s.clientUuid })).toBe(2); + expect(await db(() => Transaction.countDocuments({ saleId: s.clientUuid }))).toBe(2); expect(await stockOf(t.businessId, t.branchA, t.water20)).toBe(-2); expect(await stockOf(t.businessId, t.branchA, t.water5)).toBe(-3); }); @@ -42,7 +42,7 @@ describe("offline sale sync", () => { await post(t.cashierA, [s]); const replay = await post(t.cashierA, [s]); expect(replay.body.results[0]).toMatchObject({ clientUuid: s.clientUuid, status: "duplicate" }); - expect(await Transaction.countDocuments({ saleId: s.clientUuid })).toBe(1); + expect(await db(() => Transaction.countDocuments({ saleId: s.clientUuid }))).toBe(1); expect(await stockOf(t.businessId, t.branchA, t.water20)).toBe(-1); }); @@ -78,7 +78,7 @@ describe("offline sale sync", () => { const otherCashiersSale = sale(t, t.cashierB, [{ productId: t.water20, quantity: 1 }]); const { body } = await post(t.cashierA, [otherCashiersSale]); expect(body.results[0].status).toBe("rejected"); - expect(await Transaction.countDocuments({})).toBe(0); + expect(await db(() => Transaction.countDocuments({}))).toBe(0); }); it("keeps a sale at the branch where it happened after the cashier is moved", async () => { @@ -86,7 +86,7 @@ describe("offline sale sync", () => { const s = sale(t, t.cashierA, [{ productId: t.water20, quantity: 1 }]); // recorded at branch A const moved = { ...t.cashierA, branchId: t.branchB }; await post(moved, [s]); - const txn = await Transaction.findOne({ saleId: s.clientUuid }); + const txn = await db(() => Transaction.findOne({ saleId: s.clientUuid })); expect(txn!.branchId.toString()).toBe(t.branchA); }); @@ -97,13 +97,13 @@ describe("offline sale sync", () => { const foreignBranch = sale(t, t.cashierA, [{ productId: t.water20, quantity: 1 }], { branchId: other.branchA }); const { body } = await post(t.cashierA, [foreignProduct, foreignBranch]); expect(body.results.map((r) => r.status)).toEqual(["rejected", "rejected"]); - expect(await Transaction.countDocuments({})).toBe(0); + expect(await db(() => Transaction.countDocuments({}))).toBe(0); }); it("uses the branch price, flags a price the device showed differently, and keeps deactivated products sellable", async () => { const t = await makeTenant(); await BranchPricing.create({ businessId: t.businessId, branchId: t.branchA, productId: t.water20, unitPrice: 180 }); - await Product.updateOne({ _id: t.water5 }, { isActive: false }); + await db(() => Product.updateOne({ _id: t.water5 }, { isActive: false })); const s = sale(t, t.cashierA, [ { productId: t.water20, quantity: 1, unitPrice: 200 }, // stale price on the device { productId: t.water5, quantity: 1, unitPrice: 60 }, -- 2.52.0