From 605b0aefc65988e81d5b37e5c7658d6a88d27f9c Mon Sep 17 00:00:00 2001 From: Madhura Bhave Date: Wed, 15 Feb 2017 17:29:22 -0800 Subject: [PATCH] Simplify validation logic in ResourceServerProperties Closes gh-8306 Closes gh-8317 --- .../resource/ResourceServerProperties.java | 66 ++++------- .../ResourceServerPropertiesTests.java | 110 +++++++++++++++--- ...ServerTokenServicesConfigurationTests.java | 8 -- 3 files changed, 114 insertions(+), 70 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 275eff8d49..10a9ff2388 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 @@ -204,40 +204,34 @@ public class ResourceServerProperties implements Validator, BeanFactoryAware { } ResourceServerProperties resource = (ResourceServerProperties) target; validate(resource, errors); - } private void validate(ResourceServerProperties target, Errors errors) { - if ((StringUtils.hasText(this.jwt.getKeyUri()) - || StringUtils.hasText(this.jwt.getKeyValue())) - && StringUtils.hasText(this.jwk.getKeySetUri())) { + boolean jwtConfigPresent = StringUtils.hasText(this.jwt.getKeyUri()) + || StringUtils.hasText(this.jwt.getKeyValue()); + boolean jwkConfigPresent = StringUtils.hasText(this.jwk.getKeySetUri()); + + if (jwtConfigPresent && jwkConfigPresent) { errors.reject("ambiguous.keyUri", - "Only one of jwt.keyUri (or jwt.keyValue) and " - + "jwk.keySetUri should be configured."); + "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(target.getUserInfoUri())) { - errors.rejectValue("userInfoUri", "missing.userInfoUri", - "Missing userInfoUri (no client secret available)"); - } - } - else { - if (isPreferTokenInfo() - && !StringUtils.hasText(target.getTokenInfoUri())) { - if (StringUtils.hasText(getJwt().getKeyUri()) - || StringUtils.hasText(getJwt().getKeyValue()) - || StringUtils.hasText(getJwk().getKeySetUri())) { - // It's a JWT decoder - return; - } - if (!StringUtils.hasText(target.getUserInfoUri())) { - errors.rejectValue("tokenInfoUri", "missing.tokenInfoUri", - "Missing tokenInfoUri and userInfoUri and there is no " - + "JWT verifier key"); - } - } + if (jwtConfigPresent || jwkConfigPresent) { + // It's a JWT decoder + return; + } + if (!StringUtils.hasText(target.getUserInfoUri()) + && !StringUtils.hasText(target.getTokenInfoUri())) { + errors.rejectValue("tokenInfoUri", "missing.tokenInfoUri", + "Missing tokenInfoUri and userInfoUri and there is no " + + "JWT verifier key"); + } + if (StringUtils.hasText(target.getTokenInfoUri()) && isPreferTokenInfo()) { + if (StringUtils.hasText(this.clientId) + && !StringUtils.hasText(this.clientSecret)) { + errors.rejectValue("clientSecret", "missing.clientSecret", + "Missing client secret"); } } } @@ -276,21 +270,7 @@ public class ResourceServerProperties implements Validator, BeanFactoryAware { } public String getKeyUri() { - if (this.keyUri != null) { - return this.keyUri; - } - if (ResourceServerProperties.this.userInfoUri != null - && ResourceServerProperties.this.userInfoUri.endsWith("/userinfo")) { - return ResourceServerProperties.this.userInfoUri.replace("/userinfo", - "/token_key"); - } - if (ResourceServerProperties.this.tokenInfoUri != null - && ResourceServerProperties.this.tokenInfoUri - .endsWith("/check_token")) { - return ResourceServerProperties.this.tokenInfoUri.replace("/check_token", - "/token_key"); - } - return null; + return this.keyUri; } } 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 d2a8ed047e..5c3df8890e 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 @@ -42,6 +42,8 @@ public class ResourceServerPropertiesTests { private ResourceServerProperties properties = new ResourceServerProperties("client", "secret"); + private Errors errors = mock(Errors.class); + @Test @SuppressWarnings("unchecked") public void json() throws Exception { @@ -54,39 +56,109 @@ public class ResourceServerPropertiesTests { } @Test - public void tokenKeyDerivedFromUserInfoUri() throws Exception { - this.properties.setUserInfoUri("http://example.com/userinfo"); - assertThat(this.properties.getJwt().getKeyUri()) - .isEqualTo("http://example.com/token_key"); + public void validateWhenBothJwtAndJwkKeyUrisPresentShouldFail() throws Exception { + this.properties.getJwk().setKeySetUri("http://my-auth-server/token_keys"); + this.properties.getJwt().setKeyUri("http://my-auth-server/token_key"); + setListableBeanFactory(); + this.properties.validate(this.properties, this.errors); + verify(this.errors).reject("ambiguous.keyUri", + "Only one of jwt.keyUri (or jwt.keyValue) and jwk.keySetUri should be configured."); } @Test - public void tokenKeyDerivedFromTokenInfoUri() throws Exception { - this.properties.setTokenInfoUri("http://example.com/check_token"); - assertThat(this.properties.getJwt().getKeyUri()) - .isEqualTo("http://example.com/token_key"); + public void validateWhenBothJwtKeyValueAndJwkKeyUriPresentShouldFail() + throws Exception { + this.properties.getJwk().setKeySetUri("http://my-auth-server/token_keys"); + this.properties.getJwt().setKeyValue("my-key"); + setListableBeanFactory(); + this.properties.validate(this.properties, this.errors); + verify(this.errors).reject("ambiguous.keyUri", + "Only one of jwt.keyUri (or jwt.keyValue) and jwk.keySetUri should be configured."); } @Test - public void validateWhenBothJwtAndJwtKeyConfigurationPresentShouldFail() - throws Exception { + public void validateWhenJwkKeySetUriProvidedShouldSucceed() 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."); + this.properties.validate(this.properties, this.errors); + verifyZeroInteractions(this.errors); + } + @Test + public void validateWhenKeyValuePresentShouldSucceed() throws Exception { + this.properties.getJwt().setKeyValue("my-key"); + setListableBeanFactory(); + this.properties.validate(this.properties, this.errors); + verifyZeroInteractions(this.errors); } @Test - public void validateWhenKeySetUriProvidedShouldSucceed() throws Exception { + public void validateWhenKeysUriOrValuePresentAndUserInfoAbsentShouldNotFail() + throws Exception { + this.properties = new ResourceServerProperties("client", ""); this.properties.getJwk().setKeySetUri("http://my-auth-server/token_keys"); setListableBeanFactory(); - Errors errors = mock(Errors.class); - this.properties.validate(this.properties, errors); - verifyZeroInteractions(errors); + this.properties.validate(this.properties, this.errors); + verifyZeroInteractions(this.errors); + } + + @Test + public void validateWhenKeyConfigAbsentAndInfoUrisNotConfiguredShouldFail() + throws Exception { + setListableBeanFactory(); + this.properties.validate(this.properties, this.errors); + verify(this.errors).rejectValue("tokenInfoUri", "missing.tokenInfoUri", + "Missing tokenInfoUri and userInfoUri and there is no JWT verifier key"); + } + + @Test + public void validateWhenTokenUriConfiguredShouldNotFail() throws Exception { + this.properties.setTokenInfoUri("http://my-auth-server/userinfo"); + setListableBeanFactory(); + this.properties.validate(this.properties, this.errors); + verifyZeroInteractions(this.errors); + } + + @Test + public void validateWhenUserInfoUriConfiguredShouldNotFail() throws Exception { + this.properties.setUserInfoUri("http://my-auth-server/userinfo"); + setListableBeanFactory(); + this.properties.validate(this.properties, this.errors); + verifyZeroInteractions(this.errors); + } + + @Test + public void validateWhenTokenUriPreferredAndClientSecretAbsentShouldFail() + throws Exception { + this.properties = new ResourceServerProperties("client", ""); + this.properties.setTokenInfoUri("http://my-auth-server/check_token"); + this.properties.setUserInfoUri("http://my-auth-server/userinfo"); + setListableBeanFactory(); + this.properties.validate(this.properties, this.errors); + verify(this.errors).rejectValue("clientSecret", "missing.clientSecret", + "Missing client secret"); + } + + @Test + public void validateWhenTokenUriAbsentAndClientSecretAbsentShouldNotFail() + throws Exception { + this.properties = new ResourceServerProperties("client", ""); + this.properties.setUserInfoUri("http://my-auth-server/userinfo"); + setListableBeanFactory(); + this.properties.validate(this.properties, this.errors); + verifyZeroInteractions(this.errors); + } + + @Test + public void validateWhenTokenUriNotPreferredAndClientSecretAbsentShouldNotFail() + throws Exception { + this.properties = new ResourceServerProperties("client", ""); + this.properties.setPreferTokenInfo(false); + this.properties.setTokenInfoUri("http://my-auth-server/check_token"); + this.properties.setUserInfoUri("http://my-auth-server/userinfo"); + setListableBeanFactory(); + this.properties.validate(this.properties, this.errors); + verifyZeroInteractions(this.errors); } private void setListableBeanFactory() { 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 0fc52abec0..54a988619b 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 @@ -90,14 +90,6 @@ public class ResourceServerTokenServicesConfigurationTests { } } - @Test - public void defaultIsRemoteTokenServices() { - this.context = new SpringApplicationBuilder(ResourceConfiguration.class) - .web(false).run(); - RemoteTokenServices services = this.context.getBean(RemoteTokenServices.class); - assertThat(services).isNotNull(); - } - @Test public void useRemoteTokenServices() { EnvironmentTestUtils.addEnvironment(this.environment,