[PM-8829] Password/Fido2 credentials autofill on prepare credential list #759
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #759 +/- ##
==========================================
+ Coverage 88.06% 88.12% +0.05%
==========================================
Files 577 577
Lines 28874 29104 +230
==========================================
+ Hits 25428 25648 +220
- Misses 3446 3456 +10 ☔ View full report in Codecov by Sentry. |
|
No New Or Fixed Issues Found |
BitwardenShared/Core/Autofill/Models/PasskeyCredentialRequestParameters.swift
Outdated
Show resolved
Hide resolved
BitwardenShared/UI/Autofill/Utilities/Fido2UserInterfaceHelperTests.swift
Outdated
Show resolved
Hide resolved
BitwardenShared/UI/Autofill/Utilities/Fido2UserInterfaceHelper.swift
Outdated
Show resolved
Hide resolved
BitwardenShared/UI/Vault/Vault/AutofillList/VaultAutofillListProcessor.swift
Outdated
Show resolved
Hide resolved
BitwardenShared/UI/Vault/Vault/AutofillList/VaultAutofillListProcessor.swift
Outdated
Show resolved
Hide resolved
BitwardenShared/UI/Vault/Vault/AutofillList/VaultAutofillListProcessor.swift
Outdated
Show resolved
Hide resolved
BitwardenShared/UI/Vault/Vault/AutofillList/VaultAutofillListView.swift
Outdated
Show resolved
Hide resolved
…al with passkey request parameters to be in AutofillCredentialService to reuse the logic in there with the SDK.
…ocessor to the VaultRepository and fix race condition issue with available credentials for authentication.
| let sections = try XCTUnwrap(sectionsResult) | ||
|
|
||
| XCTAssertEqual( | ||
| sections[0], | ||
| VaultListSection( | ||
| id: Localizations.passkeysForX(expectedRpID), | ||
| items: expectedCiphersInFido2Section.map { cipherView in |
There was a problem hiding this comment.
🎨 One alternative to writing out these section assertions is to use an inline snapshot test. This uses a helper to dump a textual description of the sections. There's pros/cons to this vs writing out the full assertions, but I find it helpful to visualize the test output (and it's much less tedious to write). But up to you on what you prefer.
It would end up looking like this:
try assertInlineSnapshot(of: dumpVaultListSections(XCTUnwrap(sections)), as: .lines) {
"""
Section: Passkeys for myApp.com
- Cipher: Bitwarden
- Cipher: Café
- Cipher: Bitwarden
Section: Passwords for myApp.com
- Cipher: Example
"""
}
The result is generated by the snapshot testing framework so you can just write try assertInlineSnapshot(of: dumpVaultListSections(XCTUnwrap(sections)), as: .lines) and run the tests to have it populate the output initially.
There was a problem hiding this comment.
Oh you're right, I totally missed inline snapshots! 🎉 I'll make a tech-debt task for this so I can tackle it properly on another PR given that I'd need some update in the dump so it shows something about the inner Fido2 credentials in order to see that the item section is complete with all the information it needs.
🎟️ Tracking
PM-8829
📔 Objective
Handle prepare credential list for autofilling passwords/Fido2 credentials from the iOS Autofill extension.
📸 Screenshots
Combined.passwords.and.Fido2.credentials.autofill.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