replace crypto-js

[Badisi]

When using oidc-client-ts in an Angular application, the following warning is raised:

Warning: node_modules/oidc-client-ts/dist/esm/oidc-client-ts.js depends on 'crypto-js/enc-base64.js'. CommonJS or AMD dependencies can cause optimization bailouts.
For more info see: https://angular.io/guide/build#configuring-commonjs-dependencies

There is a solution to explicitely make the warning silent in the application :

"architect": {
    "build": {
        "options": {
            "allowedCommonJsDependencies": [
                "crypto-js"
            ]
        }
    }
}

But I was thinking that, maybe, oidc-client-ts could simply migrates to crypto-es. What do you think ?

Because unfortunately crypto-js doesn't seem to care about moving to es module.. #60 and #236

[pamapa]

When i was introducing crypto-js i was already aware of crypto-es and decided for crypto-js for several reasons:

• https://www.npmtrends.com/crypto-js-vs-crypto-es
• more active maintained
• much much more usage
• even if its not esm, you can add only those parts you need

[kherock]

I don't like to argue popularity as the primary reason for choosing one over another, but in this case I do fear that crypto-es doesn't have enough eyes on it, and ultimately this library won't gain much from switching to it. We already use deep imports which provides most of the advantage that tree-shaking the ES modules port would provide.

Looking forward, we should be able to eventually use the Web Crypto APIs directly. There are projects like webcrypto-liner which might let us move off of crypto-js sooner while we wait for ubiquitous webcrypto support, but for now I think the safest option is to continue using crypto-js.

[pamapa]

Crypto.subtle would be really nice and natively supported, but it only working in a secure context (HTTPS), which makes it useless for development proposes where most people operate with HTTP. This will be a pity in the very long run. I would really prefer native (browser + nodejs) support, which is currently not possible :-(

webcrypto-liner or an other lightweight wrapper library might help here as well.

[drk-mtr]

As an aside, these days I tend to use https for development, because if I don't I end up hitting too many CORS issues - so I find it's easier to "bite the bullet" and just use https throughout.

[CorporateActionMan]

@pamapa this is a really great library in our team and we would like to plus one 👍 this feature request

[43081j]

@pamapa this also effects esm-only environments (e.g. native browser modules).

currently it isn't possible to use oidc-client-ts in a browser natively because crypto-js ships commonjs. the benefits of oidc-client shipping an esm entrypoint go out of the window because of that.

do you have any suggestions for how this could be moved forward?

unless you plan on contributing ESM upstream to crypto-js, it looks like whatever solution you choose will result in dropping that package. if alternatives have downsides, they're probably worth it compared to this package being useless without a bundler.

happy to help figure it out if needed

[pamapa]

@43081j Have you tried https://cdnjs.com/libraries/oidc-client-ts?

[43081j]

to what end?

it isn't really a solution to move off npm to using CDNs for dependencies. and even if we did that somehow, the only CDNs that'd work are themselves running bundlers internally.

ultimately, crypto-js doesn't work natively in browsers (it is a commonjs module). therefore, oidc-client-ts doesn't work natively in browsers.

the only solutions really are to make crypto-js work in browsers (contribute upstream so they publish a browser entrypoint) or move off crypto-js to something which already works in browsers.

of course, bundlers can solve this by transforming commonjs dependencies into es modules. but we shouldn't really put constraints on consumers' workflows to be able to use this package.

its not a straight forward problem to solve though, i understand.

[pamapa]

We need so little crypto. Maybe we simply check if crypto.subtle is defined and if so we use it else (e.g. none HTTPS development session old browser) we use the current solution.

It looks like only sha256 is the hard part. For the other functions including WordArray.random we can use nowaday present native functions.

old	new	fallback needed
sha256	crypto.subtle.digest	yes
WordArray.random	Crypto.getRandomValues	no
Base64	btoa+Array	no

[43081j]

thats possible.

it may be worth looking around for any more modern wrappers around crypto too (i.e. ones which already fall back to native when available). would save us a lot of time if someone already went through the trouble

alternatively we just have two entrypoints (esm/browser and cjs) which each define their own crypto somehow (entrypoints so we don't mess around with conditionally importing stuff etc).

[43081j]

@pamapa just catching up on this again. it looks like browser support for crypto is pretty widespread at this point.

what do you think to simply using the browser's crypto implementation in a breaking version? anyone who runs on very old browsers can use the old major version, since no new features would be added anytime soon.

im happy to put a PR together if we're in agreement

one thing to also keep in mind is that crypto is async in the browser. right now you compute all this stuff in constructors of classes. a fair chunk of refactoring will need doing to move this logic into some async methods instead, which means breaking changes to the public API i imagine too.

[43081j]

a possible temporary solution is to just change the browser entrypoint to be ESM rather than IIFE.

is there really anyone out there using oidc-client-ts via an IIFE script? highly doubtful unless im missing something.

"browser" these days should really mean an ES module. in this case, one which is bundled.

we could pretty much set the browser bundle's format to "esm".

you'd still need to replace crypto eventually just because the parts depending on it will basically import a stub meanwhile.

[RoXuS]

Work as expected with crypto-es -> https://github.com/RoXuS/oidc-client-ts/tree/crypto-es

We have to fork and push this version on our private registry to use it...

[pamapa]

what do you think to simply using the browser's crypto implementation in a breaking version? anyone who runs on very old browsers can use the old major version, since no new features would be added anytime soon.

Thanks for taking efforts here. Would be awesome if you can come up with a proof of concept merge request, such we can see what all needs to change...

[43081j]

have created a draft #909 for anyone curious

don't expect that PR to land, its more to help us get this moving and bounce some ideas around

[pamapa]

I guess its time to make the switch, IE11 support was never supported by this library and all other still relevant browsers support crypto.subtle and crypto.getRandomValues. Lets bump to a new major version, which can also be used to drop some deprecated stuff as well...

[shawnrice]

Crypto-js was recently tagged with a critical CVE (see CVE-2023-46233). Is there anything I can do to help move along a PR to remove it as a dependency?

[pamapa]

@shawnrice I plan to remove it myself next week unless the author for this draft merge request #1196 resolves the conflicts and i can merge it. After this is merge you can help testing. I plan to make a v3 beta release soon...