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-15085] Testing - do not merge #5386

Draft
wants to merge 33 commits into
base: main
Choose a base branch
from
Draft

Conversation

mzieniukbw
Copy link
Contributor

@mzieniukbw mzieniukbw commented Feb 10, 2025

🎟️ Tracking

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

📔 Objective

DO NOT MERGE! For testing purposes only.

📸 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

mzieniukbw and others added 30 commits January 13, 2025 12:18
…ifications

We only register devices for organization push notifications when the organization is being created. This does not work, since we have a use case (Notification Center) of delivering notifications to all users of organization. This fixes it, by adding the organization id tag when device registers for push notifications.
Fixed IFeatureService substitute mocking for Android tests.
Added user part of organization test with organizationId tags expectation.
…e from self-hosted.

Self-hosted instance uses relay to register the mobile device against Bitwarden Cloud Api. Only the self-hosted server knows client's organization membership, which means it needs to pass in the organization id's information to the relay. Similarly, for Bitwarden Cloud, the organizaton id will come directly from the server.
…d by mobile device.

When mobile device registers on self-hosted through the relay, every single id, like user id, device id and now organization id needs to be prefixed with the installation id. This have been missing in the PushController that handles this for organization id.
Device type is now part of JWT access token, so the notification center results in the integration test are now scoped to client type web and all.
Notification Center push notification now includes all the fields.
@mzieniukbw mzieniukbw changed the title km/pm-15084-testing3 [PM-15085] Testing - do not merge Feb 10, 2025
Copy link
Contributor

Logo
Checkmarx One – Scan Summary & Detailsca500b48-0c1d-4e40-a038-0ecfee6b46a7

New Issues (42)

Checkmarx found the following issues in this Pull Request

Severity Issue Source File / Package Checkmarx Insight
MEDIUM CSRF /src/Api/Platform/Push/Controllers/PushController.cs: 51
detailsMethod DeleteAsync at line 51 of /src/Api/Platform/Push/Controllers/PushController.cs gets a parameter from a user request from DeleteAsync. This p...
Attack Vector
MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 1100
detailsMethod DeleteAttachment at line 1100 of /src/Api/Vault/Controllers/CiphersController.cs gets a parameter from a user request from id. This paramete...
Attack Vector
MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 1100
detailsMethod DeleteAttachment at line 1100 of /src/Api/Vault/Controllers/CiphersController.cs gets a parameter from a user request from DeleteAttachment....
Attack Vector
MEDIUM Privacy_Violation /src/Core/Auth/Services/Implementations/AuthRequestService.cs: 218
detailsMethod UpdateAuthRequestAsync at line 218 of /src/Core/Auth/Services/Implementations/AuthRequestService.cs sends user information outside the appli...
Attack Vector
MEDIUM Privacy_Violation /src/Core/Auth/Services/Implementations/AuthRequestService.cs: 209
detailsMethod UpdateAuthRequestAsync at line 209 of /src/Core/Auth/Services/Implementations/AuthRequestService.cs sends user information outside the appli...
Attack Vector
MEDIUM Privacy_Violation /src/Core/NotificationHub/NotificationHubPushNotificationService.cs: 306
detailsMethod PushAuthRequestAsync at line 306 of /src/Core/NotificationHub/NotificationHubPushNotificationService.cs sends user information outside the a...
Attack Vector
MEDIUM Privacy_Violation /src/Core/Auth/Services/Implementations/AuthRequestService.cs: 161
detailsMethod CreateAuthRequestAsync at line 161 of /src/Core/Auth/Services/Implementations/AuthRequestService.cs sends user information outside the appli...
Attack Vector
LOW Log_Forging /src/Api/Billing/Controllers/OrganizationBillingController.cs: 259
detailsMethod UpdateTaxInformationAsync at line 259 of /src/Api/Billing/Controllers/OrganizationBillingController.cs gets user input from element requestB...
Attack Vector
LOW Log_Forging /src/Api/Billing/Controllers/OrganizationBillingController.cs: 259
detailsMethod UpdateTaxInformationAsync at line 259 of /src/Api/Billing/Controllers/OrganizationBillingController.cs gets user input from element requestB...
Attack Vector
LOW Log_Forging /src/Api/Billing/Controllers/OrganizationBillingController.cs: 259
detailsMethod UpdateTaxInformationAsync at line 259 of /src/Api/Billing/Controllers/OrganizationBillingController.cs gets user input from element requestB...
Attack Vector
LOW Log_Forging /src/Api/Auth/Controllers/AccountsController.cs: 713
detailsMethod PostPayment at line 713 of /src/Api/Auth/Controllers/AccountsController.cs gets user input from element model. This element’s value flows th...
Attack Vector
LOW Log_Forging /src/Api/Auth/Controllers/AccountsController.cs: 713
detailsMethod PostPayment at line 713 of /src/Api/Auth/Controllers/AccountsController.cs gets user input from element model. This element’s value flows th...
Attack Vector
LOW Log_Forging /src/Api/Auth/Controllers/AccountsController.cs: 639
detailsMethod PostPremium at line 639 of /src/Api/Auth/Controllers/AccountsController.cs gets user input from element model. This element’s value flows th...
Attack Vector
LOW Log_Forging /src/Api/Auth/Controllers/AccountsController.cs: 639
detailsMethod PostPremium at line 639 of /src/Api/Auth/Controllers/AccountsController.cs gets user input from element model. This element’s value flows th...
Attack Vector
LOW Log_Forging /src/Api/Auth/Controllers/AccountsController.cs: 713
detailsMethod PostPayment at line 713 of /src/Api/Auth/Controllers/AccountsController.cs gets user input from element model. This element’s value flows th...
Attack Vector
LOW Log_Forging /src/Api/Billing/Controllers/OrganizationBillingController.cs: 169
detailsMethod UpdatePaymentMethodAsync at line 169 of /src/Api/Billing/Controllers/OrganizationBillingController.cs gets user input from element requestBo...
Attack Vector
LOW Log_Forging /src/Api/Auth/Controllers/AccountsController.cs: 639
detailsMethod PostPremium at line 639 of /src/Api/Auth/Controllers/AccountsController.cs gets user input from element model. This element’s value flows th...
Attack Vector
LOW Log_Forging /src/Api/Billing/Controllers/ProviderBillingController.cs: 104
detailsMethod UpdateTaxInformationAsync at line 104 of /src/Api/Billing/Controllers/ProviderBillingController.cs gets user input from element requestBody....
Attack Vector
LOW Log_Forging /src/Api/Billing/Controllers/ProviderBillingController.cs: 104
detailsMethod UpdateTaxInformationAsync at line 104 of /src/Api/Billing/Controllers/ProviderBillingController.cs gets user input from element requestBody....
Attack Vector
LOW Log_Forging /src/Api/Billing/Controllers/OrganizationBillingController.cs: 169
detailsMethod UpdatePaymentMethodAsync at line 169 of /src/Api/Billing/Controllers/OrganizationBillingController.cs gets user input from element requestBo...
Attack Vector
LOW Log_Forging /src/Api/Billing/Controllers/ProviderBillingController.cs: 104
detailsMethod UpdateTaxInformationAsync at line 104 of /src/Api/Billing/Controllers/ProviderBillingController.cs gets user input from element requestBody....
Attack Vector
LOW Log_Forging /src/Api/Billing/Controllers/OrganizationBillingController.cs: 169
detailsMethod UpdatePaymentMethodAsync at line 169 of /src/Api/Billing/Controllers/OrganizationBillingController.cs gets user input from element requestBo...
Attack Vector
LOW Log_Forging /src/Api/Platform/Push/Controllers/PushController.cs: 76
detailsMethod SendAsync at line 76 of /src/Api/Platform/Push/Controllers/PushController.cs gets user input from element model. This element’s value flows ...
Attack Vector
LOW Log_Forging /src/Api/AdminConsole/Controllers/OrganizationsController.cs: 185
detailsMethod CreateWithoutPaymentAsync at line 185 of /src/Api/AdminConsole/Controllers/OrganizationsController.cs gets user input from element model. Th...
Attack Vector
LOW Log_Forging /src/Api/AdminConsole/Controllers/OrganizationsController.cs: 170
detailsMethod Post at line 170 of /src/Api/AdminConsole/Controllers/OrganizationsController.cs gets user input from element model. This element’s value fl...
Attack Vector
LOW Log_Forging /src/Api/Billing/Controllers/OrganizationBillingController.cs: 169
detailsMethod UpdatePaymentMethodAsync at line 169 of /src/Api/Billing/Controllers/OrganizationBillingController.cs gets user input from element requestBo...
Attack Vector
LOW Log_Forging /src/Api/Vault/Controllers/CiphersController.cs: 173
detailsMethod PostAdmin at line 173 of /src/Api/Vault/Controllers/CiphersController.cs gets user input from element model. This element’s value flows thro...
Attack Vector
LOW Log_Forging /src/Api/Vault/Controllers/CiphersController.cs: 158
detailsMethod PostCreate at line 158 of /src/Api/Vault/Controllers/CiphersController.cs gets user input from element model. This element’s value flows thr...
Attack Vector
LOW Unsafe_Use_Of_Target_blank /src/Core/MailTemplates/Handlebars/Layouts/FullUpdated.html.hbs: 186
detailsUsing at line 186 of /src/Core/MailTemplates/Handlebars/Layouts/FullUpdated.html.hbs...
Attack Vector
LOW Unsafe_Use_Of_Target_blank /src/Core/MailTemplates/Handlebars/Layouts/FullUpdated.html.hbs: 185
detailsUsing at line 185 of /src/Core/MailTemplates/Handlebars/Layouts/FullUpdated...
Attack Vector
LOW Unsafe_Use_Of_Target_blank /src/Core/MailTemplates/Handlebars/Layouts/FullUpdated.html.hbs: 184
detailsUsing at line 184 of /src/Core/MailTemplates/Handlebars/Layouts...
Attack Vector
LOW Unsafe_Use_Of_Target_blank /src/Core/MailTemplates/Handlebars/Layouts/FullUpdated.html.hbs: 183
detailsUsing at line 183 of /src/Core/MailTemplates/Handlebars/Layouts/FullUpdated.html.hbs, witho...
Attack Vector
LOW Unsafe_Use_Of_Target_blank /src/Core/MailTemplates/Handlebars/Layouts/FullUpdated.html.hbs: 182
detailsUsing at line 182 of /src/Core/MailTemplates/Handlebars/Layouts/FullUpdated.html.hbs, w...
Attack Vector
LOW Unsafe_Use_Of_Target_blank /src/Core/MailTemplates/Handlebars/Layouts/FullUpdated.html.hbs: 181
detailsUsing at line 181 of /src/Core/MailTemplates/Handlebars/Layouts/FullUpdated.html.hbs...
Attack Vector
LOW Unsafe_Use_Of_Target_blank /src/Core/MailTemplates/Handlebars/Layouts/FullUpdated.html.hbs: 180
detailsUsing at line 180 of /src/Core/MailTemplates/Handlebars/Layouts/FullUpdated.html.hbs, with...
Attack Vector
LOW Unsafe_Use_Of_Target_blank /src/Core/MailTemplates/Handlebars/Layouts/Full.html.hbs: 157
detailsUsing at line 157 of /src/Core/MailTemplates/Handlebars/Layouts/Full.html.hbs, witho...
Attack Vector
LOW Unsafe_Use_Of_Target_blank /src/Core/MailTemplates/Handlebars/Layouts/Full.html.hbs: 156
detailsUsing at line 156 of /src/Core/MailTemplates/Handlebars/Layouts/Full.html.h...
Attack Vector
LOW Unsafe_Use_Of_Target_blank /src/Core/MailTemplates/Handlebars/Layouts/Full.html.hbs: 155
detailsUsing at line 155 of /src/Core/MailTemplates/Handlebars/Layouts...
Attack Vector
LOW Unsafe_Use_Of_Target_blank /src/Core/MailTemplates/Handlebars/Layouts/Full.html.hbs: 154
detailsUsing at line 154 of /src/Core/MailTemplates/Handlebars/Layouts/Full.html.hbs, without corr...
Attack Vector
LOW Unsafe_Use_Of_Target_blank /src/Core/MailTemplates/Handlebars/Layouts/Full.html.hbs: 153
detailsUsing at line 153 of /src/Core/MailTemplates/Handlebars/Layouts/Full.html.hbs, without ...
Attack Vector
LOW Unsafe_Use_Of_Target_blank /src/Core/MailTemplates/Handlebars/Layouts/Full.html.hbs: 152
detailsUsing at line 152 of /src/Core/MailTemplates/Handlebars/Layouts/Full.html.hbs, witho...
Attack Vector
LOW Unsafe_Use_Of_Target_blank /src/Core/MailTemplates/Handlebars/Layouts/Full.html.hbs: 151
detailsUsing at line 151 of /src/Core/MailTemplates/Handlebars/Layouts/Full.html.hbs, without cor...
Attack Vector
Fixed Issues (50)

Great job! The following issues were fixed in this Pull Request

Severity Issue Source File / Package
MEDIUM CSRF /src/Api/Tools/Controllers/OrganizationExportController.cs: 53
MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 1153
MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 1153
MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 1153
MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 1050
MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 1099
MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 1076
MEDIUM CSRF /src/Identity/Controllers/AccountsController.cs: 119
MEDIUM CSRF /src/Api/Platform/Push/Controllers/PushController.cs: 42
MEDIUM Privacy_Violation /src/Core/Auth/Services/Implementations/AuthRequestService.cs: 221
MEDIUM Privacy_Violation /src/Core/Auth/Services/Implementations/AuthRequestService.cs: 173
MEDIUM Privacy_Violation /src/Core/Auth/Services/Implementations/AuthRequestService.cs: 230
MEDIUM Privacy_Violation /src/Core/NotificationHub/NotificationHubPushNotificationService.cs: 192
MEDIUM SSRF /src/Billing/Controllers/FreshdeskController.cs: 149
MEDIUM SSRF /src/Billing/Controllers/FreshdeskController.cs: 149
LOW Heap_Inspection /src/Core/Services/Implementations/RabbitMqEventListenerService.cs: 29
LOW Heap_Inspection /src/Core/AdminConsole/Services/Implementations/RabbitMqEventWriteService.cs: 19
LOW Heap_Inspection /src/Core/Settings/GlobalSettings.cs: 269
LOW Heap_Inspection /src/Core/Constants.cs: 169
LOW Heap_Inspection /src/Core/Constants.cs: 170
LOW Log_Forging /src/Billing/Controllers/FreshdeskController.cs: 149
LOW Log_Forging /src/Billing/Controllers/FreshdeskController.cs: 149
LOW Log_Forging /src/Billing/Controllers/FreshdeskController.cs: 149
LOW Log_Forging /src/Billing/Controllers/StripeController.cs: 164
LOW Log_Forging /src/Billing/Controllers/RecoveryController.cs: 38
LOW Log_Forging /src/Billing/Controllers/StripeController.cs: 164
LOW Log_Forging /src/Billing/Controllers/RecoveryController.cs: 38
LOW Log_Forging /src/Billing/Controllers/StripeController.cs: 164
LOW Log_Forging /src/Billing/Controllers/RecoveryController.cs: 38
LOW Log_Forging /src/Billing/Controllers/RecoveryController.cs: 38
LOW Log_Forging /src/Billing/Controllers/StripeController.cs: 164
LOW Log_Forging /src/Api/Vault/Controllers/CiphersController.cs: 173
LOW Log_Forging /src/Api/Vault/Controllers/CiphersController.cs: 158
LOW Log_Forging /src/Api/Platform/Push/Controllers/PushController.cs: 75
LOW Unsafe_Use_Of_Target_blank /src/Core/MailTemplates/Handlebars/Layouts/FullUpdated.html.hbs: 186
LOW Unsafe_Use_Of_Target_blank /src/Core/MailTemplates/Handlebars/Layouts/FullUpdated.html.hbs: 185
LOW Unsafe_Use_Of_Target_blank /src/Core/MailTemplates/Handlebars/Layouts/FullUpdated.html.hbs: 184
LOW Unsafe_Use_Of_Target_blank /src/Core/MailTemplates/Handlebars/Layouts/FullUpdated.html.hbs: 183
LOW Unsafe_Use_Of_Target_blank /src/Core/MailTemplates/Handlebars/Layouts/FullUpdated.html.hbs: 182
LOW Unsafe_Use_Of_Target_blank /src/Core/MailTemplates/Handlebars/Layouts/FullUpdated.html.hbs: 181
LOW Unsafe_Use_Of_Target_blank /src/Core/MailTemplates/Handlebars/Layouts/FullUpdated.html.hbs: 180
LOW Unsafe_Use_Of_Target_blank /src/Core/MailTemplates/Handlebars/Layouts/Full.html.hbs: 157
LOW Unsafe_Use_Of_Target_blank /src/Core/MailTemplates/Handlebars/Layouts/Full.html.hbs: 156
LOW Unsafe_Use_Of_Target_blank /src/Core/MailTemplates/Handlebars/Layouts/Full.html.hbs: 155
LOW Unsafe_Use_Of_Target_blank /src/Core/MailTemplates/Handlebars/Layouts/Full.html.hbs: 154
LOW Unsafe_Use_Of_Target_blank /src/Core/MailTemplates/Handlebars/Layouts/Full.html.hbs: 153
LOW Unsafe_Use_Of_Target_blank /src/Core/MailTemplates/Handlebars/Layouts/Full.html.hbs: 152
LOW Unsafe_Use_Of_Target_blank /src/Core/MailTemplates/Handlebars/Layouts/Full.html.hbs: 151
LOW Use_Of_Hardcoded_Password /src/Core/Constants.cs: 170
LOW Use_Of_Hardcoded_Password /src/Core/Constants.cs: 169

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.

2 participants