Parse-SDK-JS icon indicating copy to clipboard operation
Parse-SDK-JS copied to clipboard

feat: Replace `CryptoJS` with `Web Crypto`

Open dplewis opened this issue 8 months ago • 10 comments

Pull Request

Issue

Active development of CryptoJS has been discontinued. Nowadays, NodeJS and modern browsers have a native crypto module. This allows the SDK to have isomorphic encryption.

Closes: https://github.com/parse-community/Parse-SDK-JS/issues/2494

Approach

  • Replace Crypto-JS ASE-CBC with Web Crypto ASE-GCM-256 due to passphases key diveration being incompatible (could change in the future) and GCM is more secure Potential breaking change
  • Remove Parse.enableEncryptedUser(), Parse.encryptedUser, Parse.isEncryptedUser as they are not needed Breaking Change
  • Remove default support for React-Native, developers will now have to polyfill webcrypto Breaking Change
  • CryptoJS is syncronous while Web Crypto is asyncronous meaning some functions won't be available when this feature is turned on, like when using AsyncStorage. Breaking Change
    • Parse.User.current
    • Parse.User.isCurrent
  • Update Readme with instructions to create a custom crypto controller
  • Remove dependency on crypto-js and react-native-crypto-js
  • Developers will no longer have to install the optional dependency crypto-js along side parse

Due to the encryption and decryption method being different, developers who used this feature before won't be able to decrypt users that are already logged in but not cached on disk. I'm not sure how many developers were using this feature as it was undocumented. There are two ways for devs to solve this.

  1. Forcefully logging out users (deleting sessions). This will clean up the local storage.
  2. Follow the readme instructions and use the old crypto-js CryptoController as your custom controller. This will require the developers to install those packages.

Tasks

  • [x] Add tests
  • [x] Add changes to documentation (guides, repository pages, code comments)

dplewis avatar Mar 13 '25 14:03 dplewis

🚀 Thanks for opening this pull request!

Codecov Report

:x: Patch coverage is 92.59259% with 6 lines in your changes missing coverage. Please review. :white_check_mark: Project coverage is 99.79%. Comparing base (492de3e) to head (c4f9dae). :warning: Report is 4 commits behind head on alpha.

Files with missing lines Patch % Lines
src/CryptoController.ts 87.75% 6 Missing :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##            alpha    #2501      +/-   ##
==========================================
- Coverage   99.88%   99.79%   -0.10%     
==========================================
  Files          64       64              
  Lines        6220     6251      +31     
  Branches     1476     1478       +2     
==========================================
+ Hits         6213     6238      +25     
- Misses          7       13       +6     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

codecov[bot] avatar Mar 13 '25 14:03 codecov[bot]

@mtrezza Do you want to plan a Version 7.0.0 release?

dplewis avatar Mar 14 '25 12:03 dplewis

The thing is that we wouldn't be able to upgrade the JS SDK in Parse Server until December 2025 if we merged this. So we can't really merge this so easily with our current branch setup. We could tag this PR for merge on next major release.

mtrezza avatar Mar 14 '25 17:03 mtrezza

What does the server have to do with this?

dplewis avatar Mar 14 '25 17:03 dplewis

The Parse JS SDK is a dependency of Parse Sever.

mtrezza avatar Mar 15 '25 13:03 mtrezza

The server has a lot of dependencies, because the SDK breaks doesn’t mean the server breaks.

In the case of this PR. You can’t log in on the server, there is no local storage like the browser, there is nothing to encrypt, so this doesn’t break the server.

dplewis avatar Mar 15 '25 14:03 dplewis

The difference is that the Parse JS SDK is a fully exposed API in Cloud Code, not merely a "hidden" dependency (which we want to change). The "hidden" dependencies are tested indirectly through the Parse Server API tests. The fully exposed JS SDK API may not be completely tested in Parse Server because not all of its APIs apply there, as you mentioned. Because of that risk, major version increments in tandem are safer. If we decide to go ahead and merge a major version upgrade of the JS SDK, we'd need to make sure that even ineffective APIs of the JS SDK do not break anything, for example methods that wouldn't make sense to be called in Cloud Code, but fail gracefully or have no effect at all. I agree that we could do the upgrade, but it requires some extra care. And it would need to be a dedicated major release, meaning we cannot combine it with other breaking changes, as we usually would do to minimize the number of major releases. Are there any other PRs that should go into a 7.0.0 release?

mtrezza avatar Mar 16 '25 18:03 mtrezza

For version 7.0.0

  • Replace CryptoJS with Web Crypto https://github.com/parse-community/Parse-SDK-JS/pull/2501
  • Replace XMLHttpRequest with Fetch https://github.com/parse-community/Parse-SDK-JS/pull/2503
  • Convert the project to a ES Module which would require us moving from browserify https://github.com/parse-community/Parse-SDK-JS/pull/2425

dplewis avatar Mar 20 '25 15:03 dplewis

Which one of these are breaking and are there interdependencies? If only this crypto PR is breaking, then we can do a 7.0.0 release without waiting for the other ones. Since this is a "special" major release, it would make it also easier to control.

mtrezza avatar Mar 21 '25 07:03 mtrezza

📝 Walkthrough

Walkthrough

Encryption was migrated from CryptoJS to the Web Crypto API (and Node.js crypto.webcrypto). CryptoController APIs were changed to support async controllers; encryption/decryption are now async. Related core types, Parse user storage APIs, docs, tests, and packaging were updated accordingly.

Changes

Cohort / File(s) Change Summary
Node runtime & deps
.nvmrc, package.json
Node version bumped from 20.15.020.19.0. Removed react-native-crypto-js and optional crypto-js entries.
Documentation
README.md
Added "Encrypt Local Storage" section describing CryptoController, how to enable encryption (Parse.secret / CoreManager.set('ENCRYPTED_KEY', ...)), Web Crypto usage, polyfill guidance, and custom controller example/links.
Crypto implementation
src/CryptoController.ts, types/CryptoController.d.ts
Replaced CryptoJS AES with Web Crypto API (uses SubtleCrypto + PBKDF2 + AES-GCM). Encryption/decryption are async; helpers added for base64/ArrayBuffer conversions; method signatures updated.
Core manager & types
src/CoreManager.ts, types/CoreManager.d.ts, types/Parse.d.ts
CryptoController changed to a discriminated union with `async: 0
Parse API surface
src/Parse.ts
Removed encryptedUser accessor and enableEncryptedUser() / isEncryptedUserEnabled() methods; secret remains.
User persistence & APIs
src/ParseUser.ts
User disk update and current-user retrieval made asynchronous; encryption checks now use ENCRYPTED_KEY; added runtime error when sync currentUser() is called while async crypto is enabled; currentUserAsync and updateUserOnDisk updated to await crypto/storage operations.
Integration & unit tests
integration/test/ParseDistTest.js, integration/test/ParseReactNativeTest.js, integration/test/ParseUserTest.js, src/__tests__/*
Tests updated/added to exercise async CryptoController and Web Crypto usage: removed deprecated flags/calls, converted sync test calls to async (await), added global TextEncoder/TextDecoder and global.crypto polyfills for Node test environments, adapted RN test to spy on Web Crypto calls, and added a dist encryption test.

Sequence Diagram(s)

sequenceDiagram
    participant App
    participant Parse
    participant CoreManager
    participant CryptoController
    participant Storage

    Note over App,Parse: Store flow (async)
    App->>Parse: set secret / sign up
    Parse->>CoreManager: getCryptoController()
    CoreManager->>CryptoController: encrypt(json, secret) [async]
    CryptoController-->>CoreManager: Promise<encryptedData>
    CoreManager-->>Parse: encryptedData
    Parse->>Storage: setItemAsync(encryptedData)

    Note over App,Parse: Retrieve flow (async)
    App->>Parse: currentUserAsync()
    Parse->>Storage: getItemAsync()
    Parse->>CoreManager: getCryptoController()
    CoreManager->>CryptoController: decrypt(encryptedData, secret) [async]
    CryptoController-->>CoreManager: Promise<json>
    CoreManager-->>Parse: json
    Parse-->>App: user object

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~40 minutes

Potential focal points for review:

  • src/CryptoController.ts: PBKDF2 params, IV/salt layout, base64 conversions, error handling for decryption failures.
  • src/ParseUser.ts: async migration correctness, race conditions, and backward compatibility for existing stored data.
  • Type declarations (types/*) and CoreManager discriminated union correctness and usages across codebase.
  • Tests and test polyfills (TextEncoder/TextDecoder, global.crypto) to ensure CI environments run reliably.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (1 passed)
Check name Status Explanation
Title Check ✅ Passed The title "feat: Replace CryptoJS with Web Crypto" clearly and specifically describes the primary change in the pull request. It directly summarizes the main objective—replacing the deprecated CryptoJS encryption library with the native Web Crypto API. The title is concise, avoids generic language, and provides immediate clarity about the purpose of the changeset. A developer scanning git history would immediately understand that this PR migrates the encryption implementation to a modern, native crypto solution.

[pre_merge_check_pass]

The pull request description follows the repository template and includes all required sections. The Issue section contains a link to the related GitHub issue (#2494) and explains the problem being addressed (deprecated CryptoJS). The Approach section provides a detailed description of the changes being made, explicitly identifying multiple breaking changes and their implications. The Tasks section shows that both test additions and documentation updates have been completed. The description also includes a comprehensive migration guide for developers affected by the breaking changes, explaining two paths forward for users with previously encrypted data.

[pre_merge_check_pass]

The code changes successfully address the core objectives from linked issue #2494. The implementation replaces deprecated CryptoJS and react-native-crypto-js with the native Web Crypto API, enabling isomorphic encryption across Node.js and browsers without external dependencies. The CryptoController interface is updated to support asynchronous operations using Web Crypto's SubtleCrypto interface, and comprehensive documentation is added to the README explaining how developers can create custom CryptoControllers. The PR removes the problematic optional dependencies and provides clear migration guidance for existing users, fulfilling the issue's goal of modernizing the encryption implementation and improving compatibility with modern build tools.

[pre_merge_check_pass]

The changes are focused on the stated objective of replacing CryptoJS with Web Crypto API. All modifications—including updates to the CryptoController implementation, API changes (removal of enableEncryptedUser and related methods), async handling in ParseUser, dependency removal, documentation additions, and test updates—are directly related to this migration. The Node.js version update in .nvmrc (from 20.15.0 to 20.19.0) is reasonably in-scope as it likely reflects version requirements for proper Web Crypto API support. No unrelated refactoring, feature additions, or utility changes appear to have been introduced. |

✨ Finishing touches
🧪 Generate unit tests (beta)
  • [ ] Create PR with unit tests
  • [ ] Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between dcf12dfe5f636054fd452c5f23a848436f84f9ac and c4f9daeaf6e1f49479736fba92e1ccc5c2fdaa7a.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (1)
  • package.json (0 hunks)
💤 Files with no reviewable changes (1)
  • package.json

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

coderabbitai[bot] avatar Jul 29 '25 16:07 coderabbitai[bot]

:white_check_mark: Snyk checks have passed. No issues have been found so far.

Status Scanner Critical High Medium Low Total (0)
:white_check_mark: Open Source Security 0 0 0 0 0 issues

:computer: Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

parseplatformorg avatar Jul 29 '25 16:07 parseplatformorg

@mtrezza I fix the conflicts this is ready for review

dplewis avatar Jul 29 '25 17:07 dplewis

@mtrezza can we get this merged? V7 was released I thought it would be apart of it

dplewis avatar Oct 25 '25 02:10 dplewis

@dplewis Would love to merge this as well; to make this easy to understand for developers, could you take a look at my questions at https://github.com/parse-community/Parse-SDK-JS/pull/2501#pullrequestreview-3068449899?

mtrezza avatar Oct 25 '25 09:10 mtrezza