You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@brooklyn.apache.org by sj...@apache.org on 2014/07/28 12:29:27 UTC
[2/7] git commit: Alters logging in BrooklynPropertiesSecurityFilter
to not read request body
Alters logging in BrooklynPropertiesSecurityFilter to not read request body
.. at least before the chain has been processed. Adds an integration test
to verify that the interaction between the filter and FormMapProvider
works as expected.
Project: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/commit/43981a65
Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/43981a65
Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/43981a65
Branch: refs/heads/master
Commit: 43981a65b0ff51bb721d9138eeb6b2e6bb014694
Parents: 1bcb929
Author: Sam Corbett <sa...@cloudsoftcorp.com>
Authored: Fri Jul 25 18:41:25 2014 +0100
Committer: Sam Corbett <sa...@cloudsoftcorp.com>
Committed: Fri Jul 25 19:31:12 2014 +0100
----------------------------------------------------------------------
.../brooklyn/test/entity/TestEntityImpl.java | 2 +-
.../BrooklynPropertiesSecurityFilter.java | 66 ++++++-----
.../BrooklynPropertiesSecurityFilterTest.java | 112 +++++++++++++++++++
3 files changed, 145 insertions(+), 35 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/43981a65/core/src/test/java/brooklyn/test/entity/TestEntityImpl.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/brooklyn/test/entity/TestEntityImpl.java b/core/src/test/java/brooklyn/test/entity/TestEntityImpl.java
index 994dc26..64131f5 100644
--- a/core/src/test/java/brooklyn/test/entity/TestEntityImpl.java
+++ b/core/src/test/java/brooklyn/test/entity/TestEntityImpl.java
@@ -84,7 +84,7 @@ public class TestEntityImpl extends AbstractEntity implements TestEntity {
public Object identityEffector(Object arg) {
if (LOG.isTraceEnabled()) LOG.trace("In identityEffector for {}", this);
callHistory.add("identityEffector");
- return arg;
+ return checkNotNull(arg, "arg");
}
@Override
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/43981a65/usage/rest-server/src/main/java/brooklyn/rest/security/BrooklynPropertiesSecurityFilter.java
----------------------------------------------------------------------
diff --git a/usage/rest-server/src/main/java/brooklyn/rest/security/BrooklynPropertiesSecurityFilter.java b/usage/rest-server/src/main/java/brooklyn/rest/security/BrooklynPropertiesSecurityFilter.java
index 801344d..bffa6a3 100644
--- a/usage/rest-server/src/main/java/brooklyn/rest/security/BrooklynPropertiesSecurityFilter.java
+++ b/usage/rest-server/src/main/java/brooklyn/rest/security/BrooklynPropertiesSecurityFilter.java
@@ -38,6 +38,7 @@ import brooklyn.management.entitlement.Entitlements;
import brooklyn.management.entitlement.WebEntitlementContext;
import brooklyn.rest.security.provider.DelegatingSecurityProvider;
import brooklyn.util.exceptions.Exceptions;
+import brooklyn.util.text.Identifiers;
import brooklyn.util.text.Strings;
import com.sun.jersey.core.util.Base64;
@@ -62,69 +63,66 @@ public class BrooklynPropertiesSecurityFilter implements Filter {
@Override
public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException, ServletException {
- if (provider==null) {
+ if (provider == null) {
log.warn("No security provider available: disallowing web access to brooklyn");
((HttpServletResponse) response).sendError(HttpServletResponse.SC_SERVICE_UNAVAILABLE);
return;
}
-
- if (authenticate((HttpServletRequest)request)) {
- String user = Strings.toString( ((HttpServletRequest)request).getSession().getAttribute(AUTHENTICATED_USER_SESSION_ATTRIBUTE) );
- if (handleLogout((HttpServletRequest)request)) {
- log.debug("REST logging out "+user+" of session "+((HttpServletRequest)request).getSession());
+
+ HttpServletRequest httpRequest = (HttpServletRequest) request;
+ if (authenticate(httpRequest)) {
+ String user = Strings.toString(httpRequest.getSession().getAttribute(AUTHENTICATED_USER_SESSION_ATTRIBUTE));
+ if (handleLogout(httpRequest)) {
+ log.debug("REST logging out " + user + " of session " + httpRequest.getSession());
// do nothing here, fall through to below
} else {
WebEntitlementContext entitlementContext = null;
- String uri = ((HttpServletRequest)request).getRequestURI();
+ String uri = httpRequest.getRequestURI();
try {
- String uid = Integer.toHexString(hashCode());
- entitlementContext = new WebEntitlementContext(user, ((HttpServletRequest)request).getRemoteAddr(), uri, uid);
- if (originalRequest.get()==null) {
+ String uid = Identifiers.makeRandomId(6);
+ entitlementContext = new WebEntitlementContext(user, httpRequest.getRemoteAddr(), uri, uid);
+ if (originalRequest.get() == null) {
// initial filter application
originalRequest.set(uri);
} else {
// this filter is being applied *again*, probably due to forwarding (e.g. from '/' to '/index.html')
- log.debug("REST request {} being forwarded from {} to {}", new Object[] { uid, originalRequest.get(), uri });
+ log.debug("REST request {} being forwarded from {} to {}", new Object[]{uid, originalRequest.get(), uri});
// clear the entitlement context before setting to avoid warnings
Entitlements.clearEntitlementContext();
}
Entitlements.setEntitlementContext(entitlementContext);
-
- log.debug("REST starting processing request {} with {}", uri, entitlementContext);
- if (!((HttpServletRequest)request).getParameterMap().isEmpty()) {
- log.debug(" REST req {} parameters: {}", uid, ((HttpServletRequest)request).getParameterMap());
- }
- if (((HttpServletRequest)request).getContentLength()>0) {
- int len = ((HttpServletRequest)request).getContentLength();
- log.debug(" REST req {} upload content type {}", uid, ((HttpServletRequest)request).getContentType()+" length "+len
- // would be nice to do this, but the stream can only be read once;
- // TODO figure out how we could peek at it
-// +(len>0 && len<4096 ? ""+Streams.readFullyString(((HttpServletRequest)request).getInputStream()) : "")
- );
- }
-
+
+ log.debug("REST req {} starting processing request {} with {}", new Object[]{uid, uri, entitlementContext});
chain.doFilter(request, response);
-
- log.debug("REST completed, code {}, processing request {} with {}",
- new Object[] { ((HttpServletResponse)response).getStatus(), uri, entitlementContext } );
+
+ // This logging must NOT happen before chain.doFilter, or FormMapProvider will not work as expected.
+ // Getting the parameter map consumes the request body and only resource methods using @FormParam
+ // will work as expected.
+ if (log.isDebugEnabled()) {
+ log.debug("REST req {} complete, responding {} for request {} with {}",
+ new Object[]{uid, ((HttpServletResponse) response).getStatus(), uri, entitlementContext});
+ if (!httpRequest.getParameterMap().isEmpty()) {
+ log.debug(" parameters were: {}", httpRequest.getParameterMap());
+ }
+ if (httpRequest.getContentLength() > 0) {
+ int len = httpRequest.getContentLength();
+ log.debug(" upload content type was {}, length={}", httpRequest.getContentType(), len);
+ }
+ }
return;
-
} catch (Throwable e) {
// NB errors are typically already caught at this point
if (log.isDebugEnabled()) {
- log.debug("REST failed processing request "+uri+" with "+entitlementContext+": "+e, e);
+ log.debug("REST failed processing request " + uri + " with " + entitlementContext + ": " + e, e);
}
-
throw Exceptions.propagate(e);
-
} finally {
originalRequest.remove();
Entitlements.clearEntitlementContext();
}
}
}
-
- ((HttpServletResponse) response).setHeader("WWW-Authenticate","Basic realm=\"brooklyn\"");
+ ((HttpServletResponse) response).setHeader("WWW-Authenticate", "Basic realm=\"brooklyn\"");
((HttpServletResponse) response).sendError(HttpServletResponse.SC_UNAUTHORIZED);
}
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/43981a65/usage/rest-server/src/test/java/brooklyn/rest/BrooklynPropertiesSecurityFilterTest.java
----------------------------------------------------------------------
diff --git a/usage/rest-server/src/test/java/brooklyn/rest/BrooklynPropertiesSecurityFilterTest.java b/usage/rest-server/src/test/java/brooklyn/rest/BrooklynPropertiesSecurityFilterTest.java
new file mode 100644
index 0000000..2f0afef
--- /dev/null
+++ b/usage/rest-server/src/test/java/brooklyn/rest/BrooklynPropertiesSecurityFilterTest.java
@@ -0,0 +1,112 @@
+/*
+ * 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 brooklyn.rest;
+
+import static org.testng.Assert.assertTrue;
+
+import java.io.IOException;
+import java.net.URI;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import org.apache.http.HttpHeaders;
+import org.apache.http.NameValuePair;
+import org.apache.http.client.HttpClient;
+import org.apache.http.client.utils.URLEncodedUtils;
+import org.apache.http.entity.ContentType;
+import org.apache.http.message.BasicNameValuePair;
+import org.eclipse.jetty.server.Server;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.testng.annotations.Test;
+
+import com.fasterxml.jackson.core.JsonParseException;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.google.common.base.Charsets;
+import com.google.common.base.Stopwatch;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Lists;
+
+import brooklyn.rest.security.provider.AnyoneSecurityProvider;
+import brooklyn.util.collections.Jsonya;
+import brooklyn.util.collections.MutableMap;
+import brooklyn.util.http.HttpTool;
+import brooklyn.util.http.HttpToolResponse;
+import brooklyn.util.time.Time;
+
+public class BrooklynPropertiesSecurityFilterTest extends BrooklynRestApiLauncherTestFixture {
+
+ private static final Logger LOG = LoggerFactory.getLogger(BrooklynPropertiesSecurityFilterTest.class);
+
+ // Would be great for this to be a unit test but it takes almost ten seconds.
+ @Test(groups = "Integration")
+ public void testInteractionOfSecurityFilterAndFormMapProvider() throws Exception {
+ Stopwatch stopwatch = Stopwatch.createStarted();
+ try {
+ Server server = useServerForTest(BrooklynRestApiLauncher.launcher()
+ .securityProvider(AnyoneSecurityProvider.class)
+ .forceUseOfDefaultCatalogWithJavaClassPath(true)
+ .withoutJsgui()
+ .start());
+ String appId = startAppAtNode(server);
+ String entityId = getTestEntityInApp(server, appId);
+ HttpClient client = HttpTool.httpClientBuilder()
+ .uri(getBaseUri(server))
+ .build();
+ List<? extends NameValuePair> nvps = Lists.newArrayList(
+ new BasicNameValuePair("arg", "bar"));
+ String effector = String.format("/v1/applications/%s/entities/%s/effectors/identityEffector", appId, entityId);
+ HttpToolResponse response = HttpTool.httpPost(client, URI.create(getBaseUri() + effector),
+ ImmutableMap.of(HttpHeaders.CONTENT_TYPE, ContentType.APPLICATION_FORM_URLENCODED.getMimeType()),
+ URLEncodedUtils.format(nvps, Charsets.UTF_8).getBytes());
+
+ LOG.info("Effector response: {}", response.getContentAsString());
+ assertTrue(HttpTool.isStatusCodeHealthy(response.getResponseCode()), "response code=" + response.getResponseCode());
+ } finally {
+ LOG.info("testInteractionOfSecurityFilterAndFormMapProvider complete in " + Time.makeTimeStringRounded(stopwatch));
+ }
+ }
+
+ private String startAppAtNode(Server server) throws Exception {
+ String blueprint = "name: TestApp\n" +
+ "location: localhost\n" +
+ "services:\n" +
+ "- type: brooklyn.test.entity.TestEntity";
+ HttpClient client = HttpTool.httpClientBuilder()
+ .uri(getBaseUri(server))
+ .build();
+ HttpToolResponse response = HttpTool.httpPost(client, URI.create(getBaseUri() + "/v1/applications"),
+ ImmutableMap.of(HttpHeaders.CONTENT_TYPE, "application/x-yaml"),
+ blueprint.getBytes());
+ assertTrue(HttpTool.isStatusCodeHealthy(response.getResponseCode()), "error creating app. response code=" + response.getResponseCode());
+ Map<String, Object> body = new ObjectMapper().readValue(response.getContent(), HashMap.class);
+ return (String) body.get("entityId");
+ }
+
+ private String getTestEntityInApp(Server server, String appId) throws Exception {
+ HttpClient client = HttpTool.httpClientBuilder()
+ .uri(getBaseUri(server))
+ .build();
+ List entities = new ObjectMapper().readValue(
+ HttpTool.httpGet(client, URI.create(getBaseUri() + "/v1/applications/" + appId + "/entities"), MutableMap.<String, String>of()).getContent(), List.class);
+ LOG.info((String) ((Map) entities.get(0)).get("id"));
+ return (String) ((Map) entities.get(0)).get("id");
+ }
+}