diff --git a/internal b/internal index be43a5f8..c41ad52f 160000 --- a/internal +++ b/internal @@ -1 +1 @@ -Subproject commit be43a5f8e412acf3ba6d0cf1d7a48573dde74709 +Subproject commit c41ad52fb51d909df8d3824f19fbd92c2ba13e22 diff --git a/src/Backend/AutopilotMonitor.Functions.Tests/AuthFunctionSideEffectTests.cs b/src/Backend/AutopilotMonitor.Functions.Tests/AuthFunctionSideEffectTests.cs index 2f9d5408..dac31f6c 100644 --- a/src/Backend/AutopilotMonitor.Functions.Tests/AuthFunctionSideEffectTests.cs +++ b/src/Backend/AutopilotMonitor.Functions.Tests/AuthFunctionSideEffectTests.cs @@ -1,5 +1,9 @@ +using AutopilotMonitor.Functions.Functions.Config; using AutopilotMonitor.Functions.Functions.Infrastructure; using AutopilotMonitor.Functions.Security; +using AutopilotMonitor.Functions.Tests.Offboarding; +using AutopilotMonitor.Shared; +using AutopilotMonitor.Shared.Models.Offboarding; using AutopilotMonitor.Functions.Services; using AutopilotMonitor.Shared.DataAccess; using AutopilotMonitor.Shared.Models; @@ -32,6 +36,7 @@ public class AuthFunctionSideEffectTests private readonly Mock _metricsRepoMock; private readonly Mock _autoApproveEnqueuerMock; private readonly FakeSignalRNotificationService _signalR; + private readonly FakeOffboardingAuditRepository _offboardingRepo = new(); private readonly AuthFunction _sut; public AuthFunctionSideEffectTests() @@ -108,7 +113,8 @@ public class AuthFunctionSideEffectTests new Mock( _metricsRepoMock.Object, _tenantConfigMock.Object, Mock.Of>()) { CallBase = false }.Object, _signalR, - adminConfigService.Object); + adminConfigService.Object, + _offboardingRepo); // Default: all fire-and-forget calls succeed _tenantConfigMock @@ -237,6 +243,86 @@ public class AuthFunctionSideEffectTests It.IsAny()), Times.Once); } + // ------------------------------------------------------------------------- + // Returning tenant (D-288): the offboarding deleted the configuration, the pointer + // carries the consumed self-service trial into the first-login write. + // ------------------------------------------------------------------------- + + private void SeedOffboardingPointer(bool trialConsumed) => + _offboardingRepo.Pointers[TenantId] = (new OffboardingByTenantPointer + { + PartitionKey = Constants.OffboardingPartitionKeys.ByTenant, + RowKey = TenantId, + TenantId = TenantId, + LatestStatus = "Completed", + OffboardCount = 1, + TrialConsumed = trialConsumed, + }, "\"etag\""); + + private static TenantConfiguration ReturningTenantDefault() + { + // What auth/me hands over once the offboarding deleted the row: an unpersisted default. + var config = TenantConfiguration.CreateDefault(TenantId); + Assert.False(config.TrialConsumed); + return config; + } + + [Fact] + public async Task HandleNewTenantDomain_ReturningTenantThatUsedTheTrial_CarriesItIntoTheOnboardingWrite() + { + SeedOffboardingPointer(trialConsumed: true); + var config = ReturningTenantDefault(); + + await _sut.HandleNewTenantDomainAsync(config, TenantId, Upn); + + Assert.True(config.TrialConsumed); + Assert.Equal(Upn, config.OnboardedBy); + _tenantConfigMock.Verify(x => x.SaveConfigurationAsync( + It.Is(c => c.TrialConsumed && c.OnboardedBy == Upn), + It.IsAny(), It.IsAny()), Times.Once); + // Re-onboarding itself stays unrestricted: same signup path, auto-approve included. + _autoApproveEnqueuerMock.Verify(x => x.EnqueueAsync( + It.IsAny(), + It.IsAny(), It.IsAny()), Times.Once); + // The verify criterion of the finding: the trial gate refuses the returning tenant. + var deny = PlanManagementFunction.EvaluateTrialStart(config, DateTime.UtcNow); + Assert.Equal(Constants.ApiErrorCodes.TrialAlreadyConsumed, deny?.Code); + } + + [Theory] + [InlineData(false)] // offboarded without ever using the trial + [InlineData(null)] // never offboarded + public async Task HandleNewTenantDomain_NoConsumedTrialOnRecord_LeavesTheTrialAvailable(bool? pointerTrialConsumed) + { + if (pointerTrialConsumed is { } consumed) + SeedOffboardingPointer(consumed); + var config = ReturningTenantDefault(); + + await _sut.HandleNewTenantDomainAsync(config, TenantId, Upn); + + Assert.False(config.TrialConsumed); + Assert.Equal("contoso.com", config.DomainName); + _tenantConfigMock.Verify(x => x.SaveConfigurationAsync(config, It.IsAny(), It.IsAny()), Times.Once); + } + + [Fact] + public async Task HandleNewTenantDomain_OffboardingLookupFails_WritesNothingAndRetriesOnTheNextLogin() + { + // An onboarded row without the carry-over would hand the trial back; no write at all instead. + _offboardingRepo.ThrowOnPointerRead = new Azure.RequestFailedException(503, "Server busy"); + var config = ReturningTenantDefault(); + + await _sut.HandleNewTenantDomainAsync(config, TenantId, Upn); + + Assert.True(string.IsNullOrEmpty(config.DomainName)); + Assert.Null(config.OnboardedBy); + _tenantConfigMock.Verify(x => x.SaveConfigurationAsync(It.IsAny(), It.IsAny(), It.IsAny()), Times.Never); + _telegramMock.Verify(x => x.SendNewTenantSignupAsync(It.IsAny(), It.IsAny()), Times.Never); + _autoApproveEnqueuerMock.Verify(x => x.EnqueueAsync( + It.IsAny(), + It.IsAny(), It.IsAny()), Times.Never); + } + // ------------------------------------------------------------------------- // HandleAuthClientIdTrackingAsync — the write must mutate a cache-BYPASSING // read, never the (possibly stale) cached view. Regression for the prod diff --git a/src/Backend/AutopilotMonitor.Functions.Tests/EndpointHarness.cs b/src/Backend/AutopilotMonitor.Functions.Tests/EndpointHarness.cs new file mode 100644 index 00000000..47204cd9 --- /dev/null +++ b/src/Backend/AutopilotMonitor.Functions.Tests/EndpointHarness.cs @@ -0,0 +1,67 @@ +using System.Net; +using System.Text; +using System.Text.Json; +using AutopilotMonitor.Functions.Helpers; +using AutopilotMonitor.Shared; +using Azure.Core.Serialization; +using Microsoft.Azure.Functions.Worker; +using Microsoft.Azure.Functions.Worker.Http; +using Microsoft.Extensions.DependencyInjection; +using Moq; + +namespace AutopilotMonitor.Functions.Tests; + +/// +/// Minimal HTTP endpoint harness: a mocked whose FunctionContext carries the +/// the policy middleware would have resolved, serialized with the production +/// wire settings. +/// +internal static class EndpointHarness +{ + public static (HttpRequestData Req, FunctionContext Context) Request( + string tenantId, string upn = "admin@contoso.com", string? jsonBody = null) + { + var services = new ServiceCollection(); + services.AddOptions(); + services.Configure(o => o.Serializer = new JsonObjectSerializer(ApiJsonOptions.Create())); + var provider = services.BuildServiceProvider(); + + var items = new Dictionary + { + ["RequestContext"] = new RequestContext + { + TenantId = tenantId, + TargetTenantId = tenantId, + UserPrincipalName = upn, + IsTenantAdmin = true, + UserRole = Constants.TenantRoles.Admin, + }, + }; + var context = new Mock(); + context.SetupGet(c => c.Items).Returns(items); + context.SetupGet(c => c.InstanceServices).Returns(provider); + + var req = new Mock(context.Object); + req.SetupGet(r => r.Headers).Returns(new HttpHeadersCollection()); + req.SetupGet(r => r.Body).Returns(new MemoryStream(Encoding.UTF8.GetBytes(jsonBody ?? ""))); + req.Setup(r => r.CreateResponse()).Returns(() => new FakeResponse(context.Object)); + return (req.Object, context.Object); + } + + /// The code of an error envelope; null when the body carries none. + public static string? ErrorCode(HttpResponseData response) + { + response.Body.Position = 0; + using var doc = JsonDocument.Parse(response.Body); + return doc.RootElement.TryGetProperty("code", out var code) ? code.GetString() : null; + } + + private sealed class FakeResponse : HttpResponseData + { + public FakeResponse(FunctionContext context) : base(context) { } + public override HttpStatusCode StatusCode { get; set; } = HttpStatusCode.OK; + public override HttpHeadersCollection Headers { get; set; } = new(); + public override Stream Body { get; set; } = new MemoryStream(); + public override HttpCookies Cookies { get; } = new Mock().Object; + } +} diff --git a/src/Backend/AutopilotMonitor.Functions.Tests/Offboarding/FakeOffboardingAuditRepository.cs b/src/Backend/AutopilotMonitor.Functions.Tests/Offboarding/FakeOffboardingAuditRepository.cs index 4f3ee874..3f2aec55 100644 --- a/src/Backend/AutopilotMonitor.Functions.Tests/Offboarding/FakeOffboardingAuditRepository.cs +++ b/src/Backend/AutopilotMonitor.Functions.Tests/Offboarding/FakeOffboardingAuditRepository.cs @@ -88,9 +88,14 @@ internal sealed class FakeOffboardingAuditRepository : IOffboardingAuditReposito return Task.CompletedTask; } + /// When set, every pointer read throws — a storage outage on the re-onboarding lookup. + public Exception? ThrowOnPointerRead { get; set; } + public Task<(OffboardingByTenantPointer? Pointer, string? ETag)> TryGetByTenantPointerAsync( string normalizedTenantId, CancellationToken ct = default) { + if (ThrowOnPointerRead is { } ex) + throw ex; if (Pointers.TryGetValue(normalizedTenantId, out var entry)) { return Task.FromResult<(OffboardingByTenantPointer?, string?)>((Clone(entry.Pointer), entry.ETag)); @@ -152,5 +157,6 @@ internal sealed class FakeOffboardingAuditRepository : IOffboardingAuditReposito PartitionKey = p.PartitionKey, RowKey = p.RowKey, TenantId = p.TenantId, LatestHistoryRowKey = p.LatestHistoryRowKey, LatestStatus = p.LatestStatus, LatestUpdatedAt = p.LatestUpdatedAt, OffboardCount = p.OffboardCount, + TrialConsumed = p.TrialConsumed, }; } diff --git a/src/Backend/AutopilotMonitor.Functions.Tests/TenantConfigUpdateTests.cs b/src/Backend/AutopilotMonitor.Functions.Tests/TenantConfigUpdateTests.cs new file mode 100644 index 00000000..bbb34dd7 --- /dev/null +++ b/src/Backend/AutopilotMonitor.Functions.Tests/TenantConfigUpdateTests.cs @@ -0,0 +1,286 @@ +using System.Net; +using AutopilotMonitor.Functions.Functions.Admin; +using AutopilotMonitor.Functions.Functions.Config; +using AutopilotMonitor.Functions.Services; +using AutopilotMonitor.Shared; +using AutopilotMonitor.Shared.DataAccess; +using AutopilotMonitor.Shared.Models; +using Microsoft.Extensions.Caching.Memory; +using Microsoft.Extensions.Logging.Abstractions; +using Moq; + +namespace AutopilotMonitor.Functions.Tests; + +/// +/// and the plan writers built on it: the decision runs +/// on the fresh row and the write is conditional on that row's ETag — a cached view (up to 5 minutes stale +/// per instance) is never saved, so it can neither rewind another writer's change nor lift the offboarding +/// tombstone during the drain barrier, and two concurrent trial starts cannot both see TrialConsumed=false. +/// +public class TenantConfigUpdateTests +{ + private const string TenantId = "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee"; + + private readonly Mock _repo = new(); + private readonly Queue<(TenantConfiguration Row, string ETag)> _freshReads = new(); + private readonly List<(TenantConfiguration Config, string ETag, string? Source, string? Reason)> _replaces = new(); + private readonly Queue _replaceResults = new(); + + public TenantConfigUpdateTests() + { + _repo.Setup(r => r.GetTenantConfigurationWithEtagAsync(TenantId)) + .ReturnsAsync(() => _freshReads.Count > 0 ? _freshReads.Dequeue() : null); + _repo.Setup(r => r.TryReplaceTenantConfigurationAsync( + It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) + .Callback((c, e, s, r) => _replaces.Add((c, e, s, r))) + .ReturnsAsync(() => _replaceResults.Count == 0 || _replaceResults.Dequeue()); + } + + private TenantConfigurationService Service(ManagedTenantProIndex? proIndex = null) => + new(_repo.Object, NullLogger.Instance, + new MemoryCache(new MemoryCacheOptions()), proIndex ?? ManagedTenantProIndex.None); + + private static TenantConfiguration Row(Action? shape = null) + { + var row = TenantConfiguration.CreateDefault(TenantId); + row.DomainName = "contoso.com"; + row.ContactEmail = "it@contoso.com"; + row.CompanyName = "Contoso"; + shape?.Invoke(row); + return row; + } + + private static TenantConfiguration Tombstone() => Row(r => + { + r.Disabled = true; + r.DisabledReason = TenantOffboardFunction.OffboardingDisabledReason; + }); + + /// Primes the service's cache with , the view a warm instance would hold. + private async Task PrimeCacheAsync(TenantConfigurationService service, TenantConfiguration cached) + { + _repo.Setup(r => r.GetTenantConfigurationAsync(TenantId)).ReturnsAsync(cached); + Assert.Same(cached, await service.GetConfigurationIfExistsAsync(TenantId)); + } + + // ── UpdateAsync ───────────────────────────────────────────────────────── + + [Fact] + public async Task UpdateAsync_MutatesTheFreshRow_UnderItsETag_NeverTheCachedView() + { + var service = Service(); + await PrimeCacheAsync(service, Row(r => r.HomedAppClientId = null)); + _freshReads.Enqueue((Row(r => r.HomedAppClientId = "flipped-meanwhile"), "etag-1")); + + var update = await service.UpdateAsync(TenantId, row => { row.PayingCustomer = true; return true; }, "plan", "why"); + + Assert.Equal(TenantConfigUpdateStatus.Updated, update.Status); + var (written, etag, source, reason) = Assert.Single(_replaces); + Assert.Equal("etag-1", etag); + Assert.Equal("flipped-meanwhile", written.HomedAppClientId); + Assert.True(written.PayingCustomer); + Assert.Equal("plan", source); + Assert.Equal("why", reason); + _repo.Verify(r => r.SaveTenantConfigurationAsync(It.IsAny(), It.IsAny(), It.IsAny()), Times.Never); + + // The cached view is gone: the next read goes to storage. + await service.GetConfigurationIfExistsAsync(TenantId); + _repo.Verify(r => r.GetTenantConfigurationAsync(TenantId), Times.Exactly(2)); + } + + [Fact] + public async Task UpdateAsync_LostRace_RereadsAndDecidesAgainOnTheNewRow() + { + var first = Row(); + var second = Row(r => r.CompanyName = "Contoso after the concurrent write"); + _freshReads.Enqueue((first, "etag-1")); + _freshReads.Enqueue((second, "etag-2")); + _replaceResults.Enqueue(false); + _replaceResults.Enqueue(true); + var seen = new List(); + + var update = await Service().UpdateAsync(TenantId, row => { seen.Add(row); return true; }, "plan", "why"); + + Assert.Equal(TenantConfigUpdateStatus.Updated, update.Status); + Assert.Equal(new[] { first, second }, seen); + Assert.Equal(new[] { "etag-1", "etag-2" }, _replaces.Select(r => r.ETag)); + Assert.Same(second, update.Config); + } + + [Fact] + public async Task UpdateAsync_EveryAttemptLosesTheRace_Conflict() + { + for (var i = 0; i < TenantConfigurationService.MaxUpdateAttempts; i++) + { + _freshReads.Enqueue((Row(), $"etag-{i}")); + _replaceResults.Enqueue(false); + } + + var update = await Service().UpdateAsync(TenantId, _ => true, "plan", "why"); + + Assert.Equal(TenantConfigUpdateStatus.Conflict, update.Status); + Assert.Equal(TenantConfigurationService.MaxUpdateAttempts, _replaces.Count); + } + + [Fact] + public async Task UpdateAsync_OffboardingTombstone_IsNeverWritten() + { + _freshReads.Enqueue((Tombstone(), "etag-1")); + var mutated = false; + + var update = await Service().UpdateAsync(TenantId, _ => mutated = true, "plan", "why"); + + Assert.Equal(TenantConfigUpdateStatus.OffboardingInProgress, update.Status); + Assert.False(mutated); + Assert.Empty(_replaces); + } + + [Fact] + public async Task UpdateAsync_MutationDeclines_NothingIsWritten() + { + var row = Row(); + _freshReads.Enqueue((row, "etag-1")); + + var update = await Service().UpdateAsync(TenantId, _ => false, "plan", "why"); + + Assert.Equal(TenantConfigUpdateStatus.Declined, update.Status); + Assert.Same(row, update.Config); + Assert.Empty(_replaces); + } + + [Fact] + public async Task UpdateAsync_NoRow_NotFound() + { + var update = await Service().UpdateAsync(TenantId, _ => true, "plan", "why"); + + Assert.Equal(TenantConfigUpdateStatus.NotFound, update.Status); + Assert.Empty(_replaces); + } + + [Fact] + public async Task UpdateAsync_TheMutationSeesTheConferredProProjection() + { + // EvaluateTrialStart's AlreadyPro verdict for a managed tenant depends on it. + _freshReads.Enqueue((Row(), "etag-1")); + string? seen = null; + + await Service(new StubManagedTenantProIndex(_ => "managing-tenant")) + .UpdateAsync(TenantId, row => { seen = row.ManagedByProTenantId; return false; }, "plan", "why"); + + Assert.Equal("managing-tenant", seen); + } + + // ── Endpoints ─────────────────────────────────────────────────────────── + + private (PlanManagementFunction Sut, Mock OpsRepo, Mock Audit) Plan( + TenantConfigurationService service) + { + var adminConfig = new Mock( + _repo.Object, NullLogger.Instance, new MemoryCache(new MemoryCacheOptions())) + { CallBase = false }; + var opsRepo = new Mock(); + var audit = new Mock(); + var sut = new PlanManagementFunction( + NullLogger.Instance, service, adminConfig.Object, audit.Object, + new OpsEventService(opsRepo.Object, NullLogger.Instance, + TestNotifications.InertOpsAlertDispatch(adminConfig.Object)), + TestProConferral.Inert()); + return (sut, opsRepo, audit); + } + + [Fact] + public async Task StartTrial_ConcurrentStartWonTheRace_SecondGets409TrialAlreadyConsumed() + { + _freshReads.Enqueue((Row(), "etag-1")); + _freshReads.Enqueue((Row(r => r.TrialConsumed = true), "etag-2")); + _replaceResults.Enqueue(false); + var (sut, opsRepo, audit) = Plan(Service()); + var (req, _) = EndpointHarness.Request(TenantId); + + var response = await sut.StartTrial(req, TenantId); + + Assert.Equal(HttpStatusCode.Conflict, response.StatusCode); + Assert.Equal(Constants.ApiErrorCodes.TrialAlreadyConsumed, EndpointHarness.ErrorCode(response)); + Assert.Single(_replaces); + opsRepo.Verify(r => r.SaveOpsEventAsync(It.IsAny()), Times.Never); + audit.Verify(a => a.LogAuditEntryAsync(It.IsAny(), It.IsAny(), It.IsAny(), + It.IsAny(), It.IsAny(), It.IsAny>()), Times.Never); + } + + [Fact] + public async Task StartTrial_WarmCacheDuringTheDrainBarrier_CannotLiftTheTombstone() + { + var service = Service(); + await PrimeCacheAsync(service, Row()); // this instance still sees the tenant enabled + _freshReads.Enqueue((Tombstone(), "etag-1")); + var (sut, _, _) = Plan(service); + var (req, _) = EndpointHarness.Request(TenantId); + + var response = await sut.StartTrial(req, TenantId); + + Assert.Equal(HttpStatusCode.Forbidden, response.StatusCode); + Assert.Equal(Constants.ApiErrorCodes.TenantSuspended, EndpointHarness.ErrorCode(response)); + Assert.Empty(_replaces); + _repo.Verify(r => r.SaveTenantConfigurationAsync(It.IsAny(), It.IsAny(), It.IsAny()), Times.Never); + } + + [Fact] + public async Task StartTrial_Allowed_WritesTheTrialOntoTheFreshRow() + { + var service = Service(); + await PrimeCacheAsync(service, Row(r => r.CompanyName = "stale")); + _freshReads.Enqueue((Row(r => r.CompanyName = "Contoso"), "etag-1")); + var (sut, opsRepo, _) = Plan(service); + var (req, _) = EndpointHarness.Request(TenantId, upn: "admin@contoso.com"); + + var response = await sut.StartTrial(req, TenantId); + + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + var (written, etag, source, reason) = Assert.Single(_replaces); + Assert.Equal("etag-1", etag); + Assert.Equal("Contoso", written.CompanyName); + Assert.True(written.TrialConsumed); + Assert.Equal("admin@contoso.com", written.TrialGrantedBy); + Assert.Equal(written.TrialStartedUtc!.Value.AddDays(PlanManagementFunction.SelfServiceTrialDays), written.TrialExpiresUtc); + Assert.Equal("plan", source); + Assert.Equal("self-service trial start", reason); + opsRepo.Verify(r => r.SaveOpsEventAsync(It.Is(e => e.EventType == OpsEventTypes.TenantTrialStarted)), Times.Once); + } + + [Fact] + public async Task SetPlanTier_WritesOnTheFreshRowUnderItsETag() + { + var service = Service(); + await PrimeCacheAsync(service, Row(r => r.CompanyName = "stale")); + _freshReads.Enqueue((Row(r => r.CompanyName = "Contoso"), "etag-1")); + var (sut, _, audit) = Plan(service); + var (req, _) = EndpointHarness.Request(TenantId, + jsonBody: $"{{\"{PlanManagementFunction.PlanPatchKeys.PayingCustomer}\":true}}"); + + var response = await sut.SetPlanTier(req, TenantId); + + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + var (written, etag, _, _) = Assert.Single(_replaces); + Assert.Equal("etag-1", etag); + Assert.Equal("Contoso", written.CompanyName); + Assert.True(written.PayingCustomer); + audit.Verify(a => a.LogAuditEntryAsync(TenantId, "UPDATE", "TenantPlan", TenantId, It.IsAny(), + It.Is>(d => d.ContainsKey("PayingCustomer"))), Times.Once); + } + + [Fact] + public async Task SetPlanTier_NothingChangesOnTheFreshRow_200WithoutAWrite() + { + _freshReads.Enqueue((Row(r => r.PayingCustomer = true), "etag-1")); + var (sut, _, audit) = Plan(Service()); + var (req, _) = EndpointHarness.Request(TenantId, + jsonBody: $"{{\"{PlanManagementFunction.PlanPatchKeys.PayingCustomer}\":true}}"); + + var response = await sut.SetPlanTier(req, TenantId); + + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + Assert.Empty(_replaces); + audit.Verify(a => a.LogAuditEntryAsync(It.IsAny(), It.IsAny(), It.IsAny(), + It.IsAny(), It.IsAny(), It.IsAny>()), Times.Never); + } +} diff --git a/src/Backend/AutopilotMonitor.Functions.Tests/TenantOffboardFunctionTests.cs b/src/Backend/AutopilotMonitor.Functions.Tests/TenantOffboardFunctionTests.cs index c5b5253a..1030658c 100644 --- a/src/Backend/AutopilotMonitor.Functions.Tests/TenantOffboardFunctionTests.cs +++ b/src/Backend/AutopilotMonitor.Functions.Tests/TenantOffboardFunctionTests.cs @@ -196,7 +196,7 @@ public sealed class TenantOffboardFunctionTests var sut = Build(repo); var historyRowKey = $"{DateTime.UtcNow:yyyyMMddHHmmssfff}_{TenantId}"; - await sut.UpsertPointerWithCasAsync(TenantId, historyRowKey, DateTime.UtcNow); + await sut.UpsertPointerWithCasAsync(TenantId, historyRowKey, DateTime.UtcNow, trialConsumed: false); Assert.True(repo.Pointers.TryGetValue(TenantId, out var entry)); Assert.Equal(historyRowKey, entry.Pointer.LatestHistoryRowKey); @@ -222,13 +222,113 @@ public sealed class TenantOffboardFunctionTests var sut = Build(repo); var newHistoryRowKey = $"{DateTime.UtcNow:yyyyMMddHHmmssfff}_{TenantId}"; - await sut.UpsertPointerWithCasAsync(TenantId, newHistoryRowKey, DateTime.UtcNow); + await sut.UpsertPointerWithCasAsync(TenantId, newHistoryRowKey, DateTime.UtcNow, trialConsumed: false); Assert.Equal(newHistoryRowKey, repo.Pointers[TenantId].Pointer.LatestHistoryRowKey); Assert.Equal("Initiated", repo.Pointers[TenantId].Pointer.LatestStatus); Assert.Equal(3, repo.Pointers[TenantId].Pointer.OffboardCount); } + [Theory] + [InlineData(false, false, false)] + [InlineData(false, true, true)] + [InlineData(true, false, true)] // a later round without a trial never clears it + [InlineData(true, true, true)] + public async Task UpsertPointer_TrialConsumed_IsStickyAcrossRounds(bool stored, bool thisRound, bool expected) + { + var repo = new FakeOffboardingAuditRepository(); + await repo.InsertByTenantPointerAsync(new OffboardingByTenantPointer + { + PartitionKey = Constants.OffboardingPartitionKeys.ByTenant, + RowKey = TenantId, + TenantId = TenantId, + LatestHistoryRowKey = "20260101000000000_" + TenantId, + LatestStatus = "Completed", + OffboardCount = 1, + TrialConsumed = stored, + }); + + await Build(repo).UpsertPointerWithCasAsync(TenantId, "20260201000000000_" + TenantId, DateTime.UtcNow, thisRound); + + Assert.Equal(expected, repo.Pointers[TenantId].Pointer.TrialConsumed); + Assert.Equal(2, repo.Pointers[TenantId].Pointer.OffboardCount); + } + + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task UpsertPointer_FreshTenant_StampsTrialConsumed(bool trialConsumed) + { + var repo = new FakeOffboardingAuditRepository(); + + await Build(repo).UpsertPointerWithCasAsync(TenantId, "20260201000000000_" + TenantId, DateTime.UtcNow, trialConsumed); + + Assert.Equal(trialConsumed, repo.Pointers[TenantId].Pointer.TrialConsumed); + } + + // ── OffboardTenant (Phase 1): the consumed trial must reach the pointer before anything is deleted ── + + private static (TenantOffboardFunction Sut, FakeOffboardingAuditRepository Repo, RecordingEnqueuer Enqueuer, Mock ConfigRepo) + BuildForOffboard(TenantConfiguration? liveConfig, Exception? readFailure = null) + { + var configRepo = new Mock(); + if (readFailure != null) + configRepo.Setup(r => r.GetTenantConfigurationAsync(It.IsAny())).ThrowsAsync(readFailure); + else + configRepo.Setup(r => r.GetTenantConfigurationAsync(It.IsAny())).ReturnsAsync(liveConfig); + configRepo.Setup(r => r.SaveTenantConfigurationAsync(It.IsAny(), It.IsAny(), It.IsAny())) + .ReturnsAsync(true); + + var repo = new FakeOffboardingAuditRepository(); + var enqueuer = new RecordingEnqueuer(); + var sut = new TenantOffboardFunction( + logger: NullLogger.Instance, + configRepo: configRepo.Object, + tenantConfigService: new TenantConfigurationService( + configRepo.Object, NullLogger.Instance, + new Microsoft.Extensions.Caching.Memory.MemoryCache(new Microsoft.Extensions.Caching.Memory.MemoryCacheOptions())), + maintenanceRepo: Mock.Of(), + offboardingRepo: repo, + offboardingEnqueuer: enqueuer, + previewWhitelistService: BuildPreviewWhitelistService(configRepo)); + return (sut, repo, enqueuer, configRepo); + } + + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task OffboardTenant_CarriesTheLiveTrialConsumedOntoThePointer(bool trialConsumed) + { + var live = TenantConfiguration.CreateDefault(TenantId); + live.DomainName = "contoso.com"; + live.TrialConsumed = trialConsumed; + var (sut, repo, enqueuer, _) = BuildForOffboard(live); + var (req, context) = EndpointHarness.Request(TenantId); + + var response = await sut.OffboardTenant(req, TenantId, context); + + Assert.Equal(System.Net.HttpStatusCode.Accepted, response.StatusCode); + Assert.Equal(trialConsumed, repo.Pointers[TenantId].Pointer.TrialConsumed); + Assert.Single(enqueuer.Sent); + } + + [Fact] + public async Task OffboardTenant_ConfigReadFails_RefusesBeforeAnyStateIsWritten() + { + // Proceeding without the read would reset the one-trial rule for the returning tenant. + var (sut, repo, enqueuer, configRepo) = BuildForOffboard(null, new RequestFailedException(503, "Server busy")); + var (req, context) = EndpointHarness.Request(TenantId); + + var response = await sut.OffboardTenant(req, TenantId, context); + + Assert.Equal(System.Net.HttpStatusCode.InternalServerError, response.StatusCode); + Assert.Empty(repo.History); + Assert.Empty(repo.Pointers); + Assert.Empty(repo.Markers); + Assert.Empty(enqueuer.Sent); + configRepo.Verify(r => r.SaveTenantConfigurationAsync(It.IsAny(), It.IsAny(), It.IsAny()), Times.Never); + } + [Fact] public async Task UpsertPointer_409OnInsert_FallsBackToEtagUpdate() { @@ -273,7 +373,7 @@ public sealed class TenantOffboardFunctionTests var sut = Build(repoMock.Object); var newHistoryRowKey = $"{DateTime.UtcNow:yyyyMMddHHmmssfff}_{TenantId}"; - await sut.UpsertPointerWithCasAsync(TenantId, newHistoryRowKey, DateTime.UtcNow); + await sut.UpsertPointerWithCasAsync(TenantId, newHistoryRowKey, DateTime.UtcNow, trialConsumed: false); Assert.NotNull(updatedPointer); Assert.Equal(newHistoryRowKey, updatedPointer!.LatestHistoryRowKey); @@ -325,7 +425,7 @@ public sealed class TenantOffboardFunctionTests var sut = Build(repoMock.Object); var newHistoryRowKey = $"{DateTime.UtcNow:yyyyMMddHHmmssfff}_{TenantId}"; - await sut.UpsertPointerWithCasAsync(TenantId, newHistoryRowKey, DateTime.UtcNow); + await sut.UpsertPointerWithCasAsync(TenantId, newHistoryRowKey, DateTime.UtcNow, trialConsumed: false); Assert.Equal(2, updateCalls); Assert.Equal(2, readIdx); @@ -484,7 +584,7 @@ public sealed class TenantOffboardFunctionTests var historyRowKey = $"{DateTime.UtcNow:yyyyMMddHHmmssfff}_{TenantId}"; await Assert.ThrowsAsync(() => - sut.UpsertPointerWithCasAsync(TenantId, historyRowKey, DateTime.UtcNow)); + sut.UpsertPointerWithCasAsync(TenantId, historyRowKey, DateTime.UtcNow, trialConsumed: false)); } // ── Cache-Drain-Barrier (plan v2 §2.2 / §2.3) ───────────────────────────── diff --git a/src/Backend/AutopilotMonitor.Functions/DataAccess/TableStorage/TableConfigRepository.cs b/src/Backend/AutopilotMonitor.Functions/DataAccess/TableStorage/TableConfigRepository.cs index e89920ea..306fe18a 100644 --- a/src/Backend/AutopilotMonitor.Functions/DataAccess/TableStorage/TableConfigRepository.cs +++ b/src/Backend/AutopilotMonitor.Functions/DataAccess/TableStorage/TableConfigRepository.cs @@ -134,6 +134,17 @@ namespace AutopilotMonitor.Functions.DataAccess.TableStorage } } + public async Task TryReplaceTenantConfigurationAsync( + TenantConfiguration config, string etag, string? backupSource, string? backupReason) + { + await TrySnapshotBeforeSaveAsync( + _tenantConfigTableClient, config.TenantId, "config", + ConvertFromTenantTableEntity, config, TenantBackupNoiseProperties, + config.UpdatedBy, backupSource, backupReason); + + return await TryReplaceTenantConfigurationAsync(config, etag); + } + /// /// Pre-write snapshot hook shared by the tenant- and admin-config save paths. /// Fail-SOFT by design: this is a safety net around long-standing writers diff --git a/src/Backend/AutopilotMonitor.Functions/DataAccess/TableStorage/TableOffboardingAuditRepository.cs b/src/Backend/AutopilotMonitor.Functions/DataAccess/TableStorage/TableOffboardingAuditRepository.cs index fd0ebebd..c147a218 100644 --- a/src/Backend/AutopilotMonitor.Functions/DataAccess/TableStorage/TableOffboardingAuditRepository.cs +++ b/src/Backend/AutopilotMonitor.Functions/DataAccess/TableStorage/TableOffboardingAuditRepository.cs @@ -282,6 +282,7 @@ namespace AutopilotMonitor.Functions.DataAccess.TableStorage ["LatestStatus"] = p.LatestStatus, ["LatestUpdatedAt"] = p.LatestUpdatedAt, ["OffboardCount"] = p.OffboardCount, + ["TrialConsumed"] = p.TrialConsumed, }; private static OffboardingByTenantPointer MapByTenantPointer(TableEntity e) => new() @@ -293,6 +294,8 @@ namespace AutopilotMonitor.Functions.DataAccess.TableStorage LatestStatus = e.GetString("LatestStatus") ?? "Initiated", LatestUpdatedAt = e.GetDateTime("LatestUpdatedAt") ?? default, OffboardCount = e.GetInt32("OffboardCount") ?? 1, + // Absent on pointers from before the column; false is correct for all of them (D-288). + TrialConsumed = e.GetBoolean("TrialConsumed") ?? false, }; } } diff --git a/src/Backend/AutopilotMonitor.Functions/Functions/Admin/TenantOffboardFunction.cs b/src/Backend/AutopilotMonitor.Functions/Functions/Admin/TenantOffboardFunction.cs index f3e371d1..d668919b 100644 --- a/src/Backend/AutopilotMonitor.Functions/Functions/Admin/TenantOffboardFunction.cs +++ b/src/Backend/AutopilotMonitor.Functions/Functions/Admin/TenantOffboardFunction.cs @@ -100,19 +100,22 @@ public class TenantOffboardFunction "TENANT OFFBOARD initiated for tenant {TenantId} by {Upn}", normalizedTenantId, upn); - // 2. Capture DomainName from TenantConfiguration so the worker's Completed/Failed - // OpsEvents can render "{domain} ({tenantId})" instead of the bare GUID. - string? domainName = null; + // 2. Capture what must outlive the TenantConfiguration row the cascade deletes: DomainName + // for the worker's Completed/Failed OpsEvents ("{domain} ({tenantId})"), and TrialConsumed + // for the pointer, which carries it to the returning tenant's first login (D-288). + // Fail-loud: without the read the one-trial-per-tenant rule would silently reset. + string? domainName; + bool trialConsumed; try { var existingConfig = await _configRepo.GetTenantConfigurationAsync(targetTenantId); domainName = existingConfig?.DomainName; + trialConsumed = existingConfig?.TrialConsumed ?? false; } catch (Exception ex) { - _logger.LogWarning(ex, - "Failed to read tenant configuration for {TenantId} before offboard; proceeding without domain name", - normalizedTenantId); + _logger.LogError(ex, "Failed to read tenant configuration for {TenantId} before offboard", normalizedTenantId); + return await Build500Async(req, "Failed to read tenant configuration"); } // 2.b — Capture the Preview-Notification-Email before Phase 2.D wipes the @@ -184,14 +187,14 @@ public class TenantOffboardFunction // 5. Upsert ByTenant pointer with ETag-CAS so OffboardCount increments race-safely. try { - await UpsertPointerWithCasAsync(normalizedTenantId, historyRowKey, now); + await UpsertPointerWithCasAsync(normalizedTenantId, historyRowKey, now, trialConsumed); } catch (Exception ex) { // History row is already committed — operator can find it under - // PartitionKey="OffboardingHistory" and retry manually. Graceful degradation - // (plan §4.4): pointer is the re-onboarding index; without it PR4's auto-wipe - // is no-op but the offboarding itself still proceeds. + // PartitionKey="OffboardingHistory". Nothing destructive has started: without the + // pointer the returning tenant would get its trial back, so refuse and let the + // admin re-click. _logger.LogError(ex, "Failed to upsert OffboardingByTenant pointer for {TenantId} after history row inserted", normalizedTenantId); @@ -869,11 +872,10 @@ public class TenantOffboardFunction } // Plan §4.4 read-modify-write CAS-loop. Bounded retry; throws on exhaustion so the - // caller turns it into a 500. The pointer is the O(1) re-onboarding index — failure - // here is recoverable (graceful degradation in PR4) but we still surface it so the - // operator can investigate. + // caller turns it into a 500. The pointer is the O(1) re-onboarding index; TrialConsumed + // is sticky across rounds (OR), never reset by an offboarding without a trial. internal async Task UpsertPointerWithCasAsync( - string normalizedTenantId, string historyRowKey, DateTime now) + string normalizedTenantId, string historyRowKey, DateTime now, bool trialConsumed) { for (var attempt = 1; attempt <= PointerCasMaxAttempts; attempt++) { @@ -890,6 +892,7 @@ public class TenantOffboardFunction LatestStatus = "Initiated", LatestUpdatedAt = now, OffboardCount = 1, + TrialConsumed = trialConsumed, }; try @@ -910,6 +913,7 @@ public class TenantOffboardFunction existing.LatestStatus = "Initiated"; existing.LatestUpdatedAt = now; existing.OffboardCount = existing.OffboardCount + 1; + existing.TrialConsumed |= trialConsumed; try { diff --git a/src/Backend/AutopilotMonitor.Functions/Functions/Config/PlanManagementFunction.cs b/src/Backend/AutopilotMonitor.Functions/Functions/Config/PlanManagementFunction.cs index e3ba4984..cb3d03b8 100644 --- a/src/Backend/AutopilotMonitor.Functions/Functions/Config/PlanManagementFunction.cs +++ b/src/Backend/AutopilotMonitor.Functions/Functions/Config/PlanManagementFunction.cs @@ -1,5 +1,6 @@ using System.Net; using System.Text.Json; +using AutopilotMonitor.Functions.Functions.Admin; using AutopilotMonitor.Functions.Helpers; using AutopilotMonitor.Functions.Security; using AutopilotMonitor.Functions.Services; @@ -198,33 +199,36 @@ namespace AutopilotMonitor.Functions.Functions.Config } } - var config = await _configService.GetConfigurationIfExistsAsync(requestCtx.TargetTenantId); - if (config == null) - { - return await req.NotFoundAsync("Tenant not found"); - } - var nowUtc = _time.GetUtcNow().UtcDateTime; var changes = new Dictionary(); + var wasPermanentPro = false; + TenantEdition editionBefore = default, editionAfter = default; - // Conferred Pro follows the PERMANENT tier of the managing tenant (a trial confers nothing): - // remember the standing before the mutation so a permanent-Pro → non-permanent drop can - // stamp the managed tenants' grace anchors below. - var wasPermanentPro = FeatureEntitlementCatalog.IsPermanentProTier(config.PlanTier); - var (editionBefore, editionAfter) = - ApplyPlanChanges(config, newPlanTier, trialProvided, newTrialExpiresUtc, caller, nowUtc, changes); - ApplyDelegatedSlotChange(config, slotsProvided, newMaxDelegatedTenants, changes); - ApplyMcpUsagePlanChange(config, mcpPlanProvided, newMcpUsagePlan, changes); - ApplyPayingCustomerChange(config, payingProvided, newPayingCustomer, changes); - - if (changes.Count > 0) + // Decided on the fresh row and written under its ETag; the mutation re-runs on a lost race. + var update = await _configService.UpdateAsync(requestCtx.TargetTenantId, row => { - config.UpdatedBy = caller; - // Fail-loud: SaveConfigurationAsync throws when the write did not persist (and - // invalidates the cached — possibly mutated — instance in its finally), so a - // failed save can never be audited or returned as 200. - await _configService.SaveConfigurationAsync(config, "plan", "plan tier change"); + changes.Clear(); + // Conferred Pro follows the PERMANENT tier of the managing tenant (a trial confers nothing): + // remember the standing before the mutation so a permanent-Pro → non-permanent drop can + // stamp the managed tenants' grace anchors below. + wasPermanentPro = FeatureEntitlementCatalog.IsPermanentProTier(row.PlanTier); + (editionBefore, editionAfter) = + ApplyPlanChanges(row, newPlanTier, trialProvided, newTrialExpiresUtc, caller, nowUtc, changes); + ApplyDelegatedSlotChange(row, slotsProvided, newMaxDelegatedTenants, changes); + ApplyMcpUsagePlanChange(row, mcpPlanProvided, newMcpUsagePlan, changes); + ApplyPayingCustomerChange(row, payingProvided, newPayingCustomer, changes); + if (changes.Count == 0) + return false; + row.UpdatedBy = caller; + return true; + }, "plan", "plan tier change"); + if (await UpdateFailureAsync(req, update) is { } failure) + return failure; + var config = update.Config!; + + if (update.Status == TenantConfigUpdateStatus.Updated) + { await _maintenanceRepo.LogAuditEntryAsync( requestCtx.TargetTenantId, "UPDATE", @@ -403,10 +407,27 @@ namespace AutopilotMonitor.Functions.Functions.Config => definitions.Select(d => TenantEntitlementService.NormalizePlanName(d.Name)).Where(n => n != null).Select(n => n!).OrderBy(n => n, StringComparer.Ordinal); /// - /// POST /api/config/{tenantId}/trial — TenantAdminOrGA (catalog-enforced). Self-service - /// 30-day Pro trial, exactly once per tenant. 409 when the trial was already - /// consumed or the tenant is already effectively Pro. + /// The response for an outcome that wrote + /// nothing for a reason other than the caller's own decision; null for Updated and Declined. /// + private static async Task UpdateFailureAsync(HttpRequestData req, TenantConfigUpdate update) + { + switch (update.Status) + { + case TenantConfigUpdateStatus.NotFound: + return await req.NotFoundAsync("Tenant not found"); + case TenantConfigUpdateStatus.OffboardingInProgress: + // The answer the suspension gate gives once its cache has caught up with the tombstone. + return await req.ErrorAsync(HttpStatusCode.Forbidden, + Constants.ApiErrorCodes.TenantSuspended, TenantOffboardFunction.OffboardingDisabledReason); + case TenantConfigUpdateStatus.Conflict: + return await req.ErrorAsync(HttpStatusCode.Conflict, + Constants.ApiErrorCodes.Conflict, "The tenant configuration was changed concurrently. Retry shortly."); + default: + return null; + } + } + /// /// Pure verdict for the self-service trial start; null = allowed. Order matters and is /// test-pinned: the terminal conditions (trial consumed, already Pro) win over the @@ -453,6 +474,12 @@ namespace AutopilotMonitor.Functions.Functions.Config return missing; } + /// + /// POST /api/config/{tenantId}/trial — TenantAdminOrGA (catalog-enforced). Self-service + /// 30-day Pro trial, exactly once per tenant — also across offboarding and re-onboarding + /// (the offboarding pointer carries over). + /// 409 when the trial was already consumed or the tenant is already effectively Pro. + /// [Function("StartTenantTrial")] public async Task StartTrial( [HttpTrigger(AuthorizationLevel.Anonymous, "post", Route = "config/{tenantId}/trial")] HttpRequestData req, @@ -464,29 +491,31 @@ namespace AutopilotMonitor.Functions.Functions.Config var caller = requestCtx.UserPrincipalName ?? "Unknown"; _logger.LogInformation("StartTenantTrial: tenantId={TenantId} by {User}", requestCtx.TargetTenantId, caller); - var config = await _configService.GetConfigurationIfExistsAsync(requestCtx.TargetTenantId); - if (config == null) - { - return await req.NotFoundAsync("Tenant not found"); - } - var nowUtc = _time.GetUtcNow().UtcDateTime; + (string Code, string Message)? deny = null; - if (EvaluateTrialStart(config, nowUtc) is { } deny) + // The verdict runs on the fresh row inside the conditional write: two concurrent + // starts cannot both see TrialConsumed=false, and a stale cached row is never saved. + var update = await _configService.UpdateAsync(requestCtx.TargetTenantId, row => { - return await req.ErrorAsync(HttpStatusCode.Conflict, deny.Code, deny.Message); - } + deny = EvaluateTrialStart(row, nowUtc); + if (deny != null) + return false; + row.TrialStartedUtc = nowUtc; + row.TrialExpiresUtc = nowUtc.AddDays(SelfServiceTrialDays); + row.TrialConsumed = true; + row.TrialGrantedBy = caller; + // Effectively Pro again — a leftover retention grace anchor is obsolete. + row.ProDowngradedUtc = null; + row.UpdatedBy = caller; + return true; + }, "plan", "self-service trial start"); - config.TrialStartedUtc = nowUtc; - config.TrialExpiresUtc = nowUtc.AddDays(SelfServiceTrialDays); - config.TrialConsumed = true; - config.TrialGrantedBy = caller; - // Effectively Pro again — a leftover retention grace anchor is obsolete. - config.ProDowngradedUtc = null; - config.UpdatedBy = caller; - - // Fail-loud: throws when the write did not persist (cache invalidated in finally). - await _configService.SaveConfigurationAsync(config, "plan", "self-service trial start"); + if (await UpdateFailureAsync(req, update) is { } failure) + return failure; + if (deny is { } refused) + return await req.ErrorAsync(HttpStatusCode.Conflict, refused.Code, refused.Message); + var config = update.Config!; await _maintenanceRepo.LogAuditEntryAsync( requestCtx.TargetTenantId, diff --git a/src/Backend/AutopilotMonitor.Functions/Functions/Infrastructure/AuthFunction.cs b/src/Backend/AutopilotMonitor.Functions/Functions/Infrastructure/AuthFunction.cs index 00462a72..fa2344ad 100644 --- a/src/Backend/AutopilotMonitor.Functions/Functions/Infrastructure/AuthFunction.cs +++ b/src/Backend/AutopilotMonitor.Functions/Functions/Infrastructure/AuthFunction.cs @@ -8,6 +8,7 @@ using AutopilotMonitor.Functions.Services; using AutopilotMonitor.Shared; using AutopilotMonitor.Shared.DataAccess; using AutopilotMonitor.Shared.Models; +using AutopilotMonitor.Shared.Models.Offboarding; using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Mvc; using Microsoft.Azure.Functions.Worker; @@ -36,6 +37,7 @@ public class AuthFunction private readonly AdminIdentityResolver _identityResolver; private readonly ISignalRNotificationService _signalRService; private readonly AdminConfigurationService _adminConfigService; + private readonly IOffboardingAuditRepository _offboardingRepo; public AuthFunction( ILogger logger, @@ -52,9 +54,11 @@ public class AuthFunction EntraAppRegistry appRegistry, AdminIdentityResolver identityResolver, ISignalRNotificationService signalRService, - AdminConfigurationService adminConfigService) + AdminConfigurationService adminConfigService, + IOffboardingAuditRepository offboardingRepo) { _logger = logger; + _offboardingRepo = offboardingRepo; _adminConfigService = adminConfigService; _identityResolver = identityResolver; _signalRService = signalRService; @@ -382,6 +386,23 @@ public class AuthFunction return; } + // A returning tenant: the offboarding deleted its configuration, so the pointer is the only + // record of a consumed self-service trial. Carried over in this same write as OnboardedBy, so + // no onboarded row (and no auto-promoted admin) ever exists without it (D-288). Unreadable ⇒ + // no write at all; the self-gate above retries on the next login. + OffboardingByTenantPointer? offboardingPointer; + try + { + (offboardingPointer, _) = await _offboardingRepo.TryGetByTenantPointerAsync(tenantId.ToLowerInvariant()); + } + catch (Exception ex) + { + _logger.LogWarning(ex, "First-login offboarding lookup failed for tenant {TenantId} — will retry on next login", tenantId); + return; + } + if (offboardingPointer is { TrialConsumed: true }) + tenantConfig.TrialConsumed = true; + _logger.LogInformation("Setting domain name for tenant {TenantId}: {Domain}", tenantId, domain); tenantConfig.DomainName = domain; tenantConfig.UpdatedBy = upn; diff --git a/src/Backend/AutopilotMonitor.Functions/Services/TenantConfigurationService.cs b/src/Backend/AutopilotMonitor.Functions/Services/TenantConfigurationService.cs index f99960ba..0e0e7317 100644 --- a/src/Backend/AutopilotMonitor.Functions/Services/TenantConfigurationService.cs +++ b/src/Backend/AutopilotMonitor.Functions/Services/TenantConfigurationService.cs @@ -2,6 +2,7 @@ using System.Collections.Concurrent; using System; using System.Collections.Generic; using System.Threading.Tasks; +using AutopilotMonitor.Functions.Functions.Admin; using AutopilotMonitor.Shared.DataAccess; using AutopilotMonitor.Shared.Models; using AutopilotMonitor.Shared.Pagination; @@ -137,8 +138,9 @@ namespace AutopilotMonitor.Functions.Services /// swallows storage exceptions and reports failure via its bool return, so ignoring it would /// let callers audit + return 200 for a save that never happened (Codex finding, 2026-07-07). /// The cached row is invalidated on EVERY attempted save (finally): on success it is stale; - /// on failure the caller may have mutated the cached instance in place (plan/trial endpoints), - /// which must not linger as phantom-saved state for the 5-minute TTL. + /// on failure the caller may have mutated the cached instance in place, which must not linger + /// as phantom-saved state for the 5-minute TTL. Callers that decide on the current state or + /// could race another writer use instead. /// public virtual Task SaveConfigurationAsync(TenantConfiguration config) => SaveConfigurationAsync(config, backupSource: null, backupReason: null); @@ -180,6 +182,64 @@ namespace AutopilotMonitor.Functions.Services } } + internal const int MaxUpdateAttempts = 3; + + /// + /// Read-modify-write for a caller that persists the WHOLE entity and decides on its current + /// state: runs on the FRESH, projected row and the write is + /// conditional on that row's ETag. A cached view is up to 5 minutes stale per instance, so + /// saving it would rewind whatever another writer changed meanwhile (the 2026-07-31 + /// HomedAppClientId class) — including the offboarding tombstone during its drain barrier. + /// + /// On a lost race the row is re-read and runs again, so it must + /// derive everything from the row it is handed. Returning false declines the write. The + /// offboarding tombstone is never written: the cascade owns the row until it deletes it. + /// Storage errors other than a lost race throw. + /// + /// + public virtual async Task UpdateAsync( + string tenantId, Func mutate, string backupSource, string backupReason) + { + for (var attempt = 1; attempt <= MaxUpdateAttempts; attempt++) + { + var read = await _configRepo.GetTenantConfigurationWithEtagAsync(tenantId); + if (read == null) + return new TenantConfigUpdate(TenantConfigUpdateStatus.NotFound, null); + + var (config, etag) = read.Value; + await ProjectAsync(config); + + if (TenantOffboardFunction.IsOffboardingTombstone(config)) + return new TenantConfigUpdate(TenantConfigUpdateStatus.OffboardingInProgress, config); + + if (!mutate(config)) + return new TenantConfigUpdate(TenantConfigUpdateStatus.Declined, config); + + config.LastUpdated = DateTime.UtcNow; + bool replaced; + try + { + replaced = await _configRepo.TryReplaceTenantConfigurationAsync(config, etag, backupSource, backupReason); + } + finally + { + InvalidateCache(tenantId); + } + + if (replaced) + { + _logger.LogInformation("Configuration updated for tenant {TenantId} by {UpdatedBy}", tenantId, config.UpdatedBy); + return new TenantConfigUpdate(TenantConfigUpdateStatus.Updated, config); + } + + _logger.LogInformation( + "Configuration update for tenant {TenantId} lost the ETag race (attempt {Attempt}/{Max})", + tenantId, attempt, MaxUpdateAttempts); + } + + return new TenantConfigUpdate(TenantConfigUpdateStatus.Conflict, null); + } + /// /// One-way seed of the tenant contact address: writes only while /// the tenant has none, and only if nothing else wrote the configuration in the meantime. @@ -342,4 +402,22 @@ namespace AutopilotMonitor.Functions.Services } } } + + public enum TenantConfigUpdateStatus + { + Updated, + /// The mutation returned false; nothing was written. + Declined, + NotFound, + /// The row is the offboarding tombstone; nothing was written. + OffboardingInProgress, + /// Every conditional write lost the ETag race. + Conflict, + } + + /// + /// Outcome of . is the + /// row as written (Updated) or as read (Declined, OffboardingInProgress); null otherwise. + /// + public sealed record TenantConfigUpdate(TenantConfigUpdateStatus Status, TenantConfiguration? Config); } diff --git a/src/Shared/AutopilotMonitor.Shared/DataAccess/IConfigRepository.cs b/src/Shared/AutopilotMonitor.Shared/DataAccess/IConfigRepository.cs index 4294f82c..ff986fd6 100644 --- a/src/Shared/AutopilotMonitor.Shared/DataAccess/IConfigRepository.cs +++ b/src/Shared/AutopilotMonitor.Shared/DataAccess/IConfigRepository.cs @@ -40,6 +40,14 @@ namespace AutopilotMonitor.Shared.DataAccess /// Task TryReplaceTenantConfigurationAsync(TenantConfiguration config, string etag); + /// + /// Conditional full replace with the SAME fail-soft pre-write backup as + /// — the + /// If-Match variant of the ordinary save, for read-modify-write callers without a + /// fail-closed snapshot of their own. Return and throw semantics as the 2-arg overload. + /// + Task TryReplaceTenantConfigurationAsync(TenantConfiguration config, string etag, string? backupSource, string? backupReason); + Task> GetAllTenantConfigurationsAsync(); /// diff --git a/src/Shared/AutopilotMonitor.Shared/Models/Offboarding/OffboardingByTenantPointer.cs b/src/Shared/AutopilotMonitor.Shared/Models/Offboarding/OffboardingByTenantPointer.cs index 8a91dc05..77bb6c93 100644 --- a/src/Shared/AutopilotMonitor.Shared/Models/Offboarding/OffboardingByTenantPointer.cs +++ b/src/Shared/AutopilotMonitor.Shared/Models/Offboarding/OffboardingByTenantPointer.cs @@ -38,5 +38,10 @@ namespace AutopilotMonitor.Shared.Models.Offboarding /// How many distinct offboarding rounds this tenant has been through. /// Incremented on each new Initiate. Useful for "Tenant ging und kam zurück"-Reporting. public int OffboardCount { get; set; } = 1; + + /// Sticky: true once any offboarded configuration had consumed the self-service Pro trial. + /// The offboarding deletes that configuration, so this is the only record left; the first login of the + /// returning tenant copies it onto the new configuration (the trial stays once per tenant). + public bool TrialConsumed { get; set; } } }