From 76c7c8730391963515ca8f241a75258f1f1e631b Mon Sep 17 00:00:00 2001 From: Keegan Caruso Date: Sat, 1 Jun 2024 20:02:33 -0700 Subject: [PATCH 1/4] Don't always test string claims for Date Time Cannot remove APIs in in these internal methods as they are friends There are serveral claim types that should never be a datetime value. This changes introduces a list of claim types that will not be tested if they are a Date Time format. This list of claim types is not exhaustive. Added test data for a test token with more claims and a benchmark using this data. --- .../BenchmarkUtils.cs | 37 ++++++ .../Program.cs | 1 + .../ValidateTokenAsyncTests.cs | 21 +++ .../Json/JsonClaimSet.cs | 4 +- .../JwtTokenUtilities.cs | 11 ++ .../Json/JsonSerializerPrimitives.cs | 122 +++++++++++++++++- .../JwtPayload.cs | 12 +- 7 files changed, 194 insertions(+), 14 deletions(-) diff --git a/benchmark/Microsoft.IdentityModel.Benchmarks/BenchmarkUtils.cs b/benchmark/Microsoft.IdentityModel.Benchmarks/BenchmarkUtils.cs index 8c14fdf03c..ecce92c3e4 100644 --- a/benchmark/Microsoft.IdentityModel.Benchmarks/BenchmarkUtils.cs +++ b/benchmark/Microsoft.IdentityModel.Benchmarks/BenchmarkUtils.cs @@ -60,6 +60,43 @@ public static Dictionary Claims } } + public static Dictionary ClaimsExtendedExample + { + get + { + DateTime now = DateTime.UtcNow; + return new Dictionary() + { + { "acct", 0 }, + { "aio", Guid.NewGuid().ToString() }, + { "amr", new List() { "pwd", "mfa" } }, + { "app_displayname", "MyApp" }, + { "appidacr", 0 }, + { "azpacr", 0 }, + { "azp", Guid.NewGuid().ToString() }, + { "groups", new List() { "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() { Guid.NewGuid().ToString() } }, + { "xms_cc", Guid.NewGuid().ToString() }, + { "role", new List() { "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); diff --git a/benchmark/Microsoft.IdentityModel.Benchmarks/Program.cs b/benchmark/Microsoft.IdentityModel.Benchmarks/Program.cs index 785f31b22b..64344834e6 100644 --- a/benchmark/Microsoft.IdentityModel.Benchmarks/Program.cs +++ b/benchmark/Microsoft.IdentityModel.Benchmarks/Program.cs @@ -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(); diff --git a/benchmark/Microsoft.IdentityModel.Benchmarks/ValidateTokenAsyncTests.cs b/benchmark/Microsoft.IdentityModel.Benchmarks/ValidateTokenAsyncTests.cs index 71bba7b668..1bd6e9fedb 100644 --- a/benchmark/Microsoft.IdentityModel.Benchmarks/ValidateTokenAsyncTests.cs +++ b/benchmark/Microsoft.IdentityModel.Benchmarks/ValidateTokenAsyncTests.cs @@ -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; @@ -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] @@ -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; @@ -43,6 +55,15 @@ public void Setup() }; } + [Benchmark] + public async Task> 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 JsonWebTokenHandler_ValidateTokenAsync() => await _jsonWebTokenHandler.ValidateTokenAsync(_jws, _validationParameters).ConfigureAwait(false); diff --git a/src/Microsoft.IdentityModel.JsonWebTokens/Json/JsonClaimSet.cs b/src/Microsoft.IdentityModel.JsonWebTokens/Json/JsonClaimSet.cs index a47e488129..0b766af583 100644 --- a/src/Microsoft.IdentityModel.JsonWebTokens/Json/JsonClaimSet.cs +++ b/src/Microsoft.IdentityModel.JsonWebTokens/Json/JsonClaimSet.cs @@ -55,7 +55,7 @@ internal static void CreateClaimFromObject(List 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) @@ -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); diff --git a/src/Microsoft.IdentityModel.JsonWebTokens/JwtTokenUtilities.cs b/src/Microsoft.IdentityModel.JsonWebTokens/JwtTokenUtilities.cs index 9fb2db5350..8c83abb974 100644 --- a/src/Microsoft.IdentityModel.JsonWebTokens/JwtTokenUtilities.cs +++ b/src/Microsoft.IdentityModel.JsonWebTokens/JwtTokenUtilities.cs @@ -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; @@ -599,8 +600,18 @@ internal static IEnumerable 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) + { + if (!string.IsNullOrEmpty(claimType) && !JsonSerializerPrimitives.TryAllStringClaimsAsDateTime() && JsonSerializerPrimitives.KnownNonDateTimesClaimTypes.Contains(claimType)) + return ClaimValueTypes.String; + if (DateTime.TryParse(str, out DateTime dateTimeValue)) { string dtUniversal = dateTimeValue.ToUniversalTime().ToString("o", CultureInfo.InvariantCulture); diff --git a/src/Microsoft.IdentityModel.Tokens/Json/JsonSerializerPrimitives.cs b/src/Microsoft.IdentityModel.Tokens/Json/JsonSerializerPrimitives.cs index cd99a503ca..6b4c00fd46 100644 --- a/src/Microsoft.IdentityModel.Tokens/Json/JsonSerializerPrimitives.cs +++ b/src/Microsoft.IdentityModel.Tokens/Json/JsonSerializerPrimitives.cs @@ -129,6 +129,14 @@ public static JsonElement CreateJsonElement(string json) } internal static object CreateObjectFromJsonElement(JsonElement jsonElement, int currentDepth) + { + return CreateObjectFromJsonElement(string.Empty, jsonElement, currentDepth); + } + + /// + /// is not considered on recursive calls. + /// + internal static object CreateObjectFromJsonElement(string claimType, JsonElement jsonElement, int currentDepth) { if (currentDepth >= MaxDepth) throw new InvalidOperationException(LogHelper.FormatInvariant( @@ -138,6 +146,9 @@ internal static object CreateObjectFromJsonElement(JsonElement jsonElement, int if (jsonElement.ValueKind == JsonValueKind.String) { + if (!string.IsNullOrEmpty(claimType) && !TryAllStringClaimsAsDateTime() && KnownNonDateTimesClaimTypes.Contains(claimType)) + return jsonElement.GetString(); + if (DateTime.TryParse(jsonElement.GetString(), CultureInfo.InvariantCulture, DateTimeStyles.RoundtripKind, out DateTime dateTime)) return dateTime; @@ -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; @@ -190,7 +201,7 @@ internal static object CreateObjectFromJsonElement(JsonElement jsonElement, int KeyValuePair[] kvps = new KeyValuePair[numItems]; foreach (JsonProperty property in jsonElement.EnumerateObject()) { - kvps[index++] = new KeyValuePair(property.Name, CreateObjectFromJsonElement(property.Value, currentDepth + 1)); + kvps[index++] = new KeyValuePair(property.Name, CreateObjectFromJsonElement(string.Empty, property.Value, currentDepth + 1)); } return kvps; @@ -295,7 +306,7 @@ public static bool TryCreateTypeFromJsonElement(JsonElement jsonElement, out Dictionary 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; @@ -386,7 +397,7 @@ public static bool TryCreateTypeFromJsonElement(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; @@ -397,7 +408,7 @@ public static bool TryCreateTypeFromJsonElement(JsonElement jsonElement, out List 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; @@ -408,7 +419,7 @@ public static bool TryCreateTypeFromJsonElement(JsonElement jsonElement, out Collection 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; @@ -695,6 +706,98 @@ internal static string ReadStringOrNumberAsString(ref Utf8JsonReader reader, str return retVal; } + internal const string TryToCreateDateTimeClaimsSwitch = "Switch.Microsoft.IdentityModel.TryAllStringClaimsAsDateTime"; + + public static bool TryAllStringClaimsAsDateTime() + { + return (AppContext.TryGetSwitch(TryToCreateDateTimeClaimsSwitch, out bool tryAsDateTime) && tryAsDateTime); + } + + /// + /// 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. + /// + public static HashSet KnownNonDateTimesClaimTypes = new(StringComparer.Ordinal) + { + "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 object ReadStringAsObject(ref Utf8JsonReader reader, string propertyName, string className, bool read = false) { // returning null keeps the same logic as JsonSerialization.ReadObject @@ -706,6 +809,13 @@ internal static object ReadStringAsObject(ref Utf8JsonReader reader, string prop CreateJsonReaderException(ref reader, "JsonTokenType.String", className, propertyName)); string originalString = reader.GetString(); + + if (!TryAllStringClaimsAsDateTime() && KnownNonDateTimesClaimTypes.Contains(propertyName)) + { + reader.Read(); + return originalString; + } + #pragma warning disable CA1031 // Do not catch general exception types try { diff --git a/src/System.IdentityModel.Tokens.Jwt/JwtPayload.cs b/src/System.IdentityModel.Tokens.Jwt/JwtPayload.cs index 7a36f83672..9bc330bae8 100644 --- a/src/System.IdentityModel.Tokens.Jwt/JwtPayload.cs +++ b/src/System.IdentityModel.Tokens.Jwt/JwtPayload.cs @@ -508,7 +508,7 @@ public virtual IEnumerable Claims claims.Add(new Claim(keyValuePair.Key, string.Empty, JsonClaimValueTypes.JsonNull, issuer, issuer)); else if (keyValuePair.Value is string str) - claims.Add(new Claim(keyValuePair.Key, str, GetClaimValueType(str), issuer, issuer)); + claims.Add(new Claim(keyValuePair.Key, str, GetClaimValueType(keyValuePair.Key, str), issuer, issuer)); else if (keyValuePair.Value is JsonElement j) AddClaimsFromJsonElement(keyValuePair.Key, issuer, j, claims); @@ -521,7 +521,7 @@ public virtual IEnumerable Claims { foreach (var item in dictionary) if (item.Value != null) - claims.Add(new Claim(keyValuePair.Key, "{" + item.Key + ":" + item.Value.ToString() + "}", GetClaimValueType(item.Value), issuer, issuer)); + claims.Add(new Claim(keyValuePair.Key, "{" + item.Key + ":" + item.Value.ToString() + "}", GetClaimValueType(item.Key, item.Value), issuer, issuer)); } else if (keyValuePair.Value is DateTime dateTime) claims.Add(new Claim(keyValuePair.Key, dateTime.ToString("o", CultureInfo.InvariantCulture), ClaimValueTypes.DateTime, issuer, issuer)); @@ -536,7 +536,7 @@ public virtual IEnumerable Claims else if (keyValuePair.Value != null) { var value = keyValuePair.Value; - var claimValueType = GetClaimValueType(value); + var claimValueType = GetClaimValueType(keyValuePair.Key, value); if (value is IFormattable formattable) claims.Add(new Claim(keyValuePair.Key, formattable.ToString(null, CultureInfo.InvariantCulture), claimValueType, issuer, issuer)); else @@ -569,7 +569,7 @@ private void AddListofObjects(string key, IEnumerable objects, List claimsCollection) this[kvp.Key] = kvp.Value; } - internal static string GetClaimValueType(object value) + private static string GetClaimValueType(string claimType, object value) { if (value == null) return JsonClaimValueTypes.JsonNull; @@ -672,7 +672,7 @@ internal static string GetClaimValueType(object value) Type objType = value.GetType(); if (value is string str) - return JwtTokenUtilities.GetStringClaimValueType(str); + return JwtTokenUtilities.GetStringClaimValueType(claimType, str); else if (objType == typeof(int)) return ClaimValueTypes.Integer32; else if (objType == typeof(long)) From 2879a6c8be4c90f7a721f845e637378af48a8f72 Mon Sep 17 00:00:00 2001 From: Keegan Caruso Date: Mon, 3 Jun 2024 11:20:34 -0700 Subject: [PATCH 2/4] Prefer to expose method over collection --- .../JwtTokenUtilities.cs | 2 +- .../Json/JsonSerializerPrimitives.cs | 17 ++++++++++++++--- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/src/Microsoft.IdentityModel.JsonWebTokens/JwtTokenUtilities.cs b/src/Microsoft.IdentityModel.JsonWebTokens/JwtTokenUtilities.cs index 8c83abb974..43a1eaa786 100644 --- a/src/Microsoft.IdentityModel.JsonWebTokens/JwtTokenUtilities.cs +++ b/src/Microsoft.IdentityModel.JsonWebTokens/JwtTokenUtilities.cs @@ -609,7 +609,7 @@ internal static string GetStringClaimValueType(string str) internal static string GetStringClaimValueType(string claimType, string str) { - if (!string.IsNullOrEmpty(claimType) && !JsonSerializerPrimitives.TryAllStringClaimsAsDateTime() && JsonSerializerPrimitives.KnownNonDateTimesClaimTypes.Contains(claimType)) + if (!string.IsNullOrEmpty(claimType) && !JsonSerializerPrimitives.TryAllStringClaimsAsDateTime() && JsonSerializerPrimitives.IsKnownToNotBeDateTime(claimType)) return ClaimValueTypes.String; if (DateTime.TryParse(str, out DateTime dateTimeValue)) diff --git a/src/Microsoft.IdentityModel.Tokens/Json/JsonSerializerPrimitives.cs b/src/Microsoft.IdentityModel.Tokens/Json/JsonSerializerPrimitives.cs index 6b4c00fd46..f562890c5f 100644 --- a/src/Microsoft.IdentityModel.Tokens/Json/JsonSerializerPrimitives.cs +++ b/src/Microsoft.IdentityModel.Tokens/Json/JsonSerializerPrimitives.cs @@ -146,7 +146,7 @@ internal static object CreateObjectFromJsonElement(string claimType, JsonElement if (jsonElement.ValueKind == JsonValueKind.String) { - if (!string.IsNullOrEmpty(claimType) && !TryAllStringClaimsAsDateTime() && KnownNonDateTimesClaimTypes.Contains(claimType)) + if (!string.IsNullOrEmpty(claimType) && !TryAllStringClaimsAsDateTime() && IsKnownToNotBeDateTime(claimType)) return jsonElement.GetString(); if (DateTime.TryParse(jsonElement.GetString(), CultureInfo.InvariantCulture, DateTimeStyles.RoundtripKind, out DateTime dateTime)) @@ -718,7 +718,7 @@ public static bool TryAllStringClaimsAsDateTime() /// sourced from expected Entra V1 and V2 claims, OpenID Connect claims, and a selection of /// restricted claim names. /// - public static HashSet KnownNonDateTimesClaimTypes = new(StringComparer.Ordinal) + private static HashSet KnownNonDateTimesClaimTypes = new(StringComparer.Ordinal) { "acr", "acrs", @@ -798,6 +798,17 @@ public static bool TryAllStringClaimsAsDateTime() "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 @@ -810,7 +821,7 @@ internal static object ReadStringAsObject(ref Utf8JsonReader reader, string prop string originalString = reader.GetString(); - if (!TryAllStringClaimsAsDateTime() && KnownNonDateTimesClaimTypes.Contains(propertyName)) + if (!TryAllStringClaimsAsDateTime() && IsKnownToNotBeDateTime(propertyName)) { reader.Read(); return originalString; From 60c2cb58023205a87d5a574bebe00f31816fba00 Mon Sep 17 00:00:00 2001 From: Keegan Caruso Date: Tue, 4 Jun 2024 11:57:58 -0700 Subject: [PATCH 3/4] Add JWT headers to known non datetimes values --- .../Json/JsonSerializerPrimitives.cs | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/src/Microsoft.IdentityModel.Tokens/Json/JsonSerializerPrimitives.cs b/src/Microsoft.IdentityModel.Tokens/Json/JsonSerializerPrimitives.cs index f562890c5f..b62cac502d 100644 --- a/src/Microsoft.IdentityModel.Tokens/Json/JsonSerializerPrimitives.cs +++ b/src/Microsoft.IdentityModel.Tokens/Json/JsonSerializerPrimitives.cs @@ -720,6 +720,21 @@ public static bool TryAllStringClaimsAsDateTime() /// private static HashSet KnownNonDateTimesClaimTypes = new(StringComparer.Ordinal) { + // Header Values. + "alg", + "cty", + "crit", + "enc", + "jku", + "jwk", + "kid", + "typ", + "x5c", + "x5t", + "x5t#S256", + "x5u", + "zip", + // JWT claims. "acr", "acrs", "access_token", @@ -756,7 +771,6 @@ public static bool TryAllStringClaimsAsDateTime() "ipaddr", "iss", "jti", - "jwk", "login_hint", "name", "nameid", @@ -780,7 +794,6 @@ public static bool TryAllStringClaimsAsDateTime() "tenant_ctry", "tenant_region_scope", "tid", - "typ", "unique_name", "upn", "uti", From 1ea144fb908d04f19f62dd377bb96ab22deee0b7 Mon Sep 17 00:00:00 2001 From: Keegan Caruso Date: Wed, 12 Jun 2024 09:54:35 -0700 Subject: [PATCH 4/4] Review feedback - Reorder params in new methods - Rename KnownNonDateTimesClaimTypes to s_knownNonDateTimeClaimTypes --- .../Json/JsonClaimSet.cs | 4 ++-- .../JwtTokenUtilities.cs | 4 ++-- .../Json/JsonSerializerPrimitives.cs | 20 +++++++++---------- .../JwtPayload.cs | 2 +- 4 files changed, 15 insertions(+), 15 deletions(-) diff --git a/src/Microsoft.IdentityModel.JsonWebTokens/Json/JsonClaimSet.cs b/src/Microsoft.IdentityModel.JsonWebTokens/Json/JsonClaimSet.cs index 0b766af583..aa6276a320 100644 --- a/src/Microsoft.IdentityModel.JsonWebTokens/Json/JsonClaimSet.cs +++ b/src/Microsoft.IdentityModel.JsonWebTokens/Json/JsonClaimSet.cs @@ -55,7 +55,7 @@ internal static void CreateClaimFromObject(List claims, string claimType, { // Json.net recognized DateTime by default. if (value is string str) - claims.Add(new Claim(claimType, str, JwtTokenUtilities.GetStringClaimValueType(claimType, str), issuer, issuer)); + claims.Add(new Claim(claimType, str, JwtTokenUtilities.GetStringClaimValueType(str, claimType), 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) @@ -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(claimType, claimValue), issuer, issuer); + return new Claim(claimType, claimValue, JwtTokenUtilities.GetStringClaimValueType(claimValue, claimType), issuer, issuer); } else if (jsonElement.ValueKind == JsonValueKind.Null) return new Claim(claimType, string.Empty, JsonClaimValueTypes.JsonNull, issuer, issuer); diff --git a/src/Microsoft.IdentityModel.JsonWebTokens/JwtTokenUtilities.cs b/src/Microsoft.IdentityModel.JsonWebTokens/JwtTokenUtilities.cs index 43a1eaa786..52b84a7b37 100644 --- a/src/Microsoft.IdentityModel.JsonWebTokens/JwtTokenUtilities.cs +++ b/src/Microsoft.IdentityModel.JsonWebTokens/JwtTokenUtilities.cs @@ -604,10 +604,10 @@ internal static IEnumerable ConcatSigningKeys(TokenValidationParame // breaking compatibility. internal static string GetStringClaimValueType(string str) { - return GetStringClaimValueType(string.Empty, str); + return GetStringClaimValueType(str, string.Empty); } - internal static string GetStringClaimValueType(string claimType, string str) + internal static string GetStringClaimValueType(string str, string claimType) { if (!string.IsNullOrEmpty(claimType) && !JsonSerializerPrimitives.TryAllStringClaimsAsDateTime() && JsonSerializerPrimitives.IsKnownToNotBeDateTime(claimType)) return ClaimValueTypes.String; diff --git a/src/Microsoft.IdentityModel.Tokens/Json/JsonSerializerPrimitives.cs b/src/Microsoft.IdentityModel.Tokens/Json/JsonSerializerPrimitives.cs index b62cac502d..f539555a8b 100644 --- a/src/Microsoft.IdentityModel.Tokens/Json/JsonSerializerPrimitives.cs +++ b/src/Microsoft.IdentityModel.Tokens/Json/JsonSerializerPrimitives.cs @@ -130,13 +130,13 @@ public static JsonElement CreateJsonElement(string json) internal static object CreateObjectFromJsonElement(JsonElement jsonElement, int currentDepth) { - return CreateObjectFromJsonElement(string.Empty, jsonElement, currentDepth); + return CreateObjectFromJsonElement(jsonElement, currentDepth, string.Empty); } /// /// is not considered on recursive calls. /// - internal static object CreateObjectFromJsonElement(string claimType, JsonElement jsonElement, int currentDepth) + internal static object CreateObjectFromJsonElement(JsonElement jsonElement, int currentDepth, string claimType) { if (currentDepth >= MaxDepth) throw new InvalidOperationException(LogHelper.FormatInvariant( @@ -186,7 +186,7 @@ internal static object CreateObjectFromJsonElement(string claimType, JsonElement int index = 0; foreach (JsonElement j in jsonElement.EnumerateArray()) { - items[index++] = CreateObjectFromJsonElement(string.Empty, j, currentDepth + 1); + items[index++] = CreateObjectFromJsonElement(j, currentDepth + 1, string.Empty); } return items; @@ -201,7 +201,7 @@ internal static object CreateObjectFromJsonElement(string claimType, JsonElement KeyValuePair[] kvps = new KeyValuePair[numItems]; foreach (JsonProperty property in jsonElement.EnumerateObject()) { - kvps[index++] = new KeyValuePair(property.Name, CreateObjectFromJsonElement(string.Empty, property.Value, currentDepth + 1)); + kvps[index++] = new KeyValuePair(property.Name, CreateObjectFromJsonElement(property.Value, currentDepth + 1, string.Empty)); } return kvps; @@ -306,7 +306,7 @@ public static bool TryCreateTypeFromJsonElement(JsonElement jsonElement, out Dictionary dictionary = new(); foreach (JsonProperty property in jsonElement.EnumerateObject()) { - dictionary[property.Name] = CreateObjectFromJsonElement(string.Empty, property.Value, currentDepth + 1); + dictionary[property.Name] = CreateObjectFromJsonElement(property.Value, currentDepth + 1, string.Empty); } t = (T)(object)dictionary; @@ -397,7 +397,7 @@ public static bool TryCreateTypeFromJsonElement(JsonElement jsonElement, out numItems = 0; foreach (JsonElement j in jsonElement.EnumerateArray()) { - items[numItems++] = CreateObjectFromJsonElement(string.Empty, j, currentDepth + 1); + items[numItems++] = CreateObjectFromJsonElement(j, currentDepth + 1, string.Empty); } t = (T)(object)items; @@ -408,7 +408,7 @@ public static bool TryCreateTypeFromJsonElement(JsonElement jsonElement, out List items = new(); foreach (JsonElement j in jsonElement.EnumerateArray()) { - items.Add(CreateObjectFromJsonElement(string.Empty, j, currentDepth + 1)); + items.Add(CreateObjectFromJsonElement(j, currentDepth + 1, string.Empty)); } t = (T)(object)items; @@ -419,7 +419,7 @@ public static bool TryCreateTypeFromJsonElement(JsonElement jsonElement, out Collection items = new(); foreach (JsonElement j in jsonElement.EnumerateArray()) { - items.Add(CreateObjectFromJsonElement(string.Empty, j, currentDepth + 1)); + items.Add(CreateObjectFromJsonElement(j, currentDepth + 1, string.Empty)); } t = (T)(object)items; @@ -718,7 +718,7 @@ public static bool TryAllStringClaimsAsDateTime() /// sourced from expected Entra V1 and V2 claims, OpenID Connect claims, and a selection of /// restricted claim names. /// - private static HashSet KnownNonDateTimesClaimTypes = new(StringComparer.Ordinal) + private static readonly HashSet s_knownNonDateTimeClaimTypes = new(StringComparer.Ordinal) { // Header Values. "alg", @@ -816,7 +816,7 @@ internal static bool IsKnownToNotBeDateTime(string claimType) if (string.IsNullOrEmpty(claimType)) return true; - if (KnownNonDateTimesClaimTypes.Contains(claimType)) + if (s_knownNonDateTimeClaimTypes.Contains(claimType)) return true; return false; diff --git a/src/System.IdentityModel.Tokens.Jwt/JwtPayload.cs b/src/System.IdentityModel.Tokens.Jwt/JwtPayload.cs index 9bc330bae8..506bd31983 100644 --- a/src/System.IdentityModel.Tokens.Jwt/JwtPayload.cs +++ b/src/System.IdentityModel.Tokens.Jwt/JwtPayload.cs @@ -672,7 +672,7 @@ private static string GetClaimValueType(string claimType, object value) Type objType = value.GetType(); if (value is string str) - return JwtTokenUtilities.GetStringClaimValueType(claimType, str); + return JwtTokenUtilities.GetStringClaimValueType(str, claimType); else if (objType == typeof(int)) return ClaimValueTypes.Integer32; else if (objType == typeof(long))