Merge pull request #25752 from abpframework/maliming/current-principal-accessor-not-null

Return anonymous principal instead of null in ThreadCurrentPrincipalAccessor

framework/src/Volo.Abp.AspNetCore.Components.Web/Volo/Abp/AspNetCore/Components/Web/Security/AbpComponentsClaimsCache.cs

@@ -8,7 +8,7 @@ namespace Volo.Abp.AspNetCore.Components.Web.Security;
 
 public class AbpComponentsClaimsCache : IScopedDependency
 {
-    public ClaimsPrincipal Principal { get; private set; } = default!;
+    public ClaimsPrincipal Principal { get; private set; } = new ClaimsPrincipal(new ClaimsIdentity());
 
     private readonly AuthenticationStateProvider? _authenticationStateProvider;
 

framework/src/Volo.Abp.Security/Volo/Abp/Security/Claims/CurrentPrincipalAccessorBase.cs

@@ -8,7 +8,7 @@ public abstract class CurrentPrincipalAccessorBase : ICurrentPrincipalAccessor
 {
     public ClaimsPrincipal Principal => _currentPrincipal.Value ?? GetClaimsPrincipal();
 
-    private readonly AsyncLocal<ClaimsPrincipal> _currentPrincipal = new AsyncLocal<ClaimsPrincipal>();
+    private readonly AsyncLocal<ClaimsPrincipal?> _currentPrincipal = new AsyncLocal<ClaimsPrincipal?>();
 
     protected abstract ClaimsPrincipal GetClaimsPrincipal();
 
@@ -19,10 +19,10 @@ public abstract class CurrentPrincipalAccessorBase : ICurrentPrincipalAccessor
 
     private IDisposable SetCurrent(ClaimsPrincipal principal)
     {
-        var parent = Principal;
+        var parent = _currentPrincipal.Value;
         _currentPrincipal.Value = principal;
 
-        return new DisposeAction<ValueTuple<AsyncLocal<ClaimsPrincipal>, ClaimsPrincipal>>(static (state) =>
+        return new DisposeAction<ValueTuple<AsyncLocal<ClaimsPrincipal?>, ClaimsPrincipal?>>(static (state) =>
         {
             var (currentPrincipal, parent) = state;
             currentPrincipal.Value = parent;

framework/src/Volo.Abp.Security/Volo/Abp/Security/Claims/ThreadCurrentPrincipalAccessor.cs

@@ -8,6 +8,12 @@ public class ThreadCurrentPrincipalAccessor : CurrentPrincipalAccessorBase, ISin
 {
     protected override ClaimsPrincipal GetClaimsPrincipal()
     {
-        return (Thread.CurrentPrincipal as ClaimsPrincipal)!;
+        var principal = Thread.CurrentPrincipal;
+        if (principal == null)
+        {
+            return new ClaimsPrincipal(new ClaimsIdentity());
+        }
+
+        return principal as ClaimsPrincipal ?? new ClaimsPrincipal(principal);
     }
 }

framework/test/Volo.Abp.Security.Tests/Volo/Abp/Security/Claims/CurrentPrincipalAccessor_Tests.cs

@@ -1,5 +1,6 @@
 using System.Collections.Generic;
 using System.Security.Claims;
+using System.Threading;
 using Shouldly;
 using Volo.Abp.Testing;
 using Xunit;
@@ -30,20 +31,71 @@ public class CurrentPrincipalAccessor_Tests : AbpIntegratedTest<AbpSecurityTestM
                 new Claim(ClaimTypes.NameIdentifier,"654321")
             }));
 
-
+        var previousPrincipal = Thread.CurrentPrincipal;
-        _currentPrincipalAccessor.Principal.ShouldBe(null);
+        Thread.CurrentPrincipal = null;
-
+        try
-        using (_currentPrincipalAccessor.Change(claimsPrincipal))
         {
-            _currentPrincipalAccessor.Principal.ShouldBe(claimsPrincipal);
+            var anonymousPrincipal = _currentPrincipalAccessor.Principal;
+            anonymousPrincipal.ShouldNotBeNull();
+            anonymousPrincipal.Identity.ShouldNotBeNull();
+            anonymousPrincipal.Identity.IsAuthenticated.ShouldBeFalse();
 
-            using (_currentPrincipalAccessor.Change(claimsPrincipal2))
+            using (_currentPrincipalAccessor.Change(claimsPrincipal))
             {
-                _currentPrincipalAccessor.Principal.ShouldBe(claimsPrincipal2);
+                _currentPrincipalAccessor.Principal.ShouldBe(claimsPrincipal);
+
+                using (_currentPrincipalAccessor.Change(claimsPrincipal2))
+                {
+                    _currentPrincipalAccessor.Principal.ShouldBe(claimsPrincipal2);
+                }
+
+                _currentPrincipalAccessor.Principal.ShouldBe(claimsPrincipal);
             }
 
-            _currentPrincipalAccessor.Principal.ShouldBe(claimsPrincipal);
+            var currentPrincipal = _currentPrincipalAccessor.Principal;
+            currentPrincipal.ShouldNotBeNull();
+            currentPrincipal.Identity.ShouldNotBeNull();
+            currentPrincipal.Identity.IsAuthenticated.ShouldBeFalse();
+        }
+        finally
+        {
+            Thread.CurrentPrincipal = previousPrincipal;
+        }
+    }
+
+    [Fact]
+    public void Should_Reflect_Underlying_Source_After_Change_Scope_Disposed()
+    {
+        var accessor = new TestCurrentPrincipalAccessor();
+
+        var changedPrincipal = new ClaimsPrincipal(new ClaimsIdentity(new List<Claim>
+        {
+            new Claim(ClaimTypes.NameIdentifier, "123456")
+        }));
+
+        using (accessor.Change(changedPrincipal))
+        {
+            accessor.Principal.ShouldBe(changedPrincipal);
+        }
+
+        // Disposing the Change scope must not pin the accessor to the fallback principal;
+        // a principal that becomes available afterwards has to be reflected.
+        var sourcePrincipal = new ClaimsPrincipal(new ClaimsIdentity(new List<Claim>
+        {
+            new Claim(ClaimTypes.NameIdentifier, "654321")
+        }));
+        accessor.SourcePrincipal = sourcePrincipal;
+
+        accessor.Principal.ShouldBe(sourcePrincipal);
+    }
+
+    private class TestCurrentPrincipalAccessor : CurrentPrincipalAccessorBase
+    {
+        public ClaimsPrincipal? SourcePrincipal { get; set; }
+
+        protected override ClaimsPrincipal GetClaimsPrincipal()
+        {
+            return SourcePrincipal ?? new ClaimsPrincipal(new ClaimsIdentity());
         }
-        _currentPrincipalAccessor.Principal.ShouldBeNull();
     }
 }