webauthn-ruby icon indicating copy to clipboard operation
webauthn-ruby copied to clipboard

feat: add capability of handling appid extension

Open santiagorodriguez96 opened this issue 4 years ago • 2 comments

What

Add capability of handling internally the extension App ID in order for it to replace RP ID in cases when user is authenticating with a migrated U2F credential.

How

Forked a new branch from the work done for providing extension outputs in order to start from there. Basically what I did was follow what was specified in the documentation for migrating from U2F

Screen Shot 2020-06-18 at 12 43 38 PM

Also, separated the tests suites for PublicKeyCredentialWithAttestation (which was the one that was before, but named PublicKeyCredential) and PublicKeyCredentialWithAssertion so we can test this feature separately.

TODO

  • Acording to the documentation:

appid should be set to the AppID that the Relying Party previously used in the legacy FIDO APIs. This might not be the same as the result of translating the Relying Party's WebAuthn RP ID to the AppID format, e.g., the previously used AppID may have been "https://accounts.example.com" but the currently used RP ID might be "example.com".

This means that the case of

rp_id: client_extension_outputs[:appid] ? domain.to_s : domain.host

does not apply in all cases, so we should provide users with ability of setting the the AppID that the Relying Party previously used. Something like what is being done with the RP ID actually.

  # You can optionally specify a different Relying Party ID
  # (https://www.w3.org/TR/webauthn/#relying-party-identifier)
  # if it differs from the default one.
  #
  # In this case the default would be "auth.example.com", but you can set it to
  # the suffix "example.com"
  #
  # config.rp_id = "example.com"

santiagorodriguez96 avatar Jun 18 '20 15:06 santiagorodriguez96

Have you considered adding the extension automatically to the output of WebAuthn::Credential.options_for_get when the user set the config?

That's actually a good idea! I'll give it a try 👀

santiagorodriguez96 avatar Jun 29 '20 18:06 santiagorodriguez96

Also, separated the tests suites for PublicKeyCredentialWithAttestation (which was the one that was before, but named PublicKeyCredential) and PublicKeyCredentialWithAssertion so we can test this feature separately.

Split off this :point_up: into it's own separate commit (https://github.com/cedarcode/webauthn-ruby/commit/f787a2f3a001ba991d08df6da48b21eab7d8f5c4) and pushed to master because that's valuable and independent of this PR.

grzuy avatar Mar 09 '21 23:03 grzuy

Also, do you know why this was not included in the RelyingParty API in 3.0.0.alpha1 ?

I don't really know, but the PR now has some conflicts to resolve that are related to the RelayingParty API. I'll try to fix them so we can get this back on track and hopefully merge it 🙂

santiagorodriguez96 avatar Oct 26 '22 17:10 santiagorodriguez96

I've updated the docs – both the U2F migration doc and the Configuration section in the README. @bdewater it would be great if you could take a quick look at them and check that they still make sense 🙂

santiagorodriguez96 avatar Oct 28 '22 20:10 santiagorodriguez96

The build is failing because there's a fixup commit in the branch. I tried rebasing to squash it but there were too much conflicts at this point. I think I'll just go ahead and Squash and Merge it instead.

santiagorodriguez96 avatar Nov 07 '22 12:11 santiagorodriguez96