Skip to content

feat(sso): Add preferred algorithms to SP metadata - #8280

Open
enmande wants to merge 2 commits into
mainfrom
auth/pm-42628/advertise-oaep-in-acs
Open

feat(sso): Add preferred algorithms to SP metadata#8280
enmande wants to merge 2 commits into
mainfrom
auth/pm-42628/advertise-oaep-in-acs

Conversation

@enmande

@enmande enmande commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

🎟️ Tracking

PM-42648

📔 Objective

  • Advertises OAEP encryption algorithms on SAML Service Provider (SP) metadata.

📸 Screenshots

N/A

@enmande enmande added needs-qa ai-review Request a Claude code review t:feature Change Type - Feature Development labels Aug 28, 2026
@github-actions

github-actions Bot commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

Reviewed the SAML SP metadata change: a static MetadataCreated notification wired onto Saml2Options that appends the accepted RSA-OAEP key-transport algorithms to every non-signing KeyDescriptor, the new SamlEncryptionAlgorithms constants, and the accompanying unit tests. GetSaml2AuthenticationScheme is the only new Saml2Options site in the repo and no other code subscribes to MetadataCreated, so every SAML scheme picks the handler up exactly once per freshly built options instance — there is no repeated-subscription or method-accumulation path, and the GetSchemeAsync test confirms the delegate survives PostConfigure and the scheme cache. Both advertised algorithms are RSA-OAEP, moving the SP off the PKCS#1 v1.5 default rather than toward a weaker one, and the ordering constraint that puts rsa-oaep-mgf1p first is documented at the constant and pinned by a literal-order test. Empty-descriptor, signing-only-key, and multiple-key cases are all covered, and the change adds no new package references, so the existing packages.lock.json files stay valid.

No findings met the bar for an inline comment.

@codecov

codecov Bot commented Aug 28, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 63.86%. Comparing base (394fcc7) to head (b4e519e).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8280      +/-   ##
==========================================
+ Coverage   63.73%   63.86%   +0.13%     
==========================================
  Files        2470     2471       +1     
  Lines      105843   105858      +15     
  Branches     9580     9583       +3     
==========================================
+ Hits        67461    67609     +148     
+ Misses      36059    35911     -148     
- Partials     2323     2338      +15     

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

@enmande
enmande marked this pull request as ready for review August 28, 2026 19:14
@enmande
enmande requested a review from a team as a code owner August 28, 2026 19:14
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 needs-qa t:feature Change Type - Feature Development

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants