Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Initial text for conditional create #1951
Initial text for conditional create #1951
Changes from 16 commits
cacd06d
eb9dd9f
65066fa
c14f20a
6c6184b
bf5480d
2b8521b
55024a6
6906fe6
d750218
e9b0448
aca236f
763831b
5d6c205
ae8e0fc
0318dee
f2ccac7
9a30613
dde3ab3
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
In the CredentialsContainer/create() the RP will pass in user. id and names.
Are these names the one the authenticator must use or can the passkey provider override them?
Is the assumption that this goes back to the password manager used to fill in the password, or would the user be given a choice of where to create 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.
The user.id and user.name will be respected as normal. The client may choose pass along which password account was used for authentication for the purposes of linking. The default passkey provider used is up to the client. I imagine it'd be a user configurable setting of all providers that could possibly satisfy such a request.
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/SHOULD/MUST
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 section is the client operation, but this is a requirement on the authenticator. This needs to go in section 6 (WebAuthn Authenticator Model) instead, or be rewritten in terms of how the client is to set the arguments to the authenticator operation.
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.
The flow here is a bit convoluted, I would formulate it something more like this:
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.
Also, is
NotAllowedError
the right behaviour here? The closest analogue currently in the spec is the "error code equivalent toConstraintError
" returned by the authenticator when UV is required but cannot be satisfied by that authenticator. Returning an error at all also differs fromconditional
mediation inget()
, which simply hangs forever on most errors. Shouldconditional
create do that too?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.
The idea with conditional registration is that you invoke it right after logging in and get a result quickly. If the user didn't consent to this type of create, then they will quickly get an error, otherwise a credential.
I think
ConstraintError
does sound okay in this case. I've made the change.