Skip to content

[PM-30144] Implement unlock-data re-encryption for key-rotation#685

Open
quexten wants to merge 4 commits intomainfrom
km/key-rotation-9
Open

[PM-30144] Implement unlock-data re-encryption for key-rotation#685
quexten wants to merge 4 commits intomainfrom
km/key-rotation-9

Conversation

@quexten
Copy link
Contributor

@quexten quexten commented Jan 16, 2026

🎟️ Tracking

https://bitwarden.atlassian.net/browse/PM-30144

📔 Objective

Implements unlock-method rotation for user-keys in the SDK. Note: This adds the key-connector enum variant and the none enum variant, but both are unsupported as of now and have comments indicating this.

🚨 Breaking Changes

⏰ Reminders before review

  • Contributor guidelines followed
  • All formatters and local linters executed and passed
  • Written new unit and / or integration tests where applicable
  • Protected functional changes with optionality (feature flags)
  • Used internationalization (i18n) for all UI strings
  • CI builds passed
  • Communicated to DevOps any deployment requirements
  • Updated any necessary documentation (Confluence, contributing docs) or informed the documentation
    team

🦮 Reviewer guidelines

  • 👍 (:+1:) or similar for great changes
  • 📝 (:memo:) or ℹ️ (:information_source:) for notes or general info
  • ❓ (:question:) for questions
  • 🤔 (:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed
    issue and could potentially benefit from discussion
  • 🎨 (:art:) for suggestions / improvements
  • ❌ (:x:) or ⚠️ (:warning:) for more significant problems or concerns needing attention
  • 🌱 (:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt
  • ⛏ (:pick:) for minor or nitpick changes

@quexten quexten changed the base branch from main to km/key-rotation-7 January 16, 2026 13:38
@quexten quexten changed the title Km/key rotation 9 Implement unlock-data re-encryption for key-rotation Jan 16, 2026
@quexten quexten changed the title Implement unlock-data re-encryption for key-rotation [PM-30144] Implement unlock-data re-encryption for key-rotation Jan 16, 2026
@github-actions
Copy link
Contributor

github-actions bot commented Jan 16, 2026

Logo
Checkmarx One – Scan Summary & Detailsce408000-e30e-4cb9-8740-6009ada0035e

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

@github-actions
Copy link
Contributor

github-actions bot commented Jan 16, 2026

🔍 SDK Breaking Change Detection Results

SDK Version: km/key-rotation-9 (7f184ee)
Completed: 2026-02-06 13:32:00 UTC
Total Time: 246s

Client Status Details
typescript ✅ No breaking changes detected TypeScript compilation passed with new SDK version - View Details

Breaking change detection completed. View SDK workflow

@quexten quexten changed the base branch from km/key-rotation-7 to km/key-rotation-11 January 16, 2026 13:53
@codecov
Copy link

codecov bot commented Jan 16, 2026

Codecov Report

❌ Patch coverage is 98.69403% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.30%. Comparing base (08a6bf3) to head (7f184ee).

Files with missing lines Patch % Lines
...-user-crypto-management/src/key_rotation/unlock.rs 98.41% 7 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #685      +/-   ##
==========================================
+ Coverage   80.02%   80.30%   +0.28%     
==========================================
  Files         312      314       +2     
  Lines       34516    35052     +536     
==========================================
+ Hits        27621    28150     +529     
- Misses       6895     6902       +7     

☔ 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.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@quexten quexten changed the base branch from km/key-rotation-11 to km/key-rotation-tmp January 16, 2026 16:30
debug_span!("reencrypt_emergency_access_key", grantee_id = ?ea.id).entered();
match UnsignedSharedKey::encapsulate(new_user_key_id, &ea.public_key, ctx) {
Ok(reencrypted_key) => Ok(EmergencyAccessWithIdRequestModel {
r#type: models::EmergencyAccessType::Takeover,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Please note: We should update the request model here. The data here is fully unused on the server-side except for the key.

salt: String,
},
/// The key-connector based unlock method.
KeyConnector,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

For the follow-up ticket, this should probably contain a key-connector key. The implementation is not done / tested here, but it is left as an example of how this is intended to be used.

}
}

fn assert_symmetric_keys_equal(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

May want to move this into keystore at some point, behind a compile-time debug flag.

@quexten quexten requested review from a team and mzieniukbw and removed request for a team and mzieniukbw January 19, 2026 10:40
@quexten quexten changed the base branch from km/key-rotation-tmp to main February 4, 2026 10:43
@quexten quexten marked this pull request as ready for review February 4, 2026 10:51
@quexten quexten requested review from a team as code owners February 4, 2026 10:51
@quexten quexten requested a review from Thomas-Avery February 4, 2026 10:51
Copy link
Contributor

@Thomas-Avery Thomas-Avery left a comment

Choose a reason for hiding this comment

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

Looking good, one question from my side.

@quexten quexten enabled auto-merge (squash) February 6, 2026 13:21
@quexten quexten requested a review from Thomas-Avery February 6, 2026 13:21
@sonarqubecloud
Copy link

sonarqubecloud bot commented Feb 6, 2026

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants