authenticator-ios icon indicating copy to clipboard operation
authenticator-ios copied to clipboard

[BITAU-121] [BITAU-123] [BITAU-124] [BITAU-125] Read From Shared Store And Add to Auth UI

Open brant-livefront opened this issue 1 year ago â€ĸ 4 comments

đŸŽŸī¸ 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 .enablePasswordManagerSync to true
  • 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:
     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: Simulator Screenshot - iPhone 15 Pro - 2024-10-02 at 16 42 10

Codes appearing in search: Simulator Screenshot - iPhone 15 Pro - 2024-10-11 at 15 11 37

⏰ 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

brant-livefront avatar Oct 02 '24 21:10 brant-livefront

Logo Checkmarx One – Scan Summary & Details – 4840e458-1505-4389-b197-33ba3961f25c

No New Or Fixed Issues Found

github-actions[bot] avatar Oct 02 '24 21:10 github-actions[bot]

@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 ItemListTotpItem so 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.

brant-livefront avatar Oct 02 '24 21:10 brant-livefront

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.

Files with missing lines Patch % Lines
...ared/Core/Platform/Services/ServiceContainer.swift 0.00% 23 Missing :warning:
...lt/Views/ItemListItemRow/ItemListItemRowView.swift 16.66% 5 Missing :warning:
...UI/Vault/ItemList/ItemList/ItemListProcessor.swift 73.33% 4 Missing :warning:
...ared/UI/Vault/ItemList/ItemList/ItemListItem.swift 95.83% 2 Missing :warning:
...ared/UI/Vault/ItemList/ItemList/ItemListView.swift 90.00% 2 Missing :warning:
...ult/Repositories/AuthenticatorItemRepository.swift 98.61% 1 Missing :warning:
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.

codecov[bot] avatar Oct 02 '24 21:10 codecov[bot]

@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).

brant-livefront avatar Oct 11 '24 19:10 brant-livefront