From 57f54edde52fc70677e390fe66003ed24e7337db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ahmet=20=C3=87elik?= Date: Mon, 22 Jun 2026 17:44:46 +0300 Subject: [PATCH 1/2] Fix Razor Pages antiforgery validation when `NormalizeUserIdClaimIssuer` is enabled --- .../AspNetCore/Mvc/AbpAspNetCoreMvcModule.cs | 4 +++ .../AbpRazorPagesAntiforgeryConfiguration.cs | 30 +++++++++++++++++++ 2 files changed, 34 insertions(+) create mode 100644 framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/AntiForgery/AbpRazorPagesAntiforgeryConfiguration.cs diff --git a/framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/AbpAspNetCoreMvcModule.cs b/framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/AbpAspNetCoreMvcModule.cs index 9da7e7454f..5c90913579 100644 --- a/framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/AbpAspNetCoreMvcModule.cs +++ b/framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/AbpAspNetCoreMvcModule.cs @@ -240,6 +240,10 @@ public class AbpAspNetCoreMvcModule : AbpModule } }); + context.Services + .AddOptions() + .PostConfigure(AbpRazorPagesAntiforgeryConfiguration.Configure); + context.Services .AddOptions() .PostConfigure>((pagesOptions, abpLocOptions) => diff --git a/framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/AntiForgery/AbpRazorPagesAntiforgeryConfiguration.cs b/framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/AntiForgery/AbpRazorPagesAntiforgeryConfiguration.cs new file mode 100644 index 0000000000..23483d7849 --- /dev/null +++ b/framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/AntiForgery/AbpRazorPagesAntiforgeryConfiguration.cs @@ -0,0 +1,30 @@ +using System.Linq; +using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.Mvc.ApplicationModels; +using Microsoft.AspNetCore.Mvc.RazorPages; + +namespace Volo.Abp.AspNetCore.Mvc.AntiForgery; + +public static class AbpRazorPagesAntiforgeryConfiguration +{ + public static void Configure(RazorPagesOptions options) + { + options.Conventions.AddFolderApplicationModelConvention("/", ReplaceAntiforgeryFilter); + } + + private static void ReplaceAntiforgeryFilter(PageApplicationModel model) + { + for (var i = model.Filters.Count - 1; i >= 0; i--) + { + if (model.Filters[i] is AutoValidateAntiforgeryTokenAttribute) + { + model.Filters.RemoveAt(i); + } + } + + if (!model.Filters.Any(f => f is AbpAutoValidateAntiforgeryTokenAttribute)) + { + model.Filters.Add(new AbpAutoValidateAntiforgeryTokenAttribute()); + } + } +} From 2f7497e761c6bec5ef5ca4f849ef66c6da91d2f6 Mon Sep 17 00:00:00 2001 From: maliming Date: Tue, 23 Jun 2026 15:27:57 +0800 Subject: [PATCH 2/2] Fix antiforgery claim issuer mismatch by decorating IAntiforgery --- .../AspNetCore/Mvc/AbpAspNetCoreMvcModule.cs | 26 ++- ...AbpAntiForgeryClaimsPrincipalNormalizer.cs | 25 ++- .../Mvc/AntiForgery/AbpAntiforgery.cs | 101 +++++++++ .../AbpRazorPagesAntiforgeryConfiguration.cs | 30 --- ...dateAntiforgeryTokenAuthorizationFilter.cs | 22 +- .../AspNetCoreAbpAntiForgeryManager.cs | 21 +- ...AbpAntiForgeryClaimsPrincipalNormalizer.cs | 3 +- ...iForgeryClaimsPrincipalNormalizer_Tests.cs | 203 +++++++++--------- .../Mvc/AntiForgery/AbpAntiforgery_Tests.cs | 182 ++++++++++++++++ 9 files changed, 426 insertions(+), 187 deletions(-) create mode 100644 framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/AntiForgery/AbpAntiforgery.cs delete mode 100644 framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/AntiForgery/AbpRazorPagesAntiforgeryConfiguration.cs create mode 100644 framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/AntiForgery/AbpAntiforgery_Tests.cs diff --git a/framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/AbpAspNetCoreMvcModule.cs b/framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/AbpAspNetCoreMvcModule.cs index 5c90913579..0f80fe219d 100644 --- a/framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/AbpAspNetCoreMvcModule.cs +++ b/framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/AbpAspNetCoreMvcModule.cs @@ -1,3 +1,4 @@ +using Microsoft.AspNetCore.Antiforgery; using Microsoft.AspNetCore.Mvc; using Microsoft.AspNetCore.Mvc.ApplicationParts; using Microsoft.AspNetCore.Mvc.Filters; @@ -216,6 +217,27 @@ public class AbpAspNetCoreMvcModule : AbpModule }); ConfigureRouteBasedCulture(context); + DecorateAntiforgery(context); + } + + protected virtual void DecorateAntiforgery(ServiceConfigurationContext context) + { + // Wrap the registered IAntiforgery (DefaultAntiforgery from AddAntiforgery by default) with + // AbpAntiforgery so every antiforgery entry point goes through the claim normalization. + // Only an implementation-type registration is wrapped; a custom factory/instance registration of + // IAntiforgery is left as-is. + var descriptor = context.Services.LastOrDefault(d => d.ServiceType == typeof(IAntiforgery)); + if (descriptor?.ImplementationType == null) + { + return; + } + + context.Services.Replace(ServiceDescriptor.Describe( + typeof(IAntiforgery), + sp => new AbpAntiforgery( + (IAntiforgery)ActivatorUtilities.CreateInstance(sp, descriptor.ImplementationType), + sp.GetRequiredService>()), + descriptor.Lifetime)); } protected virtual void ConfigureRouteBasedCulture(ServiceConfigurationContext context) @@ -240,10 +262,6 @@ public class AbpAspNetCoreMvcModule : AbpModule } }); - context.Services - .AddOptions() - .PostConfigure(AbpRazorPagesAntiforgeryConfiguration.Configure); - context.Services .AddOptions() .PostConfigure>((pagesOptions, abpLocOptions) => 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 index fd1bcdda12..9275737245 100644 --- 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 @@ -1,7 +1,6 @@ using System; using System.Linq; using System.Security.Claims; -using System.Threading.Tasks; using Volo.Abp.DependencyInjection; using Volo.Abp.Security.Claims; @@ -13,20 +12,30 @@ public class AbpAntiForgeryClaimsPrincipalNormalizer : IAbpAntiForgeryClaimsPrin protected virtual string NormalizedIssuer => UserIdClaimIssuer; - public virtual Task NormalizeAsync(ClaimsPrincipal principal) + public virtual ClaimsPrincipal Normalize(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)); + normalized.AddIdentity(NormalizeIdentity(identity)); } - return Task.FromResult(normalized); + return normalized; + } + + protected virtual ClaimsIdentity NormalizeIdentity(ClaimsIdentity identity) + { + return new ClaimsIdentity( + identity.Claims.Select(NormalizeClaim), + identity.AuthenticationType, + identity.NameClaimType, + identity.RoleClaimType) + { + Actor = identity.Actor, + BootstrapContext = identity.BootstrapContext, + Label = identity.Label + }; } protected virtual Claim NormalizeClaim(Claim claim) diff --git a/framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/AntiForgery/AbpAntiforgery.cs b/framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/AntiForgery/AbpAntiforgery.cs new file mode 100644 index 0000000000..22c4900379 --- /dev/null +++ b/framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/AntiForgery/AbpAntiforgery.cs @@ -0,0 +1,101 @@ +using System; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Antiforgery; +using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Options; + +namespace Volo.Abp.AspNetCore.Mvc.AntiForgery; + +// Wraps the framework IAntiforgery so the antiforgery token's per-user identifier is computed against a +// normalized principal on every entry point (generation and validation, controllers and Razor Pages, +// ABP and built-in filters, cookie and bearer). This keeps the same user consistent across schemes whose +// user id claim carries a different issuer (e.g. "LOCAL AUTHORITY" for the Identity cookie vs. the token +// authority for a validated JWT or an OIDC cookie). +public class AbpAntiforgery : IAntiforgery +{ + protected IAntiforgery Inner { get; } + + protected AbpAntiForgeryOptions Options { get; } + + public AbpAntiforgery( + IAntiforgery inner, + IOptions options) + { + Inner = inner; + Options = options.Value; + } + + public virtual AntiforgeryTokenSet GetAndStoreTokens(HttpContext httpContext) + { + return WithNormalizedUser(httpContext, () => Inner.GetAndStoreTokens(httpContext)); + } + + public virtual AntiforgeryTokenSet GetTokens(HttpContext httpContext) + { + return WithNormalizedUser(httpContext, () => Inner.GetTokens(httpContext)); + } + + public virtual Task IsRequestValidAsync(HttpContext httpContext) + { + return WithNormalizedUserAsync(httpContext, () => Inner.IsRequestValidAsync(httpContext)); + } + + public virtual Task ValidateRequestAsync(HttpContext httpContext) + { + return WithNormalizedUserAsync(httpContext, async () => + { + await Inner.ValidateRequestAsync(httpContext); + return true; + }); + } + + public virtual void SetCookieTokenAndHeader(HttpContext httpContext) + { + WithNormalizedUser(httpContext, () => + { + Inner.SetCookieTokenAndHeader(httpContext); + return true; + }); + } + + protected virtual T WithNormalizedUser(HttpContext httpContext, Func action) + { + if (!Options.NormalizeUserIdClaimIssuer) + { + return action(); + } + + var normalizer = httpContext.RequestServices.GetRequiredService(); + var originalPrincipal = httpContext.User; + httpContext.User = normalizer.Normalize(originalPrincipal); + try + { + return action(); + } + finally + { + httpContext.User = originalPrincipal; + } + } + + protected virtual async Task WithNormalizedUserAsync(HttpContext httpContext, Func> action) + { + if (!Options.NormalizeUserIdClaimIssuer) + { + return await action(); + } + + var normalizer = httpContext.RequestServices.GetRequiredService(); + var originalPrincipal = httpContext.User; + httpContext.User = normalizer.Normalize(originalPrincipal); + try + { + return await action(); + } + finally + { + httpContext.User = originalPrincipal; + } + } +} diff --git a/framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/AntiForgery/AbpRazorPagesAntiforgeryConfiguration.cs b/framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/AntiForgery/AbpRazorPagesAntiforgeryConfiguration.cs deleted file mode 100644 index 23483d7849..0000000000 --- a/framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/AntiForgery/AbpRazorPagesAntiforgeryConfiguration.cs +++ /dev/null @@ -1,30 +0,0 @@ -using System.Linq; -using Microsoft.AspNetCore.Mvc; -using Microsoft.AspNetCore.Mvc.ApplicationModels; -using Microsoft.AspNetCore.Mvc.RazorPages; - -namespace Volo.Abp.AspNetCore.Mvc.AntiForgery; - -public static class AbpRazorPagesAntiforgeryConfiguration -{ - public static void Configure(RazorPagesOptions options) - { - options.Conventions.AddFolderApplicationModelConvention("/", ReplaceAntiforgeryFilter); - } - - private static void ReplaceAntiforgeryFilter(PageApplicationModel model) - { - for (var i = model.Filters.Count - 1; i >= 0; i--) - { - if (model.Filters[i] is AutoValidateAntiforgeryTokenAttribute) - { - model.Filters.RemoveAt(i); - } - } - - if (!model.Filters.Any(f => f is AbpAutoValidateAntiforgeryTokenAttribute)) - { - model.Filters.Add(new AbpAutoValidateAntiforgeryTokenAttribute()); - } - } -} 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 35f4eb684b..974db8a3fb 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,9 +4,7 @@ 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; @@ -42,33 +40,15 @@ 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(httpContext); + await _antiforgery.ValidateRequestAsync(context.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 40ab855a3e..b13fe68508 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,9 +1,7 @@ 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; @@ -37,23 +35,6 @@ public class AspNetCoreAbpAntiForgeryManager : IAbpAntiForgeryManager, ITransien public virtual string GenerateToken() { - 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; - } + return _antiforgery.GetAndStoreTokens(_httpContextAccessor.HttpContext!).RequestToken!; } } 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 index 2a01d526a1..bd2a581b5f 100644 --- 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 @@ -1,5 +1,4 @@ using System.Security.Claims; -using System.Threading.Tasks; namespace Volo.Abp.AspNetCore.Mvc.AntiForgery; @@ -7,5 +6,5 @@ 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); + ClaimsPrincipal Normalize(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 index 7d2cf2d8f3..7a2393586c 100644 --- 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 @@ -1,15 +1,9 @@ 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; @@ -25,7 +19,7 @@ public class AbpAntiForgeryClaimsPrincipalNormalizer_Tests private const string AntiForgeryCookieName = "AF"; [Fact] - public async Task Normalize_should_set_a_constant_issuer_on_user_identifier_claims_only() + public void 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"; @@ -39,7 +33,7 @@ public class AbpAntiForgeryClaimsPrincipalNormalizer_Tests }, "Identity.Application")); - var normalized = await new AbpAntiForgeryClaimsPrincipalNormalizer().NormalizeAsync(principal); + var normalized = new AbpAntiForgeryClaimsPrincipalNormalizer().Normalize(principal); normalized.FindFirst("sub")!.Issuer.ShouldBe(AbpAntiForgeryClaimsPrincipalNormalizer.UserIdClaimIssuer); normalized.FindFirst(ClaimTypes.NameIdentifier)!.Issuer.ShouldBe(AbpAntiForgeryClaimsPrincipalNormalizer.UserIdClaimIssuer); @@ -57,16 +51,55 @@ public class AbpAntiForgeryClaimsPrincipalNormalizer_Tests principal.FindFirst("sub")!.Issuer.ShouldBe(CookieIssuer); } + [Fact] + public void Normalize_should_preserve_identity_metadata() + { + var actor = new ClaimsIdentity(new[] { new Claim(AbpClaimTypes.UserId, "actor-id") }, "Actor"); + var principal = new ClaimsPrincipal(new ClaimsIdentity( + new[] { new Claim("sub", UserId, ClaimValueTypes.String, BearerIssuer) }, + "Identity.Application") + { + Actor = actor, + BootstrapContext = "raw-token", + Label = "my-label" + }); + + var normalized = new AbpAntiForgeryClaimsPrincipalNormalizer().Normalize(principal); + var normalizedIdentity = (ClaimsIdentity)normalized.Identity!; + + // identity metadata that the antiforgery claim uid does not use is still preserved on the copy + normalizedIdentity.Actor.ShouldBeSameAs(actor); + normalizedIdentity.BootstrapContext.ShouldBe("raw-token"); + normalizedIdentity.Label.ShouldBe("my-label"); + // and the user id claim issuer was still normalized + normalized.FindFirst("sub")!.Issuer.ShouldBe(AbpAntiForgeryClaimsPrincipalNormalizer.UserIdClaimIssuer); + } + + [Fact] + public async Task Token_should_validate_for_the_same_cookie_principal_through_the_decorator() + { + // The common server-rendered case: a token generated and validated for the same cookie principal. + // Guards that wrapping IAntiforgery does not break the basic flow every page POST relies on. + var (antiforgery, serviceProvider) = CreateDecoratedAntiforgery(normalize: true); + + var cookiePrincipal = CreatePrincipal("Identity.Application", CookieIssuer); + var (cookieToken, requestToken) = GenerateToken(antiforgery, serviceProvider, cookiePrincipal); + + var isValid = await ValidateAsync(antiforgery, serviceProvider, cookiePrincipal, cookieToken, requestToken); + + isValid.ShouldBeTrue(); + } + [Fact] public async Task Token_issued_under_one_scheme_should_validate_under_another_when_normalization_is_enabled() { - var (antiforgery, serviceProvider) = CreateAntiforgery(); + var (antiforgery, serviceProvider) = CreateDecoratedAntiforgery(normalize: true); var cookiePrincipal = CreatePrincipal("Identity.Application", CookieIssuer); - var (cookieToken, requestToken) = GenerateToken(antiforgery, serviceProvider, cookiePrincipal, normalize: true); + var (cookieToken, requestToken) = GenerateToken(antiforgery, serviceProvider, cookiePrincipal); var bearerPrincipal = CreatePrincipal("AuthenticationTypes.Federation", BearerIssuer); - var isValid = await ValidateAsync(antiforgery, bearerPrincipal, cookieToken, requestToken, normalize: true); + var isValid = await ValidateAsync(antiforgery, serviceProvider, bearerPrincipal, cookieToken, requestToken); isValid.ShouldBeTrue(); } @@ -74,106 +107,85 @@ public class AbpAntiForgeryClaimsPrincipalNormalizer_Tests [Fact] public async Task Token_issued_under_one_scheme_should_fail_under_another_when_normalization_is_disabled() { - var (antiforgery, serviceProvider) = CreateAntiforgery(); + var (antiforgery, serviceProvider) = CreateDecoratedAntiforgery(normalize: false); var cookiePrincipal = CreatePrincipal("Identity.Application", CookieIssuer); - var (cookieToken, requestToken) = GenerateToken(antiforgery, serviceProvider, cookiePrincipal, normalize: false); + var (cookieToken, requestToken) = GenerateToken(antiforgery, serviceProvider, cookiePrincipal); var bearerPrincipal = CreatePrincipal("AuthenticationTypes.Federation", BearerIssuer); - var isValid = await ValidateAsync(antiforgery, bearerPrincipal, cookieToken, requestToken, normalize: false); + var isValid = await ValidateAsync(antiforgery, serviceProvider, bearerPrincipal, cookieToken, requestToken); isValid.ShouldBeFalse(); } [Fact] - public async Task Token_should_validate_across_schemes_when_principal_has_both_sub_and_name_identifier() + public async Task Token_should_validate_when_the_cookie_principal_user_id_issuer_is_not_local_authority() { - // 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(); + // Tiered/OIDC templates back the cookie with an OIDC principal whose user id issuer is the token + // authority. Because the decorator normalizes both generation and validation (Razor Pages validate + // through the same decorated IAntiforgery), the per-user identifier still matches. + var (antiforgery, serviceProvider) = CreateDecoratedAntiforgery(normalize: true); - var cookiePrincipal = CreatePrincipalWithSubAndNameIdentifier("Identity.Application", CookieIssuer); - var (cookieToken, requestToken) = GenerateToken(antiforgery, serviceProvider, cookiePrincipal, normalize: true); + var oidcCookiePrincipal = CreatePrincipal("Identity.Application", BearerIssuer); + var (cookieToken, requestToken) = GenerateToken(antiforgery, serviceProvider, oidcCookiePrincipal); - var bearerPrincipal = CreatePrincipalWithSubAndNameIdentifier("AuthenticationTypes.Federation", BearerIssuer); - var isValid = await ValidateAsync(antiforgery, bearerPrincipal, cookieToken, requestToken, normalize: true); + var isValid = await ValidateAsync(antiforgery, serviceProvider, oidcCookiePrincipal, cookieToken, requestToken); isValid.ShouldBeTrue(); } [Fact] - public void Manager_should_not_require_the_normalizer_service_when_normalization_is_disabled() + public async Task Token_should_validate_across_schemes_when_principal_has_both_sub_and_name_identifier() { - 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(); + // The extractor picks "sub" before NameIdentifier and a principal can carry both, so the + // normalization must cover the claim actually picked. + var (antiforgery, serviceProvider) = CreateDecoratedAntiforgery(normalize: true); - 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 cookiePrincipal = CreatePrincipalWithSubAndNameIdentifier("Identity.Application", CookieIssuer); + var (cookieToken, requestToken) = GenerateToken(antiforgery, serviceProvider, cookiePrincipal); - var token = manager.GenerateToken(); + var bearerPrincipal = CreatePrincipalWithSubAndNameIdentifier("AuthenticationTypes.Federation", BearerIssuer); + var isValid = await ValidateAsync(antiforgery, serviceProvider, bearerPrincipal, cookieToken, requestToken); - token.ShouldNotBeNullOrEmpty(); + isValid.ShouldBeTrue(); } [Fact] - public async Task Filter_should_restore_the_original_principal_after_validation() + public async Task Decorator_should_restore_the_original_principal_after_each_call() { - 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 (antiforgery, serviceProvider) = CreateDecoratedAntiforgery(normalize: true); var originalPrincipal = CreatePrincipal("AuthenticationTypes.Federation", BearerIssuer); - var httpContext = new DefaultHttpContext - { - User = originalPrincipal, - RequestServices = serviceProvider - }; - httpContext.Request.Headers["Cookie"] = $"{AntiForgeryCookieName}=invalid"; + var httpContext = new DefaultHttpContext { User = originalPrincipal, RequestServices = serviceProvider }; - var filter = new AbpValidateAntiforgeryTokenAuthorizationFilter( - serviceProvider.GetRequiredService(), - serviceProvider.GetRequiredService(), - NullLogger.Instance); + antiforgery.GetAndStoreTokens(httpContext); - var actionContext = new ActionContext(httpContext, new RouteData(), new ActionDescriptor()); - var filterContext = new AuthorizationFilterContext(actionContext, new List { filter }); + httpContext.User.ShouldBeSameAs(originalPrincipal); + httpContext.User.FindFirst(AbpClaimTypes.UserId)!.Issuer.ShouldBe(BearerIssuer); - await filter.OnAuthorizationAsync(filterContext); + httpContext.Request.Headers["Cookie"] = $"{AntiForgeryCookieName}=invalid"; + await antiforgery.IsRequestValidAsync(httpContext); - // 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); } + [Fact] + public async Task Decorator_should_not_normalize_when_disabled() + { + var (antiforgery, serviceProvider) = CreateDecoratedAntiforgery(normalize: false); + + var principal = CreatePrincipal("Identity.Application", CookieIssuer); + var httpContext = new DefaultHttpContext { User = principal, RequestServices = serviceProvider }; + + antiforgery.GetAndStoreTokens(httpContext); + httpContext.User.ShouldBeSameAs(principal); + + httpContext.Request.Headers["Cookie"] = $"{AntiForgeryCookieName}=invalid"; + await antiforgery.IsRequestValidAsync(httpContext); + httpContext.User.ShouldBeSameAs(principal); + } + private static ClaimsPrincipal CreatePrincipal(string authenticationType, string userIdClaimIssuer) { return new ClaimsPrincipal(new ClaimsIdentity( @@ -201,7 +213,7 @@ public class AbpAntiForgeryClaimsPrincipalNormalizer_Tests AbpClaimTypes.Role)); } - private static (IAntiforgery antiforgery, IServiceProvider serviceProvider) CreateAntiforgery() + private static (IAntiforgery antiforgery, IServiceProvider serviceProvider) CreateDecoratedAntiforgery(bool normalize) { var services = new ServiceCollection(); services.AddLogging(); @@ -214,43 +226,30 @@ public class AbpAntiForgeryClaimsPrincipalNormalizer_Tests services.AddTransient(); var serviceProvider = services.BuildServiceProvider(); - return (serviceProvider.GetRequiredService(), serviceProvider); + + var antiforgery = new AbpAntiforgery( + serviceProvider.GetRequiredService(), + Microsoft.Extensions.Options.Options.Create(new AbpAntiForgeryOptions { NormalizeUserIdClaimIssuer = normalize })); + + return (antiforgery, serviceProvider); } private static (string cookieToken, string requestToken) GenerateToken( - IAntiforgery antiforgery, IServiceProvider serviceProvider, ClaimsPrincipal user, bool normalize) + IAntiforgery antiforgery, IServiceProvider serviceProvider, ClaimsPrincipal user) { 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); + var tokenSet = antiforgery.GetAndStoreTokens(httpContext); + return (ExtractCookieToken(httpContext), tokenSet.RequestToken!); } private static async Task ValidateAsync( - IAntiforgery antiforgery, ClaimsPrincipal user, string cookieToken, string requestToken, bool normalize) + IAntiforgery antiforgery, IServiceProvider serviceProvider, ClaimsPrincipal user, string cookieToken, string requestToken) { - var httpContext = new DefaultHttpContext { User = user }; + var httpContext = new DefaultHttpContext { User = user, RequestServices = serviceProvider }; 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; - } + return await antiforgery.IsRequestValidAsync(httpContext); } private static string ExtractCookieToken(HttpContext httpContext) diff --git a/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/AntiForgery/AbpAntiforgery_Tests.cs b/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/AntiForgery/AbpAntiforgery_Tests.cs new file mode 100644 index 0000000000..9ac20ed4d6 --- /dev/null +++ b/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/AntiForgery/AbpAntiforgery_Tests.cs @@ -0,0 +1,182 @@ +using System; +using System.Security.Claims; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Antiforgery; +using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.DependencyInjection; +using Shouldly; +using Volo.Abp.Security.Claims; +using Xunit; + +namespace Volo.Abp.AspNetCore.Mvc.AntiForgery; + +public class AbpAntiforgery_Tests +{ + private const string BearerIssuer = "https://localhost:44361/"; + private const string UserId = "3a0e6f1c-1111-2222-3333-444455556666"; + + [Fact] + public Task GetAndStoreTokens_should_normalize_the_user_and_restore_it() => + Should_normalize_then_restore( + (antiforgery, httpContext) => { antiforgery.GetAndStoreTokens(httpContext); return Task.CompletedTask; }, + inner => inner.UserSeenByGetAndStoreTokens); + + [Fact] + public Task GetTokens_should_normalize_the_user_and_restore_it() => + Should_normalize_then_restore( + (antiforgery, httpContext) => { antiforgery.GetTokens(httpContext); return Task.CompletedTask; }, + inner => inner.UserSeenByGetTokens); + + [Fact] + public Task IsRequestValidAsync_should_normalize_the_user_and_restore_it() => + Should_normalize_then_restore( + (antiforgery, httpContext) => antiforgery.IsRequestValidAsync(httpContext), + inner => inner.UserSeenByIsRequestValid); + + [Fact] + public Task ValidateRequestAsync_should_normalize_the_user_and_restore_it() => + Should_normalize_then_restore( + (antiforgery, httpContext) => antiforgery.ValidateRequestAsync(httpContext), + inner => inner.UserSeenByValidateRequest); + + [Fact] + public Task SetCookieTokenAndHeader_should_normalize_the_user_and_restore_it() => + Should_normalize_then_restore( + (antiforgery, httpContext) => { antiforgery.SetCookieTokenAndHeader(httpContext); return Task.CompletedTask; }, + inner => inner.UserSeenBySetCookieTokenAndHeader); + + [Fact] + public void Should_delegate_the_result_to_the_inner_antiforgery() + { + var inner = new RecordingAntiforgery(); + var antiforgery = new AbpAntiforgery(inner, CreateOptions(normalize: true)); + var httpContext = CreateHttpContext(CreatePrincipal(BearerIssuer), withNormalizer: true); + + var tokenSet = antiforgery.GetAndStoreTokens(httpContext); + + tokenSet.RequestToken.ShouldBe(RecordingAntiforgery.RequestToken); + tokenSet.CookieToken.ShouldBe(RecordingAntiforgery.CookieToken); + } + + [Fact] + public void Should_not_normalize_when_disabled() + { + var inner = new RecordingAntiforgery(); + var antiforgery = new AbpAntiforgery(inner, CreateOptions(normalize: false)); + var original = CreatePrincipal(BearerIssuer); + var httpContext = CreateHttpContext(original, withNormalizer: true); + + antiforgery.GetAndStoreTokens(httpContext); + + // the inner saw the original (un-normalized) principal + inner.UserSeenByGetAndStoreTokens.ShouldBeSameAs(original); + inner.UserSeenByGetAndStoreTokens!.FindFirst(AbpClaimTypes.UserId)!.Issuer.ShouldBe(BearerIssuer); + httpContext.User.ShouldBeSameAs(original); + } + + [Fact] + public void Should_not_resolve_the_normalizer_service_when_disabled() + { + // the normalizer is intentionally not registered; the disabled fast-path must not touch RequestServices + var inner = new RecordingAntiforgery(); + var antiforgery = new AbpAntiforgery(inner, CreateOptions(normalize: false)); + var httpContext = CreateHttpContext(CreatePrincipal(BearerIssuer), withNormalizer: false); + + Should.NotThrow(() => antiforgery.GetAndStoreTokens(httpContext)); + } + + private static async Task Should_normalize_then_restore( + Func invoke, + Func userSeenByInner) + { + var inner = new RecordingAntiforgery(); + var antiforgery = new AbpAntiforgery(inner, CreateOptions(normalize: true)); + var original = CreatePrincipal(BearerIssuer); + var httpContext = CreateHttpContext(original, withNormalizer: true); + + await invoke(antiforgery, httpContext); + + // the inner ran against the normalized principal + userSeenByInner(inner)!.FindFirst(AbpClaimTypes.UserId)!.Issuer + .ShouldBe(AbpAntiForgeryClaimsPrincipalNormalizer.UserIdClaimIssuer); + // the original principal is restored after the call + httpContext.User.ShouldBeSameAs(original); + } + + private static Microsoft.Extensions.Options.IOptions CreateOptions(bool normalize) + { + return Microsoft.Extensions.Options.Options.Create( + new AbpAntiForgeryOptions { NormalizeUserIdClaimIssuer = normalize }); + } + + private static HttpContext CreateHttpContext(ClaimsPrincipal user, bool withNormalizer) + { + var services = new ServiceCollection(); + if (withNormalizer) + { + services.AddTransient(); + } + + return new DefaultHttpContext + { + User = user, + RequestServices = services.BuildServiceProvider() + }; + } + + private static ClaimsPrincipal CreatePrincipal(string userIdClaimIssuer) + { + return new ClaimsPrincipal(new ClaimsIdentity( + new[] + { + new Claim(AbpClaimTypes.UserId, UserId, ClaimValueTypes.String, userIdClaimIssuer) + }, + "AuthenticationTypes.Federation")); + } + + private sealed class RecordingAntiforgery : IAntiforgery + { + public const string RequestToken = "test-request-token"; + public const string CookieToken = "test-cookie-token"; + + public ClaimsPrincipal? UserSeenByGetAndStoreTokens { get; private set; } + public ClaimsPrincipal? UserSeenByGetTokens { get; private set; } + public ClaimsPrincipal? UserSeenByIsRequestValid { get; private set; } + public ClaimsPrincipal? UserSeenByValidateRequest { get; private set; } + public ClaimsPrincipal? UserSeenBySetCookieTokenAndHeader { get; private set; } + + public AntiforgeryTokenSet GetAndStoreTokens(HttpContext httpContext) + { + UserSeenByGetAndStoreTokens = httpContext.User; + return CreateTokenSet(); + } + + public AntiforgeryTokenSet GetTokens(HttpContext httpContext) + { + UserSeenByGetTokens = httpContext.User; + return CreateTokenSet(); + } + + public Task IsRequestValidAsync(HttpContext httpContext) + { + UserSeenByIsRequestValid = httpContext.User; + return Task.FromResult(true); + } + + public Task ValidateRequestAsync(HttpContext httpContext) + { + UserSeenByValidateRequest = httpContext.User; + return Task.CompletedTask; + } + + public void SetCookieTokenAndHeader(HttpContext httpContext) + { + UserSeenBySetCookieTokenAndHeader = httpContext.User; + } + + private static AntiforgeryTokenSet CreateTokenSet() + { + return new AntiforgeryTokenSet(RequestToken, CookieToken, "RequestVerificationToken", "RequestVerificationToken"); + } + } +}