-
Notifications
You must be signed in to change notification settings - Fork 2
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
Decide what to do with openid-client v6: it's a total rewrite, expensive migration, wait and see? #3260
Comments
I wonder whether it might make sense to move to keycloak-js as we're using keycloak already, might be smoother. @fengelniederhammer what was the thinking behind using openid-client back in the day when you chose that library? |
There was some reason, probably @JonasKellerer knows? |
At that time we also looked into this. When I remender correctly, keycloak-js needed a "window", which we dont have in the astro-backend and we could not to get it working. On the frontend it worked well. Maybe they have changed something since then? |
Thanks that's useful to know! If you happen to find a PR or notes where you tried out keycloak-js that could be usefully linked here. Did you look at other libraries? The current setup seems quite complicated, I think we could hopefully simplify it. Part of the complexity seems to be due to the client and now it looks like it'd get even worse. There must be a better way. |
At the time when we integrated Keycloak, there was no package that could handle OIDC in Astro properly. There are some libraries that handle it on client-side quite well, and there is a package for express that seemed decent (but we didn't try it). That's why we ended up implementing the flow ourselves (which is far from ideal!). I doubt that the situation has changed, but it would be good to get rid of our own implementation. |
I'm curious about this, but I think maybe I don't fully understand yet how it works. Maybe also something to discuss on monday? @chaoran-chen |
I looked briefly into the major bump of openid-client from v5 to v6. v6 is a complete rewrite. Our current auth middleware is tightly coupled to the v5 API. So upgrading is almost the same as moving to another library - unless I'm missing something.
For now it seems v5 will still get limited support, so I'll silence dependabot for now and not worry as long as things are working - dependabot should hopefully still let us know about security fixes. Though it won't let us know about other v5 bumps per dependabot/dependabot-core#9330
https://github.com/panva/openid-client
Dependabot PR: #3102
The text was updated successfully, but these errors were encountered: