diff --git a/config/src/main/java/org/springframework/security/config/websocket/WebSocketMessageBrokerSecurityBeanDefinitionParser.java b/config/src/main/java/org/springframework/security/config/websocket/WebSocketMessageBrokerSecurityBeanDefinitionParser.java index 2201713bd1..f8ad403e73 100644 --- a/config/src/main/java/org/springframework/security/config/websocket/WebSocketMessageBrokerSecurityBeanDefinitionParser.java +++ b/config/src/main/java/org/springframework/security/config/websocket/WebSocketMessageBrokerSecurityBeanDefinitionParser.java @@ -23,9 +23,7 @@ import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; -import java.util.function.Supplier; -import org.jspecify.annotations.Nullable; import org.w3c.dom.Element; import org.springframework.beans.BeansException; @@ -44,25 +42,18 @@ import org.springframework.beans.factory.support.RootBeanDefinition; import org.springframework.beans.factory.xml.BeanDefinitionParser; import org.springframework.beans.factory.xml.ParserContext; import org.springframework.beans.factory.xml.XmlReaderContext; -import org.springframework.expression.EvaluationContext; -import org.springframework.expression.Expression; import org.springframework.messaging.Message; import org.springframework.messaging.simp.SimpMessageType; import org.springframework.messaging.simp.annotation.support.SimpAnnotationMethodMessageHandler; -import org.springframework.security.access.expression.ExpressionUtils; -import org.springframework.security.access.expression.SecurityExpressionHandler; import org.springframework.security.access.vote.ConsensusBased; -import org.springframework.security.authorization.AuthorizationDecision; import org.springframework.security.authorization.AuthorizationManager; -import org.springframework.security.authorization.AuthorizationResult; import org.springframework.security.config.Elements; import org.springframework.security.config.http.MessageMatcherFactoryBean; import org.springframework.security.config.web.messaging.PathPatternMessageMatcherBuilderFactoryBean; -import org.springframework.security.core.Authentication; import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.security.core.context.SecurityContextHolderStrategy; import org.springframework.security.messaging.access.expression.ExpressionBasedMessageSecurityMetadataSourceFactory; -import org.springframework.security.messaging.access.expression.MessageAuthorizationContextSecurityExpressionHandler; +import org.springframework.security.messaging.access.expression.MessageExpressionAuthorizationManager; import org.springframework.security.messaging.access.expression.MessageExpressionVoter; import org.springframework.security.messaging.access.intercept.AuthorizationChannelInterceptor; import org.springframework.security.messaging.access.intercept.ChannelSecurityInterceptor; @@ -75,7 +66,6 @@ import org.springframework.security.messaging.util.matcher.SimpMessageTypeMatche import org.springframework.security.messaging.web.csrf.XorCsrfChannelInterceptor; import org.springframework.security.messaging.web.socket.server.CsrfTokenHandshakeInterceptor; import org.springframework.util.AntPathMatcher; -import org.springframework.util.Assert; import org.springframework.util.PathMatcher; import org.springframework.util.StringUtils; import org.springframework.util.xml.DomUtils; @@ -219,9 +209,15 @@ public final class WebSocketMessageBrokerSecurityBeanDefinitionParser implements String messageType = interceptMessage.getAttribute(TYPE_ATTR); BeanDefinition matcher = createMatcher(matcherPattern, messageType, parserContext, interceptMessage); BeanDefinitionBuilder authorizationManager = BeanDefinitionBuilder - .rootBeanDefinition(ExpressionBasedAuthorizationManager.class); + .rootBeanDefinition(MessageExpressionAuthorizationManager.class); if (StringUtils.hasText(expressionHandlerRef)) { - authorizationManager.addConstructorArgReference(expressionHandlerRef); + BeanDefinitionBuilder authorizationManagerBuilder = BeanDefinitionBuilder + .rootBeanDefinition(MessageExpressionAuthorizationManager.class); + authorizationManagerBuilder.setFactoryMethod("withSecurityExpressionHandler"); + authorizationManagerBuilder.addConstructorArgReference(expressionHandlerRef); + String authorizationManagerBuilderRef = context + .registerWithGeneratedName(authorizationManagerBuilder.getBeanDefinition()); + authorizationManager.setFactoryMethodOnBean("expression", authorizationManagerBuilderRef); } authorizationManager.addConstructorArgValue(accessExpression); matcherToExpression.put(matcher, authorizationManager.getBeanDefinition()); @@ -439,35 +435,6 @@ public final class WebSocketMessageBrokerSecurityBeanDefinitionParser implements } - private static final class ExpressionBasedAuthorizationManager - implements AuthorizationManager> { - - private final SecurityExpressionHandler> expressionHandler; - - private final Expression expression; - - private ExpressionBasedAuthorizationManager(String expression) { - this(new MessageAuthorizationContextSecurityExpressionHandler(), expression); - } - - private ExpressionBasedAuthorizationManager( - SecurityExpressionHandler> expressionHandler, String expression) { - Assert.notNull(expressionHandler, "expressionHandler cannot be null"); - Assert.notNull(expression, "expression cannot be null"); - this.expressionHandler = expressionHandler; - this.expression = this.expressionHandler.getExpressionParser().parseExpression(expression); - } - - @Override - public AuthorizationResult authorize(Supplier authentication, - MessageAuthorizationContext object) { - EvaluationContext context = this.expressionHandler.createEvaluationContext(authentication, object); - boolean granted = ExpressionUtils.evaluateAsBoolean(this.expression, context); - return new AuthorizationDecision(granted); - } - - } - private static class MessageMatcherDelegatingAuthorizationManagerFactory { private static AuthorizationManager> createMessageMatcherDelegatingAuthorizationManager( diff --git a/docs/modules/ROOT/pages/servlet/integrations/websocket.adoc b/docs/modules/ROOT/pages/servlet/integrations/websocket.adoc index d732a15d2c..2649c89095 100644 --- a/docs/modules/ROOT/pages/servlet/integrations/websocket.adoc +++ b/docs/modules/ROOT/pages/servlet/integrations/websocket.adoc @@ -212,32 +212,10 @@ This will ensure that: If you are migrating from an older version of Spring Security, your destination matchers may include SpEL expressions. It's recommended that these be changed to using concrete implementations of `AuthorizationManager` since this is independently testable. -However, to ease migration, you can also use a class like the following: +However, to ease migration, you can use +`org.springframework.security.messaging.access.expression.MessageExpressionAuthorizationManager`. -[source,java] ----- -public final class MessageExpressionAuthorizationManager implements AuthorizationManager> { - - private SecurityExpressionHandler> expressionHandler = new DefaultMessageSecurityExpressionHandler(); - - private Expression expression; - - public MessageExpressionAuthorizationManager(String expressionString) { - Assert.hasText(expressionString, "expressionString cannot be empty"); - this.expression = this.expressionHandler.getExpressionParser().parseExpression(expressionString); - } - - @Override - public AuthorizationResult authorize(Supplier authentication, MessageAuthorizationContext context) { - EvaluationContext ctx = this.expressionHandler.createEvaluationContext(authentication, context.getMessage()); - boolean granted = ExpressionUtils.evaluateAsBoolean(this.expression, ctx); - return new ExpressionAuthorizationDecision(granted, this.expression); - } - -} ----- - -And specify an instance for each matcher that you cannot get migrate: +And specify an instance for each matcher that you cannot yet migrate: [tabs] ====== @@ -252,7 +230,8 @@ public class WebSocketSecurityConfig { public AuthorizationManager> messageAuthorizationManager(MessageMatcherDelegatingAuthorizationManager.Builder messages) { messages // ... - .simpSubscribeDestMatchers("/topic/friends/{friend}").access(new MessageExpressionAuthorizationManager("#friends == 'john")); + .simpSubscribeDestMatchers("/topic/friends/{friend}") + .access(new MessageExpressionAuthorizationManager("#friend == 'john'")); // ... return messages.build(); @@ -269,7 +248,8 @@ open class WebSocketSecurityConfig { fun messageAuthorizationManager(messages: MessageMatcherDelegatingAuthorizationManager.Builder): AuthorizationManager { messages // .. - .simpSubscribeDestMatchers("/topic/friends/{friends}").access(MessageExpressionAuthorizationManager("#friends == 'john")) + .simpSubscribeDestMatchers("/topic/friends/{friend}") + .access(MessageExpressionAuthorizationManager("#friend == 'john'")) // ... return messages.build() diff --git a/messaging/src/main/java/org/springframework/security/messaging/access/expression/MessageExpressionAuthorizationManager.java b/messaging/src/main/java/org/springframework/security/messaging/access/expression/MessageExpressionAuthorizationManager.java new file mode 100644 index 0000000000..9b35005ad0 --- /dev/null +++ b/messaging/src/main/java/org/springframework/security/messaging/access/expression/MessageExpressionAuthorizationManager.java @@ -0,0 +1,137 @@ +/* + * Copyright 2004-present 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 the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.security.messaging.access.expression; + +import java.util.function.Supplier; + +import org.jspecify.annotations.Nullable; + +import org.springframework.expression.EvaluationContext; +import org.springframework.expression.Expression; +import org.springframework.security.access.expression.ExpressionUtils; +import org.springframework.security.access.expression.SecurityExpressionHandler; +import org.springframework.security.authorization.AuthorizationManager; +import org.springframework.security.authorization.AuthorizationResult; +import org.springframework.security.authorization.ExpressionAuthorizationDecision; +import org.springframework.security.core.Authentication; +import org.springframework.security.messaging.access.intercept.MessageAuthorizationContext; +import org.springframework.util.Assert; + +/** + * An expression-based {@link AuthorizationManager} that determines the access by + * evaluating the provided expression. + * + * @since 7.1 + */ +public final class MessageExpressionAuthorizationManager + implements AuthorizationManager> { + + private final SecurityExpressionHandler> expressionHandler; + + private final Expression expression; + + /** + * Creates an instance. + * @param expressionString the raw expression string to parse + */ + public MessageExpressionAuthorizationManager(String expressionString) { + this(new MessageAuthorizationContextSecurityExpressionHandler(), expressionString); + } + + private MessageExpressionAuthorizationManager( + SecurityExpressionHandler> expressionHandler, String expressionString) { + Assert.notNull(expressionHandler, "expressionHandler cannot be null"); + Assert.hasText(expressionString, "expressionString cannot be empty"); + this.expressionHandler = expressionHandler; + this.expression = expressionHandler.getExpressionParser().parseExpression(expressionString); + } + + /** + * Use a {@link MessageAuthorizationContextSecurityExpressionHandler} to create + * {@link MessageExpressionAuthorizationManager} instances. + * @return a {@link Builder} for constructing + * {@link MessageExpressionAuthorizationManager} instances + * @since 7.1 + */ + public static Builder withDefaults() { + return new Builder(new MessageAuthorizationContextSecurityExpressionHandler()); + } + + /** + * Use this {@link SecurityExpressionHandler} to create + * {@link MessageExpressionAuthorizationManager} instances. + * @param expressionHandler the expression handler to use + * @return a {@link Builder} for constructing + * {@link MessageExpressionAuthorizationManager} instances + * @since 7.1 + */ + public static Builder withSecurityExpressionHandler( + SecurityExpressionHandler> expressionHandler) { + Assert.notNull(expressionHandler, "expressionHandler cannot be null"); + return new Builder(expressionHandler); + } + + /** + * Determines the access by evaluating the provided expression. + * @param authentication the {@link Supplier} of the {@link Authentication} to check + * @param context the {@link MessageAuthorizationContext} to check + * @return an {@link ExpressionAuthorizationDecision} based on the evaluated + * expression + */ + @Override + public AuthorizationResult authorize(Supplier authentication, + MessageAuthorizationContext context) { + EvaluationContext ctx = this.expressionHandler.createEvaluationContext(authentication, context); + boolean granted = ExpressionUtils.evaluateAsBoolean(this.expression, ctx); + return new ExpressionAuthorizationDecision(granted, this.expression); + } + + @Override + public String toString() { + return "MessageExpressionAuthorizationManager[expression='" + this.expression + "']"; + } + + /** + * A {@link Builder} for constructing {@link MessageExpressionAuthorizationManager} + * instances. + * + *

+ * May be reused to create multiple instances. + * + * @since 7.1 + */ + public static final class Builder { + + private final SecurityExpressionHandler> expressionHandler; + + private Builder(SecurityExpressionHandler> expressionHandler) { + this.expressionHandler = expressionHandler; + } + + /** + * Create a {@link MessageExpressionAuthorizationManager} using this + * {@code expression}. + * @param expression the expression to evaluate + * @return the resulting {@link AuthorizationManager} + */ + public MessageExpressionAuthorizationManager expression(String expression) { + return new MessageExpressionAuthorizationManager(this.expressionHandler, expression); + } + + } + +} diff --git a/messaging/src/test/java/org/springframework/security/messaging/access/expression/MessageExpressionAuthorizationManagerTests.java b/messaging/src/test/java/org/springframework/security/messaging/access/expression/MessageExpressionAuthorizationManagerTests.java new file mode 100644 index 0000000000..5e7916e764 --- /dev/null +++ b/messaging/src/test/java/org/springframework/security/messaging/access/expression/MessageExpressionAuthorizationManagerTests.java @@ -0,0 +1,109 @@ +/* + * Copyright 2004-present 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 the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.security.messaging.access.expression; + +import org.junit.jupiter.api.Test; + +import org.springframework.expression.Expression; +import org.springframework.expression.ExpressionParser; +import org.springframework.messaging.support.GenericMessage; +import org.springframework.security.access.expression.SecurityExpressionHandler; +import org.springframework.security.authentication.TestingAuthenticationToken; +import org.springframework.security.authorization.AuthorizationResult; +import org.springframework.security.messaging.access.intercept.MessageAuthorizationContext; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; +import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; + +/** + * Tests for {@link MessageExpressionAuthorizationManager}. + */ +class MessageExpressionAuthorizationManagerTests { + + @Test + void instantiateWhenExpressionStringNullThenIllegalArgumentException() { + assertThatIllegalArgumentException().isThrownBy(() -> new MessageExpressionAuthorizationManager(null)) + .withMessage("expressionString cannot be empty"); + } + + @Test + void instantiateWhenExpressionStringEmptyThenIllegalArgumentException() { + assertThatIllegalArgumentException().isThrownBy(() -> new MessageExpressionAuthorizationManager("")) + .withMessage("expressionString cannot be empty"); + } + + @Test + void instantiateWhenExpressionStringBlankThenIllegalArgumentException() { + assertThatIllegalArgumentException().isThrownBy(() -> new MessageExpressionAuthorizationManager(" ")) + .withMessage("expressionString cannot be empty"); + } + + @Test + void withSecurityExpressionHandlerWhenNullThenIllegalArgumentException() { + assertThatIllegalArgumentException() + .isThrownBy(() -> MessageExpressionAuthorizationManager.withSecurityExpressionHandler(null)) + .withMessage("expressionHandler cannot be null"); + } + + @Test + void instantiateWhenExpressionHandlerNotSetThenDefaultUsed() { + MessageExpressionAuthorizationManager manager = new MessageExpressionAuthorizationManager("hasRole('ADMIN')"); + assertThat(manager).extracting("expressionHandler") + .isInstanceOf(MessageAuthorizationContextSecurityExpressionHandler.class); + } + + @Test + void withSecurityExpressionHandlerWhenNotNullThenVerifyExpressionHandler() { + String expressionString = "hasRole('ADMIN')"; + SecurityExpressionHandler> expressionHandler = mock( + SecurityExpressionHandler.class); + ExpressionParser expressionParser = mock(ExpressionParser.class); + Expression expression = mock(Expression.class); + given(expressionHandler.getExpressionParser()).willReturn(expressionParser); + given(expressionParser.parseExpression(expressionString)).willReturn(expression); + MessageExpressionAuthorizationManager manager = MessageExpressionAuthorizationManager + .withSecurityExpressionHandler(expressionHandler) + .expression(expressionString); + assertThat(manager).extracting("expressionHandler").isEqualTo(expressionHandler); + assertThat(manager).extracting("expression").isEqualTo(expression); + verify(expressionParser).parseExpression(expressionString); + } + + @Test + void authorizeWhenExpressionHasRoleAdminAndRoleAdminThenGrantedDecision() { + MessageExpressionAuthorizationManager manager = new MessageExpressionAuthorizationManager("hasRole('ADMIN')"); + TestingAuthenticationToken authentication = new TestingAuthenticationToken("admin", "password", "ROLE_ADMIN"); + AuthorizationResult result = manager.authorize(() -> authentication, + new MessageAuthorizationContext<>(new GenericMessage<>("message"))); + assertThat(result).isNotNull(); + assertThat(result.isGranted()).isTrue(); + } + + @Test + void authorizeWhenExpressionHasRoleAdminAndRoleUserThenDeniedDecision() { + MessageExpressionAuthorizationManager manager = new MessageExpressionAuthorizationManager("hasRole('ADMIN')"); + TestingAuthenticationToken authentication = new TestingAuthenticationToken("user", "password", "ROLE_USER"); + AuthorizationResult result = manager.authorize(() -> authentication, + new MessageAuthorizationContext<>(new GenericMessage<>("message"))); + assertThat(result).isNotNull(); + assertThat(result.isGranted()).isFalse(); + } + +}