From 23d61d43e5abb1d375c181b53139d5ca8195f099 Mon Sep 17 00:00:00 2001 From: Joe Grandja Date: Mon, 8 Jul 2019 11:56:46 -0400 Subject: [PATCH] Polish #5994 --- ...InMemoryOAuth2AuthorizedClientService.java | 29 +++++--- ...ReactiveOAuth2AuthorizedClientService.java | 22 +++--- .../client/OAuth2AuthorizedClientId.java | 42 +++++------- ...oryOAuth2AuthorizedClientServiceTests.java | 29 ++++---- .../client/OAuth2AuthorizedClientIdTests.java | 67 ++++++++----------- 5 files changed, 86 insertions(+), 103 deletions(-) diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/InMemoryOAuth2AuthorizedClientService.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/InMemoryOAuth2AuthorizedClientService.java index 77718a15c7..2164b4ba1d 100644 --- a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/InMemoryOAuth2AuthorizedClientService.java +++ b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/InMemoryOAuth2AuthorizedClientService.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2019 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -32,12 +32,13 @@ import java.util.concurrent.ConcurrentHashMap; * @since 5.0 * @see OAuth2AuthorizedClientService * @see OAuth2AuthorizedClient + * @see OAuth2AuthorizedClientId * @see ClientRegistration * @see Authentication */ public final class InMemoryOAuth2AuthorizedClientService implements OAuth2AuthorizedClientService { + private final Map authorizedClients; private final ClientRegistrationRepository clientRegistrationRepository; - private Map authorizedClients = new ConcurrentHashMap<>(); /** * Constructs an {@code InMemoryOAuth2AuthorizedClientService} using the provided parameters. @@ -47,15 +48,22 @@ public final class InMemoryOAuth2AuthorizedClientService implements OAuth2Author public InMemoryOAuth2AuthorizedClientService(ClientRegistrationRepository clientRegistrationRepository) { Assert.notNull(clientRegistrationRepository, "clientRegistrationRepository cannot be null"); this.clientRegistrationRepository = clientRegistrationRepository; + this.authorizedClients = new ConcurrentHashMap<>(); } /** - * Sets the map of authorized clients to use. - * @param authorizedClients the map of authorized clients + * Constructs an {@code InMemoryOAuth2AuthorizedClientService} using the provided parameters. + * + * @since 5.2 + * @param clientRegistrationRepository the repository of client registrations + * @param authorizedClients the initial {@code Map} of authorized client(s) keyed by {@link OAuth2AuthorizedClientId} */ - public void setAuthorizedClients(Map authorizedClients) { - Assert.notNull(authorizedClients, "authorizedClients cannot be null"); - this.authorizedClients = authorizedClients; + public InMemoryOAuth2AuthorizedClientService(ClientRegistrationRepository clientRegistrationRepository, + Map authorizedClients) { + Assert.notNull(clientRegistrationRepository, "clientRegistrationRepository cannot be null"); + Assert.notEmpty(authorizedClients, "authorizedClients cannot be empty"); + this.clientRegistrationRepository = clientRegistrationRepository; + this.authorizedClients = new ConcurrentHashMap<>(authorizedClients); } @Override @@ -67,14 +75,14 @@ public final class InMemoryOAuth2AuthorizedClientService implements OAuth2Author if (registration == null) { return null; } - return (T) this.authorizedClients.get(OAuth2AuthorizedClientId.create(registration, principalName)); + return (T) this.authorizedClients.get(new OAuth2AuthorizedClientId(clientRegistrationId, principalName)); } @Override public void saveAuthorizedClient(OAuth2AuthorizedClient authorizedClient, Authentication principal) { Assert.notNull(authorizedClient, "authorizedClient cannot be null"); Assert.notNull(principal, "principal cannot be null"); - this.authorizedClients.put(OAuth2AuthorizedClientId.create(authorizedClient.getClientRegistration(), + this.authorizedClients.put(new OAuth2AuthorizedClientId(authorizedClient.getClientRegistration().getRegistrationId(), principal.getName()), authorizedClient); } @@ -84,8 +92,7 @@ public final class InMemoryOAuth2AuthorizedClientService implements OAuth2Author Assert.hasText(principalName, "principalName cannot be empty"); ClientRegistration registration = this.clientRegistrationRepository.findByRegistrationId(clientRegistrationId); if (registration != null) { - this.authorizedClients.remove(OAuth2AuthorizedClientId.create(registration, principalName)); + this.authorizedClients.remove(new OAuth2AuthorizedClientId(clientRegistrationId, principalName)); } } - } diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/InMemoryReactiveOAuth2AuthorizedClientService.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/InMemoryReactiveOAuth2AuthorizedClientService.java index 64db840ccd..66091f8c90 100644 --- a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/InMemoryReactiveOAuth2AuthorizedClientService.java +++ b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/InMemoryReactiveOAuth2AuthorizedClientService.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2019 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -15,16 +15,15 @@ */ package org.springframework.security.oauth2.client; -import java.util.Map; -import java.util.concurrent.ConcurrentHashMap; - import org.springframework.security.core.Authentication; import org.springframework.security.oauth2.client.registration.ClientRegistration; import org.springframework.security.oauth2.client.registration.ReactiveClientRegistrationRepository; import org.springframework.util.Assert; - import reactor.core.publisher.Mono; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; + /** * An {@link OAuth2AuthorizedClientService} that stores * {@link OAuth2AuthorizedClient Authorized Client(s)} in-memory. @@ -38,11 +37,11 @@ import reactor.core.publisher.Mono; * @see Authentication */ public final class InMemoryReactiveOAuth2AuthorizedClientService implements ReactiveOAuth2AuthorizedClientService { - private final Map authorizedClients = new ConcurrentHashMap<>();; + private final Map authorizedClients = new ConcurrentHashMap<>(); private final ReactiveClientRegistrationRepository clientRegistrationRepository; /** - * Constructs an {@code InMemoryOAuth2AuthorizedClientService} using the provided parameters. + * Constructs an {@code InMemoryReactiveOAuth2AuthorizedClientService} using the provided parameters. * * @param clientRegistrationRepository the repository of client registrations */ @@ -57,7 +56,7 @@ public final class InMemoryReactiveOAuth2AuthorizedClientService implements Reac Assert.hasText(clientRegistrationId, "clientRegistrationId cannot be empty"); Assert.hasText(principalName, "principalName cannot be empty"); return (Mono) this.clientRegistrationRepository.findByRegistrationId(clientRegistrationId) - .map(clientRegistration -> OAuth2AuthorizedClientId.create(clientRegistration, principalName)) + .map(clientRegistration -> new OAuth2AuthorizedClientId(clientRegistrationId, principalName)) .flatMap(identifier -> Mono.justOrEmpty(this.authorizedClients.get(identifier))); } @@ -66,8 +65,8 @@ public final class InMemoryReactiveOAuth2AuthorizedClientService implements Reac Assert.notNull(authorizedClient, "authorizedClient cannot be null"); Assert.notNull(principal, "principal cannot be null"); return Mono.fromRunnable(() -> { - OAuth2AuthorizedClientId identifier = OAuth2AuthorizedClientId.create( - authorizedClient.getClientRegistration(), principal.getName()); + OAuth2AuthorizedClientId identifier = new OAuth2AuthorizedClientId( + authorizedClient.getClientRegistration().getRegistrationId(), principal.getName()); this.authorizedClients.put(identifier, authorizedClient); }); } @@ -77,9 +76,8 @@ public final class InMemoryReactiveOAuth2AuthorizedClientService implements Reac Assert.hasText(clientRegistrationId, "clientRegistrationId cannot be empty"); Assert.hasText(principalName, "principalName cannot be empty"); return this.clientRegistrationRepository.findByRegistrationId(clientRegistrationId) - .map(clientRegistration -> OAuth2AuthorizedClientId.create(clientRegistration, principalName)) + .map(clientRegistration -> new OAuth2AuthorizedClientId(clientRegistrationId, principalName)) .doOnNext(this.authorizedClients::remove) .then(Mono.empty()); } - } diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/OAuth2AuthorizedClientId.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/OAuth2AuthorizedClientId.java index 2501b1f22f..3c0cb7d40d 100644 --- a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/OAuth2AuthorizedClientId.java +++ b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/OAuth2AuthorizedClientId.java @@ -1,11 +1,11 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2019 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. * You may obtain a copy of the License at * - * http://www.apache.org/licenses/LICENSE-2.0 + * https://www.apache.org/licenses/LICENSE-2.0 * * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an "AS IS" BASIS, @@ -13,15 +13,14 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - package org.springframework.security.oauth2.client; +import org.springframework.security.core.SpringSecurityCoreVersion; +import org.springframework.util.Assert; + import java.io.Serializable; import java.util.Objects; -import org.springframework.security.oauth2.client.registration.ClientRegistration; -import org.springframework.util.Assert; - /** * The identifier for {@link OAuth2AuthorizedClient}. * @@ -31,37 +30,29 @@ import org.springframework.util.Assert; * @see OAuth2AuthorizedClientService */ public final class OAuth2AuthorizedClientId implements Serializable { - + private static final long serialVersionUID = SpringSecurityCoreVersion.SERIAL_VERSION_UID; private final String clientRegistrationId; - private final String principalName; - private OAuth2AuthorizedClientId(String clientRegistrationId, String principalName) { - Assert.notNull(clientRegistrationId, "clientRegistrationId cannot be null"); - Assert.notNull(principalName, "principalName cannot be null"); + /** + * Constructs an {@code OAuth2AuthorizedClientId} using the provided parameters. + * + * @param clientRegistrationId the identifier for the client's registration + * @param principalName the name of the End-User {@code Principal} (Resource Owner) + */ + public OAuth2AuthorizedClientId(String clientRegistrationId, String principalName) { + Assert.hasText(clientRegistrationId, "clientRegistrationId cannot be empty"); + Assert.hasText(principalName, "principalName cannot be empty"); this.clientRegistrationId = clientRegistrationId; this.principalName = principalName; } - /** - * Factory method for creating new {@link OAuth2AuthorizedClientId} using - * {@link ClientRegistration} and principal name. - * @param clientRegistration the client registration - * @param principalName the principal name - * @return the new authorized client id - */ - public static OAuth2AuthorizedClientId create(ClientRegistration clientRegistration, - String principalName) { - return new OAuth2AuthorizedClientId(clientRegistration.getRegistrationId(), - principalName); - } - @Override public boolean equals(Object obj) { if (this == obj) { return true; } - if (obj == null || getClass() != obj.getClass()) { + if (obj == null || this.getClass() != obj.getClass()) { return false; } OAuth2AuthorizedClientId that = (OAuth2AuthorizedClientId) obj; @@ -73,5 +64,4 @@ public final class OAuth2AuthorizedClientId implements Serializable { public int hashCode() { return Objects.hash(this.clientRegistrationId, this.principalName); } - } diff --git a/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/InMemoryOAuth2AuthorizedClientServiceTests.java b/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/InMemoryOAuth2AuthorizedClientServiceTests.java index e6482a64a8..dbb00dbfcc 100644 --- a/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/InMemoryOAuth2AuthorizedClientServiceTests.java +++ b/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/InMemoryOAuth2AuthorizedClientServiceTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2019 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -15,11 +15,7 @@ */ package org.springframework.security.oauth2.client; -import java.util.Collections; -import java.util.Map; - import org.junit.Test; - import org.springframework.security.core.Authentication; import org.springframework.security.oauth2.client.registration.ClientRegistration; import org.springframework.security.oauth2.client.registration.ClientRegistrationRepository; @@ -27,10 +23,12 @@ import org.springframework.security.oauth2.client.registration.InMemoryClientReg import org.springframework.security.oauth2.client.registration.TestClientRegistrations; import org.springframework.security.oauth2.core.OAuth2AccessToken; +import java.util.Collections; +import java.util.Map; + import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatExceptionOfType; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.ArgumentMatchers.eq; -import static org.mockito.BDDMockito.given; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -66,25 +64,24 @@ public class InMemoryOAuth2AuthorizedClientServiceTests { } @Test - public void constructorWhenAuthorizedClientsIsNullThenIllegalArgumentException() { - assertThatExceptionOfType(IllegalArgumentException.class) - .isThrownBy(() -> this.authorizedClientService.setAuthorizedClients(null)) - .withMessage("authorizedClients cannot be null"); + public void constructorWhenAuthorizedClientsIsNullThenThrowIllegalArgumentException() { + assertThatThrownBy(() -> new InMemoryOAuth2AuthorizedClientService(this.clientRegistrationRepository, null)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("authorizedClients cannot be empty"); } @Test - public void constructorWhenAuthorizedClientsIsEmptyMapThenRepositoryUsingSuppliedAuthorizedClients() { + public void constructorWhenAuthorizedClientsProvidedThenUseProvidedAuthorizedClients() { String registrationId = this.registration3.getRegistrationId(); Map authorizedClients = Collections.singletonMap( - OAuth2AuthorizedClientId.create(this.registration3, this.principalName1), + new OAuth2AuthorizedClientId(this.registration3.getRegistrationId(), this.principalName1), mock(OAuth2AuthorizedClient.class)); ClientRegistrationRepository clientRegistrationRepository = mock(ClientRegistrationRepository.class); - given(clientRegistrationRepository.findByRegistrationId(eq(registrationId))).willReturn(this.registration3); + when(clientRegistrationRepository.findByRegistrationId(eq(registrationId))).thenReturn(this.registration3); InMemoryOAuth2AuthorizedClientService authorizedClientService = new InMemoryOAuth2AuthorizedClientService( - this.clientRegistrationRepository); - authorizedClientService.setAuthorizedClients(authorizedClients); + clientRegistrationRepository, authorizedClients); assertThat((OAuth2AuthorizedClient) authorizedClientService.loadAuthorizedClient( registrationId, this.principalName1)).isNotNull(); } diff --git a/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/OAuth2AuthorizedClientIdTests.java b/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/OAuth2AuthorizedClientIdTests.java index b979f42061..338f929eaf 100644 --- a/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/OAuth2AuthorizedClientIdTests.java +++ b/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/OAuth2AuthorizedClientIdTests.java @@ -1,11 +1,11 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2019 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. * You may obtain a copy of the License at * - * http://www.apache.org/licenses/LICENSE-2.0 + * https://www.apache.org/licenses/LICENSE-2.0 * * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an "AS IS" BASIS, @@ -13,15 +13,12 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - package org.springframework.security.oauth2.client; import org.junit.Test; -import org.springframework.security.oauth2.client.registration.ClientRegistration; -import org.springframework.security.oauth2.core.AuthorizationGrantType; - import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; /** * Tests for {@link OAuth2AuthorizedClientId}. @@ -30,65 +27,59 @@ import static org.assertj.core.api.Assertions.assertThat; */ public class OAuth2AuthorizedClientIdTests { + @Test + public void constructorWhenRegistrationIdNullThenThrowIllegalArgumentException() { + assertThatThrownBy(() -> new OAuth2AuthorizedClientId(null, "test-principal")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("clientRegistrationId cannot be empty"); + } + + @Test + public void constructorWhenPrincipalNameNullThenThrowIllegalArgumentException() { + assertThatThrownBy(() -> new OAuth2AuthorizedClientId("test-client", null)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("principalName cannot be empty"); + } + @Test public void equalsWhenSameRegistrationIdAndPrincipalThenShouldReturnTrue() { - OAuth2AuthorizedClientId id1 = OAuth2AuthorizedClientId.create(testClientRegistration("test-client"), - "test-principal"); - OAuth2AuthorizedClientId id2 = OAuth2AuthorizedClientId.create(testClientRegistration("test-client"), - "test-principal"); + OAuth2AuthorizedClientId id1 = new OAuth2AuthorizedClientId("test-client", "test-principal"); + OAuth2AuthorizedClientId id2 = new OAuth2AuthorizedClientId("test-client", "test-principal"); assertThat(id1.equals(id2)).isTrue(); } @Test public void equalsWhenDifferentRegistrationIdAndSamePrincipalThenShouldReturnFalse() { - OAuth2AuthorizedClientId id1 = OAuth2AuthorizedClientId.create(testClientRegistration("test-client1"), - "test-principal"); - OAuth2AuthorizedClientId id2 = OAuth2AuthorizedClientId.create(testClientRegistration("test-client2"), - "test-principal"); + OAuth2AuthorizedClientId id1 = new OAuth2AuthorizedClientId("test-client1", "test-principal"); + OAuth2AuthorizedClientId id2 = new OAuth2AuthorizedClientId("test-client2", "test-principal"); assertThat(id1.equals(id2)).isFalse(); } @Test public void equalsWhenSameRegistrationIdAndDifferentPrincipalThenShouldReturnFalse() { - OAuth2AuthorizedClientId id1 = OAuth2AuthorizedClientId.create(testClientRegistration("test-client"), - "test-principal1"); - OAuth2AuthorizedClientId id2 = OAuth2AuthorizedClientId.create(testClientRegistration("test-client"), - "test-principal2"); + OAuth2AuthorizedClientId id1 = new OAuth2AuthorizedClientId("test-client", "test-principal1"); + OAuth2AuthorizedClientId id2 = new OAuth2AuthorizedClientId("test-client", "test-principal2"); assertThat(id1.equals(id2)).isFalse(); } @Test public void hashCodeWhenSameRegistrationIdAndPrincipalThenShouldReturnSame() { - OAuth2AuthorizedClientId id1 = OAuth2AuthorizedClientId.create(testClientRegistration("test-client"), - "test-principal"); - OAuth2AuthorizedClientId id2 = OAuth2AuthorizedClientId.create(testClientRegistration("test-client"), - "test-principal"); + OAuth2AuthorizedClientId id1 = new OAuth2AuthorizedClientId("test-client", "test-principal"); + OAuth2AuthorizedClientId id2 = new OAuth2AuthorizedClientId("test-client", "test-principal"); assertThat(id1.hashCode()).isEqualTo(id2.hashCode()); } @Test public void hashCodeWhenDifferentRegistrationIdAndSamePrincipalThenShouldNotReturnSame() { - OAuth2AuthorizedClientId id1 = OAuth2AuthorizedClientId.create(testClientRegistration("test-client1"), - "test-principal"); - OAuth2AuthorizedClientId id2 = OAuth2AuthorizedClientId.create(testClientRegistration("test-client2"), - "test-principal"); + OAuth2AuthorizedClientId id1 = new OAuth2AuthorizedClientId("test-client1", "test-principal"); + OAuth2AuthorizedClientId id2 = new OAuth2AuthorizedClientId("test-client2", "test-principal"); assertThat(id1.hashCode()).isNotEqualTo(id2.hashCode()); } @Test public void hashCodeWhenSameRegistrationIdAndDifferentPrincipalThenShouldNotReturnSame() { - OAuth2AuthorizedClientId id1 = OAuth2AuthorizedClientId.create(testClientRegistration("test-client"), - "test-principal1"); - OAuth2AuthorizedClientId id2 = OAuth2AuthorizedClientId.create(testClientRegistration("test-client"), - "test-principal2"); + OAuth2AuthorizedClientId id1 = new OAuth2AuthorizedClientId("test-client", "test-principal1"); + OAuth2AuthorizedClientId id2 = new OAuth2AuthorizedClientId("test-client", "test-principal2"); assertThat(id1.hashCode()).isNotEqualTo(id2.hashCode()); } - - private static ClientRegistration testClientRegistration(String registrationId) { - return ClientRegistration.withRegistrationId(registrationId).clientId("id").clientSecret("secret") - .authorizationGrantType(AuthorizationGrantType.AUTHORIZATION_CODE) - .redirectUriTemplate("{baseUrl}/{action}/oauth2/code/{registrationId}") - .authorizationUri("http://example.com/authorize").tokenUri("http://example.com/token").build(); - } - }