Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't always test string claims for Date Time #2622

Merged
merged 4 commits into from
Jun 13, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 37 additions & 0 deletions benchmark/Microsoft.IdentityModel.Benchmarks/BenchmarkUtils.cs
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,43 @@ public static Dictionary<string, object> Claims
}
}

public static Dictionary<string, object> ClaimsExtendedExample
{
get
{
DateTime now = DateTime.UtcNow;
return new Dictionary<string, object>()
{
{ "acct", 0 },
{ "aio", Guid.NewGuid().ToString() },
{ "amr", new List<string>() { "pwd", "mfa" } },
{ "app_displayname", "MyApp" },
{ "appidacr", 0 },
{ "azpacr", 0 },
{ "azp", Guid.NewGuid().ToString() },
{ "groups", new List<string>() { "group1", "group2" } },
{ "name", "Bob" },
{ "oid", Guid.NewGuid().ToString() },
{ "rh", Guid.NewGuid().ToString() },
{ "scp", "access_as_user" },
{ JwtRegisteredClaimNames.Sub, Guid.NewGuid().ToString() },
{ "tid", Guid.NewGuid().ToString() },
{ "family_name", "Smith" },
{ "ver", "2.0" },
{ "wids", new List<string>() { Guid.NewGuid().ToString() } },
{ "xms_cc", Guid.NewGuid().ToString() },
{ "role", new List<string>() { "role1", "Developer", "Sales"} },
{ JwtRegisteredClaimNames.Email, "Bob@contoso.com" },
{ JwtRegisteredClaimNames.Exp, EpochTime.GetIntDate(now + TimeSpan.FromDays(1)) },
{ JwtRegisteredClaimNames.Nbf, EpochTime.GetIntDate(now) },
{ JwtRegisteredClaimNames.Iat, EpochTime.GetIntDate(now) },
{ JwtRegisteredClaimNames.GivenName, "Bob" },
{ JwtRegisteredClaimNames.Iss, Issuer },
{ JwtRegisteredClaimNames.Aud, Audience }
};
}
}

public static SigningCredentials SigningCredentialsRsaSha256 => new(RsaSecurityKey, SecurityAlgorithms.RsaSha256, SecurityAlgorithms.Sha256);

public static EncryptingCredentials EncryptingCredentialsAes256Sha512 => new(SymmetricEncryptionKey512, "dir", SecurityAlgorithms.Aes256CbcHmacSha512);
Expand Down
1 change: 1 addition & 0 deletions benchmark/Microsoft.IdentityModel.Benchmarks/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ private static void DebugThroughTests()
ValidateTokenAsyncTests validateTokenAsyncTests = new ValidateTokenAsyncTests();
validateTokenAsyncTests.Setup();
TokenValidationResult tokenValidationResult = validateTokenAsyncTests.JsonWebTokenHandler_ValidateTokenAsync().Result;
var claims = validateTokenAsyncTests.JsonWebTokenHandler_ValidateTokenAsync_CreateClaims();

ValidateSignedHttpRequestAsyncTests validateSignedHttpRequestAsyncTests = new ValidateSignedHttpRequestAsyncTests();
validateSignedHttpRequestAsyncTests.Setup();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

using System.Collections.Generic;
using System.IdentityModel.Tokens.Jwt;
using System.Linq;
using System.Security.Claims;
using System.Threading.Tasks;
using BenchmarkDotNet.Attributes;
using Microsoft.IdentityModel.JsonWebTokens;
Expand All @@ -16,7 +19,9 @@ public class ValidateTokenAsyncTests
private JsonWebTokenHandler _jsonWebTokenHandler;
private JwtSecurityTokenHandler _jwtSecurityTokenHandler;
private SecurityTokenDescriptor _tokenDescriptor;
private SecurityTokenDescriptor _tokenDescriptorExtendedClaims;
private string _jws;
private string _jwsExtendedClaims;
private TokenValidationParameters _validationParameters;

[GlobalSetup]
Expand All @@ -28,8 +33,15 @@ public void Setup()
SigningCredentials = BenchmarkUtils.SigningCredentialsRsaSha256,
};

_tokenDescriptorExtendedClaims = new SecurityTokenDescriptor
{
Claims = BenchmarkUtils.ClaimsExtendedExample,
SigningCredentials = BenchmarkUtils.SigningCredentialsRsaSha256,
};

_jsonWebTokenHandler = new JsonWebTokenHandler();
_jws = _jsonWebTokenHandler.CreateToken(_tokenDescriptor);
_jwsExtendedClaims = _jsonWebTokenHandler.CreateToken(_tokenDescriptorExtendedClaims);

_jwtSecurityTokenHandler = new JwtSecurityTokenHandler();
_jwtSecurityTokenHandler.SetDefaultTimesOnTokenCreation = false;
Expand All @@ -43,6 +55,15 @@ public void Setup()
};
}

[Benchmark]
public async Task<List<Claim>> JsonWebTokenHandler_ValidateTokenAsync_CreateClaims()
{
var result = await _jsonWebTokenHandler.ValidateTokenAsync(_jwsExtendedClaims, _validationParameters).ConfigureAwait(false);
var claimsIdentity = result.ClaimsIdentity;
var claims = claimsIdentity.Claims;
return claims.ToList();
}

[Benchmark]
public async Task<TokenValidationResult> JsonWebTokenHandler_ValidateTokenAsync() => await _jsonWebTokenHandler.ValidateTokenAsync(_jws, _validationParameters).ConfigureAwait(false);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ internal static void CreateClaimFromObject(List<Claim> claims, string claimType,
{
// Json.net recognized DateTime by default.
if (value is string str)
claims.Add(new Claim(claimType, str, JwtTokenUtilities.GetStringClaimValueType(str), issuer, issuer));
claims.Add(new Claim(claimType, str, JwtTokenUtilities.GetStringClaimValueType(claimType, str), issuer, issuer));
else if (value is int i)
claims.Add(new Claim(claimType, i.ToString(CultureInfo.InvariantCulture), ClaimValueTypes.Integer32, issuer, issuer));
else if (value is long l)
Expand Down Expand Up @@ -107,7 +107,7 @@ internal static Claim CreateClaimFromJsonElement(string claimType, string issuer
if (jsonElement.ValueKind == JsonValueKind.String)
{
string claimValue = jsonElement.ToString();
return new Claim(claimType, claimValue, JwtTokenUtilities.GetStringClaimValueType(claimValue), issuer, issuer);
return new Claim(claimType, claimValue, JwtTokenUtilities.GetStringClaimValueType(claimType, claimValue), issuer, issuer);
}
else if (jsonElement.ValueKind == JsonValueKind.Null)
return new Claim(claimType, string.Empty, JsonClaimValueTypes.JsonNull, issuer, issuer);
Expand Down
11 changes: 11 additions & 0 deletions src/Microsoft.IdentityModel.JsonWebTokens/JwtTokenUtilities.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
using Microsoft.IdentityModel.Abstractions;
using Microsoft.IdentityModel.Logging;
using Microsoft.IdentityModel.Tokens;
using Microsoft.IdentityModel.Tokens.Json;

using TokenLogMessages = Microsoft.IdentityModel.Tokens.LogMessages;

Expand Down Expand Up @@ -599,8 +600,18 @@ internal static IEnumerable<SecurityKey> ConcatSigningKeys(TokenValidationParame
}

// If a string is in IS8061 format, assume a DateTime is in UTC
// Because this is a friend class, we can't remove this method without
// breaking compatibility.
internal static string GetStringClaimValueType(string str)
{
return GetStringClaimValueType(string.Empty, str);
}

internal static string GetStringClaimValueType(string claimType, string str)
keegan-caruso marked this conversation as resolved.
Show resolved Hide resolved
{
if (!string.IsNullOrEmpty(claimType) && !JsonSerializerPrimitives.TryAllStringClaimsAsDateTime() && JsonSerializerPrimitives.IsKnownToNotBeDateTime(claimType))
return ClaimValueTypes.String;

if (DateTime.TryParse(str, out DateTime dateTimeValue))
{
string dtUniversal = dateTimeValue.ToUniversalTime().ToString("o", CultureInfo.InvariantCulture);
Expand Down
133 changes: 127 additions & 6 deletions src/Microsoft.IdentityModel.Tokens/Json/JsonSerializerPrimitives.cs
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,14 @@ public static JsonElement CreateJsonElement(string json)
}

internal static object CreateObjectFromJsonElement(JsonElement jsonElement, int currentDepth)
{
return CreateObjectFromJsonElement(string.Empty, jsonElement, currentDepth);
}

/// <remarks>
/// <paramref name="claimType"/> is not considered on recursive calls.
/// </remarks>
internal static object CreateObjectFromJsonElement(string claimType, JsonElement jsonElement, int currentDepth)
keegan-caruso marked this conversation as resolved.
Show resolved Hide resolved
{
if (currentDepth >= MaxDepth)
throw new InvalidOperationException(LogHelper.FormatInvariant(
Expand All @@ -138,6 +146,9 @@ internal static object CreateObjectFromJsonElement(JsonElement jsonElement, int

if (jsonElement.ValueKind == JsonValueKind.String)
{
if (!string.IsNullOrEmpty(claimType) && !TryAllStringClaimsAsDateTime() && IsKnownToNotBeDateTime(claimType))
return jsonElement.GetString();

if (DateTime.TryParse(jsonElement.GetString(), CultureInfo.InvariantCulture, DateTimeStyles.RoundtripKind, out DateTime dateTime))
return dateTime;

Expand Down Expand Up @@ -175,7 +186,7 @@ internal static object CreateObjectFromJsonElement(JsonElement jsonElement, int
int index = 0;
foreach (JsonElement j in jsonElement.EnumerateArray())
{
items[index++] = CreateObjectFromJsonElement(j, currentDepth + 1);
items[index++] = CreateObjectFromJsonElement(string.Empty, j, currentDepth + 1);
}

return items;
Expand All @@ -190,7 +201,7 @@ internal static object CreateObjectFromJsonElement(JsonElement jsonElement, int
KeyValuePair<string, object>[] kvps = new KeyValuePair<string, object>[numItems];
foreach (JsonProperty property in jsonElement.EnumerateObject())
{
kvps[index++] = new KeyValuePair<string, object>(property.Name, CreateObjectFromJsonElement(property.Value, currentDepth + 1));
kvps[index++] = new KeyValuePair<string, object>(property.Name, CreateObjectFromJsonElement(string.Empty, property.Value, currentDepth + 1));
}

return kvps;
Expand Down Expand Up @@ -295,7 +306,7 @@ public static bool TryCreateTypeFromJsonElement<T>(JsonElement jsonElement, out
Dictionary<string, object> dictionary = new();
foreach (JsonProperty property in jsonElement.EnumerateObject())
{
dictionary[property.Name] = CreateObjectFromJsonElement(property.Value, currentDepth + 1);
dictionary[property.Name] = CreateObjectFromJsonElement(string.Empty, property.Value, currentDepth + 1);
}

t = (T)(object)dictionary;
Expand Down Expand Up @@ -386,7 +397,7 @@ public static bool TryCreateTypeFromJsonElement<T>(JsonElement jsonElement, out
numItems = 0;
foreach (JsonElement j in jsonElement.EnumerateArray())
{
items[numItems++] = CreateObjectFromJsonElement(j, currentDepth + 1);
items[numItems++] = CreateObjectFromJsonElement(string.Empty, j, currentDepth + 1);
}

t = (T)(object)items;
Expand All @@ -397,7 +408,7 @@ public static bool TryCreateTypeFromJsonElement<T>(JsonElement jsonElement, out
List<object> items = new();
foreach (JsonElement j in jsonElement.EnumerateArray())
{
items.Add(CreateObjectFromJsonElement(j, currentDepth + 1));
items.Add(CreateObjectFromJsonElement(string.Empty, j, currentDepth + 1));
}

t = (T)(object)items;
Expand All @@ -408,7 +419,7 @@ public static bool TryCreateTypeFromJsonElement<T>(JsonElement jsonElement, out
Collection<object> items = new();
foreach (JsonElement j in jsonElement.EnumerateArray())
{
items.Add(CreateObjectFromJsonElement(j, currentDepth + 1));
items.Add(CreateObjectFromJsonElement(string.Empty, j, currentDepth + 1));
}

t = (T)(object)items;
Expand Down Expand Up @@ -695,6 +706,109 @@ internal static string ReadStringOrNumberAsString(ref Utf8JsonReader reader, str
return retVal;
}

internal const string TryToCreateDateTimeClaimsSwitch = "Switch.Microsoft.IdentityModel.TryAllStringClaimsAsDateTime";
keegan-caruso marked this conversation as resolved.
Show resolved Hide resolved

public static bool TryAllStringClaimsAsDateTime()
{
return (AppContext.TryGetSwitch(TryToCreateDateTimeClaimsSwitch, out bool tryAsDateTime) && tryAsDateTime);
brentschmaltz marked this conversation as resolved.
Show resolved Hide resolved
}

/// <summary>
/// This is a non-exhaustive list of claim types that are not expected to be DateTime values
/// sourced from expected Entra V1 and V2 claims, OpenID Connect claims, and a selection of
/// restricted claim names.
/// </summary>
private static HashSet<string> KnownNonDateTimesClaimTypes = new(StringComparer.Ordinal)
keegan-caruso marked this conversation as resolved.
Show resolved Hide resolved
keegan-caruso marked this conversation as resolved.
Show resolved Hide resolved
{
"acr",
"acrs",
"access_token",
"account_type",
"acct",
"actor",
"actort",
"actortoken",
"aio",
"altsecid",
"amr",
"app_displayname",
"appid",
"appidacr",
"at_hash",
"aud",
"authorization_code",
"azp",
"azpacr",
"c_hash",
"cnf",
"capolids",
"ctry",
"email",
"family_name",
"fwd",
"gender",
"given_name",
"groups",
"hasgroups",
"idp",
"idtyp",
"in_corp",
"ipaddr",
"iss",
"jti",
"jwk",
"login_hint",
"name",
"nameid",
"nickname",
"nonce",
"oid",
"onprem_sid",
"phone_number",
"phone_number_verified",
"pop_jwk",
"preferred_username",
"prn",
"puid",
"pwd_url",
"rh",
"role",
"roles",
"secaud",
"sid",
"sub",
"tenant_ctry",
"tenant_region_scope",
"tid",
"typ",
"unique_name",
"upn",
"uti",
"ver",
"verified_primary_email",
"verified_secondary_email",
"vnet",
"website",
"wids",
"xms_cc",
"xms_edov",
"xms_pdl",
"xms_pl",
"xms_tpl",
"ztdid"
};

internal static bool IsKnownToNotBeDateTime(string claimType)
{
if (string.IsNullOrEmpty(claimType))
return true;

if (KnownNonDateTimesClaimTypes.Contains(claimType))
return true;

return false;
}

internal static object ReadStringAsObject(ref Utf8JsonReader reader, string propertyName, string className, bool read = false)
{
// returning null keeps the same logic as JsonSerialization.ReadObject
Expand All @@ -706,6 +820,13 @@ internal static object ReadStringAsObject(ref Utf8JsonReader reader, string prop
CreateJsonReaderException(ref reader, "JsonTokenType.String", className, propertyName));

string originalString = reader.GetString();

if (!TryAllStringClaimsAsDateTime() && IsKnownToNotBeDateTime(propertyName))
{
reader.Read();
return originalString;
}

#pragma warning disable CA1031 // Do not catch general exception types
try
{
Expand Down
Loading
Loading