search

passwordless-lib / fido2-net-lib

FIDO2 .NET library for FIDO2 / WebAuthn Attestation and Assertion using .NET
https://fido2-net-lib.passwordless.dev/
MIT License
1.18k stars 170 forks source link

`FidoEnumConverter`: fall back to deserializing by name even if `EnumMemberAttribute` is set #541

Closed Tyrrrz closed 2 weeks ago

Tyrrrz commented 2 months ago

Currently, FidoEnumConverter works in the following way:

  1. If an enum value is annotated by EnumMemberAttribute, then the provided custom name is used both for serialization and deserialization.
  2. If the attribute is absent, then the default name (i.e. field name) is used.

This PR adds another fallback layer:

  1. If an enum value is annotated by EnumMemberAttribute, but the default name is provided (not custom) then the deserialization would still succeed. Serialization will work the same regardless.

In other words, it would correctly resolve PublicKeyCredentialType.PublicKey from both public-key, as well as PublicKey.

This makes it a bit easier for applications to use Fido2 types directly in their payloads without building their own converters, and without relying on the intricacies of Fido2 naming conventions.

codecov-commenter commented 2 months ago

:warning: Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

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

Project coverage is 73.97%. Comparing base (cb71a15) to head (0db7b64).

Files Patch % Lines
Src/Fido2.Models/Converters/FidoEnumConverter.cs 81.81% 1 Missing and 1 partial :warning:

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

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #541 +/- ## ========================================== + Coverage 73.95% 73.97% +0.01% ========================================== Files 98 98 Lines 2638 2640 +2 Branches 446 447 +1 ========================================== + Hits 1951 1953 +2 Misses 586 586 Partials 101 101 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

iamcarbon commented 2 months ago

I had previously removed the relaxed parsing behaviors from this type, and believe they should remain strict. As a security critical library, we should continue to prioritize conformance with the specification over convenience.