WebFlux security should not overwrite the default entry point with a delegating entry point

[foo4u]

Describe the bug

When ServerHttpSecurity.build() creates a SecurityWebFilterChain, it replaces the default entry point with the last delegating entry point in this.defaultEntryPoints.

spring-security/config/src/main/java/org/springframework/security/config/web/server/ServerHttpSecurity.java

Lines 1434 to 1445 in 3cba4ec

	private ServerAuthenticationEntryPoint getAuthenticationEntryPoint() {
	if (this.authenticationEntryPoint != null || this.defaultEntryPoints.isEmpty()) {
	return this.authenticationEntryPoint;
	}
	if (this.defaultEntryPoints.size() == 1) {
	return this.defaultEntryPoints.get(0).getEntryPoint();
	}
	DelegatingServerAuthenticationEntryPoint result = new DelegatingServerAuthenticationEntryPoint(
	this.defaultEntryPoints);
	result.setDefaultEntryPoint(this.defaultEntryPoints.get(this.defaultEntryPoints.size() - 1).getEntryPoint());
	return result;
	}

Specifically:

result.setDefaultEntryPoint(this.defaultEntryPoints.get(this.defaultEntryPoints.size() - 1).getEntryPoint());

Since delegating entry points should be applied conditionally, this breaks the contract for certain default entry points, such as OAuth2LoginSpec.setDefaultEntryponits, which are designed to be conditional. For example, the OAuth2LoginSpec is designed to not redirect on XHR requests but this behavior breaks that contract.

This cannot be worked around by configuring a new default entry point (e.g., .exceptionHandling().authenticationEntryPoint(...)) because doing so triggers another bug, which I'll open another issue about.

To Reproduce

The issue is reproducible most easily with OAuth2 login. However, the problem is not unique to this scenario.

1. Configure Spring WebFlux security with OAuth2 login

@Configuration
@EnableWebFluxSecurity
public class SecurityConfig
{
    @Bean
    public SecurityWebFilterChain securityWebFilterChain(ServerHttpSecurity http)
    {
        return http
                .authorizeExchange(exchanges -> exchanges.anyExchange().authenticated())
                .oauth2Login()
                .and()
                .build();
    }
}

https://github.com/foo4u/spring-security-bugs/blob/38ba4cd007eb3a7d75f5912f5a013b7ee4ac197d/src/main/java/com/example/demo/SecurityConfig.java#L9-L22

2. Make an XHR request to an endpoint requiring authentication with XHR headers expecting an HTTP 401.

	@Test
	void respondsWithHttp401()
	{
		client
				.get()
				.accept(MediaType.APPLICATION_JSON)
				.header("X-Requested-With","XMLHttpRequest")
				.exchange()
				.expectStatus()
				.isUnauthorized();
	}

https://github.com/foo4u/spring-security-bugs/blob/38ba4cd007eb3a7d75f5912f5a013b7ee4ac197d/src/test/java/com/example/demo/DemoApplicationTests.java#L27-L37

3. Note a 302 redirect to the login provider is sent as the response instead of a 401.

   [ERROR] Failures:
   [ERROR] DemoApplicationTests.respondsWithHttp401:36 Status expected:<401 UNAUTHORIZED> but was:<302 FOUND>

Expected behavior

The web filter chain should return an HTTP 401, not redirect.

Sample

GitHub repository with minimal, reproducible sample.

Two ways to reproduce with sample:

1. Run the test suite
2. Start the server and cURL any endpoint

[pszemus]

I took me 2 days to track this problem and finally find this issue, so +1 from me.

My workaround is to create a custom ServerAuthenticationEntryPoint and redirect all requests to /oauth2/authorization/cas except from requests to /api/**:

	public class RedirectOrFailAuthenticationEntryPoint implements ServerAuthenticationEntryPoint {

		private final ServerWebExchangeMatcher failMatcher;

		private final RedirectServerAuthenticationEntryPoint redirectEntryPoint;
		private final HttpStatusServerEntryPoint httpStatusEntryPoint;

		RedirectOrFailAuthenticationEntryPoint(final String redirectionUri, final String failurePath) {
			this.failMatcher = ServerWebExchangeMatchers.pathMatchers(failurePath);
			redirectEntryPoint = new RedirectServerAuthenticationEntryPoint(redirectionUri);
			httpStatusEntryPoint = new HttpStatusServerEntryPoint(HttpStatus.UNAUTHORIZED);
		}

		@Override
		public Mono<Void> commence(ServerWebExchange exchange, AuthenticationException e) {
			return failMatcher.matches(exchange)
				.map(result -> result.isMatch() ? httpStatusEntryPoint : redirectEntryPoint)
				.flatMap(entryPoint -> entryPoint.commence(exchange, e));
		}

	}

[jgrandja]

@foo4u The test you provided respondsWithHttp401() is not the correct behaviour.

When oauth2Login() is configured, unauthenticated XHR requests will be redirected to the default login page /login. See this test for the expected behaviour.

NOTE: To be clear, the redirect is to the default login page in the client application NOT the login page of the external provider.

Can you provide another test that demonstrates if this is a bug?

[foo4u]

@jgrandja why would you intentionally redirect XHR requests to the login page? Also, the test case you mentioned doing a 3XX redirect only "works" as a side effect of this bug.

The code in OAuth2LoginSpec specifically attempts to not apply these handlers in the case of an XHR request:

spring-security/config/src/main/java/org/springframework/security/config/web/server/ServerHttpSecurity.java

Lines 3274 to 3312 in 8cefc8a

	private void setDefaultEntryPoints(ServerHttpSecurity http) {
	String defaultLoginPage = "/login";
	Map<String, String> urlToText = http.oauth2Login.getLinks();
	String providerLoginPage = null;
	if (urlToText.size() == 1) {
	providerLoginPage = urlToText.keySet().iterator().next();
	}
	MediaTypeServerWebExchangeMatcher htmlMatcher = new MediaTypeServerWebExchangeMatcher(
	MediaType.APPLICATION_XHTML_XML, new MediaType("image", "*"), MediaType.TEXT_HTML,
	MediaType.TEXT_PLAIN);
	htmlMatcher.setIgnoredMediaTypes(Collections.singleton(MediaType.ALL));
	ServerWebExchangeMatcher xhrMatcher = (exchange) -> {
	if (exchange.getRequest().getHeaders().getOrEmpty("X-Requested-With").contains("XMLHttpRequest")) {
	return ServerWebExchangeMatcher.MatchResult.match();
	}
	return ServerWebExchangeMatcher.MatchResult.notMatch();
	};
	ServerWebExchangeMatcher notXhrMatcher = new NegatedServerWebExchangeMatcher(xhrMatcher);
	ServerWebExchangeMatcher defaultEntryPointMatcher = new AndServerWebExchangeMatcher(notXhrMatcher,
	htmlMatcher);
	if (providerLoginPage != null) {
	ServerWebExchangeMatcher loginPageMatcher = new PathPatternParserServerWebExchangeMatcher(
	defaultLoginPage);
	ServerWebExchangeMatcher faviconMatcher = new PathPatternParserServerWebExchangeMatcher("/favicon.ico");
	ServerWebExchangeMatcher defaultLoginPageMatcher = new AndServerWebExchangeMatcher(
	new OrServerWebExchangeMatcher(loginPageMatcher, faviconMatcher), defaultEntryPointMatcher);
	ServerWebExchangeMatcher matcher = new AndServerWebExchangeMatcher(notXhrMatcher,
	new NegatedServerWebExchangeMatcher(defaultLoginPageMatcher));
	RedirectServerAuthenticationEntryPoint entryPoint = new RedirectServerAuthenticationEntryPoint(
	providerLoginPage);
	entryPoint.setRequestCache(http.requestCache.requestCache);
	http.defaultEntryPoints.add(new DelegateEntry(matcher, entryPoint));
	}
	RedirectServerAuthenticationEntryPoint defaultEntryPoint = new RedirectServerAuthenticationEntryPoint(
	defaultLoginPage);
	defaultEntryPoint.setRequestCache(http.requestCache.requestCache);
	http.defaultEntryPoints.add(new DelegateEntry(defaultEntryPointMatcher, defaultEntryPoint));
	}

Can you provide another test that demonstrates if this is a bug?

It is a bug, that test case should be expecting a 401, not a 3XX.

[jgrandja]

@foo4u I looked into this further and my comment still holds.

To further support the expected behaviour in OAuth2LoginSpec.setDefaultEntryPoints(), these tests pass:

@Test
void xhrRequestThenRedirectToDefaultLogin()
{
	client
			.get()
			.accept(MediaType.APPLICATION_JSON)
			.header("X-Requested-With","XMLHttpRequest")
			.exchange()
			.expectStatus()
			.is3xxRedirection()
			.expectHeader().location("/login");
	// NOTE:
	// It does not trigger the authorization request redirect -> '/oauth2/authorization/smartling'
	// This is the expected behaviour configured in setDefaultEntryPoints()
}

@Test
void notXhrRequestThenRedirectToProviderLogin()
{
	client
			.get()
			.exchange()
			.expectStatus()
			.is3xxRedirection()
			.expectHeader().location("/oauth2/authorization/smartling");
}

Does this make sense? If not, please provide additional tests to demonstrate because respondsWithHttp401() is not the expected behaviour.