Alexander Wels has uploaded a new change for review. Change subject: userportal,webadmin: additional protection for GWT RPC ......................................................................
userportal,webadmin: additional protection for GWT RPC - Added additional protection to GWT-RPC calls. Change-Id: Ia4a5ad1f33eb985aa79e1376aecb48ae365d334d Signed-off-by: Alexander Wels <[email protected]> --- M frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/communication/GWTRPCCommunicationProvider.java A frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/communication/XsrfRpcRequestBuilder.java M frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/gwtservices/GenericApiGWTService.java A frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/gwtservices/XsrfProtectedRpcService.java A frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/server/gwt/AbstractXsrfProtectedRpcServlet.java M frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/server/gwt/GenericApiGWTServiceImpl.java A frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/server/gwt/XsrfProtectedRpcServlet.java M frontend/webadmin/modules/frontend/src/main/resources/META-INF/web-fragment.xml M frontend/webadmin/modules/frontend/src/test/java/org/ovirt/engine/ui/frontend/FrontendActionTest.java M frontend/webadmin/modules/frontend/src/test/java/org/ovirt/engine/ui/frontend/FrontendTest.java M frontend/webadmin/modules/frontend/src/test/java/org/ovirt/engine/ui/frontend/communication/GWTRPCCommunicationProviderTest.java M frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/gin/BaseSystemModule.java M frontend/webadmin/modules/userportal-gwtp/src/main/webapp/WEB-INF/web.xml M frontend/webadmin/modules/webadmin/src/main/webapp/WEB-INF/web.xml 14 files changed, 596 insertions(+), 113 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/24/27024/1 diff --git a/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/communication/GWTRPCCommunicationProvider.java b/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/communication/GWTRPCCommunicationProvider.java index 7d67fed..787123c 100644 --- a/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/communication/GWTRPCCommunicationProvider.java +++ b/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/communication/GWTRPCCommunicationProvider.java @@ -15,7 +15,11 @@ import org.ovirt.engine.core.common.queries.VdcQueryType; import org.ovirt.engine.ui.frontend.gwtservices.GenericApiGWTServiceAsync; +import com.google.gwt.core.client.GWT; import com.google.gwt.user.client.rpc.AsyncCallback; +import com.google.gwt.user.client.rpc.ServiceDefTarget; +import com.google.gwt.user.client.rpc.XsrfToken; +import com.google.gwt.user.client.rpc.XsrfTokenServiceAsync; import com.google.inject.Inject; /** @@ -25,25 +29,85 @@ public class GWTRPCCommunicationProvider implements CommunicationProvider { /** + * Callback interface when retrieving the GWT RPC service. We need the callback in order to retrieve the + * XSRF token if it is not available yet. + */ + private interface ServiceCallback { + /** + * The callback method with the service. + * @param service The GWT RPC service that contains the appropriate token. + */ + void serviceFound(GenericApiGWTServiceAsync service); + /** + * The failure callback in case we are unable to retrieve the approriate token. + * @param exception The exception thrown. + */ + void onFailure(Throwable exception); + } + + /** * GWT RPC service. */ private final GenericApiGWTServiceAsync service; /** - * Get the GWT RPC service. - * @return instance of the GWT RPC service. + * GWT XSRF service. */ - private GenericApiGWTServiceAsync getService() { - return service; + private final XsrfTokenServiceAsync xsrfService; + + /** + * Builder to XSRF token to header. + */ + final XsrfRpcRequestBuilder xsrfRequestBuilder; + + /** + * Base URL; + */ + private String baseUrl; + + /** + * Get the GWT RPC service. + * @param callback The callback to use when determining the service. + */ + private void getService(final ServiceCallback callback) { + ((ServiceDefTarget) this.xsrfService).setServiceEntryPoint(getXsrfServiceEndPoint()); + if (xsrfRequestBuilder.getXsrfToken() != null) { + callback.serviceFound(service); + } else { + xsrfService.getNewXsrfToken(new AsyncCallback<XsrfToken>() { + @Override + public void onSuccess(XsrfToken token) { + xsrfRequestBuilder.setXsrfToken(token); + callback.serviceFound(service); + } + + @Override + public void onFailure(Throwable caught) { + callback.onFailure(caught); + } + }); + } + } + + private String getXsrfServiceEndPoint() { + if (baseUrl == null) { + baseUrl = GWT.getModuleBaseURL(); + } + return baseUrl + "xsrf"; //$NON-NLS-1$ } /** * Constructor. * @param asyncService GWT RPC service. + * @param xsrfTokenService The GWT XSRF token service, used to retrieve a new XSRF token. */ @Inject - public GWTRPCCommunicationProvider(final GenericApiGWTServiceAsync asyncService) { + public GWTRPCCommunicationProvider(final GenericApiGWTServiceAsync asyncService, + final XsrfTokenServiceAsync xsrfTokenService) { this.service = asyncService; + this.xsrfRequestBuilder = new XsrfRpcRequestBuilder(); + ((ServiceDefTarget) this.service).setRpcRequestBuilder(xsrfRequestBuilder); + this.xsrfService = xsrfTokenService; } /** @@ -70,16 +134,26 @@ * @param operation The operation to run. */ private void runPublicQuery(final VdcOperation<?, ?> operation) { - getService().RunPublicQuery((VdcQueryType) operation.getOperation(), - (VdcQueryParametersBase) operation.getParameter(), new AsyncCallback<VdcQueryReturnValue>() { + getService(new ServiceCallback() { @Override - public void onFailure(final Throwable exception) { - operation.getCallback().onFailure(operation, exception); + public void serviceFound(GenericApiGWTServiceAsync service) { + service.RunPublicQuery((VdcQueryType) operation.getOperation(), + (VdcQueryParametersBase) operation.getParameter(), new AsyncCallback<VdcQueryReturnValue>() { + @Override + public void onFailure(final Throwable exception) { + operation.getCallback().onFailure(operation, exception); + } + + @Override + public void onSuccess(final VdcQueryReturnValue result) { + operation.getCallback().onSuccess(operation, result); + } + }); } @Override - public void onSuccess(final VdcQueryReturnValue result) { - operation.getCallback().onSuccess(operation, result); + public void onFailure(Throwable exception) { + operation.getCallback().onFailure(operation, exception); } }); } @@ -89,16 +163,26 @@ * @param operation The operation to run. */ private void runQuery(final VdcOperation<?, ?> operation) { - getService().RunQuery((VdcQueryType) operation.getOperation(), - (VdcQueryParametersBase) operation.getParameter(), new AsyncCallback<VdcQueryReturnValue>() { + getService(new ServiceCallback() { @Override - public void onFailure(final Throwable exception) { - operation.getCallback().onFailure(operation, exception); + public void serviceFound(GenericApiGWTServiceAsync service) { + service.RunQuery((VdcQueryType) operation.getOperation(), + (VdcQueryParametersBase) operation.getParameter(), new AsyncCallback<VdcQueryReturnValue>() { + @Override + public void onFailure(final Throwable exception) { + operation.getCallback().onFailure(operation, exception); + } + + @Override + public void onSuccess(final VdcQueryReturnValue result) { + operation.getCallback().onSuccess(operation, result); + } + }); } @Override - public void onSuccess(final VdcQueryReturnValue result) { - operation.getCallback().onSuccess(operation, result); + public void onFailure(Throwable exception) { + operation.getCallback().onFailure(operation, exception); } }); } @@ -108,16 +192,26 @@ * @param operation The operation to run. */ private void runAction(final VdcOperation<?, ?> operation) { - getService().RunAction((VdcActionType) operation.getOperation(), - (VdcActionParametersBase) operation.getParameter(), new AsyncCallback<VdcReturnValueBase>() { + getService(new ServiceCallback() { @Override - public void onFailure(final Throwable exception) { - operation.getCallback().onFailure(operation, exception); + public void serviceFound(GenericApiGWTServiceAsync service) { + service.RunAction((VdcActionType) operation.getOperation(), + (VdcActionParametersBase) operation.getParameter(), new AsyncCallback<VdcReturnValueBase>() { + @Override + public void onFailure(final Throwable exception) { + operation.getCallback().onFailure(operation, exception); + } + + @Override + public void onSuccess(final VdcReturnValueBase result) { + operation.getCallback().onSuccess(operation, result); + } + }); } @Override - public void onSuccess(final VdcReturnValueBase result) { - operation.getCallback().onSuccess(operation, result); + public void onFailure(Throwable exception) { + operation.getCallback().onFailure(operation, exception); } }); } @@ -161,8 +255,8 @@ private void transmitMultipleQueries(final List<VdcOperation<?, ?>> queriesList) { if (queriesList.size() > 1 || (queriesList.size() == 1 && queriesList.get(0).getCallback() instanceof VdcOperationCallbackList)) { - List<VdcQueryType> queryTypes = new ArrayList<VdcQueryType>(); - List<VdcQueryParametersBase> parameters = new ArrayList<VdcQueryParametersBase>(); + final List<VdcQueryType> queryTypes = new ArrayList<VdcQueryType>(); + final List<VdcQueryParametersBase> parameters = new ArrayList<VdcQueryParametersBase>(); for (VdcOperation<?, ?> operation: new ArrayList<VdcOperation<?, ?>>(queriesList)) { if (operation.isPublic()) { @@ -174,38 +268,62 @@ } } - getService().RunMultipleQueries((ArrayList<VdcQueryType>) queryTypes, - (ArrayList<VdcQueryParametersBase>) parameters, - new AsyncCallback<ArrayList<VdcQueryReturnValue>>() { + getService(new ServiceCallback() { @Override - public void onFailure(final Throwable exception) { - Map<VdcOperationCallback<?, ?>, List<VdcOperation<?, ?>>> callbackMap = getCallbackMap(queriesList); - for (Map.Entry<VdcOperationCallback<?, ?>, List<VdcOperation<?, ?>>> callbackEntry: callbackMap.entrySet()) { - if (callbackEntry.getKey() instanceof VdcOperationCallbackList) { - ((VdcOperationCallbackList) callbackEntry.getKey()).onFailure(callbackEntry.getValue(), exception); - } else { - ((VdcOperationCallback) callbackEntry.getKey()).onFailure(callbackEntry.getValue().get(0), exception); + public void serviceFound(GenericApiGWTServiceAsync service) { + service.RunMultipleQueries((ArrayList<VdcQueryType>) queryTypes, + (ArrayList<VdcQueryParametersBase>) parameters, + new AsyncCallback<ArrayList<VdcQueryReturnValue>>() { + @Override + public void onFailure(final Throwable exception) { + handleMultipleQueriesFailure(queriesList, exception); } - } + + @Override + public void onSuccess(final ArrayList<VdcQueryReturnValue> result) { + Map<VdcOperationCallback<?, ?>, List<VdcOperation<?, ?>>> callbackMap = + getCallbackMap(queriesList); + for (Map.Entry<VdcOperationCallback<?, ?>, + List<VdcOperation<?, ?>>> callbackEntry: callbackMap.entrySet()) { + List<VdcQueryReturnValue> queryResult = (List<VdcQueryReturnValue>) getOperationResult( + callbackEntry.getValue(), queriesList, result); + if (callbackEntry.getKey() instanceof VdcOperationCallbackList) { + ((VdcOperationCallbackList) callbackEntry.getKey()) + .onSuccess(callbackEntry.getValue(), queryResult); + } else { + ((VdcOperationCallback) callbackEntry.getKey()) + .onSuccess(callbackEntry.getValue().get(0), queryResult.get(0)); + } + } + } + }); + } @Override - public void onSuccess(final ArrayList<VdcQueryReturnValue> result) { - Map<VdcOperationCallback<?, ?>, List<VdcOperation<?, ?>>> callbackMap = getCallbackMap(queriesList); - for (Map.Entry<VdcOperationCallback<?, ?>, List<VdcOperation<?, ?>>> callbackEntry: callbackMap.entrySet()) { - List<VdcQueryReturnValue> queryResult = (List<VdcQueryReturnValue>) getOperationResult( - callbackEntry.getValue(), queriesList, result); - if (callbackEntry.getKey() instanceof VdcOperationCallbackList) { - ((VdcOperationCallbackList) callbackEntry.getKey()).onSuccess(callbackEntry.getValue(), queryResult); - } else { - ((VdcOperationCallback) callbackEntry.getKey()).onSuccess(callbackEntry.getValue().get(0), - queryResult.get(0)); - } - } + public void onFailure(Throwable exception) { + handleMultipleQueriesFailure(queriesList, exception); } }); } else if (queriesList.size() == 1) { transmitOperation(queriesList.get(0)); + } + } + + /** + * Multiple queries failure handler. + * @param queriesList The queries list. + * @param exception The exception causing the failure. + */ + private void handleMultipleQueriesFailure(final List<VdcOperation<?, ?>> queriesList, + final Throwable exception) { + Map<VdcOperationCallback<?, ?>, List<VdcOperation<?, ?>>> callbackMap = getCallbackMap(queriesList); + for (Map.Entry<VdcOperationCallback<?, ?>, List<VdcOperation<?, ?>>> callbackEntry: callbackMap.entrySet()) { + if (callbackEntry.getKey() instanceof VdcOperationCallbackList) { + ((VdcOperationCallbackList) callbackEntry.getKey()).onFailure(callbackEntry.getValue(), exception); + } else { + ((VdcOperationCallback) callbackEntry.getKey()).onFailure(callbackEntry.getValue().get(0), exception); + } } } @@ -256,43 +374,60 @@ } private void runMultipleActions(final VdcActionType actionType, final List<VdcOperation<?, ?>> operations, - List<VdcActionParametersBase> parameters, final List<VdcOperation<?, ?>> allActionOperations, + final List<VdcActionParametersBase> parameters, final List<VdcOperation<?, ?>> allActionOperations, final boolean waitForResults) { - getService().RunMultipleActions(actionType, (ArrayList<VdcActionParametersBase>) parameters, - false, waitForResults, new AsyncCallback<ArrayList<VdcReturnValueBase>>() { - + getService(new ServiceCallback() { @Override - public void onFailure(final Throwable exception) { - Map<VdcOperationCallback<?, ?>, List<VdcOperation<?, ?>>> callbackMap = - getCallbackMap(operations); - for (Map.Entry<VdcOperationCallback<?, ?>, List<VdcOperation<?, ?>>> callbackEntry: callbackMap.entrySet()) { - if (callbackEntry.getKey() instanceof VdcOperationCallbackList) { - ((VdcOperationCallbackList) callbackEntry.getKey()).onFailure(callbackEntry.getValue(), exception); - } else { - ((VdcOperationCallback) callbackEntry.getKey()).onFailure(callbackEntry.getValue().get(0), - exception); + public void serviceFound(GenericApiGWTServiceAsync service) { + service.RunMultipleActions(actionType, (ArrayList<VdcActionParametersBase>) parameters, + false, waitForResults, new AsyncCallback<ArrayList<VdcReturnValueBase>>() { + + @Override + public void onFailure(final Throwable exception) { + handleRunMultipleActionFailure(operations, exception); } - } + + @Override + public void onSuccess(final ArrayList<VdcReturnValueBase> result) { + Map<VdcOperationCallback<?, ?>, List<VdcOperation<?, ?>>> callbackMap = + getCallbackMap(operations); + for (Map.Entry<VdcOperationCallback<?, ?>, + List<VdcOperation<?, ?>>> callbackEntry: callbackMap.entrySet()) { + List<VdcReturnValueBase> actionResult = (List<VdcReturnValueBase>) + getOperationResult(callbackEntry.getValue(), allActionOperations, result); + if (callbackEntry.getKey() instanceof VdcOperationCallbackList) { + ((VdcOperationCallbackList) callbackEntry.getKey()) + .onSuccess(callbackEntry.getValue(), actionResult); + } else { + ((VdcOperationCallback) callbackEntry.getKey()) + .onSuccess(callbackEntry.getValue().get(0), actionResult.get(0)); + } + } + } + }); } @Override - public void onSuccess(final ArrayList<VdcReturnValueBase> result) { - Map<VdcOperationCallback<?, ?>, List<VdcOperation<?, ?>>> callbackMap = - getCallbackMap(operations); - for (Map.Entry<VdcOperationCallback<?, ?>, List<VdcOperation<?, ?>>> callbackEntry: callbackMap.entrySet()) { - List<VdcReturnValueBase> actionResult = (List<VdcReturnValueBase>) - getOperationResult(callbackEntry.getValue(), allActionOperations, result); - if (callbackEntry.getKey() instanceof VdcOperationCallbackList) { - ((VdcOperationCallbackList) callbackEntry.getKey()).onSuccess(callbackEntry.getValue(), - actionResult); - } else { - ((VdcOperationCallback) callbackEntry.getKey()).onSuccess(callbackEntry.getValue().get(0), - actionResult.get(0)); - } - } + public void onFailure(Throwable exception) { + handleRunMultipleActionFailure(operations, exception); } }); } + + private void handleRunMultipleActionFailure(final List<VdcOperation<?, ?>> operations, + final Throwable exception) { + Map<VdcOperationCallback<?, ?>, List<VdcOperation<?, ?>>> callbackMap = + getCallbackMap(operations); + for (Map.Entry<VdcOperationCallback<?, ?>, List<VdcOperation<?, ?>>> callbackEntry: callbackMap.entrySet()) { + if (callbackEntry.getKey() instanceof VdcOperationCallbackList) { + ((VdcOperationCallbackList) callbackEntry.getKey()).onFailure(callbackEntry.getValue(), exception); + } else { + ((VdcOperationCallback) callbackEntry.getKey()).onFailure(callbackEntry.getValue().get(0), + exception); + } + } + } + /** * Map operations by callback, so we can properly call a single callback for all related operations. * @param operationList The list of operations to determine the map for. @@ -344,17 +479,30 @@ */ @Override public void login(final VdcOperation<VdcActionType, LoginUserParameters> loginOperation) { - getService().Login(loginOperation.getParameter().getLoginName(), loginOperation.getParameter().getPassword(), - loginOperation.getParameter().getProfileName(), loginOperation.getOperation(), - new AsyncCallback<VdcReturnValueBase>() { + getService(new ServiceCallback() { @Override - public void onSuccess(final VdcReturnValueBase result) { - loginOperation.getCallback().onSuccess(loginOperation, result); + public void serviceFound(GenericApiGWTServiceAsync service) { + service.Login(loginOperation.getParameter().getLoginName(), loginOperation.getParameter().getPassword(), + loginOperation.getParameter().getProfileName(), loginOperation.getOperation(), + new AsyncCallback<VdcReturnValueBase>() { + @Override + public void onSuccess(final VdcReturnValueBase result) { + //Remove the rpc token when logging in. Due to session fixation protection we need a new + //token based on the new session. + getXsrfRequestBuilder().setXsrfToken(null); + loginOperation.getCallback().onSuccess(loginOperation, result); + } + + @Override + public void onFailure(final Throwable caught) { + loginOperation.getCallback().onFailure(loginOperation, caught); + } + }); } @Override - public void onFailure(final Throwable caught) { - loginOperation.getCallback().onFailure(loginOperation, caught); + public void onFailure(Throwable exception) { + loginOperation.getCallback().onFailure(loginOperation, exception); } }); } @@ -366,46 +514,95 @@ */ @Override public void logout(final Object userObject, final UserCallback callback) { - getService().logOff((DbUser) userObject, new AsyncCallback<VdcReturnValueBase>() { + getService(new ServiceCallback() { + @Override - public void onSuccess(final VdcReturnValueBase result) { - callback.onSuccess(result); + public void serviceFound(GenericApiGWTServiceAsync foundService) { + foundService.logOff((DbUser) userObject, new AsyncCallback<VdcReturnValueBase>() { + @Override + public void onSuccess(final VdcReturnValueBase result) { + //Remove the rpc token when logging out. + getXsrfRequestBuilder().setXsrfToken(null); + callback.onSuccess(result); + } + + @Override + public void onFailure(final Throwable caught) { + getXsrfRequestBuilder().setXsrfToken(null); + callback.onFailure(caught); + } + }); + } @Override - public void onFailure(final Throwable caught) { - callback.onFailure(caught); + public void onFailure(Throwable exception) { + callback.onFailure(exception); } }); } @Override public void storeInHttpSession(final String key, final String value, final StorageCallback callback) { - getService().storeInHttpSession(key, value, new AsyncCallback<Void>() { + getService(new ServiceCallback() { + @Override - public void onSuccess(final Void result) { - callback.onSuccess(null); + public void serviceFound(GenericApiGWTServiceAsync service) { + service.storeInHttpSession(key, value, new AsyncCallback<Void>() { + @Override + public void onSuccess(final Void result) { + callback.onSuccess(null); + } + + @Override + public void onFailure(final Throwable caught) { + callback.onFailure(caught); + } + }); } @Override - public void onFailure(final Throwable caught) { + public void onFailure(Throwable caught) { + getXsrfRequestBuilder().setXsrfToken(null); callback.onFailure(caught); } + }); } @Override public void retrieveFromHttpSession(final String key, final StorageCallback callback) { - getService().retrieveFromHttpSession(key, new AsyncCallback<String>() { + getService(new ServiceCallback() { + @Override - public void onSuccess(final String result) { - callback.onSuccess(result); + public void serviceFound(GenericApiGWTServiceAsync service) { + service.retrieveFromHttpSession(key, new AsyncCallback<String>() { + @Override + public void onSuccess(final String result) { + callback.onSuccess(result); + } + + @Override + public void onFailure(final Throwable caught) { + callback.onFailure(caught); + } + }); + } @Override - public void onFailure(final Throwable caught) { - callback.onFailure(caught); + public void onFailure(Throwable exception) { + getXsrfRequestBuilder().setXsrfToken(null); + callback.onFailure(exception); } }); } + + public void setBaseURL(String baseURL) { + this.baseUrl = baseURL; + } + + public XsrfRpcRequestBuilder getXsrfRequestBuilder() { + return this.xsrfRequestBuilder; + } } diff --git a/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/communication/XsrfRpcRequestBuilder.java b/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/communication/XsrfRpcRequestBuilder.java new file mode 100644 index 0000000..9e8b795 --- /dev/null +++ b/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/communication/XsrfRpcRequestBuilder.java @@ -0,0 +1,27 @@ +package org.ovirt.engine.ui.frontend.communication; + +import com.google.gwt.http.client.RequestBuilder; +import com.google.gwt.user.client.rpc.RpcRequestBuilder; +import com.google.gwt.user.client.rpc.XsrfToken; + +public class XsrfRpcRequestBuilder extends RpcRequestBuilder { + public static final String XSRF_TOKEN_HEADER = "X-CSRF-Token"; //$NON-NLS-1$ + + private XsrfToken xsrfToken; + + @Override + protected void doFinish(RequestBuilder rb) { + super.doFinish(rb); + if (xsrfToken != null) { + rb.setHeader(XSRF_TOKEN_HEADER, xsrfToken.getToken()); + } + } + + public XsrfToken getXsrfToken() { + return xsrfToken; + } + + public void setXsrfToken(XsrfToken xsrfToken) { + this.xsrfToken = xsrfToken; + } +} diff --git a/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/gwtservices/GenericApiGWTService.java b/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/gwtservices/GenericApiGWTService.java index 8e4036b..81bea66 100644 --- a/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/gwtservices/GenericApiGWTService.java +++ b/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/gwtservices/GenericApiGWTService.java @@ -10,11 +10,11 @@ import org.ovirt.engine.core.common.queries.VdcQueryReturnValue; import org.ovirt.engine.core.common.queries.VdcQueryType; -import com.google.gwt.rpc.client.RpcService; import com.google.gwt.user.client.rpc.RemoteServiceRelativePath; +import com.google.gwt.user.server.rpc.NoXsrfProtect; @RemoteServiceRelativePath("GenericApiGWTService") -public interface GenericApiGWTService extends RpcService { +public interface GenericApiGWTService extends XsrfProtectedRpcService { public VdcQueryReturnValue RunQuery(VdcQueryType search, VdcQueryParametersBase searchParameters); @@ -22,6 +22,7 @@ public VdcReturnValueBase RunAction(VdcActionType actionType, VdcActionParametersBase params); + @NoXsrfProtect public VdcQueryReturnValue RunPublicQuery(VdcQueryType queryType, VdcQueryParametersBase params); @@ -41,8 +42,10 @@ public DbUser getLoggedInUser(); + @NoXsrfProtect public VdcReturnValueBase logOff(DbUser userToLogoff); + @NoXsrfProtect public VdcReturnValueBase Login(String userName, String password, String profileName, VdcActionType loginType); public void storeInHttpSession(String key, String value); diff --git a/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/gwtservices/XsrfProtectedRpcService.java b/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/gwtservices/XsrfProtectedRpcService.java new file mode 100644 index 0000000..87dafd1 --- /dev/null +++ b/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/gwtservices/XsrfProtectedRpcService.java @@ -0,0 +1,8 @@ +package org.ovirt.engine.ui.frontend.gwtservices; + +import com.google.gwt.rpc.client.RpcService; +import com.google.gwt.user.server.rpc.XsrfProtect; + +@XsrfProtect +public interface XsrfProtectedRpcService extends RpcService { +} diff --git a/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/server/gwt/AbstractXsrfProtectedRpcServlet.java b/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/server/gwt/AbstractXsrfProtectedRpcServlet.java new file mode 100644 index 0000000..854d17b --- /dev/null +++ b/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/server/gwt/AbstractXsrfProtectedRpcServlet.java @@ -0,0 +1,93 @@ +package org.ovirt.engine.ui.frontend.server.gwt; + +import java.io.IOException; +import java.lang.reflect.Method; +import java.util.Collections; +import java.util.List; + +import javax.servlet.ServletException; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; + +import org.ovirt.engine.ui.frontend.communication.XsrfRpcRequestBuilder; + +import com.google.gwt.rpc.server.RpcServlet; +import com.google.gwt.user.client.rpc.RpcToken; +import com.google.gwt.user.client.rpc.XsrfToken; +import com.google.gwt.user.server.Util; +import com.google.gwt.user.server.rpc.NoXsrfProtect; +import com.google.gwt.user.server.rpc.RPCRequest; +import com.google.gwt.user.server.rpc.XsrfProtect; + +public abstract class AbstractXsrfProtectedRpcServlet extends RpcServlet { + + /** + * Serial version UID for serialization. + */ + static final long serialVersionUID = -7274292100456700624L; + + private RpcToken token; + + /** + * The default constructor used by service implementations that extend this class. The servlet will delegate AJAX + * requests to the appropriate method in the subclass. + */ + public AbstractXsrfProtectedRpcServlet() { + super(); + } + + /** + * The wrapping constructor used by service implementations that are separate from this class. The servlet will + * delegate AJAX requests to the appropriate method in the given object. + * + * @param delegate + * The delegate object. + */ + public AbstractXsrfProtectedRpcServlet(Object delegate) { + super(delegate); + } + + @Override + protected void service(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException { + extractTokenFromRequest(req); + super.service(req, resp); + } + + @SuppressWarnings("unchecked") + private void extractTokenFromRequest(HttpServletRequest req) { + List<String> header = Collections.list(req.getHeaders(XsrfRpcRequestBuilder.XSRF_TOKEN_HEADER)); + if (header != null && header.size() == 1) { + this.token = new XsrfToken(header.get(0)); + } + } + + @Override + protected void onAfterRequestDeserialized(RPCRequest rpcRequest) { + if (shouldValidateXsrfToken(rpcRequest.getMethod())) { + validateXsrfToken(this.token, rpcRequest.getMethod()); + } + this.token = null; + } + + /** + * Override this method to change default XSRF enforcement logic. + * + * @param method + * Method being invoked + * @return {@code true} if XSRF token should be verified, {@code false} otherwise + */ + protected boolean shouldValidateXsrfToken(Method method) { + return Util.isMethodXsrfProtected(method, XsrfProtect.class, + NoXsrfProtect.class, RpcToken.class); + } + + /** + * Override this method to perform XSRF token verification. + * + * @param token + * {@link RpcToken} included with an RPC request. + * @param method + * method being invoked via this RPC call. + */ + protected abstract void validateXsrfToken(RpcToken token, Method method); +} diff --git a/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/server/gwt/GenericApiGWTServiceImpl.java b/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/server/gwt/GenericApiGWTServiceImpl.java index 5356e47..aa649ab 100644 --- a/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/server/gwt/GenericApiGWTServiceImpl.java +++ b/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/server/gwt/GenericApiGWTServiceImpl.java @@ -4,7 +4,6 @@ import java.util.Random; import javax.ejb.EJB; -import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpSession; @@ -21,10 +20,9 @@ import org.ovirt.engine.core.common.queries.VdcQueryType; import org.ovirt.engine.ui.frontend.gwtservices.GenericApiGWTService; -import com.google.gwt.rpc.server.RpcServlet; import com.google.gwt.user.client.rpc.SerializationException; -public class GenericApiGWTServiceImpl extends RpcServlet implements GenericApiGWTService { +public class GenericApiGWTServiceImpl extends XsrfProtectedRpcServlet implements GenericApiGWTService { private static final long serialVersionUID = 7395780289048030855L; @@ -45,11 +43,6 @@ public BackendLocal getBackend() { return backend; - } - - @Override - public void init() throws ServletException { - log.debug("Initializing servlet!"); //$NON-NLS-1$ } @Override diff --git a/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/server/gwt/XsrfProtectedRpcServlet.java b/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/server/gwt/XsrfProtectedRpcServlet.java new file mode 100644 index 0000000..aa7b6e0 --- /dev/null +++ b/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/server/gwt/XsrfProtectedRpcServlet.java @@ -0,0 +1,92 @@ +package org.ovirt.engine.ui.frontend.server.gwt; + +import java.lang.reflect.Method; + +import javax.servlet.ServletException; +import javax.servlet.http.Cookie; + +import com.google.gwt.user.client.rpc.RpcToken; +import com.google.gwt.user.client.rpc.RpcTokenException; +import com.google.gwt.user.client.rpc.XsrfToken; +import com.google.gwt.user.server.Util; +import com.google.gwt.user.server.rpc.XsrfTokenServiceServlet; +import com.google.gwt.util.tools.shared.Md5Utils; +import com.google.gwt.util.tools.shared.StringUtils; + +public class XsrfProtectedRpcServlet extends AbstractXsrfProtectedRpcServlet { + + // Can't use the one from XsrfTokenServiceServlet as it is not public. + static final String COOKIE_NAME_NOT_SET_ERROR_MSG = + "Session cookie name not set! Use context-param to specify session cookie name"; //$NON-NLS-1$ + + String sessionCookieName = null; + + /** + * Default constructor. + */ + public XsrfProtectedRpcServlet() { + this(null); + } + + /** + * Constructor with session cookie name. + * @param cookieName The session cookie name. + */ + public XsrfProtectedRpcServlet(String cookieName) { + this.sessionCookieName = cookieName; + } + + /** + * Constructor with delegate. + * @param delegate The delegate object + */ + public XsrfProtectedRpcServlet(Object delegate) { + this(delegate, null); + } + + /** + * Constructor with cookie name and delegate. + * @param delegate The delegate object + * @param cookieName The name of the session cookie. + */ + public XsrfProtectedRpcServlet(Object delegate, String cookieName) { + super(delegate); + this.sessionCookieName = cookieName; + } + + @Override + public void init() throws ServletException { + // do not overwrite if value is supplied in constructor + if (sessionCookieName == null) { + // servlet configuration precedes context configuration + sessionCookieName = getServletConfig().getInitParameter(XsrfTokenServiceServlet.COOKIE_NAME_PARAM); + if (sessionCookieName == null) { + sessionCookieName = getServletContext().getInitParameter(XsrfTokenServiceServlet.COOKIE_NAME_PARAM); + } + if (sessionCookieName == null) { + throw new IllegalStateException(COOKIE_NAME_NOT_SET_ERROR_MSG); + } + } + } + + @Override + protected void validateXsrfToken(RpcToken token, Method method) { + if (token == null) { + throw new RpcTokenException("XSRF token missing"); //$NON-NLS-1$ + } + Cookie sessionCookie = Util.getCookie(getThreadLocalRequest(), sessionCookieName, false); + if (sessionCookie == null || sessionCookie.getValue() == null + || sessionCookie.getValue().length() == 0) { + throw new RpcTokenException("Session cookie is missing or empty! " + //$NON-NLS-1$ + "Unable to verify XSRF cookie"); //$NON-NLS-1$ + } + + String expectedToken = StringUtils.toHexString( + Md5Utils.getMd5Digest(sessionCookie.getValue().getBytes())); + XsrfToken xsrfToken = (XsrfToken) token; + + if (!expectedToken.equals(xsrfToken.getToken())) { + throw new RpcTokenException("Invalid XSRF token"); //$NON-NLS-1$ + } + } +} diff --git a/frontend/webadmin/modules/frontend/src/main/resources/META-INF/web-fragment.xml b/frontend/webadmin/modules/frontend/src/main/resources/META-INF/web-fragment.xml index 1a13102..67350c1 100644 --- a/frontend/webadmin/modules/frontend/src/main/resources/META-INF/web-fragment.xml +++ b/frontend/webadmin/modules/frontend/src/main/resources/META-INF/web-fragment.xml @@ -3,6 +3,11 @@ <display-name>Frontend Web Fragment for GWT UI Projects</display-name> + <context-param> + <param-name>gwt.xsrf.session_cookie_name</param-name> + <param-value>JSESSIONID</param-value> + </context-param> + <filter> <filter-name>GwtCachingFilter</filter-name> <filter-class>org.ovirt.engine.ui.frontend.server.gwt.GwtCachingFilter</filter-class> @@ -86,6 +91,10 @@ <servlet-class>org.ovirt.engine.core.branding.BrandingCascadingResourceServlet</servlet-class> </servlet> + <servlet> + <servlet-name>xsrf</servlet-name> + <servlet-class>com.google.gwt.user.server.rpc.XsrfTokenServiceServlet</servlet-class> + </servlet> <!-- PageNotFound Servlet --> <servlet> <servlet-name>PageNotFoundForwardServlet</servlet-name> diff --git a/frontend/webadmin/modules/frontend/src/test/java/org/ovirt/engine/ui/frontend/FrontendActionTest.java b/frontend/webadmin/modules/frontend/src/test/java/org/ovirt/engine/ui/frontend/FrontendActionTest.java index 3c41bbd..83a4810 100644 --- a/frontend/webadmin/modules/frontend/src/test/java/org/ovirt/engine/ui/frontend/FrontendActionTest.java +++ b/frontend/webadmin/modules/frontend/src/test/java/org/ovirt/engine/ui/frontend/FrontendActionTest.java @@ -4,10 +4,12 @@ import static org.junit.Assert.assertNull; import static org.mockito.Matchers.any; import static org.mockito.Matchers.eq; +import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import static org.mockito.Mockito.withSettings; import java.util.ArrayList; import java.util.Arrays; @@ -43,7 +45,10 @@ import com.google.gwt.event.shared.EventBus; import com.google.gwt.user.client.rpc.AsyncCallback; +import com.google.gwt.user.client.rpc.ServiceDefTarget; import com.google.gwt.user.client.rpc.StatusCodeException; +import com.google.gwt.user.client.rpc.XsrfToken; +import com.google.gwt.user.client.rpc.XsrfTokenServiceAsync; @RunWith(MockitoJUnitRunner.class) /** @@ -61,8 +66,9 @@ */ FakeGWTScheduler fakeScheduler; - @Mock GenericApiGWTServiceAsync mockService; + XsrfTokenServiceAsync mockXsrfService; + @Mock ErrorTranslator mockVdsmErrorsTranslator; @Mock @@ -99,7 +105,12 @@ @Before public void setUp() throws Exception { fakeScheduler = new FakeGWTScheduler(); - CommunicationProvider communicationsProvider = new GWTRPCCommunicationProvider(mockService); + mockService = mock(GenericApiGWTServiceAsync.class, withSettings().extraInterfaces(ServiceDefTarget.class)); + mockXsrfService = mock(XsrfTokenServiceAsync.class, withSettings().extraInterfaces(ServiceDefTarget.class)); + CommunicationProvider communicationsProvider = new GWTRPCCommunicationProvider(mockService, mockXsrfService); + ((GWTRPCCommunicationProvider)communicationsProvider).getXsrfRequestBuilder() + .setXsrfToken(new XsrfToken("Something")); //$NON-NLS-1$ + ((GWTRPCCommunicationProvider)communicationsProvider).setBaseURL("/baseurl"); //$NON-NLS-1$ OperationProcessor operationProcessor = new OperationProcessor(communicationsProvider); operationProcessor.setScheduler(fakeScheduler); VdcOperationManager operationsManager = new VdcOperationManager(operationProcessor); diff --git a/frontend/webadmin/modules/frontend/src/test/java/org/ovirt/engine/ui/frontend/FrontendTest.java b/frontend/webadmin/modules/frontend/src/test/java/org/ovirt/engine/ui/frontend/FrontendTest.java index 1607ced..fbda911 100644 --- a/frontend/webadmin/modules/frontend/src/test/java/org/ovirt/engine/ui/frontend/FrontendTest.java +++ b/frontend/webadmin/modules/frontend/src/test/java/org/ovirt/engine/ui/frontend/FrontendTest.java @@ -4,10 +4,12 @@ import static org.mockito.Matchers.any; import static org.mockito.Matchers.eq; import static org.mockito.Mockito.atLeastOnce; +import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import static org.mockito.Mockito.withSettings; import java.util.ArrayList; import java.util.List; @@ -40,7 +42,10 @@ import com.google.gwt.event.shared.EventBus; import com.google.gwt.user.client.rpc.AsyncCallback; +import com.google.gwt.user.client.rpc.ServiceDefTarget; import com.google.gwt.user.client.rpc.StatusCodeException; +import com.google.gwt.user.client.rpc.XsrfToken; +import com.google.gwt.user.client.rpc.XsrfTokenServiceAsync; @RunWith(MockitoJUnitRunner.class) public class FrontendTest { @@ -57,8 +62,9 @@ */ FakeGWTScheduler fakeScheduler; - @Mock GenericApiGWTServiceAsync mockService; + XsrfTokenServiceAsync mockXsrfService; + @Mock ErrorTranslator mockVdsmErrorsTranslator; @Mock @@ -93,7 +99,12 @@ @Before public void setUp() throws Exception { fakeScheduler = new FakeGWTScheduler(); - CommunicationProvider communicationsProvider = new GWTRPCCommunicationProvider(mockService); + mockService = mock(GenericApiGWTServiceAsync.class, withSettings().extraInterfaces(ServiceDefTarget.class)); + mockXsrfService = mock(XsrfTokenServiceAsync.class, withSettings().extraInterfaces(ServiceDefTarget.class)); + CommunicationProvider communicationsProvider = new GWTRPCCommunicationProvider(mockService, mockXsrfService); + ((GWTRPCCommunicationProvider)communicationsProvider).getXsrfRequestBuilder() + .setXsrfToken(new XsrfToken("Something")); //$NON-NLS-1$ + ((GWTRPCCommunicationProvider)communicationsProvider).setBaseURL("/baseurl"); //$NON-NLS-1$ OperationProcessor operationProcessor = new OperationProcessor(communicationsProvider); operationProcessor.setScheduler(fakeScheduler); VdcOperationManager operationsManager = new VdcOperationManager(operationProcessor); diff --git a/frontend/webadmin/modules/frontend/src/test/java/org/ovirt/engine/ui/frontend/communication/GWTRPCCommunicationProviderTest.java b/frontend/webadmin/modules/frontend/src/test/java/org/ovirt/engine/ui/frontend/communication/GWTRPCCommunicationProviderTest.java index a8283c2..8652f3e 100644 --- a/frontend/webadmin/modules/frontend/src/test/java/org/ovirt/engine/ui/frontend/communication/GWTRPCCommunicationProviderTest.java +++ b/frontend/webadmin/modules/frontend/src/test/java/org/ovirt/engine/ui/frontend/communication/GWTRPCCommunicationProviderTest.java @@ -2,8 +2,11 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.fail; +import static org.mockito.Matchers.any; import static org.mockito.Matchers.eq; +import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.withSettings; import java.util.ArrayList; import java.util.List; @@ -24,11 +27,16 @@ import org.ovirt.engine.ui.frontend.gwtservices.GenericApiGWTServiceAsync; import com.google.gwt.user.client.rpc.AsyncCallback; +import com.google.gwt.user.client.rpc.ServiceDefTarget; +import com.google.gwt.user.client.rpc.XsrfToken; +import com.google.gwt.user.client.rpc.XsrfTokenServiceAsync; @RunWith(MockitoJUnitRunner.class) public class GWTRPCCommunicationProviderTest { - @Mock + GenericApiGWTServiceAsync mockService; + XsrfTokenServiceAsync mockXsrfService; + @Mock VdcOperationCallback mockOperationCallbackSingle1; @Mock @@ -53,7 +61,11 @@ @Before public void setUp() throws Exception { - testProvider = new GWTRPCCommunicationProvider(mockService); + mockService = mock(GenericApiGWTServiceAsync.class, withSettings().extraInterfaces(ServiceDefTarget.class)); + mockXsrfService = mock(XsrfTokenServiceAsync.class, withSettings().extraInterfaces(ServiceDefTarget.class)); + testProvider = new GWTRPCCommunicationProvider(mockService, mockXsrfService); + testProvider.xsrfRequestBuilder.setXsrfToken(new XsrfToken("Something")); //$NON-NLS-1$ + testProvider.setBaseURL("/baseurl"); //$NON-NLS-1$ } @Test @@ -514,6 +526,21 @@ verify(mockOperationCallbackSingle2).onSuccess(testOperation2, testQueryResult); } + @SuppressWarnings("unchecked") + @Test + public void testMissingXsrfToken() { + //Remove token so there should be a request for it. + testProvider.xsrfRequestBuilder.setXsrfToken(null); + VdcQueryParametersBase testParameters = new VdcQueryParametersBase(); + final List<VdcOperation<VdcQueryType, VdcQueryParametersBase>> operationList = + new ArrayList<VdcOperation<VdcQueryType, VdcQueryParametersBase>>(); + final VdcOperation<VdcQueryType, VdcQueryParametersBase> testOperation = + new VdcOperation<VdcQueryType, VdcQueryParametersBase>(VdcQueryType.Search, testParameters, null); + operationList.add(testOperation); + testProvider.transmitOperation(testOperation); + verify(mockXsrfService).getNewXsrfToken((AsyncCallback<XsrfToken>) any()); + } + // ******************************************************************************************************** // * Helper functions // ******************************************************************************************************** diff --git a/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/gin/BaseSystemModule.java b/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/gin/BaseSystemModule.java index dc22230..863825b 100644 --- a/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/gin/BaseSystemModule.java +++ b/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/gin/BaseSystemModule.java @@ -26,6 +26,7 @@ import com.google.gwt.event.shared.SimpleEventBus; import com.google.gwt.inject.client.AbstractGinModule; +import com.google.gwt.user.client.rpc.XsrfTokenServiceAsync; import com.google.inject.Singleton; import com.gwtplatform.mvp.client.RootPresenter; import com.gwtplatform.mvp.client.proxy.ParameterTokenFormatter; @@ -67,6 +68,7 @@ bind(OperationProcessor.class).in(Singleton.class); bind(CommunicationProvider.class).to(GWTRPCCommunicationProvider.class).in(Singleton.class); bind(GenericApiGWTServiceAsync.class).in(Singleton.class); + bind(XsrfTokenServiceAsync.class).in(Singleton.class); } protected void bindResourceConfiguration( diff --git a/frontend/webadmin/modules/userportal-gwtp/src/main/webapp/WEB-INF/web.xml b/frontend/webadmin/modules/userportal-gwtp/src/main/webapp/WEB-INF/web.xml index 82a1013..f3eba4d 100644 --- a/frontend/webadmin/modules/userportal-gwtp/src/main/webapp/WEB-INF/web.xml +++ b/frontend/webadmin/modules/userportal-gwtp/src/main/webapp/WEB-INF/web.xml @@ -75,6 +75,11 @@ </servlet-mapping> <servlet-mapping> + <servlet-name>xsrf</servlet-name> + <url-pattern>/xsrf</url-pattern> + </servlet-mapping> + + <servlet-mapping> <servlet-name>BrandingServlet</servlet-name> <url-pattern>/theme/*</url-pattern> </servlet-mapping> diff --git a/frontend/webadmin/modules/webadmin/src/main/webapp/WEB-INF/web.xml b/frontend/webadmin/modules/webadmin/src/main/webapp/WEB-INF/web.xml index b637adf..0ba41a9 100644 --- a/frontend/webadmin/modules/webadmin/src/main/webapp/WEB-INF/web.xml +++ b/frontend/webadmin/modules/webadmin/src/main/webapp/WEB-INF/web.xml @@ -75,6 +75,11 @@ <url-pattern>/GenericApiGWTService</url-pattern> </servlet-mapping> + <servlet-mapping> + <servlet-name>xsrf</servlet-name> + <url-pattern>/xsrf</url-pattern> + </servlet-mapping> + <servlet-mapping> <servlet-name>PluginResourceServlet</servlet-name> <url-pattern>/plugin/*</url-pattern> -- To view, visit http://gerrit.ovirt.org/27024 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ia4a5ad1f33eb985aa79e1376aecb48ae365d334d Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Alexander Wels <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
