Fix bug: Put a value into Collections.emptyMap().

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.

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<String, OAuth2AuthorizedClient> authorizedClients) {
         String result;
         try {
@@ -43,9 +48,14 @@ public static String serializeOAuth2AuthorizedClientMap(Map<String, OAuth2Author
         return result;
     }
 
+    /**
+     * Deserialize {@link String} to {@link Map}.
+     * @param authorizedClientsString the String to be deserialized
+     * @return The deserialized {@link Map}. Return {@link Collections#emptyMap()} if authorizedClientsString is null.
+     */
     public static Map<String, OAuth2AuthorizedClient> deserializeOAuth2AuthorizedClientMap(String authorizedClientsString) {
         if (authorizedClientsString == null) {
-            return new HashMap<>();
+            return Collections.emptyMap();
         }
         Map<String, OAuth2AuthorizedClient> authorizedClients;
         try {

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
+ * <a href="https://github.com/spring-projects/spring-security/issues/9204">spring-security/issues/9204</a>
  *
  * @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<String, OAuth2AuthorizedClient> authorizedClients = this.getAuthorizedClients(request);
+        Map<String, OAuth2AuthorizedClient> 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<String, OAuth2AuthorizedClient> authorizedClients = this.getAuthorizedClients(request);
+        Map<String, OAuth2AuthorizedClient> authorizedClients = new HashMap<>(this.getAuthorizedClients(request));
         if (authorizedClients.remove(clientRegistrationId) != null) {
             if (authorizedClients.isEmpty()) {
                 request.getSession().removeAttribute(AUTHORIZED_CLIENTS_ATTR_NAME);

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<String, OAuth2AuthorizedClient> 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());
+    }
+}