feat(auth-validator): [Auth/PM-22975] Client Version Validator
NEEDS TESTING ON FEATURE BRANCH
đī¸ Tracking
https://bitwarden.atlassian.net/browse/PM-22975
đ Objective
The objective of this ticket is to create a client version validator that checks the header data on a token request and prevent users who have migrated to v2 from logging in with older client versions.
- Extracted out KM utilities for checking isV2Encryption from RotateUserAccountKeysCommand
- Added some clarity and comments in BaseRequestValidator
- Installed the validation for client version in the appropriate place to prevent account enumeration
đ¸ Screenshots
https://github.com/user-attachments/assets/cf4bc61a-3ec8-4d28-a1c1-19690b755f6a
https://github.com/user-attachments/assets/36f51ae3-5bb0-4b65-9df3-595048b1cfbf
â° 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
Checkmarx One â Scan Summary & Details â 5a874ebf-7b33-4f45-a5e6-4cafd668cc90
New Issues (2)
Checkmarx found the following issues in this Pull Request
| Severity | Issue | Source File / Package | Checkmarx Insight |
|---|---|---|---|
![]() |
CSRF | /src/Api/Vault/Controllers/CiphersController.cs: 1519 | detailsMethod at line 1519 of /src/Api/Vault/Controllers/CiphersController.cs gets a parameter from a user request from id. This parameter value flows ...ID: dMGF5qNfAN72zlvQcA1MgbhHv%2Fc%3D |
![]() |
CSRF | /src/Api/Vault/Controllers/CiphersController.cs: 1395 | detailsMethod at line 1395 of /src/Api/Vault/Controllers/CiphersController.cs gets a parameter from a user request from id. This parameter value flows ...ID: iOCFr11iI9znjDnv46yLfiS4aDY%3D |
Claude finished @Patrick-Pimentel-Bitwarden's task ââ View job
Code Review Complete â
Tasks
- [x] Gather context and understand PR changes
- [x] Run bitwarden-code-reviewer agent to analyze the code
- [x] Agent posted inline comments and summary
- [x] Finalize review
Review Summary: The bitwarden-code-reviewer agent has completed a comprehensive review of this PR and posted detailed inline comments and a summary directly to the PR.
I am seeing 100 failures in tests, double checking what is going on, consider holding off on approving.
Codecov Report
:x: Patch coverage is 96.87500% with 3 lines in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 57.74%. Comparing base (3de2f98) to head (fc6a513).
| Files with missing lines | Patch % | Lines |
|---|---|---|
| src/Core/Entities/User.cs | 81.25% | 3 Missing :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## main #6588 +/- ##
==========================================
+ Coverage 53.86% 57.74% +3.88%
==========================================
Files 1917 1920 +3
Lines 85126 85199 +73
Branches 7620 7629 +9
==========================================
+ Hits 45853 49200 +3347
+ Misses 37508 34162 -3346
- Partials 1765 1837 +72
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
:rocket: New features to boost your workflow:
- :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
- :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
