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:01:00 UTC
[1/4] cxf-fediz git commit: Added validation of wreply URLs in the
Fediz IdP using Commons Validator.
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 <co...@apache.org>
Authored: Thu Jan 21 11:03:09 2016 +0000
Committer: Colm O hEigeartaigh <co...@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();
+ }
}
[2/4] 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/18f40e3f
Tree: http://git-wip-us.apache.org/repos/asf/cxf-fediz/tree/18f40e3f
Diff: http://git-wip-us.apache.org/repos/asf/cxf-fediz/diff/18f40e3f
Branch: refs/heads/master
Commit: 18f40e3fbd6e9c86b68f6a6070af3dbe2588ea1d
Parents: a94c0a6
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:00:56 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/18f40e3f/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 4082bef..fcd3f8e 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
@@ -317,7 +317,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);
[3/4] 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/669232dd
Tree: http://git-wip-us.apache.org/repos/asf/cxf-fediz/tree/669232dd
Diff: http://git-wip-us.apache.org/repos/asf/cxf-fediz/diff/669232dd
Branch: refs/heads/master
Commit: 669232ddc1f9602c6fbec94af88fa1de89932e7a
Parents: ae1cbec
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:00:56 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/669232dd/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 a72be8f..792b4fd 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
@@ -40,14 +40,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;
}
@@ -69,6 +62,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();
@@ -88,6 +83,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;
[4/4] 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/ae1cbece
Tree: http://git-wip-us.apache.org/repos/asf/cxf-fediz/tree/ae1cbece
Diff: http://git-wip-us.apache.org/repos/asf/cxf-fediz/diff/ae1cbece
Branch: refs/heads/master
Commit: ae1cbece32c66a6fef5a8d2e3ff26ce91674cd5d
Parents: 18f40e3
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:00:56 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/ae1cbece/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 d7f03d6..a72be8f 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
@@ -42,6 +42,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;
@@ -60,12 +64,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()
@@ -77,7 +84,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;
}