From f795fbcb4eb24f2f77779d1735d2360cb1d8af8d Mon Sep 17 00:00:00 2001 From: Oliver Kieselbach Date: Fri, 31 Jul 2026 18:12:25 +0200 Subject: [PATCH] feat(auth): dual app-reg parallel operation - legacy gktatooine app alongside new C4A8 app True parallel operation, no forced re-consent: existing tenants keep signing in via the legacy app (1a400946), new tenants land on the C4A8 app (886ab5e2). Model "default legacy, funnel primary, learn in the background": - Web: am_auth_app flag -> NEXT_PUBLIC_ENTRA_DEFAULT_APP -> primary; get-started CTA hands over with ?authapp=primary; auth/me homedApp updates the flag so an operator tenant flip takes effect on each user's NEXT login; AADSTS90094/65001 one-shot cross-app retry, AADSTS65004 -> failed screen with previous-app link; am_login_attempt pins redirect returns to the app that started them. - Backend: TenantConfiguration.HomedAppClientId (null = legacy, GA-only flip via PUT config) + LastAuthClientId/Since observability; EntraAppRegistry resolves the acting app per tenant (no-op until the legacy config pair is set); Graph client-credentials, admin-consent URLs and the surfaced client id follow the homed app (app-scoped cache keys); JwtBearer [Authorize] pipeline now shares the middleware audience builders (was single-audience - would 401 legacy tokens after the swap); middleware trusts EntraId:LegacyClientId implicitly. - well-known association file now lists both app ids (publisher domain); deploy-web passes NEXT_PUBLIC_ENTRA_LEGACY_CLIENT_ID / _DEFAULT_APP; mcp-server.bicep default client id -> C4A8 app. Backend xUnit 3730/3730, vitest 625/625 (13 new), tsc/ESLint clean, next build ok. Co-Authored-By: Claude Fable 5 --- .github/workflows/deploy-web.yml | 4 + infra/mcp-server.bicep | 4 +- .../AuthFunctionSideEffectTests.cs | 5 +- ...nticationMiddlewareTokenValidationTests.cs | 32 +++ .../EntraAppRegistryTests.cs | 133 +++++++++++++ .../GraphFeatureDetectorTests.cs | 72 ++++--- .../GraphTokenServiceTests.cs | 35 ++-- .../TenantConfigProjectionTests.cs | 1 + .../TenantConfigTableSerializationTests.cs | 36 ++++ .../TableStorage/TableConfigRepository.cs | 7 + ...utopilotDeviceValidationConsentFunction.cs | 14 +- .../UpdateTenantConfigurationFunction.cs | 53 ++++- .../GetGraphPermissionsStatusFunction.cs | 15 +- .../Functions/Infrastructure/AuthFunction.cs | 68 ++++++- .../Helpers/TenantConfigProjection.cs | 5 + .../Middleware/AuthenticationMiddleware.cs | 27 ++- .../AutopilotMonitor.Functions/Program.cs | 27 ++- .../Security/EntraAppRegistry.cs | 144 ++++++++++++++ .../Security/GraphTokenService.cs | 102 +++++++--- .../Models/Config/TenantConfiguration.cs | 26 +++ .../app/get-started/page.tsx | 2 +- .../sections/SectionAutopilotValidation.tsx | 14 ++ .../app/settings/types.ts | 6 + .../components/ProtectedRoute.tsx | 29 ++- .../components/landing/LoginButton.tsx | 19 +- .../contexts/AuthContext.tsx | 89 ++++++++- .../lib/__tests__/authApp.test.ts | 153 ++++++++++++++ src/Web/autopilot-monitor-web/lib/authApp.ts | 186 ++++++++++++++++++ .../autopilot-monitor-web/lib/msalConfig.ts | 53 ++++- .../microsoft-identity-association.json | 3 + 30 files changed, 1250 insertions(+), 114 deletions(-) create mode 100644 src/Backend/AutopilotMonitor.Functions.Tests/EntraAppRegistryTests.cs create mode 100644 src/Backend/AutopilotMonitor.Functions/Security/EntraAppRegistry.cs create mode 100644 src/Web/autopilot-monitor-web/lib/__tests__/authApp.test.ts create mode 100644 src/Web/autopilot-monitor-web/lib/authApp.ts diff --git a/.github/workflows/deploy-web.yml b/.github/workflows/deploy-web.yml index f5a0d9ed..fc69a8a5 100644 --- a/.github/workflows/deploy-web.yml +++ b/.github/workflows/deploy-web.yml @@ -39,6 +39,10 @@ jobs: uses: Azure/static-web-apps-deploy@v1 env: NEXT_PUBLIC_ENTRA_CLIENT_ID: ${{ vars.NEXT_PUBLIC_ENTRA_CLIENT_ID }} + # Dual app-reg window: previous app registration (unset = single-app behaviour) + # and the default app for browsers without a stored selection (legacy|primary). + NEXT_PUBLIC_ENTRA_LEGACY_CLIENT_ID: ${{ vars.NEXT_PUBLIC_ENTRA_LEGACY_CLIENT_ID }} + NEXT_PUBLIC_ENTRA_DEFAULT_APP: ${{ vars.NEXT_PUBLIC_ENTRA_DEFAULT_APP }} NEXT_PUBLIC_ENTRA_REDIRECT_URI: ${{ vars.NEXT_PUBLIC_ENTRA_REDIRECT_URI }} NEXT_PUBLIC_ENTRA_POST_LOGOUT_REDIRECT_URI: ${{ vars.NEXT_PUBLIC_ENTRA_POST_LOGOUT_REDIRECT_URI }} NEXT_PUBLIC_API_BASE_URL: ${{ vars.NEXT_PUBLIC_API_BASE_URL }} diff --git a/infra/mcp-server.bicep b/infra/mcp-server.bicep index 9208d6d1..5b4e590d 100644 --- a/infra/mcp-server.bicep +++ b/infra/mcp-server.bicep @@ -60,8 +60,8 @@ param apiUrl string = 'https://autopilotmonitor-api-eu.azurewebsites.net' @description('Container image tag') param imageTag string = 'latest' -@description('Entra ID app registration client_id used for the OAuth proxy. Public value (appears in JWT aud claim); not a secret.') -param entraClientId string = '1a400946-62c1-4ab4-aa37-f730ac89704d' +@description('Entra ID app registration client_id used for the OAuth proxy. Public value (appears in JWT aud claim); not a secret. Since the C4A8 app-reg move this is the NEW app (the old 1a400946-… app remains valid at the backend for legacy-homed tenants).') +param entraClientId string = '886ab5e2-6144-442c-80cc-9b28e0667731' @secure() @description('Entra ID client secret for OAuth token exchange') diff --git a/src/Backend/AutopilotMonitor.Functions.Tests/AuthFunctionSideEffectTests.cs b/src/Backend/AutopilotMonitor.Functions.Tests/AuthFunctionSideEffectTests.cs index a39b541b..16dd3c37 100644 --- a/src/Backend/AutopilotMonitor.Functions.Tests/AuthFunctionSideEffectTests.cs +++ b/src/Backend/AutopilotMonitor.Functions.Tests/AuthFunctionSideEffectTests.cs @@ -90,7 +90,10 @@ public class AuthFunctionSideEffectTests _telegramMock.Object, _globalNotificationMock.Object, mcpUserMock.Object, - _autoApproveEnqueuerMock.Object); + _autoApproveEnqueuerMock.Object, + new AutopilotMonitor.Functions.Security.EntraAppRegistry( + new Microsoft.Extensions.Configuration.ConfigurationBuilder().Build(), + Mock.Of>())); // Default: all fire-and-forget calls succeed _tenantConfigMock diff --git a/src/Backend/AutopilotMonitor.Functions.Tests/AuthenticationMiddlewareTokenValidationTests.cs b/src/Backend/AutopilotMonitor.Functions.Tests/AuthenticationMiddlewareTokenValidationTests.cs index f5ba7022..6fada4ea 100644 --- a/src/Backend/AutopilotMonitor.Functions.Tests/AuthenticationMiddlewareTokenValidationTests.cs +++ b/src/Backend/AutopilotMonitor.Functions.Tests/AuthenticationMiddlewareTokenValidationTests.cs @@ -203,6 +203,38 @@ public class AuthenticationMiddlewareTokenValidationTests Mint(RsaCreds, audience: "api://some-third-app"), ClientId, SecondaryClientId)); } + // ── Dual app-reg window (C4A8 move): primary=new app, legacy=old app via LegacyClientId ── + // The middleware folds EntraId:LegacyClientId into the additional-id set via + // CombineAdditionalClientIdSources, so both apps' tokens validate in bare AND api:// form. + + private const string NewAppId = "886ab5e2-6144-442c-80cc-9b28e0667731"; + private const string OldAppId = "1a400946-62c1-4ab4-aa37-f730ac89704d"; + + [Theory] + [InlineData(NewAppId)] + [InlineData("api://" + NewAppId)] + [InlineData(OldAppId)] + [InlineData("api://" + OldAppId)] + public void DualAppWindow_accepts_both_apps_tokens_in_both_audience_forms(string audience) + { + var clientIds = AuthenticationMiddleware.ResolveConfiguredClientIds( + NewAppId, + AuthenticationMiddleware.CombineAdditionalClientIdSources(OldAppId, null), + out _); + + Assert.NotNull(ValidateWith(Mint(RsaCreds, audience: audience), clientIds)); + } + + [Fact] + public void CombineAdditionalClientIdSources_joins_and_skips_empty_sources() + { + Assert.Equal($"{OldAppId},{SecondaryClientId}", + AuthenticationMiddleware.CombineAdditionalClientIdSources(OldAppId, SecondaryClientId)); + Assert.Equal(OldAppId, + AuthenticationMiddleware.CombineAdditionalClientIdSources($" {OldAppId} ", null, "")); + Assert.Null(AuthenticationMiddleware.CombineAdditionalClientIdSources(null, "", " ")); + } + // ── BuildValidAudiences: id → {id, api://id}, deduped, empties dropped ──────────────── [Fact] diff --git a/src/Backend/AutopilotMonitor.Functions.Tests/EntraAppRegistryTests.cs b/src/Backend/AutopilotMonitor.Functions.Tests/EntraAppRegistryTests.cs new file mode 100644 index 00000000..25a42929 --- /dev/null +++ b/src/Backend/AutopilotMonitor.Functions.Tests/EntraAppRegistryTests.cs @@ -0,0 +1,133 @@ +using System.Collections.Generic; +using AutopilotMonitor.Functions.Security; +using AutopilotMonitor.Shared.Models; +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.Logging.Abstractions; +using Xunit; + +namespace AutopilotMonitor.Functions.Tests; + +/// +/// Resolution matrix for the dual app-registration window (see EntraAppRegistry): +/// legacy unconfigured ⇒ always primary (code deploy is a no-op before the config swap); +/// null homing ⇒ legacy; known ids ⇒ that app; unknown ⇒ primary fallback. +/// +public class EntraAppRegistryTests +{ + private const string PrimaryId = "886ab5e2-6144-442c-80cc-9b28e0667731"; + private const string LegacyId = "1a400946-62c1-4ab4-aa37-f730ac89704d"; + + private static EntraAppRegistry Build(Dictionary settings) => + new(new ConfigurationBuilder().AddInMemoryCollection(settings).Build(), + NullLogger.Instance); + + private static EntraAppRegistry DualApp() => Build(new() + { + ["EntraId:ClientId"] = PrimaryId, + ["EntraId:ClientSecret"] = "primary-secret", + ["EntraId:LegacyClientId"] = LegacyId, + ["EntraId:LegacyClientSecret"] = "legacy-secret", + }); + + private static TenantConfiguration Tenant(string? homedAppClientId) => new() + { + TenantId = "11111111-1111-1111-1111-111111111111", + HomedAppClientId = homedAppClientId, + }; + + [Fact] + public void LegacyUnconfigured_AlwaysResolvesPrimary_EvenForNullHoming() + { + var registry = Build(new() + { + ["EntraId:ClientId"] = PrimaryId, + ["EntraId:ClientSecret"] = "primary-secret", + }); + + Assert.False(registry.LegacyConfigured); + Assert.Equal(PrimaryId, registry.ResolveForTenant(Tenant(null)).ClientId); + Assert.Equal(PrimaryId, registry.ResolveForTenant(null).ClientId); + Assert.False(registry.ResolveForTenant(Tenant(null)).IsLegacy); + } + + [Fact] + public void NullHoming_ResolvesLegacy() + { + var resolved = DualApp().ResolveForTenant(Tenant(null)); + Assert.True(resolved.IsLegacy); + Assert.Equal(LegacyId, resolved.ClientId); + Assert.Equal("legacy-secret", resolved.ClientSecret); + } + + [Fact] + public void MissingConfigRow_ResolvesLegacy_SameAsNullHoming() + { + Assert.True(DualApp().ResolveForTenant(null).IsLegacy); + } + + [Fact] + public void PrimaryHoming_ResolvesPrimary_CaseAndBraceInsensitive() + { + var registry = DualApp(); + Assert.False(registry.ResolveForTenant(Tenant(PrimaryId)).IsLegacy); + Assert.False(registry.ResolveForTenant(Tenant(PrimaryId.ToUpperInvariant())).IsLegacy); + Assert.False(registry.ResolveForTenant(Tenant("{" + PrimaryId + "}")).IsLegacy); + } + + [Fact] + public void LegacyHoming_ResolvesLegacy() + { + var resolved = DualApp().ResolveForTenant(Tenant(LegacyId)); + Assert.True(resolved.IsLegacy); + Assert.Equal("legacy-secret", resolved.ClientSecret); + } + + [Fact] + public void UnknownHoming_FallsBackToPrimary() + { + var resolved = DualApp().ResolveForTenant(Tenant("99999999-9999-9999-9999-999999999999")); + Assert.False(resolved.IsLegacy); + Assert.Equal(PrimaryId, resolved.ClientId); + } + + [Fact] + public void MalformedLegacyClientId_TreatedAsUnconfigured() + { + var registry = Build(new() + { + ["EntraId:ClientId"] = PrimaryId, + ["EntraId:LegacyClientId"] = "not-a-guid", + ["EntraId:LegacyClientSecret"] = "legacy-secret", + }); + + Assert.False(registry.LegacyConfigured); + Assert.False(registry.ResolveForTenant(Tenant(null)).IsLegacy); + } + + [Theory] + [InlineData(PrimaryId, true)] + [InlineData("api://" + PrimaryId, true)] + [InlineData("API://" + PrimaryId, true)] + [InlineData(LegacyId, false)] + [InlineData("api://" + LegacyId, false)] + [InlineData("https://graph.microsoft.com", false)] + [InlineData(null, false)] + [InlineData("", false)] + public void IsPrimary_MatchesBareAndApiPrefixedAudiences(string? audience, bool expected) + { + Assert.Equal(expected, DualApp().IsPrimary(audience)); + } + + [Theory] + [InlineData("api://" + LegacyId, LegacyId)] + [InlineData(LegacyId, LegacyId)] + [InlineData("API://886AB5E2-6144-442C-80CC-9B28E0667731", PrimaryId)] + [InlineData("https://graph.microsoft.com", null)] // non-GUID audience must never persist as provenance + [InlineData("api://not-a-guid", null)] + [InlineData(null, null)] + [InlineData(" ", null)] + public void NormalizeAudience_StripsApiPrefixAndRequiresGuid(string? audience, string? expected) + { + Assert.Equal(expected, EntraAppRegistry.NormalizeAudience(audience)); + } +} diff --git a/src/Backend/AutopilotMonitor.Functions.Tests/GraphResolution/GraphFeatureDetectorTests.cs b/src/Backend/AutopilotMonitor.Functions.Tests/GraphResolution/GraphFeatureDetectorTests.cs index 2a25bbee..1d899ecd 100644 --- a/src/Backend/AutopilotMonitor.Functions.Tests/GraphResolution/GraphFeatureDetectorTests.cs +++ b/src/Backend/AutopilotMonitor.Functions.Tests/GraphResolution/GraphFeatureDetectorTests.cs @@ -327,17 +327,23 @@ public class GraphFeatureDetectorTests var handler = new SwappableTokenHandler( BuildJwt(Array.Empty(), expires: DateTime.UtcNow.AddHours(1))); // pre-grant: no roles var cache = new MemoryCache(new MemoryCacheOptions()); + var tokenConfig = new ConfigurationBuilder() + .AddInMemoryCollection(new Dictionary + { + ["EntraId:ClientId"] = "test-client", + ["EntraId:ClientSecret"] = "test-secret", + }) + .Build(); var tokenService = new GraphTokenService( NullLogger.Instance, new SingleHandlerFactory(handler), cache, - new ConfigurationBuilder() - .AddInMemoryCollection(new Dictionary - { - ["EntraId:ClientId"] = "test-client", - ["EntraId:ClientSecret"] = "test-secret", - }) - .Build()); + tokenConfig, + new EntraAppRegistry(tokenConfig, NullLogger.Instance), + new AutopilotMonitor.Functions.Services.TenantConfigurationService( + Moq.Mock.Of(), + NullLogger.Instance, + new MemoryCache(new MemoryCacheOptions()))); var telemetry = new TelemetryClient(new TelemetryConfiguration { DisableTelemetry = true }); var detector = new GraphFeatureDetector( tokenService, cache, NullLogger.Instance, telemetry); @@ -454,6 +460,28 @@ public class GraphFeatureDetectorTests // ── Test seam: token service stub + detector factory ────────────────────────────── + /// Minimal EntraId config for the GraphTokenService base ctor (no legacy app). + private static Microsoft.Extensions.Configuration.IConfiguration StubEntraConfig() => + new ConfigurationBuilder() + .AddInMemoryCollection(new Dictionary + { + ["EntraId:ClientId"] = "test-client-id", + ["EntraId:ClientSecret"] = "test-secret", + }) + .Build(); + + /// + /// Dual app-reg dependencies for the GraphTokenService base ctor. No legacy app configured + /// ⇒ the registry always resolves primary and the tenant-config lookup is never consulted. + /// + private static EntraAppRegistry StubAppRegistry() => + new(StubEntraConfig(), NullLogger.Instance); + + private static AutopilotMonitor.Functions.Services.TenantConfigurationService StubTenantConfigService() => + new(Moq.Mock.Of(), + NullLogger.Instance, + new MemoryCache(new MemoryCacheOptions())); + /// /// Subclass that bypasses the real AAD HTTP call so tests can drive the detector with /// canned values. Base ctor dependencies are satisfied @@ -469,13 +497,9 @@ public class GraphFeatureDetectorTests NullLogger.Instance, new NoopHttpClientFactory(), new MemoryCache(new MemoryCacheOptions()), - new ConfigurationBuilder() - .AddInMemoryCollection(new Dictionary - { - ["EntraId:ClientId"] = "test-client-id", - ["EntraId:ClientSecret"] = "test-secret", - }) - .Build()) + StubEntraConfig(), + StubAppRegistry(), + StubTenantConfigService()) { } @@ -506,13 +530,9 @@ public class GraphFeatureDetectorTests NullLogger.Instance, new NoopHttpClientFactory(), new MemoryCache(new MemoryCacheOptions()), - new ConfigurationBuilder() - .AddInMemoryCollection(new Dictionary - { - ["EntraId:ClientId"] = "test-client-id", - ["EntraId:ClientSecret"] = "test-secret", - }) - .Build()) + StubEntraConfig(), + StubAppRegistry(), + StubTenantConfigService()) { _firstToken = firstToken; _laterToken = laterToken; @@ -549,13 +569,9 @@ public class GraphFeatureDetectorTests NullLogger.Instance, factory, cache, - new ConfigurationBuilder() - .AddInMemoryCollection(new Dictionary - { - ["EntraId:ClientId"] = "test-client-id", - ["EntraId:ClientSecret"] = "test-secret", - }) - .Build()) + StubEntraConfig(), + StubAppRegistry(), + StubTenantConfigService()) { _onInvalidate = onInvalidate; } diff --git a/src/Backend/AutopilotMonitor.Functions.Tests/GraphTokenServiceTests.cs b/src/Backend/AutopilotMonitor.Functions.Tests/GraphTokenServiceTests.cs index 1c95d760..2cad4f8f 100644 --- a/src/Backend/AutopilotMonitor.Functions.Tests/GraphTokenServiceTests.cs +++ b/src/Backend/AutopilotMonitor.Functions.Tests/GraphTokenServiceTests.cs @@ -33,15 +33,28 @@ public class GraphTokenServiceTests }) .Build(); - var svc = new GraphTokenService( - NullLogger.Instance, - new StubHttpClientFactory(handler), - new MemoryCache(new MemoryCacheOptions()), - config); + var svc = NewService(config, new StubHttpClientFactory(handler)); return (svc, handler); } + /// + /// Builds the service with the dual app-reg dependencies. No legacy app is configured in + /// these tests ⇒ the registry always resolves the primary credentials and the tenant-config + /// lookup is never consulted for the decision — a null-returning repo is fine. + /// + private static GraphTokenService NewService(IConfiguration config, System.Net.Http.IHttpClientFactory factory) => + new GraphTokenService( + NullLogger.Instance, + factory, + new MemoryCache(new MemoryCacheOptions()), + config, + new EntraAppRegistry(config, NullLogger.Instance), + new AutopilotMonitor.Functions.Services.TenantConfigurationService( + Moq.Mock.Of(), + NullLogger.Instance, + new MemoryCache(new MemoryCacheOptions()))); + private static int TokenPosts(StubHttpMessageHandler handler, string tenantId) => handler.Requests.Count(u => u.Contains($"/{tenantId}/oauth2/v2.0/token")); @@ -148,11 +161,7 @@ public class GraphTokenServiceTests ["EntraId:ClientSecret"] = "test-secret", }) .Build(); - var svc = new GraphTokenService( - NullLogger.Instance, - new SingleHandlerFactory(handler), - new MemoryCache(new MemoryCacheOptions()), - config); + var svc = NewService(config, new SingleHandlerFactory(handler)); // R1 starts; its POST parks in flight (holding the gate, before writing to the cache). var inflight = svc.GetAccessTokenAsync(TenantA); @@ -188,11 +197,7 @@ public class GraphTokenServiceTests ["EntraId:ClientSecret"] = "test-secret", }) .Build(); - var svc = new GraphTokenService( - NullLogger.Instance, - new SingleHandlerFactory(handler), - new MemoryCache(new MemoryCacheOptions()), - config); + var svc = NewService(config, new SingleHandlerFactory(handler)); var tasks = Enumerable.Range(0, 10).Select(_ => svc.GetAccessTokenAsync(TenantA)).ToArray(); diff --git a/src/Backend/AutopilotMonitor.Functions.Tests/TenantConfigProjectionTests.cs b/src/Backend/AutopilotMonitor.Functions.Tests/TenantConfigProjectionTests.cs index 274480c3..331674fe 100644 --- a/src/Backend/AutopilotMonitor.Functions.Tests/TenantConfigProjectionTests.cs +++ b/src/Backend/AutopilotMonitor.Functions.Tests/TenantConfigProjectionTests.cs @@ -30,6 +30,7 @@ public class TenantConfigProjectionTests "tenantId", "domainName", "planTier", "trialExpiresUtc", "trialConsumed", "disabled", "disabledReason", "onboardedAt", "onboardedBy", "lastUpdated", "dataRetentionDays", + "homedAppClientId", "lastAuthClientId", "lastAuthClientIdSince", }; [Fact] diff --git a/src/Backend/AutopilotMonitor.Functions.Tests/TenantConfigTableSerializationTests.cs b/src/Backend/AutopilotMonitor.Functions.Tests/TenantConfigTableSerializationTests.cs index b6e626f2..a141a878 100644 --- a/src/Backend/AutopilotMonitor.Functions.Tests/TenantConfigTableSerializationTests.cs +++ b/src/Backend/AutopilotMonitor.Functions.Tests/TenantConfigTableSerializationTests.cs @@ -146,4 +146,40 @@ public class TenantConfigTableSerializationTests var entity = new TableEntity(TenantId, "config") { { "DomainName", "fabrikam.com" } }; Assert.Null(TableConfigRepository.ConvertFromTenantTableEntity(entity).NotificationChannelsJson); } + + [Fact] + public void Roundtrip_AppRegHomingFields_SurviveStoreAndMap() + { + var since = new DateTime(2026, 7, 31, 9, 0, 0, DateTimeKind.Utc); + var config = new TenantConfiguration + { + TenantId = TenantId, + DomainName = "contoso.com", + UpdatedBy = "admin@contoso.com", + HomedAppClientId = "886ab5e2-6144-442c-80cc-9b28e0667731", + LastAuthClientId = "886ab5e2-6144-442c-80cc-9b28e0667731", + LastAuthClientIdSince = since + }; + + var mapped = TableConfigRepository.ConvertFromTenantTableEntity( + TableConfigRepository.ConvertToTenantTableEntity(config)); + + Assert.Equal("886ab5e2-6144-442c-80cc-9b28e0667731", mapped.HomedAppClientId); + Assert.Equal("886ab5e2-6144-442c-80cc-9b28e0667731", mapped.LastAuthClientId); + Assert.Equal(since, mapped.LastAuthClientIdSince); + } + + [Fact] + public void Map_LegacyRow_WithoutAppRegHomingColumns_ReadsNull() + { + // Every tenant row pre-dating the C4A8 app-reg move lacks these columns; null + // HomedAppClientId is the invariant for "homed on the legacy app registration". + var entity = new TableEntity(TenantId, "config") { { "DomainName", "fabrikam.com" } }; + + var mapped = TableConfigRepository.ConvertFromTenantTableEntity(entity); + + Assert.Null(mapped.HomedAppClientId); + Assert.Null(mapped.LastAuthClientId); + Assert.Null(mapped.LastAuthClientIdSince); + } } diff --git a/src/Backend/AutopilotMonitor.Functions/DataAccess/TableStorage/TableConfigRepository.cs b/src/Backend/AutopilotMonitor.Functions/DataAccess/TableStorage/TableConfigRepository.cs index 5769b586..cd0f971b 100644 --- a/src/Backend/AutopilotMonitor.Functions/DataAccess/TableStorage/TableConfigRepository.cs +++ b/src/Backend/AutopilotMonitor.Functions/DataAccess/TableStorage/TableConfigRepository.cs @@ -452,6 +452,9 @@ namespace AutopilotMonitor.Functions.DataAccess.TableStorage { "UnrestrictedMode", config.UnrestrictedMode }, { "EntraAppRolesEnabled", config.EntraAppRolesEnabled }, { "OnboardedAt", config.OnboardedAt }, + { "HomedAppClientId", config.HomedAppClientId }, + { "LastAuthClientId", config.LastAuthClientId }, + { "LastAuthClientIdSince", config.LastAuthClientIdSince }, { "PlanTier", config.PlanTier }, { "TrialExpiresUtc", config.TrialExpiresUtc }, { "TrialStartedUtc", config.TrialStartedUtc }, @@ -557,6 +560,10 @@ namespace AutopilotMonitor.Functions.DataAccess.TableStorage UnrestrictedMode = entity.GetBoolean("UnrestrictedMode") ?? false, EntraAppRolesEnabled = entity.GetBoolean("EntraAppRolesEnabled") ?? false, OnboardedAt = entity.GetDateTime("OnboardedAt"), + // Null = legacy app registration (rows pre-dating the C4A8 app-reg move). + HomedAppClientId = entity.GetString("HomedAppClientId"), + LastAuthClientId = entity.GetString("LastAuthClientId"), + LastAuthClientIdSince = entity.GetDateTime("LastAuthClientIdSince"), PlanTier = entity.GetString("PlanTier") ?? "free", TrialExpiresUtc = entity.GetDateTime("TrialExpiresUtc"), TrialStartedUtc = entity.GetDateTime("TrialStartedUtc"), diff --git a/src/Backend/AutopilotMonitor.Functions/Functions/Admin/AutopilotDeviceValidationConsentFunction.cs b/src/Backend/AutopilotMonitor.Functions/Functions/Admin/AutopilotDeviceValidationConsentFunction.cs index d5091c25..85eac552 100644 --- a/src/Backend/AutopilotMonitor.Functions/Functions/Admin/AutopilotDeviceValidationConsentFunction.cs +++ b/src/Backend/AutopilotMonitor.Functions/Functions/Admin/AutopilotDeviceValidationConsentFunction.cs @@ -31,6 +31,8 @@ public class AutopilotDeviceValidationConsentFunction private readonly IGraphFeatureDetector _graphFeatureDetector; private readonly TelemetryClient _telemetryClient; private readonly OpsEventService _opsEventService; + private readonly EntraAppRegistry _appRegistry; + private readonly TenantConfigurationService _tenantConfigService; /// /// Known redirect URIs registered in the Entra ID app registration for the admin consent flow. @@ -49,7 +51,9 @@ public class AutopilotDeviceValidationConsentFunction GraphTokenService graphTokenService, IGraphFeatureDetector graphFeatureDetector, TelemetryClient telemetryClient, - OpsEventService opsEventService) + OpsEventService opsEventService, + EntraAppRegistry appRegistry, + TenantConfigurationService tenantConfigService) { _logger = logger; _configuration = configuration; @@ -57,6 +61,8 @@ public class AutopilotDeviceValidationConsentFunction _graphFeatureDetector = graphFeatureDetector; _telemetryClient = telemetryClient; _opsEventService = opsEventService; + _appRegistry = appRegistry; + _tenantConfigService = tenantConfigService; } [Function("GetAutopilotDeviceValidationConsentUrl")] @@ -67,7 +73,11 @@ public class AutopilotDeviceValidationConsentFunction // Authentication enforced by PolicyEnforcementMiddleware var requestCtx = req.GetRequestContext(); - var validatorClientId = _configuration["EntraId:ClientId"]; + // Dual app-reg window: the consent URL must target the app registration that will + // actually mint this tenant's Graph tokens (legacy-homed tenants keep consenting the + // legacy app — parallel operation, no forced re-consent). + var tenantConfig = await _tenantConfigService.GetConfigurationIfExistsAsync(requestCtx.TargetTenantId); + var validatorClientId = _appRegistry.ResolveForTenant(tenantConfig).ClientId; if (string.IsNullOrWhiteSpace(validatorClientId)) { var badConfig = req.CreateResponse(HttpStatusCode.InternalServerError); diff --git a/src/Backend/AutopilotMonitor.Functions/Functions/Config/UpdateTenantConfigurationFunction.cs b/src/Backend/AutopilotMonitor.Functions/Functions/Config/UpdateTenantConfigurationFunction.cs index 25dee074..ccc543e3 100644 --- a/src/Backend/AutopilotMonitor.Functions/Functions/Config/UpdateTenantConfigurationFunction.cs +++ b/src/Backend/AutopilotMonitor.Functions/Functions/Config/UpdateTenantConfigurationFunction.cs @@ -20,15 +20,21 @@ namespace AutopilotMonitor.Functions.Functions.Config private readonly ILogger _logger; private readonly TenantConfigurationService _configService; private readonly IMaintenanceRepository _maintenanceRepo; + private readonly EntraAppRegistry _appRegistry; + private readonly Services.GraphResolution.IGraphFeatureDetector _graphFeatureDetector; public UpdateTenantConfigurationFunction( ILogger logger, TenantConfigurationService configService, - IMaintenanceRepository maintenanceRepo) + IMaintenanceRepository maintenanceRepo, + EntraAppRegistry appRegistry, + Services.GraphResolution.IGraphFeatureDetector graphFeatureDetector) { _logger = logger; _configService = configService; _maintenanceRepo = maintenanceRepo; + _appRegistry = appRegistry; + _graphFeatureDetector = graphFeatureDetector; } [Function("UpdateTenantConfiguration")] @@ -164,6 +170,7 @@ namespace AutopilotMonitor.Functions.Functions.Config config.CustomRateLimitRequestsPerMinute != existingConfig.CustomRateLimitRequestsPerMinute || config.CustomUserRateLimitRequestsPerMinute != existingConfig.CustomUserRateLimitRequestsPerMinute || config.Disabled != existingConfig.Disabled || + !string.Equals(config.HomedAppClientId, existingConfig.HomedAppClientId, StringComparison.OrdinalIgnoreCase) || config.ValidateDeviceAssociation != existingConfig.ValidateDeviceAssociation) { _logger.LogWarning( @@ -186,7 +193,37 @@ namespace AutopilotMonitor.Functions.Functions.Config // RequestContext + TenantConfigurationService). Tracked in // memory/project_devprep_followups.md. config.ValidateDeviceAssociation = existingConfig.ValidateDeviceAssociation; + // App-reg homing is the operator flip switch of the dual app-reg migration — + // never tenant-writable (a wrong value breaks the tenant's Graph + consent). + config.HomedAppClientId = existingConfig.HomedAppClientId; } + else + { + // GA flip of the tenant's app-reg homing: only null (= legacy invariant) or one + // of the two known app registrations is a valid target — anything else would + // silently route the tenant's Graph/consent to the primary fallback. + var requestedHoming = EntraAppRegistry.NormalizeClientId(config.HomedAppClientId); + if (!string.IsNullOrWhiteSpace(config.HomedAppClientId) && requestedHoming == null) + { + var badRequest = req.CreateResponse(HttpStatusCode.BadRequest); + await badRequest.WriteAsJsonAsync(new { success = false, message = "HomedAppClientId must be a GUID (or empty for the legacy app registration)." }); + return badRequest; + } + if (requestedHoming != null + && !_appRegistry.IsPrimary(requestedHoming) + && !string.Equals(requestedHoming, _appRegistry.Legacy?.ClientId, StringComparison.OrdinalIgnoreCase)) + { + var badRequest = req.CreateResponse(HttpStatusCode.BadRequest); + await badRequest.WriteAsJsonAsync(new { success = false, message = "HomedAppClientId must match the primary or legacy app registration client id." }); + return badRequest; + } + config.HomedAppClientId = requestedHoming; + } + + // 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; // Safety: if GA gate is off, force UnrestrictedMode to false if (!config.UnrestrictedModeEnabled) @@ -232,6 +269,20 @@ namespace AutopilotMonitor.Functions.Functions.Config // Save configuration await _configService.SaveConfigurationAsync(config); + // App-reg homing flip: drop the tenant's Graph token/roles caches so the next + // Graph call mints under the newly homed app instead of serving a token acquired + // under the previous one (fresh-read contract, mirrors the consent-grant path). + if (!string.Equals(config.HomedAppClientId, existingConfig.HomedAppClientId, StringComparison.OrdinalIgnoreCase)) + { + _logger.LogWarning( + "Tenant {TenantId} app-reg homing flipped: {Old} -> {New} (by {User})", + requestCtx.TargetTenantId, + existingConfig.HomedAppClientId ?? "(legacy)", + config.HomedAppClientId ?? "(legacy)", + userIdentifier); + _graphFeatureDetector.InvalidateTenant(requestCtx.TargetTenantId); + } + var changes = ConfigDiffHelper.GetChanges(existingConfig, config); await _maintenanceRepo.LogAuditEntryAsync( requestCtx.TargetTenantId, diff --git a/src/Backend/AutopilotMonitor.Functions/Functions/Graph/GetGraphPermissionsStatusFunction.cs b/src/Backend/AutopilotMonitor.Functions/Functions/Graph/GetGraphPermissionsStatusFunction.cs index 765bc787..18929db4 100644 --- a/src/Backend/AutopilotMonitor.Functions/Functions/Graph/GetGraphPermissionsStatusFunction.cs +++ b/src/Backend/AutopilotMonitor.Functions/Functions/Graph/GetGraphPermissionsStatusFunction.cs @@ -42,17 +42,23 @@ public class GetGraphPermissionsStatusFunction private readonly IGraphFeatureDetector _detector; private readonly IConfiguration _configuration; private readonly TelemetryClient _telemetry; + private readonly Security.EntraAppRegistry _appRegistry; + private readonly Services.TenantConfigurationService _tenantConfigService; public GetGraphPermissionsStatusFunction( ILogger logger, IGraphFeatureDetector detector, IConfiguration configuration, - TelemetryClient telemetry) + TelemetryClient telemetry, + Security.EntraAppRegistry appRegistry, + Services.TenantConfigurationService tenantConfigService) { _logger = logger; _detector = detector; _configuration = configuration; _telemetry = telemetry; + _appRegistry = appRegistry; + _tenantConfigService = tenantConfigService; } [Function("GetGraphPermissionsStatus")] @@ -87,9 +93,14 @@ public class GetGraphPermissionsStatusFunction EmitStatusChecked(requestCtx.TargetTenantId, requestCtx.UserPrincipalName, snapshot); + // Dual app-reg window: the surfaced client id pre-fills the customer's grant-script + // one-liner, so it must be the app that actually acts for this tenant (its homed app). + var tenantConfig = await _tenantConfigService.GetConfigurationIfExistsAsync(requestCtx.TargetTenantId); + var homedClientId = _appRegistry.ResolveForTenant(tenantConfig).ClientId; + return await req.OkAsync(new { - clientId = _configuration["EntraId:ClientId"] ?? string.Empty, + clientId = homedClientId ?? string.Empty, isTransient = snapshot.IsTransient, grantedRoles = snapshot.GrantedRoles.ToArray(), features, diff --git a/src/Backend/AutopilotMonitor.Functions/Functions/Infrastructure/AuthFunction.cs b/src/Backend/AutopilotMonitor.Functions/Functions/Infrastructure/AuthFunction.cs index 3aa5648e..9d49c595 100644 --- a/src/Backend/AutopilotMonitor.Functions/Functions/Infrastructure/AuthFunction.cs +++ b/src/Backend/AutopilotMonitor.Functions/Functions/Infrastructure/AuthFunction.cs @@ -30,6 +30,7 @@ public class AuthFunction private readonly GlobalNotificationService _globalNotificationService; private readonly McpUserService _mcpUserService; private readonly Services.Activation.ITenantAutoApproveEnqueuer _tenantAutoApproveEnqueuer; + private readonly EntraAppRegistry _appRegistry; public AuthFunction( ILogger logger, @@ -42,7 +43,8 @@ public class AuthFunction TelegramNotificationService telegramNotificationService, GlobalNotificationService globalNotificationService, McpUserService mcpUserService, - Services.Activation.ITenantAutoApproveEnqueuer tenantAutoApproveEnqueuer) + Services.Activation.ITenantAutoApproveEnqueuer tenantAutoApproveEnqueuer, + EntraAppRegistry appRegistry) { _logger = logger; _globalAdminService = globalAdminService; @@ -55,6 +57,7 @@ public class AuthFunction _globalNotificationService = globalNotificationService; _mcpUserService = mcpUserService; _tenantAutoApproveEnqueuer = tenantAutoApproveEnqueuer; + _appRegistry = appRegistry; } /// @@ -135,10 +138,14 @@ public class AuthFunction // still onboards normally; the handlers self-gate when the config already exists. var isDelegated = delegatedTenantIds.Count > 0; var isHomeTenantParticipant = !isDelegated || isGlobalAdmin || isGlobalReader || memberRole != null; + // Which app registration minted this login's token (dual app-reg window) — drives the + // onboarding homing decision and the per-tenant last-seen provenance below. + var tokenAudience = principal.GetAudience(); if (isHomeTenantParticipant) { - await HandleNewTenantDomainAsync(tenantConfig, tenantId, upn); + await HandleNewTenantDomainAsync(tenantConfig, tenantId, upn, tokenAudience); await HandleAutoReEnableAsync(tenantConfig, tenantId); + await HandleAuthClientIdTrackingAsync(tenantConfig, tenantId, tokenAudience); } // --- Pure decision logic (tested by AuthFunctionTests) --- @@ -146,7 +153,8 @@ public class AuthFunction tenantConfig, isGlobalAdmin, isGlobalReader, isApproved, memberRole, mcpCheck, existingAdmins.Count > 0, tenantId, upn, displayName ?? string.Empty, objectId ?? string.Empty, - delegatedTenantIds); + delegatedTenantIds, + homedApp: _appRegistry.ResolveForTenant(tenantConfig).IsLegacy ? "legacy" : "primary"); if (!decision.IsSuccess) { @@ -287,7 +295,7 @@ public class AuthFunction /// and fires best-effort notifications (Telegram + global notification). /// internal async Task HandleNewTenantDomainAsync( - TenantConfiguration tenantConfig, string tenantId, string upn) + TenantConfiguration tenantConfig, string tenantId, string upn, string? tokenAudience = null) { if (!string.IsNullOrEmpty(tenantConfig.DomainName) || string.IsNullOrEmpty(upn)) return; @@ -305,6 +313,18 @@ public class AuthFunction // sync jobs that mutate UpdatedBy cannot leak sentinel strings into TenantAdmins. if (string.IsNullOrWhiteSpace(tenantConfig.OnboardedBy)) tenantConfig.OnboardedBy = upn; + // Dual app-reg window: home a NEW tenant on the primary app ONLY when its first login + // actually arrived via the primary app. A first login via the legacy app leaves the field + // null (= legacy) — keeps the "null = legacy" invariant clean and never routes a tenant + // to an app it hasn't consented to. Seed the last-seen provenance in the same write. + var onboardingClientId = EntraAppRegistry.NormalizeAudience(tokenAudience); + if (onboardingClientId != null) + { + if (_appRegistry.IsPrimary(onboardingClientId)) + tenantConfig.HomedAppClientId = onboardingClientId; + tenantConfig.LastAuthClientId = onboardingClientId; + tenantConfig.LastAuthClientIdSince = DateTime.UtcNow; + } try { await _tenantConfigService.SaveConfigurationAsync(tenantConfig); @@ -348,6 +368,39 @@ public class AuthFunction TaskContinuationOptions.OnlyOnFaulted); } + /// + /// Pure observability for the app-reg migration: records which app registration this + /// tenant's logins actually arrive through (LastAuthClientId + since-when). + /// Write-on-change only — the common case (same app as last time) is a no-op, so this adds + /// zero table writes to steady-state logins. Best-effort: a failed persist must never break + /// auth/me; the next login that still differs retries. Gated on a non-empty DomainName so a + /// pre-onboarding login (row not persisted yet) can't create a phantom config row. + /// + internal async Task HandleAuthClientIdTrackingAsync( + TenantConfiguration tenantConfig, string tenantId, string? tokenAudience) + { + var clientId = EntraAppRegistry.NormalizeAudience(tokenAudience); + if (clientId == null || string.IsNullOrEmpty(tenantConfig.DomainName)) + return; + if (string.Equals(tenantConfig.LastAuthClientId, clientId, StringComparison.OrdinalIgnoreCase)) + return; + + _logger.LogInformation( + "Tenant {TenantId} logins now arrive via app registration {ClientId} (was: {Previous})", + tenantId, clientId, tenantConfig.LastAuthClientId ?? "(none recorded)"); + tenantConfig.LastAuthClientId = clientId; + tenantConfig.LastAuthClientIdSince = DateTime.UtcNow; + try + { + await _tenantConfigService.SaveConfigurationAsync(tenantConfig); + } + catch (Exception ex) + { + _logger.LogWarning(ex, + "Auth client-id tracking persist failed for tenant {TenantId} — will retry on a later login", tenantId); + } + } + /// /// If the tenant's suspension has expired (Disabled=true but DisabledUntil is past), /// clears the disabled state and persists the change. @@ -412,7 +465,8 @@ public class AuthFunction McpAccessCheckResult mcpCheck, bool hasTenantAdmins, string tenantId, string upn, string displayName, string objectId, - IReadOnlyCollection? delegatedTenantIds = null) + IReadOnlyCollection? delegatedTenantIds = null, + string homedApp = "primary") { // A delegated ("MSP") admin manages a subset of OTHER tenants. They are explicitly authorized, so — // like a Global Admin / Reader — they bypass the private-preview gate even when their own home tenant @@ -491,6 +545,10 @@ public class AuthFunction role, canManageBootstrapTokens, hasMcpAccess = mcpCheck.IsAllowed, + // Dual app-reg window: which app registration this tenant is homed on. The web app + // stores this per browser (localStorage) so the NEXT login uses the right app — + // the deferred, no-roundtrip learning mechanism of the parallel operation model. + homedApp, // EFFECTIVE availability flags (drive sidebar/section visibility): bootstrap is // included in Pro, Unrestricted Mode requires Pro + the GA on-request gate. bootstrapTokenEnabled = TenantEntitlementService.IsBootstrapEnabled(tenantConfig, DateTime.UtcNow), diff --git a/src/Backend/AutopilotMonitor.Functions/Helpers/TenantConfigProjection.cs b/src/Backend/AutopilotMonitor.Functions/Helpers/TenantConfigProjection.cs index d9732370..2f1a8980 100644 --- a/src/Backend/AutopilotMonitor.Functions/Helpers/TenantConfigProjection.cs +++ b/src/Backend/AutopilotMonitor.Functions/Helpers/TenantConfigProjection.cs @@ -30,6 +30,11 @@ namespace AutopilotMonitor.Functions.Helpers ("onboardedBy", c => c.OnboardedBy), ("lastUpdated", c => c.LastUpdated), ("dataRetentionDays", c => c.DataRetentionDays), + // App-reg migration observability: which app the tenant is homed on (null = legacy) + // and which app its users' logins actually arrive through. Public client ids, not secrets. + ("homedAppClientId", c => c.HomedAppClientId), + ("lastAuthClientId", c => c.LastAuthClientId), + ("lastAuthClientIdSince", c => c.LastAuthClientIdSince), }; /// diff --git a/src/Backend/AutopilotMonitor.Functions/Middleware/AuthenticationMiddleware.cs b/src/Backend/AutopilotMonitor.Functions/Middleware/AuthenticationMiddleware.cs index 7154701f..3dd00c31 100644 --- a/src/Backend/AutopilotMonitor.Functions/Middleware/AuthenticationMiddleware.cs +++ b/src/Backend/AutopilotMonitor.Functions/Middleware/AuthenticationMiddleware.cs @@ -165,12 +165,15 @@ public class AuthenticationMiddleware : IFunctionsWorkerMiddleware // Extracted to a pure seam so those rejects are unit-testable against locally-minted // tokens without a live OIDC ConfigurationManager. // - // Audience = the primary app registration (EntraId:ClientId) PLUS any additional - // (secondary/rotated) client IDs from EntraId:AdditionalClientIds. That config is the - // seam that lets the API trust a SECOND app registration during a tenant-move window - // (see tasks/migration-cross-tenant-runbook.md). Unset today ⇒ exactly the primary id - // ⇒ zero behaviour change. - var additionalClientIdsRaw = _configuration["EntraId:AdditionalClientIds"]; + // Audience = the primary app registration (EntraId:ClientId) PLUS the legacy app + // (EntraId:LegacyClientId — the dual app-registration parallel window, see + // EntraAppRegistry) PLUS any additional (secondary/rotated) client IDs from + // EntraId:AdditionalClientIds. Folding the legacy id in here means configuring the + // legacy pair alone is sufficient for inbound trust — no separate AdditionalClientIds + // entry required. All extra sources unset ⇒ exactly the primary id ⇒ zero change. + var additionalClientIdsRaw = CombineAdditionalClientIdSources( + _configuration["EntraId:LegacyClientId"], + _configuration["EntraId:AdditionalClientIds"]); var clientIds = ResolveConfiguredClientIds( _configuration["EntraId:ClientId"], additionalClientIdsRaw, out var rejectedAdditionalEntries); AuditClientIdTrustSet(additionalClientIdsRaw, clientIds, rejectedAdditionalEntries); @@ -342,6 +345,18 @@ public class AuthenticationMiddleware : IFunctionsWorkerMiddleware /// needed to trust a second app during a tenant-move window /// (tasks/migration-cross-tenant-runbook.md). /// + /// + /// Joins multiple "additional client id" config sources (e.g. EntraId:LegacyClientId and + /// EntraId:AdditionalClientIds) into one comma-separated string for + /// . Null/whitespace sources are skipped; all-empty + /// input yields null (identical to "no additional ids configured"). + /// + internal static string? CombineAdditionalClientIdSources(params string?[] sources) + { + var nonEmpty = sources.Where(s => !string.IsNullOrWhiteSpace(s)).Select(s => s!.Trim()).ToArray(); + return nonEmpty.Length > 0 ? string.Join(",", nonEmpty) : null; + } + internal static string[] ResolveConfiguredClientIds( string? primaryClientId, string? additionalClientIds, out string[] rejectedAdditionalEntries) { diff --git a/src/Backend/AutopilotMonitor.Functions/Program.cs b/src/Backend/AutopilotMonitor.Functions/Program.cs index 7bdb652b..6be07793 100644 --- a/src/Backend/AutopilotMonitor.Functions/Program.cs +++ b/src/Backend/AutopilotMonitor.Functions/Program.cs @@ -121,14 +121,17 @@ builder.Services.AddAuthentication(JwtBearerDefaults.AuthenticationScheme) AutopilotMonitor.Shared.Constants.EntraLoginBaseUrl + "/organizations/v2.0", "https://sts.windows.net/{tenantid}/" }, - // Temporarily accept Microsoft Graph tokens (used by frontend with User.Read scope) - // TODO: Later expose custom API and use api://{clientId} scopes - ValidAudiences = new[] - { - builder.Configuration["EntraId:ClientId"] // Our app's client ID - //"https://graph.microsoft.com", // Microsoft Graph - //"00000003-0000-0000-c000-000000000000" // Microsoft Graph App ID - } + // Full audience trust set — MUST match AuthenticationMiddleware's per-request set + // (primary + legacy + additional client ids, each in bare and api:// form). Built via + // the SAME shared helpers so the two lists cannot drift; a single-audience list here + // would 401 legacy-app tokens on [Authorize]-gated paths during the dual app-reg window. + ValidAudiences = AutopilotMonitor.Functions.Middleware.AuthenticationMiddleware.BuildValidAudiences( + AutopilotMonitor.Functions.Middleware.AuthenticationMiddleware.ResolveConfiguredClientIds( + builder.Configuration["EntraId:ClientId"], + AutopilotMonitor.Functions.Middleware.AuthenticationMiddleware.CombineAdditionalClientIdSources( + builder.Configuration["EntraId:LegacyClientId"], + builder.Configuration["EntraId:AdditionalClientIds"]), + out _)) }; }); @@ -384,6 +387,7 @@ builder.Services.AddSingleton(); builder.Services.AddSingleton(); builder.Services.AddSingleton(); builder.Services.AddSingleton(); +builder.Services.AddSingleton(); builder.Services.AddSingleton(); builder.Services.AddSingleton(); builder.Services.AddSingleton(); @@ -421,6 +425,13 @@ if (string.IsNullOrEmpty(entraClientId)) startupLogger.LogWarning("EntraId:ClientId is not configured — JWT audience validation and Graph API calls will fail"); if (string.IsNullOrEmpty(entraClientSecret)) startupLogger.LogWarning("EntraId:ClientSecret is not configured — device validation via Graph API will fail at runtime"); +// Dual app-registration window: the legacy pair must be set together — an id without a secret +// breaks Graph for every legacy-homed tenant, a secret without an id is dead config. +var entraLegacyClientId = builder.Configuration["EntraId:LegacyClientId"]; +var entraLegacyClientSecret = builder.Configuration["EntraId:LegacyClientSecret"]; +if (string.IsNullOrEmpty(entraLegacyClientId) != string.IsNullOrEmpty(entraLegacyClientSecret)) + startupLogger.LogWarning( + "EntraId:LegacyClientId and EntraId:LegacyClientSecret must be configured together — only one is set, legacy-homed tenants will fail Graph token acquisition"); // Log CORS configuration at startup so misconfigured origins are immediately visible // in the log stream. CORS is enforced by Azure infrastructure, not by function code, diff --git a/src/Backend/AutopilotMonitor.Functions/Security/EntraAppRegistry.cs b/src/Backend/AutopilotMonitor.Functions/Security/EntraAppRegistry.cs new file mode 100644 index 00000000..d386ca79 --- /dev/null +++ b/src/Backend/AutopilotMonitor.Functions/Security/EntraAppRegistry.cs @@ -0,0 +1,144 @@ +using System; +using AutopilotMonitor.Functions.Middleware; +using AutopilotMonitor.Shared.Models; +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.Logging; + +namespace AutopilotMonitor.Functions.Security +{ + /// + /// Credentials of one Entra app registration the platform operates under. + /// marks the pre-migration app (gktatooine.net home tenant). + /// + public sealed record EntraAppCredentials(string ClientId, string? ClientSecret, bool IsLegacy); + + /// + /// Single source for "which Entra app registration acts on behalf of this tenant" during the + /// dual app-registration parallel window (legacy gktatooine app ∥ new C4A8 app). + /// + /// Config axes: + /// - EntraId:ClientId / EntraId:ClientSecret — the PRIMARY app (post-swap: the new C4A8 app). + /// - EntraId:LegacyClientId / EntraId:LegacyClientSecret — the pre-migration app. + /// + /// Resolution contract (): + /// - Legacy app not configured ⇒ ALWAYS primary. This makes deploying this code a no-op until + /// the operator performs the config swap (primary→new + legacy→old) — before that swap the + /// primary IS the old app and behaviour is byte-identical to today. + /// - null ⇒ legacy (the invariant: every + /// tenant onboarded before the migration has no homing value and stays on the old app). + /// - Equal to the primary/legacy client id ⇒ that app. Unknown value ⇒ primary + warning + /// (fail toward the app new consents land on; an unknown id is an operator typo). + /// + /// Token VALIDATION (inbound audiences) is not decided here — the middleware trusts primary + + /// legacy + EntraId:AdditionalClientIds simultaneously; this registry only picks the app used + /// for OUTBOUND artifacts: Graph client-credential tokens, admin-consent URLs, and the client + /// id surfaced to tenant admins. + /// + public class EntraAppRegistry + { + private readonly ILogger _logger; + private readonly EntraAppCredentials _primary; + private readonly EntraAppCredentials? _legacy; + private readonly string? _primaryClientIdNormalized; + + public EntraAppRegistry(IConfiguration configuration, ILogger logger) + { + _logger = logger; + + // Primary stays verbatim (same contract as AuthenticationMiddleware: its handling is + // unchanged and never validated away). Normalized form is only used for comparisons. + var primaryClientId = configuration["EntraId:ClientId"] ?? string.Empty; + _primary = new EntraAppCredentials(primaryClientId, configuration["EntraId:ClientSecret"], IsLegacy: false); + _primaryClientIdNormalized = NormalizeClientId(primaryClientId); + + var legacyClientIdRaw = configuration["EntraId:LegacyClientId"]; + var legacyClientId = NormalizeClientId(legacyClientIdRaw); + if (legacyClientId != null) + { + _legacy = new EntraAppCredentials(legacyClientId, configuration["EntraId:LegacyClientSecret"], IsLegacy: true); + } + else if (!string.IsNullOrWhiteSpace(legacyClientIdRaw)) + { + // Malformed (non-GUID) legacy id: treat as unset (fail toward primary-only, i.e. + // pre-swap behaviour) but make the misconfiguration operator-visible — a typo here + // would otherwise surface as "legacy tenants lost Graph/consent" with no clue. + _logger.LogWarning( + "EntraId:LegacyClientId is not a valid GUID — ignoring the legacy app registration: {Entry}", + AuthenticationMiddleware.TruncateForLog(legacyClientIdRaw.Trim())); + } + } + + /// The primary app registration (post-swap: the new C4A8 app). + public EntraAppCredentials Primary => _primary; + + /// The legacy app registration, or null while the config swap hasn't happened. + public EntraAppCredentials? Legacy => _legacy; + + /// True once both app registrations are configured (the parallel window is active). + public bool LegacyConfigured => _legacy != null; + + /// + /// Picks the app registration that acts on behalf of the given tenant (Graph + /// client-credential tokens, admin-consent URLs, surfaced client id). + /// + public virtual EntraAppCredentials ResolveForTenant(TenantConfiguration? tenantConfig) + { + if (_legacy == null) + { + return _primary; + } + + var homed = NormalizeClientId(tenantConfig?.HomedAppClientId); + if (homed == null) + { + // Null homing = onboarded before the migration ⇒ legacy app. + return _legacy; + } + if (homed.Equals(_primaryClientIdNormalized, StringComparison.OrdinalIgnoreCase)) + { + return _primary; + } + if (homed.Equals(_legacy.ClientId, StringComparison.OrdinalIgnoreCase)) + { + return _legacy; + } + + _logger.LogWarning( + "Tenant {TenantId} has unknown HomedAppClientId {HomedAppClientId} — falling back to the primary app registration", + tenantConfig?.TenantId, homed); + return _primary; + } + + /// True when the given client id (bare or api://-prefixed) is the primary app's. + public virtual bool IsPrimary(string? clientIdOrAudience) + { + var normalized = NormalizeAudience(clientIdOrAudience); + return normalized != null + && normalized.Equals(_primaryClientIdNormalized, StringComparison.OrdinalIgnoreCase); + } + + /// + /// Normalizes a token audience to the canonical lowercase dashed client id: strips an + /// optional api:// prefix and requires a GUID. Null for anything else — a Graph or + /// unknown audience must never be persisted as app-reg provenance. + /// + public static string? NormalizeAudience(string? audience) + { + if (string.IsNullOrWhiteSpace(audience)) return null; + var value = audience.Trim(); + const string apiPrefix = "api://"; + if (value.StartsWith(apiPrefix, StringComparison.OrdinalIgnoreCase)) + { + value = value.Substring(apiPrefix.Length); + } + return NormalizeClientId(value); + } + + /// Canonical lowercase dashed GUID form, or null when not a GUID. + public static string? NormalizeClientId(string? clientId) + { + if (string.IsNullOrWhiteSpace(clientId)) return null; + return Guid.TryParse(clientId.Trim(), out var parsed) ? parsed.ToString("D") : null; + } + } +} diff --git a/src/Backend/AutopilotMonitor.Functions/Security/GraphTokenService.cs b/src/Backend/AutopilotMonitor.Functions/Security/GraphTokenService.cs index ec6f8eb4..abce949a 100644 --- a/src/Backend/AutopilotMonitor.Functions/Security/GraphTokenService.cs +++ b/src/Backend/AutopilotMonitor.Functions/Security/GraphTokenService.cs @@ -51,22 +51,55 @@ namespace AutopilotMonitor.Functions.Security private readonly IHttpClientFactory _httpClientFactory; private readonly IMemoryCache _cache; private readonly IConfiguration _configuration; + private readonly EntraAppRegistry _appRegistry; + private readonly Services.TenantConfigurationService _tenantConfigService; public GraphTokenService( ILogger logger, IHttpClientFactory httpClientFactory, IMemoryCache cache, - IConfiguration configuration) + IConfiguration configuration, + EntraAppRegistry appRegistry, + Services.TenantConfigurationService tenantConfigService) { _logger = logger; _httpClientFactory = httpClientFactory; _cache = cache; _configuration = configuration; + _appRegistry = appRegistry; + _tenantConfigService = tenantConfigService; + } + + /// + /// Picks the app registration acting for this tenant (dual app-reg window). While the + /// legacy pair is unconfigured this is always the primary app — today's behaviour. A + /// config read failure falls back to the null-homing default (legacy when configured): + /// wrong-app picks surface as an uncached permanent failure and self-heal on retry. + /// + private async Task ResolveCredentialsAsync(string tenantId) + { + if (!_appRegistry.LegacyConfigured) + { + return _appRegistry.Primary; + } + try + { + var config = await _tenantConfigService.GetConfigurationIfExistsAsync(tenantId); + return _appRegistry.ResolveForTenant(config); + } + catch (Exception ex) + { + _logger.LogWarning(ex, + "Failed to load tenant configuration for {TenantId} while resolving the Entra app — using the null-homing default", + tenantId); + return _appRegistry.ResolveForTenant(null); + } } public async Task GetConsentStatusAsync(string tenantId, CancellationToken ct = default) { - var cacheKey = ConsentCacheKey(tenantId); + var credentials = await ResolveCredentialsAsync(tenantId); + var cacheKey = ConsentCacheKey(tenantId, credentials.ClientId); if (_cache.TryGetValue(cacheKey, out GraphConsentStatusResult? cached) && cached != null) { return cached; @@ -104,21 +137,27 @@ namespace AutopilotMonitor.Functions.Security public virtual async Task GetAccessTokenAsync(string tenantId, CancellationToken ct = default) { - // Serve a still-valid cached app token for this tenant. The cache key is keyed strictly - // on the (already authenticated upstream) tenantId — the same value that drives the token - // URL below — so a token minted for one tenant can never be served to another. - if (TryGetCachedToken(tenantId, out var cachedToken)) + // Resolve which app registration acts for this tenant FIRST — the cache key includes + // the resolved client id, so a tenant flipped between apps can never be served a token + // minted under the other app. + var credentials = await ResolveCredentialsAsync(tenantId); + var clientId = credentials.ClientId; + var clientSecret = credentials.ClientSecret; + + // Serve a still-valid cached app token for this tenant+app. The cache key is keyed + // strictly on the (already authenticated upstream) tenantId — the same value that drives + // the token URL below — so a token minted for one tenant can never be served to another. + if (TryGetCachedToken(tenantId, clientId, out var cachedToken)) { return GraphTokenResult.Success(cachedToken); } - var clientId = _configuration["EntraId:ClientId"]; - var clientSecret = _configuration["EntraId:ClientSecret"]; - if (string.IsNullOrWhiteSpace(clientId) || string.IsNullOrWhiteSpace(clientSecret)) { _logger.LogError( - "Device validation is enabled but Entra ID app credentials are not configured. Set EntraId:ClientId and EntraId:ClientSecret."); + "Device validation is enabled but Entra ID app credentials are not configured for the {App} app registration. Set the EntraId {SettingPair} client id/secret pair.", + credentials.IsLegacy ? "legacy" : "primary", + credentials.IsLegacy ? "LegacyClientId/LegacyClientSecret" : "ClientId/ClientSecret"); return GraphTokenResult.PermanentFailure(); } @@ -145,7 +184,7 @@ namespace AutopilotMonitor.Functions.Security // Double-check: another thread may have populated the cache while we waited. The // gen-aware read also rejects a stale entry written by an acquire that overlapped an // InvalidateTenant, so we fall through to a fresh mint instead of serving it. - if (gotGate && TryGetCachedToken(tenantId, out var tokenAfterWait)) + if (gotGate && TryGetCachedToken(tenantId, clientId, out var tokenAfterWait)) { return GraphTokenResult.Success(tokenAfterWait); } @@ -212,7 +251,7 @@ namespace AutopilotMonitor.Functions.Security { // Cache the freshly minted token (never log its value). TTL derives from // the token's own expires_in, minus a skew margin, clamped to a safe band. - CacheToken(tenantId, accessToken!, genAtStart, (long?)tokenJson?["expires_in"]); + CacheToken(tenantId, clientId, accessToken!, genAtStart, (long?)tokenJson?["expires_in"]); } return GraphTokenResult.Success(accessToken); } @@ -299,15 +338,21 @@ namespace AutopilotMonitor.Functions.Security } } - private static string TokenCacheKey(string tenantId) => $"graph-token:{tenantId}"; - private static string ConsentCacheKey(string tenantId) => $"graph-consent-status:{tenantId}"; + // Keys include the resolved client id: during the dual app-reg window a tenant flip + // (HomedAppClientId change) must never be served a token/verdict minted under the other + // app. Normalized to lowercase so casing differences in config can't split the cache. + private static string TokenCacheKey(string tenantId, string? clientId) => + $"graph-token:{tenantId}:{clientId?.ToLowerInvariant()}"; + private static string ConsentCacheKey(string tenantId, string? clientId) => + $"graph-consent-status:{tenantId}:{clientId?.ToLowerInvariant()}"; /// - /// Drops this tenant's cached app token (and consent-status verdict) so the next acquire - /// mints a fresh JWT from AAD. Required for the consent / Graph-permission fresh-read - /// contract: after an admin grants consent or assigns app roles, the OLD token still carries - /// the OLD roles claim until it expires — serving it would make a post-grant "Refresh" - /// keep reporting "not granted" for up to the cache TTL. The single fresh-read entry point + /// Drops this tenant's cached app tokens (and consent-status verdicts) — for BOTH app + /// registrations — so the next acquire mints a fresh JWT from AAD. Required for the + /// consent / Graph-permission fresh-read contract: after an admin grants consent or assigns + /// app roles, the OLD token still carries the OLD roles claim until it expires — + /// serving it would make a post-grant "Refresh" keep reporting "not granted" for up to the + /// cache TTL. Also invoked on a HomedAppClientId flip. The single fresh-read entry point /// GraphFeatureDetector.InvalidateTenant calls this so BOTH cache layers are cleared /// together (the detector's parsed-roles cache AND this raw-token cache). /// @@ -317,18 +362,23 @@ namespace AutopilotMonitor.Functions.Security // Bump the generation FIRST so any token POST already in flight (which captured the old // generation) writes an entry that readers reject as stale — this closes the // write-after-invalidate race, not just the already-written entry. Then drop the current - // entries to free memory and clear the coarse consent-status verdict. + // entries (both apps' key shapes) to free memory and clear the consent-status verdicts. _tokenCacheGen.AddOrUpdate(tenantId, 1, (_, v) => v + 1); - _cache.Remove(TokenCacheKey(tenantId)); - _cache.Remove(ConsentCacheKey(tenantId)); + _cache.Remove(TokenCacheKey(tenantId, _appRegistry.Primary.ClientId)); + _cache.Remove(ConsentCacheKey(tenantId, _appRegistry.Primary.ClientId)); + if (_appRegistry.Legacy != null) + { + _cache.Remove(TokenCacheKey(tenantId, _appRegistry.Legacy.ClientId)); + _cache.Remove(ConsentCacheKey(tenantId, _appRegistry.Legacy.ClientId)); + } } - private bool TryGetCachedToken(string tenantId, out string token) + private bool TryGetCachedToken(string tenantId, string? clientId, out string token) { // Accept the cached token only if its generation still matches the tenant's current one. // A mismatch means an InvalidateTenant ran since this entry's acquire started, so the // token may carry a stale roles claim — treat it as a miss and re-mint. - if (_cache.TryGetValue(TokenCacheKey(tenantId), out CachedAppToken? cached) + if (_cache.TryGetValue(TokenCacheKey(tenantId, clientId), out CachedAppToken? cached) && cached != null && !string.IsNullOrEmpty(cached.Token) && cached.Gen == CurrentGen(tenantId)) @@ -340,7 +390,7 @@ namespace AutopilotMonitor.Functions.Security return false; } - private void CacheToken(string tenantId, string accessToken, long genAtStart, long? expiresInSeconds) + private void CacheToken(string tenantId, string clientId, string accessToken, long genAtStart, long? expiresInSeconds) { // expires_in is reported by AAD over TLS, so it is trustworthy — but still clamped so a // missing or pathological value cannot pin a stale token (cap) or cause thrash (floor). @@ -351,7 +401,7 @@ namespace AutopilotMonitor.Functions.Security if (ttl < MinTokenTtl) ttl = MinTokenTtl; if (ttl > MaxTokenTtl) ttl = MaxTokenTtl; - _cache.Set(TokenCacheKey(tenantId), new CachedAppToken(accessToken, genAtStart), new MemoryCacheEntryOptions + _cache.Set(TokenCacheKey(tenantId, clientId), new CachedAppToken(accessToken, genAtStart), new MemoryCacheEntryOptions { AbsoluteExpirationRelativeToNow = ttl, }); diff --git a/src/Shared/AutopilotMonitor.Shared/Models/Config/TenantConfiguration.cs b/src/Shared/AutopilotMonitor.Shared/Models/Config/TenantConfiguration.cs index 84f95b0a..8573e0e1 100644 --- a/src/Shared/AutopilotMonitor.Shared/Models/Config/TenantConfiguration.cs +++ b/src/Shared/AutopilotMonitor.Shared/Models/Config/TenantConfiguration.cs @@ -70,6 +70,32 @@ namespace AutopilotMonitor.Shared.Models /// public DateTime? OnboardedAt { get; set; } + // ===== ENTRA APP REGISTRATION HOMING ===== + + /// + /// Client id of the Entra app registration this tenant is homed on. Drives which app + /// mints Graph client-credential tokens and admin-consent URLs for the tenant, and which + /// app the portal signs the tenant's users in with (via the auth/me "homedApp" field). + /// Null = the legacy (pre-migration) app registration — the invariant for every tenant + /// onboarded before the C4A8 move. Set to the primary client id at onboarding when the + /// first login arrived via the primary app; flipped by a Global Admin after a tenant + /// re-consents to the new app (GA-only field, see UpdateTenantConfigurationFunction). + /// + public string? HomedAppClientId { get; set; } + + /// + /// Client id observed in the most recent portal login token's audience — pure + /// observability for the app-reg migration (which app a tenant's users actually + /// arrive through), never used for routing decisions. Written on change only. + /// + public string? LastAuthClientId { get; set; } + + /// + /// When last changed (i.e. logins arrive via that app + /// since this instant). Null on rows that pre-date the field. + /// + public DateTime? LastAuthClientIdSince { get; set; } + /// /// Whether this tenant is disabled/suspended /// If true, users from this tenant cannot log in diff --git a/src/Web/autopilot-monitor-web/app/get-started/page.tsx b/src/Web/autopilot-monitor-web/app/get-started/page.tsx index 53c7a570..50239424 100644 --- a/src/Web/autopilot-monitor-web/app/get-started/page.tsx +++ b/src/Web/autopilot-monitor-web/app/get-started/page.tsx @@ -87,7 +87,7 @@ export default function GetStartedPage() { {/* CTA */}
- + Sign in to get started + {homedOnLegacyApp && ( +
+ Autopilot device validation currently runs on the previous Autopilot Monitor app + registration. It keeps working as-is — at your convenience, please re-consent to the + new app registration; we will reach out with details as part of the migration. +
+ )} consumeLoginDeclined()); useEffect(() => { if (!isLoading && !isAuthenticated) { @@ -55,6 +63,12 @@ export function ProtectedRoute({ children, requireGlobalAdmin = false, requireGl if (isOnPublicHost()) { return; } + // Failed state (consent declined / earlier login error): stand down so the manual + // retry UI stays in control instead of auto-firing another redirect. + if (loginFailed) { + reloginAttempted.current = true; + return; + } if (!reloginAttempted.current) { // Trigger MSAL login redirect once. On portal this is also the // entry-point flow for users who arrived via a www → portal @@ -76,7 +90,7 @@ export function ProtectedRoute({ children, requireGlobalAdmin = false, requireGl }); } } - }, [isAuthenticated, isLoading, router, login]); + }, [isAuthenticated, isLoading, router, login, loginFailed]); // Sign-in failed (e.g. an interrupted earlier redirect left MSAL in // interaction_in_progress) — stay on this origin and offer a manual retry. @@ -95,6 +109,17 @@ export function ProtectedRoute({ children, requireGlobalAdmin = false, requireGl > Sign in + {legacyConfigured() && activeAuthApp !== "legacy" && ( +

+ Existing customer?{" "} + +

+ )}
); diff --git a/src/Web/autopilot-monitor-web/components/landing/LoginButton.tsx b/src/Web/autopilot-monitor-web/components/landing/LoginButton.tsx index 544c793d..39f1b5b2 100644 --- a/src/Web/autopilot-monitor-web/components/landing/LoginButton.tsx +++ b/src/Web/autopilot-monitor-web/components/landing/LoginButton.tsx @@ -2,13 +2,22 @@ import { useAuth } from "../../contexts/AuthContext"; import { getPortalLoginUrl, shouldCrossOriginToPortal } from "../../lib/hostRouting"; +import { getSelectedAuthApp, legacyConfigured, switchAuthApp } from "../../lib/authApp"; export function LoginButton({ className, children, + signup = false, }: { className?: string; children: React.ReactNode; + /** + * Signup-funnel CTA (get-started "Sign in to get started"): routes the login through the + * PRIMARY app registration so brand-new tenants consent the NEW app as part of the expected + * signup flow — while the plain "Sign in" default keeps existing customers on their app + * (dual app-reg window; localStorage is per-origin, hence the ?authapp handover to portal). + */ + signup?: boolean; }) { const { login } = useAuth(); @@ -19,7 +28,15 @@ export function LoginButton({ // portal after the post-auth redirect — and with prompt:"select_account" // that is not actually silent. if (shouldCrossOriginToPortal()) { - window.location.href = getPortalLoginUrl(); + window.location.href = signup && legacyConfigured() + ? `${getPortalLoginUrl()}?authapp=primary` + : getPortalLoginUrl(); + return; + } + if (signup && legacyConfigured() && getSelectedAuthApp() !== "primary") { + // Same-origin signup click while the bundle booted with the legacy app: persist the + // choice and reboot so the module-level MSAL instance is reconstructed for primary. + switchAuthApp("primary"); return; } void login(); diff --git a/src/Web/autopilot-monitor-web/contexts/AuthContext.tsx b/src/Web/autopilot-monitor-web/contexts/AuthContext.tsx index 4a26c9ad..67e72f66 100644 --- a/src/Web/autopilot-monitor-web/contexts/AuthContext.tsx +++ b/src/Web/autopilot-monitor-web/contexts/AuthContext.tsx @@ -3,10 +3,22 @@ import React, { createContext, useContext, useEffect, useState, useCallback, useRef } from 'react'; import { PublicClientApplication, AccountInfo, InteractionStatus, InteractionRequiredAuthError, BrowserAuthError } from '@azure/msal-browser'; import { MsalProvider, useMsal, useIsAuthenticated } from '@azure/msal-react'; -import { msalConfig, loginRequest, apiRequest } from '@/lib/msalConfig'; +import { msalConfig, loginRequest, apiRequest, activeAuthApp, buildMsalConfig, clientIdForApp } from '@/lib/msalConfig'; +import { + classifyEntraAuthError, + clearLoginAttemptApp, + clearLoginFallback, + legacyConfigured, + markLoginDeclined, + otherApp, + setLoginAttemptApp, + setSelectedAuthApp, + tryBeginLoginFallback, +} from '@/lib/authApp'; import { api } from '@/lib/api'; -// Initialize MSAL instance +// Initialize MSAL instance for the ACTIVE app registration (dual app-reg window: the +// selection was decided at module boot in lib/msalConfig.ts; app switches always reload). const msalInstance = new PublicClientApplication(msalConfig); // Track MSAL initialization state so components can wait for it. @@ -20,10 +32,54 @@ let msalReady = false; // otherwise a cold backend would keep the UI on a white screen. let prefetchedAuthMePromise: Promise | null> | null = null; +/** + * Safety net of the dual app-reg model, for the rare browser that lands on an app its tenant + * never consented to (no stored selection, wrong default): Entra reports "needs admin + * approval" (AADSTS90094/65001) on the redirect return, and we retry ONCE with the other app + * — silently, because the user just authenticated so an Entra session exists. An explicit + * consent DECLINE (AADSTS65004) never falls back: for a brand-new tenant that would + * mis-consent the legacy app; ProtectedRoute shows the failed screen instead. + * Returns true when a fallback redirect was started (page is navigating away). + */ +async function tryHandleRedirectAuthError(error: unknown): Promise { + const err = error as { errorMessage?: string; message?: string } | null; + const classification = classifyEntraAuthError(String(err?.errorMessage ?? err?.message ?? '')); + clearLoginAttemptApp(); + if (classification === 'declined') { + markLoginDeclined(); + return false; + } + if (classification !== 'admin-approval-required' || !legacyConfigured() || !tryBeginLoginFallback()) { + return false; + } + const target = otherApp(activeAuthApp); + console.warn(`[Auth] App registration not consented in this tenant — retrying via the ${target} app`); + try { + setLoginAttemptApp(target); + const fallbackInstance = new PublicClientApplication(buildMsalConfig(clientIdForApp(target))); + await fallbackInstance.initialize(); + // No prompt: the user just entered credentials, so the Entra session completes silently. + await fallbackInstance.loginRedirect({ ...loginRequest, prompt: undefined }); + return true; + } catch (fallbackError) { + console.error('[Auth] Cross-app fallback login failed:', fallbackError); + clearLoginAttemptApp(); + return false; + } +} + const msalInitPromise = msalInstance .initialize() .then(() => msalInstance.handleRedirectPromise()) - .then(() => { + .then((redirectResult) => { + if (redirectResult?.account) { + // Successful redirect sign-in on the active app: remember it as this browser's app + // (the "learn" half of the model — future boots go straight to the right one). + setSelectedAuthApp(activeAuthApp); + clearLoginFallback(); + } + // Redirect settled (or none was in flight) — the attempt marker has served its purpose. + clearLoginAttemptApp(); msalReady = true; // Kick off prefetch while React is still mounting. fetchUserInfo will @@ -42,9 +98,11 @@ const msalInitPromise = msalInstance }).catch(() => null); } }) - .catch((error) => { + .catch(async (error) => { console.error('[Auth] MSAL initialization/redirect error:', error); - // Mark as ready even on error so the app doesn't hang forever. + await tryHandleRedirectAuthError(error); + // Mark as ready even on error so the app doesn't hang forever (if the fallback + // started a redirect the page is navigating away anyway). // Auth operations will fail individually and trigger appropriate recovery. msalReady = true; }); @@ -68,6 +126,8 @@ async function safeAcquireTokenRedirect( } redirectInFlight = true; try { + // Pin the redirect return to the app that starts it (see lib/authApp.ts). + setLoginAttemptApp(activeAuthApp); await instance.acquireTokenRedirect({ scopes: apiRequest.scopes, account, @@ -133,6 +193,20 @@ interface AuthContextType { refreshUserInfo: () => Promise; } +/** + * Background learning of the dual app-reg model: auth/me returns which app registration the + * tenant is homed on; storing it makes the NEXT login on this browser use that app directly. + * No reload — the current session keeps its valid token (both audiences are accepted). + * This is also how an operator flip propagates: flip → next login of every user runs via the + * new app, at which point the tenant's admin consent already exists. + */ +function learnHomedAppFromAuthMe(data: Record): void { + const homedApp = data.homedApp; + if (homedApp === 'legacy' || homedApp === 'primary') { + setSelectedAuthApp(homedApp); + } +} + const AuthContext = createContext(undefined); /** @@ -169,6 +243,7 @@ function AuthProviderInternal({ children }: { children: React.ReactNode }) { const pending = prefetchedAuthMePromise; prefetchedAuthMePromise = null; const data = await pending; + if (data) learnHomedAppFromAuthMe(data); if (data) return { displayName: (data.displayName as string) || account.name || '', upn: (data.upn as string) || account.username || '', @@ -255,6 +330,8 @@ function AuthProviderInternal({ children }: { children: React.ReactNode }) { setActivationPending(false); setActivationMessage(''); + learnHomedAppFromAuthMe(data as Record); + return { displayName: data.displayName || account.name || '', upn: data.upn || account.username || '', @@ -379,6 +456,8 @@ function AuthProviderInternal({ children }: { children: React.ReactNode }) { const request = options?.auto ? { ...loginRequest, prompt: undefined } : loginRequest; + // Pin the redirect return to the app that starts it (see lib/authApp.ts). + setLoginAttemptApp(activeAuthApp); await instance.loginRedirect(request); } catch (error: unknown) { // Ignore interaction_in_progress errors - this can happen if user clicks button multiple times diff --git a/src/Web/autopilot-monitor-web/lib/__tests__/authApp.test.ts b/src/Web/autopilot-monitor-web/lib/__tests__/authApp.test.ts new file mode 100644 index 00000000..df148a3a --- /dev/null +++ b/src/Web/autopilot-monitor-web/lib/__tests__/authApp.test.ts @@ -0,0 +1,153 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { + classifyEntraAuthError, + consumeLoginDeclined, + getBootAuthApp, + getSelectedAuthApp, + legacyConfigured, + markLoginDeclined, + otherApp, + setLoginAttemptApp, + setSelectedAuthApp, + tryBeginLoginFallback, +} from "../authApp"; + +const LEGACY_ID = "1a400946-62c1-4ab4-aa37-f730ac89704d"; + +/** Minimal browser shims — vitest runs in node (no jsdom in this suite). */ +function makeStorage(): Storage { + const map = new Map(); + return { + get length() { + return map.size; + }, + clear: () => map.clear(), + getItem: (k: string) => (map.has(k) ? map.get(k)! : null), + key: (i: number) => Array.from(map.keys())[i] ?? null, + removeItem: (k: string) => void map.delete(k), + setItem: (k: string, v: string) => void map.set(k, v), + }; +} + +function stubWindow(search = ""): void { + vi.stubGlobal("window", { + localStorage: makeStorage(), + sessionStorage: makeStorage(), + location: { search, reload: vi.fn() }, + }); +} + +beforeEach(() => { + vi.unstubAllEnvs(); + vi.unstubAllGlobals(); +}); + +afterEach(() => { + vi.unstubAllEnvs(); + vi.unstubAllGlobals(); +}); + +describe("getSelectedAuthApp", () => { + it("is primary during SSR (no window)", () => { + vi.stubEnv("NEXT_PUBLIC_ENTRA_LEGACY_CLIENT_ID", LEGACY_ID); + vi.stubEnv("NEXT_PUBLIC_ENTRA_DEFAULT_APP", "legacy"); + expect(getSelectedAuthApp()).toBe("primary"); + }); + + it("is primary whenever the legacy app is not configured — pre-migration deploys unchanged", () => { + stubWindow(); + vi.stubEnv("NEXT_PUBLIC_ENTRA_DEFAULT_APP", "legacy"); + expect(legacyConfigured()).toBe(false); + expect(getSelectedAuthApp()).toBe("primary"); + }); + + it("falls back to the env default when no flag is stored (legacy during the window)", () => { + stubWindow(); + vi.stubEnv("NEXT_PUBLIC_ENTRA_LEGACY_CLIENT_ID", LEGACY_ID); + vi.stubEnv("NEXT_PUBLIC_ENTRA_DEFAULT_APP", "legacy"); + expect(getSelectedAuthApp()).toBe("legacy"); + }); + + it("defaults to primary when the default var is unset or garbage", () => { + stubWindow(); + vi.stubEnv("NEXT_PUBLIC_ENTRA_LEGACY_CLIENT_ID", LEGACY_ID); + expect(getSelectedAuthApp()).toBe("primary"); + vi.stubEnv("NEXT_PUBLIC_ENTRA_DEFAULT_APP", "bogus"); + expect(getSelectedAuthApp()).toBe("primary"); + }); + + it("prefers the stored flag over the default (the learn mechanism)", () => { + stubWindow(); + vi.stubEnv("NEXT_PUBLIC_ENTRA_LEGACY_CLIENT_ID", LEGACY_ID); + vi.stubEnv("NEXT_PUBLIC_ENTRA_DEFAULT_APP", "legacy"); + setSelectedAuthApp("primary"); + expect(getSelectedAuthApp()).toBe("primary"); + }); + + it("persists the ?authapp URL override (support lever + signup funnel handover)", () => { + stubWindow("?authapp=primary&foo=1"); + vi.stubEnv("NEXT_PUBLIC_ENTRA_LEGACY_CLIENT_ID", LEGACY_ID); + vi.stubEnv("NEXT_PUBLIC_ENTRA_DEFAULT_APP", "legacy"); + expect(getSelectedAuthApp()).toBe("primary"); + expect(window.localStorage.getItem("am_auth_app")).toBe("primary"); + }); + + it("ignores a garbage ?authapp value", () => { + stubWindow("?authapp=evil"); + vi.stubEnv("NEXT_PUBLIC_ENTRA_LEGACY_CLIENT_ID", LEGACY_ID); + vi.stubEnv("NEXT_PUBLIC_ENTRA_DEFAULT_APP", "legacy"); + expect(getSelectedAuthApp()).toBe("legacy"); + }); +}); + +describe("getBootAuthApp", () => { + it("pins the boot app to the in-flight redirect's app, not the stored flag", () => { + stubWindow(); + vi.stubEnv("NEXT_PUBLIC_ENTRA_LEGACY_CLIENT_ID", LEGACY_ID); + setSelectedAuthApp("legacy"); + setLoginAttemptApp("primary"); + expect(getBootAuthApp()).toBe("primary"); + }); + + it("uses the selection when no redirect is in flight", () => { + stubWindow(); + vi.stubEnv("NEXT_PUBLIC_ENTRA_LEGACY_CLIENT_ID", LEGACY_ID); + setSelectedAuthApp("legacy"); + expect(getBootAuthApp()).toBe("legacy"); + }); +}); + +describe("safety-net primitives", () => { + it("tryBeginLoginFallback is one-shot per tab", () => { + stubWindow(); + expect(tryBeginLoginFallback()).toBe(true); + expect(tryBeginLoginFallback()).toBe(false); + }); + + it("consumeLoginDeclined reads and clears the marker", () => { + stubWindow(); + expect(consumeLoginDeclined()).toBe(false); + markLoginDeclined(); + expect(consumeLoginDeclined()).toBe(true); + expect(consumeLoginDeclined()).toBe(false); + }); + + it("classifies Entra redirect errors driving the fallback decision", () => { + // 90094/65001 = the app has no consent in the tenant → auto-retry the other app. + expect(classifyEntraAuthError("AADSTS90094: The grant requires admin permission.")) + .toBe("admin-approval-required"); + expect(classifyEntraAuthError("AADSTS65001: The user or administrator has not consented")) + .toBe("admin-approval-required"); + // 65004 = the user actively declined → NEVER auto-fallback (a new tenant would + // mis-consent the legacy app); failed screen instead. + expect(classifyEntraAuthError("AADSTS65004: User declined to consent.")) + .toBe("declined"); + expect(classifyEntraAuthError("AADSTS50011: redirect mismatch")).toBe("other"); + expect(classifyEntraAuthError("")).toBe("other"); + }); + + it("otherApp flips the selection", () => { + expect(otherApp("legacy")).toBe("primary"); + expect(otherApp("primary")).toBe("legacy"); + }); +}); diff --git a/src/Web/autopilot-monitor-web/lib/authApp.ts b/src/Web/autopilot-monitor-web/lib/authApp.ts new file mode 100644 index 00000000..f8c090ab --- /dev/null +++ b/src/Web/autopilot-monitor-web/lib/authApp.ts @@ -0,0 +1,186 @@ +/** + * App-registration selection for the dual app-reg parallel window (legacy gktatooine app ∥ + * new C4A8 app). Model: "default legacy, funnel primary, learn in the background" — + * + * - A browser that has signed in before carries `am_auth_app` in localStorage and always + * boots MSAL with that app. auth/me returns the tenant's `homedApp` after every login and + * AuthContext re-writes the flag, so an operator flipping a tenant to the new app makes + * every user's NEXT login use it automatically (admin consent exists by then → seamless). + * - No flag → NEXT_PUBLIC_ENTRA_DEFAULT_APP decides (start: "legacy" = today's behaviour for + * every existing customer; flipped to "primary" once the re-consent campaign is done). + * - The signup funnel (get-started CTA) hands over to portal with ?authapp=primary so brand-new + * tenants consent the NEW app as part of the expected signup flow. + * - ?authapp=legacy|primary doubles as a silent support lever (persists the flag). + * + * Everything here is deliberately msal-free (no import cycles) and SSR-safe. + */ + +export type AuthApp = "primary" | "legacy"; + +const SELECTED_KEY = "am_auth_app"; // localStorage: app of the last successful login +const ATTEMPT_KEY = "am_login_attempt"; // sessionStorage: app that started the in-flight redirect +const FALLBACK_KEY = "am_login_fallback"; // sessionStorage: the one-shot 90094 auto-fallback ran +const DECLINED_KEY = "am_login_declined"; // sessionStorage: user actively declined consent (65004) + +export function legacyClientId(): string | undefined { + return process.env.NEXT_PUBLIC_ENTRA_LEGACY_CLIENT_ID || undefined; +} + +/** The parallel window is active only when the legacy client id is configured. */ +export function legacyConfigured(): boolean { + return !!legacyClientId(); +} + +export function otherApp(app: AuthApp): AuthApp { + return app === "legacy" ? "primary" : "legacy"; +} + +function defaultApp(): AuthApp { + return process.env.NEXT_PUBLIC_ENTRA_DEFAULT_APP === "legacy" ? "legacy" : "primary"; +} + +function readUrlOverride(): AuthApp | null { + try { + const param = new URLSearchParams(window.location.search).get("authapp"); + return param === "legacy" || param === "primary" ? param : null; + } catch { + return null; + } +} + +/** + * The app a fresh MSAL boot should use when NO redirect is in flight: + * URL override (persisted) → stored flag → env default. Always "primary" during SSR/build + * and whenever the legacy app isn't configured (pre-migration deploys stay byte-identical). + */ +export function getSelectedAuthApp(): AuthApp { + if (typeof window === "undefined" || !legacyConfigured()) return "primary"; + const override = readUrlOverride(); + if (override) { + setSelectedAuthApp(override); + return override; + } + try { + const stored = window.localStorage.getItem(SELECTED_KEY); + if (stored === "legacy" || stored === "primary") return stored; + } catch { + // Storage unavailable (privacy mode) — fall through to the default. + } + return defaultApp(); +} + +export function setSelectedAuthApp(app: AuthApp): void { + try { + window.localStorage.setItem(SELECTED_KEY, app); + } catch { + // Best-effort: without storage the browser just re-derives the default next time. + } +} + +/** + * The app THIS page load must construct its PublicClientApplication for. A redirect return + * must be processed by the app that started it (`am_login_attempt`), regardless of what the + * flag says — otherwise handleRedirectPromise can't redeem the auth code. + */ +export function getBootAuthApp(): AuthApp { + if (typeof window === "undefined" || !legacyConfigured()) return "primary"; + return getLoginAttemptApp() ?? getSelectedAuthApp(); +} + +export function getLoginAttemptApp(): AuthApp | null { + try { + const value = window.sessionStorage.getItem(ATTEMPT_KEY); + return value === "legacy" || value === "primary" ? value : null; + } catch { + return null; + } +} + +export function setLoginAttemptApp(app: AuthApp): void { + try { + window.sessionStorage.setItem(ATTEMPT_KEY, app); + } catch { + // Without the marker a cross-app fallback redirect can't be redeemed, but the regular + // single-app flow still works (boot app equals the flag/default). + } +} + +export function clearLoginAttemptApp(): void { + try { + window.sessionStorage.removeItem(ATTEMPT_KEY); + } catch { + /* ignore */ + } +} + +/** + * One-shot guard for the AADSTS90094 auto-fallback ("this app needs admin approval in your + * tenant" → silently retry with the other app). Returns true exactly once per browser tab + * until a successful login clears it — prevents a ping-pong loop when NEITHER app is consented. + */ +export function tryBeginLoginFallback(): boolean { + try { + if (window.sessionStorage.getItem(FALLBACK_KEY)) return false; + window.sessionStorage.setItem(FALLBACK_KEY, "1"); + return true; + } catch { + return false; + } +} + +export function clearLoginFallback(): void { + try { + window.sessionStorage.removeItem(FALLBACK_KEY); + } catch { + /* ignore */ + } +} + +/** User actively declined the consent prompt (AADSTS65004) — never auto-fallback on that. */ +export function markLoginDeclined(): void { + try { + window.sessionStorage.setItem(DECLINED_KEY, "1"); + } catch { + /* ignore */ + } +} + +/** Reads AND clears the declined marker (one-shot, consumed by ProtectedRoute). */ +export function consumeLoginDeclined(): boolean { + try { + if (!window.sessionStorage.getItem(DECLINED_KEY)) return false; + window.sessionStorage.removeItem(DECLINED_KEY); + return true; + } catch { + return false; + } +} + +/** Classification of Entra redirect errors driving the safety net. */ +export function classifyEntraAuthError(errorText: string): "admin-approval-required" | "declined" | "other" { + if (/AADSTS90094|AADSTS65001/i.test(errorText)) return "admin-approval-required"; + if (/AADSTS65004/i.test(errorText)) return "declined"; + return "other"; +} + +/** + * Switch this browser to the given app and reboot the bundle so the module-level MSAL + * instance is reconstructed for it. Purges MSAL state to avoid interaction_in_progress + * leftovers from an interrupted flow on the other app. + */ +export function switchAuthApp(app: AuthApp): void { + setSelectedAuthApp(app); + clearLoginAttemptApp(); + clearLoginFallback(); + try { + const doomed: string[] = []; + for (let i = 0; i < window.sessionStorage.length; i++) { + const key = window.sessionStorage.key(i); + if (key && (key.startsWith("msal.") || key.includes(".login.request"))) doomed.push(key); + } + doomed.forEach((key) => window.sessionStorage.removeItem(key)); + } catch { + /* ignore */ + } + window.location.reload(); +} diff --git a/src/Web/autopilot-monitor-web/lib/msalConfig.ts b/src/Web/autopilot-monitor-web/lib/msalConfig.ts index d065e5ab..c04a3636 100644 --- a/src/Web/autopilot-monitor-web/lib/msalConfig.ts +++ b/src/Web/autopilot-monitor-web/lib/msalConfig.ts @@ -1,11 +1,16 @@ import { Configuration, LogLevel, RedirectRequest } from "@azure/msal-browser"; import { ENTRA_LOGIN_URL } from "@/utils/config"; +import { AuthApp, getBootAuthApp, legacyClientId } from "./authApp"; /** * MSAL Configuration for Multi-Tenant Azure AD Authentication * * Environment Variables: - * - NEXT_PUBLIC_ENTRA_CLIENT_ID: Application (client) ID from App Registration + * - NEXT_PUBLIC_ENTRA_CLIENT_ID: Application (client) ID of the PRIMARY App Registration + * - NEXT_PUBLIC_ENTRA_LEGACY_CLIENT_ID: pre-migration App Registration (dual app-reg window; + * unset = single-app behaviour, everything below resolves to the primary app) + * - NEXT_PUBLIC_ENTRA_DEFAULT_APP: which app a browser WITHOUT a stored selection boots + * ("legacy" during the parallel window, later "primary") — consumed in lib/authApp.ts * - NEXT_PUBLIC_ENTRA_REDIRECT_URI: Fallback redirect URI for SSR/dev (browser uses window.location.origin) * - NEXT_PUBLIC_ENTRA_POST_LOGOUT_REDIRECT_URI: Fallback post-logout URI for SSR/dev */ @@ -20,10 +25,25 @@ const resolveRedirectUri = (fallback: string | undefined): string => { return fallback || "http://localhost:3000"; }; -// MSAL Configuration -export const msalConfig: Configuration = { +/** Client id for the given app selection (legacy falls back to primary when unconfigured). */ +export const clientIdForApp = (app: AuthApp): string => { + if (app === "legacy") { + return legacyClientId() || process.env.NEXT_PUBLIC_ENTRA_CLIENT_ID || "YOUR_CLIENT_ID_HERE"; + } + return process.env.NEXT_PUBLIC_ENTRA_CLIENT_ID || "YOUR_CLIENT_ID_HERE"; +}; + +/** + * The app registration THIS page load runs under. Decided once at bundle boot (redirect + * returns are pinned to the app that started them via the am_login_attempt marker); app + * switches always go through a full reload (see lib/authApp.ts switchAuthApp). + */ +export const activeAuthApp: AuthApp = getBootAuthApp(); + +/** MSAL configuration factory — one PublicClientApplication per app registration. */ +export const buildMsalConfig = (clientId: string): Configuration => ({ auth: { - clientId: process.env.NEXT_PUBLIC_ENTRA_CLIENT_ID || "YOUR_CLIENT_ID_HERE", + clientId, authority: `${ENTRA_LOGIN_URL}/organizations`, // Multi-tenant redirectUri: resolveRedirectUri(process.env.NEXT_PUBLIC_ENTRA_REDIRECT_URI), postLogoutRedirectUri: resolveRedirectUri(process.env.NEXT_PUBLIC_ENTRA_POST_LOGOUT_REDIRECT_URI), @@ -63,7 +83,14 @@ export const msalConfig: Configuration = { }, allowPlatformBroker: false, // Disables WAM Broker (renamed from allowNativeBroker in msal-browser v4) }, -}; +}); + +/** + * The configuration the module-level PublicClientApplication boots with — computed once per + * page life from the active app selection, so every existing consumer keeps importing + * `msalConfig`/`apiRequest` unchanged. + */ +export const msalConfig: Configuration = buildMsalConfig(clientIdForApp(activeAuthApp)); /** * Scopes you add here will be prompted for user consent during sign-in. @@ -83,16 +110,24 @@ export const loginRequest: RedirectRequest = { * IMPORTANT: Backend API must be exposed in Azure AD App Registration with this scope * Format: api:///access_as_user * + * The scope follows the ACTIVE app: a legacy sign-in requests the legacy app's audience, + * a primary sign-in the primary app's — the backend trusts both during the parallel window. + * * NEXT_PUBLIC_ENTRA_API_CLIENT_ID lets local dev sign in with the dev app * registration while requesting a token whose audience is the prod API app - * (the deployed backend only accepts its own audience). Unset in production, + * (the deployed backend only accepts its own audiences). Unset in production, * where one app registration serves as both SPA client and API. */ -const apiAppId = - process.env.NEXT_PUBLIC_ENTRA_API_CLIENT_ID || process.env.NEXT_PUBLIC_ENTRA_CLIENT_ID; +export const buildApiScopes = (app: AuthApp): string[] => { + const apiAppId = + app === "legacy" + ? legacyClientId() || process.env.NEXT_PUBLIC_ENTRA_CLIENT_ID + : process.env.NEXT_PUBLIC_ENTRA_API_CLIENT_ID || process.env.NEXT_PUBLIC_ENTRA_CLIENT_ID; + return [`api://${apiAppId}/access_as_user`]; +}; export const apiRequest = { - scopes: [`api://${apiAppId}/access_as_user`], + scopes: buildApiScopes(activeAuthApp), }; /** diff --git a/src/Web/autopilot-monitor-web/public/.well-known/microsoft-identity-association.json b/src/Web/autopilot-monitor-web/public/.well-known/microsoft-identity-association.json index 2d801031..c7c4c178 100644 --- a/src/Web/autopilot-monitor-web/public/.well-known/microsoft-identity-association.json +++ b/src/Web/autopilot-monitor-web/public/.well-known/microsoft-identity-association.json @@ -2,6 +2,9 @@ "associatedApplications": [ { "applicationId": "1a400946-62c1-4ab4-aa37-f730ac89704d" + }, + { + "applicationId": "886ab5e2-6144-442c-80cc-9b28e0667731" } ] } \ No newline at end of file