admin/raven
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Security review pass 3: what customer contact users can reach

Browse Source 
Confirmed on the local server as the seeded contact: any customer's work order
by id (F13), other customers' service requests (F14), the full customer list
with phones and emails through pick lists (F15), the whole service schedule
(F16). Candidates from code: notification subscriptions for another user (F17),
pick-list search text placed in SQL unescaped (F18). Review only, no code changes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
John Cardinal
2026-10-23 19:50:21 -07:00
parent 26173b3926
commit 2004bf197a
1 changed files with 97 additions and 2 deletions
Download Patch File Download Diff File Expand all files Collapse all files

99
devdocs/modernization/SECURITY-REVIEW.md
View File

@@ -13,8 +13,8 @@ keep it that way while findings are open.
|---|---|---|
| 1 | Threat model and attack-surface map | Done 2026-10-23 |
| 2 | Unauthenticated routes, login, tokens, password reset, 2FA | Done 2026-10-23 |
| 3 | Customer contact users: what they can reach and read | Next |
| 4 | Report rendering (templates, the server-side browser, report data and download routes) | |
| 3 | Customer contact users: what they can reach and read | Done 2026-10-23 |
| 4 | Report rendering (templates, the server-side browser, report data and download routes) | Next |
| 5 | File handling: attachments, uploads, exports, logs, backups, restore, translations, logo | |
| 6 | Raw SQL: data lists, saved filters, search, KPI, schedule, pick lists | |
| 7 | Staff roles: the authenticated routes without a role check in the action (list below) | |
@@ -188,6 +188,12 @@ Status: *candidate* (seen in code, not yet confirmed), *confirmed*, *fixed in ca
|---|---|---|---|
| F5 | **Critical** | **confirmed** | 2FA login step accepts an empty or missing temp token: the password is skipped for users with 2FA on |
| F1 | Critical (installs that kept it) | **confirmed** | Placeholder JWT secret in the LAN/IIS and Linux server config templates |
| F18 | Critical | candidate | Pick-list search text is put into SQL without escaping; any logged-in user can reach it |
| F13 | High | **confirmed** | Customer contacts can open any customer's work order (customer view, customer report, attachments) by id |
| F14 | High | **confirmed** | Customer contacts can read, and by code change, delete and create, any customer's service requests |
| F15 | High | **confirmed** | Customer contacts can list every customer (and vendors, users, parts...) with any pick-list columns, phones and emails included |
| F16 | High | **confirmed** | `schedule/svc` and `schedule/user` have no role check: anyone logged in sees the whole service schedule |
| F17 | High | candidate | Notification subscriptions accept any `UserId`: a contact could subscribe on a staff user's behalf with delivery to its own email |
| F3 | High | candidate | Any logged-in user, customer contacts included, can send arbitrary email through the company's SMTP server |
| F6 | Medium | confirmed | No lockout or rate limit on login or the 2FA step; no password rules |
| F9 | Medium | confirmed | Default SuperUser password, and an anonymous route that says whether it is still set |
@@ -315,6 +321,94 @@ PBKDF2-HMAC-SHA512, 10,000 iterations, 32-byte random salt (`Hasher.cs`). Curren
(OWASP) is 210,000 iterations for SHA-512. Only matters if the database or a backup leaks.
Raising it needs re-hashing at next login (store the iteration count). Roadmap item, not 8.3.0.
### Pass 3 overview: customer contact isolation
Customer and head-office contacts are users with `UserType` Customer/HeadOffice and the
`Customer` role (seeded contacts: `AuthorizationRoles.Customer`). Isolation to their own
customer is enforced **only in data lists**: `DataListController` allows them two lists
(`CustomerServiceRequestDataList`, `CustomerWorkOrderDataList`) and sets the customer
filter itself. Everywhere else the checks are role sets designed for staff, and
`AuthorizationRoles.All` includes `Customer` and `CustomerRestricted`: `Select` is `All` on
almost every type (Customer, User, Vendor, Part, Unit, WorkOrder…), `ReadFullRecord` is `All`
on Report, TaskGroup, QuoteStatus, and the `Customer` role can change any
CustomerServiceRequest. The customer-specific rights helper
(`UserBiz.CustomerUserEffectiveRightsAsync`) answers "is this feature on for this contact
and these tags", never "does this record belong to the contact's customer".
All proofs below: local dev server, seeded contact `Customer` (customer id 1), read-only
requests; seeded data is fake.
**Fix direction** (one case, possibly in steps): deny customer-type users by default, with an
allowlist of the routes the customer UI actually calls (to be listed from the client), as a
filter or in the token middleware, where `AY_USER_TYPE` is already set; then add the
ownership check in each allowed route that takes an id (work order customer view, customer
report, attachments, CSR). This also covers routes not yet reviewed.
### F13 Any customer's work order by id (pass 3, confirmed)
`GET workorder/customer/{id}` (`WorkOrderBiz.WorkOrderGetForCustomerAsync`) loads the work
order by id and never compares its `CustomerId` with the contact's. It returns serial, service
date, customer name, customer reference number, contact name, invoice number, and the wiki
when tag rules allow it. **Proof**: the contact of customer 1 read work order 1 of customer 12
(200, with that customer's name). A staff user calling it gets a 500 (the helper throws for
non-customer users). The same tag-only rights, without ownership, also gate (code reading):
- the customer work-order report (`ReportBiz`, report id `-100`): takes `SelectedRowIds[0]`,
gets the effective report from that work order's tags, then renders with the role check
skipped (`RequestIsCustomerWorkOrderReport`), so a contact can render the configured
customer report for any work order (pass 4 to confirm);
- work-order attachments (`attachment/list`, `attachment/download/{id}` for customer users):
`ThisWOCanAttachments` from tags only.
These need the customer work-order features turned on in global settings (common where the
customer portal is used); the customer view route does not, it always answers.
### F14 Customer service requests across customers (pass 3, confirmed for reading)
CSR `Change` includes `Customer`, and `HasReadFullRole` counts `Change` as read.
`CustomerServiceRequestBiz` never checks `CustomerId` against the contact (create, get, put,
delete). **Proof**: the contact read CSR 1 of customer 16 (200, with its text). Change, delete
and create for another customer were not tested (they write data) but follow from the same
code. Search also returns CSRs of all customers (it filters by role only).
### F15 Pick lists expose the whole customer list (pass 3, confirmed)
`POST pick-list/list` checks only `Select`, which the `Customer` role has for customers, head
offices, vendors, users, parts, units, contracts, projects, rates and more. The caller also
sends its own `template`, which chooses the returned columns. **Proof**: the contact got all
70 seeded customers with phone and email. `pick-list/single` works the same way. A
customer's contact can therefore download the company's customer list with contact details.
Fix: no `Select` for customer roles on these types (keep what the customer UI needs, likely
units of its own customer) and ignore `template` from customer-type users.
### F16 Service schedule open to everyone logged in (pass 3, confirmed)
`POST schedule/svc` and `POST schedule/user` have no role check. **Proof**: the contact got
1,257 schedule items, each "work order number + customer name" with times and the assigned
technicians. Fix: require read rights on `WorkOrderItemScheduledUser` (or `WorkOrder`) and
refuse customer-type users. (`schedule/personal` is the caller's own schedule.)
### F17 Notification subscription for someone else (pass 3, candidate)
`NotifySubscriptionBiz.CreateAsync` and `PutAsync` keep the `UserId` sent by the client, and
validation checks the event type against *that* user's type, not the caller's. A subscription
has its own `DeliveryAddress` and can attach a report (`LinkReportId`). So, by code, a
customer contact could create a subscription with a staff user's id, a staff event type and
its own email address, and receive staff notifications, possibly with reports attached. Not
tested live because it sends real email. Fix: force `UserId` to the caller unless the caller
has user-admin rights. Pass 8 to confirm how delivery checks rights.
### F18 Pick-list search text in SQL (pass 3, candidate, code reading only)
`PickListSqlBuilder.Build` puts the search text and the tag search text into `like` fragments
by string interpolation, and no escaping was found on the way from
`PickListController.PostList` through `PickListBiz.GetPickListAsync`. The route is open to
every logged-in user, customer contacts included. Data-list text filters escape quotes since
case 4676; pick lists were not covered. Deliberately not tested live. Fix: pass the search
texts as SQL parameters (preferred) or escape them as the data-list builder does; review the
rest of the pick-list builder in pass 6.
### F2 Windows folder permissions (from the 8.3.0 installer test)
`lan.iss` and `standalone.iss` set `Permissions: users-modify` on `{app}` (the program
@@ -348,3 +442,4 @@ by a customer contact. Pass 8.
|---|---|---|
| 2026-10-23 | 1 Threat model and map | Scope agreed with John. Middleware, JWT validation, role model, download keys, CORS and Swagger read. Route map generated (460 actions; script kept in `security/routemap.py`). No route lacks authentication by accident. 53 logged-in routes have no role check in the action: 3 resolved (erase data, trial seed: OK; direct SMTP: F3), the rest assigned to later passes. Local-request exception not reachable through the hosted nginx. Findings F1 to F4 recorded as candidates |
| 2026-10-23 | 2 Login, tokens, reset, 2FA, anonymous routes | Read `AuthController`, the token middleware, `Hasher`, download-key check, password reset, TOTP setup, and every anonymous route (`notify/hello`, `license/lc`, `translation/subset`, `logo`, API root, health). Tested on the local dev server with the seeded `Inventory` user: **F5 critical, confirmed** (2FA step without password); **F1 confirmed** (current token rebuilt from the secret in 6 tries); F8 confirmed. New: F6, F7, F9 to F12. OK: reset codes (32 random characters, 48 h, cleared on use, same reply for unknown codes), login replies do not reveal whether a login exists, roles read from the database on every request, deactivation ends the session, TOTP secret 160 bits. Leads for pass 4: `AuthController.GenRpt` issues SuperUser (id 1) tokens with an `rpl` claim for report rendering; pass 5: the logo is served with the content type given at upload |
| 2026-10-23 | 3 Customer contacts | Mapped the `Customer` role in `BizRoles.cs` and every customer-reachable route. Tested on the local dev server as seeded contact `Customer` (customer 1), read-only: F13, F14 (read), F15, F16 confirmed. F17 and F18 from code (F17 not tested because it sends email; F18 not tested by choice). OK: data lists (server sets the customer filter), KPIs (no customer roles), saved filters and notification subscription reads (scoped to the user), memos (no customer access) |