Skip to content

Conversation

@alexangelov14
Copy link
Collaborator

Migration to WebCrypto api as requested in #23

@github-actions
Copy link

🎉 Beta deployment successful!: view the changes in live preview environment: https://gridbeta-github.asterics-foundation.org/alexangelov14/-23-sjcl-to-webcrypto-api/pr

@alexangelov14
Copy link
Collaborator Author

@klues just a reminder to check this PR whenever you can. Migration to WebCrypto api as requested in #23

@alexangelov14 alexangelov14 requested a review from klues December 15, 2025 14:15
Copy link
Contributor

@klues klues left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants