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

…n in OAuth2SsoConfig when oauth2 is enabled

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.

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
+  }
+}