diff --git a/src/Backend/AutopilotMonitor.Functions.Tests/AuthFunctionTests.cs b/src/Backend/AutopilotMonitor.Functions.Tests/AuthFunctionTests.cs index 083de15e..df518c43 100644 --- a/src/Backend/AutopilotMonitor.Functions.Tests/AuthFunctionTests.cs +++ b/src/Backend/AutopilotMonitor.Functions.Tests/AuthFunctionTests.cs @@ -305,6 +305,27 @@ public class AuthFunctionTests Assert.Equal("TenantSuspended", (string)body.error); } + [Fact] + public void AutoAdmin_DelegatedFirstUser_DoesNotAutoAdminHomeTenant() + { + // A delegated ("MSP") admin logging into a home tenant with NO existing admins must NOT be silently + // made its Tenant Admin — that would convert a read-only delegated assignment (for OTHER tenants) + // into write authority over the home tenant. Regression for the preview-gate bypass opening the + // auto-admin path to delegated callers. + var result = AuthFunction.BuildAuthResult( + DefaultConfig(), isGlobalAdmin: false, isGlobalReader: false, isPreviewApproved: false, + memberRole: null, mcpCheck: McpDenied(), + hasTenantAdmins: false, + TenantId, Upn, DisplayName, ObjectId, + delegatedTenantIds: new[] { "22222222-2222-2222-2222-222222222222" }); + + Assert.True(result.IsSuccess); + Assert.False(result.NeedsAutoAdmin); + var body = ToDynamic(result.Body); + Assert.False((bool)body.isTenantAdmin); + Assert.Null((string?)body.role); + } + // ------------------------------------------------------------------------- // Auto-Admin Logic // ------------------------------------------------------------------------- diff --git a/src/Backend/AutopilotMonitor.Functions/Functions/Infrastructure/AuthFunction.cs b/src/Backend/AutopilotMonitor.Functions/Functions/Infrastructure/AuthFunction.cs index ca8d7c8b..4287e9eb 100644 --- a/src/Backend/AutopilotMonitor.Functions/Functions/Infrastructure/AuthFunction.cs +++ b/src/Backend/AutopilotMonitor.Functions/Functions/Infrastructure/AuthFunction.cs @@ -89,7 +89,10 @@ public class AuthFunction // --- Parallel data fetch: all independent queries run concurrently --- // Fail-fast: if any fetch throws, AggregateException propagates → Azure Functions returns 500. // This is intentional — all 6 queries are required for the auth decision. - var tenantConfigTask = _tenantConfigService.GetConfigurationAsync(tenantId); + // Non-creating read: do NOT auto-persist a default config row just because someone authenticated. + // A delegated-only external MSP login must not phantom-onboard its home tenant (see the side-effect + // gate below). A genuine first-user still gets their config created by HandleNewTenantDomainAsync. + var tenantConfigTask = _tenantConfigService.TryGetConfigurationAsync(tenantId); var globalRoleTask = _globalAdminService.GetGlobalRoleAsync(upn); var delegatedScopeTask = _delegatedAdminService.GetScopeAsync(upn); var isApprovedTask = _previewWhitelistService.IsApprovedAsync(tenantId); @@ -100,7 +103,7 @@ public class AuthFunction await Task.WhenAll(tenantConfigTask, globalRoleTask, delegatedScopeTask, isApprovedTask, membershipTask, mcpCheckTask, existingAdminsTask); - var tenantConfig = tenantConfigTask.Result; + var (tenantConfig, _) = tenantConfigTask.Result; var globalRole = globalRoleTask.Result; var isGlobalAdmin = globalRole == Constants.GlobalRoles.GlobalAdmin; var isGlobalReader = globalRole == Constants.GlobalRoles.GlobalReader; @@ -119,8 +122,20 @@ public class AuthFunction tableState, tableRole, principal.GetAppRoles(), tenantConfig.EntraAppRolesEnabled); // --- Side-effects that don't affect the auth decision --- - await HandleNewTenantDomainAsync(tenantConfig, tenantId, upn); - await HandleAutoReEnableAsync(tenantConfig, tenantId); + // Run first-login onboarding side-effects (domain/OnboardedBy write + "new tenant signup" + // notification, auto-re-enable, and the implicit config-row persistence) ONLY for a genuine + // home-tenant participant. A delegated-only external MSP caller — who bypassed the preview gate to + // manage OTHER tenants and has no stake in this home tenant (no membership, no platform role) — is + // excluded, so a read-only MSP login does not phantom-onboard its home tenant (false signup, default + // config row). A delegated user who legitimately participates in their own tenant (member/global) + // still onboards normally; the handlers self-gate when the config already exists. + var isDelegated = delegatedTenantIds.Count > 0; + var isHomeTenantParticipant = !isDelegated || isGlobalAdmin || isGlobalReader || memberRole != null; + if (isHomeTenantParticipant) + { + await HandleNewTenantDomainAsync(tenantConfig, tenantId, upn); + await HandleAutoReEnableAsync(tenantConfig, tenantId); + } // --- Pure decision logic (tested by AuthFunctionTests) --- var decision = BuildAuthResult( @@ -396,8 +411,15 @@ public class AuthFunction // it removes nothing, and a GlobalReader who should also administer their tenant can be added // to TenantAdmins explicitly. (A pure GlobalReader stays read-only everywhere via the write // evaluators denying a roleless caller.) + // Auto-admin is NOT an existing right — it SILENTLY grants write on first login. Decline it for any + // identity that reached this point on a cross-tenant ticket rather than as a genuine home-tenant + // first-user: a read-only GlobalReader, AND a delegated ("MSP") admin. A delegated caller logged in + // to manage OTHER tenants and bypassed the preview gate; auto-admining their own (possibly + // non-customer) home tenant would convert a read-only delegated assignment into write authority over + // it. A delegated user who legitimately administers their own tenant has a TenantAdmins row (memberRole + // != null), so this only suppresses the silent grant, never an existing right. bool isTenantAdmin = memberRole?.Role == Constants.TenantRoles.Admin; - bool needsAutoAdmin = memberRole == null && !hasTenantAdmins && !isGlobalReader; + bool needsAutoAdmin = memberRole == null && !hasTenantAdmins && !isGlobalReader && !isDelegated; if (needsAutoAdmin) { isTenantAdmin = true;