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

Jump to bottom

Specify the fields API #668

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Specify the fields API #668

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c8b28c8
Specify the fields API
cbiesinger Oct 28, 2024
5751d6c
Address comments from TallTed
cbiesinger Oct 29, 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
75 changes: 56 additions & 19 deletions spec/index.bs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -653,6 +653,7 @@ dictionary IdentityProviderRequestOptions : IdentityProviderConfig {
USVString nonce;
DOMString loginHint;
DOMString domainHint;
sequence<USVString> fields;
any params;
};
</xmp>
Expand Down Expand Up @@ -872,7 +873,7 @@ the exception thrown.
1. For each |acc| in |accountsList|:
1. If |acc| is [=eligible for auto reauthentication=] given |provider|, and |globalObject|,
set |registeredAccount| to |acc| and increase |numRegisteredAccounts| by 1.
1. Let |permission|, |disclosureTextShown|, and |isAutoSelected| be set to false.
1. Let |permission|, |permissionRequested|, and |isAutoSelected| be set to false.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do you like this new name better? This variable is true when the PP/TOS text is shown to the user. The user may still select an account, and I think that sort of counts as providing permission, so I find this new wording a bit confusing.

1. If |mediation| is not "{{CredentialMediationRequirement/required}}", |requiresUserMediation|
is false, and |numRegisteredAccounts| is equal to 1:
1. Set |account| to |registeredAccount| and |permission| to true. When doing this, the user
Expand All @@ -888,23 +889,24 @@ the exception thrown.
{{IdentityCredentialRequestOptions/context}} to customize the dialog.
1. Otherwise, let |permission| be the result of running [=request permission to sign-up=]
algorithm with |account|, |config|, |provider|, and |globalObject|. Also set
|disclosureTextShown| to true.
|permissionRequested| to true.
1. Otherwise:
1. Set |account| to the result of running the [=select an account=] from the
|accountsList|.
1. If |account| is failure, return (failure, true).
1. If [=compute the connection status=] of |account|, |provider| and |globalObject| is
[=compute the connection status/connected=], set |permission| to true.
[=compute the connection status/connected=], or if |provider|.
Copy link
Collaborator

Choose a reason for hiding this comment

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

This seems to imply that "Create a connection between the RP and the IdP account" is not invoked when fields is empty, is that correct? This is only invoked from request permission to signup so would be skipped.

{{IdentityProviderRequestOptions/fields}} is [=list/empty=], set |permission| to true.
1. Otherwise:
1. Let |permission| be the result of running the [=request permission to sign-up=]
algorithm with |account|, |config|, |provider|, and |globalObject|.
1. Set |disclosureTextShown| to true.
1. Set |permissionRequested| to true.
1. Wait until the [=user agent=]'s dialogs requesting for user choice or permission to be
closed, if any are created in the previous steps.
1. Assert: |account| is not null.
1. If |permission| is false, then return (failure, true).
1. Let |credential| be the result of running the [=fetch an identity assertion=] algorithm with
|account|'s {{IdentityProviderAccount/id}}, |disclosureTextShown|, |isAutoSelected|,
|account|'s {{IdentityProviderAccount/id}}, |permissionRequested|, |isAutoSelected|,
|provider|, |config|, and |globalObject|.
1. Return |credential|.
</div>
Expand Down Expand Up @@ -1193,18 +1195,32 @@ the token that will be provided to the [=RP=].

<div algorithm>
To <dfn>fetch an identity assertion</dfn> given a {{USVString}}
|accountId|, a boolean |disclosureTextShown|, a boolean |isAutoSelected|, an
|accountId|, a boolean |permissionRequested|, a boolean |isAutoSelected|, an
{{IdentityProviderRequestOptions}} |provider|, an {{IdentityProviderAPIConfig}} |config|,
and |globalObject|, run the following steps. This returns an {{IdentityCredential}} or failure.
1. Let |tokenUrl| be the result of [=computing the manifest URL=] given |provider|,
|config|["{{IdentityProviderAPIConfig/id_assertion_endpoint}}"], and |globalObject|.
1. If |tokenUrl| is failure, return failure.
1. Let |disclosureShownFor| and |fields| be the empty list.
1. If |permissionRequested| is true:
1. Set |fields| to |provider|.{{IdentityProviderRequestOptions/fields}}.
1. Set |disclosureShownFor| to the subset of strings in |fields| that are
in the [=list of recognized fields=].
1. Let |list| be a list with the following entries:
1. ("client_id", |provider|'s {{IdentityProviderConfig/clientId}})
1. ("nonce", |provider|'s {{IdentityProviderRequestOptions/nonce}})
1. ("account_id", |accountId|)
1. ("disclosure_text_shown", |disclosureTextShown|)
1. ("is_auto_selected", |isAutoSelected|)
1. If |fields| is not empty:
1. Let |serializedFields| be the entries of |fields| concatenated with a comma ("`,`")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmm is this commonly called serialized, or perhaps rename this one?

between elements.
1. Append ("fields", |serializedFields|) to |list|.
1. If |disclosureShownFor| is not empty:
1. Let |serializedDisclosure| be the entries of |disclosureShownFor| concatenated
with a comma ("`,`") between elements.
1. Append ("disclosure_shown_for", |serializedDisclosure|) to |list|.
1. If |disclosureShownFor| contains all of "name", "email", and "picture", append
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it actually the case that we only show disclosure text if all of these are present? My intuition was that it would be shown if at least one of these is present.

("disclosure_text_shown", true) to |list|.
1. If |provider|'s {{IdentityProviderRequestOptions/params}} is not empty:
1. Let |json| be the result of [=serializing a JavaScript value to a JSON string=]
with |provider|'s {{IdentityProviderRequestOptions/params}}.
Expand Down Expand Up @@ -1280,25 +1296,40 @@ The <a>request permission to sign-up</a> algorithm fetches the [=client metadata
waits for the user to grant permission to use the given account, and returns whether the user
granted permission or not.

<div algorithm>
<div algorithm="request permission to sign-up">
To <dfn>request permission to sign-up</dfn> the user with a given an {{IdentityProviderAccount}} |account|,
an {{IdentityProviderAPIConfig}} |config|, an {{IdentityProviderRequestOptions}} |provider|, and a
|globalObject|, run the following steps. This returns a boolean.
1. Assert: These steps are running [=in parallel=].
1. Let |metadata| be the result of running [=fetch the client metadata=] with |config|,
|provider|, and |globalObject|.
1. Let |fields| be |provider|.{{IdentityProviderRequestOptions/fields}} or, if not specified,
Copy link
Collaborator

Choose a reason for hiding this comment

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

s/specified/present. Alternatively, I think we could use = ['name', 'email', 'picture'] in the IDL

`["name", "email", "picture"]`.

Note: Unspecified is different from an explicitly specified empty list.
Copy link
Collaborator

Choose a reason for hiding this comment

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

s/unspecified/omitted
s/specified/present
Specified is generally used for spec'd so this is not the right wording

1. Prompt the user to gather explicit intent to create an account. The user agent MAY use the
{{IdentityProviderBranding}} to inform the style choices of its UI. Additionally:
1. If |metadata| is not failure, |metadata|["{{IdentityProviderClientMetadata/privacy_policy_url}}"]
is defined and the |provider|'s {{IdentityProviderConfig/clientId}} is not in the list of
|account|["{{IdentityProviderAccount/approved_clients}}"], then the user agent MUST display
the |metadata|["{{IdentityProviderClientMetadata/privacy_policy_url}}"] link.
1. If |metadata| is not failure, |metadata|["{{IdentityProviderClientMetadata/terms_of_service_url}}"]
is defined, and the |provider|'s {{IdentityProviderConfig/clientId}} is not in the list of
|account|["{{IdentityProviderAccount/approved_clients}}"], then the user agent MUST display
the |metadata|["{{IdentityProviderClientMetadata/terms_of_service_url}}"] link.
1. The user agent MAY use the {{IdentityCredentialRequestOptions/context}} to customize the
dialog shown.
1. If |provider|.{{IdentityProviderRequestOptions/fields}} is not [=list/empty=]:
Copy link
Collaborator

Choose a reason for hiding this comment

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

You can use |fields| here?

1. If |metadata| is not failure, |metadata|["{{IdentityProviderClientMetadata/privacy_policy_url}}"]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we perform the client metadata fetch in the case where fields is empty? Seems we would never use the result in this case based on the text

is defined, and the |provider|'s {{IdentityProviderConfig/clientId}} is not in the list of
|account|["{{IdentityProviderAccount/approved_clients}}"], then the user agent MUST display
the |metadata|["{{IdentityProviderClientMetadata/privacy_policy_url}}"] link.
1. If |metadata| is not failure, |metadata|["{{IdentityProviderClientMetadata/terms_of_service_url}}"]
is defined, and the |provider|'s {{IdentityProviderConfig/clientId}} is not in the list of
|account|["{{IdentityProviderAccount/approved_clients}}"], then the user agent MUST display
the |metadata|["{{IdentityProviderClientMetadata/terms_of_service_url}}"] link.
1. The user agent MUST prompt the user for permission to share the data in |fields|,
interpreting the strings in the <dfn>list of recognized fields</dfn> as follows:
: `"name"`
:: The user's name as given in {{IdentityProviderAccount}}.{{IdentityProviderAccount/name}}.
: `"email"`
:: The user's email address as given in {{IdentityProviderAccount}}.{{IdentityProviderAccount/email}}.
: `"picture"`
:: The user's profile picture as given in {{IdentityProviderAccount}}.{{IdentityProviderAccount/picture}}.

Any other string is ignored for forwards compatibility.
1. The user agent MAY use the {{IdentityCredentialRequestOptions/context}} to customize the
dialog shown.
1. If the user does not grant permission, return false.
1. [=Create a connection between the RP and the IdP account=] with |provider|, |account|, and
|globalObject|.
Expand Down Expand Up @@ -1923,6 +1954,12 @@ It will also contain the following parameters in the request body `application/x
with rp.example"), used by the [=request permission to sign-up=] algorithm for new users. It
is used as an assurance by the user agent to the [=IDP=] that it has indeed shown the terms
of service and privacy policy to the user in the cases where it is required to do so.
: <dfn>fields</dfn>
:: The list of fields that the [=RP=] has requested in {{IdentityProviderRequestOptions/fields}}.
: <dfn>disclosure_shown_for</dfn>
:: The list of fields that the user was prompted for. This can be a subset of
{{IdentityProviderRequestOptions/fields}} if a field is requested that is not in the [=list
of recognized fields=].
</dl>

For example:
Expand All @@ -1935,7 +1972,7 @@ Origin: https://rp.example/
Content-Type: application/x-www-form-urlencoded
Cookie: 0x23223
Sec-Fetch-Dest: webidentity
account_id=123&client_id=client1234&nonce=Ct60bD&disclosure_text_shown=true
account_id=123&client_id=client1234&nonce=Ct60bD&disclosure_text_shown=true&fields=name,email,picture&disclosure_shown_for=name,email,picture
```
</div>

Expand Down
Loading