Suggestion / query regarding removal of crypto-js dependency

[drk-mtr]

Thanks for the brilliant work on this library!

I should preface this by saying I'm not experienced with JS/TS or security generally, and I'm a PM not a developer, so apologies if these thoughts are a bit of a confused brain dump.

I have had a go at removing the crypto-js dependency from this library - fork here: https://github.com/drk-mtr/oidc-client-ts.

What I found was that all crypto-js dependencies are called from CryptoUtils.ts. I replaced the existing methods with the code here to use native browser methods. Whether these are specification compliant I don't know - it's unlikely - but I'll cross that bridge later!

The main challenge I found was that the native crypto.subtle.digest() method returns a promise, unlike the crypto-js method sha256() which returns a string. Because my understanding of async is poor, the easiest way I could find to work around this was to add async build() methods to the SigninRequest and SigninState classes. I then need to call these build() methods when constructing these classes elsewhere. This is rather horrible, but it does seem to work. I haven't updated unit tests in the proof of concept updates I've made, because I think my foundation here is poor.

So my question is ultimately this: At some point in future I expect there will be a desire to remove the crypto-js dependency altogether, since native browser methods could feasibly be used, thus reducing bundle size. I appreciate this won't happen soon, because you'll likely want to maintain backward compatibility for a while longer yet. However, in the meantime, would you be willing to consider returning a Promise<string> from the generateCodeChallenge() method instead of a string - because this might make the eventual transition easier when/if that happens at some point in the future?

[pamapa]

Yes we are planning at some point to remove the crypto-js dependency. It definitively makes sense to use native browser functions if available, but we must also ensure SSR still works, see #275.

For the time being i can not say yet how it will impact the internal functions of CryptoUtils.ts, therefore i do not like to change them now and later we have to change them again. Most probably they will gain a Promise, but who knows. This will also depend on what kind of shim library (or not) we are going to use.

BTW: Looks like you have already profited from the fact, that we concentrated all crypto functions into CryptoUtils.ts :-)

[drk-mtr]

That's great news - thank you.

BTW: Looks like you have already profited from the fact, that we concentrated all crypto functions into CryptoUtils.ts :-)

Hehe yes indeed!

I'll close this issue now, since there's already an existing issue around crypto-js more generally and it makes a lot of sense that you won't want to make any change to a promise until you actually remove the dependency. :)

Thanks again for sharing this library, it's great to have a typescript option.