PM-10947: Handle key connector unlock for existing user #842
Conversation
matt-livefront
requested review from
fedemkr,
phil-livefront and
KatherineInCode
|
No New Or Fixed Issues Found |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #842 +/- ##
==========================================
+ Coverage 88.51% 88.57% +0.05%
==========================================
Files 606 607 +1
Lines 30262 30299 +37
==========================================
+ Hits 26787 26836 +49
+ Misses 3475 3463 -12 ☔ View full report in Codecov by Sentry. |
| if try await canUnlockWithDeviceKey(response) { | ||
| return nil | ||
| return .deviceKey | ||
| } | ||
|
|
||
| // Return the account if the vault still needs to be unlocked. | ||
| return try await stateService.getActiveAccount() | ||
| if try await canUnlockWithKeyConnectorKey(response) { | ||
| return .keyConnector | ||
| } | ||
|
|
||
| return try await .masterPassword(stateService.getActiveAccount()) |
There was a problem hiding this comment.
🎨 I see this is repeated on func loginWithSingleSignOn(code: String, email: String) async throws -> LoginUnlockMethod. Could we extract this to its own func to be reused?
🎨 Additionally, it seems Codecov is not finding tests for the .deviceKey path and I'm not finding them neither, could a test be added for that case?
There was a problem hiding this comment.
Good callout, I'll extract this and add a test for the device key flow!
| guard response.keyConnectorUrl != nil || | ||
| response.userDecryptionOptions?.keyConnectorOption?.keyConnectorUrl != nil |
There was a problem hiding this comment.
🎨 In MAUI code we're also checking that this url is not empty.
| try await services.authRepository.unlockVaultWithDeviceKey() | ||
| coordinator.navigate(to: .complete) |
There was a problem hiding this comment.
🎨 It seems a test is missing for .deviceKey flow.
🎟️ Tracking
PM-10947
📔 Objective
This handles vault unlock for an existing user who uses key connector. If the identity token response has a key connector URL, we use the existence of that to fetch the user's key from key connector and unlock the vault.
📸 Screenshots
Simulator.Screen.Recording.-.iPhone.15.Pro.-.2024-08-19.at.13.41.38.mp4
⏰ Reminders before review
🦮 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