Repository: cxf-fediz Updated Branches: refs/heads/master 02df115e9 -> c68e48208
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/c68e4820 Tree: http://git-wip-us.apache.org/repos/asf/cxf-fediz/tree/c68e4820 Diff: http://git-wip-us.apache.org/repos/asf/cxf-fediz/diff/c68e4820 Branch: refs/heads/master Commit: c68e4820816c19241568f4a8fe8600bffb0243cd Parents: 02df115 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 17:22:27 2017 +0100 ---------------------------------------------------------------------- .../cxf/fediz/service/oidc/CSRFUtils.java | 52 ++++++++++++++++++ .../oidc/clients/ClientRegistrationService.java | 55 +++++++++++++++++--- .../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, 163 insertions(+), 12 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cxf-fediz/blob/c68e4820/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/c68e4820/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 b89f862..d00b9cd 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<>(); @@ -119,7 +124,11 @@ public class ClientRegistrationService { @Consumes(MediaType.APPLICATION_FORM_URLENCODED) @Produces(MediaType.TEXT_HTML) @Path("/{id}/remove") - public RegisteredClients removeClient(@PathParam("id") String id) { + public RegisteredClients removeClient(@PathParam("id") String id, + @FormParam("client_csrfToken") String csrfToken) { + // CSRF + checkCSRFToken(csrfToken); + Collection<Client> clients = getClientRegistrations(); for (Iterator<Client> it = clients.iterator(); it.hasNext();) { Client c = it.next(); @@ -139,7 +148,11 @@ 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 + checkCSRFToken(csrfToken); + Client c = getRegisteredClient(id); if (c.isConfidential()) { c.setClientSecret(generateClientSecret()); @@ -172,7 +185,11 @@ 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 + checkCSRFToken(csrfToken); + return doRevokeClientToken(clientId, tokenId, OAuthConstants.ACCESS_TOKEN); } @@ -181,7 +198,11 @@ 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 + checkCSRFToken(csrfToken); + return doRevokeClientToken(clientId, tokenId, OAuthConstants.REFRESH_TOKEN); } @@ -213,7 +234,11 @@ 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 + checkCSRFToken(csrfToken); + if (dataProvider instanceof AuthorizationCodeDataProvider) { ((AuthorizationCodeDataProvider)dataProvider).removeCodeGrant(code); return getClientCodeGrants(id); @@ -230,9 +255,13 @@ public class ClientRegistrationService { @FormParam("client_audience") String audience, @FormParam("client_redirectURI") String redirectURI, @FormParam("client_logoutURI") String logoutURI, - @FormParam("client_homeRealm") String homeRealm + @FormParam("client_homeRealm") String homeRealm, + @FormParam("client_csrfToken") String csrfToken ) { try { + // CSRF + checkCSRFToken(csrfToken); + // Client Name if (StringUtils.isEmpty(appName)) { throwInvalidRegistrationException("The client name must not be empty"); @@ -322,13 +351,23 @@ public class ClientRegistrationService { } } + private void 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)) { + throwInvalidRegistrationException("Invalid CSRF Token"); + } + } private void throwInvalidRegistrationException(String error) { throw new InvalidRegistrationException(error); } private boolean isValidURI(String uri, boolean requireHttps) { - + UrlValidator urlValidator = null; if (requireHttps) { http://git-wip-us.apache.org/repos/asf/cxf-fediz/blob/c68e4820/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 597bd8a..0c43fff 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> @@ -178,6 +182,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> @@ -188,6 +195,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/c68e4820/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/c68e4820/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/c68e4820/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 47796c3..ffe0541 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> @@ -113,6 +118,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/c68e4820/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 df04250..216e1ec 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 @@ -794,6 +794,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);
