fix(oauth): remove circular dependency on ExternalAuthTokenFilter bea…

…n in OAuth2SsoConfig when oauth2 is enabled (#1492)

Previously in spring 2.2.5, if oauth2 is enabled there is no circular
dependency on ExternalAuthTokenFilter bean in OAuth2SsoConfig.

In spring 2.2.13, if oauth2 is enabled there is a circular dependency
error on ExternalAuthTokenFilter bean. This circular dependency results
in an error when the gate application tries to start up. The application
fails with error:

BeanCurrentlyInCreationException: Error creating bean with name
'OAuth2SsoConfig': Bean with name 'OAuth2SsoConfig' has been injected
into other beans [externalAuthTokenFilter] in its raw version as part
of a circular reference, but has eventually been wrapped.

To fix this error, add the Component annotation to
ExternalAuthTokenFilter and remove the ExternalAuthTokenFilter bean
from OAuth2SsoConfig.

Co-authored-by: David Byron <dbyron@salesforce.com>
(cherry picked from commit 1823633)

gate-oauth2/src/main/groovy/com/netflix/spinnaker/gate/security/oauth2/ExternalAuthTokenFilter.groovy

@@ -23,6 +23,7 @@ import org.springframework.security.core.Authentication
 import org.springframework.security.oauth2.common.DefaultOAuth2AccessToken
 import org.springframework.security.oauth2.common.OAuth2AccessToken
 import org.springframework.security.oauth2.provider.authentication.BearerTokenExtractor
+import org.springframework.stereotype.Component
 
 import javax.servlet.Filter
 import javax.servlet.FilterChain
@@ -37,9 +38,12 @@ import javax.servlet.http.HttpServletRequest
  * Github-issued personal access token.
  */
 @Slf4j
+@Component
 class ExternalAuthTokenFilter implements Filter {
 
-  @Autowired
+  // UserInfoRestTemplateFactory can't be Autowired if no oauth2 configurations are set.
+  // In this case, userInfoRestTemplateFactory will be null
+  @Autowired(required = false)
   UserInfoRestTemplateFactory userInfoRestTemplateFactory
 
   BearerTokenExtractor extractor = new BearerTokenExtractor()
@@ -53,9 +57,10 @@ class ExternalAuthTokenFilter implements Filter {
       // Reassign token type to be capitalized "Bearer",
       // see https://github.com/spinnaker/spinnaker/issues/2074
       token.tokenType = OAuth2AccessToken.BEARER_TYPE
-
-      def ctx = userInfoRestTemplateFactory.getUserInfoRestTemplate().getOAuth2ClientContext()
-      ctx.accessToken = token
+      if (userInfoRestTemplateFactory != null) {
+        def ctx = userInfoRestTemplateFactory.getUserInfoRestTemplate().getOAuth2ClientContext()
+        ctx.accessToken = token
+      }
     }
     chain.doFilter(request, response)
   }

gate-oauth2/src/main/groovy/com/netflix/spinnaker/gate/security/oauth2/OAuth2SsoConfig.groovy

@@ -78,11 +78,6 @@ class OAuth2SsoConfig extends WebSecurityConfigurerAdapter {
     new SpinnakerUserInfoTokenServices()
   }
 
-  @Bean
-  ExternalAuthTokenFilter externalAuthTokenFilter() {
-    new ExternalAuthTokenFilter()
-  }
-
   @Override
   void configure(HttpSecurity http) throws Exception {
     defaultCookieSerializer.setSameSite(null)

gate-web/gate-web.gradle

@@ -64,6 +64,7 @@ dependencies {
 
   testImplementation project(":gate-ldap") // TODO: Move system tests to own module
   testImplementation project(":gate-basic")
+  testImplementation project(":gate-oauth2")
   testImplementation "com.squareup.okhttp:mockwebserver"
 
   testImplementation "com.squareup.retrofit:retrofit-mock"

gate-web/src/test/groovy/com/netflix/spinnaker/gate/security/oauth2/OAuth2Spec.groovy

@@ -0,0 +1,56 @@
+/*
+ * Copyright 2021 Salesforce.com, 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.security.oauth2
+
+
+import groovy.util.logging.Slf4j
+import org.springframework.beans.factory.annotation.Autowired
+import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc
+import org.springframework.boot.test.context.SpringBootTest
+import org.springframework.http.HttpHeaders
+import org.springframework.test.context.TestPropertySource
+import org.springframework.test.web.servlet.MockMvc
+import spock.lang.Specification
+
+import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get
+import static org.springframework.test.web.servlet.result.MockMvcResultHandlers.print
+
+@Slf4j
+// AutoConfigureMockMvc is needed here because requests made by MockMvc will need to go through the OAuth2 filter
+@AutoConfigureMockMvc
+@SpringBootTest(properties = [
+  "retrofit.enabled=true",
+  "security.oauth2.client.clientId=Spinnaker-Client",
+  "security.oauth2.resource.userInfoUri=http://localhost/userinfo"
+])
+@TestPropertySource(properties = ["spring.config.location=classpath:gate-test.yml"])
+class OAuth2Spec extends Specification {
+
+  @Autowired
+  MockMvc mockMvc
+
+  def "should redirect on oauth2 authentication"() {
+    when:
+      def result = mockMvc.perform(get("/credentials")
+          .header(HttpHeaders.AUTHORIZATION, "Bearer access_token"))
+        .andDo(print())
+        .andReturn()
+
+    then:
+      result.response.getStatus() == 302
+  }
+}