From 0e2d8a662c5d622bc71a01fc15932c072b80d2c8 Mon Sep 17 00:00:00 2001 From: Keegan Caruso Date: Thu, 27 Jul 2023 16:30:29 -0700 Subject: [PATCH] Better thread safety for JsonClaimSet Claims and JsonWebToken Audiences (#2185) --- .../Json/JsonClaimSet.cs | 16 +++-- .../Json/JsonClaimSet45.cs | 66 +++++++++++-------- .../JsonWebToken.cs | 44 ++++++++----- 3 files changed, 77 insertions(+), 49 deletions(-) diff --git a/src/Microsoft.IdentityModel.JsonWebTokens/Json/JsonClaimSet.cs b/src/Microsoft.IdentityModel.JsonWebTokens/Json/JsonClaimSet.cs index bb7c81280f..8c6c024b72 100644 --- a/src/Microsoft.IdentityModel.JsonWebTokens/Json/JsonClaimSet.cs +++ b/src/Microsoft.IdentityModel.JsonWebTokens/Json/JsonClaimSet.cs @@ -21,6 +21,7 @@ namespace Microsoft.IdentityModel.JsonWebTokens internal class JsonClaimSet { private IList _claims; + private readonly object _claimsLock = new object(); internal JsonClaimSet(JsonDocument jsonDocument) { @@ -41,10 +42,17 @@ internal JsonClaimSet(string json) internal IList Claims(string issuer) { - if (_claims != null) - return _claims; - - _claims = CreateClaims(issuer); + if (_claims == null) + { + lock (_claimsLock) + { + if (_claims == null) + { + _claims = CreateClaims(issuer); + } + } + } + return _claims; } diff --git a/src/Microsoft.IdentityModel.JsonWebTokens/Json/JsonClaimSet45.cs b/src/Microsoft.IdentityModel.JsonWebTokens/Json/JsonClaimSet45.cs index eb51ed4085..774dc2429c 100644 --- a/src/Microsoft.IdentityModel.JsonWebTokens/Json/JsonClaimSet45.cs +++ b/src/Microsoft.IdentityModel.JsonWebTokens/Json/JsonClaimSet45.cs @@ -21,6 +21,7 @@ namespace Microsoft.IdentityModel.JsonWebTokens internal class JsonClaimSet45 { IList _claims; + private readonly object _claimsLock = new object(); internal JsonClaimSet45() { @@ -122,35 +123,46 @@ internal IList CreateClaims(string issuer) internal IList Claims(string issuer) { - if (_claims != null) - return _claims; - - _claims = new List(); - - if (!RootElement.HasValues) - return _claims; - - // there is some code redundancy here that was not factored as this is a high use method. Each identity received from the host will pass through here. - foreach (var entry in RootElement) + if (_claims == null) { - if (entry.Value == null) - { - _claims.Add(new Claim(entry.Key, string.Empty, JsonClaimValueTypes.JsonNull, issuer, issuer)); - continue; - } - - if (entry.Value.Type is JTokenType.String) + lock (_claimsLock) { - var claimValue = entry.Value.ToObject(); - _claims.Add(new Claim(entry.Key, claimValue, ClaimValueTypes.String, issuer, issuer)); - continue; - } - - var jtoken = entry.Value; - if (jtoken != null) - { - AddClaimsFromJToken(_claims, entry.Key, jtoken, issuer); - continue; + if (_claims == null) + { + var claims = new List(); + + if (!RootElement.HasValues) + { + _claims = claims; + return _claims; + } + + // there is some code redundancy here that was not factored as this is a high use method. Each identity received from the host will pass through here. + foreach (var entry in RootElement) + { + if (entry.Value == null) + { + claims.Add(new Claim(entry.Key, string.Empty, JsonClaimValueTypes.JsonNull, issuer, issuer)); + continue; + } + + if (entry.Value.Type is JTokenType.String) + { + var claimValue = entry.Value.ToObject(); + claims.Add(new Claim(entry.Key, claimValue, ClaimValueTypes.String, issuer, issuer)); + continue; + } + + var jtoken = entry.Value; + if (jtoken != null) + { + AddClaimsFromJToken(claims, entry.Key, jtoken, issuer); + continue; + } + } + + _claims = claims; + } } } diff --git a/src/Microsoft.IdentityModel.JsonWebTokens/JsonWebToken.cs b/src/Microsoft.IdentityModel.JsonWebTokens/JsonWebToken.cs index 29b84db401..0075c016ed 100644 --- a/src/Microsoft.IdentityModel.JsonWebTokens/JsonWebToken.cs +++ b/src/Microsoft.IdentityModel.JsonWebTokens/JsonWebToken.cs @@ -37,6 +37,7 @@ public class JsonWebToken : SecurityToken private string _act; private string _alg; private IList _audiences; + private readonly object _audiencesLock = new object(); private string _authenticationTag; private string _ciphertext; private string _cty; @@ -650,28 +651,35 @@ public IEnumerable Audiences { if (_audiences == null) { - _audiences = new List(); -#if NET45 - if (Payload.TryGetValue(JwtRegisteredClaimNames.Aud, out JToken value)) - { - if (value.Type is JTokenType.String) - _audiences = new List { value.ToObject() }; - else if (value.Type is JTokenType.Array) - _audiences = value.ToObject>(); - } -#else - if (Payload.TryGetValue(JwtRegisteredClaimNames.Aud, out JsonElement audiences)) + lock (_audiencesLock) { - if (audiences.ValueKind == JsonValueKind.String) - _audiences = new List { audiences.GetString() }; - - if (audiences.ValueKind == JsonValueKind.Array) + if (_audiences == null) { - foreach (JsonElement jsonElement in audiences.EnumerateArray()) - _audiences.Add(jsonElement.ToString()); + var aud = new List(); +#if NET45 + if (Payload.TryGetValue(JwtRegisteredClaimNames.Aud, out JToken value)) + { + if (value.Type is JTokenType.String) + aud = new List { value.ToObject() }; + else if (value.Type is JTokenType.Array) + aud = value.ToObject>(); + } +#else + if (Payload.TryGetValue(JwtRegisteredClaimNames.Aud, out JsonElement audiences)) + { + if (audiences.ValueKind == JsonValueKind.String) + aud = new List { audiences.GetString() }; + + if (audiences.ValueKind == JsonValueKind.Array) + { + foreach (JsonElement jsonElement in audiences.EnumerateArray()) + aud.Add(jsonElement.ToString()); + } + } +#endif + _audiences = aud; } } -#endif } return _audiences;