From 25b3492e84a8812f2e8341a526bfc0fd309e15ec Mon Sep 17 00:00:00 2001 From: Sam Corbett Date: Thu, 4 May 2017 19:48:25 +0100 Subject: [PATCH 1/2] CatalogResource closes ZipFiles cleanly Rather than relying on it to occure in ZipFile's finaliser --- .../apache/brooklyn/rest/resources/CatalogResource.java | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/CatalogResource.java b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/CatalogResource.java index e03bd41794..d0f7270e0d 100644 --- a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/CatalogResource.java +++ b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/CatalogResource.java @@ -198,6 +198,14 @@ public Response createFromArchive(byte[] zipInput) { } catch (IOException e) { throw new IllegalArgumentException("Error reading catalog.bom from ZIP/JAR archive: "+e); } + + try { + zf.close(); + } catch (IOException e) { + log.debug("Swallowed exception closing zipfile. Full error logged at trace: {}", e.getMessage()); + log.trace("Exception closing zipfile", e); + } + VersionedName vn = BasicBrooklynCatalog.getVersionedName( BasicBrooklynCatalog.getCatalogMetadata(bomS) ); Manifest mf = bm.getManifest(f); From 5e38b8396c03c219f80aa86aa15ca29ae42711be Mon Sep 17 00:00:00 2001 From: Sam Corbett Date: Thu, 4 May 2017 20:10:53 +0100 Subject: [PATCH 2/2] Make invokeEffector check in ApplicationResource before invoking effector The permission was caught further down the line anyway (in EntityChangeListenerImpl#onEffectorStarting) so this is a case of failing earlier than fixing a security hole. --- .../rest/resources/ApplicationResource.java | 8 +- .../BrooklynRestApiLauncherTestFixture.java | 6 +- .../ApplicationResourceEntitlementsTest.java | 110 ++++++++++++++++++ 3 files changed, 119 insertions(+), 5 deletions(-) create mode 100644 rest/rest-server/src/test/java/org/apache/brooklyn/rest/entitlement/ApplicationResourceEntitlementsTest.java diff --git a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/ApplicationResource.java b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/ApplicationResource.java index 5d8dfb66c7..1a42103090 100644 --- a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/ApplicationResource.java +++ b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/ApplicationResource.java @@ -19,9 +19,9 @@ package org.apache.brooklyn.rest.resources; import static com.google.common.base.Preconditions.checkNotNull; +import static javax.ws.rs.core.Response.Status.ACCEPTED; import static javax.ws.rs.core.Response.created; import static javax.ws.rs.core.Response.status; -import static javax.ws.rs.core.Response.Status.ACCEPTED; import static org.apache.brooklyn.rest.util.WebResourceUtils.serviceAbsoluteUriBuilder; import java.net.URI; @@ -32,7 +32,6 @@ import java.util.List; import java.util.Map; import java.util.concurrent.Callable; - import javax.ws.rs.core.Context; import javax.ws.rs.core.Response; import javax.ws.rs.core.Response.ResponseBuilder; @@ -296,8 +295,6 @@ public Response createFromYaml(String yaml) { private Response launch(String yaml, EntitySpec spec) { try { Application app = EntityManagementUtils.createUnstarted(mgmt(), spec); - CreationResult result = EntityManagementUtils.start(app); - waitForStart(app, Duration.millis(100)); boolean isEntitled = Entitlements.isEntitled( mgmt().getEntitlementManager(), @@ -309,6 +306,9 @@ private Response launch(String yaml, EntitySpec spec) { Entitlements.getEntitlementContext().user(), spec.getType()); } + CreationResult result = EntityManagementUtils.start(app); + waitForStart(app, Duration.millis(100)); + log.info("Launched from YAML: " + yaml + " -> " + app + " (" + result.task() + ")"); URI ref = serviceAbsoluteUriBuilder(ui.getBaseUriBuilder(), ApplicationApi.class, "get").build(app.getApplicationId()); diff --git a/rest/rest-server/src/test/java/org/apache/brooklyn/rest/BrooklynRestApiLauncherTestFixture.java b/rest/rest-server/src/test/java/org/apache/brooklyn/rest/BrooklynRestApiLauncherTestFixture.java index 642ef875cd..828727a3a3 100644 --- a/rest/rest-server/src/test/java/org/apache/brooklyn/rest/BrooklynRestApiLauncherTestFixture.java +++ b/rest/rest-server/src/test/java/org/apache/brooklyn/rest/BrooklynRestApiLauncherTestFixture.java @@ -41,6 +41,7 @@ import org.testng.annotations.AfterMethod; import java.net.URI; +import java.util.Map; import static org.apache.brooklyn.util.http.HttpTool.httpClientBuilder; import static org.testng.Assert.assertTrue; @@ -104,7 +105,10 @@ protected String httpGet(String user, String path) throws Exception { } protected HttpToolResponse httpPost(String user, String path, byte[] body) throws Exception { - final ImmutableMap headers = ImmutableMap.of(); + return httpPost(user, path, body, ImmutableMap.of()); + } + + protected HttpToolResponse httpPost(String user, String path, byte[] body, Map headers) throws Exception { final URI uri = URI.create(getBaseUriRest()).resolve(path); return HttpTool.httpPost(newClient(user), uri, headers, body); } diff --git a/rest/rest-server/src/test/java/org/apache/brooklyn/rest/entitlement/ApplicationResourceEntitlementsTest.java b/rest/rest-server/src/test/java/org/apache/brooklyn/rest/entitlement/ApplicationResourceEntitlementsTest.java new file mode 100644 index 0000000000..60e3ac7b42 --- /dev/null +++ b/rest/rest-server/src/test/java/org/apache/brooklyn/rest/entitlement/ApplicationResourceEntitlementsTest.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.entitlement; + +import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertFalse; +import static org.testng.Assert.assertNotNull; +import static org.testng.Assert.assertTrue; + +import java.util.Collection; +import java.util.Set; +import java.util.concurrent.atomic.AtomicBoolean; +import javax.annotation.Nonnull; +import javax.ws.rs.WebApplicationException; + +import org.apache.brooklyn.api.entity.Application; +import org.apache.brooklyn.api.entity.Entity; +import org.apache.brooklyn.api.entity.ImplementedBy; +import org.apache.brooklyn.api.location.Location; +import org.apache.brooklyn.api.mgmt.entitlement.EntitlementClass; +import org.apache.brooklyn.api.mgmt.entitlement.EntitlementContext; +import org.apache.brooklyn.api.mgmt.entitlement.EntitlementManager; +import org.apache.brooklyn.core.entity.Attributes; +import org.apache.brooklyn.core.internal.BrooklynProperties; +import org.apache.brooklyn.core.mgmt.entitlement.Entitlements; +import org.apache.brooklyn.core.mgmt.entitlement.PerUserEntitlementManager; +import org.apache.brooklyn.core.mgmt.entitlement.WebEntitlementContext; +import org.apache.brooklyn.core.test.BrooklynAppUnitTestSupport; +import org.apache.brooklyn.core.test.entity.LocalManagementContextForTests; +import org.apache.brooklyn.entity.stock.BasicStartable; +import org.apache.brooklyn.entity.stock.BasicStartableImpl; +import org.apache.brooklyn.rest.domain.ApiError; +import org.apache.brooklyn.rest.resources.ApplicationResource; +import org.apache.brooklyn.util.exceptions.Exceptions; +import org.apache.brooklyn.util.http.HttpToolResponse; +import org.testng.annotations.BeforeMethod; +import org.testng.annotations.Test; + +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Sets; + +public class ApplicationResourceEntitlementsTest extends AbstractRestApiEntitlementsTest { + + private static final AtomicBoolean INDICATOR = new AtomicBoolean(); + + @BeforeMethod + @Override + public void setUp() throws Exception { + super.setUp(); + INDICATOR.set(false); + } + + @Test + public void testUnentitledDeploy() throws Exception { + final Set initialApps = ImmutableSet.copyOf(mgmt.getApplications()); + final String bp = "services:\n- type: " + StartRecordingEntity.class.getName(); + + StaticDelegatingEntitlementManager.setDelegate(new InvokeEffector(false)); + httpPost("myCustom", "/v1/applications", bp.getBytes(), ImmutableMap.of("Content-Type", "text/yaml")); + // Check the app wasn't started + Sets.SetView afterApps = Sets.difference(ImmutableSet.copyOf(mgmt.getApplications()), initialApps); + assertEquals(afterApps.size(), 1, "expected one element: " + afterApps); + Application newApp = afterApps.iterator().next(); + assertFalse(newApp.sensors().get(Attributes.SERVICE_UP)); + assertFalse(INDICATOR.get()); + } + + public static class InvokeEffector implements EntitlementManager { + private final boolean mayInvoke; + + public InvokeEffector(boolean mayInvoke) { + this.mayInvoke = mayInvoke; + } + + @Override + public boolean isEntitled(EntitlementContext context, @Nonnull EntitlementClass entitlementClass, T entitlementClassArgument) { + String type = entitlementClass.entitlementClassIdentifier(); + return !Entitlements.INVOKE_EFFECTOR.entitlementClassIdentifier().equals(type) || mayInvoke; + } + } + + @ImplementedBy(StartRecordingEntityImpl.class) + public interface StartRecordingEntity extends BasicStartable {} + public static class StartRecordingEntityImpl extends BasicStartableImpl implements StartRecordingEntity { + @Override + public void start(Collection locations) { + super.start(locations); + INDICATOR.set(true); + } + } + +}