-
Notifications
You must be signed in to change notification settings - Fork 169
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
Browser capability detection. #1219
Conversation
[=[WRPS]=] use this method to determine what capabilities are supported by the [=client=]. | ||
Upon invocation, the [=client=] employs a [=client platform=]-specific procedure to discover supported capabilities. | ||
The promise is resolved with the capabilities that are supported. | ||
Based on the result, the [=[RP]=] can take further actions to guide the user to create a credential. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RP may also invoke this method for getting a credential.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, this is also useful for login scenarios. I can update the text to reflect that.
index.bs
Outdated
: <dfn>areResidentKeysSupported</dfn> | ||
:: This attribute indicates whether the [=client=] supports creating [=resident credentials=]. | ||
|
||
: <dfn>isUserVerificationSupported</dfn> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this different from isUserVerifyingPlatformAuthenticatorAvailable()?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isUserVerifyingPlatformAuthenticatorAvailable currently indicates the enabled state of the UVPA, but the RP cannot distinguish between disabled/unavailable. I left isUserVerifyingPlatformAuthenticatorAvailable alone as RPs are currently using this API, but reflected the tri-state in this new API.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was asking about the purpose of isUserVerificationSupported() function. We have userVerifyingPlatformAuthenticatorAvailability() and isUVPAA(). What is the difference?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for clarifying. The intention of isUserVerificationSupported is so that the client can covey whether it supports the UserVerificationRequirement enum. On the other hand, userVerifyingPlatformAuthenticatorAvailability is intended to indicate the specific state of the UVPA on the client platform. Does that make sense?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make sense.
So, this method is for indicating the client supports userVerification option. IMO, it is better to refer userVerification member instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest that the for this (and all other similar functionality detection), we remove these methods and say that if a particular capability is not part of the response from getSupportedCapabilities()
, then it's not supported.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I understand the comment of saying to remove isUserVerifyingPlatformAuthenticatorAvailable(), browsers already implement this function and RPs already depend on it. I don't think we should remove it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would prefer the way suggested by @leshi . WebAuthn spec will keep introducing new features. Returning the supported features as a list of string is better and easy to extend.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, IIUC, whatever we do, we need to leave isUserVerifyingPlatformAuthenticatorAvailable()
nominally intact, and if we do decide to deprecate it in light of other new functionality we define, we ought to mark it (in the spec) as deprecated and point to the new functionality.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have removed references to an updated UVPAA check, as we currently don't have scenarios to leverage this additional granularity. I have also adopted the style used by Transports to return the list of supported capabilities.
index.bs
Outdated
<xmp class="idl"> | ||
dictionary SupportedCapabilities { | ||
boolean areU2FDevicesSupported; | ||
boolean areCTAPDevicesSupported; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you explain the user flow for how you plan to use areU2FDevicesSupported
and areCTAPDevicesSupported
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CTAP devices are not supported on every browser. RP will look into this field to decide what they want to support and what they want to show on their page. User Agent detection is too fragile to make these decisions and it requires constant monitoring of what browser/platform combination works.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But why do RPs care about CTAP2 vs CTAP1 (U2F)? They care about capabilities and features -- not transports. Both of which are covered below.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed that we do not currently have a use case for such granularity. I have removed these particular capabilities.
index.bs
Outdated
: <dfn>areResidentKeysSupported</dfn> | ||
:: This attribute indicates whether the [=client=] supports creating [=resident credentials=]. | ||
|
||
: <dfn>isUserVerificationSupported</dfn> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest that the for this (and all other similar functionality detection), we remove these methods and say that if a particular capability is not part of the response from getSupportedCapabilities()
, then it's not supported.
Here's an overall comment on "browser capability detection": I asked Tab Atkins (CSS spec editor) about this (back in April), and he said that the
approach, colloquially known as "feature detection", often is the best one can do from JS within a user agent. There are feature detection JS libraries — e.g., modernizr, yepnope — but results with them can be mixed. Tab says that SVG tried specifying feature label strings (i suppose one asked the svg impl 'what is supported?' and got a list of strings in return), and that sorta worked for a very short time and then implementations would lie about what they supported, then the number of features outgrew the set of strings, etc. Tab asserts similar efforts occurred in various portions of the web platform with lousy results overall. In CSS, they have a WebAuthn/FIDO2 is an especially tough problem because we have a browser->protocols->authnr stack and the answer to "is foo feature supported" question may involve component answers from several layers in the stack. |
The transports result might need to be defined as what the client + operating system can support, and not what currently is supported by available hardware and settings. Examples what this may be appropriate:
I suppose the user might toggle the UVPA as well. |
just fyi; @agl opened: issue w3ctag/design-reviews#383 with the TAG, regarding this here PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's see what the TAG has to say: #1219 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd also want to add a section about what browsers should do when they want to limit the information they reveal. Perhaps we suggest that it be 'static' or hard-coded to a safe set of defaults?
index.bs
Outdated
|
||
<xmp class="idl"> | ||
dictionary SupportedCapabilities { | ||
boolean areU2FDevicesSupported; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will this one ever be used? Can we imagine this being set to false
and for RPs to code themselves to handle it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, this has been removed.
index.bs
Outdated
:: This attribute indicates whether the [=client=] supports communicating with [[FIDO-CTAP]] devices. | ||
|
||
: <dfn>areResidentKeysSupported</dfn> | ||
:: This attribute indicates whether the [=client=] supports creating [=resident credentials=]. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you support CTAP devices, isn't this always going to be true
? Or are clients supposed to guess what users' devices in their pocket can do, and fuzzily adjust depending on what's been paired/plugged in of late?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Chrome at the moment (version 75) supports CTAP2 devices, but doesn't have support for account selection etc and so would return false
for this because resident keys aren't actually usable. I.e. I think these fields are reporting on browser capabilities so it's “assuming that the user has an authenticator that does resident keys, will it actually work with this browser?”.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Resident key support is still being returned as we have a use case for this, and browsers do react differently today.
index.bs
Outdated
<xmp class="idl"> | ||
dictionary SupportedCapabilities { | ||
boolean areU2FDevicesSupported; | ||
boolean areCTAPDevicesSupported; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Point of order: U2F == CTAP1, and CTAP2 is the new thing, right?
Seems like this should be areCTAP2DevicesSupported
? Or maybe it should just be minimumCTAPVersionSupported
and make it a float?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed about saying “CTAP2”.
Minimum version might be a problem for Safari: don't they only support CTAP2 devices at the moment?
index.bs
Outdated
: <dfn>userVerifyingPlatformAuthenticatorAvailability</dfn> | ||
:: This attribute indicates the availability of a [=user-verifying platform authenticator=] on the [=client=]. | ||
|
||
: <dfn>transportsSupported</dfn> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have to define the strict ordering of these two sequences to reduce fingerprinting opportunity.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it make sense to define them as ordered sets? I agree we should probably also explicitly define the order like we have for [[transports]]
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For a list, I always default to saying that it should be ordered lexicographically and not contain duplicates. Looks like ordered sets imply the uniqueness condition, so that would work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have applied the same logic as was previously done for transports.
|
||
<div link-for-hint="WebAuthentication/getSupportedCapabilities"> | ||
|
||
[=[WRPS]=] use this method to determine what capabilities are supported by the [=client=]. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/client/client platform/
index.bs
Outdated
: <dfn>userVerifyingPlatformAuthenticatorAvailability</dfn> | ||
:: This attribute indicates the availability of a [=user-verifying platform authenticator=] on the [=client=]. | ||
|
||
: <dfn>transportsSupported</dfn> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it make sense to define them as ordered sets? I agree we should probably also explicitly define the order like we have for [[transports]]
.
index.bs
Outdated
}; | ||
</xmp> | ||
|
||
This dictionary is used by [=[WRPS]=] to determine the supported capabilities of the [=client=]. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Throughout this section: s/[=client=]/[=client platform=]/
As was discussed on recent WG calls, I think that However: suppose we rename |
index.bs
Outdated
<xmp class="idl"> | ||
dictionary SupportedCapabilities { | ||
boolean areU2FDevicesSupported; | ||
boolean areCTAPDevicesSupported; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed about saying “CTAP2”.
Minimum version might be a problem for Safari: don't they only support CTAP2 devices at the moment?
|
||
### Capability Support - PublicKeyCredential's `getSupportedCapabilities()` Method ### {#sctn-getSupportedCapabilities} | ||
|
||
<div link-for-hint="WebAuthentication/getSupportedCapabilities"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will Microsoft's webauthn.dll export a reflection of this function for browsers running on >= 19H1? Some of the fields are browser specific, but some depend on the platform.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes
index.bs
Outdated
boolean areU2FDevicesSupported; | ||
boolean areCTAPDevicesSupported; | ||
boolean areResidentKeysSupported; | ||
boolean isUserVerificationSupported; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As I think was mentioned in the call, isUserVerificationSupported
seems odd and I think you might want isClientPINSupported
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think isUserVerificationSupported is more about whether the client (browser) supports userVerification option for create() and get() call. Plus, it's better to have isClientPINSupported as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we have other forms of user verification that hte client can do besides clientPin? If not, I think isClientPINSupported
is the correct name here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated for clientPin as opposed to UV.
index.bs
Outdated
:: This attribute indicates whether the [=client=] supports communicating with [[FIDO-CTAP]] devices. | ||
|
||
: <dfn>areResidentKeysSupported</dfn> | ||
:: This attribute indicates whether the [=client=] supports creating [=resident credentials=]. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Chrome at the moment (version 75) supports CTAP2 devices, but doesn't have support for account selection etc and so would return false
for this because resident keys aren't actually usable. I.e. I think these fields are reporting on browser capabilities so it's “assuming that the user has an authenticator that does resident keys, will it actually work with this browser?”.
index.bs
Outdated
: <dfn>userVerifyingPlatformAuthenticatorAvailability</dfn> | ||
:: This attribute indicates the availability of a [=user-verifying platform authenticator=] on the [=client=]. | ||
|
||
: <dfn>transportsSupported</dfn> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For a list, I always default to saying that it should be ordered lexicographically and not contain duplicates. Looks like ordered sets imply the uniqueness condition, so that would work.
Edited OP to add close macros for all related issues. |
|
||
### Capability Support - PublicKeyCredential's `getSupportedCapabilities()` Method ### {#sctn-getSupportedCapabilities} | ||
|
||
<div link-for-hint="WebAuthentication/getSupportedCapabilities"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes
…have concrete use cases.
@emlun @leshi @Kieun @jcjones @agl , I have made changes based on feedback. In particular, the capabilities have been scoped down significantly to concrete use cases that our Microsoft RP (and other RPs) have requested to take advantage of. Namely whether the client platform will support Client PIN and whether or not it supports Resident Keys. Both of these currently have states where some platform + browser + version combinations will say true vs. false. Look forward to more discussions on this. |
@jafisher-microsoft Is |
:: This attribute indicates whether the [=client platform=] supports [=user verification=] via client PIN. | ||
|
||
: <dfn>residentKeys</dfn> | ||
:: This attribute indicates whether the [=client platform=] supports creating [=resident credentials=]. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is also about whether the client supports resident credential selection (for external authenticators without their own display), right? Or should that be a separate capability?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1, I agree with @emlun.
I would assume that "full resident key support" means: 1) clientPin, 2) creation of resident keys, 3) exercising resident keys, 4) account selection, and maybe 5) resident key management.
I suspect that we want this boolean to mean all of those things.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@leshi, what do you mean by "account selection" ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(5), key management, isn't standardised in WebAuthn (yet?), so I'd say we can't make that requirement at this time.
<div dfn-type="enum-value" dfn-for="SupportedCapability"> | ||
Each enum value indicates a supported capability of the [=client platform=] for use by [=[WRPS]=]. | ||
: <dfn>clientPin</dfn> | ||
:: This attribute indicates whether the [=client platform=] supports [=user verification=] via client PIN. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
:: This attribute indicates whether the [=client platform=] supports [=user verification=] via client PIN. | |
:: This value indicates that the [=client platform=] supports [=user verification=] via client PIN. |
:: This attribute indicates whether the [=client platform=] supports [=user verification=] via client PIN. | ||
|
||
: <dfn>residentKeys</dfn> | ||
:: This attribute indicates whether the [=client platform=] supports creating [=resident credentials=]. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
:: This attribute indicates whether the [=client platform=] supports creating [=resident credentials=]. | |
:: This value indicates that the [=client platform=] supports creating [=resident credentials=]. |
What happens if |
on webauthn call today: @leshi relates that in the google security key experience, they learned that users were really confused if they were offered to register a security key in one browser, but not even offered in another browser. the lesson was that it was more clear to users if they go thru part of the ceremony and if the browser does not support it then fail. overall situation seems to be: |
In general I think an RP has to deal with the case that the browser doesn't even support this proposed API anyway, and potential variances in behaviour of this API, so I think it's better that the RP always deals with the error case rather than trying to pre-emptively render different UX based on what this API is proposing. |
@sbweeden has a very good point due to how the platforms implement the spec (only released published final version vs draft versions) and their support on various versions in different versions of the platform. And this API may not be present. Given this, I am not sure how much we are moving the needle and RPs has to manually be testing browser/platform combination for a while and this PR probably won't help that much. @jafisher-microsoft, What do you think? |
Closing this down as right now things are not clear at this point in time.. |
Fixes #1218. Fixes #1202. Fixes #1199.
Preview | Diff