Repository: qpid-broker-j
Updated Branches:
  refs/heads/master 06d0619ed -> d7633ce17


QPID-7923: [Java Broker] [ACL] Allow specific attribute updates to be 
controlled by existing ACL mechanism


Project: http://git-wip-us.apache.org/repos/asf/qpid-broker-j/repo
Commit: http://git-wip-us.apache.org/repos/asf/qpid-broker-j/commit/d7633ce1
Tree: http://git-wip-us.apache.org/repos/asf/qpid-broker-j/tree/d7633ce1
Diff: http://git-wip-us.apache.org/repos/asf/qpid-broker-j/diff/d7633ce1

Branch: refs/heads/master
Commit: d7633ce1789fb48a6c6f1f808778eb9d776b41e2
Parents: 06d0619
Author: Alex Rudyy <oru...@apache.org>
Authored: Mon Sep 25 13:29:06 2017 +0100
Committer: Alex Rudyy <oru...@apache.org>
Committed: Mon Sep 25 15:10:01 2017 +0100

----------------------------------------------------------------------
 .../server/model/AbstractConfiguredObject.java  |  27 +++--
 .../qpid/server/model/VirtualHostTest.java      |  16 +--
 .../AbstractStandardVirtualHostNodeTest.java    |   6 +-
 .../access/config/AclRulePredicates.java        |   5 +
 .../server/security/access/config/Action.java   |   9 +-
 .../config/LegacyAccessControlAdapter.java      |  14 ++-
 .../access/config/ObjectProperties.java         |  59 +++++++++--
 .../access/config/AclRulePredicatesTest.java    |  32 ++++--
 .../security/access/config/ActionTest.java      |  34 +++++-
 .../config/LegacyAccessControlAdapterTest.java  | 103 +++++++++----------
 .../security/access/config/RuleSetTest.java     |  22 ++++
 .../plugin/servlet/rest/RestServlet.java        |  72 ++++++-------
 .../systest/rest/acl/VirtualHostACLTest.java    |  22 +++-
 13 files changed, 275 insertions(+), 146 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/d7633ce1/broker-core/src/main/java/org/apache/qpid/server/model/AbstractConfiguredObject.java
----------------------------------------------------------------------
diff --git 
a/broker-core/src/main/java/org/apache/qpid/server/model/AbstractConfiguredObject.java
 
b/broker-core/src/main/java/org/apache/qpid/server/model/AbstractConfiguredObject.java
index 8856be0..fb0a7fb 100644
--- 
a/broker-core/src/main/java/org/apache/qpid/server/model/AbstractConfiguredObject.java
+++ 
b/broker-core/src/main/java/org/apache/qpid/server/model/AbstractConfiguredObject.java
@@ -1697,14 +1697,10 @@ public abstract class AbstractConfiguredObject<X 
extends ConfiguredObject<X>> im
                 }
                 else
                 {
-                    ConfiguredObject<?> proxyForValidation =
-                            createProxyForValidation(Collections.<String, 
Object>singletonMap(
-                                    ConfiguredObject.DESIRED_STATE,
-                                    desiredState));
-                    Set<String> desiredStateOnlySet = 
Collections.unmodifiableSet(
-                            
Collections.singleton(ConfiguredObject.DESIRED_STATE));
-                    authoriseSetAttributes(proxyForValidation, 
desiredStateOnlySet);
-                    validateChange(proxyForValidation, desiredStateOnlySet);
+                    Map<String, Object> attributes = 
Collections.singletonMap(ConfiguredObject.DESIRED_STATE, desiredState);
+                    ConfiguredObject<?> proxyForValidation = 
createProxyForValidation(attributes);
+                    authoriseSetAttributes(proxyForValidation, attributes);
+                    validateChange(proxyForValidation, attributes.keySet());
 
                     if (desiredState == State.DELETED)
                     {
@@ -2668,7 +2664,7 @@ public abstract class AbstractConfiguredObject<X extends 
ConfiguredObject<X>> im
             @Override
             public Void execute()
             {
-                authoriseSetAttributes(createProxyForValidation(attributes), 
attributes.keySet());
+                authoriseSetAttributes(createProxyForValidation(attributes), 
attributes);
                 if (!isSystemProcess())
                 {
                     validateChange(createProxyForValidation(attributes), 
attributes.keySet());
@@ -2911,13 +2907,13 @@ public abstract class AbstractConfiguredObject<X 
extends ConfiguredObject<X>> im
     protected final <C extends ConfiguredObject<?>> void 
authoriseCreateChild(Class<C> childClass, Map<String, Object> attributes) 
throws AccessControlException
     {
         ConfiguredObject<?> configuredObject = 
createProxyForAuthorisation(childClass, attributes, this);
-        authorise(configuredObject, null, Operation.CREATE, 
Collections.<String,Object>emptyMap());
+        authorise(configuredObject, null, Operation.CREATE, 
Collections.emptyMap());
     }
 
     @Override
     public final void authorise(Operation operation) throws 
AccessControlException
     {
-        authorise(this, null, operation, 
Collections.<String,Object>emptyMap());
+        authorise(this, null, operation, Collections.emptyMap());
     }
 
     @Override
@@ -2989,10 +2985,10 @@ public abstract class AbstractConfiguredObject<X 
extends ConfiguredObject<X>> im
     }
 
 
-    protected final void authoriseSetAttributes(final ConfiguredObject<?> 
proxyForValidation,
-                                                               final 
Set<String> modifiedAttributes)
+    private final void authoriseSetAttributes(final ConfiguredObject<?> 
proxyForValidation,
+                                                               final 
Map<String, Object> modifiedAttributes)
     {
-        if (modifiedAttributes.contains(DESIRED_STATE) && 
State.DELETED.equals(proxyForValidation.getDesiredState()))
+        if (modifiedAttributes.containsKey(DESIRED_STATE) && 
State.DELETED.equals(proxyForValidation.getDesiredState()))
         {
             authorise(Operation.DELETE);
             if (modifiedAttributes.size() == 1)
@@ -3001,7 +2997,8 @@ public abstract class AbstractConfiguredObject<X extends 
ConfiguredObject<X>> im
                 return;
             }
         }
-        authorise(Operation.UPDATE);
+
+        authorise(this, null, Operation.UPDATE, modifiedAttributes);
     }
 
     protected Principal getSystemPrincipal()

http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/d7633ce1/broker-core/src/test/java/org/apache/qpid/server/model/VirtualHostTest.java
----------------------------------------------------------------------
diff --git 
a/broker-core/src/test/java/org/apache/qpid/server/model/VirtualHostTest.java 
b/broker-core/src/test/java/org/apache/qpid/server/model/VirtualHostTest.java
index 51ceed6..3dce931 100644
--- 
a/broker-core/src/test/java/org/apache/qpid/server/model/VirtualHostTest.java
+++ 
b/broker-core/src/test/java/org/apache/qpid/server/model/VirtualHostTest.java
@@ -24,6 +24,7 @@ import static org.mockito.Matchers.any;
 import static org.mockito.Matchers.anyString;
 import static org.mockito.Matchers.argThat;
 import static org.mockito.Matchers.eq;
+import static org.mockito.Matchers.same;
 import static org.mockito.Mockito.doAnswer;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.never;
@@ -347,7 +348,7 @@ public class VirtualHostTest extends QpidTestCase
         String virtualHostName = getName();
         VirtualHost<?> virtualHost = createVirtualHost(virtualHostName);
 
-        when(_mockAccessControl.authorise(null, Operation.UPDATE, virtualHost, 
Collections.<String,Object>emptyMap())).thenReturn(Result.DENIED);
+        when(_mockAccessControl.authorise(eq(null), eq(Operation.UPDATE), 
eq(virtualHost), any(Map.class))).thenReturn(Result.DENIED);
 
         assertNull(virtualHost.getDescription());
 
@@ -366,12 +367,13 @@ public class VirtualHostTest extends QpidTestCase
 
     public void testStopDeniedByACL()
     {
-
         String virtualHostName = getName();
         VirtualHost<?> virtualHost = createVirtualHost(virtualHostName);
 
-        when(_mockAccessControl.authorise(null, Operation.UPDATE,
-                virtualHost, 
Collections.<String,Object>emptyMap())).thenReturn(Result.DENIED);
+        when(_mockAccessControl.authorise(eq(null),
+                                          eq(Operation.UPDATE),
+                                          same(virtualHost),
+                                          
any(Map.class))).thenReturn(Result.DENIED);
 
         try
         {
@@ -391,8 +393,10 @@ public class VirtualHostTest extends QpidTestCase
         String virtualHostName = getName();
         VirtualHost<?> virtualHost = createVirtualHost(virtualHostName);
 
-        when(_mockAccessControl.authorise(null,
-                Operation.DELETE, virtualHost, 
Collections.<String,Object>emptyMap())).thenReturn(Result.DENIED);
+        when(_mockAccessControl.authorise(eq(null),
+                                          eq(Operation.DELETE),
+                                          same(virtualHost),
+                                          
any(Map.class))).thenReturn(Result.DENIED);
 
         try
         {

http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/d7633ce1/broker-core/src/test/java/org/apache/qpid/server/virtualhostnode/AbstractStandardVirtualHostNodeTest.java
----------------------------------------------------------------------
diff --git 
a/broker-core/src/test/java/org/apache/qpid/server/virtualhostnode/AbstractStandardVirtualHostNodeTest.java
 
b/broker-core/src/test/java/org/apache/qpid/server/virtualhostnode/AbstractStandardVirtualHostNodeTest.java
index d79b453..6e056b2 100644
--- 
a/broker-core/src/test/java/org/apache/qpid/server/virtualhostnode/AbstractStandardVirtualHostNodeTest.java
+++ 
b/broker-core/src/test/java/org/apache/qpid/server/virtualhostnode/AbstractStandardVirtualHostNodeTest.java
@@ -21,6 +21,8 @@
 package org.apache.qpid.server.virtualhostnode;
 
 import static org.mockito.Matchers.any;
+import static org.mockito.Matchers.eq;
+import static org.mockito.Matchers.same;
 import static org.mockito.Mockito.doNothing;
 import static org.mockito.Mockito.doThrow;
 import static org.mockito.Mockito.mock;
@@ -287,7 +289,7 @@ public class AbstractStandardVirtualHostNodeTest extends 
QpidTestCase
         node.open();
         node.start();
 
-        when(mockAccessControl.authorise(null, Operation.UPDATE, node, 
Collections.<String,Object>emptyMap())).thenReturn(Result.DENIED);
+        when(mockAccessControl.authorise(eq(null), eq(Operation.UPDATE), 
same(node), any())).thenReturn(Result.DENIED);
 
         assertNull(node.getDescription());
         try
@@ -350,7 +352,7 @@ public class AbstractStandardVirtualHostNodeTest extends 
QpidTestCase
         node.open();
         node.start();
 
-        when(mockAccessControl.authorise(null, Operation.UPDATE, node, 
Collections.<String,Object>emptyMap())).thenReturn(Result.DENIED);
+        when(mockAccessControl.authorise(eq(null), eq(Operation.UPDATE), 
same(node), any())).thenReturn(Result.DENIED);
 
         try
         {

http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/d7633ce1/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/AclRulePredicates.java
----------------------------------------------------------------------
diff --git 
a/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/AclRulePredicates.java
 
b/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/AclRulePredicates.java
index 2027de8..2ea4ad2 100644
--- 
a/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/AclRulePredicates.java
+++ 
b/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/AclRulePredicates.java
@@ -20,6 +20,7 @@ package org.apache.qpid.server.security.access.config;
 
 import java.util.Map;
 
+import com.google.common.collect.Sets;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -79,6 +80,10 @@ public class AclRulePredicates
             checkFirewallRuleNotAlreadyDefined(property.name(), value);
             _firewallRule = 
_firewallRuleFactory.createForNetwork(value.split(SEPARATOR));
         }
+        else if (property == Property.ATTRIBUTES)
+        {
+            
_properties.setAttributeNames(Sets.newHashSet((value.split(SEPARATOR))));
+        }
         else
         {
             _properties.put(property, value);

http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/d7633ce1/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/Action.java
----------------------------------------------------------------------
diff --git 
a/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/Action.java
 
b/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/Action.java
index c465909..1c2c298 100644
--- 
a/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/Action.java
+++ 
b/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/Action.java
@@ -83,7 +83,7 @@ public class Action
 
     public boolean matches(Action a)
     {
-        return operationsMatch(a) && objectTypesMatch(a) && propertiesMatch(a);
+        return operationsMatch(a) && objectTypesMatch(a) && propertiesMatch(a) 
&& attributesMatch(a);
     }
 
     private boolean operationsMatch(Action a)
@@ -101,7 +101,7 @@ public class Action
         boolean propertiesMatch = false;
         if (_properties != null)
         {
-            propertiesMatch = _properties.matches(a.getProperties());
+            propertiesMatch = _properties.propertiesMatch(a.getProperties());
         }
         else if (a.getProperties() == null)
         {
@@ -110,6 +110,11 @@ public class Action
         return propertiesMatch;
     }
 
+    private boolean attributesMatch(final Action action)
+    {
+        return getOperation() != LegacyOperation.UPDATE || 
getProperties().attributesMatch(action.getProperties());
+    }
+
     @Override
     public boolean equals(final Object o)
     {

http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/d7633ce1/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/LegacyAccessControlAdapter.java
----------------------------------------------------------------------
diff --git 
a/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/LegacyAccessControlAdapter.java
 
b/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/LegacyAccessControlAdapter.java
index 1dae6c3..34a3d5a 100644
--- 
a/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/LegacyAccessControlAdapter.java
+++ 
b/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/LegacyAccessControlAdapter.java
@@ -70,7 +70,9 @@ class LegacyAccessControlAdapter
         return _model;
     }
 
-    Result authorise(final LegacyOperation operation, final PermissionedObject 
configuredObject)
+    Result authorise(final LegacyOperation operation,
+                     final PermissionedObject configuredObject,
+                     final Map<String, Object> arguments)
     {
         if (isAllowedOperation(operation, configuredObject))
         {
@@ -85,6 +87,10 @@ class LegacyAccessControlAdapter
         }
 
         ObjectProperties properties = getACLObjectProperties(configuredObject, 
operation);
+        if (operation == LegacyOperation.UPDATE)
+        {
+            properties.setAttributeNames(arguments.keySet());
+        }
         LegacyOperation authoriseOperation = 
validateAuthoriseOperation(operation, categoryClass);
         return _accessControl.authorise(authoriseOperation, objectType, 
properties);
 
@@ -468,11 +474,11 @@ class LegacyAccessControlAdapter
         switch(operation.getType())
         {
             case CREATE:
-                return authorise(LegacyOperation.CREATE, configuredObject);
+                return authorise(LegacyOperation.CREATE, configuredObject, 
Collections.emptyMap());
             case UPDATE:
-                return authorise(LegacyOperation.UPDATE, configuredObject);
+                return authorise(LegacyOperation.UPDATE, configuredObject, 
arguments);
             case DELETE:
-                return authorise(LegacyOperation.DELETE, configuredObject);
+                return authorise(LegacyOperation.DELETE, configuredObject, 
Collections.emptyMap());
             case INVOKE_METHOD:
                 return authoriseMethod(configuredObject, operation.getName(), 
arguments);
             case PERFORM_ACTION:

http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/d7633ce1/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/ObjectProperties.java
----------------------------------------------------------------------
diff --git 
a/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/ObjectProperties.java
 
b/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/ObjectProperties.java
index a30c199..8cd57f0 100644
--- 
a/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/ObjectProperties.java
+++ 
b/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/ObjectProperties.java
@@ -21,15 +21,18 @@ package org.apache.qpid.server.security.access.config;
 import java.util.Collections;
 import java.util.EnumMap;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.Map;
+import java.util.Set;
 
+import com.google.common.base.Joiner;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 /**
  * An set of properties for an access control v2 rule {@link ObjectType}.
  *
- * The {@link #matches(ObjectProperties)} method is intended to be used when 
determining precedence of rules, and
+ * The {@link #propertiesMatch(ObjectProperties)} method is intended to be 
used when determining precedence of rules, and
  * {@link #equals(Object)} and {@link #hashCode()} are intended for use in 
maps. This is due to the wildcard matching
  * described above.
  */
@@ -39,6 +42,7 @@ public class ObjectProperties
     static final String WILD_CARD = "*";
 
     static final ObjectProperties EMPTY = new ObjectProperties();
+    private Set<String> _attributeNames;
 
     public enum Property
     {
@@ -58,7 +62,8 @@ public class ObjectProperties
         FROM_NETWORK,
         FROM_HOSTNAME,
         VIRTUALHOST_NAME,
-        METHOD_NAME;
+        METHOD_NAME,
+        ATTRIBUTES;
 
         private static final Map<String, Property> _canonicalNameToPropertyMap 
= new HashMap<String, ObjectProperties.Property>();
 
@@ -155,6 +160,16 @@ public class ObjectProperties
         _properties.put(Property.NAME, name);
     }
 
+    Set<String> getAttributeNames()
+    {
+        return _attributeNames;
+    }
+
+    void setAttributeNames(Set<String> attributeNames)
+    {
+        _attributeNames = attributeNames == null ? null : new 
HashSet<>(attributeNames);
+    }
+
     public String put(Property key, String value)
     {
         return _properties.put(key, value == null ? "" : value.trim());
@@ -168,19 +183,19 @@ public class ObjectProperties
         }
     }
 
-    public boolean matches(ObjectProperties properties)
+    boolean propertiesMatch(ObjectProperties other)
     {
-        if (properties._properties.keySet().isEmpty())
+        if (other._properties.keySet().isEmpty())
         {
             return true;
         }
 
-        if (!_properties.keySet().containsAll(properties._properties.keySet()))
+        if (!_properties.keySet().containsAll(other._properties.keySet()))
         {
             return false;
         }
 
-        for (Map.Entry<Property,String> entry : 
properties._properties.entrySet())
+        for (Map.Entry<Property,String> entry : other._properties.entrySet())
         {
             Property key = entry.getKey();
             String ruleValue = entry.getValue();
@@ -196,6 +211,12 @@ public class ObjectProperties
         return true;
     }
 
+    boolean attributesMatch(final ObjectProperties other)
+    {
+        return !(other._attributeNames != null
+                 && (_attributeNames == null || 
!other._attributeNames.containsAll(_attributeNames)));
+    }
+
     private boolean valueMatches(String thisValue, String ruleValue)
     {
         return (ruleValue == null
@@ -222,20 +243,38 @@ public class ObjectProperties
 
         final ObjectProperties that = (ObjectProperties) o;
 
-        return !(_properties != null ? !_properties.equals(that._properties) : 
that._properties != null);
-
+        if (_attributeNames != null ? 
!_attributeNames.equals(that._attributeNames) : that._attributeNames != null)
+        {
+            return false;
+        }
+        return _properties != null ? _properties.equals(that._properties) : 
that._properties == null;
     }
 
     @Override
     public int hashCode()
     {
-        return _properties != null ? _properties.hashCode() : 0;
+        int result = _attributeNames != null ? _attributeNames.hashCode() : 0;
+        result = 31 * result + (_properties != null ? _properties.hashCode() : 
0);
+        return result;
     }
 
     @Override
     public String toString()
     {
-        return _properties.toString();
+        StringBuilder sb = new StringBuilder();
+        Joiner joiner = Joiner.on(",");
+        joiner.withKeyValueSeparator("=").appendTo(sb, _properties);
+        if (_attributeNames != null && !_attributeNames.isEmpty())
+        {
+            if (!_properties.isEmpty())
+            {
+                sb.append(",");
+            }
+            sb.append("ATTRIBUTES=[");
+            joiner.appendTo(sb, _attributeNames);
+            sb.append("]");
+        }
+        return sb.toString();
     }
 
     public boolean isEmpty()

http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/d7633ce1/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/config/AclRulePredicatesTest.java
----------------------------------------------------------------------
diff --git 
a/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/config/AclRulePredicatesTest.java
 
b/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/config/AclRulePredicatesTest.java
index cbb8488..493ffd4 100644
--- 
a/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/config/AclRulePredicatesTest.java
+++ 
b/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/config/AclRulePredicatesTest.java
@@ -18,13 +18,22 @@
  */
 package org.apache.qpid.server.security.access.config;
 
-import static 
org.apache.qpid.server.security.access.config.ObjectProperties.Property.*;
+import static 
org.apache.qpid.server.security.access.config.ObjectProperties.Property.ATTRIBUTES;
+import static 
org.apache.qpid.server.security.access.config.ObjectProperties.Property.CLASS;
+import static 
org.apache.qpid.server.security.access.config.ObjectProperties.Property.FROM_HOSTNAME;
+import static 
org.apache.qpid.server.security.access.config.ObjectProperties.Property.FROM_NETWORK;
+import static 
org.apache.qpid.server.security.access.config.ObjectProperties.Property.NAME;
+import static org.mockito.Matchers.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
 
-import org.apache.qpid.server.security.access.firewall.FirewallRule;
-import org.apache.qpid.server.security.access.firewall.FirewallRuleFactory;
+import java.util.Set;
 
-import static org.mockito.Mockito.*;
+import org.mockito.internal.util.collections.Sets;
 
+import org.apache.qpid.server.security.access.firewall.FirewallRule;
+import org.apache.qpid.server.security.access.firewall.FirewallRuleFactory;
 import org.apache.qpid.test.utils.QpidTestCase;
 
 public class AclRulePredicatesTest extends QpidTestCase
@@ -38,8 +47,8 @@ public class AclRulePredicatesTest extends QpidTestCase
         super.setUp();
         _aclRulePredicates.setFirewallRuleFactory(_firewallRuleFactory);
 
-        when(_firewallRuleFactory.createForHostname((String[]) 
any())).thenReturn(mock(FirewallRule.class));
-        when(_firewallRuleFactory.createForNetwork((String[]) 
any())).thenReturn(mock(FirewallRule.class));
+        
when(_firewallRuleFactory.createForHostname(any())).thenReturn(mock(FirewallRule.class));
+        
when(_firewallRuleFactory.createForNetwork(any())).thenReturn(mock(FirewallRule.class));
     }
 
     public void testParse()
@@ -85,4 +94,15 @@ public class AclRulePredicatesTest extends QpidTestCase
             // pass
         }
     }
+
+    public void testParseAttributesRule()
+    {
+        String attributes = "attribute1,attribute2";
+        _aclRulePredicates.parse(ATTRIBUTES.name(), attributes);
+
+        final Set<String> attributesSet = Sets.newSet(attributes.split(","));
+        assertEquals("Unexpected attributes",
+                     attributesSet,
+                     
_aclRulePredicates.getObjectProperties().getAttributeNames());
+    }
 }

http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/d7633ce1/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/config/ActionTest.java
----------------------------------------------------------------------
diff --git 
a/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/config/ActionTest.java
 
b/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/config/ActionTest.java
index 88c14ff..1b022fa 100644
--- 
a/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/config/ActionTest.java
+++ 
b/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/config/ActionTest.java
@@ -20,6 +20,8 @@ package org.apache.qpid.server.security.access.config;
 
 import static org.mockito.Mockito.*;
 
+import java.util.Collections;
+
 import org.apache.qpid.test.utils.QpidTestCase;
 
 public class ActionTest extends QpidTestCase
@@ -29,7 +31,7 @@ public class ActionTest extends QpidTestCase
 
     public void testMatchesReturnsTrueForMatchingActions()
     {
-        when(_properties1.matches(_properties2)).thenReturn(true);
+        when(_properties1.propertiesMatch(_properties2)).thenReturn(true);
 
         assertMatches(
                 new Action(LegacyOperation.CONSUME, ObjectType.QUEUE, 
_properties1),
@@ -78,6 +80,36 @@ public class ActionTest extends QpidTestCase
                 new Action(LegacyOperation.CREATE, ObjectType.QUEUE, 
(ObjectProperties)null));
     }
 
+    public void testAttributesIgnoredForCreate()
+    {
+        final ObjectProperties objectProperties1 = new ObjectProperties();
+        objectProperties1.setAttributeNames(Collections.singleton("test1"));
+        final ObjectProperties objectProperties2 = new ObjectProperties();
+        objectProperties2.setAttributeNames(Collections.singleton("test2"));
+        assertMatches(new Action(LegacyOperation.CREATE, ObjectType.QUEUE, 
objectProperties1),
+                      new Action(LegacyOperation.CREATE, ObjectType.QUEUE, 
objectProperties2));
+    }
+
+    public void testAttributesDifferForUpdate()
+    {
+        final ObjectProperties objectProperties1 = new ObjectProperties();
+        objectProperties1.setAttributeNames(Collections.singleton("test1"));
+        final ObjectProperties objectProperties2 = new ObjectProperties();
+        objectProperties2.setAttributeNames(Collections.singleton("test2"));
+        assertDoesntMatch(new Action(LegacyOperation.UPDATE, ObjectType.QUEUE, 
objectProperties1),
+                          new Action(LegacyOperation.UPDATE, ObjectType.QUEUE, 
objectProperties2));
+    }
+
+    public void testAttributesMatchForUpdate()
+    {
+        final ObjectProperties objectProperties1 = new ObjectProperties();
+        objectProperties1.setAttributeNames(Collections.singleton("test1"));
+        final ObjectProperties objectProperties2 = new ObjectProperties();
+        objectProperties2.setAttributeNames(Collections.singleton("test1"));
+        assertMatches(new Action(LegacyOperation.UPDATE, ObjectType.QUEUE, 
objectProperties1),
+                      new Action(LegacyOperation.UPDATE, ObjectType.QUEUE, 
objectProperties2));
+    }
+
     private void assertMatches(Action action1, Action action2)
     {
         assertTrue(action1 + " should match " + action2, 
action1.matches(action2));

http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/d7633ce1/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/config/LegacyAccessControlAdapterTest.java
----------------------------------------------------------------------
diff --git 
a/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/config/LegacyAccessControlAdapterTest.java
 
b/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/config/LegacyAccessControlAdapterTest.java
index 96fdbb2..1604e05 100644
--- 
a/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/config/LegacyAccessControlAdapterTest.java
+++ 
b/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/config/LegacyAccessControlAdapterTest.java
@@ -141,7 +141,8 @@ public class LegacyAccessControlAdapterTest extends 
QpidTestCase
         properties.put(ObjectProperties.Property.DURABLE, true);
         properties.put(ObjectProperties.Property.EXCLUSIVE, false);
 
-        assertAuthorization(LegacyOperation.CREATE, consumer, 
LegacyOperation.CONSUME, ObjectType.QUEUE, properties);
+        assertAuthorization(LegacyOperation.CREATE, consumer, 
LegacyOperation.CONSUME, ObjectType.QUEUE, properties,
+                            Collections.emptyMap());
     }
 
 
@@ -164,7 +165,12 @@ public class LegacyAccessControlAdapterTest extends 
QpidTestCase
         when(mock.getCategoryClass()).thenReturn(User.class);
         when(mock.getParent()).thenReturn(authenticationProvider);
         ObjectProperties properties = new ObjectProperties(mock.getName());
-        assertUpdateAuthorization(mock, LegacyOperation.UPDATE, 
ObjectType.USER, properties);
+        
properties.setAttributeNames(Collections.singleton(ConfiguredObject.DESCRIPTION));
+        assertUpdateAuthorization(mock,
+                                  LegacyOperation.UPDATE,
+                                  ObjectType.USER,
+                                  properties,
+                                  
Collections.singletonMap(ConfiguredObject.DESCRIPTION, "Test"));
     }
 
 
@@ -295,6 +301,7 @@ public class LegacyAccessControlAdapterTest extends 
QpidTestCase
     {
         VirtualHost vh = getMockVirtualHost();
         ObjectProperties expectedProperties = 
createExpectedQueueObjectProperties();
+        
expectedProperties.setAttributeNames(Collections.singleton(ConfiguredObject.DESCRIPTION));
 
         Queue queueObject = mock(Queue.class);
         when(queueObject.getAttribute(Queue.NAME)).thenReturn(TEST_QUEUE);
@@ -305,13 +312,15 @@ public class LegacyAccessControlAdapterTest extends 
QpidTestCase
         when(queueObject.getParent()).thenReturn(vh);
         when(queueObject.getCategoryClass()).thenReturn(Queue.class);
 
-        assertUpdateAuthorization(queueObject, LegacyOperation.UPDATE, 
ObjectType.QUEUE, expectedProperties);
+        assertUpdateAuthorization(queueObject, LegacyOperation.UPDATE, 
ObjectType.QUEUE, expectedProperties,
+                                  
Collections.singletonMap(ConfiguredObject.DESCRIPTION, "Test"));
     }
 
     public void testAuthoriseUpdateExchange()
     {
         VirtualHost vh = getMockVirtualHost();
         ObjectProperties expectedProperties = 
createExpectedExchangeObjectProperties();
+        
expectedProperties.setAttributeNames(Collections.singleton(ConfiguredObject.DESCRIPTION));
 
         Exchange exchange = mock(Exchange.class);
         when(exchange.getName()).thenReturn(TEST_EXCHANGE);
@@ -321,7 +330,8 @@ public class LegacyAccessControlAdapterTest extends 
QpidTestCase
         when(exchange.getParent()).thenReturn(vh);
         when(exchange.getCategoryClass()).thenReturn(Exchange.class);
 
-        assertUpdateAuthorization(exchange, LegacyOperation.UPDATE, 
ObjectType.EXCHANGE, expectedProperties);
+        assertUpdateAuthorization(exchange, LegacyOperation.UPDATE, 
ObjectType.EXCHANGE, expectedProperties,
+                                  
Collections.singletonMap(ConfiguredObject.DESCRIPTION, "Test"));
     }
 
     public void testAuthoriseDeleteExchange()
@@ -461,7 +471,13 @@ public class LegacyAccessControlAdapterTest extends 
QpidTestCase
     public void testAuthoriseUpdateVirtualHostNode()
     {
         VirtualHostNode vhn = getMockVirtualHostNode();
-        assertUpdateAuthorization(vhn, LegacyOperation.UPDATE, 
ObjectType.VIRTUALHOSTNODE, new ObjectProperties(vhn.getName()));
+        ObjectProperties expectedProperties = new 
ObjectProperties(vhn.getName());
+        
expectedProperties.setAttributeNames(Collections.singleton(ConfiguredObject.DESCRIPTION));
+        assertUpdateAuthorization(vhn,
+                                  LegacyOperation.UPDATE,
+                                  ObjectType.VIRTUALHOSTNODE,
+                                  expectedProperties,
+                                  
Collections.singletonMap(ConfiguredObject.DESCRIPTION, "Test"));
     }
 
 
@@ -520,7 +536,8 @@ public class LegacyAccessControlAdapterTest extends 
QpidTestCase
         when(mock.getCategoryClass()).thenReturn(Group.class);
         when(mock.getParent()).thenReturn(groupProvider);
         ObjectProperties properties = new ObjectProperties(mock.getName());
-        assertUpdateAuthorization(mock, LegacyOperation.UPDATE, 
ObjectType.GROUP, properties);
+        
properties.setAttributeNames(Collections.singleton(ConfiguredObject.DESCRIPTION));
+        assertUpdateAuthorization(mock, LegacyOperation.UPDATE, 
ObjectType.GROUP, properties, 
Collections.singletonMap(ConfiguredObject.DESCRIPTION, "Test"));
     }
 
     public void testAuthoriseUpdateGroupMember()
@@ -533,7 +550,12 @@ public class LegacyAccessControlAdapterTest extends 
QpidTestCase
         when(mock.getCategoryClass()).thenReturn(GroupMember.class);
         when(mock.getParent()).thenReturn(group);
         ObjectProperties properties = new ObjectProperties(mock.getName());
-        assertUpdateAuthorization(mock, LegacyOperation.UPDATE, 
ObjectType.GROUP, properties);
+        
properties.setAttributeNames(Collections.singleton(ConfiguredObject.DESCRIPTION));
+        assertUpdateAuthorization(mock,
+                                  LegacyOperation.UPDATE,
+                                  ObjectType.GROUP,
+                                  properties,
+                                  
Collections.singletonMap(ConfiguredObject.DESCRIPTION, "Test"));
     }
 
     public void testAuthoriseUpdateVirtualHost()
@@ -546,8 +568,10 @@ public class LegacyAccessControlAdapterTest extends 
QpidTestCase
         when(virtualHost.getCategoryClass()).thenReturn(VirtualHost.class);
         when(virtualHost.getParent()).thenReturn(vhn);
         ObjectProperties properties = new 
ObjectProperties(virtualHost.getName());
+        
properties.setAttributeNames(Collections.singleton(ConfiguredObject.DESCRIPTION));
         properties.put(ObjectProperties.Property.VIRTUALHOST_NAME, 
virtualHost.getName());
-        assertUpdateAuthorization(virtualHost, LegacyOperation.UPDATE, 
ObjectType.VIRTUALHOST, properties);
+        assertUpdateAuthorization(virtualHost, LegacyOperation.UPDATE, 
ObjectType.VIRTUALHOST, properties,
+                                  
Collections.singletonMap(ConfiguredObject.DESCRIPTION, "Test"));
     }
 
     public void testAuthoriseDeleteVirtualHostNode()
@@ -624,44 +648,6 @@ public class LegacyAccessControlAdapterTest extends 
QpidTestCase
         assertBrokerChildDeleteAuthorization(mock);
     }
 
-
-    public void testAuthoriseVirtualHostLoggerOperations()
-    {
-        VirtualHostLogger mock = mock(VirtualHostLogger.class);
-        when(mock.getName()).thenReturn("TEST");
-        doReturn(VirtualHostLogger.class).when(mock).getCategoryClass();
-        when(mock.getParent()).thenReturn(_virtualHost);
-        when(mock.getModel()).thenReturn(BrokerModel.getInstance());
-
-        ObjectProperties properties = new ObjectProperties(mock.getName());
-        properties.put(ObjectProperties.Property.VIRTUALHOST_NAME, 
TEST_VIRTUAL_HOST);
-        assertCreateAuthorization(mock, LegacyOperation.CREATE, 
ObjectType.VIRTUALHOST, properties);
-        assertUpdateAuthorization(mock, LegacyOperation.UPDATE, 
ObjectType.VIRTUALHOST, properties);
-        assertDeleteAuthorization(mock, LegacyOperation.DELETE, 
ObjectType.VIRTUALHOST, properties);
-    }
-
-    public void testAuthoriseVirtualHostLogInclusionRuleOperations()
-    {
-        VirtualHostLogger vhl = mock(VirtualHostLogger.class);
-        when(vhl.getName()).thenReturn("LOGGER");
-        doReturn(VirtualHostLogger.class).when(vhl).getCategoryClass();
-        when(vhl.getParent()).thenReturn(_virtualHost);
-        when(vhl.getModel()).thenReturn(BrokerModel.getInstance());
-
-        VirtualHostLogInclusionRule mock = 
mock(VirtualHostLogInclusionRule.class);
-        when(mock.getName()).thenReturn("TEST");
-        
doReturn(VirtualHostLogInclusionRule.class).when(mock).getCategoryClass();
-        when(mock.getParent()).thenReturn(vhl);
-        when(mock.getModel()).thenReturn(BrokerModel.getInstance());
-
-        ObjectProperties properties = new ObjectProperties(mock.getName());
-        properties.put(ObjectProperties.Property.VIRTUALHOST_NAME, 
TEST_VIRTUAL_HOST);
-
-        assertCreateAuthorization(mock, LegacyOperation.CREATE, 
ObjectType.VIRTUALHOST, properties);
-        assertUpdateAuthorization(mock, LegacyOperation.UPDATE, 
ObjectType.VIRTUALHOST, properties);
-        assertDeleteAuthorization(mock, LegacyOperation.DELETE, 
ObjectType.VIRTUALHOST, properties);
-    }
-
     public void testAuthoriseInvokeVirtualHostDescendantMethod()
     {
         String methodName = "clearQueue";
@@ -835,7 +821,6 @@ public class LegacyAccessControlAdapterTest extends 
QpidTestCase
 
     public void testAuthoriseCreateConnection()
     {
-
         ObjectProperties properties = new ObjectProperties();
         properties.put(ObjectProperties.Property.NAME, TEST_VIRTUAL_HOST);
         properties.put(ObjectProperties.Property.VIRTUALHOST_NAME, 
TEST_VIRTUAL_HOST);
@@ -845,7 +830,6 @@ public class LegacyAccessControlAdapterTest extends 
QpidTestCase
         verify(_accessControl).authorise(eq(LegacyOperation.ACCESS), 
eq(ObjectType.VIRTUALHOST), eq(properties));
     }
 
-
     private ObjectProperties createExpectedQueueObjectProperties()
     {
         ObjectProperties properties = new ObjectProperties();
@@ -886,8 +870,12 @@ public class LegacyAccessControlAdapterTest extends 
QpidTestCase
                                            ObjectType aclObjectType,
                                            ObjectProperties expectedProperties)
     {
-        _adapter.authorise(LegacyOperation.CREATE, configuredObject);
-        verify(_accessControl).authorise(eq(aclOperation), eq(aclObjectType), 
eq(expectedProperties));
+        assertAuthorization(LegacyOperation.CREATE,
+                            configuredObject,
+                            aclOperation,
+                            aclObjectType,
+                            expectedProperties,
+                            Collections.emptyMap());
     }
 
 
@@ -898,17 +886,19 @@ public class LegacyAccessControlAdapterTest extends 
QpidTestCase
                                            
configuredObject.getCategoryClass().getSimpleName().toLowerCase(),
                                            configuredObject.getName());
         ObjectProperties properties = new OperationLoggingDetails(description);
+        
properties.setAttributeNames(Collections.singleton(ConfiguredObject.DESCRIPTION));
 
         assertUpdateAuthorization(configuredObject, LegacyOperation.CONFIGURE, 
ObjectType.BROKER,
-                                  properties);
+                                  properties, 
Collections.singletonMap(ConfiguredObject.DESCRIPTION, "Test"));
     }
 
     private void assertUpdateAuthorization(ConfiguredObject<?> 
configuredObject,
                                            LegacyOperation aclOperation,
                                            ObjectType aclObjectType,
-                                           ObjectProperties expectedProperties)
+                                           ObjectProperties 
expectedProperties, final Map<String, Object> attributes)
     {
-        assertAuthorization(LegacyOperation.UPDATE, configuredObject, 
aclOperation, aclObjectType, expectedProperties);
+        assertAuthorization(LegacyOperation.UPDATE, configuredObject, 
aclOperation, aclObjectType, expectedProperties,
+                            attributes);
     }
 
     private void assertBrokerChildDeleteAuthorization(ConfiguredObject 
configuredObject)
@@ -929,16 +919,17 @@ public class LegacyAccessControlAdapterTest extends 
QpidTestCase
                                            ObjectType aclObjectType,
                                            ObjectProperties expectedProperties)
     {
-        assertAuthorization(LegacyOperation.DELETE, configuredObject, 
aclOperation, aclObjectType, expectedProperties);
+        assertAuthorization(LegacyOperation.DELETE, configuredObject, 
aclOperation, aclObjectType, expectedProperties,
+                            Collections.emptyMap());
     }
 
     private void assertAuthorization(LegacyOperation operation,
                                      ConfiguredObject<?> configuredObject,
                                      LegacyOperation aclOperation,
                                      ObjectType aclObjectType,
-                                     ObjectProperties expectedProperties)
+                                     ObjectProperties expectedProperties, 
final Map<String, Object> attributes)
     {
-        _adapter.authorise(operation, configuredObject);
+        _adapter.authorise(operation, configuredObject, attributes);
         verify(_accessControl).authorise(eq(aclOperation), eq(aclObjectType), 
eq(expectedProperties));
     }
 }

http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/d7633ce1/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/config/RuleSetTest.java
----------------------------------------------------------------------
diff --git 
a/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/config/RuleSetTest.java
 
b/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/config/RuleSetTest.java
index 607bc28..72db732 100644
--- 
a/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/config/RuleSetTest.java
+++ 
b/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/config/RuleSetTest.java
@@ -23,6 +23,8 @@ package org.apache.qpid.server.security.access.config;
 
 import static org.mockito.Mockito.mock;
 
+import java.util.Collections;
+
 import javax.security.auth.Subject;
 
 import org.apache.qpid.server.logging.EventLoggerProvider;
@@ -484,4 +486,24 @@ public class RuleSetTest extends QpidTestCase
 
         assertEquals(Result.ALLOWED, 
ruleSet.check(subjectInAllowedGroupAndOneOther, LegacyOperation.ACCESS, 
ObjectType.VIRTUALHOST, ObjectProperties.EMPTY));
     }
+
+    public void testUpdatedAllowedAttribute()
+    {
+        final ObjectProperties ruleProperties = new ObjectProperties();
+        ruleProperties.setAttributeNames(Collections.singleton("attribute1"));
+        _ruleSetCreator.addRule(1, TEST_USER, RuleOutcome.ALLOW,  
LegacyOperation.UPDATE, ObjectType.VIRTUALHOST, ruleProperties);
+        _ruleSetCreator.addRule(2, TEST_USER, RuleOutcome.DENY,  
LegacyOperation.UPDATE, ObjectType.VIRTUALHOST, ObjectProperties.EMPTY);
+        RuleSet ruleSet = createRuleSet();
+
+        final ObjectProperties updateProperties = new ObjectProperties();
+        assertEquals(Result.DENIED, ruleSet.check(_testSubject, 
LegacyOperation.UPDATE, ObjectType.VIRTUALHOST, updateProperties));
+
+        
updateProperties.setAttributeNames(Collections.singleton("attribute2"));
+
+        assertEquals(Result.DENIED, ruleSet.check(_testSubject, 
LegacyOperation.UPDATE, ObjectType.VIRTUALHOST, updateProperties));
+
+        
updateProperties.setAttributeNames(Collections.singleton("attribute1"));
+        assertEquals(Result.ALLOWED, ruleSet.check(_testSubject, 
LegacyOperation.UPDATE, ObjectType.VIRTUALHOST, updateProperties));
+    }
+
 }

http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/d7633ce1/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/rest/RestServlet.java
----------------------------------------------------------------------
diff --git 
a/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/rest/RestServlet.java
 
b/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/rest/RestServlet.java
index 278181c..1d42212 100644
--- 
a/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/rest/RestServlet.java
+++ 
b/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/rest/RestServlet.java
@@ -1,19 +1,22 @@
-/**
- * Licensed to the Apache Software Foundation (ASF) under one or more
- * contributor license agreements.  See the NOTICE file distributed with
- * this work for additional information regarding copyright ownership.
- * The ASF licenses this file to You under the Apache License, Version 2.0
- * (the "License"); you may not use this file except in compliance with
- * the License.  You may obtain a copy of the License at
- * <p/>
- * http://www.apache.org/licenses/LICENSE-2.0
- * <p/>
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
  */
+
 package org.apache.qpid.server.management.plugin.servlet.rest;
 
 import static 
org.apache.qpid.server.management.plugin.HttpManagementConfiguration.DEFAULT_PREFERENCE_OPERATION_TIMEOUT;
@@ -405,9 +408,8 @@ public class RestServlet extends AbstractServlet
 
                 if (isFullObjectURL)
                 {
-                    providedObject.put("name", names.get(names.size() - 1));
-                    ConfiguredObject<?> configuredObject =
-                            findObjectToUpdateInParent(objClass, 
providedObject, theParent);
+                    String name = names.get(names.size() - 1);
+                    ConfiguredObject<?> configuredObject = 
theParent.getChildByName(objClass, name);
 
                     if (configuredObject != null)
                     {
@@ -417,12 +419,15 @@ public class RestServlet extends AbstractServlet
                     }
                     else if ("POST".equalsIgnoreCase(request.getMethod()))
                     {
-                        sendJsonErrorResponse(request, response, 
HttpServletResponse.SC_NOT_FOUND, "Object with "
-                                                                               
                    + (providedObject.containsKey(
-                                "id") ? " id '" + providedObject.get("id") : " 
name '" + providedObject.get("name"))
-                                                                               
                    + "' does not exist!");
+                        sendJsonErrorResponse(request, response,
+                                              HttpServletResponse.SC_NOT_FOUND,
+                                              String.format("%s '%s' not 
found", configuredClass.getSimpleName(), name));
                         return;
                     }
+                    else
+                    {
+                        providedObject.put(ConfiguredObject.NAME, name);
+                    }
                 }
 
                 ConfiguredObject<?> configuredObject = 
theParent.createChild(objClass, providedObject);
@@ -683,10 +688,8 @@ public class RestServlet extends AbstractServlet
                         finder.findObjectParentsFromPath(names, hierarchy, 
configuredClass);
                 theParent = parent;
             }
-            Class<? extends ConfiguredObject> objClass = configuredClass;
-            Map<String, Object> objectName =
-                    Collections.<String, Object>singletonMap("name", 
names.get(names.size() - 1));
-            target = findObjectToUpdateInParent(objClass, objectName, 
theParent);
+            String name = names.get(names.size() - 1);
+            target = theParent.getChildByName(configuredClass, name);
             if (target == null)
             {
 
@@ -821,23 +824,6 @@ public class RestServlet extends AbstractServlet
         return providedObject;
     }
 
-    private ConfiguredObject<?> findObjectToUpdateInParent(Class<? extends 
ConfiguredObject> objClass,
-                                                           Map<String, Object> 
providedObject,
-                                                           ConfiguredObject 
theParent)
-    {
-        Collection<? extends ConfiguredObject> existingChildren = 
theParent.getChildren(objClass);
-
-        for (ConfiguredObject obj : existingChildren)
-        {
-            if ((providedObject.containsKey("id") && 
String.valueOf(providedObject.get("id")).equals(obj.getId().toString()))
-                    || (obj.getName().equals(providedObject.get("name")) ))
-            {
-                return obj;
-            }
-        }
-        return null;
-    }
-
     private void setResponseStatus(HttpServletRequest request, 
HttpServletResponse response, Throwable e)
             throws IOException
     {

http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/d7633ce1/systests/src/test/java/org/apache/qpid/systest/rest/acl/VirtualHostACLTest.java
----------------------------------------------------------------------
diff --git 
a/systests/src/test/java/org/apache/qpid/systest/rest/acl/VirtualHostACLTest.java
 
b/systests/src/test/java/org/apache/qpid/systest/rest/acl/VirtualHostACLTest.java
index e4fea73..cd4636b 100644
--- 
a/systests/src/test/java/org/apache/qpid/systest/rest/acl/VirtualHostACLTest.java
+++ 
b/systests/src/test/java/org/apache/qpid/systest/rest/acl/VirtualHostACLTest.java
@@ -20,6 +20,7 @@
 package org.apache.qpid.systest.rest.acl;
 
 import java.io.File;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
 
@@ -42,16 +43,18 @@ public class VirtualHostACLTest extends QpidRestTestCase
 
     private static final String ALLOWED_USER = "user1";
     private static final String DENIED_USER = "user2";
+    private static final String RESTRICTED_USER = "restricted";
 
     @Override
     protected void customizeConfiguration() throws Exception
     {
         super.customizeConfiguration();
         final TestBrokerConfiguration defaultBrokerConfiguration = 
getDefaultBrokerConfiguration();
-        
defaultBrokerConfiguration.configureTemporaryPasswordFile(ALLOWED_USER, 
DENIED_USER);
+        
defaultBrokerConfiguration.configureTemporaryPasswordFile(ALLOWED_USER, 
DENIED_USER, RESTRICTED_USER);
 
         AbstractACLTestCase.writeACLFileUtil(this, "ACL ALLOW-LOG ALL ACCESS 
MANAGEMENT",
                 "ACL ALLOW-LOG " + ALLOWED_USER + " ALL VIRTUALHOST",
+                "ACL ALLOW-LOG " + RESTRICTED_USER + " ALL VIRTUALHOST 
attributes=\"description\"",
                 "ACL DENY-LOG " + DENIED_USER + " ALL VIRTUALHOST",
                 "ACL DENY-LOG ALL ALL");
 
@@ -96,6 +99,23 @@ public class VirtualHostACLTest extends QpidRestTestCase
         assertVirtualHostExists(TEST2_VIRTUALHOST, TEST2_VIRTUALHOST);
     }
 
+    public void testUpdateRestrictedAttributes() throws Exception
+    {
+        getRestTestHelper().setUsernameAndPassword(RESTRICTED_USER, 
RESTRICTED_USER);
+
+        String virtualHostUrl = "virtualhost/" + TEST2_VIRTUALHOST + "/" + 
TEST2_VIRTUALHOST;
+        getRestTestHelper().submitRequest(virtualHostUrl,
+                                          "PUT",
+                                          
Collections.singletonMap(VirtualHost.CONTEXT,
+                                                                   
Collections.singletonMap("test1", "test2")),
+                                          HttpServletResponse.SC_FORBIDDEN);
+
+        getRestTestHelper().submitRequest(virtualHostUrl,
+                                          "PUT",
+                                          
Collections.singletonMap(VirtualHost.DESCRIPTION, "Test Description"),
+                                          HttpServletResponse.SC_OK);
+    }
+
     public void testUpdateVirtualHostDenied() throws Exception
     {
         getRestTestHelper().setUsernameAndPassword(DENIED_USER, DENIED_USER);


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@qpid.apache.org
For additional commands, e-mail: commits-h...@qpid.apache.org

Reply via email to