Explain rationale for intermediate WebIDL types, or remove them

[infinisil]

Using these intermediate types allows directly deriving JSON instances, because the field names and types of the WebIDL model line up exactly with the spec. But the alternative of just wrapping the fully-decoded model types with newtype's and writing a manual encoding/decoding also sounds intriguing and would remove some mental overhead. Although it would require duplication in the encoding and decoding parts.

Edit: Another motivation for these intermediate types is that they allow using other encoding/decoding schemas than webauthn-json.

[infinisil]

I took a closer look at this today, and it turns out that the correct encoding/decoding is more complicated than it seems. Here are some notes:

• There's no standard how to do this, though there's plans to have that, see https://github.com/w3c/webauthn/issues/1683.
• Our encoding functions from Crypto.WebAuthn.Model.WebIDL are tied to webauthn-json, which is not obvious from the names.
• The intermediate WebIDL types are mainly there to avoid having to write duplicate encoding/decoding code, the types are really webauthn-json specific.
• We could avoid both the intermediate types and the duplication with a library like aeson-schemas (which I experimented with in the aeson-schemas branch. Unfortunately the library only supports schema-based JSON decoding, it doesn't support encoding, which is what we also need. Otherwise this library looks really promising, maintained and has seen some adoption. It should be possible to extend it with support for this.
• There's another suite library attempting to do something similar which allows both encoding and decoding, hschema. Unfortunately it assumes that every value of a JSON object has to have the same type, and as such is unfit for this (or most other) use cases like this.

Given the above, and some more thought, I think this is the best approach forward:

• Change the existing WebIDL types and functions to be webauthn-json specific, and make sure to clearly point out the version number, maybe even expose the expected version number in code
• In order to allow users to easily use other serializations, expose all functions that decode/encode intermediate structures as declared in the webauthn spec. This means things like attestationObject decoding, the encoding of ResidentialKeyRequirement, etc. Really anything that doesn't depend on webauthn-json.

[lykahb]

The WebIDL module works both with the binary CBOR payload, and the JSON object that contains the payload. In my understanding, the CBOR handling is under Crypto.WebAuthn.Model.WebIDL.Internal.Binary.

Yeah, we discussed before that it is fine to have the default JSON schema to be tied to webauthn-json. I agree that it can be more obvious. Maybe a module name can be changed.

After playing around with the library, I think that it may be better to drop the WebIDL abstraction. WebIDL aligns with the CTAP part of the spec, but this library isn't meant to work client-side with the browser API.

That indirection makes it hard to discover how to compose. For example, I want to get a Credential 'Registration 'True from the client to pass to verifyRegistrationResponse. The docs for Credential has instances of ToJSON, Convert, Decode, DecodeCreated. The instances don't tell how to get the value from JSON. I only understood how to do this after reading the server example at the Main.hs that calls decodeCredentialRegistration.

It would be much simpler if we had something like this:

-- |  ...
-- For decoding see the newtype `Crypto.WebAuthn.Model.WebauthnJSON.CredentialJSON` .
data Credential (c :: CeremonyKind) raw 
...

newtype CredentialJSON (c :: CeremonyKind) raw = CredentialJSON (Credential (c :: CeremonyKind) raw)
instance FromJSON CredentialJSON ...
instance ToJSON CredentialJSON ...

Then I'd only need to know the helper newtypes and use the regular aeson types. There is SupportedAttestationStatementFormats passed to decoding that does not map cleanly toFromJSON. But I think it is easy to resolve if decoding is done in two stages - for the JSON payload with FromJSON, and with decode* that takes a CBOR value and the statement formats. The decode* functions not related to CBOR won't be necessary anymore.

[infinisil]

After playing around with the library, I think that it may be better to drop the WebIDL abstraction. WebIDL aligns with the CTAP part of the spec, but this library isn't meant to work client-side with the browser API.

Yeah very much agreed after looking more closely at it. WebIDL isn't really very involved here, I'll rename this to be more webauthn-json specific.

There is SupportedAttestationStatementFormats passed to decoding that does not map cleanly toFromJSON. But I think it is easy to resolve if decoding is done in two stages - for the JSON payload with FromJSON, and with decode* that takes a CBOR value and the statement formats.

I'm not entirely sure what you mean by this though, isn't this what is being done right now? Decoding is done with FromJSON IDLCredentialRegistration in a first step, followed by a second decodeCredentialRegistration step which takes the SupportedAttestationStatementFormats.

In the future I believe we will also need the same pattern for supported extensions (and that for both registration and authentication responses), see #35.

What I can do is just make sure that the documentation is better, e.g. as you already suggested, referencing the decoding function from the Credential type.

[lykahb]

When I said that Credential c raw doesn't map cleanly to FromJSON, I meant that aCredential c raw or a newtype over it cannot have a FromJSON instance. They need the two steps for decoding that you described where SupportedAttestationStatementFormats is supplied.

[infinisil]

This is now being cleaned up a bit in #140, though it's mostly still the same scheme of having intermediate types, but now they're specifically for webauthn-json and not really exposed