panva / openid-client

OAuth 2 / OpenID Connect Client API for JavaScript Runtimes
MIT License
1.83k stars 392 forks source link

What exactly requires node 12 in the implementation of v3? #179

Closed theogravity closed 5 years ago

theogravity commented 5 years ago

We're considering using this over passport, and noticed that you have to use node 12 for v3. Our services probably won't migrate to 12 for a while.

I understand you would use the v2 client, but there seems to be significant interface differences between v2 and v3, that I'd rather use the latest if possible.

What is it about the v3 client that requires node 12? Is there a way to make it compatible with node 10?

panva commented 5 years ago

The underlying JOSE implementation got replaced with a pure native node crypto one with almost no dependencies.

See @panva/jose which replaced node-jose.

node-jose is built to work in any environment and therefore packs 3 implementations for everything - web, node, fallback in pure js, we don’t need that for a node client ;)

And it is only in node 12 that KeyObject crypto class is available making crypto more efficient, e.g. not having to parse keys on every operation but instead having OpenSSL native key handle, in some cases having 2x improved throughput on verification ops over passing a PEM instead.

Also only node 12 supports ed25519 and ed448 keys so that openid-client can use OKP JWKs (EdDSA alg).

What exactly requires node 12 in the implementation of v3?

It comes down to the crypto standard lib which only really matured in node 12

theogravity commented 5 years ago

It could be nice if it was architected in a way that a user could pick using one or the other depending on their node version.

Would you be open to a PR to remove jose as a dependency and an abstraction layer that would allow the user to feed in one or the other?

panva commented 5 years ago

Would you be open to a PR to remove jose as a dependency and an abstraction layer that would allow the user to feed in one or the other?

@theogravity I would not, JWT is a common attack vector and I don't want to open this up. in ~2 months Node 12 enters its LTS stage and until then there's v2.x

but there seems to be significant interface differences between v2 and v3, that I'd rather use the latest if possible.

Out of curiosity, what's the difference you observe?

theogravity commented 5 years ago

Even if node 12 enters LTS, it will take time for package maintainers to update compatibility for it, and as a result, not everyone will migrate to 12 right away; not everyone is even able to migrate to 12 at all (maybe they're on node 6 or 8). Also it being a native implementation doesn't mean it is more or less secure than its non-native counterpart.

I understand this is a non-negotiable item, so I'll leave it at that. I still want to thank you / the team for building this amazing OSS project regardless of my thoughts.

Here's some that I noticed, but I'll admit that I was wrong in saying there are significant differences. The biggest one that bit me was I forgot I was reading v3 documentation, instead of v2, with the behavior in redirect_uri (I lost a bit of time troubleshooting that):

In v3, for authorizationUrl(), the redirect_uri param will use client.redirect_uris by default if there's a lone entry. response_type is a parameter.

In v2, it must be explicitly stated. The documentation does not have response_type as a parameter.

v2: authorizationCallback() -> v3 callback(), v3 has extras

panva commented 5 years ago

Well, I know what the differences are and was just curious to see what you find significant, seeing as there's really nothing like that I think you should be fine continuing with v2.x

not everyone is even able to migrate to 12 at all (maybe they're on node 6 or 8)

That's not a reason to restrict one from using new language and standard lib features. v2.x still works with lts/boron!, v3.x is just a much needed evolution for the ones sticking to latest LTS node, it's not a revolution everyone must adopt.

theogravity commented 5 years ago

By that logic, 3.x should be supporting node 10, since that is the current LTS version. I would have waited to release 3.x as the latest until node 12 is LTS (in Oct).

The place I work at will not allow node updates that are not LTS. Even if they do, it's a waiting game for us for some projects as downstream dependencies have to update to support 12.

You're right - the interfaces are close enough that using 2.x should be ok for older node versions.

panva commented 5 years ago

I would have waited to release 3.x as the latest until node 12 is LTS (in Oct).

The point of waiting being? Node 12 is the future LTS release, gets the same cadence of security fixes as other LTS/maintenance branches and is a stable release. You’d rather have the project stale for the “gap” timeframe? I’d rather continue developing.

Whats a couple months I say.

panva commented 5 years ago

@theogravity I've made @panva/jose compatible with lts/dubnium and released openid-client v3.5.0 that's being tested with Node.js ^10.13.0 || >=12.0.0