split off internal/oidc and provide separate packages for each "router package"
idc77 opened this issue · 3 comments
Motivation:
This package is rather unflexible if you'd like to write your own middleware. Writing your own middleware is not possible without yanking out internal/oidc and putting it into its own, separate package.
Different responses if an error occurs are not possible to do either. You have to "eat" the stock "400 bad request" response in gin's case without any abililty, except forking, to change that.
Also, if you only need e.g. the gin handler, you have to pull in all the other routers, like fiber etc, resulting in a larger memory footprint and larger binary.
Why not just a fork? Because changes in this package would then again need to be done manually in the forked package, because it's internal.
Please correct me if I'm wrong.
I have forked the internal/oidc and options and put it at git.icod.de/dalu/oidc
What I'll do next is take and edit just the gin handler and put it into a separate package as well then edit it to be optionally permissive and return JSON on error.
Thanks to your MIT license I can do that, so thank you for that.
But you're the author and it should be hosted and updated by you, therefore this proposal. And you seem to have some automation set up to upgrade dependencies.
Hi again @idc77!
Thank you for the feedback. 🥇
This package is rather unflexible if you'd like to write your own middleware. Writing your own middleware is not possible without yanking out internal/oidc and putting it into its own, separate package.
The main idea behind this package was to provide something that just worked for most use cases and make it easy to extend. I intentionally put the main logic in internal because I don't know what the stable API will look like and I feel the same today as when I created it.
Different responses if an error occurs are not possible to do either. You have to "eat" the stock "400 bad request" response in gin's case without any abililty, except forking, to change that.
As discussed earlier (#83) I actually don't want to support all possible ways of using the middleware - I want to keep it as simple as possible. Adding even more logic to a quite (in my opinion) large API surface will just make the maintenance burden even larger and harder to make sure it works as expected in all the different use cases.
Also, if you only need e.g. the gin handler, you have to pull in all the other routers, like fiber etc, resulting in a larger memory footprint and larger binary.
This is a great point and something I regret not fixing right away. I'm not able to do it right now, but is something I will track in the following issue: #98
Why not just a fork? Because changes in this package would then again need to be done manually in the forked package, because it's internal.
Please correct me if I'm wrong.
You are correct! And that is the whole point of using the internal package. If you fork it, you will be able to do whatever you want with it! But I won't have to "support" use cases I haven't tought up myself or accepted.
I have forked the internal/oidc and options and put it at git.icod.de/dalu/oidc
What I'll do next is take and edit just the gin handler and put it into a separate package as well then edit it to be optionally permissive and return JSON on error.
Thanks to your MIT license I can do that, so thank you for that.
But you're the author and it should be hosted and updated by you, therefore this proposal. And you seem to have some automation set up to upgrade dependencies.
Good job! This is why open source is great! 👍 I have created the following issue as well to think about if and how I could expose the functions that are needed to build your own middleware: #99
Hi again @idc77 !
The two issues (#98 & #99) have now been resolved. I haven't worked with a multi-module repo before, but I hope I got it working as it should. If you see any problems or improvements, please create another issue.
You can build your own middleware using this package now: https://github.com/XenitAB/go-oidc-middleware/tree/main/oidctoken
See the readme for additional info: https://github.com/XenitAB/go-oidc-middleware/blob/main/README.md#build-your-own-middleware