From b4134e239ec97da7cf55ff25b7e4400da712715b Mon Sep 17 00:00:00 2001 From: Madhura Bhave Date: Wed, 15 Feb 2017 11:58:53 -0800 Subject: [PATCH] Add autoconfiguration for JWKTokenStore If `jwk.key-set-uri` is present. Closes gh-4437 --- .../resource/ResourceServerProperties.java | 76 ++++++++++++++----- ...ourceServerTokenServicesConfiguration.java | 67 ++++++++++++++-- .../ResourceServerPropertiesTests.java | 40 ++++++++++ ...ServerTokenServicesConfigurationTests.java | 21 +++++ spring-boot-dependencies/pom.xml | 2 +- .../analyzer/BindFailureAnalyzer.java | 19 +++-- .../analyzer/BindFailureAnalyzerTests.java | 39 ++++++++-- 7 files changed, 225 insertions(+), 39 deletions(-) diff --git a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/security/oauth2/resource/ResourceServerProperties.java b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/security/oauth2/resource/ResourceServerProperties.java index c55ddd5de0..df290a76b7 100644 --- a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/security/oauth2/resource/ResourceServerProperties.java +++ b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/security/oauth2/resource/ResourceServerProperties.java @@ -35,6 +35,7 @@ import org.springframework.validation.Validator; * Configuration properties for OAuth2 Resources. * * @author Dave Syer + * @author Madhura Bhave * @since 1.3.0 */ @ConfigurationProperties(prefix = "security.oauth2.resource") @@ -78,6 +79,8 @@ public class ResourceServerProperties implements Validator, BeanFactoryAware { private Jwt jwt = new Jwt(); + private Jwk jwk = new Jwk(); + /** * The order of the filter chain used to authenticate tokens. Default puts it after * the actuator endpoints and before the default HTTP basic filter chain (catchall). @@ -158,6 +161,14 @@ public class ResourceServerProperties implements Validator, BeanFactoryAware { this.jwt = jwt; } + public Jwk getJwk() { + return this.jwk; + } + + public void setJwk(Jwk jwk) { + this.jwk = jwk; + } + public String getClientId() { return this.clientId; } @@ -192,29 +203,40 @@ public class ResourceServerProperties implements Validator, BeanFactoryAware { return; } ResourceServerProperties resource = (ResourceServerProperties) target; - if (StringUtils.hasText(this.clientId)) { - if (!StringUtils.hasText(this.clientSecret)) { - if (!StringUtils.hasText(resource.getUserInfoUri())) { - errors.rejectValue("userInfoUri", "missing.userInfoUri", - "Missing userInfoUri (no client secret available)"); - } - } - else { - if (isPreferTokenInfo() - && !StringUtils.hasText(resource.getTokenInfoUri())) { - if (StringUtils.hasText(getJwt().getKeyUri()) - || StringUtils.hasText(getJwt().getKeyValue())) { - // It's a JWT decoder - return; - } + + if ((StringUtils.hasText(this.jwt.getKeyUri()) + || StringUtils.hasText(this.jwt.getKeyValue())) + && StringUtils.hasText(this.jwk.getKeySetUri())) { + errors.reject("ambiguous.keyUri", "Only one of jwt.keyUri (or jwt.keyValue) and jwk.keySetUri should be configured."); + } + + else { + if (StringUtils.hasText(this.clientId)) { + if (!StringUtils.hasText(this.clientSecret)) { if (!StringUtils.hasText(resource.getUserInfoUri())) { - errors.rejectValue("tokenInfoUri", "missing.tokenInfoUri", - "Missing tokenInfoUri and userInfoUri and there is no " - + "JWT verifier key"); + errors.rejectValue("userInfoUri", "missing.userInfoUri", + "Missing userInfoUri (no client secret available)"); + } + } + else { + if (isPreferTokenInfo() + && !StringUtils.hasText(resource.getTokenInfoUri())) { + if (StringUtils.hasText(getJwt().getKeyUri()) + || StringUtils.hasText(getJwt().getKeyValue()) + || StringUtils.hasText(getJwk().getKeySetUri())) { + // It's a JWT decoder + return; + } + if (!StringUtils.hasText(resource.getUserInfoUri())) { + errors.rejectValue("tokenInfoUri", "missing.tokenInfoUri", + "Missing tokenInfoUri and userInfoUri and there is no " + + "JWT verifier key"); + } } } } } + } private int countBeans(Class type) { @@ -269,4 +291,22 @@ public class ResourceServerProperties implements Validator, BeanFactoryAware { } + public class Jwk { + + /** + * The URI to get verification keys to verify the JWT token. + * This can be set when the authorization server returns a + * set of verification keys. + */ + private String keySetUri; + + public String getKeySetUri() { + return this.keySetUri; + } + + public void setKeySetUri(String keySetUri) { + this.keySetUri = keySetUri; + } + } + } diff --git a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/security/oauth2/resource/ResourceServerTokenServicesConfiguration.java b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/security/oauth2/resource/ResourceServerTokenServicesConfiguration.java index 78c64d126c..7b8cd849a5 100644 --- a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/security/oauth2/resource/ResourceServerTokenServicesConfiguration.java +++ b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/security/oauth2/resource/ResourceServerTokenServicesConfiguration.java @@ -31,6 +31,7 @@ import org.springframework.boot.autoconfigure.condition.ConditionalOnBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingClass; +import org.springframework.boot.autoconfigure.condition.NoneNestedConditions; import org.springframework.boot.autoconfigure.condition.SpringBootCondition; import org.springframework.boot.bind.RelaxedPropertyResolver; import org.springframework.context.annotation.Bean; @@ -61,6 +62,7 @@ import org.springframework.security.oauth2.provider.token.ResourceServerTokenSer import org.springframework.security.oauth2.provider.token.TokenStore; import org.springframework.security.oauth2.provider.token.store.JwtAccessTokenConverter; import org.springframework.security.oauth2.provider.token.store.JwtTokenStore; +import org.springframework.security.oauth2.provider.token.store.jwk.JwkTokenStore; import org.springframework.social.connect.ConnectionFactoryLocator; import org.springframework.social.connect.support.OAuth2ConnectionFactory; import org.springframework.util.CollectionUtils; @@ -73,6 +75,7 @@ import org.springframework.web.client.RestTemplate; * Configuration for an OAuth2 resource server. * * @author Dave Syer + * @author Madhura Bhave * @since 1.3.0 */ @Configuration @@ -93,7 +96,7 @@ public class ResourceServerTokenServicesConfiguration { } @Configuration - @Conditional(NotJwtTokenCondition.class) + @Conditional(RemoteTokenCondition.class) protected static class RemoteTokenServicesConfiguration { @Configuration @@ -214,6 +217,30 @@ public class ResourceServerTokenServicesConfiguration { } + @Configuration + @Conditional(JwkCondition.class) + protected static class JwkTokenStoreConfiguration { + + private final ResourceServerProperties resource; + + public JwkTokenStoreConfiguration(ResourceServerProperties resource) { + this.resource = resource; + } + + @Bean + @ConditionalOnMissingBean(ResourceServerTokenServices.class) + public DefaultTokenServices jwkTokenServices() { + DefaultTokenServices services = new DefaultTokenServices(); + services.setTokenStore(jwkTokenStore()); + return services; + } + + @Bean + public TokenStore jwkTokenStore() { + return new JwkTokenStore(this.resource.getJwk().getKeySetUri()); + } + } + @Configuration @Conditional(JwtTokenCondition.class) protected static class JwtTokenServicesConfiguration { @@ -341,32 +368,56 @@ public class ResourceServerTokenServicesConfiguration { } - private static class NotTokenInfoCondition extends SpringBootCondition { - - private TokenInfoCondition tokenInfoCondition = new TokenInfoCondition(); + private static class JwkCondition extends SpringBootCondition { @Override public ConditionOutcome getMatchOutcome(ConditionContext context, AnnotatedTypeMetadata metadata) { + ConditionMessage.Builder message = ConditionMessage + .forCondition("OAuth JWK Condition"); + RelaxedPropertyResolver resolver = new RelaxedPropertyResolver( + context.getEnvironment(), "security.oauth2.resource.jwk."); + String keyUri = resolver.getProperty("key-set-uri"); + if (StringUtils.hasText(keyUri)) { + return ConditionOutcome + .match(message.foundExactly("provided jwk key set URI")); + } return ConditionOutcome - .inverse(this.tokenInfoCondition.getMatchOutcome(context, metadata)); + .noMatch(message.didNotFind("key jwk set URI not provided").atAll()); } } - private static class NotJwtTokenCondition extends SpringBootCondition { + private static class NotTokenInfoCondition extends SpringBootCondition { - private JwtTokenCondition jwtTokenCondition = new JwtTokenCondition(); + private TokenInfoCondition tokenInfoCondition = new TokenInfoCondition(); @Override public ConditionOutcome getMatchOutcome(ConditionContext context, AnnotatedTypeMetadata metadata) { return ConditionOutcome - .inverse(this.jwtTokenCondition.getMatchOutcome(context, metadata)); + .inverse(this.tokenInfoCondition.getMatchOutcome(context, metadata)); } } + private static class RemoteTokenCondition extends NoneNestedConditions { + + RemoteTokenCondition() { + super(ConfigurationPhase.PARSE_CONFIGURATION); + } + + @Conditional(JwtTokenCondition.class) + static class HasJwtConfiguration { + + } + + @Conditional(JwkCondition.class) + static class HasJwkConfiguration { + + } + } + static class AcceptJsonRequestInterceptor implements ClientHttpRequestInterceptor { @Override diff --git a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/security/oauth2/resource/ResourceServerPropertiesTests.java b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/security/oauth2/resource/ResourceServerPropertiesTests.java index 81a79ec70e..291da9b05e 100644 --- a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/security/oauth2/resource/ResourceServerPropertiesTests.java +++ b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/security/oauth2/resource/ResourceServerPropertiesTests.java @@ -21,13 +21,21 @@ import java.util.Map; import com.fasterxml.jackson.databind.ObjectMapper; import org.junit.Test; +import org.springframework.beans.factory.ListableBeanFactory; +import org.springframework.validation.Errors; +import org.springframework.web.context.support.StaticWebApplicationContext; + import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyZeroInteractions; /** * Tests for {@link ResourceServerProperties}. * * @author Dave Syer * @author Vedran Pavic + * @author Madhura Bhave */ public class ResourceServerPropertiesTests { @@ -59,4 +67,36 @@ public class ResourceServerPropertiesTests { .isEqualTo("http://example.com/token_key"); } + @Test + public void validateWhenBothJwtAndJwtKeyConfigurationPresentShouldFail() throws Exception { + this.properties.getJwk().setKeySetUri("http://my-auth-server/token_keys"); + this.properties.getJwt().setKeyUri("http://my-auth-server/token_key"); + setListableBeanFactory(); + Errors errors = mock(Errors.class); + this.properties.validate(this.properties, errors); + verify(errors).reject("ambiguous.keyUri", "Only one of jwt.keyUri (or jwt.keyValue) and jwk.keySetUri should be configured."); + + } + + @Test + public void validateWhenKeySetUriProvidedShouldSucceed() throws Exception { + this.properties.getJwk().setKeySetUri("http://my-auth-server/token_keys"); + setListableBeanFactory(); + Errors errors = mock(Errors.class); + this.properties.validate(this.properties, errors); + verifyZeroInteractions(errors); + } + + private void setListableBeanFactory() { + ListableBeanFactory beanFactory = new StaticWebApplicationContext() { + @Override + public String[] getBeanNamesForType(Class type, boolean includeNonSingletons, boolean allowEagerInit) { + if (type.isAssignableFrom(ResourceServerTokenServicesConfiguration.class)) { + return new String[]{"ResourceServerTokenServicesConfiguration"}; + } + return new String[0]; + } + }; + this.properties.setBeanFactory(beanFactory); + } } diff --git a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/security/oauth2/resource/ResourceServerTokenServicesConfigurationTests.java b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/security/oauth2/resource/ResourceServerTokenServicesConfigurationTests.java index 877dab7991..0fc52abec0 100644 --- a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/security/oauth2/resource/ResourceServerTokenServicesConfigurationTests.java +++ b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/security/oauth2/resource/ResourceServerTokenServicesConfigurationTests.java @@ -21,8 +21,11 @@ import java.util.List; import java.util.Map; import org.junit.After; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.ExpectedException; +import org.springframework.beans.factory.NoSuchBeanDefinitionException; import org.springframework.beans.factory.config.BeanDefinition; import org.springframework.beans.factory.support.BeanDefinitionRegistry; import org.springframework.boot.autoconfigure.context.PropertyPlaceholderAutoConfiguration; @@ -61,6 +64,7 @@ import static org.mockito.Mockito.mock; * Tests for {@link ResourceServerTokenServicesConfiguration}. * * @author Dave Syer + * @author Madhura Bhave */ public class ResourceServerTokenServicesConfigurationTests { @@ -76,6 +80,9 @@ public class ResourceServerTokenServicesConfigurationTests { private ConfigurableEnvironment environment = new StandardEnvironment(); + @Rule + public ExpectedException thrown = ExpectedException.none(); + @After public void close() { if (this.context != null) { @@ -186,6 +193,8 @@ public class ResourceServerTokenServicesConfigurationTests { .environment(this.environment).web(false).run(); DefaultTokenServices services = this.context.getBean(DefaultTokenServices.class); assertThat(services).isNotNull(); + this.thrown.expect(NoSuchBeanDefinitionException.class); + this.context.getBean(RemoteTokenServices.class); } @Test @@ -198,6 +207,18 @@ public class ResourceServerTokenServicesConfigurationTests { assertThat(services).isNotNull(); } + @Test + public void jwkConfiguration() throws Exception { + EnvironmentTestUtils.addEnvironment(this.environment, + "security.oauth2.resource.jwk.key-set-uri=http://my-auth-server/token_keys"); + this.context = new SpringApplicationBuilder(ResourceConfiguration.class) + .environment(this.environment).web(false).run(); + DefaultTokenServices services = this.context.getBean(DefaultTokenServices.class); + assertThat(services).isNotNull(); + this.thrown.expect(NoSuchBeanDefinitionException.class); + this.context.getBean(RemoteTokenServices.class); + } + @Test public void springSocialUserInfo() { EnvironmentTestUtils.addEnvironment(this.environment, diff --git a/spring-boot-dependencies/pom.xml b/spring-boot-dependencies/pom.xml index e9e4265f14..efcdc4d558 100644 --- a/spring-boot-dependencies/pom.xml +++ b/spring-boot-dependencies/pom.xml @@ -168,7 +168,7 @@ 1.2.0.RELEASE 4.2.1.RELEASE 1.0.7.RELEASE - 2.0.12.RELEASE + 2.0.13.BUILD-SNAPSHOT 1.3.0.RELEASE 1.1.4.RELEASE 2.0.3.RELEASE diff --git a/spring-boot/src/main/java/org/springframework/boot/diagnostics/analyzer/BindFailureAnalyzer.java b/spring-boot/src/main/java/org/springframework/boot/diagnostics/analyzer/BindFailureAnalyzer.java index a846077aaf..71307c96e3 100644 --- a/spring-boot/src/main/java/org/springframework/boot/diagnostics/analyzer/BindFailureAnalyzer.java +++ b/spring-boot/src/main/java/org/springframework/boot/diagnostics/analyzer/BindFailureAnalyzer.java @@ -21,29 +21,34 @@ import org.springframework.boot.diagnostics.FailureAnalysis; import org.springframework.util.CollectionUtils; import org.springframework.validation.BindException; import org.springframework.validation.FieldError; +import org.springframework.validation.ObjectError; /** * An {@link AbstractFailureAnalyzer} that performs analysis of failures caused by a * {@link BindException}. * * @author Andy Wilkinson + * @author Madhura Bhave */ class BindFailureAnalyzer extends AbstractFailureAnalyzer { @Override protected FailureAnalysis analyze(Throwable rootFailure, BindException cause) { - if (CollectionUtils.isEmpty(cause.getFieldErrors())) { + if (CollectionUtils.isEmpty(cause.getAllErrors())) { return null; } StringBuilder description = new StringBuilder( String.format("Binding to target %s failed:%n", cause.getTarget())); - for (FieldError fieldError : cause.getFieldErrors()) { - description.append(String.format("%n Property: %s", - cause.getObjectName() + "." + fieldError.getField())); + for (ObjectError error : cause.getAllErrors()) { + if (error instanceof FieldError) { + FieldError fieldError = (FieldError) error; + description.append(String.format("%n Property: %s", + cause.getObjectName() + "." + fieldError.getField())); + description.append( + String.format("%n Value: %s", fieldError.getRejectedValue())); + } description.append( - String.format("%n Value: %s", fieldError.getRejectedValue())); - description.append( - String.format("%n Reason: %s%n", fieldError.getDefaultMessage())); + String.format("%n Reason: %s%n", error.getDefaultMessage())); } return new FailureAnalysis(description.toString(), "Update your application's configuration", cause); diff --git a/spring-boot/src/test/java/org/springframework/boot/diagnostics/analyzer/BindFailureAnalyzerTests.java b/spring-boot/src/test/java/org/springframework/boot/diagnostics/analyzer/BindFailureAnalyzerTests.java index 60a0629171..727be26b5b 100644 --- a/spring-boot/src/test/java/org/springframework/boot/diagnostics/analyzer/BindFailureAnalyzerTests.java +++ b/spring-boot/src/test/java/org/springframework/boot/diagnostics/analyzer/BindFailureAnalyzerTests.java @@ -32,6 +32,8 @@ import org.springframework.boot.context.properties.EnableConfigurationProperties import org.springframework.boot.diagnostics.FailureAnalysis; import org.springframework.context.annotation.AnnotationConfigApplicationContext; import org.springframework.context.i18n.LocaleContextHolder; +import org.springframework.validation.Errors; +import org.springframework.validation.Validator; import org.springframework.validation.annotation.Validated; import static org.assertj.core.api.Assertions.assertThat; @@ -40,6 +42,7 @@ import static org.assertj.core.api.Assertions.assertThat; * Tests for {@link BindFailureAnalyzer}. * * @author Andy Wilkinson + * @author Madhura Bhave */ public class BindFailureAnalyzerTests { @@ -54,8 +57,8 @@ public class BindFailureAnalyzerTests { } @Test - public void bindExceptionDueToValidationFailure() { - FailureAnalysis analysis = performAnalysis(ValidationFailureConfiguration.class); + public void bindExceptionWithFieldErrorsDueToValidationFailure() { + FailureAnalysis analysis = performAnalysis(FieldValidationFailureConfiguration.class); assertThat(analysis.getDescription()) .contains(failure("test.foo.foo", "null", "may not be null")); assertThat(analysis.getDescription()) @@ -64,6 +67,13 @@ public class BindFailureAnalyzerTests { .contains(failure("test.foo.nested.bar", "null", "may not be null")); } + @Test + public void bindExceptionWithObjectErrorsDueToValidationFailure() throws Exception { + FailureAnalysis analysis = performAnalysis(ObjectValidationFailureConfiguration.class); + assertThat(analysis.getDescription()) + .contains("Reason: This object could not be bound."); + } + private static String failure(String property, String value, String reason) { return String.format("Property: %s%n Value: %s%n Reason: %s", property, value, reason); @@ -85,14 +95,19 @@ public class BindFailureAnalyzerTests { } } - @EnableConfigurationProperties(ValidationFailureProperties.class) - static class ValidationFailureConfiguration { + @EnableConfigurationProperties(FieldValidationFailureProperties.class) + static class FieldValidationFailureConfiguration { + + } + + @EnableConfigurationProperties(ObjectErrorFailureProperties.class) + static class ObjectValidationFailureConfiguration { } @ConfigurationProperties("test.foo") @Validated - static class ValidationFailureProperties { + static class FieldValidationFailureProperties { @NotNull private String foo; @@ -144,4 +159,18 @@ public class BindFailureAnalyzerTests { } + @ConfigurationProperties("foo.bar") + static class ObjectErrorFailureProperties implements Validator { + + @Override + public void validate(Object target, Errors errors) { + errors.reject("my.objectError", "This object could not be bound."); + } + + @Override + public boolean supports(Class clazz) { + return true; + } + } + }