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

Add "conditional mediation" #155

Merged
merged 14 commits into from
Dec 6, 2021
Merged

Add "conditional mediation" #155

Changes from 10 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
d40af4b
Add "conditional mediation"
equalsJeffH Feb 18, 2021
8009c48
merge from main and fix conflict
equalsJeffH Oct 18, 2021
0ad1ff8
Merge branch 'main' into jeffh-add-conditional-mediation-approach
equalsJeffH Oct 24, 2021
67f5689
it is {{conditional}} not [=conditional=]
equalsJeffH Oct 24, 2021
2209e8f
oops, it is really {{CredentialMediationRequirement/conditional}}
equalsJeffH Oct 24, 2021
3537786
Make several improvements
nsatragno Oct 25, 2021
9dc6bc8
Remove index.html
nsatragno Oct 25, 2021
9d9300b
Do not eat up errors.
nsatragno Oct 25, 2021
590437a
Merge branch 'main' into jeffh-add-conditional-mediation-approach
equalsJeffH Nov 1, 2021
c8c709e
Apply suggestions from code review
nsatragno Nov 1, 2021
b8624db
Address comments.
nsatragno Nov 4, 2021
146e74c
Apply suggestions from code review
nsatragno Nov 9, 2021
5d07770
Address comment.
nsatragno Nov 9, 2021
f970f51
merge from main and fix conflicts
equalsJeffH Nov 12, 2021
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
60 changes: 54 additions & 6 deletions index.bs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,7 @@ spec:css-syntax-3;
interface Credential {
readonly attribute USVString id;
readonly attribute DOMString type;
static boolean isConditionalMediationAvailable();
nsatragno marked this conversation as resolved.
Show resolved Hide resolved
};
</pre>
<div dfn-for="Credential">
Expand All @@ -297,6 +298,22 @@ spec:css-syntax-3;
:: This attribute's getter returns the value of the object's [=interface object=]'s
{{[[type]]}} slot, which specifies the [=credential type=] represented by this object.

: <dfn method>isConditionalMediationAvailable()</dfn>
:: Returns `true` if and only if the user agent supports the
{{CredentialMediationRequirement/conditional}} approach to
[[#mediation-requirements|mediation of credential requests]] for the [=credential type=],
`false` otherwise.

{{Credential}}'s default implementation of {{Credential/isConditionalMediationAvailable()}}:

<ol class="algorithm">
1. Return `false`.
</ol>

The specification for any [=credential type=] supporting {{CredentialMediationRequirement/conditional}} mediation must explicitly override this function to return `true`.
nsatragno marked this conversation as resolved.
Show resolved Hide resolved
Note: If this function is not present, {{CredentialMediationRequirement/conditional}}
mediation is not supported for the [=credential type=].
nsatragno marked this conversation as resolved.
Show resolved Hide resolved

: <dfn attribute>\[[type]]</dfn>
:: The {{Credential}} [=interface object=] has an internal slot named `[[type]]`, which
unsurprisingly contains a string representing the <dfn>credential type</dfn>. The slot's value
Expand Down Expand Up @@ -609,6 +626,7 @@ spec:css-syntax-3;
enum CredentialMediationRequirement {
"silent",
"optional",
"conditional",
"required"
};
</pre>
Expand All @@ -633,6 +651,24 @@ spec:css-syntax-3;
a user has just clicked "sign-in" for example, then they won't be surprised or confused to
see a [=credential chooser=] if necessary.

: <dfn>conditional</dfn>
:: Discovered credentials are presented to the user in a non-modal dialog along with an
indication of the origin which is requesting credentials. If the user makes a gesture
nsatragno marked this conversation as resolved.
Show resolved Hide resolved
outside of the dialog, the dialog closes without having the {{CredentialsContainer/get()}}
method return to the caller and without causing a user-visible error condition. If the user
nsatragno marked this conversation as resolved.
Show resolved Hide resolved
makes a gesture that selects a credential, that credential is returned to the caller. The
[=prevent silent access flag=] is treated as being `true` regardless of its actual value:
the {{CredentialMediationRequirement/conditional}} behavior always involves [=user
mediation=] of some sort if applicable credentials are discovered.

If no credentials are discovered, the user agent MAY prompt the user to take action in a way
that depends on the type of credential (e.g. to insert a device containing credentials).
Either way, the `get()` method MUST NOT return immediately with `null` to avoid revealing
the lack of applicable credentials to the website.
Copy link
Member

Choose a reason for hiding this comment

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

I'm nervous about having normative requirements in this section. They need to be reflected in the algorithm below regardless, and having them here too introduces the possibility for them to get out of sync.

Copy link
Member

Choose a reason for hiding this comment

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

👀

I think it's fine since the whole not returning empty-handed is critical to Conditional UI anyway. Either way I changed "return immediately" by "resolve immediately".

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, good point about the MUST NOT. Where does the "MAY prompt" happen? It seems like it'd fit just before the call to "ask the user to choose a Credential, but that doesn't call back to the particular credential types in the list.

This might be a pre-existing difference between the spec and implementations, in which case feel free to defer it to a separate PR.

Copy link
Member

Choose a reason for hiding this comment

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

Where does the "MAY prompt" happen?

I'm not sure I understand this.

It seems like it'd fit just before the call to "ask the user to choose a Credential,

We could definitely add a note there for credential types other than PublicKeyCredential.

that doesn't call back to the particular credential types in the list.

WebAuthn loosely specifies its own credential chooser on [[PublicKeyCredential/DiscoverFromExternalSource]], and there's a bunch of conditional mediation logic I'm adding there. The implementation matches the spec in a non obvious way: only one credential type is supported at a time, so CredMan's "ask the user to choose a Credential" does not actually prompt the user ("PublicKeyCredential" is always selected as the default) and the webauthn "picker" is shown instead (and the user being prompted to interact with their authenticator satisfies the mediation requests).

I think the way this PR deals with choosing credentials (and the spec before this PR) is correct, or at least good enough. Hopefully that makes sense?


Websites can discover the availability of {{CredentialMediationRequirement/conditional}}
mediation by querying {{Credential/isConditionalMediationAvailable()}} for the [=credential type=] of interest.
jyasskin marked this conversation as resolved.
Show resolved Hide resolved

: <dfn>required</dfn>
:: The user agent will not hand over credentials without [=user mediation=], even if the
[=prevent silent access flag=] is unset for an origin.
Expand Down Expand Up @@ -801,6 +837,9 @@ spec:css-syntax-3;
4. |options|.{{CredentialRequestOptions/mediation}} is not
"{{CredentialMediationRequirement/required}}".

5. |options|.{{CredentialRequestOptions/mediation}} is not
jyasskin marked this conversation as resolved.
Show resolved Hide resolved
"{{CredentialMediationRequirement/conditional}}".

ISSUE: This might be the wrong model. It would be nice to support a site that wished
to accept either username/passwords or webauthn-style credentials without forcing
a chooser for those users who use the former, and who wish to remain signed in.
Expand All @@ -812,18 +851,24 @@ spec:css-syntax-3;
5. Let |choice| be the result of <a abstract-op lt="ask to choose">asking the user to
choose a `Credential`</a>, given |options| and |credentials|.

6. If |choice| is `null` or a {{Credential}}, [=resolve=] |p| with |choice| and skip the
remaining steps.
6. If |choice| is `null` or a {{Credential}}, let |result| be |choice| and go to step 9.
nsatragno marked this conversation as resolved.
Show resolved Hide resolved

7. Assert: |choice| is an [=interface object=].

8. Let |result| be the result of executing |choice|'s
{{[[DiscoverFromExternalSource]](origin, options, sameOriginWithAncestors)}},
given |origin|, |options|, and |sameOriginWithAncestors|.

9. If |result| is a {{Credential}} or `null`, resolve |p| with |result|.
9. If |result| is a {{Credential}}, [=resolve=] |p| with |result|.

10. If |result| is an [=exception=], [=reject=] |p| with |result|.
nsatragno marked this conversation as resolved.
Show resolved Hide resolved

Otherwise, [=reject=] |p| with |result|.
11. If |result| is `null` and |options|.{{CredentialRequestOptions/mediation}} is not
{{CredentialMediationRequirement/conditional}}, [=resolve=] |p| with |result|.

Note: if |options|.{{CredentialRequestOptions/mediation}} is
{{CredentialMediationRequirement/conditional}} and a `null` credential is discovered,
promise |p| is not resolved.
jyasskin marked this conversation as resolved.
Show resolved Hide resolved

7. Return |p|.
</ol>
Expand Down Expand Up @@ -2120,7 +2165,7 @@ spec:css-syntax-3;
value is "{{Credential/[[discovery]]/credential store}}".
</div>

4. Extend {{CredentialRequestOptions}} with the options the new credential type needs to respond
5. Extend {{CredentialRequestOptions}} with the options the new credential type needs to respond
nsatragno marked this conversation as resolved.
Show resolved Hide resolved
reasonably to {{get()}}:

<div class="example">
Expand All @@ -2135,7 +2180,7 @@ spec:css-syntax-3;
</pre>
</div>

5. Extend {{CredentialCreationOptions}} with the data the new credential type needs to create
6. Extend {{CredentialCreationOptions}} with the data the new credential type needs to create
`Credential` objects in response to {{create()}}:

<div class="example">
Expand All @@ -2150,6 +2195,9 @@ spec:css-syntax-3;
</pre>
</div>

7. If the new credential type supports {{CredentialMediationRequirement/conditional}} [=user
mediation=], overload {{Credential/isConditionalMediationAvailable()}} and return `true`.
nsatragno marked this conversation as resolved.
Show resolved Hide resolved

You might also find that new primitives are necessary. For instance, you might want to return
many {{Credential}} objects rather than just one in some sort of complicated, multi-factor
sign-in process. That might be accomplished in a generic fashion by adding a `getAll()` method to
Expand Down