fedidcg / FedCM

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

The Login Status API Set-Login header in a Disconnect Endpoint Response #558

Open vaceksimon opened 2 months ago

vaceksimon commented 2 months ago

This is more of a Chrome implementation issue rather than a FedCM API issue.

In the Keycloak implementation of FedCM API (https://github.com/keycloak/keycloak/issues/16834) we would like to use the disconnect endpoint to perform an actual SSO logout and leverage the Login Status API. Which also means updating the user agent's Login Status Map. The easiest and most convenient way is with the HTTP header in a response from this endpoint.

According to the Login Status API specification you can set the value of the login status with JavScript or an HTTP header. However when I try to return the Set-Login: logged-out header in a response from the disconnect endpoint Chrome does not seem to recognize it and does not properly update the Login Status Map.

Here is a code snippet of the return statement in my Disconnect Endpoint implemented with JAX-RS:

 return Response.ok(id)
     .header("Access-Control-Allow-Origin", client_origin)
     .header("Access-Control-Allow-Credentials", true)
     .header("Access-Control-Allow-Headers", "Content-Type, Set-Login")
     .header("Set-Login", "logged-out")
     .type(MediaType.APPLICATION_JSON)
     .build();

After the user tries to login with IdP via FedCM again, the expected output is "Not signed in with the identity provider." but instead an empty list returns from the accounts list endpoint. I would guess this is a problem with the "cors" mode response which I am not very familiar with.

There can be made some workarounds to update the map, but it just seems unnecessary.

samuelgoto commented 2 months ago

we would like to use the disconnect endpoint to perform an actual SSO logout and leverage the Login Status API.

That's a bit surprising to me, because the disconnect endpoint was intended to be used to revoke the connection to the RP, not to sign the user out of the IdP.

By that I mean: a part from the bug that you found, are you really sure you want to log out the user at the IdP when the user revokes the connection from the RP?

If that's a deliberate choice, yeah, that's a bug we can look into, I'm just trying to make sure that the intention is clear.

npm1 commented 2 months ago

+1 to what Sam says. But I also don't think that any of the FedCM fetches themselves can set the login status of the IDP. Maybe @cbiesinger knows for sure?

cbiesinger commented 2 months ago

It's actually supposed to work. I am not immediately sure why it doesn't. Could you file a bug at crbug.com/new?

npm1 commented 2 months ago

I was imagining it would not work due to the ancestor chain check. In the spec, https://fedidcg.github.io/FedCM/#login-status-http early returns if client is null. And for disconnect request, client is null. https://fedidcg.github.io/FedCM/#disconnect-request. So if this is expected to work, the spec needs to be fixed.

vaceksimon commented 2 months ago

@cbiesinger Bug filed: https://issues.chromium.org/issues/334940026

vaceksimon commented 2 months ago

This again raises the question what the FedCM is for and what Keycloak would like to use it for. If possible Keycloak would ideally like to get rid of the Keycloak Javascript Adapter used in authentication and logout process. FedCM kind of allows users to log-in with the IdP (with the button mode and with the dynamic sign-in flow) which Keycloak benefits from; enable users to login. It would be only natural to offer the option to log them out.

The disconnect endpoint doesn't suggest an actual logout, only a browser's removal of the triplet (account, RP, IdP). But it is the only place where a logout is suited to be be offered. At least that's the idea of our prototype.

Perhaps @jonkoops and @stianst could know more about KeycloakJS and correct me.

npm1 commented 2 months ago

Can you help us understand why you need an API to logout from the IDP though? I think we have clarified that disconnect is not it, and you are right that there is no API for IDP logout, but I wonder why you think you need it. For IDP logout, it would be invoked from an IDP context and use the IDP cookies, so the IDP should be able to update the state without help from the browser, no? You just need to set additionally set the LoginStatus to logged out so FedCM works well when invoked from an RP.

thomasdarimont commented 2 months ago

I think one way to implement "disconnect" in keycloak could be:

I think this would follow the intent of the disconnect operation better. Wdyt?

npm1 commented 2 months ago

I think that makes sense. Also note that disconnect is optional, i.e. an IDP does not need to support it if it does not support this feature about disconnecting the user's IDP account from the RP.

vaceksimon commented 2 months ago

Yes, you are right. The prototype includes setting the headers in parts of Keycloak responsible for signing in and out. My problem was looking at this from the wrong perspective and seeing the disconnect process as the equivalent of signing out. What @thomasdarimont wrote definitely works.

I guess this issue is not relevant anymore.