Skip to content
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

firebase.JWKS public keys expire and don't get refreshed #59

Open
jleclanche opened this issue Dec 14, 2021 · 2 comments · Fixed by #61
Open

firebase.JWKS public keys expire and don't get refreshed #59

jleclanche opened this issue Dec 14, 2021 · 2 comments · Fixed by #61
Labels
enhancement New feature or request released Changes was released for this

Comments

@jleclanche
Copy link
Contributor

This took me a while to track down. I would sometimes get {"detail":"JWK public Attribute for authorization token not found"} after 7 days of the fastapi instance being up. This hinted towards something expiring.

The firebase.JWKsVerifier class sets self._jwks_to_key = jwks.keys; where jwks is a firebase.JWKS instance. JWKS.firebase is constructed like so:

    @classmethod
    def firebase(cls, url: str) -> "JWKS":
        """
        get and parse json into jwks from endpoint for Firebase,
        """
        certs = requests.get(url).json()
        keys = {
            kid: jwk.construct(publickey, algorithm="RS256")
            for kid, publickey in certs.items()
        }
        return cls(keys=keys)

What this means is the keys are queried with certs = requests.get(url).json() and stored for as long as the instance is up, but they are never refreshed.

@tokusumi I can raise a PR to fix this if you're too busy; but i'd like your take on how to proceed with it. I'm not sure where to even do the detection for expired keys.

@jleclanche
Copy link
Contributor Author

It's worth noting: The URL https://www.googleapis.com/robot/v1/metadata/x509/[email protected] has an expires header which indicates a ~6 hours TTL. This is much lower than the time they actually disappear; probably on purpose to give time to roll over. My take is that we should store the expires header, and simply update the keys if they have expired (probably with some kind of lock to prevent a bunch of attempts re-querying at the same time).

@tokusumi
Copy link
Owner

tokusumi commented Jan 2, 2022

@jleclanche Thank you for your issue and PR!
This problem is fixed in Release 0.4.1

@tokusumi tokusumi added enhancement New feature or request released Changes was released for this labels Jan 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request released Changes was released for this
Projects
None yet
2 participants