search

aspnet-contrib / AspNet.Security.OAuth.Providers

OAuth 2.0 social authentication providers for ASP.NET Core
Apache License 2.0
2.38k stars 538 forks source link

Added JumpCloud support #797

Closed AaronSadlerUK closed 1 year ago

AaronSadlerUK commented 1 year ago

Added support for the JumpCloud OIDC application.

https://jumpcloud.com/support/sso-with-oidc

AaronSadlerUK commented 1 year ago

I'm probably missing something simple but I'm getting the following error when I try to build:

NU1101 Unable to find package AspNet.Security.OAuth.JumpCloud. No packages exist with this id in source(s): dotnet-eng, dotnet-tools, nuget AspNet.Security.OAuth.JumpCloud

Of course I know it's not found as the package hasn't been published yet, how do I get past this?

AaronSadlerUK commented 1 year ago

I have removed the custom authentication as looking through the documentation with JumpCloud there is no mention of it.

I have added unit tests, also it looks like the domain is fixed and cannot be changed.

martincostello commented 1 year ago

Before I merge this and prepare a release, can you confirm that you've tested this as working with the real JumpCloud service?

AaronSadlerUK commented 1 year ago

Yes I have this working, happy to provide a URL privately

kevinchalet commented 1 year ago

@martincostello I guess I'm late to the party, but that would be nice to address my two remarks before shipping a public version of this provider.

kevinchalet commented 1 year ago

Well... scratch that I guess 😅

image

martincostello commented 1 year ago

Sorry, I was running the commands whilst also in a meeting so didn't see the notifications until just now.

martincostello commented 1 year ago

We could always unlist 7.0.4 and then do a 7.0.5 that has the changes you want made?

kevinchalet commented 1 year ago

Sorry, I was running the commands whilst also in a meeting so didn't see the notifications until just now.

Is that the secret sauce of being a x10 developer? Doing things in parallel? 🤣

We could always unlist 7.0.4 and then do a 7.0.5 that has the changes you want made?

Doesn't seem necessary to me: depending on @AaronSadlerUK's feedback, we'll still be able to give Domain a default value in a future minor version (or deprecate it if the domain is not configurable in JumpCloud). For the scopes, well, it would be a breaking change, so we'll have to live with it 😄

martincostello commented 1 year ago

It would technically be a breaking change, but it's only been published for ~5 minutes and I think some one would need to be a 100x developer to have already integrated it and be broken by it.

AaronSadlerUK commented 1 year ago

I can't see anywhere that a custom domain is documented... However, that's not to say the enterprise subscribers can't do it

kevinchalet commented 1 year ago

I can't see anywhere that a custom domain is documented... However, that's not to say the enterprise subscribers can't do it

I guess it would be more clearly documented if it was possible (and AFAIK, JumpCloud is cloud-only and not offered as an on-premises product, so you can't even self-host it).

@martincostello I'm not sure it's worth pushing a new release now. We can integrate these changes in a future version and document them, after all. As you said, folks are unlikely to massively start using this provider anyway 😄

kevinchalet commented 1 year ago

Note: JumpCloud fully supports PKCE (for both public and confidential apps) so we'll also want to set UsePkce = true for increased security.

martincostello commented 1 year ago

I'll make these changes now as I've just rebased the new provider into the v8 branch anyway.

AaronSadlerUK commented 1 year ago

@martincostello would it be possible to PR into the v6 version?

The site I plan on using this with uses .NET 6

martincostello commented 1 year ago

We don't typically backport new providers to older versions because it's additional maintenance for us.

AaronSadlerUK commented 1 year ago

Ah ok, this site runs on Umbraco 10 (LTS) which uses .NET 6 (LTS)