diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/socket/MessageMatcherAuthorizationManagerConfiguration.java b/config/src/main/java/org/springframework/security/config/annotation/web/socket/MessageMatcherAuthorizationManagerConfiguration.java index 8977662092..a838286690 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/web/socket/MessageMatcherAuthorizationManagerConfiguration.java +++ b/config/src/main/java/org/springframework/security/config/annotation/web/socket/MessageMatcherAuthorizationManagerConfiguration.java @@ -16,29 +16,24 @@ package org.springframework.security.config.annotation.web.socket; -import org.springframework.context.ApplicationContext; import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Fallback; import org.springframework.context.annotation.Scope; -import org.springframework.messaging.simp.annotation.support.SimpAnnotationMethodMessageHandler; +import org.springframework.security.config.web.messaging.PathPatternMessageMatcherBuilderFactoryBean; import org.springframework.security.messaging.access.intercept.MessageMatcherDelegatingAuthorizationManager; -import org.springframework.security.messaging.util.matcher.MessageMatcherFactory; -import org.springframework.util.AntPathMatcher; final class MessageMatcherAuthorizationManagerConfiguration { + @Bean + @Fallback + PathPatternMessageMatcherBuilderFactoryBean messageMatcherBuilderFactoryBean() { + return new PathPatternMessageMatcherBuilderFactoryBean(); + } + @Bean @Scope("prototype") - MessageMatcherDelegatingAuthorizationManager.Builder messageAuthorizationManagerBuilder( - ApplicationContext context) { - MessageMatcherFactory.setApplicationContext(context); - if (MessageMatcherFactory.usesPathPatterns()) { - return MessageMatcherDelegatingAuthorizationManager.builder(); - } - return MessageMatcherDelegatingAuthorizationManager.builder() - .simpDestPathMatcher( - () -> (context.getBeanNamesForType(SimpAnnotationMethodMessageHandler.class).length > 0) - ? context.getBean(SimpAnnotationMethodMessageHandler.class).getPathMatcher() - : new AntPathMatcher()); + MessageMatcherDelegatingAuthorizationManager.Builder messageAuthorizationManagerBuilder() { + return MessageMatcherDelegatingAuthorizationManager.builder(); } } diff --git a/config/src/main/java/org/springframework/security/config/http/MessageMatcherFactoryBean.java b/config/src/main/java/org/springframework/security/config/http/MessageMatcherFactoryBean.java index ac0467da22..94d9da22cb 100644 --- a/config/src/main/java/org/springframework/security/config/http/MessageMatcherFactoryBean.java +++ b/config/src/main/java/org/springframework/security/config/http/MessageMatcherFactoryBean.java @@ -23,9 +23,6 @@ import org.springframework.context.ApplicationContextAware; import org.springframework.messaging.simp.SimpMessageType; import org.springframework.security.messaging.util.matcher.MessageMatcher; import org.springframework.security.messaging.util.matcher.PathPatternMessageMatcher; -import org.springframework.security.messaging.util.matcher.SimpDestinationMessageMatcher; -import org.springframework.util.AntPathMatcher; -import org.springframework.util.PathMatcher; @Deprecated public final class MessageMatcherFactoryBean implements FactoryBean>, ApplicationContextAware { @@ -36,8 +33,6 @@ public final class MessageMatcherFactoryBean implements FactoryBean getObject() throws Exception { - if (this.builder != null) { - return this.builder.matcher(this.method, this.path); - } - if (this.method == SimpMessageType.SUBSCRIBE) { - return SimpDestinationMessageMatcher.createSubscribeMatcher(this.path, this.pathMatcher); - } - if (this.method == SimpMessageType.MESSAGE) { - return SimpDestinationMessageMatcher.createMessageMatcher(this.path, this.pathMatcher); - } - return new SimpDestinationMessageMatcher(this.path, this.pathMatcher); + return this.builder.matcher(this.method, this.path); } @Override @@ -66,13 +52,9 @@ public final class MessageMatcherFactoryBean implements FactoryBean> authorizationManager(MessageMatcherDelegatingAuthorizationManager.Builder messages) { messages - .simpDestPathMatcher(new AntPathMatcher()) .simpDestMatchers("/app/a/*").permitAll() .anyMessage().denyAll(); return messages.build(); diff --git a/messaging/src/main/java/org/springframework/security/messaging/access/expression/MessageExpressionConfigAttribute.java b/messaging/src/main/java/org/springframework/security/messaging/access/expression/MessageExpressionConfigAttribute.java index 6887342ed2..64e5ac1aa1 100644 --- a/messaging/src/main/java/org/springframework/security/messaging/access/expression/MessageExpressionConfigAttribute.java +++ b/messaging/src/main/java/org/springframework/security/messaging/access/expression/MessageExpressionConfigAttribute.java @@ -23,7 +23,6 @@ import org.springframework.expression.Expression; import org.springframework.messaging.Message; import org.springframework.security.access.ConfigAttribute; import org.springframework.security.messaging.util.matcher.MessageMatcher; -import org.springframework.security.messaging.util.matcher.SimpDestinationMessageMatcher; import org.springframework.util.Assert; /** @@ -42,7 +41,7 @@ class MessageExpressionConfigAttribute implements ConfigAttribute, EvaluationCon private final Expression authorizeExpression; - private final MessageMatcher matcher; + private final MessageMatcher matcher; /** * Creates a new instance @@ -53,7 +52,7 @@ class MessageExpressionConfigAttribute implements ConfigAttribute, EvaluationCon Assert.notNull(authorizeExpression, "authorizeExpression cannot be null"); Assert.notNull(matcher, "matcher cannot be null"); this.authorizeExpression = authorizeExpression; - this.matcher = matcher; + this.matcher = (MessageMatcher) matcher; } Expression getAuthorizeExpression() { @@ -72,12 +71,9 @@ class MessageExpressionConfigAttribute implements ConfigAttribute, EvaluationCon @Override public EvaluationContext postProcess(EvaluationContext ctx, Message message) { - if (this.matcher instanceof SimpDestinationMessageMatcher) { - Map variables = ((SimpDestinationMessageMatcher) this.matcher) - .extractPathVariables(message); - for (Map.Entry entry : variables.entrySet()) { - ctx.setVariable(entry.getKey(), entry.getValue()); - } + Map variables = this.matcher.matcher(message).getVariables(); + for (Map.Entry entry : variables.entrySet()) { + ctx.setVariable(entry.getKey(), entry.getValue()); } return ctx; } diff --git a/messaging/src/main/java/org/springframework/security/messaging/access/intercept/MessageMatcherDelegatingAuthorizationManager.java b/messaging/src/main/java/org/springframework/security/messaging/access/intercept/MessageMatcherDelegatingAuthorizationManager.java index 6f782de9a9..05441b8d25 100644 --- a/messaging/src/main/java/org/springframework/security/messaging/access/intercept/MessageMatcherDelegatingAuthorizationManager.java +++ b/messaging/src/main/java/org/springframework/security/messaging/access/intercept/MessageMatcherDelegatingAuthorizationManager.java @@ -23,6 +23,9 @@ import java.util.function.Supplier; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.springframework.beans.BeansException; +import org.springframework.context.ApplicationContext; +import org.springframework.context.ApplicationContextAware; import org.springframework.core.log.LogMessage; import org.springframework.messaging.Message; import org.springframework.messaging.simp.SimpMessageType; @@ -33,15 +36,10 @@ import org.springframework.security.authorization.AuthorizationResult; import org.springframework.security.authorization.SingleResultAuthorizationManager; import org.springframework.security.core.Authentication; import org.springframework.security.messaging.util.matcher.MessageMatcher; -import org.springframework.security.messaging.util.matcher.MessageMatcherFactory; import org.springframework.security.messaging.util.matcher.PathPatternMessageMatcher; -import org.springframework.security.messaging.util.matcher.SimpDestinationMessageMatcher; import org.springframework.security.messaging.util.matcher.SimpMessageTypeMatcher; -import org.springframework.util.AntPathMatcher; import org.springframework.util.Assert; import org.springframework.util.CollectionUtils; -import org.springframework.util.PathMatcher; -import org.springframework.util.function.SingletonSupplier; public final class MessageMatcherDelegatingAuthorizationManager implements AuthorizationManager> { @@ -99,12 +97,11 @@ public final class MessageMatcherDelegatingAuthorizationManager implements Autho /** * A builder for {@link MessageMatcherDelegatingAuthorizationManager}. */ - public static final class Builder { + public static final class Builder implements ApplicationContextAware { private final List>>> mappings = new ArrayList<>(); - @Deprecated - private Supplier pathMatcher = AntPathMatcher::new; + private PathPatternMessageMatcher.Builder messageMatcherBuilder = PathPatternMessageMatcher.withDefaults(); public Builder() { } @@ -142,11 +139,9 @@ public final class MessageMatcherDelegatingAuthorizationManager implements Autho } /** - * Maps a {@link List} of {@link SimpDestinationMessageMatcher} (or - * {@link PathPatternMessageMatcher} if the application has configured a - * {@link PathPatternMessageMatcher.Builder} bean) instances without regard to the - * {@link SimpMessageType}. If no destination is found on the Message, then the - * Matcher returns false. + * Maps a {@link List} of {@link PathPatternMessageMatcher}s instances without + * regard to the {@link SimpMessageType}. If no destination is found on the + * Message, then the Matcher returns false. * @param patterns the patterns to create {@code MessageMatcher}s from. */ public Builder.Constraint simpDestMatchers(String... patterns) { @@ -154,10 +149,8 @@ public final class MessageMatcherDelegatingAuthorizationManager implements Autho } /** - * Maps a {@link List} of {@link SimpDestinationMessageMatcher} (or - * {@link PathPatternMessageMatcher} if the application has configured a - * {@link PathPatternMessageMatcher.Builder} bean) instances that match on - * {@code SimpMessageType.MESSAGE}. If no destination is found on the Message, + * Maps a {@link List} of {@link PathPatternMessageMatcher}s instances that match + * on {@code SimpMessageType.MESSAGE}. If no destination is found on the Message, * then the Matcher returns false. * @param patterns the patterns to create {@code MessageMatcher}s from. */ @@ -166,11 +159,9 @@ public final class MessageMatcherDelegatingAuthorizationManager implements Autho } /** - * Maps a {@link List} of {@link SimpDestinationMessageMatcher} (or - * {@link PathPatternMessageMatcher} if the application has configured a - * {@link PathPatternMessageMatcher.Builder} bean) instances that match on - * {@code SimpMessageType.SUBSCRIBE}. If no destination is found on the Message, - * then the Matcher returns false. + * Maps a {@link List} of {@link PathPatternMessageMatcher}s instances that match + * on {@code SimpMessageType.SUBSCRIBE}. If no destination is found on the + * Message, then the Matcher returns false. * @param patterns the patterns to create {@code MessageMatcher}s from. */ public Builder.Constraint simpSubscribeDestMatchers(String... patterns) { @@ -178,10 +169,8 @@ public final class MessageMatcherDelegatingAuthorizationManager implements Autho } /** - * Maps a {@link List} of {@link SimpDestinationMessageMatcher} (or - * {@link PathPatternMessageMatcher} if the application has configured a - * {@link PathPatternMessageMatcher.Builder} bean) instances. If no destination is - * found on the Message, then the Matcher returns false. + * Maps a {@link List} of {@link PathPatternMessageMatcher} instances. If no + * destination is found on the Message, then the Matcher returns false. * @param type the {@link SimpMessageType} to match on. If null, the * {@link SimpMessageType} is not considered for matching. * @param patterns the patterns to create {@code MessageMatcher}s from. @@ -191,44 +180,12 @@ public final class MessageMatcherDelegatingAuthorizationManager implements Autho private Builder.Constraint simpDestMatchers(SimpMessageType type, String... patterns) { List> matchers = new ArrayList<>(patterns.length); for (String pattern : patterns) { - MessageMatcher matcher = MessageMatcherFactory.usesPathPatterns() - ? MessageMatcherFactory.matcher(type, pattern) - : new LazySimpDestinationMessageMatcher(pattern, type); + MessageMatcher matcher = this.messageMatcherBuilder.matcher(type, pattern); matchers.add(matcher); } return new Builder.Constraint(matchers); } - /** - * The {@link PathMatcher} to be used with the - * {@link Builder#simpDestMatchers(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 Builder} for further customization. - * @deprecated - */ - @Deprecated - public Builder simpDestPathMatcher(PathMatcher pathMatcher) { - Assert.notNull(pathMatcher, "pathMatcher cannot be null"); - this.pathMatcher = () -> pathMatcher; - return this; - } - - /** - * The {@link PathMatcher} to be used with the - * {@link Builder#simpDestMatchers(String...)}. Use this method to delay the - * computation or lookup of the {@link PathMatcher}. - * @param pathMatcher the {@link PathMatcher} to use. Cannot be null. - * @return the {@link Builder} for further customization. - * @deprecated - */ - @Deprecated - public Builder simpDestPathMatcher(Supplier 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. @@ -248,6 +205,12 @@ public final class MessageMatcherDelegatingAuthorizationManager implements Autho return new MessageMatcherDelegatingAuthorizationManager(this.mappings); } + @Override + public void setApplicationContext(ApplicationContext context) throws BeansException { + this.messageMatcherBuilder = context.getBeanProvider(PathPatternMessageMatcher.Builder.class) + .getIfUnique(PathPatternMessageMatcher::withDefaults); + } + /** * Represents the security constraint to be applied to the {@link MessageMatcher} * instances. @@ -379,39 +342,6 @@ public final class MessageMatcherDelegatingAuthorizationManager implements Autho } - @Deprecated - private final class LazySimpDestinationMessageMatcher implements MessageMatcher { - - private final Supplier delegate; - - private LazySimpDestinationMessageMatcher(String pattern, SimpMessageType type) { - this.delegate = SingletonSupplier.of(() -> { - PathMatcher pathMatcher = Builder.this.pathMatcher.get(); - if (type == null) { - return new SimpDestinationMessageMatcher(pattern, pathMatcher); - } - if (SimpMessageType.MESSAGE == type) { - return SimpDestinationMessageMatcher.createMessageMatcher(pattern, pathMatcher); - } - if (SimpMessageType.SUBSCRIBE == type) { - return SimpDestinationMessageMatcher.createSubscribeMatcher(pattern, pathMatcher); - } - throw new IllegalStateException(type + " is not supported since it does not have a destination"); - }); - } - - @Override - public boolean matches(Message message) { - return this.delegate.get().matches(message); - } - - @Override - public MatchResult matcher(Message message) { - return this.delegate.get().matcher(message); - } - - } - } private static final class Entry { diff --git a/messaging/src/test/java/org/springframework/security/messaging/access/expression/MessageExpressionConfigAttributeTests.java b/messaging/src/test/java/org/springframework/security/messaging/access/expression/MessageExpressionConfigAttributeTests.java index 3e97ae597c..9485ca37e8 100644 --- a/messaging/src/test/java/org/springframework/security/messaging/access/expression/MessageExpressionConfigAttributeTests.java +++ b/messaging/src/test/java/org/springframework/security/messaging/access/expression/MessageExpressionConfigAttributeTests.java @@ -28,7 +28,7 @@ import org.springframework.messaging.Message; import org.springframework.messaging.simp.SimpMessageHeaderAccessor; import org.springframework.messaging.support.MessageBuilder; import org.springframework.security.messaging.util.matcher.MessageMatcher; -import org.springframework.security.messaging.util.matcher.SimpDestinationMessageMatcher; +import org.springframework.security.messaging.util.matcher.PathPatternMessageMatcher; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; @@ -81,7 +81,7 @@ public class MessageExpressionConfigAttributeTests { @Test public void postProcessContext() { - SimpDestinationMessageMatcher matcher = new SimpDestinationMessageMatcher("/topics/{topic}/**"); + PathPatternMessageMatcher matcher = PathPatternMessageMatcher.withDefaults().matcher("/topics/{topic}/**"); // @formatter:off Message message = MessageBuilder.withPayload("M") .setHeader(SimpMessageHeaderAccessor.DESTINATION_HEADER, "/topics/someTopic/sub1") diff --git a/messaging/src/test/java/org/springframework/security/messaging/access/expression/MessageExpressionVoterTests.java b/messaging/src/test/java/org/springframework/security/messaging/access/expression/MessageExpressionVoterTests.java index 9e563e8254..4c91fce00e 100644 --- a/messaging/src/test/java/org/springframework/security/messaging/access/expression/MessageExpressionVoterTests.java +++ b/messaging/src/test/java/org/springframework/security/messaging/access/expression/MessageExpressionVoterTests.java @@ -78,6 +78,7 @@ public class MessageExpressionVoterTests { @Test public void voteGranted() { given(this.expression.getValue(any(EvaluationContext.class), eq(Boolean.class))).willReturn(true); + given(this.matcher.matcher(any())).willCallRealMethod(); assertThat(this.voter.vote(this.authentication, this.message, this.attributes)) .isEqualTo(AccessDecisionVoter.ACCESS_GRANTED); } @@ -85,6 +86,7 @@ public class MessageExpressionVoterTests { @Test public void voteDenied() { given(this.expression.getValue(any(EvaluationContext.class), eq(Boolean.class))).willReturn(false); + given(this.matcher.matcher(any())).willCallRealMethod(); assertThat(this.voter.vote(this.authentication, this.message, this.attributes)) .isEqualTo(AccessDecisionVoter.ACCESS_DENIED); } @@ -127,6 +129,7 @@ public class MessageExpressionVoterTests { given(this.expressionHandler.createEvaluationContext(this.authentication, this.message)) .willReturn(this.evaluationContext); given(this.expression.getValue(this.evaluationContext, Boolean.class)).willReturn(true); + given(this.matcher.matcher(any())).willCallRealMethod(); assertThat(this.voter.vote(this.authentication, this.message, this.attributes)) .isEqualTo(AccessDecisionVoter.ACCESS_GRANTED); verify(this.expressionHandler).createEvaluationContext(this.authentication, this.message); diff --git a/messaging/src/test/java/org/springframework/security/messaging/access/intercept/MessageMatcherDelegatingAuthorizationManagerTests.java b/messaging/src/test/java/org/springframework/security/messaging/access/intercept/MessageMatcherDelegatingAuthorizationManagerTests.java index bd1fedb1f7..e9f8e67b2c 100644 --- a/messaging/src/test/java/org/springframework/security/messaging/access/intercept/MessageMatcherDelegatingAuthorizationManagerTests.java +++ b/messaging/src/test/java/org/springframework/security/messaging/access/intercept/MessageMatcherDelegatingAuthorizationManagerTests.java @@ -37,10 +37,11 @@ import org.springframework.security.authentication.TestingAuthenticationToken; import org.springframework.security.authorization.AuthorizationDecision; import org.springframework.security.authorization.AuthorizationManager; import org.springframework.security.core.Authentication; -import org.springframework.security.messaging.util.matcher.MessageMatcherFactory; import org.springframework.security.messaging.util.matcher.PathPatternMessageMatcher; +import org.springframework.web.util.pattern.PathPatternParser; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; /** @@ -58,7 +59,7 @@ public final class MessageMatcherDelegatingAuthorizationManagerTests { @BeforeEach void setUp() { Mockito.when(this.context.getBeanProvider(PathPatternMessageMatcher.Builder.class)).thenReturn(this.provider); - MessageMatcherFactory.setApplicationContext(this.context); + Mockito.when(this.provider.getIfUnique(any())).thenReturn(PathPatternMessageMatcher.withDefaults()); } @Test @@ -135,8 +136,7 @@ public final class MessageMatcherDelegatingAuthorizationManagerTests { @Test void checkWhenMessageTypeAndPathPatternMatches() { - Mockito.when(this.provider.getIfUnique()).thenReturn(PathPatternMessageMatcher.withDefaults()); - MessageMatcherFactory.setApplicationContext(this.context); + Mockito.when(this.provider.getIfUnique(any())).thenReturn(PathPatternMessageMatcher.withDefaults()); AuthorizationManager> authorizationManager = builder().simpMessageDestMatchers("/destination") .permitAll() .simpSubscribeDestMatchers("/destination") @@ -154,10 +154,32 @@ public final class MessageMatcherDelegatingAuthorizationManagerTests { assertThat(authorizationManager.authorize(mock(Supplier.class), message2).isGranted()).isFalse(); } + @Test + void checkWhenMessageTypeAndPathPatternMatchesCaseInsensitive() { + PathPatternParser pathPatternParser = new PathPatternParser(); + pathPatternParser.setCaseSensitive(false); + PathPatternMessageMatcher.Builder messageMatcherBuilder = PathPatternMessageMatcher + .withPathPatternParser(pathPatternParser); + Mockito.when(this.provider.getIfUnique(any())).thenReturn(messageMatcherBuilder); + AuthorizationManager> authorizationManager = builder().simpMessageDestMatchers("/desTinaTion") + .permitAll() + .simpSubscribeDestMatchers("/desTinaTion") + .denyAll() + .anyMessage() + .denyAll() + .build(); + MessageHeaders headers = new MessageHeaders(Map.of(SimpMessageHeaderAccessor.MESSAGE_TYPE_HEADER, + SimpMessageType.MESSAGE, SimpMessageHeaderAccessor.DESTINATION_HEADER, "/destination")); + Message message = new GenericMessage<>(new Object(), headers); + assertThat(authorizationManager.authorize(mock(Supplier.class), message).isGranted()).isTrue(); + MessageHeaders headers2 = new MessageHeaders(Map.of(SimpMessageHeaderAccessor.MESSAGE_TYPE_HEADER, + SimpMessageType.SUBSCRIBE, SimpMessageHeaderAccessor.DESTINATION_HEADER, "/destination")); + Message message2 = new GenericMessage<>(new Object(), headers2); + assertThat(authorizationManager.authorize(mock(Supplier.class), message2).isGranted()).isFalse(); + } + @Test void checkPatternMismatch() { - Mockito.when(this.provider.getIfUnique()).thenReturn(PathPatternMessageMatcher.withDefaults()); - MessageMatcherFactory.setApplicationContext(this.context); AuthorizationManager> authorizationManager = builder().simpDestMatchers("/destination/*") .permitAll() .anyMessage() @@ -170,7 +192,10 @@ public final class MessageMatcherDelegatingAuthorizationManagerTests { } private MessageMatcherDelegatingAuthorizationManager.Builder builder() { - return MessageMatcherDelegatingAuthorizationManager.builder(); + MessageMatcherDelegatingAuthorizationManager.Builder builder = MessageMatcherDelegatingAuthorizationManager + .builder(); + builder.setApplicationContext(this.context); + return builder; } private Builder variable(String name) {