Add Async pattern for TokenValidation

[MicahZoltu]

There are a number of places where JWTSecurityTokenHandler calls into user provided methods that may be doing some IO (e.g., database or remote calls) but they do so in a synchronous way.

One example of such a place is with JWT signature validation. The user can provide their own SignatureValidator or IssuerSigningKeyResolver. The act of validating a signature or resolving signing keys may require an interaction with an external server.

For a real-world example, Google rotates its signing keys regularly (rumor is daily) and you can get the latest public key in JWK format here. Unfortunately, it is not possible to know when they are going to roll their keys so an application that is attempting to validate Google issued JWTs will need to hit that endpoint to retrieve the latest signing key. The application could be optimized to cache the signing keys but that is still an async operation and it is entirely up to Google as to how often they rotate their keys (they could start rotating them for every request if they wanted).

Another real-world example once again with Google's authentication, Google supports sending the token to here for validation rather rather than doing the validation locally. If one wants to use this mechanism then every token validation would be a remote call.

Unfortunately, I recognize that the current API is very async un-friendly and the interfaces it implements are also not async-friendly but I wanted to get this filed anyway in hopes that at some point things can be improved to support modern asynchronous use cases.

[brentschmaltz]

@Zoltu I agree with you that off-box validation is a real scenario. We should have an async pattern up and down the call graph to realize this pattern.
We stopped at metadata retrieval.

[brentschmaltz]

We will need to add an new return type: ValidationResult so the 'out' param is not needed.

[brentschmaltz]

Include this issue with this fix.
#566

[brentschmaltz]

Modified title to reflect that this is general in nature across all token types. Perhaps in SecurityTokenHandler

[shagenhokie]

Do we have an ETA on this?

[brentschmaltz]

@shagenhokie not yet

[bugproof]

@brentschmaltz the method that handles jwt authentication is async so I think it shouldn't be that difficult to add? https://github.com/aspnet/Security/blob/beaa2b443d46ef8adaf5c2a89eb475e1893037c2/src/Microsoft.AspNetCore.Authentication.JwtBearer/JwtBearerHandler.cs#L107-L128

It's a limitation if you want to implement token invalidation and call redis or any other db asynchronously inside the handler:
https://stackoverflow.com/questions/52476325/how-to-invalidate-tokens-after-password-change/52476727#52476727

Because ValidateToken token uses out parameter and async methods can't be used with out parameter that would be a breaking change, ISecurityTokenValidator would need a method which returns Task anyway. The best workaround to avoid breaking change I can think of is:

public async Task<(ClaimsPrincipal, SecurityToken)> ValidateTokenAsync(string token,
    TokenValidationParameters validationParameters)
{
    var claimsPrincipal = base.ValidateToken(token, validationParameters, out var validatedToken); 
    return (claimsPrincipal, validatedToken);
}

public override ClaimsPrincipal ValidateToken(string token, TokenValidationParameters validationParameters,
    out SecurityToken validatedToken)
{
    var tuple = ValidateTokenAsync(token, validationParameters).Result;
    validatedToken = tuple.Item2;
    return tuple.Item1;
}

[brentschmaltz]

@fragilethoughts I agree we need async, particularly with delegation to validation services, such as KeyVault and Google. Not only do we need ValidateTokenAsync, we need async on our Crypto such as SignatureProviders and KeyWrapProviders.

This is a feature we are thinking about for 5.3.1, but realistically it may be later. The other issue is we want to remove references to any libraries other than those shipped by Microsoft, such as JSON.net. With many JwtSecurityToken and JwtSecurityTokenHandler API's using JSON.net, we created a new assembly M.IM.JsonWebTokens (free from JSON.net) where ValidateToken returns a structure TokenValidationResult and can have easily have an async version. We are free to invent here.

[bugproof]

Sounds good

[dhjf]

Is the plan to create an async version of JwtSecurityTokenHandler, so it would be possible to call WriteToken asynchronously? It would help a lot, being able to go all async using IdentityServer4 for signing access tokens.

[mcthuesen]

Any status on this?

We have this issue as well where we need to do an http request to an HSM to sign access tokens. Our solution right now is to use WebClient.cs instead of the recommended way of doing http requests using HttpClient.cs. HttpClient does not support synchronous requests.

[brentschmaltz]

@mcthuesen We haven't moved forward much on this, but it is important for a number of reasons. We are not sure if this fits into our 5.x release or will need to wait for 6.0.
Our 6.0 release fits with CORE 3.0.

[mfeingol]

Speaking on behalf of Azure DevOps, we'd love to have an async variant of the AudienceValidator delegate.

[bugproof]

Pro tip: please don't close issues without linking to the commit/demo/documentation

This is still an issue without a proper message of what is done

[nathfy]

I think this is missing info: https://github.com/AzureAD/azure-activedirectory-identitymodel-extensions-for-dotnet/wiki/IdentityModel-7x#sync-over-async

"ValidateToken was a sync method calling an async method, which can lead to threadpool starvation when on the hot path, this method has been deprecated, please use ValidateTokenAsync instead. See issue #2253 for details."

Also note this issue when upgrading the nugets: https://stackoverflow.com/a/77121911

[ownerer]

I'm sorry but I don't see how this issue is actually solved?
Looking at the source code for JwtSecurityTokenHandler all this new ValidateTokenAsync method does is wrap the old synchronous method.
Everything else is still synchronous including delegates in the TokenValidationParameters class, such as IssuerSigningKeyResolver.
I'd expect to be able to pass an asynchronous delegate there; that is still impossible.
So getting the signing key from some external resource still has to happen sync-over-async...
Eg:

IssuerSigningKeyResolver = (token, securityToken, kid, parameters) =>
{
    var result = new List<SecurityKey>();
    //TODO: make this async, cfr https://github.com/AzureAD/azure-activedirectory-identitymodel-extensions-for-dotnet/issues/468
    var secret = _tokenManager.GetSecret(kid, cancellationToken).Result; // <<<---- BAD!!
    if (secret != null)
    {
        result.Add(new SymmetricSecurityKey(Encoding.ASCII.GetBytes(secret.Value)));
    }

    return result;
}

[kroymann]

@ownerer All of the async improvements appear to have gone into JsonWebTokenHandler, which is apparently the new and improved successor to JwtSecurityTokenHandler (see #945 (comment) - I sure wish this was better documented as I had no idea JwtSecurityTokenHandler was old until just now). It looks they did the minimal update necessary to have a technically functional implementation of the async methods for JwtSecurityTokenHandler (https://github.com/AzureAD/azure-activedirectory-identitymodel-extensions-for-dotnet/pull/1810/files#diff-0b1054c790c38d665f14eee85e63889a6c64f63e87ab7ded11b86708f1ed1475), but if you want a proper async-all-the-way-down option, you need to switch to JsonWebTokenHandler.

[ownerer]

Thanks for that info @kroymann , but the ValidateTokenAsync method of JsonWebTokenHandler still takes an instance of that same TokenValidationParameters type, meaning its delegates are still synchronous, so the problem I described remains. You can see how it gets called here.
This issue is not resolved.

[brentschmaltz]

@ownerer we are aware of the issue with the sync delegates.
We tried out one async,

azure-activedirectory-identitymodel-extensions-for-dotnet/src/Microsoft.IdentityModel.Tokens/TokenValidationParameters.cs

Line 115 in 3cae455

internal delegate ValueTask<string> IssuerValidatorAsync(string issuer, SecurityToken securityToken, TokenValidationParameters validationParameters);

to get a feel for the design.

[ownerer]

@brentschmaltz , aha ok.
Any idea when the others will be converted/added?
Is that work tracked somewhere, seeing as this issue is closed...

[brentschmaltz]

@ownerer we are fully booked until the new year, but new async delegates that return a typeof ValidationResult to avoid logging or throwing when multiple identity providers are in play and the second one succeeds, is important work that we are planning early next year.

[Pikhulya]

>... This is done as part of IdentityModel 7x, please try with latest version and reopen if necessary. ...

@jennyf19 , @brentschmaltz , please re-open the issue given, as outlined in the few above comments, it is not really fixed. Alternatively, if it is tracked by another tracking work item on GitHub - please share the link.

[brentschmaltz]

@Pikhulya reopened

[HarelM]

Are there any examples I can use in order to change my code to use JsonWebTokenHandler? I can't seem to find any information and I'm not able to wire it up to my app so that it does the validation...

[brentschmaltz]

@HarelM marked this a .net9 as we are going to be modifying our Delegates to make them async and return a ValidationResult rather than throwing.

[HarelM]

Yeah, I'm not going to wait for .net 9 just to find out the documentation is poor again and that I can't figure out how to properly migrate my current code.
While in most cases I disagree with the approach of "if it ain't broken don't fix it", in this case I gave up.
Here's my commit BTW that only fixes what I needed in terms of validating the token only for authorized routes
IsraelHikingMap/Site@bae2247

[jennyf19]

@HarelM we will aim to get this into our current workstream. by referring to .NET 9, we do not mean Nov 2024. Please follow this issue for updates. Appreciate the feedback and we will do better with documentation moving forward.

[jennyf19]

related issue: #2711

[bugproof]

So are we going to get this until 2077? 😂 😂 😂 😂 😂 😂 😂

Maybe in .NET 20. Sorry won't make it for .NET 10