cxf-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From cohei...@apache.org
Subject cxf-fediz git commit: Adding validation of client registration URLs
Date Thu, 21 Jan 2016 17:17:25 GMT
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 <coheigea@apache.org>
Authored: Thu Jan 21 17:17:11 2016 +0000
Committer: Colm O hEigeartaigh <coheigea@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");
             }
             


Mime
View raw message