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

Enhanced Logout support #4481

Closed
sberyozkin opened this issue Oct 9, 2019 · 46 comments
Closed

Enhanced Logout support #4481

sberyozkin opened this issue Oct 9, 2019 · 46 comments
Assignees
Milestone

Comments

@sberyozkin
Copy link
Member

sberyozkin commented Oct 9, 2019

Description
The authenticated users should have an option to logout out of OIDC adapter protected web applications.
The recommendation from the Keycloak experts is to have a combination of the RP-Initiating logout in combination with initial form of the OIDC back channel logout flow.

@stianst @pedroigor FYI

Update: Pedro has already done some work around supporting it as part of the code flow PR

@sberyozkin sberyozkin added kind/enhancement New feature or request area/oidc labels Oct 9, 2019
@stianst
Copy link
Contributor

stianst commented Oct 10, 2019

Me and @pedroigor has been working on some plans around logout:

Stateless applications can check validity of ID token using token introspection endpoint. However, this will have a performance impact as every request would require a request to the IdP.

An improvement to the above is the addition of a token introspection cache. This will not only be useful for ID tokens, but also for access tokens. For opaque tokens applications need to invoke the token introspection endpoint for every request.

The token introspection cache should be configurable with a cache expiration. Applications would first check the token by invoking introspection cache if not found it would invoke the token introspection endpoint. The expiration should be configurable on a per-token type (ID or access).

As a final improvement once OIDC backchannel logout mechanism is added the expiration in the token introspection cache can be increased as the application would be able to retrieve a logout request from the IdP which would remove the entry from the cache. This would require invalidating all entries in the cache for a specific session.

@stianst
Copy link
Contributor

stianst commented Oct 10, 2019

  1. Add support for RP initiate logout both to Keycloak and to quarkus-oidc extension
  2. Use token introspection endpoint to validate ID token for every request
  3. Add token introspection cache to validate ID token only every X minutes
    • Probably with pluggable store types, with a built-in support for Infinispan cache?
    • Entries in the cache expires after a configurable amount of minutes
    • Ideally different expiration for ID and access tokens
    • Token introspection cache can also be used with opaque tokens to prevent too many token introspection requests
  4. Add OIDC backchannel logout support to Keycloak and to quarkus-oidc extension
    • On backchannel logout request from Keycloak quarkus-oidc extension will purge ID tokens associated with the logged-out session from the token introspection cache
    • The above will allow increasing the expiration of ID tokens in the token introspection cache

1 and 2 are high priority
3 is a medium priority
4 is a lower priority and can be a stretch goal

@sberyozkin sberyozkin changed the title Logout support Enhanced Logout support Oct 29, 2019
@sberyozkin
Copy link
Member Author

sberyozkin commented Oct 29, 2019

Pedro:

Regarding logout, the current change set supports logout based on the token lifetime. I've also been working on another method that should enhance logout so that users are also logged out when their sessions are invalidated at the OpenID Connect Provider. I did not include here because I had to change Keycloak to better adhere to the specs and support silent re-authentication. Once we have that, I can grab changes from [here](https://github.com/pedroigor/quarkus/tree/issue-4480-logout).

SB: We can probably delay it as needed

@stianst
Copy link
Contributor

stianst commented Oct 29, 2019

@sberyozkin can you clarify what you mean about when their sessions are invalidated and silent re-authentication?

@sberyozkin
Copy link
Member Author

@stianst sorry I missed your question and I don't recall now what I meant or even where I said it :-).
FYI, I've added an oidc-interoperability label to this issue as well, @pedroigor recommended adding such labels to the issues where the code flow does not work with non-KC IDPs, and I guess the logout based on the specs will be in the interop space as well :-).

@sberyozkin
Copy link
Member Author

sberyozkin commented Dec 18, 2019

@stianst oh sorry, that was from a quote by Pedro @pedroigor :-)

@sebastienblanc
Copy link
Contributor

With Qute around the corner, being able to logout with the web-app mode with the oidc-extension will really be a "must have".
I have already locally a nice demo of a Server Side Page App using Qute and OIDC extension but without a proper logout feature I can not release it to the public.

@stianst
Copy link
Contributor

stianst commented Dec 19, 2019

I thought there already was logout support through RP initiated endpoint (which is a standard thing) and this is more about polish. @pedroigor ?

@sebastienblanc
Copy link
Contributor

@stianst you mean by calling /protocol/openid-connect/logout" ? That doesn't work for me right now.

@pedroigor
Copy link
Contributor

@stianst That quote was in the beginning when I was investigating the different options to logout.

By when their sessions are invalidated we should be able to recognize sessions invalidated at the OP so that local session is also invalidated.

The silent re-authentication part is related to what we discussed some time ago about checking whether or not the session is active at the OP by leveraging prom=none. As we discussed this is not the correct way and my initial changes (in the branch I also mentioned) will be discarded.

There is no support yet for RP logout.

@valones
Copy link

valones commented Dec 27, 2019

Is there any way to get the refresh_token? The /protocol/openid-connect/logout requires this token as a parameter

@sebastienblanc
Copy link
Contributor

With Qute gaining popularity I guess in the coming weeks, I think adding proper logout support is getting a higher priority ? What do you think ?

@sberyozkin
Copy link
Member Author

Hi @stianst, @pedroigor, All, I agree, IMHO this issue is becoming the major issue for quarkus-oidc for it to be seriously considered to be used in more than just POCs or simple deployments. Right now the only way for the web-app users to log out is to wait till the session cookie has expired and then, without choosing the logout option, find themselves at the IDP site again.
The users should have an option to click on the 'Logout' button whenever they want :-)
It should be a great feature to implement, likely with some oidc-interoperability fun along the way :-) as the feature should work with Keycloak, but also other OIDC providers (I think we have the users using quarkus-oidc with about 5 other providers)

@stianst
Copy link
Contributor

stianst commented Feb 26, 2020

I agree this is an important area to work on. Will have a chat with @pedroigor on what we have today, and we can create some plan for how to make things better in the not to distant future.

@pedroigor
Copy link
Contributor

Yeah, we need this one ASAP.

@sberyozkin
Copy link
Member Author

Hi Stian, Pedro, thanks, it will be super great :-)

@stianst
Copy link
Contributor

stianst commented Apr 1, 2020

Adding my 2 cents to this one:

For initiating the logout that's simple it's RP initiated logout endpoint, which we support.

For pushing logout that's more tricky. Front-channel logout is pretty much EOL due to third party cookies becoming blocked (Safari already blocks these, and Chrome will follow suite in 2022). That leaves two options:

a) Stateless - we basically need to invoke token introspection endpoint on each request to validate the token. This can be used in combination with a token introspection cache with some timeout. This is what I was thinking could be a generic purpose microservice rather than something that is baked into the app itself

b) Stateful - this would be OIDC backchannel logout requests, which we don't support in Keycloak at the moment.

@stianst
Copy link
Contributor

stianst commented Apr 1, 2020

Adding some more details:

I can think of a few options to invalidate the "authentication context" for stateless applications when logout is initiated from a different application, which is important since we are talking about SSO here:

  1. Session management session status change notification - this should not be considered for two reasons. First, "logout" doesn't happen until user actively uses the application. Second it requires iframe with access to third party cookies which are already disabled in Safari and will be disabled in other browsers in the future.

2. Front-channel logout - this is not covered by all IdPs, it's error prone and more importantly it requires access to third party cookies, which as mentioned above is EOL. It requires access to third party cookies basically due to it relying on embedding logout pages from the RP in a hidden iframe within a page from the IdP.

  1. Refresh ID token - rely on short timeout in ID token and refresh when expired. Simple and straightforward to implement, and should be supportable by most IdPs. Only downside is that there is a time between the logout happens at the IdP and the session with the app is expired. This is in terms of minutes though so is acceptable in most cases.

  2. Invoke token introspection endpoint - not all IdPs allow invoking using ID token as this is a OAuth spec rather than an OIDC spec. However, it is mostly supported. This would result in a potential high load on the IdP though, which then would result in wanting to support some form of token introspection cache to prevent every request to the app requiring a request to the IdP, which then basically turns it into a more complicated approach to option 3.

What we should do for stateless applications based on this reasoning is:

a) Add support to invoke RP initated logout endpoint. Support in IdP can be inteferred from discovery endpoint

b) Add support to refresh ID tokens when they expire

For statefull applications (is that even a thing in Quarkus) we should add support for OIDC backchannel specification.

@pedroigor
Copy link
Contributor

Yeah, a combination of both approaches should be OK. Although not yet bulletproof, as you mentioned (not surprisingly when talking about global logout).

@sberyozkin
Copy link
Member Author

@stianst thanks for the detailed analysis, yes, as @pedroigor mentions, we can start from a) + b) (I can check what can be done around b) with Vertx) and depending on the demand I guess - c) (stateful backchannel logout).
The other few things I'd like to comment about:

  • It would be good to have a logout mechanism pluggable so that you and Pedro can innovate with the new ideas
  • I'm curious if quarkus-oidc would need to register itself at the configurable JAX-RS path to help Qute/etc frontends connect with it ?
  • May be this endpoint can support redirects in response to GET or HTML form-based responses in response to POSTs
  • We probably should have a Logout config group ?

This is going to be a real fun :-)

@sebastienblanc
Copy link
Contributor

For statefull applications (is that even a thing in Quarkus) we should add support for OIDC backchannel specification.

When building an app with Qute Templating, then yeah, you have a stateful application.

@stianst
Copy link
Contributor

stianst commented Apr 2, 2020

Not sure if logout mechanism should be pluggable per say. I would perhaps rather argue we should have configurable alternatives/strategies.

quarkus-oidc for backchannel logout needs to be able to receive request at a specific endpoint. Wouldn't say that should be a jax-rs endpoint though, but rather something more low-level than that.

For initiating logout I would assume a Java method is called, rather than quarkus-oidc exposing a "initiate logout endpoint" as the latter could easily be abused.

@pedroigor
Copy link
Contributor

To initiate a logout you don't need a java method call but just the app itself to reach the logout endpoint. Considering we agreed about starting with RP-Initiated, after the logout is initiated by the app the server should expect a request to the post_logout_redirect_uri where we can then invalidate any state (e.g.: cookies) managed by the extension.

Regarding backchannel, are we considering to include it in the initial scope?

@stianst
Copy link
Contributor

stianst commented Apr 2, 2020

I think we should split this work into two issues:

  • Initiate logout
  • Support for backchannel logout

@stianst
Copy link
Contributor

stianst commented Apr 2, 2020

"To initiate a logout you don't need a java method call but just the app itself to reach the logout endpoint. Considering we agreed about starting with RP-Initiated, after the logout is initiated by the app the"

Not sure what you mean about that. The app needs to know the logout endpoint, and it also needs ti include the id_token_hint. You don't want the app to have to create the logout endpoint itself.

@pedroigor
Copy link
Contributor

@sberyozkin Sent a PR with the initial set of changes. Do you think we should have something in the guide for this too? Or the documentation added to the new configuration options are enough ?

I'll spend a bit more time trying to figure out a way to prevent the logout endpoint from CSRF attacks. As we discussed, it seems there is no way to prevent CSRF without impacting UX. At the same time, the logout is not so risky and OPs can prevent that by asking the user if they want to log out as mentioned in the specs. So ...

The way it is, it is quite simple.

pedroigor added a commit to pedroigor/quarkus that referenced this issue Apr 9, 2020
pedroigor added a commit to pedroigor/quarkus that referenced this issue Apr 10, 2020
pedroigor added a commit to pedroigor/quarkus that referenced this issue Apr 10, 2020
pedroigor added a commit to pedroigor/quarkus that referenced this issue Apr 10, 2020
pedroigor added a commit to pedroigor/quarkus that referenced this issue Apr 10, 2020
pedroigor added a commit to pedroigor/quarkus that referenced this issue Apr 10, 2020
pedroigor added a commit to pedroigor/quarkus that referenced this issue Apr 10, 2020
pedroigor added a commit to pedroigor/quarkus that referenced this issue Apr 10, 2020
pedroigor added a commit to pedroigor/quarkus that referenced this issue Apr 10, 2020
pedroigor added a commit to pedroigor/quarkus that referenced this issue Apr 11, 2020
pedroigor added a commit to pedroigor/quarkus that referenced this issue Apr 11, 2020
pedroigor added a commit to pedroigor/quarkus that referenced this issue Apr 11, 2020
pedroigor added a commit to pedroigor/quarkus that referenced this issue Apr 11, 2020
pedroigor added a commit to pedroigor/quarkus that referenced this issue Apr 13, 2020
pedroigor added a commit to pedroigor/quarkus that referenced this issue Apr 13, 2020
pedroigor added a commit to pedroigor/quarkus that referenced this issue Apr 13, 2020
pedroigor added a commit to pedroigor/quarkus that referenced this issue Apr 13, 2020
pedroigor added a commit to pedroigor/quarkus that referenced this issue Apr 13, 2020
pedroigor added a commit to pedroigor/quarkus that referenced this issue Apr 14, 2020
pedroigor added a commit to pedroigor/quarkus that referenced this issue Apr 14, 2020
pedroigor added a commit to pedroigor/quarkus that referenced this issue Apr 14, 2020
sberyozkin added a commit that referenced this issue Apr 20, 2020
[fixes #4481] - RP-Initiated Logout and session verification
@gsmet gsmet added this to the 1.5.0 milestone Apr 23, 2020
@tassadar81
Copy link

tassadar81 commented Feb 3, 2022

Hello @pedroigor @stianst is there any backchannel logout support implemented? Reading this issue i can't find a clue nor i can find in the quakus documentation. I guess that if supported a logout consumer is published by the quarkus oidc extension. Is that right?
Thanks

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

No branches or pull requests

7 participants