Allow SAML 2.0 loginProcessingURL without registrationId

Closes gh-10176
spring-projects · Oct 5, 2021 · 816e847 · 816e847
1 parent 1f919bc
commit 816e847
Show file tree

Hide file tree

Showing 4 changed files with 125 additions and 19 deletions.
diff --git a/...pringframework/security/config/annotation/web/configurers/saml2/Saml2LoginConfigurer.java b/...pringframework/security/config/annotation/web/configurers/saml2/Saml2LoginConfigurer.java
@@ -172,10 +172,19 @@ public Saml2LoginConfigurer<B> loginPage(String loginPage) {
 		return this;
 	}
 
+	/**
+	 * Specifies the URL to validate the credentials. If specified a custom URL, consider
+	 * specifying a custom {@link AuthenticationConverter} via
+	 * {@link #authenticationConverter(AuthenticationConverter)}, since the default
+	 * {@link AuthenticationConverter} implementation relies on the
+	 * <code>{registrationId}</code> path variable to be present in the URL
+	 * @param loginProcessingUrl the URL to validate the credentials
+	 * @return the {@link Saml2LoginConfigurer} for additional customization
+	 * @see Saml2WebSsoAuthenticationFilter#DEFAULT_FILTER_PROCESSES_URI
+	 */
 	@Override
 	public Saml2LoginConfigurer<B> loginProcessingUrl(String loginProcessingUrl) {
 		Assert.hasText(loginProcessingUrl, "loginProcessingUrl cannot be empty");
-		Assert.state(loginProcessingUrl.contains("{registrationId}"), "{registrationId} path variable is required");
 		this.loginProcessingUrl = loginProcessingUrl;
 		return this;
 	}
@@ -254,6 +263,8 @@ public void configure(B http) throws Exception {
 
 	private AuthenticationConverter getAuthenticationConverter(B http) {
 		if (this.authenticationConverter == null) {
+			Assert.state(this.loginProcessingUrl.contains("{registrationId}"),
+					"loginProcessingUrl must contain {registrationId} path variable");
 			return new Saml2AuthenticationTokenConverter(
 					new DefaultRelyingPartyRegistrationResolver(this.relyingPartyRegistrationRepository));
 		}

diff --git a/...framework/security/config/annotation/web/configurers/saml2/Saml2LoginConfigurerTests.java b/...framework/security/config/annotation/web/configurers/saml2/Saml2LoginConfigurerTests.java
@@ -37,6 +37,7 @@
 import org.opensaml.saml.saml2.core.Assertion;
 import org.opensaml.saml.saml2.core.AuthnRequest;
 
+import org.springframework.beans.factory.BeanCreationException;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.context.ConfigurableApplicationContext;
 import org.springframework.context.annotation.Bean;
@@ -49,6 +50,7 @@
 import org.springframework.security.authentication.AuthenticationProvider;
 import org.springframework.security.authentication.AuthenticationServiceException;
 import org.springframework.security.authentication.ProviderManager;
+import org.springframework.security.config.Customizer;
 import org.springframework.security.config.annotation.ObjectPostProcessor;
 import org.springframework.security.config.annotation.web.builders.HttpSecurity;
 import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity;
@@ -77,7 +79,9 @@
 import org.springframework.security.saml2.provider.service.registration.TestRelyingPartyRegistrations;
 import org.springframework.security.saml2.provider.service.servlet.filter.Saml2WebSsoAuthenticationFilter;
 import org.springframework.security.saml2.provider.service.web.Saml2AuthenticationRequestContextResolver;
+import org.springframework.security.saml2.provider.service.web.Saml2AuthenticationTokenConverter;
 import org.springframework.security.web.FilterChainProxy;
+import org.springframework.security.web.SecurityFilterChain;
 import org.springframework.security.web.authentication.AuthenticationConverter;
 import org.springframework.security.web.authentication.AuthenticationFailureHandler;
 import org.springframework.security.web.context.HttpRequestResponseHolder;
@@ -91,6 +95,7 @@
 import org.springframework.web.util.UriComponentsBuilder;
 
 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.ArgumentMatchers.anyString;
 import static org.mockito.BDDMockito.given;
@@ -117,6 +122,8 @@ public class Saml2LoginConfigurerTests {
 
 	private static final String SIGNED_RESPONSE = "PD94bWwgdmVyc2lvbj0iMS4wIiBlbmNvZGluZz0iVVRGLTgiPz48c2FtbDJwOlJlc3BvbnNlIHhtbG5zOnNhbWwycD0idXJuOm9hc2lzOm5hbWVzOnRjOlNBTUw6Mi4wOnByb3RvY29sIiBEZXN0aW5hdGlvbj0iaHR0cHM6Ly9ycC5leGFtcGxlLm9yZy9hY3MiIElEPSJfYzE3MzM2YTAtNTM1My00MTQ5LWI3MmMtMDNkOWY5YWYzMDdlIiBJc3N1ZUluc3RhbnQ9IjIwMjAtMDgtMDRUMjI6MDQ6NDUuMDE2WiIgVmVyc2lvbj0iMi4wIj48c2FtbDI6SXNzdWVyIHhtbG5zOnNhbWwyPSJ1cm46b2FzaXM6bmFtZXM6dGM6U0FNTDoyLjA6YXNzZXJ0aW9uIj5hcC1lbnRpdHktaWQ8L3NhbWwyOklzc3Vlcj48ZHM6U2lnbmF0dXJlIHhtbG5zOmRzPSJodHRwOi8vd3d3LnczLm9yZy8yMDAwLzA5L3htbGRzaWcjIj4KPGRzOlNpZ25lZEluZm8+CjxkczpDYW5vbmljYWxpemF0aW9uTWV0aG9kIEFsZ29yaXRobT0iaHR0cDovL3d3dy53My5vcmcvMjAwMS8xMC94bWwtZXhjLWMxNG4jIi8+CjxkczpTaWduYXR1cmVNZXRob2QgQWxnb3JpdGhtPSJodHRwOi8vd3d3LnczLm9yZy8yMDAxLzA0L3htbGRzaWctbW9yZSNyc2Etc2hhMjU2Ii8+CjxkczpSZWZlcmVuY2UgVVJJPSIjX2MxNzMzNmEwLTUzNTMtNDE0OS1iNzJjLTAzZDlmOWFmMzA3ZSI+CjxkczpUcmFuc2Zvcm1zPgo8ZHM6VHJhbnNmb3JtIEFsZ29yaXRobT0iaHR0cDovL3d3dy53My5vcmcvMjAwMC8wOS94bWxkc2lnI2VudmVsb3BlZC1zaWduYXR1cmUiLz4KPGRzOlRyYW5zZm9ybSBBbGdvcml0aG09Imh0dHA6Ly93d3cudzMub3JnLzIwMDEvMTAveG1sLWV4Yy1jMTRuIyIvPgo8L2RzOlRyYW5zZm9ybXM+CjxkczpEaWdlc3RNZXRob2QgQWxnb3JpdGhtPSJodHRwOi8vd3d3LnczLm9yZy8yMDAxLzA0L3htbGVuYyNzaGEyNTYiLz4KPGRzOkRpZ2VzdFZhbHVlPjYzTmlyenFzaDVVa0h1a3NuRWUrM0hWWU5aYWFsQW1OQXFMc1lGMlRuRDA9PC9kczpEaWdlc3RWYWx1ZT4KPC9kczpSZWZlcmVuY2U+CjwvZHM6U2lnbmVkSW5mbz4KPGRzOlNpZ25hdHVyZVZhbHVlPgpLMVlvWWJVUjBTclY4RTdVMkhxTTIvZUNTOTNoV25mOExnNnozeGZWMUlyalgzSXhWYkNvMVlYcnRBSGRwRVdvYTJKKzVOMmFNbFBHJiMxMzsKN2VpbDBZRC9xdUVRamRYbTNwQTBjZmEvY25pa2RuKzVhbnM0ZWQwanU1amo2dkpvZ2w2Smt4Q25LWUpwTU9HNzhtampmb0phengrWCYjMTM7CkM2NktQVStBYUdxeGVwUEQ1ZlhRdTFKSy9Jb3lBaitaa3k4Z2Jwc3VyZHFCSEJLRWxjdnVOWS92UGY0OGtBeFZBKzdtRGhNNUMvL1AmIzEzOwp0L084Y3NZYXB2UjZjdjZrdk45QXZ1N3FRdm9qVk1McHVxZWNJZDJwTUVYb0NSSnE2Nkd4MStNTUVPeHVpMWZZQlRoMEhhYjRmK3JyJiMxMzsKOEY2V1NFRC8xZllVeHliRkJqZ1Q4d2lEWHFBRU8wSVY4ZWRQeEE9PQo8L2RzOlNpZ25hdHVyZVZhbHVlPgo8L2RzOlNpZ25hdHVyZT48c2FtbDI6QXNzZXJ0aW9uIHhtbG5zOnNhbWwyPSJ1cm46b2FzaXM6bmFtZXM6dGM6U0FNTDoyLjA6YXNzZXJ0aW9uIiBJRD0iQWUzZjQ5OGI4LTliMTctNDA3OC05ZDM1LTg2YTA4NDA4NDk5NSIgSXNzdWVJbnN0YW50PSIyMDIwLTA4LTA0VDIyOjA0OjQ1LjA3N1oiIFZlcnNpb249IjIuMCI+PHNhbWwyOklzc3Vlcj5hcC1lbnRpdHktaWQ8L3NhbWwyOklzc3Vlcj48c2FtbDI6U3ViamVjdD48c2FtbDI6TmFtZUlEPnRlc3RAc2FtbC51c2VyPC9zYW1sMjpOYW1lSUQ+PHNhbWwyOlN1YmplY3RDb25maXJtYXRpb24gTWV0aG9kPSJ1cm46b2FzaXM6bmFtZXM6dGM6U0FNTDoyLjA6Y206YmVhcmVyIj48c2FtbDI6U3ViamVjdENvbmZpcm1hdGlvbkRhdGEgTm90QmVmb3JlPSIyMDIwLTA4LTA0VDIxOjU5OjQ1LjA5MFoiIE5vdE9uT3JBZnRlcj0iMjA0MC0wNy0zMFQyMjowNTowNi4wODhaIiBSZWNpcGllbnQ9Imh0dHBzOi8vcnAuZXhhbXBsZS5vcmcvYWNzIi8+PC9zYW1sMjpTdWJqZWN0Q29uZmlybWF0aW9uPjwvc2FtbDI6U3ViamVjdD48c2FtbDI6Q29uZGl0aW9ucyBOb3RCZWZvcmU9IjIwMjAtMDgtMDRUMjE6NTk6NDUuMDgwWiIgTm90T25PckFmdGVyPSIyMDQwLTA3LTMwVDIyOjA1OjA2LjA4N1oiLz48L3NhbWwyOkFzc2VydGlvbj48L3NhbWwycDpSZXNwb25zZT4=";
 
+	private static final AuthenticationConverter AUTHENTICATION_CONVERTER = mock(AuthenticationConverter.class);
+
 	@Autowired
 	private ConfigurableApplicationContext context;
 
@@ -230,6 +237,33 @@ public void authenticateWithInvalidDeflatedSAMLResponseThenFailureHandlerUses()
 		assertThat(exception.getCause()).isInstanceOf(IOException.class);
 	}
 
+	@Test
+	public void saml2LoginWhenLoginProcessingUrlWithoutRegistrationIdAndDefaultAuthenticationConverterThenValidates() {
+		assertThatExceptionOfType(BeanCreationException.class)
+				.isThrownBy(() -> this.spring.register(CustomLoginProcessingUrlDefaultAuthenticationConverter.class)
+						.autowire())
+				.havingRootCause().isInstanceOf(IllegalStateException.class)
+				.withMessage("loginProcessingUrl must contain {registrationId} path variable");
+	}
+
+	@Test
+	public void authenticateWhenCustomLoginProcessingUrlAndCustomAuthenticationConverterThenAuthenticate()
+			throws Exception {
+		this.spring.register(CustomLoginProcessingUrlCustomAuthenticationConverter.class).autowire();
+		RelyingPartyRegistration relyingPartyRegistration = TestRelyingPartyRegistrations.noCredentials()
+				.assertingPartyDetails((party) -> party.verificationX509Credentials(
+						(c) -> c.add(TestSaml2X509Credentials.relyingPartyVerifyingCredential())))
+				.build();
+		String response = new String(Saml2Utils.samlDecode(SIGNED_RESPONSE));
+		given(AUTHENTICATION_CONVERTER.convert(any(HttpServletRequest.class)))
+				.willReturn(new Saml2AuthenticationToken(relyingPartyRegistration, response));
+		// @formatter:off
+		MockHttpServletRequestBuilder request = post("/my/custom/url").param("SAMLResponse", SIGNED_RESPONSE);
+		// @formatter:on
+		this.mvc.perform(request).andExpect(redirectedUrl("/"));
+		verify(AUTHENTICATION_CONVERTER).convert(any(HttpServletRequest.class));
+	}
+
 	private void validateSaml2WebSsoAuthenticationFilterConfiguration() {
 		// get the OpenSamlAuthenticationProvider
 		Saml2WebSsoAuthenticationFilter filter = getSaml2SsoFilter(this.springSecurityFilterChain);
@@ -337,10 +371,10 @@ static class CustomAuthenticationRequestContextResolver extends WebSecurityConfi
 		protected void configure(HttpSecurity http) throws Exception {
 			// @formatter:off
 			http
-				.authorizeRequests((authz) -> authz
-						.anyRequest().authenticated()
-				)
-				.saml2Login(withDefaults());
+					.authorizeRequests((authz) -> authz
+							.anyRequest().authenticated()
+					)
+					.saml2Login(withDefaults());
 			// @formatter:on
 		}
 
@@ -359,11 +393,11 @@ static class CustomAuthenticationRequestContextConverterResolver extends WebSecu
 		protected void configure(HttpSecurity http) throws Exception {
 			// @formatter:off
 			http
-				.authorizeRequests((authz) -> authz
-					.anyRequest().authenticated()
-				)
-				.saml2Login((saml2) -> {
-				});
+					.authorizeRequests((authz) -> authz
+							.anyRequest().authenticated()
+					)
+					.saml2Login((saml2) -> {
+					});
 			// @formatter:on
 		}
 
@@ -395,6 +429,62 @@ protected void configure(HttpSecurity http) throws Exception {
 
 	}
 
+	@EnableWebSecurity
+	@Import(Saml2LoginConfigBeans.class)
+	static class CustomAuthenticationConverterBean {
+
+		private final Saml2AuthenticationTokenConverter authenticationConverter = mock(
+				Saml2AuthenticationTokenConverter.class);
+
+		@Bean
+		SecurityFilterChain app(HttpSecurity http) throws Exception {
+			http.authorizeHttpRequests((authz) -> authz.anyRequest().authenticated())
+					.saml2Login(Customizer.withDefaults());
+			return http.build();
+		}
+
+		@Bean
+		Saml2AuthenticationTokenConverter authenticationConverter() {
+			return this.authenticationConverter;
+		}
+
+	}
+
+	@EnableWebSecurity
+	@Import(Saml2LoginConfigBeans.class)
+	static class CustomLoginProcessingUrlDefaultAuthenticationConverter {
+
+		@Bean
+		SecurityFilterChain securityFilterChain(HttpSecurity http) throws Exception {
+			// @formatter:off
+			http
+					.authorizeRequests((authz) -> authz.anyRequest().authenticated())
+					.saml2Login((saml2) -> saml2.loginProcessingUrl("/my/custom/url"));
+			// @formatter:on
+			return http.build();
+		}
+
+	}
+
+	@EnableWebSecurity
+	@Import(Saml2LoginConfigBeans.class)
+	static class CustomLoginProcessingUrlCustomAuthenticationConverter {
+
+		@Bean
+		SecurityFilterChain securityFilterChain(HttpSecurity http) throws Exception {
+			// @formatter:off
+			http
+					.authorizeRequests((authz) -> authz.anyRequest().authenticated())
+					.saml2Login((saml2) -> saml2
+							.loginProcessingUrl("/my/custom/url")
+							.authenticationConverter(AUTHENTICATION_CONVERTER)
+					);
+			// @formatter:on
+			return http.build();
+		}
+
+	}
+
 	static class Saml2LoginConfigBeans {
 
 		@Bean

diff --git a/...ework/security/saml2/provider/service/servlet/filter/Saml2WebSsoAuthenticationFilter.java b/...ework/security/saml2/provider/service/servlet/filter/Saml2WebSsoAuthenticationFilter.java
@@ -63,23 +63,21 @@ public Saml2WebSsoAuthenticationFilter(RelyingPartyRegistrationRepository relyin
 			String filterProcessesUrl) {
 		this(new Saml2AuthenticationTokenConverter(
 				new DefaultRelyingPartyRegistrationResolver(relyingPartyRegistrationRepository)), filterProcessesUrl);
+		Assert.isTrue(filterProcessesUrl.contains("{registrationId}"),
+				"filterProcessesUrl must contain a {registrationId} match variable");
 	}
 
 	/**
 	 * Creates a {@link Saml2WebSsoAuthenticationFilter} given the provided parameters
 	 * @param authenticationConverter the strategy for converting an
 	 * {@link HttpServletRequest} into an {@link Authentication}
-	 * @param filterProcessingUrl the processing URL, must contain a {registrationId}
-	 * variable
+	 * @param filterProcessesUrl the processing URL
 	 * @since 5.4
 	 */
-	public Saml2WebSsoAuthenticationFilter(AuthenticationConverter authenticationConverter,
-			String filterProcessingUrl) {
-		super(filterProcessingUrl);
+	public Saml2WebSsoAuthenticationFilter(AuthenticationConverter authenticationConverter, String filterProcessesUrl) {
+		super(filterProcessesUrl);
 		Assert.notNull(authenticationConverter, "authenticationConverter cannot be null");
-		Assert.hasText(filterProcessingUrl, "filterProcessesUrl must contain a URL pattern");
-		Assert.isTrue(filterProcessingUrl.contains("{registrationId}"),
-				"filterProcessesUrl must contain a {registrationId} match variable");
+		Assert.hasText(filterProcessesUrl, "filterProcessesUrl must contain a URL pattern");
 		this.authenticationConverter = authenticationConverter;
 		setAllowSessionCreation(true);
 		setSessionAuthenticationStrategy(new ChangeSessionIdAuthenticationStrategy());

diff --git a/.../security/saml2/provider/service/servlet/filter/Saml2WebSsoAuthenticationFilterTests.java b/.../security/saml2/provider/service/servlet/filter/Saml2WebSsoAuthenticationFilterTests.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2020 the original author or authors.
+ * Copyright 2002-2021 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.
@@ -26,6 +26,7 @@
 import org.springframework.mock.web.MockHttpServletResponse;
 import org.springframework.security.saml2.provider.service.authentication.Saml2AuthenticationException;
 import org.springframework.security.saml2.provider.service.registration.RelyingPartyRegistrationRepository;
+import org.springframework.security.web.authentication.AuthenticationConverter;
 
 import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
 import static org.mockito.BDDMockito.given;
@@ -60,6 +61,12 @@ public void constructingFilterWithValidRegistrationIdVariableThenSucceeds() {
 		this.filter = new Saml2WebSsoAuthenticationFilter(this.repository, "/url/variable/is/present/{registrationId}");
 	}
 
+	@Test
+	public void constructingFilterWithMissingRegistrationIdVariableAndCustomAuthenticationConverterThenSucceeds() {
+		AuthenticationConverter authenticationConverter = mock(AuthenticationConverter.class);
+		this.filter = new Saml2WebSsoAuthenticationFilter(authenticationConverter, "/url/missing/variable");
+	}
+
 	@Test
 	public void requiresAuthenticationWhenHappyPathThenReturnsTrue() {
 		Assert.assertTrue(this.filter.requiresAuthentication(this.request, this.response));