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/01/21 13:10:44 UTC
[1/5] cxf-fediz git commit: Added validation of wreply URLs in the
Fediz IdP using Commons Validator.
Repository: cxf-fediz
Updated Branches:
refs/heads/1.2.x-fixes d514e7f41 -> c31963dbc
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/9a76f51d
Tree: http://git-wip-us.apache.org/repos/asf/cxf-fediz/tree/9a76f51d
Diff: http://git-wip-us.apache.org/repos/asf/cxf-fediz/diff/9a76f51d
Branch: refs/heads/1.2.x-fixes
Commit: 9a76f51df003878f3569df068b4a9e92baeeb580
Parents: d514e7f
Author: Colm O hEigeartaigh <co...@apache.org>
Authored: Thu Jan 21 11:03:09 2016 +0000
Committer: Colm O hEigeartaigh <co...@apache.org>
Committed: Thu Jan 21 12:09:54 2016 +0000
----------------------------------------------------------------------
pom.xml | 1 +
services/idp/pom.xml | 4 +++
.../service/idp/beans/STSClientAction.java | 20 ++++++++++---
.../apache/cxf/fediz/systests/idp/IdpTest.java | 31 ++++++++++++++++++++
4 files changed, 52 insertions(+), 4 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/cxf-fediz/blob/9a76f51d/pom.xml
----------------------------------------------------------------------
diff --git a/pom.xml b/pom.xml
index 379f294..32aacee 100644
--- a/pom.xml
+++ b/pom.xml
@@ -40,6 +40,7 @@
<apacheds.version>2.0.0-M19</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.0.7</cxf.version>
<cxf.build-utils.version>3.0.0</cxf.build-utils.version>
<easymock.version>3.4</easymock.version>
http://git-wip-us.apache.org/repos/asf/cxf-fediz/blob/9a76f51d/services/idp/pom.xml
----------------------------------------------------------------------
diff --git a/services/idp/pom.xml b/services/idp/pom.xml
index 0eb734b..012112a 100644
--- a/services/idp/pom.xml
+++ b/services/idp/pom.xml
@@ -230,6 +230,10 @@
<groupId>org.apache.bval</groupId>
<artifactId>bval-jsr303</artifactId>
<version>${bval.version}</version>
+ <dependency>
+ <groupId>commons-validator</groupId>
+ <artifactId>commons-validator</artifactId>
+ <version>${commons.validator.version}</version>
</dependency>
</dependencies>
http://git-wip-us.apache.org/repos/asf/cxf-fediz/blob/9a76f51d/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 3f9f6c6..a323775 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
@@ -36,6 +36,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;
@@ -200,7 +201,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
@@ -304,16 +305,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/9a76f51d/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 fa24e0c..dd0046d 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
@@ -486,4 +486,35 @@ public class IdpTest {
}
}
+ // 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();
+ }
}
[2/5] cxf-fediz git commit: Fixing last commit
Posted by co...@apache.org.
Fixing last commit
Project: http://git-wip-us.apache.org/repos/asf/cxf-fediz/repo
Commit: http://git-wip-us.apache.org/repos/asf/cxf-fediz/commit/c23eb878
Tree: http://git-wip-us.apache.org/repos/asf/cxf-fediz/tree/c23eb878
Diff: http://git-wip-us.apache.org/repos/asf/cxf-fediz/diff/c23eb878
Branch: refs/heads/1.2.x-fixes
Commit: c23eb8786279034e8492a1499f376a554dc4de80
Parents: 9a76f51
Author: Colm O hEigeartaigh <co...@apache.org>
Authored: Thu Jan 21 11:26:12 2016 +0000
Committer: Colm O hEigeartaigh <co...@apache.org>
Committed: Thu Jan 21 12:10:03 2016 +0000
----------------------------------------------------------------------
.../org/apache/cxf/fediz/service/idp/beans/STSClientAction.java | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/cxf-fediz/blob/c23eb878/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 a323775..033d5c6 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
@@ -314,7 +314,7 @@ public class STSClientAction {
// Validate it first using commons-validator
String[] schemes = {"https"};
- UrlValidator urlValidator = new UrlValidator(schemes);
+ UrlValidator urlValidator = new UrlValidator(schemes, UrlValidator.ALLOW_LOCAL_URLS);
if (!urlValidator.isValid(wreply)) {
LOG.warn("The given wreply parameter {} is not a valid URL", wreply);
throw new ProcessingException(TYPE.BAD_REQUEST);
[5/5] cxf-fediz git commit: Fixing merge
Posted by co...@apache.org.
Fixing merge
Project: http://git-wip-us.apache.org/repos/asf/cxf-fediz/repo
Commit: http://git-wip-us.apache.org/repos/asf/cxf-fediz/commit/c31963db
Tree: http://git-wip-us.apache.org/repos/asf/cxf-fediz/tree/c31963db
Diff: http://git-wip-us.apache.org/repos/asf/cxf-fediz/diff/c31963db
Branch: refs/heads/1.2.x-fixes
Commit: c31963dbcb90712352749872b8f1af828362b665
Parents: f482853
Author: Colm O hEigeartaigh <co...@apache.org>
Authored: Thu Jan 21 12:10:34 2016 +0000
Committer: Colm O hEigeartaigh <co...@apache.org>
Committed: Thu Jan 21 12:10:34 2016 +0000
----------------------------------------------------------------------
.../src/test/java/org/apache/cxf/fediz/systests/idp/IdpTest.java | 2 --
1 file changed, 2 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/cxf-fediz/blob/c31963db/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 dd0046d..78cbe56 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
@@ -514,7 +514,5 @@ public class IdpTest {
} catch (FailingHttpStatusCodeException ex) {
Assert.assertEquals(ex.getStatusCode(), 400);
}
-
- webClient.close();
}
}
[3/5] cxf-fediz git commit: Some improvements to the WFreshParser
Posted by co...@apache.org.
Some improvements to the WFreshParser
Project: http://git-wip-us.apache.org/repos/asf/cxf-fediz/repo
Commit: http://git-wip-us.apache.org/repos/asf/cxf-fediz/commit/dfc6a99d
Tree: http://git-wip-us.apache.org/repos/asf/cxf-fediz/tree/dfc6a99d
Diff: http://git-wip-us.apache.org/repos/asf/cxf-fediz/diff/dfc6a99d
Branch: refs/heads/1.2.x-fixes
Commit: dfc6a99d5a073004de604240d18d113fa6f5415d
Parents: c23eb87
Author: Colm O hEigeartaigh <co...@apache.org>
Authored: Thu Jan 21 11:51:43 2016 +0000
Committer: Colm O hEigeartaigh <co...@apache.org>
Committed: Thu Jan 21 12:10:15 2016 +0000
----------------------------------------------------------------------
.../cxf/fediz/service/idp/beans/WfreshParser.java | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/cxf-fediz/blob/dfc6a99d/services/idp/src/main/java/org/apache/cxf/fediz/service/idp/beans/WfreshParser.java
----------------------------------------------------------------------
diff --git a/services/idp/src/main/java/org/apache/cxf/fediz/service/idp/beans/WfreshParser.java b/services/idp/src/main/java/org/apache/cxf/fediz/service/idp/beans/WfreshParser.java
index 3fba1c8..a428553 100644
--- a/services/idp/src/main/java/org/apache/cxf/fediz/service/idp/beans/WfreshParser.java
+++ b/services/idp/src/main/java/org/apache/cxf/fediz/service/idp/beans/WfreshParser.java
@@ -41,6 +41,10 @@ public class WfreshParser {
SecurityToken idpToken =
(SecurityToken) WebUtils.getAttributeFromExternalContext(context, whr);
+ if (idpToken == null) {
+ return true;
+ }
+
if (tokenExpirationValidation && idpToken.isExpired()) {
LOG.info("[IDP_TOKEN=" + idpToken.getId() + "] is expired.");
return true;
@@ -59,12 +63,15 @@ public class WfreshParser {
}
if (ttl == 0) {
return true;
- } else if (ttl > 0) {
+ }
+
+ long ttlMs = ttl * 60L * 1000L;
+ if (ttlMs > 0) {
Date createdDate = idpToken.getCreated();
if (createdDate != null) {
Date expiryDate = new Date();
- expiryDate.setTime(createdDate.getTime() + (ttl * 60L * 1000L));
+ expiryDate.setTime(createdDate.getTime() + ttlMs);
if (expiryDate.before(new Date())) {
LOG.info("[IDP_TOKEN="
+ idpToken.getId()
@@ -76,7 +83,7 @@ public class WfreshParser {
LOG.info("token creation date not set. Unable to check wfresh is outdated.");
}
} else {
- LOG.info("ttl value '" + ttl + "' is negative.");
+ LOG.info("ttl value '" + ttl + "' is negative or is too large.");
}
return false;
}
[4/5] cxf-fediz git commit: Checkstyle fix
Posted by co...@apache.org.
Checkstyle fix
Project: http://git-wip-us.apache.org/repos/asf/cxf-fediz/repo
Commit: http://git-wip-us.apache.org/repos/asf/cxf-fediz/commit/f4828532
Tree: http://git-wip-us.apache.org/repos/asf/cxf-fediz/tree/f4828532
Diff: http://git-wip-us.apache.org/repos/asf/cxf-fediz/diff/f4828532
Branch: refs/heads/1.2.x-fixes
Commit: f4828532cb327427f68010cf2dc5b152c58aa1d8
Parents: dfc6a99
Author: Colm O hEigeartaigh <co...@apache.org>
Authored: Thu Jan 21 11:56:24 2016 +0000
Committer: Colm O hEigeartaigh <co...@apache.org>
Committed: Thu Jan 21 12:10:22 2016 +0000
----------------------------------------------------------------------
.../fediz/service/idp/beans/WfreshParser.java | 26 ++++++++++++++------
1 file changed, 18 insertions(+), 8 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/cxf-fediz/blob/f4828532/services/idp/src/main/java/org/apache/cxf/fediz/service/idp/beans/WfreshParser.java
----------------------------------------------------------------------
diff --git a/services/idp/src/main/java/org/apache/cxf/fediz/service/idp/beans/WfreshParser.java b/services/idp/src/main/java/org/apache/cxf/fediz/service/idp/beans/WfreshParser.java
index a428553..4478e52 100644
--- a/services/idp/src/main/java/org/apache/cxf/fediz/service/idp/beans/WfreshParser.java
+++ b/services/idp/src/main/java/org/apache/cxf/fediz/service/idp/beans/WfreshParser.java
@@ -39,14 +39,7 @@ public class WfreshParser {
public boolean authenticationRequired(String wfresh, String whr, RequestContext context)
throws Exception {
- SecurityToken idpToken =
- (SecurityToken) WebUtils.getAttributeFromExternalContext(context, whr);
- if (idpToken == null) {
- return true;
- }
-
- if (tokenExpirationValidation && idpToken.isExpired()) {
- LOG.info("[IDP_TOKEN=" + idpToken.getId() + "] is expired.");
+ if (checkIsIdpTokenExpired(whr, context)) {
return true;
}
@@ -68,6 +61,8 @@ public class WfreshParser {
long ttlMs = ttl * 60L * 1000L;
if (ttlMs > 0) {
+ SecurityToken idpToken =
+ (SecurityToken) WebUtils.getAttributeFromExternalContext(context, whr);
Date createdDate = idpToken.getCreated();
if (createdDate != null) {
Date expiryDate = new Date();
@@ -87,6 +82,21 @@ public class WfreshParser {
}
return false;
}
+
+ private boolean checkIsIdpTokenExpired(String whr, RequestContext context) {
+ SecurityToken idpToken =
+ (SecurityToken) WebUtils.getAttributeFromExternalContext(context, whr);
+ if (idpToken == null) {
+ return true;
+ }
+
+ if (tokenExpirationValidation && idpToken.isExpired()) {
+ LOG.info("[IDP_TOKEN=" + idpToken.getId() + "] is expired.");
+ return true;
+ }
+
+ return false;
+ }
public boolean isTokenExpirationValidation() {
return tokenExpirationValidation;