Serialization cleanup #140
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Makes all types decode/encode to their direct equivalent, no more special handling for arrays or maybe's of certain types. We can only do this as cleanly by acknowledging that we only implement a relying party though, because client platforms would have to have special handling for many unknown values. The only case where a relying party needs to handle unknown values is currently marked with a FIXME, which will be addressed in a future commit - In order to handle defaults, a new Crypto.WebAuthn.Model.Defaults module is introduced, which exposes defaults for optional values with a default from the spec. The decoding now uses these values instead of having them "hardcoded" in the decoding. This makes writing other encoding formats easier and decouples the webauthn-json encoding a bit more from the specification itself - The module docs of Crypto.WebAuthn.Model.Types is updated to lay out the conventions used in the module - Minor doc fixes
This decouples the enum string serializations from the webauthn-json serialization, because: - webauthn-json doesn't define these strings, the standard does - These strings are useful for other possible serializations
Unknown AuthenticatorTransport values received by the Relying Party need to still be stored and not cause a decoding error (or be ignored), which was done previously. We fix this by introducing a new `AuthenticatorTransportUnknown` constructor for this case. The `decodeAuthenticatorTransport` is adjusted to never throw an error since it can handle all inputs now See also w3c/webauthn#1587
- Creates the non-internal cleaned-up `Crypto.WebAuthn.Encoding.Binary`,
to replace the previously-internal
`Crypto.WebAuthn.Model.WebIDL.Internal.Binary.{Encoding,Decoding}`.
These binary serializations really aren't part of WebIDL or
webauthn-json
- The `Crypto.WebAuthn.Model.Types` involved with raw fields now have expanded
docs, linking to the above-introduced module
- Changes ccdCrossOrigin to be a `Maybe`. A value of `Nothing` gets
encoded as `Just False` with the default binary encoding, but there's
nothing that says `False` should be the default
- Other minor doc changes
|
TODO:
|
- Combines
Crypto.WebAuthn.Model.WebIDL.Internal.{Convert,Encoding,Decoding} into
a single Crypto.Encoding.Internal.WebAuthnJson
- Moves Crypto.WebAuthn.Model.WebIDL to Crypto.Encoding.WebAuthnJson
- Removes mentions of WebIDL from the above modules, correcting them to
WebAuthnJson, or WJ as a prefix. This renames all the exposed decoding
functions, prefixing them with wj
- Make wjDecodeCredentialRegistration use allSupportedFormats as the
SupportedAttestationStatementFormats argument.
wjDecodeCredentialRegistration' has been introduced to allow passing a
custom SupportedAttestationStatementFormats.
For the future, only the unticked functions are intended to stay
backwards compatible
- Don't have Decode and DecodeCreated, instead parametrize Decode by m,
allowing instances to add additional constraints to it. Oh also just
use mtl monad constraints in general over Either for that module
- Remove the base64url JSON serialization types from
Crypto.WebAuthn.WebIDL as that's now done in the webauthn-json module
This module is now only used by the metadata
infinisil
force-pushed
the
json-encoding-cleanup-clean
branch
from
de95fa3
to
6a7aee1
Compare
ErinvanderVeen
approved these changes
Feb 21, 2022
Comment on lines
+210
to
+211
| decodeTransports :: BS.ByteString -> IO [WA.AuthenticatorTransport] | ||
| decodeTransports bytes = case deserialiseOrFail $ LBS.fromStrict bytes of |
There was a problem hiding this comment.
Is this IO just for the error?
There was a problem hiding this comment.
Yup indeed. Using an Either or so here felt a bit pointless when it's just going to have to be thrown in IO at the call site
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Replaces #138 with a clean history. See commit logs for changes.