diff --git a/src/Backend/AutopilotMonitor.Functions.Tests/TenantConfigPatchServiceTests.cs b/src/Backend/AutopilotMonitor.Functions.Tests/TenantConfigPatchServiceTests.cs index 1ac8f5f2..4a37b6e9 100644 --- a/src/Backend/AutopilotMonitor.Functions.Tests/TenantConfigPatchServiceTests.cs +++ b/src/Backend/AutopilotMonitor.Functions.Tests/TenantConfigPatchServiceTests.cs @@ -680,6 +680,51 @@ public class TenantConfigPatchServiceTests Assert.Equal(schema.Count, schema.Select(f => f.Name).Distinct().Count()); } + /// + /// The full-model PUT (config/{tenantId}) must honour the same deny-list as the field + /// patch: a client body carrying ProDowngradedUtc (retention-downgrade grace anchor), + /// OnboardedAt, DomainName, plan/trial or provenance fields must not change the stored + /// values. Driven by the deny-list itself so a new denied field is covered automatically. + /// + [Fact] + public void PreserveDeniedFields_OverwritesEveryDeniedModelPropertyFromStored() + { + var props = typeof(TenantConfiguration) + .GetProperties(System.Reflection.BindingFlags.Public | System.Reflection.BindingFlags.Instance) + .Where(p => p.CanWrite && TenantConfigPatchService.BaseDeniedFields.Contains(p.Name)) + .ToList(); + Assert.Contains(props, p => p.Name == "ProDowngradedUtc"); + Assert.Contains(props, p => p.Name == "OnboardedAt"); + Assert.Contains(props, p => p.Name == "DomainName"); + + var stored = new TenantConfiguration { TenantId = "t1" }; + var incoming = new TenantConfiguration { TenantId = "t1" }; + foreach (var p in props) + { + p.SetValue(stored, Distinct(p.PropertyType, 1)); + p.SetValue(incoming, Distinct(p.PropertyType, 2)); + Assert.NotEqual(p.GetValue(stored), p.GetValue(incoming)); + } + + TenantConfigPatchService.PreserveDeniedFields(incoming, stored); + + foreach (var p in props) + Assert.Equal(p.GetValue(stored), p.GetValue(incoming)); + } + + private static object Distinct(Type t, int seed) + { + var u = Nullable.GetUnderlyingType(t) ?? t; + if (u == typeof(string)) return $"value-{seed}"; + if (u == typeof(bool)) return seed % 2 == 0; + if (u == typeof(int)) return seed; + if (u == typeof(long)) return (long)seed; + if (u == typeof(DateTime)) return new DateTime(2026, 1, seed, 0, 0, 0, DateTimeKind.Utc); + if (u == typeof(DateTimeOffset)) return new DateTimeOffset(2026, 1, seed, 0, 0, 0, TimeSpan.Zero); + if (u.IsEnum) return Enum.GetValues(u).GetValue(seed % Enum.GetValues(u).Length)!; + throw new NotSupportedException($"Add a Distinct() case for {u.Name}"); + } + [Fact] public void FieldsSchema_DeniedFields_MatchThePatchDenyList() { diff --git a/src/Backend/AutopilotMonitor.Functions/Functions/Config/UpdateTenantConfigurationFunction.cs b/src/Backend/AutopilotMonitor.Functions/Functions/Config/UpdateTenantConfigurationFunction.cs index f5628119..55bea4ea 100644 --- a/src/Backend/AutopilotMonitor.Functions/Functions/Config/UpdateTenantConfigurationFunction.cs +++ b/src/Backend/AutopilotMonitor.Functions/Functions/Config/UpdateTenantConfigurationFunction.cs @@ -110,11 +110,15 @@ namespace AutopilotMonitor.Functions.Functions.Config // admins are served the FULL config so this is normally a no-op for them.) config.RestoreRedactedSecretsFrom(existingConfig); - // DomainName is tenant identity: seeded once from the first login's UPN (AuthFunction) - // and never client-writable — for ANY caller, GA included — mirroring the patch flow - // ("identity — never writable"). Without this preserve the full-model PUT persisted an - // arbitrary string here, which is rendered into the welcome/farewell mails. - config.DomainName = existingConfig.DomainName; + // System-owned fields are never client-writable — for ANY caller, GA included: + // identity (DomainName, seeded once from the first login's UPN), onboarding/auth + // provenance (OnboardedAt/OnboardedBy/LastAuthClientId*), app-reg homing + // (HomedAppClientId — the 2026-07-31 stale-round-trip incident), and plan/trial/ + // downgrade state (PlanTier/Trial*/ProDowngradedUtc — mutable only via the plan + // and trial endpoints; a client-supplied ProDowngradedUtc would refresh the + // retention-downgrade grace anchor indefinitely). The set is the PATCH endpoint's + // BaseDeniedFields, so the two write paths cannot drift. + TenantConfigPatchService.PreserveDeniedFields(config, existingConfig); // Shared model validation (rate limits, contact address, webhook/Teams SSRF, custom // headers, notification channels, diagnostics SAS, retention cap) — single source @@ -164,26 +168,6 @@ namespace AutopilotMonitor.Functions.Functions.Config config.DisabledUntil = existingConfig.DisabledUntil; } - // App-reg homing is IMMUTABLE via the generic PUT — for ALL callers, GA included. - // The only writers are the consent-driven auto-flip and POST config/{tenantId}/ - // app-homing (AppHomingFunction). Prod incident 2026-07-31 18:03Z: the consent - // auto-flip landed, and 600ms later the frontend's routine "persist validation - // toggle" PUT — round-tripping a config loaded BEFORE the flip — silently reverted - // the homing to legacy, because a GA caller's stale null was indistinguishable - // from an intentional revert. A full-model PUT can never carry that intent. - config.HomedAppClientId = existingConfig.HomedAppClientId; - - // Last-seen auth provenance is system-written (AuthFunction) — never via PUT, - // for ANY caller: a round-tripped stale view must not rewind the observability. - config.LastAuthClientId = existingConfig.LastAuthClientId; - config.LastAuthClientIdSince = existingConfig.LastAuthClientIdSince; - - // OnboardedBy is system-written once at first login (AuthFunction) and immutable - // after that — never via PUT, for ANY caller: a revoke → re-approve cycle - // auto-promotes whatever UPN is stored here, so a client-supplied value could - // smuggle in an arbitrary promotable user (or null out the audit provenance). - config.OnboardedBy = existingConfig.OnboardedBy; - // Safety: if GA gate is off, force UnrestrictedMode to false if (!config.UnrestrictedModeEnabled) { @@ -193,16 +177,6 @@ namespace AutopilotMonitor.Functions.Functions.Config // MaxNdjsonPayloadSizeMB is table-only — always preserve existing value config.MaxNdjsonPayloadSizeMB = existingConfig.MaxNdjsonPayloadSizeMB; - // Plan/trial fields are mutable ONLY via the dedicated plan/trial endpoints - // (PATCH config/{tenantId}/plan, POST config/{tenantId}/trial). The generic PUT - // deserializes the full model, so without this preserve a round-tripped stale - // view would silently reset the tenant's edition/trial — for ALL callers, GA included. - config.PlanTier = existingConfig.PlanTier; - config.TrialExpiresUtc = existingConfig.TrialExpiresUtc; - config.TrialStartedUtc = existingConfig.TrialStartedUtc; - config.TrialConsumed = existingConfig.TrialConsumed; - config.TrialGrantedBy = existingConfig.TrialGrantedBy; - // Save configuration (retention cap already enforced by ValidateModel above) var writeSource = ResolveWriteSource(req.Query["intent"]); await _configService.SaveConfigurationAsync(config, writeSource, null); diff --git a/src/Backend/AutopilotMonitor.Functions/Services/TenantConfigPatchService.cs b/src/Backend/AutopilotMonitor.Functions/Services/TenantConfigPatchService.cs index 9e299967..3070fa56 100644 --- a/src/Backend/AutopilotMonitor.Functions/Services/TenantConfigPatchService.cs +++ b/src/Backend/AutopilotMonitor.Functions/Services/TenantConfigPatchService.cs @@ -127,6 +127,25 @@ namespace AutopilotMonitor.Functions.Services "ProDowngradedUtc", }; + /// + /// Full-model PUT counterpart of : copies every denied model + /// property from the stored config onto the incoming one so a client body can never write + /// them — for ANY caller, GA included. The PUT deserializes the whole model, so a hand-kept + /// preserve list drifts (ProDowngradedUtc/OnboardedAt were missing, letting a tenant admin + /// refresh the retention-downgrade grace anchor forever). Deriving it from the single + /// deny-list keeps both write paths in lock-step. Server-stamped fields (TenantId, + /// UpdatedBy, LastUpdated) are copied too; the PUT overwrites them afterwards. + /// + internal static void PreserveDeniedFields(TenantConfiguration target, TenantConfiguration stored) + { + foreach (var name in BaseDeniedFields) + { + // PartitionKey/RowKey/Timestamp/ETag are table plumbing, not model properties. + if (ModelProperties.TryGetValue(name, out var prop)) + prop.SetValue(target, prop.GetValue(stored)); + } + } + /// /// Additional deny-list for non-GA tiers (phase 2): the GA-only toggles the PUT /// endpoint silently reverts for tenant admins are an explicit 400 here.