[PM-30993] Better Error Message for Expired Invitation Tokens

[sven-bitwarden]

🎟️ Tracking

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

📔 Objective

The server currently sends a nondescript Invalid token. error message when accepting an invalid token. While technically true, we want to be kinder to users accepting expired tokens, giving them a better error message.

Other Considerations

I thought about enhancing the Tokenable API to be return a TokenableValidationResult, which could mirror the existing inheritance hierarchy to return objects that indicate what went wrong. For example, Tokenable returns TokenableValidationResult, ExpiringTokenable returns ExpiringTokenableValidationResult.. each tacking on new metadata.

I opted not to do this, as this is so far the only Tokenable area that is interested in knowing what validation failed, and I didn't want to force the creation of N result classes (one for each Tokenable derivative).

📸 Screenshots

[github-actions]

Checkmarx One – Scan Summary & Details – 2b8b9d9c-0545-48ce-87f9-c1f39172e676

---

New Issues (2)

Checkmarx found the following issues in this Pull Request

#	Severity	Issue	Source File / Package	Checkmarx Insight
1		CSRF	/src/Api/Vault/Controllers/CiphersController.cs: 1592	details Method at line 1592 of /src/Api/Vault/Controllers/CiphersController.cs gets a parameter from a user request from id. This parameter value flows ... Attack Vector
2		CSRF	/src/Api/Vault/Controllers/CiphersController.cs: 1419	details Method at line 1419 of /src/Api/Vault/Controllers/CiphersController.cs gets a parameter from a user request from id. This parameter value flows ... Attack Vector

---

Fixed Issues (1)

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

Severity	Issue	Source File / Package
	CSRF	/src/Api/Vault/Controllers/CiphersController.cs: 293

[codecov]

Codecov Report

❌ Patch coverage is 96.55172% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 57.64%. Comparing base (d977ba4) to head (e3499a4).
⚠️ Report is 8 commits behind head on main.

Files with missing lines	Patch %	Lines
...ls/Business/Tokenables/TokenableValidationError.cs	87.50%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #6971   +/-   ##
=======================================
  Coverage   57.64%   57.64%           
=======================================
  Files        2033     2034    +1     
  Lines       89622    89626    +4     
  Branches     7977     7977           
=======================================
+ Hits        51660    51668    +8     
+ Misses      36106    36104    -2     
+ Partials     1856     1854    -2     

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

[claude]

Claude finished @sven-bitwarden's task in 2m 14s —— View job

---

Code Review: APPROVE

This PR introduces a TokenableValidationError record to distinguish between expired and invalid organization invite tokens, allowing callers to surface more specific error messages to clients. The new ValidateOrgUserInvite method consolidates validation logic into a single switch expression while preserving backward compatibility through existing ValidateOrgUserInviteStringToken overloads. The IsExpired property extracted on ExpiringTokenable cleanly separates expiration checking from the composite Valid property. Test coverage is thorough, including parameterized tests across all validation branches.

• Gather context and read changed files
• Analyze changes for issues
• Post review findings

No issues found. Clean, well-structured PR with good test coverage.

[JaredSnider-Bitwarden]

Nice work!

[sven-bitwarden]

@JaredScar @JaredSnider-Bitwarden sorry y'all, my prior PR #6972 had a conflict here 🙏

[sven-bitwarden]

@JaredScar @JaredSnider-Bitwarden 🙏 Through QA (which is difficult to QA expired tokens) we found an additional area that needs to benefit from the expired token message. This lead me to consolidate the error message creation logic.

@JaredSnider-Bitwarden I wasn't sure of your preference - we could have all of the methods inside of the static class point to the new method and check if it's null or not to determine the presence of an error. I left it as-is otherwise

[JaredSnider-Bitwarden]

Excellent work! Please be sure that the standard org invite and org invite registration flows are re-tested by QA. The tests give great confidence that those will be successful though!

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud