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.

Sign up for GitHub

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

Merged
merged 19 commits into from
May 15, 2024
+37 −11
Merged

Initial text for conditional create #1951

Changes from 1 commit
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
cacd06d
Initial text for conditional create
Aug 29, 2023
eb9dd9f
Fix indentation and extraneous opening bracket
emlun Sep 6, 2023
65066fa
Address comments
Sep 11, 2023
c14f20a
Remove the requirement that conditionalCreate extension was used
Sep 12, 2023
6c6184b
Add recommended time period floor for conditionalCreateLifetimeTimer
Dec 20, 2023
bf5480d
Address comments, clarify other means other than extension may be use…
Feb 7, 2024
2b8521b
Remove extension, clarify user agent is responsible for collecting co…
Feb 21, 2024
55024a6
remove another extension reference
Feb 21, 2024
6906fe6
Update isConditionalMediationAvailable and contributors
Feb 21, 2024
d750218
Merge branch 'main' into pr-1951
emlun Mar 19, 2024
e9b0448
Fix autolinks
emlun Mar 19, 2024
aca236f
Revert unused change
emlun Mar 19, 2024
763831b
Address emlun's comments
Mar 19, 2024
5d6c205
CredentialRequestOptions->CredentialCreationOptions
Mar 19, 2024
ae8e0fc
Address emlun's other comments
Mar 19, 2024
0318dee
Require uv=0,up=0 for conditionalCreate
Apr 3, 2024
f2ccac7
Address Tim and Emlun's comments
Apr 19, 2024
9a30613
Address Tim's comment
May 1, 2024
dde3ab3
Address Matt's comments
May 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion index.bs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1744,6 +1744,8 @@ By setting <code>|options|.{{CredentialCreationOptions/mediation}}</code> to {{C
[=[RPS]=] can indicate that they would like to register a credential without prominent modal UI if user has already consented to create a credential. The [=[RP]=] SHOULD first check that {{ClientCapability/conditionalCreate}} is present
in the result of {{PublicKeyCredential/getClientCapabilities()}} in order to avoid the possibility of causing a user-visible error to be returned if the user agent does
not support {{CredentialMediationRequirement/conditional}} [=user mediation=] for {{CredentialsContainer/create()|navigator.credentials.create()}}.
The authenticator SHOULD set BOTH |userPresence| and |userVerification| to |FALSE| when <code>|options|.{{CredentialCreationOptions/mediation}}</code> is set to {{CredentialMediationRequirement/conditional}}
Copy link
Member

@timcappalli timcappalli Apr 3, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

s/SHOULD/MUST

unless explicitly collected during the ceremony.
Copy link
Member

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.


Any {{CredentialsContainer/create()|navigator.credentials.create()}} operation can be aborted by leveraging the {{AbortController}};
see [[dom#abortcontroller-api-integration]] for detailed instructions.
Expand Down Expand Up @@ -2023,7 +2025,14 @@ a numbered step. If outdented, it (today) is rendered as a bullet in the midst o
<dl class="switch">

: is set to {{UserVerificationRequirement/required}}
:: Let |userVerification| be [TRUE].
:: If <code>|options|.{{CredentialCreationOptions/mediation}}</code> is set to
<dl class="switch">
: {{CredentialMediationRequirement/conditional}}
:: throw a {{NotAllowedError}} {{DOMException}} unless it can be explicitly collected during the ceremony.

: empty or another value
:: Let |userVerification| be [TRUE].
</dl>
Copy link
Member

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:

Suggested change
:: If <code>|options|.{{CredentialCreationOptions/mediation}}</code> is set to
<dl class="switch">
: {{CredentialMediationRequirement/conditional}}
:: throw a {{NotAllowedError}} {{DOMException}} unless it can be explicitly collected during the ceremony.
: empty or another value
:: Let |userVerification| be [TRUE].
</dl>
:: 1. If <code>|options|.{{CredentialCreationOptions/mediation}}</code> is set to {{CredentialMediationRequirement/conditional}}
and [=user verification=] cannot be collected during the ceremony,
throw a {{NotAllowedError}} {{DOMException}}.
1. Let |userVerification| be [TRUE].

Copy link
Member

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 to ConstraintError" returned by the authenticator when UV is required but cannot be satisfied by that authenticator. Returning an error at all also differs from conditional mediation in get(), which simply hangs forever on most errors. Should conditional create do that too?

Copy link
Contributor Author

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.


: is set to {{UserVerificationRequirement/preferred}}
:: If the |authenticator|
Expand Down
Loading