diff --git a/framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/AntiForgery/AbpAntiForgeryClaimsPrincipalNormalizer.cs b/framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/AntiForgery/AbpAntiForgeryClaimsPrincipalNormalizer.cs new file mode 100644 index 0000000000..fd1bcdda12 --- /dev/null +++ b/framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/AntiForgery/AbpAntiForgeryClaimsPrincipalNormalizer.cs @@ -0,0 +1,57 @@ +using System; +using System.Linq; +using System.Security.Claims; +using System.Threading.Tasks; +using Volo.Abp.DependencyInjection; +using Volo.Abp.Security.Claims; + +namespace Volo.Abp.AspNetCore.Mvc.AntiForgery; + +public class AbpAntiForgeryClaimsPrincipalNormalizer : IAbpAntiForgeryClaimsPrincipalNormalizer, ITransientDependency +{ + public const string UserIdClaimIssuer = "AbpAntiForgery"; + + protected virtual string NormalizedIssuer => UserIdClaimIssuer; + + public virtual Task NormalizeAsync(ClaimsPrincipal principal) + { + var normalized = new ClaimsPrincipal(); + + foreach (var identity in principal.Identities) + { + normalized.AddIdentity(new ClaimsIdentity( + identity.Claims.Select(NormalizeClaim), + identity.AuthenticationType, + identity.NameClaimType, + identity.RoleClaimType)); + } + + return Task.FromResult(normalized); + } + + protected virtual Claim NormalizeClaim(Claim claim) + { + var newClaim = new Claim( + claim.Type, + claim.Value, + claim.ValueType, + IsUserIdentifierClaim(claim.Type) ? NormalizedIssuer : claim.Issuer, + claim.OriginalIssuer); + + foreach (var property in claim.Properties) + { + newClaim.Properties[property.Key] = property.Value; + } + + return newClaim; + } + + // The claim types DefaultClaimUidExtractor inspects, in priority order, to build the antiforgery user id. + protected virtual bool IsUserIdentifierClaim(string claimType) + { + return string.Equals(claimType, AbpClaimTypes.UserId, StringComparison.Ordinal) || + string.Equals(claimType, "sub", StringComparison.Ordinal) || + string.Equals(claimType, ClaimTypes.NameIdentifier, StringComparison.Ordinal) || + string.Equals(claimType, ClaimTypes.Upn, StringComparison.Ordinal); + } +} diff --git a/framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/AntiForgery/AbpAntiForgeryOptions.cs b/framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/AntiForgery/AbpAntiForgeryOptions.cs index 0c2cf8884c..f57a38d57a 100644 --- a/framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/AntiForgery/AbpAntiForgeryOptions.cs +++ b/framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/AntiForgery/AbpAntiForgeryOptions.cs @@ -24,6 +24,13 @@ public class AbpAntiForgeryOptions /// public bool AutoValidate { get; set; } = true; + /// + /// Normalizes the user id claim issuer before generating/validating the antiforgery token, so the + /// same user produces the same token identifier under both cookie and bearer authentication. + /// Default value: true. + /// + public bool NormalizeUserIdClaimIssuer { get; set; } = true; + /// /// A predicate to filter types to auto-validate. /// Return true to select the type to validate. diff --git a/framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/AntiForgery/AbpValidateAntiforgeryTokenAuthorizationFilter.cs b/framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/AntiForgery/AbpValidateAntiforgeryTokenAuthorizationFilter.cs index 974db8a3fb..35f4eb684b 100644 --- a/framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/AntiForgery/AbpValidateAntiforgeryTokenAuthorizationFilter.cs +++ b/framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/AntiForgery/AbpValidateAntiforgeryTokenAuthorizationFilter.cs @@ -4,7 +4,9 @@ using Microsoft.AspNetCore.Antiforgery; using Microsoft.AspNetCore.Mvc; using Microsoft.AspNetCore.Mvc.Filters; using Microsoft.AspNetCore.Mvc.ViewFeatures; +using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; using Volo.Abp.DependencyInjection; namespace Volo.Abp.AspNetCore.Mvc.AntiForgery; @@ -40,15 +42,33 @@ public class AbpValidateAntiforgeryTokenAuthorizationFilter : IAsyncAuthorizatio if (ShouldValidate(context)) { + var httpContext = context.HttpContext; + var normalizeUserIdClaimIssuer = httpContext.RequestServices + .GetRequiredService>().Value.NormalizeUserIdClaimIssuer; + + var originalPrincipal = httpContext.User; + if (normalizeUserIdClaimIssuer) + { + var normalizer = httpContext.RequestServices.GetRequiredService(); + httpContext.User = await normalizer.NormalizeAsync(originalPrincipal); + } + try { - await _antiforgery.ValidateRequestAsync(context.HttpContext); + await _antiforgery.ValidateRequestAsync(httpContext); } catch (AntiforgeryValidationException exception) { _logger.LogWarning(exception.Message, exception); context.Result = new AntiforgeryValidationFailedResult(); } + finally + { + if (normalizeUserIdClaimIssuer) + { + httpContext.User = originalPrincipal; + } + } } } diff --git a/framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/AntiForgery/AspNetCoreAbpAntiForgeryManager.cs b/framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/AntiForgery/AspNetCoreAbpAntiForgeryManager.cs index b13fe68508..40ab855a3e 100644 --- a/framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/AntiForgery/AspNetCoreAbpAntiForgeryManager.cs +++ b/framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/AntiForgery/AspNetCoreAbpAntiForgeryManager.cs @@ -1,7 +1,9 @@ using Microsoft.AspNetCore.Antiforgery; using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Options; using Volo.Abp.DependencyInjection; +using Volo.Abp.Threading; namespace Volo.Abp.AspNetCore.Mvc.AntiForgery; @@ -35,6 +37,23 @@ public class AspNetCoreAbpAntiForgeryManager : IAbpAntiForgeryManager, ITransien public virtual string GenerateToken() { - return _antiforgery.GetAndStoreTokens(_httpContextAccessor.HttpContext!).RequestToken!; + var httpContext = _httpContextAccessor.HttpContext!; + + if (!Options.NormalizeUserIdClaimIssuer) + { + return _antiforgery.GetAndStoreTokens(httpContext).RequestToken!; + } + + var normalizer = httpContext.RequestServices.GetRequiredService(); + var originalPrincipal = httpContext.User; + httpContext.User = AsyncHelper.RunSync(() => normalizer.NormalizeAsync(originalPrincipal)); + try + { + return _antiforgery.GetAndStoreTokens(httpContext).RequestToken!; + } + finally + { + httpContext.User = originalPrincipal; + } } } diff --git a/framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/AntiForgery/IAbpAntiForgeryClaimsPrincipalNormalizer.cs b/framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/AntiForgery/IAbpAntiForgeryClaimsPrincipalNormalizer.cs new file mode 100644 index 0000000000..2a01d526a1 --- /dev/null +++ b/framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/AntiForgery/IAbpAntiForgeryClaimsPrincipalNormalizer.cs @@ -0,0 +1,11 @@ +using System.Security.Claims; +using System.Threading.Tasks; + +namespace Volo.Abp.AspNetCore.Mvc.AntiForgery; + +public interface IAbpAntiForgeryClaimsPrincipalNormalizer +{ + // Returns a copy of the principal whose user identifier claims carry a stable issuer, so the + // antiforgery token's per-user identifier is the same across authentication schemes. + Task NormalizeAsync(ClaimsPrincipal principal); +} diff --git a/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/AntiForgery/AbpAntiForgeryClaimsPrincipalNormalizer_Tests.cs b/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/AntiForgery/AbpAntiForgeryClaimsPrincipalNormalizer_Tests.cs new file mode 100644 index 0000000000..7d2cf2d8f3 --- /dev/null +++ b/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/AntiForgery/AbpAntiForgeryClaimsPrincipalNormalizer_Tests.cs @@ -0,0 +1,264 @@ +using System; +using System.Collections.Generic; +using System.Security.Claims; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Antiforgery; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.Mvc.Abstractions; +using Microsoft.AspNetCore.Mvc.Filters; +using Microsoft.AspNetCore.Routing; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging.Abstractions; +using Shouldly; +using Volo.Abp.Security.Claims; +using Xunit; + +namespace Volo.Abp.AspNetCore.Mvc.AntiForgery; + +public class AbpAntiForgeryClaimsPrincipalNormalizer_Tests +{ + private const string CookieIssuer = "LOCAL AUTHORITY"; + private const string BearerIssuer = "https://localhost:44361/"; + private const string UserId = "3a0e6f1c-1111-2222-3333-444455556666"; + private const string AntiForgeryHeaderName = "RequestVerificationToken"; + private const string AntiForgeryCookieName = "AF"; + + [Fact] + public async Task Normalize_should_set_a_constant_issuer_on_user_identifier_claims_only() + { + var usernameClaim = new Claim("preferred_username", "admin", ClaimValueTypes.String, CookieIssuer); + usernameClaim.Properties["test-property"] = "test-value"; + + var principal = new ClaimsPrincipal(new ClaimsIdentity( + new[] + { + new Claim("sub", UserId, ClaimValueTypes.String, CookieIssuer), + new Claim(ClaimTypes.NameIdentifier, UserId, ClaimValueTypes.String, CookieIssuer), + usernameClaim + }, + "Identity.Application")); + + var normalized = await new AbpAntiForgeryClaimsPrincipalNormalizer().NormalizeAsync(principal); + + normalized.FindFirst("sub")!.Issuer.ShouldBe(AbpAntiForgeryClaimsPrincipalNormalizer.UserIdClaimIssuer); + normalized.FindFirst(ClaimTypes.NameIdentifier)!.Issuer.ShouldBe(AbpAntiForgeryClaimsPrincipalNormalizer.UserIdClaimIssuer); + + // value and OriginalIssuer are kept; only Issuer changes + normalized.FindFirst("sub")!.Value.ShouldBe(UserId); + normalized.FindFirst("sub")!.OriginalIssuer.ShouldBe(CookieIssuer); + + // non-identifier claims and their properties are untouched + var normalizedUsername = normalized.FindFirst("preferred_username")!; + normalizedUsername.Issuer.ShouldBe(CookieIssuer); + normalizedUsername.Properties["test-property"].ShouldBe("test-value"); + + // the original principal is not mutated + principal.FindFirst("sub")!.Issuer.ShouldBe(CookieIssuer); + } + + [Fact] + public async Task Token_issued_under_one_scheme_should_validate_under_another_when_normalization_is_enabled() + { + var (antiforgery, serviceProvider) = CreateAntiforgery(); + + var cookiePrincipal = CreatePrincipal("Identity.Application", CookieIssuer); + var (cookieToken, requestToken) = GenerateToken(antiforgery, serviceProvider, cookiePrincipal, normalize: true); + + var bearerPrincipal = CreatePrincipal("AuthenticationTypes.Federation", BearerIssuer); + var isValid = await ValidateAsync(antiforgery, bearerPrincipal, cookieToken, requestToken, normalize: true); + + isValid.ShouldBeTrue(); + } + + [Fact] + public async Task Token_issued_under_one_scheme_should_fail_under_another_when_normalization_is_disabled() + { + var (antiforgery, serviceProvider) = CreateAntiforgery(); + + var cookiePrincipal = CreatePrincipal("Identity.Application", CookieIssuer); + var (cookieToken, requestToken) = GenerateToken(antiforgery, serviceProvider, cookiePrincipal, normalize: false); + + var bearerPrincipal = CreatePrincipal("AuthenticationTypes.Federation", BearerIssuer); + var isValid = await ValidateAsync(antiforgery, bearerPrincipal, cookieToken, requestToken, normalize: false); + + isValid.ShouldBeFalse(); + } + + [Fact] + public async Task Token_should_validate_across_schemes_when_principal_has_both_sub_and_name_identifier() + { + // The extractor picks "sub" before NameIdentifier and a principal can carry both, so the + // normalization must cover the claim actually picked. + var (antiforgery, serviceProvider) = CreateAntiforgery(); + + var cookiePrincipal = CreatePrincipalWithSubAndNameIdentifier("Identity.Application", CookieIssuer); + var (cookieToken, requestToken) = GenerateToken(antiforgery, serviceProvider, cookiePrincipal, normalize: true); + + var bearerPrincipal = CreatePrincipalWithSubAndNameIdentifier("AuthenticationTypes.Federation", BearerIssuer); + var isValid = await ValidateAsync(antiforgery, bearerPrincipal, cookieToken, requestToken, normalize: true); + + isValid.ShouldBeTrue(); + } + + [Fact] + public void Manager_should_not_require_the_normalizer_service_when_normalization_is_disabled() + { + var services = new ServiceCollection(); + services.AddLogging(); + services.AddDataProtection(); + services.AddAntiforgery(options => + { + options.Cookie.Name = AntiForgeryCookieName; + options.HeaderName = AntiForgeryHeaderName; + }); + // IAbpAntiForgeryClaimsPrincipalNormalizer is intentionally not registered. + var serviceProvider = services.BuildServiceProvider(); + + var httpContext = new DefaultHttpContext + { + User = CreatePrincipal("Identity.Application", CookieIssuer), + RequestServices = serviceProvider + }; + var manager = new AspNetCoreAbpAntiForgeryManager( + serviceProvider.GetRequiredService(), + new HttpContextAccessor { HttpContext = httpContext }, + Microsoft.Extensions.Options.Options.Create(new AbpAntiForgeryOptions { NormalizeUserIdClaimIssuer = false })); + + var token = manager.GenerateToken(); + + token.ShouldNotBeNullOrEmpty(); + } + + [Fact] + public async Task Filter_should_restore_the_original_principal_after_validation() + { + var services = new ServiceCollection(); + services.AddLogging(); + services.AddDataProtection(); + services.AddAntiforgery(options => + { + options.Cookie.Name = AntiForgeryCookieName; + options.HeaderName = AntiForgeryHeaderName; + }); + services.AddTransient(); + services.AddTransient(); + services.Configure(options => + { + options.TokenCookie.Name = AntiForgeryCookieName; + options.AuthCookieSchemaName = null; // let ShouldValidate rely on the antiforgery cookie only + options.NormalizeUserIdClaimIssuer = true; + }); + var serviceProvider = services.BuildServiceProvider(); + + var originalPrincipal = CreatePrincipal("AuthenticationTypes.Federation", BearerIssuer); + var httpContext = new DefaultHttpContext + { + User = originalPrincipal, + RequestServices = serviceProvider + }; + httpContext.Request.Headers["Cookie"] = $"{AntiForgeryCookieName}=invalid"; + + var filter = new AbpValidateAntiforgeryTokenAuthorizationFilter( + serviceProvider.GetRequiredService(), + serviceProvider.GetRequiredService(), + NullLogger.Instance); + + var actionContext = new ActionContext(httpContext, new RouteData(), new ActionDescriptor()); + var filterContext = new AuthorizationFilterContext(actionContext, new List { filter }); + + await filter.OnAuthorizationAsync(filterContext); + + // The validation ran (and failed for the invalid token), but the original principal must be restored. + httpContext.User.ShouldBeSameAs(originalPrincipal); + httpContext.User.FindFirst(AbpClaimTypes.UserId)!.Issuer.ShouldBe(BearerIssuer); + } + + private static ClaimsPrincipal CreatePrincipal(string authenticationType, string userIdClaimIssuer) + { + return new ClaimsPrincipal(new ClaimsIdentity( + new[] + { + new Claim(AbpClaimTypes.UserId, UserId, ClaimValueTypes.String, userIdClaimIssuer), + new Claim("preferred_username", "admin", ClaimValueTypes.String, userIdClaimIssuer) + }, + authenticationType, + "preferred_username", + AbpClaimTypes.Role)); + } + + private static ClaimsPrincipal CreatePrincipalWithSubAndNameIdentifier(string authenticationType, string issuer) + { + return new ClaimsPrincipal(new ClaimsIdentity( + new[] + { + new Claim("sub", UserId, ClaimValueTypes.String, issuer), + new Claim(ClaimTypes.NameIdentifier, UserId, ClaimValueTypes.String, issuer), + new Claim("preferred_username", "admin", ClaimValueTypes.String, issuer) + }, + authenticationType, + "preferred_username", + AbpClaimTypes.Role)); + } + + private static (IAntiforgery antiforgery, IServiceProvider serviceProvider) CreateAntiforgery() + { + var services = new ServiceCollection(); + services.AddLogging(); + services.AddDataProtection(); + services.AddAntiforgery(options => + { + options.Cookie.Name = AntiForgeryCookieName; + options.HeaderName = AntiForgeryHeaderName; + }); + services.AddTransient(); + + var serviceProvider = services.BuildServiceProvider(); + return (serviceProvider.GetRequiredService(), serviceProvider); + } + + private static (string cookieToken, string requestToken) GenerateToken( + IAntiforgery antiforgery, IServiceProvider serviceProvider, ClaimsPrincipal user, bool normalize) + { + var httpContext = new DefaultHttpContext { User = user, RequestServices = serviceProvider }; + var manager = new AspNetCoreAbpAntiForgeryManager( + antiforgery, + new HttpContextAccessor { HttpContext = httpContext }, + Microsoft.Extensions.Options.Options.Create(new AbpAntiForgeryOptions { NormalizeUserIdClaimIssuer = normalize })); + + var requestToken = manager.GenerateToken(); + return (ExtractCookieToken(httpContext), requestToken); + } + + private static async Task ValidateAsync( + IAntiforgery antiforgery, ClaimsPrincipal user, string cookieToken, string requestToken, bool normalize) + { + var httpContext = new DefaultHttpContext { User = user }; + httpContext.Request.Headers["Cookie"] = $"{AntiForgeryCookieName}={cookieToken}"; + httpContext.Request.Headers[AntiForgeryHeaderName] = requestToken; + + if (normalize) + { + httpContext.User = await new AbpAntiForgeryClaimsPrincipalNormalizer().NormalizeAsync(httpContext.User); + } + + try + { + await antiforgery.ValidateRequestAsync(httpContext); + return true; + } + catch (AntiforgeryValidationException) + { + return false; + } + } + + private static string ExtractCookieToken(HttpContext httpContext) + { + var setCookie = httpContext.Response.Headers.SetCookie.ToString(); + var prefix = AntiForgeryCookieName + "="; + var start = setCookie.IndexOf(prefix, StringComparison.Ordinal) + prefix.Length; + var end = setCookie.IndexOf(';', start); + return end < 0 ? setCookie.Substring(start) : setCookie.Substring(start, end - start); + } +}