reverted to skipping audiences strings that are null

AzureAD · Sep 8, 2023 · 675a7fc · 675a7fc
1 parent 76aa676
commit 675a7fc
Show file tree

Hide file tree

Showing 3 changed files with 94 additions and 2 deletions.
diff --git a/src/Microsoft.IdentityModel.JsonWebTokens/Json/JsonWebToken.PayloadClaimSet.cs b/src/Microsoft.IdentityModel.JsonWebTokens/Json/JsonWebToken.PayloadClaimSet.cs
@@ -38,12 +38,14 @@ internal JsonClaimSet CreatePayloadClaimSet(byte[] bytes, int length)
                         reader.Read();
                         if (reader.TokenType == JsonTokenType.StartArray)
                         {
-                            JsonSerializerPrimitives.ReadStrings(ref reader, _audiences, JwtRegisteredClaimNames.Aud, ClassName, false);
+                            JsonSerializerPrimitives.ReadStringsSkipNulls(ref reader, _audiences, JwtRegisteredClaimNames.Aud, ClassName);
                             claims[JwtRegisteredClaimNames.Aud] = _audiences;
                         }
                         else
                         {
-                            _audiences.Add(JsonSerializerPrimitives.ReadString(ref reader, JwtRegisteredClaimNames.Aud, ClassName, false));
+                            if (reader.TokenType != JsonTokenType.Null)
+                                _audiences.Add(JsonSerializerPrimitives.ReadString(ref reader, JwtRegisteredClaimNames.Aud, ClassName));
+
                             claims[JwtRegisteredClaimNames.Aud] = _audiences;
                         }
                     }

diff --git a/src/Microsoft.IdentityModel.Tokens/Json/JsonSerializerPrimitives.cs b/src/Microsoft.IdentityModel.Tokens/Json/JsonSerializerPrimitives.cs
@@ -741,6 +741,34 @@ internal static object ReadStringAsObject(ref Utf8JsonReader reader, string prop
             return strings;
         }
 
+        // This is a special case for reading audiences where in 6x, we didn't add null strings to the list.
+        internal static void ReadStringsSkipNulls(
+            ref Utf8JsonReader reader,
+            List<string> strings,
+            string propertyName,
+            string className)
+        {
+            if (reader.TokenType == JsonTokenType.Null)
+                return;
+
+            if (!IsReaderAtTokenType(ref reader, JsonTokenType.StartArray, false))
+                throw LogHelper.LogExceptionMessage(
+                    CreateJsonReaderExceptionInvalidType(ref reader, "JsonTokenType.StartArray", className, propertyName));
+
+            while (reader.Read())
+            {
+                if (IsReaderAtTokenType(ref reader, JsonTokenType.EndArray, false))
+                    break;
+
+                if (reader.TokenType == JsonTokenType.Null)
+                    continue;
+
+                strings.Add(ReadString(ref reader, propertyName, className));
+            }
+
+            return;
+        }
+
         /// <summary>
         /// This method is called when deserializing a property value as an object.
         /// Normally we put the object into a Dictionary[string, object].
@@ -909,6 +937,12 @@ public static void WriteObjectValue(ref Utf8JsonWriter writer, object obj)
                     LogHelper.MarkAsNonPII(writer.CurrentDepth),
                     LogHelper.MarkAsNonPII(MaxDepth)));
 
+            if (obj is null)
+            {
+                writer.WriteNullValue();
+                return;
+            }
+
             Type objType = obj.GetType();
 
             if (obj is string str)

diff --git a/test/Microsoft.IdentityModel.JsonWebTokens.Tests/JsonWebTokenTests.cs b/test/Microsoft.IdentityModel.JsonWebTokens.Tests/JsonWebTokenTests.cs
@@ -239,6 +239,62 @@ public void CompareJwtSecurityTokenWithJsonSecurityTokenMultipleAudiences()
             TestUtilities.AssertFailIfErrors(context);
         }
 
+        // This test ensures audience values are skipping nulls as expected.
+        [Theory, MemberData(nameof(CheckAudienceValuesTheoryData), DisableDiscoveryEnumeration = true)]
+        public void CheckAudienceValues(GetPayloadValueTheoryData theoryData)
+        {
+            CompareContext context = TestUtilities.WriteHeader($"{this}.CheckAudienceValues", theoryData);
+            try
+            {
+                JsonWebToken jsonWebToken = new JsonWebToken(theoryData.Json);
+                IdentityComparer.AreEqual(jsonWebToken.Audiences, theoryData.PropertyValue, context);
+            }
+            catch (Exception ex)
+            {
+                theoryData.ExpectedException.ProcessException(ex.InnerException, context);
+            }
+
+            TestUtilities.AssertFailIfErrors(context);
+        }
+
+        public static TheoryData<GetPayloadValueTheoryData> CheckAudienceValuesTheoryData
+        {
+            get
+            {
+                var theoryData = new TheoryData<GetPayloadValueTheoryData>();
+
+                theoryData.Add(new GetPayloadValueTheoryData("singleNull")
+                {
+                    PropertyName = "aud",
+                    PropertyValue = new List<string>(),
+                    Json = JsonUtilities.CreateUnsignedToken("aud", null)
+                });
+
+                theoryData.Add(new GetPayloadValueTheoryData("twoNull")
+                {
+                    PropertyName = "aud",
+                    PropertyValue = new List<string>(),
+                    Json = JsonUtilities.CreateUnsignedToken("aud", new List<string>{ null, null })
+                });
+
+                theoryData.Add(new GetPayloadValueTheoryData("singleNonNull")
+                {
+                    PropertyName = "aud",
+                    PropertyValue = new List<string> { "audience"},
+                    Json = JsonUtilities.CreateUnsignedToken("aud", "audience")
+                });
+
+                theoryData.Add(new GetPayloadValueTheoryData("twoNulloneNonNull")
+                {
+                    PropertyName = "aud",
+                    PropertyValue = new List<string> { "audience1"},
+                    Json = JsonUtilities.CreateUnsignedToken("aud", new List<string> { null, "audience1", null })
+                });
+
+                return theoryData;
+            }
+        }
+
         // This test ensures that TryGetPayloadValue does not throw
         // No need to check for equal as GetPayloadValue does that
         [Theory, MemberData(nameof(GetPayloadValueTheoryData), DisableDiscoveryEnumeration = true)]