PM-38625: Chore: Store the WrappedAccountCryptographicState

[david-livefront]

🎟️ Tracking

PM-38625

📔 Objective

This PR removes the privateKey and accountKeys from our persistence layer and migrates everything we need to a new WrappedAccountCryptographicState object.

• Migration logic has been added to the initialization of the AuthDiskSource
  • All old data stored to disk should be removed
• Any places that previously stored the privateKey and/or accountKeys should now store the appropriate WrappedAccountCryptographicState
• Any places retrieving the privateKey and/or accountKeys should now retrieve the WrappedAccountCryptographicState

[github-actions]

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

This PR migrates from separately stored privateKey and accountKeys to a unified WrappedAccountCryptographicState persisted via a custom version-tagged serializer. The change includes a one-time migration in AuthDiskSourceImpl.init, updates every call site that previously read or wrote the two legacy values, and adapts the corresponding tests. The migration logic correctly prefers the wrapped private key inside AccountKeysJson when present and falls back to the standalone encrypted private key, then nulls out the legacy storage keys.

Code Review Details

• ❓ : Sync no longer clears stored cryptographic state when the response carries null keys (already raised in existing thread)
  • app/src/main/kotlin/com/x8bit/bitwarden/data/vault/manager/VaultSyncManagerImpl.kt:381

[codecov]

Codecov Report

❌ Patch coverage is 78.15126% with 26 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.80%. Comparing base (c0caaf9) to head (d4c973a).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
...en/data/auth/datasource/disk/AuthDiskSourceImpl.kt	41.37%	15 Missing and 2 partials ⚠️
...izer/WrappedAccountCryptographicStateSerializer.kt	94.28%	0 Missing and 2 partials ⚠️
...rm/repository/AuthenticatorBridgeRepositoryImpl.kt	50.00%	1 Missing and 1 partial ⚠️
...twarden/data/vault/manager/VaultLockManagerImpl.kt	50.00%	1 Missing and 1 partial ⚠️
...twarden/data/vault/manager/VaultSyncManagerImpl.kt	71.42%	0 Missing and 2 partials ⚠️
.../auth/repository/util/AccountKeysJsonExtensions.kt	90.90%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #7030      +/-   ##
==========================================
+ Coverage   85.50%   86.80%   +1.29%     
==========================================
  Files        1005      903     -102     
  Lines       66462    65029    -1433     
  Branches     9328     9285      -43     
==========================================
- Hits        56829    56448     -381     
+ Misses       6426     5383    -1043     
+ Partials     3207     3198       -9     

Flag	Coverage Δ
app-data	17.23% <78.15%> (+0.11%)	⬆️
app-ui-auth-tools	18.99% <0.00%> (-0.01%)	⬇️
app-ui-platform	16.79% <0.00%> (-0.02%)	⬇️
app-ui-vault	28.31% <0.00%> (-0.01%)	⬇️
authenticator	6.21% <0.00%> (-0.01%)	⬇️
lib-core-network-bridge	4.06% <0.00%> (-0.01%)	⬇️
lib-data-ui	1.14% <0.00%> (-0.01%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[claude]

❓ QUESTION: Sync no longer clears stored cryptographic state when the response carries null keys

Details

Previously storeProfileData unconditionally called storePrivateKey(userId, profile.privateKey) and storeAccountKeys(userId, profile.accountKeys). If a sync response returned null for those fields, the on-disk values were cleared.

The new implementation only writes when profile.privateKeyOrNull() is non-null, so a sync response with both accountKeys and the deprecated privateKey field null will leave any previously stored WrappedAccountCryptographicState untouched rather than clearing it.

Is the divergence intentional (e.g., to guard against transient null sync responses overwriting valid local state), or should we preserve the previous "clear on null" behavior by writing null to storeAccountCryptographicState when privateKeyOrNull() is null?

[david-livefront]

This logic has not changed but I was able to completely remove the createWrappedAccountCryptographicState() function