diff --git a/src/Backend/AutopilotMonitor.Functions.Tests/AuthFunctionSideEffectTests.cs b/src/Backend/AutopilotMonitor.Functions.Tests/AuthFunctionSideEffectTests.cs index cca637bf..10dc7440 100644 --- a/src/Backend/AutopilotMonitor.Functions.Tests/AuthFunctionSideEffectTests.cs +++ b/src/Backend/AutopilotMonitor.Functions.Tests/AuthFunctionSideEffectTests.cs @@ -55,7 +55,7 @@ public class AuthFunctionSideEffectTests { CallBase = false }; var previewMock = new Mock( - configRepo, cache, Mock.Of>()) + configRepo, cache, Mock.Of>(), _tenantConfigMock.Object) { CallBase = false }; _telegramMock = new Mock( diff --git a/src/Backend/AutopilotMonitor.Functions.Tests/TenantContactEmailTests.cs b/src/Backend/AutopilotMonitor.Functions.Tests/TenantContactEmailTests.cs index 559dcc58..95f0735c 100644 --- a/src/Backend/AutopilotMonitor.Functions.Tests/TenantContactEmailTests.cs +++ b/src/Backend/AutopilotMonitor.Functions.Tests/TenantContactEmailTests.cs @@ -1,4 +1,19 @@ +using System; +using System.Collections.Generic; +using System.Threading; +using System.Threading.Tasks; +using Azure; +using Azure.Data.Tables; +using AutopilotMonitor.Functions.DataAccess.TableStorage; +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; +using Microsoft.Extensions.Logging.Abstractions; +using Moq; using Xunit; namespace AutopilotMonitor.Functions.Tests; @@ -9,6 +24,8 @@ namespace AutopilotMonitor.Functions.Tests; /// public class TenantContactEmailTests { + private const string TenantId = "77777777-7777-7777-7777-777777777777"; + [Fact] public void ContactEmail_defaults_to_null_so_absence_is_distinguishable_from_empty() { @@ -22,4 +39,348 @@ public class TenantContactEmailTests var config = new TenantConfiguration { ContactEmail = "it-operations@contoso.com" }; Assert.Equal("it-operations@contoso.com", config.ContactEmail); } + + // ------------------------------------------------------------------ + // Server-side validation (UpdateTenantConfigurationFunction) + // ------------------------------------------------------------------ + + [Theory] + [InlineData("it-operations@contoso.com")] + [InlineData("first.last+autopilot@sub.fabrikam.co.uk")] + [InlineData("UPPER@CONTOSO.COM")] + public void ValidateContactEmail_accepts_real_addresses(string email) + => Assert.Null(UpdateTenantConfigurationFunction.ValidateContactEmail(email)); + + [Theory] + [InlineData(null)] + [InlineData("")] + [InlineData(" ")] + public void ValidateContactEmail_treats_absence_as_valid(string? email) + { + // No contact address is a legitimate state — it means we cannot reach this tenant. + Assert.Null(UpdateTenantConfigurationFunction.ValidateContactEmail(email)); + } + + [Theory] + [InlineData("not-an-address", "single")] // no @ at all + [InlineData("@contoso.com", "single")] // empty local part + [InlineData("ops@", "single")] // empty domain + [InlineData("ops@@contoso.com", "single")] // two @ + [InlineData("ops@localhost", "dotted")] // unreachable bare host + [InlineData("ops@.contoso.com", "dotted")] // leading dot + [InlineData("ops@contoso.", "dotted")] // trailing dot + [InlineData("ops@contoso.com, attacker@evil.test", "single address")] // recipient list + [InlineData("ops@contoso.com; attacker@evil.test", "single address")] + [InlineData("Ops Team ", "single address")] // display-name form + public void ValidateContactEmail_rejects_values_that_are_not_a_single_address(string email, string expectedHint) + { + var error = UpdateTenantConfigurationFunction.ValidateContactEmail(email); + Assert.NotNull(error); + Assert.Contains(expectedHint, error!, StringComparison.OrdinalIgnoreCase); + } + + [Fact] + public void ValidateContactEmail_rejects_header_injection() + { + // Once this address is actually mailed, an embedded CR/LF would forge mail headers. + var error = UpdateTenantConfigurationFunction.ValidateContactEmail( + $"ops@contoso.com{(char)13}{(char)10}Bcc: attacker@evil.test"); + + Assert.NotNull(error); + } + + [Fact] + public void ValidateContactEmail_rejects_an_over_long_value() + { + var local = new string('a', UpdateTenantConfigurationFunction.MaxContactEmailLength); + var error = UpdateTenantConfigurationFunction.ValidateContactEmail($"{local}@contoso.com"); + + Assert.NotNull(error); + Assert.Contains("at most", error!); + } + + // ------------------------------------------------------------------ + // Seed contract (TenantConfigurationService — the single owner) + // ------------------------------------------------------------------ + + private static (TenantConfigurationService service, Mock repo, MemoryCache cache) BuildService() + { + var repo = new Mock(); + var cache = new MemoryCache(new MemoryCacheOptions()); + var service = new TenantConfigurationService( + repo.Object, NullLogger.Instance, cache); + return (service, repo, cache); + } + + [Fact] + public async Task TrySeedContactEmail_when_the_write_lands_reports_success_and_drops_the_cached_row() + { + var (service, repo, cache) = BuildService(); + repo.Setup(r => r.TrySeedTenantContactEmailAsync(TenantId, "ops@contoso.com")).ReturnsAsync(true); + cache.Set($"tenant-config:{TenantId}", new TenantConfiguration { TenantId = TenantId }); + + Assert.True(await service.TrySeedContactEmailAsync(TenantId, "ops@contoso.com")); + + // The repository writes behind this cache; without the eviction the freshly seeded + // address would stay invisible for the full 5-minute TTL. + Assert.False(cache.TryGetValue($"tenant-config:{TenantId}", out _)); + } + + [Fact] + public async Task TrySeedContactEmail_when_the_tenant_already_owns_an_address_leaves_the_cache_alone() + { + var (service, repo, cache) = BuildService(); + repo.Setup(r => r.TrySeedTenantContactEmailAsync(It.IsAny(), It.IsAny())) + .ReturnsAsync(false); + var cached = new TenantConfiguration { TenantId = TenantId, ContactEmail = "owner@contoso.com" }; + cache.Set($"tenant-config:{TenantId}", cached); + + Assert.False(await service.TrySeedContactEmailAsync(TenantId, "ops@contoso.com")); + + Assert.True(cache.TryGetValue($"tenant-config:{TenantId}", out TenantConfiguration? still)); + Assert.Equal("owner@contoso.com", still!.ContactEmail); + } + + [Theory] + [InlineData(null)] + [InlineData("")] + [InlineData(" ")] + public async Task TrySeedContactEmail_with_no_address_never_reaches_storage(string? email) + { + var (service, repo, _) = BuildService(); + + Assert.False(await service.TrySeedContactEmailAsync(TenantId, email)); + + repo.Verify(r => r.TrySeedTenantContactEmailAsync(It.IsAny(), It.IsAny()), Times.Never); + } + + [Fact] + public async Task TrySeedContactEmail_with_no_tenant_never_reaches_storage() + { + var (service, repo, _) = BuildService(); + + Assert.False(await service.TrySeedContactEmailAsync(" ", "ops@contoso.com")); + + repo.Verify(r => r.TrySeedTenantContactEmailAsync(It.IsAny(), It.IsAny()), Times.Never); + } + + [Fact] + public async Task TrySeedContactEmail_trims_before_it_reaches_storage() + { + var (service, repo, _) = BuildService(); + repo.Setup(r => r.TrySeedTenantContactEmailAsync(TenantId, "ops@contoso.com")).ReturnsAsync(true); + + Assert.True(await service.TrySeedContactEmailAsync(TenantId, " ops@contoso.com ")); + + repo.Verify(r => r.TrySeedTenantContactEmailAsync(TenantId, "ops@contoso.com"), Times.Once); + } + + [Fact] + public async Task TrySeedContactEmail_never_writes_the_whole_configuration() + { + // Regression guard: the seed used to be a read-modify-write of the full model through + // SaveTenantConfigurationAsync, which replaces the row unconditionally — a concurrent + // portal save was clobbered by the seeder's stale snapshot of EVERY field. + var (service, repo, _) = BuildService(); + repo.Setup(r => r.TrySeedTenantContactEmailAsync(It.IsAny(), It.IsAny())) + .ReturnsAsync(true); + + await service.TrySeedContactEmailAsync(TenantId, "ops@contoso.com"); + + repo.Verify(r => r.SaveTenantConfigurationAsync(It.IsAny()), Times.Never); + } + + // ------------------------------------------------------------------ + // Seed trigger (PreviewWhitelistService) + // ------------------------------------------------------------------ + + private static (PreviewWhitelistService preview, Mock repo) BuildPreview() + { + var repo = new Mock(); + var cache = new MemoryCache(new MemoryCacheOptions()); + var tenantConfig = new TenantConfigurationService( + repo.Object, NullLogger.Instance, cache); + var preview = new PreviewWhitelistService( + repo.Object, cache, NullLogger.Instance, tenantConfig); + return (preview, repo); + } + + [Fact] + public async Task SaveNotificationEmail_persists_the_address_and_seeds_the_contact_field() + { + var (preview, repo) = BuildPreview(); + repo.Setup(r => r.TrySeedTenantContactEmailAsync(TenantId, "ops@contoso.com")).ReturnsAsync(true); + + await preview.SaveNotificationEmailAsync(TenantId, "ops@contoso.com"); + + repo.Verify(r => r.SaveNotificationEmailAsync(TenantId, "ops@contoso.com"), Times.Once); + repo.Verify(r => r.TrySeedTenantContactEmailAsync(TenantId, "ops@contoso.com"), Times.Once); + } + + [Fact] + public async Task SaveNotificationEmail_clearing_the_address_does_not_seed() + { + var (preview, repo) = BuildPreview(); + + await preview.SaveNotificationEmailAsync(TenantId, null); + + repo.Verify(r => r.SaveNotificationEmailAsync(TenantId, null), Times.Once); + repo.Verify(r => r.TrySeedTenantContactEmailAsync(It.IsAny(), It.IsAny()), Times.Never); + } + + [Fact] + public async Task SaveNotificationEmail_still_succeeds_when_the_seed_blows_up() + { + // The notification email has already been persisted at that point; a failing side + // effect must not turn a completed write into a failed request. + var (preview, repo) = BuildPreview(); + repo.Setup(r => r.TrySeedTenantContactEmailAsync(It.IsAny(), It.IsAny())) + .ThrowsAsync(new RequestFailedException(503, "throttled")); + + await preview.SaveNotificationEmailAsync(TenantId, "ops@contoso.com"); + + repo.Verify(r => r.SaveNotificationEmailAsync(TenantId, "ops@contoso.com"), Times.Once); + } + + // ------------------------------------------------------------------ + // Conditional write (TableConfigRepository) — the actual invariant + // ------------------------------------------------------------------ + + private sealed class RepoHarness + { + public TableConfigRepository Sut { get; } + public Mock Table { get; } = new(); + public List<(TableEntity Entity, ETag IfMatch, TableUpdateMode Mode)> Updates { get; } = new(); + + public RepoHarness(TableEntity? stored, RequestFailedException? throwOnUpdate = null) + { + if (stored == null) + { + Table.Setup(c => c.GetEntityAsync( + It.IsAny(), It.IsAny(), + It.IsAny>(), It.IsAny())) + .ThrowsAsync(new RequestFailedException(404, "Not Found", "ResourceNotFound", null)); + } + else + { + Table.Setup(c => c.GetEntityAsync( + It.IsAny(), It.IsAny(), + It.IsAny>(), It.IsAny())) + .ReturnsAsync(Response.FromValue(stored, Mock.Of())); + } + + Table.Setup(c => c.UpdateEntityAsync( + It.IsAny(), It.IsAny(), + It.IsAny(), It.IsAny())) + .Returns((e, tag, mode, _) => + { + if (throwOnUpdate != null) + throw throwOnUpdate; + Updates.Add((e, tag, mode)); + return Task.FromResult(Mock.Of()); + }); + + var serviceClient = new Mock(); + serviceClient.Setup(c => c.GetTableClient(It.IsAny())).Returns(Table.Object); + + Sut = new TableConfigRepository( + new TableStorageService(serviceClient.Object, NullLogger.Instance), + NullLogger.Instance); + } + } + + private static TableEntity StoredConfig(string? contactEmail, string etag = "W/\"stored\"") + { + var entity = new TableEntity(TenantId, "config") { ["DomainName"] = "contoso.com" }; + if (contactEmail != null) + entity["ContactEmail"] = contactEmail; + entity.ETag = new ETag(etag); + return entity; + } + + [Fact] + public async Task Repo_seeds_only_the_contact_field_and_only_conditionally() + { + var harness = new RepoHarness(StoredConfig(contactEmail: null)); + + Assert.True(await harness.Sut.TrySeedTenantContactEmailAsync(TenantId, " ops@contoso.com ")); + + var (entity, ifMatch, mode) = Assert.Single(harness.Updates); + // Merge, not Replace: every other stored property must survive untouched. + Assert.Equal(TableUpdateMode.Merge, mode); + Assert.Equal(new ETag("W/\"stored\""), ifMatch); + Assert.Equal("ops@contoso.com", entity["ContactEmail"]); + Assert.False(entity.ContainsKey("DomainName")); + } + + [Fact] + public async Task Repo_never_overwrites_an_address_the_tenant_already_owns() + { + var harness = new RepoHarness(StoredConfig("owner@contoso.com")); + + Assert.False(await harness.Sut.TrySeedTenantContactEmailAsync(TenantId, "ops@contoso.com")); + + Assert.Empty(harness.Updates); + } + + [Fact] + public async Task Repo_treats_a_whitespace_only_stored_value_as_absent() + { + var harness = new RepoHarness(StoredConfig(" ")); + + Assert.True(await harness.Sut.TrySeedTenantContactEmailAsync(TenantId, "ops@contoso.com")); + + Assert.Single(harness.Updates); + } + + [Fact] + public async Task Repo_loses_the_race_rather_than_clobbering_a_concurrent_write() + { + // Someone wrote the row between our read and our write. The ETag precondition turns + // that into a 412; the seed abandons rather than overwriting the winner. + var harness = new RepoHarness( + StoredConfig(contactEmail: null), + throwOnUpdate: new RequestFailedException(412, "Precondition Failed", "UpdateConditionNotSatisfied", null)); + + Assert.False(await harness.Sut.TrySeedTenantContactEmailAsync(TenantId, "ops@contoso.com")); + } + + [Fact] + public async Task Repo_with_no_configuration_row_reports_no_seed() + { + var harness = new RepoHarness(stored: null); + + Assert.False(await harness.Sut.TrySeedTenantContactEmailAsync(TenantId, "ops@contoso.com")); + + Assert.Empty(harness.Updates); + } + + [Fact] + public async Task Repo_is_fail_soft_on_a_storage_fault() + { + // Callers are side effects of an already-committed write — a lost seed is recoverable + // on the next maintenance run, a thrown exception is not. + var harness = new RepoHarness( + StoredConfig(contactEmail: null), + throwOnUpdate: new RequestFailedException(503, "Server Busy")); + + Assert.False(await harness.Sut.TrySeedTenantContactEmailAsync(TenantId, "ops@contoso.com")); + } + + [Theory] + [InlineData(null, "ops@contoso.com")] + [InlineData("", "ops@contoso.com")] + [InlineData(TenantId, null)] + [InlineData(TenantId, " ")] + public async Task Repo_rejects_incomplete_input_without_touching_storage(string? tenantId, string? email) + { + var harness = new RepoHarness(StoredConfig(contactEmail: null)); + + Assert.False(await harness.Sut.TrySeedTenantContactEmailAsync(tenantId!, email!)); + + Assert.Empty(harness.Updates); + harness.Table.Verify(c => c.GetEntityAsync( + 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 37375727..c87cae89 100644 --- a/src/Backend/AutopilotMonitor.Functions.Tests/TenantOffboardFunctionTests.cs +++ b/src/Backend/AutopilotMonitor.Functions.Tests/TenantOffboardFunctionTests.cs @@ -87,7 +87,11 @@ public sealed class TenantOffboardFunctionTests return new PreviewWhitelistService( configRepoMock.Object, cache, - NullLogger.Instance); + NullLogger.Instance, + new TenantConfigurationService( + configRepoMock.Object, + NullLogger.Instance, + cache)); } /// diff --git a/src/Backend/AutopilotMonitor.Functions/DataAccess/TableStorage/TableConfigRepository.cs b/src/Backend/AutopilotMonitor.Functions/DataAccess/TableStorage/TableConfigRepository.cs index 51888f8d..47e4097c 100644 --- a/src/Backend/AutopilotMonitor.Functions/DataAccess/TableStorage/TableConfigRepository.cs +++ b/src/Backend/AutopilotMonitor.Functions/DataAccess/TableStorage/TableConfigRepository.cs @@ -71,6 +71,54 @@ namespace AutopilotMonitor.Functions.DataAccess.TableStorage } } + /// + /// Conditional, single-property seed of ContactEmail (see ). + /// Merge mode leaves every other stored property untouched, and the ETag precondition turns + /// a concurrent write into a 412 rather than a silent overwrite. Both matter: the caller is + /// a background/side-effect path that must never beat a tenant admin's own edit. + /// + public async Task TrySeedTenantContactEmailAsync(string tenantId, string email) + { + if (string.IsNullOrWhiteSpace(tenantId) || string.IsNullOrWhiteSpace(email)) + return false; + + try + { + var existing = await _tenantConfigTableClient.GetEntityAsync(tenantId, "config"); + + // The tenant already owns an address — the seed is one-way and never re-syncs. + if (!string.IsNullOrWhiteSpace(existing.Value.GetString("ContactEmail"))) + return false; + + var patch = new TableEntity(tenantId, "config") + { + ["ContactEmail"] = email.Trim() + }; + + await _tenantConfigTableClient.UpdateEntityAsync( + patch, existing.Value.ETag, TableUpdateMode.Merge); + return true; + } + catch (RequestFailedException ex) when (ex.Status == 404) + { + // No configuration row yet — nothing to seed onto. + return false; + } + catch (RequestFailedException ex) when (ex.Status == 412) + { + // Someone wrote the row between the read and the write. They win by construction: + // a lost seed is recoverable (the backfill retries), a lost admin edit is not. + _logger.LogInformation( + "Contact address seed for {TenantId} skipped — the configuration changed concurrently", tenantId); + return false; + } + catch (Exception ex) + { + _logger.LogWarning(ex, "Error seeding contact address for tenant {TenantId}", tenantId); + return false; + } + } + public async Task> GetAllTenantConfigurationsAsync() { try diff --git a/src/Backend/AutopilotMonitor.Functions/Functions/Config/UpdateTenantConfigurationFunction.cs b/src/Backend/AutopilotMonitor.Functions/Functions/Config/UpdateTenantConfigurationFunction.cs index fdb41da8..cef4a656 100644 --- a/src/Backend/AutopilotMonitor.Functions/Functions/Config/UpdateTenantConfigurationFunction.cs +++ b/src/Backend/AutopilotMonitor.Functions/Functions/Config/UpdateTenantConfigurationFunction.cs @@ -76,6 +76,19 @@ namespace AutopilotMonitor.Functions.Functions.Config return badRequest; } + // The contact address is where enforcement actions and service notices are sent, + // so it must be an address rather than whatever string a direct API caller posts. + var contactEmailError = ValidateContactEmail(config.ContactEmail); + if (contactEmailError != null) + { + var badRequest = req.CreateResponse(HttpStatusCode.BadRequest); + await badRequest.WriteAsJsonAsync(new { success = false, message = $"Invalid contact email: {contactEmailError}" }); + return badRequest; + } + // Normalize so the stored value never carries surrounding whitespace, and an + // all-whitespace submission clears the field instead of masquerading as a value. + config.ContactEmail = string.IsNullOrWhiteSpace(config.ContactEmail) ? null : config.ContactEmail.Trim(); + // Load the stored config up-front so we can (1) restore any redacted secret placeholders // before validation/save and (2) protect GA-only fields below. var existingConfig = await _configService.GetConfigurationAsync(requestCtx.TargetTenantId); @@ -251,6 +264,52 @@ namespace AutopilotMonitor.Functions.Functions.Config private const int MaxCustomHeaderCount = 25; private const int MaxNotificationChannelsJsonLength = 65536; + // RFC 5321 caps a forward path at 254 characters. + internal const int MaxContactEmailLength = 254; + + /// + /// Validates the tenant contact address. Returns an error message, or null when valid/empty. + /// Empty is legitimate — it means we have no way to reach the tenant. + /// + /// Deliberately not an RFC 5322 parser: the job is to reject values that are not addresses + /// at all. Specifically it rejects recipient lists (a comma would silently widen who receives + /// service notices), display-name forms, and control characters (which would let a caller + /// forge mail headers once this address is actually mailed). + /// + /// + internal static string? ValidateContactEmail(string? email) + { + if (string.IsNullOrWhiteSpace(email)) + return null; + + var trimmed = email.Trim(); + + if (trimmed.Length > MaxContactEmailLength) + return $"must be at most {MaxContactEmailLength} characters."; + + foreach (var ch in trimmed) + { + if (char.IsControl(ch)) + return "must not contain control characters."; + if (char.IsWhiteSpace(ch) || ch == ',' || ch == ';' || ch == '<' || ch == '>') + return "must be a single address, without spaces, separators or angle brackets."; + } + + var at = trimmed.IndexOf('@'); + if (at <= 0 || at != trimmed.LastIndexOf('@') || at == trimmed.Length - 1) + return "must contain a single \"@\" with text on both sides."; + + // A bare host with no dot is unreachable from our sender, so it is a typo, not an address. + var domain = trimmed.Substring(at + 1); + if (!domain.Contains('.') || domain.StartsWith(".", StringComparison.Ordinal) + || domain.EndsWith(".", StringComparison.Ordinal)) + { + return "the domain part must be a dotted host name."; + } + + return null; + } + /// /// Validates the notification-channel list JSON. Returns an error message, or null when /// valid/empty. Strict counterpart of the fail-soft NotificationChannel.ParseList: diff --git a/src/Backend/AutopilotMonitor.Functions/Services/MaintenanceService.Aggregation.cs b/src/Backend/AutopilotMonitor.Functions/Services/MaintenanceService.Aggregation.cs index a026b4ec..d63fa25f 100644 --- a/src/Backend/AutopilotMonitor.Functions/Services/MaintenanceService.Aggregation.cs +++ b/src/Backend/AutopilotMonitor.Functions/Services/MaintenanceService.Aggregation.cs @@ -527,6 +527,13 @@ namespace AutopilotMonitor.Functions.Services /// the seed happens when the notification address is saved, so this converges to a /// no-op and can be removed once every existing tenant has been covered. /// + /// + /// The enumerated snapshot is only a cheap pre-filter; the authoritative "is it still + /// empty" check happens under an ETag inside + /// . A full-model save + /// here would write a snapshot that is minutes old by the time the loop reaches this + /// tenant, silently reverting whatever an admin changed in between. + /// /// /// Number of tenants that received a contact address. private async Task BackfillTenantContactEmailsAsync() @@ -546,9 +553,8 @@ namespace AutopilotMonitor.Functions.Services if (string.IsNullOrWhiteSpace(email)) continue; - config.ContactEmail = email.Trim(); - await _tenantConfigService.SaveConfigurationAsync(config); - filled++; + if (await _tenantConfigService.TrySeedContactEmailAsync(config.TenantId, email)) + filled++; } if (filled > 0) diff --git a/src/Backend/AutopilotMonitor.Functions/Services/PreviewWhitelistService.cs b/src/Backend/AutopilotMonitor.Functions/Services/PreviewWhitelistService.cs index 2cbab19f..a63ec9fc 100644 --- a/src/Backend/AutopilotMonitor.Functions/Services/PreviewWhitelistService.cs +++ b/src/Backend/AutopilotMonitor.Functions/Services/PreviewWhitelistService.cs @@ -16,16 +16,19 @@ public class PreviewWhitelistService private readonly IConfigRepository _configRepo; private readonly IMemoryCache _cache; private readonly ILogger _logger; + private readonly TenantConfigurationService _tenantConfigService; private static readonly TimeSpan CacheDuration = TimeSpan.FromMinutes(5); public PreviewWhitelistService( IConfigRepository configRepo, IMemoryCache cache, - ILogger logger) + ILogger logger, + TenantConfigurationService tenantConfigService) { _configRepo = configRepo; _cache = cache; _logger = logger; + _tenantConfigService = tenantConfigService; } /// @@ -104,42 +107,15 @@ public class PreviewWhitelistService /// /// Saves (or clears) the notification email for a tenant, and seeds the tenant's /// contact address from it the first time one is given. + /// + /// The seed is a side effect of a write that has already been persisted, so it must never + /// fail this call — is + /// fail-soft and owns the "never overwrite what the tenant owns" invariant. + /// /// public async Task SaveNotificationEmailAsync(string tenantId, string? email) { await _configRepo.SaveNotificationEmailAsync(tenantId, email); - await SeedContactEmailAsync(tenantId, email); - } - - /// - /// One-way seed: copies the preview notification address into - /// only while that field is still empty. - /// Once the tenant owns a contact address — seeded or edited in the portal — this never - /// touches it again, so a later change here cannot silently overwrite the tenant's choice. - /// Best-effort: the notification email has already been persisted by the caller, and a - /// failure to seed must not fail that write. - /// - private async Task SeedContactEmailAsync(string tenantId, string? email) - { - if (string.IsNullOrWhiteSpace(email)) - return; - - try - { - var config = await _configRepo.GetTenantConfigurationAsync(tenantId); - if (config == null || !string.IsNullOrWhiteSpace(config.ContactEmail)) - return; - - config.ContactEmail = email.Trim(); - await _configRepo.SaveTenantConfigurationAsync(config); - - _logger.LogInformation( - "Seeded tenant contact address for {TenantId} from the preview notification email", tenantId); - } - catch (Exception ex) - { - _logger.LogWarning(ex, - "Could not seed tenant contact address for {TenantId} — the notification email was still saved", tenantId); - } + await _tenantConfigService.TrySeedContactEmailAsync(tenantId, email); } } diff --git a/src/Backend/AutopilotMonitor.Functions/Services/TenantConfigurationService.cs b/src/Backend/AutopilotMonitor.Functions/Services/TenantConfigurationService.cs index 2868c675..cd592776 100644 --- a/src/Backend/AutopilotMonitor.Functions/Services/TenantConfigurationService.cs +++ b/src/Backend/AutopilotMonitor.Functions/Services/TenantConfigurationService.cs @@ -125,6 +125,48 @@ namespace AutopilotMonitor.Functions.Services } } + /// + /// 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. + /// Returns true when the seed landed. + /// + /// Single owner for both seed paths (preview notification-email save and the maintenance + /// backfill), because both need the same three things: the conditional single-property + /// write, the "never overwrite what the tenant owns" invariant, and the cache invalidation + /// below — the repository writes behind this cache, so without it a freshly seeded address + /// stays invisible for the 5-minute TTL. + /// + /// + /// Fail-soft by design, unlike : every caller is a side + /// effect of an operation that has already succeeded, and a lost seed is recoverable on the + /// next maintenance run. Enforced here rather than left to the repository, so the guarantee + /// holds for any implementation. + /// + /// + public virtual async Task TrySeedContactEmailAsync(string tenantId, string? email) + { + if (string.IsNullOrWhiteSpace(tenantId) || string.IsNullOrWhiteSpace(email)) + return false; + + try + { + var seeded = await _configRepo.TrySeedTenantContactEmailAsync(tenantId, email!.Trim()); + if (seeded) + { + _cache.Remove($"tenant-config:{tenantId}"); + _logger.LogInformation("Seeded contact address for tenant {TenantId}", tenantId); + } + + return seeded; + } + catch (Exception ex) + { + _logger.LogWarning(ex, + "Could not seed contact address for tenant {TenantId} — the triggering write still stands", tenantId); + return false; + } + } + /// /// Invalidates cache for a tenant (forces reload on next request) /// diff --git a/src/Shared/AutopilotMonitor.Shared/DataAccess/IConfigRepository.cs b/src/Shared/AutopilotMonitor.Shared/DataAccess/IConfigRepository.cs index 0c061736..259adfa1 100644 --- a/src/Shared/AutopilotMonitor.Shared/DataAccess/IConfigRepository.cs +++ b/src/Shared/AutopilotMonitor.Shared/DataAccess/IConfigRepository.cs @@ -24,6 +24,21 @@ namespace AutopilotMonitor.Shared.DataAccess /// Task> GetTenantConfigurationsPageAsync(int pageSize, string? continuation); + /// + /// Writes to the tenant's ContactEmail ONLY while that field is + /// still empty, and only if nothing else wrote the row in the meantime. Returns true when + /// the seed landed, false when the tenant already owns an address, has no config row, or + /// lost the race. + /// + /// Exists because the seed cannot be expressed as a read-modify-write of the whole model: + /// replaces the entire row unconditionally, so a + /// concurrent portal save would be clobbered by the seeder's stale snapshot — of every + /// field, not just this one. The implementation must therefore write conditionally and + /// touch no other property. + /// + /// + Task TrySeedTenantContactEmailAsync(string tenantId, string email); + // --- Admin Configuration --- Task GetAdminConfigurationAsync(); Task SaveAdminConfigurationAsync(AdminConfiguration config);