[BITAU-121] [BITAU-123] [BITAU-124] [BITAU-125] Read From Shared Store And Add to Auth UI
đī¸ Tracking
BITAU-121 BITAU-123 BITAU-124 BITAU-125
đ Objective
This PR does a first pass at bringing the AuthenticatorBridgeKit improvements into the Authenticator app, pull codes from the shared store and integrate those codes with the local codes for display. It adds the services to the AuthenticatorItemRepository where they integrate with the ItemListPublisher to display in the Item List. It also adds tests to the AuthenticatorItemRepository to make sure this is all functioning correctly.
~This is a minimalist first-pass approach, so there are a few things that need to be refined in the UI:~
- ~Editing a sync'd TOTP code should be disabled. It is allowed currently, which causes weird behavior to happen.~
- ~Search does not include any sync'd codes.~
This no longer applies as I've updated a bunch of things. See my comment below.
Important Testing Notes
- This is guarded by the feature flag, so you need to switch it on for anything to happen.
- In FeatureFlag.swift, set the default value of
.enablePasswordManagerSynctotrue
- In FeatureFlag.swift, set the default value of
- In order to write into the Store in the PM app, you'll need to turn on the feature flag and sync setting, and put some codes into the store. That will give you what you need to see this in action
- In the PM app, set the following in the new
AuthenticatorSyncService:
- In the PM app, set the following in the new
try await stateService.setSyncToAuthenticator(true)
if await configService.getFeatureFlag(FeatureFlag.enableAuthenticatorSync,
defaultValue: true) {
subscribeToAppState()
}
- Then run the app, log in, and background/foreground the app to get sync kicked off.
đ¸ Screenshots
Code sync'd over from the Bitwarden PM app:
Codes appearing in search:
â° 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
Checkmarx One â Scan Summary & Details â 4840e458-1505-4389-b197-33ba3961f25c
No New Or Fixed Issues Found
@KatherineInCode As you'll see here, I extended AuthenticatorItemView to add a way to create one from AuthenticatorBridgeItemDataView. I think this was the most direct way to get it into the UI so we can start looking at these. But I do think we'll need a good way of telling shared codes from local codes so we can do things like not allow editing.
From what I see in the code I can think of two possible paths:
- Add a new sharedTOTP item type to
ItemListItem.ItemType. - Add a type enum to
ItemListTotpItemso that you could have a.local(AuthenticatorItemView)and a.shared(AuthenticatorBridgeItemDataView)or something along those lines. That would allow them to have the same interface for the TOTPCodeModel, but a way of distinguishing them
I'd love to hear your thoughts on which of these would be better (or another idea on how to add these). In addition, I'll need to work on a way to add them to search. But I think I want to get clear on this issue first before tackling that.
Codecov Report
Attention: Patch coverage is 80.52632% with 37 lines in your changes missing coverage. Please review.
Project coverage is 66.27%. Comparing base (
a9c0bb3) to head (66e81f9). Report is 1 commits behind head on main.
Additional details and impacted files
@@ Coverage Diff @@
## main #163 +/- ##
==========================================
+ Coverage 65.94% 66.27% +0.32%
==========================================
Files 202 202
Lines 8716 8835 +119
==========================================
+ Hits 5748 5855 +107
- Misses 2968 2980 +12
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
@KatherineInCode @fedemkr @victor-livefront I've expanded this PR to cover integrating all of the UI pieces (hence we've gone from 1 to 4 tickets if you look at the description now). This picks up the columns that we added in the last PR on the PM repo and then builds a new ItemListItem.itemType. This allows us to do things like not show edit or delete on shared codes, but otherwise integrate them into the list along with the others.
Lastly, I also revamped how search works so that all of the codes (both the locally stored ones and the ones synced from the PM app) appear in the results (and get TOTP updates).