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

Allow IdPs to return JSON objects rather than Strings back to RPs #13

Open
samuelgoto opened this issue May 10, 2024 · 7 comments
Open

Comments

@samuelgoto
Copy link
Collaborator

IdPs can currently return a token which is a USVString through the id assertion endpoint, which, in theory, can actually also contain JSON via JSON.stringify(data).

w3c-fedid/FedCM#572 (comment) suggests that it would be easier for IdPs if they could actually return an object as opposed to a JSON-serialized USVString, which I think can have its merits.

@cbiesinger
Copy link

hmm that introduces an asymmetry between the JS version and returning a token directly from the token endpoint. I'm also not sure if there's much value in introducing the equivalent of a JSON serialize/parse in the browser side instead of the JS side?

@aaronpk
Copy link

aaronpk commented May 15, 2024

I just encountered another situation where it would make sense to return additional data besides the single token string, as suggested by @samuelgoto in w3c-fedid/FedCM#584. While it's not the end of the world to have to do JSON.parse(identityCredential.token), it also means the server is actually returning JSON-encoded JSON in the response, which is ugly but not ultimately a problem. I would still be in favor of letting the IdP return JSON objects here.

@sebadob
Copy link

sebadob commented May 20, 2024

As I started implementing FedCM into Rauthy today as well, I came across the same issue.

Maybe the FedCM could simply return the whole body as it is to the RP?
This would be the most flexible solution and make it possible to be really dynamic with upstream IdP's.

The current implementation is static and both parties must agree on a behavior upfront, out of band. When the FedCM would return the body as it is, the RP could deserialize it and dynamically know which protocol or type of auth would work with the IdP. It would also allow all different kinds of login flows, be it Implicit (which would be safe again, if the response would be inside the body instead of URL), authorization code, be it OAuth2 or OIDC, or other specs like IndieAuth.

The RP could deserialize the response and simply check in which scheme it fits.
The assertion response is exactly the point where I am stuck right now, because the only thing I could return there (as an OIDC IdP) is the access_token directly inside the token and throw away all the rest, which is kind of important too.

Edit:

Just another thought on this to make it even more dynamic and maybe easier for simpler IdP's to implement it:
Maybe the FedCM could look for "Some Header" returned from the assertion endpoint which decides if the current simple token body should be returned / interpreted, or if it is a more complex JSON response that should be forwarded as it is to the RP.

@samuelgoto
Copy link
Collaborator Author

I just encountered another situation where it would make sense to return additional data besides the single token string, as suggested by @samuelgoto in w3c-fedid/FedCM#584. While it's not the end of the world to have to do JSON.parse(identityCredential.token), it also means the server is actually returning JSON-encoded JSON in the response, which is ugly but not ultimately a problem. I would still be in favor of letting the IdP return JSON objects here.

Another option that occurred in the call by @aaronpk is to have the indieauth-metadata be something that is exposed in the configURL JSON payload. Since the configURL is returned in the IdentityCredential, the client can resolve the configURL to get the indieauth-metadata.

@aaronpk
Copy link

aaronpk commented May 21, 2024

I think I had actually originally prototyped it that way, but switched to this version where indieauth-metadata is included in the assertion response instead.

@samuelgoto
Copy link
Collaborator Author

Ok, so here is one proposal to make this a bit more concrete, since this is coming up in a series of discussions:

  1. We rename the id_assertion_endpoint response type from IdentityProviderToken to IdentityProviderAssertion.
  2. We introduce another optional parameter to IdentityProviderAssertion of type object called data. We also make token optional.
  3. We check that either data or token was provided, but not both
  4. We introduce another optional parameter to IdentityCredential of type object called data, that gets passed back the value that the browser received on step (2).

For example:

dictionary IdentityProviderAssertion {
  optional USVString token;
  optional object data;
};

And then:

interface IdentityCredential : Credential {
  readonly attribute USVString? token;
  readonly attribute object? data;
};

In use, this is what it would look like in an id_assertion_endpoint response:

{
  "data" : {
     "this_is_a_json": true,
     "really": true,
     "i_cant_believe_it": {
       "believe_me": ["for real"]
     }
  }
}

Which would then lead to a JS object returned by the JS API:

const {data: {really}} = await navigator.credentials.get({
  // typical call
});

// really == true

@bvandersloot-mozilla
Copy link

We probably want any instead of object here, so an id_assertion_endpoint response of ["foo", "bar"] doesn't get dropped.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants