-
Notifications
You must be signed in to change notification settings - Fork 53
[PM-8352] Fido2 creation user verification #736
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[PM-8352] Fido2 creation user verification #736
Conversation
Co-authored-by: Katherine Bertelsen <kbertelsen@bitwarden.com>
…owViewTests.swift Co-authored-by: Katherine Bertelsen <kbertelsen@bitwarden.com>
Co-authored-by: Katherine Bertelsen <kbertelsen@bitwarden.com>
…atorialTestRunner and go back mainFido2Credential to how it was to consider the case where the array is empty.
…m-8533/improve-autofill-cipher-ui # Conflicts: # BitwardenShared/UI/Platform/Application/Support/Localizations/en.lproj/Localizable.strings
…etf/pm-8863/fido2-creation
# Conflicts: # BitwardenShared/Core/Auth/Services/TestHelpers/BitwardenSdk+AuthFixtures.swift # BitwardenShared/UI/Vault/Vault/VaultList/VaultListItemTests.swift
… extension and use just the interface implementation.
…for SDK update on picked credential for creation. # Conflicts: # BitwardenShared/Core/Platform/Services/ServiceContainer.swift # BitwardenShared/UI/Vault/Vault/AutofillList/VaultAutofillListProcessor.swift # BitwardenShared/UI/Vault/VaultItem/AddEditItem/AddEditItemProcessor.swift # BitwardenShared/UI/Vault/VaultItem/VaultItemCoordinator.swift
…and move Fido2DebuggingUtils descriptions to extensions with CustomDebugStringConvertible
… autofill configuration works on iOS < 17.
… so it's more organized.
…refactored a bit the code to improve it and added unit tests. Also pre-filled the add item view when in Fido2 creation flow.
…ext/CredentialProviderContext.swift Co-authored-by: Matt Czech <matt@livefront.com>
…ng:, count:) initializer instead of mapping it.
…ate.swift Co-authored-by: Matt Czech <matt@livefront.com>
… and also added Bitwarden PIN validation to the Fido2 UV flow.
No New Or Fixed Issues Found |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #736 +/- ##
==========================================
+ Coverage 88.11% 88.19% +0.07%
==========================================
Files 573 573
Lines 28475 28540 +65
==========================================
+ Hits 25091 25170 +79
+ Misses 3384 3370 -14 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's one TODO that references this ticket, does that need to be handled here?
ios/BitwardenShared/UI/Vault/Vault/AutofillList/VaultAutofillListProcessor.swift
Lines 359 to 369 in 7228196
func onCipherForFido2CredentialPicked(cipher: CipherView) async { | |
guard let fido2appExtensionDelegate = appExtensionDelegate as? Fido2AppExtensionDelegate, | |
fido2appExtensionDelegate.isCreatingFido2Credential else { | |
return | |
} | |
services.fido2UserInterfaceHelper.pickedCredentialForCreation( | |
result: .success( | |
CheckUserAndPickCredentialForCreationResult( | |
cipher: CipherViewWrapper(cipher: cipher), | |
// TODO: PM-8352 add user verification | |
checkUserResult: CheckUserResult(userPresent: true, userVerified: true) |
BitwardenShared/UI/Auth/Utilities/UserVerificationHelperTests.swift
Outdated
Show resolved
Hide resolved
…swift Co-authored-by: Matt Czech <matt@livefront.com>
Thanks for catching that @matt-livefront, I've updated the ticket reference in the TODO comment. I was going to add the work as part of this PR but ended up splitting it so this PR wasn't that big and it requires other stuff to be able to test it normally in the app, like having the vault autofill list updated which will be done in another PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
🎟️ Tracking
PM-8352
📔 Objective
Add user verification check to Fido2 creation.
📸 Screenshots
Fido2.creation.with.PIN.UV.mov
Fido2.creation.with.MP.UV.mov
Fido2.creation.with.Bio.UV.mov
⏰ 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