mirror of
https://github.com/okieselbach/AutopilotMonitor.git
synced 2026-10-24 12:42:38 +00:00
fix(config): validate ContactEmail and seed it conditionally
The tenant contact address had no server-side validation, so a direct API caller could store any string in the field service notices are sent to. UpdateTenantConfiguration now rejects non-addresses, recipient lists (a comma would silently widen who receives those notices), display-name forms and control characters (mail-header injection), and normalizes what it stores. Empty stays valid - having no contact address is a legitimate state. The seed path had a worse problem than missing validation. It was a read-modify-write of the whole model through SaveTenantConfigurationAsync, which upserts with TableUpdateMode.Replace and no ETag: a concurrent portal save was clobbered by the seeder's stale snapshot of EVERY field, not just ContactEmail. It also wrote straight through IConfigRepository, bypassing the TenantConfigurationService cache, so a seeded address stayed invisible for the full 5-minute TTL. TrySeedTenantContactEmailAsync now writes ETag-conditionally in Merge mode - one property, and a 412 means someone else wrote the row so the seed abandons rather than overwriting the winner. A lost seed is recoverable on the next maintenance run; a lost admin edit is not. The same seed logic existed twice (preview notification-email save and the maintenance backfill). Both now call a single owner on TenantConfigurationService, which holds the never-overwrite invariant, the cache eviction and the fail-soft guarantee. Fail-soft lives in the service rather than the repository so it holds for any implementation. Tests cover the validator, the service contract, and the repository against a mocked TableClient: Merge mode, the ETag actually passed, only ContactEmail in the patch, the 412 race, 404, storage faults. Plus a regression pin that SaveTenantConfigurationAsync is never called from the seed path. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -55,7 +55,7 @@ public class AuthFunctionSideEffectTests
|
||||
{ CallBase = false };
|
||||
|
||||
var previewMock = new Mock<PreviewWhitelistService>(
|
||||
configRepo, cache, Mock.Of<ILogger<PreviewWhitelistService>>())
|
||||
configRepo, cache, Mock.Of<ILogger<PreviewWhitelistService>>(), _tenantConfigMock.Object)
|
||||
{ CallBase = false };
|
||||
|
||||
_telegramMock = new Mock<TelegramNotificationService>(
|
||||
|
||||
@@ -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;
|
||||
/// </summary>
|
||||
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 <ops@contoso.com>", "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<IConfigRepository> repo, MemoryCache cache) BuildService()
|
||||
{
|
||||
var repo = new Mock<IConfigRepository>();
|
||||
var cache = new MemoryCache(new MemoryCacheOptions());
|
||||
var service = new TenantConfigurationService(
|
||||
repo.Object, NullLogger<TenantConfigurationService>.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<string>(), It.IsAny<string>()))
|
||||
.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<string>(), It.IsAny<string>()), 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<string>(), It.IsAny<string>()), 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<string>(), It.IsAny<string>()))
|
||||
.ReturnsAsync(true);
|
||||
|
||||
await service.TrySeedContactEmailAsync(TenantId, "ops@contoso.com");
|
||||
|
||||
repo.Verify(r => r.SaveTenantConfigurationAsync(It.IsAny<TenantConfiguration>()), Times.Never);
|
||||
}
|
||||
|
||||
// ------------------------------------------------------------------
|
||||
// Seed trigger (PreviewWhitelistService)
|
||||
// ------------------------------------------------------------------
|
||||
|
||||
private static (PreviewWhitelistService preview, Mock<IConfigRepository> repo) BuildPreview()
|
||||
{
|
||||
var repo = new Mock<IConfigRepository>();
|
||||
var cache = new MemoryCache(new MemoryCacheOptions());
|
||||
var tenantConfig = new TenantConfigurationService(
|
||||
repo.Object, NullLogger<TenantConfigurationService>.Instance, cache);
|
||||
var preview = new PreviewWhitelistService(
|
||||
repo.Object, cache, NullLogger<PreviewWhitelistService>.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<string>(), It.IsAny<string>()), 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<string>(), It.IsAny<string>()))
|
||||
.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<TableClient> 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<TableEntity>(
|
||||
It.IsAny<string>(), It.IsAny<string>(),
|
||||
It.IsAny<IEnumerable<string>>(), It.IsAny<CancellationToken>()))
|
||||
.ThrowsAsync(new RequestFailedException(404, "Not Found", "ResourceNotFound", null));
|
||||
}
|
||||
else
|
||||
{
|
||||
Table.Setup(c => c.GetEntityAsync<TableEntity>(
|
||||
It.IsAny<string>(), It.IsAny<string>(),
|
||||
It.IsAny<IEnumerable<string>>(), It.IsAny<CancellationToken>()))
|
||||
.ReturnsAsync(Response.FromValue(stored, Mock.Of<Response>()));
|
||||
}
|
||||
|
||||
Table.Setup(c => c.UpdateEntityAsync(
|
||||
It.IsAny<TableEntity>(), It.IsAny<ETag>(),
|
||||
It.IsAny<TableUpdateMode>(), It.IsAny<CancellationToken>()))
|
||||
.Returns<TableEntity, ETag, TableUpdateMode, CancellationToken>((e, tag, mode, _) =>
|
||||
{
|
||||
if (throwOnUpdate != null)
|
||||
throw throwOnUpdate;
|
||||
Updates.Add((e, tag, mode));
|
||||
return Task.FromResult(Mock.Of<Response>());
|
||||
});
|
||||
|
||||
var serviceClient = new Mock<TableServiceClient>();
|
||||
serviceClient.Setup(c => c.GetTableClient(It.IsAny<string>())).Returns(Table.Object);
|
||||
|
||||
Sut = new TableConfigRepository(
|
||||
new TableStorageService(serviceClient.Object, NullLogger<TableStorageService>.Instance),
|
||||
NullLogger<TableConfigRepository>.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<TableEntity>(
|
||||
It.IsAny<string>(), It.IsAny<string>(),
|
||||
It.IsAny<IEnumerable<string>>(), It.IsAny<CancellationToken>()), Times.Never);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -87,7 +87,11 @@ public sealed class TenantOffboardFunctionTests
|
||||
return new PreviewWhitelistService(
|
||||
configRepoMock.Object,
|
||||
cache,
|
||||
NullLogger<PreviewWhitelistService>.Instance);
|
||||
NullLogger<PreviewWhitelistService>.Instance,
|
||||
new TenantConfigurationService(
|
||||
configRepoMock.Object,
|
||||
NullLogger<TenantConfigurationService>.Instance,
|
||||
cache));
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
|
||||
+48
@@ -71,6 +71,54 @@ namespace AutopilotMonitor.Functions.DataAccess.TableStorage
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Conditional, single-property seed of ContactEmail (see <see cref="IConfigRepository"/>).
|
||||
/// 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.
|
||||
/// </summary>
|
||||
public async Task<bool> TrySeedTenantContactEmailAsync(string tenantId, string email)
|
||||
{
|
||||
if (string.IsNullOrWhiteSpace(tenantId) || string.IsNullOrWhiteSpace(email))
|
||||
return false;
|
||||
|
||||
try
|
||||
{
|
||||
var existing = await _tenantConfigTableClient.GetEntityAsync<TableEntity>(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<List<TenantConfiguration>> GetAllTenantConfigurationsAsync()
|
||||
{
|
||||
try
|
||||
|
||||
+59
@@ -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;
|
||||
|
||||
/// <summary>
|
||||
/// 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.
|
||||
/// <para>
|
||||
/// 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).
|
||||
/// </para>
|
||||
/// </summary>
|
||||
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;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Validates the notification-channel list JSON. Returns an error message, or null when
|
||||
/// valid/empty. Strict counterpart of the fail-soft <c>NotificationChannel.ParseList</c>:
|
||||
|
||||
@@ -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.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// The enumerated snapshot is only a cheap pre-filter; the authoritative "is it still
|
||||
/// empty" check happens under an ETag inside
|
||||
/// <see cref="TenantConfigurationService.TrySeedContactEmailAsync"/>. 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.
|
||||
/// </para>
|
||||
/// </summary>
|
||||
/// <returns>Number of tenants that received a contact address.</returns>
|
||||
private async Task<int> 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)
|
||||
|
||||
@@ -16,16 +16,19 @@ public class PreviewWhitelistService
|
||||
private readonly IConfigRepository _configRepo;
|
||||
private readonly IMemoryCache _cache;
|
||||
private readonly ILogger<PreviewWhitelistService> _logger;
|
||||
private readonly TenantConfigurationService _tenantConfigService;
|
||||
private static readonly TimeSpan CacheDuration = TimeSpan.FromMinutes(5);
|
||||
|
||||
public PreviewWhitelistService(
|
||||
IConfigRepository configRepo,
|
||||
IMemoryCache cache,
|
||||
ILogger<PreviewWhitelistService> logger)
|
||||
ILogger<PreviewWhitelistService> logger,
|
||||
TenantConfigurationService tenantConfigService)
|
||||
{
|
||||
_configRepo = configRepo;
|
||||
_cache = cache;
|
||||
_logger = logger;
|
||||
_tenantConfigService = tenantConfigService;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
@@ -104,42 +107,15 @@ public class PreviewWhitelistService
|
||||
/// <summary>
|
||||
/// Saves (or clears) the notification email for a tenant, and seeds the tenant's
|
||||
/// contact address from it the first time one is given.
|
||||
/// <para>
|
||||
/// The seed is a side effect of a write that has already been persisted, so it must never
|
||||
/// fail this call — <see cref="TenantConfigurationService.TrySeedContactEmailAsync"/> is
|
||||
/// fail-soft and owns the "never overwrite what the tenant owns" invariant.
|
||||
/// </para>
|
||||
/// </summary>
|
||||
public async Task SaveNotificationEmailAsync(string tenantId, string? email)
|
||||
{
|
||||
await _configRepo.SaveNotificationEmailAsync(tenantId, email);
|
||||
await SeedContactEmailAsync(tenantId, email);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// One-way seed: copies the preview notification address into
|
||||
/// <see cref="TenantConfiguration.ContactEmail"/> 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.
|
||||
/// </summary>
|
||||
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);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -125,6 +125,48 @@ namespace AutopilotMonitor.Functions.Services
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// One-way seed of the tenant contact address: writes <paramref name="email"/> only while
|
||||
/// the tenant has none, and only if nothing else wrote the configuration in the meantime.
|
||||
/// Returns true when the seed landed.
|
||||
/// <para>
|
||||
/// 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.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// Fail-soft by design, unlike <see cref="SaveConfigurationAsync"/>: 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.
|
||||
/// </para>
|
||||
/// </summary>
|
||||
public virtual async Task<bool> 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;
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Invalidates cache for a tenant (forces reload on next request)
|
||||
/// </summary>
|
||||
|
||||
@@ -24,6 +24,21 @@ namespace AutopilotMonitor.Shared.DataAccess
|
||||
/// </summary>
|
||||
Task<RawPage<TenantConfiguration>> GetTenantConfigurationsPageAsync(int pageSize, string? continuation);
|
||||
|
||||
/// <summary>
|
||||
/// Writes <paramref name="email"/> 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.
|
||||
/// <para>
|
||||
/// Exists because the seed cannot be expressed as a read-modify-write of the whole model:
|
||||
/// <see cref="SaveTenantConfigurationAsync"/> 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.
|
||||
/// </para>
|
||||
/// </summary>
|
||||
Task<bool> TrySeedTenantContactEmailAsync(string tenantId, string email);
|
||||
|
||||
// --- Admin Configuration ---
|
||||
Task<AdminConfiguration?> GetAdminConfigurationAsync();
|
||||
Task<bool> SaveAdminConfigurationAsync(AdminConfiguration config);
|
||||
|
||||
Reference in New Issue
Block a user