DuendeSoftware / Duende.AccessTokenManagement

Automatic token management for machine to machine and user-centric web app OAuth and OIDC flows
Apache License 2.0
225 stars 68 forks source link

Consider ability to control `IDistributedCache` instance used #145

Open mderriey opened 1 week ago

mderriey commented 1 week ago

Which version of Duende.AccessTokenManagement are you using?

v3.0.0.

Which version of .NET are you using?

.NET 8.

Describe the bug feature request

We'd like to open a conversation with regards to how Duende.AccessTokenManagement resolves the IDistributedCache implementation it uses under the hood.

We come from a scenario where we didn't use to cache app-related data, so we registered the in-memory distributed cache implementation, which the library happily used.

Later, we decided we needed an out-of-process cache to store some commonly-used data across multiple instances. We swapped out the in-memory distributed cache implementation by the StackExchange.Redis one.

It works, but it means that each instance needs to get the token from Redis for every HTTP request going to the protected service.

It's not that bad, really, but we don't really care whether the different instances use the same access tokens, and if we had the choice we'd rather store them in memory.

We're not interested in per-client selection of IDistributedCache, I'll let you decide if it's a scenario you'd like to take into account.

Some ideas up for discussion if you agree the conversation should go forward, both of which rely on adding "service-level" options for the client credentials token management:

To Reproduce

N/A.

Expected behavior

N/A.

Log output/exception with stacktrace

N/A

Additional context

I don't think there's any apart from the feature request description. Let me know otherwise.

mderriey commented 1 week ago

We currently have a hacky solution for this, which goes into implementation detail territory, and overrides the registrations of services that depend on IDistributedCache and inject a keyed InMemoryDistributedCache implementation.

public static IServiceCollection ForceUseInMemoryCacheForClientCredentialsTokenManagement(this IServiceCollection services)
{
    const string memoryDistributedCacheKey = nameof(MemoryDistributedCache);

    return services
        .AddKeyedSingleton<IDistributedCache, MemoryDistributedCache>(memoryDistributedCacheKey)
        .AddTransient<IClientCredentialsTokenCache>(provider =>
        {
            return new DistributedClientCredentialsTokenCache(
                provider.GetRequiredKeyedService<IDistributedCache>(memoryDistributedCacheKey),
                provider.GetRequiredService<IOptions<ClientCredentialsTokenManagementOptions>>(),
                provider.GetRequiredService<ILogger<DistributedClientCredentialsTokenCache>>());
        })
        .AddTransient<IDPoPNonceStore>(provider =>
        {
            return new DistributedDPoPNonceStore(
                provider.GetRequiredKeyedService<IDistributedCache>(memoryDistributedCacheKey),
                provider.GetRequiredService<ILogger<DistributedDPoPNonceStore>>());
        });
}

It seems to be working OK, but requires us to stay on top of the library internals in case the implementation changes and introduces a new service that depends on IDistributedCache.

AndersAbel commented 3 days ago

@mderriey Thank you for the suggestion. I have worked a bit with keyed services in .NET 8 and my personal opinion is that those could be a great fit for this. We could try resolving the service through a known key first and if that is not available we would fall back to the default non-named registration.

We will review the suggestion.