Resolve TODOs and FIXMEs - Githubissues

tweag / webauthn

A library for parsing and validating webauthn/fido2 credentials

Apache License 2.0

34 stars 11 forks source link

Resolve TODOs and FIXMEs #84

Closed ErinvanderVeen closed 2 years ago

ErinvanderVeen commented 2 years ago

Based on #81

For every FIXME and TODO, we should consider if we can fix/remove it right now, or if we are comfortable leaving it in the code after release.

src/Crypto/WebAuthn/Model.hs

[x] 700: -- FIXME: We don't allow encoding it as null here, because it doesn't seem

src/Crypto/WebAuthn/Metadata/Service/Processing.hs

[x] 80: -- FIXME: Return a better error here, but we can't modify the jose libraries error type
[x] 102: -- FIXME: We're currently discarding these errors by necessity, because we're bound by the JOSE libraries error type

src/Crypto/WebAuthn/Metadata/Statement/IDL.hs

[x] 77: -- FIXME: The spec declares this as an unsigned long, but the blob they
[x] 182: -- FIXME: The spec wrongfully declares the values to be DOMString's when they really aren't in the provided blob

server/src/Main.hs

[x] 220: -- FIXME: Update signature count in database
[x] 282: -- FIXME

tests/MetadataSpec.hs

[x] 23: -- TODO: Property tests: Generate random metadata, encode and sign it, decode and verify it, make sure it's the same result

tests/PublicKeySpec.hs

[x] 139: -- TODO: Find out how encoding changed

tests/Emulation/Authenticator.hs

[x] 213: -- TODO: Use deterministic random number generator, and ensure that we
[x] 378: -- TODO: We don't suport extensions
[x] 450:-- TODO: Surely there must be a beter function than lookpMin . filter

src/Crypto/WebAuthn/Model.hs

[x] 473:-- TODO: 'RpId' is used for both https://www.w3.org/TR/webauthn-2/#dom-publickeycredentialrpentity-id
[x] 642:-- TODO: Implement a way to specify extensions, or implement them here directly
[x] 654:-- TODO: Implement a way to specify extensions, or implement them here directly
[x] 930: -- TODO: Implement this
[x] 1228: -- TODO: This property is currently not propagated by webauthn-json

src/Crypto/WebAuthn/Metadata/Statement/IDL.hs

[x] 174:-- TODO: Replace Text with https://hackage.haskell.org/package/aeson-2.0.2.0/docs/Data-Aeson-Key.html#t:Key when updating aeson
[x] 181: = -- TODO: Replace Text with https://hackage.haskell.org/package/aeson-2.0.2.0/docs/Data-Aeson-Key.html#t:Key when updating aeson

src/Crypto/WebAuthn/Operations/Assertion.hs

[x] 99: -- TODO
[x] 117: -- TODO: Implement extensions
[x] 194: -- TODO
[x] 218: -- TODO: Maybe throw warning that user verification was preferred but not provided
[x] 220: -- TODO: Maybe throw warning that user verification was discouraged but provided
[x] 232: -- TODO

src/Crypto/WebAuthn/Operations/Attestation.hs

[x] 71: | -- | TODO: Fix this description
[x] 281: -- TODO: Implement extensions
[x] 309: -- TODO: Token binding is not currently supported.
[x] 344: -- TODO: Maybe throw warning that user verification was preferred but not provided
[x] 346: -- TODO: Maybe throw warning that user verification was discouraged but provided
[x] 366: -- TODO: Extensions aren't currently implemented

src/Crypto/WebAuthn/Metadata/Service/Processing.hs

[x] 76: -- TODO: Implement step 4 of the spec, which says to try to get the chain from x5u first before trying x5c
[x] 86: -- TODO: Check CRLs, see https://github.com/tweag/haskell-fido2/issues/23

src/Crypto/WebAuthn/Metadata/Statement/Decode.hs

[x] 56: -- TODO: Remove Text.strip, it's only needed because of a spec violation, see https://github.com/tweag/haskell-fido2/issues/68
[x] 57: -- TODO: Don't use decodeLenient, it's only needed because of a spec violation, see TODO
[x] 138: -- TODO: Use non-lenient decoding, it's only needed because of a spec violation,

src/Crypto/WebAuthn/Metadata/Service/Types.hs

[x] 63: -- TODO: Implement this, currently not used in the blob however
[x] 70: -- rogueListURL, rogueListHash. TODO, but not currently used in the

src/Crypto/WebAuthn/WebIDL.hs

[x] 31:-- TODO: This implementation doesn't allow invalid UTF-16 codepoints, which
[x] 41:-- TODO: This implementation allows for surrogate code points. Figure out if

src/Crypto/WebAuthn/Operations/Attestation/AndroidKey.hs

[x] 272: -- TODO: Allow the users of the library set the required trust level

src/Crypto/WebAuthn/Model/Binary/Decoding.hs

[x] 203: -- TODO
[x] 251: -- TODO

src/Crypto/WebAuthn/Operations/Attestation/TPM.hs

[x] 580: -- TODO: Left out for now, until proven needed

src/Crypto/WebAuthn/Operations/Attestation/FidoU2F.hs

[x] 119: -- TODO: Will we only get named curves?
[x] 160: -- TODO: Pass along SignatureFailure to error

src/Crypto/WebAuthn/Model/JavaScript/Decoding.hs

[x] 65: -- TODO: Implement extension support

src/Crypto/WebAuthn/UAF.hs

[x] 23:-- TODO: Implement a way to reproduce this value

src/Crypto/WebAuthn/PublicKey.hs

[x] 137:-- TODO: We could do without this type by going from Cryptonite's NamedCurve
[x] 163: -- TODO: left?
[x] 186: -- TODO(arianvp): Implement compressed curve. Waiting for

src/Crypto/WebAuthn/Model/JavaScript/Encoding.hs

[x] 60: -- TODO: Implement extension support
[x] 183: -- TODO: Extensions aren't currently supported
[x] 205: -- TODO: Extensions are not currently supported

server/src/PendingOps.hs

[x] 220: -- TODO: Do something less invasive than printing the removed options

ErinvanderVeen commented 2 years ago

@Infinisil For the Processing.hs FIXME's we can create our own Error type and then implement the AsError typeclass for it, this would allow using custom errors with the JOSE library. I think it is best you do this, you made this module and know best the kind of errors you would like to result in.

EDIT: I just pushed a commit resolving this.