search

w3c-fedid / FedCM

A privacy preserving identity exchange Web API
https://w3c-fedid.github.io/FedCM/
Other
375 stars 72 forks source link

The name "token" seem to imply that only ID Tokens can be passed back to RPs #579

Closed samuelgoto closed 4 months ago

samuelgoto commented 5 months ago

It was raised numerous times (example) that our choice of words of token leads a reader/developer to assume that only ID Tokens can be passed back to the RP, when, in fact, it was designed to allow any arbitrary data to be passed back: it is a USVString, so it can be anything (e.g. a SAML XML response, a JSON.stringify(data), an access token, an mp3 file, a JPEG, whatever can be binary encoded into a string).

A few options that we have on this:

  1. rename token (backwards incompatible, so a bit hard)
  2. ignore the problem and solve with documentation
  3. introduce a more meaningful purpose-specific fields, e.g. code (less extensible, but more interoperable?)
  4. introduce a general purpose field, e.g. data or response (more extensible, but less interoperable?)
timcappalli commented 5 months ago

I think we could do a type field to define the semantics of the token value. Could be as simple as tokenType with an enum.

samuelgoto commented 5 months ago

I think we could do a type field to define the semantics of the token value. Could be as simple as tokenType with an enum.

Yeah, that's another way that this could go, but that wouldn't address the confusion created by the name "token", would it?

timcappalli commented 5 months ago

Ah right. This is in the context of other artifacts like code. Got it.

panva commented 5 months ago

rename token (backwards incompatible, so a bit hard)

That should not be a big concern given how little this spec adopted by the audience you're reaching out to.

introduce a more meaningful purpose-specific fields, e.g. code (less extensible, but more interoperable?)

-1, the last thing I'd want is to have to wait for browsers to adopt, support, and rollout additional fields to RPs when we define a new response shapes

introduce a general purpose field, e.g. data or response (more extensible, but less interoperable?)

+1 I thought that's what token is meant to be in the first place, in which case response as a name would be appropriate. Although we already have a response mode in OAuth which has a response field so then the RP would be getting the response.response in that one particular case ;)

aaronpk commented 5 months ago

See my comment on this giant thread here where I used this field to return an OAuth authorization code which is exchanged for actual info from the IdP server-side.

npm1 commented 5 months ago

FedCM is used in a large number of Chrome page loads, so even though the spec change would be ok to be backwards incompatible, from our perspective we want to avoid these changes if at all possible. That said, we could support passing 'token' OR 'response' if that helps with the confusion? If a developer passes both, we'd throw.

samuelgoto commented 5 months ago

That should not be a big concern given how little this spec adopted by the audience you're reaching out to.

It is still possible to make backwards compatible changes, but we don't take them lightly.

FedCM is used in a large number of Chrome page loads

To be concrete, 0.8% of page loads in Chrome:

https://twitter.com/RickByers/status/1781376930912333880 https://chromestatus.com/metrics/feature/timeline/popularity/4166

anderspitman commented 5 months ago

This is actually a very surprising number of page loads. Where is it being used?

samuelgoto commented 5 months ago

This is actually a very surprising number of page loads. Where is it being used?

Every site on the web that uses these variations of Sign in with Google:

https://developers.googleblog.com/en/federated-credential-management-fedcm-migration-for-google-identity-services/

samuelgoto commented 5 months ago

I think that this is maybe a duplicate of https://github.com/fedidcg/FedCM/issues/578, so closing. Feel free to reopen if you disagree and there is something else that needs to be captured that isn't captured in https://github.com/fedidcg/FedCM/issues/578.

panva commented 5 months ago

I think that this is maybe a duplicate of w3c-fedid/idp-registration#13, so closing. Feel free to reopen if you disagree and there is something else that needs to be captured that isn't captured in w3c-fedid/idp-registration#13.

I believe that entirely depends on what shape w3c-fedid/idp-registration#13 is going to take. If it's going to define a new property (e.g. response, or data) that can be either any JSON primitive (string, object, etc) or whether it's going to allow such for the existing token property?

samuelgoto commented 5 months ago

I believe that entirely depends on what shape w3c-fedid/idp-registration#13 is going to take. If it's going to define a new property (e.g. response, or data) that can be either any JSON primitive (string, object, etc) or whether it's going to allow such for the existing token property?

Ah, fair. Let me reopen this, and we can revisit as either moves along.

samuelgoto commented 4 months ago

Ah, fair. Let me reopen this, and we can revisit as either moves along.

Ok, I moved this a bit further in a way that I believe still makes this issue obsolete, by making the following choice:

or whether it's going to allow such for the existing token property?

I made a concrete proposal in the following issue and will close this one here as as obsolete by the other. Will reopen this if my proposal in the other issue falls through.

https://github.com/fedidcg/FedCM/issues/578#issuecomment-2173930195