Problem/Motivation

There used to be a jsonSerialize() method on the PublicKeyCredentialCreationOptions but now that is gone.

json_encode should not be used as in 5.0.+ the objects do not implement the JsonSerializable interface.

But it looks like the serializePublicKeyCredential() method could be modified to accept PublicKeyCredentialCreationOptions.

I have tried this with a quick test by hacking the module and while it did serialize my object:

{"challenge":"1rdlxMvJcwva-KAWBVGp7Q","rp":{"id":"localhost","name":"Drupal","icon":""},"user":{"id":"MjkzZTE0NjQtZWU2Yy00NWEwLWFmM2UtNGI2ODlmNWFkYWRj","name":"goat@meals.com","displayName":"goat@meals.com"},"pubKeyCredParams":[{"type":"public-key","alg":-7},{"type":"public-key","alg":-257}],"authenticatorSelection":{"userVerification":"preferred","residentKey":"preferred"},"excludeCredentials":[]}

Passing that into simplewebauthn/browser startRegistration() resulted in "TypeError: base64URLString is undefined"

This used to work so I have confidence I'm not doing anything else stupid, but I'm unable to find why this is not working at the moment.

Proposed resolution

Add PublicKeyCredentialCreationOptions as allowed option to pass into Webauthn::serializePublicKeyCredential()

CommentFileSizeAuthor
#2 3490555__Allow_serialization_of_PublicKeyCredentialCreationOptions.patch2.76 KBdahousecat

Issue fork webauthn_framework-3490555

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

dahousecat created an issue. See original summary.

dahousecat’s picture

dahousecat commented
StatusFileSize
new 3490555__Allow_serialization_of_PublicKeyCredentialCreationOptions.patch2.76 KB

I did try and create an MR but I'm having some issues with permissions when pushing and it's late so will just attach a patch for now.

ptmkenny made their first commit to this issue’s fork.

ptmkenny changed the visibility of the branch 3490555-support-seralization-of to hidden.

ptmkenny opened merge request !13

ptmkenny’s picture

ptmkenny commented

Thanks for this issue. For the serializer, I think it will be safe to accept PublicKeyCredentialOptions, so I created an MR to do that.

Please review and let me know if this solves your issue.

Passing that into simplewebauthn/browser startRegistration() resulted in "TypeError: base64URLString is undefined"

I'm guessing that's because webauthn-lib 5 no longer serializes to JSON objects, so that's not "supposed" to work anymore? I do not know why this change was made, but this module is attempting to follow the underlying library as closely as possible.

Here's the relevant page from the docs: https://webauthn-doc.spomky-labs.com/pure-php/authenticator-registration

ptmkenny’s picture

ptmkenny commented
Status: Active » Needs review
dahousecat’s picture

dahousecat commented

Hi, thanks for the swift reply :)

The MR looks good to me, better than my patch as simpler.

Yes, it's a shame this update has so many breaking changes but thanks for the pointer - I'll review the updated documentation.

dahousecat’s picture

dahousecat commented
Status: Needs review » Reviewed & tested by the community

  • ptmkenny committed 25077371 on 1.0.x 
    Issue #3490555 by dahousecat, ptmkenny: Support seralization of...
ptmkenny’s picture

ptmkenny commented
Status: Reviewed & tested by the community » Fixed

Thanks for the review!

Yes, there are a lot of breaking changes; I need to do more testing before I make a new release with 5.x support.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.