From d48d565cc886e1a96840a0f6bb46b70e6805a749 Mon Sep 17 00:00:00 2001 From: Marco Rossignoli Date: Mon, 10 Feb 2020 20:33:18 +0100 Subject: [PATCH 1/3] fix principal policy for new threads --- .../src/System/Threading/Thread.cs | 2 +- .../tests/ThreadTests.cs | 90 ++++++++++++++++++- 2 files changed, 87 insertions(+), 5 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Threading/Thread.cs b/src/libraries/System.Private.CoreLib/src/System/Threading/Thread.cs index ca1d94f6fdf291..13866bbbea99f8 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Threading/Thread.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Threading/Thread.cs @@ -133,7 +133,7 @@ public static IPrincipal? CurrentPrincipal { get { - if (s_asyncLocalPrincipal is null) + if (s_asyncLocalPrincipal is null || s_asyncLocalPrincipal.Value is null) { CurrentPrincipal = AppDomain.CurrentDomain.GetThreadPrincipal(); } diff --git a/src/libraries/System.Threading.Thread/tests/ThreadTests.cs b/src/libraries/System.Threading.Thread/tests/ThreadTests.cs index 156487c6e16048..10b1e12d1545eb 100644 --- a/src/libraries/System.Threading.Thread/tests/ThreadTests.cs +++ b/src/libraries/System.Threading.Thread/tests/ThreadTests.cs @@ -327,7 +327,8 @@ public static void CurrentCultureTest_DifferentThread() CultureInfo uiCulture = (CultureInfo)CultureInfo.CurrentCulture.Clone(); ExceptionDispatchInfo exceptionFromThread = null; - var t = new Thread(() => { + var t = new Thread(() => + { try { Assert.Same(culture, Thread.CurrentThread.CurrentCulture); @@ -432,11 +433,12 @@ public static void CurrentPrincipalContextFlowTest() { Thread.CurrentPrincipal = new ClaimsPrincipal(); - await Task.Run(async() => { + await Task.Run(async () => + { Assert.IsType(Thread.CurrentPrincipal); - await Task.Run(async() => + await Task.Run(async () => { Assert.IsType(Thread.CurrentPrincipal); @@ -465,7 +467,7 @@ public static void CurrentPrincipalContextFlowTest_NotFlow() Thread.CurrentPrincipal = new ClaimsPrincipal(); Task task; - using(ExecutionContext.SuppressFlow()) + using (ExecutionContext.SuppressFlow()) { Assert.True(ExecutionContext.IsFlowSuppressed()); @@ -1111,6 +1113,86 @@ public static void WindowsPrincipalPolicyTest_Windows() }).Dispose(); } + + [Fact] + [PlatformSpecific(TestPlatforms.Windows)] + public static void WindowsPrincipalPolicyTest_Windows_NewThreads() + { + RemoteExecutor.Invoke(() => + { + AppDomain.CurrentDomain.SetPrincipalPolicy(PrincipalPolicy.WindowsPrincipal); + + IPrincipal currentPrincipal = Thread.CurrentPrincipal; + + Assert.NotNull(currentPrincipal); + Assert.True(currentPrincipal.Identity.IsAuthenticated); + + var first = new Thread(CheckPrincipal); + first.Start(currentPrincipal); + first.Join(); + + var second = new Thread(CheckPrincipal); + second.Start(currentPrincipal); + second.Join(); + }).Dispose(); + + static void CheckPrincipal(object principal) + { + Assert.True(Thread.CurrentPrincipal.Identity.IsAuthenticated); + Assert.NotNull(Thread.CurrentPrincipal); + Assert.Equal((IPrincipal)principal, Thread.CurrentPrincipal); + } + } + + [Fact] + [PlatformSpecific(TestPlatforms.Windows)] + public static void NoPrincipalPolicyTest_Windows_NewThreads() + { + RemoteExecutor.Invoke(() => + { + AppDomain.CurrentDomain.SetPrincipalPolicy(PrincipalPolicy.NoPrincipal); + + Assert.Null(Thread.CurrentPrincipal); + + var first = new Thread(() => Assert.Null(Thread.CurrentPrincipal)); + first.Start(); + first.Join(); + + var second = new Thread(() => Assert.Null(Thread.CurrentPrincipal)); + second.Start(); + second.Join(); + }).Dispose(); + } + + [Fact] + [PlatformSpecific(TestPlatforms.Windows)] + public static void NoPrincipalToWindowsPrincipalPolicyTest_Windows_NewThreads() + { + RemoteExecutor.Invoke(() => + { + AppDomain.CurrentDomain.SetPrincipalPolicy(PrincipalPolicy.NoPrincipal); + + Assert.Null(Thread.CurrentPrincipal); + + var first = new Thread(() => Assert.Null(Thread.CurrentPrincipal)); + first.Start(); + first.Join(); + + AppDomain.CurrentDomain.SetPrincipalPolicy(PrincipalPolicy.WindowsPrincipal); + + var second = new Thread(CheckPrincipal); + second.Start(Thread.CurrentPrincipal); + second.Join(); + }).Dispose(); + + static void CheckPrincipal(object principal) + { + Assert.True(Thread.CurrentPrincipal.Identity.IsAuthenticated); + Assert.NotNull(Thread.CurrentPrincipal); + Assert.Equal((IPrincipal)principal, Thread.CurrentPrincipal); + } + } + [Fact] [PlatformSpecific(TestPlatforms.AnyUnix)] public static void WindowsPrincipalPolicyTest_Unix() From 080cda65da5290c1444f55f5070c4283b0e5be0e Mon Sep 17 00:00:00 2001 From: Marco Rossignoli Date: Tue, 11 Feb 2020 10:19:03 +0100 Subject: [PATCH 2/3] update test --- src/libraries/System.Threading.Thread/tests/ThreadTests.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/libraries/System.Threading.Thread/tests/ThreadTests.cs b/src/libraries/System.Threading.Thread/tests/ThreadTests.cs index 10b1e12d1545eb..017341ba856f26 100644 --- a/src/libraries/System.Threading.Thread/tests/ThreadTests.cs +++ b/src/libraries/System.Threading.Thread/tests/ThreadTests.cs @@ -1145,8 +1145,7 @@ static void CheckPrincipal(object principal) } [Fact] - [PlatformSpecific(TestPlatforms.Windows)] - public static void NoPrincipalPolicyTest_Windows_NewThreads() + public static void NoPrincipalPolicyTest_NewThreads() { RemoteExecutor.Invoke(() => { From f14ebc2e0ff8adb38c455f3358c91eff65d46d8d Mon Sep 17 00:00:00 2001 From: Marco Rossignoli Date: Tue, 11 Feb 2020 19:11:17 +0100 Subject: [PATCH 3/3] address PR feedback --- .../System.Private.CoreLib/src/System/Threading/Thread.cs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Threading/Thread.cs b/src/libraries/System.Private.CoreLib/src/System/Threading/Thread.cs index 13866bbbea99f8..95f7bb702c0f5d 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Threading/Thread.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Threading/Thread.cs @@ -133,11 +133,12 @@ public static IPrincipal? CurrentPrincipal { get { - if (s_asyncLocalPrincipal is null || s_asyncLocalPrincipal.Value is null) + IPrincipal? principal = s_asyncLocalPrincipal?.Value; + if (principal is null) { - CurrentPrincipal = AppDomain.CurrentDomain.GetThreadPrincipal(); + CurrentPrincipal = (principal = AppDomain.CurrentDomain.GetThreadPrincipal()); } - return s_asyncLocalPrincipal?.Value; + return principal; } set {