Skip to content

[PM-6125] Handle kp.alg is string#10285

Merged
coroiu merged 5 commits intomainfrom
PM-6125-CONTRIB-handle-kp-alg-is-string
Aug 19, 2024
Merged

[PM-6125] Handle kp.alg is string#10285
coroiu merged 5 commits intomainfrom
PM-6125-CONTRIB-handle-kp-alg-is-string

Conversation

@coroiu
Copy link
Contributor

@coroiu coroiu commented Jul 26, 2024

🎟️ Tracking

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

📔 Objective

Converts kp.alg to numbers. This PR is based on #7832 but moves the fix to outside the Fido2Client so that we can keep that service strictly spec-compliant. Original commit is still present in the PR so @AlessioC31 should still be marked as contributor

📸 Screenshots

⏰ 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

@coroiu coroiu requested a review from a team as a code owner July 26, 2024 11:33
@coroiu coroiu requested a review from shane-melton July 26, 2024 11:33
@coroiu coroiu linked an issue Jul 26, 2024 that may be closed by this pull request
1 task
@codecov
Copy link

codecov bot commented Jul 26, 2024

Codecov Report

Attention: Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.

Project coverage is 32.38%. Comparing base (a64a676) to head (0f9f46e).
Report is 59 commits behind head on main.

Files Patch % Lines
...browser/src/autofill/fido2/utils/webauthn-utils.ts 0.00% 2 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main   #10285   +/-   ##
=======================================
  Coverage   32.37%   32.38%           
=======================================
  Files        2655     2655           
  Lines       80913    80914    +1     
  Branches    15253    15253           
=======================================
+ Hits        26197    26204    +7     
+ Misses      52699    52693    -6     
  Partials     2017     2017           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@github-actions
Copy link
Contributor

github-actions bot commented Jul 26, 2024

Logo
Checkmarx One – Scan Summary & Detailsae34433d-0569-4bed-92d2-4756fa2dd85c

No New Or Fixed Issues Found

alg: Number(params.alg),
type: params.type,
}))
.filter((params) => isNaN(params.alg)),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ I believe we want .filter((params) => !isNaN(params.alg)), to exclude any NaN values

shane-melton
shane-melton previously approved these changes Jul 30, 2024
@coroiu coroiu merged commit 71413a7 into main Aug 19, 2024
@coroiu coroiu deleted the PM-6125-CONTRIB-handle-kp-alg-is-string branch August 19, 2024 08:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

"No supported key algorithms were found

6 participants