From b66b4f357a756449c7e7184be4d963fb36f5b2d4 Mon Sep 17 00:00:00 2001 From: Alex Rudyy Date: Thu, 12 Mar 2015 14:33:05 +0000 Subject: QPID-6436: Address review comments and fix issues caused by ACL refactoring git-svn-id: https://svn.apache.org/repos/asf/qpid/trunk@1666212 13f79535-47bb-0310-9956-ffa450edef68 --- .../berkeleydb/BDBHAReplicaVirtualHostImpl.java | 5 +- .../qpid/server/security/SecurityManager.java | 59 +++++++++++++--------- .../RedirectingVirtualHostImpl.java | 5 +- .../model/testmodels/TestSecurityManager.java | 40 +++++++++++++++ .../model/testmodels/hierarchy/TestKitCarImpl.java | 3 +- .../testmodels/hierarchy/TestStandardCarImpl.java | 3 +- .../testmodels/lifecycle/TestConfiguredObject.java | 3 +- .../testmodels/singleton/TestSingletonImpl.java | 6 ++- .../virtualhost/AbstractVirtualHostTest.java | 1 + 9 files changed, 91 insertions(+), 34 deletions(-) create mode 100644 qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/testmodels/TestSecurityManager.java diff --git a/qpid/java/bdbstore/src/main/java/org/apache/qpid/server/virtualhost/berkeleydb/BDBHAReplicaVirtualHostImpl.java b/qpid/java/bdbstore/src/main/java/org/apache/qpid/server/virtualhost/berkeleydb/BDBHAReplicaVirtualHostImpl.java index 205ff57fab..30fff154bb 100644 --- a/qpid/java/bdbstore/src/main/java/org/apache/qpid/server/virtualhost/berkeleydb/BDBHAReplicaVirtualHostImpl.java +++ b/qpid/java/bdbstore/src/main/java/org/apache/qpid/server/virtualhost/berkeleydb/BDBHAReplicaVirtualHostImpl.java @@ -47,6 +47,7 @@ import org.apache.qpid.server.model.port.AmqpPort; import org.apache.qpid.server.protocol.AMQConnectionModel; import org.apache.qpid.server.protocol.LinkRegistry; import org.apache.qpid.server.queue.AMQQueue; +import org.apache.qpid.server.security.SecurityManager; import org.apache.qpid.server.stats.StatisticsCounter; import org.apache.qpid.server.store.DurableConfigurationStore; import org.apache.qpid.server.store.MessageStore; @@ -355,9 +356,9 @@ public class BDBHAReplicaVirtualHostImpl extends AbstractConfiguredObject configuredObject) + { + if (configuredObject instanceof Session && (operation == Operation.CREATE || operation == Operation.UPDATE + || operation == Operation.DELETE)) + { + return true; + + } + + if (configuredObject instanceof Consumer && (operation == Operation.UPDATE || operation == Operation.DELETE)) + { + return true; + } + + if (configuredObject instanceof Connection && (operation == Operation.UPDATE || operation == Operation.DELETE)) + { + return true; + } + + return false; + } + private Model getModel() { return _aclProvidersParent.getModel(); @@ -351,7 +364,7 @@ public class SecurityManager // CREATE GROUP MEMBER is transformed into UPDATE GROUP rule return Operation.UPDATE; } - else if (isBrokerOrBrokerChild(category)) + else if (isBrokerOrBrokerChildOrPreferencesProvider(category)) { // CREATE/UPDATE broker child is transformed into CONFIGURE BROKER rule return Operation.CONFIGURE; @@ -364,10 +377,11 @@ public class SecurityManager // DELETE BINDING is transformed into UNBIND EXCHANGE rule return Operation.UNBIND; } - else if (isBrokerOrBrokerChild(category)) + else if (isBrokerOrBrokerChildOrPreferencesProvider(category)) { // DELETE broker child is transformed into CONFIGURE BROKER rule return Operation.CONFIGURE; + } else if (GroupMember.class.isAssignableFrom(category)) { @@ -378,16 +392,11 @@ public class SecurityManager return operation; } - private boolean isBrokerOrBrokerChild(Class category) + private boolean isBrokerOrBrokerChildOrPreferencesProvider(Class category) { - return Broker.class.isAssignableFrom(category) - || Port.class.isAssignableFrom(category) - || AuthenticationProvider.class.isAssignableFrom(category) - || AccessControlProvider.class.isAssignableFrom(category) - || GroupProvider.class.isAssignableFrom(category) - || KeyStore.class.isAssignableFrom(category) - || TrustStore.class.isAssignableFrom(category) - || Plugin.class.isAssignableFrom(category); + return Broker.class.isAssignableFrom(category) || + PreferencesProvider.class.isAssignableFrom(category) || + ( !VirtualHostNode.class.isAssignableFrom(category) && getModel().getChildTypes(Broker.class).contains(category)); } private ObjectProperties getACLObjectProperties(ConfiguredObject configuredObject, Operation configuredObjectOperation) @@ -428,7 +437,7 @@ public class SecurityManager Queue queue = (Queue)configuredObject.getParent(Queue.class); setQueueProperties(queue, properties); } - else if (isBrokerOrBrokerChild(configuredObjectType)) + else if (isBrokerOrBrokerChildOrPreferencesProvider(configuredObjectType)) { String description = String.format("%s %s '%s'", configuredObjectOperation == null? null : configuredObjectOperation.name().toLowerCase(), @@ -474,7 +483,7 @@ public class SecurityManager { return ObjectType.VIRTUALHOSTNODE; } - else if (isBrokerOrBrokerChild(category)) + else if (isBrokerOrBrokerChildOrPreferencesProvider(category)) { return ObjectType.BROKER; } diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/virtualhostnode/RedirectingVirtualHostImpl.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/virtualhostnode/RedirectingVirtualHostImpl.java index cacc981e9b..917c2fd9a1 100644 --- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/virtualhostnode/RedirectingVirtualHostImpl.java +++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/virtualhostnode/RedirectingVirtualHostImpl.java @@ -48,6 +48,7 @@ import org.apache.qpid.server.model.port.AmqpPort; import org.apache.qpid.server.protocol.AMQConnectionModel; import org.apache.qpid.server.protocol.LinkRegistry; import org.apache.qpid.server.queue.AMQQueue; +import org.apache.qpid.server.security.SecurityManager; import org.apache.qpid.server.stats.StatisticsCounter; import org.apache.qpid.server.store.DurableConfigurationStore; import org.apache.qpid.server.store.MessageStore; @@ -355,9 +356,9 @@ class RedirectingVirtualHostImpl } @Override - public org.apache.qpid.server.security.SecurityManager getSecurityManager() + public SecurityManager getSecurityManager() { - return null; + return super.getSecurityManager(); } @Override diff --git a/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/testmodels/TestSecurityManager.java b/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/testmodels/TestSecurityManager.java new file mode 100644 index 0000000000..de2fb8fe74 --- /dev/null +++ b/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/testmodels/TestSecurityManager.java @@ -0,0 +1,40 @@ +/* + * + * 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.model.testmodels; + + +import org.apache.qpid.server.model.ConfiguredObject; +import org.apache.qpid.server.security.SecurityManager; +import org.apache.qpid.server.security.access.Operation; + +public class TestSecurityManager extends SecurityManager +{ + public TestSecurityManager(ConfiguredObject aclProvidersParent) + { + super(aclProvidersParent, false); + } + + @Override + public void authorise(Operation operation, ConfiguredObject configuredObject) + { + // noop + } +} diff --git a/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/testmodels/hierarchy/TestKitCarImpl.java b/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/testmodels/hierarchy/TestKitCarImpl.java index 43dcecd6c8..bc60e0db68 100644 --- a/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/testmodels/hierarchy/TestKitCarImpl.java +++ b/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/testmodels/hierarchy/TestKitCarImpl.java @@ -25,6 +25,7 @@ import org.apache.qpid.server.model.AbstractConfiguredObject; import org.apache.qpid.server.model.ConfiguredObject; import org.apache.qpid.server.model.ManagedObject; import org.apache.qpid.server.model.ManagedObjectFactoryConstructor; +import org.apache.qpid.server.model.testmodels.TestSecurityManager; import org.apache.qpid.server.security.SecurityManager; @ManagedObject( category = false, @@ -39,7 +40,7 @@ public class TestKitCarImpl extends AbstractConfiguredObject public TestKitCarImpl(final Map attributes) { super(parentsMap(), attributes, newTaskExecutor(), TestModel.getInstance()); - _securityManager = new SecurityManager(this, false); + _securityManager = new TestSecurityManager(this); } @Override diff --git a/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/testmodels/hierarchy/TestStandardCarImpl.java b/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/testmodels/hierarchy/TestStandardCarImpl.java index 7582de2952..719e6315ac 100644 --- a/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/testmodels/hierarchy/TestStandardCarImpl.java +++ b/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/testmodels/hierarchy/TestStandardCarImpl.java @@ -29,6 +29,7 @@ import org.apache.qpid.server.configuration.updater.CurrentThreadTaskExecutor; import org.apache.qpid.server.model.AbstractConfiguredObject; import org.apache.qpid.server.model.ManagedObject; import org.apache.qpid.server.model.ManagedObjectFactoryConstructor; +import org.apache.qpid.server.model.testmodels.TestSecurityManager; import org.apache.qpid.server.security.SecurityManager; @ManagedObject( category = false, @@ -44,7 +45,7 @@ public class TestStandardCarImpl extends AbstractConfiguredObject attributes) { super(parentsMap(), attributes, newTaskExecutor(), TestModel.getInstance()); - _securityManager = new SecurityManager(this, false); + _securityManager = new TestSecurityManager(this); } private static CurrentThreadTaskExecutor newTaskExecutor() diff --git a/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/testmodels/lifecycle/TestConfiguredObject.java b/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/testmodels/lifecycle/TestConfiguredObject.java index 6935230aeb..f905a98729 100644 --- a/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/testmodels/lifecycle/TestConfiguredObject.java +++ b/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/testmodels/lifecycle/TestConfiguredObject.java @@ -38,6 +38,7 @@ import org.apache.qpid.server.model.ManagedObject; import org.apache.qpid.server.model.Model; import org.apache.qpid.server.model.State; import org.apache.qpid.server.model.StateTransition; +import org.apache.qpid.server.model.testmodels.TestSecurityManager; import org.apache.qpid.server.plugin.ConfiguredObjectRegistration; import org.apache.qpid.server.security.SecurityManager; @@ -78,7 +79,7 @@ public class TestConfiguredObject extends AbstractConfiguredObject { super(parents, attributes, taskExecutor, model); _opened = false; - _securityManager = new SecurityManager(this, false); + _securityManager = new TestSecurityManager(this); } @Override diff --git a/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/testmodels/singleton/TestSingletonImpl.java b/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/testmodels/singleton/TestSingletonImpl.java index 5de40042cc..794c2cfee0 100644 --- a/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/testmodels/singleton/TestSingletonImpl.java +++ b/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/testmodels/singleton/TestSingletonImpl.java @@ -24,9 +24,11 @@ import java.util.Set; import org.apache.qpid.server.configuration.updater.CurrentThreadTaskExecutor; import org.apache.qpid.server.configuration.updater.TaskExecutor; import org.apache.qpid.server.model.AbstractConfiguredObject; +import org.apache.qpid.server.model.ConfiguredObject; import org.apache.qpid.server.model.ManagedAttributeField; import org.apache.qpid.server.model.ManagedObject; import org.apache.qpid.server.model.ManagedObjectFactoryConstructor; +import org.apache.qpid.server.model.testmodels.TestSecurityManager; import org.apache.qpid.server.security.SecurityManager; @ManagedObject( category = false, type = TestSingletonImpl.TEST_SINGLETON_TYPE) @@ -73,7 +75,7 @@ public class TestSingletonImpl extends AbstractConfiguredObject attributes) { super(parentsMap(), attributes, newTaskExecutor(), TestModel.getInstance()); - _securityManager = new SecurityManager(this, false); + _securityManager = new TestSecurityManager(this); } private static CurrentThreadTaskExecutor newTaskExecutor() @@ -87,7 +89,7 @@ public class TestSingletonImpl extends AbstractConfiguredObject broker = mock(Broker.class); when(broker.getParent(SystemConfig.class)).thenReturn(systemConfig); + when(broker.getModel()).thenReturn(BrokerModel.getInstance()); when(broker.getSecurityManager()).thenReturn(new SecurityManager(broker, false)); _taskExecutor = new TaskExecutorImpl(); -- cgit v1.2.1