OpenId Module based on OpenIddict Client
hishamco opened this issue · 9 comments
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
@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
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 (grant_type=refresh_token), has more security features, better interoperability (and can be used not only in ASP.NET Core apps, but also in legacy ASP.NET 4.6.1 and Windows/Linux desktop apps, but it's not something OrchardCore would benefit from directly).
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 OrchardCore.OpenId (and OrchardCore.Google and OrchardCore.Twitter if we're feeling brave) to use it in lieu of the MSFT OIDC handler. This would avoid having to maintain two separate code bases.
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 OC.OpenId
Frankly I didn't work with this module before, so your feedback and guidance is very important here
Thanks for your response, updating the OC modules needs to be approved by the team not only me & you :)
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 😃
After all, whether the OpenID module uses the MSFT client or the OpenIddict client is mostly an implementation detail we're free to change if we all agree it's the right thing to do.
My aim was either to copy what we did in OC and use the OpenIddict client or create a new module on top of
OC.OpenId
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.)
I took a look at Kevin's aspnet-contrib/AspNet.Security.OAuth.Providers#694 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 🤣