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 <[email protected]> Authored: Tue Oct 14 09:21:45 2014 -0400 Committer: tbeerbower <[email protected]> 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<String, String> properties = new HashMap<String, String>(); - 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<String, String> properties = new HashMap<String, String>(); + + 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<String, String> applicationData = new HashMap<String, String>(); String currentUserName = viewInstanceEntity.getCurrentUserName(); @@ -305,20 +311,26 @@ public class ViewInstanceResourceProvider extends AbstractResourceProvider { Collection<ViewInstancePropertyEntity> instanceProperties = new HashSet<ViewInstancePropertyEntity>(); + boolean isUserAdmin = viewRegistry.checkAdmin(); + for (Map.Entry<String, Object> 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.<ViewInstanceDataEntity>emptyList()); + expect(viewInstanceEntity.getData()).andReturn(Collections.<ViewInstanceDataEntity>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<String, Map<String, Object>> 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<ViewInstanceEntity> instanceEntityCapture = new Capture<ViewInstanceEntity>(); 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<String, String> 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 +}
