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.

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 fedcm docs #23675

Merged
merged 14 commits into from
Mar 22, 2023
Merged

Add fedcm docs #23675

merged 14 commits into from
Mar 22, 2023

Conversation

chrisdavidmills
Copy link
Contributor

Description

This PR adds content for the FedCM API.

See my research document for more information on the exact changes required for this addition.

Motivation

Additional details

Related issues and pull requests

@chrisdavidmills chrisdavidmills requested review from a team as code owners January 16, 2023 12:14
@chrisdavidmills chrisdavidmills requested review from sideshowbarker and bsmth and removed request for a team January 16, 2023 12:14
@github-actions github-actions bot added Content:Glossary Glossary entries Content:WebAPI Web API docs labels Jan 16, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Jan 16, 2023

Preview URLs (10 pages)
Flaws (1)

Note! 9 documents with no flaws that don't need to be listed. 🎉

URL: /en-US/docs/Web/HTTP/Headers/Permissions-Policy
Title: Permissions-Policy
Flaw count: 1

  • broken_links:
    • No need for the pathname in anchor links if it's the same page
External URLs (10)

URL: /en-US/docs/Web/API/IdentityCredential
Title: IdentityCredential


URL: /en-US/docs/Web/API/IdentityCredential/token
Title: IdentityCredential.token


URL: /en-US/docs/Web/API/FedCM_API
Title: Federated Credential Management API (FedCM)


URL: /en-US/docs/Glossary/Replay_attack
Title: Replay attack

(comment last updated: 2023-03-22 07:40:13)

@chrisdavidmills chrisdavidmills marked this pull request as draft January 16, 2023 12:17
@github-actions github-actions bot added the Content:HTTP HTTP docs label Jan 16, 2023
@chrisdavidmills chrisdavidmills marked this pull request as ready for review January 16, 2023 13:59
@chrisdavidmills chrisdavidmills requested a review from a team as a code owner January 16, 2023 13:59
@chrisdavidmills chrisdavidmills requested review from teoli2003 and removed request for a team January 16, 2023 13:59
Copy link
Contributor

@agektmr agektmr left a comment

Choose a reason for hiding this comment

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

LGTM % nits


The well-known file must be served from the [eTLD+1](https://web.dev/same-site-same-origin/#site) of the IdP at `/.well-known/web-identity`.

For example, if the IdP endpoints are served under `https://accounts.idp.example/`, they must serve a well-known file at `https://idp.example/.well-known/web-identity`. The well-known file's content should look have the following JSON structure:
Copy link
Contributor

Choose a reason for hiding this comment

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

Is "should look have" a valid English expression I'm not familiar with?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Heh ... no, it's a typo ;-)

I've updated it to "should have" in my next commit.


The browser requests the IdP config file and carries out the sign-in flow detailed below. For more information on the kind of interaction a user might expect from the browser-supplied UI, see [Sign in to the relying party with the identity provider](https://developer.chrome.com/docs/privacy-sandbox/fedcm/#sign-into-rp).

### FedCM sign-in flow
Copy link
Contributor

Choose a reason for hiding this comment

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

I like how this section is separated from endpoint description and describes expected HTTP headers from RP point of view!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I like it too. I've basically copied the sign-in flow structure from one of your articles but added more details on what happens at each stage.


### Exceptions

- `NetworkError` {{domxref("DOMException")}}
- : In the case of a `get()` call with an `identity` option, this exception is thrown if the IdP does not respond within 60 seconds, or if the provided credentials are not valid/found.
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm curious, publicKey type accepts timeout value. What happens if more than 60 seconds is specified to the timeout?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good question. I've had a look through all the related specs, and can't find an answer. I'm guessing nothing, as the WebAuthn spec doesn't specify any kind of network/timeout error, and the timeout value is a hint indicating the time the caller is willing to wait, rather than a set value after which something will happen. Also, the two types don't seem to have any interplay at all.

Copy link
Contributor

Choose a reason for hiding this comment

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

OK, I noticed the concept of timeout is not part of Credential Management API and it's rather of PublicKeyCredential or IdentityCredential. So there's no contradiction. There might be a need to clarify what would happen when they are used together, but it's too early to worry about that. Please ignore this comment.

@chrisdavidmills
Copy link
Contributor Author

Thanks for the review @agektmr ! I've made updates according to your comments and provided responses above. Let me know if there is anything else you think these docs need.

@chrisdavidmills
Copy link
Contributor Author

Thanks @teoli2003 ; committed.

Copy link
Contributor

@agektmr agektmr left a comment

Choose a reason for hiding this comment

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

Added one change request. Otherwise it's ready go.

files/en-us/web/api/fedcm_api/index.md Outdated Show resolved Hide resolved

### Exceptions

- `NetworkError` {{domxref("DOMException")}}
- : In the case of a `get()` call with an `identity` option, this exception is thrown if the IdP does not respond within 60 seconds, or if the provided credentials are not valid/found.
Copy link
Contributor

Choose a reason for hiding this comment

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

OK, I noticed the concept of timeout is not part of Credential Management API and it's rather of PublicKeyCredential or IdentityCredential. So there's no contradiction. There might be a need to clarify what would happen when they are used together, but it's too early to worry about that. Please ignore this comment.

@chrisdavidmills
Copy link
Contributor Author

It looks like this one is ready to go. All reviewed, and updates made.

@teoli2003 you've already looked at this one. Can you approve and merge?

Copy link
Contributor

@teoli2003 teoli2003 left a comment

Choose a reason for hiding this comment

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

A few details, then it is good to go for me.

- Security
---

In web security, a _replay attack_ is when an attacker intercepts a previously-sent message and resends it later to get the same credentials as the original message, potentially with a different payload or instruction.
Copy link
Contributor

Choose a reason for hiding this comment

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

nit

Suggested change
In web security, a _replay attack_ is when an attacker intercepts a previously-sent message and resends it later to get the same credentials as the original message, potentially with a different payload or instruction.
In web security, a _replay attack_ happens when an attacker intercepts a previously-sent message and resends it later to get the same credentials as the original message, potentially with a different payload or instruction.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated in my next commit.

}
```

Check out [Federated Credential Management API (FedCM)](/en-US/docs/Web/API/FedCM_API#fedcm_sign-in_flow) for more details on how this works. This call will start off the sign-in flow described in [FedCM sign-in flow](http://localhost:5042/en-US/docs/Web/API/FedCM_API#fedcm_sign-in_flow).
Copy link
Contributor

@teoli2003 teoli2003 Mar 21, 2023

Choose a reason for hiding this comment

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

The 2nd link is wrong here:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The first one was also wrong ;-) Should have pointed to the top of the FedCM page. I've fixed them both now.

@@ -11,6 +11,8 @@ browser-compat: api.FederatedCredential

The **`FederatedCredential`** interface of the [Credential Management API](/en-US/docs/Web/API/Credential_Management_API) provides information about credentials from a federated identity provider. A federated identity provider is an entity that a website trusts to correctly authenticate a user, and that provides an API for that purpose. [OpenID Connect](https://openid.net/developers/specs/) is an example of a federated identity provider framework.

> **Note:** The newer [Federated Credential Management API (FedCM)](/en-US/docs/Web/API/FedCM_API) provides a more complete solution for handling identity federation in the browser, and uses the {{domxref("IdentityCredential")}} type.
Copy link
Contributor

@teoli2003 teoli2003 Mar 21, 2023

Choose a reason for hiding this comment

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

Does it means FederatedCredential is deprecated: if not, maybe remove newer.

Suggested change
> **Note:** The newer [Federated Credential Management API (FedCM)](/en-US/docs/Web/API/FedCM_API) provides a more complete solution for handling identity federation in the browser, and uses the {{domxref("IdentityCredential")}} type.
> **Note:** The [Federated Credential Management API (FedCM)](/en-US/docs/Web/API/FedCM_API) provides a more complete solution for handling identity federation in the browser, and uses the {{domxref("IdentityCredential")}} type.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure; removed. I seem to remember that it is marked for deprecation, but not officially at this time.


A successful {{domxref("CredentialsContainer.get", "navigator.credentials.get()")}} call that includes an `identity` option fulfills with an `IdentityCredential` instance.

Check out [Federated Credential Management API (FedCM)](/en-US/docs/Web/API/FedCM_API#fedcm_sign-in_flow) for more details on how this works. This call will start off the sign-in flow described in [FedCM sign-in flow](http://localhost:5042/en-US/docs/Web/API/FedCM_API#fedcm_sign-in_flow).
Copy link
Contributor

Choose a reason for hiding this comment

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

localhost!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

D'oh, schoolboy error! Again, I've fixed them both.


A successful {{domxref("CredentialsContainer.get", "navigator.credentials.get()")}} call that includes an `identity` option fulfills with an `IdentityCredential` instance, which can be used to access the token used to validate the sign-in.

Check out [Federated Credential Management API (FedCM)](/en-US/docs/Web/API/FedCM_API#fedcm_sign-in_flow) for more details on how this works. This call will start off the sign-in flow described in [FedCM sign-in flow](http://localhost:5042/en-US/docs/Web/API/FedCM_API#fedcm_sign-in_flow).
Copy link
Contributor

Choose a reason for hiding this comment

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

localhost...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

And fixed again ;-)

@chrisdavidmills
Copy link
Contributor Author

@teoli2003 thanks for the review. Fixes made as requested.

Copy link
Contributor

@teoli2003 teoli2003 left a comment

Choose a reason for hiding this comment

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

👍

We should raise a flow for https://localhost URLs as links.

@teoli2003 teoli2003 merged commit 1ac70b3 into mdn:main Mar 22, 2023
@chrisdavidmills chrisdavidmills deleted the add-fedcm-docs branch March 22, 2023 10:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Content:Glossary Glossary entries Content:HTTP HTTP docs Content:WebAPI Web API docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants