Auth: an external identity never claims a tenant (D-353)

A caller whose token shows it is not a member of the token's tenant (acct = 1, a #EXT# user name,
or an idp naming another tenant, a personal Microsoft account or an external sign-in provider) no
longer seeds DomainName/OnboardedBy or queues the activation, is never the first-user Tenant Admin,
and cannot set the activation-page address in the roleless signup window. Positive evidence only:
a token without these signals stays a member's, and tid/idp are read under their inbound-mapped
names as well. Activation promotes OnboardedBy only; the UpdatedBy fallback is gone and a stored
#EXT# UPN is refused. Explicit grants (TenantAdmins row, app role) still apply.

The once-per-interval log gate of the client-app binding becomes Helpers/LogThrottle and also
throttles the new warning, which the polling activation page would otherwise repeat.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Oliver Kieselbach
2026-10-13 01:19:56 +02:00
parent 18310b05aa
commit dd4975b73e
22 changed files with 609 additions and 185 deletions
@@ -31,6 +31,8 @@ public class AuthFunctionSideEffectTests
private readonly Mock<TenantConfigurationService> _tenantConfigMock;
private readonly Mock<TenantAdminsService> _tenantAdminsMock;
private readonly Mock<PreviewWhitelistService> _previewMock;
private readonly Mock<McpUserService> _mcpUserMock;
private readonly Mock<TelegramNotificationService> _telegramMock;
private readonly Mock<OpsEventService> _opsEventsMock;
private readonly Mock<GlobalNotificationService> _globalNotificationMock;
@@ -71,7 +73,7 @@ public class AuthFunctionSideEffectTests
adminRepo, cache, Mock.Of<ILogger<TenantAdminsService>>())
{ CallBase = false };
var previewMock = new Mock<PreviewWhitelistService>(
_previewMock = new Mock<PreviewWhitelistService>(
configRepo, cache, Mock.Of<ILogger<PreviewWhitelistService>>(), _tenantConfigMock.Object)
{ CallBase = false };
@@ -87,7 +89,7 @@ public class AuthFunctionSideEffectTests
configRepo, Mock.Of<ILogger<AdminConfigurationService>>(), cache)
{ CallBase = false };
var mcpUserMock = new Mock<McpUserService>(
_mcpUserMock = new Mock<McpUserService>(
adminRepo, new StubAdminIdentityBindingService(bound: true), cache, Mock.Of<ILogger<McpUserService>>(),
globalAdminMock.Object, delegatedAdminMock.Object, adminConfigService.Object,
new TenantMemberRoleResolver(_tenantAdminsMock.Object, _tenantConfigMock.Object), _tenantConfigMock.Object)
@@ -107,10 +109,10 @@ public class AuthFunctionSideEffectTests
_tenantConfigMock.Object,
_tenantAdminsMock.Object,
_metricsRepoMock.Object,
previewMock.Object,
_previewMock.Object,
_telegramMock.Object,
_globalNotificationMock.Object,
mcpUserMock.Object,
_mcpUserMock.Object,
_autoApproveEnqueuerMock.Object,
new AutopilotMonitor.Functions.Security.EntraAppRegistry(
new Microsoft.Extensions.Configuration.ConfigurationBuilder().Build(),
@@ -169,6 +171,13 @@ public class AuthFunctionSideEffectTests
}
private static (HttpRequestData Req, FunctionContext Ctx) BuildAuthenticatedRequest(string callerUpn, string? tenantId = null)
{
var claims = new List<Claim> { new("preferred_username", callerUpn) };
if (tenantId != null) claims.Add(new Claim("tid", tenantId));
return BuildRequest(new ClaimsPrincipal(new ClaimsIdentity(claims, "test")));
}
private static (HttpRequestData Req, FunctionContext Ctx) BuildRequest(ClaimsPrincipal principal)
{
var services = new ServiceCollection();
services.AddOptions();
@@ -177,10 +186,6 @@ public class AuthFunctionSideEffectTests
new System.Text.Json.JsonSerializerOptions(System.Text.Json.JsonSerializerDefaults.Web)));
var provider = services.BuildServiceProvider();
var claims = new List<Claim> { new("preferred_username", callerUpn) };
if (tenantId != null) claims.Add(new Claim("tid", tenantId));
var principal = new ClaimsPrincipal(new ClaimsIdentity(claims, "test"));
var context = new Mock<FunctionContext>();
context.SetupGet(c => c.Items).Returns(
new Dictionary<object, object> { ["ClaimsPrincipal"] = principal });
@@ -846,4 +851,86 @@ public class AuthFunctionSideEffectTests
await Assert.ThrowsAsync<InvalidOperationException>(
() => _sut.HandlePostDecisionSideEffectsAsync(decision, TenantId, Upn, DisplayName, ObjectId));
}
// -------------------------------------------------------------------------
// GetCurrentUser end to end: an external identity never claims the tenant
// -------------------------------------------------------------------------
/// <summary>
/// A validated token as the middleware hands it on: tid, oid and idp only under their mapped names. The
/// member's idp names its own tenant, the guest's another one — the same principal shape otherwise.
/// </summary>
private static ClaimsPrincipal SignedIn(bool external) => new(new ClaimsIdentity(new[]
{
new Claim("http://schemas.microsoft.com/identity/claims/tenantid", TenantId),
new Claim("http://schemas.microsoft.com/identity/claims/objectidentifier", ObjectId),
new Claim(ClaimTypes.Upn, Upn),
new Claim("http://schemas.microsoft.com/identity/claims/identityprovider", external
? "https://sts.windows.net/bbbbbbbb-1111-2222-3333-444444444444/"
: $"https://sts.windows.net/{TenantId}/"),
}, "test"));
private void ArrangeAuthMe(TenantConfiguration config, bool exists, bool approved)
{
_tenantConfigMock.Setup(x => x.TryGetConfigurationAsync(TenantId)).ReturnsAsync((config, exists));
_previewMock.Setup(x => x.IsApprovedAsync(TenantId)).ReturnsAsync(approved);
_tenantAdminsMock.Setup(x => x.GetTenantAdminsAsync(TenantId)).ReturnsAsync(new List<TenantAdminRow>());
_mcpUserMock
.Setup(x => x.IsAllowedAsync(It.IsAny<string?>(), It.IsAny<string?>(), It.IsAny<string?>(),
It.IsAny<IReadOnlyList<string>?>(), It.IsAny<bool>()))
.ReturnsAsync(McpAccessCheckResult.Denied("not allowed"));
_metricsRepoMock.Setup(x => x.GetUserWhatsNewSeenAsync(TenantId, Upn)).ReturnsAsync(new UserWhatsNewSeen());
}
private static async Task<string> ReadBodyAsync(HttpResponseData response)
{
response.Body.Position = 0;
return await new StreamReader(response.Body).ReadToEndAsync();
}
[Theory]
[InlineData(false)]
[InlineData(true)]
public async Task GetCurrentUser_FirstSignInOfANewTenant_OnlyAMemberOnboardsIt(bool external)
{
// The pointer read stays healthy on purpose: a throwing read would skip the seed for the guest too, and
// the test would pass for the wrong reason.
ArrangeAuthMe(DefaultConfig(), exists: false, approved: false);
var (req, ctx) = BuildRequest(SignedIn(external));
var response = await _sut.GetCurrentUser(req, ctx);
// Both wait at the activation gate; only the member's sign-in seeded the tenant and queued its activation.
Assert.Equal(HttpStatusCode.Forbidden, response.StatusCode);
Assert.Contains("PendingActivation", await ReadBodyAsync(response));
Assert.Equal(external ? 0 : 1, _store.Writes.Count);
_autoApproveEnqueuerMock.Verify(x => x.EnqueueAsync(
It.IsAny<AutopilotMonitor.Functions.Services.Activation.TenantAutoApproveEnvelope>(),
It.IsAny<TimeSpan?>(), It.IsAny<CancellationToken>()), external ? Times.Never() : Times.Once());
_opsEventsMock.Verify(x => x.RecordTenantSignupAsync(TenantId, Upn), external ? Times.Never() : Times.Once());
if (!external)
Assert.Equal(Upn, _store.Row!.OnboardedBy);
}
[Theory]
[InlineData(false)]
[InlineData(true)]
public async Task GetCurrentUser_ActivatedTenantWithoutAdmins_OnlyAMemberBecomesItsFirstAdmin(bool external)
{
var config = DefaultConfig();
config.DomainName = "contoso.com";
ArrangeAuthMe(config, exists: true, approved: true);
_tenantAdminsMock
.Setup(x => x.AddTenantAdminAsync(TenantId, Upn, "System"))
.ReturnsAsync(new TenantAdminRow());
var (req, ctx) = BuildRequest(SignedIn(external));
var response = await _sut.GetCurrentUser(req, ctx);
Assert.Equal(HttpStatusCode.OK, response.StatusCode);
_tenantAdminsMock.Verify(
x => x.AddTenantAdminAsync(It.IsAny<string>(), It.IsAny<string>(), It.IsAny<string>()),
external ? Times.Never() : Times.Once());
Assert.Contains(external ? "\"isTenantAdmin\":false" : "\"isTenantAdmin\":true", await ReadBodyAsync(response));
}
}
@@ -414,6 +414,43 @@ public class AuthFunctionTests
Assert.Null((string?)body.role);
}
[Fact]
public void AutoAdmin_ExternalIdentityFirstUser_IsNeverTheFirstAdmin()
{
// A guest signing in to an activated tenant that has no admins yet must not be made its Tenant Admin:
// it is not a member of the tenant at all.
var result = AuthFunction.BuildAuthResult(
DefaultConfig(), isGlobalAdmin: false, isGlobalReader: false, isPreviewApproved: true,
memberRole: null, mcpCheck: McpDenied(),
hasTenantAdmins: false,
TenantId, Upn, DisplayName, ObjectId,
isExternalIdentity: true);
Assert.True(result.IsSuccess);
Assert.False(result.NeedsAutoAdmin);
var body = ToDynamic(result.Body);
Assert.False((bool)body.isTenantAdmin);
Assert.Null((string?)body.role);
Assert.False((bool)body.canManageBootstrapTokens);
}
[Fact]
public void ExternalIdentity_KeepsARoleTheTenantGrantedExplicitly()
{
var result = AuthFunction.BuildAuthResult(
DefaultConfig(), isGlobalAdmin: false, isGlobalReader: false, isPreviewApproved: true,
memberRole: AdminRole(), mcpCheck: McpDenied(),
hasTenantAdmins: true,
TenantId, Upn, DisplayName, ObjectId,
isExternalIdentity: true);
Assert.True(result.IsSuccess);
Assert.False(result.NeedsAutoAdmin);
var body = ToDynamic(result.Body);
Assert.True((bool)body.isTenantAdmin);
Assert.Equal(Constants.TenantRoles.Admin, (string)body.role);
}
// -------------------------------------------------------------------------
// Auto-Admin Logic
// -------------------------------------------------------------------------
@@ -1,6 +1,7 @@
using System.IdentityModel.Tokens.Jwt;
using System.Security.Claims;
using System.Security.Cryptography;
using AutopilotMonitor.Functions.Extensions;
using AutopilotMonitor.Functions.Middleware;
using Microsoft.IdentityModel.Tokens;
using Xunit;
@@ -34,12 +35,13 @@ public class AuthenticationMiddlewareTokenValidationTests
string audience = ClientId,
string tid = Tid,
DateTime? notBefore = null,
DateTime? expires = null)
DateTime? expires = null,
IEnumerable<Claim>? extraClaims = null)
{
var token = new JwtSecurityToken(
issuer: issuer ?? V2Issuer,
audience: audience,
claims: new[] { new Claim("tid", tid) },
claims: new[] { new Claim("tid", tid) }.Concat(extraClaims ?? Array.Empty<Claim>()),
notBefore: notBefore ?? DateTime.UtcNow.AddMinutes(-5),
expires: expires ?? DateTime.UtcNow.AddMinutes(30),
signingCredentials: creds); // null creds → alg "none", unsigned
@@ -136,6 +138,39 @@ public class AuthenticationMiddlewareTokenValidationTests
Assert.ThrowsAny<SecurityTokenException>(() => Validate(forged));
}
// ── External identity: the guest signals survive inbound claim mapping ──
// A validated token carries tid and idp only under their mapped names, so a classifier reading the raw
// names would call every member with an idp external, or every guest a member.
[Theory]
[InlineData("https://sts.windows.net/bbbbbbbb-1111-2222-3333-444444444444/", true)] // a guest of another Entra tenant
[InlineData("live.com", true)] // a personal Microsoft account
[InlineData("https://sts.windows.net/" + Tid + "/", false)] // a member: own tenant, v1 form under the v2 issuer
public void IdpClaim_after_validation_decides_external_identity(string idp, bool external)
{
var principal = Validate(Mint(RsaCreds, extraClaims: new[] { new Claim("idp", idp) }));
Assert.Equal(external, principal.IsExternalIdentity());
}
[Theory]
[InlineData("1", true)]
[InlineData("0", false)]
public void AcctClaim_after_validation_is_read_as_the_number_Entra_sends(string acct, bool external)
{
var principal = Validate(Mint(RsaCreds, extraClaims: new[] { new Claim("acct", acct, ClaimValueTypes.Integer32) }));
Assert.Equal(external, principal.IsExternalIdentity());
}
[Fact]
public void MemberToken_without_guest_signals_is_not_external()
{
var principal = Validate(Mint(RsaCreds, extraClaims: new[] { new Claim("upn", "alice@contoso.example") }));
Assert.False(principal.IsExternalIdentity());
}
// ── IsValidTenantId: pre-signature tid GUID gate ─────────────────────
[Theory]
@@ -0,0 +1,108 @@
using System.Security.Claims;
using AutopilotMonitor.Functions.Extensions;
using Xunit;
namespace AutopilotMonitor.Functions.Tests;
/// <summary>
/// <see cref="ClaimsPrincipalExtensions.IsExternalIdentity"/>: a caller that is not a member of the token's tenant
/// never claims it (no first-login seed, no first-user Tenant Admin, no activation promotion). Positive evidence
/// only — every case expecting a member pins a false positive that would silently strand a genuine first sign-in.
/// A validated token carries tid and idp only under their inbound-mapped names, so both forms are covered.
/// </summary>
public class ExternalIdentityTests
{
private const string Tid = "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee";
private const string OtherTenant = "bbbbbbbb-1111-2222-3333-444444444444";
private const string MappedTenantId = "http://schemas.microsoft.com/identity/claims/tenantid";
private const string MappedIdp = "http://schemas.microsoft.com/identity/claims/identityprovider";
private static ClaimsPrincipal Principal(params Claim[] extra) => WithUpn("alice@contoso.example", extra);
private static ClaimsPrincipal WithUpn(string upn, params Claim[] extra)
{
var claims = new List<Claim> { new(MappedTenantId, Tid), new(ClaimTypes.Upn, upn) };
claims.AddRange(extra);
return new ClaimsPrincipal(new ClaimsIdentity(claims, "TestAuth"));
}
[Fact]
public void Member_WithoutAnySignal_IsNotExternal()
=> Assert.False(Principal().IsExternalIdentity());
// ── acct: Entra's guest flag ─────────────────────────────────────────────────
[Theory]
[InlineData("1", true)]
[InlineData("0", false)]
public void AcctClaim_GuestFlagDecides(string acct, bool external)
=> Assert.Equal(external, Principal(new Claim("acct", acct)).IsExternalIdentity());
// ── #EXT#: the resource-tenant UPN of an external account ───────────────────
[Theory]
[InlineData("upn")]
[InlineData(ClaimTypes.Upn)]
[InlineData("preferred_username")]
[InlineData("unique_name")]
[InlineData(ClaimTypes.Name)]
public void ExternalUpnMarker_InAnyUserNameClaim_IsExternal(string claimType)
{
var principal = new ClaimsPrincipal(new ClaimsIdentity(new[]
{
new Claim(MappedTenantId, Tid),
new Claim(claimType, "alice_contoso.example#ext#@fabrikam.example"),
}, "TestAuth"));
Assert.True(principal.IsExternalIdentity());
}
[Fact]
public void ExternalUpnMarker_InTheDisplayName_IsNoEvidence()
=> Assert.False(Principal(new Claim("name", "Alice #EXT# Example")).IsExternalIdentity());
[Fact]
public void UnderscoreExtSuffix_IsNoEvidence_MemberNamingSchemesUseIt()
=> Assert.False(WithUpn("m.muster_ext_@contoso.example").IsExternalIdentity());
// ── idp: where the account authenticated ────────────────────────────────────
[Theory]
[InlineData("https://sts.windows.net/" + OtherTenant + "/", true)] // a guest of another Entra tenant
[InlineData("https://login.microsoftonline.com/" + OtherTenant + "/v2.0", true)]
[InlineData("https://sts.windows.net/9188040d-6c67-4c5b-b112-36a304b66dad/", true)] // the personal-account tenant
[InlineData("live.com", true)] // a personal Microsoft account
[InlineData("LIVE.COM", true)]
[InlineData("mail", true)] // email one-time passcode
[InlineData("google.com", true)]
[InlineData("facebook.com", true)]
[InlineData("https://sts.windows.net/" + Tid + "/", false)] // a member: its own tenant (v1 form, also under a v2 issuer)
[InlineData("https://login.microsoftonline.com/" + Tid + "/v2.0", false)]
[InlineData("http://adfs.contoso.example/adfs/services/trust", false)] // an unknown host: no evidence either way
[InlineData("https://auth.idp.example/" + OtherTenant + "/as", false)] // a GUID segment on a foreign host: no evidence
[InlineData("", false)]
public void IdpClaim_NamesWhereTheAccountAuthenticated(string idp, bool external)
=> Assert.Equal(external, Principal(new Claim("idp", idp)).IsExternalIdentity());
[Theory]
[InlineData("idp")]
[InlineData(MappedIdp)]
public void IdpNamingAnotherTenant_IsExternal_InEitherClaimForm(string idpClaimType)
=> Assert.True(Principal(new Claim(idpClaimType, $"https://sts.windows.net/{OtherTenant}/")).IsExternalIdentity());
[Theory]
[InlineData("tid")]
[InlineData(MappedTenantId)]
public void IdpNamingTheOwnTenant_IsAMember_InEitherTenantClaimForm(string tenantClaimType)
{
// A validated token has no raw tid: comparing against it would call every member with an idp external.
var principal = new ClaimsPrincipal(new ClaimsIdentity(new[]
{
new Claim(tenantClaimType, Tid),
new Claim(ClaimTypes.Upn, "alice@contoso.example"),
new Claim(MappedIdp, $"https://sts.windows.net/{Tid}/"),
}, "TestAuth"));
Assert.False(principal.IsExternalIdentity());
}
}
@@ -0,0 +1,53 @@
using AutopilotMonitor.Functions.Helpers;
using Xunit;
namespace AutopilotMonitor.Functions.Tests;
/// <summary>
/// <see cref="LogThrottle"/>: one line per key and interval, so a polled path (auth/me under the activation page,
/// a delegated token on every request) shows up in App Insights without flooding it; bounded memory.
/// </summary>
public class LogThrottleTests
{
private static readonly DateTime T0 = new(2026, 10, 13, 8, 0, 0, DateTimeKind.Utc);
[Fact]
public void FirstLine_Passes_ARepeatWithinTheInterval_DoesNot()
{
var throttle = new LogThrottle(TimeSpan.FromHours(1));
Assert.True(throttle.ShouldLog("k", T0));
Assert.False(throttle.ShouldLog("k", T0.AddMinutes(59)));
}
[Fact]
public void ARepeat_PassesAgain_OnceTheIntervalHasElapsed()
{
var throttle = new LogThrottle(TimeSpan.FromHours(1));
Assert.True(throttle.ShouldLog("k", T0));
Assert.True(throttle.ShouldLog("k", T0.AddHours(1)));
Assert.False(throttle.ShouldLog("k", T0.AddHours(1).AddMinutes(1)));
}
[Fact]
public void Keys_AreThrottledIndependently()
{
var throttle = new LogThrottle(TimeSpan.FromHours(1));
Assert.True(throttle.ShouldLog("a", T0));
Assert.True(throttle.ShouldLog("b", T0));
Assert.False(throttle.ShouldLog("a", T0));
}
[Fact]
public void AtTheKeyCap_TheMemoryIsDropped_SoALineRepeatsEarlyInsteadOfGrowingWithoutBound()
{
var throttle = new LogThrottle(TimeSpan.FromHours(1), maxKeys: 2);
Assert.True(throttle.ShouldLog("a", T0));
Assert.True(throttle.ShouldLog("b", T0));
Assert.True(throttle.ShouldLog("c", T0));
Assert.True(throttle.ShouldLog("a", T0));
}
}
@@ -19,6 +19,7 @@ namespace AutopilotMonitor.Functions.Tests;
/// farewell mails. The route must admit roleless callers (first-touch signup) but only while the
/// tenant has no member yet — afterwards a member role is required, so an ordinary employee with a
/// valid JWT and no product role can no longer redirect or suppress the tenant's correspondence.
/// The window is never an external identity's: it does not activate the tenant.
/// </summary>
public class PreviewNotificationEmailWriteGateTests
{
@@ -43,7 +44,7 @@ public class PreviewNotificationEmailWriteGateTests
var admins = AdminsWith(enabledMember: true);
var ctx = new RequestContext { UserRole = role };
Assert.True(await PreviewWhitelistFunction.MayWriteNotificationEmailAsync(ctx, TenantId, admins.Object));
Assert.True(await PreviewWhitelistFunction.MayWriteNotificationEmailAsync(ctx, externalIdentity: false, TenantId, admins.Object));
admins.Verify(a => a.GetTenantAdminsAsync(It.IsAny<string>()), Times.Never);
}
@@ -53,7 +54,7 @@ public class PreviewNotificationEmailWriteGateTests
var admins = AdminsWith(enabledMember: true);
var ctx = new RequestContext { IsGlobalAdmin = true };
Assert.True(await PreviewWhitelistFunction.MayWriteNotificationEmailAsync(ctx, TenantId, admins.Object));
Assert.True(await PreviewWhitelistFunction.MayWriteNotificationEmailAsync(ctx, externalIdentity: false, TenantId, admins.Object));
}
[Fact]
@@ -62,7 +63,7 @@ public class PreviewNotificationEmailWriteGateTests
var admins = AdminsWith(enabledMember: false);
var ctx = new RequestContext { UserRole = "Authenticated" };
Assert.True(await PreviewWhitelistFunction.MayWriteNotificationEmailAsync(ctx, TenantId, admins.Object));
Assert.True(await PreviewWhitelistFunction.MayWriteNotificationEmailAsync(ctx, externalIdentity: false, TenantId, admins.Object));
}
[Fact]
@@ -71,7 +72,7 @@ public class PreviewNotificationEmailWriteGateTests
var admins = AdminsWith(enabledMember: true);
var ctx = new RequestContext { UserRole = "Authenticated" };
Assert.False(await PreviewWhitelistFunction.MayWriteNotificationEmailAsync(ctx, TenantId, admins.Object));
Assert.False(await PreviewWhitelistFunction.MayWriteNotificationEmailAsync(ctx, externalIdentity: false, TenantId, admins.Object));
}
[Fact]
@@ -86,7 +87,26 @@ public class PreviewNotificationEmailWriteGateTests
});
var ctx = new RequestContext { UserRole = "Authenticated" };
Assert.True(await PreviewWhitelistFunction.MayWriteNotificationEmailAsync(ctx, TenantId, admins.Object));
Assert.True(await PreviewWhitelistFunction.MayWriteNotificationEmailAsync(ctx, externalIdentity: false, TenantId, admins.Object));
}
[Fact]
public async Task External_identity_never_writes_in_the_signup_window()
{
// The window exists for the activating user, and an external identity never activates a tenant.
var admins = AdminsWith(enabledMember: false);
var ctx = new RequestContext { UserRole = "Authenticated" };
Assert.False(await PreviewWhitelistFunction.MayWriteNotificationEmailAsync(ctx, externalIdentity: true, TenantId, admins.Object));
}
[Fact]
public async Task External_identity_with_a_role_the_tenant_granted_may_write()
{
var admins = AdminsWith(enabledMember: true);
var ctx = new RequestContext { UserRole = Constants.TenantRoles.Admin };
Assert.True(await PreviewWhitelistFunction.MayWriteNotificationEmailAsync(ctx, externalIdentity: true, TenantId, admins.Object));
}
private static Mock<TenantAdminsService> AdminsWith(bool enabledMember)
@@ -1,37 +1,29 @@
using AutopilotMonitor.Functions.Services;
using AutopilotMonitor.Shared.Models;
using Xunit;
namespace AutopilotMonitor.Functions.Tests;
/// <summary>
/// Unit tests for the static helpers <c>TenantApprovalService.PickRequesterUpn</c>
/// and <c>TenantApprovalService.IsRealUserUpn</c>.
/// Unit tests for <c>TenantApprovalService.IsPromotableUpn</c>, the shape guard on the stored onboarding UPN that
/// activation promotes to TenantAdmin.
/// <para>
/// These guard the auto-promote-on-preview-approval path, which previously corrupted
/// 10 tenants' TenantAdmins rows by writing the sentinel string
/// <c>"System (Global Rate Limit Sync)"</c> as a TenantAdmin Upn. The fixes:
/// <list type="number">
/// <item>shape-based UPN validation (no enumeration of sentinel strings),</item>
/// <item>prefer the immutable <see cref="TenantConfiguration.OnboardedBy"/> over the
/// mutable <see cref="TenantConfiguration.UpdatedBy"/>.</item>
/// </list>
/// The auto-promote-on-approval path once corrupted 10 tenants' TenantAdmins rows by writing the sentinel string
/// <c>"System (Global Rate Limit Sync)"</c> as a TenantAdmin Upn. The guard is therefore shape-based (no
/// enumeration of sentinel strings), and it also refuses an external account's resource-tenant UPN. Which value is
/// promoted at all (OnboardedBy only, never UpdatedBy) is pinned in <see cref="TenantApprovalServiceTests"/>.
/// </para>
/// </summary>
public class PreviewWhitelistRequesterUpnTests
{
// -------------------------------------------------------------------------
// IsRealUserUpn — positive shape check, immune to new sentinel strings
// -------------------------------------------------------------------------
[Theory]
[InlineData("user@contoso.com")]
[InlineData("admin@tenant.onmicrosoft.com")]
[InlineData("first.last@example.de")]
[InlineData("cadm_user@contoso.onmicrosoft.com")]
public void IsRealUserUpn_AcceptsRealUpnShapes(string upn)
[InlineData("m.muster_ext_@contoso.example")] // a member naming scheme, not the external marker
public void IsPromotableUpn_AcceptsRealUpnShapes(string upn)
{
Assert.True(TenantApprovalService.IsRealUserUpn(upn));
Assert.True(TenantApprovalService.IsPromotableUpn(upn));
}
[Theory]
@@ -41,12 +33,20 @@ public class PreviewWhitelistRequesterUpnTests
[InlineData("System (Global Rate Limit Sync)")]
[InlineData("system (global rate limit sync)")]
[InlineData("System (whatever-future-sentinel)")]
public void IsRealUserUpn_RejectsAnySystemPrefixSentinel(string upn)
public void IsPromotableUpn_RejectsAnySystemPrefixSentinel(string upn)
{
// The shape-based prefix check is exactly the defense against the corruption
// pattern: any new "System (...)" sentinel introduced anywhere in the codebase
// is rejected without anyone needing to remember to update this method.
Assert.False(TenantApprovalService.IsRealUserUpn(upn));
Assert.False(TenantApprovalService.IsPromotableUpn(upn));
}
[Theory]
[InlineData("alice_fabrikam.example#EXT#@contoso.example")]
[InlineData("alice_fabrikam.example#ext#@contoso.example")]
public void IsPromotableUpn_RejectsAnExternalAccountsResourceTenantUpn(string upn)
{
Assert.False(TenantApprovalService.IsPromotableUpn(upn));
}
[Theory]
@@ -55,80 +55,8 @@ public class PreviewWhitelistRequesterUpnTests
[InlineData(" ")]
[InlineData("nodomain")]
[InlineData("just-a-string")]
public void IsRealUserUpn_RejectsNullOrNonEmailShapes(string? upn)
public void IsPromotableUpn_RejectsNullOrNonEmailShapes(string? upn)
{
Assert.False(TenantApprovalService.IsRealUserUpn(upn));
}
// -------------------------------------------------------------------------
// PickRequesterUpn — prefer OnboardedBy, fall back to UpdatedBy
// -------------------------------------------------------------------------
[Fact]
public void PickRequesterUpn_PrefersOnboardedByEvenWhenUpdatedByIsClean()
{
var cfg = new TenantConfiguration
{
OnboardedBy = "onboarder@contoso.com",
UpdatedBy = "editor@contoso.com",
};
Assert.Equal("onboarder@contoso.com", TenantApprovalService.PickRequesterUpn(cfg));
}
[Fact]
public void PickRequesterUpn_FallsBackToUpdatedByWhenOnboardedByNull()
{
var cfg = new TenantConfiguration
{
OnboardedBy = null,
UpdatedBy = "editor@contoso.com",
};
Assert.Equal("editor@contoso.com", TenantApprovalService.PickRequesterUpn(cfg));
}
[Fact]
public void PickRequesterUpn_FallsBackToUpdatedByWhenOnboardedByEmpty()
{
var cfg = new TenantConfiguration
{
OnboardedBy = "",
UpdatedBy = "editor@contoso.com",
};
Assert.Equal("editor@contoso.com", TenantApprovalService.PickRequesterUpn(cfg));
}
[Fact]
public void PickRequesterUpn_ShieldsAgainstUpdatedByClobberWhenOnboardedBySet()
{
// Regression: this is the exact scenario that corrupted 10 tenants.
// The global rate-limit sync clobbered UpdatedBy with the sentinel string;
// OnboardedBy preserves the real requester so auto-promote still picks them.
var cfg = new TenantConfiguration
{
OnboardedBy = "real.requester@contoso.com",
UpdatedBy = "System (Global Rate Limit Sync)",
};
Assert.Equal("real.requester@contoso.com", TenantApprovalService.PickRequesterUpn(cfg));
Assert.True(TenantApprovalService.IsRealUserUpn(TenantApprovalService.PickRequesterUpn(cfg)));
}
[Fact]
public void PickRequesterUpn_LegacyTenant_NoOnboardedBy_SentinelInUpdatedBy_IsRejectedByGuard()
{
// Defense-in-depth: even without OnboardedBy (pre-fix tenants), the IsRealUserUpn
// guard alone prevents the sentinel from leaking into TenantAdmins.
var cfg = new TenantConfiguration
{
OnboardedBy = null,
UpdatedBy = "System (Global Rate Limit Sync)",
};
var picked = TenantApprovalService.PickRequesterUpn(cfg);
Assert.Equal("System (Global Rate Limit Sync)", picked);
Assert.False(TenantApprovalService.IsRealUserUpn(picked));
Assert.False(TenantApprovalService.IsPromotableUpn(upn));
}
}
@@ -21,6 +21,7 @@ public class TenantApprovalServiceTests
private readonly Mock<PreviewWhitelistService> _previewMock;
private readonly Mock<TenantConfigurationService> _tenantConfigMock;
private readonly Mock<TenantAdminsService> _tenantAdminsMock;
private readonly Mock<IEmailService> _emailMock;
private readonly List<OpsEventEntry> _opsEvents = new();
private readonly TenantApprovalService _sut;
@@ -53,12 +54,15 @@ public class TenantApprovalServiceTests
configRepo, NullLogger<AdminConfigurationService>.Instance, cache);
var alertDispatch = TestNotifications.InertOpsAlertDispatch(adminConfig.Object);
// IsTenantAdminAsync is not virtual: it runs for real against the empty repository (no member yet).
_tenantAdminsMock = new Mock<TenantAdminsService>(
Mock.Of<IAdminRepository>(), cache, NullLogger<TenantAdminsService>.Instance);
_sut = new TenantApprovalService(
NullLogger<TenantApprovalService>.Instance,
_previewMock.Object,
_tenantConfigMock.Object,
new Mock<TenantAdminsService>(
Mock.Of<IAdminRepository>(), cache, NullLogger<TenantAdminsService>.Instance).Object,
_tenantAdminsMock.Object,
_emailMock.Object,
new OpsEventService(opsRepo.Object, NullLogger<OpsEventService>.Instance, alertDispatch));
}
@@ -95,8 +99,8 @@ public class TenantApprovalServiceTests
{
_previewMock.Setup(x => x.ApproveAsync(TenantId, ApprovedBy)).ReturnsAsync(true);
_previewMock.Setup(x => x.GetNotificationEmailAsync(TenantId)).ReturnsAsync((string?)null);
// Default config has no OnboardedBy/UpdatedBy UPN → auto-promote is skipped by the
// shape guard; no notification email → no mail. Only the return value is under test.
// Default config has no OnboardedBy → nothing to promote; no notification email → no
// mail. Only the return value is under test.
_tenantConfigMock.Setup(x => x.GetConfigurationAsync(TenantId))
.ReturnsAsync(TenantConfiguration.CreateDefault(TenantId));
@@ -106,6 +110,55 @@ public class TenantApprovalServiceTests
_previewMock.Verify(x => x.GetNotificationEmailAsync(TenantId), Times.Once);
}
// --- Auto-promote on activation: only the onboarding member (OnboardedBy) becomes Tenant Admin.
private void ArrangeNewlyApproved(string? onboardedBy, string updatedBy = "System")
{
var config = ConfigWith(contactEmail: null);
config.OnboardedBy = onboardedBy;
config.UpdatedBy = updatedBy;
_previewMock.Setup(x => x.ApproveAsync(TenantId, ApprovedBy)).ReturnsAsync(true);
_previewMock.Setup(x => x.GetNotificationEmailAsync(TenantId)).ReturnsAsync((string?)null);
_tenantConfigMock.Setup(x => x.GetConfigurationAsync(TenantId)).ReturnsAsync(config);
}
private void VerifyNoPromotion() =>
_tenantAdminsMock.Verify(
x => x.AddTenantAdminAsync(It.IsAny<string>(), It.IsAny<string>(), It.IsAny<string>()), Times.Never);
[Fact]
public async Task NewlyApproved_PromotesTheOnboardingMember()
{
// A sync job clobbering UpdatedBy with its sentinel once leaked into TenantAdmins; OnboardedBy decides alone.
ArrangeNewlyApproved(onboardedBy: "first.member@contoso.example", updatedBy: "System (Global Rate Limit Sync)");
await _sut.ApproveWithSideEffectsAsync(TenantId, ApprovedBy);
_tenantAdminsMock.Verify(
x => x.AddTenantAdminAsync(TenantId, "first.member@contoso.example", ApprovedBy), Times.Once);
}
[Fact]
public async Task NewlyApproved_NeverPromotesAnExternalAccountsUpn()
{
ArrangeNewlyApproved(onboardedBy: "alice_fabrikam.example#EXT#@contoso.example");
await _sut.ApproveWithSideEffectsAsync(TenantId, ApprovedBy);
VerifyNoPromotion();
}
[Fact]
public async Task NewlyApproved_WithoutOnboardedBy_NeverFallsBackToTheLastWriter()
{
// UpdatedBy is whoever wrote the row last (any signed-in user can, e.g. by accepting the DPA), not the requester.
ArrangeNewlyApproved(onboardedBy: null, updatedBy: "first.member@contoso.example");
await _sut.ApproveWithSideEffectsAsync(TenantId, ApprovedBy);
VerifyNoPromotion();
}
// --- TrySendWelcomeEmailAsync: the once-only send shared by the approval path and the
// notification-email save path. Ordering contract under test: the sent-marker must be
// consumed strictly AFTER the address check (a marker consumed with no address would
@@ -181,6 +181,6 @@ public class TenantAutoApproveHandlerTests
{
// The sentinel must never pass the promotable-user shape check — that is the
// defense against writing a system string into the TenantAdmins table.
Assert.False(TenantApprovalService.IsRealUserUpn(TenantAutoApproveHandler.AutoApprovedBy));
Assert.False(TenantApprovalService.IsPromotableUpn(TenantAutoApproveHandler.AutoApprovedBy));
}
}
@@ -86,6 +86,75 @@ public static class ClaimsPrincipalExtensions
public static bool IsCappedPrincipal(this ClaimsPrincipal principal)
=> principal.IsApplicationPrincipal() || principal.IsClientCapped();
private const string ExternalUpnMarker = "#EXT#";
private const string IdentityProviderClaimType = "http://schemas.microsoft.com/identity/claims/identityprovider";
// Raw and inbound-mapped names of the claims that carry a user name — never the display name ("name").
private static readonly string[] UserNameClaimTypes =
{ "upn", ClaimTypes.Upn, "preferred_username", "unique_name", ClaimTypes.Name };
// idp values that only an external account carries in a workforce tenant: a personal Microsoft account,
// an email one-time passcode, the social sign-in providers.
private static readonly HashSet<string> ExternalOnlyIdentityProviders =
new(StringComparer.OrdinalIgnoreCase) { "live.com", "mail", "google.com", "facebook.com" };
// The Entra token service: an idp URI on one of these hosts names the authenticating tenant first.
private static readonly HashSet<string> EntraSignInHosts = new(StringComparer.OrdinalIgnoreCase)
{
new Uri(AutopilotMonitor.Shared.Constants.EntraLoginBaseUrl).Host, "sts.windows.net", "login.windows.net", "login.microsoft.com",
};
private static readonly Guid PersonalMicrosoftAccountTenantId = new("9188040d-6c67-4c5b-b112-36a304b66dad");
/// <summary>
/// True when the token shows that the caller is not a member of the token's tenant (<c>tid</c>): a B2B guest
/// or another external identity. Such a caller keeps what the tenant grants it explicitly but never claims the
/// tenant — no first-login seed, no first-user Tenant Admin, no activation promotion, no write in the roleless
/// signup window. Positive evidence only; a token without any of these signals is a member's:
/// <list type="bullet">
/// <item><c>acct = 1</c>, Entra's guest flag (an optional claim of the API app registration);</item>
/// <item><c>#EXT#</c> in a user-name claim, the resource-tenant UPN Entra gives external accounts;</item>
/// <item>an <c>idp</c> naming a personal Microsoft account, an external sign-in provider, or another tenant on
/// an Entra sign-in host. A member's token carries no <c>idp</c> or one naming its own tenant. An <c>idp</c> on
/// any other host is no evidence: a false positive would silently strand a genuine first sign-in.</item>
/// </list>
/// Inbound claim mapping renames <c>tid</c> and <c>idp</c>, so both are read in either form.
/// </summary>
public static bool IsExternalIdentity(this ClaimsPrincipal principal)
{
if (string.Equals(principal.FindFirst("acct")?.Value?.Trim(), "1", StringComparison.Ordinal))
return true;
if (UserNameClaimTypes.Any(type => IsExternalUpn(principal.FindFirst(type)?.Value)))
return true;
var identityProvider = principal.FindFirst("idp")?.Value ?? principal.FindFirst(IdentityProviderClaimType)?.Value;
return IsForeignIdentityProvider(identityProvider, principal.GetTenantId());
}
/// <summary>True when a user name carries the <c>#EXT#</c> marker of an external account's resource-tenant UPN.</summary>
internal static bool IsExternalUpn(string? userName)
=> userName != null && userName.Contains(ExternalUpnMarker, StringComparison.OrdinalIgnoreCase);
/// <summary>
/// True when an <c>idp</c> claim value shows that the account authenticated outside <paramref name="tenantId"/>.
/// </summary>
internal static bool IsForeignIdentityProvider(string? identityProvider, string? tenantId)
{
var idp = identityProvider?.Trim();
if (string.IsNullOrEmpty(idp))
return false;
if (ExternalOnlyIdentityProviders.Contains(idp))
return true;
if (!Uri.TryCreate(idp, UriKind.Absolute, out var uri) || !EntraSignInHosts.Contains(uri.Host))
return false;
if (!Guid.TryParse(uri.AbsolutePath.Trim('/').Split('/')[0], out var authenticatingTenant))
return false;
return authenticatingTenant == PersonalMicrosoftAccountTenantId
|| !Guid.TryParse(tenantId, out var tokenTenant)
|| authenticatingTenant != tokenTenant;
}
/// <summary>
/// The caller's rate-limit budget class (<see cref="Security.ThrottleSurface"/>), from signed claims
/// only: <c>appidacr</c> (v1.0) / <c>azpacr</c> (v2.0) say how the CLIENT authenticated — <c>0</c> is a
@@ -22,6 +22,8 @@ namespace AutopilotMonitor.Functions.Functions.Infrastructure;
/// </summary>
public class AuthFunction
{
private static readonly LogThrottle ExternalIdentityLog = new(TimeSpan.FromHours(1));
private readonly ILogger<AuthFunction> _logger;
private readonly GlobalAdminService _globalAdminService;
private readonly DelegatedAdminService _delegatedAdminService;
@@ -162,8 +164,13 @@ public class AuthFunction
// 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.
// An external identity (a guest: this tid is not its home) never participates, whatever it was
// granted: its home domain would become this tenant's DomainName, which resolves cross-tenant grants,
// and its UPN the OnboardedBy that activation promotes to Tenant Admin.
var isDelegated = delegatedTenantIds.Count > 0;
var isHomeTenantParticipant = !isDelegated || isGlobalAdmin || isGlobalReader || memberRole != null;
var isExternalIdentity = principal.IsExternalIdentity();
var isHomeTenantParticipant = !isExternalIdentity
&& (!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();
@@ -176,6 +183,14 @@ public class AuthFunction
await HandleAutoReEnableAsync(tenantConfig, tenantId);
await HandleAuthClientIdTrackingAsync(tenantConfig, tenantId, tokenAudience);
}
else if (isExternalIdentity && string.IsNullOrEmpty(tenantConfig.DomainName)
&& ExternalIdentityLog.ShouldLog($"{tenantId}|{objectId}"))
{
// The activation page polls auth/me, so this is throttled per caller.
_logger.LogWarning(
"External identity {Upn} signed in to tenant {TenantId}, which no member has onboarded yet; it does not onboard it",
upn, tenantId);
}
// --- Pure decision logic (tested by AuthFunctionTests) ---
var decision = BuildAuthResult(
@@ -186,7 +201,8 @@ public class AuthFunction
homedApp: _appRegistry.ResolveForTenant(tenantConfig).IsLegacy ? "legacy" : "primary",
whatsNewSeenPlatformUtc: whatsNewSeen.PlatformUtc,
whatsNewSeenAgentUtc: whatsNewSeen.AgentUtc,
mcpClientRegistrationEnabled: (await _adminConfigService.GetConfigurationAsync())?.McpClientRegistrationEnabled ?? false);
mcpClientRegistrationEnabled: (await _adminConfigService.GetConfigurationAsync())?.McpClientRegistrationEnabled ?? false,
isExternalIdentity: isExternalIdentity);
if (!decision.IsSuccess)
{
@@ -723,7 +739,8 @@ public class AuthFunction
string homedApp = "primary",
DateTime? whatsNewSeenPlatformUtc = null,
DateTime? whatsNewSeenAgentUtc = null,
bool mcpClientRegistrationEnabled = false)
bool mcpClientRegistrationEnabled = false,
bool isExternalIdentity = false)
{
// 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
@@ -773,9 +790,10 @@ public class AuthFunction
// 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.
// != null), so this only suppresses the silent grant, never an existing right. The same holds for an
// external identity (a guest), which is not a member of this tenant at all.
bool isTenantAdmin = memberRole?.Role == Constants.TenantRoles.Admin;
bool needsAutoAdmin = memberRole == null && !hasTenantAdmins && !isGlobalReader && !isDelegated;
bool needsAutoAdmin = memberRole == null && !hasTenantAdmins && !isGlobalReader && !isDelegated && !isExternalIdentity;
if (needsAutoAdmin)
{
isTenantAdmin = true;
@@ -169,16 +169,18 @@ public class PreviewWhitelistFunction
var principal = context.GetUser();
var tenantId = principal?.GetTenantId();
if (string.IsNullOrWhiteSpace(tenantId))
if (principal == null || string.IsNullOrWhiteSpace(tenantId))
{
return await req.BadRequestAsync("Could not determine tenant");
}
if (!await MayWriteNotificationEmailAsync(req.GetRequestContext(), tenantId, _tenantAdminsService))
var externalIdentity = principal.IsExternalIdentity();
if (!await MayWriteNotificationEmailAsync(req.GetRequestContext(), externalIdentity, tenantId, _tenantAdminsService))
{
_logger.LogWarning(
"Preview notification email write denied for roleless caller {Upn} — tenant {TenantId} already has members",
principal?.GetUserPrincipalName() ?? "unknown", tenantId);
"Preview notification email write denied for roleless caller {Upn} in tenant {TenantId}: {Reason}",
principal.GetUserPrincipalName() ?? "unknown", tenantId,
externalIdentity ? "external identity" : "the tenant already has members");
return await req.ForbiddenAsync("A tenant member role is required to change the notification email");
}
@@ -218,15 +220,18 @@ public class PreviewWhitelistFunction
/// offboarding farewell, operator contact). A member role (or platform scope) always may write
/// it. A roleless caller may write it ONLY while the tenant has no enabled member at all — the
/// first-touch signup window, where the activating user is still roleless because the requester
/// is auto-promoted to Admin only on approval. Once any member exists, that window is closed for
/// good: an ordinary employee with a valid JWT but no product role must not be able to redirect
/// or suppress the tenant's correspondence.
/// is auto-promoted to Admin only on approval. The window is never an external identity's: it
/// neither onboards a tenant nor is promoted on activation. Once any member exists, that window is
/// closed for good: an ordinary employee with a valid JWT but no product role must not be able to
/// redirect or suppress the tenant's correspondence.
/// </summary>
internal static async Task<bool> MayWriteNotificationEmailAsync(
RequestContext ctx, string tenantId, TenantAdminsService tenantAdminsService)
RequestContext ctx, bool externalIdentity, string tenantId, TenantAdminsService tenantAdminsService)
{
if (ctx.IsTenantMemberOrGlobalAdmin())
return true;
if (externalIdentity)
return false;
var members = await tenantAdminsService.GetTenantAdminsAsync(tenantId);
return !members.Any(m => m.IsEnabled);
@@ -0,0 +1,34 @@
using System.Collections.Concurrent;
namespace AutopilotMonitor.Functions.Helpers;
/// <summary>
/// Lets one log line per key through per interval, for request paths that would otherwise repeat it on every
/// call (worker traces reach App Insights at Warning+ only, so a polled or hot path must not flood them).
/// Process-local and bounded: past <c>maxKeys</c> the memory is dropped, which at worst repeats a line early.
/// </summary>
public sealed class LogThrottle
{
private readonly ConcurrentDictionary<string, DateTime> _lastLogged = new();
private readonly TimeSpan _interval;
private readonly int _maxKeys;
public LogThrottle(TimeSpan interval, int maxKeys = 1000)
{
_interval = interval;
_maxKeys = maxKeys;
}
/// <summary>True when the line for <paramref name="key"/> may be written now; records that it was.</summary>
public bool ShouldLog(string key) => ShouldLog(key, DateTime.UtcNow);
internal bool ShouldLog(string key, DateTime nowUtc)
{
if (_lastLogged.TryGetValue(key, out var last) && nowUtc - last < _interval)
return false;
if (_lastLogged.Count >= _maxKeys)
_lastLogged.Clear();
_lastLogged[key] = nowUtc;
return true;
}
}
@@ -1,4 +1,3 @@
using System.Collections.Concurrent;
using System.Net;
using System.Security.Claims;
using AutopilotMonitor.Functions.Extensions;
@@ -21,12 +20,9 @@ namespace AutopilotMonitor.Functions.Middleware;
/// </summary>
public class ClientAppBindingMiddleware : IFunctionsWorkerMiddleware
{
private static readonly TimeSpan LogInterval = TimeSpan.FromHours(1);
private const int MaxLogKeys = 1000;
// Worker traces reach App Insights at Warning+ only; the request dimensions carry the per-request
// measurement, this line makes a new (tenant, client, outcome) visible without flooding.
private static readonly ConcurrentDictionary<string, DateTime> LastLogged = new();
private static readonly LogThrottle OutcomeLog = new(TimeSpan.FromHours(1));
private readonly ILogger<ClientAppBindingMiddleware> _logger;
private readonly IConfiguration _configuration;
@@ -118,13 +114,8 @@ public class ClientAppBindingMiddleware : IFunctionsWorkerMiddleware
private void LogOncePerInterval(string? tenantId, string? clientAppId, string outcome, bool enforced)
{
var key = $"{tenantId}|{clientAppId}|{outcome}|{enforced}";
var now = DateTime.UtcNow;
if (LastLogged.TryGetValue(key, out var last) && now - last < LogInterval)
if (!OutcomeLog.ShouldLog($"{tenantId}|{clientAppId}|{outcome}|{enforced}"))
return;
if (LastLogged.Count >= MaxLogKeys)
LastLogged.Clear();
LastLogged[key] = now;
_logger.LogWarning(
"[ClientAppBinding] Delegated token from client app {ClientAppId} (tid={TenantId}): {Outcome}, enforced={Enforced}",
@@ -274,7 +274,8 @@ public static class EndpointAccessPolicyCatalog
// AuthenticatedUserWithRole (not plain AuthenticatedUser): the activating user is still
// roleless during first-touch signup, so the tier must admit non-members — but the address is
// tenant-shared state feeding the welcome/farewell mails, so the function re-gates on the
// resolved role: roleless callers may only write while the tenant has no member yet.
// resolved role: roleless callers may only write while the tenant has no member yet, and never
// as an external identity, which does not activate a tenant.
new("PUT", "preview/notification-email", EndpointPolicy.AuthenticatedUserWithRole),
new("GET", "feedback/status", EndpointPolicy.AuthenticatedUser),
new("POST", "feedback", EndpointPolicy.AuthenticatedUser),
@@ -16,7 +16,7 @@ namespace AutopilotMonitor.Functions.Services.Activation
public class TenantAutoApproveHandler
{
/// <summary>ApprovedBy sentinel. Deliberately without '@' so the shape-based
/// <see cref="TenantApprovalService.IsRealUserUpn"/> guard can never treat it as a
/// <see cref="TenantApprovalService.IsPromotableUpn"/> guard can never treat it as a
/// promotable user.</summary>
public const string AutoApprovedBy = "System (auto-approve)";
@@ -1,4 +1,4 @@
using AutopilotMonitor.Shared.Models;
using AutopilotMonitor.Functions.Extensions;
using Microsoft.Extensions.Logging;
namespace AutopilotMonitor.Functions.Services;
@@ -54,22 +54,23 @@ public class TenantApprovalService
_logger.LogInformation("Tenant activated: {TenantId} by {ApprovedBy}", tenantId, approvedBy);
// Auto-promote the tenant requester (first user who triggered tenant config creation)
// as TenantAdmin if they are not already one.
// Auto-promote the onboarding member (OnboardedBy, written once by the first member login and
// never by an external identity) as TenantAdmin if they are not already one.
// This ensures whoever signed up doesn't need manual admin assignment after activation.
// Never UpdatedBy: that is the last writer, not the requester, and any signed-in user sets it.
try
{
var tenantConfig = await _tenantConfigurationService.GetConfigurationAsync(tenantId);
var requesterUpn = PickRequesterUpn(tenantConfig);
var requesterUpn = tenantConfig.OnboardedBy;
// Positive UPN-shape validation: real Azure AD UPNs always contain '@', while
// every system-written sentinel ("System", "System (auto-re-enable)",
// "System (Global Rate Limit Sync)", …) does not. An equality list against
// known sentinels previously missed a third sentinel and corrupted 10 tenants'
// TenantAdmins rows — keep this check shape-based, not enumeration-based.
if (IsRealUserUpn(requesterUpn))
if (IsPromotableUpn(requesterUpn))
{
// IsRealUserUpn returned true → requesterUpn is non-null, non-empty, contains '@'
// IsPromotableUpn returned true → requesterUpn is non-null, non-empty, contains '@'
var validUpn = requesterUpn!;
var isAlreadyAdmin = await _tenantAdminsService.IsTenantAdminAsync(tenantId, validUpn);
if (!isAlreadyAdmin)
@@ -89,8 +90,8 @@ public class TenantApprovalService
else
{
_logger.LogInformation(
"No valid tenant requester UPN found in TenantConfiguration for tenant {TenantId} (OnboardedBy: '{OnboardedBy}', UpdatedBy: '{UpdatedBy}') — skipping auto-promote",
tenantId, tenantConfig.OnboardedBy ?? "<null>", tenantConfig.UpdatedBy ?? "<null>");
"No promotable onboarding UPN in TenantConfiguration for tenant {TenantId} (OnboardedBy: '{OnboardedBy}') — skipping auto-promote",
tenantId, tenantConfig.OnboardedBy ?? "<null>");
}
}
@@ -213,32 +214,15 @@ public class TenantApprovalService
}
/// <summary>
/// Picks the UPN of the user to auto-promote as TenantAdmin on activation.
/// Prefers <see cref="TenantConfiguration.OnboardedBy"/> (immutable, set once on first
/// user login) over <see cref="TenantConfiguration.UpdatedBy"/> (mutable, can be
/// clobbered by background jobs such as the global rate-limit sync). Fall back to
/// <see cref="TenantConfiguration.UpdatedBy"/> for tenants onboarded before
/// <see cref="TenantConfiguration.OnboardedBy"/> existed — callers still guard the
/// result with <see cref="IsRealUserUpn"/> so a sentinel value cannot leak into
/// the TenantAdmins table.
/// True when a stored onboarding UPN may be promoted to TenantAdmin: shaped like a real user
/// principal name (contains '@', no "System" sentinel prefix) and not an external account's
/// resource-tenant UPN (<c>#EXT#</c>), which no member login writes.
/// </summary>
internal static string? PickRequesterUpn(TenantConfiguration config)
{
if (config == null) return null;
return !string.IsNullOrWhiteSpace(config.OnboardedBy)
? config.OnboardedBy
: config.UpdatedBy;
}
/// <summary>
/// True when the value looks like a real Azure AD user principal name
/// (contains '@' and does not start with the "System" sentinel prefix
/// used by background jobs that touch <see cref="TenantConfiguration.UpdatedBy"/>).
/// </summary>
internal static bool IsRealUserUpn(string? upn)
internal static bool IsPromotableUpn(string? upn)
{
if (string.IsNullOrWhiteSpace(upn)) return false;
if (upn.StartsWith("System", StringComparison.OrdinalIgnoreCase)) return false;
if (ClaimsPrincipalExtensions.IsExternalUpn(upn)) return false;
return upn.Contains('@');
}
}
@@ -4961,13 +4961,13 @@ export interface TenantConfigPatchOutcomeResponse {
export interface TenantConfiguration {
/** Tenant ID (PartitionKey in Table Storage) */
tenantId: string;
/** Domain name extracted from the first user's UPN Used for display purposes (e.g., contoso.com) */
/** Domain of the onboarding member's UPN (e.g. contoso.com), seeded once by the first member login and never from an external identity. Tenant identity, not only display: transactional mails render it, and cross-tenant grants resolve a never-signed-in UPN's home tenant through it. */
domainName: string;
/** When the configuration was last updated */
lastUpdated: string;
/** Updated by (user email or system) */
updatedBy: string;
/** UPN of the user whose first login created this tenant configuration. Set once in HandleNewTenantDomainAsync alongside DomainName and never overwritten. Used by the preview-approval auto-promote path so background jobs that mutate UpdatedBy (e.g. global rate-limit sync) cannot leak a sentinel string into the TenantAdmins table. Null on rows that pre-date the OnboardedBy field — auto-promote falls back to UpdatedBy with a UPN-shape guard. */
/** UPN of the member whose first login onboarded this tenant. Set once in HandleNewTenantDomainAsync alongside DomainName and never overwritten; an external identity (guest) never sets it. Activation promotes this UPN, and only this one, to Tenant Admin, so background jobs that mutate UpdatedBy (e.g. global rate-limit sync) cannot leak a sentinel or a last writer into the TenantAdmins table. Null on rows that pre-date the field: such a tenant gets no promotion on activation. */
onboardedBy?: string | null;
/** Version of the data processing agreement this tenant accepted — CurrentDpaVersion at that moment. Written once: with the onboarding when the first sign-in came through the get-started tick, otherwise when the portal's acceptance dialog is confirmed (DpaAcceptancePending). Never client-writable. Null on tenants onboarded before the field existed. */
dpaVersion?: string | null;
@@ -18,8 +18,9 @@ namespace AutopilotMonitor.Shared.Models
public string TenantId { get; set; } = default!;
/// <summary>
/// Domain name extracted from the first user's UPN
/// Used for display purposes (e.g., contoso.com)
/// Domain of the onboarding member's UPN (e.g. contoso.com), seeded once by the first member
/// login and never from an external identity. Tenant identity, not only display: transactional
/// mails render it, and cross-tenant grants resolve a never-signed-in UPN's home tenant through it.
/// </summary>
public string DomainName { get; set; } = default!;
@@ -34,13 +35,13 @@ namespace AutopilotMonitor.Shared.Models
public string UpdatedBy { get; set; } = default!;
/// <summary>
/// UPN of the user whose first login created this tenant configuration. Set once
/// in <c>HandleNewTenantDomainAsync</c> alongside <see cref="DomainName"/> and
/// never overwritten. Used by the preview-approval auto-promote path so background
/// jobs that mutate <see cref="UpdatedBy"/> (e.g. global rate-limit sync) cannot
/// leak a sentinel string into the TenantAdmins table.
/// Null on rows that pre-date the OnboardedBy field — auto-promote falls back to
/// <see cref="UpdatedBy"/> with a UPN-shape guard.
/// UPN of the member whose first login onboarded this tenant. Set once in
/// <c>HandleNewTenantDomainAsync</c> alongside <see cref="DomainName"/> and never
/// overwritten; an external identity (guest) never sets it. Activation promotes this UPN,
/// and only this one, to Tenant Admin, so background jobs that mutate
/// <see cref="UpdatedBy"/> (e.g. global rate-limit sync) cannot leak a sentinel or a last
/// writer into the TenantAdmins table. Null on rows that pre-date the field: such a tenant
/// gets no promotion on activation.
/// </summary>
public string? OnboardedBy { get; set; }
@@ -142,7 +142,7 @@ export default function AdminManagementSection({
</svg>
<p className="text-sm text-yellow-800">
<strong>Important:</strong> Make sure to keep at least one Admin in the list to maintain full access!
The first user to log in was automatically made an admin.
The first member of your organization to log in was automatically made an admin.
</p>
</div>
</div>
@@ -22742,7 +22742,7 @@
"kind": "primitive",
"name": "string"
},
"doc": "Domain name extracted from the first user's UPN Used for display purposes (e.g., contoso.com)"
"doc": "Domain of the onboarding member's UPN (e.g. contoso.com), seeded once by the first member login and never from an external identity. Tenant identity, not only display: transactional mails render it, and cross-tenant grants resolve a never-signed-in UPN's home tenant through it."
},
{
"name": "lastUpdated",
@@ -22768,7 +22768,7 @@
"nullable": true
},
"optional": true,
"doc": "UPN of the user whose first login created this tenant configuration. Set once in HandleNewTenantDomainAsync alongside DomainName and never overwritten. Used by the preview-approval auto-promote path so background jobs that mutate UpdatedBy (e.g. global rate-limit sync) cannot leak a sentinel string into the TenantAdmins table. Null on rows that pre-date the OnboardedBy field — auto-promote falls back to UpdatedBy with a UPN-shape guard."
"doc": "UPN of the member whose first login onboarded this tenant. Set once in HandleNewTenantDomainAsync alongside DomainName and never overwritten; an external identity (guest) never sets it. Activation promotes this UPN, and only this one, to Tenant Admin, so background jobs that mutate UpdatedBy (e.g. global rate-limit sync) cannot leak a sentinel or a last writer into the TenantAdmins table. Null on rows that pre-date the field: such a tenant gets no promotion on activation."
},
{
"name": "dpaVersion",
@@ -4961,13 +4961,13 @@ export interface TenantConfigPatchOutcomeResponse {
export interface TenantConfiguration {
/** Tenant ID (PartitionKey in Table Storage) */
tenantId: string;
/** Domain name extracted from the first user's UPN Used for display purposes (e.g., contoso.com) */
/** Domain of the onboarding member's UPN (e.g. contoso.com), seeded once by the first member login and never from an external identity. Tenant identity, not only display: transactional mails render it, and cross-tenant grants resolve a never-signed-in UPN's home tenant through it. */
domainName: string;
/** When the configuration was last updated */
lastUpdated: string;
/** Updated by (user email or system) */
updatedBy: string;
/** UPN of the user whose first login created this tenant configuration. Set once in HandleNewTenantDomainAsync alongside DomainName and never overwritten. Used by the preview-approval auto-promote path so background jobs that mutate UpdatedBy (e.g. global rate-limit sync) cannot leak a sentinel string into the TenantAdmins table. Null on rows that pre-date the OnboardedBy field — auto-promote falls back to UpdatedBy with a UPN-shape guard. */
/** UPN of the member whose first login onboarded this tenant. Set once in HandleNewTenantDomainAsync alongside DomainName and never overwritten; an external identity (guest) never sets it. Activation promotes this UPN, and only this one, to Tenant Admin, so background jobs that mutate UpdatedBy (e.g. global rate-limit sync) cannot leak a sentinel or a last writer into the TenantAdmins table. Null on rows that pre-date the field: such a tenant gets no promotion on activation. */
onboardedBy?: string | null;
/** Version of the data processing agreement this tenant accepted — CurrentDpaVersion at that moment. Written once: with the onboarding when the first sign-in came through the get-started tick, otherwise when the portal's acceptance dialog is confirmed (DpaAcceptancePending). Never client-writable. Null on tenants onboarded before the field existed. */
dpaVersion?: string | null;