From a22ab8ebe4ac16760e0daa3b73e7b68a67291497 Mon Sep 17 00:00:00 2001 From: Franco Fung Date: Wed, 15 Nov 2023 19:21:11 +0000 Subject: [PATCH] Merged PR 10603: Re-enable Jwt sub claim as either Number or String This PR re-enables incoming Jwt's to set the 'sub' claim as a Number type in their payloads. Added a new method "ReadStringOrNumberAsString" in the JsonSerializerPrimitives. It will process the 'sub' claim that comes in either as String or Number and will always return it back as a string. Replaced 'sub' claim logic to leverage ReadNumberAsString method in JwtPayload.cs Replaced 'sub' claim logic to leverage ReadNumberAsString method in JsonWebToken.cs Fixes: #2325 ---- #### AI-Generated Description This pull request primarily focuses on modifying the way the `sub` claim is read from JSON payloads in the **JsonWebToken.PayloadClaimSet.cs** and **JwtPayload.cs** files. - In **JsonWebToken.PayloadClaimSet.cs**, the method `JsonSerializerPrimitives.ReadString` has been replaced with `JsonSerializerPrimitives.ReadStringOrNumberAsString` for reading the `sub` claim. This allows the `sub` claim to be read as a string or number, but always returned as a string. - In **JwtPayload.cs**, the logic for reading the `sub` claim has been simplified. The previous code handled multiple token types and threw an exception for unsupported types. The new code directly uses `JsonSerializerPrimitives.ReadStringOrNumberAsString`, simplifying the process. - The **JwtSecurityTokenHandlerTests.cs** file has been updated to remove a duplicate `sub` claim from the test data. - New tests have been added in **JsonWebTokenTests.cs** to validate the reading of the `sub` claim as a string or number. - A new exception type `JsonException` has been added in **ExpectedException.cs** to handle JSON related exceptions. - A new method `ReadStringOrNumberAsString` has been added in **JsonSerializerPrimitives.cs** to read a JSON token as a string or number and always return it as a string. This method is used to read the `sub` claim in the updated files. Related work items: #2753966 --- .../Json/JsonWebToken.PayloadClaimSet.cs | 2 +- .../Json/JsonSerializerPrimitives.cs | 39 +++++- .../JwtPayload.cs | 27 +--- .../JsonWebTokenTests.cs | 126 ++++++++++++++++++ .../ExpectedException.cs | 5 + .../JwtSecurityTokenHandlerTests.cs | 5 - 6 files changed, 167 insertions(+), 37 deletions(-) diff --git a/src/Microsoft.IdentityModel.JsonWebTokens/Json/JsonWebToken.PayloadClaimSet.cs b/src/Microsoft.IdentityModel.JsonWebTokens/Json/JsonWebToken.PayloadClaimSet.cs index 39e415f6bf..24e19225c8 100644 --- a/src/Microsoft.IdentityModel.JsonWebTokens/Json/JsonWebToken.PayloadClaimSet.cs +++ b/src/Microsoft.IdentityModel.JsonWebTokens/Json/JsonWebToken.PayloadClaimSet.cs @@ -89,7 +89,7 @@ internal JsonClaimSet CreatePayloadClaimSet(byte[] bytes, int length) } else if (reader.ValueTextEquals(JwtPayloadUtf8Bytes.Sub)) { - _sub = JsonSerializerPrimitives.ReadString(ref reader, JwtRegisteredClaimNames.Sub, ClassName, true); + _sub = JsonSerializerPrimitives.ReadStringOrNumberAsString(ref reader, JwtRegisteredClaimNames.Sub, ClassName, true); claims[JwtRegisteredClaimNames.Sub] = _sub; } else diff --git a/src/Microsoft.IdentityModel.Tokens/Json/JsonSerializerPrimitives.cs b/src/Microsoft.IdentityModel.Tokens/Json/JsonSerializerPrimitives.cs index 35617d12b5..56f415a2be 100644 --- a/src/Microsoft.IdentityModel.Tokens/Json/JsonSerializerPrimitives.cs +++ b/src/Microsoft.IdentityModel.Tokens/Json/JsonSerializerPrimitives.cs @@ -648,6 +648,33 @@ internal static string ReadString(ref Utf8JsonReader reader, string propertyName return reader.GetString(); } + /// + /// This method allows a JsonTokenType to be string or number but, it will always return it as a string. + /// + /// The + /// The property name that is being read. + /// The type that is being deserialized. + /// If true reader.Read() will be called. + /// Value from reader as string. + internal static string ReadStringOrNumberAsString(ref Utf8JsonReader reader, string propertyName, string className, bool read = false) + { + if (read) + reader.Read(); + + // returning null keeps the same logic as JsonSerialization.ReadObject + if (reader.TokenType == JsonTokenType.Null) + return null; + + if (reader.TokenType == JsonTokenType.Number) + return ReadNumber(ref reader).ToString(); + + if (reader.TokenType != JsonTokenType.String) + throw LogHelper.LogExceptionMessage( + CreateJsonReaderException(ref reader, "JsonTokenType.String or JsonTokenType.Number", className, propertyName)); + + return reader.GetString(); + } + internal static object ReadStringAsObject(ref Utf8JsonReader reader, string propertyName, string className, bool read = false) { if (read) @@ -771,13 +798,13 @@ internal static void ReadStringsSkipNulls( /// /// This method is called when deserializing a property value as an object. - /// Normally we put the object into a Dictionary[string, object]. + /// Normally, we put the object into a Dictionary[string, object]. /// - /// the - /// the property name that is being read - /// the type that is being deserialized - /// if true reader.Read() will be called. - /// + /// The + /// The property name that is being read. + /// The type that is being deserialized. + /// If true reader.Read() will be called. + /// Value from reader as an object. internal static object ReadPropertyValueAsObject(ref Utf8JsonReader reader, string propertyName, string className, bool read = false) { if (read) diff --git a/src/System.IdentityModel.Tokens.Jwt/JwtPayload.cs b/src/System.IdentityModel.Tokens.Jwt/JwtPayload.cs index d89bf96874..a585f2f667 100644 --- a/src/System.IdentityModel.Tokens.Jwt/JwtPayload.cs +++ b/src/System.IdentityModel.Tokens.Jwt/JwtPayload.cs @@ -114,31 +114,8 @@ internal static JwtPayload CreatePayload(byte[] bytes, int length) } else if (reader.ValueTextEquals(JwtPayloadUtf8Bytes.Sub)) { - reader.Read(); - if (reader.TokenType == JsonTokenType.String) - { - payload._sub = JsonSerializerPrimitives.ReadString(ref reader, JwtRegisteredClaimNames.Sub, ClassName, false); - payload[JwtRegisteredClaimNames.Sub] = payload._sub; - } - else if (reader.TokenType == JsonTokenType.StartArray) - { - payload._audiences = new List(); - JsonSerializerPrimitives.ReadStrings(ref reader, payload._audiences, JwtRegisteredClaimNames.Sub, ClassName, false); - payload[JwtRegisteredClaimNames.Sub] = payload._audiences; - } - else - { - throw LogHelper.LogExceptionMessage( - new JsonException( - LogHelper.FormatInvariant( - Microsoft.IdentityModel.Tokens.LogMessages.IDX11023, - LogHelper.MarkAsNonPII("JsonTokenType.String or JsonTokenType.StartArray"), - LogHelper.MarkAsNonPII(reader.TokenType), - LogHelper.MarkAsNonPII(ClassName), - LogHelper.MarkAsNonPII(reader.TokenStartIndex), - LogHelper.MarkAsNonPII(reader.CurrentDepth), - LogHelper.MarkAsNonPII(reader.BytesConsumed)))); - } + payload._sub = JsonSerializerPrimitives.ReadStringOrNumberAsString(ref reader, JwtRegisteredClaimNames.Sub, ClassName, true); + payload[JwtRegisteredClaimNames.Sub] = payload._sub; } else { diff --git a/test/Microsoft.IdentityModel.JsonWebTokens.Tests/JsonWebTokenTests.cs b/test/Microsoft.IdentityModel.JsonWebTokens.Tests/JsonWebTokenTests.cs index a20ef8c3ef..d03c77c771 100644 --- a/test/Microsoft.IdentityModel.JsonWebTokens.Tests/JsonWebTokenTests.cs +++ b/test/Microsoft.IdentityModel.JsonWebTokens.Tests/JsonWebTokenTests.cs @@ -12,6 +12,7 @@ using System.Reflection; using System.Security.Claims; using System.Text; +using System.Text.Json; using Microsoft.IdentityModel.TestUtils; using Microsoft.IdentityModel.Tokens; using Microsoft.IdentityModel.Tokens.Json.Tests; @@ -603,6 +604,131 @@ public void TryGetPayloadValue(GetPayloadValueTheoryData theoryData) TestUtilities.AssertFailIfErrors(context); } + + [Theory, MemberData(nameof(GetPayloadSubClaimValueTheoryData), DisableDiscoveryEnumeration = true)] + public void GetPayloadSubClaimValue(GetPayloadValueTheoryData theoryData) + { + CompareContext context = TestUtilities.WriteHeader($"{this}.GetPayloadSubClaimValue", theoryData); + try + { + JsonWebToken jsonWebToken = new JsonWebToken(theoryData.Json); + string payload = Base64UrlEncoder.Decode(jsonWebToken.EncodedPayload); + MethodInfo method = typeof(JsonWebToken).GetMethod("GetPayloadValue"); + MethodInfo generic = method.MakeGenericMethod(theoryData.PropertyType); + object[] parameters = new object[] { theoryData.PropertyName }; + var retVal = generic.Invoke(jsonWebToken, parameters); + + theoryData.ExpectedException.ProcessNoException(context); + IdentityComparer.AreEqual(retVal, theoryData.PropertyValue, context); + } + catch (Exception ex) + { + theoryData.ExpectedException.ProcessException(ex.InnerException, context); + } + + TestUtilities.AssertFailIfErrors(context); + } + + public static TheoryData GetPayloadSubClaimValueTheoryData + { + get + { + var theoryData = new TheoryData(); + string[] stringArray = new string[] { "string1", "string2" }; + object propertyValue = new Dictionary { { "stringArray", stringArray } }; + + theoryData.Add(new GetPayloadValueTheoryData("SubjectAsString") + { + PropertyName = "sub", + PropertyType = typeof(string), + PropertyValue = null, + Json = JsonUtilities.CreateUnsignedToken("sub", null) + }); + theoryData.Add(new GetPayloadValueTheoryData("SubjectAsBoolTrue") + { + PropertyName = "sub", + PropertyType = typeof(bool), + PropertyValue = true, + Json = JsonUtilities.CreateUnsignedToken("sub", true), + ExpectedException = ExpectedException.JsonException("IDX11020:") + }); + theoryData.Add(new GetPayloadValueTheoryData("SubjectAsBoolFalse") + { + PropertyName = "sub", + PropertyType = typeof(bool), + PropertyValue = false, + Json = JsonUtilities.CreateUnsignedToken("sub", false), + ExpectedException = ExpectedException.JsonException("IDX11020:") + }); + + theoryData.Add(new GetPayloadValueTheoryData("SubjectAsArray") + { + PropertyName = "sub", + PropertyType = typeof(string[]), + PropertyValue = stringArray, + Json = JsonUtilities.CreateUnsignedToken("sub", stringArray), + ExpectedException = ExpectedException.JsonException("IDX11020:") + }); + theoryData.Add(new GetPayloadValueTheoryData("SubjectAsObject") + { + PropertyName = "sub", + PropertyType = typeof(object), + PropertyValue = propertyValue, + Json = JsonUtilities.CreateUnsignedToken("sub", propertyValue), + ExpectedException = ExpectedException.JsonException("IDX11020:") + }); + theoryData.Add(new GetPayloadValueTheoryData("SubjectAsDouble") + { + PropertyName = "sub", + PropertyType = typeof(double), + PropertyValue = 622.101, + Json = JsonUtilities.CreateUnsignedToken("sub", 622.101) + }); + + theoryData.Add(new GetPayloadValueTheoryData("SubjectAsDecimal") + { + PropertyName = "sub", + PropertyType = typeof(decimal), + PropertyValue = 422.101, + Json = JsonUtilities.CreateUnsignedToken("sub", 422.101) + }); + + theoryData.Add(new GetPayloadValueTheoryData("SubjectAsFloat") + { + PropertyName = "sub", + PropertyType = typeof(float), + PropertyValue = 42.1, + Json = JsonUtilities.CreateUnsignedToken("sub", 42.1) + }); + + theoryData.Add(new GetPayloadValueTheoryData("SubjectAsInteger") + { + PropertyName = "sub", + PropertyType = typeof(int), + PropertyValue = 42, + Json = JsonUtilities.CreateUnsignedToken("sub", 42) + }); + + theoryData.Add(new GetPayloadValueTheoryData("SubjectAsUInt") + { + PropertyName = "sub", + PropertyType = typeof(uint), + PropertyValue = 540, + Json = JsonUtilities.CreateUnsignedToken("sub", 540) + }); + + theoryData.Add(new GetPayloadValueTheoryData("SubjectAsUlong") + { + PropertyName = "sub", + PropertyType = typeof(ulong), + PropertyValue = 642, + Json = JsonUtilities.CreateUnsignedToken("sub", 642) + }); + + return theoryData; + } + + } // This test ensures that accessing claims from the payload works as expected. [Theory, MemberData(nameof(GetPayloadValueTheoryData), DisableDiscoveryEnumeration = true)] public void GetPayloadValue(GetPayloadValueTheoryData theoryData) diff --git a/test/Microsoft.IdentityModel.TestUtils/ExpectedException.cs b/test/Microsoft.IdentityModel.TestUtils/ExpectedException.cs index fa8d3d412f..eda53e344f 100644 --- a/test/Microsoft.IdentityModel.TestUtils/ExpectedException.cs +++ b/test/Microsoft.IdentityModel.TestUtils/ExpectedException.cs @@ -6,6 +6,7 @@ using System.IO; using System.Reflection; using System.Security.Cryptography; +using System.Text.Json; using System.Xml; using Microsoft.IdentityModel.Tokens; @@ -304,6 +305,10 @@ public static ExpectedException KeyWrapException(string substringExpected = null return new ExpectedException(typeof(SecurityTokenKeyWrapException), substringExpected, innerTypeExpected); } + public static ExpectedException JsonException(string substringExpected = null, Type innerTypeExpected = null) + { + return new ExpectedException(typeof(JsonException), substringExpected, innerTypeExpected); + } public bool IgnoreExceptionType { get; set; } = false; public bool IgnoreInnerException { get; set; } diff --git a/test/System.IdentityModel.Tokens.Jwt.Tests/JwtSecurityTokenHandlerTests.cs b/test/System.IdentityModel.Tokens.Jwt.Tests/JwtSecurityTokenHandlerTests.cs index f5f3deaf18..c0867e844b 100644 --- a/test/System.IdentityModel.Tokens.Jwt.Tests/JwtSecurityTokenHandlerTests.cs +++ b/test/System.IdentityModel.Tokens.Jwt.Tests/JwtSecurityTokenHandlerTests.cs @@ -874,7 +874,6 @@ public void InboundOutboundClaimTypeMapping() new Claim( ClaimTypes.Spn, "spn", ClaimValueTypes.String, Default.Issuer, Default.Issuer ), new Claim( JwtRegisteredClaimNames.Sub, "Subject1", ClaimValueTypes.String, Default.Issuer, Default.Issuer ), new Claim( JwtRegisteredClaimNames.Prn, "Principal1", ClaimValueTypes.String, Default.Issuer, Default.Issuer ), - new Claim( JwtRegisteredClaimNames.Sub, "Subject2", ClaimValueTypes.String, Default.Issuer, Default.Issuer ), }; @@ -912,10 +911,6 @@ public void InboundOutboundClaimTypeMapping() claim.Properties.Add(new KeyValuePair(JwtSecurityTokenHandler.ShortClaimTypeProperty, JwtRegisteredClaimNames.Prn)); expectedClaims.Add(claim); - claim = new Claim("Mapped_" + JwtRegisteredClaimNames.Sub, "Subject2", ClaimValueTypes.String, Default.Issuer, Default.Issuer); - claim.Properties.Add(new KeyValuePair(JwtSecurityTokenHandler.ShortClaimTypeProperty, JwtRegisteredClaimNames.Sub)); - expectedClaims.Add(claim); - RunClaimMappingVariation(jwt, handler, validationParameters, expectedClaims: expectedClaims, identityName: null); }