quarkusio / quarkus

Quarkus: Supersonic Subatomic Java.
https://quarkus.io
Apache License 2.0
13.82k stars 2.69k forks source link

Enhanced Logout support #4481

Closed sberyozkin closed 4 years ago

sberyozkin commented 5 years ago

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

stianst commented 5 years ago

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 commented 5 years ago
  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 commented 5 years ago

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 commented 5 years ago

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

sberyozkin commented 4 years ago

@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 commented 4 years ago

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

sebastienblanc commented 4 years ago

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 commented 4 years ago

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

sebastienblanc commented 4 years ago

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

pedroigor commented 4 years ago

@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 commented 4 years ago

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

sebastienblanc commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

Yeah, we need this one ASAP.

sberyozkin commented 4 years ago

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

stianst commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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

sberyozkin commented 4 years ago

@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:

This is going to be a real fun :-)

sebastienblanc commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

I think we should split this work into two issues:

stianst commented 4 years ago

"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 commented 4 years ago

If you mean the logout request to the end_session_endpoint I see no problem with the app creating it by itself. That is what I meant by "don't need a java method to initiate logout" as you previously suggested. Applications have access to all tokens managed by the extension at runtime and the {{end_session_endpoint}} is part of the OIDC discovery document.

Now, for the post_logout_redirect_uri we do need to provide something OOTB so that the app does not need to clear the local state/cookies manually.

pedroigor commented 4 years ago

The only constraint I see with backchannel is that in order to test the implementation we would need, ideally, to have it in Keycloak for testing. Otherwise, I'm not sure how we can write tests.

So, as we discussed before, can the initial scope be:

By defining the initial scope I can start implementing something so that we can discuss later the implementation details.

stianst commented 4 years ago

@pedroigor I completely disagree - apps should not have to create a logout URL. The underlying protocol details should be hidden. For an app to have to create this they would have to know how to create the url, what's the url for the IdP, what parameters should be included, etc. Why would we ask an app to generate that when we don't for a login URL?

stianst commented 4 years ago

Initial scope is good. We should have a separate issue for backchannel logout, and yes we don't want to contribute that to Quarkus until we have it in Keycloak.

sberyozkin commented 4 years ago

Hi Pedro, @pedroigor, re that java method - IMHO we definitely do not want the user code deal with anything related to creating a redirect etc, etc.

It has to be completely managed by quarkus-oidc. All I'd expect as an application developer is to update my HTML page's logout button to point to something like https://localhost:8080/myapp/logout and add few configuration properties into application.properties, the user presses a logout button and ends up at some landing page like https://localhost:8080/myapp/welcome.html which was probably configured as

quarkus.oidc.logout.post_logout_uri = /welcome.html

for quarkus.oidc to calculate post_logout_redirect_uri correctly. I'm not sure about a default value as we may end up looping the user if the user lands on the protected page after a logout, if this property is not set then the user stays at the OIDC site after redirect, and we will of course recommend them to set it in the docs :-)

This /logout handler is what quarkus-oidc should register itself at, and it can handle GET/POST logout requests.

Did you imply it too ?

Thanks, Sergey

pedroigor commented 4 years ago

@sberyozkin @stianst OK. Let's provide some built-in capability for building the logout request and initiate the logout.

Now that we have agreed about the initial scope, we can discuss any other detail once we have the initial set of changes.

pedroigor commented 4 years ago

@sberyozkin Yeah. The built-in logout endpoint is probably what provides a better UX. This endpoint should be protected to only authenticated users, do not accept XHR requests.

I'm just not sure about CSRF. If we are mainly covering server-side apps, where content is served by the application, I don't see an issue. Makes sense ?

stianst commented 4 years ago

@pedroigor that's my problem with something like a "/logout" endpoint is CSRF. Needs to be protected that it can only be invoked from the app directly and not linked externally.

sberyozkin commented 4 years ago

Hi Pedro, Stian, I suppose we can still check there in logout that the existing q_session which only quarkus_oidc can set is available ? If needed we can also require that the applications wishing to use this logout handler must ensure that a hidden form field such as q_logout and also a cookie with the same name is set for us to enforce the match, I'm not 100% sure (or even not 50% :-)) sure that is enough but we can set some restrictions around using the handler as needed

pedroigor commented 4 years ago

@stianst Yer ... UX-wise it is better than forcing developers to write code and invoke a method, right? If we don't provide this, developers will need the implement something by themselves (e.g: using the java method to generate the logout request) and the same concern (CSRF) is also true when doing this.

However, I don't think this is a big problem because the OP should query the End-User if he/she wants to log out from the OP too when at the logout endpoint at the OP.

@sberyozkin In fact, CSRF protection is something we are lacking in Quarkus? How apps today can protect themselves in Quarkus? I mean, other frameworks provide some built-in mechanisms for this ...

pedroigor commented 4 years ago

If needed we can also require that the applications wishing to use this logout handler must ensure that a hidden form field such as q_logout and also a cookie with the same name is set for us to enforce the match, I'm not 100% sure (or even not 50% :-)) sure that is enough but we can set some restrictions around using the handler as needed

That would force a POST request and make things a bit more complicated? And the cookie would still be sent to the application from a different origin ...

stianst commented 4 years ago

The OP will only prompt the user if not presented with a valid id_token_hint or app requires consent. So it will be important that the logout endpoint does some csrf protection.

pedroigor commented 4 years ago

@stianst That is the problem when doing drafts ... There I did not see anything about this condition but that "OP SHOULD ask ...".

stianst commented 4 years ago

I see that, but that's a pretty annoying experience from a user experience. If you can trust the app based on the passed id_token_hint, why then ask the user again if they want to logout?

stianst commented 4 years ago

Also, that's a mute point from a csrf perspective as even if OP asks, the user should already be logged out of the app.

sberyozkin commented 4 years ago

@pedroigor Hi,

CSRF protection is something we are lacking in Quarkus? How apps today can protect themselves in Quarkus? I mean, other frameworks provide some built-in mechanisms for this

Let me open an enhancement request :-)

And per your discussion with Stian - OP can ask, Keycloak will, and I'm quite sure other known OIDC providers will.

That would force a POST request and make things a bit more complicated

We'd return an HTML Form which will auto-submit itself to Keycloak, I agree it is more complex; we can do though if it will be required

And the cookie would still be sent to the application from a different origin

Why ? We can configure CORS in Quarkus, and also can configure the name of the custom logout cookie that has to be available so that it is never the same and specific per an application deployment

I'm not saying we should necessarily start from it but we can do it if needed :-)

pedroigor commented 4 years ago

I see. Another path we can explore ...

@stianst @sberyozkin We have the most important bits defined so I'm going to start implementing something.

sberyozkin commented 4 years ago

Hi @pedroigor @stianst thanks

pedroigor commented 4 years ago

@sberyozkin Some updates about the RP-Initiated Logout implementation:

The logout process works as follows:

In general, that is how it is working right now in terms of both configuration and runtime. I'll continue now with some other bits such as the refresh token check.

A question that I would like to ask is:

pedroigor commented 4 years ago

@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.

tassadar81 commented 2 years ago

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