From 899637157b314c926f97a15787bc8ce3710bae16 Mon Sep 17 00:00:00 2001 From: Steve Riesenberg <5248162+sjohnr@users.noreply.github.com> Date: Wed, 18 Sep 2024 10:50:37 -0500 Subject: [PATCH] Allow opt-in to omitting default parameters Closes gh-11298 --- ...activeOAuth2AccessTokenResponseClient.java | 48 ++++++--- ...OAuth2TokenRequestParametersConverter.java | 98 +++++++++++++++++++ 2 files changed, 133 insertions(+), 13 deletions(-) create mode 100644 oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/endpoint/DefaultOAuth2TokenRequestParametersConverter.java diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/endpoint/AbstractWebClientReactiveOAuth2AccessTokenResponseClient.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/endpoint/AbstractWebClientReactiveOAuth2AccessTokenResponseClient.java index 0e126b5efa..4360451841 100644 --- a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/endpoint/AbstractWebClientReactiveOAuth2AccessTokenResponseClient.java +++ b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/endpoint/AbstractWebClientReactiveOAuth2AccessTokenResponseClient.java @@ -24,7 +24,6 @@ import org.springframework.security.oauth2.client.registration.ClientRegistration; import org.springframework.security.oauth2.core.ClientAuthenticationMethod; import org.springframework.security.oauth2.core.endpoint.OAuth2AccessTokenResponse; -import org.springframework.security.oauth2.core.endpoint.OAuth2ParameterNames; import org.springframework.security.oauth2.core.web.reactive.function.OAuth2BodyExtractors; import org.springframework.util.Assert; import org.springframework.util.LinkedMultiValueMap; @@ -67,6 +66,8 @@ public abstract class AbstractWebClientReactiveOAuth2AccessTokenResponseClient headersConverter = new DefaultOAuth2TokenRequestHeadersConverter<>(); + private final Converter> defaultParametersConverter = new DefaultOAuth2TokenRequestParametersConverter<>(); + private Converter> parametersConverter = this::createParameters; private BodyExtractor, ReactiveHttpInputMessage> bodyExtractor = OAuth2BodyExtractors @@ -125,17 +126,7 @@ private RequestHeadersSpec populateRequest(T grantRequest) { * Token Request body */ MultiValueMap createParameters(T grantRequest) { - ClientRegistration clientRegistration = grantRequest.getClientRegistration(); - MultiValueMap parameters = new LinkedMultiValueMap<>(); - parameters.set(OAuth2ParameterNames.GRANT_TYPE, grantRequest.getGrantType().getValue()); - if (!ClientAuthenticationMethod.CLIENT_SECRET_BASIC - .equals(clientRegistration.getClientAuthenticationMethod())) { - parameters.set(OAuth2ParameterNames.CLIENT_ID, clientRegistration.getClientId()); - } - if (ClientAuthenticationMethod.CLIENT_SECRET_POST.equals(clientRegistration.getClientAuthenticationMethod())) { - parameters.set(OAuth2ParameterNames.CLIENT_SECRET, clientRegistration.getClientSecret()); - } - return parameters; + return this.defaultParametersConverter.convert(grantRequest); } /** @@ -195,13 +186,44 @@ public final void addHeadersConverter(Converter headersConverter * Sets the {@link Converter} used for converting the * {@link AbstractOAuth2AuthorizationGrantRequest} instance to a {@link MultiValueMap} * used in the OAuth 2.0 Access Token Request body. + *

+ * For backwards compatibility with Spring Security 6.3 (and earlier), this method + * ensures that default parameters for this particular grant type are provided if the + * given parameters converter does not supply them. In order to fully override or omit + * parameters, supply this method with an instance of + * {@link DefaultOAuth2TokenRequestParametersConverter} via + * {@link DefaultOAuth2TokenRequestParametersConverter#of(Converter)} and only the + * returned parameters will be provided. * @param parametersConverter the {@link Converter} used for converting the * {@link AbstractOAuth2AuthorizationGrantRequest} to {@link MultiValueMap} * @since 5.6 + * @see DefaultOAuth2TokenRequestParametersConverter#of(Converter) */ public final void setParametersConverter(Converter> parametersConverter) { Assert.notNull(parametersConverter, "parametersConverter cannot be null"); - this.parametersConverter = parametersConverter; + // Allow opting into new behavior of fully overriding parameter values when + // user provides instance of DefaultOAuth2TokenRequestParametersConverter. + if (parametersConverter instanceof DefaultOAuth2TokenRequestParametersConverter) { + this.parametersConverter = parametersConverter; + } + else { + // For backwards compatibility with 6.3, ensure default parameters are always + // populated but allow parameter values to be overridden if provided. + // TODO: Remove in Spring Security 7 + Converter> defaultParametersConverter = this::createParameters; + this.parametersConverter = (authorizationGrantRequest) -> { + MultiValueMap parameters = defaultParametersConverter + .convert(authorizationGrantRequest); + if (parameters == null) { + parameters = new LinkedMultiValueMap<>(); + } + MultiValueMap parametersToSet = parametersConverter.convert(authorizationGrantRequest); + if (parametersToSet != null) { + parameters.putAll(parametersToSet); + } + return parameters; + }; + } this.requestEntityConverter = this::populateRequest; } diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/endpoint/DefaultOAuth2TokenRequestParametersConverter.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/endpoint/DefaultOAuth2TokenRequestParametersConverter.java new file mode 100644 index 0000000000..e08dc524c4 --- /dev/null +++ b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/endpoint/DefaultOAuth2TokenRequestParametersConverter.java @@ -0,0 +1,98 @@ +/* + * Copyright 2002-2024 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.security.oauth2.client.endpoint; + +import org.springframework.core.convert.converter.Converter; +import org.springframework.security.oauth2.client.registration.ClientRegistration; +import org.springframework.security.oauth2.core.ClientAuthenticationMethod; +import org.springframework.security.oauth2.core.endpoint.OAuth2ParameterNames; +import org.springframework.util.Assert; +import org.springframework.util.LinkedMultiValueMap; +import org.springframework.util.MultiValueMap; + +/** + * Default {@link Converter} used to convert an + * {@link AbstractOAuth2AuthorizationGrantRequest} to the default {@link MultiValueMap + * parameters} of an OAuth 2.0 Access Token Request. + *

+ * This implementation does not provide grant-type specific parameters. The following + * parameters are provided: + * + *

    + *
  • {@code grant_type} - always provided
    • + *
  • {@code client_id} - provided unless the {@code clientAuthenticationMethod} is + * {@code client_secret_basic}
    • + *
  • {@code client_secret} - provided when the {@code clientAuthenticationMethod} is + * {@code client_secret_post}
    • + *
+ * + * @param type of grant request + * @author Steve Riesenberg + * @since 6.4 + * @see AbstractWebClientReactiveOAuth2AccessTokenResponseClient + * @see AbstractRestClientOAuth2AccessTokenResponseClient + */ +public final class DefaultOAuth2TokenRequestParametersConverter + implements Converter> { + + private final Converter> defaultParametersConverter; + + public DefaultOAuth2TokenRequestParametersConverter() { + this(DefaultOAuth2TokenRequestParametersConverter::defaultParameters); + } + + private DefaultOAuth2TokenRequestParametersConverter( + Converter> defaultParametersConverter) { + Assert.notNull(defaultParametersConverter, "defaultParametersConverter cannot be null"); + this.defaultParametersConverter = defaultParametersConverter; + } + + @Override + public MultiValueMap convert(T grantRequest) { + return this.defaultParametersConverter.convert(grantRequest); + } + + private static MultiValueMap defaultParameters( + T grantRequest) { + ClientRegistration clientRegistration = grantRequest.getClientRegistration(); + MultiValueMap parameters = new LinkedMultiValueMap<>(); + parameters.set(OAuth2ParameterNames.GRANT_TYPE, grantRequest.getGrantType().getValue()); + if (!ClientAuthenticationMethod.CLIENT_SECRET_BASIC + .equals(clientRegistration.getClientAuthenticationMethod())) { + parameters.set(OAuth2ParameterNames.CLIENT_ID, clientRegistration.getClientId()); + } + if (ClientAuthenticationMethod.CLIENT_SECRET_POST.equals(clientRegistration.getClientAuthenticationMethod())) { + parameters.set(OAuth2ParameterNames.CLIENT_SECRET, clientRegistration.getClientSecret()); + } + return parameters; + } + + /** + * Wrap a custom {@link Converter} in order to override or omit default parameters + * with + * {@link AbstractWebClientReactiveOAuth2AccessTokenResponseClient#setParametersConverter(Converter)}. + * @param defaultParametersConverter the {@link Converter} used for converting the + * @param type of grant request {@link AbstractOAuth2AuthorizationGrantRequest} to + * {@link MultiValueMap parameters} + * @return the wrapped {@link Converter} + */ + public static Converter> of( + Converter> defaultParametersConverter) { + return new DefaultOAuth2TokenRequestParametersConverter<>(defaultParametersConverter); + } + +}