diff --git a/.claude/commands b/.claude/commands index c1614df0..41038ca4 160000 --- a/.claude/commands +++ b/.claude/commands @@ -1 +1 @@ -Subproject commit c1614df047296bd5a0866aa9e3b2b602f723e51f +Subproject commit 41038ca4aaea744b544b4e535a67ae50f046b874 diff --git a/internal b/internal index c46d7280..b05e4332 160000 --- a/internal +++ b/internal @@ -1 +1 @@ -Subproject commit c46d728015c3f1c09921b4bef388f58ea076fce5 +Subproject commit b05e43328d643ae908bfb8e2b189954315130188 diff --git a/src/Backend/AutopilotMonitor.Functions.Tests/EmailServiceTests.cs b/src/Backend/AutopilotMonitor.Functions.Tests/EmailServiceTests.cs index 6914a8dc..3d473786 100644 --- a/src/Backend/AutopilotMonitor.Functions.Tests/EmailServiceTests.cs +++ b/src/Backend/AutopilotMonitor.Functions.Tests/EmailServiceTests.cs @@ -32,24 +32,28 @@ public sealed class EmailServiceTests // ----- configuration gates ----- [Fact] - public async Task SendAsync_EmptyApiKey_NoOps_AndLogsMissingKeyDebugLine() + public async Task SendAsync_EmptyApiKey_NoOps_ReturnsFalse_AndLogsMissingKeyWarning() { var (sut, handler, logger) = Build(apiKey: ""); - await sut.SendAsync("ops@contoso.invalid", "contoso.invalid", TenantId, CancellationToken.None); + var sent = await sut.SendAsync("ops@contoso.invalid", "contoso.invalid", TenantId, CancellationToken.None); + Assert.False(sent); Assert.Equal(0, handler.CallCount); + // Warning, not Debug: same reasoning as the welcome mail — worker logs below Warning never + // reach Application Insights, and the handler records FarewellEmailFailed on the false. Assert.Contains(logger.Entries, - e => e.Level == LogLevel.Debug && e.Message.Contains("Email:ApiKey not configured")); + e => e.Level == LogLevel.Warning && e.Message.Contains("Email:ApiKey not configured")); } [Fact] - public async Task SendAsync_EmptyRecipient_NoOps_AndLogsMissingEmailDebugLine() + public async Task SendAsync_EmptyRecipient_NoOps_ReturnsFalse_AndLogsMissingEmailDebugLine() { var (sut, handler, logger) = Build(apiKey: "any-non-empty-key"); - await sut.SendAsync("", "contoso.invalid", TenantId); + var sent = await sut.SendAsync("", "contoso.invalid", TenantId); + Assert.False(sent); Assert.Equal(0, handler.CallCount); Assert.Contains(logger.Entries, e => e.Level == LogLevel.Debug && e.Message.Contains("No notification email captured")); @@ -65,7 +69,7 @@ public sealed class EmailServiceTests await sut.SendAsync("", "contoso.invalid", TenantId); Assert.Contains(logger.Entries, - e => e.Level == LogLevel.Debug && e.Message.Contains("Email:ApiKey not configured")); + e => e.Level == LogLevel.Warning && e.Message.Contains("Email:ApiKey not configured")); Assert.DoesNotContain(logger.Entries, e => e.Message.Contains("No notification email captured")); } @@ -123,8 +127,9 @@ public sealed class EmailServiceTests { var (sut, handler, logger) = Build(apiKey: "md-test-key"); - await sut.SendAsync("ops@contoso.invalid", "contoso.invalid", TenantId); + var sent = await sut.SendAsync("ops@contoso.invalid", "contoso.invalid", TenantId); + Assert.True(sent); using var doc = JsonDocument.Parse(handler.LastBody!); var message = doc.RootElement.GetProperty("message"); Assert.Equal(EmailTemplates.OffboardingFarewellSubject, message.GetProperty("subject").GetString()); @@ -161,8 +166,9 @@ public sealed class EmailServiceTests responder: _ => Json(HttpStatusCode.InternalServerError, "{\"status\":\"error\",\"code\":-1,\"name\":\"Invalid_Key\",\"message\":\"Invalid API key\"}")); - await sut.SendAsync("ops@contoso.invalid", "contoso.invalid", TenantId); + var sent = await sut.SendAsync("ops@contoso.invalid", "contoso.invalid", TenantId); + Assert.False(sent); Assert.Contains(logger.Entries, e => e.Level == LogLevel.Warning && e.Message.Contains("500") && e.Message.Contains("Invalid_Key")); Assert.DoesNotContain(logger.Entries, e => e.Level == LogLevel.Information); @@ -175,8 +181,9 @@ public sealed class EmailServiceTests responder: _ => Json(HttpStatusCode.OK, "[{\"email\":\"ops@contoso.invalid\",\"status\":\"rejected\",\"reject_reason\":\"hard-bounce\",\"_id\":\"abc\"}]")); - await sut.SendAsync("ops@contoso.invalid", "contoso.invalid", TenantId); + var sent = await sut.SendAsync("ops@contoso.invalid", "contoso.invalid", TenantId); + Assert.False(sent); Assert.Contains(logger.Entries, e => e.Level == LogLevel.Warning && e.Message.Contains("rejected") && e.Message.Contains("hard-bounce")); Assert.DoesNotContain(logger.Entries, e => e.Level == LogLevel.Information); @@ -192,8 +199,9 @@ public sealed class EmailServiceTests responder: _ => Json(HttpStatusCode.OK, $"[{{\"email\":\"ops@contoso.invalid\",\"status\":\"{status}\",\"_id\":\"abc\"}}]")); - await sut.SendAsync("ops@contoso.invalid", "contoso.invalid", TenantId); + var sent = await sut.SendAsync("ops@contoso.invalid", "contoso.invalid", TenantId); + Assert.True(sent); Assert.Contains(logger.Entries, e => e.Level == LogLevel.Information); Assert.DoesNotContain(logger.Entries, e => e.Level == LogLevel.Warning); } @@ -204,8 +212,9 @@ public sealed class EmailServiceTests var (sut, _, logger) = Build(apiKey: "md-test-key", responder: _ => Json(HttpStatusCode.OK, "not json")); - await sut.SendAsync("ops@contoso.invalid", "contoso.invalid", TenantId); + var sent = await sut.SendAsync("ops@contoso.invalid", "contoso.invalid", TenantId); + Assert.False(sent); Assert.Contains(logger.Entries, e => e.Level == LogLevel.Warning); Assert.DoesNotContain(logger.Entries, e => e.Level == LogLevel.Information); } diff --git a/src/Backend/AutopilotMonitor.Functions.Tests/EndpointPolicyCatalogCompletenessTests.cs b/src/Backend/AutopilotMonitor.Functions.Tests/EndpointPolicyCatalogCompletenessTests.cs index 8f90f7ae..71ddbd32 100644 --- a/src/Backend/AutopilotMonitor.Functions.Tests/EndpointPolicyCatalogCompletenessTests.cs +++ b/src/Backend/AutopilotMonitor.Functions.Tests/EndpointPolicyCatalogCompletenessTests.cs @@ -237,6 +237,22 @@ public class EndpointPolicyCatalogCompletenessTests string.Join("\n", missing.Select(m => $" - {m}"))); } + /// + /// AllowedDuringOffboarding punches a hole in the tenant suspension gate (offboarding tombstone only). + /// It exists for exactly one route — the farewell feedback a departing admin submits after the offboard + /// endpoint has already tombstoned their tenant. Any second route here needs the same justification. + /// + [Fact] + public void AllowedDuringOffboarding_IsOnlyTheFarewellFeedbackRoute() + { + var routes = EndpointAccessPolicyCatalog.Entries + .Where(e => e.AllowedDuringOffboarding) + .Select(e => $"{e.HttpMethod} {e.RouteTemplate}") + .ToList(); + + Assert.Equal(new[] { "POST tenants/{tenantId}/offboard/feedback" }, routes); + } + /// /// Every entry with TenantScoping.RouteParam must have {tenantId} in its route template. /// Prevents misattributed scoping declarations. diff --git a/src/Backend/AutopilotMonitor.Functions.Tests/Offboarding/FakeOffboardFarewellEmailSender.cs b/src/Backend/AutopilotMonitor.Functions.Tests/Offboarding/FakeOffboardFarewellEmailSender.cs index d9fb3534..99318b21 100644 --- a/src/Backend/AutopilotMonitor.Functions.Tests/Offboarding/FakeOffboardFarewellEmailSender.cs +++ b/src/Backend/AutopilotMonitor.Functions.Tests/Offboarding/FakeOffboardFarewellEmailSender.cs @@ -7,18 +7,20 @@ namespace AutopilotMonitor.Functions.Tests.Offboarding; /// /// Test double for . Records every invocation so /// the handler-side tests can assert "did the farewell send fire after History → Completed?" -/// without booting the real mail provider client. Default = silent noop; set -/// to simulate downstream failures and prove the handler's fail-soft contract. +/// without booting the real mail provider client. Default = accepted ( true); +/// set false to simulate a refused send, or to +/// simulate downstream failures and prove the handler's fail-soft contract. /// internal sealed class FakeOffboardFarewellEmailSender : IOffboardFarewellEmailSender { public readonly List<(string ToEmail, string DomainName, string TenantId)> Calls = new(); + public bool Result { get; set; } = true; public System.Exception? ThrowOnSend { get; set; } - public Task SendAsync(string toEmail, string domainName, string tenantId, CancellationToken ct = default) + public Task SendAsync(string toEmail, string domainName, string tenantId, CancellationToken ct = default) { Calls.Add((toEmail, domainName, tenantId)); if (ThrowOnSend is { } ex) throw ex; - return Task.CompletedTask; + return Task.FromResult(Result); } } diff --git a/src/Backend/AutopilotMonitor.Functions.Tests/Offboarding/TenantOffboardingHandlerDrainContractTests.cs b/src/Backend/AutopilotMonitor.Functions.Tests/Offboarding/TenantOffboardingHandlerDrainContractTests.cs index 6ba54770..c60b912a 100644 --- a/src/Backend/AutopilotMonitor.Functions.Tests/Offboarding/TenantOffboardingHandlerDrainContractTests.cs +++ b/src/Backend/AutopilotMonitor.Functions.Tests/Offboarding/TenantOffboardingHandlerDrainContractTests.cs @@ -383,6 +383,77 @@ public class TenantOffboardingHandlerDrainContractTests Times.Never(), "Drain-skip-gate must also bypass the enumerate+enqueue phase"); } + // ── Farewell mail record (Side-effect 6) ──────────────────────────────────── + // The send fails soft and its success log never reaches Application Insights, so the ops + // event is the only record of whether the departing customer got the farewell mail. + + [Fact] + public async Task Completed_WithCapturedAddress_ProviderAccepts_RecordsFarewellEmailSent() + { + var harness = Harness.New(); + harness.Repo.History[HistoryRowKey].NotificationEmail = "it@contoso.invalid"; + + await harness.Sut.HandleAsync(harness.Envelope(drainPollCount: 0)); + + Assert.Equal("Completed", harness.Repo.History[HistoryRowKey].Status); + var call = Assert.Single(harness.Farewell.Calls); + Assert.Equal(("it@contoso.invalid", "contoso.invalid", TenantId), call); + var evt = Assert.Single(harness.OpsEvents, e => e.EventType.StartsWith("FarewellEmail", StringComparison.Ordinal)); + Assert.Equal(OpsEventTypes.FarewellEmailSent, evt.EventType); + Assert.Equal(OpsEventSeverity.Info, evt.Severity); + Assert.Equal(TenantId, evt.TenantId); + Assert.Contains("it@contoso.invalid", evt.Message); + } + + [Fact] + public async Task Completed_WithoutCapturedAddress_RecordsFarewellEmailSkipped_AndNeverCallsSender() + { + var harness = Harness.New(); + harness.Repo.History[HistoryRowKey].NotificationEmail = null; + + await harness.Sut.HandleAsync(harness.Envelope(drainPollCount: 0)); + + Assert.Equal("Completed", harness.Repo.History[HistoryRowKey].Status); + Assert.Empty(harness.Farewell.Calls); + var evt = Assert.Single(harness.OpsEvents, e => e.EventType.StartsWith("FarewellEmail", StringComparison.Ordinal)); + Assert.Equal(OpsEventTypes.FarewellEmailSkipped, evt.EventType); + Assert.Equal(OpsEventSeverity.Warning, evt.Severity); + Assert.Contains("no contact address", evt.Message); + } + + [Fact] + public async Task Completed_ProviderRefuses_RecordsFarewellEmailFailed() + { + var harness = Harness.New(); + harness.Repo.History[HistoryRowKey].NotificationEmail = "it@contoso.invalid"; + harness.Farewell.Result = false; + + await harness.Sut.HandleAsync(harness.Envelope(drainPollCount: 0)); + + var evt = Assert.Single(harness.OpsEvents, e => e.EventType.StartsWith("FarewellEmail", StringComparison.Ordinal)); + Assert.Equal(OpsEventTypes.FarewellEmailFailed, evt.EventType); + Assert.Equal(OpsEventSeverity.Error, evt.Severity); + Assert.Contains("did not accept", evt.Message); + } + + [Fact] + public async Task Completed_SenderThrows_RecordsFarewellEmailFailed_AndStillCompletes() + { + // Fail-soft contract: the History row is already Completed when the mail goes out; a + // throwing sender must neither propagate nor stop the TenantConfiguration delete after it. + var harness = Harness.New(); + harness.Repo.History[HistoryRowKey].NotificationEmail = "it@contoso.invalid"; + harness.Farewell.ThrowOnSend = new HttpRequestException("simulated provider outage"); + + await harness.Sut.HandleAsync(harness.Envelope(drainPollCount: 0)); + + Assert.Equal("Completed", harness.Repo.History[HistoryRowKey].Status); + Assert.Equal("Completed", harness.Repo.Markers[TenantId].Status); + var evt = Assert.Single(harness.OpsEvents, e => e.EventType.StartsWith("FarewellEmail", StringComparison.Ordinal)); + Assert.Equal(OpsEventTypes.FarewellEmailFailed, evt.EventType); + Assert.Contains("HttpRequestException", evt.Message); + } + [Fact] public async Task History_AlreadyCompleted_IsNoOp() { @@ -436,6 +507,9 @@ public class TenantOffboardingHandlerDrainContractTests public CountingSafeWipeService SafeWipeProbe { get; } = new(); public Mock Maintenance { get; } = new(); public Mock ConfigRepo { get; } = new(); + public FakeOffboardFarewellEmailSender Farewell { get; } = new(); + /// Every ops event the handler recorded, in order (the repository is a capturing mock). + public List OpsEvents { get; } = new(); public List EnumeratorYields { get; } = new(); public Exception? EnumeratorThrow { get; set; } @@ -475,7 +549,7 @@ public class TenantOffboardingHandlerDrainContractTests It.IsAny(), It.IsAny(), It.IsAny?>())) .ReturnsAsync(true); - var opsService = BuildRealOpsService(); + var opsService = BuildRealOpsService(h.OpsEvents); var unusedTableStorage = new Mock( Mock.Of(), @@ -494,7 +568,7 @@ public class TenantOffboardingHandlerDrainContractTests opsService, Mock.Of(), h.ConfigRepo.Object, - new FakeOffboardFarewellEmailSender(), + h.Farewell, NullLogger.Instance); return h; @@ -578,10 +652,12 @@ public class TenantOffboardingHandlerDrainContractTests }, "\"0xFAKE_PTR_1\""); } - private static OpsEventService BuildRealOpsService() + private static OpsEventService BuildRealOpsService(List captured) { var opsRepo = new Mock(); - opsRepo.Setup(r => r.SaveOpsEventAsync(It.IsAny())).Returns(Task.CompletedTask); + opsRepo.Setup(r => r.SaveOpsEventAsync(It.IsAny())) + .Callback(captured.Add) + .Returns(Task.CompletedTask); var memCache = new MemoryCache(new MemoryCacheOptions()); var adminConfig = new Mock( Mock.Of(), NullLogger.Instance, memCache); diff --git a/src/Backend/AutopilotMonitor.Functions.Tests/PolicyEnforcementMiddlewareTests.cs b/src/Backend/AutopilotMonitor.Functions.Tests/PolicyEnforcementMiddlewareTests.cs index 5fea20c3..3ceb9547 100644 --- a/src/Backend/AutopilotMonitor.Functions.Tests/PolicyEnforcementMiddlewareTests.cs +++ b/src/Backend/AutopilotMonitor.Functions.Tests/PolicyEnforcementMiddlewareTests.cs @@ -1,4 +1,5 @@ using System.Security.Claims; +using AutopilotMonitor.Functions.Functions.Admin; using AutopilotMonitor.Functions.Functions.Config; using AutopilotMonitor.Functions.Middleware; using AutopilotMonitor.Functions.Security; @@ -1623,6 +1624,82 @@ public class PolicyEnforcementMiddlewareTests h.ConfigRepo.Verify(r => r.GetTenantConfigurationAsync(It.IsAny()), Times.Never); } + // ── Offboarding tombstone vs. the farewell feedback route (AllowedDuringOffboarding) ────── + // The offboard endpoint tombstones the tenant (Disabled=true, offboarding reason) BEFORE it answers + // 202, so the departing admin submits the farewell feedback from an already-suspended tenant. That + // one route opts out of the tombstone gate; every other route and every other suspension stay closed. + + private static void AsOffboardingTombstone(Harness h, string tenantId) => + AsSuspended(h, tenantId, reason: TenantOffboardFunction.OffboardingDisabledReason); + + [Fact] + public async Task OffboardingTombstone_FeedbackRoute_IsAllowed_ForDepartingTenantAdmin() + { + const string upn = "admin@contoso.com"; + var h = BuildHarness(); + h.AsTenantAdmin(TenantA, upn); + AsOffboardingTombstone(h, TenantA); + + var result = await h.Middleware.DecideAsync( + "POST", $"/api/tenants/{TenantA}/offboard/feedback", null, AuthedPrincipal(TenantA, upn)); + + Assert.True(result.Allowed); + Assert.Equal(TenantA, result.Context!.TargetTenantId); + } + + [Fact] + public async Task OffboardingTombstone_OtherRoutes_StayForbidden() + { + // The opt-out is per route: the same tombstone still closes the rest of the API. + const string upn = "admin@contoso.com"; + var h = BuildHarness(); + h.AsTenantAdmin(TenantA, upn); + AsOffboardingTombstone(h, TenantA); + + var read = await h.Middleware.DecideAsync("GET", "/api/sessions", null, AuthedPrincipal(TenantA, upn)); + var write = await h.Middleware.DecideAsync("DELETE", $"/api/tenants/{TenantA}/offboard", null, AuthedPrincipal(TenantA, upn)); + + Assert.False(read.Allowed); + Assert.Equal("TenantSuspended", read.ErrorCode); + Assert.Equal(TenantOffboardFunction.OffboardingDisabledReason, read.ErrorMessage); + Assert.False(write.Allowed); + Assert.Equal("TenantSuspended", write.ErrorCode); + } + + [Fact] + public async Task OperatorSuspension_FeedbackRoute_StaysForbidden() + { + // The opt-out is for the offboarding tombstone only: a tenant an operator suspended (any other + // reason) cannot reach the feedback route either — nothing in the flag grants access. + const string upn = "admin@contoso.com"; + var h = BuildHarness(); + h.AsTenantAdmin(TenantA, upn); + AsSuspended(h, TenantA, reason: "Abuse investigation"); + + var result = await h.Middleware.DecideAsync( + "POST", $"/api/tenants/{TenantA}/offboard/feedback", null, AuthedPrincipal(TenantA, upn)); + + Assert.False(result.Allowed); + Assert.Equal("TenantSuspended", result.ErrorCode); + } + + [Fact] + public async Task OffboardingTombstone_FeedbackRoute_NonMember_StillNeedsTenantAdmin() + { + // Policy tier is untouched by the flag: a plain member (no tenant admin role) is denied by the + // TenantAdminOrGA tier before the suspension gate is even reached. + const string upn = "viewer@contoso.com"; + var h = BuildHarness(); + AsOffboardingTombstone(h, TenantA); + + var result = await h.Middleware.DecideAsync( + "POST", $"/api/tenants/{TenantA}/offboard/feedback", null, AuthedPrincipal(TenantA, upn)); + + Assert.False(result.Allowed); + Assert.Equal(403, result.StatusCode); + Assert.NotEqual("TenantSuspended", result.ErrorCode); + } + // ── Tenant MCP switch (TenantConfiguration.McpDisabled) — the per-request half ──────────── // The MCP front door (McpUserService) already denies a caller whose home tenant is closed; this gate // is what makes the switch hold per request (front-door verdicts are cached) and what keeps a closed diff --git a/src/Backend/AutopilotMonitor.Functions/Middleware/PolicyEnforcementMiddleware.cs b/src/Backend/AutopilotMonitor.Functions/Middleware/PolicyEnforcementMiddleware.cs index 626011f8..101c0134 100644 --- a/src/Backend/AutopilotMonitor.Functions/Middleware/PolicyEnforcementMiddleware.cs +++ b/src/Backend/AutopilotMonitor.Functions/Middleware/PolicyEnforcementMiddleware.cs @@ -1,6 +1,7 @@ using System.Net; using System.Security.Claims; using AutopilotMonitor.Functions.Extensions; +using AutopilotMonitor.Functions.Functions.Admin; using AutopilotMonitor.Functions.Helpers; using AutopilotMonitor.Functions.Security; using AutopilotMonitor.Functions.Services; @@ -303,10 +304,13 @@ public class PolicyEnforcementMiddleware : IFunctionsWorkerMiddleware // and nobody without platform scope reaches a suspended target. Platform operators (GA/Reader) bypass — // they must still administer the suspended tenant. Anonymous/device routes bring their own gate // (SecurityValidator → 403 TenantDisabled). Cached, side-effect-free read; a missing row is not disabled. + // An AllowedDuringOffboarding route ignores the offboarding tombstone alone (the departing admin's + // farewell feedback arrives after their own tenant was tombstoned); any other suspension still holds. if (principal != null && !hasGlobalScope) { var gateConfigs = await LoadGateConfigsAsync(jwtTenantId, targetTenantId); - var suspended = gateConfigs.FirstOrDefault(c => c.IsCurrentlyDisabled()); + var suspended = gateConfigs.FirstOrDefault(c => c.IsCurrentlyDisabled() + && !(catalogEntry.AllowedDuringOffboarding && TenantOffboardFunction.IsOffboardingTombstone(c))); if (suspended != null) { _logger.LogWarning("[PolicyEnforcement] BLOCKED suspended tenant: user={User} tenant={Tenant} path={Path}", diff --git a/src/Backend/AutopilotMonitor.Functions/Security/EndpointAccessPolicyCatalog.cs b/src/Backend/AutopilotMonitor.Functions/Security/EndpointAccessPolicyCatalog.cs index e96bbcbf..e4ddf6af 100644 --- a/src/Backend/AutopilotMonitor.Functions/Security/EndpointAccessPolicyCatalog.cs +++ b/src/Backend/AutopilotMonitor.Functions/Security/EndpointAccessPolicyCatalog.cs @@ -134,11 +134,24 @@ public sealed class EndpointPolicyEntry /// public bool ApplicationAllowed { get; } + /// + /// When true, the tenant suspension gate lets the caller through while the ONLY reason the tenant + /// is suspended is the offboarding tombstone (Disabled=true with the offboarding reason, + /// ). An operator + /// suspension (any other reason, or a DisabledUntil window) still denies. Exists for the one route + /// a departing admin must reach AFTER their tenant has been tombstoned: the farewell feedback + /// (POST tenants/{tenantId}/offboard/feedback), which the offboard endpoint tombstones + /// before it even answers 202. Never grants anything else — policy tier, cross-tenant check and + /// the function's own active-marker check stay in force. + /// + public bool AllowedDuringOffboarding { get; } + // Pre-compiled regex for matching actual request paths against the route template internal Regex RouteRegex { get; } public EndpointPolicyEntry(string httpMethod, string routeTemplate, EndpointPolicy policy, - TenantScoping tenantScoping = TenantScoping.None, bool excludeDelegated = false, bool applicationAllowed = false) + TenantScoping tenantScoping = TenantScoping.None, bool excludeDelegated = false, bool applicationAllowed = false, + bool allowedDuringOffboarding = false) { HttpMethod = httpMethod.ToUpperInvariant(); RouteTemplate = routeTemplate; @@ -146,6 +159,7 @@ public sealed class EndpointPolicyEntry TenantScoping = tenantScoping; ExcludeDelegated = excludeDelegated; ApplicationAllowed = applicationAllowed; + AllowedDuringOffboarding = allowedDuringOffboarding; RouteRegex = BuildRouteRegex(routeTemplate); } @@ -367,7 +381,9 @@ public static class EndpointAccessPolicyCatalog new("PATCH", "tenants/{tenantId}/admins/{adminUpn}/enable", EndpointPolicy.TenantAdminOrGA, TenantScoping.RouteParam), new("PATCH", "tenants/{tenantId}/admins/{adminUpn}/permissions", EndpointPolicy.TenantAdminOrGA, TenantScoping.RouteParam), new("DELETE", "tenants/{tenantId}/offboard", EndpointPolicy.TenantAdminOrGA, TenantScoping.RouteParam), - new("POST", "tenants/{tenantId}/offboard/feedback", EndpointPolicy.TenantAdminOrGA, TenantScoping.RouteParam), + // Farewell feedback is submitted AFTER the offboard endpoint has tombstoned the tenant (Disabled=true + // before the 202), so the suspension gate must let the departing admin through — for this route only. + new("POST", "tenants/{tenantId}/offboard/feedback", EndpointPolicy.TenantAdminOrGA, TenantScoping.RouteParam, allowedDuringOffboarding: true), new("GET", "config/{tenantId}/autopilot-device-validation/consent-url", EndpointPolicy.TenantAdminOrGA, TenantScoping.RouteParam), new("GET", "config/{tenantId}/autopilot-device-validation/consent-status", EndpointPolicy.TenantAdminOrGA, TenantScoping.RouteParam), new("POST", "config/{tenantId}/autopilot-device-validation/consent-failure", EndpointPolicy.TenantAdminOrGA, TenantScoping.RouteParam), diff --git a/src/Backend/AutopilotMonitor.Functions/Services/EmailService.cs b/src/Backend/AutopilotMonitor.Functions/Services/EmailService.cs index 941df972..8392cd64 100644 --- a/src/Backend/AutopilotMonitor.Functions/Services/EmailService.cs +++ b/src/Backend/AutopilotMonitor.Functions/Services/EmailService.cs @@ -94,18 +94,21 @@ public class EmailService : IEmailService, IOffboardFarewellEmailSender /// /// Sends the post-offboarding "sorry to see you go" farewell email. - /// No-op if the API key or recipient email is not configured. Best-effort: failures - /// are logged as warnings and never propagated (the offboarding correctness contract - /// does not depend on email delivery). + /// No-op (returns false) if the API key or recipient email is not configured. Best-effort: + /// failures are logged as warnings and never propagated (the offboarding correctness + /// contract does not depend on email delivery); the caller turns the result into the + /// FarewellEmailSent / FarewellEmailFailed ops events. /// - public async Task SendAsync(string toEmail, string domainName, string tenantId, CancellationToken ct = default) + public async Task SendAsync(string toEmail, string domainName, string tenantId, CancellationToken ct = default) { if (string.IsNullOrWhiteSpace(_apiKey)) { - _logger.LogDebug( + // Warning, not Debug: an unconfigured provider is operator misconfiguration, and worker + // logs below Warning never reach Application Insights. + _logger.LogWarning( "{ConfigKey} not configured — skipping offboard farewell email for tenant {TenantId}", ApiKeyConfigKey, tenantId); - return; + return false; } if (string.IsNullOrWhiteSpace(toEmail)) @@ -113,7 +116,7 @@ public class EmailService : IEmailService, IOffboardFarewellEmailSender _logger.LogDebug( "No notification email captured — skipping offboard farewell email for tenant {TenantId} ({Domain})", tenantId, domainName); - return; + return false; } var sent = await SendViaMandrillAsync( @@ -129,6 +132,8 @@ public class EmailService : IEmailService, IOffboardFarewellEmailSender "Offboard farewell email sent to {ToEmail} for tenant {TenantId} ({Domain})", toEmail, tenantId, domainName); } + + return sent; } /// diff --git a/src/Backend/AutopilotMonitor.Functions/Services/IOffboardFarewellEmailSender.cs b/src/Backend/AutopilotMonitor.Functions/Services/IOffboardFarewellEmailSender.cs index 0180a7c0..951138ab 100644 --- a/src/Backend/AutopilotMonitor.Functions/Services/IOffboardFarewellEmailSender.cs +++ b/src/Backend/AutopilotMonitor.Functions/Services/IOffboardFarewellEmailSender.cs @@ -7,9 +7,13 @@ namespace AutopilotMonitor.Functions.Services; /// Invocation point: TenantOffboardingHandler.RunPostDrainPhasesAsync immediately /// after the History terminal write (Side-effect 6). Always fail-soft — implementations /// must NOT throw; the offboarding correctness contract does not depend on email delivery. +/// The return value is what separates "sent" from "provider refused / not configured" for the +/// FarewellEmailSent / FarewellEmailFailed ops events the handler records. /// /// public interface IOffboardFarewellEmailSender { - Task SendAsync(string toEmail, string domainName, string tenantId, CancellationToken ct = default); + /// True when the provider accepted the message; false when the send was skipped + /// (provider not configured, empty recipient) or the provider refused it. + Task SendAsync(string toEmail, string domainName, string tenantId, CancellationToken ct = default); } diff --git a/src/Backend/AutopilotMonitor.Functions/Services/Offboarding/TenantOffboardingHandler.cs b/src/Backend/AutopilotMonitor.Functions/Services/Offboarding/TenantOffboardingHandler.cs index e5b5e7dc..db5581b6 100644 --- a/src/Backend/AutopilotMonitor.Functions/Services/Offboarding/TenantOffboardingHandler.cs +++ b/src/Backend/AutopilotMonitor.Functions/Services/Offboarding/TenantOffboardingHandler.cs @@ -774,22 +774,10 @@ namespace AutopilotMonitor.Functions.Services.Offboarding // Side-effect 6: post-completion farewell email to the captured Preview-Notification- // Email. Fail-soft: any sender exception MUST NOT propagate, the offboarding // correctness contract is independent of email delivery and the History row is - // already Completed. Skips silently when no email was captured at Phase 1 - // (tenant never set a preview notification address). - if (!string.IsNullOrWhiteSpace(history.NotificationEmail)) - { - try - { - await _farewellEmail.SendAsync( - history.NotificationEmail!, history.DomainName ?? string.Empty, tenantId, ct); - } - catch (Exception ex) - { - _logger.LogWarning(ex, - "Offboard farewell email send threw for tenant={Tenant} — ignored (best-effort)", - tenantId); - } - } + // already Completed. Every outcome leaves an ops event (Sent / Skipped / Failed): + // the send's own success log is Information and never reaches Application + // Insights from the worker, which made a lost farewell mail invisible. + await SendFarewellEmailAsync(history, tenantId, ct); // ── 2.F-final — TenantConfiguration delete (PR3.B Codex Finding 2 reorder) ── // @@ -831,6 +819,49 @@ namespace AutopilotMonitor.Functions.Services.Offboarding // ── Helpers ───────────────────────────────────────────────────────────── + /// + /// Side-effect 6 — the farewell mail plus its ops-event record. Never throws: the History + /// row is already Completed and the TenantConfiguration delete that follows must run. + /// Outcomes: no address captured at Phase 1 → FarewellEmailSkipped (Warning: the + /// customer left in silence, nothing is broken); provider accepted → FarewellEmailSent + /// (Info); provider refused, provider not configured or the send threw → + /// FarewellEmailFailed (Error: an address was there and the mail did not go out). + /// + private async Task SendFarewellEmailAsync(OffboardingHistoryEntry history, string tenantId, CancellationToken ct) + { + var domainName = history.DomainName ?? string.Empty; + var toEmail = history.NotificationEmail; + + if (string.IsNullOrWhiteSpace(toEmail)) + { + await _opsEvents.RecordFarewellEmailSkippedAsync(tenantId, domainName, + "no contact address was set before the offboarding"); + return; + } + + try + { + var sent = await _farewellEmail.SendAsync(toEmail!, domainName, tenantId, ct); + if (sent) + { + await _opsEvents.RecordFarewellEmailSentAsync(tenantId, domainName, toEmail!); + } + else + { + await _opsEvents.RecordFarewellEmailFailedAsync(tenantId, domainName, toEmail!, + "the email provider did not accept the message (or is not configured)"); + } + } + catch (Exception ex) + { + _logger.LogWarning(ex, + "Offboard farewell email send threw for tenant={Tenant} — ignored (best-effort)", + tenantId); + await _opsEvents.RecordFarewellEmailFailedAsync(tenantId, domainName, toEmail!, + $"{ex.GetType().Name}: {ex.Message}"); + } + } + /// /// PR3.B §3 — Phase 2.D-archive. For each rules table, archive every row to /// THEN delete that exact (PK, RK) diff --git a/src/Backend/AutopilotMonitor.Functions/Services/OpsEventService.cs b/src/Backend/AutopilotMonitor.Functions/Services/OpsEventService.cs index a39f34be..5001e06b 100644 --- a/src/Backend/AutopilotMonitor.Functions/Services/OpsEventService.cs +++ b/src/Backend/AutopilotMonitor.Functions/Services/OpsEventService.cs @@ -663,6 +663,47 @@ namespace AutopilotMonitor.Functions.Services tenantId, "System.Activation", new { domainName, toEmail, reason }); } + // ── Offboarding farewell mail ── + // Same shape as the welcome mail above, for the same reason: the send fails soft and its + // success log is Information, so without these events a lost farewell mail (2026-09-22: + // a tenant WITH a captured address, no trace either way) leaves no record at all. + // Dispatched by TenantOffboardingHandler after History → Completed. All three + // dual-registered in OpsAlertRulesSection.tsx OPS_EVENT_TYPES. + + /// The provider accepted the farewell mail. Info-tier confirmation. + public Task RecordFarewellEmailSentAsync(string tenantId, string? domainName, string toEmail) + { + var tenantLabel = string.IsNullOrWhiteSpace(domainName) ? tenantId : $"{domainName} ({tenantId})"; + return WriteAsync(OpsEventCategory.Tenant, OpsEventTypes.FarewellEmailSent, OpsEventSeverity.Info, + $"Farewell email sent to {toEmail} for offboarded tenant {tenantLabel}", + tenantId, "System.Offboarding", new { domainName, toEmail }); + } + + /// + /// The offboarding completed but no farewell mail went out because no contact address + /// was captured before the wipe. Warning-tier: nothing is broken, but a customer left + /// without the "sorry to see you go" and its feedback channels. + /// + public Task RecordFarewellEmailSkippedAsync(string tenantId, string? domainName, string reason) + { + var tenantLabel = string.IsNullOrWhiteSpace(domainName) ? tenantId : $"{domainName} ({tenantId})"; + return WriteAsync(OpsEventCategory.Tenant, OpsEventTypes.FarewellEmailSkipped, OpsEventSeverity.Warning, + $"No farewell email for offboarded tenant {tenantLabel} — {reason}", + tenantId, "System.Offboarding", new { domainName, reason }); + } + + /// + /// An address was captured and the mail did not go out: provider refused, provider not + /// configured, or the send threw. Error-tier — this one IS a defect. + /// + public Task RecordFarewellEmailFailedAsync(string tenantId, string? domainName, string toEmail, string reason) + { + var tenantLabel = string.IsNullOrWhiteSpace(domainName) ? tenantId : $"{domainName} ({tenantId})"; + return WriteAsync(OpsEventCategory.Tenant, OpsEventTypes.FarewellEmailFailed, OpsEventSeverity.Error, + $"Farewell email to {toEmail} failed for offboarded tenant {tenantLabel} — {reason}", + tenantId, "System.Offboarding", new { domainName, toEmail, reason }); + } + // ── Tenant trial lifecycle (informational — enforcement is read-time) ── // Both types are dual-registered in OpsAlertRulesSection.tsx OPS_EVENT_TYPES // (memory feedback_ops_event_types_dual_register). Dispatched by TrialExpirySweepFunction. diff --git a/src/McpServer/autopilot-monitor-mcp/src/generated/wire-vocabularies.generated.ts b/src/McpServer/autopilot-monitor-mcp/src/generated/wire-vocabularies.generated.ts index 8f2455f2..f8874f91 100644 --- a/src/McpServer/autopilot-monitor-mcp/src/generated/wire-vocabularies.generated.ts +++ b/src/McpServer/autopilot-monitor-mcp/src/generated/wire-vocabularies.generated.ts @@ -111,6 +111,9 @@ export const OPS_EVENT_TYPES = [ "WelcomeEmailSent", "WelcomeEmailSkipped", "WelcomeEmailFailed", + "FarewellEmailSent", + "FarewellEmailSkipped", + "FarewellEmailFailed", "TenantTrialStarted", "TenantTrialExpiring", "TenantTrialExpired", diff --git a/src/Shared/AutopilotMonitor.Shared/DataAccess/OpsEventTypes.cs b/src/Shared/AutopilotMonitor.Shared/DataAccess/OpsEventTypes.cs index 5d89a9e8..c7899da7 100644 --- a/src/Shared/AutopilotMonitor.Shared/DataAccess/OpsEventTypes.cs +++ b/src/Shared/AutopilotMonitor.Shared/DataAccess/OpsEventTypes.cs @@ -94,6 +94,16 @@ namespace AutopilotMonitor.Shared.DataAccess public const string WelcomeEmailSent = "WelcomeEmailSent"; public const string WelcomeEmailSkipped = "WelcomeEmailSkipped"; public const string WelcomeEmailFailed = "WelcomeEmailFailed"; + /// + /// Post-offboarding farewell mail, recorded by the offboarding worker after the History + /// row went Completed. Sent (Info) = the provider accepted it; Skipped (Warning) = no + /// contact address was captured before the wipe, the customer left in silence; Failed + /// (Error) = an address was there and the mail did not go out. The only record of the + /// send: its success log is Information and never reaches Application Insights. + /// + public const string FarewellEmailSent = "FarewellEmailSent"; + public const string FarewellEmailSkipped = "FarewellEmailSkipped"; + public const string FarewellEmailFailed = "FarewellEmailFailed"; public const string TenantTrialStarted = "TenantTrialStarted"; public const string TenantTrialExpiring = "TenantTrialExpiring"; public const string TenantTrialExpired = "TenantTrialExpired"; @@ -139,7 +149,7 @@ namespace AutopilotMonitor.Shared.DataAccess ConsentFlowStarted, ConsentFlowSuccess, ConsentFlowFailed, ConsentRedirectUriMismatch, AppHomingFlipped, AppHomingFlippedWithEntraRoles, MaintenanceCompleted, MaintenanceFailed, MaintenanceLongRunning, MaintenanceStarted, MaintenanceSkippedLocked, SessionSweepCompleted, SessionSweepFailed, OpsEventCleanup, OrphanEventsCleaned, SessionDeletionMaintenanceStarted, SessionDeletionMaintenanceBudgetExceeded, SessionDeletionMaintenanceSkippedLocked, SessionDeletionMaintenanceLongRunning, SessionDeletionMaintenanceLongRunningSevere, SessionDeletionMaintenanceFailed, SessionDeletionStrandedQueued, SessionDeletionPoisoned, SessionDeletionMaintenanceCompleted, SessionDeletionMaintenanceFanoutSkipped, CriticalTableBackupCompleted, CriticalTableBackupPartial, CriticalTableBackupFailed, CriticalTableBackupSkippedLocked, BackupRowRestored, VerdictCalibrationDrift, DeviceBlocked, VersionBlocked, SessionTenantConflict, SessionOwnerMismatch, KillSignalDelivered, EmbeddedCertExpiringSoon, EmbeddedCertExpiringUrgent, EmbeddedCertExpired, EmbeddedCertBundleEmpty, SignalRConnectionsHigh, SignalRConnectionsCritical, SignalRMessagesHigh, SignalRMessagesCritical, PoisonQueueBacklogHigh, PoisonQueueBacklogCritical, ExcessiveSessionEventsAutoActioned, PrivilegedRouteDenied, McpServicePrincipalFirstSeen, - OffboardingFeedbackReceived, TenantOffboarded, TenantOffboardingFailed, TenantAutoApproved, WelcomeEmailSent, WelcomeEmailSkipped, WelcomeEmailFailed, TenantTrialStarted, TenantTrialExpiring, TenantTrialExpired, TenantPlanDowngraded, TenantRetentionGraceExpiring, TenantRetentionGraceEnded, RuleFrequencyRegression, AppVersionDurationRegression, CollectLogsQuickConfigEnabled, DiagnosticsUploadEnabled, DiagnosticsUploadDisabled, + OffboardingFeedbackReceived, TenantOffboarded, TenantOffboardingFailed, TenantAutoApproved, WelcomeEmailSent, WelcomeEmailSkipped, WelcomeEmailFailed, FarewellEmailSent, FarewellEmailSkipped, FarewellEmailFailed, TenantTrialStarted, TenantTrialExpiring, TenantTrialExpired, TenantPlanDowngraded, TenantRetentionGraceExpiring, TenantRetentionGraceEnded, RuleFrequencyRegression, AppVersionDurationRegression, CollectLogsQuickConfigEnabled, DiagnosticsUploadEnabled, DiagnosticsUploadDisabled, SessionActionQueued, SessionTimeouts, AgentEmergencyBreak, AgentBinaryIntegrityMismatch, CmTraceTimeSkewRegression, ExcessiveSessionEvents, NewImeVersionDetected, ImePatternDriftSuspected, BlobStorageMissing, BlobStorageUnreachable, TelemetryItemsRejected, SlaBreachNotification, SlaConsecutiveFailures, SlaEvaluationCompleted, AzureMonitorAlert, diff --git a/src/Web/autopilot-monitor-web/app/admin/components/OpsAlertRulesSection.tsx b/src/Web/autopilot-monitor-web/app/admin/components/OpsAlertRulesSection.tsx index 42ff11c3..f7032ce4 100644 --- a/src/Web/autopilot-monitor-web/app/admin/components/OpsAlertRulesSection.tsx +++ b/src/Web/autopilot-monitor-web/app/admin/components/OpsAlertRulesSection.tsx @@ -186,6 +186,13 @@ const OPS_EVENT_TYPES: Record = { "WelcomeEmailSent", "WelcomeEmailSkipped", "WelcomeEmailFailed", + // Offboarding farewell mail, recorded by the offboarding worker after the wipe completed. + // Same three-way record as the welcome mail: Skipped (Warning) = no contact address was + // captured before the wipe, Failed (Error) = the provider refused an address we had, + // Sent (Info) = confirmation. Dual-register per memory feedback_ops_event_types_dual_register. + "FarewellEmailSent", + "FarewellEmailSkipped", + "FarewellEmailFailed", // A tenant admin turned on-demand diagnostics upload (the Collect Logs capability) on or // off — portal Settings, the Collect Logs quick-config dialog, MCP update_tenant_config, // or a config revert. Fires on the flip only. Backend diff --git a/src/Web/autopilot-monitor-web/app/settings/components/OffboardingSection.tsx b/src/Web/autopilot-monitor-web/app/settings/components/OffboardingSection.tsx index 640102cc..011ca2b6 100644 --- a/src/Web/autopilot-monitor-web/app/settings/components/OffboardingSection.tsx +++ b/src/Web/autopilot-monitor-web/app/settings/components/OffboardingSection.tsx @@ -1,13 +1,10 @@ "use client"; import { useEffect, useRef, useState } from "react"; -import { api } from "@/lib/api"; -import { TokenExpiredError } from "@/lib/authenticatedFetch"; -import { apiErrorText, fetchOk, jsonBody } from "@/lib/apiClient"; import { SectionCardHeader } from "@/components/SectionCardHeader"; import { DOCS_PATHS } from "@/lib/docsPaths"; -import type { SubmitOffboardingFeedbackRequest } from "@/utils/wire-types.generated"; import { ModalPortal } from "@/components/ModalPortal"; +import OffboardingFeedbackForm from "@/components/OffboardingFeedbackForm"; interface OffboardingInProgressInfo { status: string; // "Queued" | "Initiated" | "InProgress" | "Completed" | "Failed" @@ -39,13 +36,6 @@ interface OffboardingSectionProps { getAccessToken: () => Promise; } -/** localStorage key prefix for "this user already submitted feedback for historyRowKey X". - * Tied to historyRowKey rather than tenantId so a tenant that offboards twice - * (re-onboard → offboard) gets a fresh prompt each time. */ -const FEEDBACK_SUBMITTED_KEY_PREFIX = "offboard-feedback-submitted:"; - -const FEEDBACK_MAX_CHARS = 4096; - function formatRemaining(totalSeconds: number): string { if (totalSeconds <= 0) return "0 min"; return `~${Math.ceil(totalSeconds / 60)} min`; @@ -251,50 +241,6 @@ function OffboardingInProgressBanner({ const secondsLeft = useCountdown(info.earliestProcessingAt ?? null, onDrainBarrierElapsed); const inDrainBarrier = (info.earliestProcessingAt && secondsLeft > 0) === true; - // Farewell-feedback widget state. Each historyRowKey gets its own localStorage flag so a - // refresh during the countdown remembers the submit. Re-offboarding later (new historyRowKey) - // shows a fresh prompt — exactly what we want for repeat-offboard scenarios. - const localStorageKey = `${FEEDBACK_SUBMITTED_KEY_PREFIX}${info.historyRowKey}`; - const [feedbackText, setFeedbackText] = useState(""); - const [submitting, setSubmitting] = useState(false); - const [submitError, setSubmitError] = useState(null); - const [alreadySubmitted, setAlreadySubmitted] = useState(() => { - if (typeof window === "undefined") return false; - try { - return window.localStorage.getItem(localStorageKey) === "1"; - } catch { - return false; - } - }); - - const handleSubmitFeedback = async () => { - const trimmed = feedbackText.trim(); - if (trimmed.length === 0 || !tenantId) return; - - setSubmitting(true); - setSubmitError(null); - try { - await fetchOk(api.tenants.offboardFeedback(tenantId), getAccessToken, { - method: "POST", - body: jsonBody({ comment: trimmed.slice(0, FEEDBACK_MAX_CHARS) }), - }); - try { - window.localStorage.setItem(localStorageKey, "1"); - } catch { - // localStorage disabled — UI still flips via state below; the round trip already succeeded. - } - setAlreadySubmitted(true); - } catch (err) { - if (err instanceof TokenExpiredError) { - setSubmitError("Your session has expired. The offboarding will continue in the background."); - } else { - setSubmitError(apiErrorText(err, "Failed to submit feedback")); - } - } finally { - setSubmitting(false); - } - }; - // Single 4-state visual; status text drives the badge color + label. // "Preparing" is derived from earliestProcessingAt + countdown rather than from status, // because the backend status stays "Queued" during the cache-drain window. @@ -364,50 +310,14 @@ function OffboardingInProgressBanner({ {/* Farewell-feedback widget — only meaningful while the drain barrier is running. Hidden once we transition to actual deletion / completion / failure: at that point the user can no longer interact, and an admin re-clicking later would see the - banner in a non-feedback state. */} + banner in a non-feedback state. A reload during the barrier lands on the suspended + page (ProtectedRoute), which renders the same form. */} {inDrainBarrier && tenantId && ( -
-
-

We're sorry to see you go

-

- What could we have done better? Anything specific that didn't fit your needs? - (optional, max {FEEDBACK_MAX_CHARS} characters) -

-
- - {alreadySubmitted ? ( -
- - - - Thanks for the feedback — it really helps shape what we build next. -
- ) : ( - <> -