Vojtech Szocs has uploaded a new change for review. Change subject: webadmin: Fix UI plugin REST API HttpSession refresh issue ......................................................................
webadmin: Fix UI plugin REST API HttpSession refresh issue TODO(vszocs) write commit msg after getting more sleep Change-Id: I2ffd3198239040f67313238a837af98db16e79ea Bug-Url: https://bugzilla.redhat.com/1172726 Bug-Url: https://bugzilla.redhat.com/1168842 Signed-off-by: Vojtech Szocs <vsz...@redhat.com> --- M backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/filters/FiltersHelper.java A backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/filters/RefreshSessionFilter.java M backend/manager/modules/restapi/webapp/src/main/webapp/WEB-INF/web.xml M frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/communication/VdcOperationManager.java M frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/plugin/restapi/RestApiSessionManager.java 5 files changed, 120 insertions(+), 15 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/22/36622/1 diff --git a/backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/filters/FiltersHelper.java b/backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/filters/FiltersHelper.java index 924d805..7679c0c 100644 --- a/backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/filters/FiltersHelper.java +++ b/backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/filters/FiltersHelper.java @@ -24,6 +24,7 @@ public static final String HEADER_PREFER = "Prefer"; public static final String HEADER_JSESSIONID_COOKIE = "JSESSIONID"; public static final String HEADER_ENGINE_AUTH_TOKEN = "OVIRT-INTERNAL-ENGINE-AUTH-TOKEN"; + public static final String HEADER_ENGINE_REFRESH = "X-OVIRT-INTERNAL-ENGINE-REFRESH"; } public static final int PREFER_NEW_AUTH = (1<<0); diff --git a/backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/filters/RefreshSessionFilter.java b/backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/filters/RefreshSessionFilter.java new file mode 100644 index 0000000..688b3dd --- /dev/null +++ b/backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/filters/RefreshSessionFilter.java @@ -0,0 +1,39 @@ +package org.ovirt.engine.core.aaa.filters; + +import java.io.IOException; + +import javax.servlet.Filter; +import javax.servlet.FilterChain; +import javax.servlet.FilterConfig; +import javax.servlet.ServletException; +import javax.servlet.ServletRequest; +import javax.servlet.ServletResponse; +import javax.servlet.http.HttpServletRequest; + +import org.ovirt.engine.core.aaa.filters.FiltersHelper.Constants; + +public class RefreshSessionFilter implements Filter { + + @Override + public void init(FilterConfig filterConfig) throws ServletException { + } + + @Override + public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException, + ServletException { + HttpServletRequest req = (HttpServletRequest) request; + + if (FiltersHelper.isAuthenticated(req) && req.getHeader(Constants.HEADER_ENGINE_REFRESH) != null) { + // Do nothing, this request was meant to only refresh the given HttpSession's expiry timer + response.setContentType("text/plain"); + response.setContentLength(0); + } else { + chain.doFilter(request, response); + } + } + + @Override + public void destroy() { + } + +} diff --git a/backend/manager/modules/restapi/webapp/src/main/webapp/WEB-INF/web.xml b/backend/manager/modules/restapi/webapp/src/main/webapp/WEB-INF/web.xml index 9a108c4..d3eac0f 100644 --- a/backend/manager/modules/restapi/webapp/src/main/webapp/WEB-INF/web.xml +++ b/backend/manager/modules/restapi/webapp/src/main/webapp/WEB-INF/web.xml @@ -119,6 +119,15 @@ <url-pattern>/*</url-pattern> </filter-mapping> + <filter> + <filter-name>RefreshSessionFilter</filter-name> + <filter-class>org.ovirt.engine.core.aaa.filters.RefreshSessionFilter</filter-class> + </filter> + <filter-mapping> + <filter-name>RefreshSessionFilter</filter-name> + <url-pattern>/*</url-pattern> + </filter-mapping> + <!-- confidentiality --> <security-constraint> <web-resource-collection> diff --git a/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/communication/VdcOperationManager.java b/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/communication/VdcOperationManager.java index 2c00dd2..2058ec8 100644 --- a/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/communication/VdcOperationManager.java +++ b/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/communication/VdcOperationManager.java @@ -14,6 +14,12 @@ */ public class VdcOperationManager { + public interface OperationCallback { + void operationExecuted(VdcOperation<?, ?> operation); + } + + private final List<OperationCallback> operationCallbacks = new ArrayList<>(); + /** * The operation queue. It can hold any kind of VdcOperation. */ @@ -36,6 +42,10 @@ @Inject public VdcOperationManager(final OperationProcessor operationProcessor) { this.processor = operationProcessor; + } + + public void addOperationCallback(OperationCallback callback) { + operationCallbacks.add(callback); } /** @@ -73,6 +83,10 @@ if (isAllowedToExecute) { if (operationCanBeAdded && operationQueue.add(operation)) { processor.processOperation(this); + + for (OperationCallback callback : operationCallbacks) { + callback.operationExecuted(operation); + } } } @@ -169,4 +183,5 @@ public void retrieveFromHttpSession(final String key, final StorageCallback callback) { processor.retrieveFromHttpSession(key, callback); } + } 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 d162ce8..7d50ab5 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 @@ -3,9 +3,13 @@ import java.util.logging.Level; import java.util.logging.Logger; +import org.ovirt.engine.core.common.queries.VdcQueryParametersBase; import org.ovirt.engine.ui.common.utils.HttpUtils; import org.ovirt.engine.ui.frontend.Frontend; import org.ovirt.engine.ui.frontend.communication.StorageCallback; +import org.ovirt.engine.ui.frontend.communication.VdcOperation; +import org.ovirt.engine.ui.frontend.communication.VdcOperationManager; +import org.ovirt.engine.ui.frontend.communication.VdcOperationManager.OperationCallback; import org.ovirt.engine.ui.frontend.utils.BaseContextPathData; import com.google.gwt.core.client.Scheduler; @@ -38,7 +42,7 @@ * <p> * Triggers {@link RestApiSessionAcquiredEvent} upon acquiring or reusing REST API session. */ -public class RestApiSessionManager { +public class RestApiSessionManager implements OperationCallback { private static class RestApiRequestCallback implements RequestCallback { @@ -71,6 +75,7 @@ private static final String SESSION_ID_HEADER = "JSESSIONID"; //$NON-NLS-1$ private static final String CSRF_HEADER = "JSESSIONID"; //$NON-NLS-1$ private static final String ENGINE_AUTH_TOKEN_HEADER = "OVIRT-INTERNAL-ENGINE-AUTH-TOKEN"; //$NON-NLS-1$ + private static final String ENGINE_REFRESH_HEADER = "X-OVIRT-INTERNAL-ENGINE-REFRESH"; //$NON-NLS-1$ private static final String SESSION_ID_KEY = "RestApiSessionId"; //$NON-NLS-1$ private static final String DEFAULT_SESSION_TIMEOUT = "30"; //$NON-NLS-1$ @@ -84,23 +89,45 @@ private String restApiSessionTimeout = DEFAULT_SESSION_TIMEOUT; private String restApiSessionId; + private boolean refreshRestApiSession = false; + @Inject - public RestApiSessionManager(EventBus eventBus) { + public RestApiSessionManager(EventBus eventBus, VdcOperationManager operationManager) { this.eventBus = eventBus; // Note that the slash at the end of the URL is not just a whim. With the trailing slash the browser will only // send authentication headers to URLs ending in api/, otherwise it will send them to URLs ending in /, and // this causes problems in other applications, for example in the reports application. this.restApiBaseUrl = BaseContextPathData.getInstance().getPath() + "api/"; //$NON-NLS-1$ + + operationManager.addOperationCallback(this); + } + + @Override + public void operationExecuted(VdcOperation<?, ?> operation) { + if (restApiSessionId != null && engineSessionRefreshed(operation)) { + refreshRestApiSession = true; + } + } + + boolean engineSessionRefreshed(VdcOperation<?, ?> operation) { + // Actions always refresh the Engine session + if (operation.isAction()) { + return true; + } + + // Queries optionally refresh the Engine session + else if (((VdcQueryParametersBase) operation.getParameter()).getRefresh()) { + return true; + } + + return false; } public void setSessionTimeout(String sessionTimeout) { this.restApiSessionTimeout = sessionTimeout; } - /** - * Build HTTP request to keep-alive existing REST API session. - */ RequestBuilder createRequest() { RequestBuilder builder = new RequestBuilder(RequestBuilder.GET, restApiBaseUrl); @@ -121,7 +148,7 @@ /** * Build HTTP request to acquire new REST API session. */ - RequestBuilder createRequest(String engineAuthToken) { + RequestBuilder createNewSessionRequest(String engineAuthToken) { RequestBuilder builder = createRequest(); // Enforce expiry of existing session when acquiring new session @@ -130,6 +157,18 @@ // Map this (physical) REST API session to current user's (logical) Engine session builder.setHeader(ENGINE_AUTH_TOKEN_HEADER, engineAuthToken); + + return builder; + } + + /** + * Build HTTP request to keep-alive existing REST API session. + */ + RequestBuilder createKeepAliveRequest() { + RequestBuilder builder = createRequest(); + + // This request shouldn't do anything other than refresh the REST API session + builder.setHeader(ENGINE_REFRESH_HEADER, "refresh-only"); //$NON-NLS-1$ return builder; } @@ -146,16 +185,18 @@ Scheduler.get().scheduleFixedDelay(new RepeatingCommand() { @Override public boolean execute() { - if (restApiSessionId != null) { - // The browser takes care of sending JSESSIONID cookie for this request automatically - sendRequest(createRequest(), new RestApiRequestCallback()); + boolean sessionInUse = restApiSessionId != null; - // The session is still in use, proceed with the heartbeat - return true; - } else { - // The session has been released, cancel the heartbeat - return false; + if (sessionInUse && refreshRestApiSession) { + // The browser takes care of sending JSESSIONID cookie for this request automatically + sendRequest(createKeepAliveRequest(), new RestApiRequestCallback()); + + // Reset the refresh flag + refreshRestApiSession = false; } + + // Proceed with the heartbeat only when the session is still in use + return sessionInUse; } }, SESSION_HEARTBEAT_MS); } @@ -164,7 +205,7 @@ * Acquires new REST API session that maps to current user's Engine session. */ public void acquireSession(String engineAuthToken) { - sendRequest(createRequest(engineAuthToken), new RestApiRequestCallback() { + sendRequest(createNewSessionRequest(engineAuthToken), new RestApiRequestCallback() { @Override protected void processResponse(Response response) { // Obtain session ID from response header, as we're unable to access the -- To view, visit http://gerrit.ovirt.org/36622 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I2ffd3198239040f67313238a837af98db16e79ea Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Vojtech Szocs <vsz...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
