From 5ccb64ddf05cdf8fcea981f06931dbe83b970be3 Mon Sep 17 00:00:00 2001 From: David Byron Date: Thu, 4 Nov 2021 10:44:22 -0700 Subject: [PATCH] fix(oauth): remove circular dependency on ExternalAuthTokenFilter bean 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. --- .../oauth2/ExternalAuthTokenFilter.groovy | 13 +++-- .../security/oauth2/OAuth2SsoConfig.groovy | 5 -- gate-web/gate-web.gradle | 1 + .../gate/security/oauth2/OAuth2Spec.groovy | 56 +++++++++++++++++++ 4 files changed, 66 insertions(+), 9 deletions(-) create mode 100644 gate-web/src/test/groovy/com/netflix/spinnaker/gate/security/oauth2/OAuth2Spec.groovy diff --git a/gate-oauth2/src/main/groovy/com/netflix/spinnaker/gate/security/oauth2/ExternalAuthTokenFilter.groovy b/gate-oauth2/src/main/groovy/com/netflix/spinnaker/gate/security/oauth2/ExternalAuthTokenFilter.groovy index e8334baba7..7e4f511e20 100644 --- a/gate-oauth2/src/main/groovy/com/netflix/spinnaker/gate/security/oauth2/ExternalAuthTokenFilter.groovy +++ b/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) } diff --git a/gate-oauth2/src/main/groovy/com/netflix/spinnaker/gate/security/oauth2/OAuth2SsoConfig.groovy b/gate-oauth2/src/main/groovy/com/netflix/spinnaker/gate/security/oauth2/OAuth2SsoConfig.groovy index d5a3414f67..af8b6c5438 100644 --- a/gate-oauth2/src/main/groovy/com/netflix/spinnaker/gate/security/oauth2/OAuth2SsoConfig.groovy +++ b/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) diff --git a/gate-web/gate-web.gradle b/gate-web/gate-web.gradle index a4eb1f63dc..7573ae62ea 100644 --- a/gate-web/gate-web.gradle +++ b/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" diff --git a/gate-web/src/test/groovy/com/netflix/spinnaker/gate/security/oauth2/OAuth2Spec.groovy b/gate-web/src/test/groovy/com/netflix/spinnaker/gate/security/oauth2/OAuth2Spec.groovy new file mode 100644 index 0000000000..cafac4effc --- /dev/null +++ b/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 + } +}