Skip to content

Commit

Permalink
Merged PR 10603: Re-enable Jwt sub claim as either Number or String
Browse files Browse the repository at this point in the history
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
  • Loading branch information
Franco Fung committed Nov 15, 2023
1 parent 1966c05 commit a22ab8e
Show file tree
Hide file tree
Showing 6 changed files with 167 additions and 37 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -648,6 +648,33 @@ internal static string ReadString(ref Utf8JsonReader reader, string propertyName
return reader.GetString();
}

/// <summary>
/// This method allows a JsonTokenType to be string or number but, it will always return it as a string.
/// </summary>
/// <param name="reader">The<see cref="Utf8JsonReader"/></param>
/// <param name="propertyName">The property name that is being read.</param>
/// <param name="className">The type that is being deserialized.</param>
/// <param name="read">If true reader.Read() will be called.</param>
/// <returns>Value from reader as string.</returns>
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)
Expand Down Expand Up @@ -771,13 +798,13 @@ internal static void ReadStringsSkipNulls(

/// <summary>
/// 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].
/// </summary>
/// <param name="reader">the <see cref="Utf8JsonReader"/></param>
/// <param name="propertyName">the property name that is being read</param>
/// <param name="className">the type that is being deserialized</param>
/// <param name="read">if true reader.Read() will be called.</param>
/// <returns></returns>
/// <param name="reader">The <see cref="Utf8JsonReader"/></param>
/// <param name="propertyName">The property name that is being read.</param>
/// <param name="className">The type that is being deserialized.</param>
/// <param name="read">If true reader.Read() will be called.</param>
/// <returns>Value from reader as an object.</returns>
internal static object ReadPropertyValueAsObject(ref Utf8JsonReader reader, string propertyName, string className, bool read = false)
{
if (read)
Expand Down
27 changes: 2 additions & 25 deletions src/System.IdentityModel.Tokens.Jwt/JwtPayload.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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<string>();
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
{
Expand Down
126 changes: 126 additions & 0 deletions test/Microsoft.IdentityModel.JsonWebTokens.Tests/JsonWebTokenTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<GetPayloadValueTheoryData> GetPayloadSubClaimValueTheoryData
{
get
{
var theoryData = new TheoryData<GetPayloadValueTheoryData>();
string[] stringArray = new string[] { "string1", "string2" };
object propertyValue = new Dictionary<string, string[]> { { "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)
Expand Down
5 changes: 5 additions & 0 deletions test/Microsoft.IdentityModel.TestUtils/ExpectedException.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 ),
};


Expand Down Expand Up @@ -912,10 +911,6 @@ public void InboundOutboundClaimTypeMapping()
claim.Properties.Add(new KeyValuePair<string, string>(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<string, string>(JwtSecurityTokenHandler.ShortClaimTypeProperty, JwtRegisteredClaimNames.Sub));
expectedClaims.Add(claim);

RunClaimMappingVariation(jwt, handler, validationParameters, expectedClaims: expectedClaims, identityName: null);
}

Expand Down

0 comments on commit a22ab8e

Please sign in to comment.