Fix empty x5c header on JwtHeader (#2460)

* Fix empty x5c header on JwtHeader GetStandardClaim is used for first class properties on JwtHeader, one of these first class properties can be an array. Refactor to accomadate this. MultipleX5C test didn't properly exercise the header x5c API. Another test was added to show x5c can be roundtripped as well as that the value is equivalent between being fetched from a JwtSecurityToken or a JsonWebToken --------- Co-authored-by: Keegan Caruso <[email protected]>
AzureAD · Jan 27, 2024 · 2cf55c2 · 2cf55c2
1 parent 8d02bfd
commit 2cf55c2
Show file tree

Hide file tree

Showing 5 changed files with 148 additions and 18 deletions.
diff --git a/src/Microsoft.IdentityModel.Tokens/Json/JsonSerializerPrimitives.cs b/src/Microsoft.IdentityModel.Tokens/Json/JsonSerializerPrimitives.cs
@@ -80,7 +80,7 @@ public static Exception CreateJsonReaderExceptionInvalidType(ref Utf8JsonReader
                     LogHelper.MarkAsNonPII(reader.BytesConsumed)));
         }
 
-        public static JsonElement CreateJsonElement(List<string> strings)
+        public static JsonElement CreateJsonElement(IList<string> strings)
         {
             using (MemoryStream memoryStream = new())
             {

diff --git a/src/Microsoft.IdentityModel.Tokens/LogMessages.cs b/src/Microsoft.IdentityModel.Tokens/LogMessages.cs
@@ -261,6 +261,8 @@ internal static class LogMessages
         public const string IDX11022 = "IDX11022: Expecting json reader to be positioned on '{0}', reader was positioned at: '{1}', Reading: '{2}.{3}', Position: '{4}', CurrentDepth: '{5}', BytesConsumed: '{6}'.";
         public const string IDX11023 = "IDX11023: Expecting json reader to be positioned on '{0}', reader was positioned at: '{1}', Reading: '{2}', Position: '{3}', CurrentDepth: '{4}', BytesConsumed: '{5}'.";
         public const string IDX11025 = "IDX11025: Cannot serialize object of type: '{0}' into property: '{1}'.";
+        public const string IDX11026 = "IDX11026: Unable to get claim value as a string from claim type:'{0}', value type was:'{1}'. Acceptable types are String, IList<String>, and System.Text.Json.JsonElement.";
+
 
 #pragma warning restore 1591
     }

diff --git a/src/System.IdentityModel.Tokens.Jwt/JwtHeader.cs b/src/System.IdentityModel.Tokens.Jwt/JwtHeader.cs
@@ -391,6 +391,36 @@ internal string GetStandardClaim(string claimType)
                 if (value is string str)
                     return str;
 
+                if (value is JsonElement jsonElement)
+                    return jsonElement.ToString();
+                else if (value is IList<string> list)
+                {
+                    JsonElement json = JsonPrimitives.CreateJsonElement(list);
+                    return json.ToString();
+                }
+                else if (value is IList<object> objectList)
+                {
+                    var stringList = new List<string>(objectList.Count);
+                    foreach (object item in objectList)
+                    {
+                        if (item is string strItem)
+                            stringList.Add(strItem);
+                        else
+                        {
+                            // It isn't safe to ToString() an arbitrary object, so we throw here.
+                            // We could end up with a string that doesn't represent the object's value, for example a collection type.
+                            throw LogHelper.LogExceptionMessage(
+                                new JsonException(
+                                    LogHelper.FormatInvariant(
+                                    Microsoft.IdentityModel.Tokens.LogMessages.IDX11026,
+                                    LogHelper.MarkAsNonPII(claimType),
+                                    LogHelper.MarkAsNonPII(item.GetType()))));
+                        }
+                    }
+                    JsonElement json = JsonPrimitives.CreateJsonElement(stringList);
+                    return json.ToString();
+                }
+
                 // TODO - review dev
                 return string.Empty;
             }

diff --git a/test/System.IdentityModel.Tokens.Jwt.Tests/CreateAndValidateTokens.cs b/test/System.IdentityModel.Tokens.Jwt.Tests/CreateAndValidateTokens.cs
@@ -8,7 +8,6 @@
 using System.Text.Json;
 using Microsoft.IdentityModel.TestUtils;
 using Microsoft.IdentityModel.Tokens;
-using Newtonsoft.Json.Linq;
 using Xunit;
 
 namespace System.IdentityModel.Tokens.Jwt.Tests
@@ -42,8 +41,7 @@ public void MultipleX5C()
                     ValidateLifetime = false,
                 };
 
-            SecurityToken validatedSecurityToken = null;
-            var cp = handler.ValidateToken(jwt, validationParameters, out validatedSecurityToken);
+            handler.ValidateToken(jwt, validationParameters, out var validatedSecurityToken);
 
             JwtSecurityToken validatedJwt = validatedSecurityToken as JwtSecurityToken;
             object x5csInHeader = validatedJwt.Header[JwtHeaderParameterNames.X5c];
@@ -62,16 +60,7 @@ public void MultipleX5C()
                 int num = 0;
                 foreach (var str in list)
                 {
-                    var value = str as JValue;
-                    if (value != null)
-                    {
-                        string aud = value.Value as string;
-                        if (aud != null)
-                        {
-
-                        }
-                    }
-                    else if (!(str is string))
+                    if (!(str is string))
                     {
                         errors.Add("3: str is not string, is: " + str.GetType());
                         errors.Add("token : " + validatedJwt.ToString());
@@ -85,20 +74,28 @@ public void MultipleX5C()
                 }
             }
 
+            var serializedX5cs = JsonSerializer.Serialize(x5cs);
+            if (header.X5c != serializedX5cs)
+            {
+                errors.Add("5: header.X5c != serializedX5Cs");
+            }
+
             X509SecurityKey signingKey = KeyingMaterial.X509SecurityKeySelfSigned2048_SHA256;
             X509SecurityKey validateKey = KeyingMaterial.X509SecurityKeySelfSigned2048_SHA256_Public;
 
             // make sure we can still validate with existing logic.
             var signingCredentials = new SigningCredentials(signingKey, SecurityAlgorithms.RsaSha256Signature);
-            header = new JwtHeader(signingCredentials);
-            header.Add(JwtHeaderParameterNames.X5c, x5cs);
+            header = new JwtHeader(signingCredentials)
+            {
+                { JwtHeaderParameterNames.X5c, x5cs }
+            };
+
             jwtToken = new JwtSecurityToken(header, payload);
             jwt = handler.WriteToken(jwtToken);
 
             validationParameters.IssuerSigningKey = validateKey;
             validationParameters.RequireSignedTokens = true;
-            validatedSecurityToken = null;
-            cp = handler.ValidateToken(jwt, validationParameters, out validatedSecurityToken);
+            handler.ValidateToken(jwt, validationParameters, out _);
 
             TestUtilities.AssertFailIfErrors("CreateAndValidateTokens_MultipleX5C", errors);
         }

diff --git a/test/System.IdentityModel.Tokens.Jwt.Tests/JwtHeaderTests.cs b/test/System.IdentityModel.Tokens.Jwt.Tests/JwtHeaderTests.cs
@@ -2,6 +2,10 @@
 // Licensed under the MIT License.
 
 using System.Collections.Generic;
+using System.Security.Cryptography.X509Certificates;
+using System.Text.Encodings.Web;
+using System.Text.Json;
+using Microsoft.IdentityModel.JsonWebTokens;
 using Microsoft.IdentityModel.TestUtils;
 using Microsoft.IdentityModel.Tokens;
 using Xunit;
@@ -138,6 +142,103 @@ public void GetStandardClaimNull()
             var kid = jwtHeader.Kid;
             Assert.True(kid == null);
         }
+
+        [Fact]
+        public void Getx5cDirectlyFromHeader_x5cIsUnsupportedType()
+        {
+            var arrayWithUnsupportedTypes = new List<object>
+            {
+                new List<string>()
+            };
+
+            JwtHeader header = new JwtHeader
+            {
+                { JwtHeaderParameterNames.X5c, arrayWithUnsupportedTypes }
+            };
+
+            var exception = Assert.Throws<JsonException>(() => header.X5c);
+
+            Assert.Contains("IDX11026", exception.Message);
+        }
+
+        [Fact]
+        public void Getx5cDirectlyFromHeader_x5cIsList()
+        {
+            X509Chain ch = new X509Chain();
+            ch.Build(KeyingMaterial.CertSelfSigned1024_SHA256);
+
+            var x5cArray = new List<string>();
+
+            foreach (var element in ch.ChainElements)
+                x5cArray.Add(Convert.ToBase64String(element.Certificate.Export(X509ContentType.Cert)));
+
+            JwtHeader header = new JwtHeader
+            {
+                { JwtHeaderParameterNames.X5c, x5cArray }
+            };
+
+            var expectedX5c = JsonSerializer.Serialize(x5cArray, new JsonSerializerOptions
+            {
+                Encoder = JavaScriptEncoder.UnsafeRelaxedJsonEscaping
+            });
+
+            Assert.Equal(expectedX5c, header.X5c);
+        }
+
+        [Fact]
+        public void Getx5cDirectlyFromHeader_x5cIsJsonElement()
+        {
+            X509Chain ch = new X509Chain();
+            ch.Build(KeyingMaterial.CertSelfSigned1024_SHA256);
+
+            var x5cArray = new List<string>();
+
+            foreach (var element in ch.ChainElements)
+                x5cArray.Add(Convert.ToBase64String(element.Certificate.Export(X509ContentType.Cert)));
+
+            var x5cJsonElement = JsonSerializer.Serialize(x5cArray);
+
+            JwtHeader header = new JwtHeader
+            {
+                { JwtHeaderParameterNames.X5c, x5cJsonElement }
+            };
+
+            var expectedX5c = JsonSerializer.Serialize(x5cArray);
+            Assert.Equal(expectedX5c, header.X5c);
+        }
+
+        [Fact]
+        public void Getx5cRoundTrip()
+        {
+            X509Chain ch = new X509Chain();
+            ch.Build(KeyingMaterial.CertSelfSigned1024_SHA256);
+
+            var x5CArray = new List<string>();
+
+            foreach (var element in ch.ChainElements)
+                x5CArray.Add(Convert.ToBase64String(element.Certificate.Export(X509ContentType.Cert)));
+
+            JwtHeader header = new JwtHeader
+            {
+                { JwtHeaderParameterNames.X5c, x5CArray }
+            };
+
+            var payload = new JwtPayload();
+
+            SecurityToken securityToken = new JwtSecurityToken(header, payload);
+            JwtSecurityTokenHandler tokenHandler = new JwtSecurityTokenHandler();
+            string jwt = tokenHandler.WriteToken(securityToken);
+
+            var jsonWebToken = new JsonWebToken(jwt);
+
+            var x5cFromJsonWebToken = jsonWebToken.Header.GetValue<string>(JwtHeaderParameterNames.X5c);
+
+            JwtSecurityToken token = tokenHandler.ReadJwtToken(jwt);
+
+            string x5CFromJwtSecurityToken = token.Header.X5c;
+            Assert.NotEmpty(x5CFromJwtSecurityToken);
+            Assert.Equal(x5CFromJwtSecurityToken, x5cFromJsonWebToken);
+        }
     }
 
     public class JwtHeaderTheoryData : TheoryDataBase