diff --git a/identity-server/CHANGELOG.md b/identity-server/CHANGELOG.md index ed72e6e49..8d390e94d 100644 --- a/identity-server/CHANGELOG.md +++ b/identity-server/CHANGELOG.md @@ -20,7 +20,9 @@ This makes the IdentityServer route names appear in OTel traces. - Support for custom parameters in the Authorize Redirect Uri by @bhazen - Adds a new `CustomParameters` property to `AuthorizeResponse` to support adding custom query parameters to the redirect uri. This will typically be used in conjunction with a custom `IAuthorizeResponseGenerator`. - +- Updated ASP.NET Identity package to persist session claims based on an interface @bhazen + - The ASP.NET Identity integration package now persists session claims based on `ISessionClaimsFilter.FilterToSessionClaimsAsync` which comes with a default implementation + - The new interface can be implemented to customize which session claims are persisted in non-default scenarios. ## Bug Fixes - Reject Pushed Authorization Requests with parameters duplicated in a JAR by @wcabus - Emit Telemetry Event for Introspection Requests for Valid Tokens by @bhazen diff --git a/identity-server/src/AspNetIdentity/ConfigureSecurityStampValidatorOptions.cs b/identity-server/src/AspNetIdentity/ConfigureSecurityStampValidatorOptions.cs new file mode 100644 index 000000000..a400ca7f6 --- /dev/null +++ b/identity-server/src/AspNetIdentity/ConfigureSecurityStampValidatorOptions.cs @@ -0,0 +1,12 @@ +// Copyright (c) Duende Software. All rights reserved. +// See LICENSE in the project root for license information. + +using Microsoft.AspNetCore.Identity; +using Microsoft.Extensions.Options; + +namespace Duende.IdentityServer.AspNetIdentity; + +public class ConfigureSecurityStampValidatorOptions(ISessionClaimsFilter sessionClaimsFilter) : IConfigureOptions +{ + public void Configure(SecurityStampValidatorOptions options) => options.OnRefreshingPrincipal = async context => await SecurityStampValidatorCallback.UpdatePrincipal(context, sessionClaimsFilter); +} diff --git a/identity-server/src/AspNetIdentity/DefaultSessionClaimsFilter.cs b/identity-server/src/AspNetIdentity/DefaultSessionClaimsFilter.cs new file mode 100644 index 000000000..0845f2e23 --- /dev/null +++ b/identity-server/src/AspNetIdentity/DefaultSessionClaimsFilter.cs @@ -0,0 +1,22 @@ +// Copyright (c) Duende Software. All rights reserved. +// See LICENSE in the project root for license information. + +using System.Security.Claims; +using Microsoft.AspNetCore.Identity; + +namespace Duende.IdentityServer.AspNetIdentity; + +public class DefaultSessionClaimsFilter : ISessionClaimsFilter +{ + /// + public Task> FilterToSessionClaimsAsync(SecurityStampRefreshingPrincipalContext context) + { + var newClaimTypes = context.NewPrincipal.Claims.Select(x => x.Type).ToArray(); + var currentClaimsToKeep = context.CurrentPrincipal.Claims.Where(x => !newClaimTypes.Contains(x.Type)).ToArray(); + + var id = context.NewPrincipal.Identities.First(); + id.AddClaims(currentClaimsToKeep); + + return Task.FromResult>(currentClaimsToKeep); + } +} diff --git a/identity-server/src/AspNetIdentity/ISessionClaimsFilter.cs b/identity-server/src/AspNetIdentity/ISessionClaimsFilter.cs new file mode 100644 index 000000000..898871a0a --- /dev/null +++ b/identity-server/src/AspNetIdentity/ISessionClaimsFilter.cs @@ -0,0 +1,21 @@ +// Copyright (c) Duende Software. All rights reserved. +// See LICENSE in the project root for license information. + +using System.Security.Claims; +using Microsoft.AspNetCore.Identity; + +namespace Duende.IdentityServer.AspNetIdentity; + +public interface ISessionClaimsFilter +{ + /// + /// Filters the claims in the given SecurityStampRefreshingPrincipalContext to those that should be kept for the session. + /// These claims are not claims persisted by ASP.NET Identity, but are typically captured and login time and need to be + /// persisted across updates to the ClaimsPrincipal in the + /// method. + /// + /// The SecurityStampRefreshingPrincipalContext + /// in the call to . + /// The claims of the ClaimsPrincipal which should be persisted for the session. + public Task> FilterToSessionClaimsAsync(SecurityStampRefreshingPrincipalContext context); +} diff --git a/identity-server/src/AspNetIdentity/IdentityServerBuilderExtensions.cs b/identity-server/src/AspNetIdentity/IdentityServerBuilderExtensions.cs index d70295b48..5c898556f 100644 --- a/identity-server/src/AspNetIdentity/IdentityServerBuilderExtensions.cs +++ b/identity-server/src/AspNetIdentity/IdentityServerBuilderExtensions.cs @@ -10,6 +10,7 @@ using Duende.IdentityServer.AspNetIdentity; using Duende.IdentityServer.Configuration; using Microsoft.AspNetCore.Authentication.Cookies; using Microsoft.AspNetCore.Identity; +using Microsoft.Extensions.DependencyInjection.Extensions; using Microsoft.Extensions.Options; namespace Microsoft.Extensions.DependencyInjection; @@ -20,7 +21,7 @@ namespace Microsoft.Extensions.DependencyInjection; public static class IdentityServerBuilderExtensions { /// - /// Configures IdentityServer to use the ASP.NET Identity implementations + /// Configures IdentityServer to use the ASP.NET Identity implementations /// of IUserClaimsPrincipalFactory, IResourceOwnerPasswordValidator, and IProfileService. /// Also configures some of ASP.NET Identity's options for use with IdentityServer (such as claim types to use /// and authentication cookie settings). @@ -41,10 +42,8 @@ public static class IdentityServerBuilderExtensions options.ClaimsIdentity.EmailClaimType = JwtClaimTypes.Email; }); - builder.Services.Configure(opts => - { - opts.OnRefreshingPrincipal = SecurityStampValidatorCallback.UpdatePrincipal; - }); + builder.Services.TryAddTransient(); + builder.Services.ConfigureOptions(); builder.Services.ConfigureApplicationCookie(options => { diff --git a/identity-server/src/AspNetIdentity/SecurityStampValidatorCallback.cs b/identity-server/src/AspNetIdentity/SecurityStampValidatorCallback.cs index 9cb4d6120..aeb823bc3 100644 --- a/identity-server/src/AspNetIdentity/SecurityStampValidatorCallback.cs +++ b/identity-server/src/AspNetIdentity/SecurityStampValidatorCallback.cs @@ -1,7 +1,6 @@ // Copyright (c) Duende Software. All rights reserved. // See LICENSE in the project root for license information. - using Microsoft.AspNetCore.Identity; namespace Duende.IdentityServer.AspNetIdentity; @@ -16,15 +15,20 @@ public static class SecurityStampValidatorCallback /// This is needed to preserve claims such as idp, auth_time, amr. /// /// The context. + /// Instance of session claims filter used to filter the claims from the ClaimsPrincipal to + /// those that are session claims which are not persisted by ASP.NET Identity and would otherwise bee lost when the principal + /// is updated. /// - public static Task UpdatePrincipal(SecurityStampRefreshingPrincipalContext context) + public static async Task UpdatePrincipal(SecurityStampRefreshingPrincipalContext context, ISessionClaimsFilter sessionClaimsFilter) { - var newClaimTypes = context.NewPrincipal.Claims.Select(x => x.Type).ToArray(); - var currentClaimsToKeep = context.CurrentPrincipal.Claims.Where(x => !newClaimTypes.Contains(x.Type)).ToArray(); + if (context.NewPrincipal == null || !context.NewPrincipal.Identities.Any()) + { + return; + } + + var currentClaimsToKeep = await sessionClaimsFilter.FilterToSessionClaimsAsync(context); var id = context.NewPrincipal.Identities.First(); id.AddClaims(currentClaimsToKeep); - - return Task.CompletedTask; } } diff --git a/identity-server/test/IdentityServer.UnitTests/AspNetIdentity/DefaultSessionClaimsFilterTests.cs b/identity-server/test/IdentityServer.UnitTests/AspNetIdentity/DefaultSessionClaimsFilterTests.cs new file mode 100644 index 000000000..1303be50b --- /dev/null +++ b/identity-server/test/IdentityServer.UnitTests/AspNetIdentity/DefaultSessionClaimsFilterTests.cs @@ -0,0 +1,95 @@ +// Copyright (c) Duende Software. All rights reserved. +// See LICENSE in the project root for license information. + +using System.Security.Claims; +using Duende.IdentityModel; +using Duende.IdentityServer.AspNetIdentity; +using Microsoft.AspNetCore.Identity; + +namespace IdentityServer.UnitTests.AspNetIdentity; + +public class DefaultSessionClaimsFilterTests +{ + [Fact] + public async Task FilterToSessionClaimsAsync_with_session_and_non_session_claims_should_filter_to_only_session_claims() + { + var claims = new[] + { + new Claim(JwtClaimTypes.AuthenticationMethod, "pwd"), + new Claim(JwtClaimTypes.IdentityProvider, "idp"), + new Claim(JwtClaimTypes.AuthenticationTime, "123456"), + new Claim("custom", "value"), + new Claim(ClaimTypes.Name, "bob") + }; + var currentPrincipal = new ClaimsPrincipal(new ClaimsIdentity(claims)); + var newPrincipal = new ClaimsPrincipal(new ClaimsIdentity([new Claim("custom", "value"), new Claim(ClaimTypes.Name, "bob")])); + var filter = new DefaultSessionClaimsFilter(); + var context = new SecurityStampRefreshingPrincipalContext() { NewPrincipal = newPrincipal, CurrentPrincipal = currentPrincipal }; + + var result = await filter.FilterToSessionClaimsAsync(context); + + var resultTypes = result.Select(c => c.Type).ToList(); + resultTypes.Count.ShouldBe(3); + resultTypes.ShouldContain(JwtClaimTypes.AuthenticationMethod); + resultTypes.ShouldContain(JwtClaimTypes.IdentityProvider); + resultTypes.ShouldContain(JwtClaimTypes.AuthenticationTime); + resultTypes.ShouldNotContain("custom"); + resultTypes.ShouldNotContain(ClaimTypes.Name); + } + + [Fact] + public async Task FilterToSessionClaimsAsync_with_only_session_claims_should_filter_to_session_claims() + { + var claims = new[] + { + new Claim(JwtClaimTypes.AuthenticationMethod, "pwd"), + new Claim(JwtClaimTypes.IdentityProvider, "idp"), + new Claim(JwtClaimTypes.AuthenticationTime, "123456") + }; + var currentPrincipal = new ClaimsPrincipal(new ClaimsIdentity(claims)); + var newPrincipal = new ClaimsPrincipal(new ClaimsIdentity()); + var filter = new DefaultSessionClaimsFilter(); + var context = new SecurityStampRefreshingPrincipalContext { NewPrincipal = newPrincipal, CurrentPrincipal = currentPrincipal }; + + var result = await filter.FilterToSessionClaimsAsync(context); + + result.Count.ShouldBe(3); + string[] expectClaimTypes = [ + JwtClaimTypes.AuthenticationMethod, + JwtClaimTypes.IdentityProvider, + JwtClaimTypes.AuthenticationTime + ]; + result.ShouldAllBe(c => expectClaimTypes.Contains(c.Type)); + } + + [Fact] + public async Task FilterToSessionClaimsAsync_with_no_session_claims_should_return_empty() + { + var claims = new[] + { + new Claim("custom", "value"), + new Claim(ClaimTypes.Name, "bob") + }; + var currentPrincipal = new ClaimsPrincipal(new ClaimsIdentity(claims)); + var newPrincipal = new ClaimsPrincipal(new ClaimsIdentity(claims)); + var filter = new DefaultSessionClaimsFilter(); + var context = new SecurityStampRefreshingPrincipalContext { NewPrincipal = newPrincipal, CurrentPrincipal = currentPrincipal }; + + var result = await filter.FilterToSessionClaimsAsync(context); + + result.ShouldBeEmpty(); + } + + [Fact] + public async Task FilterToSessionClaimsAsync_when_principal_has_no_claims_should_return_empty() + { + var newPrincipal = new ClaimsPrincipal(new ClaimsIdentity()); + var currentPrincipal = new ClaimsPrincipal(new ClaimsIdentity()); + var filter = new DefaultSessionClaimsFilter(); + var context = new SecurityStampRefreshingPrincipalContext { NewPrincipal = newPrincipal, CurrentPrincipal = currentPrincipal }; + + var result = await filter.FilterToSessionClaimsAsync(context); + + result.ShouldBeEmpty(); + } +} diff --git a/identity-server/test/IdentityServer.UnitTests/IdentityServer.UnitTests.csproj b/identity-server/test/IdentityServer.UnitTests/IdentityServer.UnitTests.csproj index 82b7fee93..276bafa66 100644 --- a/identity-server/test/IdentityServer.UnitTests/IdentityServer.UnitTests.csproj +++ b/identity-server/test/IdentityServer.UnitTests/IdentityServer.UnitTests.csproj @@ -26,6 +26,7 @@ + diff --git a/identity-server/test/IdentityServer.UnitTests/Licensing/v2/DiagnosticEntries/RegisteredImplementationsDiagnosticEntryTests.cs b/identity-server/test/IdentityServer.UnitTests/Licensing/v2/DiagnosticEntries/RegisteredImplementationsDiagnosticEntryTests.cs index ce9f813ee..473748123 100644 --- a/identity-server/test/IdentityServer.UnitTests/Licensing/v2/DiagnosticEntries/RegisteredImplementationsDiagnosticEntryTests.cs +++ b/identity-server/test/IdentityServer.UnitTests/Licensing/v2/DiagnosticEntries/RegisteredImplementationsDiagnosticEntryTests.cs @@ -111,7 +111,8 @@ public class RegisteredImplementationsDiagnosticEntryTests .SelectMany(assembly => assembly.GetExportedTypes()) .Where(type => type.IsInterface && type.IsPublic && type.Namespace != null && type.Namespace.StartsWith("Duende.IdentityServer") - && !type.Namespace.StartsWith("Duende.IdentityServer.EntityFramework")) + && !type.Namespace.StartsWith("Duende.IdentityServer.EntityFramework") + && !type.Namespace.StartsWith("Duende.IdentityServer.AspNetIdentity")) .Select(type => type); var subject = new RegisteredImplementationsDiagnosticEntry(new ServiceCollectionAccessor(new ServiceCollection())); var typesTrackedField = typeof(RegisteredImplementationsDiagnosticEntry)