Engine (user) sessions are managed by SessionDataContainer in backend "bll" module. SessionDataContainer implements session expiration through "old" and "new" generation, i.e. touching given session moves it to "new" generation and all sessions within "old" generation are periodically pruned via scheduled job (cleanExpiredUsersSessions). Engine session TTL (time-to-live aka timeout) is controlled through vdc_options DB table: * UserSessionTimeOutInterval [a], default value = 30min * UserSessionTimeOutInvalidationInterval [b], default value = 30min [a] delay between Engine startup and initial job execution [b] delay between subsequent job executions In other words, Engine session TTL is configured globally on Engine level. However, since each Engine session represents interaction of a user with Engine backend, this might lead to problematic situations. Imagine following scenario: * client creates REST API session such as restapi_session_ttl > engine_session_ttl * client is inactive for engine_session_ttl period, so associated Engine session expires * REST API session is still alive, client now performs an action through REST API * REST API backend finds out that Engine session is dead, and sends "Auth Required" (401) request back to client Client is now confused, because even though REST API session is still alive, REST API backend tells him to re-authenticate. Because of that, client must be aware not only of REST API session timeout, but also aware of Engine session timeout. To solve this problem, systems which initiate Engine session creation (like REST API backend) must be able to create Engine session with custom TTL, i.e. if client creates REST API session with TTL=6hrs, associated Engine session should also have TTL=6hrs. Implementation-wise, there are two major changes that need to be done in "core" module: 1, modify LoginBaseCommand#attachUserToSession so that it calls SessionDataContainer#setUser with custom (per-user) TTL 2, modify SessionDataContainer implementation to support custom (per-user) TTL, i.e. modify cleanExpiredUsersSessions logic
as far as I could understand when creating a rest session it also persists it as an engine session. Not sure that I fully understand the described scenario, but I'm am not sure having per user session ttl time is the best solution. It seems to me that when ever rests' session with the ovirt engine is invalidated it should invalidate it's own session and ask the user to re authenticate thus creating a new session for rest & engine. Juan what do you suggest to do here?
When RESTAPI creates a HTTP session it does create an backend session as well, and its identifier is stored in the RESTAPI session as an attribute. When the RESTAPI HTTP session expires or is explicitly invalidated, nothing happens with the corresponding engine session, it will be cleaned up when it expires itself. We could improve here, and explicitly logout in order to clean the backend session, but this won't solve any real issue, it will only clean the session earlier. When the backend session expires but the RESTAPI HTTP session is still alive the RESTAPI just asks the client to authenticate again, in order to login again and thus create a new backend session. From the RESTAPI point of view backend sessions that survive longer than HTTP sessions are just useless, so it is good to use the same TTL in both, or to cleanup backend sessions when RESTAPI sessions are expired. But this doesn't solve the problem in the description of the bug, as that is about backend sessions that expire before the corresponding HTTP session. I those regards I think that it makes sense to sync the TTLs of both types of sessions, or just let the backend sessions created by the RESTAPI live forever and let the RESTAPI clean them explicitly as needed.
(In reply to Juan Hernández from comment #2) > When RESTAPI creates a HTTP session it does create an backend session as > well, and its identifier is stored in the RESTAPI session as an attribute. > > When the RESTAPI HTTP session expires or is explicitly invalidated, nothing > happens with the corresponding engine session, it will be cleaned up when it > expires itself. We could improve here, and explicitly logout in order to > clean the backend session, but this won't solve any real issue, it will only > clean the session earlier. I actually meant the other way around, to invalidate the reset session when the backend one expires - as you say that is already done so that's is ok. What I was concerned about is even if we try to sync both sessions the engine one could still expire first. The reason for that is that the engine involves security concerns in session creation (the time the user is allowed to be connected is calculated when he signs in to create the session) IIUC this logic is currently different in rest. > > When the backend session expires but the RESTAPI HTTP session is still alive > the RESTAPI just asks the client to authenticate again, in order to login > again and thus create a new backend session. > > From the RESTAPI point of view backend sessions that survive longer than > HTTP sessions are just useless, so it is good to use the same TTL in both, > or to cleanup backend sessions when RESTAPI sessions are expired. But this > doesn't solve the problem in the description of the bug, as that is about > backend sessions that expire before the corresponding HTTP session. I those > regards I think that it makes sense to sync the TTLs of both types of > sessions, or just let the backend sessions created by the RESTAPI live > forever and let the RESTAPI clean them explicitly as needed. So I will go ahead and implement a custom session for the engine. Thanks!
Hi guys, please find my comments below. > (In reply to Juan Hernández from comment #2) > When RESTAPI creates a HTTP session it does create an backend session as > well, and its identifier is stored in the RESTAPI session as an attribute. Indeed, so there are essentially two sessions: one for REST API application (HttpSession for restapi.war), and one for Engine backend (SessionDataContainer for bll.jar). As you mentioned, REST API drives Engine (backend) session creation, with its HttpSession pointing to Engine session via session attribute (SessionUtils.ENGINE_SESSION_ID_KEY). The problem described in this BZ is caused by duality of sessions; since each of these sessions can have different timeout, REST API client can be confused in a situation when REST API session is dead but Engine session is alive. (Since > When the RESTAPI HTTP session expires or is explicitly invalidated, nothing > happens with the corresponding engine session, it will be cleaned up when it > expires itself. We could improve here, and explicitly logout in order to > clean the backend session, but this won't solve any real issue, it will only > clean the session earlier. You are correct, however I think we should improve this scenario as well. It doesn't make sense to have Engine session last longer than REST API session, given that Engine session creation was driven by REST API application. Maybe we could use HttpSessionListener that closes the corresponding Engine session? > When the backend session expires but the RESTAPI HTTP session is still alive > the RESTAPI just asks the client to authenticate again, in order to login > again and thus create a new backend session. Yes, this is the problem described in this BZ. To me, it seems really weird that client talking to Engine via REST API can be asked to re-authenticate even if REST API session is still alive (but corresponding Engine session is dead). From client's perspective, Engine session (its timeout and implementation) can be viewed as implementation detail. > From the RESTAPI point of view backend sessions that survive longer than > HTTP sessions are just useless, so it is good to use the same TTL in both, +1 Both session should be "in sync" with each other in terms of active time period (i.e. same timeout). > or to cleanup backend sessions when RESTAPI sessions are expired. But this > doesn't solve the problem in the description of the bug, as that is about > backend sessions that expire before the corresponding HTTP session. I those > regards I think that it makes sense to sync the TTLs of both types of > sessions, or just let the backend sessions created by the RESTAPI live > forever and let the RESTAPI clean them explicitly as needed. Not sure which approach is the best, considering Mooli's comment below. > (In reply to Mooli Tayer from comment #3) > I actually meant the other way around, to invalidate the reset session when > the backend one expires - as you say that is already done so that's is ok. REST API application drives the creation of Engine sessions for the purpose of implementing persistent authentication in REST API. Because of that, in my opinion, REST API application should take care of handling the corresponding Engine session (post its creation) as well. For example, when REST API session ends, Engine session should be closed too. > What I was concerned about is even if we try to sync both sessions the engine > one could still expire first. The reason for that is that the engine > involves > security concerns in session creation (the time the user is allowed to be > connected is calculated when he signs in to create the session) IIUC this > logic is currently different in rest. Good point. Ideally, Engine session should be alive as long as REST API session is alive. One approach, as mentioned by Juan above, is to let Engine session (created for given REST API session) live "forever" and dispose it properly when given HttpSession is destroyed (either times out or is explicitly invalidated).
> One approach, as mentioned by Juan above, is to let Engine session (created > for given REST API session) live "forever" and dispose it properly when > given HttpSession is destroyed (either times out or is explicitly > invalidated). Does REST support infinite session liveliness? If so, we can fill the SessionDataContainer with sessions without ever clearing them. I know we clear sessions today when logging out, but are we sure that logout will be called when the HTTP session gets invalidated?
Alon - thoughts about that?
Hi, I think this is one of the considerations and solution that bug#1092744 should provide. Have single login provider for the entire application, single lifecycle management. Alon
Moving to 3.6.0, and adding the proper dependency. (In reply to Alon Bar-Lev from comment #7) > Hi, > > I think this is one of the considerations and solution that bug#1092744 > should provide. > > Have single login provider for the entire application, single lifecycle > management. > > Alon Thank you. Moving to 3.6.0, and adding the proper dependency.
Target release should be placed once a package build is known to fix a issue. Since this bug is not modified, the target version has been reset. Please use target milestone to plan a fix for a oVirt release.
Oved - is this a real life issue, or something we wish to touch in 4.x?
(In reply to Yaniv Kaul from comment #10) > Oved - is this a real life issue, or something we wish to touch in 4.x? no private comments in upstream bugs, please. we are for transparency. only logs are permitted as private, and this also violation. please mark this and previous comment as public. per comment#7 the ui plugins will not use http sessions any more (bug#1236976), so no reason to modify restapi behaviour for ui plugins which is the root cause of this report.
Closing based on above comment.