clients icon indicating copy to clipboard operation
clients copied to clipboard

[PM-5796] Improve desktop biometric browser integration error handling

Open dani-garcia opened this issue 1 year ago • 2 comments

Type of change

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

Objective

Enabling browser integration for biometric unlock in the desktop application requires the creation of certain manifest files in some OS and browser specific locations. These files are created only when the setting is enabled, and any errors that happen during this operation are simply logged to the console and the user is none the wiser.

This PR is an attempt to improve this situation in a couple of ways:

  • We re-register these manifests at app startup, in case they were removed due to a browser reinstall or other causes.
  • On Windows, we add the manifests unconditionally instead of trying to detect that the browser is installed. This caused some browser installs to be missed and the integration to fail (Windows Store/portable/system-wide installs).
  • Adding some basic logging in the browser extension, where previously the error was ignored.
  • Handling any errors that happen while enabling the setting on the desktop app, and surfacing a message to the user.

TODO: Oscar mentioned to me that we might want to offer a way to disable the startup manifest generation, to help when trying to test dev builds of the browser extensions.

Code changes

  • file.ext: Description of what was changed and why

Screenshots

Added a single dialog: image

dani-garcia avatar Jan 29 '24 18:01 dani-garcia

Codecov Report

Attention: Patch coverage is 0% with 91 lines in your changes are missing coverage. Please review.

Project coverage is 27.31%. Comparing base (1e0ad09) to head (605a2ca).

Files Patch % Lines
apps/desktop/src/main/native-messaging.main.ts 0.00% 65 Missing :warning:
...pps/desktop/src/app/accounts/settings.component.ts 0.00% 12 Missing :warning:
apps/desktop/src/main.ts 0.00% 6 Missing :warning:
.../app/services/native-messaging-manifest.service.ts 0.00% 4 Missing :warning:
apps/desktop/src/platform/preload.ts 0.00% 2 Missing :warning:
...owser/src/background/nativeMessaging.background.ts 0.00% 1 Missing :warning:
apps/desktop/src/app/services/services.module.ts 0.00% 1 Missing :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7727      +/-   ##
==========================================
- Coverage   27.32%   27.31%   -0.02%     
==========================================
  Files        2349     2350       +1     
  Lines       68609    68643      +34     
  Branches    12827    12831       +4     
==========================================
  Hits        18748    18748              
- Misses      48451    48485      +34     
  Partials     1410     1410              

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Jan 29 '24 18:01 codecov[bot]

Logo Checkmarx One – Scan Summary & Detailsacaf8015-5276-421a-9f7f-481f7ec1bd00

No New Or Fixed Issues Found

bitwarden-bot avatar Jan 30 '24 05:01 bitwarden-bot