-
-
Notifications
You must be signed in to change notification settings - Fork 695
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Caching of key set in PyJWKClient #615
Comments
It looks like this is fixed with #611 but not yet released as of v2.0.1: https://github.com/jpadilla/pyjwt/blob/master/CHANGELOG.rst |
#611 is a good naive implementation. For my uses, I wanted to share a cache across all of my instances and across all of the workers in all of those instances (I run in Gunicorn with from django.core.cache import cache
class CachingJWKClient(PyJWKClient):
def __init__(self, uri: str):
super().__init__(uri)
def fetch_data(self) -> Any:
return cache.get_or_set(settings.JWK_KEYSET_CACHE_KEY, super().fetch_data, timeout=None) # No auto expiration, manual expiration only
def get_jwt_signing_key(token: str) -> PyJWK:
return CachingJWKClient(settings.JWK_URL).get_signing_key_from_jwt(token)
def decode_jwt(token: str, reraise=False) -> Dict[str, str]:
try:
signing_key = get_jwt_signing_key(token)
return jwt.decode(token,
signing_key.key,
audience=settings.AUDIENCE,
issuer=settings.ISSUER,
algorithms=['RS256'])
except (InvalidSignatureError, InvalidAlgorithmError, PyJWKClientError) as e:
# Always invalidate the cache since the signature we are getting back is clearly invalid
cache.delete(settings.JWK_KEYSET_CACHE_KEY)
if reraise:
raise e
# Assume the JWKs changed and these are no longer valid, retry again now that we have deleted the cache to grab
# a new set. If it still fails, re-raise to alert us
return decode_jwt(token, True)
For completeness, on the Django-specific side of things I had this in my JWK_KEYSET_CACHE_KEY = env.str("JWK_KEYSET_CACHE_KEY", "jwk-keyset")
CACHES = {
'default': {
'BACKEND': 'django.core.cache.backends.db.DatabaseCache',
'LOCATION': 'django_cache',
}
} And then a migration to add in the from django.conf import settings
from django.core.management import call_command
from django.db import migrations
def forwards(apps, schema_editor):
call_command('createcachetable')
def reverse(apps, schema_editor):
schema_editor.execute(f"DROP TABLE {settings.CACHES['default']['LOCATION']}")
class Migration(migrations.Migration):
"""
This migration is a little strange because of https://docs.djangoproject.com/en/3.1/topics/cache/#creating-the-cache-table
"""
dependencies = [
('someapp', '...'),
]
operations = [
migrations.RunPython(forwards, reverse_code=reverse)
] |
This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 7 days |
Currently,
PyJWKClient.get_signing_key_from_jwt
will always trigger a new HTTP request (pyjwt/jwt/jwks_client.py
Line 19 in cda3ef1
To improve response times, it would be helpful to save the key set, once retrieved, in the PyJWKClient instance.
If this is considered out of scope of the class, it could be also made responsibility of the library user, by exposing a variant of
get_signing_key_from_jwt
that allows supplying the key set in an argument.The text was updated successfully, but these errors were encountered: