-
Notifications
You must be signed in to change notification settings - Fork 183
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
Backup preference for providers who support both backed-up and non-backed up credential #2253
base: main
Are you sure you want to change the base?
Conversation
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.
Only a minor editorial.
@@ -4428,6 +4428,8 @@ Note: The {{ClientCapability}} enumeration is deliberately not referenced, see [ | |||
"security-key", | |||
"client-device", | |||
"hybrid", | |||
"backed-up-cred, |
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.
Nit - missing trailing quotes
A more wholisitc comment about this approach - keep in mind that there are two flags - BE (backup eligible) and BS (backup state). I think the PR should explicitly state that the hint is in regards to BE = true/false, not BS. |
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 I'm fine with the high level concept, but I disagree on the semantics of specifying it via hints
. PublicKeyCredentialHint
communicate a preference for authenticator properties, while this proposed backup eligibility hint represents a desired credential property.
The main issue is that values in hints
are ordered by preference; I can imagine that hints processing rules would get quite complicated (and RPs would easily get confused too) if they had to accommodate all the permutations of these values and their positions in the array.
Maybe we define a new "credentialHints
"?
When we added hints to the spec, I thought the purpose of naming it as a general purpose "hint" name is that we can add more things to this array in the future without changing the IDL. I was also not thinking about Authenticator hint vs Credential hint and interestingly we named it as PublicKeyCredentialHint instead of PublicKeyAuthenticatorHint. Regarding the complication of processing rules, anything we add to hints array in the future which are not conveying already present transport info (either via AuthenticatorHints or CredentialHints), will arise in that processing complication anyway. So in effect, it looks like that hint is now effectively a static transport specific option. I have opened an alternative backup preference specific option as part of PublicKeyCredentialCreationOptions in #2259. |
I don't see a problem with having this in |
Closes #2252
This PR provides a way for an RP to indicate their backup preference regarding the credential being created at registration time via PublicKeyCredentialHint.
Note: Given the nature of different options provided by the providers/authenticators, their capabilities, user choices etc., RP must expect both backed-up and non-backed-up credentials in the registration responses.
Preview | Diff