diff --git a/Directory.Packages.props b/Directory.Packages.props index a2748e56f..1977c58f6 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -17,14 +17,14 @@ 8.0.1 - 8.0.0 + 9.0.3 [8.0.1,9.0.0) 7.0.8 9.0.0 - 9.0.0 + 9.0.3 [8.0.1,9.0.0) 7.0.8 @@ -40,7 +40,7 @@ - + diff --git a/access-token-management/samples/WorkerDI/Program.cs b/access-token-management/samples/WorkerDI/Program.cs index 81ba6c2e8..d65320b03 100644 --- a/access-token-management/samples/WorkerDI/Program.cs +++ b/access-token-management/samples/WorkerDI/Program.cs @@ -31,8 +31,6 @@ public static IHostBuilder CreateHostBuilder(string[] args) .ConfigureServices((services) => { - services.AddDistributedMemoryCache(); - services.AddClientCredentialsTokenManagement(); services.AddSingleton(new DiscoveryCache("https://demo.duendesoftware.com")); services.AddSingleton, ClientCredentialsClientConfigureOptions>(); diff --git a/access-token-management/src/AccessTokenManagement/AccessTokenManagement.csproj b/access-token-management/src/AccessTokenManagement/AccessTokenManagement.csproj index 38058944a..93c3a1c54 100644 --- a/access-token-management/src/AccessTokenManagement/AccessTokenManagement.csproj +++ b/access-token-management/src/AccessTokenManagement/AccessTokenManagement.csproj @@ -10,7 +10,7 @@ - + diff --git a/access-token-management/src/AccessTokenManagement/ClientCredentialsTokenManagementService.cs b/access-token-management/src/AccessTokenManagement/ClientCredentialsTokenManagementService.cs index 05b5552b0..313717b9b 100644 --- a/access-token-management/src/AccessTokenManagement/ClientCredentialsTokenManagementService.cs +++ b/access-token-management/src/AccessTokenManagement/ClientCredentialsTokenManagementService.cs @@ -11,8 +11,7 @@ namespace Duende.AccessTokenManagement; /// public class ClientCredentialsTokenManagementService( IClientCredentialsTokenEndpointService clientCredentialsTokenEndpointService, - IClientCredentialsTokenCache tokenCache, - ILogger logger + IClientCredentialsTokenCache tokenCache ) : IClientCredentialsTokenManagementService { @@ -24,27 +23,6 @@ public async Task GetAccessTokenAsync( { parameters ??= new TokenRequestParameters(); - if (parameters.ForceRenewal == false) - { - try - { - var item = await tokenCache.GetAsync( - clientName: clientName, - requestParameters: parameters, - cancellationToken: cancellationToken).ConfigureAwait(false); - if (item != null) - { - return item; - } - } - catch (Exception e) - { - logger.LogError(e, - "Error trying to obtain token from cache for client {clientName}. Error = {error}. Will obtain new token.", - clientName, e.Message); - } - } - return await tokenCache.GetOrCreateAsync( clientName: clientName, requestParameters: parameters, @@ -58,12 +36,12 @@ private async Task InvokeGetAccessToken(string clientNam } /// - public Task DeleteAccessTokenAsync( + public async Task DeleteAccessTokenAsync( string clientName, TokenRequestParameters? parameters = null, CancellationToken cancellationToken = default) { parameters ??= new TokenRequestParameters(); - return tokenCache.DeleteAsync(clientName, parameters, cancellationToken); + await tokenCache.DeleteAsync(clientName, parameters, cancellationToken); } } \ No newline at end of file diff --git a/access-token-management/src/AccessTokenManagement/ClientCredentialsTokenManagementServiceCollectionExtensions.cs b/access-token-management/src/AccessTokenManagement/ClientCredentialsTokenManagementServiceCollectionExtensions.cs index d7e7d16d4..7b99aaccd 100644 --- a/access-token-management/src/AccessTokenManagement/ClientCredentialsTokenManagementServiceCollectionExtensions.cs +++ b/access-token-management/src/AccessTokenManagement/ClientCredentialsTokenManagementServiceCollectionExtensions.cs @@ -2,6 +2,7 @@ // Licensed under the Apache License, Version 2.0. See LICENSE in the project root for license information. using Duende.AccessTokenManagement; +using Microsoft.Extensions.Caching.Hybrid; using Microsoft.Extensions.DependencyInjection.Extensions; using Microsoft.Extensions.Logging; @@ -48,6 +49,19 @@ public static ClientCredentialsTokenManagementBuilder AddClientCredentialsTokenM services.AddHttpClient(ClientCredentialsTokenManagementDefaults.BackChannelHttpClientName); + // We can add the hybrid cache, because it does a 'tryadd' everywhere + services.AddHybridCache(); + + // Add indirection from keyed service to non-keyed service + services.TryAddKeyedSingleton( + serviceKey: ServiceProviderKeys.DistributedClientCredentialsTokenCache, + implementationFactory: (sp, _) => sp.GetRequiredService()); + + // Add indirection from keyed service to non-keyed service + services.TryAddKeyedSingleton( + serviceKey: ServiceProviderKeys.DistributedDPoPNonceStore, + implementationFactory: (sp, _) => sp.GetRequiredService()); + return new ClientCredentialsTokenManagementBuilder(services); } diff --git a/access-token-management/src/AccessTokenManagement/DistributedClientCredentialsTokenCache.cs b/access-token-management/src/AccessTokenManagement/DistributedClientCredentialsTokenCache.cs index 6bcf88c53..032ae41c1 100644 --- a/access-token-management/src/AccessTokenManagement/DistributedClientCredentialsTokenCache.cs +++ b/access-token-management/src/AccessTokenManagement/DistributedClientCredentialsTokenCache.cs @@ -1,31 +1,33 @@ // Copyright (c) Duende Software. All rights reserved. // Licensed under the Apache License, Version 2.0. See LICENSE in the project root for license information. -using System.Collections.Concurrent; -using System.Text.Json; -using Microsoft.Extensions.Caching.Distributed; +using Microsoft.Extensions.Caching.Hybrid; +using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; namespace Duende.AccessTokenManagement; +public static class ServiceProviderKeys +{ + public const string DistributedClientCredentialsTokenCache = "DistributedClientCredentialsTokenCache"; + public const string DistributedDPoPNonceStore = "DistributedDPoPNonceStore"; +} + /// /// Client access token cache using IDistributedCache /// public class DistributedClientCredentialsTokenCache( - IDistributedCache cache, + [FromKeyedServices(ServiceProviderKeys.DistributedClientCredentialsTokenCache)]HybridCache cache, + TimeProvider time, ITokenRequestSynchronization synchronization, IOptions options, ILogger logger ) : IClientCredentialsTokenCache { - private readonly IDistributedCache _cache = cache; - private readonly ITokenRequestSynchronization _synchronization = synchronization; - private readonly ILogger _logger = logger; private readonly ClientCredentialsTokenManagementOptions _options = options.Value; - /// public async Task SetAsync( string clientName, @@ -34,21 +36,38 @@ public async Task SetAsync( CancellationToken cancellationToken = default) { ArgumentNullException.ThrowIfNull(clientName); - - var cacheExpiration = clientCredentialsToken.Expiration.AddSeconds(-_options.CacheLifetimeBuffer); - var data = JsonSerializer.Serialize(clientCredentialsToken); - var entryOptions = new DistributedCacheEntryOptions + try { - AbsoluteExpiration = cacheExpiration - }; + var entryOptions = GetHybridCacheEntryOptions(clientCredentialsToken); - _logger.LogTrace("Caching access token for client: {clientName}. Expiration: {expiration}", clientName, cacheExpiration); - - var cacheKey = GenerateCacheKey(_options, clientName, requestParameters); - await _cache.SetStringAsync(cacheKey, data, entryOptions, token: cancellationToken).ConfigureAwait(false); + var cacheKey = GenerateCacheKey(_options, clientName, requestParameters); + logger.LogTrace("Caching access token for client: {clientName}. Expiration: {expiration}", clientName, entryOptions.Expiration); + await cache.SetAsync(cacheKey, clientCredentialsToken, entryOptions, cancellationToken: cancellationToken).ConfigureAwait(false); + } + catch (Exception e) + { + logger.LogError(e, + "Error trying to set token in cache for client {clientName}. Error = {error}", + clientName, e.Message); + } + } + + private HybridCacheEntryOptions GetHybridCacheEntryOptions(ClientCredentialsToken clientCredentialsToken) + { + var absoluteCacheExpiration = clientCredentialsToken.Expiration.AddSeconds(-_options.CacheLifetimeBuffer); + var relativeCacheExpiration = absoluteCacheExpiration - time.GetUtcNow(); + var entryOptions = new HybridCacheEntryOptions() + { + Expiration = relativeCacheExpiration + }; + return entryOptions; } + private class TokenErrorException(ClientCredentialsToken token) : Exception + { + public ClientCredentialsToken Token { get; } = token; + } public async Task GetOrCreateAsync( string clientName, TokenRequestParameters requestParameters, Func> factory, @@ -58,64 +77,57 @@ public async Task GetOrCreateAsync( var cacheKey = GenerateCacheKey(_options, clientName, requestParameters); - return await _synchronization.SynchronizeAsync(cacheKey, async () => + ClientCredentialsToken? token; + if (!requestParameters.ForceRenewal) { - var token = await factory(clientName, requestParameters, cancellationToken).ConfigureAwait(false); - if (token.IsError) - { - _logger.LogError( - "Error requesting access token for client {clientName}. Error = {error}.", - clientName, token.Error); - - return token; - } + // We don't need the token to be absolutely fresh, so we can get one from the cache. + token = await cache.GetOrDefaultAsync( + key: cacheKey, + cancellationToken: cancellationToken); - try - { - await SetAsync(clientName, token, requestParameters, cancellationToken).ConfigureAwait(false); - } - catch (Exception e) + if (token?.Expiration > DateTimeOffset.MinValue) { - _logger.LogError(e, - "Error trying to set token in cache for client {clientName}. Error = {error}", - clientName, e.Message); + var absoluteCacheExpiration = token.Expiration.AddSeconds(-_options.CacheLifetimeBuffer); + + if (absoluteCacheExpiration > time.GetUtcNow()) + { + // It's possible that we have only read the token from L2 cache, not L1 cache. + // just to be sure, write the token also into L1 cache (which should be fast) + // https://github.com/dotnet/extensions/issues/5688#issuecomment-2692247434 + var defaultWriteOptions = GetHybridCacheEntryOptions(token); + await cache.SetAsync(cacheKey, token, new HybridCacheEntryOptions() + { + Flags = HybridCacheEntryFlags.DisableDistributedCacheWrite, + Expiration = defaultWriteOptions.Expiration, + LocalCacheExpiration = defaultWriteOptions.LocalCacheExpiration + }, cancellationToken: cancellationToken); + + return token; + } } + } - return token; - }).ConfigureAwait(false); - } - - /// - public async Task GetAsync( - string clientName, - TokenRequestParameters requestParameters, - CancellationToken cancellationToken = default) - { - ArgumentNullException.ThrowIfNull(clientName); - - var cacheKey = GenerateCacheKey(_options, clientName, requestParameters); - var entry = await _cache.GetStringAsync(cacheKey, token: cancellationToken).ConfigureAwait(false); - - if (entry != null) + // Apparently, there's either no value in the cache, or we want a fresh one. + // Since we aren't using GetOrCreate, we'll have to protect against cache stampedes ourselves. + return await synchronization.SynchronizeAsync(cacheKey, async () => { - try - { - _logger.LogDebug("Cache hit for access token for client: {clientName}", clientName); - return JsonSerializer.Deserialize(entry); - } - catch (Exception ex) + token = await factory(clientName, requestParameters, cancellationToken).ConfigureAwait(false); + + // Don't cache the token if there's an error. + if (!token.IsError) { - _logger.LogCritical(ex, "Error parsing cached access token for client {clientName}", clientName); - return null; + await SetAsync(clientName, token, requestParameters, cancellationToken); } - } - _logger.LogTrace("Cache miss for access token for client: {clientName}", clientName); - return null; + return token; + }); + + } + /// - public Task DeleteAsync( + public ValueTask DeleteAsync( string clientName, TokenRequestParameters requestParameters, CancellationToken cancellationToken = default) @@ -123,7 +135,7 @@ public Task DeleteAsync( if (clientName is null) throw new ArgumentNullException(nameof(clientName)); var cacheKey = GenerateCacheKey(_options, clientName, requestParameters); - return _cache.RemoveAsync(cacheKey, cancellationToken); + return cache.RemoveAsync(cacheKey, cancellationToken); } /// diff --git a/access-token-management/src/AccessTokenManagement/DistributedDPoPNonceStore.cs b/access-token-management/src/AccessTokenManagement/DistributedDPoPNonceStore.cs index 4e1c5e7d4..ecca87993 100644 --- a/access-token-management/src/AccessTokenManagement/DistributedDPoPNonceStore.cs +++ b/access-token-management/src/AccessTokenManagement/DistributedDPoPNonceStore.cs @@ -1,8 +1,10 @@ // Copyright (c) Duende Software. All rights reserved. // Licensed under the Apache License, Version 2.0. See LICENSE in the project root for license information. -using Microsoft.Extensions.Caching.Distributed; +using Microsoft.Extensions.Caching.Hybrid; +using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; +using Microsoft.IdentityModel.Tokens; namespace Duende.AccessTokenManagement; @@ -14,16 +16,21 @@ public class DistributedDPoPNonceStore : IDPoPNonceStore const string CacheKeyPrefix = "DistributedDPoPNonceStore"; const char CacheKeySeparator = ':'; - private readonly IDistributedCache _cache; + private readonly HybridCache _cache; private readonly ILogger _logger; + private static readonly HybridCacheEntryOptions NonceStoreCacheOptions = new HybridCacheEntryOptions() + { + LocalCacheExpiration = TimeSpan.FromHours(1) + }; + /// /// ctor /// /// /// public DistributedDPoPNonceStore( - IDistributedCache cache, + [FromKeyedServices(ServiceProviderKeys.DistributedDPoPNonceStore)]HybridCache cache, ILogger logger) { _cache = cache; @@ -36,7 +43,7 @@ public DistributedDPoPNonceStore( ArgumentNullException.ThrowIfNull(context); var cacheKey = GenerateCacheKey(context); - var entry = await _cache.GetStringAsync(cacheKey, token: cancellationToken).ConfigureAwait(false); + var entry = await _cache.GetOrDefaultAsync(cacheKey).ConfigureAwait(false); if (entry != null) { @@ -53,18 +60,13 @@ public virtual async Task StoreNonceAsync(DPoPNonceContext context, string nonce { ArgumentNullException.ThrowIfNull(context); - var cacheExpiration = DateTimeOffset.UtcNow.AddHours(1); var data = nonce; - var entryOptions = new DistributedCacheEntryOptions - { - AbsoluteExpiration = cacheExpiration - }; - _logger.LogTrace("Caching DPoP nonce for URL: {url}, method: {method}. Expiration: {expiration}", context.Url, context.Method, cacheExpiration); + _logger.LogTrace("Caching DPoP nonce for URL: {url}, method: {method}. Expiration: {expiration}", context.Url, context.Method, NonceStoreCacheOptions.Expiration); var cacheKey = GenerateCacheKey(context); - await _cache.SetStringAsync(cacheKey, data, entryOptions, token: cancellationToken).ConfigureAwait(false); + await _cache.SetAsync(cacheKey, data, NonceStoreCacheOptions, cancellationToken: cancellationToken).ConfigureAwait(false); } diff --git a/access-token-management/src/AccessTokenManagement/HybridCacheExtMethods.cs b/access-token-management/src/AccessTokenManagement/HybridCacheExtMethods.cs new file mode 100644 index 000000000..9c2edf2ae --- /dev/null +++ b/access-token-management/src/AccessTokenManagement/HybridCacheExtMethods.cs @@ -0,0 +1,27 @@ +using Microsoft.Extensions.Caching.Hybrid; + +namespace Duende.AccessTokenManagement; + +/// +/// This extension method is created because we don't yet have a 'GetOrDefault' method +/// on HybridCache. This is under consideration: +/// +/// https://github.com/dotnet/extensions/issues/5688#issuecomment-2692247434 +/// +public static class HybridCacheExtMethods +{ + private static readonly HybridCacheEntryOptions GetOnlyEntryOptions = new() + { + Flags = HybridCacheEntryFlags.DisableLocalCacheWrite + | HybridCacheEntryFlags.DisableDistributedCacheWrite + | HybridCacheEntryFlags.DisableUnderlyingData + }; + + public static async ValueTask GetOrDefaultAsync(this HybridCache cache, string key, CancellationToken cancellationToken = default) + { + return await cache.GetOrCreateAsync( + key, + null!, // Don't return a value if it's not in the cache. Also, don't write it to the cache + GetOnlyEntryOptions, cancellationToken: cancellationToken); + } +} \ No newline at end of file diff --git a/access-token-management/src/AccessTokenManagement/Interfaces/IClientCredentialsTokenCache.cs b/access-token-management/src/AccessTokenManagement/Interfaces/IClientCredentialsTokenCache.cs index 053e00187..a8936f3be 100644 --- a/access-token-management/src/AccessTokenManagement/Interfaces/IClientCredentialsTokenCache.cs +++ b/access-token-management/src/AccessTokenManagement/Interfaces/IClientCredentialsTokenCache.cs @@ -28,18 +28,6 @@ Task GetOrCreateAsync( Func> factory, CancellationToken cancellationToken = default); - /// - /// Retrieves a client access token from the cache - /// - /// - /// - /// - /// - Task GetAsync( - string clientName, - TokenRequestParameters requestParameters, - CancellationToken cancellationToken = default); - /// /// Deletes a client access token from the cache /// @@ -47,8 +35,7 @@ Task GetOrCreateAsync( /// /// /// - Task DeleteAsync( - string clientName, + ValueTask DeleteAsync(string clientName, TokenRequestParameters requestParameters, CancellationToken cancellationToken = default); } \ No newline at end of file diff --git a/access-token-management/test/AccessTokenManagement.Tests/BackChannelClientTests.cs b/access-token-management/test/AccessTokenManagement.Tests/BackChannelClientTests.cs index a2fd0f2f9..092333c12 100644 --- a/access-token-management/test/AccessTokenManagement.Tests/BackChannelClientTests.cs +++ b/access-token-management/test/AccessTokenManagement.Tests/BackChannelClientTests.cs @@ -7,7 +7,6 @@ using Duende.IdentityModel.Client; using Microsoft.Extensions.DependencyInjection; using RichardSzalay.MockHttp; - namespace Duende.AccessTokenManagement.Tests; public class BackChannelClientTests(ITestOutputHelper output) @@ -17,7 +16,6 @@ public async Task Get_access_token_uses_default_backchannel_client_from_factory( { var services = new ServiceCollection(); - services.AddDistributedMemoryCache(); services.AddClientCredentialsTokenManagement() .AddClient("test", client => { @@ -48,7 +46,6 @@ public async Task Get_access_token_uses_custom_backchannel_client_from_factory() { var services = new ServiceCollection(); - services.AddDistributedMemoryCache(); services.AddClientCredentialsTokenManagement() .AddClient("test", client => { @@ -80,7 +77,6 @@ public async Task Getting_a_token_with_different_scope_twice_sequentially_will_r { var services = new ServiceCollection(); - services.AddDistributedMemoryCache(); services.AddClientCredentialsTokenManagement() .AddClient("test", client => { @@ -130,7 +126,6 @@ public async Task Getting_a_token_with_different_scope_twice_concurrently_will_r { var services = new ServiceCollection(); - services.AddDistributedMemoryCache(); services.AddClientCredentialsTokenManagement() .AddClient("test", client => { @@ -199,7 +194,6 @@ public async Task Getting_a_token_with_different_parameters_twice_concurrently_w { var services = new ServiceCollection(); - services.AddDistributedMemoryCache(); services.AddClientCredentialsTokenManagement() .AddClient("test", client => { @@ -278,7 +272,6 @@ public async Task Get_access_token_uses_specific_http_client_instance() var services = new ServiceCollection(); - services.AddDistributedMemoryCache(); services.AddClientCredentialsTokenManagement() .AddClient("test", client => { diff --git a/access-token-management/test/AccessTokenManagement.Tests/ClientTokenManagementTests.cs b/access-token-management/test/AccessTokenManagement.Tests/ClientTokenManagementTests.cs index bba55fb63..bc50eea4e 100644 --- a/access-token-management/test/AccessTokenManagement.Tests/ClientTokenManagementTests.cs +++ b/access-token-management/test/AccessTokenManagement.Tests/ClientTokenManagementTests.cs @@ -113,7 +113,7 @@ public async Task Token_request_and_response_should_have_expected_values(ClientC { access_token = "access_token", token_type = "token_type", - expires_in = 60, + expires_in = 100, scope = "scope" }; @@ -236,7 +236,7 @@ public async Task Request_parameters_should_take_precedence_over_configuration() { access_token = "access_token", token_type = "token_type", - expires_in = 60, + expires_in = 100, scope = "scope_per_request" }; @@ -295,7 +295,7 @@ public async Task Request_assertions_should_be_sent_correctly() { access_token = "access_token", token_type = "token_type", - expires_in = 60, + expires_in = 100, scope = "scope" }; @@ -348,7 +348,7 @@ public async Task Service_assertions_should_be_sent_correctly() { access_token = "access_token", token_type = "token_type", - expires_in = 60, + expires_in = 100, scope = "scope" }; @@ -410,7 +410,7 @@ public async Task Request_assertion_should_take_precedence_over_service_assertio { access_token = "access_token", token_type = "token_type", - expires_in = 60, + expires_in = 100, scope = "scope" }; diff --git a/access-token-management/test/AccessTokenManagement.Tests/HybridCacheExplorationTests.cs b/access-token-management/test/AccessTokenManagement.Tests/HybridCacheExplorationTests.cs new file mode 100644 index 000000000..3e26d1ba2 --- /dev/null +++ b/access-token-management/test/AccessTokenManagement.Tests/HybridCacheExplorationTests.cs @@ -0,0 +1,42 @@ +using Microsoft.Extensions.Caching.Hybrid; +using Microsoft.Extensions.DependencyInjection; + +namespace Duende.AccessTokenManagement.Tests; + +public class HybridCacheExplorationTests +{ + [Fact] + public async Task Exception_is_not_written_to_cache() + { + var services = new ServiceCollection() + .AddHybridCache() + .Services; + + var cache = services.BuildServiceProvider() + .GetRequiredService(); + + int count = 0; + object item; + + try + { + item = await cache.GetOrCreateAsync("key", (_) => + { + count++; + throw new InvalidOperationException(); + }); + } + catch (InvalidOperationException) + { + + } + item = await cache.GetOrCreateAsync("key", (_) => + { + count++; + return ValueTask.FromResult(null); + }); + + item.ShouldBeNull(); + count.ShouldBe(2); + } +} \ No newline at end of file diff --git a/identity-model-oidc-client/test/IdentityModel.OidcClient.Tests/DPoP/Framework/DPoP/DPoPServiceCollectionExtensions.cs b/identity-model-oidc-client/test/IdentityModel.OidcClient.Tests/DPoP/Framework/DPoP/DPoPServiceCollectionExtensions.cs index 640265ab4..ab525db53 100644 --- a/identity-model-oidc-client/test/IdentityModel.OidcClient.Tests/DPoP/Framework/DPoP/DPoPServiceCollectionExtensions.cs +++ b/identity-model-oidc-client/test/IdentityModel.OidcClient.Tests/DPoP/Framework/DPoP/DPoPServiceCollectionExtensions.cs @@ -16,6 +16,7 @@ public static IServiceCollection ConfigureDPoPTokensForScheme(this IServiceColle services.AddTransient(); services.AddTransient(); services.AddDistributedMemoryCache(); + services.AddTransient(); services.AddSingleton>(new ConfigureJwtBearerOptions(scheme));