From 443d3823943f97d97ee6a768fdadea61b0c85f66 Mon Sep 17 00:00:00 2001 From: John Cardinal Date: Sat, 24 Oct 2026 14:14:43 -0700 Subject: [PATCH] case 4712 - customer contacts' pick lists limited to their own records Step 4 of 4712 (security review F15). Pick lists checked only the Select role, which customer contacts have for most types, and took the columns from the request, so a contact could list every customer with phones and emails. For contacts now: the customer list holds only their own customer (head office contacts: their head office's customers); the unit list only their own customer's units (head office contacts: the chosen customer, which must be theirs; none until one is chosen); unit models as before; any other list 403. pick-list/single names only those records. New CustomerPickList variant for a single customer. Staff unchanged. Checked: the contact's customer list went from 72 customers to its own one; units only its own; vendors and users 403. Co-Authored-By: Claude Opus 5.5 --- devdocs/modernization/NET10-PORT.md | 2 +- devdocs/modernization/SECURITY-REVIEW.md | 2 +- .../AyaNova/Controllers/PickListController.cs | 72 ++++++++++++++++++- server/AyaNova/PickList/CustomerPickList.cs | 6 +- 4 files changed, 78 insertions(+), 4 deletions(-) diff --git a/devdocs/modernization/NET10-PORT.md b/devdocs/modernization/NET10-PORT.md index 9187c4c0..f1d7117d 100644 --- a/devdocs/modernization/NET10-PORT.md +++ b/devdocs/modernization/NET10-PORT.md @@ -179,7 +179,7 @@ John's "next release" cases for AyaNova (exported 2026-10-24). Each is its own s |---|---|---| | 4710 | 2FA step requires the temp token (raven and sockeye) | F5, F41. **Done**: raven `141bdeb8` (proof script 401 on both bypasses; integration test `TwoFactor` in `8e5eec4`); sockeye `f7c1d7c` (same checks on local Sockeye with a temporary user). Sockeye to be deployed to its new droplet by John; 2FA can be turned back on once each server runs the fix | | 4711 | JWT secret setting removed: the server generates its signing key once into `jwt-signing.key` in the data folder (4698 folded in) | F1. **Done** (placeholder-forged token 401; key survives restart; auth tests pass). Hosted setup script's secret step now does nothing; tidy later in `sockeye-client` | -| 4712 | Customer contact route allowlist + ownership checks | F13, F14, F15, F16, F17. In 6 steps: 1 allowlist **done**; 2 work-order ownership **done**; 3 CSR and unit ownership **done**; 4 pick lists for contacts; 5 schedule staff rights; 6 notification subscriptions | +| 4712 | Customer contact route allowlist + ownership checks | F13, F14, F15, F16, F17. In 6 steps: 1 allowlist **done**; 2 work-order ownership **done**; 3 CSR and unit ownership **done**; 4 pick lists for contacts **done**; 5 schedule staff rights; 6 notification subscriptions | | 4713 | Contact users limited to customer roles | F28 | | 4715 | Download file names checked (one helper) | F21. **Done** (`FileUtil.FullPathInFolderOrNull` in temp/backup exists checks and both log routes; test `DownloadNames`) | | 4716 | Pick-list search text as SQL parameters | F18, and F43 (unit pick-list variant parsed as a number, found during 4712, test `FetchUnitPickListCustomerVariant`). **Done** (`@plq`/`@pltag` parameters; test `FetchUserPickListAutoCompleteApostrophe` fails on the old code, passes now; 9/9 pick-list tests) | diff --git a/devdocs/modernization/SECURITY-REVIEW.md b/devdocs/modernization/SECURITY-REVIEW.md index 57816e81..7138eaca 100644 --- a/devdocs/modernization/SECURITY-REVIEW.md +++ b/devdocs/modernization/SECURITY-REVIEW.md @@ -200,7 +200,7 @@ take the next free number; a fixed or accepted finding keeps its ID and changes | F26 | Critical | **fixed in case 4717** | KPI `interval` criterion is put into SQL unchecked; reachable by staff users down to TechRestricted | | F13 | High | **fixed in case 4712** | Customer contacts can open any customer's work order (customer view, customer report, attachments) by id | | F14 | High | **fixed in case 4712** | 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 | +| F15 | High | **fixed in case 4712** | 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 | Medium | confirmed (code) | Notification subscriptions accept any `UserId`, and object events reach subscribers without a rights check: record names to any email address | | F20 | High | candidate | Report templates receive the renderer's token (SuperUser for customer notification reports) and the report browser can reach any network address | diff --git a/server/AyaNova/Controllers/PickListController.cs b/server/AyaNova/Controllers/PickListController.cs index 9d47e7ba..87e66439 100644 --- a/server/AyaNova/Controllers/PickListController.cs +++ b/server/AyaNova/Controllers/PickListController.cs @@ -75,6 +75,13 @@ namespace AyaNova.Api.Controllers return StatusCode(403, new ApiNotAuthorizedResponse()); + //case 4712: customer contacts get only the pick lists of the service request screen, limited to their own records + var contactVariant = await ContactPickListVariantAsync(pickListParams.AyaType, pickListParams.ListVariant); + if (contactVariant == CONTACT_DENIED) + return StatusCode(403, new ApiNotAuthorizedResponse()); + if (contactVariant != null) + pickListParams.ListVariant = contactVariant; + //Instantiate the business object handler PickListBiz biz = PickListBiz.GetBiz(ct, HttpContext); @@ -122,6 +129,10 @@ namespace AyaNova.Api.Controllers if (!Authorized.HasSelectRole(HttpContext.Items, pickListSingleParams.AyaType)) return StatusCode(403, new ApiNotAuthorizedResponse()); + //case 4712: a customer contact gets names only of its own customers, their units, and unit models + if (!await ContactMayNameRecordAsync(pickListSingleParams.AyaType, pickListSingleParams.Id)) + return StatusCode(403, new ApiNotAuthorizedResponse()); + //Instantiate the business object handler PickListBiz biz = PickListBiz.GetBiz(ct, HttpContext); @@ -276,9 +287,68 @@ namespace AyaNova.Api.Controllers } + //case 4712: marker for a pick list a customer contact may not use + private const string CONTACT_DENIED = "\u0000contact-denied"; + //case 4712: for a customer contact, the variant that limits a pick list to its own records: + //null when the caller is not a contact (request used as is), CONTACT_DENIED for any list the + //customer screens do not use or a customer the contact does not own + private async Task ContactPickListVariantAsync(AyaType aType, string requestedVariant) + { + var userType = UserTypeFromContext.Type(HttpContext.Items); + if (userType != UserType.Customer && userType != UserType.HeadOffice) + return null; + var userId = UserIdFromContext.Id(HttpContext.Items); + var u = await ct.User.AsNoTracking().Where(x => x.Id == userId).Select(x => new { x.CustomerId, x.HeadOfficeId }).SingleOrDefaultAsync(); + long ownCustomerId = u?.CustomerId ?? 0; + long ownHeadOfficeId = u?.HeadOfficeId ?? 0; + switch (aType) + { + case AyaType.Customer: + if (userType == UserType.Customer) + return ownCustomerId == 0 ? CONTACT_DENIED : $"{ownCustomerId},{(int)AyaType.Customer}"; + return ownHeadOfficeId == 0 ? CONTACT_DENIED : $"{ownHeadOfficeId},{(int)AyaType.HeadOffice}"; + case AyaType.Unit: + if (userType == UserType.Customer) + return ownCustomerId == 0 ? CONTACT_DENIED : $"customerid:{ownCustomerId}"; + //head office contact: units of the customer chosen on the form, which must be one of its customers; + //none until a customer is chosen + var v = (requestedVariant ?? string.Empty).Split(':'); + long requestedCustomerId = 0; + if (v.Length == 2 && v[0].Trim() == "customerid") + long.TryParse(v[1].Trim(), System.Globalization.NumberStyles.Integer, System.Globalization.CultureInfo.InvariantCulture, out requestedCustomerId); + if (requestedCustomerId == 0) + return "customerid:-1"; + return await UserBiz.CustomerContactOwnsCustomerAsync(ct, userId, requestedCustomerId) ? $"customerid:{requestedCustomerId}" : CONTACT_DENIED; + case AyaType.UnitModel: + return requestedVariant;//unit models are not customer data + default: + return CONTACT_DENIED; + } + } - + //case 4712: may the caller see this record's pick list name (always true for staff) + private async Task ContactMayNameRecordAsync(AyaType aType, long id) + { + var userType = UserTypeFromContext.Type(HttpContext.Items); + if (userType != UserType.Customer && userType != UserType.HeadOffice) + return true; + var userId = UserIdFromContext.Id(HttpContext.Items); + switch (aType) + { + case AyaType.Customer: + return id == 0 || await UserBiz.CustomerContactOwnsCustomerAsync(ct, userId, id); + case AyaType.Unit: + if (id == 0) + return true; + var unitCustomerId = await ct.Unit.AsNoTracking().Where(x => x.Id == id).Select(x => (long?)x.CustomerId).FirstOrDefaultAsync(); + return unitCustomerId != null && await UserBiz.CustomerContactOwnsCustomerAsync(ct, userId, (long)unitCustomerId); + case AyaType.UnitModel: + return true; + default: + return false; + } + } }//eoc }//ens \ No newline at end of file diff --git a/server/AyaNova/PickList/CustomerPickList.cs b/server/AyaNova/PickList/CustomerPickList.cs index cd98031a..a75c0407 100644 --- a/server/AyaNova/PickList/CustomerPickList.cs +++ b/server/AyaNova/PickList/CustomerPickList.cs @@ -121,7 +121,7 @@ namespace AyaNova.PickList int nType = 0; if (!int.TryParse(crit[1], out nType)) return string.Empty; AyaType forType = (AyaType)nType; - if (forType != AyaType.HeadOffice) return string.Empty; + if (forType != AyaType.HeadOffice && forType != AyaType.Customer) return string.Empty; long lId = 0; if (!long.TryParse(crit[0], out lId)) return string.Empty; @@ -134,6 +134,10 @@ namespace AyaNova.PickList { return $"acustomer.headofficeid = {lId}"; } + case AyaType.Customer://case 4712: one customer only (a customer contact's own) + { + return $"acustomer.id = {lId}"; + } } } return string.Empty;