From 5edd1e5f5fac729d719219a1854a97419112e784 Mon Sep 17 00:00:00 2001 From: Alex Rudyy Date: Mon, 9 Feb 2015 15:05:37 +0000 Subject: QPID-6365: [Java Broker] Only expose secure attribute values when transport is SSL and initial configuration is requested git-svn-id: https://svn.apache.org/repos/asf/qpid/trunk@1658424 13f79535-47bb-0310-9956-ffa450edef68 --- .../rest/ConfiguredObjectToMapConverter.java | 24 +++-- .../plugin/servlet/rest/RestServlet.java | 2 +- .../rest/ConfiguredObjectToMapConverterTest.java | 105 +++++++++++++++++++-- .../apache/qpid/systest/rest/KeyStoreRestTest.java | 6 +- 4 files changed, 118 insertions(+), 19 deletions(-) diff --git a/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/rest/ConfiguredObjectToMapConverter.java b/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/rest/ConfiguredObjectToMapConverter.java index 28bbac2a61..331b50ea7c 100644 --- a/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/rest/ConfiguredObjectToMapConverter.java +++ b/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/rest/ConfiguredObjectToMapConverter.java @@ -57,13 +57,14 @@ public class ConfiguredObjectToMapConverter final boolean inheritedActuals, final boolean includeSystemContext, final boolean extractAsConfig, - final int oversizeThreshold + final int oversizeThreshold, + final boolean isSecureTransport ) { Map object = new LinkedHashMap<>(); incorporateAttributesIntoMap(confObject, object, useActualValues, inheritedActuals, includeSystemContext, - extractAsConfig, oversizeThreshold); + extractAsConfig, oversizeThreshold, isSecureTransport); if(!extractAsConfig) { incorporateStatisticsIntoMap(confObject, object); @@ -72,7 +73,7 @@ public class ConfiguredObjectToMapConverter if(depth > 0) { incorporateChildrenIntoMap(confObject, clazz, depth, object, useActualValues, inheritedActuals, - includeSystemContext, extractAsConfig, oversizeThreshold); + includeSystemContext, extractAsConfig, oversizeThreshold, isSecureTransport); } return object; } @@ -85,7 +86,8 @@ public class ConfiguredObjectToMapConverter final boolean inheritedActuals, final boolean includeSystemContext, final boolean extractAsConfig, - final int oversizeThreshold) + final int oversizeThreshold, + final boolean isSecureTransport) { // if extracting as config add a fake attribute for each secondary parent if(extractAsConfig && confObject.getModel().getParentTypes(confObject.getCategoryClass()).size()>1) @@ -159,6 +161,14 @@ public class ConfiguredObjectToMapConverter .getTypeRegistry() .getAttributeTypes(confObject.getClass()) .get(name); + + if (attribute.isSecure() && !(isSecureTransport && extractAsConfig)) + { + // do not expose actual secure attribute value + // getAttribute() returns encoded value + value = confObject.getAttribute(name); + } + if(attribute.isOversized() && !extractAsConfig) { String valueString = String.valueOf(value); @@ -240,7 +250,8 @@ public class ConfiguredObjectToMapConverter final boolean inheritedActuals, final boolean includeSystemContext, final boolean extractAsConfig, - final int oversizeThreshold) + final int oversizeThreshold, + final boolean isSecure) { List> childTypes = new ArrayList<>(confObject.getModel().getChildTypes(clazz)); @@ -283,7 +294,8 @@ public class ConfiguredObjectToMapConverter inheritedActuals, includeSystemContext, extractAsConfig, - oversizeThreshold)); + oversizeThreshold, + isSecure)); } } diff --git a/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/rest/RestServlet.java b/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/rest/RestServlet.java index 1ed0741a8e..19d498e240 100644 --- a/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/rest/RestServlet.java +++ b/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/rest/RestServlet.java @@ -371,7 +371,7 @@ public class RestServlet extends AbstractServlet { output.add(_objectConverter.convertObjectToMap(configuredObject, getConfiguredClass(), - depth, actuals, inheritedActuals, includeSystemContext, extractInitialConfig, oversizeThreshold)); + depth, actuals, inheritedActuals, includeSystemContext, extractInitialConfig, oversizeThreshold, request.isSecure())); } diff --git a/qpid/java/broker-plugins/management-http/src/test/java/org/apache/qpid/server/management/plugin/servlet/rest/ConfiguredObjectToMapConverterTest.java b/qpid/java/broker-plugins/management-http/src/test/java/org/apache/qpid/server/management/plugin/servlet/rest/ConfiguredObjectToMapConverterTest.java index b5847ad293..b3c9bd911f 100644 --- a/qpid/java/broker-plugins/management-http/src/test/java/org/apache/qpid/server/management/plugin/servlet/rest/ConfiguredObjectToMapConverterTest.java +++ b/qpid/java/broker-plugins/management-http/src/test/java/org/apache/qpid/server/management/plugin/servlet/rest/ConfiguredObjectToMapConverterTest.java @@ -68,7 +68,8 @@ public class ConfiguredObjectToMapConverterTest extends TestCase false, false, false, - 120); + 120, + false); Map statsAsMap = (Map) resultMap.get(STATISTICS_MAP_KEY); assertNotNull("Statistics should be part of map", statsAsMap); assertEquals("Unexpected number of statistics", 1, statsAsMap.size()); @@ -90,7 +91,8 @@ public class ConfiguredObjectToMapConverterTest extends TestCase false, false, false, - 120); + 120, + false); assertEquals("Unexpected number of attributes", 1, resultMap.size()); assertEquals("Unexpected attribute value", attributeValue, resultMap.get(attributeName)); } @@ -114,7 +116,8 @@ public class ConfiguredObjectToMapConverterTest extends TestCase false, false, false, - 120); + 120, + false); assertEquals("Unexpected number of attributes", 1, resultMap.size()); assertEquals("Unexpected attribute value", "attributeConfiguredObjectName", resultMap.get(attributeName)); } @@ -139,7 +142,8 @@ public class ConfiguredObjectToMapConverterTest extends TestCase false, false, false, - 120); + 120, + false); assertEquals("Unexpected parent map size", 1, resultMap.size()); final List> childList = (List>) resultMap.get("testchilds"); @@ -183,7 +187,8 @@ public class ConfiguredObjectToMapConverterTest extends TestCase false, false, false, - 120); + 120, + false); assertEquals("Unexpected parent map size", 2, resultMap.size()); assertEquals("Incorrect context", resultMap.get(ConfiguredObject.CONTEXT), actualContext); List> childList = (List>) resultMap.get("testchilds"); @@ -201,7 +206,8 @@ public class ConfiguredObjectToMapConverterTest extends TestCase false, false, false, - 120); + 120, + false); assertEquals("Unexpected parent map size", 2, resultMap.size()); Map inheritedContext = new HashMap<>(); inheritedContext.put("key","value"); @@ -243,7 +249,8 @@ public class ConfiguredObjectToMapConverterTest extends TestCase false, false, false, - 20); + 20, + false); Object children = resultMap.get("testchilds"); assertNotNull(children); assertTrue(children instanceof Collection); @@ -261,7 +268,8 @@ public class ConfiguredObjectToMapConverterTest extends TestCase false, false, false, - 8); + 8, + false); children = resultMap.get("testchilds"); assertNotNull(children); @@ -283,7 +291,8 @@ public class ConfiguredObjectToMapConverterTest extends TestCase false, false, false, - 8); + 8, + false); children = resultMap.get("testchilds"); assertNotNull(children); @@ -296,6 +305,84 @@ public class ConfiguredObjectToMapConverterTest extends TestCase } + public void testSecureAttributes() + { + + Model model = createTestModel(); + ConfiguredObjectTypeRegistry typeRegistry = model.getTypeRegistry(); + Map> attributeTypes = typeRegistry.getAttributeTypes(TestChild.class); + ConfiguredObjectAttribute secureAttribute = mock(ConfiguredObjectAttribute.class); + when(secureAttribute.isSecure()).thenReturn(true); + when(attributeTypes.get(eq("secureAttribute"))).thenReturn(secureAttribute); + + TestChild mockChild = mock(TestChild.class); + when(mockChild.getModel()).thenReturn(model); + when(_configuredObject.getModel()).thenReturn(model); + + // set encoded value + configureMockToReturnOneAttribute(mockChild, "secureAttribute", "*****"); + + // set actual values + when(mockChild.getActualAttributes()).thenReturn(Collections.singletonMap("secureAttribute", "secret")); + when(_configuredObject.getChildren(TestChild.class)).thenReturn(Arrays.asList(mockChild)); + when(model.getParentTypes(TestChild.class)).thenReturn(Collections.>singleton(TestChild.class)); + when(_configuredObject.getCategoryClass()).thenReturn(TestChild.class); + when(mockChild.isDurable()).thenReturn(true); + + Map resultMap = _converter.convertObjectToMap(_configuredObject, + ConfiguredObject.class, + 1, + false, + false, + false, + false, + 20, + false); + Object children = resultMap.get("testchilds"); + assertNotNull(children); + assertTrue(children instanceof Collection); + assertTrue(((Collection)children).size()==1); + Object attrs = ((Collection)children).iterator().next(); + assertTrue(attrs instanceof Map); + assertEquals("*****", ((Map) attrs).get("secureAttribute")); + + resultMap = _converter.convertObjectToMap(_configuredObject, + ConfiguredObject.class, + 1, + true, + true, + false, + true, + 20, + true); + + children = resultMap.get("testchilds"); + assertNotNull(children); + assertTrue(children instanceof Collection); + assertTrue(((Collection)children).size()==1); + attrs = ((Collection)children).iterator().next(); + assertTrue(attrs instanceof Map); + assertEquals("secret", ((Map) attrs).get("secureAttribute")); + + resultMap = _converter.convertObjectToMap(_configuredObject, + ConfiguredObject.class, + 1, + true, + true, + false, + false, + 20, + true); + + children = resultMap.get("testchilds"); + assertNotNull(children); + assertTrue(children instanceof Collection); + assertTrue(((Collection)children).size()==1); + attrs = ((Collection)children).iterator().next(); + assertTrue(attrs instanceof Map); + assertEquals("*****", ((Map) attrs).get("secureAttribute")); + } + private Model createTestModel() { Model model = mock(Model.class); diff --git a/qpid/java/systests/src/test/java/org/apache/qpid/systest/rest/KeyStoreRestTest.java b/qpid/java/systests/src/test/java/org/apache/qpid/systest/rest/KeyStoreRestTest.java index b0e396cf8e..a19c2e698c 100644 --- a/qpid/java/systests/src/test/java/org/apache/qpid/systest/rest/KeyStoreRestTest.java +++ b/qpid/java/systests/src/test/java/org/apache/qpid/systest/rest/KeyStoreRestTest.java @@ -78,7 +78,7 @@ public class KeyStoreRestTest extends QpidRestTestCase Map keystore = keyStores.get(0); assertEquals("Unexpected name", name, keystore.get(KeyStore.NAME)); assertEquals("unexpected path to key store", TestSSLConstants.KEYSTORE, keystore.get(FileKeyStore.STORE_URL)); - assertEquals("unexpected password", TestSSLConstants.KEYSTORE_PASSWORD, keystore.get(FileKeyStore.PASSWORD)); + assertEquals("unexpected password", AbstractConfiguredObject.SECURED_STRING_VALUE, keystore.get(FileKeyStore.PASSWORD)); assertEquals("unexpected alias", certAlias, keystore.get(FileKeyStore.CERTIFICATE_ALIAS)); } @@ -100,7 +100,7 @@ public class KeyStoreRestTest extends QpidRestTestCase Map keystore = keyStores.get(0); assertEquals("Unexpected name", name, keystore.get(KeyStore.NAME)); assertEquals("unexpected data", ConfiguredObject.OVER_SIZED_ATTRIBUTE_ALTERNATIVE_TEXT, keystore.get(FileKeyStore.STORE_URL)); - assertEquals("unexpected password", TestSSLConstants.KEYSTORE_PASSWORD, keystore.get(FileKeyStore.PASSWORD)); + assertEquals("unexpected password", AbstractConfiguredObject.SECURED_STRING_VALUE, keystore.get(FileKeyStore.PASSWORD)); assertEquals("unexpected alias", null, keystore.get(FileKeyStore.CERTIFICATE_ALIAS)); } @@ -152,7 +152,7 @@ public class KeyStoreRestTest extends QpidRestTestCase Map keystore = keyStores.get(0); assertEquals("Unexpected name", name, keystore.get(KeyStore.NAME)); assertEquals("unexpected data", TestSSLConstants.UNTRUSTED_KEYSTORE, keystore.get(FileKeyStore.STORE_URL)); - assertEquals("unexpected password", TestSSLConstants.KEYSTORE_PASSWORD, keystore.get(FileKeyStore.PASSWORD)); + assertEquals("unexpected password", AbstractConfiguredObject.SECURED_STRING_VALUE, keystore.get(FileKeyStore.PASSWORD)); assertEquals("unexpected alias", null, keystore.get(FileKeyStore.CERTIFICATE_ALIAS)); } -- cgit v1.2.1