Skip to content

PM-31772: Simplify origin for verified sources#6577

Merged
david-livefront merged 1 commit intomainfrom
PM-31772-origin-check
Feb 25, 2026
Merged

PM-31772: Simplify origin for verified sources#6577
david-livefront merged 1 commit intomainfrom
PM-31772-origin-check

Conversation

@david-livefront
Copy link
Collaborator

@david-livefront david-livefront commented Feb 24, 2026

🎟️ Tracking

PM-31772

📔 Objective

This PR simplifies the origin passed to the SDK when the request comes from a verified web source. Currently only sources in the fido2_privileged_google.json file and user approved sources are considered verified.

@david-livefront david-livefront added app:password-manager Bitwarden Password Manager app context ai-review Request a Claude code review t:bug Change Type - Bug labels Feb 24, 2026
@github-actions github-actions bot removed the t:bug Change Type - Bug label Feb 24, 2026
@claude
Copy link
Contributor

claude bot commented Feb 24, 2026

Claude finished @david-livefront's task in 2m 50s —— View job


Re-Review: PM-31772 - Simplify origin for verified sources

  • Gather context and read changed files
  • Perform code review
  • Post review findings

Summary

This PR modifies FIDO2/passkey origin handling so that when a calling application is validated against the Google privileged app allow list (a curated, trusted list), the origin passed to the Bitwarden SDK is simplified to https://<relyingPartyId> instead of using the raw origin from CallingAppInfo.getOrigin(). Non-verified sources (community list, user trust list) continue to use the raw origin.

Changes Reviewed

File Type Description
OriginManagerImpl.kt Logic Threads relyingPartyId and isVerifiedSource through the validation chain
CallingAppInfoExtensions.kt Logic Conditionally simplifies origin for verified sources using prefixHttpsIfNecessary()
OriginManagerTest.kt Test Updates expected result to include https:// prefix for Google list validation
CallingAppInfoExtensionsTest.kt Test Adds test for verified source origin modification; updates existing tests with new parameters

Analysis

Security: The change is well-scoped. Origin simplification is only applied when isVerifiedSource = true, which is exclusively set for the Google allow list (fido2_privileged_google.json). This is a reasonable trust boundary since the Google list is a curated set of verified browser applications. The relyingPartyId used to construct the simplified origin comes from the FIDO2 request itself, which is appropriate for these trusted contexts.

Correctness: The parameter threading from validateOrigin through the private methods to validatePrivilegedApp is clean and consistent. The prefixHttpsIfNecessary() utility correctly handles relying party IDs that may or may not already contain a scheme prefix.

Test Coverage: All modified and coverable lines are covered by tests. Tests cover the key scenarios: verified source returning modified origin, non-verified source returning raw origin, and existing error cases updated with new parameters.

Previous Findings Status

Both findings from the previous review have been addressed:

  1. Test name corrected from "from verified source" to "from non-verified source" for the isVerifiedSource = false test case
  2. Comment terminology corrected from "Relaying Party" to "Relying Party"

Result

No new issues found. The code is clean, well-tested, and follows project conventions.

@github-actions
Copy link
Contributor

github-actions bot commented Feb 24, 2026

Logo
Checkmarx One – Scan Summary & Detailsddc8a86d-562c-404e-adfc-fc5b93474bb1

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

@codecov
Copy link

codecov bot commented Feb 24, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.44%. Comparing base (7b1b519) to head (eb8bc76).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6577      +/-   ##
==========================================
+ Coverage   86.40%   86.44%   +0.04%     
==========================================
  Files         798      786      -12     
  Lines       57018    56784     -234     
  Branches     8258     8257       -1     
==========================================
- Hits        49266    49087     -179     
+ Misses       4887     4832      -55     
  Partials     2865     2865              

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

@david-livefront david-livefront added the t:bug Change Type - Bug label Feb 25, 2026
@github-actions github-actions bot removed the t:bug Change Type - Bug label Feb 25, 2026
@david-livefront david-livefront added the t:bug Change Type - Bug label Feb 25, 2026
@david-livefront david-livefront marked this pull request as ready for review February 25, 2026 16:54
@david-livefront david-livefront requested a review from a team as a code owner February 25, 2026 16:54
@david-livefront
Copy link
Collaborator Author

Thanks @SaintPatrck

@david-livefront david-livefront added this pull request to the merge queue Feb 25, 2026
Merged via the queue into main with commit fd6d32e Feb 25, 2026
18 of 19 checks passed
@david-livefront david-livefront deleted the PM-31772-origin-check branch February 25, 2026 18:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-review Request a Claude code review app:password-manager Bitwarden Password Manager app context t:bug Change Type - Bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants