[PM-41693] Align error behavior when Send is not accessible

[harr1424]

🎟️ Tracking

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

📔 Objective

This PR improves user experience by standardizing the server's response when a Send should not be accessed (a non-exhaustive list is in the section below). These changes come at the expense of guarding against enumeration in the event that a Send had been deliberately deleted; formerly this situation would result in one of three random error messages.

The following behavior was flagged as a defect:

• One of the random error responses indicated the Send required password authentication, this was surfaced to the user as password_hash_b64_required

• A different random error response would result in an infinite OTP auth flow

Internal product teams requested changes to align error behavior whenever a Send should not be accessible (see list below).

📸 Screenshots

The above message will be shown in ALL of the following situations:

• A Send has been deliberately deleted
• A Send has passed it's configured deletion date
• A Send has passed it's configured expiration date
• A Send is disabled
• A Send has exceeded it's configured max access count

[github-actions]

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

This PR collapses the NeverAuthenticate Send authentication method into SendInaccessible, so a missing send_id now returns the same invalid_grant / send_id_invalid response as a disabled, expired, deleted, or max-access-exceeded Send. The removal remains complete and consistent: no references to NeverAuthenticate, SendAccessConstants.EnumerationProtection, or GlobalSettings.SendDefaultHashKey survive anywhere in src/, test/, util/, or deployment templates; the DI registration was dropped and the SendAccess readme updated. EnumerationProtectionHelpers is correctly left in place since AccountsController still uses it, SendConstantsSnapshotTests is unaffected because no SDK-shared constant values changed, and dropping the SendDefaultHashKey property is safe for self-hosted instances that still set it since nothing in the repo enables strict configuration binding.

The latest commit adds two end-to-end tests (SendAccessGrant_ExistingAccessibleSend_ReturnsAccessToken and SendAccessGrant_DeletedSend_ReturnsInvalidGrant) that exercise the real SendAuthenticationQuery and SendAccessGrantValidator with only ISendRepository substituted — this is the right seam for the behavior being changed, and it directly covers the null-Send path that previously routed through the deleted validator. The (Send?)null cast is fine given test projects build with Nullable=annotations, and the singleton repository substitute registered via WithWebHostBuilder correctly takes precedence over the scoped EF registration.

Code Review Details

No new findings.

The previously raised question about the removal of the Send enumeration protection was answered by the author with the internally tracked ticket PM-41693, so it is not reopened here.

[claude]

❓ QUESTION: Was the removal of the Send enumeration protection reviewed with the security team?

Context

This change (plus the deletion of SendNeverAuthenticateRequestValidator and GlobalSettings.SendDefaultHashKey) removes a control that was deliberately added to prevent distinguishing a non-existent send_id from one that exists and requires auth. After this PR, an unknown send_id deterministically returns invalid_grant / send_id_invalid, while an existing, accessible, password- or OTP-protected Send returns password_hash_b64_required / email_required — so the two states become reliably distinguishable.

The PR description acknowledges the trade-off, and the residual risk looks low given Send IDs are 122 bits of randomness (offline enumeration is infeasible; the practical leak is limited to someone who already holds a link learning whether the Send is still live). Asking mainly so the sign-off is recorded on the PR — was AppSec looped in, given this reverses an explicitly documented anti-enumeration measure?

[harr1424]

See internally tracked ticket

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 63.27%. Comparing base (f5e940c) to head (a764ecc).
⚠️ Report is 13 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #8229      +/-   ##
==========================================
- Coverage   63.30%   63.27%   -0.03%     
==========================================
  Files        2383     2400      +17     
  Lines      103898   103986      +88     
  Branches     9402     9421      +19     
==========================================
+ Hits        65771    65802      +31     
- Misses      35883    35929      +46     
- Partials     2244     2255      +11     

☔ 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.

[ike-kottlowski]

Just the comment about notifying SRE of the globalsettings shape change.

[ike-kottlowski]

⚠️ : We need to communicate to SRE that this value is no longer needed in our upper environs.

We want to make sure we're keeping our settings files lean so we will want to remove this setting.