9.7 KiB
Handoff: Broken Access Control (IDOR) audit & fixes — Fiesta backend
Repo: backend_fiesta (Go + Fiber + GORM), consumed by nearledaily/daily_merchant_web (React/TS) and a mobile app (not in this repo).
1. The root problem (still not fully fixed — read this first)
There is no authentication system in this backend. Grep confirms:
- No JWT/session token is ever issued.
Login,TenantLogin,TenantWebLogin,AppLogin(incontrollers/userController.go) just look up the user/tenant and return their info in the JSON body — no token. - No auth middleware exists anywhere.
routes/routes.go/main.goonly wire up CORS middleware. Every route is wide open — anyone who can reach the API can call any endpoint with any query params.
Because of that, every endpoint trusts client-supplied query params (tenantid, customerid, partnerid, etc.) as the sole source of "who is asking." There is currently nothing stopping a logged-in store admin for tenant 1135 from just requesting ?tenantid=1136 and getting another tenant's data — the frontend happens to always send the logged-in user's own tenantid, but the backend never checks it.
This session's fixes only close one specific hole, described below. The real fix — deriving identity server-side from a verified token instead of trusting query params — has not been started. Whoever picks this up should treat that as the actual next milestone.
2. The specific bug that was found and fixed this session
Pattern found repeatedly across the codebase: repository functions build SQL dynamically, e.g.
query := "SELECT ... FROM orders WHERE 1=1"
if tenantID != 0 {
query += " AND tenantid = ?"
params = append(params, tenantID)
}
// ...similar optional blocks for partnerid, customerid, etc.
If none of the scoping params were supplied (0 / empty), the query silently fell through to "no WHERE clause" and returned every row in the table across every tenant. This was directly reachable — e.g. orders/getorders with no tenantid returned all ~300 orders in the DB rather than 400ing, which is how the user first noticed this (logged in as a store admin, expected only their store's orders, saw everyone's).
Fix pattern applied
Rather than rewriting every repository query (large surface area, higher regression risk), a controller-level guard was added to each affected endpoint: if none of the valid scoping ids are present in the query string, return 400 immediately instead of calling the service/repo at all.
Standard error shape used everywhere:
{ "status": false, "code": 400, "message": "<specific message>" }
3. Endpoints fixed (8 total)
| # | Endpoint | File / function | Guard added |
|---|---|---|---|
| 1 | GET /v1/web/orders/getorders (+ mob) |
controllers/orderController.go GetOrders (line 24) |
requires one of tenantid, partnerid, customerid, applocationid, appuserid — else 400 (line ~102-110). Previously the else branch called GetAllOrders (unscoped). |
| 2 | GET /v1/web/orders/getordersummary |
controllers/orderController.go GetOrderSummary (line 129) |
requires one of tenantid, partnerid, customerid, locationid (line 137-143) |
| 3 | GET /v1/web/orders/getlocationsummary |
controllers/orderController.go GetlocationOrderSummary (line 163) |
requires tenantid (line 167-173) |
| 4 | GET /v1/web/users/getallusers |
controllers/userController.go GetAllUsers (line 22) |
requires tenantid (line 29-35). Note: this endpoint's query selects a.pin (login PIN) — this was a high-severity leak (PINs across all tenants) before the fix. |
| 5 | GET /v1/web/deliveries/getdeliveries (+ mob) |
controllers/deliveriesController.go GetDeliveries (line 194) |
requires one of tenantid, partnerid, customerid, applocationid, userid, appuserid (line 212-218) |
| 6 | GET /v1/web/partners/getriders (+ mob) |
controllers/partnerController.go GetActiveRiders (line 19) |
requires one of tenantid, partnerid, applocationid, userid (line 25-31). Lower severity — underlying repo query defaults to userid = 0 rather than a full dump, but fixed for consistency. |
| 7 | GET /v1/web/partners/getriderlogs (+ mob) |
controllers/partnerController.go GetRiderLogs (line 121) |
requires one of partnerid, applocationid (line 127-133). Also fixed an unrelated bug in the same function: tdate was reading c.Query("fromdate") (copy-paste error) so the end of any date range was always silently overwritten with the start date. Now correctly reads c.Query("todate") (line 125). |
| 8 | POST /v1/mob/orders/getcustomerorders |
controllers/orderController.go GetCustomerOrders (line 374) |
requires customerid (line 394-400) |
Also fixed alongside #2: SQL injection in GetOrderSummary
repositories/orderRepository.go GetOrderSummary previously built the date filter by string-concatenating fdate/tdate directly into raw SQL. Rewritten to use parameterized ? placeholders passed through r.db.Raw(query, params...). The strconv import was removed from that file since it became unused after the rewrite (verified via grep no other usage remained).
4. Reviewed and explicitly NOT changed (don't re-flag these)
Same WHERE 1=1 pattern exists elsewhere but was judged not to be a bug, or already safe:
repositories/tenantRepository.goGetAllTenants— intentionally lists all tenants for a platform/super-admin console. The gap here is "no RBAC to restrict who can call this," which is the same root-cause auth gap from section 1, not a scoping bug to patch individually.repositories/productRepository.goGetProductSubCategory— has an explanatory comment: subcategories are intentionally shared/global master data plus tenant-owned overrides. Not a bug.repositories/productRepository.goGetProductCount— returns aggregate counts only (no PII), low severity, left as-is.repositories/utilsRepository.goGetSubcategories— global taxonomy/reference data; the model has no tenant field at all.repositories/orderRepository.goGetAdminOrders— hasWHERE 1=1internally but is safe because its only caller (GetOrderscontroller) only invokes it whenapplocationid != 0.repositories/orderRepository.goGetAllOrders— now dead code (unreachable) after fix #1 above; confirmed via grep it's no longer called anywhere. Could be deleted as cleanup but left in place.repositories/tenantRepository.goGetTenantLocations— already always filtersWHERE tenantid = ?. Safe, unchanged.
5. Known pre-existing bug found during this audit, NOT yet fixed anywhere
Frontend/backend path mismatch on rider logs. In nearledaily/daily_merchant_web/src/services/fiestaApi.ts, getRiderLogs() (~line 1132) calls:
fiestaGet('riders/getriderlogs', {...})
FIESTA_BASE is https://fiesta.nearle.app/live/api/v1/web, so this resolves to .../v1/web/riders/getriderlogs. But the backend only registers this route under the partners group (routes/partnerroutes.go): partner.Get("/getriderlogs", ...) on api.Group("/v1/web/partners"), i.e. the real path is .../v1/web/partners/getriderlogs. Confirmed via grep there is no /v1/web/riders route group anywhere in the backend.
This means getRiderLogs() in the web console has likely been 404ing already, independent of anything fixed this session. Fix is a one-line FE change: 'riders/getriderlogs' → 'partners/getriderlogs'. Not fixed yet because it's a frontend-repo change and wasn't the scope of this backend security pass — flagging it here so it isn't lost.
6. Frontend compatibility check (already done, no FE changes needed for the 8 fixes above)
Checked daily_merchant_web/src/services/fiestaApi.ts against every fix — the web console already sends the now-required params in all cases:
getOrders,getAllUsers,getDeliveries,getOrderSummary,getLocationSummary— all declaretenantid: numberas a required (non-optional) TS field already.getRiders— always sendsapplocationid: opts.applocationid ?? FIESTA_APPLOCATION_ID(never zero/undefined) plus requiredtenantid.getRiderLogs— sendstenantid/applocationidwhen available, but see the path bug in section 5 — worth re-verifying once that's fixed.mob/orders/getcustomerorders(fix #8) is called from the mobile app, which is not in this repo — whoever owns that codebase needs to verify every call site always sendscustomerid. Not verified in this session.
7. Environment note
No Go toolchain is available in the sandbox this session ran in (command not found: go). All edits above were manually reviewed (imports, syntax, call sites checked via Read/grep) but never compiled. Run go build ./... and the existing test suite (if any) before deploying any of this.
8. Suggested next steps for whoever picks this up
go build ./...and smoke-test all 8 changed endpoints (call with and without the required param, confirm 200 vs 400).- Fix the
riders/getriderlogs→partners/getriderlogspath bug infiestaApi.ts(section 5). - Verify the mobile app always sends
customeridtomob/orders/getcustomerordersbefore this ships, since that's the one fixed endpoint not verified from a frontend contract. - Scope and plan the real fix: JWT/session auth issuance + middleware, so
tenantid/customerid/etc. are derived from a verified server-side identity instead of trusted from query params. Until that lands, the 8 fixes in this doc only prevent the "forgot to pass an id → get everything" failure mode — they do not prevent a malicious or buggy client from passing a different tenant's/customer's/partner's real id and getting their data.