From ee59efff48e50b02a04271d2005a88a79a0fd07e Mon Sep 17 00:00:00 2001 From: John Cardinal Date: Sat, 24 Oct 2026 15:59:35 -0700 Subject: [PATCH] case 4713 - customer contacts hold and act with customer roles only Users who may edit contacts but not users (Service, Sales, Accounting) could give a contact login any role, BizAdmin and OpsAdmin included, and roles are what every check uses (security review F28). Now: - saving a Customer or HeadOffice user with any role other than Customer or CustomerRestricted is refused (error on Roles); - UserBiz.EffectiveRoles masks a contact's roles to the customer roles wherever they are applied: the token middleware, download-key routes, and the notification subscriber check, so contacts that already have staff roles stored act without them. Checked: Service creating a contact with BizAdmin refused, with Customer works; a contact with OpsAdmin stored in the database gets 403 downloading a log file. Co-Authored-By: Claude Opus 5.5 --- devdocs/modernization/NET10-PORT.md | 2 +- devdocs/modernization/SECURITY-REVIEW.md | 2 +- server/AyaNova/Startup.cs | 3 ++- server/AyaNova/biz/NotifyEventHelper.cs | 4 ++-- server/AyaNova/biz/UserBiz.cs | 18 ++++++++++++++++++ 5 files changed, 24 insertions(+), 5 deletions(-) diff --git a/devdocs/modernization/NET10-PORT.md b/devdocs/modernization/NET10-PORT.md index de859aa9..ef5319dd 100644 --- a/devdocs/modernization/NET10-PORT.md +++ b/devdocs/modernization/NET10-PORT.md @@ -180,7 +180,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 **done**; 5 schedule staff rights **done**; 6 notification subscriptions **done**. All steps done. John clicked through the customer screens as Customer, HeadOffice and CustomerRestricted contacts (2026-10-24): no failures; contact attachments checked by API (own work order list and download work, another customer's 403) | -| 4713 | Contact users limited to customer roles | F28 | +| 4713 | Contact users limited to customer roles | F28. **Done** (validation refuses staff roles on contacts; contacts act with customer roles only, also on download routes; test `ContactRoles`) | | 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) | | 4717 | KPI interval allowlist | F26. **Done** (`KPIFetcher.DateTruncUnitOrNull` in the 7 KPIs reading `interval`; test `KpiInterval`) | diff --git a/devdocs/modernization/SECURITY-REVIEW.md b/devdocs/modernization/SECURITY-REVIEW.md index c35a4c0b..29260411 100644 --- a/devdocs/modernization/SECURITY-REVIEW.md +++ b/devdocs/modernization/SECURITY-REVIEW.md @@ -205,7 +205,7 @@ take the next free number; a fixed or accepted finding keeps its ID and changes | F17 | Medium | **fixed in case 4712** | 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 | | F27 | High | **confirmed** | Restricted roles (TechRestricted, SubContractor, SubContractorRestricted) get whole work-order items through report data, rendering and export | -| F28 | High | candidate | Users with change rights on Customer (Service, Sales, Accounting) can give customer contact logins any role, BizAdmin and OpsAdmin included | +| F28 | High | **fixed in case 4713** | Users with change rights on Customer (Service, Sales, Accounting) can give customer contact logins any role, BizAdmin and OpsAdmin included | | F33 | High | confirmed (code) | Client renders some record text as HTML without escaping (search results, work-order address dialog, unit warranty info, dialogs) | | F3 | High | confirmed (code) | Any logged-in user, customer contacts included, can send arbitrary email through the company's SMTP server | | F23 | Medium | confirmed (code) | Logo upload accepts any content type and the anonymous logo route serves it inline from the app's origin (stored XSS) | diff --git a/server/AyaNova/Startup.cs b/server/AyaNova/Startup.cs index 390157d6..2f72d4ab 100644 --- a/server/AyaNova/Startup.cs +++ b/server/AyaNova/Startup.cs @@ -406,7 +406,8 @@ namespace AyaNova //get the user record var u = await ct.User.AsNoTracking().Where(a => a.Id == userId).Select(m => new { roles = m.Roles, name = m.Name, m.UserType, id = m.Id, translationId = m.UserOptions.TranslationId, currentAuthToken = m.CurrentAuthToken }).FirstAsync(); - context.Request.HttpContext.Items["AY_ROLES"] = u.roles; + //case 4713: a customer contact acts only with the customer roles, whatever roles are stored + context.Request.HttpContext.Items["AY_ROLES"] = UserBiz.EffectiveRoles(u.UserType, u.roles); context.Request.HttpContext.Items["AY_USERNAME"] = u.name; context.Request.HttpContext.Items["AY_USER_ID"] = u.id; context.Request.HttpContext.Items["AY_TRANSLATION_ID"] = u.translationId; diff --git a/server/AyaNova/biz/NotifyEventHelper.cs b/server/AyaNova/biz/NotifyEventHelper.cs index 63245b39..91f3c039 100644 --- a/server/AyaNova/biz/NotifyEventHelper.cs +++ b/server/AyaNova/biz/NotifyEventHelper.cs @@ -75,8 +75,8 @@ namespace AyaNova.Biz //case 4712: object events (created, modified, deleted, age) go only to subscribers with read rights on that type private static async Task SubscriberMayReadAsync(AyContext ct, long userId, AyaType aType) { - var roles = await ct.User.AsNoTracking().Where(z => z.Id == userId).Select(z => z.Roles).FirstOrDefaultAsync(); - return AyaNova.Api.ControllerHelpers.Authorized.HasReadFullRole(roles, aType); + var u = await ct.User.AsNoTracking().Where(z => z.Id == userId).Select(z => new { z.UserType, z.Roles }).FirstOrDefaultAsync(); + return u != null && AyaNova.Api.ControllerHelpers.Authorized.HasReadFullRole(UserBiz.EffectiveRoles(u.UserType, u.Roles), aType); } public static async Task ProcessStandardObjectCreatedEvents(ICoreBizObjectModel newObject, AyContext ct) diff --git a/server/AyaNova/biz/UserBiz.cs b/server/AyaNova/biz/UserBiz.cs index f83bebb4..4e366fed 100644 --- a/server/AyaNova/biz/UserBiz.cs +++ b/server/AyaNova/biz/UserBiz.cs @@ -185,6 +185,14 @@ namespace AyaNova.Biz } } + //case 4713: the roles a user acts with: customer contacts only ever the customer roles, whatever is stored + internal static AuthorizationRoles EffectiveRoles(UserType userType, AuthorizationRoles storedRoles) + { + if (userType == UserType.Customer || userType == UserType.HeadOffice) + return storedRoles & (AuthorizationRoles.Customer | AuthorizationRoles.CustomerRestricted); + return storedRoles; + } + //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) @@ -1035,6 +1043,14 @@ namespace AyaNova.Biz AddError(ApiErrorCode.VALIDATION_INVALID_VALUE, "Roles"); } + //case 4713: customer contacts may hold only the customer roles (users who may edit contacts + //but not users, such as Sales, could otherwise give a contact login administrator roles) + if ((proposedObj.UserType == UserType.Customer || proposedObj.UserType == UserType.HeadOffice) + && EffectiveRoles(proposedObj.UserType, proposedObj.Roles) != proposedObj.Roles) + { + AddError(ApiErrorCode.VALIDATION_INVALID_VALUE, "Roles"); + } + //Any form customizations to validate? //contact is type if customer user @@ -1146,6 +1162,8 @@ namespace AyaNova.Biz var utcNow = new DateTimeOffset(DateTime.Now.ToUniversalTime(), TimeSpan.Zero); if (DownloadUser.DlKeyExpire < utcNow.DateTime) return null; + //case 4713: download routes check these roles, so a customer contact gets only the customer roles here too + DownloadUser.Roles = EffectiveRoles(DownloadUser.UserType, DownloadUser.Roles); return DownloadUser; }