fedidcg / FedCM

A privacy preserving identity exchange Web API
https://fedidcg.github.io/FedCM
Other
357 stars 66 forks source link

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

Closed samuelgoto closed 2 weeks ago

samuelgoto commented 1 month ago

When an RP uses any with #240 and #585 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 #581).

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 1 month ago

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

aaronpk commented 1 month 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 1 month ago

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

aaronpk commented 1 month 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 1 month 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 1 month ago

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

aaronpk commented 1 month 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 1 month 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 1 month 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 1 month 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 1 month 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 1 month 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 1 month 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 1 month 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 2 weeks 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.