Vojtech Szocs has uploaded a new change for review.

Change subject: webadmin: Fix repeated logins due to UI plugin REST API 
heartbeat
......................................................................

webadmin: Fix repeated logins due to UI plugin REST API heartbeat

This patch addresses a problem (behavior) of web browser sending
HTTP 'Authorization' header unconditionally (always) for each
request to given origin after the HTTP 'Authorization' header has
been set for the first time (e.g. via XmlHttpRequest).

This generally means the JavaScript application doesn't have full
control over HTTP 'Authorization' header; once set for the initial
request, web browser will always send this header until the browser
window is closed by the user.

To address this problem in UI plugin REST API integration:

1. all plugins will receive single session ID using GUI (WebAdmin)
   user credentials, session timeout is set to 6 hours

2. WebAdmin will not try to keep-alive the session via periodic
   heartbeat requests due to the problem with HTTP 'Authorization'
   header handling in web browser

These changes have following implications:
* REST API session will be acquired with reasonably long timeout
* it's up to plugins (or other systems using the session) to keep
  the session alive as necessary

In future, we should work around the HTTP 'Authorization' header
problem and revisit the general contract of UI plugin REST API
integration, i.e. whether to keep-alive the session by WebAdmin,
or whether to push session acquiry responsibility to individual
plugins.

Change-Id: I72c2d4952daac4daa17554b7661ed775c72cb97a
Bug-Url: https://bugzilla.redhat.com/894687
Bug-Url: https://bugzilla.redhat.com/906046
Signed-off-by: Vojtech Szocs <[email protected]>
---
M 
frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/plugin/restapi/RestApiSessionManager.java
M 
frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/system/ApplicationInit.java
2 files changed, 28 insertions(+), 59 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/11/14411/1

diff --git 
a/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/plugin/restapi/RestApiSessionManager.java
 
b/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/plugin/restapi/RestApiSessionManager.java
index 1c32a14..c67dd0e 100644
--- 
a/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/plugin/restapi/RestApiSessionManager.java
+++ 
b/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/plugin/restapi/RestApiSessionManager.java
@@ -6,8 +6,6 @@
 import org.ovirt.engine.ui.common.system.ClientStorage;
 import org.ovirt.engine.ui.frontend.utils.FrontendUrlUtils;
 
-import com.google.gwt.core.client.Scheduler;
-import com.google.gwt.core.client.Scheduler.RepeatingCommand;
 import com.google.gwt.event.shared.EventBus;
 import com.google.gwt.http.client.Request;
 import com.google.gwt.http.client.RequestBuilder;
@@ -22,30 +20,32 @@
  * This class has following responsibilities:
  * <ul>
  * <li>acquire new session upon successful user authentication (classic login)
- * <li>reuse current session if the user is already authenticated (auto login)
- * <li>keep the current session alive while the user stays authenticated
- * <li>release the current session when the user signs out
+ * <li>reuse existing session if the user is already authenticated (auto login)
  * </ul>
  * <p>
- * Triggers {@link RestApiSessionAcquiredEvent} upon acquiring REST API 
session.
+ * Note that the REST API session is not closed upon user logout, as there 
might be other systems still working with it.
+ * <p>
+ * Triggers {@link RestApiSessionAcquiredEvent} upon acquiring or reusing REST 
API session.
  */
 public class RestApiSessionManager {
 
-    private static class RestApiRequestCallback implements RequestCallback {
+    private static class RestApiCallback implements RequestCallback {
 
         @Override
         public void onResponseReceived(Request request, Response response) {
             if (response.getStatusCode() == Response.SC_OK) {
                 processResponse(response);
             } else {
-                RestApiSessionManager.logger.warning("Engine REST API response 
received with non-OK status code " //$NON-NLS-1$
-                        + response.getStatusCode());
+                RestApiSessionManager.logger.warning(
+                        "Engine REST API responded with non-OK status code " 
//$NON-NLS-1$
+                                + response.getStatusCode());
             }
         }
 
         @Override
         public void onError(Request request, Throwable exception) {
-            RestApiSessionManager.logger.log(Level.WARNING, "Error while 
requesting Engine REST API", exception); //$NON-NLS-1$
+            RestApiSessionManager.logger.log(Level.WARNING,
+                    "Error while dispatching Engine REST API request", 
exception); //$NON-NLS-1$
         }
 
         protected void processResponse(Response response) {
@@ -56,11 +56,9 @@
 
     private static final Logger logger = 
Logger.getLogger(RestApiSessionManager.class.getName());
 
+    private static final String SESSION_TIMEOUT = "360"; //$NON-NLS-1$
     private static final String SESSION_ID_HEADER = "JSESSIONID"; //$NON-NLS-1$
     private static final String SESSION_ID_KEY = "RestApiSessionId"; 
//$NON-NLS-1$
-
-    // Heartbeat (delay) between REST API keep-alive requests
-    private static final int SESSION_HEARTBEAT_MS = 1000 * 60; // 1 minute
 
     private final EventBus eventBus;
     private final ClientStorage clientStorage;
@@ -73,11 +71,7 @@
         this.restApiBaseUrl = FrontendUrlUtils.getRootURL() + "api"; 
//$NON-NLS-1$
     }
 
-    void setPersistentAuthHeader(RequestBuilder requestBuilder) {
-        requestBuilder.setHeader("Prefer", "persistent-auth"); //$NON-NLS-1$ 
//$NON-NLS-2$
-    }
-
-    void sendRequest(RequestBuilder requestBuilder, RestApiRequestCallback 
callback) {
+    void sendRequest(RequestBuilder requestBuilder, RestApiCallback callback) {
         try {
             requestBuilder.sendRequest(null, callback);
         } catch (RequestException e) {
@@ -85,41 +79,17 @@
         }
     }
 
-    RequestBuilder createRequest() {
-        return new RequestBuilder(RequestBuilder.GET, restApiBaseUrl);
-    }
-
-    void scheduleKeepAliveHeartbeat() {
-        Scheduler.get().scheduleFixedDelay(new RepeatingCommand() {
-            @Override
-            public boolean execute() {
-                if (getCurrentSessionId() != null) {
-                    // The browser takes care of setting Engine REST API 
JSESSIONID
-                    // cookie automatically as part of processing the HTTP 
request
-                    RequestBuilder requestBuilder = createRequest();
-                    setPersistentAuthHeader(requestBuilder);
-                    sendRequest(requestBuilder, new RestApiRequestCallback());
-
-                    // Proceed with the heartbeat
-                    return true;
-                } else {
-                    // The session has been released, cancel the heartbeat
-                    return false;
-                }
-            }
-        }, SESSION_HEARTBEAT_MS);
-    }
-
     /**
      * Acquires new REST API session using the given credentials.
      */
     public void acquireSession(String userNameWithDomain, String password) {
-        RequestBuilder requestBuilder = createRequest();
-        setPersistentAuthHeader(requestBuilder);
+        RequestBuilder requestBuilder = new RequestBuilder(RequestBuilder.GET, 
restApiBaseUrl);
+        requestBuilder.setHeader("Prefer", "persistent-auth"); //$NON-NLS-1$ 
//$NON-NLS-2$
+        requestBuilder.setHeader("Session-TTL", SESSION_TIMEOUT); //$NON-NLS-1$
         requestBuilder.setUser(userNameWithDomain);
         requestBuilder.setPassword(password);
 
-        sendRequest(requestBuilder, new RestApiRequestCallback() {
+        sendRequest(requestBuilder, new RestApiCallback() {
             @Override
             protected void processResponse(Response response) {
                 // Obtain the session ID from response header, as we're unable
@@ -128,23 +98,22 @@
                 String sessionIdFromHeader = 
response.getHeader(SESSION_ID_HEADER);
 
                 if (sessionIdFromHeader != null) {
-                    setCurrentSessionId(sessionIdFromHeader);
+                    setSessionId(sessionIdFromHeader);
                 }
 
-                reuseCurrentSession();
+                reuseSession();
             }
         });
     }
 
     /**
-     * Attempts to reuse current REST API session.
+     * Attempts to reuse existing REST API session that was previously 
{@linkplain #acquireSession acquired}.
      */
-    public void reuseCurrentSession() {
-        String currentSessionId = getCurrentSessionId();
+    public void reuseSession() {
+        String sessionId = getSessionId();
 
-        if (currentSessionId != null) {
-            RestApiSessionAcquiredEvent.fire(eventBus, currentSessionId);
-            scheduleKeepAliveHeartbeat();
+        if (sessionId != null) {
+            RestApiSessionAcquiredEvent.fire(eventBus, sessionId);
         } else {
             RestApiSessionManager.logger.severe("Engine REST API session ID is 
not available"); //$NON-NLS-1$
         }
@@ -154,18 +123,18 @@
      * Releases REST API session currently in use.
      */
     public void releaseSession() {
-        clearCurrentSessionId();
+        clearSessionId();
     }
 
-    String getCurrentSessionId() {
+    String getSessionId() {
         return clientStorage.getLocalItem(SESSION_ID_KEY);
     }
 
-    void setCurrentSessionId(String sessionId) {
+    void setSessionId(String sessionId) {
         clientStorage.setLocalItem(SESSION_ID_KEY, sessionId);
     }
 
-    void clearCurrentSessionId() {
+    void clearSessionId() {
         clientStorage.removeLocalItem(SESSION_ID_KEY);
     }
 
diff --git 
a/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/system/ApplicationInit.java
 
b/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/system/ApplicationInit.java
index 953c6b6..d2b641d 100644
--- 
a/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/system/ApplicationInit.java
+++ 
b/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/system/ApplicationInit.java
@@ -119,7 +119,7 @@
             @Override
             public void execute() {
                 // Assume the REST API session has been acquired and is still 
active
-                restApiSessionManager.reuseCurrentSession();
+                restApiSessionManager.reuseSession();
             }
         });
     }


--
To view, visit http://gerrit.ovirt.org/14411
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I72c2d4952daac4daa17554b7661ed775c72cb97a
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Vojtech Szocs <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to