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

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Conversation

quexten
Copy link
Contributor

@quexten quexten commented Aug 1, 2024

🎟️ Tracking

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

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

📔 Objective

Add server support for the new ssh key cipher type. This is mostly copy paste from the other cipher types, with the one exception that we are filtering out ssh keys for older clients, using SSHKeyCipherMinimumVersion. We will update this once we know which release ssh keys will be in.

📸 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

Copy link

codecov bot commented Aug 1, 2024

Codecov Report

Attention: Patch coverage is 30.61224% with 34 lines in your changes missing coverage. Please review.

Project coverage is 41.89%. Comparing base (2d762f8) to head (39ab9a7).
Report is 64 commits behind head on main.

Files Patch % Lines
src/Api/Vault/Models/Request/CipherRequestModel.cs 6.25% 15 Missing ⚠️
src/Api/Vault/Models/CipherSSHKeyModel.cs 40.00% 6 Missing ⚠️
...c/Api/Vault/Models/Response/CipherResponseModel.cs 0.00% 6 Missing ⚠️
src/Core/Vault/Models/Data/CipherSSHKeyData.cs 0.00% 4 Missing ⚠️
src/Api/Vault/Controllers/SyncController.cs 76.92% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4575      +/-   ##
==========================================
+ Coverage   41.64%   41.89%   +0.24%     
==========================================
  Files        1271     1290      +19     
  Lines       60187    60961     +774     
  Branches     5527     5599      +72     
==========================================
+ Hits        25066    25540     +474     
- Misses      33951    34231     +280     
- Partials     1170     1190      +20     

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

@quexten quexten added the hold Hold this PR or item until later; DO NOT MERGE label Aug 1, 2024
@quexten quexten changed the title [PM-10394] Add ssh key item type [PM-10394] Add new item type ssh key Aug 1, 2024
Copy link
Contributor

github-actions bot commented Aug 1, 2024

Logo
Checkmarx One – Scan Summary & Details2d0264ec-67cb-49f5-8760-c66c22e0da7c

New Issues

Severity Issue Source File / Package Checkmarx Insight
MEDIUM CSRF /src/Api/Public/Controllers/CollectionsController.cs: 87 Attack Vector
MEDIUM CSRF /src/Api/Controllers/CollectionsController.cs: 171 Attack Vector
MEDIUM CSRF /src/Api/Controllers/CollectionsController.cs: 143 Attack Vector
MEDIUM CSRF /src/Api/AdminConsole/Controllers/OrganizationsController.cs: 316 Attack Vector
MEDIUM CSRF /src/Api/AdminConsole/Controllers/OrganizationsController.cs: 316 Attack Vector
MEDIUM CSRF /src/Api/AdminConsole/Controllers/OrganizationsController.cs: 316 Attack Vector
MEDIUM CSRF /src/Api/AdminConsole/Controllers/OrganizationsController.cs: 316 Attack Vector
MEDIUM CSRF /src/Api/AdminConsole/Controllers/OrganizationsController.cs: 316 Attack Vector
MEDIUM Privacy_Violation /src/Core/Auth/Services/Implementations/AuthRequestService.cs: 147 Attack Vector
MEDIUM Privacy_Violation /src/Core/Services/Implementations/UserService.cs: 840 Attack Vector
LOW Log_Forging /src/Api/Billing/Controllers/ProviderClientsController.cs: 29 Attack Vector
LOW Open_Redirect /src/Admin/Auth/Controllers/LoginController.cs: 50 Attack Vector
LOW Unsafe_Use_Of_Target_blank /src/Core/MailTemplates/Handlebars/Welcome.html.hbs: 63 Attack Vector
LOW Unsafe_Use_Of_Target_blank /src/Core/MailTemplates/Handlebars/Welcome.html.hbs: 63 Attack Vector
LOW Unsafe_Use_Of_Target_blank /src/Core/MailTemplates/Handlebars/Welcome.html.hbs: 63 Attack Vector
LOW Unsafe_Use_Of_Target_blank /src/Core/MailTemplates/Handlebars/Welcome.html.hbs: 63 Attack Vector
LOW Unsafe_Use_Of_Target_blank /src/Core/MailTemplates/Handlebars/Welcome.html.hbs: 50 Attack Vector
LOW Unsafe_Use_Of_Target_blank /src/Core/MailTemplates/Handlebars/Welcome.html.hbs: 43 Attack Vector
LOW Unsafe_Use_Of_Target_blank /src/Core/MailTemplates/Handlebars/Welcome.html.hbs: 30 Attack Vector
LOW Unsafe_Use_Of_Target_blank /src/Core/MailTemplates/Handlebars/Welcome.html.hbs: 23 Attack Vector
LOW Unsafe_Use_Of_Target_blank /src/Core/MailTemplates/Handlebars/Welcome.html.hbs: 23 Attack Vector

Fixed Issues

Severity Issue Source File / Package
HIGH Passwords And Secrets - Generic Password /test-database.yml: 92
HIGH Passwords And Secrets - Generic Password /test-database.yml: 170
HIGH Passwords And Secrets - Generic Password /test-database.yml: 111
HIGH Passwords And Secrets - Generic Password /test-database.yml: 105
HIGH Passwords And Secrets - Generic Password /test-database.yml: 80
HIGH Passwords And Secrets - Generic Password /test-database.yml: 177
HIGH Passwords And Secrets - Generic Password /test-database.yml: 69
HIGH Passwords And Secrets - Generic Password /test-database.yml: 173
MEDIUM CSRF /src/Api/Controllers/CollectionsController.cs: 143
MEDIUM CSRF /src/Api/Controllers/CollectionsController.cs: 171
MEDIUM CSRF /src/Api/Public/Controllers/CollectionsController.cs: 87
MEDIUM CSRF /src/Api/Vault/Controllers/SyncController.cs: 59
MEDIUM CSRF /src/Api/SecretsManager/Controllers/CountsController.cs: 37
MEDIUM CSRF /src/Api/Auth/Controllers/TwoFactorController.cs: 118
MEDIUM CSRF /src/Api/Controllers/CollectionsController.cs: 171
MEDIUM CSRF /src/Api/Controllers/CollectionsController.cs: 143
MEDIUM CSRF /src/Api/Public/Controllers/CollectionsController.cs: 87
MEDIUM CSRF /src/Api/SecretsManager/Controllers/CountsController.cs: 37
MEDIUM CSRF /src/Api/Vault/Controllers/SyncController.cs: 59
MEDIUM Privacy_Violation /src/Api/Vault/Models/Request/CipherRequestModel.cs: 198
MEDIUM Privacy_Violation /src/Core/Services/Implementations/UserService.cs: 851
MEDIUM Privacy_Violation /src/Core/Auth/Services/Implementations/AuthRequestService.cs: 160
MEDIUM Privacy_Violation /src/Api/Vault/Models/Request/CipherRequestModel.cs: 198
MEDIUM Unpinned Actions Full Length Commit SHA /build.yml: 515
MEDIUM Unpinned Actions Full Length Commit SHA /build.yml: 598
MEDIUM Unpinned Actions Full Length Commit SHA /build.yml: 548
LOW Heap_Inspection /src/Api/Vault/Models/Request/CipherRequestModel.cs: 165
LOW Heap_Inspection /src/Api/Vault/Models/Request/CipherRequestModel.cs: 165
LOW Log_Forging /src/Api/Auth/Controllers/EmergencyAccessController.cs: 159
LOW Log_Forging /src/Api/Auth/Controllers/TwoFactorController.cs: 118
LOW Log_Forging /src/Api/Auth/Controllers/TwoFactorController.cs: 151
LOW Log_Forging /src/Api/Auth/Controllers/TwoFactorController.cs: 118
LOW Log_Forging /src/Api/Auth/Controllers/TwoFactorController.cs: 151
LOW Log_Forging /src/Api/Auth/Controllers/TwoFactorController.cs: 118
LOW Log_Forging /src/Api/Auth/Controllers/TwoFactorController.cs: 151
LOW Log_Forging /src/Api/Auth/Controllers/TwoFactorController.cs: 151
LOW Log_Forging /src/Api/Auth/Controllers/TwoFactorController.cs: 118
LOW Log_Forging /src/Api/Auth/Controllers/TwoFactorController.cs: 118
LOW Log_Forging /src/Api/Auth/Controllers/TwoFactorController.cs: 151
LOW Log_Forging /src/Api/Auth/Controllers/TwoFactorController.cs: 151
LOW Log_Forging /src/Api/Auth/Controllers/TwoFactorController.cs: 118
LOW Log_Forging /src/Api/AdminConsole/Controllers/ProvidersController.cs: 72
LOW Unsafe_Use_Of_Target_blank /src/Core/MailTemplates/Handlebars/Billing/TrialInitiationVerifyEmail.html.hbs: 17
LOW Unsafe_Use_Of_Target_blank /src/Core/MailTemplates/Handlebars/Billing/TrialInitiationVerifyEmail.html.hbs: 17

@rawkode
Copy link

rawkode commented Aug 10, 2024

Very excited by this! Preparing to ditch 1Password as I type 🙂

@quexten
Copy link
Contributor Author

quexten commented Aug 16, 2024

Setting this to ready for review, but it will not be merged until all ssh of the ssh key features are ready.

@quexten quexten marked this pull request as ready for review August 16, 2024 12:25
@quexten quexten requested a review from a team as a code owner August 16, 2024 12:25
@quexten quexten removed the hold Hold this PR or item until later; DO NOT MERGE label Aug 21, 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