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

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

Open
vaceksimon opened this issue Apr 15, 2024 · 10 comments
Open

Comments

@vaceksimon
Copy link

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

In the Keycloak implementation of FedCM API (keycloak/keycloak#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
Copy link
Collaborator

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
Copy link
Collaborator

npm1 commented Apr 15, 2024

+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
Copy link
Collaborator

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
Copy link
Collaborator

npm1 commented Apr 15, 2024

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
Copy link
Author

@vaceksimon
Copy link
Author

vaceksimon commented Apr 16, 2024

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
Copy link
Collaborator

npm1 commented Apr 16, 2024

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
Copy link

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

  • remove user client / RP session
  • remove consent for client / RP if present
    With this the user session with the Idp / Keycloak would remain intact, while the user has the "experience" of disconnecting the client / RP.

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

@npm1
Copy link
Collaborator

npm1 commented Apr 18, 2024

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
Copy link
Author

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.

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