[2/3] brooklyn-server git commit: Make invokeEffector check in ApplicationResource before invoking effector

[dr...@apache.org]

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.


Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo
Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/5e38b839
Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/5e38b839
Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/5e38b839

Branch: refs/heads/master
Commit: 5e38b8396c03c219f80aa86aa15ca29ae42711be
Parents: 25b3492
Author: Sam Corbett <sa...@cloudsoftcorp.com>
Authored: Thu May 4 20:10:53 2017 +0100
Committer: Sam Corbett <sa...@cloudsoftcorp.com>
Committed: Thu May 4 20:10:53 2017 +0100

----------------------------------------------------------------------
 .../rest/resources/ApplicationResource.java     |   8 +-
 .../BrooklynRestApiLauncherTestFixture.java     |   6 +-
 .../ApplicationResourceEntitlementsTest.java    | 110 +++++++++++++++++++
 3 files changed, 119 insertions(+), 5 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/5e38b839/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/ApplicationResource.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 5d8dfb6..1a42103 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.Iterator;
 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 class ApplicationResource extends AbstractBrooklynRestResource implements
     private Response launch(String yaml, EntitySpec<? extends Application> spec) {
         try {
             Application app = EntityManagementUtils.createUnstarted(mgmt(), spec);
-            CreationResult<Application,Void> result = EntityManagementUtils.start(app);
-            waitForStart(app, Duration.millis(100));
 
             boolean isEntitled = Entitlements.isEntitled(
                     mgmt().getEntitlementManager(),
@@ -309,6 +306,9 @@ public class ApplicationResource extends AbstractBrooklynRestResource implements
                     Entitlements.getEntitlementContext().user(), spec.getType());
             }
 
+            CreationResult<Application,Void> 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());

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/5e38b839/rest/rest-server/src/test/java/org/apache/brooklyn/rest/BrooklynRestApiLauncherTestFixture.java
----------------------------------------------------------------------
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 642ef87..828727a 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.Assert;
 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 @@ public abstract class BrooklynRestApiLauncherTestFixture {
     }
 
     protected HttpToolResponse httpPost(String user, String path, byte[] body) throws Exception {
-        final ImmutableMap<String, String> headers = ImmutableMap.of();
+        return httpPost(user, path, body, ImmutableMap.<String, String>of());
+    }
+
+    protected HttpToolResponse httpPost(String user, String path, byte[] body, Map<String, String> headers) throws Exception {
         final URI uri = URI.create(getBaseUriRest()).resolve(path);
         return HttpTool.httpPost(newClient(user), uri, headers, body);
     }

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/5e38b839/rest/rest-server/src/test/java/org/apache/brooklyn/rest/entitlement/ApplicationResourceEntitlementsTest.java
----------------------------------------------------------------------
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 0000000..60e3ac7
--- /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<Application> 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<Application> 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 <T> boolean isEntitled(EntitlementContext context, @Nonnull EntitlementClass<T> 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<? extends Location> locations) {
+            super.start(locations);
+            INDICATOR.set(true);
+        }
+    }
+
+}