w3c-fedid / FedCM

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

In `any` mode, make the browser automatically generate `clientId` based on the RP's origin when omitted #586

Closed samuelgoto closed 4 months ago

samuelgoto commented 5 months ago

When an RP uses any with w3c-fedid/idp-registration#2 and w3c-fedid/idp-registration#1 it is, by design, not an RP that is registered with the IdP ahead of time, so things like clientId isn't something that they need to specify (e.g. similarly, they wouldn't have registered terms of service and privacy policies, as described in w3c-fedid/idp-registration#8).

So, instead of making every RP pass clientId: window.location.origin, maybe it would be helpful for the browser to generate that value, so that it can decrease the number of ways RPs could get wrong.

const identityCredential = await navigator.credentials.get({
  identity: {
    context: "signin",
      providers: [{
        type: "indieauth",
        // clientId: window.location.origin+"/", when omitted, defaults to window.location.origin when `type` or `any` is available
     }],
  },
});
cbiesinger commented 5 months ago

since we already send an Origin header, why not just make clientId optional in this case?

aaronpk commented 5 months ago

That's an interesting idea, that implies that the OAuth server would effectively use the Origin header as the client_id. I suppose that does work, tho only in a web context, so it would have to have some processing rule like "if Origin header is present, client_id=Origin, otherwise look in the query string".

cbiesinger commented 5 months ago

oh, what you're saying is existing servers assume that clientId==origin?

aaronpk commented 5 months ago

Right now, the client_id is included in the query string because it's a redirect. So existing IndieAuth/OAuth servers are always expecting a client_id parameter in the request. You could make the argument that the Origin header here works as a replacement, but that will require changing the OAuth server specifically for FedCM.

cbiesinger commented 5 months ago

I am surprised by your implication that they don't otherwise need changes for FedCM. I mean, don't they need to check things like sec-fetch-dest matching "webidentity", etc.?

npm1 commented 5 months ago

I'm curious how client ID is currently determined in BYO IDP cases if not using the Origin

aaronpk commented 5 months ago

That's stuff at the new FedCM endpoints, which is all net new code. Changing how client_id works means changing more of the underlying OAuth part, which may not be practical if you're trying to build this on top of an existing OAuth server.

aaronpk commented 5 months ago

I'm curious how client ID is currently determined in BYO IDP cases if not using the Origin

The client provides its URL in the initial redirect to the authorization endpoint. Since the Origin header only exists in browsers, non-browser clients need a solution too.

aaronpk commented 5 months ago

For example, the client would construct this URL and redirect the browser to it:

https://authorization-server.com/authorize?client_id=https://example-app.com/&....

Since it's a redirect, the origin header that the authorization server sees is the authorization server itself of course.

cbiesinger commented 5 months ago

That's stuff at the new FedCM endpoints, which is all net new code. Changing how client_id works means changing more of the underlying OAuth part, which may not be practical if you're trying to build this on top of an existing OAuth server.

it seems like the fedcm endpoint could fill in the client_id then?

aaronpk commented 5 months ago

Ok looking at how I implemented this, I did make a new endpoint for the assertion endpoint (rather than trying to reuse the authorization endpoint). That's where it does the check for the Sec-Fetch-Dest header. I also had already added a "if origin hostname matches client_id hostname" check. When all the validations pass, it calls into the same function that the OAuth authorization endpoint calls to create the authorization code. So the client_id query parameter here is redundant, since I could make it pass the Origin header into the generateAuthorizationCode function as the client_id.

ThisIsMissEm commented 5 months ago

Wouldn't this tie FedCM too much to an implementation detail in IndieAuth (@aaronpk I think we discussed a better solution for client_id for both)

cbiesinger commented 5 months ago

Ok looking at how I implemented this, I did make a new endpoint for the assertion endpoint (rather than trying to reuse the authorization endpoint). That's where it does the check for the Sec-Fetch-Dest header. I also had already added a "if origin hostname matches client_id hostname" check. When all the validations pass, it calls into the same function that the OAuth authorization endpoint calls to create the authorization code. So the client_id query parameter here is redundant, since I could make it pass the Origin header into the generateAuthorizationCode function as the client_id.

Based on this, should we close this as wontfix or do you still think we should do something here?

(maybe we should make clientId optional? I believe it's currently required)

aaronpk commented 5 months ago

I'm leaning towards not recommending FedCM do anything automatic with client_id here.

We've been talking about changing the client_id parameter in IndieAuth to be a full URL to a JSON document with the client metadata (https://github.com/indieweb/indieauth/issues/133) rather than just the origin, which wouldn't be compatible with this new suggestion either.

samuelgoto commented 4 months ago

I'm leaning towards not recommending FedCM do anything automatic with client_id here.

Ok, I'm going to close this for now and reopen in case the need appears.