cleanup, add checks to arguments and responses, add log statements
authorgilbert <gilbert@ab3bd59b-922f-494d-bb5f-6f0a3c29deca>
Mon, 20 Jun 2005 13:00:59 +0000 (13:00 +0000)
committergilbert <gilbert@ab3bd59b-922f-494d-bb5f-6f0a3c29deca>
Mon, 20 Jun 2005 13:00:59 +0000 (13:00 +0000)
git-svn-id: https://subversion.switch.ch/svn/shibboleth/java-idp/trunk@1641 ab3bd59b-922f-494d-bb5f-6f0a3c29deca

src/edu/internet2/middleware/shibboleth/common/ShibBrowserProfile.java
src/edu/internet2/middleware/shibboleth/serviceprovider/AssertionConsumerServlet.java
src/edu/internet2/middleware/shibboleth/serviceprovider/ContextListener.java
src/edu/internet2/middleware/shibboleth/serviceprovider/FilterSupportImpl.java
src/edu/internet2/middleware/shibboleth/serviceprovider/ServletContextInitializer.java
src/edu/internet2/middleware/shibboleth/serviceprovider/Session.java
src/edu/internet2/middleware/shibboleth/serviceprovider/SessionManager.java
src/edu/internet2/middleware/shibboleth/serviceprovider/ShibBinding.java

index 963f7fd..b489e9f 100644 (file)
@@ -27,6 +27,9 @@ import org.opensaml.SAMLBrowserProfile;
 import org.opensaml.SAMLBrowserProfileFactory;
 import org.opensaml.SAMLException;
 import org.opensaml.TrustException;
+import org.opensaml.SAMLBrowserProfile.ArtifactMapper;
+import org.opensaml.SAMLBrowserProfile.BrowserProfileRequest;
+import org.opensaml.SAMLBrowserProfile.BrowserProfileResponse;
 
 import edu.internet2.middleware.shibboleth.metadata.EntityDescriptor;
 import edu.internet2.middleware.shibboleth.metadata.IDPSSODescriptor;
@@ -40,7 +43,7 @@ import edu.internet2.middleware.shibboleth.serviceprovider.ServiceProviderConfig
  * 
  * @author Scott Cantor @created April 11, 2002
  */
-public class ShibBrowserProfile implements SAMLBrowserProfile {
+public class ShibBrowserProfile  {
 
        private static Logger   log                     = Logger.getLogger(ShibBrowserProfile.class.getName());
 
@@ -68,7 +71,6 @@ public class ShibBrowserProfile implements SAMLBrowserProfile {
             StringBuffer issuer,
             HttpServletRequest reqContext,
             String recipient,
-            int supportedProfiles,
             ReplayCache replayCache,
             ArtifactMapper artifactMapper,
             int minorVersion
@@ -78,7 +80,8 @@ public class ShibBrowserProfile implements SAMLBrowserProfile {
         issuer.setLength(0);
         
         // Let SAML do all the decoding and parsing
-        BrowserProfileResponse bpr = profile.receive(issuer, reqContext, recipient, supportedProfiles, replayCache, artifactMapper, minorVersion);
+        BrowserProfileRequest bpRequest = profile.receive(reqContext);
+        BrowserProfileResponse bpr = profile.receive(issuer, bpRequest, recipient, replayCache, artifactMapper, minorVersion);
         
         /*
          * Now find the Metadata for the Entity that send this assertion.
index 8c0e33f..a928f3d 100644 (file)
@@ -61,7 +61,6 @@ import javax.servlet.http.HttpServletResponse;
 
 import org.apache.log4j.Logger;
 import org.opensaml.SAMLAudienceRestrictionCondition;
-import org.opensaml.SAMLBrowserProfile;
 import org.opensaml.SAMLCondition;
 import org.opensaml.SAMLException;
 import org.opensaml.SAMLResponse;
@@ -73,12 +72,14 @@ import edu.internet2.middleware.shibboleth.resource.AuthenticationFilter;
 import edu.internet2.middleware.shibboleth.serviceprovider.ServiceProviderConfig.ApplicationInfo;
 
 /**
- * AuthenticatonAssertionConsumerServlet
+ * Process the Authentication Assertion POST data to create a Session.
  * 
  * @author Howard Gilbert
  */
 public class AssertionConsumerServlet extends HttpServlet {
 
+       private static final String SESSIONCOOKIE = "ShibbolethSPSession";
+
        private static Logger log = Logger.getLogger(AssertionConsumerServlet.class.getName());
        
        private static ServiceProviderContext context = ServiceProviderContext.getInstance();
@@ -105,13 +106,10 @@ public class AssertionConsumerServlet extends HttpServlet {
         * 
         * @param request the request send by the client to the server
         * @param response the response send by the server to the client
-        * @throws ServletException if an error occurred
-        * @throws IOException if an error occurred
         */
        public void doPost(
                HttpServletRequest request,
                HttpServletResponse response)
-               // throws ServletException, IOException 
                {
            ServletContextInitializer.beginService(request,response);
                try {
@@ -135,9 +133,9 @@ public class AssertionConsumerServlet extends HttpServlet {
            
             if (appSessionValues.getShireSSL()&& // Requires SSL
                        !request.isSecure()) {       // isn't SSL
-               log.error("Authentication Assersion not posted over SSL.");
+               log.error("Authentication Assertion not posted over SSL.");
                try {
-                    response.sendRedirect("/shibboleth/shireError.html");
+                    response.sendRedirect("shireError.html");
                 } catch (IOException e1) {
                 }
                return;
@@ -148,9 +146,14 @@ public class AssertionConsumerServlet extends HttpServlet {
 
             String sessionId = createSessionFromPost(ipaddr, request, applicationId, shireURL, providerId, null);
             
-            Cookie cookie = new Cookie("ShibbolethSPSession",sessionId);
+            Cookie cookie = new Cookie(SESSIONCOOKIE,sessionId);
             response.addCookie(cookie);
             
+            /**
+             * This is included as a diagnostic, although it was suggested by a user
+             * as a future API for someone holding an Authentication and wishing to
+             * know what it means.
+             */
             try {
                                if (target.equals("SendAttributesBackToMe")) {
                                        ServletOutputStream outputStream = response.getOutputStream();
@@ -168,7 +171,7 @@ public class AssertionConsumerServlet extends HttpServlet {
         catch (MetadataException e) {
             log.error("Authentication Assertion source not found in Metadata.");
             try {
-                response.sendRedirect("/shibboleth/shireError.html");
+                response.sendRedirect("shireError.html");
             }
             catch (IOException e1) {
             }
@@ -176,7 +179,7 @@ public class AssertionConsumerServlet extends HttpServlet {
         catch (SAMLException e) {
             log.error("Authentication Assertion had invalid format.");
             try {
-                response.sendRedirect("/shibboleth/shireError.html");
+                response.sendRedirect("shireError.html");
             }
             catch (IOException e1) {
             }
@@ -189,6 +192,10 @@ public class AssertionConsumerServlet extends HttpServlet {
     /**
      * Create a Session object from SHIRE POST data
      * 
+     * <p>Used within this class to handle POSTs to the SP itself, but
+     * also by the FilterSupport logic when the POST is to the RM 
+     * context.</p>
+     * 
      * @param ipaddr IP Address of Browser
      * @param bin64Assertion Authentication assertion from POST
      * @param applicationId from RequestMap
@@ -216,7 +223,6 @@ public class AssertionConsumerServlet extends HttpServlet {
                 pproviderId,
                 req,
                 shireURL,   // My URL (Why??) To prevent attackers from redirecting messages. 
-                SAMLBrowserProfile.PROFILE_POST,    // TODO: support both profiles 
                 context.getReplayCache(),
                 null,
                 1
index 1ed81b8..55974e6 100644 (file)
@@ -49,7 +49,7 @@ import edu.internet2.middleware.commons.log4j.ThreadLocalAppender;
 public class ContextListener implements ServletContextListener {
     
     //TODO: Change before release
-       private static final Level defaultLogLevel = Level.DEBUG;
+       private static final Level defaultLogLevel = Level.INFO;
     
     // Initialization, parsing files, and setting up
        public static final String SHIBBOLETH_INIT = "shibboleth.init";
index 82f90bb..7b9e8af 100644 (file)
@@ -104,10 +104,8 @@ public class FilterSupportImpl implements FilterSupport {
         * @return     true if Shibboleth is required
         */
        public boolean isProtected(String url) {
-           // TODO Add some real logic. This is just a placeholder
-           if (url.endsWith("test.txt"))
-               return true;
-           return false;
+               //TODO: get info from requestmap
+           return true;
        }
 
        /**
@@ -191,9 +189,7 @@ public class FilterSupportImpl implements FilterSupport {
      * @return SessionId of empty session
      */
     public String createSession(String applicationId) {
-        String id = context.getSessionManager().newSession(
-                applicationId, null, null, null, null, null);
-        log.info("Session ID reserved for RM: "+id);
+        String id = context.getSessionManager().reserveSession(applicationId);
         return id;
     }
 }
index 9556624..b142688 100644 (file)
@@ -109,8 +109,10 @@ public class ServletContextInitializer {
         * 
         * @param ServletContext or null 
         * @return String filename
+        * @throws ShibbolethConfigurationException 
         */
-       private static String getServiceProviderConfigFile(ServletContext scontext) {
+       private static String getServiceProviderConfigFile(ServletContext scontext) 
+               throws ShibbolethConfigurationException {
            
                if (scontext!=null) {
                        String servletparm = scontext.getInitParameter("ServiceProviderConfigFile");
@@ -118,8 +120,8 @@ public class ServletContextInitializer {
                                return servletparm;
                        }
                }
-
-               return "/conf/shibboleth.xml";
+               log.error("ServiceProviderConfigFile parameter missing in WEB-INF/web.xml");
+               throw new ShibbolethConfigurationException("ServiceProviderConfigFile parameter missing");
                
        }
 
index 772c4c7..28ee7e6 100644 (file)
@@ -42,9 +42,24 @@ import org.opensaml.SAMLResponse;
  */
 public class Session implements Serializable {
        
+       // Default values from Shibboleth documentation
+       private static final int DEFAULTTIMEOUT = 1800000;
+       public static final int DEFAULTLIFETIME = 3600000;
+       
        Session(String key) {
                // Should only be created by SessionManager
+               if (key==null)
+                       throw new IllegalArgumentException();
+           this.key=key;
+           this.timestamp = System.currentTimeMillis();
+       }
+       
+       /**
+        * For testing, create a Session that may already be timed out.
+        */
+       Session(String key, long timestamp) {
            this.key=key;
+           this.timestamp = timestamp;
        }
        
        // Properties
@@ -71,14 +86,14 @@ public class Session implements Serializable {
        }
        
        private String entityId = null; // a.k.a providerId
-       
        public String getEntityId() {
                return entityId;
        }
        public void setEntityId(String entityId) {
                this.entityId = entityId;
        }
-       private long lifetime;
+       
+       private long lifetime = DEFAULTLIFETIME;
        public long getLifetime() {
                return lifetime;
        }
@@ -86,7 +101,7 @@ public class Session implements Serializable {
                this.lifetime = lifetime;
        }
        
-       private long timeout;
+       private long timeout=DEFAULTTIMEOUT;
        public long getTimeout() {
                return timeout;
        }
@@ -95,7 +110,16 @@ public class Session implements Serializable {
        }
        
     // private persisted variable
-       private long timestamp = System.currentTimeMillis();
+       private long timestamp = 0;
+       
+       public boolean isExpired() {
+               long now = System.currentTimeMillis();
+               if (lifetime>0 && lifetime<now)
+                       return true;
+               if (timeout>0 && timestamp+timeout<now)
+                       return true;
+               return false;
+       }
        
        
        // Stuff saved from the POST
index be3c3c8..c8a2f90 100644 (file)
@@ -93,6 +93,8 @@ public class SessionManager {
        
        
        public synchronized Session findSession(String sessionId, String applicationId ) {
+               if (sessionId==null || applicationId==null)
+                       throw new IllegalArgumentException();
                Session s = (Session) sessions.get(sessionId);
                if (s==null) {
                        log.warn("Session not found with ID "+sessionId);
@@ -106,10 +108,17 @@ public class SessionManager {
                        log.error("Session ID "+sessionId+" doesn't match application "+applicationId);
                        return null;
                }
+               if (s.isExpired()) {
+                       log.error("Session ID "+sessionId+" has expired.");
+                       // return null;
+               }
+               s.renew();
                return s;
        }
 
        private synchronized Session findEmptySession(String sessionId) {
+               if (sessionId==null)
+                       throw new IllegalArgumentException();
                Session s = (Session) sessions.get(sessionId);
                if (s==null) {
                        log.warn("Session not found with ID "+sessionId);
@@ -119,11 +128,14 @@ public class SessionManager {
                        log.error("Active Session found when looking for reserved ID:"+sessionId);
                    return null;
                }
+               s.renew();
                return s;
        }
        
        
        protected synchronized void add(Session s) {
+               if (s==null)
+                       throw new IllegalArgumentException();
                log.debug("Session added: "+s.getKey());
                sessions.put(s.getKey(), s);
                if (cache!=null)
@@ -131,6 +143,9 @@ public class SessionManager {
        }
        
        protected synchronized void update(Session s) {
+               if (s==null)
+                       throw new IllegalArgumentException();
+               s.renew();
                log.debug("Session updated: "+s.getKey());
                sessions.put(s.getKey(), s);
                if (cache!=null)
@@ -138,40 +153,56 @@ public class SessionManager {
        }
        
        protected synchronized void remove(Session s) {
+               if (s==null)
+                       throw new IllegalArgumentException();
                log.debug("Session removed: "+s.getKey());
                sessions.remove(s.getKey());
                if (cache!=null)
                        cache.remove(s);
        }
        
-
-       /**
-        * Test for valid Session
-        * 
-        * @param sessionId      typically, the cookie value from client browser
-        * @param applicationId  id of target application asking about session
-        * @param ipaddr         null, or IP address of client
-        * @return
-        */
-       public 
-                       boolean 
-       isValid(
-                       String sessionId,   
-                       String applicationId, 
-                       String ipaddr         
-                       ){
-               Session session = findSession(sessionId,applicationId);
-               ServiceProviderConfig.ApplicationInfo application = context.getServiceProviderConfig().getApplication(applicationId);
-               if (session==null)
-                       return false; // Cookie value did not match cached session
-               if (application == null)
-                       return false; // ApplicationConfig ID invalid
-               if (ipaddr!=null && !ipaddr.equals(session.getIpaddr()))
-                       return false; // Client coming from a different machine
-               // check for timeout
-               // Note: RPC prefetches attributes here
-               return true;
+       protected synchronized void expireSessions() {
+               Iterator iterator = sessions.entrySet().iterator();
+               while (iterator.hasNext()) {
+                       Map.Entry entry = (Map.Entry) iterator.next();
+                       Session session = (Session) entry.getValue();
+                       if (session.isExpired()) {
+                               log.info("Session " + session.getKey() + " has expired.");
+                               iterator.remove();
+                       }
+               }
        }
+       
+//  This was generated from a C++ routine, but it doesn't seem to be needed
+//     /**
+//      * Test for valid Session
+//      * 
+//      * @param sessionId      typically, the cookie value from client browser
+//      * @param applicationId  id of target application asking about session
+//      * @param ipaddr         null, or IP address of client
+//      * @return
+//      */
+//     public 
+//                     boolean 
+//     isValid(
+//                     String sessionId,   
+//                     String applicationId, 
+//                     String ipaddr         
+//                     ){
+//             if (sessionId==null || applicationId==null)
+//                     throw new IllegalArgumentException();
+//             Session session = findSession(sessionId,applicationId);
+//             ServiceProviderConfig.ApplicationInfo application = context.getServiceProviderConfig().getApplication(applicationId);
+//             if (session==null)
+//                     return false; // Cookie value did not match cached session
+//             if (application == null)
+//                     return false; // ApplicationConfig ID invalid
+//             if (ipaddr!=null && !ipaddr.equals(session.getIpaddr()))
+//                     return false; // Client coming from a different machine
+//             // check for timeout
+//             // Note: RPC prefetches attributes here
+//             return true;
+//     }
 
        
        /**
@@ -201,6 +232,7 @@ public class SessionManager {
                Sessions appSessionValues = appinfo.getApplicationConfig().getSessions();
                
                String sessionId = null;
+               boolean isUpdate = false;
                
                Session session;
                if (emptySessionId==null) {
@@ -209,6 +241,8 @@ public class SessionManager {
                    session = findEmptySession(emptySessionId);
                    if (session==null) {
                            session = new Session(generateKey());
+                   } else {
+                       isUpdate=true;
                    }
                }
                session.setApplicationId(applicationId);
@@ -218,17 +252,45 @@ public class SessionManager {
                session.setAuthenticationAssertion(assertion);
                session.setAuthenticationStatement(authenticationStatement);
                
-               session.setLifetime(appSessionValues.getLifetime());
-               session.setTimeout(appSessionValues.getTimeout());
+               // Get lifetime and timeout from Applications/Sessions in config file 
+               session.setLifetime(appSessionValues.getLifetime()*1000);
+               session.setTimeout(appSessionValues.getTimeout()*1000);
                
                sessionId = session.getKey();
 
-               // This static method finds its unique instance variable
-               add(session);
+               if (isUpdate)
+                       update(session);
+               else
+                       add(session);
+               
            log.debug("New Session created "+sessionId);
 
                return sessionId;
        }
+       public 
+       String 
+reserveSession(
+       String applicationId 
+       ){
+
+ServiceProviderConfig config = context.getServiceProviderConfig();
+ApplicationInfo appinfo = config.getApplication(applicationId);
+Sessions appSessionValues = appinfo.getApplicationConfig().getSessions();
+
+String sessionId = null;
+boolean isUpdate = false;
+
+Session session= new Session(generateKey());
+session.setApplicationId(applicationId);
+
+sessionId = session.getKey();
+
+add(session);
+
+log.debug("SessionId reserved "+sessionId);
+
+return sessionId;
+}
        /**
         * <p>IOC wiring point to plug in an external SessionCache implementation.
         * </p>
@@ -239,6 +301,8 @@ public class SessionManager {
        setCache(
                        SessionCache cache) {
                
+               if (cache==null)
+                       throw new IllegalArgumentException();
            log.info("Enabling Session Cache");
                /*
                 * The following code supports dynamic switching from
@@ -352,5 +416,4 @@ public class SessionManager {
            return attributeMap;
        }
        
-       
 }
index b6c772a..f95ca89 100644 (file)
@@ -37,6 +37,7 @@ import org.opensaml.SAMLAssertion;
 import org.opensaml.SAMLAuthorityBinding;
 import org.opensaml.SAMLBinding;
 import org.opensaml.SAMLBindingFactory;
+import org.opensaml.SAMLCondition;
 import org.opensaml.SAMLException;
 import org.opensaml.SAMLRequest;
 import org.opensaml.SAMLResponse;
@@ -83,7 +84,7 @@ public class ShibBinding {
         */
        public 
        ShibBinding(
-                       String applicationId) throws NoSuchProviderException {
+                       String applicationId)  {
                this.applicationId=applicationId;
        }
 
@@ -197,7 +198,11 @@ public class ShibBinding {
                while (assertions.hasNext()) {
                        SAMLAssertion assertion = (SAMLAssertion) assertions.next();
                        
-                       // TODO Dropped some logic validating conditions
+                       Iterator conditions = assertion.getConditions();
+                       while (conditions.hasNext()) {
+                               SAMLCondition condition = (SAMLCondition) conditions.next();
+                               // TODO C++ only seems to validate that the audience string is present
+                       }
                        
                        if (assertion.isSigned() && 
                                !appinfo.validate(assertion,role)) {