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 the IDP sign-in status API to the spec #436

Merged
merged 47 commits into from
Oct 13, 2023
Merged
Changes from 7 commits
Commits
Show all changes
47 commits
Select commit Hold shift + click to select a range
e2f48cc
Add the IDP sign-in status API to the spec
cbiesinger Feb 14, 2023
8304968
Address comments
cbiesinger Feb 15, 2023
465c717
Add the JavaScript API
cbiesinger Feb 15, 2023
896e95d
void->undefined and add an Issue comment regarding Promise
cbiesinger Feb 15, 2023
07eb407
Slightly more direct way to get the origin
cbiesinger Feb 15, 2023
8948f99
Add a multi-IDP note and clarify a signin note
cbiesinger Feb 15, 2023
f19c445
Elaborate on state clearing
cbiesinger Feb 16, 2023
663f421
Merge remote-tracking branch 'upstream/main' into status2
cbiesinger Aug 11, 2023
090262d
Fix bikeshed syntax errors
cbiesinger Aug 11, 2023
d682586
Merge branch 'syntax' into status2
cbiesinger Aug 11, 2023
9557220
Merge remote-tracking branch 'upstream/main' into status2
cbiesinger Aug 11, 2023
0c154cb
define interaction with clear-site-data
cbiesinger Aug 11, 2023
d932e3f
better structure
cbiesinger Aug 11, 2023
f128898
less prescriptive about parameter location
cbiesinger Aug 11, 2023
71083c8
typo
cbiesinger Aug 11, 2023
3fbe365
period
cbiesinger Aug 11, 2023
238fdf4
fix merge error
cbiesinger Aug 11, 2023
13ca4f7
elaborate on cookie deletion
cbiesinger Aug 11, 2023
4db1336
Merge remote-tracking branch 'upstream/main' into status2
cbiesinger Aug 23, 2023
0c77ac9
review comments
cbiesinger Aug 23, 2023
afe7360
fix IDL syntax error
cbiesinger Aug 23, 2023
ddb3f24
automation
cbiesinger Aug 23, 2023
2098134
fix most comments from the PR
cbiesinger Sep 7, 2023
5b091a0
more editorial fixes
cbiesinger Sep 7, 2023
6a38c4b
add subheadings to signin status api section
cbiesinger Sep 7, 2023
654908f
specify IDP signin flow
cbiesinger Sep 8, 2023
652b436
Better specify popup creation
cbiesinger Sep 19, 2023
fb02431
Editorial comments from TallTed
cbiesinger Sep 20, 2023
d70fa1f
npm comments
cbiesinger Sep 20, 2023
620848c
update HTTP header section
cbiesinger Sep 20, 2023
b6b9be3
editorial fixes
cbiesinger Sep 20, 2023
436ee85
update IDL
cbiesinger Sep 20, 2023
3c8eb4a
fix typo
cbiesinger Sep 20, 2023
04e7156
better handle header origins
cbiesinger Sep 25, 2023
99147b3
clarify some traversable things
cbiesinger Sep 25, 2023
e7ab849
signin -> login
cbiesinger Sep 25, 2023
1fbab48
missed one
cbiesinger Sep 25, 2023
7f6eef5
npm comments
cbiesinger Sep 27, 2023
0f0c4fe
review comments and change signin to login
cbiesinger Sep 28, 2023
e80e54a
signin_url -> login_url
cbiesinger Sep 28, 2023
bf30c3d
replace a few more sign-ins
cbiesinger Sep 28, 2023
49fe2ad
linkify
cbiesinger Sep 28, 2023
2133a0b
review comments
cbiesinger Oct 3, 2023
d28201f
get a login status entry algorithm
cbiesinger Oct 3, 2023
0d55b05
review comments
cbiesinger Oct 10, 2023
60cdb08
review comments
cbiesinger Oct 12, 2023
48e4d1a
last? comments
cbiesinger Oct 13, 2023
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
104 changes: 104 additions & 0 deletions spec/index.bs
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,76 @@ value (which is used when a resource loaded as a third-party, not first-party).
for an [=IDP=] to adopt the FedCM API without introducing security issues on the API, since the
[=RP=] cannot inspect the results from the fetches in any way.

<!-- ============================================================ -->
## IDP Sign-in Status ## {#browser-api-idp-sign-in-status}
cbiesinger marked this conversation as resolved.
Show resolved Hide resolved
<!-- ============================================================ -->

Each [=user agent=] keeps a global, persistent <dfn>IDP Sign-in Status
cbiesinger marked this conversation as resolved.
Show resolved Hide resolved
map</dfn>, an initially empty [=map=]. The [=map/keys=] in this map are
[=/origin=] (of IDPs), and the [=map/values=] are enums that can be one of
Copy link
Collaborator

Choose a reason for hiding this comment

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

We need to find a way to store the "type=idp" here too.

In addition to the "type, ideally, we would also make this extensible, so that different "types" could store different things. Here is a possible use of the Login Status API:

https://github.com/samuelgoto/login-status-api#browser-status-ui

// Records that the user is logging-in to a FedCM-compliant Identity Provider.
navigator.setLoggedIn({
  profile: {
    name: "John Doe",
    picture: "https://website.com/john-doe/profile.png",
  }
});

So, maybe, we should have something that stores dict[origin] -≥ (status, dict[type] -> blob) ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Actually, as long as this is only used by FedCM, I think it's fine to only store something if type=idp was specified. The API should allow more but since the backend storage is abstract anyway and UAs will store it in a browser-specific way, I think there's no harm in pretending we only store type=idp data with no extra metadata.

cbiesinger marked this conversation as resolved.
Show resolved Hide resolved
"<dfn><code>unknown</code></dfn>", "<dfn><code>signed-in</code></dfn>",
and "<dfn><code>signed-out</code></dfn>"

The user agent checks all page and subresource loads for an HTTP response
[[RFC9110#header.fields|header]] named
<dfn><code>IdP-SignIn-Status</code></dfn>, whose value is parsed at HTTP
[[RFC9110#parameter|Parameter]]s.
If that header exists and the first parameter has a name of `action`,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Presumably this needs to be parsed? Can we perhaps encapsulate this in an algorithm

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I tried to clarify my intention here (the parsing is defined by the HTTP spec, is what I was trying to say)

the user agent must process it as follows:

cbiesinger marked this conversation as resolved.
Show resolved Hide resolved
* If this is a subresource request, and the response does not have the
Copy link
Collaborator

Choose a reason for hiding this comment

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

"this is a subresource request" is a bit unclear for inside an algorithm. I wonder if this should use https://fetch.spec.whatwg.org/#concept-request-mode, or something else?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I ended up dropping the subresource part because it's not necessary IMO

same [=/origin] as the toplevel page, ignore the header.
Copy link
Collaborator

Choose a reason for hiding this comment

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

What do you mean by the origin of the subresource? Presumably the final origin? I think this needs to be pulled from the response itself (I imagine the request may only have the initial origin)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done, I think, unless you had something else in mind for the wording?

cbiesinger marked this conversation as resolved.
Show resolved Hide resolved
* Otherwise:
* If the parameter value is `signin`, [=map/set=] an entry in the
[=IDP Sign-in Status map=] with the key being the origin of the resource
cbiesinger marked this conversation as resolved.
Show resolved Hide resolved
and the value being [=signed-in=]
* If the parameter value is `signout-all`, [=map/set=] an entry in the
[=IDP Sign-in Status map=] with the key being the origin of the resource
cbiesinger marked this conversation as resolved.
Show resolved Hide resolved
and the value being [=signed-out=]

Issue: Should the header checking be integrated into the fetch spec instead,
since it affects all resources?

User agents must also clear the [=IDP Sign-in Status map=] data when:
cbiesinger marked this conversation as resolved.
Show resolved Hide resolved
: the user clears all cookies or site settings data
:: The user agent must clear the entire map
cbiesinger marked this conversation as resolved.
Show resolved Hide resolved
: the user clears cookies or site data for a specific origin
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think what you mean here is that the user clears all cookies or site data which is accessible by a specific origin? That is, there is no way the user is still signed in to that origin. I'm not sure this is clear from the text

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

the intent was:
if the user clears all cookies for the origin google.com, we also reset the signin status for accounts.google.com, because it is possible that the login state was relying on a domain cookie for .google.com

Tried to clarify this...

:: The user agent must remote all entries that would be affected
by the deleted cookies.
Copy link
Contributor

Choose a reason for hiding this comment

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

This is the part that I think needs to be clearer. It might even need some more thought.

When you clear state for an RP, you probably just forget those identities (and the associated IdPs) that were used.

However, when you clear state for an IdP, there will be a bunch of RPs that have received identity information from the IdP. I see a two options, each with drawbacks.

  1. You could clear ALL state for any RPs as well on the basis that retaining state at the RP might lead to information the RP has could propagate to the IdP, re-establishing same-site recognition when it shouldn't (see below for an attack on this). The drawback here is that clearing state in one place has an wider effect than users might expect.
  2. You could clear just the login state, so that the RP would need to ask again. The attack here is that, if the user chooses to use the IdP again, the RP could remind the IdP of the user identity. The drawback is that now clearing state for the IdP ends up having unintended consequences. And though the reminding thing is a real problem, that only happens if the user gives permission to the RP to use the IdP again AND the RP state is not cleared AND the user intends to have a clean break. I don't believe that there is anything in FedCM that might leak the information prior to the user performing that grant (with the usual caveats about the effectiveness of cross-site information leakage protections).

Note that in the first case, if you allow for the possibility that each RP might also have established links to other IdPs, then you might also need to clear state for any of those RPs as well. It's a connected graph and the only way to make clean break is to burn out all the links. Of course, that might be even more surprising. At some point though, you need to recognize that this sort of purging needs to consider all forms of linkability, so you might want to stop before the only action you permit involves clearing all browser state.

I don't think that there is an easy cut to make between these two. I can see how different browsers might choose to take different approaches on the basis that they take different perspectives on how to empower their users. One option favours user control over privacy, the other favours convenience. For a specification, we generally try to document the considerations carefully and clearly, then leave these decisions to implementations. It is OK to do that because there are no guarantees on retention of site-level state, just as there are no firm rules about how private information is managed, so each browser gets to choose their own approach.

Note that I'm talking about a problem with broader scope than your change. You're the lucky one to have to contend with this.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, the more I think on this, the more I realize that this cuts in both directions. The only thing that might make the RP clearing case simpler is that we have a different perception of the IdP role in this process.

  • If the user deliberately chooses to reuse an identity, then the connection is remade, which is essentially an explicit request to re-establish state.

  • If the user chooses a different identity, they may or may not have an expectation that the IdP not share the fact that this RP previously had access to the previous identity.

For the latter, we already rely fairly heavily on the user making a trust decision with respect to the information the IdP might have about them. That is not a symmetric relationship with the RP. We don't assume that the user trusts the RP in the same way.

(Lots of stuff to write...)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have elaborated on the line you commented on to describe the intent I had.

As you say, I think a lot of what you brought up here is only tangentially related to the IDP signin status API (this PR) and applies to FedCM in general.

However, the way I think about it is that the connection between RP and IDP is more akin to a permission. Permissions in general don't get deleted when the user clears cookies... To be honest, I think it would be surprising to users if deleting (e.g.) google.com cookies logged them out of (e.g.) Pinterest and Tripit?

I think I'm going to file a github issue to discuss all this, because it should probably involve not just you and me :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I filed #493


Note: For example, domain cookies may affect subdomains of
the deleted origin.
: if the user agent allows deleting individual cookies
:: the behavior is user agent-defined.

Note: The user agent may want to reset the state to undefined,
cbiesinger marked this conversation as resolved.
Show resolved Hide resolved
since is impossible to know whether this cookie affects
authorization state.

Note: Website-initiated cookie changes should not affect this map. When
IDP sign-in state changes, it should send an explicit
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think that this is true: https://w3c.github.io/webappsec-clear-site-data/

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have added a section on interactions with Clear-Site-Data.

[=IDP-SignIn-Status=].
RP state should not affect this map since it only reflects IDP state.

IDPs can also use a JavaScript API to update the stored sign-in status:

<pre class="idl">
[Exposed=Window, SecureContext]
interface IdentityProvider {
static undefined recordSignIn();
static undefined recordSignOut();
};
</pre>

Issue: Should the functions return Promise<undefined>, so the caller can know
when the call is finished?

When {{IdentityProvider/recordSignIn}} or {{IdentityProvider/recordSignOut}} is
called, the user agent [=map/set|sets=] the entry in the [=IDP Sign-in Status
Map=] with the [=map/key=] being [=/this=]'s [=/relevant settings object=]'s
[=environment settings object/origin=] and the value being [=signed-in=] (for
`recordSignIn`) or [=signed-out=] (for `recordSignOut`).

<!-- ============================================================ -->
## The State Machine ## {#browser-api-state-machine}
<!-- ============================================================ -->
Expand Down Expand Up @@ -490,11 +560,44 @@ To <dfn>create an IdentityCredential</dfn> given an {{IdentityProviderConfig}}
|provider| and a |globalObject|, run the following steps. This returns an {{IdentityCredential}} or
failure.
1. Assert: These steps are running [=in parallel=].
1. Let |signinStatus| be the value of the entry in [=IDP Sign-in Status map=]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
1. Let |signinStatus| be the value of the entry in [=IDP Sign-in Status map=]
1. Let |signinStatus| be the value of the entry in [=Sign-in Status map=]

Copy link
Collaborator

Choose a reason for hiding this comment

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

Also need to get by "type=idp" here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not applicable with my note above

with the key being the origin of |provider|'s configURL. If there is
cbiesinger marked this conversation as resolved.
Show resolved Hide resolved
no such entry, set it to a user-agent specific default (either [=unknown=] or
[=signed-out=]).
1. If |signinStatus| is [=signed-out=], return failure.
Issue: Should the user agent instead show UI to let the user sign
cbiesinger marked this conversation as resolved.
Show resolved Hide resolved
in to the IDP? This could be especially useful in the multi-IDP
case.
1. Let |config| be the result of running [=fetch the config file=] with |provider| and
|globalObject|.
1. If |config| is failure, return failure.
1. Let |accountsList| be the result of [=fetch the accounts list=] with |config|, |provider|,
and |globalObject|.
1. If the fetch failed, or the size of |accountsList| is 0:
cbiesinger marked this conversation as resolved.
Show resolved Hide resolved
1. [=map/set|Set=] an entry in the [=IDP Sign-in Status map=] with the
cbiesinger marked this conversation as resolved.
Show resolved Hide resolved
key being the origin of the configURL and the value being [=signed-out=].
cbiesinger marked this conversation as resolved.
Show resolved Hide resolved
A user agent may decide to skip this step if no credentials were sent to
server.

Note: For example, if the fetch failed due to a DNS error, no
credentials were sent. In this situation, we do not know whether
the user is signed in or not and so we may not want to reset
cbiesinger marked this conversation as resolved.
Show resolved Hide resolved
the status.
1. If |signinStatus| is [=signed-in=], show a dialog to the user and return
cbiesinger marked this conversation as resolved.
Show resolved Hide resolved
failure.

Note: Since this situation happens when the browser expects the user
to be signed in, but the accounts fetch indicated that the user
is signed out, it is recommended that this dialog provides a
"sign in" button allowing the user to sign in to the IDP using
{{IdentityProviderAPIConfig/signin_url}}.

Note: This dialog ensures that silent tracking of the user is
impossible by always showing UI of some kind when credentials
were sent to the server.
1. If |signinStatus| is [=unknown=], [=map/set=] an entry in the [=IDP
sign-in status map=] with the key being the origin of the configURL and
the value being [=signed-in=].
1. For each |account| in |accountsList|:
1. If |account|["{{IdentityProviderAccount/picture}}"] is present,
[=fetch the account picture=] with |account| and |globalObject|.
Expand Down Expand Up @@ -675,6 +778,7 @@ dictionary IdentityProviderAPIConfig {
required USVString accounts_endpoint;
required USVString client_metadata_endpoint;
required USVString id_assertion_endpoint;
USVString signin_url;
IdentityProviderBranding branding;
};
</xmp>
Expand Down