From 9b5e5f32e1cb5c0b359e49e2b6f93750a297de3e Mon Sep 17 00:00:00 2001 From: Madhura Bhave Date: Thu, 17 Aug 2017 16:49:47 -0700 Subject: [PATCH] ClientId not always required for client credentials Fixes gh-10013 --- .../OAuth2RestOperationsConfiguration.java | 5 +- ...Auth2RestOperationsConfigurationTests.java | 95 +++++++++++++++++-- 2 files changed, 88 insertions(+), 12 deletions(-) diff --git a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/security/oauth2/client/OAuth2RestOperationsConfiguration.java b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/security/oauth2/client/OAuth2RestOperationsConfiguration.java index 3c5c983d8d8..5b8c92cad3f 100644 --- a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/security/oauth2/client/OAuth2RestOperationsConfiguration.java +++ b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/security/oauth2/client/OAuth2RestOperationsConfiguration.java @@ -65,7 +65,6 @@ import org.springframework.util.StringUtils; */ @Configuration @ConditionalOnClass(EnableOAuth2Client.class) -@Conditional(OAuth2ClientIdCondition.class) public class OAuth2RestOperationsConfiguration { @Configuration @@ -89,7 +88,7 @@ public class OAuth2RestOperationsConfiguration { @Configuration @ConditionalOnBean(OAuth2ClientConfiguration.class) - @Conditional(NoClientCredentialsCondition.class) + @Conditional({OAuth2ClientIdCondition.class, NoClientCredentialsCondition.class}) @Import(OAuth2ProtectedResourceDetailsConfiguration.class) protected static class SessionScopedConfiguration { @@ -128,7 +127,7 @@ public class OAuth2RestOperationsConfiguration { // refresh tokens you need to @EnableOAuth2Client @Configuration @ConditionalOnMissingBean(OAuth2ClientConfiguration.class) - @Conditional(NoClientCredentialsCondition.class) + @Conditional({ OAuth2ClientIdCondition.class, NoClientCredentialsCondition.class }) @Import(OAuth2ProtectedResourceDetailsConfiguration.class) protected static class RequestScopedConfiguration { diff --git a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/security/oauth2/client/OAuth2RestOperationsConfigurationTests.java b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/security/oauth2/client/OAuth2RestOperationsConfigurationTests.java index 9d5e2fed551..dd14ce3cd63 100644 --- a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/security/oauth2/client/OAuth2RestOperationsConfigurationTests.java +++ b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/security/oauth2/client/OAuth2RestOperationsConfigurationTests.java @@ -19,13 +19,22 @@ package org.springframework.boot.autoconfigure.security.oauth2.client; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; +import org.mockito.Mockito; import org.springframework.beans.factory.NoSuchBeanDefinitionException; +import org.springframework.boot.autoconfigure.security.SecurityProperties; import org.springframework.boot.builder.SpringApplicationBuilder; +import org.springframework.boot.context.embedded.MockEmbeddedServletContainerFactory; import org.springframework.boot.test.util.EnvironmentTestUtils; import org.springframework.context.ConfigurableApplicationContext; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.Import; import org.springframework.core.env.ConfigurableEnvironment; import org.springframework.core.env.StandardEnvironment; +import org.springframework.security.oauth2.client.DefaultOAuth2ClientContext; +import org.springframework.security.oauth2.client.token.grant.client.ClientCredentialsResourceDetails; +import org.springframework.security.oauth2.config.annotation.web.configuration.OAuth2ClientConfiguration; import static org.assertj.core.api.Assertions.assertThat; @@ -44,23 +53,91 @@ public class OAuth2RestOperationsConfigurationTests { public ExpectedException thrown = ExpectedException.none(); @Test - public void clientIdConditionMatches() throws Exception { + public void clientCredentialsWithClientId() throws Exception { EnvironmentTestUtils.addEnvironment(this.environment, "security.oauth2.client.client-id=acme"); - this.context = new SpringApplicationBuilder( - OAuth2RestOperationsConfiguration.class).environment(this.environment) - .web(false).run(); + initializeContext(OAuth2RestOperationsConfiguration.class, true); assertThat(this.context.getBean(OAuth2RestOperationsConfiguration.class)) .isNotNull(); + assertThat(this.context.getBean(ClientCredentialsResourceDetails.class)) + .isNotNull(); + } + + @Test + public void clientCredentialsWithNoClientId() throws Exception { + EnvironmentTestUtils.addEnvironment(this.environment); + initializeContext(OAuth2RestOperationsConfiguration.class, true); + assertThat(this.context.getBean(OAuth2RestOperationsConfiguration.class)) + .isNotNull(); + assertThat(this.context.getBean(ClientCredentialsResourceDetails.class)) + .isNotNull(); + } + + @Test + public void requestScopedWithClientId() throws Exception { + EnvironmentTestUtils.addEnvironment(this.environment, + "security.oauth2.client.client-id=acme"); + initializeContext(ConfigForRequestScopedConfiguration.class, false); + assertThat(this.context.containsBean("oauth2ClientContext")) + .isTrue(); + } + + @Test + public void requestScopedWithNoClientId() throws Exception { + EnvironmentTestUtils.addEnvironment(this.environment); + initializeContext(ConfigForRequestScopedConfiguration.class, false); + this.thrown.expect(NoSuchBeanDefinitionException.class); + this.context.getBean(DefaultOAuth2ClientContext.class); + } + + @Test + public void sessionScopedWithClientId() throws Exception { + EnvironmentTestUtils.addEnvironment(this.environment, + "security.oauth2.client.client-id=acme"); + initializeContext(ConfigForSessionScopedConfiguration.class, false); + assertThat(this.context.containsBean("oauth2ClientContext")) + .isTrue(); } @Test - public void clientIdConditionDoesNotMatch() throws Exception { - this.context = new SpringApplicationBuilder( - OAuth2RestOperationsConfiguration.class).environment(this.environment) - .web(false).run(); + public void sessionScopedWithNoClientId() throws Exception { + EnvironmentTestUtils.addEnvironment(this.environment); + initializeContext(ConfigForSessionScopedConfiguration.class, false); this.thrown.expect(NoSuchBeanDefinitionException.class); - this.context.getBean(OAuth2RestOperationsConfiguration.class); + this.context.getBean(DefaultOAuth2ClientContext.class); + } + + private void initializeContext(Class configuration, boolean isClientCredentials) { + this.context = new SpringApplicationBuilder(configuration) + .environment(this.environment) + .web(!isClientCredentials).run(); + } + + @Configuration + @Import({ OAuth2RestOperationsConfiguration.class }) + protected static class WebApplicationConfiguration { + + @Bean + public MockEmbeddedServletContainerFactory embeddedServletContainerFactory() { + return new MockEmbeddedServletContainerFactory(); + } + + } + + @Configuration + @Import({ OAuth2ClientConfiguration.class, OAuth2RestOperationsConfiguration.class }) + protected static class ConfigForSessionScopedConfiguration extends WebApplicationConfiguration { + + @Bean + public SecurityProperties securityProperties() { + return Mockito.mock(SecurityProperties.class); + } + + } + + @Configuration + protected static class ConfigForRequestScopedConfiguration extends WebApplicationConfiguration { + } }