From 07c10f6542832d6a165d38d9b3984d2fd154ed6d Mon Sep 17 00:00:00 2001 From: John Cardinal Date: Fri, 23 Oct 2026 20:00:31 -0700 Subject: [PATCH] Security review: draft customer contact route allowlist for the F13 to F17 fix Routes the client calls for customer contacts, read from raven-client, with the ownership check each id route needs, and the proposed shape of the fix (allowlist in the token middleware plus per-route ownership checks). Co-Authored-By: Claude Opus 5.5 --- devdocs/modernization/SECURITY-REVIEW.md | 39 ++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/devdocs/modernization/SECURITY-REVIEW.md b/devdocs/modernization/SECURITY-REVIEW.md index f872b537..6750b14e 100644 --- a/devdocs/modernization/SECURITY-REVIEW.md +++ b/devdocs/modernization/SECURITY-REVIEW.md @@ -443,6 +443,45 @@ Any logged-in user can add a general notification, with the sender's name and an any user id. Low on its own (in-app only); relevant with F3 for social engineering of staff by a customer contact. Pass 8. +## Customer contact route allowlist (draft, for the F13 to F17 fix) + +The API routes the client can call for a customer contact (`UserType` Customer or HeadOffice), +read from `raven-client` on 2026-10-23: the customer menu in `api/initialize.js` (user +settings, notification subscriptions, work orders, service requests), the screens behind it +(`customer-workorders`, `customer-workorder`, `customer-csr`, `customer-csr-edit`, +`home-user-settings`, `home-password`, `home-security`, `home-notify-subscription(s)`, +`home-notifications`, `ay-about`, `ay-report-view`, `login`, `home-reset`), and the components +they use. Staff-only parts of shared components are left out: data-table extensions (export, +batch delete, tags, direct SMTP) are not on customer screens, attachments and wiki are +read-only there, and CSR accept/reject are staff menu items. Method used: +`customer_routes.py` (kept outside the repo) follows each screen's imports and `gz-` components +and lists every `$gz.api` call; the result was then checked by hand. + +Everything not on this list would answer 403 to a customer-type user. **Ownership** says what +each id route must also check. + +| Area | Routes | Ownership check needed | +|---|---|---| +| Login, session (already anonymous or exempt) | `auth`, `auth/tfa-authenticate`, `auth/reset-password`, `notify/hello`, `translation/subset[/{id}]`, `logo/{size}`, `license/lc` (F10) | — | +| Own account | `auth/change-password`, `auth/totp`, `auth/totp-validate`, `auth/totp-disable/{id}`, `user-option/{id}` GET and PUT | `{id}` = the caller (`totp-disable` already; check `user-option`) | +| App shell | `global-biz-setting/client`, `enum-list/list/{key}`, `form-custom/{formkey}`, `translation/list`, `server-info`, `notify/new-count`, `notify/app-notifications`, `DELETE notify/{id}` | `notify/{id}`: the caller's own notification (pass 7) | +| Data lists (work orders, CSRs) | `data-list` (server already forces the customer filter), `data-list/listfields`, `data-list-column-view…`, `data-list-filter…`, `tag-list/list`, `pick-list/list` from the filter editor | saved filters: already scoped to the user | +| Work order (customer view) | `workorder/customer/{id}`, `attachment/list` (work order only), `attachment/download/{id}` (download key), `report/render-job` with report `-100` only, `job-operations/status|logs/{gid}`, `report/request-cancel/{gid}`, `report/download/{file}` | work order belongs to the contact's customer (or a customer of its head office): **F13**; job and report file belong to the caller (pass 4) | +| Service requests | `csr` POST/PUT, `csr/{id}` GET/DELETE (not `accept`/`reject`), `unit/customerunit` POST, `pick-list/list` and `pick-list/single` for Customer (head-office variant), Unit (own customer) and UnitModel only, with the server's own template | CSR and unit belong to the contact's customer; `CustomerId` forced on create: **F14**, **F15** | +| Notification subscriptions | `notify-subscription/list`, `notify-subscription/{id}` GET/DELETE, `notify-subscription` POST/PUT, `work-order-status/list`, `quote-status/list` (names only, used by the editor) | `UserId` forced to the caller: **F17** | + +To confirm while fixing: whether the `open-object-handler` routes (`attachment/parent`, +`form-custom/form-key`, `user/inside-type`, `search/ancestor`) are used when a customer opens +a notification; whether `server-info` (about page) must stay open to contacts. + +**Shape of the fix.** One check in the token middleware, which already loads `UserType` for +every request: for Customer and HeadOffice users, compare the matched endpoint's route +template (`context.GetEndpoint()`, available because `UseRouting` runs first) with this +allowlist and answer 403 otherwise. New routes then stay closed to contacts unless added +on purpose. The ownership checks stay in the routes themselves (F13, F14, F15, F17). F16 +(schedule) is closed by the allowlist alone; its staff role check is still needed for staff +roles that should not see the schedule (pass 7). + ## Pass log | Date | Pass | Result |