JsonWebKeySet stores the String it was created with (#2755)

* store original string, edit tests * create string for test * ignore creation string in serialization tests * fix tests using jwks * add justifying comments for compare ignores * add jsonignore to new property * rename * rename in ignores * test improvements * Update codeql-analysis.yml * Revert "Update codeql-analysis.yml" This reverts commit 667ad73. * Update src/Microsoft.IdentityModel.Tokens/JsonWebKeySet.cs Co-authored-by: Keegan Caruso <Keegan.Caruso@microsoft.com> * Update test/Microsoft.IdentityModel.Tokens.Tests/Json/JsonWebKeySetTheoryData.cs Co-authored-by: Keegan Caruso <Keegan.Caruso@microsoft.com> * Update src/Microsoft.IdentityModel.Tokens/JsonWebKeySet.cs Co-authored-by: Keegan Caruso <Keegan.Caruso@microsoft.com> --------- Co-authored-by: Keegan Caruso <Keegan.Caruso@microsoft.com>
AzureAD · Aug 6, 2024 · 12fe1ec · 12fe1ec
1 parent 46b1fd2
commit 12fe1ec
Show file tree

Hide file tree

Showing 5 changed files with 61 additions and 6 deletions.
diff --git a/src/Microsoft.IdentityModel.Tokens/JsonWebKeySet.cs b/src/Microsoft.IdentityModel.Tokens/JsonWebKeySet.cs
@@ -20,6 +20,7 @@ public class JsonWebKeySet
     {
         internal const string ClassName = "Microsoft.IdentityModel.Tokens.JsonWebKeySet";
         private Dictionary<string, object> _additionalData;
+        private string _jsonData = string.Empty;
 
         /// <summary>
         /// Returns a new instance of <see cref="JsonWebKeySet"/>.
@@ -54,6 +55,8 @@ public JsonWebKeySet(string json)
             if (string.IsNullOrEmpty(json))
                 throw LogHelper.LogArgumentNullException(nameof(json));
 
+            _jsonData = json;
+
             try
             {
                 if (LogHelper.IsEnabled(EventLogLevel.Verbose))
@@ -97,6 +100,22 @@ public JsonWebKeySet(string json)
         [JsonIgnore]
         public bool SkipUnresolvedJsonWebKeys { get; set; } = DefaultSkipUnresolvedJsonWebKeys;
 
+        /// <summary>
+        /// The original string used to create this instance if a string was provided.
+        /// </summary>
+        [JsonIgnore]
+        internal string JsonData
+        {
+            get
+            {
+                return _jsonData;
+            }
+            set
+            {
+                _jsonData = value;
+            }
+        }
+
         /// <summary>
         /// Returns the JsonWebKeys as a <see cref="IList{SecurityKey}"/>.
         /// </summary>

diff --git a/...t.IdentityModel.Protocols.OpenIdConnect.Tests/OpenIdConnectConfigurationRetrieverTests.cs b/...t.IdentityModel.Protocols.OpenIdConnect.Tests/OpenIdConnectConfigurationRetrieverTests.cs
@@ -2,12 +2,14 @@
 // Licensed under the MIT License.
 
 using System;
+using System.Collections.Generic;
 using System.IO;
 using System.Net.Http;
 using System.Security.Cryptography;
 using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.IdentityModel.TestUtils;
+using Microsoft.IdentityModel.Tokens;
 using Xunit;
 
 namespace Microsoft.IdentityModel.Protocols.OpenIdConnect.Tests
@@ -28,7 +30,15 @@ public async Task FromNetwork()
         [Fact]
         public async Task FromFile()
         {
-            var context = new CompareContext();
+            var context = new CompareContext
+            {
+                PropertiesToIgnoreWhenComparing = new Dictionary<Type, List<string>>
+                {
+                    // If the objects being compared are created from the same string and they are equal, the string itself can be ignored.
+                    // The strings may not be equal because of whitespace, but the json they represent is semantically identical.
+                    { typeof(JsonWebKeySet), [ "JsonData" ] },
+                }
+            };
             var configuration = await GetConfigurationAsync(
                 OpenIdConfigData.JsonFile,
                 ExpectedException.NoExceptionExpected,
@@ -52,7 +62,15 @@ public async Task FromFile()
         [Fact]
         public async Task FromJson()
         {
-            var context = new CompareContext();
+            var context = new CompareContext
+            {
+                PropertiesToIgnoreWhenComparing = new Dictionary<Type, List<string>>
+                {
+                    // If the objects being compared are created from the same string and they are equal, the string itself can be ignored.
+                    // The strings may not be equal because of whitespace, but the json they represent is semantically identical.
+                    { typeof(JsonWebKeySet), [ "JsonData" ] },
+                }
+            };
             var configuration = await GetConfigurationFromMixedAsync(
                 OpenIdConfigData.OpenIdConnectMetadataPingString,
                 expectedException: ExpectedException.NoExceptionExpected);

diff --git a/test/Microsoft.IdentityModel.Tokens.Tests/Json/DataSets.cs b/test/Microsoft.IdentityModel.Tokens.Tests/Json/DataSets.cs
@@ -729,9 +729,7 @@ public static JsonWebKeySet JsonWebKeySet1
         {
             get
             {
-                JsonWebKeySet jsonWebKeySet = new JsonWebKeySet();
-                jsonWebKeySet.Keys.Add(JsonWebKey1);
-                jsonWebKeySet.Keys.Add(JsonWebKey2);
+                JsonWebKeySet jsonWebKeySet = new JsonWebKeySet(JsonWebKeySetString1);
 
                 return jsonWebKeySet;
             }
@@ -753,6 +751,7 @@ public static JsonWebKeySet JsonWebKeySetX509Data
 
                 JsonWebKeySet jsonWebKeySet = new JsonWebKeySet();
                 jsonWebKeySet.Keys.Add(jsonWebKey);
+                jsonWebKeySet.JsonData = JsonWebKeySetX509DataString;
 
                 return jsonWebKeySet;
             }
@@ -766,6 +765,7 @@ public static JsonWebKeySet JsonWebKeySetEC
                 jsonWebKeySet.Keys.Add(JsonWebKeyES256);
                 jsonWebKeySet.Keys.Add(JsonWebKeyES384);
                 jsonWebKeySet.Keys.Add(JsonWebKeyES512);
+                jsonWebKeySet.JsonData = JsonWebKeySetECCString;
 
                 return jsonWebKeySet;
             }
@@ -785,6 +785,7 @@ public static JsonWebKeySet JsonWebKeySetOnlyX5t
 
                 JsonWebKeySet jsonWebKeySet = new JsonWebKeySet();
                 jsonWebKeySet.Keys.Add(jsonWebKey);
+                jsonWebKeySet.JsonData = JsonWebKeySetOnlyX5tString;
 
                 return jsonWebKeySet;
             }

diff --git a/test/Microsoft.IdentityModel.Tokens.Tests/Json/JsonWebKeySetSerializationTests.cs b/test/Microsoft.IdentityModel.Tokens.Tests/Json/JsonWebKeySetSerializationTests.cs
@@ -2,6 +2,7 @@
 // Licensed under the MIT License.
 
 using System;
+using System.Collections.Generic;
 using Microsoft.IdentityModel.TestUtils;
 using Xunit;
 
@@ -17,6 +18,12 @@ public class JsonWebKeySetSerializationTests
         public void Serialize(JsonWebKeySetTheoryData theoryData)
         {
             var context = TestUtilities.WriteHeader($"{this}.Serialize", theoryData);
+            context.PropertiesToIgnoreWhenComparing = new Dictionary<Type, List<string>>
+            {
+                // If the objects being compared are created from the same string and they are equal, the string itself can be ignored.
+                // The strings may not be equal because of whitespace, but the json they represent is semantically identical.
+                { typeof(JsonWebKeySet), [ "JsonData" ] },
+            };
 
             try
             {
@@ -29,7 +36,7 @@ public void Serialize(JsonWebKeySetTheoryData theoryData)
                 // compare our utf8Reader with expected value
                 if (!IdentityComparer.AreEqual(jsonWebKeySetUtf8Reader, theoryData.JsonWebKeySet, context))
                 {
-                    context.Diffs.Add("jsonWebKeySetUtf8Reader != theoryData.JsonWebKeySet1");
+                    context.Diffs.Add("jsonWebKeySetUtf8Reader != theoryData.JsonWebKeySet");
                     context.Diffs.Add("=========================================");
                 }
             }
@@ -60,6 +67,12 @@ public static TheoryData<JsonWebKeySetTheoryData> SerializeDataSet
         public void Deserialize(JsonWebKeySetTheoryData theoryData)
         {
             var context = TestUtilities.WriteHeader($"{this}.Deserialize", theoryData);
+            context.PropertiesToIgnoreWhenComparing = new Dictionary<Type, List<string>>
+            {
+                // If the objects being compared are created from the same string and they are equal, the string itself can be ignored.
+                // The strings may not be equal because of whitespace, but the json they represent is semantically identical.
+                { typeof(JsonWebKeySet), [ "JsonData" ] },
+            };
 
             try
             {

diff --git a/test/Microsoft.IdentityModel.Tokens.Tests/JsonWebKeySetTests.cs b/test/Microsoft.IdentityModel.Tokens.Tests/JsonWebKeySetTests.cs
@@ -23,7 +23,11 @@ public void Constructors(JsonWebKeySetTheoryData theoryData)
             {
                 var jsonWebKeys = new JsonWebKeySet(theoryData.Json);
                 var keys = jsonWebKeys.GetSigningKeys();
+                var originalString = jsonWebKeys.JsonData;
                 theoryData.ExpectedException.ProcessNoException(context);
+
+                IdentityComparer.AreStringsEqual(originalString, theoryData.Json, context);
+
                 if (theoryData.JsonWebKeySet != null)
                     IdentityComparer.AreEqual(jsonWebKeys, theoryData.JsonWebKeySet, context);