From c747c36c7dcdbadd7965261937272c5e24c4c739 Mon Sep 17 00:00:00 2001 From: vvdb-architecture Date: Tue, 8 Oct 2024 12:56:24 +0200 Subject: [PATCH 1/3] Eliminate token cache timeout setting in most cases --- .../Token/Cache.cs | 15 ++- .../AppPrincipalTransform.cs | 101 +++++++++--------- .../Extensions/AddClaimsFillerExtension.cs | 28 +++-- .../BasicAuthenticationMiddleware.cs | 5 +- .../Options/ClaimsFillerOptions.cs | 7 +- .../CredentialTokenCacheTokenProvider.cs | 5 +- .../Extensions/TokenCacheExtension.cs | 7 -- .../Options/TokenCacheOptions.cs | 4 - .../Token/ApplicationCache.cs | 6 +- .../Token/ApplicationLocalDataCache.cs | 3 +- .../Token/ITokenCache.cs | 12 ++- .../Security/JwtHttpHandlerTests.cs | 16 +-- .../Security/TokenCacheOptionsTests.cs | 24 ++--- .../Security/gRpcInterceptorTests.cs | 17 ++- 14 files changed, 118 insertions(+), 132 deletions(-) diff --git a/src/Arc4u.Standard.OAuth2.AspNetCore.Adal/Token/Cache.cs b/src/Arc4u.Standard.OAuth2.AspNetCore.Adal/Token/Cache.cs index dd40e4a0..2df181bb 100644 --- a/src/Arc4u.Standard.OAuth2.AspNetCore.Adal/Token/Cache.cs +++ b/src/Arc4u.Standard.OAuth2.AspNetCore.Adal/Token/Cache.cs @@ -1,9 +1,7 @@ -using Arc4u.Dependency; +using Arc4u.Dependency; using Arc4u.Diagnostics; using Microsoft.Extensions.Logging; using Microsoft.IdentityModel.Clients.ActiveDirectory; -using System; -using System.Collections.Generic; namespace Arc4u.OAuth2.Token.Adal { @@ -75,7 +73,16 @@ private void AfterAccessNotification(TokenCacheNotificationArgs args) if (HasStateChanged) { Logger.Technical().From().System($"Adding token information to the cache for the identifier: {_identifier}.").Log(); - TokenCache.Put(_identifier, SerializeAdalV3()); + var now = DateTimeOffset.UtcNow; + var maxExpires = now; + foreach (var tokenCacheItem in ReadItems()) + { + if (tokenCacheItem.ExpiresOn > maxExpires) + { + maxExpires = tokenCacheItem.ExpiresOn; + } + } + TokenCache.Put(_identifier, maxExpires - now, SerializeAdalV3()); Logger.Technical().From().System($"Added token information to the cache for the identifier: {_identifier}.").Log(); HasStateChanged = false; diff --git a/src/Arc4u.Standard.OAuth2.AspNetCore/AppPrincipalTransform.cs b/src/Arc4u.Standard.OAuth2.AspNetCore/AppPrincipalTransform.cs index 2b523326..a1d35a8f 100644 --- a/src/Arc4u.Standard.OAuth2.AspNetCore/AppPrincipalTransform.cs +++ b/src/Arc4u.Standard.OAuth2.AspNetCore/AppPrincipalTransform.cs @@ -1,11 +1,7 @@ -using System.Collections.Generic; -using System; using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.Globalization; -using System.Linq; using System.Security.Claims; -using System.Threading.Tasks; using Arc4u.Configuration; using Arc4u.Dependency.Attribute; using Arc4u.Diagnostics; @@ -14,9 +10,9 @@ using Arc4u.OAuth2.Token; using Arc4u.Security.Principal; using Microsoft.AspNetCore.Authentication; +using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; -using Microsoft.Extensions.DependencyInjection; namespace Arc4u.OAuth2; @@ -103,7 +99,7 @@ public async Task TransformAsync(ClaimsPrincipal principal) #region Handling extra claims private const string tokenExpirationClaimType = "exp"; - private static readonly string[] ClaimsToExclude = { "aud", "iss", "iat", "nbf", "acr", "aio", "appidacr", "ipaddr", "scp", "sub", "tid", "uti", "unique_name", "apptype", "appid", "ver", "http://schemas.microsoft.com/ws/2008/06/identity/claims/authenticationinstant", "http://schemas.microsoft.com/identity/claims/scope" }; + private static readonly HashSet ClaimsToExclude = new HashSet(StringComparer.OrdinalIgnoreCase) { "aud", "iss", "iat", "nbf", "acr", "aio", "appidacr", "ipaddr", "scp", "sub", "tid", "uti", "unique_name", "apptype", "appid", "ver", "http://schemas.microsoft.com/ws/2008/06/identity/claims/authenticationinstant", "http://schemas.microsoft.com/identity/claims/scope" }; /// /// This code is similar to the code in AppPrincipalFactory where the claims are stored in a secureCache. @@ -128,21 +124,13 @@ private async Task LoadExtraClaimsAsync(ClaimsIdentity? identity) // Something already in the cache? Avoid a expensive backend call. var cachedClaims = GetClaimsFromCache(cacheKey); - // check expirity. - var cachedExpiredClaim = cachedClaims.FirstOrDefault(c => c.ClaimType.Equals(tokenExpirationClaimType, StringComparison.OrdinalIgnoreCase)); - - if (cachedExpiredClaim is not null && long.TryParse(cachedExpiredClaim.Value, out var cachedExpiredTicks)) + // if cachedClaims is not null, it means that the information was in the cache and didn't expire yet. + if (cachedClaims is not null) { - var expDate = DateTimeOffset.FromUnixTimeSeconds(cachedExpiredTicks).UtcDateTime; - if (expDate > DateTime.UtcNow) - { - identity.AddClaims(cachedClaims.Where(c => c.ClaimType != tokenExpirationClaimType) - .Where(c => !ClaimsToExclude.Any(arg => arg.Equals(c.ClaimType))) - .Where(c => !identity.Claims.Any(c1 => c1.Type == c.ClaimType)) - .Select(c => new Claim(c.ClaimType, c.Value))); - - return; - } + identity.AddClaims(cachedClaims.Where(c => !ClaimsToExclude.Contains(c.ClaimType)) + .Where(c => !identity.Claims.Any(c1 => StringComparer.OrdinalIgnoreCase.Equals(c1.Type, c.ClaimType))) + .Select(c => new Claim(c.ClaimType, c.Value))); + return; } // Add Telemetry. @@ -154,41 +142,57 @@ private async Task LoadExtraClaimsAsync(ClaimsIdentity? identity) // Should receive specific extra claims. This is the responsibility of the caller to provide the right claims. // We expect the exp claim to be present. // if not the persistence time will be the default one. - var claims = (await _claimsFiller.GetAsync(identity, settings, null).ConfigureAwait(false)).Where(c => !ClaimsToExclude.Any(arg => arg.Equals(c.ClaimType))).ToList(); + var claims = (await _claimsFiller.GetAsync(identity, settings, null).ConfigureAwait(false)).Where(c => !ClaimsToExclude.Contains(c.ClaimType)).ToList(); // Load the claims into the identity but exclude the exp claim and the one already present. - identity.AddClaims(claims.Where(c => c.ClaimType != tokenExpirationClaimType) - .Where(c => !ClaimsToExclude.Any(arg => arg.Equals(c.ClaimType))) - .Where(c => !identity.Claims.Any(c1 => c1.Type == c.ClaimType)) + identity.AddClaims(claims.Where(c => !StringComparer.OrdinalIgnoreCase.Equals(c.ClaimType, tokenExpirationClaimType)) + .Where(c => !ClaimsToExclude.Contains(c.ClaimType)) + .Where(c => !identity.Claims.Any(c1 => StringComparer.OrdinalIgnoreCase.Equals(c1.Type, c.ClaimType))) .Select(c => new Claim(c.ClaimType, c.Value))); - SaveClaimsToCache(claims, cacheKey); - } - - } + // Check expiry claim explicitly returned in the call to _claimsFiller.GetAsync. + var cachedExpiredClaim = claims.FirstOrDefault(c => StringComparer.OrdinalIgnoreCase.Equals(c.ClaimType, tokenExpirationClaimType))?.Value; - private List GetClaimsFromCache(string? cacheKey) - { - var claims = new List(); + DateTimeOffset expDate; + if (cachedExpiredClaim is not null && long.TryParse(cachedExpiredClaim, NumberStyles.Integer, CultureInfo.InvariantCulture, out var cachedExpiredTicks)) + { + expDate = DateTimeOffset.FromUnixTimeSeconds(cachedExpiredTicks); + } + else + { + // No expiry claim found in the call to _claimsFiller.GetAsync, try to locate one in the existing identity claims (which are normally obtained from the bearer token) + cachedExpiredClaim = identity.Claims.FirstOrDefault(c => StringComparer.OrdinalIgnoreCase.Equals(c.Type, tokenExpirationClaimType))?.Value; + if (cachedExpiredClaim is not null && long.TryParse(cachedExpiredClaim, NumberStyles.Integer, CultureInfo.InvariantCulture, out cachedExpiredTicks)) + { + expDate = DateTimeOffset.FromUnixTimeSeconds(cachedExpiredTicks); + } + else + { + expDate = DateTimeOffset.UtcNow.Add(_options.MaxTime); + } + } - if (string.IsNullOrWhiteSpace(cacheKey)) - { - return claims; + SaveClaimsToCache(claims, cacheKey, expDate - DateTimeOffset.UtcNow); } + } - try - { - claims = _cacheHelper.GetCache().Get>(cacheKey) ?? new List(); - } - catch (Exception ex) + private List? GetClaimsFromCache(string? cacheKey) + { + if (!string.IsNullOrWhiteSpace(cacheKey)) { - _logger.Technical().Exception(ex).Log(); + try + { + return _cacheHelper.GetCache().Get>(cacheKey); + } + catch (Exception ex) + { + _logger.Technical().Exception(ex).Log(); + } } - - return claims; + return null; } - private void SaveClaimsToCache([DisallowNull] IEnumerable claims, string? cacheKey) + private void SaveClaimsToCache([DisallowNull] IEnumerable claims, string? cacheKey, TimeSpan timeout) { if (string.IsNullOrWhiteSpace(cacheKey)) { @@ -199,16 +203,7 @@ private void SaveClaimsToCache([DisallowNull] IEnumerable claims, stri try { - var cachedExpiredClaim = claims.FirstOrDefault(c => c.ClaimType.Equals(tokenExpirationClaimType, StringComparison.OrdinalIgnoreCase)); - - // if no expiration claim exist we assume the lifetime of the extra claims is defined by the cache context for the principal. - if (cachedExpiredClaim is null) - { - cachedExpiredClaim = new ClaimDto(tokenExpirationClaimType, DateTimeOffset.UtcNow.Add(_cacheOptions.MaxTime).ToUnixTimeSeconds().ToString(CultureInfo.InvariantCulture)); - claims = new List(claims) { cachedExpiredClaim }; - } - - _cacheHelper.GetCache().Put(cacheKey, _cacheOptions.MaxTime, claims); + _cacheHelper.GetCache().Put(cacheKey, timeout, claims); } catch (Exception ex) { diff --git a/src/Arc4u.Standard.OAuth2.AspNetCore/Extensions/AddClaimsFillerExtension.cs b/src/Arc4u.Standard.OAuth2.AspNetCore/Extensions/AddClaimsFillerExtension.cs index 2c4ed1d2..b36a9bf8 100644 --- a/src/Arc4u.Standard.OAuth2.AspNetCore/Extensions/AddClaimsFillerExtension.cs +++ b/src/Arc4u.Standard.OAuth2.AspNetCore/Extensions/AddClaimsFillerExtension.cs @@ -1,5 +1,3 @@ -using System; -using System.Linq; using Arc4u.Configuration; using Arc4u.OAuth2.Options; using Microsoft.Extensions.Configuration; @@ -13,9 +11,21 @@ public static void AddClaimsFiller(this IServiceCollection services, Action(options); @@ -39,10 +49,10 @@ public static void AddClaimsFiller(this IServiceCollection services, IConfigurat } } - AddClaimsFiller(services, o => - { - o.LoadClaimsFromClaimsFillerProvider = options.LoadClaimsFromClaimsFillerProvider; - o.SettingsKeys = options.SettingsKeys; - }); + AddClaimsFiller(services, o => + { + o.LoadClaimsFromClaimsFillerProvider = options.LoadClaimsFromClaimsFillerProvider; + o.SettingsKeys = options.SettingsKeys; + }); } } diff --git a/src/Arc4u.Standard.OAuth2.AspNetCore/Middleware/BasicAuthenticationMiddleware.cs b/src/Arc4u.Standard.OAuth2.AspNetCore/Middleware/BasicAuthenticationMiddleware.cs index e65c9ec0..d2b33c35 100644 --- a/src/Arc4u.Standard.OAuth2.AspNetCore/Middleware/BasicAuthenticationMiddleware.cs +++ b/src/Arc4u.Standard.OAuth2.AspNetCore/Middleware/BasicAuthenticationMiddleware.cs @@ -1,11 +1,8 @@ -using System; using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.Globalization; -using System.Linq; using System.Net.Http.Headers; using System.Text; -using System.Threading.Tasks; using Arc4u.Configuration; using Arc4u.Dependency; using Arc4u.Diagnostics; @@ -89,7 +86,7 @@ public async Task Invoke(HttpContext context) { tokenInfo = await GetTokenFromCredentialAsync(credential, context.RequestServices).ConfigureAwait(false); - tokenCache?.Put(cacheKey, tokenInfo); + tokenCache?.Put(cacheKey, tokenInfo.ExpiresOnUtc - DateTime.UtcNow, tokenInfo); } if (tokenInfo is not null) diff --git a/src/Arc4u.Standard.OAuth2.AspNetCore/Options/ClaimsFillerOptions.cs b/src/Arc4u.Standard.OAuth2.AspNetCore/Options/ClaimsFillerOptions.cs index dc991aa2..e779408b 100644 --- a/src/Arc4u.Standard.OAuth2.AspNetCore/Options/ClaimsFillerOptions.cs +++ b/src/Arc4u.Standard.OAuth2.AspNetCore/Options/ClaimsFillerOptions.cs @@ -1,5 +1,3 @@ -using System.Collections.Generic; - namespace Arc4u.OAuth2.Options; /// @@ -10,6 +8,11 @@ public class ClaimsFillerOptions // By default, a specific claims filler is needed to manage the right. public bool LoadClaimsFromClaimsFillerProvider { get; set; } = true; + /// + /// If is true, the extra claims are cached for ."/> + /// + public TimeSpan MaxTime { get; set; } = TimeSpan.FromMinutes(20); + /// /// The settings key to load the claims from. => registered as a named /// By default no on behalf of scenario is defined => AOauth2 must be added to perform the on behalf of scenario. diff --git a/src/Arc4u.Standard.OAuth2.AspNetCore/TokenProvider/CredentialTokenCacheTokenProvider.cs b/src/Arc4u.Standard.OAuth2.AspNetCore/TokenProvider/CredentialTokenCacheTokenProvider.cs index 758a38d6..1afbaff1 100644 --- a/src/Arc4u.Standard.OAuth2.AspNetCore/TokenProvider/CredentialTokenCacheTokenProvider.cs +++ b/src/Arc4u.Standard.OAuth2.AspNetCore/TokenProvider/CredentialTokenCacheTokenProvider.cs @@ -1,6 +1,3 @@ -using System; -using System.Threading.Tasks; -using Arc4u.Configuration; using Arc4u.Dependency; using Arc4u.Dependency.Attribute; using Arc4u.Diagnostics; @@ -84,7 +81,7 @@ public async Task GetTokenAsync(IKeyValueSettings settings, Credentia try { _logger.Technical().System($"Save the token in the cache for {cacheKey}, will expire at {tokenInfo.ExpiresOnUtc} Utc.").Log(); - TokenCache.Put(cacheKey, tokenInfo); + TokenCache.Put(cacheKey, tokenInfo.ExpiresOnUtc - DateTime.UtcNow, tokenInfo); } catch (Exception ex) { diff --git a/src/Arc4u.Standard.OAuth2/Extensions/TokenCacheExtension.cs b/src/Arc4u.Standard.OAuth2/Extensions/TokenCacheExtension.cs index 6ab46b1a..474b5551 100644 --- a/src/Arc4u.Standard.OAuth2/Extensions/TokenCacheExtension.cs +++ b/src/Arc4u.Standard.OAuth2/Extensions/TokenCacheExtension.cs @@ -1,4 +1,3 @@ -using System; using Arc4u.Configuration; using Arc4u.OAuth2.Options; using Microsoft.Extensions.Configuration; @@ -54,15 +53,9 @@ private static void AddTokenCache(IServiceCollection services, TokenCacheOptions throw new ConfigurationException("TokenCacheOptions.CacheName is not defined in the configuration file."); } - if (TimeSpan.Zero == tokenCacheOptions.MaxTime) - { - throw new ConfigurationException("TokenCacheOptions.MaxTime is not defined in the configuration file."); - } - services.Configure(options => { options.CacheName = tokenCacheOptions.CacheName; - options.MaxTime = tokenCacheOptions.MaxTime; }); } } diff --git a/src/Arc4u.Standard.OAuth2/Options/TokenCacheOptions.cs b/src/Arc4u.Standard.OAuth2/Options/TokenCacheOptions.cs index 2324d9df..3c0396f4 100644 --- a/src/Arc4u.Standard.OAuth2/Options/TokenCacheOptions.cs +++ b/src/Arc4u.Standard.OAuth2/Options/TokenCacheOptions.cs @@ -1,9 +1,5 @@ -using System; - namespace Arc4u.OAuth2.Options; public class TokenCacheOptions { - public TimeSpan MaxTime { get; set; } = TimeSpan.FromMinutes(20); - public string CacheName { get; set; } } diff --git a/src/Arc4u.Standard.OAuth2/Token/ApplicationCache.cs b/src/Arc4u.Standard.OAuth2/Token/ApplicationCache.cs index f115c97c..96badeaa 100644 --- a/src/Arc4u.Standard.OAuth2/Token/ApplicationCache.cs +++ b/src/Arc4u.Standard.OAuth2/Token/ApplicationCache.cs @@ -4,8 +4,6 @@ using Arc4u.OAuth2.Options; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; -using System; -using System.Collections.Generic; namespace Arc4u.OAuth2.Token { @@ -40,7 +38,7 @@ public void DeleteItem(string id) _logger.Technical().From().System($"Deleted information from the token cache for the id: {id}.").Log(); } - public void Put(string key, T data) + public void Put(string key, TimeSpan timeout, T data) { if (null == data) { @@ -49,7 +47,7 @@ public void Put(string key, T data) } _logger.Technical().From().System($"Adding token data information to the cache: {key}.").Log(); - _cache.Put(GetKey(key), _tokenCacheOptions.MaxTime, data); + _cache.Put(GetKey(key), timeout, data); _logger.Technical().From().System($"Added token data information to the cache: {key}.").Log(); } diff --git a/src/Arc4u.Standard.OAuth2/Token/ApplicationLocalDataCache.cs b/src/Arc4u.Standard.OAuth2/Token/ApplicationLocalDataCache.cs index c01df2b2..c9b392d2 100644 --- a/src/Arc4u.Standard.OAuth2/Token/ApplicationLocalDataCache.cs +++ b/src/Arc4u.Standard.OAuth2/Token/ApplicationLocalDataCache.cs @@ -2,7 +2,6 @@ using Arc4u.Dependency.Attribute; using Arc4u.Diagnostics; using Microsoft.Extensions.Logging; -using System; namespace Arc4u.OAuth2.Token { @@ -37,7 +36,7 @@ public void DeleteItem(string key) Clear(key); } - public void Put(string key, T data) + public void Put(string key, TimeSpan timeout, T data) { try { diff --git a/src/Arc4u.Standard.OAuth2/Token/ITokenCache.cs b/src/Arc4u.Standard.OAuth2/Token/ITokenCache.cs index bd1bb95a..7e0aae49 100644 --- a/src/Arc4u.Standard.OAuth2/Token/ITokenCache.cs +++ b/src/Arc4u.Standard.OAuth2/Token/ITokenCache.cs @@ -1,4 +1,4 @@ -namespace Arc4u.OAuth2.Token +namespace Arc4u.OAuth2.Token { public interface ITokenCache { @@ -9,7 +9,15 @@ public interface ITokenCache void DeleteItem(string key); - void Put(string key, T data); + /// + /// Set or overwrite the item with key with . + /// The item expires after + /// + /// + /// + /// + /// + void Put(string key, TimeSpan timeout, T data); T Get(string key); diff --git a/src/Arc4u.Standard.UnitTest/Security/JwtHttpHandlerTests.cs b/src/Arc4u.Standard.UnitTest/Security/JwtHttpHandlerTests.cs index 249f2edd..bf9bd7ff 100644 --- a/src/Arc4u.Standard.UnitTest/Security/JwtHttpHandlerTests.cs +++ b/src/Arc4u.Standard.UnitTest/Security/JwtHttpHandlerTests.cs @@ -1,13 +1,7 @@ -using System; -using System.Collections.Generic; using System.Diagnostics; using System.IdentityModel.Tokens.Jwt; -using System.Linq; using System.Net; -using System.Net.Http; using System.Security.Claims; -using System.Threading; -using System.Threading.Tasks; using Arc4u.Caching; using Arc4u.Configuration; using Arc4u.Dependency.ComponentModel; @@ -115,13 +109,13 @@ public async Task Jwt_With_OAuth2_And_Principal_With_OIDC_Token_Should() // Create a scope to be in the context majority of the time a business code is. using var scopedContainer = container.CreateScope(); - + scopedServiceAccessor.ServiceProvider = scopedContainer.ServiceProvider; var tokenRefresh = scopedContainer.Resolve(); tokenRefresh.RefreshToken = new TokenInfo("refresh_token", Guid.NewGuid().ToString(), DateTime.UtcNow.AddHours(1)); tokenRefresh.AccessToken = new TokenInfo("access_token", accessToken); - + // Define a Principal with no OAuth2Bearer token here => we test the injection. var appContext = scopedContainer.Resolve(); appContext.SetPrincipal(new AppPrincipal(new Arc4u.Security.Principal.Authorization(), new ClaimsIdentity(Constants.CookiesAuthenticationType), "S-1-0-0")); @@ -201,7 +195,7 @@ public async Task Jwt_With_OAuth2_And_Principal_With_Bearer_Token_Should() // Define a Principal with no OAuth2Bearer token here => we test the injection. var appContext = scopedContainer.Resolve(); - appContext.SetPrincipal(new AppPrincipal(new Arc4u.Security.Principal.Authorization(), new ClaimsIdentity(Constants.BearerAuthenticationType) { BootstrapContext = accessToken}, "S-1-0-0")); + appContext.SetPrincipal(new AppPrincipal(new Arc4u.Security.Principal.Authorization(), new ClaimsIdentity(Constants.BearerAuthenticationType) { BootstrapContext = accessToken }, "S-1-0-0")); var setingsOptions = scopedContainer.Resolve>(); @@ -247,7 +241,7 @@ public async Task Jwt_With_ClientSecet_Should() ["Authentication:ClientSecrets:Client1:Credential"] = $"{options.User}:password", ["Authentication:DefaultAuthority:Url"] = "https://login.microsoft.com" }; - foreach(var scope in options.Scopes) + foreach (var scope in options.Scopes) { configDic.Add($"Authentication:ClientSecrets:Client1:Scopes:{options.Scopes.IndexOf(scope)}", scope); } @@ -277,7 +271,7 @@ public async Task Jwt_With_ClientSecet_Should() // Mock the cache used by the Credential token provider. var mockTokenCache = _fixture.Freeze>(); mockTokenCache.Setup(m => m.Get(It.IsAny())).Returns((TokenInfo)null); - mockTokenCache.Setup(m => m.Put(It.IsAny(), It.IsAny())); + mockTokenCache.Setup(m => m.Put(It.IsAny(), TimeSpan.MaxValue, It.IsAny())); var mockHttpContextAccessor = _fixture.Freeze>(); diff --git a/src/Arc4u.Standard.UnitTest/Security/TokenCacheOptionsTests.cs b/src/Arc4u.Standard.UnitTest/Security/TokenCacheOptionsTests.cs index 0c1e826c..67a07066 100644 --- a/src/Arc4u.Standard.UnitTest/Security/TokenCacheOptionsTests.cs +++ b/src/Arc4u.Standard.UnitTest/Security/TokenCacheOptionsTests.cs @@ -1,15 +1,13 @@ -using AutoFixture.AutoMoq; -using AutoFixture; -using Xunit; -using Microsoft.Extensions.DependencyInjection; +using Arc4u.Configuration; using Arc4u.OAuth2.Extensions; using Arc4u.OAuth2.Options; -using Microsoft.Extensions.Options; +using AutoFixture; +using AutoFixture.AutoMoq; using FluentAssertions; -using System; using Microsoft.Extensions.Configuration; -using System.Collections.Generic; -using Arc4u.Configuration; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Options; +using Xunit; namespace Arc4u.UnitTest.Security; @@ -41,7 +39,6 @@ public void TockeCacheOption_From_Action_Should() var sut = options.Value; sut.CacheName.Should().Be("test"); - sut.MaxTime.Should().Be(TimeSpan.FromMinutes(20)); } [Fact] @@ -56,7 +53,6 @@ public void TockeCacheOption_From_Config_Should() new Dictionary { ["Authentication:TokenCache:CacheName"] = options.CacheName, - ["Authentication:TokenCache:MaxTime"] = options.MaxTime.ToString(), }).Build(); IConfiguration configuration = new ConfigurationRoot(new List(config.Providers)); @@ -70,7 +66,6 @@ public void TockeCacheOption_From_Config_Should() var sut = sutOptions.Value; sut.CacheName.Should().Be(options.CacheName); - sut.MaxTime.Should().Be(options.MaxTime); } [Fact] @@ -99,7 +94,6 @@ public void TockeCacheOption_From_Config_With_Default_MaxTime_Should() var sut = sutOptions.Value; sut.CacheName.Should().Be(options.CacheName); - sut.MaxTime.Should().Be(defaultOptions.MaxTime); } [Fact] @@ -113,8 +107,7 @@ public void TockeCacheOption_From_Config_With_MaxTime_To_Zero_Exception_Should() .AddInMemoryCollection( new Dictionary { - ["Authentication:TokenCache:CacheName"] = options.CacheName, - ["Authentication:TokenCache:MaxTime"] = TimeSpan.Zero.ToString(), + ["Authentication:TokenCache:CacheName"] = options.CacheName }).Build(); IConfiguration configuration = new ConfigurationRoot(new List(config.Providers)); @@ -134,8 +127,7 @@ public void TockeCacheOption_From_Config_With_No_CacheName_Exception_Should() .AddInMemoryCollection( new Dictionary { - ["Authentication:TokenCache:CacheName"] = "", - ["Authentication:TokenCache:MaxTime"] = TimeSpan.FromMinutes(20).ToString(), + ["Authentication:TokenCache:CacheName"] = "" }).Build(); IConfiguration configuration = new ConfigurationRoot(new List(config.Providers)); diff --git a/src/Arc4u.Standard.UnitTest/Security/gRpcInterceptorTests.cs b/src/Arc4u.Standard.UnitTest/Security/gRpcInterceptorTests.cs index 4383a246..8a6355d9 100644 --- a/src/Arc4u.Standard.UnitTest/Security/gRpcInterceptorTests.cs +++ b/src/Arc4u.Standard.UnitTest/Security/gRpcInterceptorTests.cs @@ -1,9 +1,6 @@ -using System; -using System.Collections.Generic; using System.Diagnostics; using System.IdentityModel.Tokens.Jwt; using System.Security.Claims; -using System.Threading; using Arc4u.Caching; using Arc4u.Configuration; using Arc4u.Dependency; @@ -11,7 +8,9 @@ using Arc4u.Diagnostics; using Arc4u.gRPC.Interceptors; using Arc4u.OAuth2; +using Arc4u.OAuth2.AspNetCore; using Arc4u.OAuth2.Extensions; +using Arc4u.OAuth2.Options; using Arc4u.OAuth2.Security.Principal; using Arc4u.OAuth2.Token; using Arc4u.OAuth2.TokenProvider; @@ -20,7 +19,9 @@ using AutoFixture; using AutoFixture.AutoMoq; using FluentAssertions; +using Grpc.Core; using Grpc.Core.Interceptors; +using Microsoft.AspNetCore.Http; using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; @@ -29,10 +30,6 @@ using Moq; using Xunit; using static Grpc.Core.Interceptors.Interceptor; -using Grpc.Core; -using Arc4u.OAuth2.Options; -using Microsoft.AspNetCore.Http; -using Arc4u.OAuth2.AspNetCore; namespace Arc4u.UnitTest.Security; @@ -233,7 +230,7 @@ public void Jwt_With_ClientSecet_Should() ["Authentication:ClientSecrets:Client1:Credential"] = $"{options.User}:password", ["Authentication:DefaultAuthority:Url"] = "https://login.microsoft.com" }; - foreach(var scope in options.Scopes) + foreach (var scope in options.Scopes) { configDic.Add($"Authentication:ClientSecrets:Client1:Scopes:{options.Scopes.IndexOf(scope)}", scope); } @@ -266,7 +263,7 @@ public void Jwt_With_ClientSecet_Should() // Mock the cache used by the Credential token provider. var mockTokenCache = _fixture.Freeze>(); mockTokenCache.Setup(m => m.Get(It.IsAny())).Returns((TokenInfo)null); - mockTokenCache.Setup(m => m.Put(It.IsAny(), It.IsAny())); + mockTokenCache.Setup(m => m.Put(It.IsAny(), TimeSpan.MaxValue, It.IsAny())); // Register the different TokenProvider and CredentialTokenProviders. var container = new ComponentModelContainer(services); @@ -539,7 +536,7 @@ public void Jwt_With_Principal_With_OAuth2_Token_For_Client_Scenario_Should() services.AddScoped(); services.AddSingleton(typeof(ILogger<>), typeof(NullLogger<>)); - // Register the different TokenProvider and CredentialTokenProviders. + // Register the different TokenProvider and CredentialTokenProviders. var container = new ComponentModelContainer(services); container.Register("Bootstrap"); container.CreateContainer(); From 8332c5ca66b6887d3d29cd17f17cf249664f9591 Mon Sep 17 00:00:00 2001 From: vvdb-architecture Date: Tue, 8 Oct 2024 15:06:51 +0200 Subject: [PATCH 2/3] Added selected suggestions from CodeRabbit --- .../Token/Cache.cs | 18 +++--- .../AppPrincipalTransform.cs | 58 ++++++++++--------- .../Options/ClaimsFillerOptions.cs | 3 +- .../Token/ITokenCache.cs | 2 +- 4 files changed, 45 insertions(+), 36 deletions(-) diff --git a/src/Arc4u.Standard.OAuth2.AspNetCore.Adal/Token/Cache.cs b/src/Arc4u.Standard.OAuth2.AspNetCore.Adal/Token/Cache.cs index 2df181bb..09b58a02 100644 --- a/src/Arc4u.Standard.OAuth2.AspNetCore.Adal/Token/Cache.cs +++ b/src/Arc4u.Standard.OAuth2.AspNetCore.Adal/Token/Cache.cs @@ -73,17 +73,19 @@ private void AfterAccessNotification(TokenCacheNotificationArgs args) if (HasStateChanged) { Logger.Technical().From().System($"Adding token information to the cache for the identifier: {_identifier}.").Log(); + var now = DateTimeOffset.UtcNow; - var maxExpires = now; - foreach (var tokenCacheItem in ReadItems()) + var maxExpires = ReadItems().Select(item => item.ExpiresOn).DefaultIfEmpty(now).Max(); + var timeout = maxExpires - now; + if (timeout > TimeSpan.Zero) + { + TokenCache.Put(_identifier, timeout, SerializeAdalV3()); + Logger.Technical().From().System($"Added token information to the cache for the identifier: {_identifier}.").Log(); + } + else { - if (tokenCacheItem.ExpiresOn > maxExpires) - { - maxExpires = tokenCacheItem.ExpiresOn; - } + Logger.Technical().From().System($"No valid tokens to cache for identifier: {_identifier} because of negative duration {timeout}.").Log(); } - TokenCache.Put(_identifier, maxExpires - now, SerializeAdalV3()); - Logger.Technical().From().System($"Added token information to the cache for the identifier: {_identifier}.").Log(); HasStateChanged = false; } diff --git a/src/Arc4u.Standard.OAuth2.AspNetCore/AppPrincipalTransform.cs b/src/Arc4u.Standard.OAuth2.AspNetCore/AppPrincipalTransform.cs index a1d35a8f..c726ba58 100644 --- a/src/Arc4u.Standard.OAuth2.AspNetCore/AppPrincipalTransform.cs +++ b/src/Arc4u.Standard.OAuth2.AspNetCore/AppPrincipalTransform.cs @@ -127,8 +127,8 @@ private async Task LoadExtraClaimsAsync(ClaimsIdentity? identity) // if cachedClaims is not null, it means that the information was in the cache and didn't expire yet. if (cachedClaims is not null) { - identity.AddClaims(cachedClaims.Where(c => !ClaimsToExclude.Contains(c.ClaimType)) - .Where(c => !identity.Claims.Any(c1 => StringComparer.OrdinalIgnoreCase.Equals(c1.Type, c.ClaimType))) + // Add the new claims to the identity + identity.AddClaims(cachedClaims.Where(c => !identity.Claims.Any(c1 => StringComparer.OrdinalIgnoreCase.Equals(c1.Type, c.ClaimType))) .Select(c => new Claim(c.ClaimType, c.Value))); return; } @@ -146,34 +146,33 @@ private async Task LoadExtraClaimsAsync(ClaimsIdentity? identity) // Load the claims into the identity but exclude the exp claim and the one already present. identity.AddClaims(claims.Where(c => !StringComparer.OrdinalIgnoreCase.Equals(c.ClaimType, tokenExpirationClaimType)) - .Where(c => !ClaimsToExclude.Contains(c.ClaimType)) .Where(c => !identity.Claims.Any(c1 => StringComparer.OrdinalIgnoreCase.Equals(c1.Type, c.ClaimType))) .Select(c => new Claim(c.ClaimType, c.Value))); - // Check expiry claim explicitly returned in the call to _claimsFiller.GetAsync. - var cachedExpiredClaim = claims.FirstOrDefault(c => StringComparer.OrdinalIgnoreCase.Equals(c.ClaimType, tokenExpirationClaimType))?.Value; + // Check expiry claim explicitly returned in the call to _claimsFiller.GetAsync + // If no expiry claim found in the call to _claimsFiller.GetAsync, try to locate one in the existing identity claims (which are normally obtained from the bearer token) + DateTimeOffset? expDate; - DateTimeOffset expDate; - if (cachedExpiredClaim is not null && long.TryParse(cachedExpiredClaim, NumberStyles.Integer, CultureInfo.InvariantCulture, out var cachedExpiredTicks)) + if (!TryParseExpirationDate(claims.FirstOrDefault(c => StringComparer.OrdinalIgnoreCase.Equals(c.ClaimType, tokenExpirationClaimType))?.Value, out expDate) && !TryParseExpirationDate(identity.Claims.FirstOrDefault(c => StringComparer.OrdinalIgnoreCase.Equals(c.Type, tokenExpirationClaimType))?.Value, out expDate)) { - expDate = DateTimeOffset.FromUnixTimeSeconds(cachedExpiredTicks); - } - else - { - // No expiry claim found in the call to _claimsFiller.GetAsync, try to locate one in the existing identity claims (which are normally obtained from the bearer token) - cachedExpiredClaim = identity.Claims.FirstOrDefault(c => StringComparer.OrdinalIgnoreCase.Equals(c.Type, tokenExpirationClaimType))?.Value; - if (cachedExpiredClaim is not null && long.TryParse(cachedExpiredClaim, NumberStyles.Integer, CultureInfo.InvariantCulture, out cachedExpiredTicks)) - { - expDate = DateTimeOffset.FromUnixTimeSeconds(cachedExpiredTicks); - } - else - { - expDate = DateTimeOffset.UtcNow.Add(_options.MaxTime); - } + // fall back to the configured max time. + expDate = DateTimeOffset.UtcNow.Add(_options.MaxTime); } - SaveClaimsToCache(claims, cacheKey, expDate - DateTimeOffset.UtcNow); + SaveClaimsToCache(claims, cacheKey, expDate!.Value - DateTimeOffset.UtcNow); + } + } + + private static bool TryParseExpirationDate(string? value, out DateTimeOffset? expirationDate) + { + if (value is not null && long.TryParse(value, NumberStyles.Integer, CultureInfo.InvariantCulture, out var ticks)) + { + expirationDate = DateTimeOffset.FromUnixTimeSeconds(ticks); + return true; } + + expirationDate = null; + return false; } private List? GetClaimsFromCache(string? cacheKey) @@ -201,13 +200,20 @@ private void SaveClaimsToCache([DisallowNull] IEnumerable claims, stri ArgumentNullException.ThrowIfNull(claims); - try + if (timeout > TimeSpan.Zero) { - _cacheHelper.GetCache().Put(cacheKey, timeout, claims); + try + { + _cacheHelper.GetCache().Put(cacheKey, timeout, claims); + } + catch (Exception ex) + { + _logger.Technical().Exception(ex).Log(); + } } - catch (Exception ex) + else { - _logger.Technical().Exception(ex).Log(); + _logger.Technical().LogWarning("The timeout is not valid to save the claims in the cache."); } } #endregion diff --git a/src/Arc4u.Standard.OAuth2.AspNetCore/Options/ClaimsFillerOptions.cs b/src/Arc4u.Standard.OAuth2.AspNetCore/Options/ClaimsFillerOptions.cs index e779408b..b45bea4f 100644 --- a/src/Arc4u.Standard.OAuth2.AspNetCore/Options/ClaimsFillerOptions.cs +++ b/src/Arc4u.Standard.OAuth2.AspNetCore/Options/ClaimsFillerOptions.cs @@ -9,7 +9,8 @@ public class ClaimsFillerOptions public bool LoadClaimsFromClaimsFillerProvider { get; set; } = true; /// - /// If is true, the extra claims are cached for ."/> + /// If is true, the extra claims are cached for . + /// The default value of 20 minutes provides a balance between performance and data freshness for most use cases. /// public TimeSpan MaxTime { get; set; } = TimeSpan.FromMinutes(20); diff --git a/src/Arc4u.Standard.OAuth2/Token/ITokenCache.cs b/src/Arc4u.Standard.OAuth2/Token/ITokenCache.cs index 7e0aae49..2d849b7d 100644 --- a/src/Arc4u.Standard.OAuth2/Token/ITokenCache.cs +++ b/src/Arc4u.Standard.OAuth2/Token/ITokenCache.cs @@ -11,7 +11,7 @@ public interface ITokenCache /// /// Set or overwrite the item with key with . - /// The item expires after + /// The item expires after . /// /// /// From f0c3fc690a5b0f1aa3f603007d6fc64ae0610017 Mon Sep 17 00:00:00 2001 From: vvdb-architecture Date: Wed, 9 Oct 2024 10:30:15 +0200 Subject: [PATCH 3/3] According to https://www.rfc-editor.org/rfc/rfc7519 section 10.1, claim names (a.k.a. "types") are case-sensitive. Claims manipulations have been factored out in a separate utility class. --- .../AppPrincipalTransform.cs | 43 +++---- .../Security/AppServicePrincipalFactory.cs | 12 +- .../Security/Principal/AppPrincipalFactory.cs | 40 +++--- .../Security/ClaimsUtility.cs | 117 ++++++++++++++++++ 4 files changed, 150 insertions(+), 62 deletions(-) create mode 100644 src/Arc4u.Standard.OAuth2/Security/ClaimsUtility.cs diff --git a/src/Arc4u.Standard.OAuth2.AspNetCore/AppPrincipalTransform.cs b/src/Arc4u.Standard.OAuth2.AspNetCore/AppPrincipalTransform.cs index c726ba58..03d2a4d3 100644 --- a/src/Arc4u.Standard.OAuth2.AspNetCore/AppPrincipalTransform.cs +++ b/src/Arc4u.Standard.OAuth2.AspNetCore/AppPrincipalTransform.cs @@ -1,6 +1,5 @@ using System.Diagnostics; using System.Diagnostics.CodeAnalysis; -using System.Globalization; using System.Security.Claims; using Arc4u.Configuration; using Arc4u.Dependency.Attribute; @@ -9,6 +8,7 @@ using Arc4u.OAuth2.Options; using Arc4u.OAuth2.Token; using Arc4u.Security.Principal; +using Arc4u.Standard.OAuth2.Security; using Microsoft.AspNetCore.Authentication; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; @@ -98,8 +98,6 @@ public async Task TransformAsync(ClaimsPrincipal principal) } #region Handling extra claims - private const string tokenExpirationClaimType = "exp"; - private static readonly HashSet ClaimsToExclude = new HashSet(StringComparer.OrdinalIgnoreCase) { "aud", "iss", "iat", "nbf", "acr", "aio", "appidacr", "ipaddr", "scp", "sub", "tid", "uti", "unique_name", "apptype", "appid", "ver", "http://schemas.microsoft.com/ws/2008/06/identity/claims/authenticationinstant", "http://schemas.microsoft.com/identity/claims/scope" }; /// /// This code is similar to the code in AppPrincipalFactory where the claims are stored in a secureCache. @@ -127,9 +125,8 @@ private async Task LoadExtraClaimsAsync(ClaimsIdentity? identity) // if cachedClaims is not null, it means that the information was in the cache and didn't expire yet. if (cachedClaims is not null) { - // Add the new claims to the identity - identity.AddClaims(cachedClaims.Where(c => !identity.Claims.Any(c1 => StringComparer.OrdinalIgnoreCase.Equals(c1.Type, c.ClaimType))) - .Select(c => new Claim(c.ClaimType, c.Value))); + // Add the new claims to the identity. Note that we need to merge the same way as in the case they are retrieved (see below) + identity.MergeClaims(cachedClaims); return; } @@ -142,37 +139,28 @@ private async Task LoadExtraClaimsAsync(ClaimsIdentity? identity) // Should receive specific extra claims. This is the responsibility of the caller to provide the right claims. // We expect the exp claim to be present. // if not the persistence time will be the default one. - var claims = (await _claimsFiller.GetAsync(identity, settings, null).ConfigureAwait(false)).Where(c => !ClaimsToExclude.Contains(c.ClaimType)).ToList(); + var claims = (await _claimsFiller.GetAsync(identity, settings, null).ConfigureAwait(false)).Sanitize().ToList(); // Load the claims into the identity but exclude the exp claim and the one already present. - identity.AddClaims(claims.Where(c => !StringComparer.OrdinalIgnoreCase.Equals(c.ClaimType, tokenExpirationClaimType)) - .Where(c => !identity.Claims.Any(c1 => StringComparer.OrdinalIgnoreCase.Equals(c1.Type, c.ClaimType))) - .Select(c => new Claim(c.ClaimType, c.Value))); + identity.MergeClaims(claims); + + TimeSpan timeout; // Check expiry claim explicitly returned in the call to _claimsFiller.GetAsync // If no expiry claim found in the call to _claimsFiller.GetAsync, try to locate one in the existing identity claims (which are normally obtained from the bearer token) - DateTimeOffset? expDate; - - if (!TryParseExpirationDate(claims.FirstOrDefault(c => StringComparer.OrdinalIgnoreCase.Equals(c.ClaimType, tokenExpirationClaimType))?.Value, out expDate) && !TryParseExpirationDate(identity.Claims.FirstOrDefault(c => StringComparer.OrdinalIgnoreCase.Equals(c.Type, tokenExpirationClaimType))?.Value, out expDate)) + if (claims.TryGetExpiration(out var ticks) || identity.Claims.TryGetExpiration(out ticks)) + { + // there's an expiration claim in the claims returned by the claims filler or in the existing identity claims, so we can compute an expiration date + timeout = DateTimeOffset.FromUnixTimeSeconds(ticks) - DateTimeOffset.UtcNow; + } + else { // fall back to the configured max time. - expDate = DateTimeOffset.UtcNow.Add(_options.MaxTime); + timeout = _options.MaxTime; } - SaveClaimsToCache(claims, cacheKey, expDate!.Value - DateTimeOffset.UtcNow); - } - } - - private static bool TryParseExpirationDate(string? value, out DateTimeOffset? expirationDate) - { - if (value is not null && long.TryParse(value, NumberStyles.Integer, CultureInfo.InvariantCulture, out var ticks)) - { - expirationDate = DateTimeOffset.FromUnixTimeSeconds(ticks); - return true; + SaveClaimsToCache(claims, cacheKey, timeout); } - - expirationDate = null; - return false; } private List? GetClaimsFromCache(string? cacheKey) @@ -198,6 +186,7 @@ private void SaveClaimsToCache([DisallowNull] IEnumerable claims, stri return; } + // Only if the cache key is valid does it make sense to test the claims validity ArgumentNullException.ThrowIfNull(claims); if (timeout > TimeSpan.Zero) diff --git a/src/Arc4u.Standard.OAuth2.AspNetCore/Security/AppServicePrincipalFactory.cs b/src/Arc4u.Standard.OAuth2.AspNetCore/Security/AppServicePrincipalFactory.cs index 604b98db..b0d55442 100644 --- a/src/Arc4u.Standard.OAuth2.AspNetCore/Security/AppServicePrincipalFactory.cs +++ b/src/Arc4u.Standard.OAuth2.AspNetCore/Security/AppServicePrincipalFactory.cs @@ -1,10 +1,6 @@ -using System; using System.Diagnostics; using System.IdentityModel.Tokens.Jwt; -using System.Linq; using System.Security.Claims; -using System.Threading; -using System.Threading.Tasks; using Arc4u.Configuration; using Arc4u.Dependency; using Arc4u.Dependency.Attribute; @@ -12,6 +8,7 @@ using Arc4u.OAuth2.Token; using Arc4u.Security.Principal; using Arc4u.ServiceModel; +using Arc4u.Standard.OAuth2.Security; using Microsoft.AspNetCore.Authentication; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; @@ -23,8 +20,6 @@ public class AppServicePrincipalFactory : IAppPrincipalFactory { public const string ProviderKey = "ProviderId"; - public static readonly string tokenExpirationClaimType = "exp"; - public static readonly string[] ClaimsToExclude = { "exp", "aud", "iss", "iat", "nbf", "acr", "aio", "appidacr", "ipaddr", "scp", "sub", "tid", "uti", "unique_name", "apptype", "appid", "ver", "http://schemas.microsoft.com/ws/2008/06/identity/claims/authenticationinstant", "http://schemas.microsoft.com/identity/claims/scope" }; private readonly IContainerResolve _container; private readonly ILogger _logger; @@ -102,13 +97,12 @@ private async Task BuildTheIdentity(ClaimsIdentity identity, IKeyValueSettings s } // Check the settings contains the service url. - TokenInfo? token = null; try { - token = await provider.GetTokenAsync(settings, parameter).ConfigureAwait(true); + var token = await provider.GetTokenAsync(settings, parameter).ConfigureAwait(true); identity.BootstrapContext = token.Token; var jwtToken = new JwtSecurityToken(token.Token); - identity.AddClaims(jwtToken.Claims.Where(c => !ClaimsToExclude.Any(arg => arg.Equals(c.Type))).Select(c => new Claim(c.Type, c.Value))); + identity.MergeClaims(jwtToken.Claims.Sanitize()); } catch (Exception ex) { diff --git a/src/Arc4u.Standard.OAuth2.Client/Security/Principal/AppPrincipalFactory.cs b/src/Arc4u.Standard.OAuth2.Client/Security/Principal/AppPrincipalFactory.cs index 4017ea14..819ce4ec 100644 --- a/src/Arc4u.Standard.OAuth2.Client/Security/Principal/AppPrincipalFactory.cs +++ b/src/Arc4u.Standard.OAuth2.Client/Security/Principal/AppPrincipalFactory.cs @@ -1,11 +1,5 @@ -using System; -using System.Collections.Generic; -using System.Globalization; using System.IdentityModel.Tokens.Jwt; -using System.Linq; using System.Security.Claims; -using System.Threading; -using System.Threading.Tasks; using Arc4u.Caching; using Arc4u.Configuration; using Arc4u.Dependency; @@ -16,6 +10,7 @@ using Arc4u.OAuth2.Token; using Arc4u.Security.Principal; using Arc4u.ServiceModel; +using Arc4u.Standard.OAuth2.Security; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; @@ -28,8 +23,6 @@ public class AppPrincipalFactory : IAppPrincipalFactory public const string DefaultSettingsResolveName = "OAuth2"; public const string PlatformParameters = "platformParameters"; - public static readonly string tokenExpirationClaimType = "exp"; - public static readonly string[] ClaimsToExclude = { "exp", "aud", "iss", "iat", "nbf", "acr", "aio", "appidacr", "ipaddr", "scp", "sub", "tid", "uti", "unique_name", "apptype", "appid", "ver", "http://schemas.microsoft.com/ws/2008/06/identity/claims/authenticationinstant", "http://schemas.microsoft.com/identity/claims/scope" }; private readonly IContainerResolve _container; private readonly INetworkInformation _networkInformation; @@ -118,10 +111,6 @@ private async Task BuildTheIdentity(ClaimsIdentity identity, IKeyValueSettings s // The token has claims filled by the STS. // We can fill the new federated identity with the claims from the token. var jwtToken = new JwtSecurityToken(token.Token); - var expTokenClaim = jwtToken.Claims.FirstOrDefault(c => c.Type.Equals(tokenExpirationClaimType, StringComparison.InvariantCultureIgnoreCase)); - long expTokenTicks = 0; - if (null != expTokenClaim) - long.TryParse(expTokenClaim.Value, NumberStyles.Integer, CultureInfo.InvariantCulture, out expTokenTicks); // The key for the cache is based on the claims from a ClaimsIdentity => build a dummy identity with the claim from the token. var dummyIdentity = new ClaimsIdentity(jwtToken.Claims); @@ -132,16 +121,12 @@ private async Task BuildTheIdentity(ClaimsIdentity identity, IKeyValueSettings s // if we have a token "cached" from the system, we can take the authorization claims from the cache (if exists)... // so we avoid too many backend calls for nothing. // But every time we have a token that has been refreshed, we will call the backend (if available and reload the claims). - var cachedExpiredClaim = cachedClaims.FirstOrDefault(c => c.ClaimType.Equals(tokenExpirationClaimType, StringComparison.InvariantCultureIgnoreCase)); - long cachedExpiredTicks = 0; - - if (null != cachedExpiredClaim) - long.TryParse(cachedExpiredClaim.Value, NumberStyles.Integer, CultureInfo.InvariantCulture, out cachedExpiredTicks); // we only call the backend if the ticks are not the same. - bool copyClaimsFromCache = cachedExpiredTicks > 0 && expTokenTicks > 0 && cachedClaims.Count > 0 && cachedExpiredTicks == expTokenTicks; - - if (copyClaimsFromCache) + var copyClaimsFromCache = jwtToken.Claims.TryGetExpiration(out Claim? expTokenClaim) && expTokenClaim.Value.TryParseTicks(out var expTokenTicks) + && cachedClaims.TryGetExpiration(out var cachedExpiredTicks) + && cachedExpiredTicks == expTokenTicks; + if (copyClaimsFromCache && cachedClaims.Count > 0) { identity.AddClaims(cachedClaims.Select(p => new Claim(p.ClaimType, p.Value))); messages.Add(new Message(ServiceModel.MessageCategory.Technical, MessageType.Information, "Create the principal from the cache, token has not been refreshed.")); @@ -149,21 +134,24 @@ private async Task BuildTheIdentity(ClaimsIdentity identity, IKeyValueSettings s else { // Fill the claims based on the token and the backend call - identity.AddClaims(jwtToken.Claims.Where(c => !ClaimsToExclude.Any(arg => arg.Equals(c.Type))).Select(c => new Claim(c.Type, c.Value))); - identity.AddClaim(expTokenClaim); + identity.AddClaims(jwtToken.Claims.Sanitize().Select(c => new Claim(c.Type, c.Value))); + // there is no guarantee that expTokenClaim is not null, so we need to check it. + if (expTokenClaim is not null) + { + identity.AddClaim(expTokenClaim); + } if (_container.TryResolve(out IClaimsFiller claimFiller)) // Fill the claims with more information. { try { // Get the claims and clean any technical claims in case of. - var claims = (await claimFiller.GetAsync(identity, new List { settings }, parameter)) - .Where(c => !ClaimsToExclude.Any(arg => arg.Equals(c.ClaimType))).ToList(); + var claims = (await claimFiller.GetAsync(identity, new List { settings }, parameter)).Sanitize().ToList(); // We copy the claims from the backend but the exp claim will be the value of the token (front end definition) and not the backend one. Otherwhise there will be always a difference. - identity.AddClaims(claims.Where(c => !identity.Claims.Any(c1 => c1.Type == c.ClaimType)).Select(c => new Claim(c.ClaimType, c.Value))); + identity.MergeClaims(claims); - messages.Add(new Message(ServiceModel.MessageCategory.Technical, MessageType.Information, $"Add {claims.Count()} claims to the principal.")); + messages.Add(new Message(ServiceModel.MessageCategory.Technical, MessageType.Information, $"Add {claims.Count} claims to the principal.")); messages.Add(new Message(ServiceModel.MessageCategory.Technical, MessageType.Information, $"Save claims to the cache.")); } catch (Exception e) diff --git a/src/Arc4u.Standard.OAuth2/Security/ClaimsUtility.cs b/src/Arc4u.Standard.OAuth2/Security/ClaimsUtility.cs new file mode 100644 index 00000000..0dbaeb43 --- /dev/null +++ b/src/Arc4u.Standard.OAuth2/Security/ClaimsUtility.cs @@ -0,0 +1,117 @@ +#if NET6_0_OR_GREATER +using System.Diagnostics.CodeAnalysis; +#endif +using System.Globalization; +using System.Security.Claims; +using Arc4u.IdentityModel.Claims; + +namespace Arc4u.Standard.OAuth2.Security; + +/// +/// Claims utilities and extension methods, used in AppPrincipalFactory and AppPrincipalTransform. +/// +/// +/// Claim type comparison is always case-sensitive. See https://www.rfc-editor.org/rfc/rfc7519 section 10.1. +/// +public static class ClaimsUtility +{ + private const string ExpirationClaimType = "exp"; + private static readonly HashSet ClaimsToExclude = ["aud", "iss", "iat", "nbf", "acr", "aio", "appidacr", "ipaddr", "scp", "sub", "tid", "uti", "unique_name", "apptype", "appid", "ver", "http://schemas.microsoft.com/ws/2008/06/identity/claims/authenticationinstant", "http://schemas.microsoft.com/identity/claims/scope"]; + + /// + /// Sanitize the list of external claims by removing the claims we don't need. + /// The may come from an external system. If there is an "exp" claim in the list, it will be included. + /// + /// + /// + public static IEnumerable Sanitize(this IEnumerable claims) + { + foreach (var claim in claims) + { + if (ClaimsToExclude.Contains(claim.ClaimType)) + { + continue; + } + yield return claim; + } + } + + /// + /// Sanitize the list of internal claims by removing the claims we don't need. + /// The come from the token claims . If there is an "exp" claim in the list, it will be *excluded* as well + /// since it may be replaced by the equivalent external claim. + /// + /// + /// + public static IEnumerable Sanitize(this IEnumerable claims) + { + foreach (var claim in claims) + { + if (ClaimsToExclude.Contains(claim.Type) || claim.Type == ExpirationClaimType) + { + continue; + } + yield return claim; + } + } + + /// + /// Merge the into the , but only the claims that are not already present. + /// The "exp" claim is always excluded from the merge. + /// + /// + /// + public static void MergeClaims(this ClaimsIdentity claimsIdentity, IEnumerable claimsToMerge) + { + claimsIdentity.AddClaims(claimsToMerge.Where(c => c.ClaimType != ExpirationClaimType && !claimsIdentity.HasClaim(c1 => c1.Type == c.ClaimType)).Select(c => new Claim(c.ClaimType, c.Value))); + } + + /// + /// Merge the into the , but only the claims that are not already present. + /// The "exp" claim is always excluded from the merge. + /// + /// + /// + public static void MergeClaims(this ClaimsIdentity claimsIdentity, IEnumerable claimsToMerge) + { + claimsIdentity.AddClaims(claimsToMerge.Where(c => c.Type != ExpirationClaimType && !claimsIdentity.HasClaim(c1 => c1.Type == c.Type)).Select(c => new Claim(c.Type, c.Value))); + } + +#if NET6_0_OR_GREATER + public static bool TryGetExpiration(this IEnumerable claims, [NotNullWhen(true)] out Claim? expClaim) +#else + public static bool TryGetExpiration(this IEnumerable claims, out Claim expClaim) +#endif + { + expClaim = claims.FirstOrDefault(c => c.Type == ExpirationClaimType); + return expClaim is not null; + } + + public static bool TryGetExpiration(this IEnumerable claims, out long ticks) + { + var claim = claims.FirstOrDefault(c => c.Type == ExpirationClaimType); + + if (claim is null) + { + ticks = 0; + return false; + } + + return claim.Value.TryParseTicks(out ticks); + } + + public static bool TryGetExpiration(this IEnumerable claims, out long ticks) + { + var claim = claims.FirstOrDefault(c => c.ClaimType == ExpirationClaimType); + + if (claim is null) + { + ticks = 0; + return false; + } + + return claim.Value.TryParseTicks(out ticks); + } + + public static bool TryParseTicks(this string s, out long ticks) => long.TryParse(s, NumberStyles.Integer, CultureInfo.InvariantCulture, out ticks); +}