From 4fd5612de0c45146bbfb1c609fd176def33a181d Mon Sep 17 00:00:00 2001 From: dakirily Date: Tue, 5 Dec 2023 14:16:08 +0100 Subject: [PATCH 1/2] QPID-8657: [Broker-J] ACL - Posting unknown attributes leaves broker in bad internal state --- ...dVirtualHostAccessControlProviderImpl.java | 23 +++--- ...tualHostAccessControlProviderImplTest.java | 81 +++++++++++++++++++ 2 files changed, 91 insertions(+), 13 deletions(-) create mode 100644 broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/plugins/RuleBasedVirtualHostAccessControlProviderImplTest.java diff --git a/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/plugins/RuleBasedVirtualHostAccessControlProviderImpl.java b/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/plugins/RuleBasedVirtualHostAccessControlProviderImpl.java index 712303edac..e7f7884640 100644 --- a/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/plugins/RuleBasedVirtualHostAccessControlProviderImpl.java +++ b/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/plugins/RuleBasedVirtualHostAccessControlProviderImpl.java @@ -36,40 +36,37 @@ public class RuleBasedVirtualHostAccessControlProviderImpl implements RuleBasedVirtualHostAccessControlProvider { private static final EnumSet ALLOWED_OBJECT_TYPES = EnumSet.of(ObjectType.ALL, - ObjectType.QUEUE, - ObjectType.EXCHANGE, - ObjectType.VIRTUALHOST, - ObjectType.METHOD); + ObjectType.QUEUE, + ObjectType.EXCHANGE, + ObjectType.VIRTUALHOST, + ObjectType.METHOD); static { Handler.register(); } - - @ManagedObjectFactoryConstructor - public RuleBasedVirtualHostAccessControlProviderImpl(Map attributes, QueueManagingVirtualHost virtualHost) + public RuleBasedVirtualHostAccessControlProviderImpl(final Map attributes, + final QueueManagingVirtualHost virtualHost) { super(attributes, virtualHost); } - @Override protected void validateChange(final ConfiguredObject proxyForValidation, final Set changedAttributes) { super.validateChange(proxyForValidation, changedAttributes); - if(changedAttributes.contains(RULES)) + if (changedAttributes.contains(RULES)) { - for(AclRule rule : ((RuleBasedVirtualHostAccessControlProvider)proxyForValidation).getRules()) + for (AclRule rule : ((RuleBasedVirtualHostAccessControlProvider) proxyForValidation).getRules()) { - if(!ALLOWED_OBJECT_TYPES.contains(rule.getObjectType())) + if (!ALLOWED_OBJECT_TYPES.contains(rule.getObjectType())) { throw new IllegalArgumentException("Cannot use the object type " + rule.getObjectType() + " only the following object types are allowed: " + ALLOWED_OBJECT_TYPES); } + rule.getAttributes(); } } } - - } diff --git a/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/plugins/RuleBasedVirtualHostAccessControlProviderImplTest.java b/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/plugins/RuleBasedVirtualHostAccessControlProviderImplTest.java new file mode 100644 index 0000000000..101ef10c28 --- /dev/null +++ b/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/plugins/RuleBasedVirtualHostAccessControlProviderImplTest.java @@ -0,0 +1,81 @@ +/* + * + * 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.security.access.plugins; + +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import java.util.List; +import java.util.Map; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import org.apache.qpid.server.model.BrokerTestHelper; +import org.apache.qpid.server.virtualhost.QueueManagingVirtualHost; +import org.apache.qpid.server.virtualhost.TestMemoryVirtualHost; +import org.apache.qpid.test.utils.UnitTestBase; + +public class RuleBasedVirtualHostAccessControlProviderImplTest extends UnitTestBase +{ + private RuleBasedVirtualHostAccessControlProviderImpl _aclProvider; + + @BeforeEach + void setUp() + { + final Map virtualHostAttributes = Map.of(QueueManagingVirtualHost.NAME, "testVH", + QueueManagingVirtualHost.TYPE, TestMemoryVirtualHost.VIRTUAL_HOST_TYPE); + final Map attributes = Map.of(RuleBasedAccessControlProvider.NAME, RuleBasedVirtualHostAccessControlProviderImplTest.class.getName()); + final QueueManagingVirtualHost virtualHost = BrokerTestHelper.createVirtualHost(virtualHostAttributes, this); + _aclProvider = new RuleBasedVirtualHostAccessControlProviderImpl(attributes, virtualHost); + _aclProvider.create(); + } + + @Test + void setValidAttributes() + { + final List rules = List.of(Map.of("identity", "user", + "operation", "PUBLISH", + "outcome", "ALLOW_LOG", + "objectType", "EXCHANGE", + "attributes", Map.of("ROUTING_KEY", "routing_key", "NAME", "xxx"))); + final Map attributes = Map.of("name", "changed", "rules", rules); + + assertDoesNotThrow(() ->_aclProvider.setAttributes(attributes)); + } + + @Test + void setInvalidAttributes() + { + final List rules = List.of(Map.of("identity", "user", + "operation", "PUBLISH", + "outcome", "ALLOW_LOG", + "objectType", "EXCHANGE", + "attributes", Map.of("FOO", "bar", "ROUTING_KEY", "routing_key", "NAME", "xxx"))); + final Map attributes = Map.of("name", "changed", "rules", rules); + + final IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, + () -> _aclProvider.setAttributes(attributes), "Expected exception not thrown"); + + assertEquals("No enum constant org.apache.qpid.server.security.access.config.Property.FOO", exception.getMessage()); + } +} From 0185f9077c7aaf2077b44ab453678de8db198cfc Mon Sep 17 00:00:00 2001 From: vavrtom Date: Wed, 13 Dec 2023 09:25:18 +0100 Subject: [PATCH 2/2] Updated formatting of RuleBasedVirtualHostAccessControlProviderImplTest.java --- .../RuleBasedVirtualHostAccessControlProviderImplTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/plugins/RuleBasedVirtualHostAccessControlProviderImplTest.java b/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/plugins/RuleBasedVirtualHostAccessControlProviderImplTest.java index 101ef10c28..dcf0eecb2d 100644 --- a/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/plugins/RuleBasedVirtualHostAccessControlProviderImplTest.java +++ b/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/plugins/RuleBasedVirtualHostAccessControlProviderImplTest.java @@ -73,8 +73,8 @@ void setInvalidAttributes() "attributes", Map.of("FOO", "bar", "ROUTING_KEY", "routing_key", "NAME", "xxx"))); final Map attributes = Map.of("name", "changed", "rules", rules); - final IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, - () -> _aclProvider.setAttributes(attributes), "Expected exception not thrown"); + final IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, + () -> _aclProvider.setAttributes(attributes), "Expected exception not thrown"); assertEquals("No enum constant org.apache.qpid.server.security.access.config.Property.FOO", exception.getMessage()); }