github-starred/android
2
0
Fork 0
mirror of https://github.com/bitwarden/android.git synced 2026-05-07 19:39:41 -05:00
Code Issues 1.1k Packages Projects Releases 122 Wiki Activity

[PR #2404] [MERGED] [EC-981] OTP handling - Set to selected cipher #29554

New Issue
Closed
opened 2026-04-18 13:31:22 -05:00 by GiteaMirror · 0 comments
GiteaMirror commented 2026-04-18 13:31:22 -05:00
Owner
Copy Link

📋 Pull Request Information

Original PR: https://github.com/bitwarden/android/pull/2404
Author: @fedemkr
Created: 3/6/2023
Status: ✅ Merged
Merged: 3/7/2023
Merged by: @fedemkr

Base: feature/EC-979-iOS-third-party-2fa ← Head: feature/ios-third-party-2fa/EC-981-add-to-existing-cipher

📝 Commits (8)

  • ede36d8 EC-981 Started adding OTP to existing cipher. Reused AutofillCiphersPage for the cipher selection and refactored it so that we have more code reuse
  • 4a457e8 Merge branch 'master' into feature/ios-third-party-2fa/EC-981-add-to-existing-cipher
  • f9a5619 Merge branch 'master' into feature/ios-third-party-2fa/EC-981-add-to-existing-cipher
  • a59d9b2 EC-981 Fix navigation on otp handling
  • c1db294 Merge branch 'master' into feature/ios-third-party-2fa/EC-981-add-to-existing-cipher
  • 26a4797 EC-981 Fix formatting
  • 74b010d Merge branch 'feature/EC-979-iOS-third-party-2fa' into feature/ios-third-party-2fa/EC-981-add-to-existing-cipher
  • 477d395 EC-981 Added otp cipher selection callout and add close toolbar item when needed

📊 Changes

25 files changed (+672 additions, -297 deletions)

View changed files

📝 src/Android/MainActivity.cs (+4 -4)
📝 src/App/Abstractions/IAccountsManager.cs (+1 -0)
📝 src/App/Abstractions/IDeepLinkContext.cs (+1 -4)
📝 src/App/App.xaml.cs (+28 -11)
📝 src/App/Models/AppOptions.cs (+3 -0)
📝 src/App/Pages/Vault/AutofillCiphersPageViewModel.cs (+51 -169)
📝 src/App/Pages/Vault/CipherAddEditPageViewModel.cs (+23 -0)
📝 src/App/Pages/Vault/CipherSelectionPage.xaml (+37 -12)
📝 src/App/Pages/Vault/CipherSelectionPage.xaml.cs (+49 -22)
➕ src/App/Pages/Vault/CipherSelectionPageViewModel.cs (+161 -0)
📝 src/App/Pages/Vault/CiphersPage.xaml.cs (+9 -5)
📝 src/App/Pages/Vault/CiphersPageViewModel.cs (+51 -21)
➕ src/App/Pages/Vault/OTPCipherSelectionPageViewModel.cs (+70 -0)
📝 src/App/Resources/AppResources.Designer.cs (+13 -7)
📝 src/App/Resources/AppResources.resx (+3 -0)
📝 src/App/Services/DeepLinkContext.cs (+10 -9)
📝 src/App/Utilities/AccountManagement/AccountsManager.cs (+11 -1)
📝 src/App/Utilities/AppHelpers.cs (+4 -2)
📝 src/App/Utilities/AppSetup.cs (+1 -1)
📝 src/Core/Enums/NavigationTarget.cs (+2 -1)

...and 5 more files

📄 Description

Type of change

  • Bug fix
  • New feature development
  • Tech debt (refactoring, code cleanup, dependency upgrades, etc)
  • Build/deploy pipeline (DevOps)
  • Other

Objective

Add the possibility to choose a cipher to set the scanned OTP handled by the Bitwarden app.

Code changes

AutofillCiphersPage -> CipherSelectionPage

Given that selecting a cipher on Autofill and selecting it on OTP handling is pretty much the same, the AutofillCiphersPage was taken and renamed to CipherSelectionPage to have a common UI to share between these flows. The context changes depending on the AppOptions passed to the page creating the appropriate ViewModel for each flow.

A diagram can illustrate this better:

classDiagram
    class CipherSelectionPageViewModel
    <<Abstract>> CipherSelectionPageViewModel
    CipherSelectionPageViewModel <|-- AutofillCiphersPageViewModel
    CipherSelectionPageViewModel <|-- OTPCipherSelectionPageViewModel
    CipherSelectionPage --> CipherSelectionPageViewModel

Other changes

  • AccountsManager: Added method to start the navigation flow (as in changing an account) that allows the caller to perform any action to the current AppOptions
  • DeepLinkContext: Added method to handle new incoming Uris
  • App.xaml.cs: Added otp handling logic and updated some hardcoded message commands values with consts
  • MainActivity: Updated hardcoded message commands with consts
  • AppOptions: Added OtpData to get the data from the otp uri
  • CipherSelectionPage.xaml: Added close toolbar item and updated obsolete properties. Also, changed selection to new approach and added callout for the OTP cipher selection.
  • CipherSelectionPage.xaml.cs: Added logic to choose between the Autofill and OTP viewmodels depending on the AppOptions. Also, added logic for Close. Something to highlight, is that the Account Switching toolbar item was left with the default .. icon on iOS because there is an issue and it's not being updated correctly (it just shows a white backgrounded circle). So, for now I've just left it with the default one which IMO is better than the white circle.
  • CipherSelectionPageViewModel.cs: Moved common things to this abstract class and added template methods to be implemented by the children
  • AutofillCiphersPageViewModel: Add inheritance from CipherSelectionPageVIewModel and removed things that are now in the abstract class. Additionally, the select flow was changed to accept an IGroupingsPageListItem to be more generic and flexible.
  • OTPCipherSelectionPageVIewModel: Implemented for OTP handling given the base CipherSelectionPageVIewModel class. The suggested ciphers to show on loading are the ones which include the Issuer of the OTP if exists or the AccountName otherwise.
  • CipherAddEditPageViewModel: Added automatic set of OTP key when coming from OTP flow, the tooltip and the appropriate navigation when saving.
  • CiphersPage.xaml.cs: Moved some initialization to the VM using the Prepare(...) method to have less logic on the view.
  • CiphersPageViewModel: Updated CipherOptionsCommand to use the new async way. Added logic to show all ciphers if there's no search term and it comes from the OTP flow. Also, on selection it goes to the edit page of the cipher when coming from the OTP flow.
  • AppHelpers: Added navigation logic for OTP flow
  • NavigationTarget: Added new target for OTP flow
  • OTPData: Added this struct which is the responsible to get all the attributes/parameters from an OTP Uri to be reused throughout the app.
  • TOTPService: Updated it to use the new OTPData struct.
  • UriExtensions: Added method to unescape a URI string like a OTP URI (it needs to be done multiple times to correctly unescape everything)

Screenshots

Cipher Selection

Cipher selection OTP

Cipher Search

Cipher search OTP

OTP set on Cipher edition

Cipher edition OTP set

Before you submit

  • Please check for formatting errors (dotnet format --verify-no-changes) (required)
  • Please add unit tests where it makes sense to do so (encouraged but not required)
  • If this change requires a documentation update - notify the documentation team
  • If this change has particular deployment requirements - notify the DevOps team

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/bitwarden/android/pull/2404 **Author:** [@fedemkr](https://github.com/fedemkr) **Created:** 3/6/2023 **Status:** ✅ Merged **Merged:** 3/7/2023 **Merged by:** [@fedemkr](https://github.com/fedemkr) **Base:** `feature/EC-979-iOS-third-party-2fa` ← **Head:** `feature/ios-third-party-2fa/EC-981-add-to-existing-cipher` --- ### 📝 Commits (8) - [`ede36d8`](https://github.com/bitwarden/android/commit/ede36d899c631f503980e06e65c38b5f9e4f2649) EC-981 Started adding OTP to existing cipher. Reused AutofillCiphersPage for the cipher selection and refactored it so that we have more code reuse - [`4a457e8`](https://github.com/bitwarden/android/commit/4a457e8f1dff6060c2cc3c17c1d64d28711c0e09) Merge branch 'master' into feature/ios-third-party-2fa/EC-981-add-to-existing-cipher - [`f9a5619`](https://github.com/bitwarden/android/commit/f9a5619d4ae047e4df1ba1d1c9fe1cf206fb9976) Merge branch 'master' into feature/ios-third-party-2fa/EC-981-add-to-existing-cipher - [`a59d9b2`](https://github.com/bitwarden/android/commit/a59d9b2e6bae26652733612c987df01450451bca) EC-981 Fix navigation on otp handling - [`c1db294`](https://github.com/bitwarden/android/commit/c1db294f42d53a6e1ba83fe24fb033780fcc7174) Merge branch 'master' into feature/ios-third-party-2fa/EC-981-add-to-existing-cipher - [`26a4797`](https://github.com/bitwarden/android/commit/26a4797c7302aeb4b24c70b3666d29a88764abcc) EC-981 Fix formatting - [`74b010d`](https://github.com/bitwarden/android/commit/74b010d27622cb7013ff8c8791679bffce2b6801) Merge branch 'feature/EC-979-iOS-third-party-2fa' into feature/ios-third-party-2fa/EC-981-add-to-existing-cipher - [`477d395`](https://github.com/bitwarden/android/commit/477d395b58bf26153ef55d4ea81baad00c9bdb20) EC-981 Added otp cipher selection callout and add close toolbar item when needed ### 📊 Changes **25 files changed** (+672 additions, -297 deletions) <details> <summary>View changed files</summary> 📝 `src/Android/MainActivity.cs` (+4 -4) 📝 `src/App/Abstractions/IAccountsManager.cs` (+1 -0) 📝 `src/App/Abstractions/IDeepLinkContext.cs` (+1 -4) 📝 `src/App/App.xaml.cs` (+28 -11) 📝 `src/App/Models/AppOptions.cs` (+3 -0) 📝 `src/App/Pages/Vault/AutofillCiphersPageViewModel.cs` (+51 -169) 📝 `src/App/Pages/Vault/CipherAddEditPageViewModel.cs` (+23 -0) 📝 `src/App/Pages/Vault/CipherSelectionPage.xaml` (+37 -12) 📝 `src/App/Pages/Vault/CipherSelectionPage.xaml.cs` (+49 -22) ➕ `src/App/Pages/Vault/CipherSelectionPageViewModel.cs` (+161 -0) 📝 `src/App/Pages/Vault/CiphersPage.xaml.cs` (+9 -5) 📝 `src/App/Pages/Vault/CiphersPageViewModel.cs` (+51 -21) ➕ `src/App/Pages/Vault/OTPCipherSelectionPageViewModel.cs` (+70 -0) 📝 `src/App/Resources/AppResources.Designer.cs` (+13 -7) 📝 `src/App/Resources/AppResources.resx` (+3 -0) 📝 `src/App/Services/DeepLinkContext.cs` (+10 -9) 📝 `src/App/Utilities/AccountManagement/AccountsManager.cs` (+11 -1) 📝 `src/App/Utilities/AppHelpers.cs` (+4 -2) 📝 `src/App/Utilities/AppSetup.cs` (+1 -1) 📝 `src/Core/Enums/NavigationTarget.cs` (+2 -1) _...and 5 more files_ </details> ### 📄 Description ## Type of change - [ ] Bug fix - [X] New feature development - [ ] Tech debt (refactoring, code cleanup, dependency upgrades, etc) - [ ] Build/deploy pipeline (DevOps) - [ ] Other ## Objective <!--Describe what the purpose of this PR is. For example: what bug you're fixing or what new feature you're adding--> Add the possibility to choose a cipher to set the scanned OTP handled by the Bitwarden app. ## Code changes <!--Explain the changes you've made to each file or major component. This should help the reviewer understand your changes--> <!--Also refer to any related changes or PRs in other repositories--> ### AutofillCiphersPage -> CipherSelectionPage Given that selecting a cipher on `Autofill` and selecting it on `OTP handling` is pretty much the same, the `AutofillCiphersPage` was taken and renamed to `CipherSelectionPage` to have a common UI to share between these flows. The context changes depending on the `AppOptions` passed to the page creating the appropriate `ViewModel` for each flow. A diagram can illustrate this better: ```mermaid classDiagram class CipherSelectionPageViewModel <<Abstract>> CipherSelectionPageViewModel CipherSelectionPageViewModel <|-- AutofillCiphersPageViewModel CipherSelectionPageViewModel <|-- OTPCipherSelectionPageViewModel CipherSelectionPage --> CipherSelectionPageViewModel ``` ### Other changes * **AccountsManager:** Added method to start the navigation flow (as in changing an account) that allows the caller to perform any action to the current `AppOptions` * **DeepLinkContext:** Added method to handle new incoming Uris * **App.xaml.cs:** Added otp handling logic and updated some hardcoded message commands values with consts * **MainActivity:** Updated hardcoded message commands with consts * **AppOptions:** Added `OtpData` to get the data from the otp uri * **CipherSelectionPage.xaml:** Added close toolbar item and updated obsolete properties. Also, changed selection to new approach and added callout for the OTP cipher selection. * **CipherSelectionPage.xaml.cs:** Added logic to choose between the `Autofill` and `OTP` viewmodels depending on the `AppOptions`. Also, added logic for `Close`. Something to highlight, is that the `Account Switching` toolbar item was left with the default `..` icon on iOS because there is an issue and it's not being updated correctly (it just shows a white backgrounded circle). So, for now I've just left it with the default one which IMO is better than the white circle. * **CipherSelectionPageViewModel.cs:** Moved common things to this abstract class and added template methods to be implemented by the children * **AutofillCiphersPageViewModel:** Add inheritance from `CipherSelectionPageVIewModel` and removed things that are now in the abstract class. Additionally, the select flow was changed to accept an `IGroupingsPageListItem` to be more generic and flexible. * **OTPCipherSelectionPageVIewModel:** Implemented for OTP handling given the base `CipherSelectionPageVIewModel` class. The suggested ciphers to show on loading are the ones which include the `Issuer` of the OTP if exists or the `AccountName` otherwise. * **CipherAddEditPageViewModel:** Added automatic set of OTP key when coming from OTP flow, the tooltip and the appropriate navigation when saving. * **CiphersPage.xaml.cs:** Moved some initialization to the VM using the `Prepare(...)` method to have less logic on the view. * **CiphersPageViewModel:** Updated `CipherOptionsCommand` to use the new async way. Added logic to show all ciphers if there's no search term and it comes from the OTP flow. Also, on selection it goes to the edit page of the cipher when coming from the OTP flow. * **AppHelpers:** Added navigation logic for OTP flow * **NavigationTarget:** Added new target for OTP flow * **OTPData:** Added this struct which is the responsible to get all the attributes/parameters from an OTP Uri to be reused throughout the app. * **TOTPService:** Updated it to use the new `OTPData` struct. * **UriExtensions:** Added method to unescape a URI string like a OTP URI (it needs to be done multiple times to correctly unescape everything) ## Screenshots <!--Required for any UI changes. Delete if not applicable--> ### Cipher Selection <img width="314" alt="Cipher selection OTP" src="https://user-images.githubusercontent.com/15682323/223222993-70bc8f83-1b67-452b-b9dc-f414cb2e71a9.jpeg"> ### Cipher Search <img width="314" alt="Cipher search OTP" src="https://user-images.githubusercontent.com/15682323/223175976-03440dad-79e3-4d2a-8b43-c3d19565d7b4.PNG"> ### OTP set on Cipher edition <img width="314" alt="Cipher edition OTP set" src="https://user-images.githubusercontent.com/15682323/223175958-a0350df1-6a6a-49bc-bd3c-d7be7e57f47e.PNG"> ## Before you submit - Please check for formatting errors (`dotnet format --verify-no-changes`) (required) - Please add **unit tests** where it makes sense to do so (encouraged but not required) - If this change requires a **documentation update** - notify the documentation team - If this change has particular **deployment requirements** - notify the DevOps team --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
GiteaMirror added the pull-request label 2026-04-18 13:31:22 -05:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
app:authenticator
app:password-manager
bug
bug-passkey
customer-support
duplicate
enhancement
good first issue
help wanted
needs-reply
needs-response
pull-request
Mirrored from GitHub Pull Request
 question
No Label pull-request
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
GiteaMirror ninjasurge
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: github-starred/android#29554
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?