-
Notifications
You must be signed in to change notification settings - Fork 32
Define "present credential requests" algorithm. #419
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
base: main
Are you sure you want to change the base?
Changes from all commits
db99148
6f2ab6e
4a2fb73
fd2b479
a98d215
01af9ee
81b1aea
53206fd
a63ee13
c252aeb
684b38b
fa246e6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -779,7 +779,124 @@ <h3> | |
| </h3> | ||
| <p> | ||
| To be written. | ||
| </p><!-- | ||
| // MARK: Present the credential request | ||
| --> | ||
| <h3> | ||
| Present the credential request | ||
| </h3> | ||
| <p> | ||
| To <dfn data-dfn-for="credential request coordinator">present the | ||
| credential request</dfn> given a [=Document=] |document|, a [=sequence=] | ||
| of validated credential requests |validatedRequests|, and an optional | ||
| {{AbortSignal}} |signal|: | ||
| </p> | ||
| <ol class="algorithm"> | ||
| <li>Let |requestData| be [=struct=] consisting of |validatedRequests|, | ||
| |document|'s [=relevant settings object=]'s [=environment settings | ||
| object/origin=], and |document|'s [=Document/origin=]. | ||
| </li> | ||
| <li>Present a [=credential chooser=] with |requestData| and wait for the | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The credential choose linked here is the CredMan chooser which is different from the chooser intended by this algorithm I assume.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok, yeah... this is where the "digital wallet" concept would have helped.... I guess even then, it would still be "show a UI to select a digital wallet with request data" and the digital wallet itself is a [=credential chooser=]. Could you help me with the wording here, @mohamedamir ... or we can come up with something together when we next chat?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here is one proposal: And we can then refer to that one in our spec. Happy to discuss/brainstorm in our weekly as well
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep, I'm fine with that.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We might need to clarify what happens when there's multiple matching requests or equivalent requests that are valid to present. |
||
| user to either: | ||
| <ul> | ||
| <li>Select a [=digital credential=] and [=holder=] that can fulfill | ||
| the request, or | ||
| </li> | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Discussed with @mohamedamir, we are going to slot in an issue here for "Presenting with CTAP #456" so it's clear when CTAP comes into play. And we can craft text around the requirements for cross device / cross platform support here of CTAP and potentially the protocols. note that this is independent of "Mandate one presentation protocol MUST be supported" #454. |
||
| <li>Cancel the operation. | ||
| </li> | ||
| </ul> | ||
| </li> | ||
|
Comment on lines
+799
to
+808
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The text below this makes it clear that we might exit the credential chooser due to the abort controller or an error. Those cases should be mentioned here.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point, @jschanck... I'll describe them. |
||
| <li>If |signal| was passed and |signal| is [=AbortSignal/aborted=]: | ||
| <ol> | ||
| <li>Return. | ||
| <p class="note" title="Abort already handled"> | ||
| The algorithm [=AbortSignal/add|added=] to |signal| by the | ||
| [=credential request coordinator/prepare credential requests=] | ||
| steps handle tearing down the [=credential chooser=]. | ||
| </p> | ||
| </li> | ||
| </ol> | ||
| </li> | ||
| <li>If the user cancels the operation or no credential was selected: | ||
| <ol> | ||
| <li>Let |error| be a newly created {{"AbortError"}} {{DOMException}}. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In both cases, "the operation was aborted [by the user]." I don't think we should distinguish them. User cancelling the operation is generally an
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I disagree :-) and even more explicit in the level 3 And Firefox even fixed a bug related to that for webauthn. CredMan however, returns But I do think we shouldn't mix both, user cancelling and developer cancelling the request
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This spec is not Web Auth and we can do better. Using a catchall is not a good thing. It means developers will rely on error messages. People criticize Cred Man and Web Auth AFAIK for not using a range of exceptions. We can elevate this to the TAG if need be, but seems like a not great use of time. I’m strongly of the opinion that this should be an AbortError. We have a great range of exception types for a reason - we should use them. |
||
| </li> | ||
| <li>[=credential request coordinator/Complete credential request | ||
| with=] |error|. | ||
| </li> | ||
| <li>Return. | ||
| </li> | ||
| </ol> | ||
| </li> | ||
| <li>If the platform returns a platform-specific error: | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is the first mention of the platform, right?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You are right. However, presenting the UI and so on was presumedly done by the platform too... should clarify that. For what it's worth, in WebKit, we have the following errors coming back potentially from the OS (from the "Identity Document Framework", which is provided by the OS/platform): switch (error.code) {
case WKIdentityDocumentPresentmentErrorNotEntitled:
exceptionData = { ExceptionCode::NotAllowedError, "Not allowed because not entitled."_s };
break;
case WKIdentityDocumentPresentmentErrorInvalidRequest:
exceptionData = { ExceptionCode::TypeError, "Invalid request."_s };
break;
case WKIdentityDocumentPresentmentErrorRequestInProgress:
exceptionData = { ExceptionCode::InvalidStateError, "Request already in progress."_s };
break;
case WKIdentityDocumentPresentmentErrorCancelled:
exceptionData = { ExceptionCode::AbortError, "Request was cancelled."_s };
break;
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Apart from the AbortError being discussed in another comment thread, I think it's always the platform that does the credential selection. WDYT?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, that makes sense. Open to suggestions or can try to clarify things.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. discussed with @mohamedamir ... will switch For default case where the error is unknown, we should default to "OperationError". I'll rewrite these in a general form and @mohamedamir will check if they align with Android's platform errors. |
||
| <ol> | ||
| <li>Let |error| be the appropriate {{DOMException}} for that error. | ||
| </li> | ||
| <li>[=credential request coordinator/Complete credential request | ||
| with=] |error|. | ||
| </li> | ||
| <li>Return. | ||
| </li> | ||
| </ol> | ||
| </li> | ||
| <li>If a [=digital credential=] was selected by the user: | ||
| <ol> | ||
| <li>Let |responseData| be a [=string=] [=digital | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this guaranteed to be a string?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it has to be a string, as we have to:
|
||
| credential/presentation response=] or [=string=] [=digital | ||
| credential/issuance response=] returned by the [=holder=]. | ||
| </li> | ||
| <li>Let |parsedDataOrError| be the result of [=parse a JSON string to | ||
| a JavaScript value=] passing |responseData|. | ||
| </li> | ||
| <li>If |parsedDataOrError| is an [=exception=]: | ||
| <ol> | ||
| <li>[=credential request coordinator/Complete credential request | ||
| with=] |parsedDataOrError|. | ||
| </li> | ||
| <li>Return. | ||
| </li> | ||
| </ol> | ||
| </li> | ||
| <li>If |parsedDataOrError| is not an [=object=]: | ||
| <ol> | ||
| <li>Let |error| be a newly created {{TypeError}}. | ||
| </li> | ||
| <li>[=credential request coordinator/Complete credential request | ||
| with=] |error|. | ||
| </li> | ||
| <li>Return. | ||
| </li> | ||
| </ol> | ||
| </li> | ||
| <li>Let |protocol:DigitalCredentialProtocol| be the | ||
| {{DigitalCredentialProtocol}} [=enumeration value=] that matches the | ||
| [=digital credential/protocol identifier=] used in the exchange. | ||
| </li> | ||
| <li>Let |credential| be a newly created {{DigitalCredential}} | ||
| instance with its {{DigitalCredential/data}} initialized to | ||
| |parsedDataOrError| and its {{DigitalCredential/protocol}} | ||
| initialized to |protocol|. | ||
| </li> | ||
| <li>[=Queue a global task=] on the [=DOM manipulation task source=] | ||
| given |document|'s [=relevant global object=] to perform the | ||
| following steps: | ||
| <ol> | ||
| <li>Set the [=credential request coordinator=] [=credential | ||
| request coordinator/interaction state=] to "[=credential request | ||
| coordinator/idle=]". | ||
| </li> | ||
| <li>[=Resolve=] the [=credential request coordinator=]'s | ||
| [=credential request coordinator/active promise=] with | ||
| |credential|. | ||
| </li> | ||
| <li>Set the [=credential request coordinator=]'s [=credential | ||
| request coordinator/active promise=] to `null`. | ||
| </li> | ||
| </ol> | ||
| </li> | ||
| </ol> | ||
| </li> | ||
| </ol> | ||
| <h3> | ||
| Complete credential request with error | ||
| </h3> | ||
|
|
@@ -828,13 +945,7 @@ <h3> | |
| </li> | ||
| </ol> | ||
| </li> | ||
| </ol> | ||
| <h3> | ||
| Present the credential request | ||
| </h3> | ||
| <p> | ||
| To be written. | ||
| </p><!-- | ||
| </ol><!-- | ||
| // MARK: The Digital Credentials API | ||
| --> | ||
| <h2> | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
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.
Discussed with @mohamedamir and @simoneonofri... I'm going to split this out into two variables, so it's more clear.
In WebKit, this looks like: