E53E Update System.IdentityModel.Tokens.Jwt and xunit by iamcarbon · Pull Request #431 · passwordless-lib/fido2-net-lib · GitHub
[go: up one dir, main page]

Skip to content

Conversation

iamcarbon
Copy link
Contributor

This PR updates the following dependencies:

  • System.IdentityModel.Tokens.Jwt -> v7.0.2
  • xunit -> 2.5.1.
  • Microsoft.NET.Test.Sdk -> 17.7.2

And reacts to the following breaking changes:

System.IdentityModel.Tokens.Jwt v7.0.2 no longer serializes byte arrays as base64 strings. When generating tokens for testing, we need to base64 encode manually. In the main library, we only validate tokens... so we are OK.

xunit 2.5.1 has an updated analyzer that require async method calls to occur in an async context. The tests have been updated accordingly.

@iamcarbon iamcarbon changed the title Update System.IdentityModel.Tokens.Jwt Update System.IdentityModel.Tokens.Jwt and xunit Oct 4, 2023
@iamcarbon
Copy link
Contributor Author

@abergs Ready for review.

@codecov-commenter
Copy link
codecov-commenter commented Oct 4, 2023

Codecov Report

Attention: 6 lines in your changes are missing coverage. Please review.

Comparison is base (15263ca) 77.50% compared to head (bb367f6) 77.50%.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #431   +/-   ##
=======================================
  Coverage   77.50%   77.50%           
=======================================
  Files          98       98           
  Lines        2716     2716           
  Branches      445      445           
=======================================
  Hits         2105     2105           
  Misses        492      492           
  Partials      119      119           
Files Coverage Δ
...c/Fido2/Metadata/Fido2MetadataServiceRepository.cs 65.51% <0.00%> (ø)

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

@abergs abergs self-requested a review October 5, 2023 09:50
@iamcarbon
Copy link
Contributor Author

If we get this merged in, I'd like to take a look at migrating to Microsoft.IdentityModel.JsonWebTokens as well. This would require that AttestationVerifier.Verify by changed to AttestationVerifier.VerifyAsync.

@aseigler
Copy link
Collaborator

This looks good to me!

@iamcarbon
Copy link
Contributor Author

@abergs Let me know if there's anything left on this one.

Copy link
Contributor
@joegoldman2 joegoldman2 left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@abergs abergs merged commit 934abe4 into passwordless-lib:master Nov 2, 2023
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.

5 participants

0