clients icon indicating copy to clipboard operation
clients copied to clipboard

[PM-10938] Import ssh-keys from 1password 1pux

Open djsmith85 opened this issue 1 year ago â€ĸ 2 comments

đŸŽŸī¸ Tracking

https://bitwarden.atlassian.net/browse/PM-10938

📔 Objective

With the introduction of the SSH keys item type, we can now also support importing ssh keys from other password managers. This add support for 1password's 1pux format.

The 1pux format has changed and the updates to fix it have been done at https://github.com/bitwarden/clients/pull/10778. This PR requires the aforementioned to be merged first

⏰ 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

djsmith85 avatar Aug 29 '24 08:08 djsmith85

Logo Checkmarx One – Scan Summary & Details – 11827cf6-210a-448f-bb04-2579387ef1b2

New Issues

Severity Issue Source File / Package Checkmarx Insight
HIGH CVE-2024-11115 Npm-electron-33.2.1 Vulnerable Package

Fixed Issues

Severity Issue Source File / Package
MEDIUM Client_Privacy_Violation /apps/browser/src/vault/popup/components/vault-v2/vault-list-items-container/vault-list-items-container.component.html: 15

github-actions[bot] avatar Aug 29 '24 08:08 github-actions[bot]

@quexten Usually I'd say the conversion needs to happen on import, as the target system dictates what is supported. I'll need to look into libraries then to convert PKCS#8 to OPENSSH.

Idea for a future improvement: Adding support for users to manually add PKCS#8 into their vault and convert it on save?

djsmith85 avatar Aug 29 '24 10:08 djsmith85

@djsmith85 Merge & squashing the target branch PR to the feature branch added all the squashed commits to your PR's diff (since they were squashed in the target) so I did an interactive rebase & force push to not have this PR be convoluted. Sorry about the inconvenience.

quexten avatar Aug 30 '24 14:08 quexten

@djsmith85 Adding support for PKCS#8 parsing to rust importing code here: https://github.com/bitwarden/clients/pull/11048, this will be ported over to sdk later on. Once sdk is included in all clients, it should not matter what format 1password provides, it will be parsed either way. For now users should be able to either go the 1password->file-openssh->bitwarden-desktop-import or 1password->copypaste-pkcs#8->bitwarden-desktop route.

quexten avatar Sep 14 '24 19:09 quexten

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 33.79%. Comparing base (395258d) to head (a9e9971).

:white_check_mark: All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #10787   +/-   ##
=======================================
  Coverage   33.79%   33.79%           
=======================================
  Files        2912     2912           
  Lines       90701    90713   +12     
  Branches    17153    17155    +2     
=======================================
+ Hits        30648    30660   +12     
  Misses      57667    57667           
  Partials     2386     2386           

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

codecov[bot] avatar Dec 02 '24 20:12 codecov[bot]