Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

PM-16170 removing deprecated send file endpoint #5222

Conversation

prograhamming
Copy link
Contributor

🎟️ Tracking

https://bitwarden.atlassian.net/jira/software/c/projects/BW/boards/92?selectedIssue=PM-16170

📔 Objective

Within server there’s a deprecated endpoint for uploading files to a Bitwarden Send (SendsController). This endpoint can be removed. In addition any backwards compatible logic on the client-side (send-api.service) can also be removed.

📸 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

@prograhamming prograhamming requested a review from a team as a code owner January 6, 2025 20:57
Copy link

codecov bot commented Jan 6, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 43.71%. Comparing base (cd7c4bf) to head (64997df).
Report is 8 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5222      +/-   ##
==========================================
+ Coverage   43.70%   43.71%   +0.01%     
==========================================
  Files        1469     1469              
  Lines       67901    67885      -16     
  Branches     6156     6155       -1     
==========================================
  Hits        29673    29673              
+ Misses      36934    36918      -16     
  Partials     1294     1294              

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

Copy link
Contributor

github-actions bot commented Jan 6, 2025

Logo
Checkmarx One – Scan Summary & Details7d79dee4-ce8d-42ea-81d7-fbbcdaa2c748

Fixed Issues

Severity Issue Source File / Package
MEDIUM CSRF /src/Billing/Controllers/StripeController.cs: 164
MEDIUM CSRF /src/Billing/Controllers/RecoveryController.cs: 38
MEDIUM CSRF /src/Api/Tools/Controllers/SendsController.cs: 193
MEDIUM CSRF /src/Admin/AdminConsole/Controllers/OrganizationsController.cs: 372
MEDIUM CSRF /src/Api/Tools/Controllers/SendsController.cs: 193

Copy link
Contributor

@djsmith85 djsmith85 left a comment

Choose a reason for hiding this comment

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

@prograhamming Changes are looking good.

Adding the hold label to prevent merging, until the client-side calls to this deprecated endpoint have been removed

@djsmith85 djsmith85 added the hold Hold this PR or item until later; DO NOT MERGE label Jan 8, 2025
@prograhamming prograhamming removed the hold Hold this PR or item until later; DO NOT MERGE label Jan 13, 2025
@prograhamming prograhamming merged commit 95893bd into main Jan 14, 2025
50 of 52 checks passed
@prograhamming prograhamming deleted the tools/pm-16170/remove-deprecated-send-file-endpoint-from-server branch January 14, 2025 19:17
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants