Conversation
|
🎉 Beta deployment successful!: view the changes in live preview environment: https://gridbeta-github.asterics-foundation.org/alexangelov14/-23-sjcl-to-webcrypto-api/pr |
There was a problem hiding this comment.
Great that you've started with this 👍
Looks good at first sight, but since it's really something at the core and very important that it works, we should test very much. Did you already test with online users? You can use let _serverUrl = constants.IS_ENVIRONMENT_PROD || true in loginService.js to test with online users locally. Important are two tests:
- register / create user with the new crypto enabled
- register / create user with old crypto and test if it still works with the new implementation (fallback)
What we also could think about if it could be solved with this update is the issue I've explained in this comment: #541 (comment)
It would be great if we would get rid of metadata.id as encryption salt and maybe use the current username instead (which is much more stable and shouldn't change at all). So: keep as-is for the sjcl part, but don't use metadata.id as salt anymore for the new web crypto implementation.
| } | ||
|
|
||
| // Use SJCL for hash to maintain compatibility with existing hashes | ||
| // WebCrypto SHA-256 produces same result but we keep SJCL for consistency |
There was a problem hiding this comment.
Yeah, as long we need sjcl for backwards compatibilty, probably it makes sense to always use sjcl for hashes, since they're not the performance issue.
| */ | ||
| function getCrypto() { | ||
| if (typeof window !== 'undefined' && window.crypto) { | ||
| return window.crypto; |
There was a problem hiding this comment.
didn't know that web crypto is available in the main thread in window. When I looked at it some years ago, it was only available for webworkers, and that was the reason I never looked at it, it seemed to much work to implement.
Migration to WebCrypto api as requested in #23