From 7f22a3459fba6f1c115d484bc66f3cb959ee5f9b Mon Sep 17 00:00:00 2001 From: Josh Cummings <3627351+jzheaux@users.noreply.github.com> Date: Fri, 21 Mar 2025 15:12:25 -0600 Subject: [PATCH] Polish Tests Issue gh-16444 --- .../authentication/ProviderManagerTests.java | 75 +++++++++---------- 1 file changed, 36 insertions(+), 39 deletions(-) diff --git a/core/src/test/java/org/springframework/security/authentication/ProviderManagerTests.java b/core/src/test/java/org/springframework/security/authentication/ProviderManagerTests.java index 9b98bd522a..3bdf852558 100644 --- a/core/src/test/java/org/springframework/security/authentication/ProviderManagerTests.java +++ b/core/src/test/java/org/springframework/security/authentication/ProviderManagerTests.java @@ -18,7 +18,6 @@ package org.springframework.security.authentication; import java.util.ArrayList; import java.util.Arrays; -import java.util.Collections; import java.util.List; import org.junit.jupiter.api.Test; @@ -47,7 +46,7 @@ import static org.mockito.Mockito.verifyNoMoreInteractions; public class ProviderManagerTests { @Test - public void authenticationFailsWithUnsupportedToken() { + void authenticationFailsWithUnsupportedToken() { Authentication token = new AbstractAuthenticationToken(null) { @Override public Object getCredentials() { @@ -65,7 +64,7 @@ public class ProviderManagerTests { } @Test - public void credentialsAreClearedByDefault() { + void credentialsAreClearedByDefault() { UsernamePasswordAuthenticationToken token = UsernamePasswordAuthenticationToken.unauthenticated("Test", "Password"); ProviderManager mgr = makeProviderManager(); @@ -78,8 +77,8 @@ public class ProviderManagerTests { } @Test - public void authenticationSucceedsWithSupportedTokenAndReturnsExpectedObject() { - final Authentication a = mock(Authentication.class); + void authenticationSucceedsWithSupportedTokenAndReturnsExpectedObject() { + Authentication a = mock(Authentication.class); ProviderManager mgr = new ProviderManager(createProviderWhichReturns(a)); AuthenticationEventPublisher publisher = mock(AuthenticationEventPublisher.class); mgr.setAuthenticationEventPublisher(publisher); @@ -89,8 +88,8 @@ public class ProviderManagerTests { } @Test - public void authenticationSucceedsWhenFirstProviderReturnsNullButSecondAuthenticates() { - final Authentication a = mock(Authentication.class); + void authenticationSucceedsWhenFirstProviderReturnsNullButSecondAuthenticates() { + Authentication a = mock(Authentication.class); ProviderManager mgr = new ProviderManager( Arrays.asList(createProviderWhichReturns(null), createProviderWhichReturns(a))); AuthenticationEventPublisher publisher = mock(AuthenticationEventPublisher.class); @@ -101,24 +100,24 @@ public class ProviderManagerTests { } @Test - public void testStartupFailsIfProvidersNotSetAsList() { + void testStartupFailsIfProvidersNotSetAsList() { assertThatIllegalArgumentException().isThrownBy(() -> new ProviderManager((List) null)); } @Test - public void testStartupFailsIfProvidersNotSetAsVarargs() { + void testStartupFailsIfProvidersNotSetAsVarargs() { assertThatIllegalArgumentException().isThrownBy(() -> new ProviderManager((AuthenticationProvider) null)); } @Test - public void testStartupFailsIfProvidersContainNullElement() { + void testStartupFailsIfProvidersContainNullElement() { assertThatIllegalArgumentException() .isThrownBy(() -> new ProviderManager(Arrays.asList(mock(AuthenticationProvider.class), null))); } // gh-8689 @Test - public void constructorWhenUsingListOfThenNoException() { + void constructorWhenUsingListOfThenNoException() { List providers = spy(ArrayList.class); // List.of(null) in JDK 9 throws a NullPointerException given(providers.contains(eq(null))).willThrow(NullPointerException.class); @@ -127,7 +126,7 @@ public class ProviderManagerTests { } @Test - public void detailsAreNotSetOnAuthenticationTokenIfAlreadySetByProvider() { + void detailsAreNotSetOnAuthenticationTokenIfAlreadySetByProvider() { Object requestDetails = "(Request Details)"; final Object resultDetails = "(Result Details)"; // A provider which sets the details object @@ -151,7 +150,7 @@ public class ProviderManagerTests { } @Test - public void detailsAreSetOnAuthenticationTokenIfNotAlreadySetByProvider() { + void detailsAreSetOnAuthenticationTokenIfNotAlreadySetByProvider() { Object details = new Object(); ProviderManager authMgr = makeProviderManager(); TestingAuthenticationToken request = createAuthenticationToken(); @@ -162,8 +161,8 @@ public class ProviderManagerTests { } @Test - public void authenticationExceptionIsIgnoredIfLaterProviderAuthenticates() { - final Authentication authReq = mock(Authentication.class); + void authenticationExceptionIsIgnoredIfLaterProviderAuthenticates() { + Authentication authReq = mock(Authentication.class); ProviderManager mgr = new ProviderManager( createProviderWhichThrows(new BadCredentialsException("", new Throwable())), createProviderWhichReturns(authReq)); @@ -171,7 +170,7 @@ public class ProviderManagerTests { } @Test - public void authenticationExceptionIsRethrownIfNoLaterProviderAuthenticates() { + void authenticationExceptionIsRethrownIfNoLaterProviderAuthenticates() { ProviderManager mgr = new ProviderManager(Arrays .asList(createProviderWhichThrows(new BadCredentialsException("")), createProviderWhichReturns(null))); assertThatExceptionOfType(BadCredentialsException.class) @@ -180,7 +179,7 @@ public class ProviderManagerTests { // SEC-546 @Test - public void accountStatusExceptionPreventsCallsToSubsequentProviders() { + void accountStatusExceptionPreventsCallsToSubsequentProviders() { AuthenticationProvider iThrowAccountStatusException = createProviderWhichThrows(new AccountStatusException("") { }); AuthenticationProvider otherProvider = mock(AuthenticationProvider.class); @@ -191,48 +190,47 @@ public class ProviderManagerTests { } @Test - public void parentAuthenticationIsUsedIfProvidersDontAuthenticate() { + void parentAuthenticationIsUsedIfProvidersDontAuthenticate() { AuthenticationManager parent = mock(AuthenticationManager.class); Authentication authReq = mock(Authentication.class); given(parent.authenticate(authReq)).willReturn(authReq); - ProviderManager mgr = new ProviderManager(Collections.singletonList(mock(AuthenticationProvider.class)), - parent); + ProviderManager mgr = new ProviderManager(List.of(mock(AuthenticationProvider.class)), parent); assertThat(mgr.authenticate(authReq)).isSameAs(authReq); } @Test - public void parentIsNotCalledIfAccountStatusExceptionIsThrown() { + void parentIsNotCalledIfAccountStatusExceptionIsThrown() { AuthenticationProvider iThrowAccountStatusException = createProviderWhichThrows( new AccountStatusException("", new Throwable()) { }); AuthenticationManager parent = mock(AuthenticationManager.class); - ProviderManager mgr = new ProviderManager(Collections.singletonList(iThrowAccountStatusException), parent); + ProviderManager mgr = new ProviderManager(List.of(iThrowAccountStatusException), parent); assertThatExceptionOfType(AccountStatusException.class) .isThrownBy(() -> mgr.authenticate(mock(Authentication.class))); verifyNoInteractions(parent); } @Test - public void providerNotFoundFromParentIsIgnored() { + void providerNotFoundFromParentIsIgnored() { final Authentication authReq = mock(Authentication.class); AuthenticationEventPublisher publisher = mock(AuthenticationEventPublisher.class); AuthenticationManager parent = mock(AuthenticationManager.class); given(parent.authenticate(authReq)).willThrow(new ProviderNotFoundException("")); // Set a provider that throws an exception - this is the exception we expect to be // propagated - ProviderManager mgr = new ProviderManager( - Collections.singletonList(createProviderWhichThrows(new BadCredentialsException(""))), parent); + ProviderManager mgr = new ProviderManager(List.of(createProviderWhichThrows(new BadCredentialsException(""))), + parent); mgr.setAuthenticationEventPublisher(publisher); assertThatExceptionOfType(BadCredentialsException.class).isThrownBy(() -> mgr.authenticate(authReq)) .satisfies((ex) -> verify(publisher).publishAuthenticationFailure(ex, authReq)); } @Test - public void authenticationExceptionFromParentOverridesPreviousOnes() { + void authenticationExceptionFromParentOverridesPreviousOnes() { AuthenticationManager parent = mock(AuthenticationManager.class); - ProviderManager mgr = new ProviderManager( - Collections.singletonList(createProviderWhichThrows(new BadCredentialsException(""))), parent); - final Authentication authReq = mock(Authentication.class); + ProviderManager mgr = new ProviderManager(List.of(createProviderWhichThrows(new BadCredentialsException(""))), + parent); + Authentication authReq = mock(Authentication.class); AuthenticationEventPublisher publisher = mock(AuthenticationEventPublisher.class); mgr.setAuthenticationEventPublisher(publisher); // Set a provider that throws an exception - this is the exception we expect to be @@ -244,12 +242,11 @@ public class ProviderManagerTests { } @Test - public void statusExceptionIsPublished() { + void statusExceptionIsPublished() { AuthenticationManager parent = mock(AuthenticationManager.class); - final LockedException expected = new LockedException(""); - ProviderManager mgr = new ProviderManager(Collections.singletonList(createProviderWhichThrows(expected)), - parent); - final Authentication authReq = mock(Authentication.class); + LockedException expected = new LockedException(""); + ProviderManager mgr = new ProviderManager(List.of(createProviderWhichThrows(expected)), parent); + Authentication authReq = mock(Authentication.class); AuthenticationEventPublisher publisher = mock(AuthenticationEventPublisher.class); mgr.setAuthenticationEventPublisher(publisher); assertThatExceptionOfType(LockedException.class).isThrownBy(() -> mgr.authenticate(authReq)); @@ -258,7 +255,7 @@ public class ProviderManagerTests { // SEC-2367 @Test - public void providerThrowsInternalAuthenticationServiceException() { + void providerThrowsInternalAuthenticationServiceException() { InternalAuthenticationServiceException expected = new InternalAuthenticationServiceException("Expected"); ProviderManager mgr = new ProviderManager(Arrays.asList(createProviderWhichThrows(expected), createProviderWhichThrows(new BadCredentialsException("Oops"))), null); @@ -269,15 +266,15 @@ public class ProviderManagerTests { // gh-6281 @Test - public void authenticateWhenFailsInParentAndPublishesThenChildDoesNotPublish() { + void authenticateWhenFailsInParentAndPublishesThenChildDoesNotPublish() { BadCredentialsException badCredentialsExParent = new BadCredentialsException("Bad Credentials in parent"); ProviderManager parentMgr = new ProviderManager(createProviderWhichThrows(badCredentialsExParent)); - ProviderManager childMgr = new ProviderManager(Collections.singletonList( - createProviderWhichThrows(new BadCredentialsException("Bad Credentials in child"))), parentMgr); + ProviderManager childMgr = new ProviderManager( + List.of(createProviderWhichThrows(new BadCredentialsException("Bad Credentials in child"))), parentMgr); AuthenticationEventPublisher publisher = mock(AuthenticationEventPublisher.class); parentMgr.setAuthenticationEventPublisher(publisher); childMgr.setAuthenticationEventPublisher(publisher); - final Authentication authReq = mock(Authentication.class); + Authentication authReq = mock(Authentication.class); assertThatExceptionOfType(BadCredentialsException.class).isThrownBy(() -> childMgr.authenticate(authReq)) .isSameAs(badCredentialsExParent); verify(publisher).publishAuthenticationFailure(badCredentialsExParent, authReq); // Parent