From 617aaeb7aa35e07948ad4cc075c86998b0b2033d Mon Sep 17 00:00:00 2001
From: David Byron <82477955+dbyron-sf@users.noreply.github.com>
Date: Fri, 9 Aug 2024 21:21:22 -0700
Subject: [PATCH] fix(core): remove ErrorPageSecurityFilter bean named
errorPageSecurityFilter (#1819)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
* test(web): demonstrate bug in MultiAutoSupport
where handling of certain error responses generates html:
HTTP Status 400 – Bad Request
HTTP Status 400 – Bad Request
instead of json, and the following exception in the logs:
java.lang.UnsupportedOperationException: public abstract int javax.servlet.ServletRequest.getLocalPort() is not supported
at org.springframework.security.web.FilterInvocation$UnsupportedOperationExceptionInvocationHandler.invoke(FilterInvocation.java:326)
at jdk.proxy2/jdk.proxy2.$Proxy256.getLocalPort(Unknown Source)
at javax.servlet.ServletRequestWrapper.getLocalPort(ServletRequestWrapper.java:329)
at com.netflix.spinnaker.gate.config.MultiAuthSupport$1.lambda$requestMatcher$0(MultiAuthSupport.java:30)
at org.springframework.security.web.DefaultSecurityFilterChain.matches(DefaultSecurityFilterChain.java:72)
at org.springframework.security.web.access.RequestMatcherDelegatingWebInvocationPrivilegeEvaluator.getDelegate(RequestMatcherDelegatingWebInvocationPrivilegeEvaluator.java:120)
at org.springframework.security.web.access.RequestMatcherDelegatingWebInvocationPrivilegeEvaluator.isAllowed(RequestMatcherDelegatingWebInvocationPrivilegeEvaluator.java:71)
at org.springframework.boot.web.servlet.filter.ErrorPageSecurityFilter.isAllowed(ErrorPageSecurityFilter.java:88)
at org.springframework.boot.web.servlet.filter.ErrorPageSecurityFilter.doFilter(ErrorPageSecurityFilter.java:76)
at org.springframework.boot.web.servlet.filter.ErrorPageSecurityFilter.doFilter(ErrorPageSecurityFilter.java:70)
at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:178)
at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:153)
at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:337)
at org.springframework.security.web.access.intercept.FilterSecurityInterceptor.invoke(FilterSecurityInterceptor.java:106)
at org.springframework.security.web.access.intercept.FilterSecurityInterceptor.doFilter(FilterSecurityInterceptor.java:81)
at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:346)
at org.springframework.security.web.access.ExceptionTranslationFilter.doFilter(ExceptionTranslationFilter.java:122)
at org.springframework.security.web.access.ExceptionTranslationFilter.doFilter(ExceptionTranslationFilter.java:116)
at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:346)
at org.springframework.security.web.session.SessionManagementFilter.doFilter(SessionManagementFilter.java:87)
at org.springframework.security.web.session.SessionManagementFilter.doFilter(SessionManagementFilter.java:81)
at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:346)
at org.springframework.security.web.authentication.AnonymousAuthenticationFilter.doFilter(AnonymousAuthenticationFilter.java:109)
at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:346)
at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:102)
at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:346)
at org.springframework.security.web.servletapi.SecurityContextHolderAwareRequestFilter.doFilter(SecurityContextHolderAwareRequestFilter.java:149)
at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:346)
at org.springframework.security.web.savedrequest.RequestCacheAwareFilter.doFilter(RequestCacheAwareFilter.java:63)
at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:346)
at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:102)
at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:346)
at org.springframework.security.web.authentication.AbstractAuthenticationProcessingFilter.doFilter(AbstractAuthenticationProcessingFilter.java:219)
at org.springframework.security.web.authentication.AbstractAuthenticationProcessingFilter.doFilter(AbstractAuthenticationProcessingFilter.java:213)
at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:346)
at javax.servlet.FilterChain$doFilter.call(Unknown Source)
at org.codehaus.groovy.runtime.callsite.CallSiteArray.defaultCall(CallSiteArray.java:47)
at javax.servlet.FilterChain$doFilter.call(Unknown Source)
at com.netflix.spinnaker.gate.security.oauth2.ExternalAuthTokenFilter.doFilter(ExternalAuthTokenFilter.groovy:65)
at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:346)
at org.springframework.security.web.authentication.logout.LogoutFilter.doFilter(LogoutFilter.java:103)
at org.springframework.security.web.authentication.logout.LogoutFilter.doFilter(LogoutFilter.java:89)
at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:346)
at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:102)
at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:346)
at org.springframework.security.web.context.SecurityContextPersistenceFilter.doFilter(SecurityContextPersistenceFilter.java:110)
at org.springframework.security.web.context.SecurityContextPersistenceFilter.doFilter(SecurityContextPersistenceFilter.java:80)
at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:346)
at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:102)
at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:346)
at org.springframework.security.web.FilterChainProxy.doFilterInternal(FilterChainProxy.java:221)
at org.springframework.security.web.FilterChainProxy.doFilter(FilterChainProxy.java:186)
at org.springframework.web.filter.DelegatingFilterProxy.invokeDelegate(DelegatingFilterProxy.java:354)
at org.springframework.web.filter.DelegatingFilterProxy.doFilter(DelegatingFilterProxy.java:267)
at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:178)
at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:153)
at org.springframework.web.filter.RequestContextFilter.doFilterInternal(RequestContextFilter.java:100)
at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:117)
at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:178)
at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:153)
at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:102)
at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:178)
at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:153)
at org.springframework.session.web.http.SessionRepositoryFilter.doFilterInternal(SessionRepositoryFilter.java:142)
at org.springframework.session.web.http.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:82)
at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:178)
at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:153)
at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:102)
at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:178)
at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:153)
at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:102)
at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:178)
at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:153)
at org.apache.catalina.core.ApplicationDispatcher.invoke(ApplicationDispatcher.java:661)
at org.apache.catalina.core.ApplicationDispatcher.processRequest(ApplicationDispatcher.java:427)
at org.apache.catalina.core.ApplicationDispatcher.doForward(ApplicationDispatcher.java:357)
at org.apache.catalina.core.ApplicationDispatcher.forward(ApplicationDispatcher.java:294)
at org.apache.catalina.core.StandardHostValve.custom(StandardHostValve.java:377)
at org.apache.catalina.core.StandardHostValve.status(StandardHostValve.java:237)
at org.apache.catalina.core.StandardHostValve.invoke(StandardHostValve.java:166)
at org.apache.catalina.valves.ErrorReportValve.invoke(ErrorReportValve.java:93)
at org.apache.catalina.core.StandardEngineValve.invoke(StandardEngineValve.java:74)
at org.apache.catalina.valves.RemoteIpValve.invoke(RemoteIpValve.java:765)
at org.apache.catalina.connector.CoyoteAdapter.service(CoyoteAdapter.java:346)
at org.apache.coyote.http11.Http11Processor.service(Http11Processor.java:390)
at org.apache.coyote.AbstractProcessorLight.process(AbstractProcessorLight.java:63)
at org.apache.coyote.AbstractProtocol$ConnectionHandler.process(AbstractProtocol.java:928)
at org.apache.tomcat.util.net.NioEndpoint$SocketProcessor.doRun(NioEndpoint.java:1794)
at org.apache.tomcat.util.net.SocketProcessorBase.run(SocketProcessorBase.java:52)
at org.apache.tomcat.util.threads.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1191)
at org.apache.tomcat.util.threads.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:659)
at org.apache.tomcat.util.threads.TaskThread$WrappingRunnable.run(TaskThread.java:63)
at java.base/java.lang.Thread.run(Thread.java:840)
The test uses basic auth, but we've seen this in production using oauth2.
* fix(core): remove ErrorPageSecurityFilter bean named errorPageSecurityInterceptor
to prevent java.lang.UnsupportedOperationException: public abstract int
javax.servlet.ServletRequest.getLocalPort() is not supported when processing error
responses.
See https://github.com/spring-projects/spring-security/issues/11055#issuecomment-1098061598 for background.
* refactor(basic): use constructor injection in BasicAuthConfig
to facilitate testing
* test(web): verify some error handling behavior of AuthConfig
* fix(core): update the name of the ErrorPageSecurityFilter bean for spring boot 2.7.x
---
gate-basic/gate-basic.gradle | 1 +
.../gate/security/basic/BasicAuthConfig.java | 11 +-
.../spinnaker/gate/config/AuthConfig.java | 10 +
.../gate/config/MultiAuthSupport.java | 28 +++
.../spinnaker/gate/AuthConfigTest.java | 183 ++++++++++++++++++
.../spinnaker/gate/MultiAuthSupportTest.java | 132 +++++++++++++
6 files changed, 362 insertions(+), 3 deletions(-)
create mode 100644 gate-web/src/test/java/com/netflix/spinnaker/gate/AuthConfigTest.java
create mode 100644 gate-web/src/test/java/com/netflix/spinnaker/gate/MultiAuthSupportTest.java
diff --git a/gate-basic/gate-basic.gradle b/gate-basic/gate-basic.gradle
index a2b6bf2f86..42b2127af5 100644
--- a/gate-basic/gate-basic.gradle
+++ b/gate-basic/gate-basic.gradle
@@ -1,5 +1,6 @@
dependencies {
implementation project(":gate-core")
+ implementation "io.spinnaker.kork:kork-annotations"
implementation "io.spinnaker.kork:kork-security"
implementation "org.springframework.session:spring-session-core"
}
diff --git a/gate-basic/src/main/java/com/netflix/spinnaker/gate/security/basic/BasicAuthConfig.java b/gate-basic/src/main/java/com/netflix/spinnaker/gate/security/basic/BasicAuthConfig.java
index 4816a70761..74f8e0f669 100644
--- a/gate-basic/src/main/java/com/netflix/spinnaker/gate/security/basic/BasicAuthConfig.java
+++ b/gate-basic/src/main/java/com/netflix/spinnaker/gate/security/basic/BasicAuthConfig.java
@@ -18,6 +18,7 @@
import com.netflix.spinnaker.gate.config.AuthConfig;
import com.netflix.spinnaker.gate.security.SpinnakerAuthConfig;
+import com.netflix.spinnaker.kork.annotations.VisibleForTesting;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.autoconfigure.condition.ConditionalOnExpression;
import org.springframework.boot.autoconfigure.security.SecurityProperties;
@@ -35,16 +36,20 @@
@EnableWebSecurity
public class BasicAuthConfig extends WebSecurityConfigurerAdapter {
- private final AuthConfig authConfig;
+ @VisibleForTesting protected final AuthConfig authConfig;
private final BasicAuthProvider authProvider;
- @Autowired DefaultCookieSerializer defaultCookieSerializer;
+ @VisibleForTesting protected final DefaultCookieSerializer defaultCookieSerializer;
@Autowired
- public BasicAuthConfig(AuthConfig authConfig, SecurityProperties securityProperties) {
+ public BasicAuthConfig(
+ AuthConfig authConfig,
+ SecurityProperties securityProperties,
+ DefaultCookieSerializer defaultCookieSerializer) {
this.authConfig = authConfig;
this.authProvider = new BasicAuthProvider(securityProperties);
+ this.defaultCookieSerializer = defaultCookieSerializer;
}
@Override
diff --git a/gate-core/src/main/java/com/netflix/spinnaker/gate/config/AuthConfig.java b/gate-core/src/main/java/com/netflix/spinnaker/gate/config/AuthConfig.java
index a1c68c287c..ee82c4312d 100644
--- a/gate-core/src/main/java/com/netflix/spinnaker/gate/config/AuthConfig.java
+++ b/gate-core/src/main/java/com/netflix/spinnaker/gate/config/AuthConfig.java
@@ -76,6 +76,16 @@ public void configure(HttpSecurity http) throws Exception {
.authorizeRequests(
registry -> {
registry
+ // https://github.com/spring-projects/spring-security/issues/11055#issuecomment-1098061598 suggests
+ //
+ // filterSecurityInterceptorOncePerRequest(false)
+ //
+ // until spring boot 3.0. Since
+ //
+ // .antMatchers("/error").permitAll()
+ //
+ // permits unauthorized access to /error, filterSecurityInterceptorOncePerRequest
+ // isn't relevant.
.antMatchers("/error")
.permitAll()
.antMatchers("/favicon.ico")
diff --git a/gate-core/src/main/java/com/netflix/spinnaker/gate/config/MultiAuthSupport.java b/gate-core/src/main/java/com/netflix/spinnaker/gate/config/MultiAuthSupport.java
index 15b79854c4..2107746210 100644
--- a/gate-core/src/main/java/com/netflix/spinnaker/gate/config/MultiAuthSupport.java
+++ b/gate-core/src/main/java/com/netflix/spinnaker/gate/config/MultiAuthSupport.java
@@ -1,6 +1,8 @@
package com.netflix.spinnaker.gate.config;
import org.springframework.beans.factory.annotation.Value;
+import org.springframework.beans.factory.config.BeanFactoryPostProcessor;
+import org.springframework.beans.factory.support.DefaultListableBeanFactory;
import org.springframework.boot.web.context.WebServerApplicationContext;
import org.springframework.boot.web.embedded.tomcat.TomcatWebServer;
import org.springframework.boot.web.server.WebServer;
@@ -19,6 +21,32 @@ public class MultiAuthSupport {
@Value("${default.legacy-server-port:-1}")
private int legacyServerPort;
+ /**
+ * From https://github.com/spring-projects/spring-security/issues/11055#issuecomment-1098061598,
+ * to fix java.lang.UnsupportedOperationException: public abstract int
+ * javax.servlet.ServletRequest.getLocalPort() is not supported when processing error responses
+ * for spring boot >= 2.6.4 and <= 3.0.0.
+ *
+ *
https://github.com/spring-projects/spring-boot/commit/71acc90da8 removed
+ * ErrorPageSecurityFilterConfiguration (which registered the errorPageSecurityInterceptor bean of
+ * type FilterRegistrationBean for 2.7.x), but added
+ * ErrorPageSecurityFilterConfiguration to SpringBootWebSecurityConfiguration which registered a
+ * bean named errorPageSecurityFilter of the same type.
+ *
+ *
https://github.com/spring-projects/spring-boot/commit/4bd3534b7d91f922ad903a75beb19b6bdca39e5c
+ * reverted those changes for 3.0.0-M4 and 3.0.0-M5.
+ *
+ *
https://github.com/spring-projects/spring-boot/commit/cedd553b836d97a04d769322771bc1a8499e7282
+ * removed ErrorPageSecurityFilter and the corresponding filter for good in 3.0.0-RC1.
+ *
+ *
Deleting a bean by name fails if the bean doesn't exist.
+ */
+ @Bean
+ public static BeanFactoryPostProcessor removeErrorSecurityFilter() {
+ return beanFactory ->
+ ((DefaultListableBeanFactory) beanFactory).removeBeanDefinition("errorPageSecurityFilter");
+ }
+
@Bean
RequestMatcherProvider multiAuthRequestMatcherProvider(ApplicationContext applicationContext) {
return new RequestMatcherProvider() {
diff --git a/gate-web/src/test/java/com/netflix/spinnaker/gate/AuthConfigTest.java b/gate-web/src/test/java/com/netflix/spinnaker/gate/AuthConfigTest.java
new file mode 100644
index 0000000000..4934f114e3
--- /dev/null
+++ b/gate-web/src/test/java/com/netflix/spinnaker/gate/AuthConfigTest.java
@@ -0,0 +1,183 @@
+/*
+ * Copyright 2024 Salesforce, Inc.
+ *
+ * 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
+ *
+ * http://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 com.netflix.spinnaker.gate;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.netflix.spinnaker.gate.config.AuthConfig;
+import com.netflix.spinnaker.gate.health.DownstreamServicesHealthIndicator;
+import com.netflix.spinnaker.gate.security.basic.BasicAuthConfig;
+import com.netflix.spinnaker.gate.services.ApplicationService;
+import com.netflix.spinnaker.gate.services.DefaultProviderLookupService;
+import java.io.IOException;
+import java.util.Map;
+import javax.servlet.http.HttpServletResponse;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestInfo;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.boot.autoconfigure.security.SecurityProperties;
+import org.springframework.boot.test.context.SpringBootTest;
+import org.springframework.boot.test.mock.mockito.MockBean;
+import org.springframework.boot.test.web.client.TestRestTemplate;
+import org.springframework.context.annotation.Bean;
+import org.springframework.context.annotation.Configuration;
+import org.springframework.context.annotation.Primary;
+import org.springframework.core.ParameterizedTypeReference;
+import org.springframework.http.HttpMethod;
+import org.springframework.http.HttpStatus;
+import org.springframework.http.ResponseEntity;
+import org.springframework.security.config.annotation.web.builders.HttpSecurity;
+import org.springframework.session.web.http.DefaultCookieSerializer;
+import org.springframework.test.context.TestPropertySource;
+import org.springframework.web.bind.annotation.GetMapping;
+import org.springframework.web.bind.annotation.RestController;
+
+/** AuthConfig is in gate-core, but is about matching http requests, so use gate-web to test it. */
+@SpringBootTest(
+ classes = {Main.class, AuthConfigTest.TestConfiguration.class},
+ webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT)
+@TestPropertySource(
+ properties = {
+ "spring.config.location=classpath:gate-test.yml",
+ "spring.security.user.name=testuser",
+ "spring.security.user.password=testpassword",
+ "security.basicform.enabled=true"
+ })
+class AuthConfigTest {
+
+ private static final String TEST_USER = "testuser";
+
+ private static final String TEST_PASSWORD = "testpassword";
+
+ private static final ParameterizedTypeReference