You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cxf.apache.org by co...@apache.org on 2016/12/19 14:18:35 UTC

[1/3] cxf-fediz git commit: Fixing test for Tomcat plugins

Repository: cxf-fediz
Updated Branches:
  refs/heads/1.2.x-fixes 3164f0405 -> 73a11b5f2


Fixing test for Tomcat plugins


Project: http://git-wip-us.apache.org/repos/asf/cxf-fediz/repo
Commit: http://git-wip-us.apache.org/repos/asf/cxf-fediz/commit/185174b9
Tree: http://git-wip-us.apache.org/repos/asf/cxf-fediz/tree/185174b9
Diff: http://git-wip-us.apache.org/repos/asf/cxf-fediz/diff/185174b9

Branch: refs/heads/1.2.x-fixes
Commit: 185174b9abff33ca59128790faea17d67238de4e
Parents: 3164f04
Author: Colm O hEigeartaigh <co...@apache.org>
Authored: Mon Dec 19 10:52:29 2016 +0000
Committer: Colm O hEigeartaigh <co...@apache.org>
Committed: Mon Dec 19 14:16:59 2016 +0000

----------------------------------------------------------------------
 .../java/org/apache/cxf/fediz/integrationtests/AbstractTests.java | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cxf-fediz/blob/185174b9/systests/tests/src/test/java/org/apache/cxf/fediz/integrationtests/AbstractTests.java
----------------------------------------------------------------------
diff --git a/systests/tests/src/test/java/org/apache/cxf/fediz/integrationtests/AbstractTests.java b/systests/tests/src/test/java/org/apache/cxf/fediz/integrationtests/AbstractTests.java
index 3481c34..40c2d59 100644
--- a/systests/tests/src/test/java/org/apache/cxf/fediz/integrationtests/AbstractTests.java
+++ b/systests/tests/src/test/java/org/apache/cxf/fediz/integrationtests/AbstractTests.java
@@ -742,7 +742,8 @@ public abstract class AbstractTests {
             // expected
             Assert.assertTrue(ex.getMessage().contains("401 Unauthorized")
                               || ex.getMessage().contains("401 Authentication Failed")
-                              || ex.getMessage().contains("403 Forbidden"));
+                              || ex.getMessage().contains("403 Forbidden")
+                              || ex.getMessage().contains("408 Request Timeout"));
         }
         
         // webClient.close();


[2/3] cxf-fediz git commit: Fixing Spring plugins

Posted by co...@apache.org.
Fixing Spring plugins


Project: http://git-wip-us.apache.org/repos/asf/cxf-fediz/repo
Commit: http://git-wip-us.apache.org/repos/asf/cxf-fediz/commit/707b8f95
Tree: http://git-wip-us.apache.org/repos/asf/cxf-fediz/tree/707b8f95
Diff: http://git-wip-us.apache.org/repos/asf/cxf-fediz/diff/707b8f95

Branch: refs/heads/1.2.x-fixes
Commit: 707b8f95395a3a9ba6d2643053578081e91e5673
Parents: 185174b
Author: Colm O hEigeartaigh <co...@apache.org>
Authored: Mon Dec 19 13:20:30 2016 +0000
Committer: Colm O hEigeartaigh <co...@apache.org>
Committed: Mon Dec 19 14:18:00 2016 +0000

----------------------------------------------------------------------
 .../web/FederationAuthenticationEntryPoint.java |  8 ++++
 .../web/FederationAuthenticationFilter.java     | 39 +++++++++++++++-----
 .../web/FederationAuthenticationEntryPoint.java |  8 ++++
 .../web/FederationAuthenticationFilter.java     | 34 +++++++++++++----
 4 files changed, 73 insertions(+), 16 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cxf-fediz/blob/707b8f95/plugins/spring/src/main/java/org/apache/cxf/fediz/spring/web/FederationAuthenticationEntryPoint.java
----------------------------------------------------------------------
diff --git a/plugins/spring/src/main/java/org/apache/cxf/fediz/spring/web/FederationAuthenticationEntryPoint.java b/plugins/spring/src/main/java/org/apache/cxf/fediz/spring/web/FederationAuthenticationEntryPoint.java
index 3c63554..641c673 100644
--- a/plugins/spring/src/main/java/org/apache/cxf/fediz/spring/web/FederationAuthenticationEntryPoint.java
+++ b/plugins/spring/src/main/java/org/apache/cxf/fediz/spring/web/FederationAuthenticationEntryPoint.java
@@ -27,6 +27,7 @@ import java.util.Map.Entry;
 import javax.servlet.ServletException;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
+import javax.servlet.http.HttpSession;
 
 import org.w3c.dom.Document;
 
@@ -63,6 +64,11 @@ import org.springframework.util.Assert;
 public class FederationAuthenticationEntryPoint implements AuthenticationEntryPoint,
     InitializingBean, ApplicationContextAware {
     
+    /**
+     * The key used to save the context of the request
+     */
+    public static final String SAVED_CONTEXT = "SAVED_CONTEXT";
+    
     private static final Logger LOG = LoggerFactory.getLogger(FederationAuthenticationEntryPoint.class);
     
     private ApplicationContext appContext;
@@ -128,6 +134,8 @@ public class FederationAuthenticationEntryPoint implements AuthenticationEntryPo
                 }
             }
             
+            HttpSession session = servletRequest.getSession(true);
+            session.setAttribute(SAVED_CONTEXT, redirectionResponse.getRequestState().getState());
         } catch (ProcessingException ex) {
             LOG.warn("Failed to create SignInRequest", ex);
             throw new ServletException("Failed to create SignInRequest: " + ex.getMessage());

http://git-wip-us.apache.org/repos/asf/cxf-fediz/blob/707b8f95/plugins/spring/src/main/java/org/apache/cxf/fediz/spring/web/FederationAuthenticationFilter.java
----------------------------------------------------------------------
diff --git a/plugins/spring/src/main/java/org/apache/cxf/fediz/spring/web/FederationAuthenticationFilter.java b/plugins/spring/src/main/java/org/apache/cxf/fediz/spring/web/FederationAuthenticationFilter.java
index 1c13f18..c18d238 100644
--- a/plugins/spring/src/main/java/org/apache/cxf/fediz/spring/web/FederationAuthenticationFilter.java
+++ b/plugins/spring/src/main/java/org/apache/cxf/fediz/spring/web/FederationAuthenticationFilter.java
@@ -26,6 +26,7 @@ import java.util.Date;
 import javax.servlet.ServletRequest;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
+import javax.servlet.http.HttpSession;
 
 import org.apache.cxf.fediz.core.FederationConstants;
 import org.apache.cxf.fediz.core.SAMLSSOConstants;
@@ -33,6 +34,7 @@ import org.apache.cxf.fediz.core.processor.FedizRequest;
 import org.apache.cxf.fediz.spring.FederationConfig;
 import org.apache.cxf.fediz.spring.authentication.ExpiredTokenException;
 import org.apache.cxf.fediz.spring.authentication.FederationAuthenticationToken;
+import org.springframework.security.authentication.BadCredentialsException;
 import org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
 import org.springframework.security.core.Authentication;
 import org.springframework.security.core.AuthenticationException;
@@ -55,15 +57,12 @@ public class FederationAuthenticationFilter extends AbstractAuthenticationProces
     public Authentication attemptAuthentication(final HttpServletRequest request, final HttpServletResponse response)
         throws AuthenticationException, IOException {
 
-        SecurityContext context = SecurityContextHolder.getContext();
-        if (context != null) {
-            Authentication authentication = context.getAuthentication();
-            if (authentication instanceof FederationAuthenticationToken) {
-                // If we reach this point then the token must be expired
-                throw new ExpiredTokenException("Token is expired");
-            }
+        if (isTokenExpired()) {
+            throw new ExpiredTokenException("Token is expired");
         }
- 
+        
+        verifySavedState(request);
+        
         String wa = request.getParameter(FederationConstants.PARAM_ACTION);
         String responseToken = getResponseToken(request);
         
@@ -106,7 +105,7 @@ public class FederationAuthenticationFilter extends AbstractAuthenticationProces
             
         return false;
     }
-  
+    
     private String getResponseToken(ServletRequest request) {
         if (request.getParameter(FederationConstants.PARAM_RESULT) != null) {
             return request.getParameter(FederationConstants.PARAM_RESULT);
@@ -116,7 +115,29 @@ public class FederationAuthenticationFilter extends AbstractAuthenticationProces
         
         return null;
     }
+    
+    private String getState(ServletRequest request) {
+        if (request.getParameter(FederationConstants.PARAM_CONTEXT) != null) {
+            return request.getParameter(FederationConstants.PARAM_CONTEXT);
+        } else if (request.getParameter(SAMLSSOConstants.RELAY_STATE) != null) {
+            return request.getParameter(SAMLSSOConstants.RELAY_STATE);
+        }
+        
+        return null;
+    }
 
+    private void verifySavedState(HttpServletRequest request) {
+        HttpSession session = request.getSession(false);
+        if (session != null) {
+            String savedContext = (String)session.getAttribute(FederationAuthenticationEntryPoint.SAVED_CONTEXT);
+            String state = getState(request);
+            if (savedContext != null && !savedContext.equals(state)) {
+                logger.warn("The received state does not match the state saved in the context");
+                throw new BadCredentialsException("The received state does not match the state saved in the context");
+            }
+        }
+    }
+    
     /**
      * 
      */

http://git-wip-us.apache.org/repos/asf/cxf-fediz/blob/707b8f95/plugins/spring2/src/main/java/org/apache/cxf/fediz/spring/web/FederationAuthenticationEntryPoint.java
----------------------------------------------------------------------
diff --git a/plugins/spring2/src/main/java/org/apache/cxf/fediz/spring/web/FederationAuthenticationEntryPoint.java b/plugins/spring2/src/main/java/org/apache/cxf/fediz/spring/web/FederationAuthenticationEntryPoint.java
index 3ba4693..2517afc 100644
--- a/plugins/spring2/src/main/java/org/apache/cxf/fediz/spring/web/FederationAuthenticationEntryPoint.java
+++ b/plugins/spring2/src/main/java/org/apache/cxf/fediz/spring/web/FederationAuthenticationEntryPoint.java
@@ -29,6 +29,7 @@ import javax.servlet.ServletRequest;
 import javax.servlet.ServletResponse;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
+import javax.servlet.http.HttpSession;
 
 import org.w3c.dom.Document;
 
@@ -65,6 +66,11 @@ import org.springframework.util.Assert;
 public class FederationAuthenticationEntryPoint implements AuthenticationEntryPoint,
     InitializingBean, ApplicationContextAware {
     
+    /**
+     * The key used to save the context of the request
+     */
+    public static final String SAVED_CONTEXT = "SAVED_CONTEXT";
+    
     private static final Logger LOG = LoggerFactory.getLogger(FederationAuthenticationEntryPoint.class);
     
     private ApplicationContext appContext;
@@ -163,6 +169,8 @@ public class FederationAuthenticationEntryPoint implements AuthenticationEntryPo
                 }
             }
             
+            HttpSession session = ((HttpServletRequest)request).getSession(true);
+            session.setAttribute(SAVED_CONTEXT, redirectionResponse.getRequestState().getState());
         } catch (ProcessingException ex) {
             System.err.println("Failed to create SignInRequest: " + ex.getMessage());
             LOG.warn("Failed to create SignInRequest: " + ex.getMessage());

http://git-wip-us.apache.org/repos/asf/cxf-fediz/blob/707b8f95/plugins/spring2/src/main/java/org/apache/cxf/fediz/spring/web/FederationAuthenticationFilter.java
----------------------------------------------------------------------
diff --git a/plugins/spring2/src/main/java/org/apache/cxf/fediz/spring/web/FederationAuthenticationFilter.java b/plugins/spring2/src/main/java/org/apache/cxf/fediz/spring/web/FederationAuthenticationFilter.java
index 61d5f60..7727c27 100644
--- a/plugins/spring2/src/main/java/org/apache/cxf/fediz/spring/web/FederationAuthenticationFilter.java
+++ b/plugins/spring2/src/main/java/org/apache/cxf/fediz/spring/web/FederationAuthenticationFilter.java
@@ -28,6 +28,7 @@ import java.util.Map.Entry;
 import javax.servlet.ServletRequest;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
+import javax.servlet.http.HttpSession;
 
 import org.apache.cxf.fediz.core.FederationConstants;
 import org.apache.cxf.fediz.core.SAMLSSOConstants;
@@ -106,15 +107,12 @@ public class FederationAuthenticationFilter extends AbstractProcessingFilter {
     @Override
     public Authentication attemptAuthentication(HttpServletRequest request) throws AuthenticationException {
         
-        SecurityContext context = SecurityContextHolder.getContext();
-        if (context != null) {
-            Authentication authentication = context.getAuthentication();
-            if (authentication instanceof FederationAuthenticationToken) {
-                // If we reach this point then the token must be expired
-                throw new ExpiredTokenException("Token is expired");
-            }
+        if (isTokenExpired()) {
+            throw new ExpiredTokenException("Token is expired");
         }
         
+        verifySavedState(request);
+        
         String wa = request.getParameter(FederationConstants.PARAM_ACTION);
         String responseToken = getResponseToken(request);
         FedizRequest wfReq = new FedizRequest();
@@ -134,6 +132,28 @@ public class FederationAuthenticationFilter extends AbstractProcessingFilter {
         return this.getAuthenticationManager().authenticate(authRequest);
     }
     
+    private void verifySavedState(HttpServletRequest request) {
+        HttpSession session = request.getSession(false);
+        if (session != null) {
+            String savedContext = (String)session.getAttribute(FederationAuthenticationEntryPoint.SAVED_CONTEXT);
+            String state = getState(request);
+            if (savedContext != null && !savedContext.equals(state)) {
+                logger.warn("The received state does not match the state saved in the context");
+                throw new BadCredentialsException("The received state does not match the state saved in the context");
+            }
+        }
+    }
+    
+    private String getState(ServletRequest request) {
+        if (request.getParameter(FederationConstants.PARAM_CONTEXT) != null) {
+            return request.getParameter(FederationConstants.PARAM_CONTEXT);
+        } else if (request.getParameter(SAMLSSOConstants.RELAY_STATE) != null) {
+            return request.getParameter(SAMLSSOConstants.RELAY_STATE);
+        }
+        
+        return null;
+    }
+    
     @Override
     public void onUnsuccessfulAuthentication(HttpServletRequest request, HttpServletResponse response,
                                              AuthenticationException authException) {


[3/3] cxf-fediz git commit: Enabling CSRF tests for the spring plugins

Posted by co...@apache.org.
Enabling CSRF tests for the spring plugins


Project: http://git-wip-us.apache.org/repos/asf/cxf-fediz/repo
Commit: http://git-wip-us.apache.org/repos/asf/cxf-fediz/commit/73a11b5f
Tree: http://git-wip-us.apache.org/repos/asf/cxf-fediz/tree/73a11b5f
Diff: http://git-wip-us.apache.org/repos/asf/cxf-fediz/diff/73a11b5f

Branch: refs/heads/1.2.x-fixes
Commit: 73a11b5f2aa799a4218d6777bdafbafcf695ff0c
Parents: 707b8f9
Author: Colm O hEigeartaigh <co...@apache.org>
Authored: Mon Dec 19 13:21:05 2016 +0000
Committer: Colm O hEigeartaigh <co...@apache.org>
Committed: Mon Dec 19 14:18:28 2016 +0000

----------------------------------------------------------------------
 .../apache/cxf/fediz/integrationtests/Spring2Test.java   |  9 +++++++++
 .../apache/cxf/fediz/integrationtests/SpringTest.java    |  8 ++++++++
 .../apache/cxf/fediz/integrationtests/AbstractTests.java | 11 ++++++-----
 3 files changed, 23 insertions(+), 5 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cxf-fediz/blob/73a11b5f/systests/spring/src/test/java/org/apache/cxf/fediz/integrationtests/Spring2Test.java
----------------------------------------------------------------------
diff --git a/systests/spring/src/test/java/org/apache/cxf/fediz/integrationtests/Spring2Test.java b/systests/spring/src/test/java/org/apache/cxf/fediz/integrationtests/Spring2Test.java
index d763132..52cc06f 100644
--- a/systests/spring/src/test/java/org/apache/cxf/fediz/integrationtests/Spring2Test.java
+++ b/systests/spring/src/test/java/org/apache/cxf/fediz/integrationtests/Spring2Test.java
@@ -240,4 +240,13 @@ public class Spring2Test extends AbstractTests {
     public void testEntityExpansionAttack() throws Exception {
 
     }
+    
+    @Override
+    @org.junit.Test
+    public void testCSRFAttack() throws Exception {
+        String url = "https://localhost:" + getRpHttpsPort() + "/" + getServletContextName() 
+            + "/j_spring_fediz_security_check";
+        csrfAttackTest(url);
+    }
+    
 }

http://git-wip-us.apache.org/repos/asf/cxf-fediz/blob/73a11b5f/systests/spring/src/test/java/org/apache/cxf/fediz/integrationtests/SpringTest.java
----------------------------------------------------------------------
diff --git a/systests/spring/src/test/java/org/apache/cxf/fediz/integrationtests/SpringTest.java b/systests/spring/src/test/java/org/apache/cxf/fediz/integrationtests/SpringTest.java
index 036b189..93b4201 100644
--- a/systests/spring/src/test/java/org/apache/cxf/fediz/integrationtests/SpringTest.java
+++ b/systests/spring/src/test/java/org/apache/cxf/fediz/integrationtests/SpringTest.java
@@ -149,4 +149,12 @@ public class SpringTest extends AbstractTests {
     public void testConcurrentRequests() throws Exception {
         // super.testConcurrentRequests();
     }
+    
+    @Override
+    @org.junit.Test
+    public void testCSRFAttack() throws Exception {
+        String url = "https://localhost:" + getRpHttpsPort() + "/" + getServletContextName() 
+            + "/j_spring_fediz_security_check";
+        csrfAttackTest(url);
+    }
 }

http://git-wip-us.apache.org/repos/asf/cxf-fediz/blob/73a11b5f/systests/tests/src/test/java/org/apache/cxf/fediz/integrationtests/AbstractTests.java
----------------------------------------------------------------------
diff --git a/systests/tests/src/test/java/org/apache/cxf/fediz/integrationtests/AbstractTests.java b/systests/tests/src/test/java/org/apache/cxf/fediz/integrationtests/AbstractTests.java
index 40c2d59..30d99d3 100644
--- a/systests/tests/src/test/java/org/apache/cxf/fediz/integrationtests/AbstractTests.java
+++ b/systests/tests/src/test/java/org/apache/cxf/fediz/integrationtests/AbstractTests.java
@@ -680,6 +680,11 @@ public abstract class AbstractTests {
     @org.junit.Ignore
     public void testCSRFAttack() throws Exception {
         String url = "https://localhost:" + getRpHttpsPort() + "/" + getServletContextName() + "/secure/fedservlet";
+        csrfAttackTest(url);
+    }
+    
+    protected void csrfAttackTest(String rpURL) throws Exception {
+        String url = "https://localhost:" + getRpHttpsPort() + "/" + getServletContextName() + "/secure/fedservlet";
         String user = "alice";
         String password = "ecila";
         
@@ -718,7 +723,7 @@ public abstract class AbstractTests {
         // 3. Now instead of clicking on the form, send the form via alice's WebClient instead
         
         // Send with context...
-        WebRequest request = new WebRequest(new URL(url), HttpMethod.POST);
+        WebRequest request = new WebRequest(new URL(rpURL), HttpMethod.POST);
         request.setRequestParameters(new ArrayList<NameValuePair>());
         
         DomNodeList<DomElement> results = idpPage2.getElementsByTagName("input");
@@ -740,10 +745,6 @@ public abstract class AbstractTests {
             Assert.fail("Failure expected on a CSRF attack");
         } catch (FailingHttpStatusCodeException ex) {
             // expected
-            Assert.assertTrue(ex.getMessage().contains("401 Unauthorized")
-                              || ex.getMessage().contains("401 Authentication Failed")
-                              || ex.getMessage().contains("403 Forbidden")
-                              || ex.getMessage().contains("408 Request Timeout"));
         }
         
         // webClient.close();