-
-
Notifications
You must be signed in to change notification settings - Fork 157
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
Confusing API: MakeAssertionAsync #518
Comments
I followed the Same goes for the errorMessage - it's always empty. |
If I recall, the reason for this weirdness was because the conformance tests required the models to look like that. Of course, we improve on this. I agree - will look into cleaning this up as part of v4. |
See #529 |
That looks way less confusing :) |
The function
fido2-net-lib/Src/Fido2/IFido2.cs
Line 16 in 9ad038b
IFido2.MakeAssertionAsync()
returns an instance ofVerifyAssertionResult
. This leads to questions and some confusion about how to use it:fido2-net-lib/BlazorWasmDemo/Server/Controllers/UserController.cs
Line 284 in 9ad038b
{ status = "ok" }
anderrorMessage = null
despite the sample indicating otherwise.MakeAssertionAsync()
will always throw on error, which is okay, but should be clearly communicated.VerifyAssertionResult.Status
property is of type string and is neither populated by an enum nor by an constant, so if that property is relevant, the caller needs to read the code to understand the possible values.I propose:
VerifyAssertionResult
toFido2ResponseBase
since that brings in the two problematic fields.VerifyAssertionResult
indicating successful verification and thus remove all throws and replace it with returnVerifiyAssertionResult.Error(string reason)
setting that bool to false (or an enum indicating success and failure to make it clearly distinguishable).If you'd accept the Idea, I'd implement it and provide a PR.
The text was updated successfully, but these errors were encountered: