SEC-2179: Allow customize PathMatcher for SimpDestinationMessageMatcher

config/src/main/java/org/springframework/security/config/annotation/web/messaging/MessageSecurityMetadataSourceRegistry.java

@@ -21,13 +21,12 @@ import org.springframework.security.messaging.access.expression.ExpressionBasedM
 import org.springframework.security.messaging.access.intercept.MessageSecurityMetadataSource;
 import org.springframework.security.messaging.util.matcher.MessageMatcher;
 import org.springframework.security.messaging.util.matcher.SimpDestinationMessageMatcher;
+import org.springframework.util.AntPathMatcher;
 import org.springframework.util.Assert;
+import org.springframework.util.PathMatcher;
 import org.springframework.util.StringUtils;
 
-import java.util.ArrayList;
+import java.util.*;
-import java.util.Arrays;
-import java.util.LinkedHashMap;
-import java.util.List;
 
 /**
  * Allows mapping security constraints using {@link MessageMatcher} to the security expressions.
@@ -43,7 +42,9 @@ public class MessageSecurityMetadataSourceRegistry {
     private static final String fullyAuthenticated = "fullyAuthenticated";
     private static final String rememberMe = "rememberMe";
 
-    private final LinkedHashMap<MessageMatcher<?>,String> matcherToExpression = new LinkedHashMap<MessageMatcher<?>,String>();
+    private final LinkedHashMap<MatcherBuilder,String> matcherToExpression = new LinkedHashMap<MatcherBuilder,String>();
+
+    private PathMatcher pathMatcher = new AntPathMatcher();
 
     /**
      * Maps any {@link Message} to a security expression.
@@ -51,25 +52,39 @@ public class MessageSecurityMetadataSourceRegistry {
      * @return the Expression to associate
      */
     public Constraint anyMessage() {
-        return new Constraint(Arrays.<MessageMatcher<?>>asList(MessageMatcher.ANY_MESSAGE));
+        return matchers(MessageMatcher.ANY_MESSAGE);
     }
 
     /**
      * Maps a {@link List} of {@link org.springframework.security.messaging.util.matcher.SimpDestinationMessageMatcher} instances.
      *
-     * @param antPatterns the ant patterns to create {@link org.springframework.security.messaging.util.matcher.SimpDestinationMessageMatcher}
+     * @param patterns the patterns to create {@link org.springframework.security.messaging.util.matcher.SimpDestinationMessageMatcher}
-     *                    from
+     *                    from. Uses {@link MessageSecurityMetadataSourceRegistry#pathMatcher(PathMatcher)}.
      *
      * @return the {@link Constraint}  that is associated to the {@link MessageMatcher}
+     * @see {@link MessageSecurityMetadataSourceRegistry#pathMatcher(PathMatcher)} 
      */
-    public Constraint antMatchers(String... antPatterns) {
+    public Constraint destinationMatchers(String... patterns) {
-        List<MessageMatcher<?>> matchers = new ArrayList<MessageMatcher<?>>(antPatterns.length);
+        List<MatcherBuilder> matchers = new ArrayList<MatcherBuilder>(patterns.length);
-        for(String pattern : antPatterns) {
+        for(String pattern : patterns) {
-            matchers.add(new SimpDestinationMessageMatcher(pattern));
+            matchers.add(new PathMatcherMessageMatcherBuilder(pattern));
         }
         return new Constraint(matchers);
     }
 
+    /**
+     * The {@link PathMatcher} to be used with the {@link MessageSecurityMetadataSourceRegistry#destinationMatchers(String...)}.
+     * The default is to use the default constructor of {@link AntPathMatcher}.
+     *
+     * @param pathMatcher the {@link PathMatcher} to use. Cannot be null.
+     * @return the {@link MessageSecurityMetadataSourceRegistry} for further customization.
+     */
+    public MessageSecurityMetadataSourceRegistry pathMatcher(PathMatcher pathMatcher) {
+        Assert.notNull(pathMatcher, "pathMatcher cannot be null");
+        this.pathMatcher = pathMatcher;
+        return this;
+    }
+
     /**
      * Maps a {@link List} of {@link MessageMatcher} instances to a security expression.
      *
@@ -77,7 +92,11 @@ public class MessageSecurityMetadataSourceRegistry {
      * @return The {@link Constraint} that is associated to the {@link MessageMatcher} instances
      */
     public Constraint matchers(MessageMatcher<?>... matchers) {
-        return new Constraint(Arrays.asList(matchers));
+        List<MatcherBuilder> builders = new ArrayList<MatcherBuilder>(matchers.length);
+        for(MessageMatcher<?> matcher : matchers) {
+            builders.add(new PreBuiltMatcherBuilder(matcher));
+        }
+        return new Constraint(builders);
     }
 
     /**
@@ -88,6 +107,10 @@ public class MessageSecurityMetadataSourceRegistry {
      * @return the {@link MessageSecurityMetadataSource} to use
      */
     protected MessageSecurityMetadataSource createMetadataSource() {
+        LinkedHashMap<MessageMatcher<?>,String> matcherToExpression = new LinkedHashMap<MessageMatcher<?>,String>();
+        for(Map.Entry<MatcherBuilder,String> entry : this.matcherToExpression.entrySet()) {
+            matcherToExpression.put(entry.getKey().build(), entry.getValue());
+        }
         return ExpressionBasedMessageSecurityMetadataSourceFactory.createExpressionMessageMetadataSource(matcherToExpression);
     }
 
@@ -95,14 +118,14 @@ public class MessageSecurityMetadataSourceRegistry {
      * Represents the security constraint to be applied to the {@link MessageMatcher} instances.
      */
     public class Constraint {
-        private final List<MessageMatcher<?>> messageMatchers;
+        private final List<MatcherBuilder> messageMatchers;
 
         /**
          * Creates a new instance
          *
          * @param messageMatchers the {@link MessageMatcher} instances to map to this constraint
          */
-        public Constraint(List<MessageMatcher<?>> messageMatchers) {
+        private Constraint(List<MatcherBuilder> messageMatchers) {
             Assert.notEmpty(messageMatchers, "messageMatchers cannot be null or empty");
             this.messageMatchers = messageMatchers;
         }
@@ -219,7 +242,7 @@ public class MessageSecurityMetadataSourceRegistry {
          * @return the {@link MessageSecurityMetadataSourceRegistry} for further customization
          */
         public MessageSecurityMetadataSourceRegistry access(String attribute) {
-            for(MessageMatcher<?> messageMatcher : messageMatchers) {
+            for(MatcherBuilder messageMatcher : messageMatchers) {
                 matcherToExpression.put(messageMatcher, attribute);
             }
             return MessageSecurityMetadataSourceRegistry.this;
@@ -247,4 +270,34 @@ public class MessageSecurityMetadataSourceRegistry {
         String anyAuthorities = StringUtils.arrayToDelimitedString(authorities, "','");
         return "hasAnyAuthority('" + anyAuthorities + "')";
     }
+
+    private class PreBuiltMatcherBuilder implements MatcherBuilder {
+        private MessageMatcher<?> matcher;
+
+        private PreBuiltMatcherBuilder(MessageMatcher<?> matcher) {
+            this.matcher = matcher;
+        }
+
+        @Override
+        public MessageMatcher<?> build() {
+            return matcher;
+        }
+    }
+
+    private class PathMatcherMessageMatcherBuilder implements MatcherBuilder {
+        private final String pattern;
+
+        private PathMatcherMessageMatcherBuilder(String pattern) {
+            this.pattern = pattern;
+        }
+
+        @Override
+        public MessageMatcher<?> build() {
+            return new SimpDestinationMessageMatcher(pattern, pathMatcher);
+        }
+    }
+
+    private interface MatcherBuilder {
+        MessageMatcher<?> build();
+    }
 }

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

@@ -48,9 +48,9 @@ import java.util.List;
  *   @Override
  *   protected void configure(MessageSecurityMetadataSourceRegistry messages) {
  *     messages
- *       .antMatchers("/user/queue/errors").permitAll()
+ *       .destinationMatchers("/user/queue/errors").permitAll()
- *       .antMatchers("/admin/**").hasRole("ADMIN")
+ *       .destinationMatchers("/admin/**").hasRole("ADMIN")
- *       .antMatchers("/**").authenticated();
+ *       .anyMessage().authenticated();
  *   }
  * }
  * </pre>

config/src/test/java/org/springframework/security/config/annotation/web/messaging/MessageSecurityMetadataSourceRegistryTests.java

@@ -26,6 +26,7 @@ import org.springframework.messaging.support.MessageBuilder;
 import org.springframework.security.access.ConfigAttribute;
 import org.springframework.security.messaging.access.intercept.MessageSecurityMetadataSource;
 import org.springframework.security.messaging.util.matcher.MessageMatcher;
+import org.springframework.util.AntPathMatcher;
 
 import java.util.Collection;
 
@@ -50,6 +51,58 @@ public class MessageSecurityMetadataSourceRegistryTests {
                    .build();
     }
 
+    // See https://github.com/spring-projects/spring-security/commit/3f30529039c76facf335d6ca69d18d8ae287f3f9#commitcomment-7412712
+    // https://jira.spring.io/browse/SPR-11660
+    @Test
+    public void destinationMatcherCustom() {
+        message = MessageBuilder
+                .withPayload("Hi")
+                .setHeader(SimpMessageHeaderAccessor.DESTINATION_HEADER, "price.stock.1.2")
+                .build();
+        messages
+                .pathMatcher(new AntPathMatcher("."))
+                .destinationMatchers("price.stock.*").permitAll();
+
+        assertThat(getAttribute()).isNull();
+
+        message = MessageBuilder
+                .withPayload("Hi")
+                .setHeader(SimpMessageHeaderAccessor.DESTINATION_HEADER, "price.stock.1.2")
+                .build();
+        messages
+                .pathMatcher(new AntPathMatcher("."))
+                .destinationMatchers("price.stock.**").permitAll();
+
+        assertThat(getAttribute()).isEqualTo("permitAll");
+    }
+
+    @Test
+    public void destinationMatcherCustomSetAfterMatchersDoesNotMatter() {
+        message = MessageBuilder
+                .withPayload("Hi")
+                .setHeader(SimpMessageHeaderAccessor.DESTINATION_HEADER, "price.stock.1.2")
+                .build();
+        messages
+                .destinationMatchers("price.stock.*").permitAll()
+                .pathMatcher(new AntPathMatcher("."));
+
+        assertThat(getAttribute()).isNull();
+
+        message = MessageBuilder
+                .withPayload("Hi")
+                .setHeader(SimpMessageHeaderAccessor.DESTINATION_HEADER, "price.stock.1.2")
+                .build();
+        messages
+                .destinationMatchers("price.stock.**").permitAll()
+                .pathMatcher(new AntPathMatcher("."));
+
+        assertThat(getAttribute()).isEqualTo("permitAll");
+    }
+    @Test(expected = IllegalArgumentException.class)
+    public void pathMatcherNull() {
+        messages.pathMatcher(null);
+    }
+
     @Test
     public void matchersFalse() {
         messages
@@ -68,99 +121,99 @@ public class MessageSecurityMetadataSourceRegistryTests {
     }
 
     @Test
-    public void antMatcherExact() {
+    public void destinationMatcherExact() {
         messages
-                .antMatchers("location").permitAll();
+                .destinationMatchers("location").permitAll();
 
         assertThat(getAttribute()).isEqualTo("permitAll");
     }
 
     @Test
-    public void antMatcherMulti() {
+    public void destinationMatcherMulti() {
         messages
-                .antMatchers("admin/**","api/**").hasRole("ADMIN")
+                .destinationMatchers("admin/**","api/**").hasRole("ADMIN")
-                .antMatchers("location").permitAll();
+                .destinationMatchers("location").permitAll();
 
         assertThat(getAttribute()).isEqualTo("permitAll");
     }
 
     @Test
-    public void antMatcherRole() {
+    public void destinationMatcherRole() {
         messages
-                .antMatchers("admin/**","location/**").hasRole("ADMIN")
+                .destinationMatchers("admin/**","location/**").hasRole("ADMIN")
                 .anyMessage().denyAll();
 
         assertThat(getAttribute()).isEqualTo("hasRole('ROLE_ADMIN')");
     }
 
     @Test
-    public void antMatcherAnyRole() {
+    public void destinationMatcherAnyRole() {
         messages
-                .antMatchers("admin/**","location/**").hasAnyRole("ADMIN", "ROOT")
+                .destinationMatchers("admin/**","location/**").hasAnyRole("ADMIN", "ROOT")
                 .anyMessage().denyAll();
 
         assertThat(getAttribute()).isEqualTo("hasAnyRole('ROLE_ADMIN','ROLE_ROOT')");
     }
 
     @Test
-    public void antMatcherAuthority() {
+    public void destinationMatcherAuthority() {
         messages
-                .antMatchers("admin/**","location/**").hasAuthority("ROLE_ADMIN")
+                .destinationMatchers("admin/**","location/**").hasAuthority("ROLE_ADMIN")
                 .anyMessage().fullyAuthenticated();
 
         assertThat(getAttribute()).isEqualTo("hasAuthority('ROLE_ADMIN')");
     }
 
     @Test
-    public void antMatcherAccess() {
+    public void destinationMatcherAccess() {
         String expected = "hasRole('ROLE_ADMIN') and fullyAuthenticated";
         messages
-                .antMatchers("admin/**","location/**").access(expected)
+                .destinationMatchers("admin/**","location/**").access(expected)
                 .anyMessage().denyAll();
 
         assertThat(getAttribute()).isEqualTo(expected);
     }
 
     @Test
-    public void antMatcherAnyAuthority() {
+    public void destinationMatcherAnyAuthority() {
         messages
-                .antMatchers("admin/**","location/**").hasAnyAuthority("ROLE_ADMIN", "ROLE_ROOT")
+                .destinationMatchers("admin/**","location/**").hasAnyAuthority("ROLE_ADMIN", "ROLE_ROOT")
                 .anyMessage().denyAll();
 
         assertThat(getAttribute()).isEqualTo("hasAnyAuthority('ROLE_ADMIN','ROLE_ROOT')");
     }
 
     @Test
-    public void antMatcherRememberMe() {
+    public void destinationMatcherRememberMe() {
         messages
-                .antMatchers("admin/**","location/**").rememberMe()
+                .destinationMatchers("admin/**","location/**").rememberMe()
                 .anyMessage().denyAll();
 
         assertThat(getAttribute()).isEqualTo("rememberMe");
     }
 
     @Test
-    public void antMatcherAnonymous() {
+    public void destinationMatcherAnonymous() {
         messages
-                .antMatchers("admin/**","location/**").anonymous()
+                .destinationMatchers("admin/**","location/**").anonymous()
                 .anyMessage().denyAll();
 
         assertThat(getAttribute()).isEqualTo("anonymous");
     }
 
     @Test
-    public void antMatcherFullyAuthenticated() {
+    public void destinationMatcherFullyAuthenticated() {
         messages
-                .antMatchers("admin/**","location/**").fullyAuthenticated()
+                .destinationMatchers("admin/**","location/**").fullyAuthenticated()
                 .anyMessage().denyAll();
 
         assertThat(getAttribute()).isEqualTo("fullyAuthenticated");
     }
 
     @Test
-    public void antMatcherDenyAll() {
+    public void destinationMatcherDenyAll() {
         messages
-                .antMatchers("admin/**","location/**").denyAll()
+                .destinationMatchers("admin/**","location/**").denyAll()
                 .anyMessage().permitAll();
 
         assertThat(getAttribute()).isEqualTo("denyAll");

messaging/src/main/java/org/springframework/security/messaging/util/matcher/SimpDestinationMessageMatcher.java

@@ -19,40 +19,65 @@ import org.springframework.messaging.Message;
 import org.springframework.messaging.simp.SimpMessageHeaderAccessor;
 import org.springframework.util.AntPathMatcher;
 import org.springframework.util.Assert;
+import org.springframework.util.PathMatcher;
 
 /**
  * <p>
- * MessageMatcher which compares a pre-defined ant-style pattern against the destination of a {@link Message}.
+ * MessageMatcher which compares a pre-defined pattern against the destination of a {@link Message}.
  * </p>
  *
- * <p>The mapping matches destinations using the following rules:
+ * @since 4.0
- *
- * <ul>
- * <li>? matches one character</li>
- * <li>* matches zero or more characters</li>
- * <li>** matches zero or more 'directories' in a path</li>
- * </ul>
- *
- * <p>Some examples:
- *
- * <ul>
- * <li>{@code com/t?st.jsp} - matches {@code com/test} but also
- * {@code com/tast} or {@code com/txst}</li>
- * <li>{@code com/*suffix} - matches all files ending in {@code suffix} in the {@code com} directory</li>
- * <li>{@code com/&#42;&#42;/test} - matches all destinations ending with {@code test} underneath the {@code com} path</li>
- * </ul>
- *
  * @author Rob Winch
  */
 public final class SimpDestinationMessageMatcher implements MessageMatcher<Object> {
-    private final AntPathMatcher matcher = new AntPathMatcher();
+    private final PathMatcher matcher;
     private final String pattern;
 
-    public SimpDestinationMessageMatcher(String pattern) {
+    /**
+     * <p>
+     * Creates a new instance with the specified pattern and a {@link AntPathMatcher} created from the default
+     * constructor.
+     * </p>
+     *
+     * @param pattern the pattern to use
+     * @param pathMatcher the {@link PathMatcher} to use.
+     */
+    public SimpDestinationMessageMatcher(String pattern, PathMatcher pathMatcher) {
         Assert.notNull(pattern, "pattern cannot be null");
+        Assert.notNull(pathMatcher, "pathMatcher cannot be null");
+        this.matcher = pathMatcher;
         this.pattern = pattern;
     }
 
+    /**
+     * <p>
+     * Creates a new instance with the specified pattern and a {@link AntPathMatcher} created from the default
+     * constructor.
+     * </p>
+     *
+     * <p>The mapping matches destinations using the following rules:
+     *
+     * <ul>
+     * <li>? matches one character</li>
+     * <li>* matches zero or more characters</li>
+     * <li>** matches zero or more 'directories' in a path</li>
+     * </ul>
+     *
+     * <p>Some examples:
+     *
+     * <ul>
+     * <li>{@code com/t?st.jsp} - matches {@code com/test} but also
+     * {@code com/tast} or {@code com/txst}</li>
+     * <li>{@code com/*suffix} - matches all files ending in {@code suffix} in the {@code com} directory</li>
+     * <li>{@code com/&#42;&#42;/test} - matches all destinations ending with {@code test} underneath the {@code com} path</li>
+     * </ul>
+     *
+     * @param pattern the pattern to use
+     */
+    public SimpDestinationMessageMatcher(String pattern) {
+        this(pattern, new AntPathMatcher());
+    }
+
     @Override
     public boolean matches(Message<? extends Object> message) {
         String destination = SimpMessageHeaderAccessor.getDestination(message.getHeaders());