SEC-2864: Default Spring Security WebSocket PathMatcher (fails Circular Bean)

Author: Rob Winch

config/src/main/java/org/springframework/security/config/annotation/web/socket/AbstractSecurityWebSocketMessageBrokerConfigurer.java

@@ -15,13 +15,19 @@
  */
 package org.springframework.security.config.annotation.web.socket;
 
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+
+import org.springframework.beans.factory.NoSuchBeanDefinitionException;
 import org.springframework.beans.factory.SmartInitializingSingleton;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.context.ApplicationContext;
 import org.springframework.context.annotation.Bean;
 import org.springframework.core.Ordered;
 import org.springframework.core.annotation.Order;
 import org.springframework.messaging.handler.invocation.HandlerMethodArgumentResolver;
+import org.springframework.messaging.simp.annotation.support.SimpAnnotationMethodMessageHandler;
 import org.springframework.messaging.simp.config.ChannelRegistration;
 import org.springframework.security.access.AccessDecisionVoter;
 import org.springframework.security.access.vote.AffirmativeBased;
@@ -33,6 +39,7 @@
 import org.springframework.security.messaging.context.SecurityContextChannelInterceptor;
 import org.springframework.security.messaging.web.csrf.CsrfChannelInterceptor;
 import org.springframework.security.messaging.web.socket.server.CsrfTokenHandshakeInterceptor;
+import org.springframework.util.PathMatcher;
 import org.springframework.web.servlet.handler.SimpleUrlHandlerMapping;
 import org.springframework.web.socket.config.annotation.AbstractWebSocketMessageBrokerConfigurer;
 import org.springframework.web.socket.config.annotation.StompEndpointRegistry;
@@ -42,10 +49,6 @@
 import org.springframework.web.socket.sockjs.support.SockJsHttpRequestHandler;
 import org.springframework.web.socket.sockjs.transport.TransportHandlingSockJsService;
 
-import java.util.ArrayList;
-import java.util.List;
-import java.util.Map;
-
 /**
  * Allows configuring WebSocket Authorization.
  *
@@ -57,7 +60,7 @@
  * @Configuration
  * public class WebSocketSecurityConfig extends
  * 		AbstractSecurityWebSocketMessageBrokerConfigurer {
- * 
+ *
  * 	@Override
  * 	protected void configureInbound(MessageSecurityMetadataSourceRegistry messages) {
  * 		messages.simpDestMatchers("/user/queue/errors").permitAll()
@@ -94,11 +97,23 @@ public final void configureClientInboundChannel(ChannelRegistration registration
 			registration.setInterceptors(csrfChannelInterceptor());
 		}
 		if (inboundRegistry.containsMapping()) {
+			PathMatcher pathMatcher = getDefaultPathMatcher();
+			if(pathMatcher != null) {
+				inboundRegistry.simpDestPathMatcher(pathMatcher);
+			}
 			registration.setInterceptors(inboundChannelSecurity);
 		}
 		customizeClientInboundChannel(registration);
 	}
 
+	private PathMatcher getDefaultPathMatcher() {
+		try {
+			return context.getBean(SimpAnnotationMethodMessageHandler.class).getPathMatcher();
+		} catch(NoSuchBeanDefinitionException e) {
+			return null;
+		}
+	}
+
 	/**
 	 * <p>
 	 * Determines if a CSRF token is required for connecting. This protects against remote

config/src/test/java/org/springframework/security/config/annotation/web/socket/AbstractSecurityWebSocketMessageBrokerConfigurerTests.java

@@ -1,5 +1,4 @@
 /*
- * Copyright 2002-2015 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License"); you may not
  * use this file except in compliance with the License. You may obtain a copy of
@@ -17,7 +16,6 @@
 
 import org.junit.After;
 import org.junit.Before;
-
 import org.junit.Test;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
@@ -46,6 +44,7 @@
 import org.springframework.security.web.csrf.MissingCsrfTokenException;
 import org.springframework.stereotype.Controller;
 import org.springframework.test.util.ReflectionTestUtils;
+import org.springframework.util.AntPathMatcher;
 import org.springframework.web.HttpRequestHandler;
 import org.springframework.web.context.support.AnnotationConfigWebApplicationContext;
 import org.springframework.web.servlet.HandlerMapping;
@@ -59,6 +58,7 @@
 import org.springframework.web.socket.sockjs.transport.session.WebSocketServerSockJsSession;
 
 import javax.servlet.http.HttpServletRequest;
+
 import java.util.HashMap;
 import java.util.Map;
 
@@ -232,6 +232,58 @@ public void messagesConnectWebSocketUseCsrfTokenHandshakeInterceptor()
 		assertHandshake(request);
 	}
 
+	@Test
+	public void msmsRegistryCustomPatternMatcher()
+			throws Exception {
+		loadConfig(MsmsRegistryCustomPatternMatcherConfig.class);
+
+		clientInboundChannel().send(message("/app/a.b"));
+
+		try {
+			clientInboundChannel().send(message("/app/a.b.c"));
+			fail("Expected Exception");
+		}
+		catch (MessageDeliveryException expected) {
+			assertThat(expected.getCause()).isInstanceOf(AccessDeniedException.class);
+		}
+	}
+
+	@Configuration
+	@EnableWebSocketMessageBroker
+	@Import(SyncExecutorConfig.class)
+	static class MsmsRegistryCustomPatternMatcherConfig extends
+			AbstractSecurityWebSocketMessageBrokerConfigurer {
+
+		public void registerStompEndpoints(StompEndpointRegistry registry) {
+			registry.addEndpoint("/other").setHandshakeHandler(testHandshakeHandler())
+					.withSockJS().setInterceptors(new HttpSessionHandshakeInterceptor());
+
+			registry.addEndpoint("/chat").setHandshakeHandler(testHandshakeHandler())
+					.withSockJS().setInterceptors(new HttpSessionHandshakeInterceptor());
+		}
+
+		// @formatter:off
+		@Override
+		protected void configureInbound(MessageSecurityMetadataSourceRegistry messages) {
+			messages
+				.simpDestMatchers("/app/a.*").permitAll()
+				.anyMessage().denyAll();
+		}
+		// @formatter:on
+
+		@Override
+		public void configureMessageBroker(MessageBrokerRegistry registry) {
+			registry.setPathMatcher(new AntPathMatcher("."));
+			registry.enableSimpleBroker("/queue/", "/topic/");
+			registry.setApplicationDestinationPrefixes("/app");
+		}
+
+		@Bean
+		public TestHandshakeHandler testHandshakeHandler() {
+			return new TestHandshakeHandler();
+		}
+	}
+
 	private void assertHandshake(HttpServletRequest request) {
 		TestHandshakeHandler handshakeHandler = context
 				.getBean(TestHandshakeHandler.class);
@@ -358,10 +410,14 @@ public void registerStompEndpoints(StompEndpointRegistry registry) {
 					.withSockJS().setInterceptors(new HttpSessionHandshakeInterceptor());
 		}
 
+		// @formatter:off
 		@Override
 		protected void configureInbound(MessageSecurityMetadataSourceRegistry messages) {
-			messages.simpDestMatchers("/permitAll/**").permitAll().anyMessage().denyAll();
+			messages
+				.simpDestMatchers("/permitAll/**").permitAll()
+				.anyMessage().denyAll();
 		}
+		// @formatter:on
 
 		@Override
 		public void configureMessageBroker(MessageBrokerRegistry registry) {
@@ -431,10 +487,14 @@ public void registerStompEndpoints(StompEndpointRegistry registry) {
 					.addInterceptors(new HttpSessionHandshakeInterceptor());
 		}
 
+		// @formatter:off
 		@Override
 		protected void configureInbound(MessageSecurityMetadataSourceRegistry messages) {
-			messages.simpDestMatchers("/permitAll/**").permitAll().anyMessage().denyAll();
+			messages
+				.simpDestMatchers("/permitAll/**").permitAll()
+				.anyMessage().denyAll();
 		}
+		// @formatter:on
 
 		@Bean
 		public TestHandshakeHandler testHandshakeHandler() {