How are you extending Duende.AccessTokenManagement?

[Erwinvandervalk]

As we work on adding new features to AccessTokenManagement, we've encountered some challenges with its current design, particularly regarding extensibility. The existing approach leans towards inheritance rather than composition, which comes with certain drawbacks. For instance, adding a constructor parameter can immediately introduce a breaking change.

For example, one area where this poses a problem is our caching strategy. To customize the cache key, you currently have to inherit from the cache and override the GenerateCacheKey method. In the next version of Duende.AccessTokenManagement (V4), this inheritance-based approach will be deprecated in favor of injecting a cache key generator.

Looking ahead, our plan is to:

• Minimize the public surface area of the library, exposing only the necessary types for system registration.
• Introduce targeted extensibility points for key scenarios (e.g., cache key generation).

This maximizes the flexibility of the library while minimizing the chances for introducing breaking changes.

To make these improvements as effective as possible, we’d love to hear from you—how are you using and extending Duende.AccessTokenManagement? Your feedback will help shape the future of the library.

[jstclair]

We're extending it today in order to encrypt the token before caching, and then decrypting after reading. For our purposes (and maybe others), it would be useful to expose the value serialization/deserialization as well as choice of key. Maybe someone wants to encrypt the value, or use protobuf rather than json.

[jstclair]

Sure that sounds great. We were already planning to roll out hybrid cache across our APIs so sounds like a plan.

The only thing I would prefer is exposing the HC configuration in a way that doesn't assume it's the default. For instance, the current cache assumes that any IDistributedCache implementation is safe to use for token caching. That makes it hard to plug in if you're already using a IDC for something else. Perhaps the keyed service DI feature could be used when registering the cache?

[Erwinvandervalk]

This is also an improvement we've lined up for V4.

By default, we use the 'default' cache instance, but you can inject a different cache instance using keyed services:

https://github.com/DuendeSoftware/foss/blob/main/access-token-management/src/AccessTokenManagement/HybridClientCredentialsTokenCache.cs#L15

https://github.com/DuendeSoftware/foss/blob/main/access-token-management/src/AccessTokenManagement/DistributedClientCredentialsTokenCache.cs#L16

During registration, we TryAdd a registration that links the keyed implementation to the default one, but you can easily override this.

There is a challenge with the hybrid cache in it's current state. They only allow you to register a single hybrid cache per container. We're looking into adding a sample that demonstrates how you can still register a second hybrid cache. I'm hopeful that Microsoft will lift this limitation though.

[jstclair]

But just to wrap it up, for encryption of the access token cache when using the MS HybridCache, we would no longer need to create our own extension of the TokenCache, but rather just configure the HC's AddSerializer. We would then pass that into the TokenCache using your support for keyed services.

The only drawback I see here is that this works for the HC, but not for the more-general DistributedCacheClientCredentialsTokenCache, which would still require extension. Is that correct?

[Erwinvandervalk]

IDistributedCache doesn't have built-in support for customizing serializtion.

If you wanted to encrypt data going into an IDistributedCache and wish to keep using DistributedCacheClientCredentialsTokenCache, then you'd need to create a decorator for IDistributedCache that handles the encryption / decryption. You can inject this decorator using the keyed service support.

[jstclair]

Sorry to follow up -- are you using decorator in a general or a more-specific sense (something I've missed in the docs)? Getting back to the original issue, the main drawback is having to re-implement the entire DistributedCacheClientCredentialsTokenCache rather than just inheriting and overriding a protected virtual ValueTask<string> SerializeValue(AccessToken token) (and the complement Deserialize method) to go along with the the new key functions. As you point out, that's built into the HybridCache, so wouldn't be needed there. But my 2 norwegian crowns would be to provide complementary functionality in both concrete implementations if possible.

[maartenba]

Related: https://github.com/orgs/DuendeSoftware/discussions/143

[michaeldimoudis]

We are extending Duende.AccessTokenManagement in a few ways. I've just tried to upgrade our implementation to 4.0.0-RC1.5. It was straight forward for client credentials, we add mandatory custom token parameters.

We added an in memory cache option, although with named key support we could remove this.

One thing we are also doing is extending Duende.AccessTokenManagement for token exchange, using TokenExchangeTokenRequest in Duende.IdentityModel. It's not supported at all in Duende.AccessTokenManagement, because you can't update the token request parameter grant type with urn:ietf:params:oauth:grant-type:token-exchange as it already has client_credentials, so we had to go a bit deeper. Although we still just return ClientCredentialsToken model as that's ingrained in Duende.AccessTokenManagement.

We also extend the AccessTokenHandler, and now we can not use the base class in 4.0 anymore. We extend it to automatically detect the current tenant or user in the request scope, or pass values via HttpRequestMessage.Options Property and alter the GetAccessTokenAsync call with either additional tenant or user token request parameters.

So really with this new model we will struggle to extend it and will have to write more of our own code, or forego Duende.AccessTokenManagement entirely unless we get access to some more public classes to extend.

[Erwinvandervalk]

Hi Michael,

Thank you for sharing how you are extending the library. Our aim is to make sure we end up with a model that provides similar (or better) extensibility options than the current model, but not through inheritance but through composition.

By flagging the existing implementation as obsolete, we're trying to announce to the community that there will be changes, but also gather feedback on the new direction. We're looking into adding a preview feature to V4 that will demonstrate a new extensibility model which will hopefully satisfy your requirements without the need for inheritance. Once this is available, I'll reach out. Your feedback on this would be greatly appreciated.

Kind regards,
Erwin

[michaeldimoudis]

Thanks Erwin that sounds good. Looking forward to trying that new feature out. 👍

[Erwinvandervalk]

As a follow up, we just released Duende.AccessTokenManagement 4 preview 2.

https://github.com/orgs/DuendeSoftware/discussions/209

[cmeyertons]

@Erwinvandervalk we have some use cases that work in v3 where we use some ambient context to dynamically determine which credential flow to use (client creds vs user-based flow) - this is helpful when using some libraries that don't natively support multiple named / typed HttpClients and we cannot feasibly define two separate http request pipelines.

This was possible & somewhat clean in v3 where we did not have to copy any business logic (do some derivation of the respective handlers to expose the GetAccessTokenAsync logic)

However, this is not possible in v4 as the handlers' access token logic have now become internalized in the Token Retrievers and we cannot use a wrapping handler to dynamically instantiate and select them, having to result to some naughty logic to instantiate the internal Token Retriever.

Is it possible / advisable to publicize the implemented ITokenRetrievers (OpenIdConnectUserAccessTokenRetriever and ClientCredentialsTokenRetriever etc) - the underlying interface is public so i don't see why the implementations would be both internal and not accessible via DI 😢

[Erwinvandervalk]

Hi @cmeyertons ,

Thanks for bringing your scenario under our attention.

The original v3 design, while very flexible, made it very difficult for us to evolve the library without every change causing a breaking changes. It also made it almost impossible for us to predict how people are extending our library, which causes situations like you're describing now.

The new design relies on composition, not on inheritance, to provide flexibility. You have access to all components that are needed to achieve your scenario: The ITokenRetrievers don't perform the logic to retrieve the access tokens. That 'business' logic is coded solely in the underlying infrastructure. All that they do is perform a form of glue between the configuration and the implementation that retrieves the token.

It looks like you have different 'glue' requirements, so you can easily implement your own ITokenRetriever and link this to your http client:

public class MyTokenRetriever(
    UserTokenRequestParameters parameters,
    IClientCredentialsTokenManager clientCredentialsTokenManager,
    IUserTokenManager userTokenManagement,
    IUserAccessor userAccessor,
    ClientCredentialsClientName clientName) : AccessTokenRequestHandler.ITokenRetriever
{

    public async Task<TokenResult<AccessTokenRequestHandler.IToken>> GetTokenAsync(HttpRequestMessage request, CancellationToken ct)
    {
        // You'll have to make a decision on what token parameters to use
        // The defaults (that are used in our handlers) likely don't work for you
        var param = parameters with
        {
            Scope = Scope.Parse("some scope"),
            ForceTokenRenewal = request.GetForceRenewal() // for retry policies. 
        };

        AccessTokenRequestHandler.IToken token;

        int tokenType = new Random().Next(1, 2); // Or get the type from the context. 

        if (tokenType == 1)
        {
            var getTokenResult = await clientCredentialsTokenManager.GetAccessTokenAsync(clientName, param, ct);
            if (!getTokenResult.Succeeded)
            {
                return getTokenResult.FailedResult;
            }

            token = getTokenResult.Token;
        }
        else
        {
            var user = await userAccessor.GetCurrentUserAsync(ct).ConfigureAwait(false);

            var getTokenResult = await userTokenManagement.GetAccessTokenAsync(user, param, ct).ConfigureAwait(false);
            if (!getTokenResult.Succeeded)
            {
                return getTokenResult.FailedResult;
            }
            token = getTokenResult.Token;
        }

        return TokenResult.Success(token);
    }
}

Hope this helps.

[cmeyertons]

in that case, i'm effectively duplicating the underlying token retriever logic and losing out on any potential updates / enhancements to the underlying code i have now duplicated & increasing my maintenance plane.

vs invoking ActivatorUtilities.CreateInstance<OpenIdConnectUserAccessTokenRetriever>(serviceProvider) (which we are doing now by just grabbing the internal type by name, which is cludg-y and gross (but preferable to losing out on important token retrieval changes on future upgrades of the library that I would have no idea i'm missing)