diff --git a/devdocs/modernization/NET10-PORT.md b/devdocs/modernization/NET10-PORT.md index e16d6e19..faebcd6d 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; 3 CSR and unit ownership; 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; 4 pick lists for contacts; 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. **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 d6825378..799fa246 100644 --- a/devdocs/modernization/SECURITY-REVIEW.md +++ b/devdocs/modernization/SECURITY-REVIEW.md @@ -197,7 +197,7 @@ take the next free number; a fixed or accepted finding keeps its ID and changes | F21 | Critical | **fixed in case 4715** | Download file names from the URL are not checked: `report/download` and `export/download` may serve files outside the temp folder on Windows | | F18 | Critical | **fixed in case 4716** | Pick-list search text is put into SQL without escaping; any logged-in user can reach it | | F26 | Critical | **fixed in case 4717** | KPI `interval` criterion is put into SQL unchecked; reachable by staff users down to TechRestricted | -| F13 | High | **confirmed** | Customer contacts can open any customer's work order (customer view, customer report, attachments) by id | +| F13 | High | **fixed in case 4712** | 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 | diff --git a/server/AyaNova/Controllers/AttachmentController.cs b/server/AyaNova/Controllers/AttachmentController.cs index 5617a74e..982a6923 100644 --- a/server/AyaNova/Controllers/AttachmentController.cs +++ b/server/AyaNova/Controllers/AttachmentController.cs @@ -157,11 +157,16 @@ namespace AyaNova.Api.Controllers //Is this a customer user attempting to view a wo attachments?? var userType = UserTypeFromContext.Type(HttpContext.Items); - if (ayaType == AyaType.WorkOrder && (userType == UserType.Customer || userType == UserType.HeadOffice)) + //case 4712: customer contacts see attachments only on their own customer's work orders + if (userType == UserType.Customer || userType == UserType.HeadOffice) { + if (ayaType != AyaType.WorkOrder) + return StatusCode(403, new ApiNotAuthorizedResponse()); + var wo = await ct.WorkOrder.AsNoTracking().Where(x => x.Id == ayaId).Select(x => new { x.Tags, x.CustomerId }).FirstOrDefaultAsync(); + if (wo == null || !await UserBiz.CustomerContactOwnsCustomerAsync(ct, UserIdFromContext.Id(HttpContext.Items), wo.CustomerId)) + return StatusCode(403, new ApiNotAuthorizedResponse()); //check if allowed - var woTags = await ct.WorkOrder.AsNoTracking().Where(x => x.Id == ayaId).Select(x => x.Tags).FirstOrDefaultAsync(); - var custUserRights = await UserBiz.CustomerUserEffectiveRightsAsync(UserIdFromContext.Id(HttpContext.Items), woTags); + var custUserRights = await UserBiz.CustomerUserEffectiveRightsAsync(UserIdFromContext.Id(HttpContext.Items), wo.Tags); if (!custUserRights.ThisWOCanAttachments) return StatusCode(403, new ApiNotAuthorizedResponse()); } @@ -553,11 +558,16 @@ namespace AyaNova.Api.Controllers //is this allowed? //Is this a customer user attempting to view a wo attachments?? - if (dbObject.AttachToAType == AyaType.WorkOrder && (DownloadUser.UserType == UserType.Customer || DownloadUser.UserType == UserType.HeadOffice)) + //case 4712: customer contacts download attachments only from their own customer's work orders + if (DownloadUser.UserType == UserType.Customer || DownloadUser.UserType == UserType.HeadOffice) { + if (dbObject.AttachToAType != AyaType.WorkOrder) + return StatusCode(403, new ApiNotAuthorizedResponse()); + var wo = await ct.WorkOrder.AsNoTracking().Where(x => x.Id == dbObject.AttachToObjectId).Select(x => new { x.Tags, x.CustomerId }).FirstOrDefaultAsync(); + if (wo == null || !await UserBiz.CustomerContactOwnsCustomerAsync(ct, DownloadUser.Id, wo.CustomerId)) + return StatusCode(403, new ApiNotAuthorizedResponse()); //check if allowed - var woTags = await ct.WorkOrder.AsNoTracking().Where(x => x.Id == dbObject.AttachToObjectId).Select(x => x.Tags).FirstOrDefaultAsync(); - var custUserRights = await UserBiz.CustomerUserEffectiveRightsAsync(DownloadUser.Id, woTags); + var custUserRights = await UserBiz.CustomerUserEffectiveRightsAsync(DownloadUser.Id, wo.Tags); if (!custUserRights.ThisWOCanAttachments) return StatusCode(403, new ApiNotAuthorizedResponse()); } diff --git a/server/AyaNova/biz/ReportBiz.cs b/server/AyaNova/biz/ReportBiz.cs index 326d726f..1ab5a910 100644 --- a/server/AyaNova/biz/ReportBiz.cs +++ b/server/AyaNova/biz/ReportBiz.cs @@ -581,13 +581,24 @@ What could be the issue here? { log.LogDebug("customer user report requested"); //get the user and workorder data and set the actual report id or return null if not found - var woTags = await ct + //case 4712: exactly one work order, and it must belong to the contact's customer (the render + //below uses every id in SelectedRowIds, so extra ids are not passed on) + if (reportRequest.SelectedRowIds == null || reportRequest.SelectedRowIds.Length != 1) + { + AddError(ApiErrorCode.NOT_FOUND, "id"); + return null; + } + var wo = await ct .WorkOrder.AsNoTracking() .Where(x => x.Id == reportRequest.SelectedRowIds[0]) - .Select(x => x.Tags) + .Select(x => new { x.Tags, x.CustomerId }) .FirstOrDefaultAsync(); - if (woTags == null) + if (wo == null || !await UserBiz.CustomerContactOwnsCustomerAsync(ct, UserId, wo.CustomerId)) + { + AddError(ApiErrorCode.NOT_FOUND, "id"); return null; + } + var woTags = wo.Tags; var cr = await UserBiz.CustomerUserEffectiveRightsAsync(UserId, woTags); if (cr.ThisWOEffectiveWOReportId == null) return null; diff --git a/server/AyaNova/biz/UserBiz.cs b/server/AyaNova/biz/UserBiz.cs index f2b5b5ef..f83bebb4 100644 --- a/server/AyaNova/biz/UserBiz.cs +++ b/server/AyaNova/biz/UserBiz.cs @@ -185,6 +185,20 @@ namespace AyaNova.Biz } } + //case 4712: true if the customer is a customer contact's own customer or, for a head office contact, + //one of its head office's customers; false for any other user type + internal static async Task CustomerContactOwnsCustomerAsync(AyContext ct, long userId, long customerId) + { + var u = await ct.User.AsNoTracking().Where(x => x.Id == userId).Select(x => new { x.UserType, x.CustomerId, x.HeadOfficeId }).FirstOrDefaultAsync(); + if (u == null) + return false; + if (u.UserType == UserType.Customer) + return u.CustomerId != null && u.CustomerId == customerId; + if (u.UserType == UserType.HeadOffice) + return u.HeadOfficeId != null && await ct.Customer.AsNoTracking().AnyAsync(x => x.Id == customerId && x.HeadOfficeId == u.HeadOfficeId); + return false; + } + //For auth and access in client, also when opening wo and also when reporting wo internal static async Task CustomerUserEffectiveRightsAsync(long userId, List thisWorkOrderTags = null) { diff --git a/server/AyaNova/biz/WorkOrderBiz.cs b/server/AyaNova/biz/WorkOrderBiz.cs index 848f16b5..c3dc57c9 100644 --- a/server/AyaNova/biz/WorkOrderBiz.cs +++ b/server/AyaNova/biz/WorkOrderBiz.cs @@ -314,6 +314,8 @@ namespace AyaNova.Biz { var sourceWo = await ct.WorkOrder.AsNoTracking().FirstOrDefaultAsync(z => z.Id == id); if (sourceWo == null) return null; + //case 4712: another customer's work order is answered as not found + if (!await UserBiz.CustomerContactOwnsCustomerAsync(ct, UserId, sourceWo.CustomerId)) return null; var customerEffectiveRights = await UserBiz.CustomerUserEffectiveRightsAsync(UserId, sourceWo.Tags); var ret = new CustomerWorkOrder(); ret.Id = sourceWo.Id;