Skip to content

Commit 605b0ae

Browse files
mbhavewilkinsona
authored andcommitted
Simplify validation logic in ResourceServerProperties
Closes gh-8306 Closes gh-8317
1 parent 3e0e4e8 commit 605b0ae

File tree

3 files changed

+114
-70
lines changed

3 files changed

+114
-70
lines changed

spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/security/oauth2/resource/ResourceServerProperties.java

Lines changed: 23 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -204,40 +204,34 @@ public void validate(Object target, Errors errors) {
204204
}
205205
ResourceServerProperties resource = (ResourceServerProperties) target;
206206
validate(resource, errors);
207-
208207
}
209208

210209
private void validate(ResourceServerProperties target, Errors errors) {
211-
if ((StringUtils.hasText(this.jwt.getKeyUri())
212-
|| StringUtils.hasText(this.jwt.getKeyValue()))
213-
&& StringUtils.hasText(this.jwk.getKeySetUri())) {
210+
boolean jwtConfigPresent = StringUtils.hasText(this.jwt.getKeyUri())
211+
|| StringUtils.hasText(this.jwt.getKeyValue());
212+
boolean jwkConfigPresent = StringUtils.hasText(this.jwk.getKeySetUri());
213+
214+
if (jwtConfigPresent && jwkConfigPresent) {
214215
errors.reject("ambiguous.keyUri",
215-
"Only one of jwt.keyUri (or jwt.keyValue) and "
216-
+ "jwk.keySetUri should be configured.");
216+
"Only one of jwt.keyUri (or jwt.keyValue) and jwk.keySetUri should"
217+
+ " be configured.");
217218
}
218219
else {
219-
if (StringUtils.hasText(this.clientId)) {
220-
if (!StringUtils.hasText(this.clientSecret)) {
221-
if (!StringUtils.hasText(target.getUserInfoUri())) {
222-
errors.rejectValue("userInfoUri", "missing.userInfoUri",
223-
"Missing userInfoUri (no client secret available)");
224-
}
225-
}
226-
else {
227-
if (isPreferTokenInfo()
228-
&& !StringUtils.hasText(target.getTokenInfoUri())) {
229-
if (StringUtils.hasText(getJwt().getKeyUri())
230-
|| StringUtils.hasText(getJwt().getKeyValue())
231-
|| StringUtils.hasText(getJwk().getKeySetUri())) {
232-
// It's a JWT decoder
233-
return;
234-
}
235-
if (!StringUtils.hasText(target.getUserInfoUri())) {
236-
errors.rejectValue("tokenInfoUri", "missing.tokenInfoUri",
237-
"Missing tokenInfoUri and userInfoUri and there is no "
238-
+ "JWT verifier key");
239-
}
240-
}
220+
if (jwtConfigPresent || jwkConfigPresent) {
221+
// It's a JWT decoder
222+
return;
223+
}
224+
if (!StringUtils.hasText(target.getUserInfoUri())
225+
&& !StringUtils.hasText(target.getTokenInfoUri())) {
226+
errors.rejectValue("tokenInfoUri", "missing.tokenInfoUri",
227+
"Missing tokenInfoUri and userInfoUri and there is no "
228+
+ "JWT verifier key");
229+
}
230+
if (StringUtils.hasText(target.getTokenInfoUri()) && isPreferTokenInfo()) {
231+
if (StringUtils.hasText(this.clientId)
232+
&& !StringUtils.hasText(this.clientSecret)) {
233+
errors.rejectValue("clientSecret", "missing.clientSecret",
234+
"Missing client secret");
241235
}
242236
}
243237
}
@@ -276,21 +270,7 @@ public void setKeyUri(String keyUri) {
276270
}
277271

278272
public String getKeyUri() {
279-
if (this.keyUri != null) {
280-
return this.keyUri;
281-
}
282-
if (ResourceServerProperties.this.userInfoUri != null
283-
&& ResourceServerProperties.this.userInfoUri.endsWith("/userinfo")) {
284-
return ResourceServerProperties.this.userInfoUri.replace("/userinfo",
285-
"/token_key");
286-
}
287-
if (ResourceServerProperties.this.tokenInfoUri != null
288-
&& ResourceServerProperties.this.tokenInfoUri
289-
.endsWith("/check_token")) {
290-
return ResourceServerProperties.this.tokenInfoUri.replace("/check_token",
291-
"/token_key");
292-
}
293-
return null;
273+
return this.keyUri;
294274
}
295275

296276
}

spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/security/oauth2/resource/ResourceServerPropertiesTests.java

Lines changed: 91 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,8 @@ public class ResourceServerPropertiesTests {
4242
private ResourceServerProperties properties = new ResourceServerProperties("client",
4343
"secret");
4444

45+
private Errors errors = mock(Errors.class);
46+
4547
@Test
4648
@SuppressWarnings("unchecked")
4749
public void json() throws Exception {
@@ -54,39 +56,109 @@ public void json() throws Exception {
5456
}
5557

5658
@Test
57-
public void tokenKeyDerivedFromUserInfoUri() throws Exception {
58-
this.properties.setUserInfoUri("http://example.com/userinfo");
59-
assertThat(this.properties.getJwt().getKeyUri())
60-
.isEqualTo("http://example.com/token_key");
59+
public void validateWhenBothJwtAndJwkKeyUrisPresentShouldFail() throws Exception {
60+
this.properties.getJwk().setKeySetUri("http://my-auth-server/token_keys");
61+
this.properties.getJwt().setKeyUri("http://my-auth-server/token_key");
62+
setListableBeanFactory();
63+
this.properties.validate(this.properties, this.errors);
64+
verify(this.errors).reject("ambiguous.keyUri",
65+
"Only one of jwt.keyUri (or jwt.keyValue) and jwk.keySetUri should be configured.");
6166
}
6267

6368
@Test
64-
public void tokenKeyDerivedFromTokenInfoUri() throws Exception {
65-
this.properties.setTokenInfoUri("http://example.com/check_token");
66-
assertThat(this.properties.getJwt().getKeyUri())
67-
.isEqualTo("http://example.com/token_key");
69+
public void validateWhenBothJwtKeyValueAndJwkKeyUriPresentShouldFail()
70+
throws Exception {
71+
this.properties.getJwk().setKeySetUri("http://my-auth-server/token_keys");
72+
this.properties.getJwt().setKeyValue("my-key");
73+
setListableBeanFactory();
74+
this.properties.validate(this.properties, this.errors);
75+
verify(this.errors).reject("ambiguous.keyUri",
76+
"Only one of jwt.keyUri (or jwt.keyValue) and jwk.keySetUri should be configured.");
6877
}
6978

7079
@Test
71-
public void validateWhenBothJwtAndJwtKeyConfigurationPresentShouldFail()
72-
throws Exception {
80+
public void validateWhenJwkKeySetUriProvidedShouldSucceed() throws Exception {
7381
this.properties.getJwk().setKeySetUri("http://my-auth-server/token_keys");
74-
this.properties.getJwt().setKeyUri("http://my-auth-server/token_key");
7582
setListableBeanFactory();
76-
Errors errors = mock(Errors.class);
77-
this.properties.validate(this.properties, errors);
78-
verify(errors).reject("ambiguous.keyUri",
79-
"Only one of jwt.keyUri (or jwt.keyValue) and jwk.keySetUri should be configured.");
83+
this.properties.validate(this.properties, this.errors);
84+
verifyZeroInteractions(this.errors);
85+
}
8086

87+
@Test
88+
public void validateWhenKeyValuePresentShouldSucceed() throws Exception {
89+
this.properties.getJwt().setKeyValue("my-key");
90+
setListableBeanFactory();
91+
this.properties.validate(this.properties, this.errors);
92+
verifyZeroInteractions(this.errors);
8193
}
8294

8395
@Test
84-
public void validateWhenKeySetUriProvidedShouldSucceed() throws Exception {
96+
public void validateWhenKeysUriOrValuePresentAndUserInfoAbsentShouldNotFail()
97+
throws Exception {
98+
this.properties = new ResourceServerProperties("client", "");
8599
this.properties.getJwk().setKeySetUri("http://my-auth-server/token_keys");
86100
setListableBeanFactory();
87-
Errors errors = mock(Errors.class);
88-
this.properties.validate(this.properties, errors);
89-
verifyZeroInteractions(errors);
101+
this.properties.validate(this.properties, this.errors);
102+
verifyZeroInteractions(this.errors);
103+
}
104+
105+
@Test
106+
public void validateWhenKeyConfigAbsentAndInfoUrisNotConfiguredShouldFail()
107+
throws Exception {
108+
setListableBeanFactory();
109+
this.properties.validate(this.properties, this.errors);
110+
verify(this.errors).rejectValue("tokenInfoUri", "missing.tokenInfoUri",
111+
"Missing tokenInfoUri and userInfoUri and there is no JWT verifier key");
112+
}
113+
114+
@Test
115+
public void validateWhenTokenUriConfiguredShouldNotFail() throws Exception {
116+
this.properties.setTokenInfoUri("http://my-auth-server/userinfo");
117+
setListableBeanFactory();
118+
this.properties.validate(this.properties, this.errors);
119+
verifyZeroInteractions(this.errors);
120+
}
121+
122+
@Test
123+
public void validateWhenUserInfoUriConfiguredShouldNotFail() throws Exception {
124+
this.properties.setUserInfoUri("http://my-auth-server/userinfo");
125+
setListableBeanFactory();
126+
this.properties.validate(this.properties, this.errors);
127+
verifyZeroInteractions(this.errors);
128+
}
129+
130+
@Test
131+
public void validateWhenTokenUriPreferredAndClientSecretAbsentShouldFail()
132+
throws Exception {
133+
this.properties = new ResourceServerProperties("client", "");
134+
this.properties.setTokenInfoUri("http://my-auth-server/check_token");
135+
this.properties.setUserInfoUri("http://my-auth-server/userinfo");
136+
setListableBeanFactory();
137+
this.properties.validate(this.properties, this.errors);
138+
verify(this.errors).rejectValue("clientSecret", "missing.clientSecret",
139+
"Missing client secret");
140+
}
141+
142+
@Test
143+
public void validateWhenTokenUriAbsentAndClientSecretAbsentShouldNotFail()
144+
throws Exception {
145+
this.properties = new ResourceServerProperties("client", "");
146+
this.properties.setUserInfoUri("http://my-auth-server/userinfo");
147+
setListableBeanFactory();
148+
this.properties.validate(this.properties, this.errors);
149+
verifyZeroInteractions(this.errors);
150+
}
151+
152+
@Test
153+
public void validateWhenTokenUriNotPreferredAndClientSecretAbsentShouldNotFail()
154+
throws Exception {
155+
this.properties = new ResourceServerProperties("client", "");
156+
this.properties.setPreferTokenInfo(false);
157+
this.properties.setTokenInfoUri("http://my-auth-server/check_token");
158+
this.properties.setUserInfoUri("http://my-auth-server/userinfo");
159+
setListableBeanFactory();
160+
this.properties.validate(this.properties, this.errors);
161+
verifyZeroInteractions(this.errors);
90162
}
91163

92164
private void setListableBeanFactory() {

spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/security/oauth2/resource/ResourceServerTokenServicesConfigurationTests.java

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -90,14 +90,6 @@ public void close() {
9090
}
9191
}
9292

93-
@Test
94-
public void defaultIsRemoteTokenServices() {
95-
this.context = new SpringApplicationBuilder(ResourceConfiguration.class)
96-
.web(false).run();
97-
RemoteTokenServices services = this.context.getBean(RemoteTokenServices.class);
98-
assertThat(services).isNotNull();
99-
}
100-
10193
@Test
10294
public void useRemoteTokenServices() {
10395
EnvironmentTestUtils.addEnvironment(this.environment,

0 commit comments

Comments
 (0)