.NET: [BREAKING] Fix hosting bugs (#6388)

* Fix hosting bugs

* Address PR comments
This commit is contained in:
westey
2026-06-08 17:17:54 +01:00
committed by GitHub
Unverified
parent 6169df04cb
commit 6a2efeae7c
6 changed files with 219 additions and 25 deletions
@@ -44,18 +44,33 @@ public static class HostedFoundryMemoryProviderScopes
session => new FoundryMemoryProvider.State(new FoundryMemoryProviderScope(GetRequiredHostedContext(session).ChatId));
/// <summary>
/// Returns a <c>stateInitializer</c> that scopes memories per (user, chat) pair, using
/// <c>"{UserId}:{ChatId}"</c> as the partition key. Use this when memories should be visible
/// only to the same user within the same conversation.
/// Returns a <c>stateInitializer</c> that scopes memories per (user, chat) pair, composing
/// <see cref="HostedSessionContext.UserId"/> and <see cref="HostedSessionContext.ChatId"/> into a
/// single delimiter-safe partition key. Use this when memories should be visible only to the same
/// user within the same conversation.
/// </summary>
/// <remarks>
/// Both identity values are opaque strings that may contain any characters, including the <c>:</c>
/// delimiter. To keep the composite key injective (so two distinct (user, chat) pairs can never
/// collide), each part is escaped (<c>\</c> becomes <c>\\</c>, then <c>:</c> becomes <c>\:</c>) before
/// being joined with a <c>::</c> separator.
/// </remarks>
/// <returns>A delegate suitable for the <c>stateInitializer</c> argument of <see cref="FoundryMemoryProvider"/>.</returns>
public static Func<AgentSession?, FoundryMemoryProvider.State> PerUserAndChat() =>
session =>
{
var ctx = GetRequiredHostedContext(session);
return new FoundryMemoryProvider.State(new FoundryMemoryProviderScope($"{ctx.UserId}:{ctx.ChatId}"));
return new FoundryMemoryProvider.State(
new FoundryMemoryProviderScope($"{EscapeScopePart(ctx.UserId)}::{EscapeScopePart(ctx.ChatId)}"));
};
/// <summary>
/// Escapes special characters in a scope part so that distinct (user, chat) pairs produce distinct
/// composite scope keys. Backslashes are escaped first (<c>\</c> becomes <c>\\</c>), then colons
/// (<c>:</c> becomes <c>\:</c>), ensuring the <c>{user}::{chat}</c> format is unambiguous.
/// </summary>
private static string EscapeScopePart(string part) => part.Replace("\\", "\\\\").Replace(":", "\\:");
private static HostedSessionContext GetRequiredHostedContext(AgentSession? session) =>
session?.GetHostedContext()
?? throw new InvalidOperationException(
@@ -21,6 +21,18 @@ namespace Microsoft.Agents.AI.Hosting;
/// from the ambient <see cref="HttpContext"/>.
/// </para>
/// <para>
/// <strong>Security warning:</strong> The configured <see cref="ClaimsIdentitySessionIsolationKeyProviderOptions.ClaimType"/>
/// must uniquely identify the principal within the served population. Display names, usernames, email
/// aliases, and other mutable or non-unique claims are <strong>unsafe</strong> isolation keys unless the
/// host can prove their uniqueness across all callers: two distinct principals that share the same value
/// would receive the same isolation key and could read or overwrite one another's persisted sessions.
/// The default claim type is <see cref="ClaimTypes.NameIdentifier"/>, a stable unique subject identifier
/// that is typically populated from the OpenID Connect <c>sub</c> claim via the default JWT inbound claim
/// mapping (note that this differs from Entra's object identifier <c>oid</c> claim; override
/// <see cref="ClaimsIdentitySessionIsolationKeyProviderOptions.ClaimType"/> if you need <c>oid</c> or your
/// provider maps a different claim).
/// </para>
/// <para>
/// If the <see cref="HttpContext"/> is unavailable, the user is not authenticated, or the specified claim
/// is missing, the provider returns <see langword="null"/>. The consuming <see cref="IsolationKeyScopedAgentSessionStore"/>
/// will then enforce strict or pass-through behavior based on its configuration.
@@ -60,18 +72,24 @@ public class ClaimsIdentitySessionIsolationKeyProvider : SessionIsolationKeyProv
/// <param name="cancellationToken">The <see cref="CancellationToken"/> to monitor for cancellation requests.</param>
/// <returns>
/// A task that represents the asynchronous operation. The task result contains the value of the
/// configured claim type from the current user's identity, or <see langword="null"/> if the claim
/// is not present or the HTTP context is unavailable.
/// configured claim type from the current user's identity, or <see langword="null"/> if the HTTP
/// context is unavailable, the user is not authenticated, or the claim is not present.
/// </returns>
/// <remarks>
/// This method retrieves the claim value from <c>HttpContext.User.Claims</c>. If multiple claims
/// of the specified type exist, the first match is returned.
/// This method only reads claims from an authenticated principal: if the current request has no
/// authenticated user, it returns <see langword="null"/> rather than trusting claims on an
/// unauthenticated identity. The claim value is retrieved from <c>HttpContext.User.Claims</c>; if
/// multiple claims of the specified type exist, the first match is returned.
/// </remarks>
public override ValueTask<string?> GetSessionIsolationKeyAsync(CancellationToken cancellationToken = default)
{
Claim? claim = this._httpContextAccessor?
.HttpContext?
.User?.Claims.FirstOrDefault(c => c.Type == this._claimType);
ClaimsPrincipal? user = this._httpContextAccessor?.HttpContext?.User;
if (user?.Identity?.IsAuthenticated != true)
{
return new ValueTask<string?>((string?)null);
}
Claim? claim = user?.Claims.FirstOrDefault(c => c.Type == this._claimType);
return new ValueTask<string?>(claim?.Value);
}
@@ -14,17 +14,30 @@ public class ClaimsIdentitySessionIsolationKeyProviderOptions
/// </summary>
/// <remarks>
/// <para>
/// Defaults to <see cref="ClaimsIdentity.DefaultNameClaimType"/>, which typically corresponds to
/// the user's name or unique identifier claim.
/// Defaults to <see cref="ClaimTypes.NameIdentifier"/>, which corresponds to a stable, unique
/// subject identifier for the authenticated principal. For OpenID Connect tokens (including those
/// issued by Microsoft Entra ID), this is typically populated from the <c>sub</c> claim via the
/// default JWT inbound claim mapping. Note that <c>sub</c> is distinct from Entra's object
/// identifier (<c>oid</c>) claim; if you require the <c>oid</c> claim, or your provider does not map
/// a unique identifier onto <see cref="ClaimTypes.NameIdentifier"/>, override <see cref="ClaimType"/>
/// with the appropriate claim type.
/// </para>
/// <para>
/// <strong>Security warning:</strong> The configured claim must uniquely identify the principal
/// within the served population. Display names (<see cref="ClaimsIdentity.DefaultNameClaimType"/>
/// / <see cref="ClaimTypes.Name"/>), usernames, email aliases, and other mutable or non-unique
/// claims are <strong>unsafe</strong> isolation keys unless the host can prove their uniqueness
/// across all callers. Two distinct principals that share the same value for a non-unique claim
/// would receive the same session-isolation key and could read or overwrite one another's
/// persisted sessions. Only override this value with a claim that is guaranteed unique and stable.
/// </para>
/// <para>
/// Common alternatives include:
/// <list type="bullet">
/// <item><description><c>ClaimTypes.NameIdentifier</c> — Stable user identifier</description></item>
/// <item><description><c>ClaimTypes.Email</c> — Email address</description></item>
/// <item><description>Custom claim types specific to your authentication provider</description></item>
/// <item><description>A composite of tenant and subject identifiers — required for multi-tenant hosts where the subject is only unique per tenant</description></item>
/// <item><description>Custom claim types specific to your authentication provider, provided they are unique and stable</description></item>
/// </list>
/// </para>
/// </remarks>
public string ClaimType { get; set; } = ClaimsIdentity.DefaultNameClaimType;
public string ClaimType { get; set; } = ClaimTypes.NameIdentifier;
}
@@ -1,6 +1,7 @@
// Copyright (c) Microsoft. All rights reserved.
using System;
using System.Security.Claims;
using Microsoft.AspNetCore.Http;
using Microsoft.Extensions.DependencyInjection;
@@ -19,8 +20,28 @@ public static class ServiceCollectionExtensions
/// <param name="options"> Optional configuration for the claims-based session isolation key provider.</param>
/// <returns>The <see cref="IServiceCollection"/> so that additional calls can be chained.</returns>
/// <remarks>
/// <para>
/// This method requires <see cref="IHttpContextAccessor"/> to be registered in the service collection.
/// Ensure that <c>services.AddHttpContextAccessor()</c> has been called before using this method.
/// </para>
/// <para>
/// When <paramref name="options"/> is not supplied, the isolation key is derived from the
/// <see cref="ClaimTypes.NameIdentifier"/> claim, a stable unique subject identifier. For OpenID
/// Connect tokens (including Microsoft Entra ID), this is typically mapped from the <c>sub</c> claim
/// by the default JWT inbound claim mapping. Authentication schemes that do not project a unique
/// identifier onto <see cref="ClaimTypes.NameIdentifier"/> (or hosts that require a different claim
/// such as Entra's <c>oid</c>) should override
/// <see cref="ClaimsIdentitySessionIsolationKeyProviderOptions.ClaimType"/>; otherwise the key may be
/// absent, which causes strict-mode session stores to fail.
/// </para>
/// <para>
/// <strong>Security warning:</strong> If you override
/// <see cref="ClaimsIdentitySessionIsolationKeyProviderOptions.ClaimType"/>, the chosen claim must
/// uniquely identify the principal within the served population. Display names, usernames, email
/// aliases, and other mutable or non-unique claims are <strong>unsafe</strong> isolation keys unless
/// the host can prove their uniqueness across all callers, because distinct principals that share the
/// same claim value would receive the same isolation key and could access one another's sessions.
/// </para>
/// </remarks>
public static IServiceCollection UseClaimsBasedSessionIsolation(
this IServiceCollection services,
@@ -43,7 +43,7 @@ public class HostedFoundryMemoryProviderScopesTests
}
[Fact]
public void PerUserAndChat_ComposesUserAndChatWithColon()
public void PerUserAndChat_ComposesUserAndChatWithEscapedSeparator()
{
// Arrange
var session = CreateTaggedSession(TestUserId, TestChatId);
@@ -54,7 +54,51 @@ public class HostedFoundryMemoryProviderScopesTests
// Assert
Assert.NotNull(state);
Assert.Equal($"{TestUserId}:{TestChatId}", state.Scope.Scope);
Assert.Equal($"{TestUserId}::{TestChatId}", state.Scope.Scope);
}
[Fact]
public void PerUserAndChat_EscapesColonsInUserAndChat()
{
// Arrange
var session = CreateTaggedSession("alice:finance", "q2:final");
var initializer = HostedFoundryMemoryProviderScopes.PerUserAndChat();
// Act
var state = initializer(session);
// Assert - colons inside each part are escaped as \: , parts joined with ::
Assert.Equal(@"alice\:finance::q2\:final", state.Scope.Scope);
}
[Fact]
public void PerUserAndChat_EscapesBackslashesInUserAndChat()
{
// Arrange
var session = CreateTaggedSession(@"alice\corp", @"chat\1");
var initializer = HostedFoundryMemoryProviderScopes.PerUserAndChat();
// Act
var state = initializer(session);
// Assert - backslashes escaped first as \\ , parts joined with ::
Assert.Equal(@"alice\\corp::chat\\1", state.Scope.Scope);
}
[Fact]
public void PerUserAndChat_DistinctContextsDoNotCollide()
{
// Arrange - two distinct (UserId, ChatId) pairs that collide under raw-colon composition.
var sessionA = CreateTaggedSession("alice:finance", "q2");
var sessionB = CreateTaggedSession("alice", "finance:q2");
var initializer = HostedFoundryMemoryProviderScopes.PerUserAndChat();
// Act
var scopeA = initializer(sessionA).Scope.Scope;
var scopeB = initializer(sessionB).Scope.Scope;
// Assert
Assert.NotEqual(scopeA, scopeB);
}
[Fact]
@@ -16,6 +16,7 @@ public class ClaimsIdentitySessionIsolationKeyProviderTests
private const string TestUserId = "test-user-id";
private const string CustomClaimType = "custom-claim-type";
private const string CustomClaimValue = "custom-claim-value";
private const string TestAuthenticationType = "TestAuth";
private readonly Mock<IHttpContextAccessor> _httpContextAccessorMock;
@@ -101,7 +102,7 @@ public class ClaimsIdentitySessionIsolationKeyProviderTests
public async Task GetSessionIsolationKeyAsyncExtractsDefaultClaimTypeAsync()
{
// Arrange
this.SetupHttpContextWithClaim(ClaimsIdentity.DefaultNameClaimType, TestUserId);
this.SetupHttpContextWithClaim(ClaimTypes.NameIdentifier, TestUserId);
var provider = new ClaimsIdentitySessionIsolationKeyProvider(this._httpContextAccessorMock.Object);
// Act
@@ -111,6 +112,25 @@ public class ClaimsIdentitySessionIsolationKeyProviderTests
Assert.Equal(TestUserId, result);
}
/// <summary>
/// Verify that the default claim type is the stable, unique NameIdentifier claim rather than the
/// non-unique display name claim. This guards against the session-isolation collision described in
/// the security report where two principals sharing the same name claim received the same key.
/// </summary>
[Fact]
public async Task GetSessionIsolationKeyAsyncIgnoresNameClaimByDefaultAsync()
{
// Arrange - only a display-name claim is present; the default provider must not use it.
this.SetupHttpContextWithClaim(ClaimsIdentity.DefaultNameClaimType, TestUserId);
var provider = new ClaimsIdentitySessionIsolationKeyProvider(this._httpContextAccessorMock.Object);
// Act
string? result = await provider.GetSessionIsolationKeyAsync();
// Assert
Assert.Null(result);
}
/// <summary>
/// Verify that GetSessionIsolationKeyAsync uses custom claim type when specified.
/// </summary>
@@ -191,10 +211,10 @@ public class ClaimsIdentitySessionIsolationKeyProviderTests
const string SecondValue = "second-value";
var claims = new[]
{
new Claim(ClaimsIdentity.DefaultNameClaimType, FirstValue),
new Claim(ClaimsIdentity.DefaultNameClaimType, SecondValue),
new Claim(ClaimTypes.NameIdentifier, FirstValue),
new Claim(ClaimTypes.NameIdentifier, SecondValue),
};
var identity = new ClaimsIdentity(claims);
var identity = new ClaimsIdentity(claims, TestAuthenticationType);
var principal = new ClaimsPrincipal(identity);
var httpContext = new DefaultHttpContext
@@ -219,7 +239,7 @@ public class ClaimsIdentitySessionIsolationKeyProviderTests
public async Task GetSessionIsolationKeyAsyncHandlesEmptyClaimValueAsync()
{
// Arrange
this.SetupHttpContextWithClaim(ClaimsIdentity.DefaultNameClaimType, string.Empty);
this.SetupHttpContextWithClaim(ClaimTypes.NameIdentifier, string.Empty);
var provider = new ClaimsIdentitySessionIsolationKeyProvider(this._httpContextAccessorMock.Object);
// Act
@@ -229,6 +249,66 @@ public class ClaimsIdentitySessionIsolationKeyProviderTests
Assert.Equal(string.Empty, result);
}
/// <summary>
/// Regression test for the session-isolation collision security report: two distinct authenticated
/// principals that share the same display-name claim but have different stable identifiers and tenants
/// must produce distinct isolation keys under the default options.
/// </summary>
[Fact]
public async Task GetSessionIsolationKeyAsyncDistinctForPrincipalsSharingNameClaimAsync()
{
// Arrange - both principals share the same name claim but differ by NameIdentifier and tenant.
const string CommonName = "John Doe";
var principalA = CreatePrincipal(
new Claim(ClaimsIdentity.DefaultNameClaimType, CommonName),
new Claim(ClaimTypes.NameIdentifier, "oid-user-a"),
new Claim("http://schemas.microsoft.com/identity/claims/tenantid", "tenant-a"));
var principalB = CreatePrincipal(
new Claim(ClaimsIdentity.DefaultNameClaimType, CommonName),
new Claim(ClaimTypes.NameIdentifier, "oid-user-b"),
new Claim("http://schemas.microsoft.com/identity/claims/tenantid", "tenant-b"));
var provider = new ClaimsIdentitySessionIsolationKeyProvider(this._httpContextAccessorMock.Object);
// Act
this._httpContextAccessorMock.Setup(x => x.HttpContext).Returns(new DefaultHttpContext { User = principalA });
string? principalAKey = await provider.GetSessionIsolationKeyAsync();
this._httpContextAccessorMock.Setup(x => x.HttpContext).Returns(new DefaultHttpContext { User = principalB });
string? principalBKey = await provider.GetSessionIsolationKeyAsync();
// Assert
Assert.Equal("oid-user-a", principalAKey);
Assert.Equal("oid-user-b", principalBKey);
Assert.NotEqual(principalAKey, principalBKey);
}
/// <summary>
/// Verify that GetSessionIsolationKeyAsync returns null when the request's user is not authenticated,
/// even if a claim of the configured type is present. The provider must not derive an isolation key
/// from claims on an unauthenticated identity.
/// </summary>
[Fact]
public async Task GetSessionIsolationKeyAsyncReturnsNullWhenUserNotAuthenticatedAsync()
{
// Arrange - identity has the claim but no authentication type, so IsAuthenticated is false.
var claims = new[] { new Claim(ClaimTypes.NameIdentifier, TestUserId) };
var unauthenticatedIdentity = new ClaimsIdentity(claims);
var principal = new ClaimsPrincipal(unauthenticatedIdentity);
var httpContext = new DefaultHttpContext { User = principal };
this._httpContextAccessorMock.Setup(x => x.HttpContext).Returns(httpContext);
var provider = new ClaimsIdentitySessionIsolationKeyProvider(this._httpContextAccessorMock.Object);
// Act
string? result = await provider.GetSessionIsolationKeyAsync();
// Assert
Assert.False(unauthenticatedIdentity.IsAuthenticated);
Assert.Null(result);
}
#endregion
#region Helper Methods
@@ -236,7 +316,7 @@ public class ClaimsIdentitySessionIsolationKeyProviderTests
private void SetupHttpContextWithClaim(string claimType, string claimValue)
{
var claims = new[] { new Claim(claimType, claimValue) };
var identity = new ClaimsIdentity(claims);
var identity = new ClaimsIdentity(claims, TestAuthenticationType);
var principal = new ClaimsPrincipal(identity);
var httpContext = new DefaultHttpContext
@@ -247,5 +327,8 @@ public class ClaimsIdentitySessionIsolationKeyProviderTests
this._httpContextAccessorMock.Setup(x => x.HttpContext).Returns(httpContext);
}
private static ClaimsPrincipal CreatePrincipal(params Claim[] claims)
=> new(new ClaimsIdentity(claims, TestAuthenticationType));
#endregion
}