Repository: ambari Updated Branches: refs/heads/trunk 1ad425fc3 -> e15399aa0
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/e15399aa Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/e15399aa Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/e15399aa Branch: refs/heads/trunk Commit: e15399aa05c944f19ade734e63c3a79c5b631848 Parents: 1ad425f Author: tbeerbower <[email protected]> Authored: Wed Oct 1 17:18:36 2014 -0400 Committer: tbeerbower <[email protected]> Committed: Tue Oct 14 09:16:37 2014 -0400 ---------------------------------------------------------------------- .../internal/ViewInstanceResourceProvider.java | 44 +++++++++++------- .../apache/ambari/server/view/ViewRegistry.java | 9 ++++ .../ViewInstanceResourceProviderTest.java | 48 ++++++++++++++++++-- 3 files changed, 80 insertions(+), 21 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/ambari/blob/e15399aa/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/e15399aa/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 7cf13bc..d525f46 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 @@ -704,6 +704,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/e15399aa/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..c0ecddc 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 {
