Skip to content

Commit 17f1540

Browse files
evgeniychebanjgrandja
evgeniycheban
authored and
jgrandja
committed
Resolve oauth2 client placeholders
Closes gh-8453
1 parent 4e1fcd3 commit 17f1540

File tree

3 files changed

+88
-35
lines changed

3 files changed

+88
-35
lines changed

‎config/src/main/java/org/springframework/security/config/oauth2/client/ClientRegistrationsBeanDefinitionParser.java

Lines changed: 41 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@
4242

4343
/**
4444
* @author Ruby Hartono
45+
* @author Evgeniy Cheban
4546
* @since 5.3
4647
*/
4748
public final class ClientRegistrationsBeanDefinitionParser implements BeanDefinitionParser {
@@ -87,7 +88,7 @@ public BeanDefinition parse(Element element, ParserContext parserContext) {
8788
CompositeComponentDefinition compositeDef = new CompositeComponentDefinition(element.getTagName(),
8889
parserContext.extractSource(element));
8990
parserContext.pushContainingComponent(compositeDef);
90-
Map<String, Map<String, String>> providers = getProviders(element);
91+
Map<String, Map<String, String>> providers = getProviders(element, parserContext);
9192
List<ClientRegistration> clientRegistrations = getClientRegistrations(element, parserContext, providers);
9293
BeanDefinition clientRegistrationRepositoryBean = BeanDefinitionBuilder
9394
.rootBeanDefinition(InMemoryClientRegistrationRepository.class)
@@ -107,75 +108,79 @@ private List<ClientRegistration> getClientRegistrations(Element element, ParserC
107108
for (Element clientRegistrationElt : clientRegistrationElts) {
108109
String registrationId = clientRegistrationElt.getAttribute(ATT_REGISTRATION_ID);
109110
String providerId = clientRegistrationElt.getAttribute(ATT_PROVIDER_ID);
110-
ClientRegistration.Builder builder = getBuilderFromIssuerIfPossible(registrationId, providerId, providers);
111+
ClientRegistration.Builder builder = getBuilderFromIssuerIfPossible(parserContext, registrationId,
112+
providerId, providers);
111113
if (builder == null) {
112-
builder = getBuilder(registrationId, providerId, providers);
114+
builder = getBuilder(parserContext, registrationId, providerId, providers);
113115
if (builder == null) {
114116
Object source = parserContext.extractSource(element);
115117
parserContext.getReaderContext().error(getErrorMessage(providerId, registrationId), source);
116118
// error on the config skip to next element
117119
continue;
118120
}
119121
}
120-
getOptionalIfNotEmpty(clientRegistrationElt.getAttribute(ATT_CLIENT_ID)).ifPresent(builder::clientId);
121-
getOptionalIfNotEmpty(clientRegistrationElt.getAttribute(ATT_CLIENT_SECRET))
122+
getOptionalIfNotEmpty(parserContext, clientRegistrationElt.getAttribute(ATT_CLIENT_ID))
123+
.ifPresent(builder::clientId);
124+
getOptionalIfNotEmpty(parserContext, clientRegistrationElt.getAttribute(ATT_CLIENT_SECRET))
122125
.ifPresent(builder::clientSecret);
123-
getOptionalIfNotEmpty(clientRegistrationElt.getAttribute(ATT_CLIENT_AUTHENTICATION_METHOD))
126+
getOptionalIfNotEmpty(parserContext, clientRegistrationElt.getAttribute(ATT_CLIENT_AUTHENTICATION_METHOD))
124127
.map(ClientAuthenticationMethod::new).ifPresent(builder::clientAuthenticationMethod);
125-
getOptionalIfNotEmpty(clientRegistrationElt.getAttribute(ATT_AUTHORIZATION_GRANT_TYPE))
128+
getOptionalIfNotEmpty(parserContext, clientRegistrationElt.getAttribute(ATT_AUTHORIZATION_GRANT_TYPE))
126129
.map(AuthorizationGrantType::new).ifPresent(builder::authorizationGrantType);
127-
getOptionalIfNotEmpty(clientRegistrationElt.getAttribute(ATT_REDIRECT_URI)).ifPresent(builder::redirectUri);
128-
getOptionalIfNotEmpty(clientRegistrationElt.getAttribute(ATT_SCOPE))
130+
getOptionalIfNotEmpty(parserContext, clientRegistrationElt.getAttribute(ATT_REDIRECT_URI))
131+
.ifPresent(builder::redirectUri);
132+
getOptionalIfNotEmpty(parserContext, clientRegistrationElt.getAttribute(ATT_SCOPE))
129133
.map(StringUtils::commaDelimitedListToSet).ifPresent(builder::scope);
130-
getOptionalIfNotEmpty(clientRegistrationElt.getAttribute(ATT_CLIENT_NAME)).ifPresent(builder::clientName);
134+
getOptionalIfNotEmpty(parserContext, clientRegistrationElt.getAttribute(ATT_CLIENT_NAME))
135+
.ifPresent(builder::clientName);
131136
clientRegistrations.add(builder.build());
132137
}
133138
return clientRegistrations;
134139
}
135140

136-
private Map<String, Map<String, String>> getProviders(Element element) {
141+
private Map<String, Map<String, String>> getProviders(Element element, ParserContext parserContext) {
137142
List<Element> providerElts = DomUtils.getChildElementsByTagName(element, ELT_PROVIDER);
138143
Map<String, Map<String, String>> providers = new HashMap<>();
139144
for (Element providerElt : providerElts) {
140145
Map<String, String> provider = new HashMap<>();
141146
String providerId = providerElt.getAttribute(ATT_PROVIDER_ID);
142147
provider.put(ATT_PROVIDER_ID, providerId);
143-
getOptionalIfNotEmpty(providerElt.getAttribute(ATT_AUTHORIZATION_URI))
148+
getOptionalIfNotEmpty(parserContext, providerElt.getAttribute(ATT_AUTHORIZATION_URI))
144149
.ifPresent((value) -> provider.put(ATT_AUTHORIZATION_URI, value));
145-
getOptionalIfNotEmpty(providerElt.getAttribute(ATT_TOKEN_URI))
150+
getOptionalIfNotEmpty(parserContext, providerElt.getAttribute(ATT_TOKEN_URI))
146151
.ifPresent((value) -> provider.put(ATT_TOKEN_URI, value));
147-
getOptionalIfNotEmpty(providerElt.getAttribute(ATT_USER_INFO_URI))
152+
getOptionalIfNotEmpty(parserContext, providerElt.getAttribute(ATT_USER_INFO_URI))
148153
.ifPresent((value) -> provider.put(ATT_USER_INFO_URI, value));
149-
getOptionalIfNotEmpty(providerElt.getAttribute(ATT_USER_INFO_AUTHENTICATION_METHOD))
154+
getOptionalIfNotEmpty(parserContext, providerElt.getAttribute(ATT_USER_INFO_AUTHENTICATION_METHOD))
150155
.ifPresent((value) -> provider.put(ATT_USER_INFO_AUTHENTICATION_METHOD, value));
151-
getOptionalIfNotEmpty(providerElt.getAttribute(ATT_USER_INFO_USER_NAME_ATTRIBUTE))
156+
getOptionalIfNotEmpty(parserContext, providerElt.getAttribute(ATT_USER_INFO_USER_NAME_ATTRIBUTE))
152157
.ifPresent((value) -> provider.put(ATT_USER_INFO_USER_NAME_ATTRIBUTE, value));
153-
getOptionalIfNotEmpty(providerElt.getAttribute(ATT_JWK_SET_URI))
158+
getOptionalIfNotEmpty(parserContext, providerElt.getAttribute(ATT_JWK_SET_URI))
154159
.ifPresent((value) -> provider.put(ATT_JWK_SET_URI, value));
155-
getOptionalIfNotEmpty(providerElt.getAttribute(ATT_ISSUER_URI))
160+
getOptionalIfNotEmpty(parserContext, providerElt.getAttribute(ATT_ISSUER_URI))
156161
.ifPresent((value) -> provider.put(ATT_ISSUER_URI, value));
157162
providers.put(providerId, provider);
158163
}
159164
return providers;
160165
}
161166

162-
private static ClientRegistration.Builder getBuilderFromIssuerIfPossible(String registrationId,
163-
String configuredProviderId, Map<String, Map<String, String>> providers) {
167+
private static ClientRegistration.Builder getBuilderFromIssuerIfPossible(ParserContext parserContext,
168+
String registrationId, String configuredProviderId, Map<String, Map<String, String>> providers) {
164169
String providerId = (configuredProviderId != null) ? configuredProviderId : registrationId;
165170
if (providers.containsKey(providerId)) {
166171
Map<String, String> provider = providers.get(providerId);
167172
String issuer = provider.get(ATT_ISSUER_URI);
168173
if (!StringUtils.isEmpty(issuer)) {
169174
ClientRegistration.Builder builder = ClientRegistrations.fromIssuerLocation(issuer)
170175
.registrationId(registrationId);
171-
return getBuilder(builder, provider);
176+
return getBuilder(parserContext, builder, provider);
172177
}
173178
}
174179
return null;
175180
}
176181

177-
private static ClientRegistration.Builder getBuilder(String registrationId, String configuredProviderId,
178-
Map<String, Map<String, String>> providers) {
182+
private static ClientRegistration.Builder getBuilder(ParserContext parserContext, String registrationId,
183+
String configuredProviderId, Map<String, Map<String, String>> providers) {
179184
String providerId = (configuredProviderId != null) ? configuredProviderId : registrationId;
180185
CommonOAuth2Provider provider = getCommonProvider(providerId);
181186
if (provider == null && !providers.containsKey(providerId)) {
@@ -184,26 +189,27 @@ private static ClientRegistration.Builder getBuilder(String registrationId, Stri
184189
ClientRegistration.Builder builder = (provider != null) ? provider.getBuilder(registrationId)
185190
: ClientRegistration.withRegistrationId(registrationId);
186191
if (providers.containsKey(providerId)) {
187-
return getBuilder(builder, providers.get(providerId));
192+
return getBuilder(parserContext, builder, providers.get(providerId));
188193
}
189194
return builder;
190195
}
191196

192-
private static ClientRegistration.Builder getBuilder(ClientRegistration.Builder builder,
193-
Map<String, String> provider) {
194-
getOptionalIfNotEmpty(provider.get(ATT_AUTHORIZATION_URI)).ifPresent(builder::authorizationUri);
195-
getOptionalIfNotEmpty(provider.get(ATT_TOKEN_URI)).ifPresent(builder::tokenUri);
196-
getOptionalIfNotEmpty(provider.get(ATT_USER_INFO_URI)).ifPresent(builder::userInfoUri);
197-
getOptionalIfNotEmpty(provider.get(ATT_USER_INFO_AUTHENTICATION_METHOD)).map(AuthenticationMethod::new)
198-
.ifPresent(builder::userInfoAuthenticationMethod);
199-
getOptionalIfNotEmpty(provider.get(ATT_JWK_SET_URI)).ifPresent(builder::jwkSetUri);
200-
getOptionalIfNotEmpty(provider.get(ATT_USER_INFO_USER_NAME_ATTRIBUTE))
197+
private static ClientRegistration.Builder getBuilder(ParserContext parserContext,
198+
ClientRegistration.Builder builder, Map<String, String> provider) {
199+
getOptionalIfNotEmpty(parserContext, provider.get(ATT_AUTHORIZATION_URI)).ifPresent(builder::authorizationUri);
200+
getOptionalIfNotEmpty(parserContext, provider.get(ATT_TOKEN_URI)).ifPresent(builder::tokenUri);
201+
getOptionalIfNotEmpty(parserContext, provider.get(ATT_USER_INFO_URI)).ifPresent(builder::userInfoUri);
202+
getOptionalIfNotEmpty(parserContext, provider.get(ATT_USER_INFO_AUTHENTICATION_METHOD))
203+
.map(AuthenticationMethod::new).ifPresent(builder::userInfoAuthenticationMethod);
204+
getOptionalIfNotEmpty(parserContext, provider.get(ATT_JWK_SET_URI)).ifPresent(builder::jwkSetUri);
205+
getOptionalIfNotEmpty(parserContext, provider.get(ATT_USER_INFO_USER_NAME_ATTRIBUTE))
201206
.ifPresent(builder::userNameAttributeName);
202207
return builder;
203208
}
204209

205-
private static Optional<String> getOptionalIfNotEmpty(String str) {
206-
return Optional.ofNullable(str).filter((s) -> !s.isEmpty());
210+
private static Optional<String> getOptionalIfNotEmpty(ParserContext parserContext, String str) {
211+
return Optional.ofNullable(str).filter((s) -> !s.isEmpty())
212+
.map(parserContext.getReaderContext().getEnvironment()::resolvePlaceholders);
207213
}
208214

209215
private static CommonOAuth2Provider getCommonProvider(String providerId) {

‎config/src/test/java/org/springframework/security/config/oauth2/client/ClientRegistrationsBeanDefinitionParserTests.java

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
* Tests for {@link ClientRegistrationsBeanDefinitionParser}.
4242
*
4343
* @author Ruby Hartono
44+
* @author Evgeniy Cheban
4445
*/
4546
public class ClientRegistrationsBeanDefinitionParserTests {
4647

@@ -218,6 +219,20 @@ public void parseWhenMultipleClientsConfiguredThenAvailableInRepository() {
218219
assertThat(githubProviderDetails.getUserInfoEndpoint().getUserNameAttributeName()).isEqualTo("id");
219220
}
220221

222+
@Test
223+
public void parseWhenClientPlaceholdersThenResolvePlaceholders() {
224+
System.setProperty("oauth2.client.id", "github-client-id");
225+
System.setProperty("oauth2.client.secret", "github-client-secret");
226+
227+
this.spring.configLocations(xml("ClientPlaceholders")).autowire();
228+
229+
assertThat(this.clientRegistrationRepository).isInstanceOf(InMemoryClientRegistrationRepository.class);
230+
231+
ClientRegistration githubRegistration = this.clientRegistrationRepository.findByRegistrationId("github");
232+
assertThat(githubRegistration.getClientId()).isEqualTo("github-client-id");
233+
assertThat(githubRegistration.getClientSecret()).isEqualTo("github-client-secret");
234+
}
235+
221236
private static MockResponse jsonResponse(String json) {
222237
return new MockResponse().setHeader(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_JSON_VALUE).setBody(json);
223238
}

‎config/src/test/resources/org/springframework/security/config/oauth2/client/ClientRegistrationsBeanDefinitionParserTests-ClientPlaceholders.xml

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<!--
3+
~ Copyright 2002-2020 the original author or authors.
4+
~
5+
~ Licensed under the Apache License, Version 2.0 (the "License");
6+
~ you may not use this file except in compliance with the License.
7+
~ You may obtain a copy of the License at
8+
~
9+
~ https://www.apache.org/licenses/LICENSE-2.0
10+
~
11+
~ Unless required by applicable law or agreed to in writing, software
12+
~ distributed under the License is distributed on an "AS IS" BASIS,
13+
~ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
~ See the License for the specific language governing permissions and
15+
~ limitations under the License.
16+
-->
17+
18+
<b:beans xmlns:b="http://www.springframework.org/schema/beans"
19+
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
20+
xmlns="http://www.springframework.org/schema/security"
21+
xsi:schemaLocation="
22+
http://www.springframework.org/schema/security
23+
https://www.springframework.org/schema/security/spring-security.xsd
24+
http://www.springframework.org/schema/beans
25+
https://www.springframework.org/schema/beans/spring-beans.xsd">
26+
<client-registrations>
27+
<client-registration registration-id="github"
28+
client-id="${oauth2.client.id}"
29+
client-secret="${oauth2.client.secret}"
30+
provider-id="github"/>
31+
</client-registrations>
32+
</b:beans>

0 commit comments

Comments
 (0)