From: Olli Tietäväinen Date: Thu, 10 Aug 2017 13:07:42 +0000 (+0300) Subject: Use separate identifier for push connections (#9150) X-Git-Tag: 7.7.11~11 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=97fa55d3d447098bbf605054b1ccfa08a848dc3e;p=vaadin-framework.git Use separate identifier for push connections (#9150) By using a separate id we can avoid sending the sessions CSRF token as a GET parameter when initializing a push connection. Cherry-picked from #8700 to the 7.7 branch. --- diff --git a/client/src/main/java/com/vaadin/client/communication/AtmospherePushConnection.java b/client/src/main/java/com/vaadin/client/communication/AtmospherePushConnection.java index 34e1c5ff30..a38bc7268b 100644 --- a/client/src/main/java/com/vaadin/client/communication/AtmospherePushConnection.java +++ b/client/src/main/java/com/vaadin/client/communication/AtmospherePushConnection.java @@ -207,10 +207,10 @@ public class AtmospherePushConnection implements PushConnection { String extraParams = UIConstants.UI_ID_PARAMETER + "=" + connection.getConfiguration().getUIId(); - String csrfToken = connection.getMessageHandler().getCsrfToken(); - if (!csrfToken.equals(ApplicationConstants.CSRF_TOKEN_DEFAULT_VALUE)) { - extraParams += "&" + ApplicationConstants.CSRF_TOKEN_PARAMETER + "=" - + csrfToken; + String pushId = connection.getMessageHandler().getPushId(); + if (pushId != null) { + extraParams += "&" + ApplicationConstants.PUSH_ID_PARAMETER + "=" + + pushId; } // uri is needed to identify the right connection when closing diff --git a/client/src/main/java/com/vaadin/client/communication/MessageHandler.java b/client/src/main/java/com/vaadin/client/communication/MessageHandler.java index 38b9a783e5..14cd31ec32 100644 --- a/client/src/main/java/com/vaadin/client/communication/MessageHandler.java +++ b/client/src/main/java/com/vaadin/client/communication/MessageHandler.java @@ -133,6 +133,9 @@ public class MessageHandler { // will hold the CSRF token once received private String csrfToken = ApplicationConstants.CSRF_TOKEN_DEFAULT_VALUE; + // holds the push identifier once received + private String pushId = null; + /** Timer for automatic redirect to SessionExpiredURL */ private Timer redirectTimer; @@ -349,6 +352,12 @@ public class MessageHandler { csrfToken = json .getString(ApplicationConstants.UIDL_SECURITY_TOKEN_ID); } + + // Get push id if present + if (json.containsKey(ApplicationConstants.UIDL_PUSH_ID)) { + pushId = json.getString(ApplicationConstants.UIDL_PUSH_ID); + } + getLogger().info(" * Handling resources from server"); if (json.containsKey("resources")) { @@ -361,7 +370,7 @@ public class MessageHandler { } } handleUIDLDuration - .logDuration(" * Handling resources from server completed", 10); + .logDuration(" * Handling resources from server completed", 10); getLogger().info(" * Handling type inheritance map from server"); @@ -489,7 +498,7 @@ public class MessageHandler { if (json.containsKey("dd")) { // response contains data for drag and drop service VDragAndDropManager.get() - .handleServerResponse(json.getValueMap("dd")); + .handleServerResponse(json.getValueMap("dd")); } unregisterRemovedConnectors( @@ -647,13 +656,13 @@ public class MessageHandler { if (child instanceof ComponentConnector && ((ComponentConnector) child) - .delegateCaptionHandling()) { + .delegateCaptionHandling()) { ServerConnector parent = child.getParent(); if (parent instanceof HasComponentsConnector) { Profiler.enter( "HasComponentsConnector.updateCaption"); ((HasComponentsConnector) parent) - .updateCaption((ComponentConnector) child); + .updateCaption((ComponentConnector) child); Profiler.leave( "HasComponentsConnector.updateCaption"); } @@ -732,7 +741,7 @@ public class MessageHandler { throw new RuntimeException( "Missing data needed to invoke @DelegateToWidget for " + component.getClass().getSimpleName(), - e); + e); } } @@ -827,7 +836,7 @@ public class MessageHandler { } getLogger().info("* Unregistered " + detachedArray.length() - + " connectors"); + + " connectors"); Profiler.leave("unregisterRemovedConnectors"); } @@ -908,7 +917,7 @@ public class MessageHandler { } getLogger() - .info(" * Passing UIDL to Vaadin 6 style connectors"); + .info(" * Passing UIDL to Vaadin 6 style connectors"); // update paintables for (int i = 0; i < length; i++) { try { @@ -935,14 +944,14 @@ public class MessageHandler { } else if (legacyConnector == null) { getLogger().severe( "Received update for " + uidl.getTag() - + ", but there is no such paintable (" - + connectorId + ") rendered."); + + ", but there is no such paintable (" + + connectorId + ") rendered."); } else { getLogger() - .severe("Server sent Vaadin 6 style updates for " - + Util.getConnectorString( - legacyConnector) - + " but this is not a Vaadin 6 Paintable"); + .severe("Server sent Vaadin 6 style updates for " + + Util.getConnectorString( + legacyConnector) + + " but this is not a Vaadin 6 Paintable"); } } catch (final Throwable e) { @@ -1035,8 +1044,8 @@ public class MessageHandler { if (connector instanceof HasJavaScriptConnectorHelper) { ((HasJavaScriptConnectorHelper) connector) - .getJavascriptConnectorHelper() - .setNativeState(jso); + .getJavascriptConnectorHelper() + .setNativeState(jso); } SharedState state = connector.getState(); @@ -1244,7 +1253,7 @@ public class MessageHandler { newChildren.add(childConnector); if (childConnector instanceof ComponentConnector) { newComponents - .add((ComponentConnector) childConnector); + .add((ComponentConnector) childConnector); } else if (!(childConnector instanceof AbstractExtensionConnector)) { throw new IllegalStateException(Util .getConnectorString(childConnector) @@ -1419,7 +1428,7 @@ public class MessageHandler { Profiler.enter( prefix + "recursivelyDetach clear children and parent"); connector - .setChildren(Collections. emptyList()); + .setChildren(Collections. emptyList()); connector.setParent(null); Profiler.leave( prefix + "recursivelyDetach clear children and parent"); @@ -1465,7 +1474,7 @@ public class MessageHandler { Profiler.enter("handleRpcInvocations"); getLogger() - .info(" * Performing server to client RPC calls"); + .info(" * Performing server to client RPC calls"); JsonArray rpcCalls = Util .jso2json(json.getJavaScriptObject("rpc")); @@ -1693,6 +1702,17 @@ public class MessageHandler { return csrfToken; } + /** + * Gets the push connection identifier for this session. Used when + * establishing a push connection with the client. + * + * @return the push connection identifier string + * @since 7.7.11 + */ + public String getPushId() { + return pushId; + } + /** * Checks whether state changes are currently being processed. Certain * operations are not allowed when the internal state of the application diff --git a/server/src/main/java/com/vaadin/server/VaadinSession.java b/server/src/main/java/com/vaadin/server/VaadinSession.java index 7014a5f1f5..8cb833e68f 100644 --- a/server/src/main/java/com/vaadin/server/VaadinSession.java +++ b/server/src/main/java/com/vaadin/server/VaadinSession.java @@ -508,8 +508,8 @@ public class VaadinSession implements HttpSessionBindingListener, Serializable { private void refreshLock() { assert lock == null || lock == service.getSessionLock( session) : "Cannot change the lock from one instance to another"; - assert hasLock(service, session); - lock = service.getSessionLock(session); + assert hasLock(service, session); + lock = service.getSessionLock(session); } public void setCommunicationManager( @@ -747,6 +747,8 @@ public class VaadinSession implements HttpSessionBindingListener, Serializable { private final String csrfToken = UUID.randomUUID().toString(); + private final String pushId = UUID.randomUUID().toString(); + /** * Generate an id for the given Connector. Connectors must not call this * method more than once, the first time they need an id. @@ -1008,12 +1010,12 @@ public class VaadinSession implements HttpSessionBindingListener, Serializable { getLogger().log(Level.SEVERE, "Exception while cleaning connector map for ui " + ui.getUIId(), - e); + e); } catch (AssertionError e) { getLogger().log(Level.SEVERE, "Exception while cleaning connector map for ui " + ui.getUIId(), - e); + e); } } } @@ -1089,7 +1091,7 @@ public class VaadinSession implements HttpSessionBindingListener, Serializable { } if (value != null && !type.isInstance(value)) { throw new IllegalArgumentException("value of type " + type.getName() - + " expected but got " + value.getClass().getName()); + + " expected but got " + value.getClass().getName()); } setAttribute(type.getName(), value); } @@ -1287,7 +1289,7 @@ public class VaadinSession implements HttpSessionBindingListener, Serializable { protected void setState(State state) { assert hasLock(); assert this.state.isValidChange(state) : "Invalid session state change " - + this.state + "->" + state; + + this.state + "->" + state; this.state = state; } @@ -1422,6 +1424,18 @@ public class VaadinSession implements HttpSessionBindingListener, Serializable { return csrfToken; } + /** + * Gets the push connection identifier for this session. Used when + * establishing a push connection with the client. + * + * @return the push connection identifier string + * @since 7.7.11 + */ + public String getPushId() { + assert hasLock(); + return pushId; + } + /** * Override default deserialization logic to account for transient * {@link #pendingAccessQueue}. diff --git a/server/src/main/java/com/vaadin/server/communication/PushHandler.java b/server/src/main/java/com/vaadin/server/communication/PushHandler.java index 7cc41aeda3..2477a76d5c 100644 --- a/server/src/main/java/com/vaadin/server/communication/PushHandler.java +++ b/server/src/main/java/com/vaadin/server/communication/PushHandler.java @@ -91,10 +91,10 @@ public class PushHandler { } String requestToken = resource.getRequest() - .getParameter(ApplicationConstants.CSRF_TOKEN_PARAMETER); - if (!VaadinService.isCsrfTokenValid(session, requestToken)) { + .getParameter(ApplicationConstants.PUSH_ID_PARAMETER); + if (!isPushIdValid(session, requestToken)) { getLogger().log(Level.WARNING, - "Invalid CSRF token in new connection received from {0}", + "Invalid identifier in new connection received from {0}", resource.getRequest().getRemoteHost()); // Refresh on client side, create connection just for // sending a message @@ -472,6 +472,25 @@ public class PushHandler { return Logger.getLogger(PushHandler.class.getName()); } + /** + * Checks whether a given push id matches the session's push id. + * + * @param session + * the vaadin session for which the check should be done + * @param requestPushId + * the push id provided in the request + * @return {@code true} if the id is valid, {@code false} otherwise + */ + private static boolean isPushIdValid(VaadinSession session, + String requestPushId) { + + String sessionPushId = session.getPushId(); + if (requestPushId == null || !requestPushId.equals(sessionPushId)) { + return false; + } + return true; + } + /** * Called when a new push connection is requested to be opened by the client * diff --git a/server/src/main/java/com/vaadin/server/communication/UIInitHandler.java b/server/src/main/java/com/vaadin/server/communication/UIInitHandler.java index aa167d137f..9ef63c3138 100644 --- a/server/src/main/java/com/vaadin/server/communication/UIInitHandler.java +++ b/server/src/main/java/com/vaadin/server/communication/UIInitHandler.java @@ -220,7 +220,7 @@ public abstract class UIInitHandler extends SynchronizedRequestHandler { session.addUI(ui); if (initException != null) { ui.getSession().getCommunicationManager() - .handleConnectorRelatedException(ui, initException); + .handleConnectorRelatedException(ui, initException); } // Warn if the window can't be preserved if (embedId == null @@ -288,6 +288,7 @@ public abstract class UIInitHandler extends SynchronizedRequestHandler { if (session.getConfiguration().isXsrfProtectionEnabled()) { writer.write(getSecurityKeyUIDL(session)); } + writer.write(getPushIdUIDL(session)); new UidlWriter().write(uI, writer, false); writer.write("}"); @@ -310,7 +311,20 @@ public abstract class UIInitHandler extends SynchronizedRequestHandler { String seckey = session.getCsrfToken(); return "\"" + ApplicationConstants.UIDL_SECURITY_TOKEN_ID + "\":\"" - + seckey + "\","; + + seckey + "\","; + } + + /** + * Gets the push connection identifier as UIDL. + * + * @param session + * the vaadin session to which the security key belongs + * @return the push identifier UIDL + * @since 7.7.11 + */ + private static String getPushIdUIDL(VaadinSession session) { + return "\"" + ApplicationConstants.UIDL_PUSH_ID + "\":\"" + + session.getPushId() + "\","; } private static final Logger getLogger() { diff --git a/shared/src/main/java/com/vaadin/shared/ApplicationConstants.java b/shared/src/main/java/com/vaadin/shared/ApplicationConstants.java index 8ce9cef2d5..2a2d4c8ccb 100644 --- a/shared/src/main/java/com/vaadin/shared/ApplicationConstants.java +++ b/shared/src/main/java/com/vaadin/shared/ApplicationConstants.java @@ -49,6 +49,8 @@ public class ApplicationConstants implements Serializable { public static final String UIDL_SECURITY_TOKEN_ID = "Vaadin-Security-Key"; + public static final String UIDL_PUSH_ID = "Vaadin-Push-ID"; + @Deprecated public static final String UPDATE_VARIABLE_INTERFACE = "v"; @Deprecated @@ -108,6 +110,13 @@ public class ApplicationConstants implements Serializable { */ public static final String CSRF_TOKEN_PARAMETER = "v-csrfToken"; + /** + * Name of the parameter used to transmit the push connection identifier. + * + * @since 7.7.11 + */ + public static final String PUSH_ID_PARAMETER = "v-pushId"; + /** * The name of the parameter used to transmit RPC invocations *