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-10395] Add new item type ssh key #10360

Merged
merged 8 commits into from
Aug 30, 2024

Conversation

quexten
Copy link
Contributor

@quexten quexten commented Aug 1, 2024

🎟️ Tracking

Server: bitwarden/server#4575
Add Item Type: #10360
Add SSH Agent: #10293
Add Import/Export: #10529

Jira: https://bitwarden.atlassian.net/browse/PM-10395

📔 Objective

Add the new SSH key item type, consisting of private key, and derived fingerprint and public key. We are not generating these on the fly since we do not have the sdk available on all clients.

Note

This will be merged into a feature branch. This PR does not support key generation or import, thus testing will be done in the feature branch.

📸 Screenshots

⏰ 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

@quexten quexten added the hold do not merge, do not approve yet label Aug 1, 2024
@quexten quexten changed the title Add ssh key item type [PM-10395] Add new item type ssh key Aug 1, 2024
Copy link
Contributor

github-actions bot commented Aug 1, 2024

Logo
Checkmarx One – Scan Summary & Detailsf80306d0-9209-43a1-be9e-9f7103101256

New Issues

Severity Issue Source File / Package Checkmarx Insight
MEDIUM Absolute_Path_Traversal /apps/cli/src/oss-serve-configurator.ts: 282 Attack Vector
MEDIUM Absolute_Path_Traversal /apps/cli/src/oss-serve-configurator.ts: 282 Attack Vector
MEDIUM Absolute_Path_Traversal /apps/cli/src/oss-serve-configurator.ts: 314 Attack Vector
MEDIUM Absolute_Path_Traversal /apps/cli/src/oss-serve-configurator.ts: 314 Attack Vector
MEDIUM Angular_Improper_Type_Pipe_Usage /apps/web/src/app/layouts/product-switcher/product-switcher.component.html: 1 Attack Vector
MEDIUM Angular_Improper_Type_Pipe_Usage /libs/components/src/navigation/nav-divider.component.html: 1 Attack Vector
MEDIUM Angular_Improper_Type_Pipe_Usage /apps/browser/src/autofill/popup/fido2/fido2-use-browser-link.component.html: 1 Attack Vector
MEDIUM Client_Privacy_Violation /apps/browser/src/autofill/background/overlay.background.ts: 475 Attack Vector
MEDIUM Client_Privacy_Violation /apps/browser/src/autofill/background/overlay.background.ts: 475 Attack Vector
MEDIUM Client_Privacy_Violation /apps/browser/src/autofill/background/overlay.background.ts: 445 Attack Vector
MEDIUM Client_Privacy_Violation /apps/browser/src/autofill/background/overlay.background.ts: 445 Attack Vector
MEDIUM Client_Privacy_Violation /apps/browser/src/autofill/background/overlay.background.ts: 445 Attack Vector
MEDIUM Client_Privacy_Violation /apps/browser/src/autofill/background/overlay.background.ts: 475 Attack Vector
MEDIUM Client_Privacy_Violation /apps/browser/src/autofill/overlay/inline-menu/pages/list/autofill-inline-menu-list.ts: 859 Attack Vector
MEDIUM Client_Privacy_Violation /apps/browser/src/autofill/overlay/inline-menu/pages/list/autofill-inline-menu-list.ts: 883 Attack Vector
MEDIUM Client_Privacy_Violation /apps/browser/src/autofill/overlay/inline-menu/pages/list/autofill-inline-menu-list.ts: 880 Attack Vector
MEDIUM Client_Privacy_Violation /apps/browser/src/autofill/overlay/inline-menu/pages/list/autofill-inline-menu-list.ts: 880 Attack Vector
MEDIUM Client_Privacy_Violation /apps/browser/src/autofill/overlay/inline-menu/pages/list/autofill-inline-menu-list.ts: 883 Attack Vector
MEDIUM Client_Privacy_Violation /apps/browser/src/autofill/background/overlay.background.ts: 541 Attack Vector
MEDIUM Client_Privacy_Violation /apps/browser/src/autofill/background/overlay.background.ts: 541 Attack Vector
MEDIUM Client_Privacy_Violation /apps/browser/src/autofill/background/overlay.background.ts: 541 Attack Vector
MEDIUM Client_Privacy_Violation /apps/browser/src/autofill/background/overlay.background.ts: 1518 Attack Vector
MEDIUM Client_Privacy_Violation /apps/browser/src/autofill/background/overlay.background.ts: 1517 Attack Vector
MEDIUM Client_Privacy_Violation /apps/browser/src/autofill/overlay/inline-menu/pages/list/autofill-inline-menu-list.ts: 597 Attack Vector
MEDIUM Client_Privacy_Violation /apps/browser/src/autofill/overlay/inline-menu/pages/list/autofill-inline-menu-list.ts: 883 Attack Vector
MEDIUM Client_Privacy_Violation /apps/browser/src/autofill/overlay/inline-menu/pages/list/autofill-inline-menu-list.ts: 859 Attack Vector
MEDIUM Client_Privacy_Violation /apps/browser/src/autofill/overlay/inline-menu/pages/list/autofill-inline-menu-list.ts: 859 Attack Vector
MEDIUM Client_Privacy_Violation /apps/browser/src/autofill/overlay/inline-menu/pages/list/autofill-inline-menu-list.ts: 880 Attack Vector
MEDIUM Client_Privacy_Violation /apps/web/src/app/auth/lock.component.ts: 18 Attack Vector
MEDIUM Client_Privacy_Violation /apps/web/src/app/auth/lock.component.ts: 46 Attack Vector
MEDIUM Client_Privacy_Violation /apps/web/src/app/tools/password-generator-history.component.html: 11 Attack Vector
MEDIUM Client_Privacy_Violation /apps/web/src/app/tools/password-generator-history.component.html: 11 Attack Vector
MEDIUM Client_Privacy_Violation /apps/desktop/src/vault/app/vault/view.component.html: 583 Attack Vector
MEDIUM Client_Privacy_Violation /apps/web/src/connectors/webauthn-fallback.ts: 116 Attack Vector
MEDIUM Client_Privacy_Violation /bitwarden_license/bit-web/src/app/auth/sso/sso.component.ts: 161 Attack Vector
MEDIUM Client_Privacy_Violation /bitwarden_license/bit-web/src/app/auth/sso/sso.component.ts: 161 Attack Vector
MEDIUM Client_Privacy_Violation /libs/components/src/color-password/color-password.component.ts: 14 Attack Vector
MEDIUM Client_Privacy_Violation /apps/desktop/src/vault/app/vault/view.component.html: 60 Attack Vector
MEDIUM Client_Privacy_Violation /apps/desktop/src/vault/app/vault/view.component.html: 56 Attack Vector
MEDIUM Client_Privacy_Violation /apps/browser/src/tools/popup/generator/password-generator-history.component.html: 26 Attack Vector
MEDIUM Client_Privacy_Violation /apps/browser/src/vault/popup/components/vault/password-history.component.html: 18 Attack Vector
MEDIUM Client_Privacy_Violation /apps/desktop/src/app/tools/password-generator-history.component.html: 15 Attack Vector
MEDIUM Client_Privacy_Violation /apps/desktop/src/vault/app/vault/password-history.component.html: 12 Attack Vector
MEDIUM Client_Privacy_Violation /apps/desktop/src/vault/app/vault/view.component.html: 50 Attack Vector
MEDIUM Missing_HSTS_Header /apps/cli/src/auth/commands/login.command.ts: 707 Attack Vector
MEDIUM Unpinned Actions Full Length Commit SHA /publish-cli.yml: 171 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /version-bump.yml: 82 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /build-cli.yml: 402 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /publish-cli.yml: 92 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /version-bump.yml: 188 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /build-desktop.yml: 1291 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /build-browser.yml: 379 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /publish-desktop.yml: 115 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /version-bump.yml: 516 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /version-bump.yml: 360 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /version-bump.yml: 181 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /version-bump.yml: 500 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /retrieve-current-desktop-rollout.yml: 23 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /publish-web.yml: 44 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /publish-cli.yml: 129 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /version-bump.yml: 248 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /version-bump.yml: 508 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /release-desktop-beta.yml: 668 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /version-bump.yml: 298 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /version-bump.yml: 195 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /version-bump.yml: 492 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /version-bump.yml: 202 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /build-browser.yml: 420 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /build-desktop.yml: 1245 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /version-bump.yml: 48 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /publish-desktop.yml: 124 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /version-bump.yml: 163 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
LOW Angular_Usage_of_Unsafe_DOM_Sanitizer /libs/components/src/avatar/avatar.component.ts: 80 Attack Vector
LOW Angular_Usage_of_Unsafe_DOM_Sanitizer /apps/desktop/src/app/components/avatar.component.ts: 75 Attack Vector
LOW Angular_Usage_of_Unsafe_DOM_Sanitizer /libs/components/src/icon/icon.component.ts: 18 Attack Vector
LOW Client_DOM_Open_Redirect /apps/web/src/connectors/common.ts: 2 Attack Vector
LOW Client_DOM_Open_Redirect /apps/web/src/connectors/common.ts: 2 Attack Vector
LOW Client_DOM_Open_Redirect /apps/desktop/src/auth/accessibility-cookie.component.html: 18 Attack Vector
LOW Client_DOM_Open_Redirect /apps/web/src/connectors/common.ts: 2 Attack Vector
LOW Client_DOM_Open_Redirect /apps/web/src/connectors/sso.ts: 21 Attack Vector
LOW Client_DOM_Open_Redirect /apps/web/src/connectors/common.ts: 2 Attack Vector
LOW Client_DOM_Open_Redirect /apps/web/src/connectors/sso.ts: 19 Attack Vector
LOW Client_DOM_Open_Redirect /apps/web/src/connectors/common.ts: 2 Attack Vector
LOW Client_DOM_Open_Redirect /apps/web/src/connectors/sso.ts: 15 Attack Vector
LOW Client_DOM_Open_Redirect /apps/browser/src/auth/popup/login-via-auth-request.component.ts: 52 Attack Vector
LOW Client_DOM_Open_Redirect /apps/desktop/src/auth/login/login-via-auth-request.component.ts: 60 Attack Vector
LOW Client_DOM_Open_Redirect /apps/desktop/src/auth/login/login-via-auth-request.component.ts: 60 Attack Vector
LOW Client_DOM_Open_Redirect /apps/browser/src/auth/popup/login-via-auth-request.component.ts: 52 Attack Vector
LOW Client_DOM_Open_Redirect /apps/browser/src/tools/popup/generator/password-generator-history.component.ts: 19 Attack Vector
LOW Client_DOM_Open_Redirect /apps/browser/src/auth/popup/account-switching/current-account.component.ts: 35 Attack Vector
LOW Client_DOM_Open_Redirect /apps/browser/src/vault/popup/components/vault/password-history.component.ts: 23 Attack Vector
LOW Client_DOM_Open_Redirect /apps/browser/src/vault/popup/components/vault/attachments.component.ts: 33 Attack Vector
LOW Client_DOM_Open_Redirect /apps/browser/src/billing/popup/settings/premium.component.ts: 27 Attack Vector
LOW Client_DOM_Open_Redirect /libs/common/src/auth/iframe-component.ts: 49 Attack Vector
LOW Client_DOM_Open_Redirect /apps/desktop/src/auth/scripts/duo.js: 277 Attack Vector
LOW Client_DOM_Open_Redirect /libs/common/src/auth/webauthn-iframe.ts: 25 Attack Vector
LOW Client_DOM_Open_Redirect /apps/desktop/src/auth/scripts/duo.js: 277 Attack Vector
LOW Client_Hardcoded_Domain /libs/common/src/billing/services/payment-processors/stripe.service.ts: 23 Attack Vector
LOW Client_Hardcoded_Domain /apps/web/src/app/billing/shared/payment.component.ts: 68 Attack Vector
LOW Client_Hardcoded_Domain /apps/web/src/app/billing/shared/payment.component.ts: 68 Attack Vector
LOW Client_Hardcoded_Domain /apps/web/src/connectors/captcha.ts: 57 Attack Vector
LOW Client_JQuery_Deprecated_Symbols /apps/cli/src/models/response.ts: 36 Attack Vector
LOW Client_JQuery_Deprecated_Symbols /apps/cli/src/commands/get.command.ts: 149 Attack Vector
LOW Client_JQuery_Deprecated_Symbols /apps/cli/src/commands/get.command.ts: 142 Attack Vector
LOW Client_JQuery_Deprecated_Symbols /libs/importer/src/importers/base-importer.ts: 314 Attack Vector
LOW Client_JQuery_Deprecated_Symbols /apps/cli/src/commands/get.command.ts: 324 Attack Vector
LOW Client_JQuery_Deprecated_Symbols /apps/cli/src/commands/download.command.ts: 22 Attack Vector
LOW Client_JQuery_Deprecated_Symbols /apps/cli/src/base-program.ts: 115 Attack Vector
LOW Client_JQuery_Deprecated_Symbols /apps/cli/src/auth/commands/login.command.ts: 575 Attack Vector
LOW Client_JQuery_Deprecated_Symbols /libs/angular/src/auth/components/change-password.component.ts: 95 Attack Vector
LOW Client_JQuery_Deprecated_Symbols /libs/angular/src/auth/components/update-temp-password.component.ts: 137

More results are available on AST platform

Copy link

codecov bot commented Aug 1, 2024

Codecov Report

Attention: Patch coverage is 49.18033% with 62 lines in your changes missing coverage. Please review.

Project coverage is 32.93%. Comparing base (cfdc52e) to head (cc36ebb).
Report is 1 commits behind head on feature/ssh-keys.

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...ponents/sshkey-section/sshkey-section.component.ts 50.00% 7 Missing and 2 partials ⚠️
libs/common/src/vault/models/domain/cipher.ts 11.11% 8 Missing ⚠️
.../common/src/vault/models/request/cipher.request.ts 11.11% 8 Missing ⚠️
libs/common/src/vault/models/domain/ssh-key.ts 78.94% 4 Missing ⚠️
...ommon/src/vault/models/response/cipher.response.ts 0.00% 4 Missing ⚠️
libs/common/src/vault/models/api/ssh-key.api.ts 66.66% 3 Missing ⚠️
libs/common/src/vault/models/view/cipher.view.ts 40.00% 3 Missing ⚠️
libs/common/src/vault/services/cipher.service.ts 25.00% 3 Missing ⚠️
...ault/src/components/copy-cipher-field.directive.ts 0.00% 3 Missing ⚠️
...v2/item-copy-action/item-copy-actions.component.ts 0.00% 2 Missing ⚠️
... and 9 more
Additional details and impacted files
@@                 Coverage Diff                  @@
##           feature/ssh-keys   #10360      +/-   ##
====================================================
+ Coverage             32.92%   32.93%   +0.01%     
====================================================
  Files                  2671     2677       +6     
  Lines                 81785    81907     +122     
  Branches              15418    15435      +17     
====================================================
+ Hits                  26928    26977      +49     
- Misses                52774    52841      +67     
- Partials               2083     2089       +6     

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

@quexten quexten force-pushed the auth/pm-10395/ssh-keys-item-type branch from e1debc0 to 22fe2da Compare August 21, 2024 11:59
@quexten quexten removed the hold do not merge, do not approve yet label Aug 21, 2024
@quexten quexten force-pushed the auth/pm-10395/ssh-keys-item-type branch from a0c9686 to 4ee2944 Compare August 21, 2024 12:18
@quexten quexten force-pushed the auth/pm-10395/ssh-keys-item-type branch from 4ee2944 to 99f10b1 Compare August 21, 2024 12:20
@quexten quexten marked this pull request as ready for review August 21, 2024 15:46
@quexten quexten requested a review from a team as a code owner August 21, 2024 15:46
@merissaacosta merissaacosta requested review from gbubemismith and removed request for LRNcardozoWDF August 21, 2024 16:03
Copy link
Member

@gbubemismith gbubemismith left a comment

Choose a reason for hiding this comment

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

Thank you for your work on this, @quexten. I have noted a few areas that need adjustment, but overall this looks really good 🚀

@quexten quexten requested a review from a team as a code owner August 30, 2024 09:30
@quexten quexten force-pushed the auth/pm-10395/ssh-keys-item-type branch from 1f6b02f to cc36ebb Compare August 30, 2024 09:35
@quexten quexten requested review from gbubemismith and removed request for a team and dani-garcia August 30, 2024 09:35
@quexten
Copy link
Contributor Author

quexten commented Aug 30, 2024

Merging to the feature branch; QA testing will be done on the feature-branch.

@quexten quexten merged commit b18fa68 into feature/ssh-keys Aug 30, 2024
65 of 66 checks passed
@quexten quexten deleted the auth/pm-10395/ssh-keys-item-type branch August 30, 2024 14:16
@quexten quexten mentioned this pull request Oct 1, 2024
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.

3 participants