cxf-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From cohei...@apache.org
Subject [1/4] cxf-fediz git commit: Added validation of wreply URLs in the Fediz IdP using Commons Validator.
Date Thu, 21 Jan 2016 12:01:00 GMT
Repository: cxf-fediz
Updated Branches:
  refs/heads/master 8b8b0b9ef -> 669232ddc


Added validation of wreply URLs in the Fediz IdP using Commons Validator.


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

Branch: refs/heads/master
Commit: a94c0a6fcbdf3cd568a14df5bba88f895932e6f7
Parents: 8b8b0b9
Author: Colm O hEigeartaigh <coheigea@apache.org>
Authored: Thu Jan 21 11:03:09 2016 +0000
Committer: Colm O hEigeartaigh <coheigea@apache.org>
Committed: Thu Jan 21 12:00:55 2016 +0000

----------------------------------------------------------------------
 pom.xml                                         |  1 +
 services/idp/pom.xml                            | 11 +++++--
 .../service/idp/beans/STSClientAction.java      | 20 ++++++++++---
 .../apache/cxf/fediz/systests/idp/IdpTest.java  | 31 ++++++++++++++++++++
 4 files changed, 56 insertions(+), 7 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cxf-fediz/blob/a94c0a6f/pom.xml
----------------------------------------------------------------------
diff --git a/pom.xml b/pom.xml
index c4f5423..584fbf9 100644
--- a/pom.xml
+++ b/pom.xml
@@ -40,6 +40,7 @@
         <apacheds.version>2.0.0-M21</apacheds.version>
         <commons.lang.version>3.4</commons.lang.version>
         <commons.logging.version>1.2</commons.logging.version>
+        <commons.validator.version>1.5.0</commons.validator.version>
         <cxf.version>3.1.5-SNAPSHOT</cxf.version>
         <cxf.build-utils.version>3.1.0</cxf.build-utils.version>
         <easymock.version>3.4</easymock.version>

http://git-wip-us.apache.org/repos/asf/cxf-fediz/blob/a94c0a6f/services/idp/pom.xml
----------------------------------------------------------------------
diff --git a/services/idp/pom.xml b/services/idp/pom.xml
index 9b68184..2db7b8e 100644
--- a/services/idp/pom.xml
+++ b/services/idp/pom.xml
@@ -244,9 +244,14 @@
             <version>${javax.el.version}</version>
         </dependency>
         <dependency>
-        	<groupId>io.swagger</groupId>
-        	<artifactId>swagger-jaxrs</artifactId>
-        	<version>1.5.6</version>
+            <groupId>io.swagger</groupId>
+            <artifactId>swagger-jaxrs</artifactId>
+            <version>1.5.6</version>
+        </dependency>
+        <dependency>
+            <groupId>commons-validator</groupId>
+            <artifactId>commons-validator</artifactId>
+            <version>${commons.validator.version}</version>
         </dependency>
     </dependencies>
     <build>

http://git-wip-us.apache.org/repos/asf/cxf-fediz/blob/a94c0a6f/services/idp/src/main/java/org/apache/cxf/fediz/service/idp/beans/STSClientAction.java
----------------------------------------------------------------------
diff --git a/services/idp/src/main/java/org/apache/cxf/fediz/service/idp/beans/STSClientAction.java
b/services/idp/src/main/java/org/apache/cxf/fediz/service/idp/beans/STSClientAction.java
index efe7fd6..4082bef 100644
--- a/services/idp/src/main/java/org/apache/cxf/fediz/service/idp/beans/STSClientAction.java
+++ b/services/idp/src/main/java/org/apache/cxf/fediz/service/idp/beans/STSClientAction.java
@@ -37,6 +37,7 @@ import org.w3c.dom.Document;
 import org.w3c.dom.Element;
 import org.w3c.dom.NodeList;
 import org.apache.commons.lang3.StringEscapeUtils;
+import org.apache.commons.validator.routines.UrlValidator;
 import org.apache.cxf.Bus;
 import org.apache.cxf.BusFactory;
 import org.apache.cxf.binding.soap.SoapFault;
@@ -201,7 +202,7 @@ public class STSClientAction {
             throw new ProcessingException(TYPE.BAD_REQUEST);
         }
         
-        // Check wreply parameter against passive requestor endpoint constraint
+        // Check that the wreply parameter is valid
         validateApplicationEndpoint(serviceConfig, context);
         
         // Parse wreq parameter - we only support parsing TokenType and KeyType for now
@@ -307,16 +308,27 @@ public class STSClientAction {
     }
     
     // The wreply address must match the passive endpoint requestor constraint (if it is
specified)
-    private void validateApplicationEndpoint(Application serviceConfig, RequestContext context)

+    // Also, it must be a valid URL + start with https
+    protected void validateApplicationEndpoint(Application serviceConfig, RequestContext
context) 
         throws ProcessingException {
+        
+        String wreply = 
+            (String)WebUtils.getAttributeFromFlowScope(context, FederationConstants.PARAM_REPLY);
+        
+        // Validate it first using commons-validator
+        String[] schemes = {"https"};
+        UrlValidator urlValidator = new UrlValidator(schemes);
+        if (!urlValidator.isValid(wreply)) {
+            LOG.warn("The given wreply parameter {} is not a valid URL", wreply);
+            throw new ProcessingException(TYPE.BAD_REQUEST);
+        }
+        
         if (serviceConfig.getCompiledPassiveRequestorEndpointConstraint() == null) {
             LOG.warn("No passive requestor endpoint constraint is configured for the application.
"
                      + "This could lead to a malicious redirection attack");
             return;
         }
         
-        String wreply = 
-            (String)WebUtils.getAttributeFromFlowScope(context, FederationConstants.PARAM_REPLY);
         if (wreply != null) {
             Matcher matcher = serviceConfig.getCompiledPassiveRequestorEndpointConstraint().matcher(wreply);
             if (!matcher.matches()) {

http://git-wip-us.apache.org/repos/asf/cxf-fediz/blob/a94c0a6f/systests/idp/src/test/java/org/apache/cxf/fediz/systests/idp/IdpTest.java
----------------------------------------------------------------------
diff --git a/systests/idp/src/test/java/org/apache/cxf/fediz/systests/idp/IdpTest.java b/systests/idp/src/test/java/org/apache/cxf/fediz/systests/idp/IdpTest.java
index 3fc1539..1116759 100644
--- a/systests/idp/src/test/java/org/apache/cxf/fediz/systests/idp/IdpTest.java
+++ b/systests/idp/src/test/java/org/apache/cxf/fediz/systests/idp/IdpTest.java
@@ -508,4 +508,35 @@ public class IdpTest {
         webClient.close();
     }
     
+    // Send a bad wreply value. This will pass the reg ex validation but fail the commons-validator

+    // validation
+    @org.junit.Test
+    public void testWReplyWithDoubleSlashes() throws Exception {
+        String url = "https://localhost:" + getIdpHttpsPort() + "/fediz-idp/federation?";
+        url += "wa=wsignin1.0";
+        url += "&whr=urn:org:apache:cxf:fediz:idp:realm-A";
+        url += "&wtrealm=urn:org:apache:cxf:fediz:fedizhelloworld";
+        String wreply = "https://localhost:" + getRpHttpsPort() + "/" + getServletContextName()

+            + "/secure//fedservlet";
+        url += "&wreply=" + wreply;
+
+        String user = "alice";
+        String password = "ecila";
+
+        final WebClient webClient = new WebClient();
+        webClient.getOptions().setUseInsecureSSL(true);
+        webClient.getCredentialsProvider().setCredentials(
+            new AuthScope("localhost", Integer.parseInt(getIdpHttpsPort())),
+            new UsernamePasswordCredentials(user, password));
+
+        webClient.getOptions().setJavaScriptEnabled(false);
+        try {
+            webClient.getPage(url);
+            Assert.fail("Failure expected on a bad wreply value");
+        } catch (FailingHttpStatusCodeException ex) {
+            Assert.assertEquals(ex.getStatusCode(), 400);
+        }
+
+        webClient.close();
+    }
 }


Mime
View raw message