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 CSRF support to the OIDC client reg webapp
Date Tue, 18 Apr 2017 17:05:00 GMT
Repository: cxf-fediz
Updated Branches:
  refs/heads/1.3.x-fixes 12051ad2d -> 71480c3f7


Adding CSRF support to the OIDC client reg webapp


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

Branch: refs/heads/1.3.x-fixes
Commit: 71480c3f7e516cf0d535fdd5abec63ab455e4d06
Parents: 12051ad
Author: Colm O hEigeartaigh <coheigea@apache.org>
Authored: Tue Apr 18 17:22:27 2017 +0100
Committer: Colm O hEigeartaigh <coheigea@apache.org>
Committed: Tue Apr 18 18:04:54 2017 +0100

----------------------------------------------------------------------
 .../cxf/fediz/service/oidc/CSRFUtils.java       | 52 +++++++++++
 .../oidc/clients/ClientRegistrationService.java | 98 ++++++++++++++++----
 .../src/main/webapp/WEB-INF/views/client.jsp    | 12 ++-
 .../webapp/WEB-INF/views/clientCodeGrants.jsp   |  7 +-
 .../main/webapp/WEB-INF/views/clientTokens.jsp  |  7 +-
 .../webapp/WEB-INF/views/registerClient.jsp     | 10 +-
 .../cxf/fediz/systests/oidc/OIDCTest.java       | 32 +++++++
 7 files changed, 197 insertions(+), 21 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cxf-fediz/blob/71480c3f/services/oidc/src/main/java/org/apache/cxf/fediz/service/oidc/CSRFUtils.java
----------------------------------------------------------------------
diff --git a/services/oidc/src/main/java/org/apache/cxf/fediz/service/oidc/CSRFUtils.java
b/services/oidc/src/main/java/org/apache/cxf/fediz/service/oidc/CSRFUtils.java
new file mode 100644
index 0000000..79e078a
--- /dev/null
+++ b/services/oidc/src/main/java/org/apache/cxf/fediz/service/oidc/CSRFUtils.java
@@ -0,0 +1,52 @@
+/**
+ * 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;
+
+import javax.servlet.http.HttpServletRequest;
+
+import org.apache.cxf.common.util.StringUtils;
+import org.apache.cxf.rt.security.crypto.CryptoUtils;
+
+public final class CSRFUtils {
+
+    public static final String CSRF_TOKEN = "CSRF_TOKEN";
+
+    private CSRFUtils() {
+        // complete
+    }
+
+    public static String getCSRFToken(HttpServletRequest request, boolean create) {
+        if (request != null && request.getSession() != null) {
+            // Return an existing token first
+            String savedToken = (String)request.getSession().getAttribute(CSRF_TOKEN);
+            if (savedToken != null) {
+                return savedToken;
+            }
+
+            // If no existing token then create a new one, save it, and return it
+            if (create) {
+                String token = StringUtils.toHexString(CryptoUtils.generateSecureRandomBytes(16));
+                request.getSession().setAttribute(CSRF_TOKEN, token);
+                return token;
+            }
+        }
+
+        return null;
+    }
+}

http://git-wip-us.apache.org/repos/asf/cxf-fediz/blob/71480c3f/services/oidc/src/main/java/org/apache/cxf/fediz/service/oidc/clients/ClientRegistrationService.java
----------------------------------------------------------------------
diff --git a/services/oidc/src/main/java/org/apache/cxf/fediz/service/oidc/clients/ClientRegistrationService.java
b/services/oidc/src/main/java/org/apache/cxf/fediz/service/oidc/clients/ClientRegistrationService.java
index 5bd8efd..107dbf2 100644
--- a/services/oidc/src/main/java/org/apache/cxf/fediz/service/oidc/clients/ClientRegistrationService.java
+++ b/services/oidc/src/main/java/org/apache/cxf/fediz/service/oidc/clients/ClientRegistrationService.java
@@ -38,6 +38,7 @@ import java.util.SortedSet;
 import java.util.TreeSet;
 import java.util.logging.Logger;
 
+import javax.servlet.http.HttpServletRequest;
 import javax.ws.rs.Consumes;
 import javax.ws.rs.FormParam;
 import javax.ws.rs.GET;
@@ -56,6 +57,9 @@ import org.apache.commons.validator.routines.UrlValidator;
 import org.apache.cxf.common.logging.LogUtils;
 import org.apache.cxf.common.util.Base64UrlUtility;
 import org.apache.cxf.common.util.StringUtils;
+import org.apache.cxf.fediz.service.oidc.CSRFUtils;
+import org.apache.cxf.message.Message;
+import org.apache.cxf.phase.PhaseInterceptorChain;
 import org.apache.cxf.rs.security.oauth2.common.Client;
 import org.apache.cxf.rs.security.oauth2.common.ServerAccessToken;
 import org.apache.cxf.rs.security.oauth2.common.UserSubject;
@@ -67,10 +71,11 @@ import org.apache.cxf.rs.security.oauth2.tokens.refresh.RefreshToken;
 import org.apache.cxf.rs.security.oauth2.utils.OAuthConstants;
 import org.apache.cxf.rs.security.oidc.idp.OidcUserSubject;
 import org.apache.cxf.rt.security.crypto.CryptoUtils;
+import org.apache.cxf.transport.http.AbstractHTTPDestination;
 
 @Path("/")
 public class ClientRegistrationService {
-    
+
     private static final Logger LOG = LogUtils.getL7dLogger(ClientRegistrationService.class);
 
     private Map<String, Collection<Client>> registrations = new HashMap<String,
Collection<Client>>();
@@ -119,8 +124,14 @@ public class ClientRegistrationService {
     @Consumes(MediaType.APPLICATION_FORM_URLENCODED)
     @Produces(MediaType.TEXT_HTML)
     @Path("/{id}/remove")
-    public RegisteredClients removeClient(@PathParam("id") String id) {
-        Collection<Client> clients = getClientRegistrations(); 
+    public RegisteredClients removeClient(@PathParam("id") String id,
+                                          @FormParam("client_csrfToken") String csrfToken)
{
+        // CSRF
+        if (!checkCSRFToken(csrfToken)) {
+            throw new InvalidRegistration("Invalid CSRF Token");
+        }
+
+        Collection<Client> clients = getClientRegistrations();
         for (Iterator<Client> it = clients.iterator(); it.hasNext();) {
             Client c = it.next();
             if (c.getClientId().equals(id)) {
@@ -139,7 +150,13 @@ public class ClientRegistrationService {
     @Consumes(MediaType.APPLICATION_FORM_URLENCODED)
     @Produces(MediaType.TEXT_HTML)
     @Path("/{id}/reset")
-    public Client resetClient(@PathParam("id") String id) {
+    public Client resetClient(@PathParam("id") String id,
+                              @FormParam("client_csrfToken") String csrfToken) {
+        // CSRF
+        if (!checkCSRFToken(csrfToken)) {
+            throw new InvalidRegistration("Invalid CSRF Token");
+        }
+
         Client c = getRegisteredClient(id);
         if (c.isConfidential()) {
             c.setClientSecret(generateClientSecret());
@@ -172,7 +189,13 @@ public class ClientRegistrationService {
     @Produces(MediaType.TEXT_HTML)
     @Path("/{id}/at/{tokenId}/revoke")
     public ClientTokens revokeClientAccessToken(@PathParam("id") String clientId,
-                                                      @PathParam("tokenId") String tokenId)
{
+                                                      @PathParam("tokenId") String tokenId,
+                                                      @FormParam("client_csrfToken") String
csrfToken) {
+        // CSRF
+        if (!checkCSRFToken(csrfToken)) {
+            throw new InvalidRegistration("Invalid CSRF Token");
+        }
+
         return doRevokeClientToken(clientId, tokenId, OAuthConstants.ACCESS_TOKEN);
     }
     
@@ -181,7 +204,13 @@ public class ClientRegistrationService {
     @Produces(MediaType.TEXT_HTML)
     @Path("/{id}/rt/{tokenId}/revoke")
     public ClientTokens revokeClientRefreshToken(@PathParam("id") String clientId,
-                                                      @PathParam("tokenId") String tokenId)
{
+                                                      @PathParam("tokenId") String tokenId,
+                                                      @FormParam("client_csrfToken") String
csrfToken) {
+        // CSRF
+        if (!checkCSRFToken(csrfToken)) {
+            throw new InvalidRegistration("Invalid CSRF Token");
+        }
+
         return doRevokeClientToken(clientId, tokenId, OAuthConstants.REFRESH_TOKEN);
     }
     
@@ -213,7 +242,13 @@ public class ClientRegistrationService {
     @Produces(MediaType.TEXT_HTML)
     @Path("/{id}/codes/{code}/revoke")
     public ClientCodeGrants revokeClientCodeGrant(@PathParam("id") String id,
-                                                  @PathParam("code") String code) {
+                                                  @PathParam("code") String code,
+                                                  @FormParam("client_csrfToken") String csrfToken)
{
+        // CSRF
+        if (!checkCSRFToken(csrfToken)) {
+            throw new InvalidRegistration("Invalid CSRF Token");
+        }
+
         if (dataProvider instanceof AuthorizationCodeDataProvider) {
             ((AuthorizationCodeDataProvider)dataProvider).removeCodeGrant(code);
             return getClientCodeGrants(id);
@@ -226,22 +261,27 @@ public class ClientRegistrationService {
     @Produces(MediaType.TEXT_HTML)
     @Path("/")
     public Response registerForm(@FormParam("client_name") String appName,
-                                           @FormParam("client_type") String appType, 
-                                           @FormParam("client_audience") String audience,
-                                           @FormParam("client_redirectURI") String redirectURI,
-                                           @FormParam("client_homeRealm") String homeRealm
+                                 @FormParam("client_type") String appType,
+                                 @FormParam("client_audience") String audience,
+                                 @FormParam("client_redirectURI") String redirectURI,
+                                 @FormParam("client_homeRealm") String homeRealm,
+                                 @FormParam("client_csrfToken") String csrfToken
     ) {
-        
+        // CSRF
+        if (!checkCSRFToken(csrfToken)) {
+            return invalidRegistrationException("Invalid CSRF Token");
+        }
+
         // Client Name
         if (StringUtils.isEmpty(appName)) {
-            return invalidRegistrationResponse("The client name must not be empty");
+            return invalidRegistrationException("The client name must not be empty");
         }
         // Client Type
         if (StringUtils.isEmpty(appType)) {
-            return invalidRegistrationResponse("The client type must not be empty");
+            return invalidRegistrationException("The client type must not be empty");
         }
         if (!("confidential".equals(appType) || "public".equals(appType))) {
-            return invalidRegistrationResponse("An invalid client type was specified: " +
appType);
+            return invalidRegistrationException("An invalid client type was specified: "
+ appType);
         }
         // Client ID
         String clientId = generateClientId();
@@ -252,7 +292,7 @@ public class ClientRegistrationService {
             : null;
 
         Client newClient = new Client(clientId, clientSecret, isConfidential, appName);
-        
+
         // User who registered this client
         String userName = sc.getUserPrincipal().getName();
         UserSubject userSubject = new OidcUserSubject(userName);
@@ -260,6 +300,17 @@ public class ClientRegistrationService {
 
         // Client Registration Time
         newClient.setRegisteredAt(System.currentTimeMillis() / 1000);
+
+        // Client Realm
+        if (homeRealm != null) {
+            newClient.setHomeRealm(homeRealm);
+            if (homeRealms.containsKey(homeRealm)) {
+                newClient.getProperties().put("homeRealmAlias", homeRealms.get(homeRealm));
+            }
+        }
+
+        // Client Registration Time
+        newClient.setRegisteredAt(System.currentTimeMillis() / 1000);
         
         // Client Realm
         if (homeRealm != null) {
@@ -311,8 +362,21 @@ public class ClientRegistrationService {
         return Response.ok(new InvalidRegistration(error)).build();
     }
 
+    private boolean checkCSRFToken(String csrfToken) {
+        // CSRF
+        Message message = PhaseInterceptorChain.getCurrentMessage();
+        HttpServletRequest httpRequest = (HttpServletRequest) message.get(AbstractHTTPDestination.HTTP_REQUEST);
+        String savedToken = CSRFUtils.getCSRFToken(httpRequest, false);
+        if (StringUtils.isEmpty(csrfToken) || StringUtils.isEmpty(savedToken)
+            || !savedToken.equals(csrfToken)) {
+            return false;
+        }
+
+        return true;
+    }
+
     private boolean isValidURI(String uri, boolean requireHttps) {
-        
+
         UrlValidator urlValidator = null;
         
         if (requireHttps) {

http://git-wip-us.apache.org/repos/asf/cxf-fediz/blob/71480c3f/services/oidc/src/main/webapp/WEB-INF/views/client.jsp
----------------------------------------------------------------------
diff --git a/services/oidc/src/main/webapp/WEB-INF/views/client.jsp b/services/oidc/src/main/webapp/WEB-INF/views/client.jsp
index 6bdd74d..0c00395 100644
--- a/services/oidc/src/main/webapp/WEB-INF/views/client.jsp
+++ b/services/oidc/src/main/webapp/WEB-INF/views/client.jsp
@@ -4,6 +4,7 @@
 <%@ page import="java.util.Locale"%>
 <%@ page import="java.util.TimeZone"%>
 <%@ page import="javax.servlet.http.HttpServletRequest" %>
+<%@ page import="org.apache.cxf.fediz.service.oidc.CSRFUtils" %>
 <%@ page import="org.owasp.esapi.ESAPI" %>
 
 <%
@@ -16,7 +17,10 @@
     String basePath = request.getContextPath() + request.getServletPath();
     if (!basePath.endsWith("/")) {
         basePath += "/";
-    } 
+    }
+    
+    // Get or generate the CSRF token
+    String token = CSRFUtils.getCSRFToken(request, true);
 %>
 <html xmlns="http://www.w3.org/1999/xhtml">
 <head>
@@ -168,6 +172,9 @@
 %>
 <td class="td_no_border">
 <form name="resetSecretForm" action="<%=basePath%>console/clients/<%= client.getClientId()
+ "/reset"%>" method="POST">
+    <div class="form-line">
+        <input type="hidden" value="<%=token%>" name="client_csrfToken" />
+    </div>
      <div data-type="control_button" class="form-line">
 	<button name="submit_reset_button" class="form-submit-button" type="submit">Reset
Client Secret</button>
 </form>
@@ -178,6 +185,9 @@
 %>
 <td class="td_no_border">
 <form name="deleteForm" action="<%=basePath%>console/clients/<%= client.getClientId()
+ "/remove"%>" method="POST">
+    <div class="form-line">
+        <input type="hidden" value="<%=token%>" name="client_csrfToken" />
+    </div>
         <div data-type="control_button" class="form-line">
 	<button name="submit_delete_button" class="form-submit-button" type="submit">Delete
Client</button>
         </div>

http://git-wip-us.apache.org/repos/asf/cxf-fediz/blob/71480c3f/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 44a4646..8254c50 100644
--- a/services/oidc/src/main/webapp/WEB-INF/views/clientCodeGrants.jsp
+++ b/services/oidc/src/main/webapp/WEB-INF/views/clientCodeGrants.jsp
@@ -6,6 +6,7 @@
 <%@ page import="java.util.Locale"%>
 <%@ page import="java.util.TimeZone"%>
 <%@ page import="javax.servlet.http.HttpServletRequest" %>
+<%@ page import="org.apache.cxf.fediz.service.oidc.CSRFUtils" %>
 <%@ page import="org.apache.cxf.fediz.service.oidc.clients.ClientCodeGrants" %>
 <%@ page import="org.owasp.esapi.ESAPI" %>
 
@@ -15,7 +16,10 @@
     String basePath = request.getContextPath() + request.getServletPath();
     if (!basePath.endsWith("/")) {
         basePath += "/";
-    } 
+    }
+    
+    // Get or generate the CSRF token
+    String csrfToken = CSRFUtils.getCSRFToken(request, true);
 %>
 <html xmlns="http://www.w3.org/1999/xhtml">
 <head>
@@ -76,6 +80,7 @@
 		   %>
            <td>
                <form action="<%=basePath%>console/clients/<%= client.getClientId()
+ "/codes/" + token.getCode() + "/revoke"%>" method="POST">
+                 <input type="hidden" value="<%=csrfToken%>" name="client_csrfToken"
/>
 		         <input type="submit" value="Delete"/>
                </form>
            </td>

http://git-wip-us.apache.org/repos/asf/cxf-fediz/blob/71480c3f/services/oidc/src/main/webapp/WEB-INF/views/clientTokens.jsp
----------------------------------------------------------------------
diff --git a/services/oidc/src/main/webapp/WEB-INF/views/clientTokens.jsp b/services/oidc/src/main/webapp/WEB-INF/views/clientTokens.jsp
index b341739..ed96511 100644
--- a/services/oidc/src/main/webapp/WEB-INF/views/clientTokens.jsp
+++ b/services/oidc/src/main/webapp/WEB-INF/views/clientTokens.jsp
@@ -7,6 +7,7 @@
 <%@ page import="java.util.Locale"%>
 <%@ page import="java.util.TimeZone"%>
 <%@ page import="javax.servlet.http.HttpServletRequest" %>
+<%@ page import="org.apache.cxf.fediz.service.oidc.CSRFUtils" %>
 <%@ page import="org.apache.cxf.fediz.service.oidc.clients.ClientTokens" %>
 <%@ page import="org.owasp.esapi.ESAPI" %>
 
@@ -19,7 +20,9 @@
     } 
     SimpleDateFormat dateFormat = new SimpleDateFormat("EEE, dd MMM yyyy HH:mm:ss", Locale.US);
     dateFormat.setTimeZone(TimeZone.getTimeZone("GMT"));
-
+    
+    // Get or generate the CSRF token
+    String csrfToken = CSRFUtils.getCSRFToken(request, true);
 %>
 <html xmlns="http://www.w3.org/1999/xhtml">
 <head>
@@ -111,6 +114,7 @@
 	       %>
            <td>
                <form action="<%=basePath%>console/clients/<%= client.getClientId()
+ "/at/" + token.getTokenKey() + "/revoke"%>" method="POST">
+                   <input type="hidden" value="<%=csrfToken%>" name="client_csrfToken"
/>
 		           <input type="submit" value="Delete"/>  
                </form>
            </td>
@@ -170,6 +174,7 @@
 	       
            <td>
                <form action="<%=basePath%>console/clients/<%= client.getClientId()
+ "/rt/" + token.getTokenKey() + "/revoke"%>" method="POST">
+                 <input type="hidden" value="<%=csrfToken%>" name="client_csrfToken"
/>
 		         <input type="submit" value="Delete"/>
                </form>
            </td>

http://git-wip-us.apache.org/repos/asf/cxf-fediz/blob/71480c3f/services/oidc/src/main/webapp/WEB-INF/views/registerClient.jsp
----------------------------------------------------------------------
diff --git a/services/oidc/src/main/webapp/WEB-INF/views/registerClient.jsp b/services/oidc/src/main/webapp/WEB-INF/views/registerClient.jsp
index eb51409..a5ee829 100644
--- a/services/oidc/src/main/webapp/WEB-INF/views/registerClient.jsp
+++ b/services/oidc/src/main/webapp/WEB-INF/views/registerClient.jsp
@@ -1,11 +1,16 @@
 <%@ page
-	import="javax.servlet.http.HttpServletRequest,java.util.Map,java.util.Iterator,org.apache.cxf.fediz.service.oidc.clients.RegisterClient"%>
+	import="javax.servlet.http.HttpServletRequest,java.util.Map,java.util.Iterator,org.apache.cxf.fediz.service.oidc.clients.RegisterClient,
+	org.apache.cxf.fediz.service.oidc.CSRFUtils"
+%>
 <%
     RegisterClient reg = (RegisterClient)request.getAttribute("data");
     String basePath = request.getContextPath() + request.getServletPath();
     if (!basePath.endsWith("/")) {
         basePath += "/";
     }
+
+    // Get or generate the CSRF token
+    String csrfToken = CSRFUtils.getCSRFToken(request, true);
 %>
 <html xmlns="http://www.w3.org/1999/xhtml">
 <head>
@@ -107,6 +112,9 @@ input, select, button {
 					%>
 				</select>
 			</div>
+			<div class="form-line">
+				<input type="hidden" value="<%=csrfToken%>" name="client_csrfToken" />
+			</div>
 			<div data-type="control_button" class="form-line">
 				<button name="submit_button" class="form-submit-button" type="submit">Register
API Client</button>
 			</div>

http://git-wip-us.apache.org/repos/asf/cxf-fediz/blob/71480c3f/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 5266473..ffd4a77 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
@@ -714,6 +714,38 @@ public class OIDCTest {
         webClient.close();
     }
 
+    // Test that the form has the correct CSRF token in it when creating a client
+    @org.junit.Test
+    public void testCSRFClientRegistration() throws Exception {
+        String url = "https://localhost:" + getRpHttpsPort() + "/fediz-oidc/console/clients";
+        String user = "alice";
+        String password = "ecila";
+
+        // Login to the client page successfully
+        WebClient webClient = setupWebClient(user, password, getIdpHttpsPort());
+        HtmlPage loginPage = login(url, webClient);
+        final String bodyTextContent = loginPage.getBody().getTextContent();
+        Assert.assertTrue(bodyTextContent.contains("Registered Clients"));
+
+        // Register a new client
+
+        WebRequest request = new WebRequest(new URL(url), HttpMethod.POST);
+        request.setRequestParameters(new ArrayList<NameValuePair>());
+
+        request.getRequestParameters().add(new NameValuePair("client_name", "bad_client"));
+        request.getRequestParameters().add(new NameValuePair("client_type", "confidential"));
+        request.getRequestParameters().add(new NameValuePair("client_redirectURI", "https://127.0.0.1"));
+        request.getRequestParameters().add(new NameValuePair("client_audience", ""));
+        request.getRequestParameters().add(new NameValuePair("client_logoutURI", ""));
+        request.getRequestParameters().add(new NameValuePair("client_homeRealm", ""));
+        request.getRequestParameters().add(new NameValuePair("client_csrfToken", "12345"));
+
+        HtmlPage registeredClientPage = webClient.getPage(request);
+        Assert.assertTrue(registeredClientPage.asXml().contains("Invalid CSRF Token"));
+
+        webClient.close();
+    }
+
     private static WebClient setupWebClient(String user, String password, String idpPort)
{
         final WebClient webClient = new WebClient();
         webClient.getOptions().setUseInsecureSSL(true);


Mime
View raw message