From ecfa59d0496a902ee5dcb9d67b281632c11e5f8a Mon Sep 17 00:00:00 2001 From: Oliver Kieselbach Date: Sat, 29 Aug 2026 23:57:35 +0200 Subject: [PATCH] fix(security): PUT preview/notification-email requires a member role once the tenant has members; roleless writes only in the signup window Co-Authored-By: Claude Fable 5 --- .../backend/tenant-activation-welcome-mail.md | 8 ++ docs/log.md | 4 + .../PreviewNotificationEmailWriteGateTests.cs | 102 ++++++++++++++++++ .../Rules/PreviewWhitelistFunction.cs | 39 ++++++- .../Security/EndpointAccessPolicyCatalog.cs | 6 +- 5 files changed, 156 insertions(+), 3 deletions(-) create mode 100644 src/Backend/AutopilotMonitor.Functions.Tests/PreviewNotificationEmailWriteGateTests.cs diff --git a/docs/backend/tenant-activation-welcome-mail.md b/docs/backend/tenant-activation-welcome-mail.md index b511ab86..f4e206f6 100644 --- a/docs/backend/tenant-activation-welcome-mail.md +++ b/docs/backend/tenant-activation-welcome-mail.md @@ -36,6 +36,14 @@ same table) — the only reliable address: it is typed by the user on the activa page (`PUT /api/preview/notification-email`). The signup admin UPN may have no mailbox and `ContactEmail` is merely seeded from the notification email. +The PUT is `AuthenticatedUserWithRole`, not a member tier, because the signup user is still +roleless until approval auto-promotes them to Admin. The function re-gates on the resolved +role (`PreviewWhitelistFunction.MayWriteNotificationEmailAsync`): a member role or platform +scope always may write; a roleless caller may write only while the tenant has **no enabled +member** at all. Once any member exists the window is closed for good — the address feeds the +welcome and farewell mails and is shown to operators as the tenant contact, so an ordinary +employee with a JWT but no product role must not be able to redirect or clear it (403). + That makes activation and address entry concurrent writers. With auto-approve, activation usually wins (observed in production: approve at T+64 s, address saved at T+69 s → the approval path found no address and the mail was silently skipped). diff --git a/docs/log.md b/docs/log.md index 6ac8bba9..d870a68a 100644 --- a/docs/log.md +++ b/docs/log.md @@ -1,5 +1,9 @@ # Log +## 2026-08-29 (12) + +* **Update**: `backend/tenant-activation-welcome-mail.md` — `PUT preview/notification-email` moved to `AuthenticatedUserWithRole`; roleless callers may write the address only while the tenant has no enabled member (signup window), afterwards a member role is required. + ## 2026-08-29 (11) * **Update**: `rules/gather-rule-phase-scoping.md` — `on_event` feedback-loop guard: `TimelineEventStream` publishes the event source and `GatherRuleExecutorHost` never dispatches `on_event` for gather-emitted events (a rule triggering on its own output type looped for the whole session); per-rule on_event execution cap (100/session); backend rejects `on_event` rules whose trigger equals their output type or `gather_result`. diff --git a/src/Backend/AutopilotMonitor.Functions.Tests/PreviewNotificationEmailWriteGateTests.cs b/src/Backend/AutopilotMonitor.Functions.Tests/PreviewNotificationEmailWriteGateTests.cs new file mode 100644 index 00000000..5e6d5fce --- /dev/null +++ b/src/Backend/AutopilotMonitor.Functions.Tests/PreviewNotificationEmailWriteGateTests.cs @@ -0,0 +1,102 @@ +using System.Collections.Generic; +using System.Threading.Tasks; +using AutopilotMonitor.Functions.Functions.Rules; +using AutopilotMonitor.Functions.Helpers; +using AutopilotMonitor.Functions.Security; +using AutopilotMonitor.Functions.Services; +using AutopilotMonitor.Shared; +using AutopilotMonitor.Shared.DataAccess; +using Microsoft.Extensions.Caching.Memory; +using Microsoft.Extensions.Logging; +using Moq; +using Xunit; + +namespace AutopilotMonitor.Functions.Tests; + +/// +/// PUT /api/preview/notification-email writes tenant-shared state that feeds the welcome and +/// farewell mails. The route must admit roleless callers (first-touch signup) but only while the +/// tenant has no member yet — afterwards a member role is required, so an ordinary employee with a +/// valid JWT and no product role can no longer redirect or suppress the tenant's correspondence. +/// +public class PreviewNotificationEmailWriteGateTests +{ + private const string TenantId = "11111111-2222-3333-4444-555555555555"; + + [Fact] + public void Route_is_AuthenticatedUserWithRole_so_the_function_can_see_the_resolved_role() + { + var entry = EndpointAccessPolicyCatalog.FindPolicy("PUT", "/api/preview/notification-email"); + + Assert.NotNull(entry); + Assert.Equal("preview/notification-email", entry!.RouteTemplate); + Assert.Equal(EndpointPolicy.AuthenticatedUserWithRole, entry.Policy); + } + + [Theory] + [InlineData(Constants.TenantRoles.Admin)] + [InlineData(Constants.TenantRoles.Operator)] + [InlineData(Constants.TenantRoles.Viewer)] + public async Task Member_role_may_write_even_when_the_tenant_has_members(string role) + { + var admins = AdminsWith(enabledMember: true); + var ctx = new RequestContext { UserRole = role }; + + Assert.True(await PreviewWhitelistFunction.MayWriteNotificationEmailAsync(ctx, TenantId, admins.Object)); + admins.Verify(a => a.GetTenantAdminsAsync(It.IsAny()), Times.Never); + } + + [Fact] + public async Task Global_admin_may_write() + { + var admins = AdminsWith(enabledMember: true); + var ctx = new RequestContext { IsGlobalAdmin = true }; + + Assert.True(await PreviewWhitelistFunction.MayWriteNotificationEmailAsync(ctx, TenantId, admins.Object)); + } + + [Fact] + public async Task Roleless_caller_may_write_while_the_tenant_has_no_member_yet() + { + var admins = AdminsWith(enabledMember: false); + var ctx = new RequestContext { UserRole = "Authenticated" }; + + Assert.True(await PreviewWhitelistFunction.MayWriteNotificationEmailAsync(ctx, TenantId, admins.Object)); + } + + [Fact] + public async Task Roleless_caller_is_rejected_once_the_tenant_has_an_enabled_member() + { + var admins = AdminsWith(enabledMember: true); + var ctx = new RequestContext { UserRole = "Authenticated" }; + + Assert.False(await PreviewWhitelistFunction.MayWriteNotificationEmailAsync(ctx, TenantId, admins.Object)); + } + + [Fact] + public async Task Disabled_members_do_not_close_the_signup_window() + { + var admins = new Mock( + Mock.Of(), Mock.Of(), Mock.Of>()) + { CallBase = false }; + admins.Setup(a => a.GetTenantAdminsAsync(TenantId)).ReturnsAsync(new List + { + new() { TenantId = TenantId, Upn = "former@contoso.com", IsEnabled = false, Role = Constants.TenantRoles.Admin }, + }); + var ctx = new RequestContext { UserRole = "Authenticated" }; + + Assert.True(await PreviewWhitelistFunction.MayWriteNotificationEmailAsync(ctx, TenantId, admins.Object)); + } + + private static Mock AdminsWith(bool enabledMember) + { + var admins = new Mock( + Mock.Of(), Mock.Of(), Mock.Of>()) + { CallBase = false }; + var members = new List(); + if (enabledMember) + members.Add(new TenantAdminEntity { TenantId = TenantId, Upn = "admin@contoso.com", IsEnabled = true, Role = Constants.TenantRoles.Admin }); + admins.Setup(a => a.GetTenantAdminsAsync(TenantId)).ReturnsAsync(members); + return admins; + } +} diff --git a/src/Backend/AutopilotMonitor.Functions/Functions/Rules/PreviewWhitelistFunction.cs b/src/Backend/AutopilotMonitor.Functions/Functions/Rules/PreviewWhitelistFunction.cs index 7f0fcaee..ec014bf6 100644 --- a/src/Backend/AutopilotMonitor.Functions/Functions/Rules/PreviewWhitelistFunction.cs +++ b/src/Backend/AutopilotMonitor.Functions/Functions/Rules/PreviewWhitelistFunction.cs @@ -1,6 +1,7 @@ using System.Net; using AutopilotMonitor.Functions.Extensions; using AutopilotMonitor.Functions.DataAccess.TableStorage; +using AutopilotMonitor.Functions.Helpers; using AutopilotMonitor.Functions.Services; using Microsoft.AspNetCore.Authorization; using Microsoft.Azure.Functions.Worker; @@ -11,7 +12,8 @@ namespace AutopilotMonitor.Functions.Functions.Rules; /// /// CRUD endpoints for managing the tenant-activation whitelist (table: PreviewWhitelist). -/// All endpoints are Global Admin only (except notification-email which is AuthenticatedUser). +/// All endpoints are Global Admin only, except PUT notification-email: that one is +/// AuthenticatedUserWithRole and gated in-function (member role, or the tenant has no members yet). /// public class PreviewWhitelistFunction { @@ -19,6 +21,7 @@ public class PreviewWhitelistFunction private readonly PreviewWhitelistService _previewWhitelistService; private readonly TenantApprovalService _tenantApprovalService; private readonly TenantConfigurationService _tenantConfigurationService; + private readonly TenantAdminsService _tenantAdminsService; private readonly IEmailService _emailService; public PreviewWhitelistFunction( @@ -26,12 +29,14 @@ public class PreviewWhitelistFunction PreviewWhitelistService previewWhitelistService, TenantApprovalService tenantApprovalService, TenantConfigurationService tenantConfigurationService, + TenantAdminsService tenantAdminsService, IEmailService emailService) { _logger = logger; _previewWhitelistService = previewWhitelistService; _tenantApprovalService = tenantApprovalService; _tenantConfigurationService = tenantConfigurationService; + _tenantAdminsService = tenantAdminsService; _emailService = emailService; } @@ -156,7 +161,8 @@ public class PreviewWhitelistFunction /// /// PUT /api/preview/notification-email /// Saves the caller's notification email for the activation notice. - /// AuthenticatedUser policy — preview-blocked users can call this. + /// AuthenticatedUserWithRole policy — preview-blocked users can call this, but only while the + /// tenant has no members (see ). /// [Function("SavePreviewNotificationEmail")] [Authorize] @@ -174,6 +180,16 @@ public class PreviewWhitelistFunction return bad; } + if (!await MayWriteNotificationEmailAsync(req.GetRequestContext(), tenantId, _tenantAdminsService)) + { + _logger.LogWarning( + "Preview notification email write denied for roleless caller {Upn} — tenant {TenantId} already has members", + principal?.GetUserPrincipalName() ?? "unknown", tenantId); + var forbidden = req.CreateResponse(HttpStatusCode.Forbidden); + await forbidden.WriteAsJsonAsync(new { error = "A tenant member role is required to change the notification email" }); + return forbidden; + } + var body = await req.ReadFromJsonAsync(); var email = body?.Email?.Trim(); @@ -208,6 +224,25 @@ public class PreviewWhitelistFunction return response; } + /// + /// The notification email is tenant-shared state consumed by privileged flows (welcome mail, + /// offboarding farewell, operator contact). A member role (or platform scope) always may write + /// it. A roleless caller may write it ONLY while the tenant has no enabled member at all — the + /// first-touch signup window, where the activating user is still roleless because the requester + /// is auto-promoted to Admin only on approval. Once any member exists, that window is closed for + /// good: an ordinary employee with a valid JWT but no product role must not be able to redirect + /// or suppress the tenant's correspondence. + /// + internal static async Task MayWriteNotificationEmailAsync( + RequestContext ctx, string tenantId, TenantAdminsService tenantAdminsService) + { + if (ctx.IsTenantMemberOrGlobalAdmin()) + return true; + + var members = await tenantAdminsService.GetTenantAdminsAsync(tenantId); + return !members.Any(m => m.IsEnabled); + } + /// /// POST /api/preview/send-welcome-email/{tenantId} /// Sends (or resends) the activation welcome email. Global Admin only. diff --git a/src/Backend/AutopilotMonitor.Functions/Security/EndpointAccessPolicyCatalog.cs b/src/Backend/AutopilotMonitor.Functions/Security/EndpointAccessPolicyCatalog.cs index 26aaaba5..7fe3d311 100644 --- a/src/Backend/AutopilotMonitor.Functions/Security/EndpointAccessPolicyCatalog.cs +++ b/src/Backend/AutopilotMonitor.Functions/Security/EndpointAccessPolicyCatalog.cs @@ -223,7 +223,11 @@ public static class EndpointAccessPolicyCatalog // ProgressPortalFunction; regression history c4dabeee). new("GET", "progress/sessions/lookup", EndpointPolicy.AuthenticatedUserWithRole, TenantScoping.QueryParam), new("GET", "progress/sessions/{sessionId}/events", EndpointPolicy.AuthenticatedUser, TenantScoping.QueryParam), - new("PUT", "preview/notification-email", EndpointPolicy.AuthenticatedUser), + // AuthenticatedUserWithRole (not plain AuthenticatedUser): the activating user is still + // roleless during first-touch signup, so the tier must admit non-members — but the address is + // tenant-shared state feeding the welcome/farewell mails, so the function re-gates on the + // resolved role: roleless callers may only write while the tenant has no member yet. + new("PUT", "preview/notification-email", EndpointPolicy.AuthenticatedUserWithRole), new("GET", "feedback/status", EndpointPolicy.AuthenticatedUser), new("POST", "feedback", EndpointPolicy.AuthenticatedUser), new("GET", "config/latest-versions", EndpointPolicy.AuthenticatedUser),