diff --git a/devdocs/modernization/NET10-PORT.md b/devdocs/modernization/NET10-PORT.md index c96b5fc5..151d44f4 100644 --- a/devdocs/modernization/NET10-PORT.md +++ b/devdocs/modernization/NET10-PORT.md @@ -189,7 +189,7 @@ John's "next release" cases for AyaNova (exported 2026-10-24). Each is its own s | 4720 | Restricted roles filtered in report/export data | F27. **Done** (partial fetch filtered like the full one; BizObjectFactory passed UserType.Service for everyone, now the real type; test `RestrictedReportData`) | | 4721 | Client escaping of record text | F33. **Done** (raven-client: `escapeHtml` helper for the warranty summary; address dialog as text; search excerpt escaped with the terms highlighted as plain text; dialog messages through DOMPurify. The excerpt is not encoded on the server as well: the client escaping covers it. John checked search, warranty info with `` in a vendor name, the address dialog and confirmation dialogs) | | 4722 | DOMPurify and Vuetify upgrades (raven-client) | F34. **Done** (raven-client `44411ec1`: DOMPurify 2.5.9, Vuetify 2.7.2, builds on Node 12.22.9; Vuetify 2.7's calendar `eventSummary()` returns an element, so the six calendar event templates render it with a new `gz-vnodes` component instead of `v-html`. John checked wiki, schedules (drag, stretch, lock icons), date pickers, lists, dialogs and the reminder and review widgets. John also checked the scheduled work order widget as a Tech user. Still to do: Cypress on `devtestn10` at the final gate) | -| 4723 | Direct SMTP: address from the record only | F3, F4 | +| 4723 | Direct SMTP and direct notification: staff only, record rights | F3, F4. **Done**, plan revised with John: staff keep sending to any address (stock report template and a customer script use it); contacts refused in both actions, an address from a record needs read rights on it, direct notifications refused to subcontractors. Test `DirectSend` (the subcontractor case returned 204 on the old code). No changelog line: nothing that works changes | | 4724 | SMTP: certificate checked, password never returned | F30, F31 | | 4725 | Sessions: password change ends sessions, exact-path token exemption, default SuperUser refused when licensed | F7, F8, F9 | | 4726 | Login throttling/lockout | F6 | @@ -249,6 +249,7 @@ FastMail password after the old Sockeye server is retired), John's cases. Found during the port, out of scope. One line each. +- `NotifyController.SendNotifySmtpDirectMessage`, User branch: the missing-address and inactive messages say "Vendor" instead of "User" (found in case 4723) - `License.DoFetchAsync` (`License.cs:629-661`) rethrows at `x == 3`, so the fifth server (Sockeye) is never tried, and it can return `null` to a caller that dereferences it - Stray `raven-client\ayanova\cypress.json`: the Cypress suite lives in `raven-test\e2e` - **Race, affects production after every restart:** `FormFieldReference.FormFieldReferenceList` (`biz/FormFieldReference.cs:57-59`) builds a static dictionary lazily with no lock, assigning it before filling it. A concurrent first request sees it half-built and throws `FormFieldReferenceList: is not valid`. Seen twice on .NET 10, each time on the first form-field use after boot: `VendorCrud.CRUD`, then `FormCustom.FormCustomUpdate`. Fix: build the dictionary fully before publishing it (`Lazy` or a static initialiser). Pre-existing, not a port change. John's decision: its own case, fixed with the other cases going into this release, not under the port diff --git a/devdocs/modernization/SECURITY-REVIEW.md b/devdocs/modernization/SECURITY-REVIEW.md index 62d27517..dbcaf623 100644 --- a/devdocs/modernization/SECURITY-REVIEW.md +++ b/devdocs/modernization/SECURITY-REVIEW.md @@ -207,7 +207,7 @@ take the next free number; a fixed or accepted finding keeps its ID and changes | F27 | High | **fixed in case 4720** | Restricted roles (TechRestricted, SubContractor, SubContractorRestricted) get whole work-order items through report data, rendering and export | | 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 | **fixed in case 4721** | 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 | +| F3 | High | **fixed in cases 4712 and 4723** (staff sending to any address kept, by design) | 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) | | F24 | Medium | **fixed in case 4718** | Translation import has no role check: any login can add translations | | F30 | Medium | confirmed (code) | SMTP: the mail server's TLS certificate is never checked | @@ -221,7 +221,7 @@ take the next free number; a fixed or accepted finding keeps its ID and changes | F2 | Medium | confirmed (code) | Windows installers give all local users modify rights on the program and data folders, `config.json` included | | F7 | Low | confirmed | Password change or reset does not end existing sessions, and makes the download key non-expiring | | F8 | Low | confirmed | Latest-token check skipped for any URL ending in `/auth` or `notify/hello` | -| F4 | Low | confirmed (code) | Any logged-in user can send an in-app notification to any user | +| F4 | Low | **fixed in cases 4712 and 4723** | Any logged-in user can send an in-app notification to any user | | F10 | Low | confirmed | Anyone can record acceptance of the licence agreement (`license/lc`) | | F11 | Low | confirmed | API root page shows licensee, version, licence terms (and DB ID when unlicensed) to anyone | | F12 | Low | confirmed | Password hashing: PBKDF2-SHA512 with 10,000 iterations | @@ -858,6 +858,8 @@ 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: confirmed from code (`NotifyController.SendNotifyDirectMessage`, no role check, any `ToUserId`). Fix: staff only, or only to users the caller may see. +**Fixed (F3, F4), 2026-10-24.** Customer contacts were already refused by the case 4712 route allowlist; case 4723 also refuses them in both actions. `direct-smtp`: an address taken from a Customer, HeadOffice, Vendor or User record needs read rights on that type (a contact's user record: User or Customer rights, as on the Contact form); the record checks run before the check that email delivery is on. Staff may still give any `ToAddress`, which John kept on purpose: the stock report template "z_API send SMTP from work order report" (`objectId 0`, `toAddress` from the work order) and at least one customer's report script rely on it. Every send is in the event log. Accepted remaining risk: a staff login can send email with any content to any address through the company's mail server. `direct-message`: refused to subcontractors too, as the client already hides it from them. Test `DirectSend` (refusals only, so no email is sent). + ## Cases John's cases for the findings (created 2026-10-24; full list in `NET10-PORT.md`, "Release 8.3.0 diff --git a/server/AyaNova/Controllers/NotifyController.cs b/server/AyaNova/Controllers/NotifyController.cs index f6c15593..bcfc8dcb 100644 --- a/server/AyaNova/Controllers/NotifyController.cs +++ b/server/AyaNova/Controllers/NotifyController.cs @@ -204,6 +204,10 @@ namespace AyaNova.Api.Controllers if (!ModelState.IsValid) return BadRequest(new ApiErrorResponse(ModelState)); + //case 4723: staff only, as in the client (no customer contacts or subcontractors) + var userType = UserTypeFromContext.Type(HttpContext.Items); + if (userType == UserType.Customer || userType == UserType.HeadOffice || userType == UserType.ServiceContractor) + return StatusCode(403, new ApiNotAuthorizedResponse()); foreach (long l in notifyDirectMessage.Users) { @@ -247,11 +251,13 @@ namespace AyaNova.Api.Controllers if (!ModelState.IsValid) return BadRequest(new ApiErrorResponse(ModelState)); - if (!ServerGlobalOpsSettingsCache.Notify.SmtpDeliveryActive) - return BadRequest(new ApiErrorResponse(ApiErrorCode.VALIDATION_MISSING_PROPERTY, null, "Email notifications are set to OFF at server, unable to send 'on request' type SMTP notification")); - - //for now I'm allowing any authenticated user to call this route intentionally as the use case is for our own internal messaging from the Customer form only or for API users who want to send - //email when rendering a report (case 4310). + //Staff users may send to any address: the use case is our own messaging from the Customer form etc and + //API users and report scripts that send email when rendering a report (case 4310). + //case 4723: customer contacts may not use this route at all; an address taken from a record + //needs read rights on that record + var userType = UserTypeFromContext.Type(HttpContext.Items); + if (userType == UserType.Customer || userType == UserType.HeadOffice) + return StatusCode(403, new ApiNotAuthorizedResponse()); //validate incoming data if (string.IsNullOrWhiteSpace(notifyDirectSMTP.Subject)) @@ -275,6 +281,8 @@ namespace AyaNova.Api.Controllers return BadRequest(new ApiErrorResponse(ApiErrorCode.VALIDATION_MISSING_PROPERTY, "ToAddress", "No address or object type and id specified no where to send this")); case AyaType.Customer: { + if (!Authorized.HasReadFullRole(HttpContext.Items, AyaType.Customer)) + return StatusCode(403, new ApiNotAuthorizedResponse()); var o = await ct.Customer.AsNoTracking().Where(x => x.Id == notifyDirectSMTP.ObjectId).Select(x => new { x.Name, x.EmailAddress, x.Active }).FirstOrDefaultAsync(); if (o == null) { @@ -289,6 +297,8 @@ namespace AyaNova.Api.Controllers break; case AyaType.HeadOffice: { + if (!Authorized.HasReadFullRole(HttpContext.Items, AyaType.HeadOffice)) + return StatusCode(403, new ApiNotAuthorizedResponse()); var o = await ct.HeadOffice.AsNoTracking().Where(x => x.Id == notifyDirectSMTP.ObjectId).Select(x => new { x.Name, x.EmailAddress, x.Active }).FirstOrDefaultAsync(); if (o == null) { @@ -303,6 +313,8 @@ namespace AyaNova.Api.Controllers break; case AyaType.Vendor: { + if (!Authorized.HasReadFullRole(HttpContext.Items, AyaType.Vendor)) + return StatusCode(403, new ApiNotAuthorizedResponse()); var o = await ct.Vendor.AsNoTracking().Where(x => x.Id == notifyDirectSMTP.ObjectId).Select(x => new { x.Name, x.EmailAddress, x.Active }).FirstOrDefaultAsync(); if (o == null) { @@ -317,11 +329,15 @@ namespace AyaNova.Api.Controllers break; case AyaType.User: { - var o = await ct.User.Include(z => z.UserOptions).AsNoTracking().Where(x => x.Id == notifyDirectSMTP.ObjectId).Select(x => new { x.Name, x.UserOptions.EmailAddress, x.Active }).FirstOrDefaultAsync(); + var o = await ct.User.Include(z => z.UserOptions).AsNoTracking().Where(x => x.Id == notifyDirectSMTP.ObjectId).Select(x => new { x.Name, x.UserOptions.EmailAddress, x.Active, x.UserType }).FirstOrDefaultAsync(); if (o == null) { return BadRequest(new ApiErrorResponse(ApiErrorCode.NOT_FOUND, "ObjectId")); } + //a customer contact's user record is read with Customer rights, as on the Contact form + bool isContact = o.UserType == UserType.Customer || o.UserType == UserType.HeadOffice; + if (!Authorized.HasReadFullRole(HttpContext.Items, AyaType.User) && !(isContact && Authorized.HasReadFullRole(HttpContext.Items, AyaType.Customer))) + return StatusCode(403, new ApiNotAuthorizedResponse()); if (string.IsNullOrWhiteSpace(o.EmailAddress)) return BadRequest(new ApiErrorResponse(ApiErrorCode.VALIDATION_MISSING_PROPERTY, "EmailAddress", $"Vendor {o.Name} doesn't have an email address no where to send this")); if (o.Active == false) @@ -335,6 +351,10 @@ namespace AyaNova.Api.Controllers } } + + if (!ServerGlobalOpsSettingsCache.Notify.SmtpDeliveryActive) + return BadRequest(new ApiErrorResponse(ApiErrorCode.VALIDATION_MISSING_PROPERTY, null, "Email notifications are set to OFF at server, unable to send 'on request' type SMTP notification")); + var UserId = UserIdFromContext.Id(HttpContext.Items); IMailer m = AyaNova.Util.ServiceProviderProvider.Mailer;