Better documentation on using JWK needed

[butla]

I was wondering whether this library (which I've used for JWTs for some time) supports JWKs. When I found issue #144 to be open, I thought I was out of luck. It seemed that another library, python-jose, had that already, at least that's what it said in the first few lines of the README. The problem with that library was, it used PyCrypto (which didn't have a release in a long time) instead of cryptography. I thought I was in a bind.

A bit desparate, I went back to #144 to check if there was any progress, or ways I could help. It turned out, that there's a working implementation, which I could use (with some added effort) for reading the Google OAuth public keys!

I think that this short story shows the importance of docs and the README. If I was less persistant, I wouldn't find this functionality. Do you agree that JWK should be advertised in the README and real usage example documented in the docs? I could probably help with that at some point.

[fgblomqvist]

Wanna share how one decodes/verifies a token using a JWK? Can't figure it out myself.

[butla]

@fgblomqvist Below is my (almost) full flow using aiohttp. I'll redact some application-specific things.
You can think of decode_google_id_token as the entry point - we have and ID token that came from Google and we want to verify it using Google's JWK Set:

import aiocache
import aiohttp
import cryptography.hazmat.backends.openssl.rsa
import jwt


async def decode_google_id_token(id_token: str, google_oauth_client_id: str):
    token_signing_key_id = _get_token_signing_key_id(id_token)
    google_keys = await _get_google_public_keys(app_config)
    # TODO if the key ID is not in the set we should return 401?
    token_key = google_keys[token_signing_key_id]

    # verifying the ID token is an important part of securing the OAuth flow with Google
    # https://developers.google.com/identity/sign-in/web/backend-auth
    return jwt.decode(
        id_token,
        key=token_key,
        audience=google_oauth_client_id)


def _get_token_signing_key_id(id_token: str) -> str:
    # This isn't ideal, but I can either use that or reimplement and test it, which doesn't make
    # sense either. It'd be good to submit a pull request upstream that would expose that as
    # a public function.
    # https://github.com/jpadilla/pyjwt/issues/292
    parse_token_output = jwt.PyJWS()._load(id_token)  # pylint: disable=protected-access
    # the outputs are payload, signing_input, header, signature
    token_header = parse_token_output[2]
    return token_header['kid']


RSAPublicKey = cryptography.hazmat.backends.openssl.rsa._RSAPublicKey # pylint: disable=protected-access

@aiocache.cached(ttl=60*60)
async def _get_google_public_keys() -> Dict[str, RSAPublicKey]:
    """Gets the set of JWKs (https://tools.ietf.org/html/rfc7517#section-5) provided by Google.

    These keys will change, but they live for longer than 24 hours, so one hour of caching (which
    this function does), should be OK.

    Returns:
        Mapping of key ID's ("kid" fields) to RSA public keys usable by PyJWT.
    """
    google_public_keys_url = 'https://www.googleapis.com/oauth2/v3/certs'
    async with aiohttp.ClientSession() as session:
        with async_timeout.timeout(5):
            async with session.get(google_public_keys_url) as response:
                jwk_set = await response.json()
    public_keys = {}
    for key_dict in jwk_set['keys']:
        public_key = jwt.algorithms.RSAAlgorithm.from_jwk(json.dumps(key_dict))
        public_keys[key_dict['kid']] = public_key
    return public_keys

[fgblomqvist]

I see, so the key (no pun intended) is to convert the key first:
jwt.algorithms.RSAAlgorithm.from_jwk(json.dumps(key_dict))
I'm getting an error that algorithms does not have the property RSAAlgorithm. But nontheless, I think I'll be sticking with python-jose for now since their implementation is cleaner and actually documented.

Just some advice regarding the caching: afaik, the way it should work is that if you get a jwt with a kid that is not cached, you should pull the keys from Google, add all new ones to the cache, and so forth. Aka. the kid is unique for each key. Caching them for an hour might result in you not being able to verify a token for a whole hour if they just released new keys after you cached them. I'm using a simple dictionary for the cache, which is fine just because the kid is unique (e.g. it does not matter if 2 threads set the same kid, it will contain the same key).

Also, your _get_token_signing_key_id can be replaced by jwt.get_unverified_header(id_token)['kid']. Just to simplify things :)

But thanks for sharing!

[butla]

Do you have cryptography installed? Does the key type (it's in the JWK structure) starts with RS? If not then it isn't an RSA key.

As for caching - it hasn't bit me yet. From what I've observed, each keys lives for a bit more than 24 hours. Also, they probably aren't used for signing as soon as they enter the set. But yeah, that is a potential issue to think about.

And thanks with get_unverified_header - it'll come in handy :)

And as for python-jose, they might have a better interface and all, but they're based on pycrypto, which didn't have a release for more than 3 years. I don't know, maybe they've handled everything perfectly, but an unmaintained crypto project is a red flag for me.

[fgblomqvist]

Yeah I have cryptography installed and the key is RS. And yeah, the reason why I would rather use a different library than python-jose is specifically that one, but oh well, what I am developing will not be used in production anytime soon so there will be time to change things up once either this project gets an update or a different library does.

[jnyborg]

I also had no idea this library supported JWKs! It would definitely be useful to mention its usage somewhere.

[butla]

@fgblomqvist Just a word of caution - assuming there will be more time for security (or more time for fixing bugs, writing tests, or any quality-providing activity) later on in project isn't right in many cases... But I'm just a random person from the Internet :) Hope you won't lead to any major data-breaches in three years, or something :)

[fgblomqvist]

You're a 100% right. This is just a school project but if it wasn't, then I would for sure care more ;)

[rayluo]

@fgblomqvist @butla Thanks for a very informational conversation! It seems the python-jose switched its crypto backend since its 3.0.0 version.

[butla]

Alright, that's good news. I'm gonna close this then. People from the future that come across this - use python-jose for JWKs :)

[jpadilla]

👋 I'm currently working on improving the use of JWKs in this library.

[jpadilla]

Hey all, sorry for the long delay on improving support for JWKs. I've just started putting together a few things for v2. You can follow along on #470. Any feedback would be appreciated.