Skip to content

Comments

LG-9873: Use "transports" to optimize platform authentication prompt#8639

Merged
aduth merged 1 commit intomainfrom
aduth-lg-9873-webauthn-transports
Jul 6, 2023
Merged

LG-9873: Use "transports" to optimize platform authentication prompt#8639
aduth merged 1 commit intomainfrom
aduth-lg-9873-webauthn-transports

Conversation

@aduth
Copy link
Contributor

@aduth aduth commented Jun 22, 2023

🎫 Ticket

LG-9873

🛠 Summary of changes

Records transports associated with WebAuthn configurations during enrollment and uses those values in the authentication prompt, so that the browser UI will optimize the display for the user to select the correct authenticator device. For example, this should prevent users from being shown options for using a "Security Key" to complete Face or Touch Unlock.

Via WebAuthn spec:

The [credential request] items SHOULD specify transports whenever possible. This helps the client optimize the user experience for any given situation.

https://w3c.github.io/webauthn/#dom-publickeycredentialrequestoptions-allowcredentials

When registering a new credential, the Relying Party SHOULD store the value returned from getTransports(). When creating a PublicKeyCredentialDescriptor for that credential, the Relying Party SHOULD retrieve that stored value and set it as the value of the transports member.

https://w3c.github.io/webauthn/#dictionary-credential-descriptor

📜 Testing Plan

  1. On a device supporting platform authenticators, go to http://localhost:3000 (see mobile testing docs)
  2. Create an account
  3. Choose "Face or Touch Unlock" as the MFA method
  4. Finish account creation
  5. From your desktop computer, sign in to the account created in steps 2-4
  6. Click "Continue" when prompted for Face or Touch Unlock

Before: You'd be asked to select between QR code & security key.
After: You're immediately shown a QR code.

👀 Screenshots

Before After
before after

@aduth aduth force-pushed the aduth-lg-9873-webauthn-transports branch 2 times, most recently from 8f83fb3 to 45f834f Compare June 26, 2023 20:42
@aduth
Copy link
Contributor Author

aduth commented Jun 26, 2023

We should probably log something here for us to be notified, in case more transports options are added in the future. The specification makes some disclaimers about future-compatibility, though I don't know that we'd want to go as far as to allow any value here.

It is important for backwards compatibility that client platforms and Relying Parties handle unknown values.

https://w3c.github.io/webauthn/#sct-domstring-backwards-compatibility

I came across another snippet from the spec today, which does explicitly say that we should store unknown values (emphasis mine):

The values SHOULD be members of AuthenticatorTransport but Relying Parties SHOULD accept and store unknown values.

Source: https://w3c.github.io/webauthn/#dom-authenticatorattestationresponse-transports-slot

I still don't feel particularly good about storing arbitrary values from the frontend.

Based on the "SHOULD"s, it seems like this may be both unlikely and not strictly necessary to support, and at least our current implementation here would fall back to omitting the transports altogether if any unknown values are detected, which should revert to the behavior of providing the user with all possible options on subsequent authentications.

@aduth aduth marked this pull request as ready for review June 26, 2023 21:04
@aduth aduth requested a review from a team June 26, 2023 21:05
@aduth aduth force-pushed the aduth-lg-9873-webauthn-transports branch from 45f834f to ad91ac3 Compare June 29, 2023 15:49
Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com>

changelog: Upcoming Features, Face or Touch Unlock, Pass browser hints to optimize authentication prompt
@aduth aduth force-pushed the aduth-lg-9873-webauthn-transports branch from ad91ac3 to 55345f7 Compare July 5, 2023 13:41
@aduth aduth merged commit 25d039a into main Jul 6, 2023
@aduth aduth deleted the aduth-lg-9873-webauthn-transports branch July 6, 2023 12:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants