uruk-project / Jwt

JSON Web Token implementation for .Net & .Net Core
MIT License
80 stars 13 forks source link

Manual refresh for JwksHttpKeyProvider #553

Closed watfordgnf closed 3 years ago

watfordgnf commented 3 years ago

We're currently wrapping JwksHttpKeyProvider in a custom key provider factory in order to support the following requirement (supports automatic key rotation for our services):

When a key is missing from the JWKS, the validation service shall attempt to reload the JWKS from the /.well-known/jwks.json endpoint. This shall be attempted at a rate of no more than once per minute.

I propose adding the following to JwksHttpKeyProvider.cs:

namespace JsonWebToken
{
    /// <summary>Represents a <see cref="IKeyProvider"/> that retrieve the key set from an HTTP resource as JWKS.</summary>
    public sealed class JwksHttpKeyProvider : IKeyProvider, IDisposable
    {
...

        // Possible implementation
        public void Refresh()
        {
            _refreshLock.Wait();
            try
            {
                _syncAfter = EpochTime.UtcNow + RefreshInterval;
            }
            finally
            {
                _refreshLock.Release();
            }
        }
    }
}

This would allow us to setup refreshing the JWKS if a key is missing.

Since this is a 2.0 release, perhaps this could be rolled up to IKeyProvider?

ycrumeyrolle commented 3 years ago

~~Can you expose a bit more your requirement ? Currently the provider is refreshing automatically at a defined interval.~~ Indeed if you detect that a key is not found, the cache will still be valid. The proposed method Refresh() is a way to force the eviction of the cache.

Another way would be to refresh the keys if the key lookup fail: https://github.com/uruk-project/Jwt/blob/55fee40ed0e35411e5fa99010dcb31f2e42bd9da/src/JsonWebToken/Reader/JwksHttpKeyProvider.cs#L160-L166 Replaced by:

 bool mustRetry = false;
 if (_currentJwks != null && _syncAfter > now) 
 { 
     var keys = _currentJwks.GetKeys(kid); 
     if (keys.Count != 0)
     {
        return keys;
     }

     mustRetry = true;
 }

 if (mustRetry  || _syncAfter <= now)
 {

By doing so, the JwksHttpKeyProvider is automaticaly refreshing if it detects the key is not found. This also avoid the be vulnerable to an bad kid attack: Sending many invalid tokens with invalid kid, causing an HTTP request for each validation attempt.

watfordgnf commented 3 years ago

That approach would work for us as well, and perhaps even nicer in that we don't have to manage this separately.

Whichever the mechanism, I concur that you would want to rate limit the lookups for a given key.