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 18:17:25 UTC
cxf-fediz git commit: Adding validation of client registration URLs
Repository: cxf-fediz
Updated Branches:
refs/heads/master 5c354ce09 -> 1ce901698
Adding validation of client registration URLs
Project: http://git-wip-us.apache.org/repos/asf/cxf-fediz/repo
Commit: http://git-wip-us.apache.org/repos/asf/cxf-fediz/commit/1ce90169
Tree: http://git-wip-us.apache.org/repos/asf/cxf-fediz/tree/1ce90169
Diff: http://git-wip-us.apache.org/repos/asf/cxf-fediz/diff/1ce90169
Branch: refs/heads/master
Commit: 1ce901698713075a07e9da363a121fa531d3c5c7
Parents: 5c354ce
Author: Colm O hEigeartaigh <co...@apache.org>
Authored: Thu Jan 21 17:17:11 2016 +0000
Committer: Colm O hEigeartaigh <co...@apache.org>
Committed: Thu Jan 21 17:17:11 2016 +0000
----------------------------------------------------------------------
services/oidc/pom.xml | 8 ++-
.../service/oidc/ClientRegistrationService.java | 23 +++++++-
.../oidc/InvalidRegistrationException.java | 32 +++++++++++
.../webapp/WEB-INF/views/clientCodeGrants.jsp | 2 +-
.../cxf/fediz/systests/oidc/OIDCTest.java | 60 +++++++++++++++++---
5 files changed, 112 insertions(+), 13 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/cxf-fediz/blob/1ce90169/services/oidc/pom.xml
----------------------------------------------------------------------
diff --git a/services/oidc/pom.xml b/services/oidc/pom.xml
index bc9a73c..02be0c0 100644
--- a/services/oidc/pom.xml
+++ b/services/oidc/pom.xml
@@ -62,11 +62,17 @@
<artifactId>spring-web</artifactId>
<version>${spring.version}</version>
</dependency>
- <dependency>
+ <dependency>
<groupId>org.slf4j</groupId>
<artifactId>slf4j-log4j12</artifactId>
<version>${slf4j.version}</version>
</dependency>
+ <dependency>
+ <groupId>commons-validator</groupId>
+ <artifactId>commons-validator</artifactId>
+ <version>${commons.validator.version}</version>
+ </dependency>
+
</dependencies>
<build>
<plugins>
http://git-wip-us.apache.org/repos/asf/cxf-fediz/blob/1ce90169/services/oidc/src/main/java/org/apache/cxf/fediz/service/oidc/ClientRegistrationService.java
----------------------------------------------------------------------
diff --git a/services/oidc/src/main/java/org/apache/cxf/fediz/service/oidc/ClientRegistrationService.java b/services/oidc/src/main/java/org/apache/cxf/fediz/service/oidc/ClientRegistrationService.java
index ff01d41..df01e41 100644
--- a/services/oidc/src/main/java/org/apache/cxf/fediz/service/oidc/ClientRegistrationService.java
+++ b/services/oidc/src/main/java/org/apache/cxf/fediz/service/oidc/ClientRegistrationService.java
@@ -42,6 +42,7 @@ import javax.ws.rs.core.Context;
import javax.ws.rs.core.MediaType;
import javax.ws.rs.core.SecurityContext;
+import org.apache.commons.validator.routines.UrlValidator;
import org.apache.cxf.common.util.Base64UrlUtility;
import org.apache.cxf.common.util.StringUtils;
import org.apache.cxf.rs.security.oauth2.common.Client;
@@ -192,8 +193,26 @@ public class ClientRegistrationService {
@FormParam("client_type") String appType,
@FormParam("client_audience") String audience,
@FormParam("client_redirectURI") String redirectURI,
- @FormParam("client_homeRealm") String homeRealm) {
- //TODO Check for mandatory parameters
+ @FormParam("client_homeRealm") String homeRealm
+ ) throws InvalidRegistrationException {
+
+ // Check parameters
+ if (appName == null || "".equals(appName)) {
+ throw new InvalidRegistrationException("The client id must not be empty");
+ }
+ if (appType == null) {
+ throw new InvalidRegistrationException("The client type must not be empty");
+ }
+ if (!("confidential".equals(appType) || "public".equals(appType))) {
+ throw new InvalidRegistrationException("An invalid client type was specified: " + appType);
+ }
+ if (redirectURI != null) {
+ String[] schemes = {"https"};
+ UrlValidator urlValidator = new UrlValidator(schemes, UrlValidator.ALLOW_LOCAL_URLS);
+ if (!urlValidator.isValid(redirectURI)) {
+ throw new InvalidRegistrationException("An invalid redirect URI was specified: " + redirectURI);
+ }
+ }
String clientId = generateClientId();
boolean isConfidential = "confidential".equals(appType);
http://git-wip-us.apache.org/repos/asf/cxf-fediz/blob/1ce90169/services/oidc/src/main/java/org/apache/cxf/fediz/service/oidc/InvalidRegistrationException.java
----------------------------------------------------------------------
diff --git a/services/oidc/src/main/java/org/apache/cxf/fediz/service/oidc/InvalidRegistrationException.java b/services/oidc/src/main/java/org/apache/cxf/fediz/service/oidc/InvalidRegistrationException.java
new file mode 100644
index 0000000..d115f31
--- /dev/null
+++ b/services/oidc/src/main/java/org/apache/cxf/fediz/service/oidc/InvalidRegistrationException.java
@@ -0,0 +1,32 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.cxf.fediz.service.oidc;
+
+public class InvalidRegistrationException extends Exception {
+
+ /**
+ *
+ */
+ private static final long serialVersionUID = 6251451448320551293L;
+
+ public InvalidRegistrationException(String message) {
+ super(message);
+ }
+}
\ No newline at end of file
http://git-wip-us.apache.org/repos/asf/cxf-fediz/blob/1ce90169/services/oidc/src/main/webapp/WEB-INF/views/clientCodeGrants.jsp
----------------------------------------------------------------------
diff --git a/services/oidc/src/main/webapp/WEB-INF/views/clientCodeGrants.jsp b/services/oidc/src/main/webapp/WEB-INF/views/clientCodeGrants.jsp
index 885b150..1213e9a 100644
--- a/services/oidc/src/main/webapp/WEB-INF/views/clientCodeGrants.jsp
+++ b/services/oidc/src/main/webapp/WEB-INF/views/clientCodeGrants.jsp
@@ -41,7 +41,7 @@
</head>
<body>
<div class="padded">
-<h1>Code Grants issued to <%= client.getApplicationName() + "(" + client.getClientId() + ")"%></h1>
+<h1>Code Grants issued to <%= client.getApplicationName() + " (" + client.getClientId() + ")"%></h1>
<br/>
<table border="1">
<tr><th>ID</th><th>Issue Date</th><th>Expiry Date</th><th>Action</th></tr>
http://git-wip-us.apache.org/repos/asf/cxf-fediz/blob/1ce90169/systests/oidc/src/test/java/org/apache/cxf/fediz/systests/oidc/OIDCTest.java
----------------------------------------------------------------------
diff --git a/systests/oidc/src/test/java/org/apache/cxf/fediz/systests/oidc/OIDCTest.java b/systests/oidc/src/test/java/org/apache/cxf/fediz/systests/oidc/OIDCTest.java
index 0114a65..3c2aed3 100644
--- a/systests/oidc/src/test/java/org/apache/cxf/fediz/systests/oidc/OIDCTest.java
+++ b/systests/oidc/src/test/java/org/apache/cxf/fediz/systests/oidc/OIDCTest.java
@@ -40,6 +40,7 @@ import com.gargoylesoftware.htmlunit.html.DomNodeList;
import com.gargoylesoftware.htmlunit.html.HtmlButton;
import com.gargoylesoftware.htmlunit.html.HtmlForm;
import com.gargoylesoftware.htmlunit.html.HtmlPage;
+import com.gargoylesoftware.htmlunit.html.HtmlSelect;
import com.gargoylesoftware.htmlunit.html.HtmlSubmitInput;
import com.gargoylesoftware.htmlunit.html.HtmlTable;
import com.gargoylesoftware.htmlunit.html.HtmlTextInput;
@@ -230,11 +231,11 @@ public class OIDCTest {
// Now try to register a new client
HtmlPage registeredClientPage =
- registerNewClient(webClient, url, "new-client", "http://127.0.0.1");
+ registerNewClient(webClient, url, "new-client", "https://127.0.0.1");
String registeredClientPageBody = registeredClientPage.getBody().getTextContent();
Assert.assertTrue(registeredClientPageBody.contains("Registered Clients"));
Assert.assertTrue(registeredClientPageBody.contains("new-client"));
- Assert.assertTrue(registeredClientPageBody.contains("http://127.0.0.1"));
+ Assert.assertTrue(registeredClientPageBody.contains("https://127.0.0.1"));
HtmlTable table = registeredClientPage.getHtmlElementById("registered_clients");
storedClientId = table.getCellAt(1, 1).asText().trim();
@@ -242,13 +243,13 @@ public class OIDCTest {
// Try to register another new client
registeredClientPage =
- registerNewClient(webClient, url, "new-client2", "http://127.0.1.1");
+ registerNewClient(webClient, url, "new-client2", "https://127.0.1.1");
registeredClientPageBody = registeredClientPage.getBody().getTextContent();
Assert.assertTrue(registeredClientPageBody.contains("Registered Clients"));
Assert.assertTrue(registeredClientPageBody.contains("new-client"));
- Assert.assertTrue(registeredClientPageBody.contains("http://127.0.0.1"));
+ Assert.assertTrue(registeredClientPageBody.contains("https://127.0.0.1"));
Assert.assertTrue(registeredClientPageBody.contains("new-client2"));
- Assert.assertTrue(registeredClientPageBody.contains("http://127.0.1.1"));
+ Assert.assertTrue(registeredClientPageBody.contains("https://127.0.1.1"));
table = registeredClientPage.getHtmlElementById("registered_clients");
storedClient2Id = table.getCellAt(2, 1).asText().trim();
@@ -269,6 +270,8 @@ public class OIDCTest {
// Set new client values
final HtmlTextInput clientNameInput = form.getInputByName("client_name");
clientNameInput.setValueAttribute(clientName);
+ final HtmlSelect clientTypeSelect = form.getSelectByName("client_type");
+ clientTypeSelect.setSelectedAttribute("confidential", true);
final HtmlTextInput redirectURIInput = form.getInputByName("client_redirectURI");
redirectURIInput.setValueAttribute(redirectURI);
@@ -354,8 +357,8 @@ public class OIDCTest {
// Check the redirect URI
String redirectURI = table.getCellAt(1, 3).asText().trim();
- Assert.assertTrue("http://127.0.0.1".equals(redirectURI)
- || "http://127.0.1.1".equals(redirectURI));
+ Assert.assertTrue("https://127.0.0.1".equals(redirectURI)
+ || "https://127.0.1.1".equals(redirectURI));
// Now check the specific client page
HtmlPage clientPage = webClient.getPage(url + "/" + clientId);
@@ -468,6 +471,45 @@ public class OIDCTest {
}
}
+ @org.junit.Test
+ public void testBadClientId() throws Exception {
+
+ String url = "https://localhost:" + getRpHttpsPort() + "/fediz-oidc/idp/authorize?";
+ url += "client_id=" + storedClientId + 2;
+ url += "&response_type=code";
+ url += "&scope=openid";
+ String user = "alice";
+ String password = "ecila";
+
+ // Login to the OIDC token endpoint + get the authorization code
+ WebClient webClient = setupWebClient(user, password, getIdpHttpsPort());
+
+ String authorizationCode = loginAndGetAuthorizationCode(url, webClient);
+ Assert.assertNull(authorizationCode);
+
+ webClient.close();
+ }
+
+ @org.junit.Test
+ public void testIncorrectRedirectURI() throws Exception {
+
+ String url = "https://localhost:" + getRpHttpsPort() + "/fediz-oidc/idp/authorize?";
+ url += "client_id=" + storedClientId;
+ url += "&response_type=code";
+ url += "&scope=openid";
+ url += "&redirect_uri=https://127.0.0.5";
+ String user = "alice";
+ String password = "ecila";
+
+ // Login to the OIDC token endpoint + get the authorization code
+ WebClient webClient = setupWebClient(user, password, getIdpHttpsPort());
+
+ String authorizationCode = loginAndGetAuthorizationCode(url, webClient);
+ Assert.assertNull(authorizationCode);
+
+ webClient.close();
+ }
+
private static WebClient setupWebClient(String user, String password, String idpPort) {
final WebClient webClient = new WebClient();
webClient.getOptions().setUseInsecureSSL(true);
@@ -529,7 +571,7 @@ public class OIDCTest {
final HtmlSubmitInput button = form.getInputByName("_eventId_submit");
// Bit of a hack here to get the authorization code - necessary as HtmlUnit tries
- // to follow the server redirect to "http://127.0.0.1" - the redirect URI
+ // to follow the server redirect to "https://127.0.0.1" - the redirect URI
CodeWebConnectionWrapper wrapper = new CodeWebConnectionWrapper(webClient);
try {
@@ -553,7 +595,7 @@ public class OIDCTest {
public WebResponse getResponse(WebRequest request) throws IOException {
WebResponse response = super.getResponse(request);
String location = response.getResponseHeaderValue("Location");
- if (location != null && location.contains("code")) {
+ if (location != null && location.contains("code=")) {
code = getSubstring(location, "code");
}