Confusing API: MakeAssertionAsync
glatzert opened this issue · 4 comments
glatzert commented
The function
fido2-net-lib/Src/Fido2/IFido2.cs
Line 16 in 9ad038b
IFido2.MakeAssertionAsync()
returns an instance of VerifyAssertionResult
. This leads to questions and some confusion about how to use it:
- The return type contains an status and an errorMessage, that is used in the samples like here
, but will never be anything else then
{ status = "ok" }
anderrorMessage = null
despite the sample indicating otherwise.MakeAssertionAsync()
will always throw on error, which is okay, but should be clearly communicated. - the
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:
- removing the inheritance of
VerifyAssertionResult
toFido2ResponseBase
since that brings in the two problematic fields. - adding a bool to
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.