Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable nullable for Fido2.Models #466

Open
dradovic opened this issue Nov 18, 2023 · 2 comments
Open

Enable nullable for Fido2.Models #466

dradovic opened this issue Nov 18, 2023 · 2 comments

Comments

@dradovic
Copy link

dradovic commented Nov 18, 2023

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
Copy link
Sponsor Contributor

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
Copy link
Author

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.
`

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

No branches or pull requests

2 participants