From ce0db93d4feaf9ea11981862431d51090990e9c9 Mon Sep 17 00:00:00 2001 From: Alex Heneveld Date: Wed, 9 Nov 2016 12:03:49 +0000 Subject: [PATCH 1/5] REST API supports client requiring a CSRF header, and requesting such a header, and if required POST requests fail if it wasn't supplied --- .../brooklyn/launcher/BrooklynWebServer.java | 4 +- .../brooklyn/rest/filter/CsrfTokenFilter.java | 127 ++++++++++++++++++ .../jaas/SecurityProviderHttpSession.java | 1 + .../resources/OSGI-INF/blueprint/service.xml | 1 + .../rest/filter/CsrfTokenFilterTest.java | 110 +++++++++++++++ .../testing/BrooklynRestResourceTest.java | 1 + .../src/main/webapp/WEB-INF/web.xml | 1 + .../rest/BrooklynRestApiLauncher.java | 4 +- .../rest/CsrfTokenFilterLauncherTest.java | 107 +++++++++++++++ 9 files changed, 354 insertions(+), 2 deletions(-) create mode 100644 rest/rest-resources/src/main/java/org/apache/brooklyn/rest/filter/CsrfTokenFilter.java create mode 100644 rest/rest-resources/src/test/java/org/apache/brooklyn/rest/filter/CsrfTokenFilterTest.java create mode 100644 rest/rest-server/src/test/java/org/apache/brooklyn/rest/CsrfTokenFilterLauncherTest.java diff --git a/launcher/src/main/java/org/apache/brooklyn/launcher/BrooklynWebServer.java b/launcher/src/main/java/org/apache/brooklyn/launcher/BrooklynWebServer.java index 43854e1b1a..933ce68dc5 100644 --- a/launcher/src/main/java/org/apache/brooklyn/launcher/BrooklynWebServer.java +++ b/launcher/src/main/java/org/apache/brooklyn/launcher/BrooklynWebServer.java @@ -50,6 +50,7 @@ import org.apache.brooklyn.location.localhost.LocalhostMachineProvisioningLocation; import org.apache.brooklyn.rest.BrooklynWebConfig; import org.apache.brooklyn.rest.RestApiSetup; +import org.apache.brooklyn.rest.filter.CsrfTokenFilter; import org.apache.brooklyn.rest.filter.EntitlementContextFilter; import org.apache.brooklyn.rest.filter.HaHotCheckResourceFilter; import org.apache.brooklyn.rest.filter.LoggingFilter; @@ -460,7 +461,8 @@ private WebAppContext deployRestApi(WebAppContext context) { new RequestTaggingRsFilter(), new NoCacheFilter(), new HaHotCheckResourceFilter(), - new EntitlementContextFilter()); + new EntitlementContextFilter(), + new CsrfTokenFilter()); RestApiSetup.installServletFilters(context, RequestTaggingFilter.class, LoggingFilter.class); diff --git a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/filter/CsrfTokenFilter.java b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/filter/CsrfTokenFilter.java new file mode 100644 index 0000000000..aa7e62aac8 --- /dev/null +++ b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/filter/CsrfTokenFilter.java @@ -0,0 +1,127 @@ +/* + * 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.brooklyn.rest.filter; + +import java.io.IOException; + +import javax.annotation.Priority; +import javax.servlet.http.HttpServletRequest; +import javax.ws.rs.HttpMethod; +import javax.ws.rs.container.ContainerRequestContext; +import javax.ws.rs.container.ContainerRequestFilter; +import javax.ws.rs.container.ContainerResponseContext; +import javax.ws.rs.container.ContainerResponseFilter; +import javax.ws.rs.core.Context; +import javax.ws.rs.core.Response; +import javax.ws.rs.ext.Provider; + +import org.apache.brooklyn.rest.domain.ApiError; +import org.apache.brooklyn.util.text.Identifiers; + +/** + * If client specifies {@link #REQUEST_CSRF_TOKEN_HEADER} + * (as any of the values {@link #REQUEST_CSRF_TOKEN_HEADER_LOGIN}, {@link #REQUEST_CSRF_TOKEN_HEADER_NEW}, {@link #REQUEST_CSRF_TOKEN_HEADER_TRUE}) + * then the server will return a {@link #REQUIRED_CSRF_TOKEN_HEADER} header containing a token. + *

+ * The server will require that header with the given token on subsequent POST requests + * (and other actions -- excluding GET which is always permitted; + * future enhancement may allow client to control whether GET is allowed or not). + */ +@Provider +@Priority(400) +public class CsrfTokenFilter implements ContainerRequestFilter, ContainerResponseFilter { + + public static final String REQUIRED_CSRF_TOKEN_ATTR = CsrfTokenFilter.class.getName()+"."+"REQUIRED_CSRF_TOKEN"; + + public static final String REQUIRED_CSRF_TOKEN_HEADER = CsrfTokenFilter.class.getName()+"."+"X-CsrfToken"; + public static final String REQUEST_CSRF_TOKEN_HEADER = CsrfTokenFilter.class.getName()+"."+"X-CsrfToken-Request"; + /** means to return the token */ + public static final String REQUEST_CSRF_TOKEN_HEADER_TRUE = "true"; + /** means to create a new token */ + public static final String REQUEST_CSRF_TOKEN_HEADER_NEW = "new"; + /** means to create and return a token on login only (ie if one doesn't exist for the session) */ + public static final String REQUEST_CSRF_TOKEN_HEADER_LOGIN = "login"; + + @Context + private HttpServletRequest request; + + @Override + public void filter(ContainerRequestContext requestContext) throws IOException { + Object requiredToken = request.getSession().getAttribute(REQUIRED_CSRF_TOKEN_ATTR); + if (requiredToken!=null) { + String suppliedToken = request.getHeader(REQUIRED_CSRF_TOKEN_HEADER); + if (suppliedToken==null) { + if (HttpMethod.GET.equals(requestContext.getMethod())) { + // GETs are permitted with no token (but an invalid token will be caught) + return; + } + + fail(requestContext, ApiError.builder().errorCode(Response.Status.UNAUTHORIZED) + .message(REQUIRED_CSRF_TOKEN_HEADER+" header required containing token previously supplied") + .build()); + } else if (suppliedToken.equals(requiredToken)) { + // approved + } else { + fail(requestContext, ApiError.builder().errorCode(Response.Status.UNAUTHORIZED) + .message(REQUIRED_CSRF_TOKEN_HEADER+" header did not match current token") + .build()); + } + } + } + + private void fail(ContainerRequestContext requestContext, ApiError apiError) { + requestContext.abortWith(apiError.asJsonResponse()); + } + + @Override + public void filter(ContainerRequestContext requestContext, ContainerResponseContext responseContext) throws IOException { + String requestHeader = request.getHeader(REQUEST_CSRF_TOKEN_HEADER); + if (requestHeader==null) return; + String oldToken = (String) request.getSession().getAttribute(REQUIRED_CSRF_TOKEN_ATTR); + String tokenToReturn = null; + boolean createToken = false; + + if (REQUEST_CSRF_TOKEN_HEADER_LOGIN.equals(requestHeader)) { + if (oldToken==null) createToken = true; + else return; + } else if (REQUEST_CSRF_TOKEN_HEADER_TRUE.equals(requestHeader)) { + tokenToReturn = oldToken; + if (tokenToReturn==null) createToken = true; + } else if (REQUEST_CSRF_TOKEN_HEADER_NEW.equals(requestHeader)) { + createToken = true; + } + + if (createToken) { + tokenToReturn = Identifiers.makeRandomId(16); + request.getSession().setAttribute(REQUIRED_CSRF_TOKEN_ATTR, tokenToReturn); + } + + if (tokenToReturn==null) { + fail(requestContext, ApiError.builder().errorCode(Response.Status.UNAUTHORIZED) + .message(REQUEST_CSRF_TOKEN_HEADER+" contained invalid value; expected: "+ + REQUEST_CSRF_TOKEN_HEADER_TRUE+" | "+ + REQUEST_CSRF_TOKEN_HEADER_LOGIN+" | "+ + REQUEST_CSRF_TOKEN_HEADER_NEW) + .build()); + } else { + responseContext.getHeaders().add(REQUIRED_CSRF_TOKEN_HEADER, tokenToReturn); + } + } + +} diff --git a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/security/jaas/SecurityProviderHttpSession.java b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/security/jaas/SecurityProviderHttpSession.java index 52243192ec..75a22670ca 100644 --- a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/security/jaas/SecurityProviderHttpSession.java +++ b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/security/jaas/SecurityProviderHttpSession.java @@ -29,6 +29,7 @@ import org.apache.brooklyn.util.text.Identifiers; +/** mock session, used only for performing authentication */ public class SecurityProviderHttpSession implements HttpSession { String id = Identifiers.makeRandomId(5); Map attributes = new ConcurrentHashMap<>(); diff --git a/rest/rest-resources/src/main/resources/OSGI-INF/blueprint/service.xml b/rest/rest-resources/src/main/resources/OSGI-INF/blueprint/service.xml index 4137f1f0ca..c53c623f68 100644 --- a/rest/rest-resources/src/main/resources/OSGI-INF/blueprint/service.xml +++ b/rest/rest-resources/src/main/resources/OSGI-INF/blueprint/service.xml @@ -101,6 +101,7 @@ limitations under the License. TODO ShutdownHandlerProvider, sync with init work. Needs to be custom OSGi implementation? --> + diff --git a/rest/rest-resources/src/test/java/org/apache/brooklyn/rest/filter/CsrfTokenFilterTest.java b/rest/rest-resources/src/test/java/org/apache/brooklyn/rest/filter/CsrfTokenFilterTest.java new file mode 100644 index 0000000000..70920e039c --- /dev/null +++ b/rest/rest-resources/src/test/java/org/apache/brooklyn/rest/filter/CsrfTokenFilterTest.java @@ -0,0 +1,110 @@ +/* + * 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.brooklyn.rest.filter; + +import static org.testng.Assert.assertEquals; + +import javax.ws.rs.GET; +import javax.ws.rs.POST; +import javax.ws.rs.Path; +import javax.ws.rs.core.MediaType; +import javax.ws.rs.core.Response; + +import org.apache.brooklyn.rest.testing.BrooklynRestResourceTest; +import org.apache.cxf.jaxrs.JAXRSServerFactoryBean; +import org.apache.cxf.jaxrs.client.WebClient; +import org.apache.http.HttpStatus; + +/** TODO test fails using BRRT because we have no session; see CsrfTokenFilterLauncherTest in different project, + * and comments in here; would prefer to have this test, so keep if we can enable sessions here */ +public class CsrfTokenFilterTest extends BrooklynRestResourceTest { + + public static class SampleActionResource { + @GET + @Path("/test-get") + public String testGet() { + return "got"; + } + @POST + @Path("/test-post") + public String testPost() { + return "posted"; + } + } + + @Override + protected void addBrooklynResources() { + addResource(new CsrfTokenFilter()); + addResource(new SampleActionResource()); + } + + @Override + protected void configureCXF(JAXRSServerFactoryBean sf) { + super.configureCXF(sf); + + /* + TODO how can we turn on sessions?? + + // normal handler way + val webapp = new ServletContextHandler(ServletContextHandler.SESSIONS) + SessionHandler sh = new SessionHandler(); + webapp.setSessionHandler() + + try { + // suggested bean way - but only for new servers + sf.getBus().getExtension(JettyHTTPServerEngineFactory.class) + .createJettyHTTPServerEngine(8000, "http") + .setSessionSupport(true); + } catch (Exception e) { + throw Exceptions.propagate(e); + } + */ + } + +// @Test +// public void testRequestToken() { +// client().path("/logout").post(null); +// Response response = request("/logout").header( +//// Response response = request("/test-get").header( +// CsrfTokenFilter.REQUEST_CSRF_TOKEN_HEADER, CsrfTokenFilter.REQUEST_CSRF_TOKEN_HEADER_TRUE).get(); +// +// // comes back okay +// assertOkayResponse(response, "got"); +// +// String token = response.getHeaderString(CsrfTokenFilter.REQUIRED_CSRF_TOKEN_HEADER); +// Assert.assertNotNull(token); +// +// // can get subsequently +// response = request("/test-get").get(); +// assertOkayResponse(response, "got"); +// } + + protected void assertOkayResponse(Response response, String expecting) { + assertEquals(response.getStatus(), HttpStatus.SC_OK); + String content = (String) response.readEntity(String.class); + assertEquals(content, expecting); + } + + protected WebClient request(String path) { + return WebClient.create(getEndpointAddress(), clientProviders, "admin", "admin", null) + .path(path) + .accept(MediaType.APPLICATION_JSON_TYPE); + } + +} diff --git a/rest/rest-resources/src/test/java/org/apache/brooklyn/rest/testing/BrooklynRestResourceTest.java b/rest/rest-resources/src/test/java/org/apache/brooklyn/rest/testing/BrooklynRestResourceTest.java index 505bb58738..069b454028 100644 --- a/rest/rest-resources/src/test/java/org/apache/brooklyn/rest/testing/BrooklynRestResourceTest.java +++ b/rest/rest-resources/src/test/java/org/apache/brooklyn/rest/testing/BrooklynRestResourceTest.java @@ -92,6 +92,7 @@ protected synchronized void startServer() throws Exception { clientProviders = sf.getProviders(); } configureCXF(sf); + sf.setAddress(getEndpointAddress()); sf.setFeatures(ImmutableList.of(new org.apache.cxf.feature.LoggingFeature())); server = sf.create(); diff --git a/rest/rest-server/src/main/webapp/WEB-INF/web.xml b/rest/rest-server/src/main/webapp/WEB-INF/web.xml index e53649e11a..6aac098683 100644 --- a/rest/rest-server/src/main/webapp/WEB-INF/web.xml +++ b/rest/rest-server/src/main/webapp/WEB-INF/web.xml @@ -75,6 +75,7 @@ org.apache.brooklyn.rest.filter.NoCacheFilter, org.apache.brooklyn.rest.filter.HaHotCheckResourceFilter, org.apache.brooklyn.rest.filter.EntitlementContextFilter, + org.apache.brooklyn.rest.filter.CsrfTokenFilter, org.apache.brooklyn.rest.util.ManagementContextProvider diff --git a/rest/rest-server/src/test/java/org/apache/brooklyn/rest/BrooklynRestApiLauncher.java b/rest/rest-server/src/test/java/org/apache/brooklyn/rest/BrooklynRestApiLauncher.java index 4dc3d662f2..ffb5db4a1d 100644 --- a/rest/rest-server/src/test/java/org/apache/brooklyn/rest/BrooklynRestApiLauncher.java +++ b/rest/rest-server/src/test/java/org/apache/brooklyn/rest/BrooklynRestApiLauncher.java @@ -36,6 +36,7 @@ import org.apache.brooklyn.core.mgmt.internal.ManagementContextInternal; import org.apache.brooklyn.core.server.BrooklynServerConfig; import org.apache.brooklyn.core.server.BrooklynServiceAttributes; +import org.apache.brooklyn.rest.filter.CsrfTokenFilter; import org.apache.brooklyn.rest.filter.EntitlementContextFilter; import org.apache.brooklyn.rest.filter.HaHotCheckResourceFilter; import org.apache.brooklyn.rest.filter.LoggingFilter; @@ -227,7 +228,8 @@ private WebAppContext servletContextHandler(ManagementContext managementContext) new RequestTaggingRsFilter(), new NoCacheFilter(), new HaHotCheckResourceFilter(), - new EntitlementContextFilter()); + new EntitlementContextFilter(), + new CsrfTokenFilter()); RestApiSetup.installServletFilters(context, this.filters); context.setContextPath("/"); diff --git a/rest/rest-server/src/test/java/org/apache/brooklyn/rest/CsrfTokenFilterLauncherTest.java b/rest/rest-server/src/test/java/org/apache/brooklyn/rest/CsrfTokenFilterLauncherTest.java new file mode 100644 index 0000000000..f9f2ec22e0 --- /dev/null +++ b/rest/rest-server/src/test/java/org/apache/brooklyn/rest/CsrfTokenFilterLauncherTest.java @@ -0,0 +1,107 @@ +package org.apache.brooklyn.rest; +/* + * 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. + */ + + +import static org.testng.Assert.assertEquals; + +import java.net.URI; +import java.util.List; + +import javax.ws.rs.core.HttpHeaders; + +import org.apache.brooklyn.rest.filter.CsrfTokenFilter; +import org.apache.brooklyn.rest.filter.CsrfTokenFilterTest; +import org.apache.brooklyn.util.http.HttpTool; +import org.apache.brooklyn.util.http.HttpToolResponse; +import org.apache.http.HttpStatus; +import org.apache.http.client.HttpClient; +import org.testng.Assert; +import org.testng.annotations.Test; + +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.Iterables; + +/** TODO would prefer to run without launcher, as in {@link CsrfTokenFilterTest}; see comments there + * (but tests more fleshed out here) */ +public class CsrfTokenFilterLauncherTest extends BrooklynRestApiLauncherTestFixture { + + @Test + public void testRequestToken() { + useServerForTest(BrooklynRestApiLauncher.launcher() + .withoutJsgui() + .start()); + + HttpToolResponse response = HttpTool.httpGet( + client(), URI.create(getBaseUriRest() + "server/status"), + ImmutableMap.of( + CsrfTokenFilter.REQUEST_CSRF_TOKEN_HEADER, CsrfTokenFilter.REQUEST_CSRF_TOKEN_HEADER_TRUE )); + + // comes back okay + assertOkayResponse(response, "MASTER"); + + System.out.println(response.getHeaderLists()); + List tokens = response.getHeaderLists().get(CsrfTokenFilter.REQUIRED_CSRF_TOKEN_HEADER); + String token = Iterables.getOnlyElement(tokens); + Assert.assertNotNull(token); + + List cookies = response.getHeaderLists().get(HttpHeaders.SET_COOKIE); + String cookie = Iterables.getOnlyElement(cookies); + Assert.assertNotNull(cookie); + + // can post subsequently with token + response = HttpTool.httpPost( + client(), URI.create(getBaseUriRest() + "script/groovy"), + ImmutableMap.of( + HttpHeaders.CONTENT_TYPE, "application/text", + HttpHeaders.COOKIE, cookie, + CsrfTokenFilter.REQUIRED_CSRF_TOKEN_HEADER, token ), + "return 0;".getBytes()); + assertOkayResponse(response, "{\"result\":\"0\"}"); + + // but fails without token + response = HttpTool.httpPost( + client(), URI.create(getBaseUriRest() + "script/groovy"), + ImmutableMap.of( + HttpHeaders.COOKIE, cookie, + HttpHeaders.CONTENT_TYPE, "application/text" ), + "return 0;".getBytes()); + assertEquals(response.getResponseCode(), HttpStatus.SC_UNAUTHORIZED); + + // but you can get subsequently without token + response = HttpTool.httpGet( + client(), URI.create(getBaseUriRest() + "server/status"), + ImmutableMap.of( + HttpHeaders.COOKIE, cookie )); + + assertOkayResponse(response, "MASTER"); + } + + protected HttpClient client() { + return HttpTool.httpClientBuilder() + .uri(getBaseUriRest(server)) + .build(); + } + + protected void assertOkayResponse(HttpToolResponse response, String expecting) { + assertEquals(response.getResponseCode(), HttpStatus.SC_OK); + assertEquals(response.getContentAsString(), expecting); + } + +} From c8186f8df25a72caf885020b2536a94ae976df23 Mon Sep 17 00:00:00 2001 From: Alex Heneveld Date: Sun, 13 Nov 2016 02:45:58 +0000 Subject: [PATCH 2/5] switch CSRF to use cookies for tokens now supports AngularJS semantics. also now it doesn't needlessly create sessions. --- .../brooklyn/rest/filter/CsrfTokenFilter.java | 272 ++++++++++++++---- .../rest/CsrfTokenFilterLauncherTest.java | 54 ++-- .../brooklyn/util/http/HttpToolResponse.java | 26 ++ 3 files changed, 275 insertions(+), 77 deletions(-) diff --git a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/filter/CsrfTokenFilter.java b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/filter/CsrfTokenFilter.java index aa7e62aac8..67889bd5d0 100644 --- a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/filter/CsrfTokenFilter.java +++ b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/filter/CsrfTokenFilter.java @@ -19,69 +19,191 @@ package org.apache.brooklyn.rest.filter; import java.io.IOException; +import java.util.Arrays; +import java.util.List; import javax.annotation.Priority; import javax.servlet.http.HttpServletRequest; -import javax.ws.rs.HttpMethod; +import javax.servlet.http.HttpSession; import javax.ws.rs.container.ContainerRequestContext; import javax.ws.rs.container.ContainerRequestFilter; import javax.ws.rs.container.ContainerResponseContext; import javax.ws.rs.container.ContainerResponseFilter; import javax.ws.rs.core.Context; +import javax.ws.rs.core.NewCookie; import javax.ws.rs.core.Response; import javax.ws.rs.ext.Provider; +import org.apache.brooklyn.core.mgmt.entitlement.Entitlements; +import org.apache.brooklyn.rest.api.ServerApi; import org.apache.brooklyn.rest.domain.ApiError; import org.apache.brooklyn.util.text.Identifiers; +import org.apache.brooklyn.util.text.Strings; +import org.apache.commons.collections.EnumerationUtils; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import com.google.common.base.Predicates; +import com.google.common.collect.Iterables; +import com.google.common.collect.Lists; /** - * If client specifies {@link #REQUEST_CSRF_TOKEN_HEADER} - * (as any of the values {@link #REQUEST_CSRF_TOKEN_HEADER_LOGIN}, {@link #REQUEST_CSRF_TOKEN_HEADER_NEW}, {@link #REQUEST_CSRF_TOKEN_HEADER_TRUE}) - * then the server will return a {@link #REQUIRED_CSRF_TOKEN_HEADER} header containing a token. + * Causes the server to return a "Csrf-Token" cookie in certain circumstances, + * and thereafter to require clients to send that token as an "X-Csrf-Token" header on certain requests. + *

+ * To enable REST requests to work from the br CLI and regular curl without CSRF, + * this CSRF protection only applies when a session is in effect. + * Sessions are only established by some REST requests: + * {@link ServerApi#getUpExtended()} is a good choice (/v1/server/up/extended). + * It can also be forced by using the {@link #CSRF_TOKEN_REQUIRED_HEADER} header on any REST request. + * Browser-based UI clients should use one of the above methods early in operation. + *

+ * The standard model is that the token is required for all write (ie non-GET) requests being made + * with a valid session cookie (ie the request is part of an ongoing session). + * In such cases, the client must send the X-Csrf-Token as a header. + * This prevents a third-party site from effecting a mutating cross-site request via the browser. + *

+ * For transitional reasons, the default behaviour in the current implementation is to warn (not fail) + * if no token is supplied in the above case. This will likely be changed to enforce the standard model + * in a subsequent version, but it avoids breaking backwards compatibility for any existing session-based clients. + *

+ * Clients can force different required behaviour (e.g. "fail") by including the + * {@link #CSRF_TOKEN_REQUIRED_HEADER} with one of the values in {@link CsrfTokenRequiredForRequests}, + * viz. to require the header on ALL requests, or on NONE, instead of just on WRITE requests (the default). + * If such a mode is explicitly specified it is enforced (instead of just displaying the transitional warning), + * so while transitioning to enforce CSRF this header should be supplied. + * The Brooklyn UI does this. + *

+ * This uses *two* names, Csrf-Token and Xsrf-Token. + * The former seems the usual convention, but Angular apps use the latter. + * This strategy should mean that Angular apps should get CSRF protection with no special configuration. + *

+ * The cookies are sent by the client on every request, by virtue of being cookies, + * although this is not necessary (and rather wasteful). A client may optimise by deleting the cookies + * and caching the information in another form. + * The cookie names do not have any salt/uid, so in dev on localhost you may get conflicts, e.g. between + * multiple Brooklyn instances or other apps that use the same token names. + * (In contrast the session ID token, usually JSESSIONID, has a BROOKLYN field at runtime to prevent + * such collisions.) *

- * The server will require that header with the given token on subsequent POST requests - * (and other actions -- excluding GET which is always permitted; - * future enhancement may allow client to control whether GET is allowed or not). + * Additional CSRF strategies might be worth considering, in addition to the one described above: + *

+ * More info at: https://www.owasp.org/index.php/Cross-Site_Request_Forgery_(CSRF)_Prevention_Cheat_Sheet . + * (These have not been implemented because the cookie pattern is sufficient, and one of the strongest.) + * */ @Provider @Priority(400) public class CsrfTokenFilter implements ContainerRequestFilter, ContainerResponseFilter { - public static final String REQUIRED_CSRF_TOKEN_ATTR = CsrfTokenFilter.class.getName()+"."+"REQUIRED_CSRF_TOKEN"; + private static final Logger log = LoggerFactory.getLogger(CsrfTokenFilter.class); + + private static final String CSRF_TOKEN_VALUE_BASE = "Csrf-Token"; + public static final String CSRF_TOKEN_VALUE_COOKIE = CSRF_TOKEN_VALUE_BASE.toUpperCase(); + public static final String CSRF_TOKEN_VALUE_HEADER = HEADER_OF_COOKIE(CSRF_TOKEN_VALUE_BASE); + // also send this so angular works as expected + public static final String CSRF_TOKEN_VALUE_BASE_ANGULAR_NAME = "Xsrf-Token"; + public static final String CSRF_TOKEN_VALUE_COOKIE_ANGULAR_NAME = CSRF_TOKEN_VALUE_BASE_ANGULAR_NAME.toUpperCase(); + public static final String CSRF_TOKEN_VALUE_HEADER_ANGULAR_NAME = HEADER_OF_COOKIE(CSRF_TOKEN_VALUE_BASE_ANGULAR_NAME); + + public static final String CSRF_TOKEN_VALUE_ATTR = CsrfTokenFilter.class.getName()+"."+CSRF_TOKEN_VALUE_COOKIE; - public static final String REQUIRED_CSRF_TOKEN_HEADER = CsrfTokenFilter.class.getName()+"."+"X-CsrfToken"; - public static final String REQUEST_CSRF_TOKEN_HEADER = CsrfTokenFilter.class.getName()+"."+"X-CsrfToken-Request"; - /** means to return the token */ - public static final String REQUEST_CSRF_TOKEN_HEADER_TRUE = "true"; - /** means to create a new token */ - public static final String REQUEST_CSRF_TOKEN_HEADER_NEW = "new"; - /** means to create and return a token on login only (ie if one doesn't exist for the session) */ - public static final String REQUEST_CSRF_TOKEN_HEADER_LOGIN = "login"; + public static String HEADER_OF_COOKIE(String cookieName) { + return "X-"+cookieName; + } + public static final String CSRF_TOKEN_REQUIRED_HEADER = "X-Csrf-Token-Required-For-Requests"; + public static final String CSRF_TOKEN_REQUIRED_ATTR = CsrfTokenFilter.class.getName()+"."+CSRF_TOKEN_REQUIRED_HEADER; + public static enum CsrfTokenRequiredForRequests { ALL, WRITE, NONE, } + public static CsrfTokenRequiredForRequests DEFAULT_REQUIRED_FOR_REQUESTS = CsrfTokenRequiredForRequests.WRITE; + @Context private HttpServletRequest request; @Override public void filter(ContainerRequestContext requestContext) throws IOException { - Object requiredToken = request.getSession().getAttribute(REQUIRED_CSRF_TOKEN_ATTR); - if (requiredToken!=null) { - String suppliedToken = request.getHeader(REQUIRED_CSRF_TOKEN_HEADER); - if (suppliedToken==null) { - if (HttpMethod.GET.equals(requestContext.getMethod())) { - // GETs are permitted with no token (but an invalid token will be caught) - return; - } - - fail(requestContext, ApiError.builder().errorCode(Response.Status.UNAUTHORIZED) - .message(REQUIRED_CSRF_TOKEN_HEADER+" header required containing token previously supplied") - .build()); - } else if (suppliedToken.equals(requiredToken)) { - // approved + // if header supplied, check it is valid + String requiredWhenS = request.getHeader(CSRF_TOKEN_REQUIRED_HEADER); + if (Strings.isNonBlank(requiredWhenS) && getRequiredForRequests(requiredWhenS, null)==null) { + fail(requestContext, ApiError.builder().errorCode(Response.Status.BAD_REQUEST) + .message(HEADER_OF_COOKIE(CSRF_TOKEN_REQUIRED_HEADER)+" header if supplied must be one of " + + Arrays.asList(CsrfTokenRequiredForRequests.values())) + .build()); + return; + } + + if (!request.isRequestedSessionIdValid()) { + // no session; just return + return; + } + + @SuppressWarnings("unchecked") + List suppliedTokensDefault = (List) EnumerationUtils.toList(request.getHeaders(CSRF_TOKEN_VALUE_HEADER)); + @SuppressWarnings("unchecked") + List suppliedTokensAngular = (List) EnumerationUtils.toList(request.getHeaders(CSRF_TOKEN_VALUE_HEADER_ANGULAR_NAME)); + List suppliedTokens = Lists.newArrayList(suppliedTokensDefault); + suppliedTokens.addAll(suppliedTokensAngular); + + Object requiredToken = request.getSession().getAttribute(CSRF_TOKEN_VALUE_ATTR); + CsrfTokenRequiredForRequests whenRequired = (CsrfTokenRequiredForRequests) request.getSession().getAttribute(CSRF_TOKEN_REQUIRED_ATTR); + + boolean isRequired; + + if (whenRequired==null) { + if (suppliedTokens.isEmpty()) { + log.warn("No CSRF token expected or supplied but a cookie-session is active: client should be updated" + + " (in future CSRF tokens or instructions may be required for session-based clients)" + + " - " + Entitlements.getEntitlementContext()); + whenRequired = CsrfTokenRequiredForRequests.NONE; } else { - fail(requestContext, ApiError.builder().errorCode(Response.Status.UNAUTHORIZED) - .message(REQUIRED_CSRF_TOKEN_HEADER+" header did not match current token") - .build()); + // default + whenRequired = DEFAULT_REQUIRED_FOR_REQUESTS; } + // remember it to suppress warnings subsequently + request.getSession().setAttribute(CSRF_TOKEN_REQUIRED_ATTR, whenRequired); + } + + switch (whenRequired) { + case NONE: + isRequired = false; + break; + case WRITE: + isRequired = !org.eclipse.jetty.http.HttpMethod.GET.toString().equals(requestContext.getMethod()); + break; + case ALL: + isRequired = true; + break; + default: + log.warn("Unexpected "+CSRF_TOKEN_REQUIRED_ATTR+" value "+whenRequired); + isRequired = true; + } + + if (Iterables.any(suppliedTokens, Predicates.equalTo(requiredToken))) { + // matching token supplied, or not being used + return; + } + + if (!isRequired) { + // csrf not required, but it doesn't match + if (requiredToken!=null) { + log.trace("CSRF optional token mismatch: client did not send valid token, but it isn't required so proceeding"); + } + return; + } + + if (suppliedTokens.isEmpty()) { + fail(requestContext, ApiError.builder().errorCode(Response.Status.UNAUTHORIZED) + .message(HEADER_OF_COOKIE(CSRF_TOKEN_VALUE_COOKIE)+" header is required, containing token previously returned from server in cookie") + .build()); + } else { + fail(requestContext, ApiError.builder().errorCode(Response.Status.UNAUTHORIZED) + .message(HEADER_OF_COOKIE(CSRF_TOKEN_VALUE_COOKIE)+" header did not match expected CSRF token") + .build()); } } @@ -91,37 +213,69 @@ private void fail(ContainerRequestContext requestContext, ApiError apiError) { @Override public void filter(ContainerRequestContext requestContext, ContainerResponseContext responseContext) throws IOException { - String requestHeader = request.getHeader(REQUEST_CSRF_TOKEN_HEADER); - if (requestHeader==null) return; - String oldToken = (String) request.getSession().getAttribute(REQUIRED_CSRF_TOKEN_ATTR); - String tokenToReturn = null; - boolean createToken = false; + HttpSession session = request.getSession(false); + String token = (String) (session==null ? null : session.getAttribute(CSRF_TOKEN_VALUE_ATTR)); + String requiredWhenS = request.getHeader(CSRF_TOKEN_REQUIRED_HEADER); - if (REQUEST_CSRF_TOKEN_HEADER_LOGIN.equals(requestHeader)) { - if (oldToken==null) createToken = true; - else return; - } else if (REQUEST_CSRF_TOKEN_HEADER_TRUE.equals(requestHeader)) { - tokenToReturn = oldToken; - if (tokenToReturn==null) createToken = true; - } else if (REQUEST_CSRF_TOKEN_HEADER_NEW.equals(requestHeader)) { - createToken = true; - } - - if (createToken) { - tokenToReturn = Identifiers.makeRandomId(16); - request.getSession().setAttribute(REQUIRED_CSRF_TOKEN_ATTR, tokenToReturn); + if (session==null) { + if (Strings.isBlank(requiredWhenS)) { + // no session and no requirement specified, bail out + return; + } + // explicit requirement request forces a session + session = request.getSession(); } - if (tokenToReturn==null) { - fail(requestContext, ApiError.builder().errorCode(Response.Status.UNAUTHORIZED) - .message(REQUEST_CSRF_TOKEN_HEADER+" contained invalid value; expected: "+ - REQUEST_CSRF_TOKEN_HEADER_TRUE+" | "+ - REQUEST_CSRF_TOKEN_HEADER_LOGIN+" | "+ - REQUEST_CSRF_TOKEN_HEADER_NEW) - .build()); + CsrfTokenRequiredForRequests requiredWhen; + if (Strings.isNonBlank(requiredWhenS)) { + requiredWhen = getRequiredForRequests(requiredWhenS, DEFAULT_REQUIRED_FOR_REQUESTS); + request.getSession().setAttribute(CSRF_TOKEN_REQUIRED_ATTR, requiredWhen); + if (Strings.isNonBlank(token)) { + // already set csrf token, and the client got it + // (with the session token if they are in a session; + // or if client didn't get it it isn't in a session) + return; + } } else { - responseContext.getHeaders().add(REQUIRED_CSRF_TOKEN_HEADER, tokenToReturn); + if (Strings.isNonBlank(token)) { + // already set csrf token, and the client got it + // (with the session token if they are in a session; + // or if client didn't get it it isn't in a session) + return; + } + requiredWhen = (CsrfTokenRequiredForRequests) request.getSession().getAttribute(CSRF_TOKEN_REQUIRED_ATTR); + if (requiredWhen==null) { + requiredWhen = DEFAULT_REQUIRED_FOR_REQUESTS; + request.getSession().setAttribute(CSRF_TOKEN_REQUIRED_ATTR, requiredWhen); + } + } + + if (requiredWhen==CsrfTokenRequiredForRequests.NONE) { + // not required; so don't set + return; + } + + // create the token + token = Identifiers.makeRandomId(16); + request.getSession().setAttribute(CSRF_TOKEN_VALUE_ATTR, token); + + addCookie(responseContext, CSRF_TOKEN_VALUE_COOKIE, token, "Clients should send this value in header "+CSRF_TOKEN_VALUE_HEADER+" for validation"); + addCookie(responseContext, CSRF_TOKEN_VALUE_COOKIE_ANGULAR_NAME, token, "Compatibility cookie for "+CSRF_TOKEN_VALUE_COOKIE+" following AngularJS conventions"); + } + + protected NewCookie addCookie(ContainerResponseContext responseContext, String cookieName, String token, String comment) { + NewCookie cookie = new NewCookie(cookieName, token, "/", null, comment, -1, false); + responseContext.getHeaders().add("Set-Cookie", cookie); + return cookie; + } + + protected CsrfTokenRequiredForRequests getRequiredForRequests(String requiredWhenS, CsrfTokenRequiredForRequests defaultMode) { + CsrfTokenRequiredForRequests result = null; + if (requiredWhenS!=null) { + result = CsrfTokenRequiredForRequests.valueOf(requiredWhenS.toUpperCase()); } + if (result!=null) return result; + return defaultMode; } } diff --git a/rest/rest-server/src/test/java/org/apache/brooklyn/rest/CsrfTokenFilterLauncherTest.java b/rest/rest-server/src/test/java/org/apache/brooklyn/rest/CsrfTokenFilterLauncherTest.java index f9f2ec22e0..a9af873ebd 100644 --- a/rest/rest-server/src/test/java/org/apache/brooklyn/rest/CsrfTokenFilterLauncherTest.java +++ b/rest/rest-server/src/test/java/org/apache/brooklyn/rest/CsrfTokenFilterLauncherTest.java @@ -23,6 +23,7 @@ import java.net.URI; import java.util.List; +import java.util.Map; import javax.ws.rs.core.HttpHeaders; @@ -48,49 +49,66 @@ public void testRequestToken() { .withoutJsgui() .start()); + HttpClient client = client(); + HttpToolResponse response = HttpTool.httpGet( - client(), URI.create(getBaseUriRest() + "server/status"), + client, URI.create(getBaseUriRest() + "server/status"), ImmutableMap.of( - CsrfTokenFilter.REQUEST_CSRF_TOKEN_HEADER, CsrfTokenFilter.REQUEST_CSRF_TOKEN_HEADER_TRUE )); + CsrfTokenFilter.CSRF_TOKEN_REQUIRED_HEADER, CsrfTokenFilter.CsrfTokenRequiredForRequests.WRITE.toString())); // comes back okay assertOkayResponse(response, "MASTER"); - System.out.println(response.getHeaderLists()); - List tokens = response.getHeaderLists().get(CsrfTokenFilter.REQUIRED_CSRF_TOKEN_HEADER); - String token = Iterables.getOnlyElement(tokens); + Map> cookies = response.getCookieKeyValues(); + String token = Iterables.getOnlyElement(cookies.get(CsrfTokenFilter.CSRF_TOKEN_VALUE_COOKIE)); Assert.assertNotNull(token); + String tokenAngular = Iterables.getOnlyElement(cookies.get(CsrfTokenFilter.CSRF_TOKEN_VALUE_COOKIE_ANGULAR_NAME)); + Assert.assertEquals(token, tokenAngular); - List cookies = response.getHeaderLists().get(HttpHeaders.SET_COOKIE); - String cookie = Iterables.getOnlyElement(cookies); - Assert.assertNotNull(cookie); - // can post subsequently with token response = HttpTool.httpPost( - client(), URI.create(getBaseUriRest() + "script/groovy"), + client, URI.create(getBaseUriRest() + "script/groovy"), ImmutableMap.of( HttpHeaders.CONTENT_TYPE, "application/text", - HttpHeaders.COOKIE, cookie, - CsrfTokenFilter.REQUIRED_CSRF_TOKEN_HEADER, token ), + CsrfTokenFilter.CSRF_TOKEN_VALUE_HEADER, token ), "return 0;".getBytes()); assertOkayResponse(response, "{\"result\":\"0\"}"); // but fails without token response = HttpTool.httpPost( - client(), URI.create(getBaseUriRest() + "script/groovy"), + client, URI.create(getBaseUriRest() + "script/groovy"), ImmutableMap.of( - HttpHeaders.COOKIE, cookie, HttpHeaders.CONTENT_TYPE, "application/text" ), "return 0;".getBytes()); assertEquals(response.getResponseCode(), HttpStatus.SC_UNAUTHORIZED); - // but you can get subsequently without token + // can get without token response = HttpTool.httpGet( - client(), URI.create(getBaseUriRest() + "server/status"), + client, URI.create(getBaseUriRest() + "server/status"), + ImmutableMap.of()); + assertOkayResponse(response, "MASTER"); + + // but if we set required ALL then need a token to get + response = HttpTool.httpGet( + client, URI.create(getBaseUriRest() + "server/status"), ImmutableMap.of( - HttpHeaders.COOKIE, cookie )); - + CsrfTokenFilter.CSRF_TOKEN_REQUIRED_HEADER, CsrfTokenFilter.CsrfTokenRequiredForRequests.ALL.toString().toLowerCase() + )); assertOkayResponse(response, "MASTER"); + response = HttpTool.httpGet( + client, URI.create(getBaseUriRest() + "server/status"), + ImmutableMap.of()); + assertEquals(response.getResponseCode(), HttpStatus.SC_UNAUTHORIZED); + + // however note if we use a new client, with no session, then we can post with no token + // (ie we don't guard against CSRF if your brooklyn is unsecured) + client = client(); + response = HttpTool.httpPost( + client, URI.create(getBaseUriRest() + "script/groovy"), + ImmutableMap.of( + HttpHeaders.CONTENT_TYPE, "application/text" ), + "return 0;".getBytes()); + assertOkayResponse(response, "{\"result\":\"0\"}"); } protected HttpClient client() { diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/http/HttpToolResponse.java b/utils/common/src/main/java/org/apache/brooklyn/util/http/HttpToolResponse.java index 811b3e1790..27dc237017 100644 --- a/utils/common/src/main/java/org/apache/brooklyn/util/http/HttpToolResponse.java +++ b/utils/common/src/main/java/org/apache/brooklyn/util/http/HttpToolResponse.java @@ -39,8 +39,10 @@ import com.google.common.base.Objects; import com.google.common.base.Throwables; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.Lists; import com.google.common.collect.Maps; import com.google.common.io.ByteStreams; +import com.google.common.net.HttpHeaders; /** * A response class for use with {@link HttpTool}. @@ -149,6 +151,30 @@ public Map> getHeaderLists() { return headerLists; } + public Map> getCookieKeyValues() { + List cookiesRaw = getHeaderLists().get(HttpHeaders.SET_COOKIE); + Map> result = Maps.newLinkedHashMap(); + for (String c: cookiesRaw) { + // poor man's parse; would need to copy routines in jetty's CookieCutter (not in scope) or similar + // to treat quotes/escapes correctly, and ideally return typed cookies + + int keyI = c.indexOf('='); + if (keyI<0) continue; //not a valid cookie + String key = c.substring(0, keyI); + String value = c.substring(keyI+1); + int flagsI = value.indexOf(';'); + if (flagsI>=0) value = value.substring(0, flagsI); + + List values = result.get(key); + if (values==null) { + values = Lists.newArrayList(); + result.put(key, values); + } + values.add(value); + } + return result; + } + public byte[] getContent() { synchronized (mutex) { if (content == null) { From 30a2d8524d7eb920766728cef20fa893f3bd42b4 Mon Sep 17 00:00:00 2001 From: Alex Heneveld Date: Sun, 13 Nov 2016 13:33:48 +0000 Subject: [PATCH 3/5] set session/cookie on some server requests so client gets it early helps establish csrf protection. done in /server/user and /server/up/extended, the two main places which an interactive app will hit early. --- .../main/java/org/apache/brooklyn/rest/api/ServerApi.java | 6 ++++-- .../org/apache/brooklyn/rest/resources/ServerResource.java | 6 ++++++ 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/rest/rest-api/src/main/java/org/apache/brooklyn/rest/api/ServerApi.java b/rest/rest-api/src/main/java/org/apache/brooklyn/rest/api/ServerApi.java index 90a195aa8e..0b56e3c040 100644 --- a/rest/rest-api/src/main/java/org/apache/brooklyn/rest/api/ServerApi.java +++ b/rest/rest-api/src/main/java/org/apache/brooklyn/rest/api/ServerApi.java @@ -109,7 +109,8 @@ public void shutdown( @GET @Path("/up/extended") - @ApiOperation(value = "Returns extended server-up information, a map including up (/up), shuttingDown (/shuttingDown), healthy (/healthy), and ha (/ha/states) (qv)") + @ApiOperation(value = "Returns extended server-up information, a map including up (/up), shuttingDown (/shuttingDown), healthy (/healthy), and ha (/ha/states) (qv)" + + "; also forces a session, so a useful general-purpose call for a UI client to do when starting") public Map getUpExtended(); @GET @@ -198,7 +199,8 @@ public Response exportPersistenceData( @GET @Path("/user") - @ApiOperation(value = "Return user information for this Brooklyn instance", + @ApiOperation(value = "Return user information for this Brooklyn instance" + + "; also forces a session, so a useful general-purpose call for a UI client to do when starting", response = String.class, responseContainer = "List") public String getUser(); diff --git a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/ServerResource.java b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/ServerResource.java index 7be07de041..cfed891008 100644 --- a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/ServerResource.java +++ b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/ServerResource.java @@ -30,6 +30,7 @@ import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.AtomicBoolean; +import javax.servlet.http.HttpServletRequest; import javax.ws.rs.core.Context; import javax.ws.rs.core.MediaType; import javax.ws.rs.core.Response; @@ -94,6 +95,9 @@ public class ServerResource extends AbstractBrooklynRestResource implements Serv @Context private ContextResolver shutdownHandler; + @Context + private HttpServletRequest request; + @Override public void reloadBrooklynProperties() { if (Entitlements.isEntitled(mgmt().getEntitlementManager(), Entitlements.SEE_ALL_SERVER_INFO, null)) { @@ -365,6 +369,7 @@ public boolean isHealthy() { @Override public Map getUpExtended() { + request.getSession(); return MutableMap.of( "up", isUp(), "shuttingDown", isShuttingDown(), @@ -451,6 +456,7 @@ public Response clearHighAvailabilityPlaneStates() { @Override public String getUser() { + request.getSession(); EntitlementContext entitlementContext = Entitlements.getEntitlementContext(); if (entitlementContext!=null && entitlementContext.user()!=null){ return entitlementContext.user(); From a7ce5b6542533ce49a3eb492d719f6575e2a9837 Mon Sep 17 00:00:00 2001 From: Alex Heneveld Date: Sun, 13 Nov 2016 13:34:12 +0000 Subject: [PATCH 4/5] logout rest code tidy - behaves nicer if no user previously `curl /v1/logout` would throw 500 server error --- .../rest/resources/LogoutResource.java | 20 +++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/LogoutResource.java b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/LogoutResource.java index 66ce968fbd..d24b8d358b 100644 --- a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/LogoutResource.java +++ b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/LogoutResource.java @@ -30,23 +30,35 @@ import org.apache.brooklyn.core.mgmt.entitlement.Entitlements; import org.apache.brooklyn.core.mgmt.entitlement.WebEntitlementContext; import org.apache.brooklyn.rest.api.LogoutApi; +import org.apache.brooklyn.rest.security.jaas.BrooklynLoginModule; import org.apache.brooklyn.util.exceptions.Exceptions; +import com.google.common.net.HttpHeaders; + public class LogoutResource extends AbstractBrooklynRestResource implements LogoutApi { + + private static final String BASIC_REALM_WEBCONSOLE = "Basic realm=\""+BrooklynLoginModule.DEFAULT_ROLE+"\""; + @Context HttpServletRequest req; @Context UriInfo uri; @Override public Response logout() { WebEntitlementContext ctx = (WebEntitlementContext) Entitlements.getEntitlementContext(); + + if (ctx==null) { + return Response.status(Status.BAD_REQUEST) + .entity("No user logged in") + .header(HttpHeaders.WWW_AUTHENTICATE, BASIC_REALM_WEBCONSOLE) + .build(); + } + URI dest = uri.getBaseUriBuilder().path(LogoutApi.class).path(LogoutApi.class, "logoutUser").build(ctx.user()); // When execution gets here we don't know whether this is the first fetch of logout() or a subsequent one // with a re-authenticated user. The only way to tell is compare if user names changed. So redirect to an URL // which contains the user name. - return Response.status(Status.TEMPORARY_REDIRECT) - .header("Location", dest.toASCIIString()) - .build(); + return Response.temporaryRedirect(dest).build(); } @Override @@ -58,7 +70,7 @@ public Response logoutUser(String user) { doLogout(); return Response.status(Status.UNAUTHORIZED) - .header("WWW-Authenticate", "Basic realm=\"webconsole\"") + .header(HttpHeaders.WWW_AUTHENTICATE, BASIC_REALM_WEBCONSOLE) .build(); } else { return Response.temporaryRedirect(uri.getAbsolutePathBuilder().replacePath("/").build()).build(); From 45686e4c837478f682d6aec3792c4371b9d0e7cf Mon Sep 17 00:00:00 2001 From: Alex Heneveld Date: Sun, 13 Nov 2016 13:34:59 +0000 Subject: [PATCH 5/5] tidy - warnings / unused imports in rest --- .../rest/resources/AbstractBrooklynRestResource.java | 1 - .../apache/brooklyn/rest/resources/EntityConfigResource.java | 1 - .../org/apache/brooklyn/rest/resources/SensorResource.java | 1 - .../rest/security/jaas/SecurityProviderHttpSession.java | 5 ++--- .../org/apache/brooklyn/rest/BrooklynRestApiLauncher.java | 1 - 5 files changed, 2 insertions(+), 7 deletions(-) diff --git a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/AbstractBrooklynRestResource.java b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/AbstractBrooklynRestResource.java index 02e856fe6b..082a6204f1 100644 --- a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/AbstractBrooklynRestResource.java +++ b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/AbstractBrooklynRestResource.java @@ -24,7 +24,6 @@ import javax.ws.rs.ext.ContextResolver; import org.apache.brooklyn.api.entity.Entity; -import org.apache.brooklyn.api.entity.EntityLocal; import org.apache.brooklyn.api.mgmt.ManagementContext; import org.apache.brooklyn.core.config.render.RendererHints; import org.apache.brooklyn.core.mgmt.internal.ManagementContextInternal; diff --git a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/EntityConfigResource.java b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/EntityConfigResource.java index fb9dc32dfd..d85f1d76d0 100644 --- a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/EntityConfigResource.java +++ b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/EntityConfigResource.java @@ -38,7 +38,6 @@ import org.apache.brooklyn.rest.util.WebResourceUtils; import org.apache.brooklyn.util.core.flags.TypeCoercions; import org.apache.brooklyn.util.core.task.Tasks; -import org.apache.brooklyn.util.core.task.ValueResolver; import org.apache.brooklyn.util.text.Strings; import org.apache.brooklyn.util.time.Duration; import org.slf4j.Logger; diff --git a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/SensorResource.java b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/SensorResource.java index c51043e235..067af0a1b3 100644 --- a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/SensorResource.java +++ b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/SensorResource.java @@ -35,7 +35,6 @@ import org.apache.brooklyn.rest.filter.HaHotStateRequired; import org.apache.brooklyn.rest.transform.SensorTransformer; import org.apache.brooklyn.rest.util.WebResourceUtils; -import org.apache.brooklyn.util.core.task.ValueResolver; import org.apache.brooklyn.util.text.Strings; import org.apache.brooklyn.util.time.Duration; import org.slf4j.Logger; diff --git a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/security/jaas/SecurityProviderHttpSession.java b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/security/jaas/SecurityProviderHttpSession.java index 75a22670ca..188df4041d 100644 --- a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/security/jaas/SecurityProviderHttpSession.java +++ b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/security/jaas/SecurityProviderHttpSession.java @@ -25,7 +25,6 @@ import javax.servlet.ServletContext; import javax.servlet.http.HttpSession; -import javax.servlet.http.HttpSessionContext; import org.apache.brooklyn.util.text.Identifiers; @@ -63,8 +62,8 @@ public int getMaxInactiveInterval() { return 0; } - @Override - public HttpSessionContext getSessionContext() { + @Deprecated //in interface + public javax.servlet.http.HttpSessionContext getSessionContext() { return null; } diff --git a/rest/rest-server/src/test/java/org/apache/brooklyn/rest/BrooklynRestApiLauncher.java b/rest/rest-server/src/test/java/org/apache/brooklyn/rest/BrooklynRestApiLauncher.java index ffb5db4a1d..fd29317db1 100644 --- a/rest/rest-server/src/test/java/org/apache/brooklyn/rest/BrooklynRestApiLauncher.java +++ b/rest/rest-server/src/test/java/org/apache/brooklyn/rest/BrooklynRestApiLauncher.java @@ -60,7 +60,6 @@ import org.eclipse.jetty.server.NetworkConnector; import org.eclipse.jetty.server.Server; import org.eclipse.jetty.server.handler.ContextHandler; -import org.eclipse.jetty.servlet.ServletContextHandler; import org.eclipse.jetty.webapp.WebAppContext; import org.reflections.util.ClasspathHelper; import org.slf4j.Logger;