server icon indicating copy to clipboard operation
server copied to clipboard

feat(auth-validator): [Auth/PM-22975] Client Version Validator

Open Patrick-Pimentel-Bitwarden opened this issue 1 month ago â€ĸ 4 comments

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.

  1. Extracted out KM utilities for checking isV2Encryption from RotateUserAccountKeysCommand
  2. Added some clarity and comments in BaseRequestValidator
  3. 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

Logo 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
MEDIUM 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
Attack Vector
MEDIUM 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
Attack Vector
Fixed Issues (1)

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

Severity Issue Source File / Package
MEDIUM ~~CSRF~~ /src/Api/Vault/Controllers/CiphersController.cs: 300

github-actions[bot] avatar Nov 17 '25 20:11 github-actions[bot]

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.


claude[bot] avatar Nov 20 '25 19:11 claude[bot]

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.

codecov[bot] avatar Dec 02 '25 21:12 codecov[bot]