[keycloak-dev] Session SPI for adapters

[Stian Thorgersen]

----- Original Message -----
> From: "Marek Posolda" <mposolda at redhat.com>
> To: "Stian Thorgersen" <stian at redhat.com>
> Cc: "Bill Burke" <bburke at redhat.com>, keycloak-dev at lists.jboss.org
> Sent: Monday, 6 October, 2014 2:03:14 PM
> Subject: Re: [keycloak-dev] Session SPI for adapters
> 
> On 6.10.2014 13:23, Stian Thorgersen wrote:
> >
> > ----- Original Message -----
> >> From: "Marek Posolda" <mposolda at redhat.com>
> >> To: "Bill Burke" <bburke at redhat.com>, "Stian Thorgersen"
> >> <stian at redhat.com>
> >> Cc: keycloak-dev at lists.jboss.org
> >> Sent: Sunday, 5 October, 2014 10:59:28 PM
> >> Subject: Re: [keycloak-dev] Session SPI for adapters
> >>
> >> On 3.10.2014 17:17, Bill Burke wrote:
> >>> On 10/3/2014 2:38 AM, Stian Thorgersen wrote:
> >>>> Let's take a step-back here and consider the problem instead of a
> >>>> potential solution.
> >>>>
> >>>> Currently we require all applications to have an HTTP session to keep
> >>>> track of the users session as well as storing the tokens. When an
> >>>> application is clustered this requires replicating the HTTP session to
> >>>> all nodes, or using sticky load balancer sessions.
> >>>>
> >>>> I think it's quite likely that some don't want to have a HTTP session
> >>>> and
> >>>> instead store the token in a cookie to make the application fully
> >>>> stateless. That's simple enough to add right? And storing the token in a
> >>>> http-only cookie should be safe as well. However, one issue remains how
> >>>> do we deal with single-sign out in this case. As there's no state kept
> >>>> in
> >>>> the application a logout admin event from Keycloak isn't going to work.
> >>>> So would be just rely on the access token expiring in this case? Or
> >>>> could
> >>>> we add something better?
> >>>>
> >>> You don't have to change anything for bearer-only apps.  Blacklist and
> >>> time-based revocation policies are enough.
> >>>
> >>> For browser apps, the problem is logout.  Cookie won't work for logout.
> >>>     While you could logout one user via a browser redirect protocol,
> >>> doesn't work when an admin wants to initiate a logout to one or all
> >>> users from the admin console.  This is because a logout generates an
> >>> out-of-band request to each application.  The applications match the
> >>> UserSessionModel id with an HTTP Session and invalidate the session.  So
> >>> the adapter needs a UserSessionModel->HttpSession id map.
> >>>
> >>> Maybe the access code to token protocol could be modified to send the
> >>> Http session id?  This could be stored in the client session and sent
> >>> with the out-of-band logout request.
> >>>
> >>>
> >> I think that this will be good . Perhaps HttpSession ID could be stored
> >> on ClientSessionModel as note, so no changes on UserSession model
> >> required?
> > Would have to be ClientSession (user session doesn't work as one user
> > session can be linked to multiple application http sessions), so yes
> >
> >> Related issue is that our UserSessionManagement implementations (for
> >> both undertow and AS7) are stateful. Is it issue to refactor them to be
> >> stateless? With HttpSession passed around from KC it will be possible imo.
> >>
> >> Stateful impls have issues like:
> >> - State in UndertowUserSessionManagement and CatalinaUserManagement is
> >> stored in ConcurrentHashMaps and hence is not cluster-aware . This is
> >> causing issues especially with logout (for example login happens on
> >> node1, but admin out-of-bound request is handled on node2 -> it won't be
> >> able to find session in it's maps -> logout won't happen as user is
> >> still logged on node1)
> >> - Memory leaks https://issues.jboss.org/browse/KEYCLOAK-734
> >>
> >> I experimented on Friday and did prototype of UserSessionManagement
> >> impls backed by infinispan (this can address both cluster and memory
> >> leak), but tbh I am not so happy with it. IMO stateless
> >> UserSessionManagement is a way to go. What I can see for particular
> >> UserSessionManagement operations:
> >> - logoutKeycloakSession will be easy to handle as long as logout req
> >> from KC will send HttpSessionID. So we don't need any state with mapping
> >> from kcSessions to HttpSessions.
> >> - logoutUser --- Instead of sending username, Keycloak can send list of
> >> all HttpSessions of user for this app. Those all will be invalidated
> >> - logoutAll --- Both undertow Manager and org.apache.catalina.Manager
> >> has possibility to list all HttpSessions and invalidate them (This will
> >> invalidate also anonymous HttpSessions, but is this an issue?)
> > Stateless UserSessionManagement sounds good to me. A few questions though:
> >
> >   a) How does the app pass the http session id to Keycloak?
> Bill suggested to pass it in codeToToken request and that's what I am
> doing now in my prototype (I have it working on Wildfly/Undertow already).
> 
> I wonder that another option might be to pass it directly in "state"
> parameter? This would mean that there is no need to have another cookie
> OAuth_Token_Request_State, which we now have and also no changes
> required to protocol. The disadvantage is that it would require creating
> HttpSession even before user is redirected to Keycloak login screen.

Sounds hacky to me. Better to have a separate (optional) param in codeToToken IMO.

> >   b) Is there anything in OpenID Connect for this already?
> No idea. I will try to check...
> >   c) How does it affect SAML?
> >   d) How does it affect applications that don't use Keycloak adapters
> UserSessionManagement is used just by Wildfly, AS7/EAP6 and Tomcat
> adapters. Other applications are not affected at all.
> 
> For the future, I hope that frameworks like PHP, Ruby on rails, node.js
> have concept of HttpSession. So as long as we can pass their "sessionId"
> and they're able to lookup session by sessionId and logout it, we should
> be good.
> 
> In general, I think that passing application "sessionId" instead of
> keycloakSessionID to adapters is always better as in case that we pass
> keycloakSessionID, adapter need to maintain some state for mapping of
> kcSessions to httpSessions as is now.

What I mean is that this is a Keycloak proprietary feature and we need to make sure it doesn't affect applications that don't use Keycloak adapters, but instead use a standard OpenID Connect (or SAML?) adapter. 

> >
> >> Other methods for UserSessionManagement are for handle statistics, but
> >> do we really need them? Endpoint for
> >> PreAuthActionsHandler.handleGetUserStats() is not called from any place
> >> in Keycloak admin console. KC admin console is just using UserSession
> >> API directly. Is it an issue to remove
> >> PreAuthActionsHandler.handleGetUserStats() and all related methods from
> >> UserSessionManagement?
> > I believe we can remove these, they're from before we had user/client
> > sessions.
> Great!
> 
> If we still want to keep them, it's possible. It's not an issue to keep
> getActiveSessions() (It can just count number of httpSessions). For
> tracking all other stats, which are available now, we would need to
> iterate over httpSessions and read the data from them. So it would be
> performance killer, but it's doable;-)
> 
> For now, I am counting with the possibility to remove them.

My vote is to get rid of it, but check with Bill first

> 
> Marek
> >
> >> Marek
> >>
> >>
> 
>