Specify the fields API #668
Conversation
|
Thanks @TallTed, addresssed your comments. |
| @@ -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. | |||
There was a problem hiding this comment.
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. ("is_auto_selected", |isAutoSelected|) | ||
| 1. If |fields| is not empty: | ||
| 1. Let |serializedFields| be the entries of |fields| concatenated with a comma ("`,`") |
There was a problem hiding this comment.
Hmm is this commonly called serialized, or perhaps rename this one?
| 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 |
There was a problem hiding this comment.
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.
| 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, |
There was a problem hiding this comment.
s/specified/present. Alternatively, I think we could use = ['name', 'email', 'picture'] in the IDL
| 1. Let |fields| be |provider|.{{IdentityProviderRequestOptions/fields}} or, if not specified, | ||
| `["name", "email", "picture"]`. | ||
|
|
||
| Note: Unspecified is different from an explicitly specified empty list. |
There was a problem hiding this comment.
s/unspecified/omitted
s/specified/present
Specified is generally used for spec'd so this is not the right wording
| 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=]: |
| 1. The user agent MAY use the {{IdentityCredentialRequestOptions/context}} to customize the | ||
| dialog shown. | ||
| 1. If |provider|.{{IdentityProviderRequestOptions/fields}} is not [=list/empty=]: | ||
| 1. If |metadata| is not failure, |metadata|["{{IdentityProviderClientMetadata/privacy_policy_url}}"] |
There was a problem hiding this comment.
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
| 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|. |
There was a problem hiding this comment.
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.
Bug: w3c-fedid/custom-requests#4
Preview | Diff