Add BannedApiAnalyzers to prevent use of ClaimsIdentity constructors (#…

…2778) * Add BannedApiAnalyzers to prevent use of ClaimsIdentity constructors * Update tests.
AzureAD · Aug 17, 2024 · 34a421f · 34a421f
1 parent eb4df8f
commit 34a421f
Show file tree

Hide file tree

Showing 13 changed files with 54 additions and 2 deletions.
diff --git a/.editorconfig b/.editorconfig
@@ -157,6 +157,9 @@ csharp_space_between_square_brackets = false
 dotnet_code_quality.ca1802.api_surface = private, internal
 dotnet_code_quality.CA2208.api_surface = public
 
+# RS0030: Do not used banned APIs
+dotnet_diagnostic.RS0030.severity = error
+
 # Threading analyzers https://github.com/microsoft/vs-threading/blob/main/doc/analyzers/index.md
 
 # Use .ConfigureAwait(bool)

diff --git a/BannedSymbols.txt b/BannedSymbols.txt
@@ -0,0 +1,10 @@
+M:System.Security.Claims.ClaimsIdentity.#ctor(); Use Microsoft.IdentityModel.Tokens.CaseSensitiveClaimsIdentity instead.
+M:System.Security.Claims.ClaimsIdentity.#ctor(System.Security.Principal.IIdentity); Use Microsoft.IdentityModel.Tokens.CaseSensitiveClaimsIdentity instead.
+M:System.Security.Claims.ClaimsIdentity.#ctor(System.Collections.Generic.IEnumerable{System.Security.Claims.Claim}); Use Microsoft.IdentityModel.Tokens.CaseSensitiveClaimsIdentity instead.
+M:System.Security.Claims.ClaimsIdentity.#ctor(System.String); Use Microsoft.IdentityModel.Tokens.CaseSensitiveClaimsIdentity instead.
+M:System.Security.Claims.ClaimsIdentity.#ctor(System.Collections.Generic.IEnumerable{System.Security.Claims.Claim},System.String); Use Microsoft.IdentityModel.Tokens.CaseSensitiveClaimsIdentity instead.
+M:System.Security.Claims.ClaimsIdentity.#ctor(System.Security.Principal.IIdentity,System.Collections.Generic.IEnumerable{System.Security.Claims.Claim}); Use Microsoft.IdentityModel.Tokens.CaseSensitiveClaimsIdentity instead.
+M:System.Security.Claims.ClaimsIdentity.#ctor(System.String,System.String,System.String); Use Microsoft.IdentityModel.Tokens.CaseSensitiveClaimsIdentity instead.
+M:System.Security.Claims.ClaimsIdentity.#ctor(System.Collections.Generic.IEnumerable{System.Security.Claims.Claim},System.String,System.String,System.String); Use Microsoft.IdentityModel.Tokens.CaseSensitiveClaimsIdentity instead.
+M:System.Security.Claims.ClaimsIdentity.#ctor(System.Security.Principal.IIdentity,System.Collections.Generic.IEnumerable{System.Security.Claims.Claim},System.String,System.String,System.String); Use Microsoft.IdentityModel.Tokens.CaseSensitiveClaimsIdentity instead.
+M:System.Security.Claims.ClaimsIdentity.#ctor(System.IO.BinaryReader); Use Microsoft.IdentityModel.Tokens.CaseSensitiveClaimsIdentity instead.
diff --git a/Wilson.sln b/Wilson.sln
@@ -75,6 +75,7 @@ EndProject
 Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Solution Items", "Solution Items", "{4655DBB4-70C6-475D-8971-FE6619B85F70}"
 	ProjectSection(SolutionItems) = preProject
 		.editorconfig = .editorconfig
+		BannedSymbols.txt = BannedSymbols.txt
 		buildPack.bat = buildPack.bat
 		buildTestPack.bat = buildTestPack.bat
 	EndProjectSection

diff --git a/benchmark/Microsoft.IdentityModel.Benchmarks/Microsoft.IdentityModel.Benchmarks.csproj b/benchmark/Microsoft.IdentityModel.Benchmarks/Microsoft.IdentityModel.Benchmarks.csproj
@@ -28,6 +28,14 @@
     <PackageReference Include="BenchmarkDotNet" Version="0.13.12" />
   </ItemGroup>
 
+  <ItemGroup>
+    <PackageReference Include="Microsoft.CodeAnalysis.BannedApiAnalyzers" Version="3.3.4">
+      <PrivateAssets>all</PrivateAssets>
+      <IncludeAssets>runtime; build; native; contentfiles; analyzers</IncludeAssets>
+    </PackageReference>
+    <AdditionalFiles Include="$(MSBuildThisFileDirectory)..\..\BannedSymbols.txt" />
+  </ItemGroup>
+
   <PropertyGroup Condition=" '$(Configuration)' == 'Release' ">
     <Optimize>true</Optimize>
   </PropertyGroup>

diff --git a/build/common.props b/build/common.props
@@ -66,6 +66,14 @@
     <PackageReference Include="Microsoft.SourceLink.GitHub" Version="$(MicrosoftSourceLinkGitHubVersion)" PrivateAssets="All"/>
   </ItemGroup>
 
+  <ItemGroup>
+    <PackageReference Include="Microsoft.CodeAnalysis.BannedApiAnalyzers" Version="$(BannedApiAnalyzersVersion)">
+      <PrivateAssets>all</PrivateAssets>
+      <IncludeAssets>runtime; build; native; contentfiles; analyzers</IncludeAssets>
+    </PackageReference>
+    <AdditionalFiles Include="$(MSBuildThisFileDirectory)..\BannedSymbols.txt" />
+  </ItemGroup>
+
   <ItemGroup Condition="'$(TargetFrameworkIdentifier)' != '.NETCoreApp'">
     <PackageReference Include="Microsoft.CodeAnalysis.FxCopAnalyzers" Version="3.3.1">
       <PrivateAssets>all</PrivateAssets>

diff --git a/build/commonTest.props b/build/commonTest.props
@@ -39,4 +39,12 @@
     <PackageReference Include="xunit" Version="$(XunitVersion)" />
   </ItemGroup>
 
+  <ItemGroup>
+    <PackageReference Include="Microsoft.CodeAnalysis.BannedApiAnalyzers" Version="$(BannedApiAnalyzersVersion)">
+      <PrivateAssets>all</PrivateAssets>
+      <IncludeAssets>runtime; build; native; contentfiles; analyzers</IncludeAssets>
+    </PackageReference>
+    <AdditionalFiles Include="$(MSBuildThisFileDirectory)..\BannedSymbols.txt" />
+  </ItemGroup>
+
 </Project>
diff --git a/build/dependencies.props b/build/dependencies.props
@@ -2,6 +2,7 @@
 
   <PropertyGroup>
     <AspNetCoreMinSupportedVersion>2.1.1</AspNetCoreMinSupportedVersion>
+    <BannedApiAnalyzersVersion>3.3.4</BannedApiAnalyzersVersion>
     <MicrosoftCSharpVersion>4.5.0</MicrosoftCSharpVersion>
     <MicrosoftSourceLinkGitHubVersion>1.0.0</MicrosoftSourceLinkGitHubVersion>
     <NetStandardVersion>2.0.3</NetStandardVersion>

diff --git a/build/dependenciesTest.props b/build/dependenciesTest.props
@@ -1,12 +1,13 @@
 <Project>
   <PropertyGroup>
+    <BannedApiAnalyzersVersion>3.3.4</BannedApiAnalyzersVersion>
     <DotNetCoreAppRuntimeVersion>2.1.30</DotNetCoreAppRuntimeVersion>
     <MicrosoftAzureKeyVaultCryptographyVersion>2.0.5</MicrosoftAzureKeyVaultCryptographyVersion>
     <MicrosoftNETTestSdkVersion>16.10.0</MicrosoftNETTestSdkVersion>
     <NetStandardVersion>2.0.3</NetStandardVersion>
     <NewtonsoftVersion>13.0.3</NewtonsoftVersion>
     <SystemSecurityClaimsVersion>4.3.0</SystemSecurityClaimsVersion>
-    <XunitVersion>2.4.0</XunitVersion>
     <SystemTextJson>8.0.4</SystemTextJson>
+    <XunitVersion>2.4.0</XunitVersion>
   </PropertyGroup>
 </Project>
diff --git a/src/Microsoft.IdentityModel.Tokens/CaseSensitiveClaimsIdentity.cs b/src/Microsoft.IdentityModel.Tokens/CaseSensitiveClaimsIdentity.cs
@@ -7,6 +7,8 @@
 
 namespace Microsoft.IdentityModel.Tokens
 {
+#pragma warning disable RS0030 // Do not use banned APIs
+
     /// <summary>
     /// A derived <see cref="ClaimsIdentity"/> where claim retrieval is case-sensitive. The current <see cref="ClaimsIdentity"/> retrieves claims in a case-insensitive manner which is different than querying the underlying <see cref="SecurityToken"/>. The <see cref="CaseSensitiveClaimsIdentity"/> provides consistent retrieval logic between the <see cref="SecurityToken"/> and <see cref="ClaimsIdentity"/>.
     /// </summary>
@@ -119,4 +121,6 @@ public override bool HasClaim(string type, string value)
                 && claim?.Value.Equals(value, StringComparison.Ordinal) == true);
         }
     }
+
+#pragma warning disable RS0030 // Do not use banned APIs
 }
diff --git a/src/Microsoft.IdentityModel.Tokens/ClaimsIdentityFactory.cs b/src/Microsoft.IdentityModel.Tokens/ClaimsIdentityFactory.cs
@@ -6,6 +6,8 @@
 
 namespace Microsoft.IdentityModel.Tokens
 {
+#pragma warning disable RS0030 // Do not use banned APIs
+
     /// <summary>
     /// Facilitates the creation of <see cref="ClaimsIdentity"/> and <see cref="CaseSensitiveClaimsIdentity"/> instances based on the <see cref="AppContextSwitches.UseClaimsIdentityTypeSwitch"/>.
     /// </summary>
@@ -38,4 +40,6 @@ internal static ClaimsIdentity Create(string authenticationType, string nameType
             };
         }
     }
+
+#pragma warning restore RS0030 // Do not use banned APIs
 }
diff --git a/src/Microsoft.IdentityModel.Tokens/Validation/ValidationParameters.cs b/src/Microsoft.IdentityModel.Tokens/Validation/ValidationParameters.cs
@@ -230,7 +230,9 @@ public virtual ClaimsIdentity CreateClaimsIdentity(SecurityToken securityToken,
             if (LogHelper.IsEnabled(EventLogLevel.Informational))
                 LogHelper.LogInformation(LogMessages.IDX10245, securityToken);
 
+#pragma warning disable RS0030 // Do not use banned APIs
             return new ClaimsIdentity(authenticationType: AuthenticationType ?? DefaultAuthenticationType, nameType: nameClaimType ?? ClaimsIdentity.DefaultNameClaimType, roleType: roleClaimType ?? ClaimsIdentity.DefaultRoleClaimType);
+#pragma warning disable RS0030 // Do not use banned APIs
         }
 
         /// <summary>

diff --git a/test/Microsoft.IdentityModel.JsonWebTokens.Tests/JsonWebTokenHandlerTests.cs b/test/Microsoft.IdentityModel.JsonWebTokens.Tests/JsonWebTokenHandlerTests.cs
@@ -2483,7 +2483,7 @@ public void RoundTripWithEmptyPayload(CreateTokenTheoryData theoryData)
             var tokenValidationResult = theoryData.JsonWebTokenHandler.ValidateTokenAsync(jwtString, theoryData.ValidationParameters).Result;
             var validatedToken = tokenValidationResult.SecurityToken as JsonWebToken;
             var claimsIdentity = tokenValidationResult.ClaimsIdentity;
-            IdentityComparer.AreEqual(new ClaimsIdentity("AuthenticationTypes.Federation"), claimsIdentity, context);
+            IdentityComparer.AreEqual(new CaseSensitiveClaimsIdentity("AuthenticationTypes.Federation"), claimsIdentity, context);
             IdentityComparer.AreEqual("", Base64UrlEncoder.Decode(validatedToken.EncodedPayload), context);
 
             TestUtilities.AssertFailIfErrors(context);

diff --git a/test/Microsoft.IdentityModel.Tokens.Tests/ClaimsIdentityFactoryTests.cs b/test/Microsoft.IdentityModel.Tokens.Tests/ClaimsIdentityFactoryTests.cs
@@ -114,7 +114,9 @@ public override ClaimsIdentity CreateClaimsIdentity(SecurityToken securityToken,
                     }
                 }
 
+#pragma warning disable RS0030 // Do not used banned APIs
                 return new ClaimsIdentity(AuthenticationType, NameClaimType, RoleClaimType);
+#pragma warning restore RS0030 // Do not used banned APIs
             }
         }
-Original file line number
+Diff line change
@@ Expand Up @@
                         }
                     }
+    #pragma warning disable RS0030 // Do not used banned APIs
                     return new ClaimsIdentity(AuthenticationType, NameClaimType, RoleClaimType);
+    #pragma warning restore RS0030 // Do not used banned APIs
                 }
             }
@@ Expand Down @@