Skip to content

[PM-39979] Add full API support for Item-type Sends - #8192

Merged
mcamirault merged 3 commits into
mainfrom
tools/pm-39979/item-type-send-full-support
Aug 20, 2026
Merged

[PM-39979] Add full API support for Item-type Sends#8192
mcamirault merged 3 commits into
mainfrom
tools/pm-39979/item-type-send-full-support

Conversation

@mcamirault

Copy link
Copy Markdown
Contributor

🎟️ Tracking

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

📔 Objective

This PR adds the full backend logic necessary to create and edit Item-type Sends.

@mcamirault
mcamirault requested a review from a team as a code owner August 12, 2026 04:50
@github-actions

github-actions Bot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

🤖 Bitwarden Claude Code Review

Overall Assessment: REQUEST CHANGES

Reviewed the full API support for Item-type Sends: the structured SendDataModel/SendItemData payload replacing the raw string blob, the pm-34203-temporary-item-sharing flag gating on Get/GetAll/AccessUsingAuth/Post/Put, the new premium and email-verification requirements, and the accompanying unit tests. The response-model changes correctly keep owner-only Notes out of SendAccessResponseModel, and an empty Emails list resolves to EmailOtp([]), which denies access. Three findings concern the enforcement of the new email-verification invariant, gating on client-supplied Type during edits, and back-compat for Item Sends stored in the previous format.

Code Review Details
  • ❌ : Item Sends created without an explicit AuthType end up with Emails == null and AuthType.Email, which SendAuthenticationQuery resolves to NotAuthenticated — the grant validator auto-issues an access token, so the link works with no email OTP while the API reports email verification. PUT /sends/{id}/remove-auth bypasses the same invariant.
    • src/Api/Tools/Utilities/InferAuthType.cs:10
  • ⚠️ : Put evaluates the flag, premium, and email-verification guards against the client-supplied model.Type, while UpdateSend branches on the stored existingSend.Type, letting a client edit an Item Send by declaring Type: Text.
    • src/Api/Tools/Controllers/SendsController.cs:361
  • ⚠️ : GetAll filters after projecting, so Item Send data is deserialized even when the flag is off; Item Sends persisted in the previous raw-string format now throw JsonException and fail the entire list with no migration path.
    • src/Api/Tools/Controllers/SendsController.cs:136
  • ❓ : SyncResponseModel builds SendResponseModel for every Send with no flag filter, so Item Sends still reach clients through /sync when TemporaryItemSharing is off — was leaving sync ungated intentional?
    • src/Api/Vault/Models/Response/SyncResponseModel.cs:68

Comment thread src/Api/Tools/Utilities/InferAuthType.cs
Comment thread src/Api/Tools/Controllers/SendsController.cs
Comment thread src/Api/Tools/Controllers/SendsController.cs Outdated
@codecov

codecov Bot commented Aug 13, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 66.19718% with 24 lines in your changes missing coverage. Please review.
✅ Project coverage is 63.27%. Comparing base (4f8c0f0) to head (3265d7c).
⚠️ Report is 68 commits behind head on main.

Files with missing lines Patch % Lines
src/Api/Tools/Controllers/SendsController.cs 77.77% 4 Missing and 2 partials ⚠️
src/Api/Tools/Models/Response/SendResponseModel.cs 44.44% 4 Missing and 1 partial ⚠️
src/Api/Tools/Models/SendDataModel.cs 37.50% 5 Missing ⚠️
src/Api/Tools/Models/Request/SendRequestModel.cs 55.55% 2 Missing and 2 partials ⚠️
...i/Tools/Models/Response/SendAccessResponseModel.cs 42.85% 3 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8192      +/-   ##
==========================================
+ Coverage   63.04%   63.27%   +0.23%     
==========================================
  Files        2315     2385      +70     
  Lines      100510   103978    +3468     
  Branches     9043     9418     +375     
==========================================
+ Hits        63364    65792    +2428     
- Misses      34957    35932     +975     
- Partials     2189     2254      +65     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

harr1424
harr1424 previously approved these changes Aug 14, 2026
Comment thread src/Api/Tools/Controllers/SendsController.cs
Comment on lines +382 to +385
if (send.Type != model.Type)
{
throw new BadRequestException("Cannot change a Send's type");
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This was already enforced by the UI and CLI, and enforcing it here allows the UpdateSend logic in SendRequestModel.cs to work

@mcamirault
mcamirault requested a review from harr1424 August 17, 2026 17:18
@mcamirault mcamirault added the t:feature Change Type - Feature Development label Aug 20, 2026
@mcamirault
mcamirault merged commit 3968418 into main Aug 20, 2026
61 of 62 checks passed
@mcamirault
mcamirault deleted the tools/pm-39979/item-type-send-full-support branch August 20, 2026 12:43
mcamirault added a commit to bitwarden/sdk-internal that referenced this pull request Aug 26, 2026
Updates the API bindings to `fc5ea9ab05bf7a1679163fc13ec351ff96363000`

The main purpose of this update is to accommodate for the server changes
introduced in [this PR](bitwarden/server#8192),
but there were a couple changes that had to happen for the SDK to build,
hence why @bitwarden/team-key-management-dev got pulled in.

The associated clients PR is [this
one](bitwarden/clients#22159), tested against a
local SDK build on this branch.

---------

Co-authored-by: bw-ghapp[bot] <178206702+bw-ghapp[bot]@users.noreply.github.com>
Co-authored-by: Mike Amirault <mamirault@bitwarden.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

t:feature Change Type - Feature Development

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants