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.

Sign up for GitHub

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

Jump to bottom

Enable nullable for Fido2.Models #466

Open
dradovic opened this issue Nov 18, 2023 · 3 comments · May be fixed by #581
Open

Enable nullable for Fido2.Models #466

dradovic opened this issue Nov 18, 2023 · 3 comments · May be fixed by #581

Comments

@dradovic
Copy link
Contributor

dradovic commented Nov 18, 2023 •
edited
Loading

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

@abergs
Copy link
Collaborator

abergs commented Nov 5, 2024

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

@iamcarbon iamcarbon linked a pull request Nov 22, 2024 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[Models] Enable nullable (1 of x) carbon/fido2-net-lib
3 participants
@iamcarbon @dradovic @abergs