-
Notifications
You must be signed in to change notification settings - Fork 22.8k
Editorial review: Document recent FedCM additions, Chrome 122-136 #40411
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
Editorial review: Document recent FedCM additions, Chrome 122-136 #40411
Conversation
@chrisdavidmills Can you ping me or/and Will when you're ready for the editorial review. |
- `loginHint` {{optional_inline}} | ||
- : A string providing a hint about the account option(s) the browser should provide for the user to sign in with. This is useful in cases where the user has already signed in and the site asks them to reauthenticate. Otherwise, the reauthentication process can be confusing when a user has multiple accounts and can't remember which one they used to sign in previously. The value for the `loginHint` property can be taken from the user's previous sign-in, and is matched against the `login_hints` values provided by the IdP in the array of user information returned from the IdP's [accounts list endpoint](/en-US/docs/Web/API/FedCM_API/IDP_integration#the_accounts_list_endpoint). | ||
- `nonce` {{optional_inline}} | ||
- : A random string that can be included to ensure the response is issued specifically for this request and prevent {{glossary("replay attack", "replay attacks")}}. | ||
|
||
> [!NOTE] | ||
> Currently FedCM only allows the API to be invoked with a single IdP, i.e., the `providers` array has to have a length of 1. Multiple IdPs must be supported via different `get()` calls. |
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.
FedCM still only supports a single get()
(subsequent get requests will fail), but it can support multiple IdPs in the providers
array. Maybe it's worthy to call it out?
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.
Possibly, but this isn't really the right place to do so. A better place would be the get()
page "Exceptions" section. But then, that raises more questions:
- What exactly is the nature of the failure? Is it simply that a
get()
call cannot be make when a previousget()
call promise remains unresolved? Or is it that you can only be signed in using one federated identity provider per domain, and all other galls will fail until the existing sign-in is signed out again? Is it both? - I'm assuming the exception type would be an
InvalidStateError
, but I could be wrong. - Is this the case for all
get()
call types, or just for FedCM/identity types?
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.
I think I'm going to ask for an editorial review of this PR now, but feel free to chime in more on this issue. Thanks for the review, @ewewraw!
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.
FWIW I agree with you here Chris (unless I misunderstand the point). The existing note is incorrect because you can have multiple providers. More information needed on how get()
fails, but likely and exception on get, and not something to mention in this particular interface.
This one is ready for an editorial review, @hamishwillee and @wbamberg. Whoever fancies it. |
Suggest asking BCD for how to do this. You might need to invent a way. |
files/en-us/web/api/identitycredential/disconnect_static/index.md
Outdated
Show resolved
Hide resolved
files/en-us/web/api/identitycredential/disconnect_static/index.md
Outdated
Show resolved
Hide resolved
files/en-us/web/api/identitycredential/disconnect_static/index.md
Outdated
Show resolved
Hide resolved
files/en-us/web/api/identitycredential/disconnect_static/index.md
Outdated
Show resolved
Hide resolved
As long as people can work out the support story then fine. |
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.
Thanks for the comprehensive changes in response. Looks great. A few nits. I'd quite like to resolve #40411 (comment) before merging.
Co-authored-by: Natalia Markoborodova <50494110+ewewraw@users.noreply.github.com>
Co-authored-by: Hamish Willee <hamishwillee@gmail.com>
Co-authored-by: Hamish Willee <hamishwillee@gmail.com>
Co-authored-by: Hamish Willee <hamishwillee@gmail.com>
Co-authored-by: Hamish Willee <hamishwillee@gmail.com>
Co-authored-by: Hamish Willee <hamishwillee@gmail.com>
Co-authored-by: Hamish Willee <hamishwillee@gmail.com>
Co-authored-by: Hamish Willee <hamishwillee@gmail.com>
Co-authored-by: Hamish Willee <hamishwillee@gmail.com>
Co-authored-by: Hamish Willee <hamishwillee@gmail.com>
951e306
to
367235a
Compare
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.
Thanks very much @chrisdavidmills - looks excellent. Approving.
Super, thanks for the awesome review, @hamishwillee! |
Description
Chrome has added several FedCM features that we are missing from MDN. This PR adds docs for them.
The features are:
get()
call, as specified inside theproviders
array, added in Chrome 136: https://chromestatus.com/feature/5067784766095360IdentityCredential.configURL
(if you look at the associated spec changes)IdentityCredential.disconnect()
, added in Chrome 122: https://chromestatus.com/feature/5202286040580096.identity.mode
inget()
calls, added in Chrome 132: https://chromestatus.com/feature/4689551782313984.identity.fields
andidentity.params
inget()
calls, added in Chrome 132. I'm not sure what ChromeStatus entry covers these, but developer.chrome.com says they were added in Chrome 132.account_label
/label_hints
in the IdP config file / accounts list endpoint. This was added in Chrome 132. Note that this won't have browser compat data - we don't really have a way of recording data for features in endpoints related to an API.supports_use_other_account
in the IdP config file.domain_hints
in the accounts list endpoint anddomainHint
inget()
calls.One point I'd especially like the tech review to comment on — the specified options object for
disconnect()
does not match up with the description in the Chrome docs. I think the latter makes more sense, but I'd like to know what is going on here.Motivation
Additional details
Related issues and pull requests