Skip to content

PM-27237: feat: v2 encryption for key connector#6814

Merged
david-livefront merged 1 commit intomainfrom
PM-27237-key-connector-v2-encryption
Apr 22, 2026
Merged

PM-27237: feat: v2 encryption for key connector#6814
david-livefront merged 1 commit intomainfrom
PM-27237-key-connector-v2-encryption

Conversation

@david-livefront
Copy link
Copy Markdown
Collaborator

@david-livefront david-livefront commented Apr 21, 2026

🎟️ Tracking

PM-27237

📔 Objective

This PR adds the updated v2 encryption flow for a SSO Key Connector new user.

@david-livefront david-livefront requested a review from a team as a code owner April 21, 2026 14:18
@david-livefront david-livefront added the ai-review-vnext Request a Claude code review using the vNext workflow label Apr 21, 2026
@github-actions github-actions Bot added app:password-manager Bitwarden Password Manager app context app:authenticator Bitwarden Authenticator app context t:deps Change Type - Dependencies t:feature Change Type - Feature Development labels Apr 21, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 21, 2026

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

Reviewed the v2 encryption path for new SSO Key Connector users (PM-27237). The change adds AuthSdkSource.postKeysForKeyConnectorRegistration, introduces a feature-flagged V2 branch in KeyConnectorManagerImpl.migrateNewUserToKeyConnector that delegates key generation and server storage to the SDK, and threads the new MigrateNewUserToKeyConnectorResult model through AuthRepositoryImpl. Legacy V1 behavior is preserved behind the V2EncryptionKeyConnector flag, and unit tests cover both paths including SDK-failure scenarios.

Code Review Details

No blocking findings. Notable observations considered and dismissed:

  • The MigrateNewUserToKeyConnectorResult.privateKey field is technically redundant with accountCryptographicState.privateKey (both V1/V2 variants carry it), but the current shape simplifies call sites and is not incorrect.
  • The new authDiskSource.storeAccountTokens(userId, accountTokens = null) in the vault-unlock-error callback at AuthRepositoryImpl.kt:1674 is a defensive cleanup; it is harmless since tokens are only persisted later in the success path.
  • Only the V2 branch wraps the SDK call in withContext(dispatcherManager.io); the legacy branch does not. This is an acceptable inconsistency for a feature-flagged rollout.

Dependency Changes

Package Change Ecosystem
bitwardenSdk 2.0.0-6074-f373e7f3 → 2.0.0-6216-92bfeaa8 Gradle

@david-livefront david-livefront removed the t:deps Change Type - Dependencies label Apr 21, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 21, 2026

Codecov Report

❌ Patch coverage is 96.77419% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.75%. Comparing base (39240b3) to head (ae7f6ed).
⚠️ Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
...arden/data/auth/manager/KeyConnectorManagerImpl.kt 97.05% 0 Missing and 1 partial ⚠️
...twarden/data/auth/repository/AuthRepositoryImpl.kt 95.65% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6814      +/-   ##
==========================================
- Coverage   85.81%   85.75%   -0.07%     
==========================================
  Files         856      829      -27     
  Lines       59571    58659     -912     
  Branches     8560     8554       -6     
==========================================
- Hits        51121    50301     -820     
+ Misses       5499     5407      -92     
  Partials     2951     2951              
Flag Coverage Δ
app-data 17.47% <96.77%> (+0.32%) ⬆️
app-ui-auth-tools 20.35% <0.00%> (-0.02%) ⬇️
app-ui-platform 15.22% <0.00%> (-0.02%) ⬇️
app-ui-vault 26.00% <0.00%> (-0.64%) ⬇️
authenticator 6.69% <0.00%> (-0.03%) ⬇️
lib-core-network-bridge 4.27% <0.00%> (-0.01%) ⬇️
lib-data-ui 1.03% <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 Sentry.
📢 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.

@github-actions
Copy link
Copy Markdown
Contributor

Logo
Checkmarx One – Scan Summary & Detailsdd4647a5-2454-46bc-b8e5-5eb467ca7c33

Great job! No new security vulnerabilities introduced in this pull request

@david-livefront
Copy link
Copy Markdown
Collaborator Author

Thanks @SaintPatrck

@david-livefront david-livefront added this pull request to the merge queue Apr 21, 2026
Merged via the queue into main with commit 308a1f9 Apr 22, 2026
27 of 30 checks passed
@david-livefront david-livefront deleted the PM-27237-key-connector-v2-encryption branch April 22, 2026 00:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-review-vnext Request a Claude code review using the vNext workflow app:authenticator Bitwarden Authenticator app context app:password-manager Bitwarden Password Manager app context t:feature Change Type - Feature Development

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants