Skip to content
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-15904] Implement Credential Exchange Import flow #1223

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

fedemkr
Copy link
Member

@fedemkr fedemkr commented Dec 19, 2024

🎟️ Tracking

PM-15904

📔 Objective

Implement Credential Exchange import flow.
This is part of PM-14800 to implement the new APIs for Credential Exchange on iOS 18.2. Also a continuation of #1198

This also updates the SDK revision to 02332b257970cde14e48a8708e12e722f4a236d9 so that it includes the Credential Exchange functions of the SDK necessary to import/export credentials.

Environment Support

⚠️ This requires Xcode 16.2. The code has several compiler checks #if compiler(>=6.0.3) to ensure that block of code is only compiled when in the proper version of the compiler/Xcode; this ensures Xcode 16.1 (currently in CI/CD) version to continue compiling the project.

Feature enablement

⚠️ In order to enabled Credential Exchange alpha_update_cxp_infoplist.sh needs to be run first to update the Info.plist files with the proper configuration. I've also added this on select_variant.sh with a compiler flag SUPPORTS_CXP so we can trigger builds with this enabled from the CI/CD passing such flag.

Make sure you have OS Settings -> Developer -> Credential Exchange enabled and (in simulator) also a Face ID enrolled and enabled.

Additionally, particularly for the import flow there is a feature flag called cxp-import-mobile that also needs to be enabled. Otherwise, the import flow will show an error when the Bitwarden app opens. If on DEBUG mode, you can also change it in the DEBUG view while seeing the error and retry the operation.

📸 Screenshots

Import.Credential.Exchange.flow.mov

⏰ 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

# Conflicts:
#	BitwardenShared/Core/Platform/Services/ServiceContainer.swift
#	BitwardenShared/Core/Platform/Services/TestHelpers/ServiceContainer+Mocks.swift
Copy link
Contributor

github-actions bot commented Dec 19, 2024

Logo
Checkmarx One – Scan Summary & Details96896670-fbfd-4a15-96b9-8b27dab9f1f8

No New Or Fixed Issues Found

Copy link

codecov bot commented Dec 27, 2024

Codecov Report

Attention: Patch coverage is 92.48705% with 29 lines in your changes missing coverage. Please review.

Project coverage is 89.59%. Comparing base (43e1883) to head (9e3b678).

Files with missing lines Patch % Lines
...denShared/UI/Tools/ImportCXP/ImportCXPModule.swift 0.00% 6 Missing ⚠️
...nShared/UI/Platform/Application/AppProcessor.swift 66.66% 5 Missing ⚠️
...Tools/ImportCXP/ImportCXP/ImportCXPProcessor.swift 87.80% 5 Missing ⚠️
...twardenShared/Core/Platform/Utilities/AnyKey.swift 50.00% 4 Missing ⚠️
...e/Tools/Repositories/ImportCiphersRepository.swift 66.66% 3 Missing ⚠️
...ore/Tools/Utilities/CredentialManagerFactory.swift 50.00% 3 Missing ⚠️
...d/UI/Tools/ImportCXP/ImportCXP/ImportCXPView.swift 97.43% 2 Missing ⚠️
...nShared/Core/Platform/Services/ClientService.swift 66.66% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1223      +/-   ##
==========================================
+ Coverage   89.47%   89.59%   +0.11%     
==========================================
  Files         700      709       +9     
  Lines       44733    45079     +346     
==========================================
+ Hits        40026    40387     +361     
+ Misses       4707     4692      -15     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants