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

Add the IDP sign-in status API to the spec #436

Merged
merged 47 commits into from
Oct 13, 2023
Merged

Add the IDP sign-in status API to the spec #436

Changes from 3 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
90 changes: 46 additions & 44 deletions spec/index.bs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -316,14 +316,14 @@ for an [=IDP=] to adopt the FedCM API. It doesn't introduce security issues on t
[=RP=] cannot inspect the results from the fetches in any way.

<!-- ============================================================ -->
## The Login Status API ## {#browser-api-sign-in-status}
## The Login Status API ## {#browser-api-login-status}
<!-- ============================================================ -->

Issue: Reconcile this section with the
[Login Status API](https://github.com/privacycg/is-logged-in).
See also [this PR](https://github.com/privacycg/is-logged-in/pull/54).

### HTTP header API ### {#signin-status-http}
### HTTP header API ### {#login-status-http}

Issue: The HTTP header checking should move into the Fetch spec, since it
affects all resource loads.
Expand All @@ -339,7 +339,7 @@ be the result of [=get a structured field value=] from the response's header
list with name "<dfn><code>Set-Login</code></dfn>" and type "`item`". If |value| is not null,
process this header as follows:

<div algorithm="process the signin status header">
<div algorithm="process the login status header">
* If the request's [=request/current url=]'s [=/origin=] is not [=same origin=]
cbiesinger marked this conversation as resolved.
Show resolved Hide resolved
with the [=environment/top-level origin=], ignore the header.
* Otherwise:
Expand All @@ -354,9 +354,9 @@ process this header as follows:

</div>

### JavaScript API ### {#signin-status-javascript}
### JavaScript API ### {#login-status-javascript}

[=IDPs=] can also use a JavaScript API to update the stored sign-in status:
[=IDPs=] can also use a JavaScript API to update the stored login status:
cbiesinger marked this conversation as resolved.
Show resolved Hide resolved


<pre class="idl">
Expand All @@ -377,17 +377,17 @@ partial interface Navigator {

<div algorithm="setStatus">
When {{NavigatorLogin/setStatus()}} is called with argument |status|:
cbiesinger marked this conversation as resolved.
Show resolved Hide resolved
* If the [=/origin=] of the [=current settings object=] is not [=same origin=]
1. If the [=/origin=] of the [=current settings object=] is not [=same origin=]
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 care about A -embeds- B -embeds- A case?

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 think you're right that we should since in that situation the inner A only gets partitioned (or no) cookies and thus is not aware of the "real" toplevel login state. I have updated this (and also the HTTP header part)

with the [=top-level origin=], throw a {{SecurityError}} {{DOMException}}.
* Let |value| be [=logged-in=] if |status| is `"logged-in"` or [=logged-out=]
1. Let |value| be [=logged-in=] if |status| is `"logged-in"` or [=logged-out=]
if |status| is `"logged-out"`.
* [=map/set|Set=] the entry in the [=Login Status Map=] with the [=map/key=]
1. [=map/set|Set=] the entry in the [=Login Status Map=] with the [=map/key=]
cbiesinger marked this conversation as resolved.
Show resolved Hide resolved
being [=/this=]'s [=/relevant settings object=]'s
[=environment settings object/origin=] and the value being |value|.

</div>

### Clearing map data ### {#signin-status-clear-data}
### Clearing map data ### {#login-status-clear-data}
cbiesinger marked this conversation as resolved.
Show resolved Hide resolved

User agents must also clear the [=Login Status map=] data when:
cbiesinger marked this conversation as resolved.
Show resolved Hide resolved
: the user clears all cookies or site settings data
Expand All @@ -412,8 +412,7 @@ User agents must also clear the [=Login Status map=] data when:
the [=map/key=] is the input origin.

Note: Other website-initiated cookie changes should not affect this map. When
IDP sign-in state changes, it should send an explicit
[=Set-Login=] header.
IDP login state changes, it should send an explicit [=Set-Login=] header.
cbiesinger marked this conversation as resolved.
Show resolved Hide resolved
RP state should not affect this map since it only reflects IDP state.

<!-- ============================================================ -->
Expand Down Expand Up @@ -671,7 +670,7 @@ the exception thrown.
* Return (failure, false).
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this to allow UAs to reduce prompt spam?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes

* Prompt the user whether to continue. If the user continues, the user
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this so that we support button flows? Or is this strictly necessarily for dealing with the timing attack? If this is only to support button flows, can't we handle this case in a separate PR?

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 added it so that the following flow works reasonably, which I think is what Firefox wants to do:

  • UA prompts user to select an IDP before it does anything
  • user clicks the IDP
  • IDP is logged out
  • UA should probably should show something in response to that user clicks?

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 inside the "create an IdentityCredential" algorithm: nothing happens before this, right? That is, "UA prompts user to select an IDP before it does anything" isn't captured before we get to this point, right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

What do you mean? The potential IDP selector is handled on (new) line 623, already existing in the spec. "create an IdentityCredential" is called by DiscoverFromExternalSource.

agent SHOULD set |loginStatus| to [=unknown=]. This MAY include an
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
agent SHOULD set |loginStatus| to [=unknown=]. This MAY include an
agent SHOULD set |loginStatus| to [=unknown=]. This MUST include an

Isn't this a MUST? Otherwise, how can we move forward with the algorithm if the loginStatus is 'logged-out'?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Don't we also have to "wait" somewhere here until the user becomes 'logged-in'?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That happens if the user triggers the affordance to "show an IDP login dialog" and is encapsulated in that algorithm.

I don't think the SHOULD needs to be a MUST; the algorithm should work fine as-is, no? I left it that way because some user agents don't really want to use an unknown state.

affordance to [=show an IDP sign-in dialog=]. If the user cancels
affordance to [=show an IDP login dialog=]. If the user cancels
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: this requires parameter

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.

or that algorithm returns failure, return (failure, true).

Issue: We should perhaps provide a way to let the [=RP=] request that
Expand All @@ -694,15 +693,15 @@ the exception thrown.
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 and therefore the IDP did not learn
credentials were sent and therefore the [=IDP=] did not learn
the user's identity. 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 |loginStatus| is [=logged-in=], show a dialog to the user. The
contents of this dialog are defined by the user agent. This dialog
SHOULD provide an affordance for the user to trigger the [=show an
IDP sign-in dialog=] algorithm; this dialog is the <dfn>confirm
IDP sign-in dialog</dfn>.
1. <dfn>Mismatch dialog step</dfn>: If |loginStatus| is [=logged-in=], show a
dialog to the user. The contents of this dialog are defined by the user
agent. This dialog SHOULD provide an affordance for the user to trigger
the [=show an IDP login dialog=] algorithm; this dialog is the
cbiesinger marked this conversation as resolved.
Show resolved Hide resolved
<dfn>confirm IDP login dialog</dfn>.

Note: This situation happens when the browser expects the user
to be signed in, but the accounts fetch indicated that the user
Expand All @@ -712,17 +711,19 @@ the exception thrown.
impossible by always showing UI of some kind when credentials
were sent to the server.

* If the user closes the dialog, return (failure, true).
1. Wait until one of the following occurs:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does this need to be a substep of the mismatch dialog step? Or can it just be step 3 (the step after)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, it needs to be a substep, because all of this only happens if loginStatus is logged-in.


* If the [=show an IDP sign-in dialog=] algorithm was triggered:
* If the user closes the dialog, return (failure, true).
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why does this return failure immediately?

This allows a user-visible read of the logged in bit.

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 sure I understand, since this requires the user to do something the timing is effectively random, indistinguishable from the delayed promise rejection?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Rejecting in a user-specified time is different than rejecting after a fixed 120s. So by seeing a rejection before that point, the IDP can determine they were not "logged-in".

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah, as of c8609ec the 120s has been replaced with a random timer.


1. Let |result| be the result of that algorithm.
1. If |result| is failure, return (failure, true). The user
agent MAY show a dialog to the user before or after
returning failure indicating this failure.
1. Otherwise, go back to the [=fetch config step=]. As an
optimization, the user agent MAY instead go to the
[=fetch accounts list step=].
* If the [=show an IDP login dialog=] algorithm was triggered:

1. Let |result| be the result of that algorithm.
1. If |result| is failure, return (failure, true). The user
agent MAY show a dialog to the user before or after
returning failure indicating this failure.
1. Otherwise, go back to the [=fetch config step=]. As an
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's pick one 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.

Done.

optimization, the user agent MAY instead go to the
[=fetch accounts list step=].

1. Assert: |accountsList| is not failure and the size of |accountsList| is not 0.
1. [=map/Set=] an entry in the [=login status map=] with the key being the
Expand All @@ -732,6 +733,7 @@ the exception thrown.
1. For every |account| in |accountList|, remove |account| from |accountList| if |account|'s
cbiesinger marked this conversation as resolved.
Show resolved Hide resolved
{{IdentityProviderAccount/login_hints}} does not [=list/contain=] |provider|'s
{{IdentityProviderConfig/loginHint}}.
1. If |accountList| is now empty, go to the [=mismatch dialog step=].
1. For each |acc| in |accountsList|:
1. If |acc|["{{IdentityProviderAccount/picture}}"] is present, [=fetch the account picture=]
with |acc| and |globalObject|.
Expand Down Expand Up @@ -923,7 +925,7 @@ dictionary IdentityProviderAPIConfig {
required USVString accounts_endpoint;
required USVString client_metadata_endpoint;
required USVString id_assertion_endpoint;
USVString signin_url;
USVString login_url;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is this not required like the other ones? Not that it matters since we don't really use this in JS but still

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good point, done.

IdentityProviderBranding branding;
};
</xmp>
Expand Down Expand Up @@ -1283,10 +1285,10 @@ and a |responseBody|, run the following steps. This returns an [=ordered map=].
</div>

<div algorithm>
To <dfn>show an IDP sign-in dialog</dfn> given an {{IdentityProviderAPIConfig}} |config|, run
To <dfn>show an IDP login dialog</dfn> given an {{IdentityProviderAPIConfig}} |config|, run
Copy link
Collaborator

Choose a reason for hiding this comment

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

This algorithm doesn't work for redirect login flows, correct?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It should work for same-origin redirects since all that is required is that the close() call (and the status setting) is from the same origin. Is there a use case for cross-origin redirect flows?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I meant for instances where the login dialog is in the same navigatable, and then navigate the user back to this page. I think I remember that being out of scope 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.

I'm not positive I understand you correctly but I think that's out of scope yeah

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess this refers to flow that require cross origin navigation, for technical reasons or cases with authentication intermediaries which are currently not properly covered by this

the following steps. This returns success or failure.
1. [=Create a fresh top-level traversable=] with URL
Copy link
Collaborator

Choose a reason for hiding this comment

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

What if the URL is null?

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 added a validation step to fetching the config file to ensure that signin_url is also valid here. I also think that this represents the desired semantics better (don't accept the config file if there is no login_url even if login_url is not triggered during this specific request)

|config|.{{IdentityProviderAPIConfig/signin_url}}.
|config|.{{IdentityProviderAPIConfig/login_url}}.
1. The user agent MAY [=set up browsing context features=] or otherwise
affect the presentation of this traversable in an implementation-defined
way.
Expand All @@ -1296,11 +1298,11 @@ the following steps. This returns success or failure.
context of this new traversable:
1. Close the traversable.
1. Let |loginStatus| be the value of the entry in [=Login Status map=]
with the key being the origin of the {{IdentityProviderAPIConfig/signin_url}}.
with the key being the origin of the {{IdentityProviderAPIConfig/login_url}}.

Note: The IDP sign-in flow may set this value to logged-in using
either the [[#signin-status-javascript|JavaScript]] or
[[#signin-status-http|HTTP header]] API during the signin
Note: The IDP login flow may set this value to logged-in using
either the [[#login-status-javascript|JavaScript]] or
[[#login-status-http|HTTP header]] API during the login
flow. It is also possible that this change happened in
a different browsing context.
1. If |loginStatus| is [=logged-in=], return success.
Expand Down Expand Up @@ -1332,13 +1334,13 @@ Issue: [Decide](https://github.com/fedidcg/FedCM/issues/476) whether {{IdentityP
correct location for the {{IdentityProvider/getUserInfo()}} method.

A {{IdentityProvider/close}} function is provided to signal to the browser that
the signin flow is finished. The reason for this function in addition to the
header is that even when the user is already logged in, the signin flow may not
the login flow is finished. The reason for this function in addition to the
header is that even when the user is already logged in, the login flow may not
be finished yet; for example, an [=IDP=] may want to prompt the user to verify
their phone number. To allow for such flows, the [=IDP=] must call
{{IdentityProvider/close}} when the flow is fully done.

See the [=show an IDP sign-in dialog=] algorithm for more details.
See the [=show an IDP login dialog=] algorithm for more details.

An {{IdentityUserInfo}} represents user account information from a user. This information is exposed
to the [=IDP=] once the user has already used the FedCM API to login in the [=RP=]. That is, it is
Expand Down Expand Up @@ -1948,9 +1950,9 @@ The [=remote end steps=] are:

1. Return [=success=] with data `null`.

## Confirm IDP signin ## {#webdriver-confirmidpsignin}
## Confirm IDP login ## {#webdriver-confirmidplogin}

<figure id="table-webdriver-confirmidpsignin" class="table">
<figure id="table-webdriver-confirmidplogin" class="table">
<table class="data">
<thead>
<tr>
Expand All @@ -1961,7 +1963,7 @@ The [=remote end steps=] are:
<tbody>
<tr>
<td>POST</td>
<td>`/session/{session id}/fedcm/confirmidpsignin`</td>
<td>`/session/{session id}/fedcm/confirmidplogin`</td>
</tr>
</tbody>
</table>
Expand All @@ -1970,11 +1972,11 @@ The [=remote end steps=] are:
The [=remote end steps=] are:

1. If no FedCM dialog is currently open, or the dialog is not a [=confirm IDP
sign-in dialog=] return a [=error|WebDriver error=] with [=error code=]
login dialog=] return a [=error|WebDriver error=] with [=error code=]
[=no such alert=].

1. Act as if the user had clicked the "continue" button in the dialog and
initiate the signin flow.
initiate the login flow.

1. Return [=success=] with data `null`.

Expand Down Expand Up @@ -2089,8 +2091,8 @@ The [=remote end steps=] are:
[=error code=] [=no such alert=].

1. Let |type| be a string that is "`AutoReauthn`" if the user is being [=auto-reauthenticated=],
or "`AccountChooser`" is the dialog is an account chooser, or "`ConfirmIdpSignin`" if the
dialog is a [=confirm IDP sign-in dialog=].
or "`AccountChooser`" is the dialog is an account chooser, or "`ConfirmIdpLogin`" if the
dialog is a [=confirm IDP login dialog=].

1. Return [=success=] with data |type|.

Expand Down
Loading