Return null from ClaimsPrincipal.Current if no thread principal. (#42622)

* Return null from ClaimsPrincipal.Current if no thread principal.

In the .NET Framework, the AppDomain's default PrincipalPolicy is
UnauthenticatedPrincipal. When falling back to the thread's principal,
this would no throw because a GenericPrincipal was returned.
If the policy was changed to NoPrincipal, ClaimsPrincipal.Current would
throw constructing the ClaimsPrincipal since there is no principal.
However that behavior was opt-in.

In .NET Core, the default policy was changed to NoPrincipal, which would
mean unless a principal was explicitly set, Current would always throw.

This changes the behavior to return null instead of throw for
NoPrincipal. This is to continue supporting the existing behavior in
.NET Core 3 of returning null if there is no principal, but continue
to allow falling back to the thread's principal.

* Code review feedback.

Co-authored-by: Stephen Toub <stoub@microsoft.com>
* Fix typos

Co-authored-by: Stephen Toub <stoub@microsoft.com>

diff --git a/src/libraries/System.Security.Claims/src/System/Security/Claims/ClaimsPrincipal.cs b/src/libraries/System.Security.Claims/src/System/Security/Claims/ClaimsPrincipal.cs
index 6762cdd..4e99666 100644 (file)
--- a/src/libraries/System.Security.Claims/src/System/Security/Claims/ClaimsPrincipal.cs
+++ b/src/libraries/System.Security.Claims/src/System/Security/Claims/ClaimsPrincipal.cs
@@ -27,9 +27,21 @@ namespace System.Security.Claims
         private static Func<IEnumerable<ClaimsIdentity>, ClaimsIdentity?> s_identitySelector = SelectPrimaryIdentity;
         private static Func<ClaimsPrincipal> s_principalSelector = ClaimsPrincipalSelector;
 
-        private static ClaimsPrincipal SelectClaimsPrincipal()
+        private static ClaimsPrincipal? SelectClaimsPrincipal()
         {
-            return (Thread.CurrentPrincipal is ClaimsPrincipal claimsPrincipal) ? claimsPrincipal : new ClaimsPrincipal(Thread.CurrentPrincipal!);
+            // Diverging behavior from .NET Framework: In Framework, the default PrincipalPolicy is
+            // UnauthenticatedPrincipal. In .NET Core, the default is NoPrincipal. .NET Framework
+            // would throw an ArgumentNullException when constructing the ClaimsPrincipal with a
+            // null principal from the thread if it were set to use NoPrincipal. In .NET Core, since
+            // NoPrincipal is the default, we return null instead of throw.
+
+            IPrincipal? threadPrincipal = Thread.CurrentPrincipal;
+
+            return threadPrincipal switch {
+                ClaimsPrincipal claimsPrincipal => claimsPrincipal,
+                not null => new ClaimsPrincipal(threadPrincipal),
+                null => null
+            };
         }
 
         protected ClaimsPrincipal(SerializationInfo info, StreamingContext context)

diff --git a/src/libraries/System.Security.Claims/tests/ClaimsPrincipalTests.cs b/src/libraries/System.Security.Claims/tests/ClaimsPrincipalTests.cs
index c28b770..c51d2dc 100644 (file)
--- a/src/libraries/System.Security.Claims/tests/ClaimsPrincipalTests.cs
+++ b/src/libraries/System.Security.Claims/tests/ClaimsPrincipalTests.cs
@@ -1,6 +1,7 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
+using System;
 using System.Collections.Generic;
 using System.IO;
 using System.Linq;
@@ -203,7 +204,7 @@ namespace System.Security.Claims
         }
 
         [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
-        public void Current_FallsBackToThread()
+        public void Current_FallsBackToThread_NoPrincipalPolicy()
         {
             RemoteExecutor.Invoke(() =>
             {
@@ -225,9 +226,19 @@ namespace System.Security.Claims
                 Assert.NotNull(current);
                 Assert.Equal("NonClaimsIdentity_Name", current.Identity.Name);
 
-                // match .NET Framework behavior by throwing ArgumentNullException when Thread.CurrentPrincipal is null
                 Thread.CurrentPrincipal = null;
-                Assert.Throws<ArgumentNullException>(() => ClaimsPrincipal.Current);
+                Assert.Null(ClaimsPrincipal.Current);
+            }).Dispose();
+        }
+
+        [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
+        public void Current_FallsBackToThread_UnauthenticatedPrincipalPolicy()
+        {
+            RemoteExecutor.Invoke(() =>
+            {
+                AppDomain.CurrentDomain.SetPrincipalPolicy(PrincipalPolicy.UnauthenticatedPrincipal);
+                Thread.CurrentPrincipal = null;
+                Assert.IsType<GenericPrincipal>(ClaimsPrincipal.Current);
             }).Dispose();
         }