[PM-37886] refactor: Update Key Connector vault unlock to use SDK's keyConnectorUrl method#2748
Conversation
🤖 Bitwarden Claude Code ReviewOverall Assessment: APPROVE Reviewed the Key Connector vault-unlock refactor that delegates the Key Connector fetch to the SDK via Code Review DetailsNo findings. This is a clean, well-scoped tech-debt refactor:
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2748 +/- ##
==========================================
+ Coverage 80.80% 80.82% +0.01%
==========================================
Files 1019 1018 -1
Lines 64849 64840 -9
==========================================
+ Hits 52402 52407 +5
+ Misses 12447 12433 -14 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…eyConnectorUrl method
8517d4c to
221b3f5
Compare
| coordinator.hideLoadingOverlay() | ||
| await coordinator.handleEvent(.didCompleteAuth) | ||
| } catch StateServiceError.noAccountCryptographicState { | ||
| coordinator.hideLoadingOverlay() |
There was a problem hiding this comment.
🎨 To one of the things Claude generated for this, is there a way we could test to make sure the overlay is hidden here?
🤔 Would the hiding of the overlay be something we could defer?
There was a problem hiding this comment.
🎨 To one of the things Claude generated for this, is there a way we could test to make sure the overlay is hidden here?
Yep, I updated the tests for this.
🤔 Would the hiding of the overlay be something we could
defer?
That would be ideal. At one point we ran into a problem where defer was causing the loading overlay to not hide when transitioning from the auth flow to the vault, which is why they are manually hidden in some places like this before completing auth. I'd like to clean this up, but I'd prefer to do it separately. I created a ticket: https://bitwarden.atlassian.net/browse/PM-38748.
|
Warning @matt-livefront Uploading code coverage report failed. Please check the "Upload to codecov.io" step of Process Test Reports job for more details. |
🎟️ Tracking
PM-37886
📔 Objective
Migrates the Key Connector vault unlock to let the SDK handle Key Connector communication, removing the app's direct
GET /user-keysnetwork call.Changes
AuthRepository.unlockVaultWithKeyConnectorKey: switches fromInitUserCryptoMethod.keyConnector(masterKey:userKey:)(which required a priorgetMasterKeyFromKeyConnectorAPI call) toInitUserCryptoMethod.keyConnectorUrl(url:keyConnectorKeyWrappedUserKey:), which delegates the Key Connector fetch to the SDK.getMasterKeyFromKeyConnectorfromKeyConnectorServiceandKeyConnectorAPIServiceprotocols and implementations.KeyConnectorUserKeyRequestandKeyConnectorUserKeyResponseModel.