Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a hints element for both create and get. #1884

Merged
merged 8 commits into from
Aug 25, 2023
Merged

Add a hints element for both create and get. #1884

merged 8 commits into from
Aug 25, 2023

Conversation

agl
Copy link
Contributor

@agl agl commented May 1, 2023

As discussed at the face-to-face meeting of the working group on 2023-04-21, this change adds a hints element to both sets of options in order to allow sites to guide user agents in how best to complete the request.


Preview | Diff

As discussed at the face-to-face meeting of the working group on
2023-04-21, this change adds a `hints` element to both sets of options
in order to allow sites to guide user agents in how best to complete the
request.
@Kieun
Copy link
Member

Kieun commented May 2, 2023

Why not leveraging authenticatorSelection with authenticatorAttachment?
Adding hints for the request by the RPs might offer better UX, but still the UX would be different per the browser and OS combination and the UX is not deterministic which still makes some users' confusion.

The authenticatorAttachement has such modality option for RPs to specify the authenticator.
With current enum, the security key cannot be expressed explicitly since the smart phone could be cross-platform authenticator as well. If this is the case, why not introducing additional enum value?

@agl
Copy link
Contributor Author

agl commented May 2, 2023

Why not leveraging authenticatorSelection with authenticatorAttachment?

Adding a new attachment would break existing user-agents. We could work around that with a feature-detection system but then we would still have the issue that authenticatorSelection only exists for create(). For discoverable get() there wouldn't be any corresponding mechanism.

index.bs Outdated Show resolved Hide resolved
@Firstyear
Copy link
Contributor

Why not leveraging authenticatorSelection with authenticatorAttachment?

Adding a new attachment would break existing user-agents. We could work around that with a feature-detection system but then we would still have the issue that authenticatorSelection only exists for create(). For discoverable get() there wouldn't be any corresponding mechanism.

That would be a bug in that user-agent then, because any rp could fill them with unknown values today and cause issues. So I think authenticator attachment still seems like a better place for this.

It's a bit of a concern that we are adding a third area for authentication selection, given that we already have transports and attachament selectors. So I think the bigger risk is for there to be inconsistent and odd behaviours if you specify multiple or combinations of these parameters.

@Kieun
Copy link
Member

Kieun commented May 3, 2023

There is a open issue (#1267) related to the authenticator selection for get operation.
We still have been suffering those issue after introducing passkey and webauthn to our customers.
I agree with @Firstyear's comments.

index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
@denniskniep
Copy link

denniskniep commented May 3, 2023

I like the idea to have a property where a rp can put generic hints for the user interface. And not limit it in any way to selection of the authenticator.

I would vote for two additional hints:

  • mechanism-selection: Indicates that the rp should show as "landing page" the list of all options
  • cable-qrcode: Indicates that the rp should show as "landing page" the QR Code to initialize cable

Furthermore it would be nice as browser to have the ability to specify custom hints which are not defined (yet) in the spec. That would provide the opportunity to adopt quickly and later (if it makes sense) integrate them into the spec. As far as I know same mechanism is used in CSS specification. Then it is necessary to specify a pattern for browser specific prefixes (e.g. -<browsername>-<hintname>)

@timcappalli
Copy link
Member

I would vote for two additional hints:

  • mechanism-selection: Indicates that the rp should show as "landing page" the list of all options
  • cable-qrcode: Indicates that the rp should show as "landing page" the QR Code to initialize cable

Both of these are already accommodated with a combination of the two options in this PR and authenticator attachment

@agl
Copy link
Contributor Author

agl commented May 3, 2023

That would be a bug in that user-agent then, because any rp could fill them with unknown values today and cause issues. So I think authenticator attachment still seems like a better place for this.

A WebAuthn level one implementation can reject any unknown values for authenticatorAttachment. A level two implementation should ignore unknown values. But my point is that, as an RP, you can't start using new values of authenticatorAttachment without knowing that the user-agent supports them, lest your attachment preference becomes completely ignored.

That's one of the things that this proposal is trying to address: by having a list of strings we can define new hints in the future without RPs needing to worry that adding them will break older user agents.

It's a bit of a concern that we are adding a third area for authentication selection, given that we already have transports and attachament selectors. So I think the bigger risk is for there to be inconsistent and odd behaviours if you specify multiple or combinations of these parameters.

I agree, but I think the direction that we should head in is to move away from the current authenticatorSelection and transport hints.

The platform/cross-platform values were intended to express whether a credential was desired for local reauth or for signing in. And the transport hints were intended to guide the UI between prompting for a security key or not. But both are getting quite awkward:

People took "cross-platform" to mean "security key", but now it means phones too and doesn't even mean "not platform" any more: if the platform authenticator is syncing to a phone then it's a candidate for attachment=cross-platform. Likewise, attachment=platform can get you a credential that you can use on another machine (e.g. making a credential on a hybrid-capable phone).

Transports are stressed by the fact that authenticators can gain transports now—they're not fixed-function. Lots of credentials with transports=["internal"] became effectively ["internal", "hybrid"] when we updated Android, but of course weren't actually updated.

So, overall, my thoughts are that:

  • A list of strings is more future proof than single values.
  • This mechanism isn't just about attachment, it should be the mechanism for any number of things in the future.
  • authenticatorAttachment and transports are increasingly inapplicable.

Co-authored-by: Emil Lundberg <emil@yubico.com>
@Kieun
Copy link
Member

Kieun commented May 3, 2023

So, overall, my thoughts are that:
A list of strings is more future proof than single values.
This mechanism isn't just about attachment, it should be the mechanism for any number of things in the future.
authenticatorAttachment and transports are increasingly inapplicable.

@agl You mean that at some point the authenticatorAttachment and transport hints are retired?
If so, the RP expects and requires the platform authenticator with internal transport to register the credential with non-security-key hint, but the client still offers hybrid registration UX which is not expected.

Is it better to define more specific hints? e.g., platform, security-key, hybrid

@agl
Copy link
Contributor Author

agl commented May 3, 2023

I would vote for two additional hints:

mechanism-selection: Indicates that the rp should show as "landing page" the list of all options.

Does that apply to both create() and get() in your mind? I ask because we're rethinking that UI in Chrome for the get() case. (And I assume that you've some familiarity since you're using the Chromium terminology.)

cable-qrcode: Indicates that the rp should show as "landing page" the QR Code to initialize cable

Are you thinking that this always shows a QR code, even if the user has a linked phone? Or is it more generally suggesting using a phone? The latter is what I was getting at with non-security-key. I don't like that name, but I hesitated to call it anything too specific. Maybe "smartphone" is a good answer?

@agl
Copy link
Contributor Author

agl commented May 3, 2023

You mean that at some point the authenticatorAttachment and transport hints are retired?

Potentially. Or at least sites often stop populating them. (Many sites have never supported transport hints.)

Is it better to define more specific hints? e.g., platform, security-key, hybrid

I was aiming at defining the general interface first but perhaps that's just obscuring things. Have added these in the latest update.

@denniskniep
Copy link

@agl

Does that apply to both create() and get() in your mind? I ask because we're rethinking that UI in Chrome for the get() case. (And I assume that you've some familiarity since you're using the Chromium terminology.)

I would see this feature definitely in create(). get() is working fine if it uses the last used option (if possible and makes sense). No need to start always at the very beginning with the selection. It should be as quick as possible, because we use get() a lot more than create().
Indeed, I recognized these terms in the chromium code base while I was searching what was changed that now the QR Code is the first screen (for Cross-Plattform + RK + WithoutRegisteredPhone)

Are you thinking that this always shows a QR code, even if the user has a linked phone? Or is it more generally suggesting using a phone? The latter is what I was getting at with non-security-key. I don't like that name, but I hesitated to call it anything too specific. Maybe "smartphone" is a good answer?

I like the "smartphone" approach (technically the enum value could maybe more generic like DeviceWithBLEAndCamera?). An reasonable implementation could be that the UI shows the list of registered devices (phones, tablets, ...) and offers a button "Use different phone or tablet".

@Firstyear
Copy link
Contributor

People took "cross-platform" to mean "security key", but now it means phones too and doesn't even mean "not platform" any more: if the platform authenticator is syncing to a phone then it's a candidate for attachment=cross-platform. Likewise, attachment=platform can get you a credential that you can use on another machine (e.g. making a credential on a hybrid-capable phone).

Yeah, I'd certainly agree here - as an enterprise it's currently impossible to really filter authenticators ahead of time with registration with the current fields. And as you correctly note, the lines are certainly becoming blurred with cable and phones as platform authenticators, that also can be cross platform.

If we are proceeding in this way, would it also be worth future consideration to deprecate the "transports/attachment" parameters or to add elements in the spec since they are quite confusing and often not always correct?

@MasterKale
Copy link
Contributor

For the record, in general we've been fine with how browsers handle authentication when we return transports during auth. Thus I'm evaluating this all through the lens of "how can we guide users to successful registration."

My higher-level requirement has been centered around the desire to individually trigger browsers' "security key" and "hybrid" flows so that we could show corresponding verbiage shown beforehand. The verbiage would help users understand what the browser will ask of them since some aspects of passkeys registration are still relatively new.

If I'm understanding this correctly, the intent of hints would be that an RP like me adds the new property to our registration options to "suggest" to the browser that it only show the hybrid flow?

{
  // ...
  authenticatorSelection: {
    authenticatorAttachment: 'cross-platform'
  },
  hints: ['non-security-key'],
}

Maybe "smartphone" is a good answer?

What about "other-device" or external-device? It leaves the door open for other form factors to participate (e.g. someone could use an iPad, which doesn't really fit into the idea of "smartphone")

@Firstyear
Copy link
Contributor

If I'm understanding this correctly, the intent of hints would be that an RP like me adds the new property to our registration options to "suggest" to the browser that it only show the hybrid flow?

{
  // ...
  authenticatorSelection: {
    authenticatorAttachment: 'cross-platform'
  },
  hints: ['non-security-key'],
}

Maybe "smartphone" is a good answer?

What about "other-device" or external-device? It leaves the door open for other form factors to participate (e.g. someone could use an iPad, which doesn't really fit into the idea of "smartphone")

For enterprise, we'll need the opposite - to completely deny smartphones and hybrid due to the fact these platforms are unattested and third party backed. We'll need a way to limit to only tpm's and fido2 security key devices (generally, things that have aaguids). This is why in the past we've requested that during registration we can send a list of aaguids as a filter for "these are the only devices acceptable under our policy, so only proceed with them".

@Kieun
Copy link
Member

Kieun commented May 8, 2023

As far as I understand, the hints are not requirements. If so, the clients might not respect RP's preference and intention and the same request from the RP might be handled by clients differently.

@Firstyear
Copy link
Contributor

Yes, I know they are hints, but they are still hints that would improve enterprise workflows in most cases by not proceeding with devices that are unable to succeed.

@MasterKale
Copy link
Contributor

Thinking out loud about potential string values for hints:

security-key
platform-local
platform-roaming

🤔

@agl
Copy link
Contributor Author

agl commented May 17, 2023

(Notes from the call of 2023-05-17: I should update to include three values: security-key, local-device, and hybrid. Should document expected values of attachment for those cases and define that attachment (and transports?) are ignored when hints are given.)

index.bs Outdated
:: Indicates that the [=[RP]=] believes that users will satisfy this request with a physical security key. For example, an enterprise [=[RP]=] may set this hint if they have issued security keys to their employees and will only accept those [=authenticators=] for [=registration ceremony|registration=] and [=authentication ceremony|authentication=]. For compatibility with older user agents, when this hint is used in {{PublicKeyCredentialCreationOptions}}, the {{AuthenticatorSelectionCriteria/authenticatorAttachment}} SHOULD be set to {{AuthenticatorAttachment/cross-platform}}.

: <dfn>local-device</dfn>
:: Indicates that the [=[RP]=] believes that users will satisfy this request with a [=platform authenticator=]. For compatibility with older user agents, when this hint is used in {{PublicKeyCredentialCreationOptions}}, the {{AuthenticatorSelectionCriteria/authenticatorAttachment}} SHOULD be set to {{AuthenticatorAttachment/platform}}.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

...users will satisfy this request with a [=platform authenticator=]

I think this would benefit from something explicitly tying this option to the "locally available" platform authenticator, the same device as the device the user is logging into. For me the ambiguity comes from the fact that hybrid implies use of a platform authenticator, but of course on a separate device than the access device the user is actually using.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hybrid doesn't preclude security key on a remote device over cable though. We already have this working via webauthn-rs. hybrid just means "caBLE" here.

I'd say platform authenticator is reasonable here since to me it implies "same device" but perhaps "device bound authenticator" is clearer.

@nadalin nadalin added the @Risk Items that are at risk for L3 label Jun 27, 2023
@emlun emlun self-requested a review June 28, 2023 19:20
Since hybrid allows remote platform authenticators to be used, the
previous wording was potentially ambiguous.
<div dfn-type="enum-value" dfn-for="PublicKeyCredentialHints">
[=[WRPS]=] may use this enumeration to communicate hints to the user-agent about how a request may be best completed. These hints are not requirements, and do not bind the user-agent, but may guide it in providing the best experience by using contextual information that the [=[RP]=] has about the request. Hints are provided in order of decreasing preference so, if two hints are contradictory, the first one controls. Hints may also overlap: if a more-specific hint is defined a [=[RP]=] may still wish to send less specific ones for user-agents that may not recognise the more specific one. In this case the most specific hint should be sent before the less-specific ones.

Hints MAY contradict information contained in credential {{PublicKeyCredentialDescriptor/transports}} and {{AuthenticatorSelectionCriteria/authenticatorAttachment}}. When this occurs, the hints take precedence. (Note that {{PublicKeyCredentialDescriptor/transports}} values are not provided when using [=discoverable credentials=], leaving hints as the only avenue for expressing some aspects of such a request.)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

...Note that {{PublicKeyCredentialDescriptor/transports}} values are not provided when using [=discoverable credentials=]

Might this statement need a qualification that we're talking about usernameless authentication here, with an empty allowCredentials? Because passwordless authentication, that identifies the user beforehand (e.g. SSO account discovery precluding WebAuthn use) can definitely use discoverable credentials with transports by populating allowCredentials like normal.

index.bs Outdated
: <dfn>security-key</dfn>
:: Indicates that the [=[RP]=] believes that users will satisfy this request with a physical security key. For example, an enterprise [=[RP]=] may set this hint if they have issued security keys to their employees and will only accept those [=authenticators=] for [=registration ceremony|registration=] and [=authentication ceremony|authentication=]. For compatibility with older user agents, when this hint is used in {{PublicKeyCredentialCreationOptions}}, the {{AuthenticatorSelectionCriteria/authenticatorAttachment}} SHOULD be set to {{AuthenticatorAttachment/cross-platform}}.

: <dfn>local-device</dfn>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

client-device, not local device.

Copy link
Contributor

@MasterKale MasterKale left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved pending the replacement of "local-device" with "client-device"

@nadalin nadalin requested a review from akshayku July 12, 2023 19:50
@MasterKale
Copy link
Contributor

MasterKale commented Jul 12, 2023

As per the WAWG meeting on 7/12 this is okay to be merged before the next meeting when it's ready.

@agl
Copy link
Contributor Author

agl commented Jul 12, 2023

From call of 2023-07-12: ok to land if Akshay and Emil are happy.

I missed, when looking that the spec has a definition for "client
device" already. Based on discussions in the WG, we want to use the
already-defined term rather than invent a new alias.
Copy link
Member

@emlun emlun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Just one minor, optional nit.

index.bs Outdated Show resolved Hide resolved
Co-authored-by: Emil Lundberg <emil@yubico.com>
@agl
Copy link
Contributor Author

agl commented Aug 16, 2023

From call of 2023-08-16: good to merge.

@agl agl merged commit 410d0f7 into main Aug 25, 2023
1 check passed
@agl agl deleted the hints branch August 25, 2023 14:03
github-actions bot added a commit that referenced this pull request Aug 25, 2023
SHA: 410d0f7
Reason: push, by agl

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@Risk Items that are at risk for L3
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants