Loading JWKS should not use URLConnection cache

[hamlaoui khaled]

Hello,
inside the Get class and within the get method the URLConnection should not use cache.
it could be disabled by setUseCaches to false.

This could be critical if the IDP do key rotation and our URLConnection instance continue to use its cache.

Please let me know if I'm wrong in my analysis.

Regards

[Brian Campbell]

The analysis seems generally correct. With HttpsJwks and HttpsJwksVerificationKeyResolver, the library does do some caching and forcing updates based on perceived key rotation. But that’s wouldn’t work if the underlying HttpsURLConnection is caching locally.

Have you experienced an issue with this? Or is it a speculative concern?

I wasn’t aware of the setUseCaches on Https/URLConnection. Does it actually use a local cache? I’ve never observed it doing so. And I’m not aware of anyone experiencing this issue.

But adding aurlConnection.setUseCaches(false) to Get.get(...) seems reasonable.

[hamlaoui khaled]

Thanks for you quick answer!

No, did not get an issue with your lib, but in another contexte, yes !

For me regarding the existing code, we don’t have the control on the caching over different system layers.
In plus to setUseCaches to false we could also add setRequestProperty("Cache-Control", "no-cache").
If you accepte PR, i could contribute.

[hamlaoui khaled]

Do you have any suggestion please ? i could contribute if you accept PR!

[Brian Campbell]

I would consider a PR.

[hamlaoui khaled]

How i could create a new branch? Could you point me your guide for contribution ?

[Brian Campbell]

This workflow https://www.atlassian.com/git/tutorials/comparing-workflows/forking-workflow is commonly used in public open-source projects.

[Brian Campbell]

Or I could just add the one or two lines…

[hamlaoui khaled]

I forked the project and did the change, however i could not create PR, could you please give me the correct permission ?

[Brian Campbell]

No special permission should be needed. I don’t know why you can’t create a PR. But https://bitbucket.org/khamlaoui/jose4j/commits/4c73787c94e8f77f9ed1dec61eb6df20f7cd29fc has more changes in it than I’d accept, regardless.

You can have HttpsJwks use your own SimpleGet implementation via https://javadoc.io/doc/org.bitbucket.b_c/jose4j/latest/org/jose4j/jwk/HttpsJwks.html#setSimpleHttpGet(org.jose4j.http.SimpleGet) if you really need/want all that.

‌

[hamlaoui khaled]

I could remove the disableServerSideCache option to keep it more simple.
We could keep only the preventHttpCaching method.
You could see my last commit (the http headers are really need to disable the servers/ proxies cache)

[hamlaoui khaled]

PR created @Brian Campbell

[Brian Campbell]

urlConnection.setUseCaches(false) is fine.

I would be okay with setRequestProperty("Cache-Control", "no-cache")

not more

[hamlaoui khaled]

Ok, i updated my PR.
Thanks for your quick response!

[Brian Campbell]

Merged pull request #20 to address this issue #195

[hamlaoui khaled]

Thanks Brian for your support.
Do you have an estimated date of the new version release ?

‌

[Brian Campbell]

I don’t currently have concrete plans around the next release.

[hamlaoui khaled]

I need this new changes for my projets, please let me know when you publish a new version!
Thanks

[hamlaoui khaled]

Hello Brian,
Any idea about the next release date please ? i need the new version with recent changes please.

Thanks

[Brian Campbell]

released with jose4j-0.7.10

[hamlaoui khaled]

Thank you so much Brian!