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

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<String, Object> 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<Class<? extends ConfiguredObject>> 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<String, Object> statsAsMap = (Map<String, Object>) 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<Map<String, Object>> childList = (List<Map<String, Object>>) 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<Map<String, Object>> childList = (List<Map<String, Object>>) 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<String, Object> 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<String, ConfiguredObjectAttribute<?, ?>> 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.<Class<? extends ConfiguredObject>>singleton(TestChild.class));
+        when(_configuredObject.getCategoryClass()).thenReturn(TestChild.class);
+        when(mockChild.isDurable()).thenReturn(true);
+
+        Map<String, Object> 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<String, Object> 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<String, Object> 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<String, Object> 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));
     }