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 <[email protected]> Authored: Tue Apr 18 17:22:27 2017 +0100 Committer: Colm O hEigeartaigh <[email protected]> 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);
