From 6d9232f3b7e60819b7fb696583ff9d54e7e3838a Mon Sep 17 00:00:00 2001 From: Suriya Date: Wed, 5 Aug 2026 18:39:34 +0530 Subject: [PATCH] fix: close the remaining express-console tenant leaks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The first scoping pass covered tables carrying a tenantid column. These five have no such column and were still returning every client's data to a client login, which is what made creating info@dailygrubs.com unsafe: - GET /admin/customers — scoped through the bookings placed for them, since a customer carries no tenant of their own (the same person can order from two clients). - GET /admin/exceptions — scoped through the consignment the exception was raised against. - GET /admin/tenantcustomers — the legacy customers table predates tenant attribution entirely, so no row can be proven to belong to a client. Returns empty for client logins rather than handing over the whole list. - GET /admin/tripsheets — a vehicle run routinely carries several clients' parcels on one manifest, so there is no honest per-client view. Doormile staff only. - GET /admin/milers — scoped through appusers.tenantid, so a client sees their own riders rather than the whole roster. Also guards PUT /admin/consignments/:id/status, which took the id straight from the path and would have let a client move another client's parcel through the network. Co-Authored-By: Claude Opus 5 (1M context) --- controllers/adminController.go | 67 +++++++++++++++++++++++++++++++++- 1 file changed, 65 insertions(+), 2 deletions(-) diff --git a/controllers/adminController.go b/controllers/adminController.go index 014b349..5097450 100644 --- a/controllers/adminController.go +++ b/controllers/adminController.go @@ -57,6 +57,31 @@ func scopeToOwnTenant(c *fiber.Ctx, query *gorm.DB, column string) *gorm.DB { return query.Where(column+" = ?", tenantID) } +// scopeViaBookings restricts a query to rows whose foreign key appears on one of +// the tenant's bookings. Customers carry no tenant of their own — the same +// person can order from two different clients — so the relationship only exists +// through the bookings placed for them. +func scopeViaBookings(c *fiber.Ctx, query *gorm.DB, column string) *gorm.DB { + tenantID := consoleTenantID(c) + if tenantID == 0 { + return query + } + return query.Where(column+" IN (?)", + db.DB.Model(&models.PickupBooking{}).Select("appcustomerid").Where("tenantid = ?", tenantID)) +} + +// scopeViaConsignments restricts a query to rows attached to one of the tenant's +// consignments — used for exceptions, which inherit their owner from the parcel +// they were raised against. +func scopeViaConsignments(c *fiber.Ctx, query *gorm.DB, column string) *gorm.DB { + tenantID := consoleTenantID(c) + if tenantID == 0 { + return query + } + return query.Where(column+" IN (?)", + db.DB.Model(&models.Consignment{}).Select("consignmentid").Where("tenantid = ?", tenantID)) +} + // canAccessTenant reports whether the caller may act on the given tenant. // Used where the tenant is addressed by a path parameter or request body rather // than filtered in a query — scoping a WHERE clause does nothing when the @@ -747,6 +772,12 @@ func UpdateTenantLocation(c *fiber.Ctx) error { // -------------------- func GetTenantCustomers(c *fiber.Ctx) error { + // The legacy customers table carries no tenant column and predates tenant + // attribution, so there is no way to prove any row belongs to a given + // client. Rather than hand a client the whole list, they get none of it. + if !isDoormileConsoleStaff(c) { + return utils.List(c, []models.Customer{}, 0) + } var customers []models.Customer if err := db.DB.Find(&customers).Error; err != nil { return utils.Internal(c, "failed to fetch customers") @@ -769,6 +800,9 @@ func GetAdminCustomers(c *fiber.Ctx) error { like := "%" + keyword + "%" query = query.Where("firstname ILIKE ? OR lastname ILIKE ? OR phone ILIKE ?", like, like, like) } + // A client sees only the customers they have actually delivered to, not + // Doormile's whole B2C address book. + query = scopeViaBookings(c, query, "appcustomerid") var total int64 if err := query.Count(&total).Error; err != nil { @@ -1309,6 +1343,13 @@ func GetMilers(c *fiber.Ctx) error { if appLocStr := c.Query("applocationid"); appLocStr != "" { query = query.Where("applocationid = ?", appLocStr) } + // Riders belong to a client through their appusers row; a client sees their + // own fleet, not Doormile's whole roster. + if tenantID := consoleTenantID(c); tenantID != 0 { + query = query.Where("userid IN (?)", + db.DB.Model(&models.AppUser{}).Select("userid"). + Where("tenantid = ? AND roleid = ?", tenantID, 5)) + } if err := query.Find(&profiles).Error; err != nil { return utils.Internal(c, "failed to fetch milers") } @@ -2184,6 +2225,18 @@ func GetAdminConsignmentTracking(c *fiber.Ctx) error { func AdminUpdateConsignmentStatus(c *fiber.Ctx) error { id, _ := strconv.Atoi(c.Params("id")) + // Addressed by id, so the ownership check has to happen before the write — + // otherwise a client could move another client's parcel through the network. + if own := consoleTenantID(c); own != 0 { + var owner models.Consignment + if err := db.DB.Select("consignmentid", "tenantid").First(&owner, id).Error; err != nil { + return utils.NotFound(c, "consignment not found") + } + if owner.Tenantid != own { + return utils.NotFound(c, "consignment not found") + } + } + type StatusUpdate struct { Status string `json:"status"` Remarks string `json:"remarks"` @@ -2236,6 +2289,13 @@ func AdminUpdateConsignmentStatus(c *fiber.Ctx) error { // -------------------- func GetTripsheets(c *fiber.Ctx) error { + // A tripsheet is a Doormile vehicle run and routinely carries several + // clients' parcels on the same manifest, so there is no honest way to show + // one to a single client. Doormile staff only. + if !isDoormileConsoleStaff(c) { + return utils.List(c, []models.Tripsheet{}, 0) + } + page := utils.ParsePage(c) var total int64 @@ -2614,13 +2674,16 @@ func GetPricingQuoteSimulate(c *fiber.Ctx) error { func GetExceptions(c *fiber.Ctx) error { page := utils.ParsePage(c) + // An exception belongs to whoever owns the parcel it was raised against. var total int64 - if err := db.DB.Model(&models.ConsignmentException{}).Where("deletedat IS NULL").Count(&total).Error; err != nil { + if err := scopeViaConsignments(c, db.DB.Model(&models.ConsignmentException{}), "consignmentid"). + Where("deletedat IS NULL").Count(&total).Error; err != nil { return utils.Internal(c, "failed to count exceptions") } var list []models.ConsignmentException - if err := page.Apply(db.DB.Where("deletedat IS NULL")).Find(&list).Error; err != nil { + if err := page.Apply(scopeViaConsignments(c, db.DB, "consignmentid").Where("deletedat IS NULL")). + Find(&list).Error; err != nil { return utils.Internal(c, "failed to fetch exceptions") } return utils.Paginated(c, list, total, page)