Merge branch 'main' of https://github.com/spring-projects/spring-secu…

…rity into spring-projectsgh-16286
ngocnhan-tran1996 · Dec 21, 2024 · 9954101 · 9954101
2 parents 230f662 + fd267df
commit 9954101
Show file tree

Hide file tree

Showing 76 changed files with 2,788 additions and 107 deletions.
diff --git a/.github/dependabot.yml b/.github/dependabot.yml
@@ -19,6 +19,7 @@ updates:
       - dependency-name: com.nimbusds:nimbus-jose-jwt
       - dependency-name: org.python:jython
       - dependency-name: org.apache.directory.server:*
+      - dependency-name: org.apache.directory.shared:*
       - dependency-name: org.junit:junit-bom
         update-types:
           - version-update:semver-major
@@ -44,6 +45,7 @@ updates:
       - dependency-name: com.nimbusds:nimbus-jose-jwt
       - dependency-name: org.python:jython
       - dependency-name: org.apache.directory.server:*
+      - dependency-name: org.apache.directory.shared:*
       - dependency-name: org.junit:junit-bom
         update-types:
           - version-update:semver-major
@@ -69,6 +71,7 @@ updates:
       - dependency-name: com.nimbusds:nimbus-jose-jwt
       - dependency-name: org.python:jython
       - dependency-name: org.apache.directory.server:*
+      - dependency-name: org.apache.directory.shared:*
       - dependency-name: org.junit:junit-bom
         update-types:
           - version-update:semver-major

diff --git a/...g/annotation/authentication/configuration/InitializeUserDetailsBeanManagerConfigurer.java b/...g/annotation/authentication/configuration/InitializeUserDetailsBeanManagerConfigurer.java
@@ -95,14 +95,10 @@ else if (beanNames.length > 1) {
 			PasswordEncoder passwordEncoder = getBeanOrNull(PasswordEncoder.class);
 			UserDetailsPasswordService passwordManager = getBeanOrNull(UserDetailsPasswordService.class);
 			CompromisedPasswordChecker passwordChecker = getBeanOrNull(CompromisedPasswordChecker.class);
-			DaoAuthenticationProvider provider;
+			DaoAuthenticationProvider provider = new DaoAuthenticationProvider(userDetailsService);
 			if (passwordEncoder != null) {
-				provider = new DaoAuthenticationProvider(passwordEncoder);
+				provider.setPasswordEncoder(passwordEncoder);
 			}
-			else {
-				provider = new DaoAuthenticationProvider();
-			}
-			provider.setUserDetailsService(userDetailsService);
 			if (passwordManager != null) {
 				provider.setUserDetailsPasswordService(passwordManager);
 			}

diff --git a/...nnotation/authentication/configurers/userdetails/AbstractDaoAuthenticationConfigurer.java b/...nnotation/authentication/configurers/userdetails/AbstractDaoAuthenticationConfigurer.java
@@ -36,7 +36,7 @@
 public abstract class AbstractDaoAuthenticationConfigurer<B extends ProviderManagerBuilder<B>, C extends AbstractDaoAuthenticationConfigurer<B, C, U>, U extends UserDetailsService>
 		extends UserDetailsAwareConfigurer<B, U> {
 
-	private DaoAuthenticationProvider provider = new DaoAuthenticationProvider();
+	private DaoAuthenticationProvider provider;
 
 	private final U userDetailsService;
 
@@ -46,7 +46,7 @@ public abstract class AbstractDaoAuthenticationConfigurer<B extends ProviderMana
 	 */
 	AbstractDaoAuthenticationConfigurer(U userDetailsService) {
 		this.userDetailsService = userDetailsService;
-		this.provider.setUserDetailsService(userDetailsService);
+		this.provider = new DaoAuthenticationProvider(userDetailsService);
 		if (userDetailsService instanceof UserDetailsPasswordService) {
 			this.provider.setUserDetailsPasswordService((UserDetailsPasswordService) userDetailsService);
 		}

diff --git a/...ingframework/security/config/annotation/web/builders/WebSecurityFilterChainValidator.java b/...ingframework/security/config/annotation/web/builders/WebSecurityFilterChainValidator.java
@@ -18,28 +18,45 @@
 
 import java.util.List;
 
+import jakarta.servlet.Filter;
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+
 import org.springframework.security.web.DefaultSecurityFilterChain;
 import org.springframework.security.web.FilterChainProxy;
 import org.springframework.security.web.SecurityFilterChain;
+import org.springframework.security.web.UnreachableFilterChainException;
+import org.springframework.security.web.access.intercept.AuthorizationFilter;
+import org.springframework.security.web.access.intercept.FilterSecurityInterceptor;
 import org.springframework.security.web.util.matcher.AnyRequestMatcher;
 
 /**
  * A filter chain validator for filter chains built by {@link WebSecurity}
  *
+ * @author Josh Cummings
+ * @author Max Batischev
  * @since 6.5
  */
 final class WebSecurityFilterChainValidator implements FilterChainProxy.FilterChainValidator {
 
+	private final Log logger = LogFactory.getLog(getClass());
+
 	@Override
 	public void validate(FilterChainProxy filterChainProxy) {
 		List<SecurityFilterChain> chains = filterChainProxy.getFilterChains();
+		checkForAnyRequestRequestMatcher(chains);
+		checkForDuplicateMatchers(chains);
+		checkAuthorizationFilters(chains);
+	}
+
+	private void checkForAnyRequestRequestMatcher(List<SecurityFilterChain> chains) {
 		DefaultSecurityFilterChain anyRequestFilterChain = null;
 		for (SecurityFilterChain chain : chains) {
 			if (anyRequestFilterChain != null) {
 				String message = "A filter chain that matches any request [" + anyRequestFilterChain
 						+ "] has already been configured, which means that this filter chain [" + chain
 						+ "] will never get invoked. Please use `HttpSecurity#securityMatcher` to ensure that there is only one filter chain configured for 'any request' and that the 'any request' filter chain is published last.";
-				throw new IllegalArgumentException(message);
+				throw new UnreachableFilterChainException(message, anyRequestFilterChain, chain);
 			}
 			if (chain instanceof DefaultSecurityFilterChain defaultChain) {
 				if (defaultChain.getRequestMatcher() instanceof AnyRequestMatcher) {
@@ -49,4 +66,48 @@ public void validate(FilterChainProxy filterChainProxy) {
 		}
 	}
 
+	private void checkForDuplicateMatchers(List<SecurityFilterChain> chains) {
+		DefaultSecurityFilterChain filterChain = null;
+		for (SecurityFilterChain chain : chains) {
+			if (filterChain != null) {
+				if (chain instanceof DefaultSecurityFilterChain defaultChain) {
+					if (defaultChain.getRequestMatcher().equals(filterChain.getRequestMatcher())) {
+						throw new UnreachableFilterChainException(
+								"The FilterChainProxy contains two filter chains using the" + " matcher "
+										+ defaultChain.getRequestMatcher(),
+								filterChain, defaultChain);
+					}
+				}
+			}
+			if (chain instanceof DefaultSecurityFilterChain defaultChain) {
+				filterChain = defaultChain;
+			}
+		}
+	}
+
+	private void checkAuthorizationFilters(List<SecurityFilterChain> chains) {
+		Filter authorizationFilter = null;
+		Filter filterSecurityInterceptor = null;
+		for (SecurityFilterChain chain : chains) {
+			for (Filter filter : chain.getFilters()) {
+				if (filter instanceof AuthorizationFilter) {
+					authorizationFilter = filter;
+				}
+				if (filter instanceof FilterSecurityInterceptor) {
+					filterSecurityInterceptor = filter;
+				}
+			}
+			if (authorizationFilter != null && filterSecurityInterceptor != null) {
+				this.logger.warn(
+						"It is not recommended to use authorizeRequests in the configuration. Please only use authorizeHttpRequests");
+			}
+			if (filterSecurityInterceptor != null) {
+				this.logger.warn(
+						"Usage of authorizeRequests is deprecated. Please use authorizeHttpRequests in the configuration");
+			}
+			authorizationFilter = null;
+			filterSecurityInterceptor = null;
+		}
+	}
+
 }
diff --git a/...ringframework/security/config/annotation/web/configurers/SessionManagementConfigurer.java b/...ringframework/security/config/annotation/web/configurers/SessionManagementConfigurer.java
@@ -47,6 +47,7 @@
 import org.springframework.security.web.authentication.session.RegisterSessionAuthenticationStrategy;
 import org.springframework.security.web.authentication.session.SessionAuthenticationStrategy;
 import org.springframework.security.web.authentication.session.SessionFixationProtectionStrategy;
+import org.springframework.security.web.authentication.session.SessionLimit;
 import org.springframework.security.web.context.DelegatingSecurityContextRepository;
 import org.springframework.security.web.context.HttpSessionSecurityContextRepository;
 import org.springframework.security.web.context.NullSecurityContextRepository;
@@ -123,7 +124,7 @@ public final class SessionManagementConfigurer<H extends HttpSecurityBuilder<H>>
 
 	private SessionRegistry sessionRegistry;
 
-	private Integer maximumSessions;
+	private SessionLimit sessionLimit;
 
 	private String expiredUrl;
 
@@ -329,7 +330,7 @@ public SessionManagementConfigurer<H> sessionFixation(
 	 * @return the {@link SessionManagementConfigurer} for further customizations
 	 */
 	public ConcurrencyControlConfigurer maximumSessions(int maximumSessions) {
-		this.maximumSessions = maximumSessions;
+		this.sessionLimit = SessionLimit.of(maximumSessions);
 		this.propertiesThatRequireImplicitAuthentication.add("maximumSessions = " + maximumSessions);
 		return new ConcurrencyControlConfigurer();
 	}
@@ -570,7 +571,7 @@ private SessionAuthenticationStrategy getSessionAuthenticationStrategy(H http) {
 			SessionRegistry sessionRegistry = getSessionRegistry(http);
 			ConcurrentSessionControlAuthenticationStrategy concurrentSessionControlStrategy = new ConcurrentSessionControlAuthenticationStrategy(
 					sessionRegistry);
-			concurrentSessionControlStrategy.setMaximumSessions(this.maximumSessions);
+			concurrentSessionControlStrategy.setMaximumSessions(this.sessionLimit);
 			concurrentSessionControlStrategy.setExceptionIfMaximumExceeded(this.maxSessionsPreventsLogin);
 			concurrentSessionControlStrategy = postProcess(concurrentSessionControlStrategy);
 			RegisterSessionAuthenticationStrategy registerSessionStrategy = new RegisterSessionAuthenticationStrategy(
@@ -614,7 +615,7 @@ private void registerDelegateApplicationListener(H http, ApplicationListener<?>
 	 * @return
 	 */
 	private boolean isConcurrentSessionControlEnabled() {
-		return this.maximumSessions != null;
+		return this.sessionLimit != null;
 	}
 
 	/**
@@ -706,7 +707,19 @@ private ConcurrencyControlConfigurer() {
 		 * @return the {@link ConcurrencyControlConfigurer} for further customizations
 		 */
 		public ConcurrencyControlConfigurer maximumSessions(int maximumSessions) {
-			SessionManagementConfigurer.this.maximumSessions = maximumSessions;
+			SessionManagementConfigurer.this.sessionLimit = SessionLimit.of(maximumSessions);
+			return this;
+		}
+
+		/**
+		 * Determines the behaviour when a session limit is detected.
+		 * @param sessionLimit the {@link SessionLimit} to check the maximum number of
+		 * sessions for a user
+		 * @return the {@link ConcurrencyControlConfigurer} for further customizations
+		 * @since 6.5
+		 */
+		public ConcurrencyControlConfigurer maximumSessions(SessionLimit sessionLimit) {
+			SessionManagementConfigurer.this.sessionLimit = sessionLimit;
 			return this;
 		}
 

diff --git a/.../org/springframework/security/config/authentication/AuthenticationManagerFactoryBean.java b/.../org/springframework/security/config/authentication/AuthenticationManagerFactoryBean.java
@@ -65,8 +65,7 @@ public AuthenticationManager getObject() throws Exception {
 			if (uds == null) {
 				throw new NoSuchBeanDefinitionException(BeanIds.AUTHENTICATION_MANAGER, MISSING_BEAN_ERROR_MESSAGE);
 			}
-			DaoAuthenticationProvider provider = new DaoAuthenticationProvider();
-			provider.setUserDetailsService(uds);
+			DaoAuthenticationProvider provider = new DaoAuthenticationProvider(uds);
 			PasswordEncoder passwordEncoder = this.bf.getBeanProvider(PasswordEncoder.class).getIfUnique();
 			if (passwordEncoder != null) {
 				provider.setPasswordEncoder(passwordEncoder);

diff --git a/...g/src/main/java/org/springframework/security/config/http/DefaultFilterChainValidator.java b/...g/src/main/java/org/springframework/security/config/http/DefaultFilterChainValidator.java
@@ -39,6 +39,7 @@
 import org.springframework.security.web.FilterChainProxy;
 import org.springframework.security.web.FilterInvocation;
 import org.springframework.security.web.SecurityFilterChain;
+import org.springframework.security.web.UnreachableFilterChainException;
 import org.springframework.security.web.access.ExceptionTranslationFilter;
 import org.springframework.security.web.access.intercept.AuthorizationFilter;
 import org.springframework.security.web.access.intercept.FilterInvocationSecurityMetadataSource;
@@ -53,7 +54,6 @@
 import org.springframework.security.web.servletapi.SecurityContextHolderAwareRequestFilter;
 import org.springframework.security.web.session.SessionManagementFilter;
 import org.springframework.security.web.util.matcher.AnyRequestMatcher;
-import org.springframework.security.web.util.matcher.RequestMatcher;
 
 public class DefaultFilterChainValidator implements FilterChainProxy.FilterChainValidator {
 
@@ -69,31 +69,67 @@ public void validate(FilterChainProxy fcp) {
 		}
 		checkPathOrder(new ArrayList<>(fcp.getFilterChains()));
 		checkForDuplicateMatchers(new ArrayList<>(fcp.getFilterChains()));
+		checkAuthorizationFilters(new ArrayList<>(fcp.getFilterChains()));
 	}
 
 	private void checkPathOrder(List<SecurityFilterChain> filterChains) {
 		// Check that the universal pattern is listed at the end, if at all
 		Iterator<SecurityFilterChain> chains = filterChains.iterator();
 		while (chains.hasNext()) {
-			RequestMatcher matcher = ((DefaultSecurityFilterChain) chains.next()).getRequestMatcher();
-			if (AnyRequestMatcher.INSTANCE.equals(matcher) && chains.hasNext()) {
-				throw new IllegalArgumentException("A universal match pattern ('/**') is defined "
-						+ " before other patterns in the filter chain, causing them to be ignored. Please check the "
-						+ "ordering in your <security:http> namespace or FilterChainProxy bean configuration");
+			if (chains.next() instanceof DefaultSecurityFilterChain securityFilterChain) {
+				if (AnyRequestMatcher.INSTANCE.equals(securityFilterChain.getRequestMatcher()) && chains.hasNext()) {
+					throw new UnreachableFilterChainException("A universal match pattern ('/**') is defined "
+							+ " before other patterns in the filter chain, causing them to be ignored. Please check the "
+							+ "ordering in your <security:http> namespace or FilterChainProxy bean configuration",
+							securityFilterChain, chains.next());
+				}
 			}
 		}
 	}
 
 	private void checkForDuplicateMatchers(List<SecurityFilterChain> chains) {
-		while (chains.size() > 1) {
-			DefaultSecurityFilterChain chain = (DefaultSecurityFilterChain) chains.remove(0);
-			for (SecurityFilterChain test : chains) {
-				if (chain.getRequestMatcher().equals(((DefaultSecurityFilterChain) test).getRequestMatcher())) {
-					throw new IllegalArgumentException("The FilterChainProxy contains two filter chains using the"
-							+ " matcher " + chain.getRequestMatcher() + ". If you are using multiple <http> namespace "
-							+ "elements, you must use a 'pattern' attribute to define the request patterns to which they apply.");
+		DefaultSecurityFilterChain filterChain = null;
+		for (SecurityFilterChain chain : chains) {
+			if (filterChain != null) {
+				if (chain instanceof DefaultSecurityFilterChain defaultChain) {
+					if (defaultChain.getRequestMatcher().equals(filterChain.getRequestMatcher())) {
+						throw new UnreachableFilterChainException(
+								"The FilterChainProxy contains two filter chains using the" + " matcher "
+										+ defaultChain.getRequestMatcher()
+										+ ". If you are using multiple <http> namespace "
+										+ "elements, you must use a 'pattern' attribute to define the request patterns to which they apply.",
+								defaultChain, chain);
+					}
 				}
 			}
+			if (chain instanceof DefaultSecurityFilterChain defaultChain) {
+				filterChain = defaultChain;
+			}
+		}
+	}
+
+	private void checkAuthorizationFilters(List<SecurityFilterChain> chains) {
+		Filter authorizationFilter = null;
+		Filter filterSecurityInterceptor = null;
+		for (SecurityFilterChain chain : chains) {
+			for (Filter filter : chain.getFilters()) {
+				if (filter instanceof AuthorizationFilter) {
+					authorizationFilter = filter;
+				}
+				if (filter instanceof FilterSecurityInterceptor) {
+					filterSecurityInterceptor = filter;
+				}
+			}
+			if (authorizationFilter != null && filterSecurityInterceptor != null) {
+				this.logger.warn(
+						"It is not recommended to use authorizeRequests in the configuration. Please only use authorizeHttpRequests");
+			}
+			if (filterSecurityInterceptor != null) {
+				this.logger.warn(
+						"Usage of authorizeRequests is deprecated. Please use authorizeHttpRequests in the configuration");
+			}
+			authorizationFilter = null;
+			filterSecurityInterceptor = null;
 		}
 	}
 

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
@@ -122,6 +122,10 @@ class HttpConfigurationBuilder {
 
 	private static final String ATT_SESSION_AUTH_STRATEGY_REF = "session-authentication-strategy-ref";
 
+	private static final String ATT_MAX_SESSIONS_REF = "max-sessions-ref";
+
+	private static final String ATT_MAX_SESSIONS = "max-sessions";
+
 	private static final String ATT_SESSION_AUTH_ERROR_URL = "session-authentication-error-url";
 
 	private static final String ATT_SECURITY_CONTEXT_HOLDER_STRATEGY = "security-context-holder-strategy-ref";
@@ -485,10 +489,16 @@ else if (StringUtils.hasText(sessionAuthStratRef)) {
 			concurrentSessionStrategy.addConstructorArgValue(this.sessionRegistryRef);
 			String maxSessions = this.pc.getReaderContext()
 				.getEnvironment()
-				.resolvePlaceholders(sessionCtrlElt.getAttribute("max-sessions"));
+				.resolvePlaceholders(sessionCtrlElt.getAttribute(ATT_MAX_SESSIONS));
 			if (StringUtils.hasText(maxSessions)) {
 				concurrentSessionStrategy.addPropertyValue("maximumSessions", maxSessions);
 			}
+			String maxSessionsRef = this.pc.getReaderContext()
+				.getEnvironment()
+				.resolvePlaceholders(sessionCtrlElt.getAttribute(ATT_MAX_SESSIONS_REF));
+			if (StringUtils.hasText(maxSessionsRef)) {
+				concurrentSessionStrategy.addPropertyReference("maximumSessions", maxSessionsRef);
+			}
 			String exceptionIfMaximumExceeded = sessionCtrlElt.getAttribute("error-if-maximum-exceeded");
 			if (StringUtils.hasText(exceptionIfMaximumExceeded)) {
 				concurrentSessionStrategy.addPropertyValue("exceptionIfMaximumExceeded", exceptionIfMaximumExceeded);
@@ -591,6 +601,12 @@ private void createConcurrencyControlFilterAndSessionRegistry(Element element) {
 				.error("Cannot use 'expired-url' attribute and 'expired-session-strategy-ref'" + " attribute together.",
 						source);
 		}
+		String maxSessions = element.getAttribute(ATT_MAX_SESSIONS);
+		String maxSessionsRef = element.getAttribute(ATT_MAX_SESSIONS_REF);
+		if (StringUtils.hasText(maxSessions) && StringUtils.hasText(maxSessionsRef)) {
+			this.pc.getReaderContext()
+				.error("Cannot use 'max-sessions' attribute and 'max-sessions-ref' attribute together.", source);
+		}
 		if (StringUtils.hasText(expiryUrl)) {
 			BeanDefinitionBuilder expiredSessionBldr = BeanDefinitionBuilder
 				.rootBeanDefinition(SimpleRedirectSessionInformationExpiredStrategy.class);

diff --git a/...c/main/java/org/springframework/security/config/http/Saml2LogoutBeanDefinitionParser.java b/...c/main/java/org/springframework/security/config/http/Saml2LogoutBeanDefinitionParser.java
@@ -146,6 +146,7 @@ public BeanDefinition parse(Element element, ParserContext pc) {
 		BeanMetadataElement saml2LogoutRequestSuccessHandler = BeanDefinitionBuilder
 			.rootBeanDefinition(Saml2RelyingPartyInitiatedLogoutSuccessHandler.class)
 			.addConstructorArgValue(logoutRequestResolver)
+			.addPropertyValue("logoutRequestRepository", logoutRequestRepository)
 			.getBeanDefinition();
 		this.logoutFilter = BeanDefinitionBuilder.rootBeanDefinition(LogoutFilter.class)
 			.addConstructorArgValue(saml2LogoutRequestSuccessHandler)