diff --git a/core/src/main/java/org/springframework/security/authentication/ProviderManager.java b/core/src/main/java/org/springframework/security/authentication/ProviderManager.java index ee487d472bd..8c2f3a72b78 100644 --- a/core/src/main/java/org/springframework/security/authentication/ProviderManager.java +++ b/core/src/main/java/org/springframework/security/authentication/ProviderManager.java @@ -15,6 +15,7 @@ */ package org.springframework.security.authentication; +import java.util.Arrays; import java.util.Collections; import java.util.List; @@ -104,6 +105,10 @@ public ProviderManager(List providers) { this(providers, null); } + public ProviderManager(AuthenticationProvider... providers) { + this(Arrays.asList(providers), null); + } + public ProviderManager(List providers, AuthenticationManager parent) { Assert.notNull(providers, "providers list cannot be null"); @@ -124,6 +129,9 @@ private void checkState() { throw new IllegalArgumentException( "A parent AuthenticationManager or a list " + "of AuthenticationProviders is required"); + } else if (providers.contains(null)) { + throw new IllegalArgumentException( + "providers list cannot contain null values"); } } 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 d15a84915b2..8b9249b8181 100644 --- a/core/src/test/java/org/springframework/security/authentication/ProviderManagerTests.java +++ b/core/src/test/java/org/springframework/security/authentication/ProviderManagerTests.java @@ -35,11 +35,10 @@ * * @author Ben Alex */ -@SuppressWarnings("unchecked") public class ProviderManagerTests { @Test(expected = ProviderNotFoundException.class) - public void authenticationFailsWithUnsupportedToken() throws Exception { + public void authenticationFailsWithUnsupportedToken() { Authentication token = new AbstractAuthenticationToken(null) { public Object getCredentials() { return ""; @@ -55,7 +54,7 @@ public Object getPrincipal() { } @Test - public void credentialsAreClearedByDefault() throws Exception { + public void credentialsAreClearedByDefault() { UsernamePasswordAuthenticationToken token = new UsernamePasswordAuthenticationToken( "Test", "Password"); ProviderManager mgr = makeProviderManager(); @@ -71,8 +70,7 @@ public void credentialsAreClearedByDefault() throws Exception { @Test public void authenticationSucceedsWithSupportedTokenAndReturnsExpectedObject() { final Authentication a = mock(Authentication.class); - ProviderManager mgr = new ProviderManager( - Arrays.asList(createProviderWhichReturns(a))); + ProviderManager mgr = new ProviderManager(createProviderWhichReturns(a)); AuthenticationEventPublisher publisher = mock(AuthenticationEventPublisher.class); mgr.setAuthenticationEventPublisher(publisher); @@ -95,8 +93,13 @@ public void authenticationSucceedsWhenFirstProviderReturnsNullButSecondAuthentic } @Test(expected = IllegalArgumentException.class) - public void testStartupFailsIfProvidersNotSet() { - new ProviderManager(null); + public void testStartupFailsIfProvidersNotSetAsList() { + new ProviderManager((List) null); + } + + @Test(expected = IllegalArgumentException.class) + public void testStartupFailsIfProvidersNotSetAsVarargs() { + new ProviderManager((AuthenticationProvider) null); } @Test @@ -117,7 +120,7 @@ public boolean supports(Class authentication) { } }; - ProviderManager authMgr = new ProviderManager(Arrays.asList(provider)); + ProviderManager authMgr = new ProviderManager(provider); TestingAuthenticationToken request = createAuthenticationToken(); request.setDetails(requestDetails); @@ -127,8 +130,7 @@ public boolean supports(Class authentication) { } @Test - public void detailsAreSetOnAuthenticationTokenIfNotAlreadySetByProvider() - throws Exception { + public void detailsAreSetOnAuthenticationTokenIfNotAlreadySetByProvider() { Object details = new Object(); ProviderManager authMgr = makeProviderManager(); @@ -144,8 +146,8 @@ public void detailsAreSetOnAuthenticationTokenIfNotAlreadySetByProvider() public void authenticationExceptionIsIgnoredIfLaterProviderAuthenticates() { final Authentication authReq = mock(Authentication.class); ProviderManager mgr = new ProviderManager( - Arrays.asList(createProviderWhichThrows(new BadCredentialsException("", - new Throwable())), createProviderWhichReturns(authReq))); + createProviderWhichThrows(new BadCredentialsException("", + new Throwable())), createProviderWhichReturns(authReq)); assertThat(mgr.authenticate(mock(Authentication.class))).isSameAs(authReq); } @@ -180,7 +182,7 @@ public void accountStatusExceptionPreventsCallsToSubsequentProviders() { } catch (AccountStatusException expected) { } - verifyZeroInteractions(otherProvider); + verifyNoInteractions(otherProvider); } @Test @@ -189,7 +191,7 @@ public void parentAuthenticationIsUsedIfProvidersDontAuthenticate() { Authentication authReq = mock(Authentication.class); when(parent.authenticate(authReq)).thenReturn(authReq); ProviderManager mgr = new ProviderManager( - Arrays.asList(mock(AuthenticationProvider.class)), parent); + Collections.singletonList(mock(AuthenticationProvider.class)), parent); assertThat(mgr.authenticate(authReq)).isSameAs(authReq); } @@ -200,14 +202,14 @@ public void parentIsNotCalledIfAccountStatusExceptionIsThrown() { }); AuthenticationManager parent = mock(AuthenticationManager.class); ProviderManager mgr = new ProviderManager( - Arrays.asList(iThrowAccountStatusException), parent); + Collections.singletonList(iThrowAccountStatusException), parent); try { mgr.authenticate(mock(Authentication.class)); fail("Expected exception"); } catch (AccountStatusException expected) { } - verifyZeroInteractions(parent); + verifyNoInteractions(parent); } @Test @@ -220,7 +222,7 @@ public void providerNotFoundFromParentIsIgnored() { // Set a provider that throws an exception - this is the exception we expect to be // propagated ProviderManager mgr = new ProviderManager( - Arrays.asList(createProviderWhichThrows(new BadCredentialsException(""))), + Collections.singletonList(createProviderWhichThrows(new BadCredentialsException(""))), parent); mgr.setAuthenticationEventPublisher(publisher); @@ -237,7 +239,7 @@ public void providerNotFoundFromParentIsIgnored() { public void authenticationExceptionFromParentOverridesPreviousOnes() { AuthenticationManager parent = mock(AuthenticationManager.class); ProviderManager mgr = new ProviderManager( - Arrays.asList(createProviderWhichThrows(new BadCredentialsException(""))), + Collections.singletonList(createProviderWhichThrows(new BadCredentialsException(""))), parent); final Authentication authReq = mock(Authentication.class); AuthenticationEventPublisher publisher = mock(AuthenticationEventPublisher.class); @@ -257,12 +259,11 @@ public void authenticationExceptionFromParentOverridesPreviousOnes() { } @Test - @SuppressWarnings("deprecation") public void statusExceptionIsPublished() { AuthenticationManager parent = mock(AuthenticationManager.class); final LockedException expected = new LockedException(""); ProviderManager mgr = new ProviderManager( - Arrays.asList(createProviderWhichThrows(expected)), parent); + Collections.singletonList(createProviderWhichThrows(expected)), parent); final Authentication authReq = mock(Authentication.class); AuthenticationEventPublisher publisher = mock(AuthenticationEventPublisher.class); mgr.setAuthenticationEventPublisher(publisher); @@ -298,10 +299,9 @@ public void providerThrowsInternalAuthenticationServiceException() { @Test public void authenticateWhenFailsInParentAndPublishesThenChildDoesNotPublish() { BadCredentialsException badCredentialsExParent = new BadCredentialsException("Bad Credentials in parent"); - ProviderManager parentMgr = new ProviderManager( - Collections.singletonList(createProviderWhichThrows(badCredentialsExParent))); + ProviderManager parentMgr = new ProviderManager(createProviderWhichThrows(badCredentialsExParent)); ProviderManager childMgr = new ProviderManager(Collections.singletonList(createProviderWhichThrows( - new BadCredentialsException("Bad Credentials in child"))), parentMgr); + new BadCredentialsException("Bad Credentials in child"))), parentMgr); AuthenticationEventPublisher publisher = mock(AuthenticationEventPublisher.class); parentMgr.setAuthenticationEventPublisher(publisher); @@ -343,17 +343,14 @@ private TestingAuthenticationToken createAuthenticationToken() { } private ProviderManager makeProviderManager() { - MockProvider provider1 = new MockProvider(); - List providers = new ArrayList<>(); - providers.add(provider1); - - return new ProviderManager(providers); + MockProvider provider = new MockProvider(); + return new ProviderManager(provider); } // ~ Inner Classes // ================================================================================================== - private class MockProvider implements AuthenticationProvider { + private static class MockProvider implements AuthenticationProvider { public Authentication authenticate(Authentication authentication) throws AuthenticationException { if (supports(authentication.getClass())) { @@ -367,7 +364,7 @@ public Authentication authenticate(Authentication authentication) public boolean supports(Class authentication) { return TestingAuthenticationToken.class.isAssignableFrom(authentication) || UsernamePasswordAuthenticationToken.class - .isAssignableFrom(authentication); + .isAssignableFrom(authentication); } } }