From 76565cefcbc9b8c41c6d03c2001971cd3c080535 Mon Sep 17 00:00:00 2001 From: Rujun Chen Date: Wed, 12 Oct 2022 17:18:31 +0800 Subject: [PATCH 1/5] Fix #31191. duplicated "scope" parameter. --- ...zationCodeGrantRequestEntityConverter.java | 21 ++++++++++--------- ...nCodeGrantRequestEntityConverterTests.java | 19 ++++++++++++++++- 2 files changed, 29 insertions(+), 11 deletions(-) diff --git a/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/aad/implementation/oauth2/AbstractOAuth2AuthorizationCodeGrantRequestEntityConverter.java b/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/aad/implementation/oauth2/AbstractOAuth2AuthorizationCodeGrantRequestEntityConverter.java index af91381bb842..0d54f8e23d36 100644 --- a/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/aad/implementation/oauth2/AbstractOAuth2AuthorizationCodeGrantRequestEntityConverter.java +++ b/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/aad/implementation/oauth2/AbstractOAuth2AuthorizationCodeGrantRequestEntityConverter.java @@ -4,12 +4,12 @@ package com.azure.spring.cloud.autoconfigure.aad.implementation.oauth2; import com.azure.spring.cloud.core.implementation.util.AzureSpringIdentifier; -import org.springframework.core.convert.converter.Converter; import org.springframework.http.HttpHeaders; import org.springframework.http.RequestEntity; import org.springframework.security.oauth2.client.endpoint.OAuth2AuthorizationCodeGrantRequest; import org.springframework.security.oauth2.client.endpoint.OAuth2AuthorizationCodeGrantRequestEntityConverter; import org.springframework.util.MultiValueMap; +import org.springframework.util.MultiValueMapAdapter; import java.util.Collections; import java.util.UUID; @@ -20,6 +20,14 @@ public abstract class AbstractOAuth2AuthorizationCodeGrantRequestEntityConverter extends OAuth2AuthorizationCodeGrantRequestEntityConverter { + private static final MultiValueMap EMPTY_MULTI_VALUE_MAP = + new MultiValueMapAdapter<>(Collections.emptyMap()); + + protected AbstractOAuth2AuthorizationCodeGrantRequestEntityConverter() { + addHeadersConverter(this::getHttpHeaders); + addParametersConverter(this::getHttpBody); + } + /** * Gets the application ID. * @@ -28,22 +36,15 @@ public abstract class AbstractOAuth2AuthorizationCodeGrantRequestEntityConverter protected abstract String getApplicationId(); @Override - @SuppressWarnings("unchecked") public RequestEntity convert(OAuth2AuthorizationCodeGrantRequest request) { - addHeadersConverter(headersConverter); - addParametersConverter(parametersConverter); return super.convert(request); } - private final Converter headersConverter = (request) -> getHttpHeaders(); - - private final Converter> parametersConverter = this::getHttpBody; - /** * Additional default headers information. * @return HttpHeaders */ - public HttpHeaders getHttpHeaders() { + public HttpHeaders getHttpHeaders(OAuth2AuthorizationCodeGrantRequest request) { HttpHeaders httpHeaders = new HttpHeaders(); httpHeaders.put("x-client-SKU", Collections.singletonList(getApplicationId())); httpHeaders.put("x-client-VER", Collections.singletonList(AzureSpringIdentifier.VERSION)); @@ -57,6 +58,6 @@ public HttpHeaders getHttpHeaders() { * @return MultiValueMap */ public MultiValueMap getHttpBody(OAuth2AuthorizationCodeGrantRequest request) { - return null; + return EMPTY_MULTI_VALUE_MAP; } } diff --git a/sdk/spring/spring-cloud-azure-autoconfigure/src/test/java/com/azure/spring/cloud/autoconfigure/aad/implementation/webapp/AadOAuth2AuthorizationCodeGrantRequestEntityConverterTests.java b/sdk/spring/spring-cloud-azure-autoconfigure/src/test/java/com/azure/spring/cloud/autoconfigure/aad/implementation/webapp/AadOAuth2AuthorizationCodeGrantRequestEntityConverterTests.java index 1e110859858b..b5982dbed002 100644 --- a/sdk/spring/spring-cloud-azure-autoconfigure/src/test/java/com/azure/spring/cloud/autoconfigure/aad/implementation/webapp/AadOAuth2AuthorizationCodeGrantRequestEntityConverterTests.java +++ b/sdk/spring/spring-cloud-azure-autoconfigure/src/test/java/com/azure/spring/cloud/autoconfigure/aad/implementation/webapp/AadOAuth2AuthorizationCodeGrantRequestEntityConverterTests.java @@ -61,6 +61,23 @@ void addScopeForAuthorizationCodeClient() { }); } + @Test + void onlyAddScopeOnceEvenConvertMethodExecutedMultipleTimes() { + getContextRunner().run(context -> { + AadClientRegistrationRepository repository = + (AadClientRegistrationRepository) context.getBean(ClientRegistrationRepository.class); + AadOAuth2AuthorizationCodeGrantRequestEntityConverter converter = + new AadOAuth2AuthorizationCodeGrantRequestEntityConverter(repository.getAzureClientAccessTokenScopes()); + ClientRegistration azure = repository.findByRegistrationId(AZURE_CLIENT_REGISTRATION_ID); + OAuth2AuthorizationCodeGrantRequest request = createCodeGrantRequest(azure); + // Convert method execute 2 times + converter.convert(request); + RequestEntity entity = converter.convert(request); + MultiValueMap map = WebApplicationContextRunnerUtils.toMultiValueMap(entity); + assertEquals(1, map.get("scope").size()); + }); + } + @Test @SuppressWarnings("unchecked") void addHeadersForAzureClient() { @@ -97,7 +114,7 @@ private HttpHeaders convertedHeaderOf(AadClientRegistrationRepository repository private Object[] expectedHeaders(AadClientRegistrationRepository repository) { return new AadOAuth2AuthorizationCodeGrantRequestEntityConverter(repository.getAzureClientAccessTokenScopes()) - .getHttpHeaders() + .getHttpHeaders(null) .entrySet() .stream() .filter(entry -> !entry.getKey().equals("client-request-id")) From 5baeae8b59b0bbd918d9c905376ebfe46daa2812 Mon Sep 17 00:00:00 2001 From: Rujun Chen Date: Wed, 12 Oct 2022 17:39:08 +0800 Subject: [PATCH 2/5] Add a new item in CHANGELOG.md. --- sdk/spring/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/sdk/spring/CHANGELOG.md b/sdk/spring/CHANGELOG.md index 5a45222c216c..438aa095c637 100644 --- a/sdk/spring/CHANGELOG.md +++ b/sdk/spring/CHANGELOG.md @@ -5,6 +5,7 @@ Upgrade Spring Boot dependencies version to 2.7.4 and Spring Cloud dependencies #### Bugs Fixed - Fix bug: Put a value into Collections.emptyMap(). [#31190](https://github.com/Azure/azure-sdk-for-java/issues/31190). +- Fix bug: Duplicated "scope" parameter. [#31191](https://github.com/Azure/azure-sdk-for-java/issues/31191). ## 4.4.0 (2022-09-26) Upgrade Spring Boot dependencies version to 2.7.3 and Spring Cloud dependencies version to 2021.0.3 From 46e4fd413ac87ca6348d31654f5b993e9900e16c Mon Sep 17 00:00:00 2001 From: Rujun Chen Date: Fri, 14 Oct 2022 10:13:32 +0800 Subject: [PATCH 3/5] Delete unnecessary method: Override and did nothing except call "super.xxx". --- ...tOAuth2AuthorizationCodeGrantRequestEntityConverter.java | 6 ------ 1 file changed, 6 deletions(-) diff --git a/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/aad/implementation/oauth2/AbstractOAuth2AuthorizationCodeGrantRequestEntityConverter.java b/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/aad/implementation/oauth2/AbstractOAuth2AuthorizationCodeGrantRequestEntityConverter.java index 0d54f8e23d36..7cd6b6c48909 100644 --- a/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/aad/implementation/oauth2/AbstractOAuth2AuthorizationCodeGrantRequestEntityConverter.java +++ b/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/aad/implementation/oauth2/AbstractOAuth2AuthorizationCodeGrantRequestEntityConverter.java @@ -5,7 +5,6 @@ import com.azure.spring.cloud.core.implementation.util.AzureSpringIdentifier; import org.springframework.http.HttpHeaders; -import org.springframework.http.RequestEntity; import org.springframework.security.oauth2.client.endpoint.OAuth2AuthorizationCodeGrantRequest; import org.springframework.security.oauth2.client.endpoint.OAuth2AuthorizationCodeGrantRequestEntityConverter; import org.springframework.util.MultiValueMap; @@ -35,11 +34,6 @@ protected AbstractOAuth2AuthorizationCodeGrantRequestEntityConverter() { */ protected abstract String getApplicationId(); - @Override - public RequestEntity convert(OAuth2AuthorizationCodeGrantRequest request) { - return super.convert(request); - } - /** * Additional default headers information. * @return HttpHeaders From fda0a41e0b4021508524b9ed7ff34d1f6a286ca4 Mon Sep 17 00:00:00 2001 From: Rujun Chen Date: Fri, 14 Oct 2022 16:49:23 +0800 Subject: [PATCH 4/5] 1. Change "getHttpHeaders" and "getHttpBody" to "protected" method. 2. Update related tests caused by step 1. --- ...izationCodeGrantRequestEntityConverter.java | 4 ++-- ...onCodeGrantRequestEntityConverterTests.java | 18 +++++++++--------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/aad/implementation/oauth2/AbstractOAuth2AuthorizationCodeGrantRequestEntityConverter.java b/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/aad/implementation/oauth2/AbstractOAuth2AuthorizationCodeGrantRequestEntityConverter.java index 7cd6b6c48909..eefe948c4e36 100644 --- a/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/aad/implementation/oauth2/AbstractOAuth2AuthorizationCodeGrantRequestEntityConverter.java +++ b/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/aad/implementation/oauth2/AbstractOAuth2AuthorizationCodeGrantRequestEntityConverter.java @@ -38,7 +38,7 @@ protected AbstractOAuth2AuthorizationCodeGrantRequestEntityConverter() { * Additional default headers information. * @return HttpHeaders */ - public HttpHeaders getHttpHeaders(OAuth2AuthorizationCodeGrantRequest request) { + protected HttpHeaders getHttpHeaders(OAuth2AuthorizationCodeGrantRequest request) { HttpHeaders httpHeaders = new HttpHeaders(); httpHeaders.put("x-client-SKU", Collections.singletonList(getApplicationId())); httpHeaders.put("x-client-VER", Collections.singletonList(AzureSpringIdentifier.VERSION)); @@ -51,7 +51,7 @@ public HttpHeaders getHttpHeaders(OAuth2AuthorizationCodeGrantRequest request) { * @param request OAuth2AuthorizationCodeGrantRequest * @return MultiValueMap */ - public MultiValueMap getHttpBody(OAuth2AuthorizationCodeGrantRequest request) { + protected MultiValueMap getHttpBody(OAuth2AuthorizationCodeGrantRequest request) { return EMPTY_MULTI_VALUE_MAP; } } diff --git a/sdk/spring/spring-cloud-azure-autoconfigure/src/test/java/com/azure/spring/cloud/autoconfigure/aad/implementation/webapp/AadOAuth2AuthorizationCodeGrantRequestEntityConverterTests.java b/sdk/spring/spring-cloud-azure-autoconfigure/src/test/java/com/azure/spring/cloud/autoconfigure/aad/implementation/webapp/AadOAuth2AuthorizationCodeGrantRequestEntityConverterTests.java index b5982dbed002..81ad7e78336b 100644 --- a/sdk/spring/spring-cloud-azure-autoconfigure/src/test/java/com/azure/spring/cloud/autoconfigure/aad/implementation/webapp/AadOAuth2AuthorizationCodeGrantRequestEntityConverterTests.java +++ b/sdk/spring/spring-cloud-azure-autoconfigure/src/test/java/com/azure/spring/cloud/autoconfigure/aad/implementation/webapp/AadOAuth2AuthorizationCodeGrantRequestEntityConverterTests.java @@ -5,6 +5,7 @@ import com.azure.spring.cloud.autoconfigure.aad.AadClientRegistrationRepository; import com.azure.spring.cloud.autoconfigure.aad.implementation.WebApplicationContextRunnerUtils; +import com.azure.spring.cloud.core.implementation.util.AzureSpringIdentifier; import org.hamcrest.Matcher; import org.junit.jupiter.api.Test; import org.springframework.boot.test.context.runner.WebApplicationContextRunner; @@ -19,6 +20,7 @@ import org.springframework.security.oauth2.core.endpoint.OAuth2AuthorizationResponse; import org.springframework.util.MultiValueMap; +import java.util.Collections; import java.util.Optional; import static com.azure.spring.cloud.autoconfigure.aad.AadClientRegistrationRepository.AZURE_CLIENT_REGISTRATION_ID; @@ -86,7 +88,7 @@ void addHeadersForAzureClient() { (AadClientRegistrationRepository) context.getBean(ClientRegistrationRepository.class); ClientRegistration azure = repository.findByRegistrationId(AZURE_CLIENT_REGISTRATION_ID); HttpHeaders httpHeaders = convertedHeaderOf(repository, createCodeGrantRequest(azure)); - assertThat(httpHeaders.entrySet(), (Matcher) hasItems(expectedHeaders(repository))); + assertThat(httpHeaders.entrySet(), (Matcher) hasItems(expectedHeaders())); }); } @@ -98,7 +100,7 @@ void addHeadersForAuthorizationCodeClient() { (AadClientRegistrationRepository) context.getBean(ClientRegistrationRepository.class); ClientRegistration arm = repository.findByRegistrationId("arm"); HttpHeaders httpHeaders = convertedHeaderOf(repository, createCodeGrantRequest(arm)); - assertThat(httpHeaders.entrySet(), (Matcher) hasItems(expectedHeaders(repository))); + assertThat(httpHeaders.entrySet(), (Matcher) hasItems(expectedHeaders())); }); } @@ -112,13 +114,11 @@ private HttpHeaders convertedHeaderOf(AadClientRegistrationRepository repository .orElse(null); } - private Object[] expectedHeaders(AadClientRegistrationRepository repository) { - return new AadOAuth2AuthorizationCodeGrantRequestEntityConverter(repository.getAzureClientAccessTokenScopes()) - .getHttpHeaders(null) - .entrySet() - .stream() - .filter(entry -> !entry.getKey().equals("client-request-id")) - .toArray(); + private Object[] expectedHeaders() { + HttpHeaders httpHeaders = new HttpHeaders(); + httpHeaders.put("x-client-SKU", Collections.singletonList(AzureSpringIdentifier.AZURE_SPRING_AAD)); + httpHeaders.put("x-client-VER", Collections.singletonList(AzureSpringIdentifier.VERSION)); + return httpHeaders.entrySet().toArray(); } private MultiValueMap convertedBodyOf(AadClientRegistrationRepository repository, From d7e2d4553086efe1200c32e80bf76224035d92de Mon Sep 17 00:00:00 2001 From: Rujun Chen Date: Fri, 14 Oct 2022 17:02:17 +0800 Subject: [PATCH 5/5] Update the logic of testing http header. --- ...nCodeGrantRequestEntityConverterTests.java | 21 ++++++++----------- 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/sdk/spring/spring-cloud-azure-autoconfigure/src/test/java/com/azure/spring/cloud/autoconfigure/aad/implementation/webapp/AadOAuth2AuthorizationCodeGrantRequestEntityConverterTests.java b/sdk/spring/spring-cloud-azure-autoconfigure/src/test/java/com/azure/spring/cloud/autoconfigure/aad/implementation/webapp/AadOAuth2AuthorizationCodeGrantRequestEntityConverterTests.java index 81ad7e78336b..126d019049f0 100644 --- a/sdk/spring/spring-cloud-azure-autoconfigure/src/test/java/com/azure/spring/cloud/autoconfigure/aad/implementation/webapp/AadOAuth2AuthorizationCodeGrantRequestEntityConverterTests.java +++ b/sdk/spring/spring-cloud-azure-autoconfigure/src/test/java/com/azure/spring/cloud/autoconfigure/aad/implementation/webapp/AadOAuth2AuthorizationCodeGrantRequestEntityConverterTests.java @@ -6,7 +6,6 @@ import com.azure.spring.cloud.autoconfigure.aad.AadClientRegistrationRepository; import com.azure.spring.cloud.autoconfigure.aad.implementation.WebApplicationContextRunnerUtils; import com.azure.spring.cloud.core.implementation.util.AzureSpringIdentifier; -import org.hamcrest.Matcher; import org.junit.jupiter.api.Test; import org.springframework.boot.test.context.runner.WebApplicationContextRunner; import org.springframework.http.HttpEntity; @@ -24,9 +23,8 @@ import java.util.Optional; import static com.azure.spring.cloud.autoconfigure.aad.AadClientRegistrationRepository.AZURE_CLIENT_REGISTRATION_ID; -import static org.hamcrest.CoreMatchers.hasItems; -import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; class AadOAuth2AuthorizationCodeGrantRequestEntityConverterTests { @@ -81,26 +79,24 @@ void onlyAddScopeOnceEvenConvertMethodExecutedMultipleTimes() { } @Test - @SuppressWarnings("unchecked") void addHeadersForAzureClient() { getContextRunner().run(context -> { AadClientRegistrationRepository repository = (AadClientRegistrationRepository) context.getBean(ClientRegistrationRepository.class); ClientRegistration azure = repository.findByRegistrationId(AZURE_CLIENT_REGISTRATION_ID); HttpHeaders httpHeaders = convertedHeaderOf(repository, createCodeGrantRequest(azure)); - assertThat(httpHeaders.entrySet(), (Matcher) hasItems(expectedHeaders())); + testHttpHeaders(httpHeaders); }); } @Test - @SuppressWarnings("unchecked") void addHeadersForAuthorizationCodeClient() { getContextRunner().run(context -> { AadClientRegistrationRepository repository = (AadClientRegistrationRepository) context.getBean(ClientRegistrationRepository.class); ClientRegistration arm = repository.findByRegistrationId("arm"); HttpHeaders httpHeaders = convertedHeaderOf(repository, createCodeGrantRequest(arm)); - assertThat(httpHeaders.entrySet(), (Matcher) hasItems(expectedHeaders())); + testHttpHeaders(httpHeaders); }); } @@ -114,11 +110,12 @@ private HttpHeaders convertedHeaderOf(AadClientRegistrationRepository repository .orElse(null); } - private Object[] expectedHeaders() { - HttpHeaders httpHeaders = new HttpHeaders(); - httpHeaders.put("x-client-SKU", Collections.singletonList(AzureSpringIdentifier.AZURE_SPRING_AAD)); - httpHeaders.put("x-client-VER", Collections.singletonList(AzureSpringIdentifier.VERSION)); - return httpHeaders.entrySet().toArray(); + private void testHttpHeaders(HttpHeaders headers) { + assertTrue(headers.containsKey("x-client-SKU")); + assertEquals(Collections.singletonList(AzureSpringIdentifier.AZURE_SPRING_AAD), headers.get("x-client-SKU")); + assertTrue(headers.containsKey("x-client-VER")); + assertEquals(Collections.singletonList(AzureSpringIdentifier.VERSION), headers.get("x-client-VER")); + assertTrue(headers.containsKey("client-request-id")); } private MultiValueMap convertedBodyOf(AadClientRegistrationRepository repository,