From d447402cdba4633fc508332e555d9f326f08d6cf Mon Sep 17 00:00:00 2001 From: Rujun Chen Date: Tue, 11 Oct 2022 16:37:41 +0800 Subject: [PATCH 01/12] Fix bug: Put a value into Collections.emptyMap(). --- ...ssionOAuth2AuthorizedClientRepository.java | 8 +- ...nOAuth2AuthorizedClientRepositoryTest.java | 237 ++++++++++++++++++ 2 files changed, 242 insertions(+), 3 deletions(-) create mode 100644 sdk/spring/spring-cloud-azure-autoconfigure/src/test/java/com/azure/spring/cloud/autoconfigure/aad/implementation/oauth2/JacksonHttpSessionOAuth2AuthorizedClientRepositoryTest.java 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..a45b12015386 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; @@ -48,10 +49,11 @@ 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)); + serializeOAuth2AuthorizedClientMap(Collections.unmodifiableMap(authorizedClients))); } @Override @@ -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..8bcbb83b389e --- /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,237 @@ +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; + +public 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 + public void setup() { + this.request = new MockHttpServletRequest(); + this.response = new MockHttpServletResponse(); + } + + @Test + public void loadAuthorizedClientWhenClientRegistrationIdIsNullThenThrowIllegalArgumentException() { + assertThatIllegalArgumentException().isThrownBy(() -> + this.authorizedClientRepository.loadAuthorizedClient(null, null, this.request)); + } + + @Test + public void loadAuthorizedClientWhenPrincipalNameIsNullThenExceptionNotThrown() { + this.authorizedClientRepository.loadAuthorizedClient(this.registrationId1, null, this.request); + } + + @Test + public void loadAuthorizedClientWhenRequestIsNullThenThrowIllegalArgumentException() { + assertThatIllegalArgumentException().isThrownBy(() -> + this.authorizedClientRepository.loadAuthorizedClient(this.registrationId1, null, null)); + } + + @Test + public void loadAuthorizedClientWhenClientRegistrationNotFoundThenReturnNull() { + OAuth2AuthorizedClient authorizedClient = + this.authorizedClientRepository.loadAuthorizedClient("registration-not-found", null, this.request); + assertThat(authorizedClient).isNull(); + } + + @Test + public 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 + public void saveAuthorizedClientWhenAuthorizedClientIsNullThenThrowIllegalArgumentException() { + assertThatIllegalArgumentException().isThrownBy(() -> + this.authorizedClientRepository.saveAuthorizedClient(null, null, this.request, this.response)); + } + + @Test + public void saveAuthorizedClientWhenAuthenticationIsNullThenExceptionNotThrown() { + this.authorizedClientRepository.saveAuthorizedClient(authorizedClient1, null, this.request, this.response); + } + + @Test + public void saveAuthorizedClientWhenRequestIsNullThenThrowIllegalArgumentException() { + assertThatIllegalArgumentException().isThrownBy(() -> + this.authorizedClientRepository.saveAuthorizedClient(authorizedClient1, null, null, this.response)); + } + + @Test + public void saveAuthorizedClientWhenResponseIsNullThenThrowIllegalArgumentException() { + assertThatIllegalArgumentException().isThrownBy(() -> + this.authorizedClientRepository.saveAuthorizedClient(authorizedClient1, null, this.request, null)); + } + + @Test + public 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 + public void removeAuthorizedClientWhenClientRegistrationIdIsNullThenThrowIllegalArgumentException() { + assertThatIllegalArgumentException().isThrownBy(() -> + this.authorizedClientRepository.removeAuthorizedClient(null, null, this.request, this.response)); + } + + @Test + public void removeAuthorizedClientWhenPrincipalNameIsNullThenExceptionNotThrown() { + this.authorizedClientRepository.removeAuthorizedClient(this.registrationId1, null, this.request, this.response); + } + + @Test + public void removeAuthorizedClientWhenRequestIsNullThenThrowIllegalArgumentException() { + assertThatIllegalArgumentException().isThrownBy(() -> + this.authorizedClientRepository.removeAuthorizedClient(this.registrationId1, null, null, this.response)); + } + + @Test + public void removeAuthorizedClientWhenResponseIsNullThenExceptionNotThrown() { + this.authorizedClientRepository.removeAuthorizedClient(this.registrationId1, null, this.request, null); + } + + @Test + public void removeAuthorizedClientWhenNotSavedThenSessionNotCreated() { + this.authorizedClientRepository.removeAuthorizedClient(this.registrationId2, null, this.request, this.response); + assertThat(this.request.getSession(false)).isNull(); + } + + @Test + public 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 + public 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 + public 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 + public 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()); + } +} From 2d2ab2d736139108832aefee877f9283e6fde24c Mon Sep 17 00:00:00 2001 From: Rujun Chen Date: Tue, 11 Oct 2022 17:24:03 +0800 Subject: [PATCH 02/12] Fix pipeline failure by adding "// Copyright (c) ..." --- ...JacksonHttpSessionOAuth2AuthorizedClientRepositoryTest.java | 3 +++ 1 file changed, 3 insertions(+) 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 index 8bcbb83b389e..c6f4bf740a18 100644 --- 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 @@ -1,3 +1,6 @@ +// 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; From f8975785137a0a81f11ef4e5ac9ff406131a2d7b Mon Sep 17 00:00:00 2001 From: Rujun Chen Date: Tue, 11 Oct 2022 17:30:01 +0800 Subject: [PATCH 03/12] Add a new item about this bug fixing in CHANGELOG.md --- sdk/spring/CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) 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. From 3c7ecbd010b08dd8b9dfba2d03b249241cf5924d Mon Sep 17 00:00:00 2001 From: Rujun Chen Date: Wed, 12 Oct 2022 09:46:11 +0800 Subject: [PATCH 04/12] Make the returned value immutable in SerializerUtils.deserializeOAuth2AuthorizedClientMap. --- .../aad/implementation/jackson/SerializerUtils.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) 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..08e545fe15db 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 { @@ -45,11 +45,11 @@ public static String serializeOAuth2AuthorizedClientMap(Map deserializeOAuth2AuthorizedClientMap(String authorizedClientsString) { if (authorizedClientsString == null) { - return new HashMap<>(); + return Collections.emptyMap(); } Map authorizedClients; try { - authorizedClients = OBJECT_MAPPER.readValue(authorizedClientsString, TYPE_REFERENCE); + authorizedClients = Collections.unmodifiableMap(OBJECT_MAPPER.readValue(authorizedClientsString, TYPE_REFERENCE)); } catch (JsonProcessingException e) { throw new IllegalStateException(e); } From 1f5b3cde3ce88a92c5a4f4828f32251fb3cfe424 Mon Sep 17 00:00:00 2001 From: Rujun Chen Date: Wed, 12 Oct 2022 09:57:09 +0800 Subject: [PATCH 05/12] Not use Collections.unmodifiableMap() to wap the result of "OBJECT_MAPPER.readValue()". Because "deserialize" should just do the "deserialize" work. After wrapped by "Collections.unmodifiableMap()", the returned value's class is changed. --- .../aad/implementation/jackson/SerializerUtils.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 08e545fe15db..93c9bee6222e 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 @@ -49,7 +49,7 @@ public static Map deserializeOAuth2AuthorizedCli } Map authorizedClients; try { - authorizedClients = Collections.unmodifiableMap(OBJECT_MAPPER.readValue(authorizedClientsString, TYPE_REFERENCE)); + authorizedClients = OBJECT_MAPPER.readValue(authorizedClientsString, TYPE_REFERENCE); } catch (JsonProcessingException e) { throw new IllegalStateException(e); } From 835f26b02dae7165c453113a57d1bdc7590873c4 Mon Sep 17 00:00:00 2001 From: Rujun Chen Date: Wed, 12 Oct 2022 10:09:32 +0800 Subject: [PATCH 06/12] Change "serializeOAuth2AuthorizedClientMap(authorizedClients))" to "serializeOAuth2AuthorizedClientMap(Collections.unmodifiableMap(authorizedClients)))". --- .../JacksonHttpSessionOAuth2AuthorizedClientRepository.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 a45b12015386..97d858652f6d 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 @@ -67,7 +67,7 @@ public void removeAuthorizedClient(String clientRegistrationId, Authentication p request.getSession().removeAttribute(AUTHORIZED_CLIENTS_ATTR_NAME); } else { request.getSession().setAttribute(AUTHORIZED_CLIENTS_ATTR_NAME, - serializeOAuth2AuthorizedClientMap(authorizedClients)); + serializeOAuth2AuthorizedClientMap(Collections.unmodifiableMap(authorizedClients))); } } From bf4f638d0c3f5819e56dae2d1efdbac0b0b4b687 Mon Sep 17 00:00:00 2001 From: Rujun Chen Date: Wed, 12 Oct 2022 13:25:59 +0800 Subject: [PATCH 07/12] Add java doc in SerializerUtils. --- .../aad/implementation/jackson/SerializerUtils.java | 10 ++++++++++ 1 file changed, 10 insertions(+) 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 93c9bee6222e..4ad338e800e4 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 @@ -33,6 +33,11 @@ public final class SerializerUtils { private SerializerUtils() { } + /** + * Serialize 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,6 +48,11 @@ public static String serializeOAuth2AuthorizedClientMap(Map. + * @param authorizedClientsString the String to be deserialized + * @return The deserialized {@link Map}. + */ public static Map deserializeOAuth2AuthorizedClientMap(String authorizedClientsString) { if (authorizedClientsString == null) { return Collections.emptyMap(); From 05e87a6f191cc619ff0637927c06c351462efb12 Mon Sep 17 00:00:00 2001 From: Rujun Chen Date: Wed, 12 Oct 2022 13:28:10 +0800 Subject: [PATCH 08/12] Remove "Collections.unmodifiableMap()" when call "serializeOAuth2AuthorizedClientMap" because: 1. "Collections.unmodifiableMap()" needs extra cost. 2. "serializeOAuth2AuthorizedClientMap"'s java doc already said that it will not modify the parameter. --- .../JacksonHttpSessionOAuth2AuthorizedClientRepository.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 97d858652f6d..269a8943b80f 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 @@ -53,7 +53,7 @@ public void saveAuthorizedClient(OAuth2AuthorizedClient authorizedClient, Authen new HashMap<>(this.getAuthorizedClients(request)); authorizedClients.put(authorizedClient.getClientRegistration().getRegistrationId(), authorizedClient); request.getSession().setAttribute(AUTHORIZED_CLIENTS_ATTR_NAME, - serializeOAuth2AuthorizedClientMap(Collections.unmodifiableMap(authorizedClients))); + serializeOAuth2AuthorizedClientMap(authorizedClients)); } @Override @@ -67,7 +67,7 @@ public void removeAuthorizedClient(String clientRegistrationId, Authentication p request.getSession().removeAttribute(AUTHORIZED_CLIENTS_ATTR_NAME); } else { request.getSession().setAttribute(AUTHORIZED_CLIENTS_ATTR_NAME, - serializeOAuth2AuthorizedClientMap(Collections.unmodifiableMap(authorizedClients))); + serializeOAuth2AuthorizedClientMap(authorizedClients)); } } From 368511826b4d005cfd930e22370796eca149ebce Mon Sep 17 00:00:00 2001 From: Rujun Chen Date: Wed, 12 Oct 2022 13:30:26 +0800 Subject: [PATCH 09/12] Improve java doc of JacksonHttpSessionOAuth2AuthorizedClientRepository. --- .../JacksonHttpSessionOAuth2AuthorizedClientRepository.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 269a8943b80f..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 @@ -22,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 From d491ab5eece52b0d3d622699f5a1ffdf89abb28b Mon Sep 17 00:00:00 2001 From: Rujun Chen Date: Wed, 12 Oct 2022 13:40:13 +0800 Subject: [PATCH 10/12] Improve java doc of SerializerUtils#deserializeOAuth2AuthorizedClientMap. --- .../aad/implementation/jackson/SerializerUtils.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 4ad338e800e4..32b772349a13 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 @@ -51,7 +51,7 @@ public static String serializeOAuth2AuthorizedClientMap(Map. * @param authorizedClientsString the String to be deserialized - * @return The deserialized {@link Map}. + * @return The deserialized {@link Map}. Return {@link Collections#emptyMap()} if authorizedClientsString is null. */ public static Map deserializeOAuth2AuthorizedClientMap(String authorizedClientsString) { if (authorizedClientsString == null) { From 53544a16dfbed43097998e1aca302dff1ae0cb1a Mon Sep 17 00:00:00 2001 From: Rujun Chen Date: Wed, 12 Oct 2022 14:46:48 +0800 Subject: [PATCH 11/12] Remove all "public" in "JacksonHttpSessionOAuth2AuthorizedClientRepositoryTest". --- ...nOAuth2AuthorizedClientRepositoryTest.java | 42 +++++++++---------- 1 file changed, 21 insertions(+), 21 deletions(-) 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 index c6f4bf740a18..01b51112160d 100644 --- 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 @@ -23,7 +23,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.springframework.security.oauth2.core.OAuth2AccessToken.TokenType.BEARER; -public class JacksonHttpSessionOAuth2AuthorizedClientRepositoryTest { +class JacksonHttpSessionOAuth2AuthorizedClientRepositoryTest { private final String principalName1 = "principalName-1"; private final String principalName2 = "principalName-2"; @@ -77,37 +77,37 @@ public class JacksonHttpSessionOAuth2AuthorizedClientRepositoryTest { private MockHttpServletResponse response; @BeforeEach - public void setup() { + void setup() { this.request = new MockHttpServletRequest(); this.response = new MockHttpServletResponse(); } @Test - public void loadAuthorizedClientWhenClientRegistrationIdIsNullThenThrowIllegalArgumentException() { + void loadAuthorizedClientWhenClientRegistrationIdIsNullThenThrowIllegalArgumentException() { assertThatIllegalArgumentException().isThrownBy(() -> this.authorizedClientRepository.loadAuthorizedClient(null, null, this.request)); } @Test - public void loadAuthorizedClientWhenPrincipalNameIsNullThenExceptionNotThrown() { + void loadAuthorizedClientWhenPrincipalNameIsNullThenExceptionNotThrown() { this.authorizedClientRepository.loadAuthorizedClient(this.registrationId1, null, this.request); } @Test - public void loadAuthorizedClientWhenRequestIsNullThenThrowIllegalArgumentException() { + void loadAuthorizedClientWhenRequestIsNullThenThrowIllegalArgumentException() { assertThatIllegalArgumentException().isThrownBy(() -> this.authorizedClientRepository.loadAuthorizedClient(this.registrationId1, null, null)); } @Test - public void loadAuthorizedClientWhenClientRegistrationNotFoundThenReturnNull() { + void loadAuthorizedClientWhenClientRegistrationNotFoundThenReturnNull() { OAuth2AuthorizedClient authorizedClient = this.authorizedClientRepository.loadAuthorizedClient("registration-not-found", null, this.request); assertThat(authorizedClient).isNull(); } @Test - public void loadAuthorizedClientWhenSavedThenReturnAuthorizedClient() { + void loadAuthorizedClientWhenSavedThenReturnAuthorizedClient() { this.authorizedClientRepository.saveAuthorizedClient(authorizedClient1, null, this.request, this.response); OAuth2AuthorizedClient loadedAuthorizedClient = this.authorizedClientRepository.loadAuthorizedClient(this.registrationId1, null, this.request); @@ -115,30 +115,30 @@ public void loadAuthorizedClientWhenSavedThenReturnAuthorizedClient() { } @Test - public void saveAuthorizedClientWhenAuthorizedClientIsNullThenThrowIllegalArgumentException() { + void saveAuthorizedClientWhenAuthorizedClientIsNullThenThrowIllegalArgumentException() { assertThatIllegalArgumentException().isThrownBy(() -> this.authorizedClientRepository.saveAuthorizedClient(null, null, this.request, this.response)); } @Test - public void saveAuthorizedClientWhenAuthenticationIsNullThenExceptionNotThrown() { + void saveAuthorizedClientWhenAuthenticationIsNullThenExceptionNotThrown() { this.authorizedClientRepository.saveAuthorizedClient(authorizedClient1, null, this.request, this.response); } @Test - public void saveAuthorizedClientWhenRequestIsNullThenThrowIllegalArgumentException() { + void saveAuthorizedClientWhenRequestIsNullThenThrowIllegalArgumentException() { assertThatIllegalArgumentException().isThrownBy(() -> this.authorizedClientRepository.saveAuthorizedClient(authorizedClient1, null, null, this.response)); } @Test - public void saveAuthorizedClientWhenResponseIsNullThenThrowIllegalArgumentException() { + void saveAuthorizedClientWhenResponseIsNullThenThrowIllegalArgumentException() { assertThatIllegalArgumentException().isThrownBy(() -> this.authorizedClientRepository.saveAuthorizedClient(authorizedClient1, null, this.request, null)); } @Test - public void saveAuthorizedClientWhenSavedThenSavedToSession() { + void saveAuthorizedClientWhenSavedThenSavedToSession() { this.authorizedClientRepository.saveAuthorizedClient(authorizedClient1, null, this.request, this.response); HttpSession session = this.request.getSession(false); @@ -153,35 +153,35 @@ public void saveAuthorizedClientWhenSavedThenSavedToSession() { } @Test - public void removeAuthorizedClientWhenClientRegistrationIdIsNullThenThrowIllegalArgumentException() { + void removeAuthorizedClientWhenClientRegistrationIdIsNullThenThrowIllegalArgumentException() { assertThatIllegalArgumentException().isThrownBy(() -> this.authorizedClientRepository.removeAuthorizedClient(null, null, this.request, this.response)); } @Test - public void removeAuthorizedClientWhenPrincipalNameIsNullThenExceptionNotThrown() { + void removeAuthorizedClientWhenPrincipalNameIsNullThenExceptionNotThrown() { this.authorizedClientRepository.removeAuthorizedClient(this.registrationId1, null, this.request, this.response); } @Test - public void removeAuthorizedClientWhenRequestIsNullThenThrowIllegalArgumentException() { + void removeAuthorizedClientWhenRequestIsNullThenThrowIllegalArgumentException() { assertThatIllegalArgumentException().isThrownBy(() -> this.authorizedClientRepository.removeAuthorizedClient(this.registrationId1, null, null, this.response)); } @Test - public void removeAuthorizedClientWhenResponseIsNullThenExceptionNotThrown() { + void removeAuthorizedClientWhenResponseIsNullThenExceptionNotThrown() { this.authorizedClientRepository.removeAuthorizedClient(this.registrationId1, null, this.request, null); } @Test - public void removeAuthorizedClientWhenNotSavedThenSessionNotCreated() { + void removeAuthorizedClientWhenNotSavedThenSessionNotCreated() { this.authorizedClientRepository.removeAuthorizedClient(this.registrationId2, null, this.request, this.response); assertThat(this.request.getSession(false)).isNull(); } @Test - public void removeAuthorizedClientWhenClient1SavedAndClient2RemovedThenClient1NotRemoved() { + 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); @@ -192,7 +192,7 @@ public void removeAuthorizedClientWhenClient1SavedAndClient2RemovedThenClient1No } @Test - public void removeAuthorizedClientWhenSavedThenRemoved() { + void removeAuthorizedClientWhenSavedThenRemoved() { this.authorizedClientRepository.saveAuthorizedClient(authorizedClient1, null, this.request, this.response); OAuth2AuthorizedClient loadedAuthorizedClient = this.authorizedClientRepository.loadAuthorizedClient(this.registrationId1, null, this.request); @@ -203,7 +203,7 @@ public void removeAuthorizedClientWhenSavedThenRemoved() { } @Test - public void removeAuthorizedClientWhenSavedThenRemovedFromSession() { + void removeAuthorizedClientWhenSavedThenRemovedFromSession() { this.authorizedClientRepository.saveAuthorizedClient(authorizedClient1, null, this.request, this.response); OAuth2AuthorizedClient loadedAuthorizedClient = this.authorizedClientRepository.loadAuthorizedClient(this.registrationId1, null, this.request); @@ -215,7 +215,7 @@ public void removeAuthorizedClientWhenSavedThenRemovedFromSession() { } @Test - public void removeAuthorizedClientWhenClient1Client2SavedAndClient1RemovedThenClient2NotRemoved() { + 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); From 4f75d30c8ca70978a04dcf352d1f9ac0a06b2864 Mon Sep 17 00:00:00 2001 From: Rujun Chen Date: Wed, 12 Oct 2022 14:49:41 +0800 Subject: [PATCH 12/12] Fix error reported by "maven-checkstyle-plugin". --- .../aad/implementation/jackson/SerializerUtils.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 32b772349a13..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 @@ -34,7 +34,7 @@ private SerializerUtils() { } /** - * Serialize Map to {@link String}. + * 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}. */ @@ -49,7 +49,7 @@ public static String serializeOAuth2AuthorizedClientMap(Map. + * 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. */