joken-elixir / joken_jwks

A Joken 2 hook for fetching the signer from a public JWKS url
Apache License 2.0
29 stars 24 forks source link

Allow dynamically starting strategies + expand GenServer-ness of the DefaultStrategyTemplate #39

Closed lovebes closed 3 months ago

lovebes commented 1 year ago
victorolinasc commented 1 year ago

I am reviewing this at the velocity of a turtle... but I am looking into it. Sorry for the long delays here!

victorolinasc commented 1 year ago

@lovebes I was reviewing and testing this but I got the conclusion we need some bigger changes here...

For instance, issue #37 . We need to be more compliant with the specification about possible kidless and duplicated kids in a set.

So, I am gathering feedback to a breaking change which would allow us to change the strategies here. Maybe we should rename the current strategy to something like SingleSourceStrategy and then have a MultipleSourcesStrategy that would chain many single sources and then a new DynamicSourcesStrategy. I see the use case for all of them in the wild and would allow us to assume intent behind choosing each of them.

Other than that, an issue with synchronous initialization was caught that simply showed that we are not properly initializing the ETS here. This is some ancient code that I want to refactor properly too and not maintain it like it is now forever.

So, I've pushed a change with the CI exactly like it is currently in Joken and some logging improvements. This broke your PR here but I wanted to tell you that your help will be much appreciated when we start tackling 2.0. Your code here is an eye opener for these different strategies. So, first of all, thanks a lot :) Secondly, let's discuss better what other issues and uses we might want with a breaking change.

Wdyt?

lovebes commented 1 year ago

@victorolinasc I like all of your decisions, and yeah having read #37 I see what you mean. I am all for a 2.0 for this very important library. I don't think there's anything like it in the Elixir ecosphere that does Joken and JWKS simply and would like for it to have more support. I like how you want to keep the current strategy as-is, as that was my original intention but wanted to keep the footprint small and not introduce more files ;)

So yeah please ping me when you see I might be of use for 2.0, etc - thank you for this library! Feel free to close this PR, leave it open, whichever you prefer!