Fix Open Id connect parsing bug. (#2776)

Error in state machine logic, missing "else". Introduced in #2657 Fixes #2772 Co-authored-by: Keegan Caruso <keegancaruso@microsoft.com>
AzureAD · Aug 12, 2024 · 7841666 · 7841666
1 parent 5853e4c
commit 7841666
Show file tree

Hide file tree

Showing 6 changed files with 44 additions and 4 deletions.
diff --git a/...rosoft.IdentityModel.Protocols.OpenIdConnect/Json/OpenIdConnectConfigurationSerializer.cs b/...rosoft.IdentityModel.Protocols.OpenIdConnect/Json/OpenIdConnectConfigurationSerializer.cs
@@ -163,7 +163,7 @@ public static OpenIdConnectConfiguration Read(ref Utf8JsonReader reader, OpenIdC
                     if (reader.ValueTextEquals(Utf8Bytes.AcrValuesSupported))
                         JsonPrimitives.ReadStrings(ref reader, config.AcrValuesSupported, MetadataName.AcrValuesSupported, ClassName, true);
 
-                    if (reader.ValueTextEquals(Utf8Bytes.AuthorizationDetailsTypesSupported))
+                    else if (reader.ValueTextEquals(Utf8Bytes.AuthorizationDetailsTypesSupported))
                         JsonPrimitives.ReadStrings(ref reader, config.AuthorizationDetailsTypesSupported, MetadataName.AuthorizationDetailsTypesSupported, ClassName, true);
 
                     else if (reader.ValueTextEquals(Utf8Bytes.AuthorizationEndpoint))
@@ -382,7 +382,7 @@ public static OpenIdConnectConfiguration Read(ref Utf8JsonReader reader, OpenIdC
                             if (propertyName.Equals(MetadataName.AcrValuesSupported, StringComparison.OrdinalIgnoreCase))
                                 JsonPrimitives.ReadStrings(ref reader, config.AcrValuesSupported, propertyName, ClassName);
 
-                            if (propertyName.Equals(MetadataName.AuthorizationDetailsTypesSupported, StringComparison.OrdinalIgnoreCase))
+                            else if (propertyName.Equals(MetadataName.AuthorizationDetailsTypesSupported, StringComparison.OrdinalIgnoreCase))
                                 JsonPrimitives.ReadStrings(ref reader, config.AuthorizationDetailsTypesSupported, propertyName, ClassName);
 
                             else if (propertyName.Equals(MetadataName.AuthorizationEndpoint, StringComparison.OrdinalIgnoreCase))

diff --git a/test/Microsoft.IdentityModel.Protocols.OpenIdConnect.Tests/End2EndTests.cs b/test/Microsoft.IdentityModel.Protocols.OpenIdConnect.Tests/End2EndTests.cs
@@ -22,6 +22,9 @@ public void OpenIdConnect(OpenIdConnectTheoryData theoryData)
             try
             {
                 OpenIdConnectConfiguration configuration = OpenIdConnectConfigurationRetriever.GetAsync(theoryData.OpenIdConnectMetadataFileName, new FileDocumentRetriever(), CancellationToken.None).Result;
+
+                theoryData.AdditionalValidation?.Invoke(configuration);
+
                 JwtSecurityTokenHandler tokenHandler = new JwtSecurityTokenHandler();
                 JwtSecurityToken jwtToken =
                     tokenHandler.CreateJwtSecurityToken(
@@ -102,7 +105,22 @@ public static TheoryData<OpenIdConnectTheoryData> OpenIdConnectTheoryData()
                         ),
                     ExpectedException = ExpectedException.SecurityTokenSignatureKeyNotFoundException(),
                     TestId = "Ecdsa384KeyNotPartOfJWKS"
-                }
+                },
+                new OpenIdConnectTheoryData
+                {
+                    OpenIdConnectMetadataFileName = OpenIdConfigData.OpenIdConnectMetadataEnd2EndAcrValuesLast,
+                    SigningCredentials = new SigningCredentials(
+                            KeyingMaterial.RsaSecurityKey_2048,
+                            SecurityAlgorithms.RsaSha256
+                        ),
+                    TestId = "AcrValuesLast",
+                    AdditionalValidation = (OpenIdConnectConfiguration config) =>
+                    {
+                        Assert.Contains("0", config.AcrValuesSupported);
+                        Assert.Contains("id-simple", config.AcrValuesSupported);
+                        Assert.Contains("id-multifactor", config.AcrValuesSupported);
+                    }
+                },
             };
         }
     }

diff --git a/...rotocols.OpenIdConnect.Tests/Microsoft.IdentityModel.Protocols.OpenIdConnect.Tests.csproj b/...rotocols.OpenIdConnect.Tests/Microsoft.IdentityModel.Protocols.OpenIdConnect.Tests.csproj
@@ -12,7 +12,7 @@
   </PropertyGroup>
 
   <ItemGroup>
-    <None Update="JsonWebKeySet.json;JsonWebKeySetBadBase64Data.json;JsonWebKeySetBadX509Data.json;JsonWebKeySetEnd2End.json;JsonWebKeySetSingleX509Data.json;OpenIdConnectMetadata.json;OpenIdConnectMetadata2.json;JsonWebKeySetUnrecognizedKty.json;JsonWebKeySetBadRsaDataMissingComponent.json;OpenIdConnectMetadataBadBase64Data.json;OpenIdConnectMetadataBadX509Data.json;OpenIdConnectMetadataEnd2End.json;OpenIdConnectMetadataJsonWebKeySetBadUri.json;PingLabsJWKS.json;PingLabs-openid-configuration.json;;JsonWebKeySetEnd2EndEC.json;OpenIdConnectMetadataEnd2EndEC.json;OpenIdConnectMetadataUnrecognizedKty.json;OpenIdConnectMetadataBadRsaDataMissingComponent.json">
+    <None Update="JsonWebKeySet.json;JsonWebKeySetBadBase64Data.json;JsonWebKeySetBadX509Data.json;JsonWebKeySetEnd2End.json;JsonWebKeySetSingleX509Data.json;OpenIdConnectMetadata.json;OpenIdConnectMetadata2.json;JsonWebKeySetUnrecognizedKty.json;JsonWebKeySetBadRsaDataMissingComponent.json;OpenIdConnectMetadataBadBase64Data.json;OpenIdConnectMetadataBadX509Data.json;OpenIdConnectMetadataEnd2End.json;OpenIdConnectMetadataJsonWebKeySetBadUri.json;PingLabsJWKS.json;PingLabs-openid-configuration.json;;JsonWebKeySetEnd2EndEC.json;OpenIdConnectMetadataEnd2EndEC.json;OpenIdConnectMetadataUnrecognizedKty.json;OpenIdConnectMetadataBadRsaDataMissingComponent.json;OpenIdConnectMetadataEnd2EndAcrValuesLast.json">
       <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
     </None>
   </ItemGroup>

diff --git a/test/Microsoft.IdentityModel.Protocols.OpenIdConnect.Tests/OpenIdConfigData.cs b/test/Microsoft.IdentityModel.Protocols.OpenIdConnect.Tests/OpenIdConfigData.cs
@@ -170,6 +170,8 @@ public static OpenIdConnectConfiguration FullyPopulatedWithKeys
         public static string OpenIdConnectMetadataBadFormatString = @"{""issuer""::""https://sts.windows.net/d062b2b0-9aca-4ff7-b32a-ba47231a4002/""}";
         public static string OpenIdConnectMetadataPingLabsJWKSString = @"{""jwks_uri"": ""PingLabsJWKS.json""}";
         public static string OpenIdConnectMetatadataBadJson = @"{...";
+
+        public static string OpenIdConnectMetadataEnd2EndAcrValuesLast = "OpenIdConnectMetadataEnd2EndAcrValuesLast.json";
         #endregion
 
         #region WellKnownConfigurationStrings

diff --git a/...dentityModel.Protocols.OpenIdConnect.Tests/OpenIdConnectMetadataEnd2EndAcrValuesLast.json b/...dentityModel.Protocols.OpenIdConnect.Tests/OpenIdConnectMetadataEnd2EndAcrValuesLast.json
@@ -0,0 +1,17 @@
+{
+    "issuer": "https://sts.windows.net/d062b2b0-9aca-4ff7-b32a-ba47231a4002/",
+    "authorization_endpoint": "https://login.windows.net/d062b2b0-9aca-4ff7-b32a-ba47231a4002/oauth2/authorize",
+    "token_endpoint": "https://login.windows.net/d062b2b0-9aca-4ff7-b32a-ba47231a4002/oauth2/token",
+    "token_endpoint_auth_methods_supported": [ "client_secret_post", "private_key_jwt" ],
+    "jwks_uri": "JsonWebKeySetEnd2End.json",
+    "response_types_supported": [ "code", "id_token", "code id_token" ],
+    "response_modes_supported": [ "query", "fragment", "form_post" ],
+    "subject_types_supported": [ "pairwise" ],
+    "scopes_supported": [ "openid" ],
+    "id_token_signing_alg_values_supported": [ "RS256" ],
+    "microsoft_multi_refresh_token": true,
+    "check_session_iframe": "https://login.windows.net/d062b2b0-9aca-4ff7-b32a-ba47231a4002/oauth2/checksession",
+    "end_session_endpoint": "https://login.windows.net/d062b2b0-9aca-4ff7-b32a-ba47231a4002/oauth2/logout",
+    "authorization_details_types_supported": [ "account_information" ],
+    "acr_values_supported": [ "0", "id-simple", "id-multifactor" ]
+}
diff --git a/test/Microsoft.IdentityModel.Protocols.OpenIdConnect.Tests/OpenIdConnectTheoryData.cs b/test/Microsoft.IdentityModel.Protocols.OpenIdConnect.Tests/OpenIdConnectTheoryData.cs
@@ -1,6 +1,7 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT License.
 
+using System;
 using Microsoft.IdentityModel.TestUtils;
 using Microsoft.IdentityModel.Tokens;
 
@@ -22,5 +23,7 @@ public OpenIdConnectTheoryData(string testId) : base(testId) { }
         public string OpenIdConnectMetadataFileName { get; set; }
 
         public SigningCredentials SigningCredentials { get; set; }
+
+        public Action<OpenIdConnectConfiguration> AdditionalValidation { get; set; }
     }
 }