8000 Strongly type AaGuid by iamcarbon · Pull Request #362 · passwordless-lib/fido2-net-lib · GitHub
[go: up one dir, main page]

Skip to content

Conversation

iamcarbon
Copy link
Contributor
@iamcarbon iamcarbon commented Jan 16, 2023

This PR updates AaGuid fields from a string to a Guid. This eliminates some allocations and the need to parse the Guid later.

Making as a draft until mds3.certinfra.fidoalliance.org goes back online and tests are green.

public string CredType { get; set; }
public System.Guid Aaguid { get; set; }

public Guid AaGuid { get; set; }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Breaking change: updated casing for consistency.

@iamcarbon
Copy link
Contributor Author

mds3.certinfra.fidoalliance.org is currently offline -- causing the tests to fail.

@iamcarbon iamcarbon marked this pull request as draft January 16, 2023 03:12
@abergs abergs added enhancement Enhancements or general improvements breaking change Indicate that something is a breaking change labels Jan 16, 2023
@iamcarbon
Copy link
Contributor Author
iamcarbon commented Jan 18, 2023

@abergs @aseigler Ready for review.

Since this PR also contains breaking changes, we may be shaping up for a v4 release with Passkey support.

NOTE: This PR will fail until #363 is merged.

@abergs
Copy link
Collaborator
abergs commented Jan 18, 2023

Agreed, I think v4 should contain the bits for passkey.

@iamcarbon iamcarbon marked this pull request as ready for review January 18, 2023 22:06
@codecov-commenter
Copy link

Codecov Report

Merging #362 (3a199d3) into master (3044795) will decrease coverage by 0.45%.
The diff coverage is 34.37%.

@@            Coverage Diff             @@
##           master     #362      +/-   ##
==========================================
- Coverage   77.25%   76.80%   -0.45%     
==========================================
  Files          89       89              
  Lines        2519     2531      +12     
  Branches      426      427       +1     
==========================================
- Hits         1946     1944       -2     
- Misses        453      466      +13     
- Partials      120      121       +1     
Impacted Files Coverage Δ
Src/Fido2/ConformanceMetadataService.cs 0.00% <0.00%> (ø)
Src/Fido2/Metadata/FileSystemMetadataRepository.cs 0.00% <0.00%> (ø)
Src/Fido2/Extensions/IBufferWriterExtensions.cs 38.09% <29.41%> (-61.91%) ⬇️
...rc/Fido2.AspNet/DistributedCacheMetadataService.cs 83.63% <50.00%> (-1.05%) ⬇️
.../Fido2.Models/Metadata/MetadataBLOBPayloadEntry.cs 100.00% <100.00%> (ø)
Src/Fido2.Models/Metadata/MetadataStatement.cs 100.00% <100.00%> (ø)
...2.Models/Objects/AttestationVerificationSuccess.cs 100.00% <100.00%> (ø)
Src/Fido2/AttestationFormat/AttestationVerifier.cs 85.00% <100.00%> (ø)
Src/Fido2/AuthenticatorAttestationResponse.cs 86.95% <100.00%> (ø)
... and 1 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Copy link
Collaborator
@abergs abergs left a comment

Choose a reason for hiding this comment

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

Reviewed, looks good!

@abergs abergs merged commit 5e72564 into passwordless-lib:master Jan 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change Indicate that something is a breaking change enhancement Enhancements or general improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

4 participants
0