From 41457d2ace9a16ecd43f7bbf73402dc79403137c Mon Sep 17 00:00:00 2001 From: Arjen Poutsma Date: Sat, 16 Feb 2008 15:32:16 +0000 Subject: [PATCH] Fixed SWS-285 in 1.0 branch --- .../AbstractWsSecurityInterceptor.java | 34 ++++++++++++------- .../security/xwss/XwsSecurityInterceptor.java | 18 ++++++++++ .../xwss/callback/CleanupCallback.java | 33 ++++++++++++++++++ ...iCertificateValidationCallbackHandler.java | 6 +++- ...gestPasswordValidationCallbackHandler.java | 5 +++ ...TextPasswordValidationCallbackHandler.java | 7 +++- ...sCertificateValidationCallbackHandler.java | 2 +- .../xwss/XwsSecurityInterceptorTest.java | 3 +- ...tificateValidationCallbackHandlerTest.java | 20 ++++++++++- ...PasswordValidationCallbackHandlerTest.java | 24 ++++++++++++- ...PasswordValidationCallbackHandlerTest.java | 21 +++++++++++- .../callback/jaas/CertificateLoginModule.java | 3 +- 12 files changed, 154 insertions(+), 22 deletions(-) create mode 100644 security/src/main/java/org/springframework/ws/soap/security/xwss/callback/CleanupCallback.java diff --git a/security/src/main/java/org/springframework/ws/soap/security/AbstractWsSecurityInterceptor.java b/security/src/main/java/org/springframework/ws/soap/security/AbstractWsSecurityInterceptor.java index a6b6a917..b4380c1e 100644 --- a/security/src/main/java/org/springframework/ws/soap/security/AbstractWsSecurityInterceptor.java +++ b/security/src/main/java/org/springframework/ws/soap/security/AbstractWsSecurityInterceptor.java @@ -88,22 +88,27 @@ public abstract class AbstractWsSecurityInterceptor implements SoapEndpointInter } public final boolean handleResponse(MessageContext messageContext, Object endpoint) throws Exception { - if (secureResponse) { - Assert.isTrue(messageContext.getResponse() instanceof SoapMessage, - "WsSecurityInterceptor requires a SoapMessage response"); - try { - secureMessage((SoapMessage) messageContext.getResponse()); + try { + if (secureResponse) { + Assert.isTrue(messageContext.hasResponse(), "MessageContext contains no response"); + Assert.isInstanceOf(SoapMessage.class, messageContext.getResponse()); + try { + secureMessage((SoapMessage) messageContext.getResponse()); + return true; + } + catch (WsSecuritySecurementException ex) { + return handleSecurementException(ex, messageContext); + } + catch (WsSecurityFaultException ex) { + return handleFaultException(ex, messageContext); + } + } + else { return true; } - catch (WsSecuritySecurementException ex) { - return handleSecurementException(ex, messageContext); - } - catch (WsSecurityFaultException ex) { - return handleFaultException(ex, messageContext); - } } - else { - return true; + finally { + cleanUp(); } } @@ -111,6 +116,7 @@ public abstract class AbstractWsSecurityInterceptor implements SoapEndpointInter * Returns true, i.e. faults are not secured. */ public boolean handleFault(MessageContext messageContext, Object endpoint) throws Exception { + cleanUp(); return true; } @@ -191,4 +197,6 @@ public abstract class AbstractWsSecurityInterceptor implements SoapEndpointInter * @throws WsSecuritySecurementException in case of securement errors */ protected abstract void secureMessage(SoapMessage soapMessage) throws WsSecuritySecurementException; + + protected abstract void cleanUp(); } diff --git a/security/src/main/java/org/springframework/ws/soap/security/xwss/XwsSecurityInterceptor.java b/security/src/main/java/org/springframework/ws/soap/security/xwss/XwsSecurityInterceptor.java index dcfd27d4..c20de4ad 100644 --- a/security/src/main/java/org/springframework/ws/soap/security/xwss/XwsSecurityInterceptor.java +++ b/security/src/main/java/org/springframework/ws/soap/security/xwss/XwsSecurityInterceptor.java @@ -16,8 +16,11 @@ package org.springframework.ws.soap.security.xwss; +import java.io.IOException; import java.io.InputStream; +import javax.security.auth.callback.Callback; import javax.security.auth.callback.CallbackHandler; +import javax.security.auth.callback.UnsupportedCallbackException; import javax.xml.soap.SOAPMessage; import com.sun.xml.wss.ProcessingContext; @@ -33,6 +36,7 @@ import org.springframework.ws.soap.saaj.SaajSoapMessage; import org.springframework.ws.soap.security.AbstractWsSecurityInterceptor; import org.springframework.ws.soap.security.WsSecurityValidationException; import org.springframework.ws.soap.security.xwss.callback.CallbackHandlerChain; +import org.springframework.ws.soap.security.xwss.callback.CleanupCallback; /** * WS-Security endpoint interceptor that is based on Sun's XML and Web Services Security package (XWSS). This @@ -162,4 +166,18 @@ public class XwsSecurityInterceptor extends AbstractWsSecurityInterceptor implem } } + protected void cleanUp() { + if (callbackHandler != null) { + try { + CleanupCallback cleanupCallback = new CleanupCallback(); + callbackHandler.handle(new Callback[]{cleanupCallback}); + } + catch (IOException ex) { + logger.warn("Cleanup callback resulted in IOException", ex); + } + catch (UnsupportedCallbackException ex) { + // ignore + } + } + } } diff --git a/security/src/main/java/org/springframework/ws/soap/security/xwss/callback/CleanupCallback.java b/security/src/main/java/org/springframework/ws/soap/security/xwss/callback/CleanupCallback.java new file mode 100644 index 00000000..5381a0aa --- /dev/null +++ b/security/src/main/java/org/springframework/ws/soap/security/xwss/callback/CleanupCallback.java @@ -0,0 +1,33 @@ +/* + * Copyright 2008 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 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.ws.soap.security.xwss.callback; + +import java.io.Serializable; +import javax.security.auth.callback.Callback; + +/** + * Underlying security services instantiate and pass a CleanupCallback to the handle method of + * a CallbackHandler to clean up security state. + * + * @author Arjen Poutsma + * @since 1.0.4 + */ +public class CleanupCallback implements Callback, Serializable { + + private static final long serialVersionUID = 4744181820980888237L; + +} diff --git a/security/src/main/java/org/springframework/ws/soap/security/xwss/callback/acegi/AcegiCertificateValidationCallbackHandler.java b/security/src/main/java/org/springframework/ws/soap/security/xwss/callback/acegi/AcegiCertificateValidationCallbackHandler.java index d8a75845..b63021ae 100644 --- a/security/src/main/java/org/springframework/ws/soap/security/xwss/callback/acegi/AcegiCertificateValidationCallbackHandler.java +++ b/security/src/main/java/org/springframework/ws/soap/security/xwss/callback/acegi/AcegiCertificateValidationCallbackHandler.java @@ -29,6 +29,7 @@ import org.acegisecurity.context.SecurityContextHolder; import org.acegisecurity.providers.x509.X509AuthenticationToken; import org.springframework.util.Assert; import org.springframework.ws.soap.security.xwss.callback.AbstractCallbackHandler; +import org.springframework.ws.soap.security.xwss.callback.CleanupCallback; /** * Callback handler that validates a certificate using an Acegi AuthenticationManager. Logic based on @@ -78,6 +79,9 @@ public class AcegiCertificateValidationCallbackHandler extends AbstractCallbackH if (callback instanceof CertificateValidationCallback) { ((CertificateValidationCallback) callback).setValidator(new AcegiCertificateValidator()); } + else if (callback instanceof CleanupCallback) { + SecurityContextHolder.clearContext(); + } else { throw new UnsupportedCallbackException(callback); } @@ -103,7 +107,7 @@ public class AcegiCertificateValidationCallbackHandler extends AbstractCallbackH logger.debug("Authentication request for certificate with DN [" + certificate.getSubjectX500Principal().getName() + "] failed: " + failed.toString()); } - SecurityContextHolder.getContext().setAuthentication(null); + SecurityContextHolder.clearContext(); result = ignoreFailure; } return result; diff --git a/security/src/main/java/org/springframework/ws/soap/security/xwss/callback/acegi/AcegiDigestPasswordValidationCallbackHandler.java b/security/src/main/java/org/springframework/ws/soap/security/xwss/callback/acegi/AcegiDigestPasswordValidationCallbackHandler.java index 7e77d2fa..033a025c 100644 --- a/security/src/main/java/org/springframework/ws/soap/security/xwss/callback/acegi/AcegiDigestPasswordValidationCallbackHandler.java +++ b/security/src/main/java/org/springframework/ws/soap/security/xwss/callback/acegi/AcegiDigestPasswordValidationCallbackHandler.java @@ -32,6 +32,7 @@ import org.acegisecurity.userdetails.UsernameNotFoundException; import org.springframework.dao.DataAccessException; import org.springframework.util.Assert; import org.springframework.ws.soap.security.xwss.callback.AbstractCallbackHandler; +import org.springframework.ws.soap.security.xwss.callback.CleanupCallback; import org.springframework.ws.soap.security.xwss.callback.DefaultTimestampValidator; /** @@ -98,6 +99,10 @@ public class AcegiDigestPasswordValidationCallbackHandler extends AbstractCallba timestampCallback.setValidator(new DefaultTimestampValidator()); } + else if (callback instanceof CleanupCallback) { + SecurityContextHolder.clearContext(); + return; + } throw new UnsupportedCallbackException(callback); } diff --git a/security/src/main/java/org/springframework/ws/soap/security/xwss/callback/acegi/AcegiPlainTextPasswordValidationCallbackHandler.java b/security/src/main/java/org/springframework/ws/soap/security/xwss/callback/acegi/AcegiPlainTextPasswordValidationCallbackHandler.java index b35b7170..54251ce0 100644 --- a/security/src/main/java/org/springframework/ws/soap/security/xwss/callback/acegi/AcegiPlainTextPasswordValidationCallbackHandler.java +++ b/security/src/main/java/org/springframework/ws/soap/security/xwss/callback/acegi/AcegiPlainTextPasswordValidationCallbackHandler.java @@ -28,6 +28,7 @@ import org.acegisecurity.context.SecurityContextHolder; import org.acegisecurity.providers.UsernamePasswordAuthenticationToken; import org.springframework.util.Assert; import org.springframework.ws.soap.security.xwss.callback.AbstractCallbackHandler; +import org.springframework.ws.soap.security.xwss.callback.CleanupCallback; /** * Callback handler that validates a certificate uses an Acegi AuthenticationManager. Logic based on @@ -80,6 +81,10 @@ public class AcegiPlainTextPasswordValidationCallbackHandler extends AbstractCal return; } } + else if (callback instanceof CleanupCallback) { + SecurityContextHolder.clearContext(); + return; + } throw new UnsupportedCallbackException(callback); } @@ -103,7 +108,7 @@ public class AcegiPlainTextPasswordValidationCallbackHandler extends AbstractCal logger.debug("Authentication request for user '" + plainTextRequest.getUsername() + "' failed: " + failed.toString()); } - SecurityContextHolder.getContext().setAuthentication(null); + SecurityContextHolder.clearContext(); return ignoreFailure; } } diff --git a/security/src/main/java/org/springframework/ws/soap/security/xwss/callback/jaas/JaasCertificateValidationCallbackHandler.java b/security/src/main/java/org/springframework/ws/soap/security/xwss/callback/jaas/JaasCertificateValidationCallbackHandler.java index edd3f44c..9e4c7dd4 100644 --- a/security/src/main/java/org/springframework/ws/soap/security/xwss/callback/jaas/JaasCertificateValidationCallbackHandler.java +++ b/security/src/main/java/org/springframework/ws/soap/security/xwss/callback/jaas/JaasCertificateValidationCallbackHandler.java @@ -58,9 +58,9 @@ public class JaasCertificateValidationCallbackHandler extends AbstractJaasValida public boolean validate(X509Certificate certificate) throws CertificateValidationCallback.CertificateValidationException { - LoginContext loginContext = null; Subject subject = new Subject(); subject.getPrincipals().add(certificate.getSubjectX500Principal()); + LoginContext loginContext; try { loginContext = new LoginContext(getLoginContextName(), subject); } diff --git a/security/src/test/java/org/springframework/ws/soap/security/xwss/XwsSecurityInterceptorTest.java b/security/src/test/java/org/springframework/ws/soap/security/xwss/XwsSecurityInterceptorTest.java index 016c6428..807c00b1 100644 --- a/security/src/test/java/org/springframework/ws/soap/security/xwss/XwsSecurityInterceptorTest.java +++ b/security/src/test/java/org/springframework/ws/soap/security/xwss/XwsSecurityInterceptorTest.java @@ -74,8 +74,9 @@ public class XwsSecurityInterceptorTest extends TestCase { SOAPMessage request = messageFactory.createMessage(); MessageContext context = new DefaultMessageContext(new SaajSoapMessage(request), new SaajSoapMessageFactory(messageFactory)); + context.getResponse(); interceptor.handleResponse(context, null); assertEquals("Invalid response", securedResponse, ((SaajSoapMessage) context.getResponse()).getSaajMessage()); } -} \ No newline at end of file +} diff --git a/security/src/test/java/org/springframework/ws/soap/security/xwss/callback/acegi/AcegiCertificateValidationCallbackHandlerTest.java b/security/src/test/java/org/springframework/ws/soap/security/xwss/callback/acegi/AcegiCertificateValidationCallbackHandlerTest.java index e8076e84..e1bc7668 100644 --- a/security/src/test/java/org/springframework/ws/soap/security/xwss/callback/acegi/AcegiCertificateValidationCallbackHandlerTest.java +++ b/security/src/test/java/org/springframework/ws/soap/security/xwss/callback/acegi/AcegiCertificateValidationCallbackHandlerTest.java @@ -25,11 +25,13 @@ import junit.framework.TestCase; import org.acegisecurity.AuthenticationManager; import org.acegisecurity.BadCredentialsException; import org.acegisecurity.GrantedAuthority; +import org.acegisecurity.context.SecurityContextHolder; import org.acegisecurity.providers.TestingAuthenticationToken; import org.acegisecurity.providers.x509.X509AuthenticationToken; import org.easymock.MockControl; import org.springframework.core.io.ClassPathResource; +import org.springframework.ws.soap.security.xwss.callback.CleanupCallback; public class AcegiCertificateValidationCallbackHandlerTest extends TestCase { @@ -63,6 +65,10 @@ public class AcegiCertificateValidationCallbackHandlerTest extends TestCase { callback = new CertificateValidationCallback(certificate); } + protected void tearDown() throws Exception { + SecurityContextHolder.clearContext(); + } + public void testValidateCertificateValid() throws Exception { mock.authenticate(new X509AuthenticationToken(certificate)); control.setMatcher(MockControl.ALWAYS_MATCHER); @@ -71,6 +77,7 @@ public class AcegiCertificateValidationCallbackHandlerTest extends TestCase { callbackHandler.handleInternal(callback); boolean authenticated = callback.getResult(); assertTrue("Not authenticated", authenticated); + assertNotNull("No Authentication created", SecurityContextHolder.getContext().getAuthentication()); control.verify(); } @@ -82,7 +89,18 @@ public class AcegiCertificateValidationCallbackHandlerTest extends TestCase { callbackHandler.handleInternal(callback); boolean authenticated = callback.getResult(); assertFalse("Authenticated", authenticated); + assertNull("Authentication created", SecurityContextHolder.getContext().getAuthentication()); control.verify(); } -} \ No newline at end of file + public void testCleanUp() throws Exception { + TestingAuthenticationToken authentication = + new TestingAuthenticationToken(new Object(), new Object(), new GrantedAuthority[0]); + SecurityContextHolder.getContext().setAuthentication(authentication); + + CleanupCallback cleanupCallback = new CleanupCallback(); + callbackHandler.handleInternal(cleanupCallback); + assertNull("Authentication created", SecurityContextHolder.getContext().getAuthentication()); + } + +} diff --git a/security/src/test/java/org/springframework/ws/soap/security/xwss/callback/acegi/AcegiDigestPasswordValidationCallbackHandlerTest.java b/security/src/test/java/org/springframework/ws/soap/security/xwss/callback/acegi/AcegiDigestPasswordValidationCallbackHandlerTest.java index 0742a944..4266824b 100644 --- a/security/src/test/java/org/springframework/ws/soap/security/xwss/callback/acegi/AcegiDigestPasswordValidationCallbackHandlerTest.java +++ b/security/src/test/java/org/springframework/ws/soap/security/xwss/callback/acegi/AcegiDigestPasswordValidationCallbackHandlerTest.java @@ -19,11 +19,15 @@ package org.springframework.ws.soap.security.xwss.callback.acegi; import com.sun.xml.wss.impl.callback.PasswordValidationCallback; import junit.framework.TestCase; import org.acegisecurity.GrantedAuthority; +import org.acegisecurity.context.SecurityContextHolder; +import org.acegisecurity.providers.TestingAuthenticationToken; import org.acegisecurity.userdetails.User; import org.acegisecurity.userdetails.UserDetailsService; import org.acegisecurity.userdetails.UsernameNotFoundException; import org.easymock.MockControl; +import org.springframework.ws.soap.security.xwss.callback.CleanupCallback; + public class AcegiDigestPasswordValidationCallbackHandlerTest extends TestCase { private AcegiDigestPasswordValidationCallbackHandler callbackHandler; @@ -53,12 +57,17 @@ public class AcegiDigestPasswordValidationCallbackHandlerTest extends TestCase { callback = new PasswordValidationCallback(request); } + protected void tearDown() throws Exception { + SecurityContextHolder.clearContext(); + } + public void testAuthenticateUserDigestUserNotFound() throws Exception { control.expectAndThrow(mock.loadUserByUsername(username), new UsernameNotFoundException(username)); control.replay(); callbackHandler.handleInternal(callback); boolean authenticated = callback.getResult(); assertFalse("Authenticated", authenticated); + assertNull("Authentication created", SecurityContextHolder.getContext().getAuthentication()); control.verify(); } @@ -69,6 +78,7 @@ public class AcegiDigestPasswordValidationCallbackHandlerTest extends TestCase { callbackHandler.handleInternal(callback); boolean authenticated = callback.getResult(); assertTrue("Not authenticated", authenticated); + assertNotNull("No Authentication created", SecurityContextHolder.getContext().getAuthentication()); control.verify(); } @@ -79,6 +89,18 @@ public class AcegiDigestPasswordValidationCallbackHandlerTest extends TestCase { callbackHandler.handleInternal(callback); boolean authenticated = callback.getResult(); assertFalse("Authenticated", authenticated); + assertNull("Authentication created", SecurityContextHolder.getContext().getAuthentication()); control.verify(); } -} \ No newline at end of file + + public void testCleanUp() throws Exception { + TestingAuthenticationToken authentication = + new TestingAuthenticationToken(new Object(), new Object(), new GrantedAuthority[0]); + SecurityContextHolder.getContext().setAuthentication(authentication); + + CleanupCallback cleanupCallback = new CleanupCallback(); + callbackHandler.handleInternal(cleanupCallback); + assertNull("Authentication created", SecurityContextHolder.getContext().getAuthentication()); + } + +} diff --git a/security/src/test/java/org/springframework/ws/soap/security/xwss/callback/acegi/AcegiPlainTextPasswordValidationCallbackHandlerTest.java b/security/src/test/java/org/springframework/ws/soap/security/xwss/callback/acegi/AcegiPlainTextPasswordValidationCallbackHandlerTest.java index 353dcd44..6e906b53 100644 --- a/security/src/test/java/org/springframework/ws/soap/security/xwss/callback/acegi/AcegiPlainTextPasswordValidationCallbackHandlerTest.java +++ b/security/src/test/java/org/springframework/ws/soap/security/xwss/callback/acegi/AcegiPlainTextPasswordValidationCallbackHandlerTest.java @@ -22,10 +22,13 @@ import org.acegisecurity.Authentication; import org.acegisecurity.AuthenticationManager; import org.acegisecurity.BadCredentialsException; import org.acegisecurity.GrantedAuthority; +import org.acegisecurity.context.SecurityContextHolder; import org.acegisecurity.providers.TestingAuthenticationToken; import org.acegisecurity.providers.UsernamePasswordAuthenticationToken; import org.easymock.MockControl; +import org.springframework.ws.soap.security.xwss.callback.CleanupCallback; + public class AcegiPlainTextPasswordValidationCallbackHandlerTest extends TestCase { private AcegiPlainTextPasswordValidationCallbackHandler callbackHandler; @@ -52,6 +55,10 @@ public class AcegiPlainTextPasswordValidationCallbackHandlerTest extends TestCas callback = new PasswordValidationCallback(request); } + protected void tearDown() throws Exception { + SecurityContextHolder.clearContext(); + } + public void testAuthenticateUserPlainTextValid() throws Exception { Authentication authResult = new TestingAuthenticationToken(username, password, new GrantedAuthority[0]); control.expectAndReturn(mock.authenticate(new UsernamePasswordAuthenticationToken(username, password)), @@ -60,6 +67,7 @@ public class AcegiPlainTextPasswordValidationCallbackHandlerTest extends TestCas callbackHandler.handleInternal(callback); boolean authenticated = callback.getResult(); assertTrue("Not authenticated", authenticated); + assertNotNull("No Authentication created", SecurityContextHolder.getContext().getAuthentication()); control.verify(); } @@ -70,7 +78,18 @@ public class AcegiPlainTextPasswordValidationCallbackHandlerTest extends TestCas callbackHandler.handleInternal(callback); boolean authenticated = callback.getResult(); assertFalse("Authenticated", authenticated); + assertNull("Authentication created", SecurityContextHolder.getContext().getAuthentication()); control.verify(); } -} \ No newline at end of file + public void testCleanUp() throws Exception { + TestingAuthenticationToken authentication = + new TestingAuthenticationToken(new Object(), new Object(), new GrantedAuthority[0]); + SecurityContextHolder.getContext().setAuthentication(authentication); + + CleanupCallback cleanupCallback = new CleanupCallback(); + callbackHandler.handleInternal(cleanupCallback); + assertNull("Authentication created", SecurityContextHolder.getContext().getAuthentication()); + } + +} diff --git a/security/src/test/java/org/springframework/ws/soap/security/xwss/callback/jaas/CertificateLoginModule.java b/security/src/test/java/org/springframework/ws/soap/security/xwss/callback/jaas/CertificateLoginModule.java index 9ab46094..1957f565 100644 --- a/security/src/test/java/org/springframework/ws/soap/security/xwss/callback/jaas/CertificateLoginModule.java +++ b/security/src/test/java/org/springframework/ws/soap/security/xwss/callback/jaas/CertificateLoginModule.java @@ -18,7 +18,6 @@ package org.springframework.ws.soap.security.xwss.callback.jaas; import java.security.Principal; import java.util.Iterator; - import javax.security.auth.Subject; import javax.security.auth.callback.CallbackHandler; import javax.security.auth.login.LoginException; @@ -72,7 +71,7 @@ public class CertificateLoginModule implements LoginModule { for (Iterator iterator = subject.getPrincipals().iterator(); iterator.hasNext();) { Principal principal = (Principal) iterator.next(); if (principal instanceof X500Principal) { - return ((X500Principal) principal).getName(); + return principal.getName(); } } return null;