diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/builders/WebSecurity.java b/config/src/main/java/org/springframework/security/config/annotation/web/builders/WebSecurity.java index 479a6ae60c..329859a1f5 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/web/builders/WebSecurity.java +++ b/config/src/main/java/org/springframework/security/config/annotation/web/builders/WebSecurity.java @@ -35,7 +35,9 @@ import org.springframework.core.ResolvableType; import org.springframework.security.access.PermissionEvaluator; import org.springframework.security.access.expression.SecurityExpressionHandler; import org.springframework.security.access.hierarchicalroles.RoleHierarchy; +import org.springframework.security.authorization.AuthorizationDecision; import org.springframework.security.authorization.AuthorizationManager; +import org.springframework.security.authorization.SingleResultAuthorizationManager; import org.springframework.security.config.ObjectPostProcessor; import org.springframework.security.config.annotation.AbstractConfiguredSecurityBuilder; import org.springframework.security.config.annotation.SecurityBuilder; @@ -58,6 +60,8 @@ import org.springframework.security.web.access.WebInvocationPrivilegeEvaluator; import org.springframework.security.web.access.expression.DefaultWebSecurityExpressionHandler; import org.springframework.security.web.access.intercept.AuthorizationFilter; import org.springframework.security.web.access.intercept.FilterSecurityInterceptor; +import org.springframework.security.web.access.intercept.RequestAuthorizationContext; +import org.springframework.security.web.access.intercept.RequestMatcherDelegatingAuthorizationManager; import org.springframework.security.web.debug.DebugFilter; import org.springframework.security.web.firewall.CompositeRequestRejectedHandler; import org.springframework.security.web.firewall.HttpFirewall; @@ -65,6 +69,7 @@ import org.springframework.security.web.firewall.HttpStatusRequestRejectedHandle import org.springframework.security.web.firewall.ObservationMarkingRequestRejectedHandler; import org.springframework.security.web.firewall.RequestRejectedHandler; import org.springframework.security.web.firewall.StrictHttpFirewall; +import org.springframework.security.web.util.matcher.AnyRequestMatcher; import org.springframework.security.web.util.matcher.RequestMatcher; import org.springframework.security.web.util.matcher.RequestMatcherEntry; import org.springframework.util.Assert; @@ -230,8 +235,8 @@ public final class WebSecurity extends AbstractConfiguredSecurityBuilder securityFilterChains = new ArrayList<>(chainSize); - List>> requestMatcherPrivilegeEvaluatorsEntries = new ArrayList<>(); + RequestMatcherDelegatingAuthorizationManager.Builder builder = RequestMatcherDelegatingAuthorizationManager + .builder(); + boolean mappings = false; for (RequestMatcher ignoredRequest : this.ignoredRequests) { WebSecurity.this.logger.warn("You are asking Spring Security to ignore " + ignoredRequest + ". This is not recommended -- please use permitAll via HttpSecurity#authorizeHttpRequests instead."); SecurityFilterChain securityFilterChain = new DefaultSecurityFilterChain(ignoredRequest); securityFilterChains.add(securityFilterChain); - requestMatcherPrivilegeEvaluatorsEntries - .add(getRequestMatcherPrivilegeEvaluatorsEntry(securityFilterChain)); + builder.add(ignoredRequest, SingleResultAuthorizationManager.permitAll()); + mappings = true; } for (SecurityBuilder securityFilterChainBuilder : this.securityFilterChainBuilders) { SecurityFilterChain securityFilterChain = securityFilterChainBuilder.build(); securityFilterChains.add(securityFilterChain); - requestMatcherPrivilegeEvaluatorsEntries - .add(getRequestMatcherPrivilegeEvaluatorsEntry(securityFilterChain)); + mappings = addAuthorizationManager(securityFilterChain, builder) || mappings; } if (this.privilegeEvaluator == null) { + AuthorizationManager authorizationManager = mappings ? builder.build() + : SingleResultAuthorizationManager.permitAll(); + AuthorizationManagerWebInvocationPrivilegeEvaluator privilegeEvaluator = new AuthorizationManagerWebInvocationPrivilegeEvaluator( + authorizationManager); + privilegeEvaluator.setServletContext(this.servletContext); + if (this.privilegeEvaluatorRequestTransformer != null) { + privilegeEvaluator.setRequestTransformer(this.privilegeEvaluatorRequestTransformer); + } this.privilegeEvaluator = new RequestMatcherDelegatingWebInvocationPrivilegeEvaluator( - requestMatcherPrivilegeEvaluatorsEntries); + List.of(new RequestMatcherEntry<>(AnyRequestMatcher.INSTANCE, List.of(privilegeEvaluator)))); } FilterChainProxy filterChainProxy = new FilterChainProxy(securityFilterChains); if (this.httpFirewall != null) { @@ -350,30 +364,32 @@ public final class WebSecurity extends AbstractConfiguredSecurityBuilder> getRequestMatcherPrivilegeEvaluatorsEntry( - SecurityFilterChain securityFilterChain) { - List privilegeEvaluators = new ArrayList<>(); + private boolean addAuthorizationManager(SecurityFilterChain securityFilterChain, + RequestMatcherDelegatingAuthorizationManager.Builder builder) { + boolean mappings = false; for (Filter filter : securityFilterChain.getFilters()) { - if (filter instanceof FilterSecurityInterceptor) { - DefaultWebInvocationPrivilegeEvaluator defaultWebInvocationPrivilegeEvaluator = new DefaultWebInvocationPrivilegeEvaluator( - (FilterSecurityInterceptor) filter); - defaultWebInvocationPrivilegeEvaluator.setServletContext(this.servletContext); - privilegeEvaluators.add(defaultWebInvocationPrivilegeEvaluator); + if (filter instanceof FilterSecurityInterceptor securityInterceptor) { + DefaultWebInvocationPrivilegeEvaluator privilegeEvaluator = new DefaultWebInvocationPrivilegeEvaluator( + securityInterceptor); + privilegeEvaluator.setServletContext(this.servletContext); + AuthorizationManager authorizationManager = (authentication, context) -> { + HttpServletRequest request = context.getRequest(); + boolean result = privilegeEvaluator.isAllowed(request.getContextPath(), request.getRequestURI(), + request.getMethod(), authentication.get()); + return new AuthorizationDecision(result); + }; + builder.add(securityFilterChain::matches, authorizationManager); + mappings = true; continue; } - if (filter instanceof AuthorizationFilter) { - AuthorizationManager authorizationManager = ((AuthorizationFilter) filter) - .getAuthorizationManager(); - AuthorizationManagerWebInvocationPrivilegeEvaluator evaluator = new AuthorizationManagerWebInvocationPrivilegeEvaluator( - authorizationManager); - evaluator.setServletContext(this.servletContext); - if (this.privilegeEvaluatorRequestTransformer != null) { - evaluator.setRequestTransformer(this.privilegeEvaluatorRequestTransformer); - } - privilegeEvaluators.add(evaluator); + if (filter instanceof AuthorizationFilter authorization) { + AuthorizationManager authorizationManager = authorization.getAuthorizationManager(); + builder.add(securityFilterChain::matches, + (authentication, context) -> authorizationManager.check(authentication, context.getRequest())); + mappings = true; } } - return new RequestMatcherEntry<>(securityFilterChain::matches, privilegeEvaluators); + return mappings; } @Override diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/configuration/WebMvcSecurityConfiguration.java b/config/src/main/java/org/springframework/security/config/annotation/web/configuration/WebMvcSecurityConfiguration.java index 3f03140a46..85dd907ff8 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/web/configuration/WebMvcSecurityConfiguration.java +++ b/config/src/main/java/org/springframework/security/config/annotation/web/configuration/WebMvcSecurityConfiguration.java @@ -78,6 +78,8 @@ class WebMvcSecurityConfiguration implements WebMvcConfigurer, ApplicationContex private static final String HANDLER_MAPPING_INTROSPECTOR_BEAN_NAME = "mvcHandlerMappingIntrospector"; + private static final String PATH_PATTERN_REQUEST_TRANSFORMER_BEAN_NAME = "pathPatternRequestTransformer"; + private BeanResolver beanResolver; private SecurityContextHolderStrategy securityContextHolderStrategy = SecurityContextHolder @@ -119,18 +121,8 @@ class WebMvcSecurityConfiguration implements WebMvcConfigurer, ApplicationContex } } - /** - * Used to ensure Spring MVC request matching is cached. - * - * Creates a {@link BeanDefinitionRegistryPostProcessor} that detects if a bean named - * HANDLER_MAPPING_INTROSPECTOR_BEAN_NAME is defined. If so, it moves the - * AbstractSecurityWebApplicationInitializer.DEFAULT_FILTER_NAME to another bean name - * and then adds a {@link CompositeFilter} that contains - * {@link HandlerMappingIntrospector#createCacheFilter()} and the original - * FilterChainProxy under the original Bean name. - * @return - */ @Bean + @Deprecated static BeanDefinitionRegistryPostProcessor springSecurityHandlerMappingIntrospectorBeanDefinitionRegistryPostProcessor() { return new BeanDefinitionRegistryPostProcessor() { @Override @@ -144,12 +136,15 @@ class WebMvcSecurityConfiguration implements WebMvcConfigurer, ApplicationContex } String hmiRequestTransformerBeanName = HANDLER_MAPPING_INTROSPECTOR_BEAN_NAME + "RequestTransformer"; - if (!registry.containsBeanDefinition(hmiRequestTransformerBeanName)) { - BeanDefinition hmiRequestTransformer = BeanDefinitionBuilder - .rootBeanDefinition(HandlerMappingIntrospectorRequestTransformer.class) - .addConstructorArgReference(HANDLER_MAPPING_INTROSPECTOR_BEAN_NAME) - .getBeanDefinition(); - registry.registerBeanDefinition(hmiRequestTransformerBeanName, hmiRequestTransformer); + if (!registry.containsBeanDefinition(PATH_PATTERN_REQUEST_TRANSFORMER_BEAN_NAME) + && !registry.containsBeanDefinition(hmiRequestTransformerBeanName)) { + if (!registry.containsBeanDefinition(hmiRequestTransformerBeanName)) { + BeanDefinition hmiRequestTransformer = BeanDefinitionBuilder + .rootBeanDefinition(HandlerMappingIntrospectorRequestTransformer.class) + .addConstructorArgReference(HANDLER_MAPPING_INTROSPECTOR_BEAN_NAME) + .getBeanDefinition(); + registry.registerBeanDefinition(hmiRequestTransformerBeanName, hmiRequestTransformer); + } } BeanDefinition filterChainProxy = registry @@ -178,7 +173,11 @@ class WebMvcSecurityConfiguration implements WebMvcConfigurer, ApplicationContex /** * {@link FactoryBean} to defer creation of * {@link HandlerMappingIntrospector#createCacheFilter()} + * + * @deprecated see {@link WebSecurityConfiguration} for + * {@link org.springframework.web.util.pattern.PathPattern} replacement */ + @Deprecated static class HandlerMappingIntrospectorCacheFilterFactoryBean implements ApplicationContextAware, FactoryBean { @@ -207,7 +206,11 @@ class WebMvcSecurityConfiguration implements WebMvcConfigurer, ApplicationContex * Extends {@link FilterChainProxy} to provide as much passivity as possible but * delegates to {@link CompositeFilter} for * {@link #doFilter(ServletRequest, ServletResponse, FilterChain)}. + * + * @deprecated see {@link WebSecurityConfiguration} for + * {@link org.springframework.web.util.pattern.PathPattern} replacement */ + @Deprecated static class CompositeFilterChainProxy extends FilterChainProxy { /** diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/configuration/WebSecurityConfiguration.java b/config/src/main/java/org/springframework/security/config/annotation/web/configuration/WebSecurityConfiguration.java index f58e9e55fc..d1f6ae8699 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/web/configuration/WebSecurityConfiguration.java +++ b/config/src/main/java/org/springframework/security/config/annotation/web/configuration/WebSecurityConfiguration.java @@ -16,16 +16,29 @@ package org.springframework.security.config.annotation.web.configuration; +import java.io.IOException; import java.util.Collections; import java.util.List; import java.util.Map; import jakarta.servlet.Filter; +import jakarta.servlet.FilterChain; +import jakarta.servlet.ServletException; +import jakarta.servlet.ServletRequest; +import jakarta.servlet.ServletResponse; +import jakarta.servlet.http.HttpServletRequest; +import org.springframework.beans.BeanMetadataElement; +import org.springframework.beans.BeansException; import org.springframework.beans.factory.ObjectProvider; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.beans.factory.config.BeanDefinition; import org.springframework.beans.factory.config.BeanFactoryPostProcessor; import org.springframework.beans.factory.config.ConfigurableListableBeanFactory; +import org.springframework.beans.factory.support.BeanDefinitionBuilder; +import org.springframework.beans.factory.support.BeanDefinitionRegistry; +import org.springframework.beans.factory.support.BeanDefinitionRegistryPostProcessor; +import org.springframework.beans.factory.support.ManagedList; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.DependsOn; @@ -45,11 +58,18 @@ import org.springframework.security.config.annotation.web.builders.HttpSecurity; import org.springframework.security.config.annotation.web.builders.WebSecurity; import org.springframework.security.config.crypto.RsaKeyConversionServicePostProcessor; import org.springframework.security.context.DelegatingApplicationListener; +import org.springframework.security.core.context.SecurityContextHolderStrategy; import org.springframework.security.web.FilterChainProxy; import org.springframework.security.web.FilterInvocation; import org.springframework.security.web.SecurityFilterChain; import org.springframework.security.web.access.WebInvocationPrivilegeEvaluator; import org.springframework.security.web.context.AbstractSecurityWebApplicationInitializer; +import org.springframework.security.web.debug.DebugFilter; +import org.springframework.security.web.firewall.HttpFirewall; +import org.springframework.security.web.firewall.RequestRejectedHandler; +import org.springframework.web.filter.CompositeFilter; +import org.springframework.web.filter.ServletRequestPathFilter; +import org.springframework.web.servlet.handler.HandlerMappingIntrospector; /** * Uses a {@link WebSecurity} to create the {@link FilterChainProxy} that performs the web @@ -186,6 +206,56 @@ public class WebSecurityConfiguration implements ImportAware { } } + /** + * Used to ensure Spring MVC request matching is cached. + * + * Creates a {@link BeanDefinitionRegistryPostProcessor} that detects if a bean named + * HANDLER_MAPPING_INTROSPECTOR_BEAN_NAME is defined. If so, it moves the + * AbstractSecurityWebApplicationInitializer.DEFAULT_FILTER_NAME to another bean name + * and then adds a {@link CompositeFilter} that contains + * {@link HandlerMappingIntrospector#createCacheFilter()} and the original + * FilterChainProxy under the original Bean name. + * @return + */ + @Bean + static BeanDefinitionRegistryPostProcessor springSecurityPathPatternParserBeanDefinitionRegistryPostProcessor() { + return new BeanDefinitionRegistryPostProcessor() { + @Override + public void postProcessBeanFactory(ConfigurableListableBeanFactory beanFactory) throws BeansException { + } + + @Override + public void postProcessBeanDefinitionRegistry(BeanDefinitionRegistry registry) throws BeansException { + BeanDefinition filterChainProxy = registry + .getBeanDefinition(AbstractSecurityWebApplicationInitializer.DEFAULT_FILTER_NAME); + + if (filterChainProxy.getResolvableType().isAssignableFrom(CompositeFilterChainProxy.class)) { + return; + } + + BeanDefinitionBuilder pppCacheFilterBldr = BeanDefinitionBuilder + .rootBeanDefinition(ServletRequestPathFilter.class) + .setRole(BeanDefinition.ROLE_INFRASTRUCTURE); + + ManagedList filters = new ManagedList<>(); + filters.add(pppCacheFilterBldr.getBeanDefinition()); + filters.add(filterChainProxy); + BeanDefinitionBuilder compositeSpringSecurityFilterChainBldr = BeanDefinitionBuilder + .rootBeanDefinition(CompositeFilterChainProxy.class) + .addConstructorArgValue(filters); + + if (filterChainProxy.getResolvableType().isInstance(DebugFilter.class)) { + compositeSpringSecurityFilterChainBldr = BeanDefinitionBuilder.rootBeanDefinition(DebugFilter.class) + .addConstructorArgValue(compositeSpringSecurityFilterChainBldr.getBeanDefinition()); + } + + registry.removeBeanDefinition(AbstractSecurityWebApplicationInitializer.DEFAULT_FILTER_NAME); + registry.registerBeanDefinition(AbstractSecurityWebApplicationInitializer.DEFAULT_FILTER_NAME, + compositeSpringSecurityFilterChainBldr.getBeanDefinition()); + } + }; + } + /** * A custom version of the Spring provided AnnotationAwareOrderComparator that uses * {@link AnnotationUtils#findAnnotation(Class, Class)} to look on super class @@ -219,4 +289,126 @@ public class WebSecurityConfiguration implements ImportAware { } + /** + * Extends {@link FilterChainProxy} to provide as much passivity as possible but + * delegates to {@link CompositeFilter} for + * {@link #doFilter(ServletRequest, ServletResponse, FilterChain)}. + */ + static class CompositeFilterChainProxy extends FilterChainProxy { + + /** + * Used for {@link #doFilter(ServletRequest, ServletResponse, FilterChain)} + */ + private final Filter doFilterDelegate; + + private final FilterChainProxy springSecurityFilterChain; + + /** + * Creates a new instance + * @param filters the Filters to delegate to. One of which must be + * FilterChainProxy. + */ + CompositeFilterChainProxy(List filters) { + this.doFilterDelegate = createDoFilterDelegate(filters); + this.springSecurityFilterChain = findFilterChainProxy(filters); + } + + CompositeFilterChainProxy(Filter delegate, FilterChainProxy filterChain) { + this.doFilterDelegate = delegate; + this.springSecurityFilterChain = filterChain; + } + + @Override + public void afterPropertiesSet() { + this.springSecurityFilterChain.afterPropertiesSet(); + } + + @Override + public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) + throws IOException, ServletException { + this.doFilterDelegate.doFilter(request, response, chain); + } + + @Override + public List getFilters(String url) { + return this.springSecurityFilterChain.getFilters(url); + } + + @Override + public List getFilterChains() { + return this.springSecurityFilterChain.getFilterChains(); + } + + @Override + public void setSecurityContextHolderStrategy(SecurityContextHolderStrategy securityContextHolderStrategy) { + this.springSecurityFilterChain.setSecurityContextHolderStrategy(securityContextHolderStrategy); + } + + @Override + public void setFilterChainValidator(FilterChainValidator filterChainValidator) { + this.springSecurityFilterChain.setFilterChainValidator(filterChainValidator); + } + + @Override + public void setFilterChainDecorator(FilterChainDecorator filterChainDecorator) { + this.springSecurityFilterChain.setFilterChainDecorator(filterChainDecorator); + } + + @Override + public void setFirewall(HttpFirewall firewall) { + this.springSecurityFilterChain.setFirewall(firewall); + } + + @Override + public void setRequestRejectedHandler(RequestRejectedHandler requestRejectedHandler) { + this.springSecurityFilterChain.setRequestRejectedHandler(requestRejectedHandler); + } + + /** + * Used through reflection by Spring Security's Test support to lookup the + * FilterChainProxy Filters for a specific HttpServletRequest. + * @param request + * @return + */ + private List getFilters(HttpServletRequest request) { + List filterChains = this.springSecurityFilterChain.getFilterChains(); + for (SecurityFilterChain chain : filterChains) { + if (chain.matches(request)) { + return chain.getFilters(); + } + } + return null; + } + + /** + * Creates the Filter to delegate to for doFilter + * @param filters the Filters to delegate to. + * @return the Filter for doFilter + */ + private static Filter createDoFilterDelegate(List filters) { + CompositeFilter delegate = new CompositeFilter(); + delegate.setFilters(filters); + return delegate; + } + + /** + * Find the FilterChainProxy in a List of Filter + * @param filters + * @return non-null FilterChainProxy + * @throws IllegalStateException if the FilterChainProxy cannot be found + */ + private static FilterChainProxy findFilterChainProxy(List filters) { + for (Filter filter : filters) { + if (filter instanceof FilterChainProxy fcp) { + return fcp; + } + if (filter instanceof DebugFilter debugFilter) { + return new CompositeFilterChainProxy(debugFilter, debugFilter.getFilterChainProxy()); + } + } + throw new IllegalStateException("Couldn't find FilterChainProxy in " + filters); + } + + } + } diff --git a/config/src/main/java/org/springframework/security/config/aot/hint/WebSecurityConfigurationRuntimeHints.java b/config/src/main/java/org/springframework/security/config/aot/hint/WebSecurityConfigurationRuntimeHints.java new file mode 100644 index 0000000000..bf1b184179 --- /dev/null +++ b/config/src/main/java/org/springframework/security/config/aot/hint/WebSecurityConfigurationRuntimeHints.java @@ -0,0 +1,40 @@ +/* + * Copyright 2002-2023 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.config.aot.hint; + +import org.springframework.aot.hint.MemberCategory; +import org.springframework.aot.hint.RuntimeHints; +import org.springframework.aot.hint.RuntimeHintsRegistrar; +import org.springframework.aot.hint.TypeReference; + +/** + * Runtime hints for + * {@link org.springframework.security.config.annotation.web.configuration.WebMvcSecurityConfiguration} + * + * @author Marcus da Coregio + */ +class WebSecurityConfigurationRuntimeHints implements RuntimeHintsRegistrar { + + @Override + public void registerHints(RuntimeHints hints, ClassLoader classLoader) { + hints.reflection() + .registerType(TypeReference + .of("org.springframework.security.config.annotation.web.configuration.WebSecurityConfiguration$CompositeFilterChainProxy"), + MemberCategory.INVOKE_DECLARED_CONSTRUCTORS); + } + +} diff --git a/config/src/main/java/org/springframework/security/config/http/HttpConfigurationBuilder.java b/config/src/main/java/org/springframework/security/config/http/HttpConfigurationBuilder.java index fd18eb3362..1b22c044f6 100644 --- a/config/src/main/java/org/springframework/security/config/http/HttpConfigurationBuilder.java +++ b/config/src/main/java/org/springframework/security/config/http/HttpConfigurationBuilder.java @@ -51,6 +51,7 @@ import org.springframework.security.core.session.SessionRegistryImpl; import org.springframework.security.web.access.AuthorizationManagerWebInvocationPrivilegeEvaluator; import org.springframework.security.web.access.DefaultWebInvocationPrivilegeEvaluator; import org.springframework.security.web.access.HandlerMappingIntrospectorRequestTransformer; +import org.springframework.security.web.access.PathPatternRequestTransformer; import org.springframework.security.web.access.channel.ChannelDecisionManagerImpl; import org.springframework.security.web.access.channel.ChannelProcessingFilter; import org.springframework.security.web.access.channel.InsecureChannelProcessor; @@ -974,10 +975,17 @@ class HttpConfigurationBuilder { @Override public AuthorizationManagerWebInvocationPrivilegeEvaluator.HttpServletRequestTransformer getObject() throws Exception { + AuthorizationManagerWebInvocationPrivilegeEvaluator.HttpServletRequestTransformer requestTransformer = this.applicationContext + .getBeanProvider( + AuthorizationManagerWebInvocationPrivilegeEvaluator.HttpServletRequestTransformer.class) + .getIfUnique(); + if (requestTransformer != null) { + return requestTransformer; + } HandlerMappingIntrospector hmi = this.applicationContext.getBeanProvider(HandlerMappingIntrospector.class) .getIfAvailable(); return (hmi != null) ? new HandlerMappingIntrospectorRequestTransformer(hmi) - : AuthorizationManagerWebInvocationPrivilegeEvaluator.HttpServletRequestTransformer.IDENTITY; + : new PathPatternRequestTransformer(); } @Override diff --git a/config/src/test/java/org/springframework/security/config/annotation/web/configuration/WebSecurityConfigurationTests.java b/config/src/test/java/org/springframework/security/config/annotation/web/configuration/WebSecurityConfigurationTests.java index e546ffb6a1..f96d67809e 100644 --- a/config/src/test/java/org/springframework/security/config/annotation/web/configuration/WebSecurityConfigurationTests.java +++ b/config/src/test/java/org/springframework/security/config/annotation/web/configuration/WebSecurityConfigurationTests.java @@ -57,9 +57,12 @@ import org.springframework.security.core.Authentication; import org.springframework.security.web.FilterChainProxy; import org.springframework.security.web.FilterInvocation; import org.springframework.security.web.SecurityFilterChain; +import org.springframework.security.web.access.AuthorizationManagerWebInvocationPrivilegeEvaluator; +import org.springframework.security.web.access.PathPatternRequestTransformer; import org.springframework.security.web.access.RequestMatcherDelegatingWebInvocationPrivilegeEvaluator; import org.springframework.security.web.access.WebInvocationPrivilegeEvaluator; import org.springframework.security.web.access.expression.DefaultWebSecurityExpressionHandler; +import org.springframework.security.web.servlet.util.matcher.PathPatternRequestMatcher; import org.springframework.security.web.util.matcher.AntPathRequestMatcher; import org.springframework.test.web.servlet.MockMvc; import org.springframework.util.ClassUtils; @@ -69,8 +72,12 @@ import org.springframework.web.servlet.config.annotation.EnableWebMvc; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.atLeastOnce; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.verify; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; @@ -320,6 +327,27 @@ public class WebSecurityConfigurationTests { assertThat(privilegeEvaluator.isAllowed("/ignoring1/child", null)).isTrue(); } + @Test + public void loadConfigWhenUsePathPatternThenEvaluates() { + this.spring.register(UsePathPatternConfig.class).autowire(); + WebInvocationPrivilegeEvaluator privilegeEvaluator = this.spring.getContext() + .getBean(WebInvocationPrivilegeEvaluator.class); + assertUserPermissions(privilegeEvaluator); + assertAdminPermissions(privilegeEvaluator); + assertAnotherUserPermission(privilegeEvaluator); + // null authentication + assertThat(privilegeEvaluator.isAllowed("/user", null)).isFalse(); + assertThat(privilegeEvaluator.isAllowed("/admin", null)).isFalse(); + assertThat(privilegeEvaluator.isAllowed("/another", null)).isTrue(); + assertThat(privilegeEvaluator.isAllowed("/ignoring1", null)).isTrue(); + assertThat(privilegeEvaluator.isAllowed("/ignoring1/child", null)).isTrue(); + AuthorizationManagerWebInvocationPrivilegeEvaluator.HttpServletRequestTransformer requestTransformer = this.spring + .getContext() + .getBean(AuthorizationManagerWebInvocationPrivilegeEvaluator.HttpServletRequestTransformer.class); + verify(requestTransformer, atLeastOnce()).transform(any()); + + } + @Test public void loadConfigWhenTwoSecurityFilterChainsPresentAndSecondWithAnyRequestThenException() { assertThatExceptionOfType(BeanCreationException.class) @@ -862,6 +890,53 @@ public class WebSecurityConfigurationTests { } + @Configuration + @EnableWebSecurity + @EnableWebMvc + @Import(AuthenticationTestConfiguration.class) + static class UsePathPatternConfig { + + @Bean + AuthorizationManagerWebInvocationPrivilegeEvaluator.HttpServletRequestTransformer pathPatternRequestTransformer() { + return spy(new PathPatternRequestTransformer()); + } + + @Bean + public WebSecurityCustomizer webSecurityCustomizer() { + return (web) -> web.ignoring().requestMatchers("/ignoring1/**"); + } + + @Bean + @Order(Ordered.HIGHEST_PRECEDENCE) + public SecurityFilterChain notAuthorized(HttpSecurity http) throws Exception { + // @formatter:off + http + .securityMatchers((requests) -> requests.requestMatchers(PathPatternRequestMatcher.withDefaults().matcher("/user"))) + .authorizeHttpRequests((requests) -> requests.anyRequest().hasRole("USER")); + // @formatter:on + return http.build(); + } + + @Bean + @Order(Ordered.HIGHEST_PRECEDENCE + 1) + public SecurityFilterChain admin(HttpSecurity http) throws Exception { + // @formatter:off + http + .securityMatchers((requests) -> requests.requestMatchers(PathPatternRequestMatcher.withDefaults().matcher("/admin"))) + .authorizeHttpRequests((requests) -> requests.anyRequest().hasRole("ADMIN")); + // @formatter:on + return http.build(); + } + + @Bean + @Order(Ordered.LOWEST_PRECEDENCE) + public SecurityFilterChain permitAll(HttpSecurity http) throws Exception { + http.authorizeHttpRequests((requests) -> requests.anyRequest().permitAll()); + return http.build(); + } + + } + @Configuration @EnableWebSecurity @EnableWebMvc diff --git a/docs/modules/ROOT/pages/migration-7/web.adoc b/docs/modules/ROOT/pages/migration-7/web.adoc index aa898da4fd..330d87ddc2 100644 --- a/docs/modules/ROOT/pages/migration-7/web.adoc +++ b/docs/modules/ROOT/pages/migration-7/web.adoc @@ -339,6 +339,54 @@ casAuthentication.setProxyReceptorUrl(PathPatternRequestMatcher.withDefaults().m ---- ====== +=== Migrate your WebInvocationPrivilegeEvaluator + +If you are using Spring Security's JSP Taglibs or are using `WebInvocationPrivilegeEvaluator` directly, be aware of the following changes: + +1. `RequestMatcherWebInvocationPrivilegeEvaluator` is deprecated in favor of `AuthorizationManagerWebInvocationPrivilegeEvaluator` +2. `HandlerMappingIntrospectorRequestTransformer` is deprecated in favor of `PathPatternRequestTransformer` + +If you are not constructing these directly, you can opt-in to both changes in advance by publishing a `PathPatternRequestTransformer` like so: + +[tabs] +====== +Java:: ++ +[source,java,role="primary"] +---- +@Bean +HttpServletRequestTransformer pathPatternRequestTransformer() { + return new PathPatternRequestTransformer(); +} +---- + +Kotlin:: ++ +[source,kotlin,role="secondary"] +---- +@Bean +fun pathPatternRequestTransformer(): HttpServletRequestTransformer { + return PathPatternRequestTransformer() +} +---- + +Xml:: ++ +[source,xml,role="secondary"] +---- + +---- +====== + +Spring Security will take this as a signal to use the new implementations. + +[[NOTE]] +---- +One difference you may notice is that `AuthorizationManagerWebPrivilegeInvocationEvaluator` allows the authentication to be `null` if the authorization rule is `permitAll`. + +Test your endpoints that `permitAll` in case JSP requests using this same require should not, in fact, be permitted. +---- + == Include the Servlet Path Prefix in Authorization Rules For many applications <> will make no difference since most commonly all URIs listed are matched by the default servlet. diff --git a/web/src/main/java/org/springframework/security/web/FilterChainProxy.java b/web/src/main/java/org/springframework/security/web/FilterChainProxy.java index f9ad696436..c71ea74930 100644 --- a/web/src/main/java/org/springframework/security/web/FilterChainProxy.java +++ b/web/src/main/java/org/springframework/security/web/FilterChainProxy.java @@ -46,7 +46,6 @@ import org.springframework.security.web.util.matcher.RequestMatcher; import org.springframework.util.Assert; import org.springframework.web.filter.DelegatingFilterProxy; import org.springframework.web.filter.GenericFilterBean; -import org.springframework.web.filter.ServletRequestPathFilter; /** * Delegates {@code Filter} requests to a list of Spring-managed filter beans. As of @@ -163,8 +162,6 @@ public class FilterChainProxy extends GenericFilterBean { private FilterChainDecorator filterChainDecorator = new VirtualFilterChainDecorator(); - private Filter springWebFilter = new ServletRequestPathFilter(); - public FilterChainProxy() { } @@ -213,29 +210,27 @@ public class FilterChainProxy extends GenericFilterBean { throws IOException, ServletException { FirewalledRequest firewallRequest = this.firewall.getFirewalledRequest((HttpServletRequest) request); HttpServletResponse firewallResponse = this.firewall.getFirewalledResponse((HttpServletResponse) response); - this.springWebFilter.doFilter(firewallRequest, firewallResponse, (r, s) -> { - List filters = getFilters(firewallRequest); - if (filters == null || filters.isEmpty()) { - if (logger.isTraceEnabled()) { - logger.trace(LogMessage.of(() -> "No security for " + requestLine(firewallRequest))); - } - firewallRequest.reset(); - this.filterChainDecorator.decorate(chain).doFilter(firewallRequest, firewallResponse); - return; + List filters = getFilters(firewallRequest); + if (filters == null || filters.isEmpty()) { + if (logger.isTraceEnabled()) { + logger.trace(LogMessage.of(() -> "No security for " + requestLine(firewallRequest))); } + firewallRequest.reset(); + this.filterChainDecorator.decorate(chain).doFilter(firewallRequest, firewallResponse); + return; + } + if (logger.isDebugEnabled()) { + logger.debug(LogMessage.of(() -> "Securing " + requestLine(firewallRequest))); + } + FilterChain reset = (req, res) -> { if (logger.isDebugEnabled()) { - logger.debug(LogMessage.of(() -> "Securing " + requestLine(firewallRequest))); + logger.debug(LogMessage.of(() -> "Secured " + requestLine(firewallRequest))); } - FilterChain reset = (req, res) -> { - if (logger.isDebugEnabled()) { - logger.debug(LogMessage.of(() -> "Secured " + requestLine(firewallRequest))); - } - // Deactivate path stripping as we exit the security filter chain - firewallRequest.reset(); - chain.doFilter(req, res); - }; - this.filterChainDecorator.decorate(reset, filters).doFilter(firewallRequest, firewallResponse); - }); + // Deactivate path stripping as we exit the security filter chain + firewallRequest.reset(); + chain.doFilter(req, res); + }; + this.filterChainDecorator.decorate(reset, filters).doFilter(firewallRequest, firewallResponse); } /** diff --git a/web/src/main/java/org/springframework/security/web/access/HandlerMappingIntrospectorRequestTransformer.java b/web/src/main/java/org/springframework/security/web/access/HandlerMappingIntrospectorRequestTransformer.java index 42cf51c3c3..f78ad46a50 100644 --- a/web/src/main/java/org/springframework/security/web/access/HandlerMappingIntrospectorRequestTransformer.java +++ b/web/src/main/java/org/springframework/security/web/access/HandlerMappingIntrospectorRequestTransformer.java @@ -35,7 +35,9 @@ import org.springframework.web.servlet.handler.HandlerMappingIntrospector; * default throw {@link UnsupportedOperationException}. * * @author Rob Winch + * @deprecated please use {@link PathPatternRequestTransformer} instead */ +@Deprecated(forRemoval = true) public class HandlerMappingIntrospectorRequestTransformer implements AuthorizationManagerWebInvocationPrivilegeEvaluator.HttpServletRequestTransformer { diff --git a/web/src/main/java/org/springframework/security/web/access/PathPatternRequestTransformer.java b/web/src/main/java/org/springframework/security/web/access/PathPatternRequestTransformer.java new file mode 100644 index 0000000000..c77072e3ac --- /dev/null +++ b/web/src/main/java/org/springframework/security/web/access/PathPatternRequestTransformer.java @@ -0,0 +1,70 @@ +/* + * Copyright 2002-2025 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; + +import java.util.HashMap; +import java.util.Map; + +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletRequestWrapper; + +import org.springframework.security.web.servlet.util.matcher.PathPatternRequestMatcher; +import org.springframework.web.util.ServletRequestPathUtils; + +/** + * Prepares the privilege evaluator's request for {@link PathPatternRequestMatcher} + * authorization rules. + * + * @author Josh Cummings + * @since 6.5 + */ +public final class PathPatternRequestTransformer + implements AuthorizationManagerWebInvocationPrivilegeEvaluator.HttpServletRequestTransformer { + + @Override + public HttpServletRequest transform(HttpServletRequest request) { + HttpServletRequest wrapped = new AttributesSupportingHttpServletRequest(request); + ServletRequestPathUtils.parseAndCache(wrapped); + return wrapped; + } + + private static final class AttributesSupportingHttpServletRequest extends HttpServletRequestWrapper { + + private final Map attributes = new HashMap<>(); + + AttributesSupportingHttpServletRequest(HttpServletRequest request) { + super(request); + } + + @Override + public Object getAttribute(String name) { + return this.attributes.get(name); + } + + @Override + public void setAttribute(String name, Object value) { + this.attributes.put(name, value); + } + + @Override + public void removeAttribute(String name) { + this.attributes.remove(name); + } + + } + +} diff --git a/web/src/main/java/org/springframework/security/web/access/RequestMatcherDelegatingWebInvocationPrivilegeEvaluator.java b/web/src/main/java/org/springframework/security/web/access/RequestMatcherDelegatingWebInvocationPrivilegeEvaluator.java index f410bf19ef..e50153ce5d 100644 --- a/web/src/main/java/org/springframework/security/web/access/RequestMatcherDelegatingWebInvocationPrivilegeEvaluator.java +++ b/web/src/main/java/org/springframework/security/web/access/RequestMatcherDelegatingWebInvocationPrivilegeEvaluator.java @@ -17,20 +17,17 @@ package org.springframework.security.web.access; import java.util.Collections; -import java.util.HashMap; import java.util.List; -import java.util.Map; import jakarta.servlet.ServletContext; import jakarta.servlet.http.HttpServletRequest; -import jakarta.servlet.http.HttpServletRequestWrapper; +import org.springframework.security.authorization.AuthorizationManager; import org.springframework.security.core.Authentication; import org.springframework.security.web.FilterInvocation; import org.springframework.security.web.util.matcher.RequestMatcherEntry; import org.springframework.util.Assert; import org.springframework.web.context.ServletContextAware; -import org.springframework.web.util.ServletRequestPathUtils; /** * A {@link WebInvocationPrivilegeEvaluator} which delegates to a list of @@ -39,7 +36,11 @@ import org.springframework.web.util.ServletRequestPathUtils; * * @author Marcus Da Coregio * @since 5.5.5 + * @deprecated please use {@link AuthorizationManagerWebInvocationPrivilegeEvaluator} and + * adapt any delegate {@link WebInvocationPrivilegeEvaluator}s into + * {@link AuthorizationManager}s */ +@Deprecated public final class RequestMatcherDelegatingWebInvocationPrivilegeEvaluator implements WebInvocationPrivilegeEvaluator, ServletContextAware { @@ -120,8 +121,7 @@ public final class RequestMatcherDelegatingWebInvocationPrivilegeEvaluator private List getDelegate(String contextPath, String uri, String method) { FilterInvocation filterInvocation = new FilterInvocation(contextPath, uri, method, this.servletContext); - HttpServletRequest request = new AttributesSupportingHttpServletRequest(filterInvocation.getHttpRequest()); - ServletRequestPathUtils.parseAndCache(request); + HttpServletRequest request = filterInvocation.getHttpRequest(); for (RequestMatcherEntry> delegate : this.delegates) { if (delegate.getRequestMatcher().matches(request)) { return delegate.getEntry(); @@ -135,29 +135,4 @@ public final class RequestMatcherDelegatingWebInvocationPrivilegeEvaluator this.servletContext = servletContext; } - private static final class AttributesSupportingHttpServletRequest extends HttpServletRequestWrapper { - - private final Map attributes = new HashMap<>(); - - AttributesSupportingHttpServletRequest(HttpServletRequest request) { - super(request); - } - - @Override - public Object getAttribute(String name) { - return this.attributes.get(name); - } - - @Override - public void setAttribute(String name, Object value) { - this.attributes.put(name, value); - } - - @Override - public void removeAttribute(String name) { - this.attributes.remove(name); - } - - } - } diff --git a/web/src/test/java/org/springframework/security/web/access/AuthorizationManagerWebInvocationPrivilegeEvaluatorTests.java b/web/src/test/java/org/springframework/security/web/access/AuthorizationManagerWebInvocationPrivilegeEvaluatorTests.java index afb5ad9e97..0fd298ec96 100644 --- a/web/src/test/java/org/springframework/security/web/access/AuthorizationManagerWebInvocationPrivilegeEvaluatorTests.java +++ b/web/src/test/java/org/springframework/security/web/access/AuthorizationManagerWebInvocationPrivilegeEvaluatorTests.java @@ -31,6 +31,8 @@ import org.springframework.security.authentication.TestAuthentication; import org.springframework.security.authorization.AuthorizationDecision; import org.springframework.security.authorization.AuthorizationManager; import org.springframework.security.web.access.AuthorizationManagerWebInvocationPrivilegeEvaluator.HttpServletRequestTransformer; +import org.springframework.security.web.access.intercept.RequestMatcherDelegatingAuthorizationManager; +import org.springframework.security.web.servlet.util.matcher.PathPatternRequestMatcher; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; @@ -111,4 +113,19 @@ class AuthorizationManagerWebInvocationPrivilegeEvaluatorTests { verify(this.authorizationManager).check(any(), eq(request)); } + // gh-16771 + @Test + void isAllowedWhenInvokesDelegateThenCachesRequestPath() { + RequestMatcherDelegatingAuthorizationManager authorizationManager = RequestMatcherDelegatingAuthorizationManager + .builder() + .add(PathPatternRequestMatcher.withDefaults().matcher("/test/**"), + (authentication, context) -> this.authorizationManager.check(authentication, context.getRequest())) + .build(); + AuthorizationManagerWebInvocationPrivilegeEvaluator privilegeEvaluator = new AuthorizationManagerWebInvocationPrivilegeEvaluator( + authorizationManager); + privilegeEvaluator.setRequestTransformer(new PathPatternRequestTransformer()); + privilegeEvaluator.isAllowed("/test", TestAuthentication.authenticatedUser()); + verify(this.authorizationManager).check(any(), any()); + } + } diff --git a/web/src/test/java/org/springframework/security/web/access/RequestMatcherDelegatingWebInvocationPrivilegeEvaluatorTests.java b/web/src/test/java/org/springframework/security/web/access/RequestMatcherDelegatingWebInvocationPrivilegeEvaluatorTests.java index 8847cfa93e..ca39fef4be 100644 --- a/web/src/test/java/org/springframework/security/web/access/RequestMatcherDelegatingWebInvocationPrivilegeEvaluatorTests.java +++ b/web/src/test/java/org/springframework/security/web/access/RequestMatcherDelegatingWebInvocationPrivilegeEvaluatorTests.java @@ -22,16 +22,12 @@ import jakarta.servlet.http.HttpServletRequest; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.mockito.ArgumentCaptor; -import org.mockito.MockedStatic; -import org.mockito.Mockito; import org.springframework.mock.web.MockServletContext; import org.springframework.security.authentication.TestingAuthenticationToken; import org.springframework.security.core.Authentication; -import org.springframework.security.web.servlet.util.matcher.PathPatternRequestMatcher; import org.springframework.security.web.util.matcher.RequestMatcher; import org.springframework.security.web.util.matcher.RequestMatcherEntry; -import org.springframework.web.util.ServletRequestPathUtils; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; @@ -178,19 +174,6 @@ class RequestMatcherDelegatingWebInvocationPrivilegeEvaluatorTests { .withMessageContaining("requestMatcher cannot be null"); } - // gh-16771 - @Test - void isAllowedWhenInvokesDelegateThenCachesRequestPath() { - PathPatternRequestMatcher path = PathPatternRequestMatcher.withDefaults().matcher("/path/**"); - PathPatternRequestMatcher any = PathPatternRequestMatcher.withDefaults().matcher("/**"); - WebInvocationPrivilegeEvaluator delegating = evaluator(deny(path), deny(any)); - try (MockedStatic utils = Mockito.mockStatic(ServletRequestPathUtils.class, - Mockito.CALLS_REAL_METHODS)) { - delegating.isAllowed("/uri", null); - utils.verify(() -> ServletRequestPathUtils.parseAndCache(any()), times(1)); - } - } - @SuppressWarnings({ "rawtypes", "unchecked" }) private RequestMatcherDelegatingWebInvocationPrivilegeEvaluator evaluator(RequestMatcherEntry... entries) { return new RequestMatcherDelegatingWebInvocationPrivilegeEvaluator(List.of(entries));