diff --git a/dotnet/src/Microsoft.Agents.AI.Foundry.Hosting/HostedFoundryMemoryProviderScopes.cs b/dotnet/src/Microsoft.Agents.AI.Foundry.Hosting/HostedFoundryMemoryProviderScopes.cs index 31b3f9c3e4..f08064e0eb 100644 --- a/dotnet/src/Microsoft.Agents.AI.Foundry.Hosting/HostedFoundryMemoryProviderScopes.cs +++ b/dotnet/src/Microsoft.Agents.AI.Foundry.Hosting/HostedFoundryMemoryProviderScopes.cs @@ -44,18 +44,33 @@ public static class HostedFoundryMemoryProviderScopes session => new FoundryMemoryProvider.State(new FoundryMemoryProviderScope(GetRequiredHostedContext(session).ChatId)); /// - /// Returns a stateInitializer that scopes memories per (user, chat) pair, using - /// "{UserId}:{ChatId}" as the partition key. Use this when memories should be visible - /// only to the same user within the same conversation. + /// Returns a stateInitializer that scopes memories per (user, chat) pair, composing + /// and into a + /// single delimiter-safe partition key. Use this when memories should be visible only to the same + /// user within the same conversation. /// + /// + /// Both identity values are opaque strings that may contain any characters, including the : + /// delimiter. To keep the composite key injective (so two distinct (user, chat) pairs can never + /// collide), each part is escaped (\ becomes \\, then : becomes \:) before + /// being joined with a :: separator. + /// /// A delegate suitable for the stateInitializer argument of . public static Func 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)}")); }; + /// + /// Escapes special characters in a scope part so that distinct (user, chat) pairs produce distinct + /// composite scope keys. Backslashes are escaped first (\ becomes \\), then colons + /// (: becomes \:), ensuring the {user}::{chat} format is unambiguous. + /// + private static string EscapeScopePart(string part) => part.Replace("\\", "\\\\").Replace(":", "\\:"); + private static HostedSessionContext GetRequiredHostedContext(AgentSession? session) => session?.GetHostedContext() ?? throw new InvalidOperationException( diff --git a/dotnet/src/Microsoft.Agents.AI.Hosting.AspNetCore/ClaimsIdentitySessionIsolationKeyProvider.cs b/dotnet/src/Microsoft.Agents.AI.Hosting.AspNetCore/ClaimsIdentitySessionIsolationKeyProvider.cs index 3d61b9bea1..7cb1022bae 100644 --- a/dotnet/src/Microsoft.Agents.AI.Hosting.AspNetCore/ClaimsIdentitySessionIsolationKeyProvider.cs +++ b/dotnet/src/Microsoft.Agents.AI.Hosting.AspNetCore/ClaimsIdentitySessionIsolationKeyProvider.cs @@ -21,6 +21,18 @@ namespace Microsoft.Agents.AI.Hosting; /// from the ambient . /// /// +/// Security warning: The configured +/// must uniquely identify the principal within the served population. Display names, usernames, email +/// aliases, and other mutable or non-unique claims are unsafe 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 , a stable unique subject identifier +/// that is typically populated from the OpenID Connect sub claim via the default JWT inbound claim +/// mapping (note that this differs from Entra's object identifier oid claim; override +/// if you need oid or your +/// provider maps a different claim). +/// +/// /// If the is unavailable, the user is not authenticated, or the specified claim /// is missing, the provider returns . The consuming /// will then enforce strict or pass-through behavior based on its configuration. @@ -60,18 +72,24 @@ public class ClaimsIdentitySessionIsolationKeyProvider : SessionIsolationKeyProv /// The to monitor for cancellation requests. /// /// 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 if the claim - /// is not present or the HTTP context is unavailable. + /// configured claim type from the current user's identity, or if the HTTP + /// context is unavailable, the user is not authenticated, or the claim is not present. /// /// - /// This method retrieves the claim value from HttpContext.User.Claims. 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 rather than trusting claims on an + /// unauthenticated identity. The claim value is retrieved from HttpContext.User.Claims; if + /// multiple claims of the specified type exist, the first match is returned. /// public override ValueTask 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?)null); + } + + Claim? claim = user?.Claims.FirstOrDefault(c => c.Type == this._claimType); return new ValueTask(claim?.Value); } diff --git a/dotnet/src/Microsoft.Agents.AI.Hosting.AspNetCore/ClaimsIdentitySessionIsolationKeyProviderOptions.cs b/dotnet/src/Microsoft.Agents.AI.Hosting.AspNetCore/ClaimsIdentitySessionIsolationKeyProviderOptions.cs index 13845bc680..f929134a5a 100644 --- a/dotnet/src/Microsoft.Agents.AI.Hosting.AspNetCore/ClaimsIdentitySessionIsolationKeyProviderOptions.cs +++ b/dotnet/src/Microsoft.Agents.AI.Hosting.AspNetCore/ClaimsIdentitySessionIsolationKeyProviderOptions.cs @@ -14,17 +14,30 @@ public class ClaimsIdentitySessionIsolationKeyProviderOptions /// /// /// - /// Defaults to , which typically corresponds to - /// the user's name or unique identifier claim. + /// Defaults to , 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 sub claim via the + /// default JWT inbound claim mapping. Note that sub is distinct from Entra's object + /// identifier (oid) claim; if you require the oid claim, or your provider does not map + /// a unique identifier onto , override + /// with the appropriate claim type. + /// + /// + /// Security warning: The configured claim must uniquely identify the principal + /// within the served population. Display names ( + /// / ), usernames, email aliases, and other mutable or non-unique + /// claims are unsafe 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. /// /// /// Common alternatives include: /// - /// ClaimTypes.NameIdentifier — Stable user identifier - /// ClaimTypes.Email — Email address - /// Custom claim types specific to your authentication provider + /// A composite of tenant and subject identifiers — required for multi-tenant hosts where the subject is only unique per tenant + /// Custom claim types specific to your authentication provider, provided they are unique and stable /// /// /// - public string ClaimType { get; set; } = ClaimsIdentity.DefaultNameClaimType; + public string ClaimType { get; set; } = ClaimTypes.NameIdentifier; } diff --git a/dotnet/src/Microsoft.Agents.AI.Hosting.AspNetCore/ServiceCollectionExtensions.cs b/dotnet/src/Microsoft.Agents.AI.Hosting.AspNetCore/ServiceCollectionExtensions.cs index 0ff8d37371..8196d40e81 100644 --- a/dotnet/src/Microsoft.Agents.AI.Hosting.AspNetCore/ServiceCollectionExtensions.cs +++ b/dotnet/src/Microsoft.Agents.AI.Hosting.AspNetCore/ServiceCollectionExtensions.cs @@ -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 /// Optional configuration for the claims-based session isolation key provider. /// The so that additional calls can be chained. /// + /// /// This method requires to be registered in the service collection. /// Ensure that services.AddHttpContextAccessor() has been called before using this method. + /// + /// + /// When is not supplied, the isolation key is derived from the + /// claim, a stable unique subject identifier. For OpenID + /// Connect tokens (including Microsoft Entra ID), this is typically mapped from the sub claim + /// by the default JWT inbound claim mapping. Authentication schemes that do not project a unique + /// identifier onto (or hosts that require a different claim + /// such as Entra's oid) should override + /// ; otherwise the key may be + /// absent, which causes strict-mode session stores to fail. + /// + /// + /// Security warning: If you override + /// , 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 unsafe 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. + /// /// public static IServiceCollection UseClaimsBasedSessionIsolation( this IServiceCollection services, diff --git a/dotnet/tests/Microsoft.Agents.AI.Foundry.Hosting.UnitTests/HostedFoundryMemoryProviderScopesTests.cs b/dotnet/tests/Microsoft.Agents.AI.Foundry.Hosting.UnitTests/HostedFoundryMemoryProviderScopesTests.cs index 80883bd259..46f2c236d2 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Foundry.Hosting.UnitTests/HostedFoundryMemoryProviderScopesTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Foundry.Hosting.UnitTests/HostedFoundryMemoryProviderScopesTests.cs @@ -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] diff --git a/dotnet/tests/Microsoft.Agents.AI.Hosting.UnitTests/ClaimsIdentitySessionIsolationKeyProviderTests.cs b/dotnet/tests/Microsoft.Agents.AI.Hosting.UnitTests/ClaimsIdentitySessionIsolationKeyProviderTests.cs index e22feec1a9..6434d0cdae 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Hosting.UnitTests/ClaimsIdentitySessionIsolationKeyProviderTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Hosting.UnitTests/ClaimsIdentitySessionIsolationKeyProviderTests.cs @@ -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 _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); } + /// + /// 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. + /// + [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); + } + /// /// Verify that GetSessionIsolationKeyAsync uses custom claim type when specified. /// @@ -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); } + /// + /// 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. + /// + [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); + } + + /// + /// 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. + /// + [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 }