From 1f835fec431ddd08cc006190f89b652eace1f733 Mon Sep 17 00:00:00 2001 From: Rob Winch Date: Fri, 30 Dec 2011 13:59:04 -0600 Subject: [PATCH] SEC-1867: Perform null check on Authentication.getCredentials() prior to calling toString() --- .../rcp/RemoteAuthenticationProvider.java | 3 ++- .../rcp/RemoteAuthenticationProviderTests.java | 12 ++++++++++++ .../rmi/ContextPropagatingRemoteInvocation.java | 6 +++++- .../rmi/ContextPropagatingRemoteInvocationTests.java | 10 ++++++++++ 4 files changed, 29 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/org/springframework/security/authentication/rcp/RemoteAuthenticationProvider.java b/core/src/main/java/org/springframework/security/authentication/rcp/RemoteAuthenticationProvider.java index be9a3288bb..f093a903b6 100644 --- a/core/src/main/java/org/springframework/security/authentication/rcp/RemoteAuthenticationProvider.java +++ b/core/src/main/java/org/springframework/security/authentication/rcp/RemoteAuthenticationProvider.java @@ -57,7 +57,8 @@ public class RemoteAuthenticationProvider implements AuthenticationProvider, Ini public Authentication authenticate(Authentication authentication) throws AuthenticationException { String username = authentication.getPrincipal().toString(); - String password = authentication.getCredentials().toString(); + Object credentials = authentication.getCredentials(); + String password = credentials == null ? null : credentials.toString(); Collection authorities = remoteAuthenticationManager.attemptAuthentication(username, password); return new UsernamePasswordAuthenticationToken(username, password, authorities); diff --git a/core/src/test/java/org/springframework/security/authentication/rcp/RemoteAuthenticationProviderTests.java b/core/src/test/java/org/springframework/security/authentication/rcp/RemoteAuthenticationProviderTests.java index 616a1be643..6946c43ff8 100644 --- a/core/src/test/java/org/springframework/security/authentication/rcp/RemoteAuthenticationProviderTests.java +++ b/core/src/test/java/org/springframework/security/authentication/rcp/RemoteAuthenticationProviderTests.java @@ -21,6 +21,7 @@ import junit.framework.TestCase; import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; import org.springframework.security.core.Authentication; +import org.springframework.security.core.AuthenticationException; import org.springframework.security.core.GrantedAuthority; import org.springframework.security.core.authority.AuthorityUtils; @@ -77,6 +78,17 @@ public class RemoteAuthenticationProviderTests extends TestCase { assertTrue(AuthorityUtils.authorityListToSet(result.getAuthorities()).contains("foo")); } + public void testNullCredentialsDoesNotCauseNullPointerException() { + RemoteAuthenticationProvider provider = new RemoteAuthenticationProvider(); + provider.setRemoteAuthenticationManager(new MockRemoteAuthenticationManager(false)); + + try { + provider.authenticate(new UsernamePasswordAuthenticationToken("rod", null)); + fail("Expected Exception"); + } catch(RemoteAuthenticationException success) {} + + } + public void testSupports() { RemoteAuthenticationProvider provider = new RemoteAuthenticationProvider(); assertTrue(provider.supports(UsernamePasswordAuthenticationToken.class)); diff --git a/remoting/src/main/java/org/springframework/security/remoting/rmi/ContextPropagatingRemoteInvocation.java b/remoting/src/main/java/org/springframework/security/remoting/rmi/ContextPropagatingRemoteInvocation.java index 517bb979f0..edba7638e9 100644 --- a/remoting/src/main/java/org/springframework/security/remoting/rmi/ContextPropagatingRemoteInvocation.java +++ b/remoting/src/main/java/org/springframework/security/remoting/rmi/ContextPropagatingRemoteInvocation.java @@ -66,13 +66,17 @@ public class ContextPropagatingRemoteInvocation extends RemoteInvocation { if (currentUser != null) { principal = currentUser.getName(); - credentials = currentUser.getCredentials().toString(); + Object userCredentials = currentUser.getCredentials(); + credentials = userCredentials == null ? null : userCredentials.toString(); } else { principal = credentials = null; } if (logger.isDebugEnabled()) { logger.debug("RemoteInvocation now has principal: " + principal); + if(credentials == null) { + logger.debug("RemoteInvocation now has null credentials."); + } } } diff --git a/remoting/src/test/java/org/springframework/security/remoting/rmi/ContextPropagatingRemoteInvocationTests.java b/remoting/src/test/java/org/springframework/security/remoting/rmi/ContextPropagatingRemoteInvocationTests.java index 95dfd7c698..47f4565a80 100644 --- a/remoting/src/test/java/org/springframework/security/remoting/rmi/ContextPropagatingRemoteInvocationTests.java +++ b/remoting/src/test/java/org/springframework/security/remoting/rmi/ContextPropagatingRemoteInvocationTests.java @@ -22,6 +22,7 @@ import org.springframework.security.authentication.UsernamePasswordAuthenticatio import org.springframework.security.core.Authentication; import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.security.util.SimpleMethodInvocation; +import org.springframework.test.util.ReflectionTestUtils; import java.lang.reflect.Method; @@ -95,4 +96,13 @@ public class ContextPropagatingRemoteInvocationTests extends TestCase { assertEquals("some_string Authentication empty", remoteInvocation.invoke(new TargetObject())); } + + // SEC-1867 + public void testNullCredentials() throws Exception { + Authentication clientSideAuthentication = new UsernamePasswordAuthenticationToken("rod", null); + SecurityContextHolder.getContext().setAuthentication(clientSideAuthentication); + + ContextPropagatingRemoteInvocation remoteInvocation = getRemoteInvocation(); + assertEquals(null, ReflectionTestUtils.getField(remoteInvocation, "credentials")); + } }