Bug 1172726 - Webadmin access not cleared on session expiration
Summary: Webadmin access not cleared on session expiration
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Red Hat Enterprise Virtualization Manager
Classification: Red Hat
Component: ovirt-engine-webadmin-portal
Version: 3.5.0
Hardware: Unspecified
OS: Unspecified
urgent
urgent
Target Milestone: ---
: 3.5.0
Assignee: Vojtech Szocs
QA Contact: Pavel Novotny
URL:
Whiteboard: ux
Depends On:
Blocks: rhev35rcblocker rhev35gablocker
TreeView+ depends on / blocked
 
Reported: 2014-12-10 15:50 UTC by Ravi Nori
Modified: 2015-02-15 09:14 UTC (History)
13 users (show)

Fixed In Version: org.ovirt.engine-root-3.5.0-30
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2015-02-15 09:14:58 UTC
oVirt Team: ---


Attachments (Terms of Use)


Links
System ID Priority Status Summary Last Updated
Red Hat Product Errata RHBA-2015:0230 normal SHIPPED_LIVE Red Hat Enterprise Virtualization Manager 3.5.0-1 ASYNC 2015-02-16 19:50:27 UTC
oVirt gerrit 35897 None None None Never
oVirt gerrit 35986 None None None Never
oVirt gerrit 36622 master MERGED webadmin: Fix UI plugin REST API / Engine session refresh issue Never
oVirt gerrit 36710 ovirt-engine-3.5 MERGED webadmin: Fix UI plugin REST API / Engine session refresh issue Never
oVirt gerrit 36737 master MERGED webadmin: Adapt UI plugin infra REST session timeout Never
oVirt gerrit 36738 ovirt-engine-3.5 MERGED webadmin: Adapt UI plugin infra REST session timeout Never
Red Hat Bugzilla 1168842 None None None Never
Red Hat Bugzilla 1175313 None None None Never

Internal Links: 1168842 1175313

Description Ravi Nori 2014-12-10 15:50:28 UTC
Description of problem: When the webadmin is left open for extended period of time the session is invalidated in the backend but the webadmin access is not cleared. 

Version-Release number of selected component (if applicable): 3.5

How reproducible: Not always reproducible. 

1. Leave webadmin open for extended period.

Actual results: The user should be logged out and redirected to the webadmin login screen.

Additional info: This breaks the sso between webadmin and reports. Now when the reports tab is clicked/reports are accessed the user is redirected to reports login screen.

Comment 1 Einav Cohen 2014-12-12 20:39:25 UTC
guessing that this could be related to one or more of the following:

- http://gerrit.ovirt.org/#/c/35185/
- bug 1168842 / http://gerrit.ovirt.org/#/c/35897/ (which fixes bug 1168842)

@Alexander: Please consult with Ravi/Vojtech/Alon[/Juan/Yair] if needed. thanks.

Comment 2 Alexander Wels 2014-12-15 15:01:33 UTC
I am unable to reproduce in master which has [1] already merged, I can reproduce in 3.5 branch which does not have [2] merged yet. I will wait for [2] to get merged to retest in 3.5

[1] http://gerrit.ovirt.org/#/c/35897/
[2] http://gerrit.ovirt.org/#/c/35986/

Comment 3 Alexander Wels 2014-12-15 15:25:46 UTC
(In reply to Alexander Wels from comment #2)
> I am unable to reproduce in master which has [1] already merged, I can
> reproduce in 3.5 branch which does not have [2] merged yet. I will wait for
> [2] to get merged to retest in 3.5
> 
> [1] http://gerrit.ovirt.org/#/c/35897/
> [2] http://gerrit.ovirt.org/#/c/35986/

I manually applied [2] to my 3.5 setup and after it has been applied I am no longer able to reproduce the problem. So I will mark this as a duplicate of bz1168842

*** This bug has been marked as a duplicate of bug 1168842 ***

Comment 4 Alexander Wels 2014-12-15 17:08:18 UTC
Should not have marked it duplicate of upstream issue.

Comment 5 Einav Cohen 2014-12-17 00:43:09 UTC
[fixed reference to RHEV gerrit]

Comment 6 Pavel Novotny 2015-01-02 17:02:23 UTC
FailedQA in rhevm-3.5.0-0.27.el6ev.noarch (vt13.5).

I've set UserSessionTimeOutInterval to 5 (minutes) in engine.
Using Firefox 34 with clean fresh profile.
After several hours of inactivity in Webadmin I am still logged in. Navigating through the WebUI now or reloading the whole page also doesn't redirect me to login page.

I guess it's related to bug 1175313 and bug 1168842 which are still assigned/modified. 

@Yair: Not sure how to set the Blocks/Depends On fields in this BZ. If you believe this particular bug is fixed and only depends on the others, feel free to update the dependencies and move back to on_qa.

Comment 7 Oved Ourfali 2015-01-04 11:49:08 UTC
This was introduced when making the UI plugins session the same as the webadmin session. See Bug 1161734 ("Double backend login during frontend login").

So now both the webadmin and the UI plugins use the same session. In addition, the UI plugins infrastructure has a mechanism to keep the session alive, which is triggered every minute. As a result, the engine session is valid until the user logs out explicitly.

Moving this bug to UX.
Einav / Vojtech - can you take a look?

CC-ing also Alon, who might have an idea here.

Comment 8 Oved Ourfali 2015-01-04 11:56:00 UTC
I verified that commenting out the session keep-alive code on the UX side resolves the issue.

Comment 9 Alon Bar-Lev 2015-01-04 12:15:48 UTC
Hi,
Due to the fact that we use session based restapi, we need to find a way to refresh the http session without triggering a refresh of engine session.
How do we refresh now?

Comment 10 Einav Cohen 2015-01-05 13:19:01 UTC
clearing 'needinfo's, assigning to Alexander - original assignee of this BZ. 
@Alexander - please investigate; discuss with oourfali, alonbl and pnovotny (QA) as necessary.

Comment 11 Vojtech Szocs 2015-01-05 15:43:38 UTC
Oved is right, with UX patch [1] applied, both WebAdmin RPC HttpSession and REST API HttpSession now point to the same (logical) Engine user session. This is manifested by a single "user has logged in" event upon successful login.

[1] http://gerrit.ovirt.org/#/c/35185/

This is actually my mistake, I forgot to realize that WebAdmin UI plugin infra's keep-alive requests will eventually prevent Engine user session from timing-out.

Alon's suggestion above (refresh REST API HttpSession but don't refresh Engine user session when making REST requests) is possible but perhaps unnecessary, since the REST API HttpSession is already created with timeout ("Session-TTL") equal to Engine user session timeout ("UserSessionTimeOutInterval"). Since both HttpSessions point to single Engine user session, having a keep-alive mechanism isn't relevant anymore.

TL;DR - I think we can just remove the WebAdmin UI plugin infra's keep-alive requests.

Comment 12 Vojtech Szocs 2015-01-05 17:12:57 UTC
As discussed with Alex, we'll need to ensure that REST API HttpSession won't die due to inactivity on side of UI plugins. We'll simply need to refresh REST API HttpSession whenever the other (WebAdmin RPC) HttpSession is refreshed.

Comment 13 Vojtech Szocs 2015-01-05 18:41:44 UTC
Here are my findings regarding the "refresh" of Engine session when making backend *query* call. Note that when making backend *action* call, Engine session will always be refreshed, if I read following correctly:

  // CommandBase line 190
  DbUser user = SessionDataContainer.getInstance().getUser(cmdContext.getEngineContext().getSessionId(), true);

Note that "refresh" of Engine session simply means "reset session expiration to <now> + <UserSessionTimeOutInterval>".

When invoking backend query directly (using GWT RPC as invocation protocol), VdcQueryParametersBase contains "refresh" flag set to true by default. There are many "setRefresh()" calls across the frontend code, for example:

  queryParams.setRefresh(getIsQueryFirstTime());

However, when invoking backend query indirectly via REST API, the "refresh" flag is not customizable at all, so it always stays true. For example, in BackendBookmarksResource:

  @Override
  public Bookmarks list() {
    return mapCollection(getBackendCollection(VdcQueryType.GetAllBookmarks, new VdcQueryParametersBase()));
  }

So not only is "refresh" flag not customizable when invoking queries via REST API, but implementing such customization would most probably involve significant REST code refactoring (i.e. refactor "new XxParameters()" occuring on many places into some well-defined control flow).

Anyway, the good news is (unless I miss something) that we can use HTTP GET /ovirt-engine/api -> BackendApiResource#get while modifying BackendApiResource#getSystemStatistics to specify "refresh=false" when calling:

  VdcQueryReturnValue result = runQuery(VdcQueryType.GetSystemStatistics, new GetSystemStatisticsQueryParameters(-1));

@Juan, I'd like to hear your opinion on above. Ideally, there would be a way to "refresh" REST HttpSession in a more clean way, for example in BackendApiResource#get:

  } else if (QueryHelper.hasConstraint(getUriInfo(), KEEPALIVE_CONSTRAINT_PARAMETER)) {
    return emptyResponse(); // the purpose is to keep HttpSession alive, no need for any data in response
  }

I'm going to make a patch that keeps REST HttpSession in "sync" with RPC HttpSession -> whenever frontend makes runQuery/runAction call, we'll also issue REST keepalive request. The periodic 60s REST heartbeat logic will be removed.

Comment 14 Alon Bar-Lev 2015-01-05 18:59:45 UTC
If turns out difficult we can just add X-OVIRT-INTERNAL-ENGINE-REFRESH header and add a filter that will do nothing if this header exists... so that we can keep http session alive without actually doing anything.

Comment 15 Vojtech Szocs 2015-01-05 20:17:31 UTC
(In reply to Alon Bar-Lev from comment #14)
> If turns out difficult we can just add X-OVIRT-INTERNAL-ENGINE-REFRESH
> header and add a filter that will do nothing if this header exists... so
> that we can keep http session alive without actually doing anything.

This is better than what I suggested, thanks.

I've discussed this with Einav and she proposed another solution - when frontend makes a backend call that refreshes Engine session (action or query with "refresh=true"), make HTTP call to /ovirt-engine/api to refresh REST HttpSession too (which will also refresh Engine session due to stuff like BackendApiResource#getSystemStatistics etc. but at this point we don't care because Engine session was already refreshed).

To generalize, refresh REST HttpSession whenever Engine user session is refreshed. Note that REST HttpSession is acquired with same timeout as Engine user session.

There is also one important consequence of using same Engine session for both WebAdmin & UI plugins -> imagine there is UI plugin that periodically makes REST calls -> periodically refreshes Engine session -> prevents natural web application timeout due to user's in-activity.

Comment 16 Alon Bar-Lev 2015-01-05 20:22:43 UTC
if ui plugin creates an activity then it should be considered as activity no?

if for each refresh=true query you create a /api request just to refresh, we actually load the server more than we should, instead, we should call /api once per interval if at that interval there was refresh=true, no?

Comment 17 Vojtech Szocs 2015-01-05 20:31:14 UTC
(In reply to Alon Bar-Lev from comment #16)
> if ui plugin creates an activity then it should be considered as activity no?

Yes. But this might not be visible to the end user. Anyway, UI plugins are extensions of the core application and their activity should be considered as activity coming from the application itself, so you are right.

> 
> if for each refresh=true query you create a /api request just to refresh, we
> actually load the server more than we should, instead, we should call /api
> once per interval if at that interval there was refresh=true, no?

Right, we shouldn't put too much load on the server. Good idea, we can refresh max. once per interval.

Comment 18 Vojtech Szocs 2015-01-05 20:40:01 UTC
Another idea -- write backend infra to support the following use case: whenever Engine user session is refreshed [SessionDataContainer#refresh(SessionInfo)] also refresh all associated HttpSession(s) and sync their timeout with Engine session timeout.

The advantage of that would be no UI code or extra HTTP traffic necessary. With that, we could just remove periodic REST heartbeat on the client side.

Comment 19 Alon Bar-Lev 2015-01-05 20:47:52 UTC
(In reply to vszocs from comment #18)
> Another idea -- write backend infra to support the following use case:
> whenever Engine user session is refreshed
> [SessionDataContainer#refresh(SessionInfo)] also refresh all associated
> HttpSession(s) and sync their timeout with Engine session timeout.
> 
> The advantage of that would be no UI code or extra HTTP traffic necessary.
> With that, we could just remove periodic REST heartbeat on the client side.

this is ejb, it is unaware (should not be aware) of http session, nor http objects.

Comment 20 Vojtech Szocs 2015-01-05 21:03:43 UTC
(In reply to Alon Bar-Lev from comment #19)
> (In reply to vszocs from comment #18)
> > Another idea -- write backend infra to support the following use case:
> > whenever Engine user session is refreshed
> > [SessionDataContainer#refresh(SessionInfo)] also refresh all associated
> > HttpSession(s) and sync their timeout with Engine session timeout.
> > 
> > The advantage of that would be no UI code or extra HTTP traffic necessary.
> > With that, we could just remove periodic REST heartbeat on the client side.
> 
> this is ejb, it is unaware (should not be aware) of http session, nor http
> objects.

AFAIK SessionDataContainer isn't EJB, it's just a helper to manage logical Engine user sessions.

What I had in mind is to write HttpSessionListener (applied per-webapp) whose responsibility would be to intercept creation of HttpSession, at which point Engine user is already authenticated -> Engine session ID is already present as HttpRequest attribute (SessionConstants.HTTP_SESSION_ENGINE_SESSION_ID_KEY) so we could do something like this:

  String engineSessionId = (String) request.getAttribute(SessionConstants.HTTP_SESSION_ENGINE_SESSION_ID_KEY);
  SessionDataContainer.addRefreshListenerForSession(engineSessionId, new EngineSessionRefreshListener() {
    @Override
    public void sessionRefreshed(Date softLimit) {
      // 1. synchronize HttpSession timeout with Engine session timeout
      // 2. refresh HttpSession to ensure it's in sync with Engine session
    }
  });

Comment 21 Vojtech Szocs 2015-01-05 21:06:41 UTC
What I wanted to say in my above comment is the idea that end user (web client) wouldn't need to worry about manually managing physical HttpSession that is usable only when it's associated logical Engine session is alive; instead, the server would ensure that physical HttpSession is usable as long as Engine session is alive (which in my opinion makes more sense than client attempting to keep physical HttpSession alive on its own responsibility anyway).

Comment 22 Alon Bar-Lev 2015-01-05 21:06:58 UTC
The SessionDataContainer lives within the backend it is not a utility, it should not be accessed by webapp. please do not go this route.

Comment 23 Einav Cohen 2015-01-13 14:27:24 UTC
it seems that vt13.7 doesn't include everything in 'ovirt-engine-3.5.0-rhevm' since vt13.6 - it includes only a single fix since then. moving back to MODIFIED.

Comment 24 Pavel Novotny 2015-01-28 12:58:23 UTC
Verified in rhevm-3.5.0-0.30.el6ev.noarch (vt13.8).

Clients:
- firefox-31.4.0-1.el6_6.x86_64 ESR
- firefox-35.0+build3-0ubuntu0.14.04.2

Verification steps:
1) On engine, set the user session timeout to 5 minutes:
   # engine-config -s UserSessionTimeOutInterval=5 && service ovirt-engine restart
2) On client, open Webadmin page, log in and then do no actions in the webUI. 
3) Wait 5-6 minutes.

Result:
After 5 minutes, the user session expires and user is redirected to login page.

Comment 26 Eyal Edri 2015-02-15 09:14:58 UTC
bugs were moved by ERRATA to RELEASE PENDING bug not closed probably due to errata error.
closing as 3.5.0 is released.


Note You need to log in before you can comment on or make changes to this bug.