diff --git a/sdk/spring/CHANGELOG.md b/sdk/spring/CHANGELOG.md index affbf6fb1f23..5a45222c216c 100644 --- a/sdk/spring/CHANGELOG.md +++ b/sdk/spring/CHANGELOG.md @@ -3,6 +3,9 @@ ## 4.5.0-beta.2 (Unreleased) Upgrade Spring Boot dependencies version to 2.7.4 and Spring Cloud dependencies version to 2021.0.4 +#### Bugs Fixed +- Fix bug: Put a value into Collections.emptyMap(). [#31190](https://github.com/Azure/azure-sdk-for-java/issues/31190). + ## 4.4.0 (2022-09-26) Upgrade Spring Boot dependencies version to 2.7.3 and Spring Cloud dependencies version to 2021.0.3 Upgrade Spring Boot dependencies version to 2.7.2 and Spring Cloud dependencies version to 2021.0.3. diff --git a/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/aad/implementation/jackson/SerializerUtils.java b/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/aad/implementation/jackson/SerializerUtils.java index f5ba0fde806c..16616fafb63b 100644 --- a/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/aad/implementation/jackson/SerializerUtils.java +++ b/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/aad/implementation/jackson/SerializerUtils.java @@ -10,7 +10,7 @@ import org.springframework.security.oauth2.client.OAuth2AuthorizedClient; import org.springframework.security.oauth2.client.jackson2.OAuth2ClientJackson2Module; -import java.util.HashMap; +import java.util.Collections; import java.util.Map; public final class SerializerUtils { @@ -33,6 +33,11 @@ public final class SerializerUtils { private SerializerUtils() { } + /** + * Serialize {@link Map} to {@link String}. + * @param authorizedClients the map to be serialized. It will not be modified in this method. + * @return The serialized {@link String}. + */ public static String serializeOAuth2AuthorizedClientMap(Map authorizedClients) { String result; try { @@ -43,9 +48,14 @@ public static String serializeOAuth2AuthorizedClientMap(Map deserializeOAuth2AuthorizedClientMap(String authorizedClientsString) { if (authorizedClientsString == null) { - return new HashMap<>(); + return Collections.emptyMap(); } Map authorizedClients; try { diff --git a/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/aad/implementation/oauth2/JacksonHttpSessionOAuth2AuthorizedClientRepository.java b/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/aad/implementation/oauth2/JacksonHttpSessionOAuth2AuthorizedClientRepository.java index 4516ec6e1151..2a929e93d4ee 100644 --- a/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/aad/implementation/oauth2/JacksonHttpSessionOAuth2AuthorizedClientRepository.java +++ b/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/aad/implementation/oauth2/JacksonHttpSessionOAuth2AuthorizedClientRepository.java @@ -13,6 +13,7 @@ import javax.servlet.http.HttpServletResponse; import javax.servlet.http.HttpSession; import java.util.Collections; +import java.util.HashMap; import java.util.Map; import java.util.Optional; @@ -21,7 +22,7 @@ /** * An implementation of an {@link OAuth2AuthorizedClientRepository} that stores {@link OAuth2AuthorizedClient}'s in the * {@code HttpSession}. To make it compatible with different spring versions. Refs: - * https://github.com/spring-projects/spring-security/issues/9204 + * spring-security/issues/9204 * * @see OAuth2AuthorizedClientRepository * @see OAuth2AuthorizedClient @@ -48,7 +49,8 @@ public void saveAuthorizedClient(OAuth2AuthorizedClient authorizedClient, Authen Assert.notNull(authorizedClient, "authorizedClient cannot be null"); Assert.notNull(request, MSG_REQUEST_CANNOT_BE_NULL); Assert.notNull(response, "response cannot be null"); - Map authorizedClients = this.getAuthorizedClients(request); + Map authorizedClients = + new HashMap<>(this.getAuthorizedClients(request)); authorizedClients.put(authorizedClient.getClientRegistration().getRegistrationId(), authorizedClient); request.getSession().setAttribute(AUTHORIZED_CLIENTS_ATTR_NAME, serializeOAuth2AuthorizedClientMap(authorizedClients)); @@ -59,7 +61,7 @@ public void removeAuthorizedClient(String clientRegistrationId, Authentication p HttpServletRequest request, HttpServletResponse response) { Assert.hasText(clientRegistrationId, "clientRegistrationId cannot be empty"); Assert.notNull(request, MSG_REQUEST_CANNOT_BE_NULL); - Map authorizedClients = this.getAuthorizedClients(request); + Map authorizedClients = new HashMap<>(this.getAuthorizedClients(request)); if (authorizedClients.remove(clientRegistrationId) != null) { if (authorizedClients.isEmpty()) { request.getSession().removeAttribute(AUTHORIZED_CLIENTS_ATTR_NAME); diff --git a/sdk/spring/spring-cloud-azure-autoconfigure/src/test/java/com/azure/spring/cloud/autoconfigure/aad/implementation/oauth2/JacksonHttpSessionOAuth2AuthorizedClientRepositoryTest.java b/sdk/spring/spring-cloud-azure-autoconfigure/src/test/java/com/azure/spring/cloud/autoconfigure/aad/implementation/oauth2/JacksonHttpSessionOAuth2AuthorizedClientRepositoryTest.java new file mode 100644 index 000000000000..01b51112160d --- /dev/null +++ b/sdk/spring/spring-cloud-azure-autoconfigure/src/test/java/com/azure/spring/cloud/autoconfigure/aad/implementation/oauth2/JacksonHttpSessionOAuth2AuthorizedClientRepositoryTest.java @@ -0,0 +1,240 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package com.azure.spring.cloud.autoconfigure.aad.implementation.oauth2; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.springframework.mock.web.MockHttpServletRequest; +import org.springframework.mock.web.MockHttpServletResponse; +import org.springframework.security.oauth2.client.OAuth2AuthorizedClient; +import org.springframework.security.oauth2.client.registration.ClientRegistration; +import org.springframework.security.oauth2.core.AuthorizationGrantType; +import org.springframework.security.oauth2.core.ClientAuthenticationMethod; +import org.springframework.security.oauth2.core.OAuth2AccessToken; + +import javax.servlet.http.HttpSession; +import java.time.Instant; +import java.util.Map; + +import static com.azure.spring.cloud.autoconfigure.aad.implementation.jackson.SerializerUtils.deserializeOAuth2AuthorizedClientMap; +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.springframework.security.oauth2.core.OAuth2AccessToken.TokenType.BEARER; + +class JacksonHttpSessionOAuth2AuthorizedClientRepositoryTest { + private final String principalName1 = "principalName-1"; + private final String principalName2 = "principalName-2"; + + private final ClientRegistration registration1 = ClientRegistration + .withRegistrationId("registration-id-1") + .redirectUri("{baseUrl}/{action}/oauth2/code/{registrationId}") + .clientAuthenticationMethod(ClientAuthenticationMethod.CLIENT_SECRET_BASIC) + .authorizationGrantType(AuthorizationGrantType.AUTHORIZATION_CODE) + .scope("scope-1") + .authorizationUri("https://example1.com/login/oauth/authorize") + .tokenUri("https://example1.com/login/oauth/access_token") + .jwkSetUri("https://example1.com/oauth2/jwk") + .issuerUri("https://example1.com") + .userInfoUri("https://api.example1.com/user") + .userNameAttributeName("id-1") + .clientName("Client Name 1") + .clientId("client-id-1") + .clientSecret("client-secret-1") + .build(); + + private final ClientRegistration registration2 = ClientRegistration + .withRegistrationId("registration-id-2") + .redirectUri("{baseUrl}/{action}/oauth2/code/{registrationId}") + .clientAuthenticationMethod(ClientAuthenticationMethod.CLIENT_SECRET_BASIC) + .authorizationGrantType(AuthorizationGrantType.AUTHORIZATION_CODE) + .scope("scope-2") + .authorizationUri("https://example2.com/login/oauth/authorize") + .tokenUri("https://example2.com/login/oauth/access_token") + .userInfoUri("https://api.example2.com/user") + .userNameAttributeName("id-2") + .clientName("Client Name 2") + .clientId("client-id-2") + .clientSecret("client-secret-2") + .build(); + + private final String registrationId1 = this.registration1.getRegistrationId(); + + private final String registrationId2 = this.registration2.getRegistrationId(); + + private final OAuth2AccessToken oAuth2AccessToken1 = new OAuth2AccessToken(BEARER, "tokenValue1", Instant.now(), Instant.now().plusMillis(3_600_000)); + private final OAuth2AccessToken oAuth2AccessToken2 = new OAuth2AccessToken(BEARER, "tokenValue2", Instant.now(), Instant.now().plusMillis(3_600_000)); + + private final OAuth2AuthorizedClient authorizedClient1 = new OAuth2AuthorizedClient(this.registration1, this.principalName1, oAuth2AccessToken1); + + private final OAuth2AuthorizedClient authorizedClient2 = new OAuth2AuthorizedClient(this.registration2, this.principalName2, oAuth2AccessToken2); + + private final JacksonHttpSessionOAuth2AuthorizedClientRepository authorizedClientRepository = + new JacksonHttpSessionOAuth2AuthorizedClientRepository(); + private MockHttpServletRequest request; + + private MockHttpServletResponse response; + + @BeforeEach + void setup() { + this.request = new MockHttpServletRequest(); + this.response = new MockHttpServletResponse(); + } + + @Test + void loadAuthorizedClientWhenClientRegistrationIdIsNullThenThrowIllegalArgumentException() { + assertThatIllegalArgumentException().isThrownBy(() -> + this.authorizedClientRepository.loadAuthorizedClient(null, null, this.request)); + } + + @Test + void loadAuthorizedClientWhenPrincipalNameIsNullThenExceptionNotThrown() { + this.authorizedClientRepository.loadAuthorizedClient(this.registrationId1, null, this.request); + } + + @Test + void loadAuthorizedClientWhenRequestIsNullThenThrowIllegalArgumentException() { + assertThatIllegalArgumentException().isThrownBy(() -> + this.authorizedClientRepository.loadAuthorizedClient(this.registrationId1, null, null)); + } + + @Test + void loadAuthorizedClientWhenClientRegistrationNotFoundThenReturnNull() { + OAuth2AuthorizedClient authorizedClient = + this.authorizedClientRepository.loadAuthorizedClient("registration-not-found", null, this.request); + assertThat(authorizedClient).isNull(); + } + + @Test + void loadAuthorizedClientWhenSavedThenReturnAuthorizedClient() { + this.authorizedClientRepository.saveAuthorizedClient(authorizedClient1, null, this.request, this.response); + OAuth2AuthorizedClient loadedAuthorizedClient = + this.authorizedClientRepository.loadAuthorizedClient(this.registrationId1, null, this.request); + assertSame(authorizedClient1, loadedAuthorizedClient); + } + + @Test + void saveAuthorizedClientWhenAuthorizedClientIsNullThenThrowIllegalArgumentException() { + assertThatIllegalArgumentException().isThrownBy(() -> + this.authorizedClientRepository.saveAuthorizedClient(null, null, this.request, this.response)); + } + + @Test + void saveAuthorizedClientWhenAuthenticationIsNullThenExceptionNotThrown() { + this.authorizedClientRepository.saveAuthorizedClient(authorizedClient1, null, this.request, this.response); + } + + @Test + void saveAuthorizedClientWhenRequestIsNullThenThrowIllegalArgumentException() { + assertThatIllegalArgumentException().isThrownBy(() -> + this.authorizedClientRepository.saveAuthorizedClient(authorizedClient1, null, null, this.response)); + } + + @Test + void saveAuthorizedClientWhenResponseIsNullThenThrowIllegalArgumentException() { + assertThatIllegalArgumentException().isThrownBy(() -> + this.authorizedClientRepository.saveAuthorizedClient(authorizedClient1, null, this.request, null)); + } + + @Test + void saveAuthorizedClientWhenSavedThenSavedToSession() { + this.authorizedClientRepository.saveAuthorizedClient(authorizedClient1, null, this.request, this.response); + + HttpSession session = this.request.getSession(false); + assertThat(session).isNotNull(); + String authorizedClientsString = (String) session.getAttribute( + JacksonHttpSessionOAuth2AuthorizedClientRepository.class.getName() + ".AUTHORIZED_CLIENTS"); + Map authorizedClients = deserializeOAuth2AuthorizedClientMap(authorizedClientsString); + assertThat(authorizedClients).isNotEmpty(); + assertThat(authorizedClients).hasSize(1); + OAuth2AuthorizedClient loadedAuthorizedClient = authorizedClients.values().iterator().next(); + assertSame(authorizedClient1, loadedAuthorizedClient); + } + + @Test + void removeAuthorizedClientWhenClientRegistrationIdIsNullThenThrowIllegalArgumentException() { + assertThatIllegalArgumentException().isThrownBy(() -> + this.authorizedClientRepository.removeAuthorizedClient(null, null, this.request, this.response)); + } + + @Test + void removeAuthorizedClientWhenPrincipalNameIsNullThenExceptionNotThrown() { + this.authorizedClientRepository.removeAuthorizedClient(this.registrationId1, null, this.request, this.response); + } + + @Test + void removeAuthorizedClientWhenRequestIsNullThenThrowIllegalArgumentException() { + assertThatIllegalArgumentException().isThrownBy(() -> + this.authorizedClientRepository.removeAuthorizedClient(this.registrationId1, null, null, this.response)); + } + + @Test + void removeAuthorizedClientWhenResponseIsNullThenExceptionNotThrown() { + this.authorizedClientRepository.removeAuthorizedClient(this.registrationId1, null, this.request, null); + } + + @Test + void removeAuthorizedClientWhenNotSavedThenSessionNotCreated() { + this.authorizedClientRepository.removeAuthorizedClient(this.registrationId2, null, this.request, this.response); + assertThat(this.request.getSession(false)).isNull(); + } + + @Test + void removeAuthorizedClientWhenClient1SavedAndClient2RemovedThenClient1NotRemoved() { + this.authorizedClientRepository.saveAuthorizedClient(authorizedClient1, null, this.request, this.response); + // Remove registrationId2 (never added so is not removed either) + this.authorizedClientRepository.removeAuthorizedClient(this.registrationId2, null, this.request, this.response); + OAuth2AuthorizedClient loadedAuthorizedClient1 = + this.authorizedClientRepository.loadAuthorizedClient(this.registrationId1, null, this.request); + assertThat(loadedAuthorizedClient1).isNotNull(); + assertSame(authorizedClient1, loadedAuthorizedClient1); + } + + @Test + void removeAuthorizedClientWhenSavedThenRemoved() { + this.authorizedClientRepository.saveAuthorizedClient(authorizedClient1, null, this.request, this.response); + OAuth2AuthorizedClient loadedAuthorizedClient = + this.authorizedClientRepository.loadAuthorizedClient(this.registrationId1, null, this.request); + assertSame(authorizedClient1, loadedAuthorizedClient); + this.authorizedClientRepository.removeAuthorizedClient(this.registrationId1, null, this.request, this.response); + loadedAuthorizedClient = this.authorizedClientRepository.loadAuthorizedClient(this.registrationId1, null, this.request); + assertThat(loadedAuthorizedClient).isNull(); + } + + @Test + void removeAuthorizedClientWhenSavedThenRemovedFromSession() { + this.authorizedClientRepository.saveAuthorizedClient(authorizedClient1, null, this.request, this.response); + OAuth2AuthorizedClient loadedAuthorizedClient = + this.authorizedClientRepository.loadAuthorizedClient(this.registrationId1, null, this.request); + assertSame(authorizedClient1, loadedAuthorizedClient); + this.authorizedClientRepository.removeAuthorizedClient(this.registrationId1, null, this.request, this.response); + HttpSession session = this.request.getSession(false); + assertThat(session).isNotNull(); + assertThat(session.getAttribute(JacksonHttpSessionOAuth2AuthorizedClientRepository.class.getName() + ".AUTHORIZED_CLIENTS")).isNull(); + } + + @Test + void removeAuthorizedClientWhenClient1Client2SavedAndClient1RemovedThenClient2NotRemoved() { + this.authorizedClientRepository.saveAuthorizedClient(authorizedClient1, null, this.request, this.response); + this.authorizedClientRepository.saveAuthorizedClient(authorizedClient2, null, this.request, this.response); + this.authorizedClientRepository.removeAuthorizedClient(this.registrationId1, null, this.request, this.response); + OAuth2AuthorizedClient loadedAuthorizedClient2 = + this.authorizedClientRepository.loadAuthorizedClient(this.registrationId2, null, this.request); + assertThat(loadedAuthorizedClient2).isNotNull(); + assertSame(authorizedClient2, loadedAuthorizedClient2); + } + + private void assertSame(OAuth2AuthorizedClient client1, OAuth2AuthorizedClient client2) { + assertEquals(client1.getClientRegistration().getClientId(), client2.getClientRegistration().getClientId()); + assertEquals(client1.getClientRegistration().getRegistrationId(), client2.getClientRegistration().getRegistrationId()); + assertEquals(client1.getClientRegistration().getClientName(), client2.getClientRegistration().getClientName()); + assertEquals(client1.getClientRegistration().getClientSecret(), client2.getClientRegistration().getClientSecret()); + assertEquals(client1.getClientRegistration().getClientAuthenticationMethod(), client2.getClientRegistration().getClientAuthenticationMethod()); + assertEquals(client1.getClientRegistration().getAuthorizationGrantType(), client2.getClientRegistration().getAuthorizationGrantType()); + assertEquals(client1.getPrincipalName(), client2.getPrincipalName()); + assertEquals(client1.getAccessToken().getTokenType(), client2.getAccessToken().getTokenType()); + assertEquals(client1.getAccessToken().getTokenValue(), client2.getAccessToken().getTokenValue()); + assertEquals(client1.getAccessToken().getScopes(), client2.getAccessToken().getScopes()); + } +}