Skip to content

[PM-31787] Users can access the sends after the limit was reached#6958

Merged
harr1424 merged 7 commits intomainfrom
PM-31787-Defect-Users-can-access-the-sends-after-the-limit-was-reached
Feb 10, 2026
Merged

[PM-31787] Users can access the sends after the limit was reached#6958
harr1424 merged 7 commits intomainfrom
PM-31787-Defect-Users-can-access-the-sends-after-the-limit-was-reached

Conversation

@harr1424
Copy link
Contributor

@harr1424 harr1424 commented Feb 6, 2026

🎟️ Tracking

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

📔 Objective

The new non-anonymous send access endpoints introduced flawed handling of Send access count enforcement. This PR restores parity between how Send access count is handled by these new endpoints with the prior anonymous endpoints:

  • file type sends have access count incremented only at file download
  • text type sends are incremented at each access

Additional logic was added to short circuit the Send access request if any of the following are true for a given send:

  • the send has exceeded it's configured max access count
  • the send has reached deletion date
  • the send has reached expiration date
  • the send is disabled

@github-actions
Copy link
Contributor

github-actions bot commented Feb 6, 2026

Logo
Checkmarx One – Scan Summary & Detailsd77484bd-fc79-4036-ad50-066f1ccc73f4

Great job! No new security vulnerabilities introduced in this pull request

@harr1424 harr1424 added the ai-review Request a Claude code review label Feb 6, 2026
@claude
Copy link
Contributor

claude bot commented Feb 6, 2026

Claude finished @harr1424's task in 1m 18s —— View job


Code Review: PM-31787 - Users can access the sends after the limit was reached

Summary

This PR fixes a bug where authenticated users could access Sends after the access limit was reached. The fix correctly differentiates access counting behavior between Text and File Sends:

  • Text Sends: AccessCount incremented on every view
  • File Sends: AccessCount incremented only when the file is downloaded

Status: Approved

All previous findings have been addressed:

Finding Status
Missing unit tests for new access control logic Resolved - Comprehensive tests added in commit 4690f45
Code duplication with SendAuthorizationService.SendCanBeAccessed Acknowledged - Developer confirmed duplication is temporary; Anonymous endpoints will be removed

What Looks Good

  • The fix correctly addresses the access control bug
  • Clear code comments explaining the different behavior for Text vs File Sends
  • Comprehensive test coverage added for GetSendFileDownloadUrlAsync, SendCanBeAccessed, and controller endpoints
  • Tests cover all denial scenarios: disabled, max access reached, expired, deletion date passed

🤖 Generated with Claude Code

@harr1424 harr1424 removed the ai-review Request a Claude code review label Feb 6, 2026
@harr1424 harr1424 marked this pull request as ready for review February 6, 2026 21:57
@harr1424 harr1424 requested a review from a team as a code owner February 6, 2026 21:57
@codecov
Copy link

codecov bot commented Feb 6, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 56.25%. Comparing base (67ba9bc) to head (25947af).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6958      +/-   ##
==========================================
- Coverage   60.27%   56.25%   -4.02%     
==========================================
  Files        1982     1983       +1     
  Lines       87653    87692      +39     
  Branches     7816     7825       +9     
==========================================
- Hits        52830    49331    -3499     
- Misses      32904    36531    +3627     
+ Partials     1919     1830      -89     

☔ View full report in Codecov by Sentry.
📢 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.

mcamirault
mcamirault previously approved these changes Feb 9, 2026
Copy link
Contributor

@mcamirault mcamirault left a comment

Choose a reason for hiding this comment

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

One tiny suggested cleanup but otherwise looks good!

…mit-was-reached' of github.com:bitwarden/server into PM-31787-Defect-Users-can-access-the-sends-after-the-limit-was-reached

merge latest from main
@sonarqubecloud
Copy link

sonarqubecloud bot commented Feb 9, 2026

@harr1424 harr1424 requested a review from mcamirault February 10, 2026 00:06
@harr1424 harr1424 merged commit bc94934 into main Feb 10, 2026
41 of 43 checks passed
@harr1424 harr1424 deleted the PM-31787-Defect-Users-can-access-the-sends-after-the-limit-was-reached branch February 10, 2026 14:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants