Editorial suggestions for spec/index.bs

[bvandersloot-mozilla]

There are several changes I think would improve the quality of the specification. Rather than filing them as independent issues, I am collecting them here. I am willing to file PRs for these as one or several stages if you like.

The goal here is to "tighten up" the spec to be more managable and to narrow the scope of the spec itself so that there is less non-normative and non-behavioral information here. This is with eyes toward moving out of a descriptive and toward a prescriptive document.

1. Remove or provide explanation in text for the typogram in the Introduction
2. Is the last sentence in Section 1.0 still true? Does this API provide a way to revoke tokens?
3. Move Use Cases Section to a different file: perhaps the explainer or a new file?
4. Move the example from the Examples section to the Introduction.
5. Remove the Terminology section and instead define needed terms inline. For example, it would be useful in the Introduction to know what an IDP and RP are by reading prose and without having to follow a link. Several are only used in that section and can just be dropped. Others are technical terms to the spec and should be defined in the appropriate section (e.g. registered and unregistered)
6. Move the High Level Design to the introduction.
7. Move the Identity Provider API section to after the Browser API section and re-name it something like "Identity Provider Server Endpoints" to make clear that these aren't implemented in the browser and the implementation requirements aren't fully defined here.
8. Define the IDL dictionaries from the Identity Provider API inline as they are used in the Browser API section.
9. Add a note for the nonce in the Identity Assertions subsection similar to the client_id
10. Remove the Relying Party API section.
11. Either remove references to the token being a JWT rather than an opaque token, or require it to be a JWT normatively.
12. In The Browser API section, the "three things" should just be two: introduce a new credential and introduce a new state machine in the state machine map. The other two things in the list currently are just part of introducing a new Credential, per the Credential Management API.
13. Move the state machine's algorithms (compute account state and sign-in) to its section. This may be easier if the state machine section is moved after the DiscoverFromExternalSource section.
14. I've heard conflicting information about the Logout API. If it is a core part of FedCM, move it to its own subsection (currently that would be a peer with the DiscoverFromExternalSource section. If not, move it to a proposal markdown file elsewhere.
15. Note that "Display an account chooser..." in select an account is non-normative.
16. In the Privacy Principals section, combine the assumption list into the actor list. Also, if possible, edit the actors' descriptions to be precise threat models.
17. Move the Privacy > Network requests table into the discussion of the IDP APIs for clarity
18. Move the next paragraph "For fetches that are sent with cookies..." somewhere into the Browser API section to normatively describe that we are sending first-party cookies.
19. Replace the remainder of the Privacy > Network requests section with a discussion of the invariants we want to hold (e.g. no credentialed requests without the user acknowledging the IDP as such, no referer until the user selects an account, etc.)
20. Move timing attack mitigations to another document
21. Move other attack scenarios (currently 10.4.*) to be 10.3.[4-7].

Acknowledging in advance that this is a big list of non-trivial changes!

[npm1]

Hi Ben, thanks a lot for putting together this list of suggestions! They generally sound good to me, and I think this will help the spec be more navigable for future implementers :) I look forward to all of these spec improvements!

There are several changes I think would improve the quality of the specification. Rather than filing them as independent issues, I am collecting them here. I am willing to file PRs for these as one or several stages if you like.

I think either way is fine. Since they are editorial changes, it seems fine to have them lumped in a larger PR.

2. Is the last sentence in Section 1.0 still true? Does this API provide a way to revoke tokens?

We removed that from the spec, so currently not true and we can remove. However note that we do have revocation as part of our future proposals.

3. Move Use Cases Section to a different file: perhaps the explainer or a new file?

SGTM (plus revocation section could also be removed for now to align the use-cases with what we have in the spec).

5. Remove the Terminology section and instead define needed terms inline. For example, it would be useful in the Introduction to know what an IDP and RP are by reading prose and without having to follow a link. Several are only used in that section and can just be dropped. Others are technical terms to the spec and should be defined in the appropriate section (e.g. registered and unregistered)

+1 I never understood why we need a large terminology section in the spec.

11. Either remove references to the token being a JWT rather than an opaque token, or require it to be a JWT normatively.

I think I only saw one reference. We do not check the format of the token in Chrome, so I would prefer that we remove the reference.

12. In The Browser API section, the "three things" should just be two: introduce a new credential and introduce a new state machine in the state machine map. The other two things in the list currently are just part of introducing a new Credential, per the Credential Management API.

I mean IMO that 'three things' part can be removed altogether.

13. Move the state machine's algorithms (compute account state and sign-in) to its section. This may be easier if the state machine section is moved after the DiscoverFromExternalSource section.

I see what you mean, although it seems slightly better to me to keep the state machine description before the DiscoverFromExternalSource since that state machine is used entirely in DiscoverFromExternalSource. We can always move some methods to that section though.

14. I've heard conflicting information about the Logout API. If it is a core part of FedCM, move it to its own subsection (currently that would be a peer with the DiscoverFromExternalSource section. If not, move it to a proposal markdown file elsewhere.

Chrome has not shipped the logoutRPs API and we don't have a timeline for it. Since it seems you want to consolidate, I propose moving the relevant parts to a separate file. Fortunately this API is fairly isolated from the rest.

15. Note that "Display an account chooser..." in select an account is non-normative.

Why do you think it is non-normative? I feel like this is normative. Of course, the exact shape of such UI is not specified since it can vary across various user agents.

17. Move the Privacy > Network requests table into the discussion of the IDP APIs for clarity

Can you clarify for me where exactly you'd like to see this?

18. Move the next paragraph "For fetches that are sent with cookies..." somewhere into the Browser API section to normatively describe that we are sending first-party cookies.

The way to normatively describe this is the fetch's request's credentials mode, right?

20. Move timing attack mitigations to another document

This seems fine, although I would want to have this still accessible in bikeshed since there is plenty of algorithm in there. I guess we can always have another bikeshed file which produces a 'draft spec' for the future proposals which are not yet mature enough to be in the main spec? Or how would you wanna go about this?

Acknowledging in advance that this is a big list of non-trivial changes!

Yes, but lots of really good suggestions!

[achimschloss]

Either remove references to the token being a JWT rather than an opaque token, or require it to be a JWT normatively.

I think I only saw one reference. We do not check the format of the token in Chrome, so I would prefer that we remove the reference.

As also discussed here #318 - remove the reference to JWT is something to I'd also prefer to be clear on the token being opaque as it relates to the specification of FedCM

[samuelgoto]

There are several changes I think would improve the quality of the specification. Rather than filing them as independent issues, I am collecting them here. I am willing to file PRs for these as one or several stages if you like.

These suggestions make a LOT of sense to me and we can work with you to get them merged (did you say that you were planning to start filing PRs?).

Chrome has not shipped the logoutRPs API and we don't have a timeline for it. Since it seems you want to consolidate, I propose moving the relevant parts to a separate file. Fortunately this API is fairly isolated from the rest.

SGTM. We can move it to the "proposals" directory.

Move timing attack mitigations to another document

That seems reasonable, but only up until the point we agree on the mitigation design and can merge it back into the main algorithm.

What I think we can agree on is the following:

• things in the spec are things that multiple implementations (say, chrome and firefox) agree that they should implement and ship
• future extensions are tracked outside of the main spec, as explainers or PRs

As also discussed here #318 - remove the reference to JWT is something to I'd also prefer to be clear on the token being opaque as it relates to the specification of FedCM

SGTM

Acknowledging in advance that this is a big list of non-trivial changes!

These are great!

[bvandersloot-mozilla]

These suggestions make a LOT of sense to me and we can work with you to get them merged (did you say that you were planning to start filing PRs?).

I plan to start sending them Monday!

What I think we can agree on is the following:

• things in the spec are things that multiple implementations (say, chrome and firefox) agree that they should implement and ship
• future extensions are tracked outside of the main spec, as explainers or PRs

I think this makes a lot of sense as a first draft. Having future extensions in more than one place is a little confusing, but it isn't bad.

[samuelgoto]

Having future extensions in more than one place is a little confusing, but it isn't bad.

At the moment, we are keeping track of all proposals to extend FedCM proposed by chrome here. The intent was that would be a common place where we can fit any proposal, made by anyone (e.g. maybe we could move some of this there?).

That to say: at least the two things that are currently embedded into the spec (the Front-channel logout API and the IdP Sign-in Status API) could be easily moved to the proposals directory seamlessly.

[npm1]

I do wonder what's the best strategy for the spec of newer features. We could have a 'experimental spec' in proposals for sure but that has downsides: 1) harder to link the existing methods, since they are in the main spec 2) easier for stuff that is shared to go out of sync. The benefit is that it does not taint the main spec and it's clear what is experimental vs what is not. Would a "Experimental features" section in the main spec not work for this purpose? It would still be clear what features are less mature, while getting rid of some of the downsides of having an entirely separate spec file for the experimental features. Anyways, I think I'm ok with whatever option we go with but wanted to propose an alternative.

[samuelgoto]

We could have a 'experimental spec' in proposals for sure but that has downsides

I was thinking, maybe, we'd have only markdown explainers in proposals and pending spec pull requests against the main one (the github autobots generate nice spec previews).

Would that work?

[npm1]

That could work. I suspect it is still higher maintenance cost than my proposal, but the actual spec could remain nice and clean from future feature intrusion :P