LinusU / secure-remote-password

A modern SRP implementation for Node.js and Web Browsers
101 stars 22 forks source link

Example does not run in browser due to lack of crypto module #3

Closed alexgoldstone closed 6 years ago

alexgoldstone commented 6 years ago

My test client is a web app running in the browser.

I have added the Signing up example code from the README but am getting an error (please see screenshot attached) due to the lack of crypto module in the browser.

screen shot 2017-10-29 at 01 47 03

I am testing in Chrome Version 62.0.3202.75 for OS X.

I assume the Web Crypto API should be used for this: https://developer.mozilla.org/en-US/docs/Web/API/SubtleCrypto

LinusU commented 6 years ago

🤔 hmm, how are you bundeling/packaging the code to run in the browser?

I've been tested this with both browserify (I think 😄) and WebPack, and I think that in both cases the crypto module gets replaced with something like this https://github.com/crypto-browserify/crypto-browserify

I'd of corse be open to adding support in other ways if it would help you 👍

alexgoldstone commented 6 years ago

Thank you.

Your answer is helpful as I am using the standard Angular 4 CLI build tool which is based on Webpack but now I know what to look for I see that Angular CLI does include an equivalent to the Node crypto module.

Looks like you only reference crypto in two places so should be easy to resolve. I think the fix would be to use the Browser's crypto functions when available. I will try to determine the smallest possible change and then report back or raise a merge request if generic.

alexgoldstone commented 6 years ago

Based on your earlier reply, I have made the two Browserify functions available as dependencies rather than relying on the build tools to do it.

Pull request here: https://github.com/LinusU/secure-remote-password/pull/4

I have confirmed that this enables the library to run in my Angular 4 application and I can see the generated Salt and Verifier based on the first stage of the example in the README. I haven't yet proceeded with the rest of your example.

The changes are relatively small but I do not have a previously working implementation to compare against so will be grateful if you can please review and merge the changes if you consider them appropriate.

If you would prefer a different approach then let me know.

Thanks.

LinusU commented 6 years ago

Fixed by #5, released as 0.2.1