From c4766e64fe7d8b2b622ec072ef1d8f8a32045e20 Mon Sep 17 00:00:00 2001 From: Evgeniy Cheban Date: Wed, 27 Apr 2022 17:09:28 +0300 Subject: [PATCH] Add AuthorizationManager that uses ExpressionHandler Closes gh-11105 --- .../AuthorizeHttpRequestsConfigurerTests.java | 134 ++++++++++++++++++ .../DefaultHttpSecurityExpressionHandler.java | 78 ++++++++++ .../ExpressionAuthorizationDecision.java | 55 +++++++ .../WebExpressionAuthorizationManager.java | 84 +++++++++++ .../expression/WebSecurityExpressionRoot.java | 18 ++- ...ebExpressionAuthorizationManagerTests.java | 105 ++++++++++++++ 6 files changed, 471 insertions(+), 3 deletions(-) create mode 100644 web/src/main/java/org/springframework/security/web/access/expression/DefaultHttpSecurityExpressionHandler.java create mode 100644 web/src/main/java/org/springframework/security/web/access/expression/ExpressionAuthorizationDecision.java create mode 100644 web/src/main/java/org/springframework/security/web/access/expression/WebExpressionAuthorizationManager.java create mode 100644 web/src/test/java/org/springframework/security/web/access/expression/WebExpressionAuthorizationManagerTests.java diff --git a/config/src/test/java/org/springframework/security/config/annotation/web/configurers/AuthorizeHttpRequestsConfigurerTests.java b/config/src/test/java/org/springframework/security/config/annotation/web/configurers/AuthorizeHttpRequestsConfigurerTests.java index 9987a79b0b..7c22bd0b5a 100644 --- a/config/src/test/java/org/springframework/security/config/annotation/web/configurers/AuthorizeHttpRequestsConfigurerTests.java +++ b/config/src/test/java/org/springframework/security/config/annotation/web/configurers/AuthorizeHttpRequestsConfigurerTests.java @@ -37,11 +37,13 @@ import org.springframework.security.config.test.SpringTestContext; import org.springframework.security.config.test.SpringTestContextExtension; import org.springframework.security.core.authority.SimpleGrantedAuthority; import org.springframework.security.web.SecurityFilterChain; +import org.springframework.security.web.access.expression.WebExpressionAuthorizationManager; import org.springframework.security.web.access.intercept.AuthorizationFilter; import org.springframework.security.web.access.intercept.RequestAuthorizationContext; import org.springframework.security.web.access.intercept.RequestMatcherDelegatingAuthorizationManager; import org.springframework.test.web.servlet.MockMvc; import org.springframework.test.web.servlet.request.MockHttpServletRequestBuilder; +import org.springframework.test.web.servlet.request.RequestPostProcessor; import org.springframework.web.bind.annotation.GetMapping; import org.springframework.web.bind.annotation.PostMapping; import org.springframework.web.bind.annotation.RestController; @@ -395,6 +397,90 @@ public class AuthorizeHttpRequestsConfigurerTests { this.mvc.perform(requestWithUser).andExpect(status().isOk()); } + @Test + public void getWhenExpressionHasRoleUserConfiguredAndRoleIsUserThenRespondsWithOk() throws Exception { + this.spring.register(ExpressionRoleUserConfig.class, BasicController.class).autowire(); + // @formatter:off + MockHttpServletRequestBuilder requestWithUser = get("/") + .with(user("user") + .roles("USER")); + // @formatter:on + this.mvc.perform(requestWithUser).andExpect(status().isOk()); + } + + @Test + public void getWhenExpressionHasRoleUserConfiguredAndRoleIsAdminThenRespondsWithForbidden() throws Exception { + this.spring.register(ExpressionRoleUserConfig.class, BasicController.class).autowire(); + // @formatter:off + MockHttpServletRequestBuilder requestWithAdmin = get("/") + .with(user("user") + .roles("ADMIN")); + // @formatter:on + this.mvc.perform(requestWithAdmin).andExpect(status().isForbidden()); + } + + @Test + public void getWhenExpressionRoleUserOrAdminConfiguredAndRoleIsUserThenRespondsWithOk() throws Exception { + this.spring.register(ExpressionRoleUserOrAdminConfig.class, BasicController.class).autowire(); + // @formatter:off + MockHttpServletRequestBuilder requestWithUser = get("/") + .with(user("user") + .roles("USER")); + // @formatter:on + this.mvc.perform(requestWithUser).andExpect(status().isOk()); + } + + @Test + public void getWhenExpressionRoleUserOrAdminConfiguredAndRoleIsAdminThenRespondsWithOk() throws Exception { + this.spring.register(ExpressionRoleUserOrAdminConfig.class, BasicController.class).autowire(); + // @formatter:off + MockHttpServletRequestBuilder requestWithAdmin = get("/") + .with(user("user") + .roles("ADMIN")); + // @formatter:on + this.mvc.perform(requestWithAdmin).andExpect(status().isOk()); + } + + @Test + public void getWhenExpressionRoleUserOrAdminConfiguredAndRoleIsOtherThenRespondsWithForbidden() throws Exception { + this.spring.register(ExpressionRoleUserOrAdminConfig.class, BasicController.class).autowire(); + // @formatter:off + MockHttpServletRequestBuilder requestWithRoleOther = get("/") + .with(user("user") + .roles("OTHER")); + // @formatter:on + this.mvc.perform(requestWithRoleOther).andExpect(status().isForbidden()); + } + + @Test + public void getWhenExpressionHasIpAddressLocalhostConfiguredIpAddressIsLocalhostThenRespondsWithOk() + throws Exception { + this.spring.register(ExpressionIpAddressLocalhostConfig.class, BasicController.class).autowire(); + // @formatter:off + MockHttpServletRequestBuilder requestFromLocalhost = get("/") + .with(remoteAddress("127.0.0.1")); + // @formatter:on + this.mvc.perform(requestFromLocalhost).andExpect(status().isOk()); + } + + @Test + public void getWhenExpressionHasIpAddressLocalhostConfiguredIpAddressIsOtherThenRespondsWithForbidden() + throws Exception { + this.spring.register(ExpressionIpAddressLocalhostConfig.class, BasicController.class).autowire(); + // @formatter:off + MockHttpServletRequestBuilder requestFromOtherHost = get("/") + .with(remoteAddress("192.168.0.1")); + // @formatter:on + this.mvc.perform(requestFromOtherHost).andExpect(status().isForbidden()); + } + + private static RequestPostProcessor remoteAddress(String remoteAddress) { + return (request) -> { + request.setRemoteAddr(remoteAddress); + return request; + }; + } + @EnableWebSecurity static class NoRequestsConfig { @@ -713,6 +799,54 @@ public class AuthorizeHttpRequestsConfigurerTests { } + @EnableWebSecurity + static class ExpressionRoleUserConfig { + + @Bean + SecurityFilterChain filterChain(HttpSecurity http) throws Exception { + // @formatter:off + return http + .authorizeHttpRequests((requests) -> requests + .anyRequest().access(new WebExpressionAuthorizationManager("hasRole('USER')")) + ) + .build(); + // @formatter:on + } + + } + + @EnableWebSecurity + static class ExpressionRoleUserOrAdminConfig { + + @Bean + SecurityFilterChain filterChain(HttpSecurity http) throws Exception { + // @formatter:off + return http + .authorizeHttpRequests((requests) -> requests + .anyRequest().access(new WebExpressionAuthorizationManager("hasRole('USER') or hasRole('ADMIN')")) + ) + .build(); + // @formatter:on + } + + } + + @EnableWebSecurity + static class ExpressionIpAddressLocalhostConfig { + + @Bean + SecurityFilterChain filterChain(HttpSecurity http) throws Exception { + // @formatter:off + return http + .authorizeHttpRequests((requests) -> requests + .anyRequest().access(new WebExpressionAuthorizationManager("hasIpAddress('127.0.0.1')")) + ) + .build(); + // @formatter:on + } + + } + @Configuration static class AuthorizationEventPublisherConfig { diff --git a/web/src/main/java/org/springframework/security/web/access/expression/DefaultHttpSecurityExpressionHandler.java b/web/src/main/java/org/springframework/security/web/access/expression/DefaultHttpSecurityExpressionHandler.java new file mode 100644 index 0000000000..8533e214af --- /dev/null +++ b/web/src/main/java/org/springframework/security/web/access/expression/DefaultHttpSecurityExpressionHandler.java @@ -0,0 +1,78 @@ +/* + * Copyright 2002-2022 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.web.access.expression; + +import org.springframework.security.access.expression.AbstractSecurityExpressionHandler; +import org.springframework.security.access.expression.SecurityExpressionHandler; +import org.springframework.security.access.expression.SecurityExpressionOperations; +import org.springframework.security.authentication.AuthenticationTrustResolver; +import org.springframework.security.authentication.AuthenticationTrustResolverImpl; +import org.springframework.security.core.Authentication; +import org.springframework.security.web.access.intercept.RequestAuthorizationContext; +import org.springframework.util.Assert; + +/** + * A {@link SecurityExpressionHandler} that uses a {@link RequestAuthorizationContext} to + * create a {@link WebSecurityExpressionRoot}. + * + * @author Evgeniy Cheban + * @since 5.8 + */ +public class DefaultHttpSecurityExpressionHandler extends AbstractSecurityExpressionHandler + implements SecurityExpressionHandler { + + private AuthenticationTrustResolver trustResolver = new AuthenticationTrustResolverImpl(); + + private String defaultRolePrefix = "ROLE_"; + + @Override + protected SecurityExpressionOperations createSecurityExpressionRoot(Authentication authentication, + RequestAuthorizationContext context) { + WebSecurityExpressionRoot root = new WebSecurityExpressionRoot(authentication, context.getRequest()); + root.setRoleHierarchy(getRoleHierarchy()); + root.setPermissionEvaluator(getPermissionEvaluator()); + root.setTrustResolver(this.trustResolver); + root.setDefaultRolePrefix(this.defaultRolePrefix); + return root; + } + + /** + * Sets the {@link AuthenticationTrustResolver} to be used. The default is + * {@link AuthenticationTrustResolverImpl}. + * @param trustResolver the {@link AuthenticationTrustResolver} to use + */ + public void setTrustResolver(AuthenticationTrustResolver trustResolver) { + Assert.notNull(trustResolver, "trustResolver cannot be null"); + this.trustResolver = trustResolver; + } + + /** + * Sets the default prefix to be added to + * {@link org.springframework.security.access.expression.SecurityExpressionRoot#hasAnyRole(String...)} + * or + * {@link org.springframework.security.access.expression.SecurityExpressionRoot#hasRole(String)}. + * For example, if hasRole("ADMIN") or hasRole("ROLE_ADMIN") is passed in, then the + * role ROLE_ADMIN will be used when the defaultRolePrefix is "ROLE_" (default). + * @param defaultRolePrefix the default prefix to add to roles. The default is + * "ROLE_". + */ + public void setDefaultRolePrefix(String defaultRolePrefix) { + Assert.hasText(defaultRolePrefix, "defaultRolePrefix cannot be empty"); + this.defaultRolePrefix = defaultRolePrefix; + } + +} diff --git a/web/src/main/java/org/springframework/security/web/access/expression/ExpressionAuthorizationDecision.java b/web/src/main/java/org/springframework/security/web/access/expression/ExpressionAuthorizationDecision.java new file mode 100644 index 0000000000..7095899d29 --- /dev/null +++ b/web/src/main/java/org/springframework/security/web/access/expression/ExpressionAuthorizationDecision.java @@ -0,0 +1,55 @@ +/* + * Copyright 2002-2022 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.web.access.expression; + +import org.springframework.expression.Expression; +import org.springframework.security.authorization.AuthorizationDecision; + +/** + * An expression-based {@link AuthorizationDecision}. + * + * @author Evgeniy Cheban + * @since 5.8 + */ +public final class ExpressionAuthorizationDecision extends AuthorizationDecision { + + private final Expression expression; + + /** + * Creates an instance. + * @param granted the decision to use + * @param expression the {@link Expression} to use + */ + public ExpressionAuthorizationDecision(boolean granted, Expression expression) { + super(granted); + this.expression = expression; + } + + /** + * Returns the {@link Expression}. + * @return the {@link Expression} to use + */ + public Expression getExpression() { + return this.expression; + } + + @Override + public String toString() { + return "ExpressionAuthorizationDecision[granted=" + isGranted() + ", expression='" + this.expression + "']"; + } + +} diff --git a/web/src/main/java/org/springframework/security/web/access/expression/WebExpressionAuthorizationManager.java b/web/src/main/java/org/springframework/security/web/access/expression/WebExpressionAuthorizationManager.java new file mode 100644 index 0000000000..d5d943f5f9 --- /dev/null +++ b/web/src/main/java/org/springframework/security/web/access/expression/WebExpressionAuthorizationManager.java @@ -0,0 +1,84 @@ +/* + * Copyright 2002-2022 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.web.access.expression; + +import java.util.function.Supplier; + +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.AuthorizationDecision; +import org.springframework.security.authorization.AuthorizationManager; +import org.springframework.security.core.Authentication; +import org.springframework.security.web.access.intercept.RequestAuthorizationContext; +import org.springframework.util.Assert; + +/** + * An expression-based {@link AuthorizationManager} that determines the access by + * evaluating the provided expression. + * + * @author Evgeniy Cheban + * @since 5.8 + */ +public final class WebExpressionAuthorizationManager implements AuthorizationManager { + + private SecurityExpressionHandler expressionHandler = new DefaultHttpSecurityExpressionHandler(); + + private Expression expression; + + /** + * Creates an instance. + * @param expressionString the raw expression string to parse + */ + public WebExpressionAuthorizationManager(String expressionString) { + Assert.hasText(expressionString, "expressionString cannot be empty"); + this.expression = this.expressionHandler.getExpressionParser().parseExpression(expressionString); + } + + /** + * Sets the {@link SecurityExpressionHandler} to be used. The default is + * {@link DefaultHttpSecurityExpressionHandler}. + * @param expressionHandler the {@link SecurityExpressionHandler} to use + */ + public void setExpressionHandler(SecurityExpressionHandler expressionHandler) { + Assert.notNull(expressionHandler, "expressionHandler cannot be null"); + this.expressionHandler = expressionHandler; + this.expression = expressionHandler.getExpressionParser() + .parseExpression(this.expression.getExpressionString()); + } + + /** + * Determines the access by evaluating the provided expression. + * @param authentication the {@link Supplier} of the {@link Authentication} to check + * @param context the {@link RequestAuthorizationContext} to check + * @return an {@link ExpressionAuthorizationDecision} based on the evaluated + * expression + */ + @Override + public AuthorizationDecision check(Supplier authentication, RequestAuthorizationContext context) { + EvaluationContext ctx = this.expressionHandler.createEvaluationContext(authentication.get(), context); + boolean granted = ExpressionUtils.evaluateAsBoolean(this.expression, ctx); + return new ExpressionAuthorizationDecision(granted, this.expression); + } + + @Override + public String toString() { + return "WebExpressionAuthorizationManager[expression='" + this.expression + "']"; + } + +} diff --git a/web/src/main/java/org/springframework/security/web/access/expression/WebSecurityExpressionRoot.java b/web/src/main/java/org/springframework/security/web/access/expression/WebSecurityExpressionRoot.java index e5027ebf2c..497e9b844a 100644 --- a/web/src/main/java/org/springframework/security/web/access/expression/WebSecurityExpressionRoot.java +++ b/web/src/main/java/org/springframework/security/web/access/expression/WebSecurityExpressionRoot.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2016 the original author or authors. + * Copyright 2002-2022 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. @@ -25,6 +25,7 @@ import org.springframework.security.web.util.matcher.IpAddressMatcher; /** * @author Luke Taylor + * @author Evgeniy Cheban * @since 3.0 */ public class WebSecurityExpressionRoot extends SecurityExpressionRoot { @@ -35,8 +36,19 @@ public class WebSecurityExpressionRoot extends SecurityExpressionRoot { public final HttpServletRequest request; public WebSecurityExpressionRoot(Authentication a, FilterInvocation fi) { - super(a); - this.request = fi.getRequest(); + this(a, fi.getRequest()); + } + + /** + * Creates an instance for the given {@link Authentication} and + * {@link HttpServletRequest}. + * @param authentication the {@link Authentication} to use + * @param request the {@link HttpServletRequest} to use + * @since 5.8 + */ + public WebSecurityExpressionRoot(Authentication authentication, HttpServletRequest request) { + super(authentication); + this.request = request; } /** diff --git a/web/src/test/java/org/springframework/security/web/access/expression/WebExpressionAuthorizationManagerTests.java b/web/src/test/java/org/springframework/security/web/access/expression/WebExpressionAuthorizationManagerTests.java new file mode 100644 index 0000000000..c10ae293ef --- /dev/null +++ b/web/src/test/java/org/springframework/security/web/access/expression/WebExpressionAuthorizationManagerTests.java @@ -0,0 +1,105 @@ +/* + * Copyright 2002-2022 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.web.access.expression; + +import org.junit.jupiter.api.Test; + +import org.springframework.expression.Expression; +import org.springframework.expression.ExpressionParser; +import org.springframework.mock.web.MockHttpServletRequest; +import org.springframework.security.authentication.TestAuthentication; +import org.springframework.security.authorization.AuthorizationDecision; +import org.springframework.security.web.access.intercept.RequestAuthorizationContext; + +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 WebExpressionAuthorizationManager}. + * + * @author Evgeniy Cheban + */ +class WebExpressionAuthorizationManagerTests { + + @Test + void instantiateWhenExpressionStringNullThenIllegalArgumentException() { + assertThatIllegalArgumentException().isThrownBy(() -> new WebExpressionAuthorizationManager(null)) + .withMessage("expressionString cannot be empty"); + } + + @Test + void instantiateWhenExpressionStringEmptyThenIllegalArgumentException() { + assertThatIllegalArgumentException().isThrownBy(() -> new WebExpressionAuthorizationManager("")) + .withMessage("expressionString cannot be empty"); + } + + @Test + void instantiateWhenExpressionStringBlankThenIllegalArgumentException() { + assertThatIllegalArgumentException().isThrownBy(() -> new WebExpressionAuthorizationManager(" ")) + .withMessage("expressionString cannot be empty"); + } + + @Test + void instantiateWhenExpressionHandlerNotSetThenDefaultUsed() { + WebExpressionAuthorizationManager manager = new WebExpressionAuthorizationManager("hasRole('ADMIN')"); + assertThat(manager).extracting("expressionHandler").isInstanceOf(DefaultHttpSecurityExpressionHandler.class); + } + + @Test + void setExpressionHandlerWhenNullThenIllegalArgumentException() { + WebExpressionAuthorizationManager manager = new WebExpressionAuthorizationManager("hasRole('ADMIN')"); + assertThatIllegalArgumentException().isThrownBy(() -> manager.setExpressionHandler(null)) + .withMessage("expressionHandler cannot be null"); + } + + @Test + void setExpressionHandlerWhenNotNullThenVerifyExpressionHandler() { + String expressionString = "hasRole('ADMIN')"; + WebExpressionAuthorizationManager manager = new WebExpressionAuthorizationManager(expressionString); + DefaultHttpSecurityExpressionHandler expressionHandler = new DefaultHttpSecurityExpressionHandler(); + ExpressionParser mockExpressionParser = mock(ExpressionParser.class); + Expression mockExpression = mock(Expression.class); + given(mockExpressionParser.parseExpression(expressionString)).willReturn(mockExpression); + expressionHandler.setExpressionParser(mockExpressionParser); + manager.setExpressionHandler(expressionHandler); + assertThat(manager).extracting("expressionHandler").isEqualTo(expressionHandler); + assertThat(manager).extracting("expression").isEqualTo(mockExpression); + verify(mockExpressionParser).parseExpression(expressionString); + } + + @Test + void checkWhenExpressionHasRoleAdminConfiguredAndRoleAdminThenGrantedDecision() { + WebExpressionAuthorizationManager manager = new WebExpressionAuthorizationManager("hasRole('ADMIN')"); + AuthorizationDecision decision = manager.check(TestAuthentication::authenticatedAdmin, + new RequestAuthorizationContext(new MockHttpServletRequest())); + assertThat(decision).isNotNull(); + assertThat(decision.isGranted()).isTrue(); + } + + @Test + void checkWhenExpressionHasRoleAdminConfiguredAndRoleUserThenDeniedDecision() { + WebExpressionAuthorizationManager manager = new WebExpressionAuthorizationManager("hasRole('ADMIN')"); + AuthorizationDecision decision = manager.check(TestAuthentication::authenticatedUser, + new RequestAuthorizationContext(new MockHttpServletRequest())); + assertThat(decision).isNotNull(); + assertThat(decision.isGranted()).isFalse(); + } + +}