Skip to content
This repository was archived by the owner on May 14, 2025. It is now read-only.

[SG-416] Updates to Bitwarden Authenticator#1964

Merged
andrebispo5 merged 56 commits intofeature/feature-totpfrom
feature/totp-tab
Jul 26, 2022
Merged

[SG-416] Updates to Bitwarden Authenticator#1964
andrebispo5 merged 56 commits intofeature/feature-totpfrom
feature/totp-tab

Conversation

@andrebispo5
Copy link
Contributor

Type of change

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

Objective

Rework on Authenticator UI/UX in order to improve its usability and promote the use of scanner the feature for TOTP setup. Also hoping to clarify the feature differences based on the plan selected.

Code changes

Clients PR: bitwarden/clients#3045

  • src/App/Controls/AuthenticatorViewCell/AuthenticatorViewCell.xaml:
    New cell UI to show TOTP code circular progress and cipher info. Will appear on cipher list with TOTP toggle on.
  • src/App/Controls/CircularProgressbarView.xaml.cs:
    Circular progress control has some properties to customise its appearance. The file also contains some helper classes that are only used here in this control.
  • src/App/Pages/Vault/AddEditPage.xaml:
    In order to make the button with the camera icon and text work, avoiding many forms bugs, i had to create a frame and place an IconLabel inside. There is a bug with the ButtonView where the text gets clipped.
  • src/App/Pages/Vault/GroupingsPage/GroupingsPage.xaml:
    New header to have the toggle for TOTP ciphers. Added new data template to support the authenticator cells.
  • src/App/Pages/Vault/GroupingsPage/GroupingsPageListGroup.cs:
    Changed inherit list type to the generic interface in order to use the GroupingsPageTOTPListItem in the selector.
  • src/App/Pages/Vault/ScanPage.xaml
    Added SKCanvasView to hold the scanning animation. Also added the new UI to enter the code manually.
  • src/App/Pages/Vault/ScanPage.xaml.cs
    Added method to draw the scanning squared corners into the SKCanvasView. The object being drawn gets its size from a global variable _scale that is calculated based on a Sine function and a Stopwatch. Currently it is being drawn by the animation loop at a rate of 1/30 of a second, if we see there is some kind of performance impairment, we can increase this value.

Screenshots

New toggle to show only ciphers with TOTP codes:
image
New list of ciphers with only TOTP codes:
image
Toast when copy TOTP value:
image
New QRCode scanning animation, the square corners pulse and turn green when code found also phone vibrates:
image
New UI to enter the code manually:
image
Free users won't be able to see the TOTP code, instead helper text is shown:
image
Premium users will see a circular progress and the TOTP code:
image
Authentication key when empty, will only show a button the scan a QRCode:
image

Before you submit

  • I have checked for formatting errors (dotnet tool run dotnet-format --check) (required)
  • I have added unit tests where it makes sense to do so (encouraged but not required)
  • This change requires a documentation update (notify the documentation team)
  • This change has particular deployment requirements (notify the DevOps team)

cmuentes and others added 30 commits February 11, 2022 15:22
# Conflicts:
#	src/App/App.csproj
# Conflicts:
#	src/App/Pages/Authenticator/AuthenticatorPage.xaml
#	src/App/Pages/Authenticator/AuthenticatorPage.xaml.cs
#	src/App/Pages/Authenticator/AuthenticatorPageViewModel.cs
…y text of switch because android was overlapping text.
…en. Changed existing labels on scanner screen.
@andrebispo5 andrebispo5 changed the title [PS-70] Updates to Bitwarden Authenticator [SSG-416] Updates to Bitwarden Authenticator Jul 6, 2022
@andrebispo5 andrebispo5 marked this pull request as ready for review July 7, 2022 14:28
# Conflicts:
#	src/App/Resources/AppResources.Designer.cs
#	src/App/Resources/AppResources.resx
@fedemkr fedemkr changed the title [SSG-416] Updates to Bitwarden Authenticator [SG-416] Updates to Bitwarden Authenticator Jul 7, 2022
Copy link
Member

@fedemkr fedemkr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work!! 😄 , here you have some proposed changes to improve the code, let me know if you have any doubts on them

Comment on lines +2276 to 2278
<comment>Authenticator TOTP feature</comment>
</data>
<data name="Name" xml:space="preserve">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should coordinate with crowdin updates and when possible update this to have the proper name AuthenticatorKey and update Authenticator Key (TOTP) to have the name AuthenticatorKeyTOTP instead of using the one that would for this. Don't do it ASAP, first talk it through the team.

… it places an helper text next to the switch making it invisible. Also removed from the label because it already reads the text from the label
@vvolkgang vvolkgang added the hold do not merge yet label Jul 14, 2022
@vvolkgang
Copy link
Member

This shouldn't be included in our next release, we'll merge it in a feature branch for the time being.

# Conflicts:
#	src/App/Resources/AppResources.Designer.cs
#	src/App/Resources/AppResources.resx
@andrebispo5 andrebispo5 changed the base branch from master to feature/feature-totp July 18, 2022 13:22
@andrebispo5 andrebispo5 requested a review from fedemkr July 18, 2022 13:22
Copy link
Member

@fedemkr fedemkr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great improvements, I've left some more; let me know what you think

@andrebispo5 andrebispo5 merged commit 2f2b90a into feature/feature-totp Jul 26, 2022
@andrebispo5 andrebispo5 deleted the feature/totp-tab branch July 26, 2022 19:26
@vvolkgang vvolkgang removed the hold do not merge yet label Jul 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants