QPID-8356: [Broker-J] Fix loading of ACL rules containing firewall properties
diff --git a/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/AclAction.java b/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/AclAction.java
index 20adbd2..78866f5 100644
--- a/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/AclAction.java
+++ b/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/AclAction.java
@@ -18,6 +18,10 @@
*/
package org.apache.qpid.server.security.access.config;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Set;
+
import org.apache.qpid.server.security.access.firewall.FirewallRule;
public class AclAction
@@ -96,4 +100,21 @@
", firewallRule=" + _firewallRule +
']';
}
+
+ public Map<ObjectProperties.Property, String> getAttributes()
+ {
+ final ObjectProperties properties = _action.getProperties();
+ final Map<ObjectProperties.Property, String> attributes = new HashMap<>(properties.asPropertyMap());
+ final Set<String> attributeNames = properties.getAttributeNames();
+ if (attributeNames != null && !attributeNames.isEmpty())
+ {
+ attributes.put(ObjectProperties.Property.ATTRIBUTES, String.join(",", attributeNames));
+ }
+ final FirewallRule firewallRule = getFirewallRule();
+ if (firewallRule != null)
+ {
+ attributes.put(firewallRule.getPropertyName(), firewallRule.getPropertyValue());
+ }
+ return attributes;
+ }
}
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 4d2423b..5b5b73d 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
@@ -119,6 +119,11 @@
return _properties;
}
+ public ObjectProperties getFirewallProperties()
+ {
+ return _properties;
+ }
+
void setFirewallRuleFactory(FirewallRuleFactory firewallRuleFactory)
{
_firewallRuleFactory = firewallRuleFactory;
diff --git a/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/Rule.java b/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/Rule.java
index bf82b6c..37e5a31 100644
--- a/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/Rule.java
+++ b/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/Rule.java
@@ -20,6 +20,8 @@
*/
package org.apache.qpid.server.security.access.config;
+import java.util.Map;
+
import org.apache.qpid.server.security.access.plugins.RuleOutcome;
/**
@@ -112,5 +114,8 @@
']';
}
-
+ public Map<ObjectProperties.Property, String> getAttributes()
+ {
+ return _action.getAttributes();
+ }
}
diff --git a/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/firewall/FirewallRule.java b/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/firewall/FirewallRule.java
index 482a795..72d8d69 100644
--- a/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/firewall/FirewallRule.java
+++ b/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/firewall/FirewallRule.java
@@ -20,7 +20,13 @@
import java.net.InetAddress;
+import org.apache.qpid.server.security.access.config.ObjectProperties;
+
public interface FirewallRule
{
boolean matches(InetAddress addressOfClient);
+
+ ObjectProperties.Property getPropertyName();
+
+ String getPropertyValue();
}
diff --git a/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/firewall/HostnameFirewallRule.java b/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/firewall/HostnameFirewallRule.java
index 6990634..dfc0e22 100644
--- a/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/firewall/HostnameFirewallRule.java
+++ b/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/firewall/HostnameFirewallRule.java
@@ -30,6 +30,8 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import org.apache.qpid.server.security.access.config.ObjectProperties;
+
public class HostnameFirewallRule implements FirewallRule
{
private static final Logger LOGGER = LoggerFactory.getLogger(HostnameFirewallRule.class);
@@ -82,6 +84,17 @@
return false;
}
+ @Override
+ public ObjectProperties.Property getPropertyName()
+ {
+ return ObjectProperties.Property.FROM_HOSTNAME;
+ }
+
+ @Override
+ public String getPropertyValue()
+ {
+ return String.join(",", _hostnames);
+ }
/**
* @param remote
diff --git a/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/firewall/NetworkFirewallRule.java b/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/firewall/NetworkFirewallRule.java
index 5e37cf9..de7511f 100644
--- a/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/firewall/NetworkFirewallRule.java
+++ b/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/firewall/NetworkFirewallRule.java
@@ -25,14 +25,17 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import org.apache.qpid.server.security.access.config.ObjectProperties;
+
public class NetworkFirewallRule implements FirewallRule
{
private static final Logger LOGGER = LoggerFactory.getLogger(NetworkFirewallRule.class);
-
+ private final String _originalNetworks;
private List<InetNetwork> _networks;
public NetworkFirewallRule(String... networks)
{
+ _originalNetworks = String.join(",", networks);
_networks = new ArrayList<InetNetwork>();
for (int i = 0; i < networks.length; i++)
{
@@ -104,4 +107,16 @@
"networks=" + _networks +
']';
}
+
+ @Override
+ public ObjectProperties.Property getPropertyName()
+ {
+ return ObjectProperties.Property.FROM_NETWORK;
+ }
+
+ @Override
+ public String getPropertyValue()
+ {
+ return _originalNetworks;
+ }
}
diff --git a/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/plugins/AbstractCommonRuleBasedAccessControlProvider.java b/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/plugins/AbstractCommonRuleBasedAccessControlProvider.java
index 0719e67..ed581bc 100644
--- a/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/plugins/AbstractCommonRuleBasedAccessControlProvider.java
+++ b/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/plugins/AbstractCommonRuleBasedAccessControlProvider.java
@@ -149,7 +149,7 @@
@Override
public Map<ObjectProperties.Property, String> getAttributes()
{
- return _rule.getAction().getProperties().asPropertyMap();
+ return _rule.getAttributes();
}
@Override
diff --git a/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/config/AclActionTest.java b/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/config/AclActionTest.java
index 6fb8974..2edc47b 100644
--- a/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/config/AclActionTest.java
+++ b/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/config/AclActionTest.java
@@ -18,11 +18,17 @@
*/
package org.apache.qpid.server.security.access.config;
+import static org.hamcrest.CoreMatchers.allOf;
+import static org.hamcrest.Matchers.aMapWithSize;
+import static org.hamcrest.Matchers.hasEntry;
import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
+import java.util.Map;
+
import org.junit.Test;
import org.apache.qpid.server.security.access.firewall.FirewallRule;
@@ -30,6 +36,9 @@
public class AclActionTest extends UnitTestBase
{
+ private static final String TEST_HOSTNAME = "localhost";
+ private static final String TEST_ATTRIBUTES = "test";
+
@Test
public void testEqualsAndHashCode()
{
@@ -57,6 +66,26 @@
aclAction.equals(new AclAction(operation, objectType, createAclRulePredicates())));
}
+ @Test
+ public void testGetAttributes()
+ {
+ final ObjectType objectType = ObjectType.VIRTUALHOST;
+ final LegacyOperation operation = LegacyOperation.ACCESS;
+ final AclRulePredicates predicates = new AclRulePredicates();
+ predicates.parse(ObjectProperties.Property.FROM_HOSTNAME.name(), TEST_HOSTNAME);
+ predicates.parse(ObjectProperties.Property.ATTRIBUTES.name(), TEST_ATTRIBUTES);
+ predicates.parse(ObjectProperties.Property.NAME.name(), getTestName());
+
+ final AclAction aclAction = new AclAction(operation, objectType, predicates);
+ final Map<ObjectProperties.Property, String> attributes = aclAction.getAttributes();
+
+ assertThat(attributes,
+ allOf(aMapWithSize(3),
+ hasEntry(ObjectProperties.Property.FROM_HOSTNAME, TEST_HOSTNAME),
+ hasEntry(ObjectProperties.Property.ATTRIBUTES, TEST_ATTRIBUTES),
+ hasEntry(ObjectProperties.Property.NAME, getTestName())));
+ }
+
private AclRulePredicates createAclRulePredicates()
{
AclRulePredicates predicates = mock(AclRulePredicates.class);
diff --git a/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/plugins/AclRuleImplTest.java b/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/plugins/AclRuleImplTest.java
new file mode 100644
index 0000000..5466255
--- /dev/null
+++ b/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/plugins/AclRuleImplTest.java
@@ -0,0 +1,56 @@
+/*
+ *
+ * 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.hamcrest.Matchers.is;
+import static org.hamcrest.core.IsEqual.equalTo;
+import static org.junit.Assert.assertThat;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import java.util.Collections;
+import java.util.Map;
+
+import org.junit.Test;
+
+import org.apache.qpid.server.security.access.config.ObjectProperties;
+import org.apache.qpid.server.security.access.config.Rule;
+import org.apache.qpid.test.utils.UnitTestBase;
+
+public class AclRuleImplTest extends UnitTestBase
+{
+ @Test
+ public void testGetAttributes()
+ {
+ final Rule rule = mock(Rule.class);
+ final Map<ObjectProperties.Property, String> expected =
+ Collections.singletonMap(ObjectProperties.Property.NAME, getTestName());
+ when(rule.getAttributes()).thenReturn(expected);
+
+ final AbstractCommonRuleBasedAccessControlProvider.AclRuleImpl aclRule =
+ new AbstractCommonRuleBasedAccessControlProvider.AclRuleImpl(rule);
+ assertThat(aclRule.getAttributes(), is(equalTo(expected)));
+ verify(rule).getAttributes();
+ }
+
+}
diff --git a/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/plugins/RuleBasedAccessControlProviderImplTest.java b/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/plugins/RuleBasedAccessControlProviderImplTest.java
new file mode 100644
index 0000000..b17e25f
--- /dev/null
+++ b/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/plugins/RuleBasedAccessControlProviderImplTest.java
@@ -0,0 +1,119 @@
+/*
+ *
+ * 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 java.nio.charset.StandardCharsets.UTF_8;
+import static org.hamcrest.CoreMatchers.allOf;
+import static org.hamcrest.CoreMatchers.equalTo;
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.CoreMatchers.notNullValue;
+import static org.hamcrest.Matchers.aMapWithSize;
+import static org.hamcrest.Matchers.hasEntry;
+import static org.junit.Assert.assertThat;
+
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+
+import org.junit.Before;
+import org.junit.Test;
+
+import org.apache.qpid.server.model.Broker;
+import org.apache.qpid.server.model.BrokerTestHelper;
+import org.apache.qpid.server.model.ConfiguredObject;
+import org.apache.qpid.server.security.access.config.LegacyOperation;
+import org.apache.qpid.server.security.access.config.ObjectProperties;
+import org.apache.qpid.server.security.access.config.ObjectType;
+import org.apache.qpid.server.util.DataUrlUtils;
+import org.apache.qpid.test.utils.UnitTestBase;
+
+public class RuleBasedAccessControlProviderImplTest extends UnitTestBase
+{
+ private static final String ACL_RULE_PRINCIPAL = "guest";
+ private RuleBasedAccessControlProviderImpl _aclProvider;
+ private String _nameAttributeValue;
+
+ @Before
+ public void setUp()
+ {
+ final Map<String, Object>
+ attributes = Collections.singletonMap(RuleBasedAccessControlProvider.NAME, getTestName());
+ final Broker<?> broker = BrokerTestHelper.createBrokerMock();
+ _aclProvider = new RuleBasedAccessControlProviderImpl(attributes, broker);
+ _aclProvider.create();
+
+ _nameAttributeValue = getTestName();
+ }
+
+ @Test
+ public void testLoadACLWithFromHostnameFirewallRule()
+ {
+ loadAclAndAssertRule(ObjectProperties.Property.FROM_HOSTNAME, "localhost");
+ }
+
+ @Test
+ public void testLoadACLWithFromNetworkFirewallRule()
+ {
+ loadAclAndAssertRule(ObjectProperties.Property.FROM_NETWORK, "192.168.1.0/24");
+ }
+
+ @Test
+ public void testLoadACLWithFromNetworkFirewallRuleContainingWildcard()
+ {
+ loadAclAndAssertRule(ObjectProperties.Property.FROM_NETWORK, "192.168.1.*");
+ }
+
+ @Test
+ public void testLoadACLWithAttributes()
+ {
+ loadAclAndAssertRule(ObjectProperties.Property.ATTRIBUTES,
+ String.join(",", ConfiguredObject.NAME, ConfiguredObject.LIFETIME_POLICY));
+ }
+
+ private void loadAclAndAssertRule(final ObjectProperties.Property attributeName,
+ final String attributeValue)
+ {
+ final String acl = String.format("ACL ALLOW-LOG %s ACCESS VIRTUALHOST %s=\"%s\" name=\"%s\"",
+ ACL_RULE_PRINCIPAL,
+ attributeName,
+ attributeValue,
+ _nameAttributeValue);
+
+ _aclProvider.loadFromFile(DataUrlUtils.getDataUrlForBytes(acl.getBytes(UTF_8)));
+
+ final List<AclRule> rules = _aclProvider.getRules();
+ assertThat(rules, is(notNullValue()));
+ assertThat(rules.size(), is(equalTo(1)));
+
+ final AclRule rule = rules.get(0);
+ assertThat(rule, is(notNullValue()));
+ assertThat(rule.getObjectType(), is(equalTo(ObjectType.VIRTUALHOST)));
+ assertThat(rule.getIdentity(), is(equalTo(ACL_RULE_PRINCIPAL)));
+ assertThat(rule.getOperation(), is(equalTo(LegacyOperation.ACCESS)));
+ assertThat(rule.getOutcome(), is(equalTo(RuleOutcome.ALLOW_LOG)));
+ assertThat(rule.getAttributes(),
+ allOf(aMapWithSize(2),
+ hasEntry(attributeName, attributeValue),
+ hasEntry(ObjectProperties.Property.NAME, _nameAttributeValue)));
+ }
+
+}
