From 1bc24137d82338a6137c42a97ef13df3bdd1b5d3 Mon Sep 17 00:00:00 2001 From: Daniil Kirilyuk Date: Wed, 13 Dec 2023 09:40:13 +0100 Subject: [PATCH] QPID-8657: [Broker-J] ACL - Posting unknown attributes leaves broker in bad internal state (#229) * QPID-8657: [Broker-J] ACL - Posting unknown attributes leaves broker in bad internal state * Updated formatting of RuleBasedVirtualHostAccessControlProviderImplTest.java --------- Co-authored-by: vavrtom --- ...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..dcf0eecb2d --- /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()); + } +}