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 168 forks source link

Enable nullable for Fido2.Models #466

Open dradovic opened 11 months ago

dradovic commented 11 months ago

I would be really helpful to annotate the nullability of the Fido2.Models types. In other words, to set <Nullable>enable</Nullable> in the Fido2.Models project.

This way, it would become clear for FIDO2 newbies to understand which information is optional and which one is required.

For example, in the demo, you have a line of code that stores a credential coming from MakeNewCredentialAsync:

DevicePublicKeys = new List<byte[]>() { success.Result.DevicePublicKey }

If both Fido2.Models and Demo were NRT enabled, this would pop up as a compilation error. Righteously so, because I don't think it make sense to store a list containing a null along the credential. So in this example, it would really help if RegisteredPublicKeyCredential.DevicePubliKey was declared as byte[]?. And I assume there's plenty more.

iamcarbon commented 10 months ago

We can't use required members yet (the net6.0 system.text.json source generator doesn't support them) -- so there's no clean way to annotate non-null members in the models library right now.

We should, however, continue to annotate nullable members inside of #nullable enable blocks. If we drop net6.0 when it goes out of support, it should be trivial to annotate the remaining members.

dradovic commented 10 months ago

Yes. I guess the first step should be to have <Nullable>enable</Nullable> in the Fido2.Models project (just as you already have it in the Fido2 project). This issue is about that.

And then in a later step, the require keyword can be used to make sure that props are correctly initialized. `

abergs commented 3 days ago

@iamcarbon As November 12th is coming up, marking the end of LTS .net6 - is there anything we should do here?