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

JWT Keys Request is made every time a key is decoded #160

Closed
nikz opened this issue May 3, 2021 · 3 comments · Fixed by #161
Closed

JWT Keys Request is made every time a key is decoded #160

nikz opened this issue May 3, 2021 · 3 comments · Fixed by #161
Assignees

Comments

@nikz
Copy link

nikz commented May 3, 2021

Every time you request a decoded ID token/Access Token from the client or run through the connection process, a request is made to: https://identity.xero.com/.well-known/openid-configuration/jwks

This is the line that makes the request as part of the decode_jwt method.

This endpoint appears to have a different rate limit to the other API endpoints, and if you hit that you are given the following response:

IMG_7213

Does Xero have a TTL/expiration policy for these keys? Is there an amount of time that it might be safe to cache the value of this response for? Is the rate limit published anywhere or is there a different way to avoid being rate limited?

Thanks!

@SerKnight
Copy link
Contributor

Hey niks, thanks for reporting this issue. A good idea on the TTL I’ll get that sorted this week witch a cache or something.

Out of curiosity do you know roughly how many tokens you were deciding when this happened?

Also curious on the use case. We’re you deciding the access or if tokens when this happened?

@SerKnight
Copy link
Contributor

SerKnight commented May 3, 2021

So I changed this to only invoke the actual JWT validation on the callback from when tokens are received. You can also use the verify_jwt(validate=false||false) changed method to validate whenever you see fit.

I changed the default of the decoded id access token methods to not make those API calls so don't have to worry about caching or TTL and I think this will fix for 99.9% of use cases and still persist a good security measure.

@nikz
Copy link
Author

nikz commented May 3, 2021

@SerKnight hitting it due to fanning out a sync job per month - so each job was rematerialising a stored token as part of execution (to be fair it could cache the data pulled out, but I expected decoding a JWT to be a relatively light operation)

I would say maybe 6 tokens? So I don't think I was hitting it particularly hard... it's also only my development machine.

Change looks good, thanks 👍 I would say that's the "principle of least surprise" way to do it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants