You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@shiro.apache.org by lh...@apache.org on 2009/03/28 23:11:15 UTC

svn commit: r759607 - in /incubator/jsecurity/trunk: core/src/main/java/org/apache/ki/session/mgt/ samples/spring/src/main/webapp/WEB-INF/resources/ support/spring/src/main/java/org/apache/ki/spring/remoting/ support/spring/src/test/java/org/apache/ki/...

Author: lhazlewood
Date: Sat Mar 28 22:11:15 2009
New Revision: 759607

URL: http://svn.apache.org/viewvc?rev=759607&view=rev
Log:
modifications to clean up spring-remoting support (was causing stack overflows)

Added:
    incubator/jsecurity/trunk/support/spring/src/test/java/org/apache/ki/spring/remoting/
    incubator/jsecurity/trunk/support/spring/src/test/java/org/apache/ki/spring/remoting/SecureRemoteInvocationFactoryTest.java
Modified:
    incubator/jsecurity/trunk/core/src/main/java/org/apache/ki/session/mgt/AbstractValidatingSessionManager.java
    incubator/jsecurity/trunk/core/src/main/java/org/apache/ki/session/mgt/DelegatingSession.java
    incubator/jsecurity/trunk/samples/spring/src/main/webapp/WEB-INF/resources/jsecurity.jnlp.jsp
    incubator/jsecurity/trunk/support/spring/src/main/java/org/apache/ki/spring/remoting/SecureRemoteInvocationExecutor.java
    incubator/jsecurity/trunk/support/spring/src/main/java/org/apache/ki/spring/remoting/SecureRemoteInvocationFactory.java
    incubator/jsecurity/trunk/support/spring/src/test/java/org/apache/ki/spring/SpringKiFilterTest.java

Modified: incubator/jsecurity/trunk/core/src/main/java/org/apache/ki/session/mgt/AbstractValidatingSessionManager.java
URL: http://svn.apache.org/viewvc/incubator/jsecurity/trunk/core/src/main/java/org/apache/ki/session/mgt/AbstractValidatingSessionManager.java?rev=759607&r1=759606&r2=759607&view=diff
==============================================================================
--- incubator/jsecurity/trunk/core/src/main/java/org/apache/ki/session/mgt/AbstractValidatingSessionManager.java (original)
+++ incubator/jsecurity/trunk/core/src/main/java/org/apache/ki/session/mgt/AbstractValidatingSessionManager.java Sat Mar 28 22:11:15 2009
@@ -218,7 +218,7 @@
             String msg = "The " + getClass().getName() + " implementation only supports validating " +
                 "Session implementations of the " + ValidatingSession.class.getName() + " interface.  " +
                 "Please either implement this interface in your session implementation or override the " +
-                getClass().getName() + ".validate(Session) method to perform validation.";
+                AbstractValidatingSessionManager.class.getName() + ".doValidate(Session) method to perform validation.";
             throw new IllegalStateException(msg);
         }
     }

Modified: incubator/jsecurity/trunk/core/src/main/java/org/apache/ki/session/mgt/DelegatingSession.java
URL: http://svn.apache.org/viewvc/incubator/jsecurity/trunk/core/src/main/java/org/apache/ki/session/mgt/DelegatingSession.java?rev=759607&r1=759606&r2=759607&view=diff
==============================================================================
--- incubator/jsecurity/trunk/core/src/main/java/org/apache/ki/session/mgt/DelegatingSession.java (original)
+++ incubator/jsecurity/trunk/core/src/main/java/org/apache/ki/session/mgt/DelegatingSession.java Sat Mar 28 22:11:15 2009
@@ -22,6 +22,7 @@
 import java.net.InetAddress;
 import java.util.Collection;
 import java.util.Date;
+import java.util.Collections;
 
 import org.apache.ki.session.InvalidSessionException;
 import org.apache.ki.session.ReplacedSessionException;
@@ -32,10 +33,10 @@
  * {@link org.apache.ki.session.Session Session}.
  * This implementation is basically a proxy to a server-side {@link SessionManager SessionManager},
  * which will return the proper results for each method call.
- *
+ * <p/>
  * <p>A <tt>DelegatingSession</tt> will cache data when appropriate to avoid a remote method invocation,
  * only communicating with the server when necessary.
- *
+ * <p/>
  * <p>Of course, if used in-process with a SessionManager business POJO, as might be the case in a
  * web-based application where the web classes and server-side business pojos exist in the same
  * JVM, a remote method call will not be incurred.
@@ -54,9 +55,7 @@
     private Date startTimestamp = null;
     private InetAddress hostAddress = null;
 
-    /**
-     * Handle to a server-side SessionManager.  See {@link #setSessionManager} for details.
-     */
+    /** Handle to a server-side SessionManager.  See {@link #setSessionManager} for details. */
     private SessionManager sessionManager = null;
 
 
@@ -85,7 +84,7 @@
      * probably be a remoting proxy which executes remote method invocations.  In a single-process
      * environment (e.g. a web  application deployed in the same JVM of the application server),
      * the <tt>SessionManager</tt> can be the actual business POJO implementation.
-     *
+     * <p/>
      * <p>You'll notice the {@link Session Session} interface and the {@link SessionManager}
      * interface are nearly identical.  This is to ensure the SessionManager can support
      * most method calls in the Session interface, via this handle/proxy technique.  The session
@@ -110,16 +109,12 @@
         this.id = id;
     }
 
-    /**
-     * @see org.apache.ki.session.Session#getId()
-     */
+    /** @see org.apache.ki.session.Session#getId() */
     public Serializable getId() {
         return id;
     }
 
-    /**
-     * @see org.apache.ki.session.Session#getStartTimestamp()
-     */
+    /** @see org.apache.ki.session.Session#getStartTimestamp() */
     public Date getStartTimestamp() {
         if (startTimestamp == null) {
             try {
@@ -132,9 +127,7 @@
         return startTimestamp;
     }
 
-    /**
-     * @see org.apache.ki.session.Session#getLastAccessTime()
-     */
+    /** @see org.apache.ki.session.Session#getLastAccessTime() */
     public Date getLastAccessTime() {
         //can't cache - only business pojo knows the accurate time:
         try {
@@ -163,9 +156,7 @@
         }
     }
 
-    /**
-     * @see org.apache.ki.session.Session#getHostAddress()
-     */
+    /** @see org.apache.ki.session.Session#getHostAddress() */
     public InetAddress getHostAddress() {
         if (hostAddress == null) {
             try {
@@ -178,57 +169,51 @@
         return hostAddress;
     }
 
-    /**
-     * @see org.apache.ki.session.Session#touch()
-     */
+    /** @see org.apache.ki.session.Session#touch() */
     public void touch() throws InvalidSessionException {
         try {
             sessionManager.touch(id);
         } catch (ReplacedSessionException e) {
             this.id = e.getNewSessionId();
-            sessionManager.touch(id);
+            // No need to 'hit' the session manager again - a newly created session is 'touched' at the time of creation
         }
     }
 
-    /**
-     * @see org.apache.ki.session.Session#stop()
-     */
+    /** @see org.apache.ki.session.Session#stop() */
     public void stop() throws InvalidSessionException {
         try {
             sessionManager.stop(id);
         } catch (ReplacedSessionException e) {
             this.id = e.getNewSessionId();
+            //TODO - prevent sessionManager from creating new session when 'stop' is already requested.
             sessionManager.stop(id);
         }
     }
 
-    /**
-     * @see org.apache.ki.session.Session#getAttributeKeys
-     */
+    /** @see org.apache.ki.session.Session#getAttributeKeys */
+    @SuppressWarnings({"unchecked"})
     public Collection<Object> getAttributeKeys() throws InvalidSessionException {
         try {
             return sessionManager.getAttributeKeys(id);
         } catch (ReplacedSessionException e) {
             this.id = e.getNewSessionId();
-            return sessionManager.getAttributeKeys(id);
+            // No need to 'hit' the session manager again - a new session won't have any attributes:
+            return Collections.EMPTY_SET;
         }
     }
 
-    /**
-     * @see org.apache.ki.session.Session#getAttribute(Object key)
-     */
+    /** @see org.apache.ki.session.Session#getAttribute(Object key) */
     public Object getAttribute(Object key) throws InvalidSessionException {
         try {
             return sessionManager.getAttribute(id, key);
         } catch (ReplacedSessionException e) {
             this.id = e.getNewSessionId();
-            return sessionManager.getAttribute(id, key);
+            // No need to 'hit' the session manager again - a new session won't have any attributes
+            return null;
         }
     }
 
-    /**
-     * @see Session#setAttribute(Object key, Object value)
-     */
+    /** @see Session#setAttribute(Object key, Object value) */
     public void setAttribute(Object key, Object value) throws InvalidSessionException {
         if (value == null) {
             removeAttribute(key);
@@ -242,15 +227,14 @@
         }
     }
 
-    /**
-     * @see Session#removeAttribute(Object key)
-     */
+    /** @see Session#removeAttribute(Object key) */
     public Object removeAttribute(Object key) throws InvalidSessionException {
         try {
             return sessionManager.removeAttribute(id, key);
         } catch (ReplacedSessionException e) {
             this.id = e.getNewSessionId();
-            return sessionManager.removeAttribute(id, key);
+            // No need to 'hit' the session manager again - a new session won't have any attributes:
+            return null;
         }
     }
 }

Modified: incubator/jsecurity/trunk/samples/spring/src/main/webapp/WEB-INF/resources/jsecurity.jnlp.jsp
URL: http://svn.apache.org/viewvc/incubator/jsecurity/trunk/samples/spring/src/main/webapp/WEB-INF/resources/jsecurity.jnlp.jsp?rev=759607&r1=759606&r2=759607&view=diff
==============================================================================
--- incubator/jsecurity/trunk/samples/spring/src/main/webapp/WEB-INF/resources/jsecurity.jnlp.jsp (original)
+++ incubator/jsecurity/trunk/samples/spring/src/main/webapp/WEB-INF/resources/jsecurity.jnlp.jsp Sat Mar 28 22:11:15 2009
@@ -38,7 +38,7 @@
     <resources>
         <j2se version="1.5"/>
         <jar href="ki-spring-sample.jar"/>
-        <jar href="ki.jar"/>
+        <jar href="ki-all.jar"/>
         <jar href="spring.jar"/>
         <jar href="slf4j-api.jar"/>
         <property name="ki.session.id" value="${sessionId}"/>

Modified: incubator/jsecurity/trunk/support/spring/src/main/java/org/apache/ki/spring/remoting/SecureRemoteInvocationExecutor.java
URL: http://svn.apache.org/viewvc/incubator/jsecurity/trunk/support/spring/src/main/java/org/apache/ki/spring/remoting/SecureRemoteInvocationExecutor.java?rev=759607&r1=759606&r2=759607&view=diff
==============================================================================
--- incubator/jsecurity/trunk/support/spring/src/main/java/org/apache/ki/spring/remoting/SecureRemoteInvocationExecutor.java (original)
+++ incubator/jsecurity/trunk/support/spring/src/main/java/org/apache/ki/spring/remoting/SecureRemoteInvocationExecutor.java Sat Mar 28 22:11:15 2009
@@ -20,6 +20,7 @@
 
 import java.io.Serializable;
 import java.lang.reflect.InvocationTargetException;
+import java.net.InetAddress;
 
 import org.springframework.remoting.support.DefaultRemoteInvocationExecutor;
 import org.springframework.remoting.support.RemoteInvocation;
@@ -77,7 +78,13 @@
     ============================================*/
     @SuppressWarnings({"unchecked"})
     public Object invoke(RemoteInvocation invocation, Object targetObject) throws NoSuchMethodException, IllegalAccessException, InvocationTargetException {
+
         try {
+            InetAddress inet = (InetAddress)invocation.getAttribute(SecureRemoteInvocationFactory.INET_ADDRESS_KEY);
+            if (inet != null) {
+                ThreadContext.bind(inet);
+            }
+            
             Serializable sessionId = invocation.getAttribute(SecureRemoteInvocationFactory.SESSION_ID_KEY);
             if (sessionId != null) {
                 ThreadContext.bindSessionId(sessionId);
@@ -88,6 +95,7 @@
                             "on an existing Session will not be available during the method invocatin.");
                 }
             }
+            
             ThreadContext.bind(securityManager);
             ThreadContext.bind(securityManager.getSubject());
 

Modified: incubator/jsecurity/trunk/support/spring/src/main/java/org/apache/ki/spring/remoting/SecureRemoteInvocationFactory.java
URL: http://svn.apache.org/viewvc/incubator/jsecurity/trunk/support/spring/src/main/java/org/apache/ki/spring/remoting/SecureRemoteInvocationFactory.java?rev=759607&r1=759606&r2=759607&view=diff
==============================================================================
--- incubator/jsecurity/trunk/support/spring/src/main/java/org/apache/ki/spring/remoting/SecureRemoteInvocationFactory.java (original)
+++ incubator/jsecurity/trunk/support/spring/src/main/java/org/apache/ki/spring/remoting/SecureRemoteInvocationFactory.java Sat Mar 28 22:11:15 2009
@@ -19,6 +19,7 @@
 package org.apache.ki.spring.remoting;
 
 import java.io.Serializable;
+import java.net.InetAddress;
 
 import org.aopalliance.intercept.MethodInvocation;
 import org.springframework.remoting.support.DefaultRemoteInvocationFactory;
@@ -29,7 +30,9 @@
 import org.slf4j.LoggerFactory;
 
 import org.apache.ki.SecurityUtils;
+import org.apache.ki.util.ThreadContext;
 import org.apache.ki.session.Session;
+import org.apache.ki.session.mgt.SessionManager;
 import org.apache.ki.subject.Subject;
 
 
@@ -50,6 +53,7 @@
     private static final Logger log = LoggerFactory.getLogger(SecureRemoteInvocationFactory.class);
 
     public static final String SESSION_ID_KEY = Session.class.getName() + "_ID_KEY";
+    public static final String INET_ADDRESS_KEY = InetAddress.class.getName() + "_KEY";
 
     private static final String SESSION_ID_SYSTEM_PROPERTY_NAME = "ki.session.id";
 
@@ -57,35 +61,63 @@
      * Creates a {@link RemoteInvocation} with the current session ID as an
      * {@link RemoteInvocation#getAttribute(String) attribute}.
      *
-     * @param methodInvocation the method invocation that the remote invocation should
-     *                         be based on.
+     * @param mi the method invocation that the remote invocation should be based on.
      * @return a remote invocation object containing the current session ID as an attribute.
      */
-    public RemoteInvocation createRemoteInvocation(MethodInvocation methodInvocation) {
+    public RemoteInvocation createRemoteInvocation(MethodInvocation mi) {
+
         Serializable sessionId = null;
-        Subject subject = SecurityUtils.getSubject();
-        if (subject != null) {
+        InetAddress inet = null;
+        boolean sessionManagerMethodInvocation = false;
+
+        //If the calling MI is for a remoting SessionManager proxy, we need to acquire the session ID from the method
+        //argument and NOT interact with SecurityUtils/subject.getSession to avoid a stack overflow
+        if (SessionManager.class.equals(mi.getMethod().getDeclaringClass())) {
+            sessionManagerMethodInvocation = true;
+            //for SessionManager calls, all method calls require the session id as the first argument, with
+            //the exception of 'start' that takes in an InetAddress.  So, ignore that one case:
+            Object firstArg = mi.getArguments()[0];
+            if (!(firstArg instanceof InetAddress)) {
+                sessionId = (Serializable) firstArg;
+            }
+        }
+
+        //tried the proxy.  If sessionId is still null, only then try the Subject:
+        if (sessionId == null && !sessionManagerMethodInvocation) {
+            Subject subject = SecurityUtils.getSubject();
             Session session = subject.getSession(false);
             if (session != null) {
+                inet = session.getHostAddress();                
                 sessionId = session.getId();
             }
         }
 
+        //No call to the sessionManager, and the Subject doesn't have a session.  Try a system property
+        //as a last result:
         if (sessionId == null) {
             if (log.isTraceEnabled()) {
                 log.trace("No Session found for the currently executing subject via subject.getSession(false).  " +
-                        "Attempting to revert back to the 'ki.session.id' system property...");
+                    "Attempting to revert back to the 'ki.session.id' system property...");
             }
-        }
-        sessionId = System.getProperty(SESSION_ID_SYSTEM_PROPERTY_NAME);
-        if (sessionId == null && log.isTraceEnabled()) {
-            log.trace("No 'ki.session.id' system property found.  Heuristics have been exhausted; " +
+            sessionId = System.getProperty(SESSION_ID_SYSTEM_PROPERTY_NAME);
+            if (sessionId == null && log.isTraceEnabled()) {
+                log.trace("No 'ki.session.id' system property found.  Heuristics have been exhausted; " +
                     "RemoteInvocation will not contain a sessionId.");
+            }
+        }
+
+        if ( inet == null ) {
+            //try thread context:
+            inet = ThreadContext.getInetAddress();
         }
-        RemoteInvocation ri = new RemoteInvocation(methodInvocation);
+
+        RemoteInvocation ri = new RemoteInvocation(mi);
         if (sessionId != null) {
             ri.addAttribute(SESSION_ID_KEY, sessionId);
         }
+        if ( inet != null ) {
+            ri.addAttribute(INET_ADDRESS_KEY, inet);
+        }
 
         return ri;
     }

Modified: incubator/jsecurity/trunk/support/spring/src/test/java/org/apache/ki/spring/SpringKiFilterTest.java
URL: http://svn.apache.org/viewvc/incubator/jsecurity/trunk/support/spring/src/test/java/org/apache/ki/spring/SpringKiFilterTest.java?rev=759607&r1=759606&r2=759607&view=diff
==============================================================================
--- incubator/jsecurity/trunk/support/spring/src/test/java/org/apache/ki/spring/SpringKiFilterTest.java (original)
+++ incubator/jsecurity/trunk/support/spring/src/test/java/org/apache/ki/spring/SpringKiFilterTest.java Sat Mar 28 22:11:15 2009
@@ -18,19 +18,16 @@
  */
 package org.apache.ki.spring;
 
-import java.util.HashMap;
-import java.util.Map;
-import javax.servlet.FilterConfig;
-import javax.servlet.ServletContext;
-
+import org.apache.ki.mgt.SecurityManager;
+import org.apache.ki.web.servlet.KiFilter;
 import static org.easymock.EasyMock.*;
 import org.junit.Test;
-import org.apache.ki.web.servlet.KiFilter;
 import org.springframework.web.context.WebApplicationContext;
 
-import org.apache.ki.mgt.SecurityManager;
-import org.apache.ki.spring.SpringIniWebConfiguration;
-import org.apache.ki.spring.SpringKiFilter;
+import javax.servlet.FilterConfig;
+import javax.servlet.ServletContext;
+import java.util.HashMap;
+import java.util.Map;
 
 
 /**

Added: incubator/jsecurity/trunk/support/spring/src/test/java/org/apache/ki/spring/remoting/SecureRemoteInvocationFactoryTest.java
URL: http://svn.apache.org/viewvc/incubator/jsecurity/trunk/support/spring/src/test/java/org/apache/ki/spring/remoting/SecureRemoteInvocationFactoryTest.java?rev=759607&view=auto
==============================================================================
--- incubator/jsecurity/trunk/support/spring/src/test/java/org/apache/ki/spring/remoting/SecureRemoteInvocationFactoryTest.java (added)
+++ incubator/jsecurity/trunk/support/spring/src/test/java/org/apache/ki/spring/remoting/SecureRemoteInvocationFactoryTest.java Sat Mar 28 22:11:15 2009
@@ -0,0 +1,103 @@
+package org.apache.ki.spring.remoting;
+
+import org.aopalliance.intercept.MethodInvocation;
+import org.apache.ki.session.mgt.SessionManager;
+import org.apache.ki.subject.Subject;
+import org.apache.ki.util.ThreadContext;
+import static org.easymock.EasyMock.*;
+import org.junit.After;
+import static org.junit.Assert.*;
+import org.junit.Before;
+import org.junit.Test;
+import org.springframework.remoting.support.RemoteInvocation;
+
+import java.lang.reflect.Method;
+import java.net.InetAddress;
+import java.util.UUID;
+
+/**
+ * //TODO - Class JavaDoc!
+ *
+ * @author Les Hazlewood
+ * @since Mar 28, 2009 4:14:01 PM
+ */
+public class SecureRemoteInvocationFactoryTest {
+
+    @Before
+    public void setup() {
+        ThreadContext.clear();
+    }
+
+    protected void bind( Subject subject ) {
+        ThreadContext.bind(subject);
+    }
+
+    @After
+    public void tearDown() {
+        ThreadContext.clear();
+    }
+
+    protected Method getMethod(String name, Class clazz) {
+        Method[] methods = clazz.getMethods();
+        for( Method method : methods ) {
+            if ( method.getName().equals(name) ) {
+                return method;
+            }
+        }
+        throw new IllegalStateException( "'" + name + "' method should exist." );
+    }
+
+    @Test
+    public void testSessionManagerProxyStartRemoteInvocation() throws Exception {
+
+        SecureRemoteInvocationFactory factory = new SecureRemoteInvocationFactory();
+
+        MethodInvocation mi = createMock(MethodInvocation.class);
+        Method startMethod = getMethod("start", SessionManager.class);
+        expect(mi.getMethod()).andReturn(startMethod).anyTimes();
+        
+        Object[] args = {InetAddress.getLocalHost()};
+        expect(mi.getArguments()).andReturn(args).anyTimes();
+
+        replay(mi);
+
+        RemoteInvocation ri = factory.createRemoteInvocation(mi);
+
+        verify(mi);
+
+        assertNull(ri.getAttribute(SecureRemoteInvocationFactory.SESSION_ID_KEY));
+    }
+
+    @Test
+    public void testSessionManagerProxyNonStartRemoteInvocation() throws Exception {
+
+        SecureRemoteInvocationFactory factory = new SecureRemoteInvocationFactory();
+
+        MethodInvocation mi = createMock(MethodInvocation.class);
+        Method method = getMethod("isValid", SessionManager.class);
+        expect(mi.getMethod()).andReturn(method).anyTimes();
+
+        String dummySessionId = UUID.randomUUID().toString();
+        Object[] args = {dummySessionId};
+        expect(mi.getArguments()).andReturn(args).anyTimes();
+
+        replay(mi);
+
+        RemoteInvocation ri = factory.createRemoteInvocation(mi);
+
+        verify(mi);
+
+        assertEquals(ri.getAttribute(SecureRemoteInvocationFactory.SESSION_ID_KEY), dummySessionId);
+    }
+
+    /*@Test
+    public void testNonSessionManagerCall() throws Exception {
+
+        SecureRemoteInvocationFactory factory = new SecureRemoteInvocationFactory();
+
+        MethodInvocation mi = createMock(MethodInvocation.class);
+        Method method = getMethod("login", SecurityManager.class);
+        expect(mi.getMethod()).andReturn(method).anyTimes();
+    }*/
+
+}