Locking Down the TokenAcquirerFactory

.github/workflows/dotnetcore.yml

@@ -42,15 +42,15 @@ jobs:
     - name: Build solution
       run: msbuild Microsoft.Identity.Web.sln -r -t:build -verbosity:m -property:Configuration=Release
 
-    - name: Test with .NET 6.0.x
-      run: dotnet test --no-restore --no-build Microsoft.Identity.Web.sln -f net6.0 -v normal -p:FROM_GITHUB_ACTION=true --configuration Release --filter "(FullyQualifiedName!~Microsoft.Identity.Web.Test.Integration)&(FullyQualifiedName!~WebAppUiTests)&(FullyQualifiedName!~IntegrationTests)&(FullyQualifiedName!~TokenAcquirerTests)"
-
     - name: Test with .NET 8.0.x
       run: dotnet test --no-restore --no-build Microsoft.Identity.Web.sln -f net8.0 -v normal -p:FROM_GITHUB_ACTION=true --configuration Release --collect "Xplat Code Coverage" --filter "(FullyQualifiedName!~Microsoft.Identity.Web.Test.Integration)&(FullyQualifiedName!~WebAppUiTests)&(FullyQualifiedName!~IntegrationTests)&(FullyQualifiedName!~TokenAcquirerTests)"
 
     - name: Test with .NET 9.0.x
       run: dotnet test --no-restore --no-build Microsoft.Identity.Web.sln -f net9.0 -v normal -p:FROM_GITHUB_ACTION=true --configuration Release --collect "Xplat Code Coverage" --filter "(FullyQualifiedName!~Microsoft.Identity.Web.Test.Integration)&(FullyQualifiedName!~WebAppUiTests)&(FullyQualifiedName!~IntegrationTests)&(FullyQualifiedName!~TokenAcquirerTests)"
 
+    - name: Test with .NET 6.0.x
+      run: dotnet test Microsoft.Identity.Web.sln -f net6.0 -v normal -p:FROM_GITHUB_ACTION=true --configuration Release --filter "(FullyQualifiedName!~Microsoft.Identity.Web.Test.Integration)&(FullyQualifiedName!~WebAppUiTests)&(FullyQualifiedName!~IntegrationTests)&(FullyQualifiedName!~TokenAcquirerTests)"
+
     - name: Create code coverage report
       run: |
         dotnet tool install -g dotnet-reportgenerator-globaltool --version 5.4.1

src/Microsoft.Identity.Web.TokenAcquisition/TokenAcquirerFactory.cs

@@ -7,6 +7,7 @@
 using System.IO;
 using System.Linq;
 using System.Reflection;
+using System.Threading;
 using Microsoft.Extensions.Configuration;
 using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Identity.Abstractions;
@@ -50,6 +51,13 @@ private set
             }
         }
         private ServiceCollection _services = new ServiceCollection();
+#if NET9_0_OR_GREATER
+        private static readonly Lock s_defaultInstanceLock = new();
+        private readonly Lock _buildLock = new();
+#else
+        private static readonly object s_defaultInstanceLock = new();
+        private readonly object _buildLock = new();
+#endif
 
         /// <summary>
         /// Constructor
@@ -78,25 +86,30 @@ protected TokenAcquirerFactory()
             T instance;
             if (defaultInstance == null)
             {
-                instance = new T();
-                instance.ReadConfiguration();
-                defaultInstance = instance;
-                instance.Services.AddTokenAcquisition();
-                instance.Services.AddHttpClient();
-                instance.Services.Configure<MicrosoftIdentityApplicationOptions>(option =>
+                lock (s_defaultInstanceLock)
                 {
-                    instance.Configuration.GetSection(configSection).Bind(option);
+                    if (defaultInstance == null)
+                    {
+                        instance = new T();
+                        instance.ReadConfiguration();
+                        defaultInstance = instance;
+                        instance.Services.AddTokenAcquisition();
+                        instance.Services.AddHttpClient();
+                        instance.Services.Configure<MicrosoftIdentityApplicationOptions>(option =>
+                        {
+                            instance.Configuration.GetSection(configSection).Bind(option);
 
-                    // This is temporary and will be removed eventually.
-                    CiamAuthorityHelper.BuildCiamAuthorityIfNeeded(option);
-                });
-                instance.Services.AddSingleton<ITokenAcquirerFactory, DefaultTokenAcquirerFactoryImplementation>();
-                instance.Services.AddSingleton(defaultInstance.Configuration);
+                            // This is temporary and will be removed eventually.
+                            CiamAuthorityHelper.BuildCiamAuthorityIfNeeded(option);
+                        });
+                        instance.Services.AddSingleton<ITokenAcquirerFactory, DefaultTokenAcquirerFactoryImplementation>();
+                        instance.Services.AddSingleton(defaultInstance.Configuration);
+                    }
+                }
             }
             return (defaultInstance as T)!;
         }
 
-
         /// <summary>
         /// Get the default instance. Use this method to retrieve the instance, optionally add some services to 
         /// the service collection, and build the instance.
@@ -116,20 +129,26 @@ static public TokenAcquirerFactory GetDefaultInstance(string configSection = "Az
             TokenAcquirerFactory instance;
             if (defaultInstance == null)
             {
-                instance = new TokenAcquirerFactory();
-                instance.ReadConfiguration();
-                defaultInstance = instance;
-                instance.Services.AddTokenAcquisition();
-                instance.Services.AddHttpClient();
-                instance.Services.Configure<MicrosoftIdentityApplicationOptions>(option =>
+                lock (s_defaultInstanceLock)
                 {
-                    instance.Configuration.GetSection(configSection).Bind(option);
+                    if (defaultInstance == null)
+                    {
+                        instance = new TokenAcquirerFactory();
+                        instance.ReadConfiguration();
+                        defaultInstance = instance;
+                        instance.Services.AddTokenAcquisition();
+                        instance.Services.AddHttpClient();
+                        instance.Services.Configure<MicrosoftIdentityApplicationOptions>(option =>
+                        {
+                            instance.Configuration.GetSection(configSection).Bind(option);
 
-                    // This is temporary and will be removed eventually.
-                    CiamAuthorityHelper.BuildCiamAuthorityIfNeeded(option);
-                });
-                instance.Services.AddSingleton<ITokenAcquirerFactory, DefaultTokenAcquirerFactoryImplementation>();
-                instance.Services.AddSingleton(defaultInstance.Configuration);
+                            // This is temporary and will be removed eventually.
+                            CiamAuthorityHelper.BuildCiamAuthorityIfNeeded(option);
+                        });
+                        instance.Services.AddSingleton<ITokenAcquirerFactory, DefaultTokenAcquirerFactoryImplementation>();
+                        instance.Services.AddSingleton(defaultInstance.Configuration);
+                    }
+                }
             }
             return defaultInstance!;
         }
@@ -157,9 +176,18 @@ public IServiceProvider Build()
                 throw new InvalidOperationException("You shouldn't call Build() twice");
             }
 
-            // Additional processing before creating the service provider
-            PreBuild();
-            ServiceProvider = Services.BuildServiceProvider();
+            lock(_buildLock)
+            {
+                if (ServiceProvider != null)
+                {
+                    throw new InvalidOperationException("You shouldn't call Build() twice");
+                }
+
+                // Additional processing before creating the service provider
+                PreBuild();
+                ServiceProvider = Services.BuildServiceProvider();
+            }
+
             return ServiceProvider;
         }
 

tests/DevApps/aspnet-mvc/OwinWebApi/Web.config

@@ -58,11 +58,11 @@
 			</dependentAssembly>
 			<dependentAssembly>
 				<assemblyIdentity name="System.IdentityModel.Tokens.Jwt" publicKeyToken="31BF3856AD364E35" culture="neutral"/>
-				<bindingRedirect oldVersion="0.0.0.0-8.6.0.0" newVersion="8.6.0.0"/>
+				<bindingRedirect oldVersion="0.0.0.0-8.3.1.0" newVersion="8.3.1.0"/>
 			</dependentAssembly>
 			<dependentAssembly>
 				<assemblyIdentity name="System.Diagnostics.DiagnosticSource" publicKeyToken="CC7B13FFCD2DDD51" culture="neutral"/>
-				<bindingRedirect oldVersion="0.0.0.0-6.0.0.2" newVersion="6.0.0.2"/>
+				<bindingRedirect oldVersion="0.0.0.0-6.0.0.1" newVersion="6.0.0.1"/>
 			</dependentAssembly>
 			<dependentAssembly>
 				<assemblyIdentity name="System.Buffers" publicKeyToken="CC7B13FFCD2DDD51" culture="neutral"/>
@@ -74,31 +74,31 @@
 			</dependentAssembly>
 			<dependentAssembly>
 				<assemblyIdentity name="Microsoft.IdentityModel.Tokens" publicKeyToken="31BF3856AD364E35" culture="neutral"/>
-				<bindingRedirect oldVersion="0.0.0.0-8.6.0.0" newVersion="8.6.0.0"/>
+				<bindingRedirect oldVersion="0.0.0.0-8.3.1.0" newVersion="8.3.1.0"/>
 			</dependentAssembly>
 			<dependentAssembly>
 				<assemblyIdentity name="Microsoft.IdentityModel.Protocols.WsFederation" publicKeyToken="31BF3856AD364E35" culture="neutral"/>
 				<bindingRedirect oldVersion="0.0.0.0-5.5.0.0" newVersion="5.5.0.0"/>
 			</dependentAssembly>
 			<dependentAssembly>
 				<assemblyIdentity name="Microsoft.IdentityModel.Protocols.OpenIdConnect" publicKeyToken="31BF3856AD364E35" culture="neutral"/>
-				<bindingRedirect oldVersion="0.0.0.0-8.6.0.0" newVersion="8.6.0.0"/>
+				<bindingRedirect oldVersion="0.0.0.0-8.3.1.0" newVersion="8.3.1.0"/>
 			</dependentAssembly>
 			<dependentAssembly>
 				<assemblyIdentity name="Microsoft.IdentityModel.Protocols" publicKeyToken="31BF3856AD364E35" culture="neutral"/>
-				<bindingRedirect oldVersion="0.0.0.0-8.6.0.0" newVersion="8.6.0.0"/>
+				<bindingRedirect oldVersion="0.0.0.0-8.3.1.0" newVersion="8.3.1.0"/>
 			</dependentAssembly>
 			<dependentAssembly>
 				<assemblyIdentity name="Microsoft.IdentityModel.Logging" publicKeyToken="31BF3856AD364E35" culture="neutral"/>
-				<bindingRedirect oldVersion="0.0.0.0-8.6.0.0" newVersion="8.6.0.0"/>
+				<bindingRedirect oldVersion="0.0.0.0-8.3.1.0" newVersion="8.3.1.0"/>
 			</dependentAssembly>
 			<dependentAssembly>
 				<assemblyIdentity name="Microsoft.IdentityModel.Abstractions" publicKeyToken="31BF3856AD364E35" culture="neutral"/>
-				<bindingRedirect oldVersion="0.0.0.0-8.6.0.0" newVersion="8.6.0.0"/>
+				<bindingRedirect oldVersion="0.0.0.0-8.3.1.0" newVersion="8.3.1.0"/>
 			</dependentAssembly>
 			<dependentAssembly>
 				<assemblyIdentity name="Microsoft.Identity.Client" publicKeyToken="0A613F4DD989E8AE" culture="neutral"/>
-				<bindingRedirect oldVersion="0.0.0.0-4.68.0.0" newVersion="4.68.0.0"/>
+				<bindingRedirect oldVersion="0.0.0.0-4.67.2.0" newVersion="4.67.2.0"/>
 			</dependentAssembly>
 			<dependentAssembly>
 				<assemblyIdentity name="Microsoft.Extensions.Primitives" publicKeyToken="ADB9793829DDAE60" culture="neutral"/>

tests/DevApps/aspnet-mvc/OwinWebApp/Web.config

@@ -59,11 +59,11 @@
 			</dependentAssembly>
 			<dependentAssembly>
 				<assemblyIdentity name="System.IdentityModel.Tokens.Jwt" publicKeyToken="31BF3856AD364E35" culture="neutral"/>
-				<bindingRedirect oldVersion="0.0.0.0-8.6.0.0" newVersion="8.6.0.0"/>
+				<bindingRedirect oldVersion="0.0.0.0-8.3.1.0" newVersion="8.3.1.0"/>
 			</dependentAssembly>
 			<dependentAssembly>
 				<assemblyIdentity name="System.Diagnostics.DiagnosticSource" publicKeyToken="CC7B13FFCD2DDD51" culture="neutral"/>
-				<bindingRedirect oldVersion="0.0.0.0-6.0.0.2" newVersion="6.0.0.2"/>
+				<bindingRedirect oldVersion="0.0.0.0-6.0.0.1" newVersion="6.0.0.1"/>
 			</dependentAssembly>
 			<dependentAssembly>
 				<assemblyIdentity name="System.Buffers" publicKeyToken="CC7B13FFCD2DDD51" culture="neutral"/>
@@ -75,31 +75,31 @@
 			</dependentAssembly>
 			<dependentAssembly>
 				<assemblyIdentity name="Microsoft.IdentityModel.Tokens" publicKeyToken="31BF3856AD364E35" culture="neutral"/>
-				<bindingRedirect oldVersion="0.0.0.0-8.6.0.0" newVersion="8.6.0.0"/>
+				<bindingRedirect oldVersion="0.0.0.0-8.3.1.0" newVersion="8.3.1.0"/>
 			</dependentAssembly>
 			<dependentAssembly>
 				<assemblyIdentity name="Microsoft.IdentityModel.Protocols.WsFederation" publicKeyToken="31BF3856AD364E35" culture="neutral"/>
 				<bindingRedirect oldVersion="0.0.0.0-5.5.0.0" newVersion="5.5.0.0"/>
 			</dependentAssembly>
 			<dependentAssembly>
 				<assemblyIdentity name="Microsoft.IdentityModel.Protocols.OpenIdConnect" publicKeyToken="31BF3856AD364E35" culture="neutral"/>
-				<bindingRedirect oldVersion="0.0.0.0-8.6.0.0" newVersion="8.6.0.0"/>
+				<bindingRedirect oldVersion="0.0.0.0-8.3.1.0" newVersion="8.3.1.0"/>
 			</dependentAssembly>
 			<dependentAssembly>
 				<assemblyIdentity name="Microsoft.IdentityModel.Protocols" publicKeyToken="31BF3856AD364E35" culture="neutral"/>
-				<bindingRedirect oldVersion="0.0.0.0-8.6.0.0" newVersion="8.6.0.0"/>
+				<bindingRedirect oldVersion="0.0.0.0-8.3.1.0" newVersion="8.3.1.0"/>
 			</dependentAssembly>
 			<dependentAssembly>
 				<assemblyIdentity name="Microsoft.IdentityModel.Logging" publicKeyToken="31BF3856AD364E35" culture="neutral"/>
-				<bindingRedirect oldVersion="0.0.0.0-8.6.0.0" newVersion="8.6.0.0"/>
+				<bindingRedirect oldVersion="0.0.0.0-8.3.1.0" newVersion="8.3.1.0"/>
 			</dependentAssembly>
 			<dependentAssembly>
 				<assemblyIdentity name="Microsoft.IdentityModel.Abstractions" publicKeyToken="31BF3856AD364E35" culture="neutral"/>
-				<bindingRedirect oldVersion="0.0.0.0-8.6.0.0" newVersion="8.6.0.0"/>
+				<bindingRedirect oldVersion="0.0.0.0-8.3.1.0" newVersion="8.3.1.0"/>
 			</dependentAssembly>
 			<dependentAssembly>
 				<assemblyIdentity name="Microsoft.Identity.Client" publicKeyToken="0A613F4DD989E8AE" culture="neutral"/>
-				<bindingRedirect oldVersion="0.0.0.0-4.68.0.0" newVersion="4.68.0.0"/>
+				<bindingRedirect oldVersion="0.0.0.0-4.67.2.0" newVersion="4.67.2.0"/>
 			</dependentAssembly>
 			<dependentAssembly>
 				<assemblyIdentity name="Microsoft.Extensions.Primitives" publicKeyToken="ADB9793829DDAE60" culture="neutral"/>

tests/E2E Tests/CustomSignedAssertionProviderTests/CustomSignedAssertionProviderExtensibilityTests.cs

@@ -8,11 +8,13 @@
 using Microsoft.Identity.Abstractions;
 using Microsoft.Identity.Client;
 using Microsoft.Identity.Web;
+using Microsoft.Identity.Web.Test.Common;
 using Xunit.Sdk;
 
 
 namespace CustomSignedAssertionProviderTests
 {
+    [Collection(nameof(TokenAcquirerFactorySingletonProtection))]
     public class CustomSignedAssertionProviderExtensibilityTests
     {
         [Fact]

tests/E2E Tests/CustomSignedAssertionProviderTests/CustomSignedAssertionProviderTests.csproj

@@ -28,6 +28,7 @@
     <ProjectReference Include="..\..\..\src\Microsoft.Identity.Web\Microsoft.Identity.Web.csproj" />
     <ProjectReference Include="..\..\..\src\Microsoft.Identity.Web.Certificate\Microsoft.Identity.Web.Certificate.csproj" />
     <ProjectReference Include="..\..\..\src\Microsoft.Identity.Web.Certificateless\Microsoft.Identity.Web.Certificateless.csproj" />
+    <ProjectReference Include="..\..\Microsoft.Identity.Web.Test.Common\Microsoft.Identity.Web.Test.Common.csproj" />
   </ItemGroup>
 
   <ItemGroup>

tests/E2E Tests/TokenAcquirerTests/CertificateRotationTest.cs

@@ -19,6 +19,7 @@
 
 namespace TokenAcquirerTests
 {
+    [Collection(nameof(TokenAcquirerFactorySingletonProtection))]
     public sealed class CertificateRotationTest : ICertificatesObserver
     {
         const string MicrosoftGraphAppId = "00000003-0000-0000-c000-000000000000";

tests/E2E Tests/TokenAcquirerTests/TokenAcquirer.cs

@@ -23,6 +23,7 @@
 
 namespace TokenAcquirerTests
 {
+    [CollectionDefinition(nameof(TokenAcquirerFactorySingletonProtection))]
 #if !FROM_GITHUB_ACTION
     public class TokenAcquirer
     {

tests/Microsoft.Identity.Web.Test.Common/Microsoft.Identity.Web.Test.Common.csproj

@@ -1,4 +1,4 @@
-<Project Sdk="Microsoft.NET.Sdk">
+<Project Sdk="Microsoft.NET.Sdk">
 
   <PropertyGroup>
     <TargetFrameworks>net462; net472; net6.0; net8.0; net9.0</TargetFrameworks>
@@ -17,7 +17,6 @@
     <PackageReference Include="Microsoft.Identity.Lab.Api" Version="$(MicrosoftIdentityLabApiVersion)" />
     <PackageReference Include="NSubstitute" Version="$(NSubstituteVersion)" />
     <PackageReference Include="xunit" Version="$(XunitVersion)" />
-    <PackageReference Include="xunit.assert" Version="$(XunitAssertVersion)" />
     <PackageReference Include="System.Text.Json" Version="$(SystemTextJsonVersion)" />
     <PackageReference Include="System.Net.Http" Version="$(SystemNetHttpVersion)" />
     <PackageReference Include="System.Text.RegularExpressions" Version="$(SystemTextRegularExpressions)" />

tests/Microsoft.Identity.Web.Test.Common/TokenAcquirerFactorySingletonProtection.cs

@@ -0,0 +1,14 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+using Xunit;
+
+namespace Microsoft.Identity.Web.Test.Common
+{
+    [CollectionDefinition(nameof(TokenAcquirerFactorySingletonProtection))]
+    public class TokenAcquirerFactorySingletonProtection
+    {
+        // This class has no code, and is never created. Its purpose is to prevent test classes using the
+        // static singleton DefaultTokenAcquirerFactory from running in parallel as some tests modify this singleton.
+    }
+}

tests/Microsoft.Identity.Web.Test/BaseAuthorizationHeaderProviderTest.cs

@@ -10,11 +10,13 @@
 using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Identity.Abstractions;
 using Microsoft.Identity.Client;
+using Microsoft.Identity.Web.Test.Common;
 using Microsoft.Identity.Web.Extensibility;
 using Xunit;
 
 namespace Microsoft.Identity.Web.Test
 {
+    [Collection(nameof(TokenAcquirerFactorySingletonProtection))]
     public class BaseAuthorizationHeaderProviderTest
     {
         public BaseAuthorizationHeaderProviderTest()

tests/Microsoft.Identity.Web.Test/TestTokenAcquisitionHost.cs

@@ -4,10 +4,12 @@
 using System.Linq;
 using Microsoft.AspNetCore.Builder;
 using Microsoft.Extensions.DependencyInjection;
+using Microsoft.Identity.Web.Test.Common;
 using Xunit;
 
 namespace Microsoft.Identity.Web.Test
 {
+    [Collection(nameof(TokenAcquirerFactorySingletonProtection))]
     public class TestTokenAcquisitionHost
     {
         [Fact]