Return-Path: X-Original-To: apmail-ambari-commits-archive@www.apache.org Delivered-To: apmail-ambari-commits-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id B5BEA17522 for ; Tue, 14 Oct 2014 13:22:09 +0000 (UTC) Received: (qmail 52807 invoked by uid 500); 14 Oct 2014 13:22:09 -0000 Delivered-To: apmail-ambari-commits-archive@ambari.apache.org Received: (qmail 52777 invoked by uid 500); 14 Oct 2014 13:22:09 -0000 Mailing-List: contact commits-help@ambari.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: ambari-dev@ambari.apache.org Delivered-To: mailing list commits@ambari.apache.org Received: (qmail 52768 invoked by uid 99); 14 Oct 2014 13:22:09 -0000 Received: from tyr.zones.apache.org (HELO tyr.zones.apache.org) (140.211.11.114) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 14 Oct 2014 13:22:09 +0000 Received: by tyr.zones.apache.org (Postfix, from userid 65534) id 4DC2B92D49C; Tue, 14 Oct 2014 13:22:09 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: tbeerbower@apache.org To: commits@ambari.apache.org Message-Id: <8664f2d8a269425f9f042b2b4dd0499e@git.apache.org> X-Mailer: ASF-Git Admin Mailer Subject: git commit: AMBARI-7598 - Views: the properties object should only be accessible from REST API if AMBARI.ADMIN Date: Tue, 14 Oct 2014 13:22:09 +0000 (UTC) Repository: ambari Updated Branches: refs/heads/branch-1.7.0 4081c3d33 -> cc2d8a393 AMBARI-7598 - Views: the properties object should only be accessible from REST API if AMBARI.ADMIN Project: http://git-wip-us.apache.org/repos/asf/ambari/repo Commit: http://git-wip-us.apache.org/repos/asf/ambari/commit/cc2d8a39 Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/cc2d8a39 Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/cc2d8a39 Branch: refs/heads/branch-1.7.0 Commit: cc2d8a393e1ddd8175aeab7f1c3e817f11cd9fdf Parents: 4081c3d Author: tbeerbower Authored: Tue Oct 14 09:21:45 2014 -0400 Committer: tbeerbower Committed: Tue Oct 14 09:21:45 2014 -0400 ---------------------------------------------------------------------- .../internal/ViewInstanceResourceProvider.java | 44 ++++++++++------- .../apache/ambari/server/view/ViewRegistry.java | 9 ++++ .../ViewInstanceResourceProviderTest.java | 50 +++++++++++++++++--- 3 files changed, 81 insertions(+), 22 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/ambari/blob/cc2d8a39/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ViewInstanceResourceProvider.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ViewInstanceResourceProvider.java b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ViewInstanceResourceProvider.java index 8e47d88..158f66d 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ViewInstanceResourceProvider.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ViewInstanceResourceProvider.java @@ -221,18 +221,24 @@ public class ViewInstanceResourceProvider extends AbstractResourceProvider { setResourceProperty(resource, DESCRIPTION_PROPERTY_ID, viewInstanceEntity.getDescription(), requestedIds); setResourceProperty(resource, VISIBLE_PROPERTY_ID, viewInstanceEntity.isVisible(), requestedIds); setResourceProperty(resource, STATIC_PROPERTY_ID, viewInstanceEntity.isXmlDriven(), requestedIds); - Map properties = new HashMap(); - for (ViewInstancePropertyEntity viewInstancePropertyEntity : viewInstanceEntity.getProperties()) { - properties.put(viewInstancePropertyEntity.getName(), viewInstancePropertyEntity.getValue()); - } - for (ViewParameterEntity viewParameterEntity : viewEntity.getParameters()) { - if (!properties.containsKey(viewParameterEntity.getName())) { - properties.put(viewParameterEntity.getName(), null); + // only allow an admin to access the view properties + if (ViewRegistry.getInstance().checkAdmin()) { + + Map properties = new HashMap(); + + for (ViewInstancePropertyEntity viewInstancePropertyEntity : viewInstanceEntity.getProperties()) { + properties.put(viewInstancePropertyEntity.getName(), viewInstancePropertyEntity.getValue()); + } + for (ViewParameterEntity viewParameterEntity : viewEntity.getParameters()) { + if (!properties.containsKey(viewParameterEntity.getName())) { + properties.put(viewParameterEntity.getName(), null); + } } + setResourceProperty(resource, PROPERTIES_PROPERTY_ID, + properties, requestedIds); } - setResourceProperty(resource, PROPERTIES_PROPERTY_ID, - properties, requestedIds); + Map applicationData = new HashMap(); String currentUserName = viewInstanceEntity.getCurrentUserName(); @@ -305,20 +311,26 @@ public class ViewInstanceResourceProvider extends AbstractResourceProvider { Collection instanceProperties = new HashSet(); + boolean isUserAdmin = viewRegistry.checkAdmin(); + for (Map.Entry entry : properties.entrySet()) { String propertyName = entry.getKey(); if (propertyName.startsWith(PROPERTIES_PREFIX)) { - ViewInstancePropertyEntity viewInstancePropertyEntity = new ViewInstancePropertyEntity(); - viewInstancePropertyEntity.setViewName(viewName); - viewInstancePropertyEntity.setViewInstanceName(name); - viewInstancePropertyEntity.setName(entry.getKey().substring(PROPERTIES_PREFIX.length())); - viewInstancePropertyEntity.setValue((String) entry.getValue()); - viewInstancePropertyEntity.setViewInstanceEntity(viewInstanceEntity); + // only allow an admin to access the view properties + if (isUserAdmin) { + ViewInstancePropertyEntity viewInstancePropertyEntity = new ViewInstancePropertyEntity(); - instanceProperties.add(viewInstancePropertyEntity); + viewInstancePropertyEntity.setViewName(viewName); + viewInstancePropertyEntity.setViewInstanceName(name); + viewInstancePropertyEntity.setName(entry.getKey().substring(PROPERTIES_PREFIX.length())); + viewInstancePropertyEntity.setValue((String) entry.getValue()); + viewInstancePropertyEntity.setViewInstanceEntity(viewInstanceEntity); + + instanceProperties.add(viewInstancePropertyEntity); + } } else if (propertyName.startsWith(DATA_PREFIX)) { viewInstanceEntity.putInstanceData(entry.getKey().substring(DATA_PREFIX.length()), (String) entry.getValue()); } http://git-wip-us.apache.org/repos/asf/ambari/blob/cc2d8a39/ambari-server/src/main/java/org/apache/ambari/server/view/ViewRegistry.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/view/ViewRegistry.java b/ambari-server/src/main/java/org/apache/ambari/server/view/ViewRegistry.java index 33afc7c..83e91cd 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/view/ViewRegistry.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/view/ViewRegistry.java @@ -718,6 +718,15 @@ public class ViewRegistry { } /** + * Determine whether or not the current view user has admin rights. + * + * @return true if the current view user is an admin + */ + public boolean checkAdmin() { + return checkAuthorization(null); + } + + /** * Determine whether or not the given view definition resource should be included * based on the permissions granted to the current user. * http://git-wip-us.apache.org/repos/asf/ambari/blob/cc2d8a39/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ViewInstanceResourceProviderTest.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ViewInstanceResourceProviderTest.java b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ViewInstanceResourceProviderTest.java index 8f34916..0352aa6 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ViewInstanceResourceProviderTest.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ViewInstanceResourceProviderTest.java @@ -44,6 +44,7 @@ import java.util.Set; import static org.junit.Assert.*; import static org.easymock.EasyMock.*; +import static org.junit.Assert.assertEquals; public class ViewInstanceResourceProviderTest { @@ -65,7 +66,7 @@ public class ViewInstanceResourceProviderTest { propertyIds.add(ViewInstanceResourceProvider.PROPERTIES_PROPERTY_ID); ViewInstanceEntity viewInstanceEntity = createNiceMock(ViewInstanceEntity.class); ViewEntity viewEntity = createNiceMock(ViewEntity.class); - expect(viewInstanceEntity.getViewEntity()).andReturn(viewEntity); + expect(viewInstanceEntity.getViewEntity()).andReturn(viewEntity).anyTimes(); ViewInstancePropertyEntity propertyEntity1 = createNiceMock(ViewInstancePropertyEntity.class); expect(propertyEntity1.getName()).andReturn("par1").anyTimes(); @@ -81,10 +82,14 @@ public class ViewInstanceResourceProviderTest { expect(parameter2.getName()).andReturn("par2").anyTimes(); expect(viewEntity.getParameters()).andReturn(Arrays.asList(parameter1, parameter2)); - expect(viewInstanceEntity.getData()).andReturn(Collections.emptyList()); + expect(viewInstanceEntity.getData()).andReturn(Collections.emptyList()).anyTimes(); - replay(viewEntity, viewInstanceEntity, parameter1, parameter2, propertyEntity1, propertyEntity3); + expect(singleton.checkAdmin()).andReturn(true); + expect(singleton.checkAdmin()).andReturn(false); + replay(singleton, viewEntity, viewInstanceEntity, parameter1, parameter2, propertyEntity1, propertyEntity3); + + // as admin Resource resource = provider.toResource(viewInstanceEntity, propertyIds); Map> properties = resource.getPropertiesMap(); assertEquals(1, properties.size()); @@ -94,6 +99,14 @@ public class ViewInstanceResourceProviderTest { assertEquals("val1", props.get("par1")); assertEquals("val3", props.get("par3")); assertNull(props.get("par2")); + + // as non-admin + resource = provider.toResource(viewInstanceEntity, propertyIds); + properties = resource.getPropertiesMap(); + props = properties.get("ViewInstanceInfo/properties"); + assertNull(props); + + verify(singleton); } @Test @@ -107,6 +120,7 @@ public class ViewInstanceResourceProviderTest { propertyMap.put(ViewInstanceResourceProvider.VIEW_NAME_PROPERTY_ID, "V1"); propertyMap.put(ViewInstanceResourceProvider.VIEW_VERSION_PROPERTY_ID, "1.0.0"); propertyMap.put(ViewInstanceResourceProvider.INSTANCE_NAME_PROPERTY_ID, "I1"); + propertyMap.put(ViewInstanceResourceProvider.PROPERTIES_PROPERTY_ID + "/test_property", "test_value"); properties.add(propertyMap); @@ -114,24 +128,44 @@ public class ViewInstanceResourceProviderTest { viewInstanceEntity.setViewName("V1{1.0.0}"); viewInstanceEntity.setName("I1"); + ViewInstanceEntity viewInstanceEntity2 = new ViewInstanceEntity(); + viewInstanceEntity2.setViewName("V1{1.0.0}"); + viewInstanceEntity2.setName("I1"); + ViewEntity viewEntity = new ViewEntity(); viewEntity.setStatus(ViewDefinition.ViewStatus.DEPLOYED); viewEntity.setName("V1{1.0.0}"); viewInstanceEntity.setViewEntity(viewEntity); + viewInstanceEntity2.setViewEntity(viewEntity); expect(singleton.instanceExists(viewInstanceEntity)).andReturn(false); + expect(singleton.instanceExists(viewInstanceEntity2)).andReturn(false); expect(singleton.getInstanceDefinition("V1", "1.0.0", "I1")).andReturn(viewInstanceEntity); - expect(singleton.getDefinition("V1", null)).andReturn(viewEntity); + expect(singleton.getInstanceDefinition("V1", "1.0.0", "I1")).andReturn(viewInstanceEntity2); + expect(singleton.getDefinition("V1", null)).andReturn(viewEntity).anyTimes(); Capture instanceEntityCapture = new Capture(); singleton.installViewInstance(capture(instanceEntityCapture)); + expectLastCall().anyTimes(); + + expect(singleton.checkAdmin()).andReturn(true); + expect(singleton.checkAdmin()).andReturn(false); replay(singleton); + // as admin provider.createResources(PropertyHelper.getCreateRequest(properties, null)); + assertEquals(viewInstanceEntity, instanceEntityCapture.getValue()); + Map props = viewInstanceEntity.getPropertyMap(); + assertEquals(1, props.size()); + assertEquals("test_value", props.get("test_property")); - Assert.assertEquals(viewInstanceEntity, instanceEntityCapture.getValue()); + // as non-admin + provider.createResources(PropertyHelper.getCreateRequest(properties, null)); + assertEquals(viewInstanceEntity2, instanceEntityCapture.getValue()); + props = viewInstanceEntity2.getPropertyMap(); + assertTrue(props.isEmpty()); verify(singleton); } @@ -164,6 +198,8 @@ public class ViewInstanceResourceProviderTest { expect(singleton.getInstanceDefinition("V1", "1.0.0", "I1")).andReturn(viewInstanceEntity); expect(singleton.getDefinition("V1", null)).andReturn(viewEntity); + expect(singleton.checkAdmin()).andReturn(true); + replay(singleton); try { @@ -201,6 +237,8 @@ public class ViewInstanceResourceProviderTest { expect(singleton.getInstanceDefinition("V1", "1.0.0", "I1")).andReturn(viewInstanceEntity); expect(singleton.getDefinition("V1", null)).andReturn(viewEntity); + expect(singleton.checkAdmin()).andReturn(true); + replay(singleton); try { @@ -281,4 +319,4 @@ public class ViewInstanceResourceProviderTest { verify(singleton); } -} \ No newline at end of file +}