-
-
Notifications
You must be signed in to change notification settings - Fork 17
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
OpenId Module based on OpenIddict Client #90
Comments
Like the MSFT OIDC handler, the new OpenIddict client supports OIDC, but it also supports OAuth 2.0-only servers, which allows integrating with tons of services that can't be used with Microsoft's OIDC handler. It also supports things like token refreshing ( That said, I'm not so sure developing a new module from scratch would make a ton of sense. If we think it's worth having something based on the OpenIddict client, we should likely just update |
Thanks for your response, updating the OC modules needs to be approved by the team not only me & you :). My aim was either to copy what we did in OC and use the OpenIddict client or create a new module on top of Frankly I didn't work with this module before, so your feedback and guidance is very important here |
Sure, but if we think it's worth investing on that and come with a list of pros and cons that tends to confirm it would be a good idea, I don't see why it couldn't happen 😃
It's certainly doable, but it will basically be a completely separate module that you'll need to maintain. Also, since it won't be part of OC itself, it will have less visibility and will likely receive less contributions (and hem, the "official" OpenID OC module already doesn't receive a lot of love from external contributors 😅) To be clear, I'm not trying to deter you from working on such a module, but it may be a lot of work. Specially if you want to support all the OAuth 2.0/OpenID Connect services the OpenIddict client supports (the complete list is here: https://github.com/openiddict/openiddict-core/blob/dev/src/OpenIddict.Client.WebIntegration/OpenIddictClientWebIntegrationProviders.xml). I'd love to hear what @MichaelPetrinolis (who contributed the OpenID client feature), @Skrypt, @jtkech and @sebastienros think about that 😃 |
If it is a breaking change then it needs to be a new module. Then, we could have both. |
I don't think having both modules on the same repo is a good idea, either we replace the MSFT client or create a community module |
I agree with @kevinchalet, if we decide to implement clients with OpenIdDict, we should replace the Microsoft implementation maintaining the compatibility at OrchardCore module level, in order to maintain one codebase. I took a look at Kevin's announcement about The OpenIdDict client, and he states that the handler is registered as a single scheme. So we could introduce a new module that can handle and configure all the available providers. Then we could replace the implemented features (OpenId, Google and Twitter) to use that module services for their implementation. We should also add special handling in the account Controller for the OpenIdDict scheme (what providers are configured etc.) |
OpenIddict 4.7 will introduce a new built-in authentication scheme forwarding mechanism that should make that unnecessary: openiddict/openiddict-core#1839 I started working on prototype of the OpenID client feature using the OpenIddict client and it's really promising 😃 |
Thanks for your efforts @kevinchalet |
@hishamco you're welcome 😄 I opened OrchardCMS/OrchardCore#14021: feel free to give it a try and torture it 🤣 |
As mentioned here we could create a new OpenId module based on OpenIddict client
@kevinchalet while you're our OpenId guru, is there any advantages to use OpenIddict Client? Any features we could added if it's possible. Please your suggestion coz you might drive this by your thoughts & suggestions
The text was updated successfully, but these errors were encountered: