You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@qpid.apache.org by or...@apache.org on 2021/09/19 19:43:41 UTC
[qpid-broker-j] branch main updated (d711a17 -> 967cf84)
This is an automated email from the ASF dual-hosted git repository.
orudyy pushed a change to branch main
in repository https://gitbox.apache.org/repos/asf/qpid-broker-j.git.
from d711a17 NO-JIRA: Update Qpid JMS client to version 0.59.0
new a8545b2 QPID-8548: [Broker-J] Enhance ACL file loading and parsing
new 967cf84 QPID-8548: [Broker-J] Fix import order and formatting
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
.../security/access/config/AclFileParser.java | 450 ++++++++++++---------
.../security/access/config/RuleSetCreator.java | 100 +++--
.../security/access/config/AclFileParserTest.java | 441 +++++++++++++++-----
.../security/access/config/RuleSetCreatorTest.java | 99 +++++
4 files changed, 750 insertions(+), 340 deletions(-)
create mode 100644 broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/config/RuleSetCreatorTest.java
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org
[qpid-broker-j] 02/02: QPID-8548: [Broker-J] Fix import order and
formatting
Posted by or...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
orudyy pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/qpid-broker-j.git
commit 967cf84e047a08070f31f6fc5a0ed743618cf137
Author: Alex Rudyy <or...@apache.org>
AuthorDate: Sun Sep 19 20:41:17 2021 +0100
QPID-8548: [Broker-J] Fix import order and formatting
---
.../security/access/config/AclFileParser.java | 31 ++--
.../security/access/config/AclFileParserTest.java | 156 ++++++++++++---------
.../security/access/config/RuleSetCreatorTest.java | 52 +++++--
3 files changed, 146 insertions(+), 93 deletions(-)
diff --git a/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/AclFileParser.java b/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/AclFileParser.java
index 08ee994..ded49e9 100644
--- a/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/AclFileParser.java
+++ b/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/AclFileParser.java
@@ -20,13 +20,6 @@
*/
package org.apache.qpid.server.security.access.config;
-import org.apache.qpid.server.configuration.IllegalConfigurationException;
-import org.apache.qpid.server.logging.EventLoggerProvider;
-import org.apache.qpid.server.security.Result;
-import org.apache.qpid.server.security.access.plugins.RuleOutcome;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStreamReader;
@@ -50,6 +43,14 @@ import java.util.regex.Matcher;
import java.util.regex.Pattern;
import java.util.stream.Collectors;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import org.apache.qpid.server.configuration.IllegalConfigurationException;
+import org.apache.qpid.server.logging.EventLoggerProvider;
+import org.apache.qpid.server.security.Result;
+import org.apache.qpid.server.security.access.plugins.RuleOutcome;
+
public final class AclFileParser
{
private static final Logger LOGGER = LoggerFactory.getLogger(AclFileParser.class);
@@ -378,13 +379,17 @@ public final class AclFileParser
return parseEnum(OBJECT_TYPE_MAP, text, line, "object type");
}
- private <T extends Enum<T>> T parseEnum(final Map<String, T> map, final String text, final int line, final String typeDescription)
+ private <T extends Enum<T>> T parseEnum(final Map<String, T> map,
+ final String text,
+ final int line,
+ final String typeDescription)
{
- return Optional.ofNullable(
- map.get(text.toUpperCase(Locale.ENGLISH))
- ).orElseThrow(
- () -> new IllegalConfigurationException(String.format(PARSE_TOKEN_FAILED_MSG, line),
- new IllegalArgumentException(String.format(INVALID_ENUM, typeDescription, text))));
+ return Optional.ofNullable(map.get(text.toUpperCase(Locale.ENGLISH)))
+ .orElseThrow(() -> new IllegalConfigurationException(String.format(PARSE_TOKEN_FAILED_MSG, line),
+ new IllegalArgumentException(String.format(
+ INVALID_ENUM,
+ typeDescription,
+ text))));
}
private Reader getReaderFromURLString(String urlString)
diff --git a/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/config/AclFileParserTest.java b/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/config/AclFileParserTest.java
index 7cc1d0b..c1203ca 100644
--- a/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/config/AclFileParserTest.java
+++ b/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/config/AclFileParserTest.java
@@ -18,13 +18,12 @@
*/
package org.apache.qpid.server.security.access.config;
-import org.apache.qpid.server.configuration.IllegalConfigurationException;
-import org.apache.qpid.server.logging.EventLoggerProvider;
-import org.apache.qpid.server.security.Result;
-import org.apache.qpid.server.security.access.config.ObjectProperties.Property;
-import org.apache.qpid.test.utils.UnitTestBase;
-import org.junit.Test;
-import org.mockito.Mockito;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.doThrow;
+import static org.mockito.Mockito.mock;
import java.io.File;
import java.io.FileWriter;
@@ -33,12 +32,14 @@ import java.io.PrintWriter;
import java.io.Reader;
import java.nio.CharBuffer;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
-import static org.mockito.Mockito.doReturn;
-import static org.mockito.Mockito.doThrow;
-import static org.mockito.Mockito.mock;
+import org.junit.Test;
+import org.mockito.Mockito;
+
+import org.apache.qpid.server.configuration.IllegalConfigurationException;
+import org.apache.qpid.server.logging.EventLoggerProvider;
+import org.apache.qpid.server.security.Result;
+import org.apache.qpid.server.security.access.config.ObjectProperties.Property;
+import org.apache.qpid.test.utils.UnitTestBase;
public class AclFileParserTest extends UnitTestBase
{
@@ -318,8 +319,8 @@ public class AclFileParserTest extends UnitTestBase
final ObjectProperties expectedProperties = new ObjectProperties();
expectedProperties.setName("value");
assertEquals("Rule has unexpected object properties",
- expectedProperties,
- rule.getAclAction().getAction().getProperties());
+ expectedProperties,
+ rule.getAclAction().getAction().getProperties());
}
/**
@@ -338,8 +339,8 @@ public class AclFileParserTest extends UnitTestBase
final ObjectProperties expectedProperties = new ObjectProperties();
expectedProperties.setName("value");
assertEquals("Rule has unexpected object properties",
- expectedProperties,
- rule.getAclAction().getAction().getProperties());
+ expectedProperties,
+ rule.getAclAction().getAction().getProperties());
}
/**
@@ -358,7 +359,9 @@ public class AclFileParserTest extends UnitTestBase
final ObjectProperties expectedProperties = new ObjectProperties();
expectedProperties.setName("name1");
expectedProperties.put(Property.OWNER, "owner1");
- assertEquals("Rule has unexpected operation", expectedProperties, rule.getAclAction().getAction().getProperties());
+ assertEquals("Rule has unexpected operation",
+ expectedProperties,
+ rule.getAclAction().getAction().getProperties());
}
/**
@@ -369,8 +372,8 @@ public class AclFileParserTest extends UnitTestBase
public void testValidRuleWithWildcardProperties() throws Exception
{
final RuleSet rules = writeACLConfig("ACL ALLOW all CREATE EXCHANGE routingKey = 'news.#'",
- "ACL ALLOW all CREATE EXCHANGE routingKey = 'news.co.#'",
- "ACL ALLOW all CREATE EXCHANGE routingKey = *.co.medellin");
+ "ACL ALLOW all CREATE EXCHANGE routingKey = 'news.co.#'",
+ "ACL ALLOW all CREATE EXCHANGE routingKey = *.co.medellin");
assertEquals(3, rules.getAllRules().size());
final Rule rule1 = rules.getAllRules().get(0);
@@ -380,45 +383,45 @@ public class AclFileParserTest extends UnitTestBase
final ObjectProperties expectedProperties1 = new ObjectProperties();
expectedProperties1.put(Property.ROUTING_KEY, "news.#");
assertEquals("Rule has unexpected object properties",
- expectedProperties1,
- rule1.getAclAction().getAction().getProperties());
+ expectedProperties1,
+ rule1.getAclAction().getAction().getProperties());
final Rule rule2 = rules.getAllRules().get(1);
final ObjectProperties expectedProperties2 = new ObjectProperties();
expectedProperties2.put(Property.ROUTING_KEY, "news.co.#");
assertEquals("Rule has unexpected object properties",
- expectedProperties2,
- rule2.getAclAction().getAction().getProperties());
+ expectedProperties2,
+ rule2.getAclAction().getAction().getProperties());
final Rule rule3 = rules.getAllRules().get(2);
final ObjectProperties expectedProperties3 = new ObjectProperties();
expectedProperties3.put(Property.ROUTING_KEY, "*.co.medellin");
assertEquals("Rule has unexpected object properties",
- expectedProperties3,
- rule3.getAclAction().getAction().getProperties());
+ expectedProperties3,
+ rule3.getAclAction().getAction().getProperties());
}
@Test
public void testOrderedValidRule() throws Exception
{
final RuleSet rules = writeACLConfig("5 ACL DENY all CREATE EXCHANGE",
- "3 ACL ALLOW all CREATE EXCHANGE routingKey = 'news.co.#'",
- "1 ACL ALLOW all CREATE EXCHANGE routingKey = *.co.medellin");
+ "3 ACL ALLOW all CREATE EXCHANGE routingKey = 'news.co.#'",
+ "1 ACL ALLOW all CREATE EXCHANGE routingKey = *.co.medellin");
assertEquals(3, rules.getAllRules().size());
final Rule rule1 = rules.getAllRules().get(0);
final ObjectProperties expectedProperties3 = new ObjectProperties();
expectedProperties3.put(Property.ROUTING_KEY, "*.co.medellin");
assertEquals("Rule has unexpected object properties",
- expectedProperties3,
- rule1.getAclAction().getAction().getProperties());
+ expectedProperties3,
+ rule1.getAclAction().getAction().getProperties());
final Rule rule3 = rules.getAllRules().get(1);
final ObjectProperties expectedProperties2 = new ObjectProperties();
expectedProperties2.put(Property.ROUTING_KEY, "news.co.#");
assertEquals("Rule has unexpected object properties",
- expectedProperties2,
- rule3.getAclAction().getAction().getProperties());
+ expectedProperties2,
+ rule3.getAclAction().getAction().getProperties());
final Rule rule5 = rules.getAllRules().get(2);
assertEquals("Rule has unexpected identity", "all", rule5.getIdentity());
@@ -426,8 +429,8 @@ public class AclFileParserTest extends UnitTestBase
assertEquals("Rule has unexpected operation", ObjectType.EXCHANGE, rule5.getAction().getObjectType());
final ObjectProperties expectedProperties1 = new ObjectProperties();
assertEquals("Rule has unexpected object properties",
- expectedProperties1,
- rule5.getAclAction().getAction().getProperties());
+ expectedProperties1,
+ rule5.getAclAction().getAction().getProperties());
}
@Test
@@ -436,8 +439,8 @@ public class AclFileParserTest extends UnitTestBase
try
{
writeACLConfig("5 ACL DENY all CREATE EXCHANGE",
- "3 ACL ALLOW all CREATE EXCHANGE routingKey = 'news.co.#'",
- "5 ACL ALLOW all CREATE EXCHANGE routingKey = *.co.medellin");
+ "3 ACL ALLOW all CREATE EXCHANGE routingKey = 'news.co.#'",
+ "5 ACL ALLOW all CREATE EXCHANGE routingKey = *.co.medellin");
fail("fail");
}
catch (IllegalConfigurationException ce)
@@ -469,8 +472,8 @@ public class AclFileParserTest extends UnitTestBase
assertEquals("Rule has unexpected operation", ObjectType.EXCHANGE, rule.getAction().getObjectType());
final ObjectProperties expectedProperties = new ObjectProperties("AmQ.dIrect");
assertEquals("Rule has unexpected object properties",
- expectedProperties,
- rule.getAclAction().getAction().getProperties());
+ expectedProperties,
+ rule.getAclAction().getAction().getProperties());
}
/**
@@ -482,8 +485,8 @@ public class AclFileParserTest extends UnitTestBase
public void testCommentsSupported() throws Exception
{
final RuleSet rules = writeACLConfig("#Comment",
- "ACL DENY-LOG user1 ACCESS VIRTUALHOST # another comment",
- " # final comment with leading whitespace");
+ "ACL DENY-LOG user1 ACCESS VIRTUALHOST # another comment",
+ " # final comment with leading whitespace");
assertEquals(1, rules.getAllRules().size());
final Rule rule = rules.getAllRules().get(0);
@@ -491,8 +494,8 @@ public class AclFileParserTest extends UnitTestBase
assertEquals("Rule has unexpected operation", LegacyOperation.ACCESS, rule.getAction().getOperation());
assertEquals("Rule has unexpected operation", ObjectType.VIRTUALHOST, rule.getAction().getObjectType());
assertEquals("Rule has unexpected object properties",
- EMPTY,
- rule.getAclAction().getAction().getProperties());
+ EMPTY,
+ rule.getAclAction().getAction().getProperties());
}
/**
@@ -531,7 +534,7 @@ public class AclFileParserTest extends UnitTestBase
public void testLineContinuation() throws Exception
{
final RuleSet rules = writeACLConfig("ACL DENY-LOG user1 \\",
- "ACCESS VIRTUALHOST");
+ "ACCESS VIRTUALHOST");
assertEquals(1, rules.getAllRules().size());
final Rule rule = rules.getAllRules().get(0);
@@ -539,56 +542,56 @@ public class AclFileParserTest extends UnitTestBase
assertEquals("Rule has unexpected operation", LegacyOperation.ACCESS, rule.getAction().getOperation());
assertEquals("Rule has unexpected operation", ObjectType.VIRTUALHOST, rule.getAction().getObjectType());
assertEquals("Rule has unexpected object properties",
- EMPTY,
- rule.getAclAction().getAction().getProperties());
+ EMPTY,
+ rule.getAclAction().getAction().getProperties());
}
@Test
public void testUserRuleParsing() throws Exception
{
validateRule(writeACLConfig("ACL ALLOW user1 CREATE USER"),
- "user1", LegacyOperation.CREATE, ObjectType.USER, EMPTY);
+ "user1", LegacyOperation.CREATE, ObjectType.USER, EMPTY);
validateRule(writeACLConfig("ACL ALLOW user1 CREATE USER name=\"otherUser\""),
- "user1", LegacyOperation.CREATE, ObjectType.USER, new ObjectProperties("otherUser"));
+ "user1", LegacyOperation.CREATE, ObjectType.USER, new ObjectProperties("otherUser"));
validateRule(writeACLConfig("ACL ALLOW user1 DELETE USER"),
- "user1", LegacyOperation.DELETE, ObjectType.USER, EMPTY);
+ "user1", LegacyOperation.DELETE, ObjectType.USER, EMPTY);
validateRule(writeACLConfig("ACL ALLOW user1 DELETE USER name=\"otherUser\""),
- "user1", LegacyOperation.DELETE, ObjectType.USER, new ObjectProperties("otherUser"));
+ "user1", LegacyOperation.DELETE, ObjectType.USER, new ObjectProperties("otherUser"));
validateRule(writeACLConfig("ACL ALLOW user1 UPDATE USER"),
- "user1", LegacyOperation.UPDATE, ObjectType.USER, EMPTY);
+ "user1", LegacyOperation.UPDATE, ObjectType.USER, EMPTY);
validateRule(writeACLConfig("ACL ALLOW user1 UPDATE USER name=\"otherUser\""),
- "user1", LegacyOperation.UPDATE, ObjectType.USER, new ObjectProperties("otherUser"));
+ "user1", LegacyOperation.UPDATE, ObjectType.USER, new ObjectProperties("otherUser"));
validateRule(writeACLConfig("ACL ALLOW user1 ALL USER"),
- "user1", LegacyOperation.ALL, ObjectType.USER, EMPTY);
+ "user1", LegacyOperation.ALL, ObjectType.USER, EMPTY);
validateRule(writeACLConfig("ACL ALLOW user1 ALL USER name=\"otherUser\""),
- "user1", LegacyOperation.ALL, ObjectType.USER, new ObjectProperties("otherUser"));
+ "user1", LegacyOperation.ALL, ObjectType.USER, new ObjectProperties("otherUser"));
}
@Test
public void testGroupRuleParsing() throws Exception
{
validateRule(writeACLConfig("ACL ALLOW user1 CREATE GROUP"),
- "user1", LegacyOperation.CREATE, ObjectType.GROUP, EMPTY);
+ "user1", LegacyOperation.CREATE, ObjectType.GROUP, EMPTY);
validateRule(writeACLConfig("ACL ALLOW user1 CREATE GROUP name=\"groupName\""),
- "user1", LegacyOperation.CREATE, ObjectType.GROUP, new ObjectProperties("groupName"));
+ "user1", LegacyOperation.CREATE, ObjectType.GROUP, new ObjectProperties("groupName"));
validateRule(writeACLConfig("ACL ALLOW user1 DELETE GROUP"),
- "user1", LegacyOperation.DELETE, ObjectType.GROUP, EMPTY);
+ "user1", LegacyOperation.DELETE, ObjectType.GROUP, EMPTY);
validateRule(writeACLConfig("ACL ALLOW user1 DELETE GROUP name=\"groupName\""),
- "user1", LegacyOperation.DELETE, ObjectType.GROUP, new ObjectProperties("groupName"));
+ "user1", LegacyOperation.DELETE, ObjectType.GROUP, new ObjectProperties("groupName"));
validateRule(writeACLConfig("ACL ALLOW user1 UPDATE GROUP"),
- "user1", LegacyOperation.UPDATE, ObjectType.GROUP, EMPTY);
+ "user1", LegacyOperation.UPDATE, ObjectType.GROUP, EMPTY);
validateRule(writeACLConfig("ACL ALLOW user1 UPDATE GROUP name=\"groupName\""),
- "user1", LegacyOperation.UPDATE, ObjectType.GROUP, new ObjectProperties("groupName"));
+ "user1", LegacyOperation.UPDATE, ObjectType.GROUP, new ObjectProperties("groupName"));
validateRule(writeACLConfig("ACL ALLOW user1 ALL GROUP"),
- "user1", LegacyOperation.ALL, ObjectType.GROUP, EMPTY);
+ "user1", LegacyOperation.ALL, ObjectType.GROUP, EMPTY);
validateRule(writeACLConfig("ACL ALLOW user1 ALL GROUP name=\"groupName\""),
- "user1", LegacyOperation.ALL, ObjectType.GROUP, new ObjectProperties("groupName"));
+ "user1", LegacyOperation.ALL, ObjectType.GROUP, new ObjectProperties("groupName"));
}
/**
@@ -626,24 +629,32 @@ public class AclFileParserTest extends UnitTestBase
public void testManagementRuleParsing() throws Exception
{
validateRule(writeACLConfig("ACL ALLOW user1 ALL MANAGEMENT"),
- "user1", LegacyOperation.ALL, ObjectType.MANAGEMENT, EMPTY);
+ "user1", LegacyOperation.ALL, ObjectType.MANAGEMENT, EMPTY);
validateRule(writeACLConfig("ACL ALLOW user1 ACCESS MANAGEMENT"),
- "user1", LegacyOperation.ACCESS, ObjectType.MANAGEMENT, EMPTY);
+ "user1", LegacyOperation.ACCESS, ObjectType.MANAGEMENT, EMPTY);
}
@Test
public void testDynamicRuleParsing() throws Exception
{
validateRule(writeACLConfig("ACL ALLOW all ACCESS VIRTUALHOST connection_limit=10 connection_frequency_limit=12"),
- Rule.ALL, LegacyOperation.ACCESS, ObjectType.VIRTUALHOST, EMPTY);
+ Rule.ALL, LegacyOperation.ACCESS, ObjectType.VIRTUALHOST, EMPTY);
}
@Test
public void testBrokerRuleParsing() throws Exception
{
- validateRule(writeACLConfig("ACL ALLOW user1 CONFIGURE BROKER"), "user1", LegacyOperation.CONFIGURE, ObjectType.BROKER, EMPTY);
- validateRule(writeACLConfig("ACL ALLOW user1 ALL BROKER"), "user1", LegacyOperation.ALL, ObjectType.BROKER, EMPTY);
+ validateRule(writeACLConfig("ACL ALLOW user1 CONFIGURE BROKER"),
+ "user1",
+ LegacyOperation.CONFIGURE,
+ ObjectType.BROKER,
+ EMPTY);
+ validateRule(writeACLConfig("ACL ALLOW user1 ALL BROKER"),
+ "user1",
+ LegacyOperation.ALL,
+ ObjectType.BROKER,
+ EMPTY);
}
@Test
@@ -676,7 +687,8 @@ public class AclFileParserTest extends UnitTestBase
doThrow(RuntimeException.class).when(reader).read();
doThrow(RuntimeException.class).when(reader).read(Mockito.any(CharBuffer.class));
doThrow(RuntimeException.class).when(reader).read(Mockito.any(char[].class));
- doThrow(RuntimeException.class).when(reader).read(Mockito.any(char[].class), Mockito.anyInt(), Mockito.anyInt());
+ doThrow(RuntimeException.class).when(reader)
+ .read(Mockito.any(char[].class), Mockito.anyInt(), Mockito.anyInt());
try
{
new AclFileParser().readAndParse(reader);
@@ -688,13 +700,19 @@ public class AclFileParserTest extends UnitTestBase
}
}
- private void validateRule(final RuleSet rules, String username, LegacyOperation operation, ObjectType objectType, ObjectProperties objectProperties)
+ private void validateRule(final RuleSet rules,
+ String username,
+ LegacyOperation operation,
+ ObjectType objectType,
+ ObjectProperties objectProperties)
{
assertEquals(1, rules.getAllRules().size());
final Rule rule = rules.getAllRules().get(0);
assertEquals("Rule has unexpected identity", username, rule.getIdentity());
assertEquals("Rule has unexpected operation", operation, rule.getAction().getOperation());
assertEquals("Rule has unexpected operation", objectType, rule.getAction().getObjectType());
- assertEquals("Rule has unexpected object properties", objectProperties, rule.getAclAction().getAction().getProperties());
+ assertEquals("Rule has unexpected object properties",
+ objectProperties,
+ rule.getAclAction().getAction().getProperties());
}
}
diff --git a/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/config/RuleSetCreatorTest.java b/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/config/RuleSetCreatorTest.java
index e0508fb..98d543b 100644
--- a/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/config/RuleSetCreatorTest.java
+++ b/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/config/RuleSetCreatorTest.java
@@ -18,28 +18,48 @@
*/
package org.apache.qpid.server.security.access.config;
-import junit.framework.TestCase;
-import org.apache.qpid.server.logging.EventLoggerProvider;
-import org.apache.qpid.server.security.access.plugins.RuleOutcome;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
import org.junit.Test;
import org.mockito.Mockito;
-public class RuleSetCreatorTest extends TestCase
+import org.apache.qpid.server.logging.EventLoggerProvider;
+import org.apache.qpid.server.security.access.plugins.RuleOutcome;
+import org.apache.qpid.test.utils.UnitTestBase;
+
+public class RuleSetCreatorTest extends UnitTestBase
{
@Test
public void testAddRule()
{
final RuleSetCreator creator = new RuleSetCreator();
creator.addRule(4, Rule.ALL, RuleOutcome.ALLOW, LegacyOperation.ACCESS);
- creator.addRule(3, Rule.ALL, RuleOutcome.DENY, LegacyOperation.PUBLISH, ObjectType.EXCHANGE, new AclRulePredicates());
+ creator.addRule(3,
+ Rule.ALL,
+ RuleOutcome.DENY,
+ LegacyOperation.PUBLISH,
+ ObjectType.EXCHANGE,
+ new AclRulePredicates());
creator.addRule(6, Rule.ALL, RuleOutcome.ALLOW, LegacyOperation.ACCESS);
- creator.addRule(7, Rule.ALL, RuleOutcome.DENY, LegacyOperation.PUBLISH, ObjectType.EXCHANGE, new AclRulePredicates());
+ creator.addRule(7,
+ Rule.ALL,
+ RuleOutcome.DENY,
+ LegacyOperation.PUBLISH,
+ ObjectType.EXCHANGE,
+ new AclRulePredicates());
RuleSet ruleSet = creator.createRuleSet(Mockito.mock(EventLoggerProvider.class));
assertNotNull(ruleSet);
assertEquals(2, ruleSet.getAllRules().size());
- assertEquals(new Rule(Rule.ALL, new AclAction(LegacyOperation.ACCESS), RuleOutcome.ALLOW), ruleSet.getAllRules().get(1));
- assertEquals(new Rule(Rule.ALL, new AclAction(LegacyOperation.PUBLISH, ObjectType.EXCHANGE, new AclRulePredicates()), RuleOutcome.DENY), ruleSet.getAllRules().get(0));
+ assertEquals(new Rule(Rule.ALL, new AclAction(LegacyOperation.ACCESS), RuleOutcome.ALLOW),
+ ruleSet.getAllRules().get(1));
+ assertEquals(new Rule(Rule.ALL,
+ new AclAction(LegacyOperation.PUBLISH, ObjectType.EXCHANGE, new AclRulePredicates()),
+ RuleOutcome.DENY), ruleSet.getAllRules().get(0));
}
@Test
@@ -48,7 +68,12 @@ public class RuleSetCreatorTest extends TestCase
final RuleSetCreator creator = new RuleSetCreator();
try
{
- creator.addRule(3, Rule.ALL, RuleOutcome.DENY, LegacyOperation.DELETE, ObjectType.MANAGEMENT, new ObjectProperties());
+ creator.addRule(3,
+ Rule.ALL,
+ RuleOutcome.DENY,
+ LegacyOperation.DELETE,
+ ObjectType.MANAGEMENT,
+ new ObjectProperties());
fail("An exception is required");
}
catch (IllegalArgumentException e)
@@ -62,8 +87,13 @@ public class RuleSetCreatorTest extends TestCase
{
final RuleSetCreator creator = new RuleSetCreator();
creator.addRule(4, Rule.ALL, RuleOutcome.ALLOW, LegacyOperation.ACCESS);
- creator.addRule(3, Rule.ALL, RuleOutcome.DENY, LegacyOperation.PUBLISH, ObjectType.EXCHANGE, new AclRulePredicates());
+ creator.addRule(3,
+ Rule.ALL,
+ RuleOutcome.DENY,
+ LegacyOperation.PUBLISH,
+ ObjectType.EXCHANGE,
+ new AclRulePredicates());
assertTrue(creator.isValidNumber(5));
assertFalse(creator.isValidNumber(4));
}
-}
\ No newline at end of file
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org
[qpid-broker-j] 01/02: QPID-8548: [Broker-J] Enhance ACL file
loading and parsing
Posted by or...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
orudyy pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/qpid-broker-j.git
commit a8545b2f4b00fdb859578a295e1f346f57426d2d
Author: Marek Laca <mk...@gmail.com>
AuthorDate: Fri Sep 10 20:42:51 2021 +0200
QPID-8548: [Broker-J] Enhance ACL file loading and parsing
This closes #107
---
.../security/access/config/AclFileParser.java | 461 ++++++++++++---------
.../security/access/config/RuleSetCreator.java | 100 +++--
.../security/access/config/AclFileParserTest.java | 461 +++++++++++++++------
.../security/access/config/RuleSetCreatorTest.java | 69 +++
4 files changed, 724 insertions(+), 367 deletions(-)
diff --git a/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/AclFileParser.java b/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/AclFileParser.java
index 0662691..08ee994 100644
--- a/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/AclFileParser.java
+++ b/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/AclFileParser.java
@@ -20,31 +20,40 @@
*/
package org.apache.qpid.server.security.access.config;
+import org.apache.qpid.server.configuration.IllegalConfigurationException;
+import org.apache.qpid.server.logging.EventLoggerProvider;
+import org.apache.qpid.server.security.Result;
+import org.apache.qpid.server.security.access.plugins.RuleOutcome;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
import java.io.BufferedReader;
-import java.io.File;
import java.io.IOException;
import java.io.InputStreamReader;
import java.io.Reader;
import java.io.StreamTokenizer;
import java.net.MalformedURLException;
import java.net.URL;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.ArrayDeque;
+import java.util.Arrays;
import java.util.HashMap;
import java.util.Iterator;
-import java.util.List;
+import java.util.Locale;
import java.util.Map;
-import java.util.Stack;
-
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
-import org.apache.qpid.server.configuration.IllegalConfigurationException;
-import org.apache.qpid.server.logging.EventLoggerProvider;
-import org.apache.qpid.server.security.Result;
-import org.apache.qpid.server.security.access.plugins.RuleOutcome;
+import java.util.Optional;
+import java.util.Queue;
+import java.util.function.Function;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+import java.util.stream.Collectors;
public final class AclFileParser
{
private static final Logger LOGGER = LoggerFactory.getLogger(AclFileParser.class);
+
public static final String DEFAULT_ALLOW = "defaultallow";
public static final String DEFAULT_DEFER = "defaultdefer";
public static final String DEFAULT_DENY = "defaultdeny";
@@ -54,71 +63,76 @@ public final class AclFileParser
public static final String ACL = "acl";
private static final String CONFIG = "config";
+ private static final String GROUP = "GROUP";
- private static final String UNRECOGNISED_INITIAL_MSG = "Unrecognised initial token '%s' at line %d";
+ static final String UNRECOGNISED_INITIAL_MSG = "Unrecognised initial token '%s' at line %d";
static final String NOT_ENOUGH_TOKENS_MSG = "Not enough tokens at line %d";
- private static final String NUMBER_NOT_ALLOWED_MSG = "Number not allowed before '%s' at line %d";
- private static final String CANNOT_LOAD_MSG = "I/O Error while reading configuration";
+ static final String NUMBER_NOT_ALLOWED_MSG = "Number not allowed before '%s' at line %d";
+ static final String CANNOT_LOAD_MSG = "I/O Error while reading configuration";
static final String PREMATURE_CONTINUATION_MSG = "Premature continuation character at line %d";
- private static final String PREMATURE_EOF_MSG = "Premature end of file reached at line %d";
static final String PARSE_TOKEN_FAILED_MSG = "Failed to parse token at line %d";
static final String NOT_ENOUGH_ACL_MSG = "Not enough data for an acl at line %d";
- private static final String NOT_ENOUGH_CONFIG_MSG = "Not enough data for config at line %d";
- private static final String BAD_ACL_RULE_NUMBER_MSG = "Invalid rule number at line %d";
+ static final String NOT_ENOUGH_CONFIG_MSG = "Not enough data for config at line %d";
+ static final String BAD_ACL_RULE_NUMBER_MSG = "Invalid rule number at line %d";
static final String PROPERTY_KEY_ONLY_MSG = "Incomplete property (key only) at line %d";
static final String PROPERTY_NO_EQUALS_MSG = "Incomplete property (no equals) at line %d";
static final String PROPERTY_NO_VALUE_MSG = "Incomplete property (no value) at line %d";
+ static final String GROUP_NOT_SUPPORTED = "GROUP keyword not supported at line %d." +
+ " Groups should defined via a Group Provider, not in the ACL file.";
+ private static final String INVALID_ENUM = "Not a valid %s: %s";
+ private static final String INVALID_URL = "Cannot convert %s to a readable resource";
+ private static final Map<String, RuleOutcome> PERMISSION_MAP;
+ private static final Map<String, LegacyOperation> OPERATION_MAP;
+ private static final Map<String, ObjectType> OBJECT_TYPE_MAP;
- private AclFileParser()
+ private static final Pattern NUMBER = Pattern.compile("\\s*(\\d+)\\s*");
+
+ static
{
+ PERMISSION_MAP = new HashMap<>();
+ for (final RuleOutcome value : RuleOutcome.values())
+ {
+ PERMISSION_MAP.put(value.name().toUpperCase(Locale.ENGLISH), value);
+ PERMISSION_MAP.put(value.name().replace('_', '-').toUpperCase(Locale.ENGLISH), value);
+ }
+
+ OPERATION_MAP = Arrays.stream(LegacyOperation.values()).collect(
+ Collectors.toMap(value -> value.name().toUpperCase(Locale.ENGLISH), Function.identity()));
+
+ OBJECT_TYPE_MAP = Arrays.stream(ObjectType.values()).collect(
+ Collectors.toMap(value -> value.name().toUpperCase(Locale.ENGLISH), Function.identity()));
}
- private static Reader getReaderFromURLString(String urlString)
+ public static RuleSet parse(String name, EventLoggerProvider eventLogger)
{
- try
- {
- URL url;
+ return new AclFileParser().readAndParse(name, eventLogger);
+ }
- try
- {
- url = new URL(urlString);
- }
- catch (MalformedURLException e)
- {
- File file = new File(urlString);
- try
- {
- url = file.toURI().toURL();
- }
- catch (MalformedURLException notAFile)
- {
- throw new IllegalConfigurationException("Cannot convert " + urlString + " to a readable resource", notAFile);
- }
+ public static RuleSet parse(Reader reader, EventLoggerProvider eventLogger)
+ {
+ return new AclFileParser().readAndParse(reader, eventLogger);
+ }
- }
- return new InputStreamReader(url.openStream());
- }
- catch (IOException e)
- {
- throw new IllegalConfigurationException("Cannot convert " + urlString + " to a readable resource", e);
- }
+ RuleSet readAndParse(String name, EventLoggerProvider eventLogger)
+ {
+ return readAndParse(getReaderFromURLString(name)).createRuleSet(eventLogger);
}
- public static RuleSet parse(String name, EventLoggerProvider eventLoggerProvider)
+ RuleSet readAndParse(Reader reader, EventLoggerProvider eventLogger)
{
- return parse(getReaderFromURLString(name), eventLoggerProvider);
+ return readAndParse(reader).createRuleSet(eventLogger);
}
- public static RuleSet parse(final Reader configReader, EventLoggerProvider eventLogger)
+ RuleSetCreator readAndParse(final Reader configReader)
{
- RuleSetCreator ruleSetCreator = new RuleSetCreator();
+ final RuleSetCreator ruleSetCreator = new RuleSetCreator();
int line = 0;
- try(Reader fileReader = configReader)
+ try (Reader fileReader = configReader)
{
LOGGER.debug("About to load ACL file");
- StreamTokenizer tokenizer = new StreamTokenizer(new BufferedReader(fileReader));
+ final StreamTokenizer tokenizer = new StreamTokenizer(new BufferedReader(fileReader));
tokenizer.resetSyntax(); // setup the tokenizer
tokenizer.commentChar(COMMENT); // single line comments
@@ -137,234 +151,273 @@ public final class AclFileParser
tokenizer.wordChars('*', '*'); // star
tokenizer.wordChars('@', '@'); // at
tokenizer.wordChars(':', ':'); // colon
+ tokenizer.wordChars('+', '+'); // plus
// parse the acl file lines
- Stack<String> stack = new Stack<>();
+ final Queue<String> stack = new ArrayDeque<>();
int current;
- do {
+ do
+ {
current = tokenizer.nextToken();
- line = tokenizer.lineno()-1;
+ line = tokenizer.lineno() - 1;
switch (current)
{
case StreamTokenizer.TT_EOF:
case StreamTokenizer.TT_EOL:
- if (stack.isEmpty())
- {
- break; // blank line
- }
-
- // pull out the first token from the bottom of the stack and check arguments exist
- String first = stack.firstElement();
- stack.removeElementAt(0);
- if (stack.isEmpty())
- {
- throw new IllegalConfigurationException(String.format(NOT_ENOUGH_TOKENS_MSG, line));
- }
-
- // check for and parse optional initial number for ACL lines
- Integer number = null;
- if (first != null && first.matches("\\d+"))
- {
- // set the acl number and get the next element
- number = Integer.valueOf(first);
- first = stack.firstElement();
- stack.removeElementAt(0);
- }
-
- if (ACL.equalsIgnoreCase(first))
- {
- parseAcl(number, stack, ruleSetCreator, line);
- }
- else if (number == null)
- {
- if("GROUP".equalsIgnoreCase(first))
- {
- throw new IllegalConfigurationException(String.format("GROUP keyword not supported at "
- + "line %d. Groups should defined "
- + "via a Group Provider, not in "
- + "the ACL file.",
- line));
- }
- else if (CONFIG.equalsIgnoreCase(first))
- {
- parseConfig(stack, ruleSetCreator, line);
- }
- else
- {
- throw new IllegalConfigurationException(String.format(UNRECOGNISED_INITIAL_MSG, first, line));
- }
- }
- else
- {
- throw new IllegalConfigurationException(String.format(NUMBER_NOT_ALLOWED_MSG, first, line));
- }
-
- // reset stack, start next line
- stack.clear();
+ processLine(ruleSetCreator, line, stack);
break;
case StreamTokenizer.TT_NUMBER:
- stack.push(Integer.toString(Double.valueOf(tokenizer.nval).intValue()));
+ // Dead code because the parsing numbers is turned off, see StreamTokenizer::parseNumbers.
+ addLast(stack, Integer.toString(Double.valueOf(tokenizer.nval).intValue()));
break;
case StreamTokenizer.TT_WORD:
- stack.push(tokenizer.sval); // token
+ addLast(stack, tokenizer.sval); // token
break;
default:
- if (tokenizer.ttype == CONTINUATION)
- {
- int next = tokenizer.nextToken();
- line = tokenizer.lineno()-1;
- if (next == StreamTokenizer.TT_EOL)
- {
- break; // continue reading next line
- }
-
- // invalid location for continuation character (add one to line because we ate the EOL)
- throw new IllegalConfigurationException(String.format(PREMATURE_CONTINUATION_MSG, line + 1));
- }
- else if (tokenizer.ttype == '\'' || tokenizer.ttype == '"')
- {
- stack.push(tokenizer.sval); // quoted token
- }
- else
- {
- stack.push(Character.toString((char) tokenizer.ttype)); // single character
- }
+ parseToken(tokenizer, stack);
}
- } while (current != StreamTokenizer.TT_EOF);
-
- if (!stack.isEmpty())
- {
- throw new IllegalConfigurationException(String.format(PREMATURE_EOF_MSG, line));
}
+ while (current != StreamTokenizer.TT_EOF);
}
- catch (IllegalArgumentException iae)
+ catch (IllegalConfigurationException ice)
{
- throw new IllegalConfigurationException(String.format(PARSE_TOKEN_FAILED_MSG, line), iae);
+ throw ice;
}
catch (IOException ioe)
{
throw new IllegalConfigurationException(CANNOT_LOAD_MSG, ioe);
}
- return ruleSetCreator.createRuleSet(eventLogger);
+ catch (RuntimeException re)
+ {
+ throw new IllegalConfigurationException(String.format(PARSE_TOKEN_FAILED_MSG, line), re);
+ }
+ return ruleSetCreator;
}
- private static void parseAcl(Integer number, List<String> args, final RuleSetCreator ruleSetCreator, final int line)
+ private void processLine(RuleSetCreator ruleSetCreator, int line, Queue<String> stack)
{
- if (args.size() < 3)
+ if (stack.isEmpty())
{
- throw new IllegalConfigurationException(String.format(NOT_ENOUGH_ACL_MSG, line));
+ return;
}
- String text = args.get(0);
- RuleOutcome outcome;
+ // pull out the first token from the bottom of the stack and check arguments exist
+ final String first = stack.poll();
+ if (stack.isEmpty())
+ {
+ throw new IllegalConfigurationException(String.format(NOT_ENOUGH_TOKENS_MSG, line));
+ }
- try
+ // check for and parse optional initial number for ACL lines
+ final Matcher matcher = NUMBER.matcher(first);
+ if (matcher.matches())
+ {
+ // get the next element and set the acl number
+ final String type = stack.poll();
+ if (ACL.equalsIgnoreCase(type))
+ {
+ final Integer number = validateNumber(Integer.valueOf(matcher.group()), ruleSetCreator, line);
+ parseAcl(number, stack, ruleSetCreator, line);
+ // reset stack, start next line
+ stack.clear();
+ return;
+ }
+ throw new IllegalConfigurationException(String.format(NUMBER_NOT_ALLOWED_MSG, type, line));
+ }
+
+ if (ACL.equalsIgnoreCase(first))
{
- outcome = RuleOutcome.valueOf(text.replace('-', '_').toUpperCase());
+ parseAcl(null, stack, ruleSetCreator, line);
}
- catch(IllegalArgumentException e)
+ else if (CONFIG.equalsIgnoreCase(first))
{
- throw new IllegalArgumentException("Not a valid permission: " + text, e);
+ parseConfig(stack, ruleSetCreator, line);
+ }
+ else if (GROUP.equalsIgnoreCase(first))
+ {
+ throw new IllegalConfigurationException(String.format(GROUP_NOT_SUPPORTED, line));
+ }
+ else
+ {
+ throw new IllegalConfigurationException(String.format(UNRECOGNISED_INITIAL_MSG, first, line));
}
- String identity = args.get(1);
- LegacyOperation operation = LegacyOperation.valueOf(args.get(2).toUpperCase());
- if (number != null && !ruleSetCreator.isValidNumber(number))
+ // reset stack, start next line
+ stack.clear();
+ }
+
+ private void parseToken(StreamTokenizer tokenizer, Queue<String> stack) throws IOException
+ {
+ if (tokenizer.ttype == CONTINUATION)
+ {
+ if (tokenizer.nextToken() != StreamTokenizer.TT_EOL)
+ {
+ // invalid location for continuation character (add one to line because we ate the EOL)
+ throw new IllegalConfigurationException(String.format(PREMATURE_CONTINUATION_MSG, tokenizer.lineno()));
+ }
+ // continue reading next line
+ }
+ else if (tokenizer.ttype == '\'' || tokenizer.ttype == '"')
+ {
+ addLast(stack, tokenizer.sval); // quoted token
+ }
+ else if (!Character.isWhitespace(tokenizer.ttype))
+ {
+ addLast(stack, Character.toString((char) tokenizer.ttype)); // single character
+ }
+ }
+
+ private void addLast(Queue<String> queue, String value)
+ {
+ if (value != null)
+ {
+ queue.add(value);
+ }
+ }
+
+ private Integer validateNumber(Integer number, RuleSetCreator ruleSetCreator, int line)
+ {
+ if (!ruleSetCreator.isValidNumber(number))
{
throw new IllegalConfigurationException(String.format(BAD_ACL_RULE_NUMBER_MSG, line));
}
+ return number;
+ }
+
+ private void parseAcl(Integer number, Queue<String> args, final RuleSetCreator ruleSetCreator, final int line)
+ {
+ if (args.size() < 3)
+ {
+ throw new IllegalConfigurationException(String.format(NOT_ENOUGH_ACL_MSG, line));
+ }
- if (args.size() == 3)
+ final RuleOutcome outcome = parsePermission(args.poll(), line);
+ final String identity = args.poll();
+ final LegacyOperation operation = parseOperation(args.poll(), line);
+
+ if (args.isEmpty())
{
ruleSetCreator.addRule(number, identity, outcome, operation);
}
else
{
- ObjectType object = ObjectType.valueOf(args.get(3).toUpperCase());
- AclRulePredicates predicates = toRulePredicates(args.subList(4, args.size()), line);
+ final ObjectType object = parseObjectType(args.poll(), line);
+
+ final AclRulePredicates predicates = new AclRulePredicates();
+ final Iterator<String> tokenIterator = args.iterator();
+ while (tokenIterator.hasNext())
+ {
+ predicates.parse(tokenIterator.next(), readValue(tokenIterator, line));
+ }
ruleSetCreator.addRule(number, identity, outcome, operation, object, predicates);
}
}
- private static void parseConfig(List<String> args, final RuleSetCreator ruleSetCreator, final int line)
+ private static void parseConfig(final Queue<String> args, final RuleSetCreator ruleSetCreator, final int line)
{
if (args.size() < 3)
{
throw new IllegalConfigurationException(String.format(NOT_ENOUGH_CONFIG_MSG, line));
}
- Map<String, Boolean> properties = toPluginProperties(args, line);
-
+ final Iterator<String> i = args.iterator();
+ while (i.hasNext())
+ {
+ final String key = i.next().toLowerCase(Locale.ENGLISH);
+ final Boolean value = Boolean.valueOf(readValue(i, line));
+ if (Boolean.TRUE.equals(value))
+ {
+ switch (key)
+ {
+ case DEFAULT_ALLOW:
+ ruleSetCreator.setDefaultResult(Result.ALLOWED);
+ break;
+ case DEFAULT_DEFER:
+ ruleSetCreator.setDefaultResult(Result.DEFER);
+ break;
+ case DEFAULT_DENY:
+ ruleSetCreator.setDefaultResult(Result.DENIED);
+ break;
+ default:
+ }
+ }
+ }
+ }
- if (Boolean.TRUE.equals(properties.get(DEFAULT_ALLOW)))
+ private static String readValue(Iterator<String> tokenIterator, int line)
+ {
+ if (!tokenIterator.hasNext())
{
- ruleSetCreator.setDefaultResult(Result.ALLOWED);
+ throw new IllegalConfigurationException(String.format(PROPERTY_KEY_ONLY_MSG, line));
}
- if (Boolean.TRUE.equals(properties.get(DEFAULT_DEFER)))
+ if (!"=".equals(tokenIterator.next()))
{
- ruleSetCreator.setDefaultResult(Result.DEFER);
+ throw new IllegalConfigurationException(String.format(PROPERTY_NO_EQUALS_MSG, line));
}
- if (Boolean.TRUE.equals(properties.get(DEFAULT_DENY)))
+ if (!tokenIterator.hasNext())
{
- ruleSetCreator.setDefaultResult(Result.DENIED);
+ throw new IllegalConfigurationException(String.format(PROPERTY_NO_VALUE_MSG, line));
}
+ return tokenIterator.next();
+ }
+ private RuleOutcome parsePermission(final String text, final int line)
+ {
+ return parseEnum(PERMISSION_MAP, text, line, "permission");
}
- private static AclRulePredicates toRulePredicates(List<String> args, final int line)
+ private LegacyOperation parseOperation(final String text, final int line)
{
- AclRulePredicates predicates = new AclRulePredicates();
- Iterator<String> i = args.iterator();
- while (i.hasNext())
- {
- String key = i.next();
- if (!i.hasNext())
- {
- throw new IllegalConfigurationException(String.format(PROPERTY_KEY_ONLY_MSG, line));
- }
- if (!"=".equals(i.next()))
- {
- throw new IllegalConfigurationException(String.format(PROPERTY_NO_EQUALS_MSG, line));
- }
- if (!i.hasNext())
- {
- throw new IllegalConfigurationException(String.format(PROPERTY_NO_VALUE_MSG, line));
- }
- String value = i.next();
+ return parseEnum(OPERATION_MAP, text, line, "operation");
+ }
- predicates.parse(key, value);
- }
- return predicates;
+ private ObjectType parseObjectType(final String text, final int line)
+ {
+ return parseEnum(OBJECT_TYPE_MAP, text, line, "object type");
}
- /** Converts a {@link List} of "name", "=", "value" tokens into a {@link Map}. */
- private static Map<String, Boolean> toPluginProperties(List<String> args, final int line)
+ private <T extends Enum<T>> T parseEnum(final Map<String, T> map, final String text, final int line, final String typeDescription)
{
- Map<String, Boolean> properties = new HashMap<>();
- Iterator<String> i = args.iterator();
- while (i.hasNext())
+ return Optional.ofNullable(
+ map.get(text.toUpperCase(Locale.ENGLISH))
+ ).orElseThrow(
+ () -> new IllegalConfigurationException(String.format(PARSE_TOKEN_FAILED_MSG, line),
+ new IllegalArgumentException(String.format(INVALID_ENUM, typeDescription, text))));
+ }
+
+ private Reader getReaderFromURLString(String urlString)
+ {
+ try
{
- String key = i.next().toLowerCase();
- if (!i.hasNext())
- {
- throw new IllegalConfigurationException(String.format(PROPERTY_KEY_ONLY_MSG, line));
- }
- if (!"=".equals(i.next()))
- {
- throw new IllegalConfigurationException(String.format(PROPERTY_NO_EQUALS_MSG, line));
- }
- if (!i.hasNext())
- {
- throw new IllegalConfigurationException(String.format(PROPERTY_NO_VALUE_MSG, line));
- }
+ return new InputStreamReader(new URL(urlString).openStream(), StandardCharsets.UTF_8);
+ }
+ catch (MalformedURLException e)
+ {
+ return getReaderFromPath(urlString);
+ }
+ catch (IOException | RuntimeException e)
+ {
+ throw createReaderError(urlString, e);
+ }
+ }
- // parse property value and save
- Boolean value = Boolean.valueOf(i.next());
- properties.put(key, value);
+ private Reader getReaderFromPath(String path)
+ {
+ try
+ {
+ final Path file = Paths.get(path);
+ return new InputStreamReader(file.toUri().toURL().openStream(), StandardCharsets.UTF_8);
+ }
+ catch (IOException | RuntimeException e)
+ {
+ throw createReaderError(path, e);
}
- return properties;
}
+ private IllegalConfigurationException createReaderError(String urlString, Exception e)
+ {
+ return new IllegalConfigurationException(String.format(INVALID_URL, urlString), e);
+ }
}
diff --git a/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/RuleSetCreator.java b/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/RuleSetCreator.java
index 0758468..24ef986 100644
--- a/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/RuleSetCreator.java
+++ b/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/RuleSetCreator.java
@@ -20,24 +20,34 @@
*/
package org.apache.qpid.server.security.access.config;
-import java.util.SortedMap;
+import java.util.HashSet;
+import java.util.NavigableMap;
+import java.util.Objects;
+import java.util.Set;
import java.util.TreeMap;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
import org.apache.qpid.server.logging.EventLoggerProvider;
import org.apache.qpid.server.security.Result;
import org.apache.qpid.server.security.access.plugins.RuleOutcome;
final class RuleSetCreator
{
- private final SortedMap<Integer, Rule> _rules = new TreeMap<Integer, Rule>();
+ private static final Logger LOGGER = LoggerFactory.getLogger(RuleSetCreator.class);
private static final Integer INCREMENT = 10;
+
+ private final NavigableMap<Integer, Rule> _rules = new TreeMap<>();
+ private final Set<RuleKey> _ruleSet = new HashSet<>();
+
private Result _defaultResult = Result.DENIED;
RuleSetCreator()
{
+ super();
}
-
boolean isValidNumber(Integer number)
{
return !_rules.containsKey(number);
@@ -45,8 +55,7 @@ final class RuleSetCreator
void addRule(Integer number, String identity, RuleOutcome ruleOutcome, LegacyOperation operation)
{
- AclAction action = new AclAction(operation);
- addRule(number, identity, ruleOutcome, action);
+ addRule(number, identity, ruleOutcome, new AclAction(operation));
}
void addRule(Integer number,
@@ -56,8 +65,7 @@ final class RuleSetCreator
ObjectType object,
ObjectProperties properties)
{
- AclAction action = new AclAction(operation, object, properties);
- addRule(number, identity, ruleOutcome, action);
+ addRule(number, identity, ruleOutcome, new AclAction(operation, object, properties));
}
void addRule(Integer number,
@@ -67,37 +75,23 @@ final class RuleSetCreator
ObjectType object,
AclRulePredicates predicates)
{
- AclAction aclAction = new AclAction(operation, object, predicates);
- addRule(number, identity, ruleOutcome, aclAction);
+ addRule(number, identity, ruleOutcome, new AclAction(operation, object, predicates));
}
- private boolean ruleExists(String identity, AclAction action)
- {
- for (Rule rule : _rules.values())
- {
- if (rule.getIdentity().equals(identity) && rule.getAclAction().equals(action))
- {
- return true;
- }
- }
- return false;
- }
-
-
private void addRule(Integer number, String identity, RuleOutcome ruleOutcome, AclAction action)
{
-
if (!action.isAllowed())
{
throw new IllegalArgumentException("Action is not allowed: " + action);
}
- if (ruleExists(identity, action))
+ final RuleKey key = new RuleKey(identity, action);
+ if (_ruleSet.contains(key))
{
+ LOGGER.warn("Duplicate rule for the {}", key);
return;
}
-
// set rule number if needed
- Rule rule = new Rule(identity, action, ruleOutcome);
+ final Rule rule = new Rule(identity, action, ruleOutcome);
if (number == null)
{
if (_rules.isEmpty())
@@ -109,9 +103,9 @@ final class RuleSetCreator
number = _rules.lastKey() + INCREMENT;
}
}
-
// save rule
_rules.put(number, rule);
+ _ruleSet.add(new RuleKey(rule));
}
void setDefaultResult(final Result defaultResult)
@@ -119,18 +113,54 @@ final class RuleSetCreator
_defaultResult = defaultResult;
}
- Result getDefaultResult()
+ RuleSet createRuleSet(EventLoggerProvider eventLoggerProvider)
{
- return _defaultResult;
+ return new RuleSet(eventLoggerProvider, _rules.values(), _defaultResult);
}
- SortedMap<Integer, Rule> getRules()
+ private static final class RuleKey
{
- return _rules;
- }
+ private final String _identity;
+ private final AclAction _action;
- RuleSet createRuleSet(EventLoggerProvider eventLoggerProvider)
- {
- return new RuleSet(eventLoggerProvider, _rules.values(), _defaultResult);
+ RuleKey(String identity, AclAction action)
+ {
+ super();
+ _identity = identity;
+ _action = action;
+ }
+
+ RuleKey(Rule rule)
+ {
+ this(rule.getIdentity(), rule.getAclAction());
+ }
+
+ @Override
+ public int hashCode()
+ {
+ return Objects.hash(_identity, _action);
+ }
+
+ @Override
+ public boolean equals(Object o)
+ {
+ if (this == o)
+ {
+ return true;
+ }
+ if (o == null || getClass() != o.getClass())
+ {
+ return false;
+ }
+
+ final RuleKey ruleKey = (RuleKey) o;
+ return Objects.equals(_identity, ruleKey._identity) && Objects.equals(_action, ruleKey._action);
+ }
+
+ @Override
+ public String toString()
+ {
+ return "RuleKey[identity=" + _identity + ", action=" + _action + "]";
+ }
}
}
diff --git a/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/config/AclFileParserTest.java b/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/config/AclFileParserTest.java
index ed49b45..7cc1d0b 100644
--- a/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/config/AclFileParserTest.java
+++ b/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/config/AclFileParserTest.java
@@ -18,28 +18,33 @@
*/
package org.apache.qpid.server.security.access.config;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
-import static org.mockito.Mockito.mock;
-
-import java.io.File;
-import java.io.FileReader;
-import java.io.FileWriter;
-import java.io.PrintWriter;
-import java.util.List;
-
-import org.junit.Test;
-
import org.apache.qpid.server.configuration.IllegalConfigurationException;
import org.apache.qpid.server.logging.EventLoggerProvider;
import org.apache.qpid.server.security.Result;
import org.apache.qpid.server.security.access.config.ObjectProperties.Property;
import org.apache.qpid.test.utils.UnitTestBase;
+import org.junit.Test;
+import org.mockito.Mockito;
+
+import java.io.File;
+import java.io.FileWriter;
+import java.io.IOException;
+import java.io.PrintWriter;
+import java.io.Reader;
+import java.nio.CharBuffer;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.doThrow;
+import static org.mockito.Mockito.mock;
public class AclFileParserTest extends UnitTestBase
{
- private RuleSet writeACLConfig(String...aclData) throws Exception
+ private static final ObjectProperties EMPTY = new ObjectProperties();
+
+ private RuleSet writeACLConfig(String... aclData) throws Exception
{
File acl = File.createTempFile(getClass().getName() + getTestName(), "acl");
acl.deleteOnExit();
@@ -54,16 +59,17 @@ public class AclFileParserTest extends UnitTestBase
}
// Load ruleset
- return AclFileParser.parse(new FileReader(acl), mock(EventLoggerProvider.class));
+ return AclFileParser.parse(acl.toURI().toURL().toString(), mock(EventLoggerProvider.class));
}
@Test
public void testEmptyRuleSetDefaults() throws Exception
{
RuleSet ruleSet = writeACLConfig();
- assertEquals((long) 0, (long) ruleSet.getRuleCount());
+ assertEquals(0, ruleSet.getAllRules().size());
assertEquals(Result.DENIED, ruleSet.getDefault());
}
+
@Test
public void testACLFileSyntaxContinuation() throws Exception
{
@@ -89,10 +95,33 @@ public class AclFileParserTest extends UnitTestBase
catch (IllegalConfigurationException ce)
{
assertEquals(String.format(AclFileParser.PARSE_TOKEN_FAILED_MSG, 1), ce.getMessage());
- final boolean condition = ce.getCause() instanceof IllegalArgumentException;
- assertTrue(condition);
+ assertTrue(ce.getCause() instanceof IllegalArgumentException);
assertEquals("Not a valid permission: unparsed", ce.getCause().getMessage());
}
+
+ try
+ {
+ writeACLConfig("ACL DENY ALL unparsed");
+ fail("fail");
+ }
+ catch (IllegalConfigurationException ce)
+ {
+ assertEquals(String.format(AclFileParser.PARSE_TOKEN_FAILED_MSG, 1), ce.getMessage());
+ assertTrue(ce.getCause() instanceof IllegalArgumentException);
+ assertEquals("Not a valid operation: unparsed", ce.getCause().getMessage());
+ }
+
+ try
+ {
+ writeACLConfig("ACL DENY ALL ALL unparsed");
+ fail("fail");
+ }
+ catch (IllegalConfigurationException ce)
+ {
+ assertEquals(String.format(AclFileParser.PARSE_TOKEN_FAILED_MSG, 1), ce.getMessage());
+ assertTrue(ce.getCause() instanceof IllegalArgumentException);
+ assertEquals("Not a valid object type: unparsed", ce.getCause().getMessage());
+ }
}
@Test
@@ -121,6 +150,64 @@ public class AclFileParserTest extends UnitTestBase
{
assertEquals(String.format(AclFileParser.NOT_ENOUGH_TOKENS_MSG, 1), ce.getMessage());
}
+
+ try
+ {
+ writeACLConfig("CONFIG defaultdeny");
+ fail("fail");
+ }
+ catch (IllegalConfigurationException ce)
+ {
+ assertEquals(String.format(AclFileParser.NOT_ENOUGH_CONFIG_MSG, 1), ce.getMessage());
+ }
+ }
+
+ @Test
+ public void testACLFileSyntaxOrderedConfig() throws Exception
+ {
+ try
+ {
+ writeACLConfig("3 CONFIG defaultdeny=true");
+ fail("fail");
+ }
+ catch (IllegalConfigurationException ce)
+ {
+ assertEquals(String.format(AclFileParser.NUMBER_NOT_ALLOWED_MSG, "CONFIG", 1), ce.getMessage());
+ }
+ }
+
+ @Test
+ public void testACLFileSyntaxInvalidConfig() throws Exception
+ {
+ try
+ {
+ writeACLConfig("CONFIG default=false defaultdeny");
+ fail("fail");
+ }
+ catch (IllegalConfigurationException ce)
+ {
+ assertEquals(String.format(AclFileParser.PROPERTY_KEY_ONLY_MSG, 1), ce.getMessage());
+ }
+
+ try
+ {
+ writeACLConfig("CONFIG default=false defaultdeny true");
+ fail("fail");
+ }
+ catch (IllegalConfigurationException ce)
+ {
+ assertEquals(String.format(AclFileParser.PROPERTY_NO_EQUALS_MSG, 1), ce.getMessage());
+ }
+
+ try
+ {
+ writeACLConfig("CONFIG default=false defaultdeny=");
+ fail("fail");
+ }
+ catch (IllegalConfigurationException ce)
+ {
+ assertEquals(String.format(AclFileParser.PROPERTY_NO_VALUE_MSG, 1), ce.getMessage());
+ }
}
@Test
@@ -183,29 +270,36 @@ public class AclFileParserTest extends UnitTestBase
public void testValidConfig() throws Exception
{
RuleSet ruleSet = writeACLConfig("CONFIG defaultdefer=true");
- assertEquals("Unexpected number of rules", (long) 0, (long) ruleSet.getRuleCount());
+ assertEquals("Unexpected number of rules", 0, ruleSet.getAllRules().size());
assertEquals("Unexpected number of rules", Result.DEFER, ruleSet.getDefault());
+
+ ruleSet = writeACLConfig("CONFIG defaultdeny=true");
+ assertEquals("Unexpected number of rules", 0, ruleSet.getAllRules().size());
+ assertEquals("Unexpected number of rules", Result.DENIED, ruleSet.getDefault());
+
+ ruleSet = writeACLConfig("CONFIG defaultallow=true");
+ assertEquals("Unexpected number of rules", 0, ruleSet.getAllRules().size());
+ assertEquals("Unexpected number of rules", Result.ALLOWED, ruleSet.getDefault());
+
+ ruleSet = writeACLConfig("CONFIG defaultdefer=false defaultallow=true defaultdeny=false df=false");
+ assertEquals("Unexpected number of rules", 0, ruleSet.getAllRules().size());
+ assertEquals("Unexpected number of rules", Result.ALLOWED, ruleSet.getDefault());
}
/**
* Tests interpretation of an acl rule with no object properties.
- *
*/
@Test
public void testValidRule() throws Exception
{
- final RuleSet rs = writeACLConfig("ACL DENY-LOG user1 ACCESS VIRTUALHOST");
- assertEquals((long) 1, (long) rs.getRuleCount());
+ final RuleSet rules = writeACLConfig("ACL DENY-LOG user1 ACCESS VIRTUALHOST");
+ assertEquals(1, rules.getAllRules().size());
- final List<Rule> rules = rs.getAllRules();
- assertEquals((long) 1, (long) rules.size());
- final Rule rule = rules.get(0);
+ final Rule rule = rules.getAllRules().get(0);
assertEquals("Rule has unexpected identity", "user1", rule.getIdentity());
assertEquals("Rule has unexpected operation", LegacyOperation.ACCESS, rule.getAction().getOperation());
assertEquals("Rule has unexpected operation", ObjectType.VIRTUALHOST, rule.getAction().getObjectType());
- assertEquals("Rule has unexpected object properties",
- ObjectProperties.EMPTY,
- rule.getAction().getProperties());
+ assertEquals("Rule has unexpected object properties", EMPTY, rule.getAclAction().getAction().getProperties());
}
/**
@@ -214,20 +308,18 @@ public class AclFileParserTest extends UnitTestBase
@Test
public void testValidRuleWithSingleQuotedProperty() throws Exception
{
- final RuleSet rs = writeACLConfig("ACL ALLOW all CREATE EXCHANGE name = \'value\'");
- assertEquals((long) 1, (long) rs.getRuleCount());
+ final RuleSet rules = writeACLConfig("ACL ALLOW all CREATE EXCHANGE name = 'value'");
+ assertEquals(1, rules.getAllRules().size());
- final List<Rule> rules = rs.getAllRules();
- assertEquals((long) 1, (long) rules.size());
- final Rule rule = rules.get(0);
+ final Rule rule = rules.getAllRules().get(0);
assertEquals("Rule has unexpected identity", "all", rule.getIdentity());
assertEquals("Rule has unexpected operation", LegacyOperation.CREATE, rule.getAction().getOperation());
assertEquals("Rule has unexpected operation", ObjectType.EXCHANGE, rule.getAction().getObjectType());
final ObjectProperties expectedProperties = new ObjectProperties();
expectedProperties.setName("value");
assertEquals("Rule has unexpected object properties",
- expectedProperties,
- rule.getAction().getProperties());
+ expectedProperties,
+ rule.getAclAction().getAction().getProperties());
}
/**
@@ -236,20 +328,18 @@ public class AclFileParserTest extends UnitTestBase
@Test
public void testValidRuleWithDoubleQuotedProperty() throws Exception
{
- final RuleSet rs = writeACLConfig("ACL ALLOW all CREATE EXCHANGE name = \"value\"");
- assertEquals((long) 1, (long) rs.getRuleCount());
+ final RuleSet rules = writeACLConfig("ACL ALLOW all CREATE EXCHANGE name = \"value\"");
- final List<Rule> rules = rs.getAllRules();
- assertEquals((long) 1, (long) rules.size());
- final Rule rule = rules.get(0);
+ assertEquals(1, rules.getAllRules().size());
+ final Rule rule = rules.getAllRules().get(0);
assertEquals("Rule has unexpected identity", "all", rule.getIdentity());
assertEquals("Rule has unexpected operation", LegacyOperation.CREATE, rule.getAction().getOperation());
assertEquals("Rule has unexpected operation", ObjectType.EXCHANGE, rule.getAction().getObjectType());
final ObjectProperties expectedProperties = new ObjectProperties();
expectedProperties.setName("value");
assertEquals("Rule has unexpected object properties",
- expectedProperties,
- rule.getAction().getProperties());
+ expectedProperties,
+ rule.getAclAction().getAction().getProperties());
}
/**
@@ -258,19 +348,17 @@ public class AclFileParserTest extends UnitTestBase
@Test
public void testValidRuleWithManyProperties() throws Exception
{
- final RuleSet rs = writeACLConfig("ACL ALLOW admin DELETE QUEUE name=name1 owner = owner1");
- assertEquals((long) 1, (long) rs.getRuleCount());
+ final RuleSet rules = writeACLConfig("ACL ALLOW admin DELETE QUEUE name=name1 owner = owner1");
+ assertEquals(1, rules.getAllRules().size());
- final List<Rule> rules = rs.getAllRules();
- assertEquals((long) 1, (long) rules.size());
- final Rule rule = rules.get(0);
+ final Rule rule = rules.getAllRules().get(0);
assertEquals("Rule has unexpected identity", "admin", rule.getIdentity());
assertEquals("Rule has unexpected operation", LegacyOperation.DELETE, rule.getAction().getOperation());
assertEquals("Rule has unexpected operation", ObjectType.QUEUE, rule.getAction().getObjectType());
final ObjectProperties expectedProperties = new ObjectProperties();
expectedProperties.setName("name1");
expectedProperties.put(Property.OWNER, "owner1");
- assertEquals("Rule has unexpected operation", expectedProperties, rule.getAction().getProperties());
+ assertEquals("Rule has unexpected operation", expectedProperties, rule.getAclAction().getAction().getProperties());
}
/**
@@ -280,36 +368,90 @@ public class AclFileParserTest extends UnitTestBase
@Test
public void testValidRuleWithWildcardProperties() throws Exception
{
- final RuleSet rs = writeACLConfig("ACL ALLOW all CREATE EXCHANGE routingKey = \'news.#\'",
- "ACL ALLOW all CREATE EXCHANGE routingKey = \'news.co.#\'",
- "ACL ALLOW all CREATE EXCHANGE routingKey = *.co.medellin");
- assertEquals((long) 3, (long) rs.getRuleCount());
+ final RuleSet rules = writeACLConfig("ACL ALLOW all CREATE EXCHANGE routingKey = 'news.#'",
+ "ACL ALLOW all CREATE EXCHANGE routingKey = 'news.co.#'",
+ "ACL ALLOW all CREATE EXCHANGE routingKey = *.co.medellin");
+ assertEquals(3, rules.getAllRules().size());
- final List<Rule> rules = rs.getAllRules();
- assertEquals((long) 3, (long) rules.size());
- final Rule rule1 = rules.get(0);
+ final Rule rule1 = rules.getAllRules().get(0);
assertEquals("Rule has unexpected identity", "all", rule1.getIdentity());
assertEquals("Rule has unexpected operation", LegacyOperation.CREATE, rule1.getAction().getOperation());
assertEquals("Rule has unexpected operation", ObjectType.EXCHANGE, rule1.getAction().getObjectType());
final ObjectProperties expectedProperties1 = new ObjectProperties();
- expectedProperties1.put(Property.ROUTING_KEY,"news.#");
+ expectedProperties1.put(Property.ROUTING_KEY, "news.#");
assertEquals("Rule has unexpected object properties",
- expectedProperties1,
- rule1.getAction().getProperties());
+ expectedProperties1,
+ rule1.getAclAction().getAction().getProperties());
- final Rule rule2 = rules.get(1);
+ final Rule rule2 = rules.getAllRules().get(1);
final ObjectProperties expectedProperties2 = new ObjectProperties();
- expectedProperties2.put(Property.ROUTING_KEY,"news.co.#");
+ expectedProperties2.put(Property.ROUTING_KEY, "news.co.#");
assertEquals("Rule has unexpected object properties",
- expectedProperties2,
- rule2.getAction().getProperties());
+ expectedProperties2,
+ rule2.getAclAction().getAction().getProperties());
- final Rule rule3 = rules.get(2);
+ final Rule rule3 = rules.getAllRules().get(2);
final ObjectProperties expectedProperties3 = new ObjectProperties();
- expectedProperties3.put(Property.ROUTING_KEY,"*.co.medellin");
+ expectedProperties3.put(Property.ROUTING_KEY, "*.co.medellin");
+ assertEquals("Rule has unexpected object properties",
+ expectedProperties3,
+ rule3.getAclAction().getAction().getProperties());
+ }
+
+ @Test
+ public void testOrderedValidRule() throws Exception
+ {
+ final RuleSet rules = writeACLConfig("5 ACL DENY all CREATE EXCHANGE",
+ "3 ACL ALLOW all CREATE EXCHANGE routingKey = 'news.co.#'",
+ "1 ACL ALLOW all CREATE EXCHANGE routingKey = *.co.medellin");
+ assertEquals(3, rules.getAllRules().size());
+
+ final Rule rule1 = rules.getAllRules().get(0);
+ final ObjectProperties expectedProperties3 = new ObjectProperties();
+ expectedProperties3.put(Property.ROUTING_KEY, "*.co.medellin");
+ assertEquals("Rule has unexpected object properties",
+ expectedProperties3,
+ rule1.getAclAction().getAction().getProperties());
+
+ final Rule rule3 = rules.getAllRules().get(1);
+ final ObjectProperties expectedProperties2 = new ObjectProperties();
+ expectedProperties2.put(Property.ROUTING_KEY, "news.co.#");
assertEquals("Rule has unexpected object properties",
- expectedProperties3,
- rule3.getAction().getProperties());
+ expectedProperties2,
+ rule3.getAclAction().getAction().getProperties());
+
+ final Rule rule5 = rules.getAllRules().get(2);
+ assertEquals("Rule has unexpected identity", "all", rule5.getIdentity());
+ assertEquals("Rule has unexpected operation", LegacyOperation.CREATE, rule5.getAction().getOperation());
+ assertEquals("Rule has unexpected operation", ObjectType.EXCHANGE, rule5.getAction().getObjectType());
+ final ObjectProperties expectedProperties1 = new ObjectProperties();
+ assertEquals("Rule has unexpected object properties",
+ expectedProperties1,
+ rule5.getAclAction().getAction().getProperties());
+ }
+
+ @Test
+ public void testOrderedInValidRule() throws Exception
+ {
+ try
+ {
+ writeACLConfig("5 ACL DENY all CREATE EXCHANGE",
+ "3 ACL ALLOW all CREATE EXCHANGE routingKey = 'news.co.#'",
+ "5 ACL ALLOW all CREATE EXCHANGE routingKey = *.co.medellin");
+ fail("fail");
+ }
+ catch (IllegalConfigurationException ce)
+ {
+ assertEquals(String.format(AclFileParser.BAD_ACL_RULE_NUMBER_MSG, 3), ce.getMessage());
+ }
+ }
+
+ @Test
+ public void testShortValidRule() throws Exception
+ {
+ final RuleSet rules = writeACLConfig("ACL DENY user UPDATE");
+ assertEquals(1, rules.getAllRules().size());
+ validateRule(rules, "user", LegacyOperation.UPDATE, ObjectType.ALL, EMPTY);
}
/**
@@ -318,19 +460,17 @@ public class AclFileParserTest extends UnitTestBase
@Test
public void testMixedCaseRuleInterpretation() throws Exception
{
- final RuleSet rs = writeACLConfig("AcL deny-LOG User1 BiND Exchange Name=AmQ.dIrect");
- assertEquals((long) 1, (long) rs.getRuleCount());
+ final RuleSet rules = writeACLConfig("AcL deny-LOG User1 BiND Exchange Name=AmQ.dIrect");
+ assertEquals(1, rules.getAllRules().size());
- final List<Rule> rules = rs.getAllRules();
- assertEquals((long) 1, (long) rules.size());
- final Rule rule = rules.get(0);
+ final Rule rule = rules.getAllRules().get(0);
assertEquals("Rule has unexpected identity", "User1", rule.getIdentity());
assertEquals("Rule has unexpected operation", LegacyOperation.BIND, rule.getAction().getOperation());
assertEquals("Rule has unexpected operation", ObjectType.EXCHANGE, rule.getAction().getObjectType());
final ObjectProperties expectedProperties = new ObjectProperties("AmQ.dIrect");
assertEquals("Rule has unexpected object properties",
- expectedProperties,
- rule.getAction().getProperties());
+ expectedProperties,
+ rule.getAclAction().getAction().getProperties());
}
/**
@@ -341,41 +481,47 @@ public class AclFileParserTest extends UnitTestBase
@Test
public void testCommentsSupported() throws Exception
{
- final RuleSet rs = writeACLConfig("#Comment",
- "ACL DENY-LOG user1 ACCESS VIRTUALHOST # another comment",
- " # final comment with leading whitespace");
- assertEquals((long) 1, (long) rs.getRuleCount());
+ final RuleSet rules = writeACLConfig("#Comment",
+ "ACL DENY-LOG user1 ACCESS VIRTUALHOST # another comment",
+ " # final comment with leading whitespace");
+ assertEquals(1, rules.getAllRules().size());
- final List<Rule> rules = rs.getAllRules();
- assertEquals((long) 1, (long) rules.size());
- final Rule rule = rules.get(0);
+ final Rule rule = rules.getAllRules().get(0);
assertEquals("Rule has unexpected identity", "user1", rule.getIdentity());
assertEquals("Rule has unexpected operation", LegacyOperation.ACCESS, rule.getAction().getOperation());
assertEquals("Rule has unexpected operation", ObjectType.VIRTUALHOST, rule.getAction().getObjectType());
assertEquals("Rule has unexpected object properties",
- ObjectProperties.EMPTY,
- rule.getAction().getProperties());
+ EMPTY,
+ rule.getAclAction().getAction().getProperties());
}
/**
* Tests interpretation of an acl rule using mixtures of tabs/spaces as token separators.
- *
*/
@Test
public void testWhitespace() throws Exception
{
- final RuleSet rs = writeACLConfig("ACL\tDENY-LOG\t\t user1\t \tACCESS VIRTUALHOST");
- assertEquals((long) 1, (long) rs.getRuleCount());
+ final RuleSet rules = writeACLConfig("ACL\tDENY-LOG\t\t user1\t \tACCESS VIRTUALHOST");
+ assertEquals(1, rules.getAllRules().size());
- final List<Rule> rules = rs.getAllRules();
- assertEquals((long) 1, (long) rules.size());
- final Rule rule = rules.get(0);
+ final Rule rule = rules.getAllRules().get(0);
assertEquals("Rule has unexpected identity", "user1", rule.getIdentity());
assertEquals("Rule has unexpected operation", LegacyOperation.ACCESS, rule.getAction().getOperation());
assertEquals("Rule has unexpected operation", ObjectType.VIRTUALHOST, rule.getAction().getObjectType());
- assertEquals("Rule has unexpected object properties",
- ObjectProperties.EMPTY,
- rule.getAction().getProperties());
+ assertEquals("Rule has unexpected object properties", EMPTY, rule.getAclAction().getAction().getProperties());
+ }
+
+ @Test
+ public void testWhitespace2() throws Exception
+ {
+ final RuleSet rules = writeACLConfig("ACL\u000B DENY-LOG\t\t user1\t \tACCESS VIRTUALHOST\u001E");
+ assertEquals(1, rules.getAllRules().size());
+
+ final Rule rule = rules.getAllRules().get(0);
+ assertEquals("Rule has unexpected identity", "user1", rule.getIdentity());
+ assertEquals("Rule has unexpected operation", LegacyOperation.ACCESS, rule.getAction().getOperation());
+ assertEquals("Rule has unexpected operation", ObjectType.VIRTUALHOST, rule.getAction().getObjectType());
+ assertEquals("Rule has unexpected object properties", EMPTY, rule.getAclAction().getAction().getProperties());
}
/**
@@ -384,70 +530,70 @@ public class AclFileParserTest extends UnitTestBase
@Test
public void testLineContinuation() throws Exception
{
- final RuleSet rs = writeACLConfig("ACL DENY-LOG user1 \\",
- "ACCESS VIRTUALHOST");
- assertEquals((long) 1, (long) rs.getRuleCount());
+ final RuleSet rules = writeACLConfig("ACL DENY-LOG user1 \\",
+ "ACCESS VIRTUALHOST");
+ assertEquals(1, rules.getAllRules().size());
- final List<Rule> rules = rs.getAllRules();
- assertEquals((long) 1, (long) rules.size());
- final Rule rule = rules.get(0);
+ final Rule rule = rules.getAllRules().get(0);
assertEquals("Rule has unexpected identity", "user1", rule.getIdentity());
assertEquals("Rule has unexpected operation", LegacyOperation.ACCESS, rule.getAction().getOperation());
assertEquals("Rule has unexpected operation", ObjectType.VIRTUALHOST, rule.getAction().getObjectType());
assertEquals("Rule has unexpected object properties",
- ObjectProperties.EMPTY,
- rule.getAction().getProperties());
+ EMPTY,
+ rule.getAclAction().getAction().getProperties());
}
@Test
public void testUserRuleParsing() throws Exception
{
validateRule(writeACLConfig("ACL ALLOW user1 CREATE USER"),
- "user1", LegacyOperation.CREATE, ObjectType.USER, ObjectProperties.EMPTY);
+ "user1", LegacyOperation.CREATE, ObjectType.USER, EMPTY);
validateRule(writeACLConfig("ACL ALLOW user1 CREATE USER name=\"otherUser\""),
- "user1", LegacyOperation.CREATE, ObjectType.USER, new ObjectProperties("otherUser"));
+ "user1", LegacyOperation.CREATE, ObjectType.USER, new ObjectProperties("otherUser"));
validateRule(writeACLConfig("ACL ALLOW user1 DELETE USER"),
- "user1", LegacyOperation.DELETE, ObjectType.USER, ObjectProperties.EMPTY);
+ "user1", LegacyOperation.DELETE, ObjectType.USER, EMPTY);
validateRule(writeACLConfig("ACL ALLOW user1 DELETE USER name=\"otherUser\""),
- "user1", LegacyOperation.DELETE, ObjectType.USER, new ObjectProperties("otherUser"));
+ "user1", LegacyOperation.DELETE, ObjectType.USER, new ObjectProperties("otherUser"));
validateRule(writeACLConfig("ACL ALLOW user1 UPDATE USER"),
- "user1", LegacyOperation.UPDATE, ObjectType.USER, ObjectProperties.EMPTY);
+ "user1", LegacyOperation.UPDATE, ObjectType.USER, EMPTY);
validateRule(writeACLConfig("ACL ALLOW user1 UPDATE USER name=\"otherUser\""),
- "user1", LegacyOperation.UPDATE, ObjectType.USER, new ObjectProperties("otherUser"));
+ "user1", LegacyOperation.UPDATE, ObjectType.USER, new ObjectProperties("otherUser"));
validateRule(writeACLConfig("ACL ALLOW user1 ALL USER"),
- "user1", LegacyOperation.ALL, ObjectType.USER, ObjectProperties.EMPTY);
+ "user1", LegacyOperation.ALL, ObjectType.USER, EMPTY);
validateRule(writeACLConfig("ACL ALLOW user1 ALL USER name=\"otherUser\""),
- "user1", LegacyOperation.ALL, ObjectType.USER, new ObjectProperties("otherUser"));
+ "user1", LegacyOperation.ALL, ObjectType.USER, new ObjectProperties("otherUser"));
}
@Test
public void testGroupRuleParsing() throws Exception
{
validateRule(writeACLConfig("ACL ALLOW user1 CREATE GROUP"),
- "user1", LegacyOperation.CREATE, ObjectType.GROUP, ObjectProperties.EMPTY);
+ "user1", LegacyOperation.CREATE, ObjectType.GROUP, EMPTY);
validateRule(writeACLConfig("ACL ALLOW user1 CREATE GROUP name=\"groupName\""),
- "user1", LegacyOperation.CREATE, ObjectType.GROUP, new ObjectProperties("groupName"));
+ "user1", LegacyOperation.CREATE, ObjectType.GROUP, new ObjectProperties("groupName"));
validateRule(writeACLConfig("ACL ALLOW user1 DELETE GROUP"),
- "user1", LegacyOperation.DELETE, ObjectType.GROUP, ObjectProperties.EMPTY);
+ "user1", LegacyOperation.DELETE, ObjectType.GROUP, EMPTY);
validateRule(writeACLConfig("ACL ALLOW user1 DELETE GROUP name=\"groupName\""),
- "user1", LegacyOperation.DELETE, ObjectType.GROUP, new ObjectProperties("groupName"));
+ "user1", LegacyOperation.DELETE, ObjectType.GROUP, new ObjectProperties("groupName"));
validateRule(writeACLConfig("ACL ALLOW user1 UPDATE GROUP"),
- "user1", LegacyOperation.UPDATE, ObjectType.GROUP, ObjectProperties.EMPTY);
+ "user1", LegacyOperation.UPDATE, ObjectType.GROUP, EMPTY);
validateRule(writeACLConfig("ACL ALLOW user1 UPDATE GROUP name=\"groupName\""),
- "user1", LegacyOperation.UPDATE, ObjectType.GROUP, new ObjectProperties("groupName"));
+ "user1", LegacyOperation.UPDATE, ObjectType.GROUP, new ObjectProperties("groupName"));
validateRule(writeACLConfig("ACL ALLOW user1 ALL GROUP"),
- "user1", LegacyOperation.ALL, ObjectType.GROUP, ObjectProperties.EMPTY);
+ "user1", LegacyOperation.ALL, ObjectType.GROUP, EMPTY);
validateRule(writeACLConfig("ACL ALLOW user1 ALL GROUP name=\"groupName\""),
- "user1", LegacyOperation.ALL, ObjectType.GROUP, new ObjectProperties("groupName"));
+ "user1", LegacyOperation.ALL, ObjectType.GROUP, new ObjectProperties("groupName"));
}
- /** explicitly test for exception indicating that this functionality has been moved to Group Providers */
+ /**
+ * explicitly test for exception indicating that this functionality has been moved to Group Providers
+ */
@Test
public void testGroupDefinitionThrowsException() throws Exception
{
@@ -456,40 +602,99 @@ public class AclFileParserTest extends UnitTestBase
writeACLConfig("GROUP group1 bob alice");
fail("Expected exception not thrown");
}
- catch(IllegalConfigurationException e)
+ catch (IllegalConfigurationException e)
{
assertTrue(e.getMessage().contains("GROUP keyword not supported"));
}
}
@Test
+ public void testUnknownDefinitionThrowsException() throws Exception
+ {
+ try
+ {
+ writeACLConfig("Unknown group1 bob alice");
+ fail("Expected exception not thrown");
+ }
+ catch (IllegalConfigurationException e)
+ {
+ assertEquals(String.format(AclFileParser.UNRECOGNISED_INITIAL_MSG, "Unknown", 1), e.getMessage());
+ }
+ }
+
+ @Test
public void testManagementRuleParsing() throws Exception
{
validateRule(writeACLConfig("ACL ALLOW user1 ALL MANAGEMENT"),
- "user1", LegacyOperation.ALL, ObjectType.MANAGEMENT, ObjectProperties.EMPTY);
+ "user1", LegacyOperation.ALL, ObjectType.MANAGEMENT, EMPTY);
validateRule(writeACLConfig("ACL ALLOW user1 ACCESS MANAGEMENT"),
- "user1", LegacyOperation.ACCESS, ObjectType.MANAGEMENT, ObjectProperties.EMPTY);
+ "user1", LegacyOperation.ACCESS, ObjectType.MANAGEMENT, EMPTY);
+ }
+
+ @Test
+ public void testDynamicRuleParsing() throws Exception
+ {
+ validateRule(writeACLConfig("ACL ALLOW all ACCESS VIRTUALHOST connection_limit=10 connection_frequency_limit=12"),
+ Rule.ALL, LegacyOperation.ACCESS, ObjectType.VIRTUALHOST, EMPTY);
}
@Test
public void testBrokerRuleParsing() throws Exception
{
- validateRule(writeACLConfig("ACL ALLOW user1 CONFIGURE BROKER"), "user1", LegacyOperation.CONFIGURE, ObjectType.BROKER,
- ObjectProperties.EMPTY);
- validateRule(writeACLConfig("ACL ALLOW user1 ALL BROKER"), "user1", LegacyOperation.ALL, ObjectType.BROKER, ObjectProperties.EMPTY);
+ validateRule(writeACLConfig("ACL ALLOW user1 CONFIGURE BROKER"), "user1", LegacyOperation.CONFIGURE, ObjectType.BROKER, EMPTY);
+ validateRule(writeACLConfig("ACL ALLOW user1 ALL BROKER"), "user1", LegacyOperation.ALL, ObjectType.BROKER, EMPTY);
+ }
+
+ @Test
+ public void testReaderIOException() throws IOException
+ {
+ Reader reader = mock(Reader.class);
+ doReturn(true).when(reader).ready();
+ doReturn(1L).when(reader).skip(Mockito.anyLong());
+ doThrow(IOException.class).when(reader).read();
+ doThrow(IOException.class).when(reader).read(Mockito.any(CharBuffer.class));
+ doThrow(IOException.class).when(reader).read(Mockito.any(char[].class));
+ doThrow(IOException.class).when(reader).read(Mockito.any(char[].class), Mockito.anyInt(), Mockito.anyInt());
+ try
+ {
+ new AclFileParser().readAndParse(reader);
+ fail("Expected exception not thrown");
+ }
+ catch (IllegalConfigurationException e)
+ {
+ assertEquals(AclFileParser.CANNOT_LOAD_MSG, e.getMessage());
+ }
}
- private void validateRule(final RuleSet rs, String username, LegacyOperation operation, ObjectType objectType, ObjectProperties objectProperties)
+ @Test
+ public void testReaderRuntimeException() throws IOException
{
- assertEquals((long) 1, (long) rs.getRuleCount());
+ Reader reader = mock(Reader.class);
+ doReturn(true).when(reader).ready();
+ doReturn(1L).when(reader).skip(Mockito.anyLong());
+ doThrow(RuntimeException.class).when(reader).read();
+ doThrow(RuntimeException.class).when(reader).read(Mockito.any(CharBuffer.class));
+ doThrow(RuntimeException.class).when(reader).read(Mockito.any(char[].class));
+ doThrow(RuntimeException.class).when(reader).read(Mockito.any(char[].class), Mockito.anyInt(), Mockito.anyInt());
+ try
+ {
+ new AclFileParser().readAndParse(reader);
+ fail("Expected exception not thrown");
+ }
+ catch (IllegalConfigurationException e)
+ {
+ assertEquals(String.format(AclFileParser.PARSE_TOKEN_FAILED_MSG, 0), e.getMessage());
+ }
+ }
- final List<Rule> rules = rs.getAllRules();
- assertEquals((long) 1, (long) rules.size());
- final Rule rule = rules.get(0);
+ private void validateRule(final RuleSet rules, String username, LegacyOperation operation, ObjectType objectType, ObjectProperties objectProperties)
+ {
+ assertEquals(1, rules.getAllRules().size());
+ final Rule rule = rules.getAllRules().get(0);
assertEquals("Rule has unexpected identity", username, rule.getIdentity());
assertEquals("Rule has unexpected operation", operation, rule.getAction().getOperation());
assertEquals("Rule has unexpected operation", objectType, rule.getAction().getObjectType());
- assertEquals("Rule has unexpected object properties", objectProperties, rule.getAction().getProperties());
+ assertEquals("Rule has unexpected object properties", objectProperties, rule.getAclAction().getAction().getProperties());
}
}
diff --git a/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/config/RuleSetCreatorTest.java b/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/config/RuleSetCreatorTest.java
new file mode 100644
index 0000000..e0508fb
--- /dev/null
+++ b/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/config/RuleSetCreatorTest.java
@@ -0,0 +1,69 @@
+/*
+ * 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.config;
+
+import junit.framework.TestCase;
+import org.apache.qpid.server.logging.EventLoggerProvider;
+import org.apache.qpid.server.security.access.plugins.RuleOutcome;
+import org.junit.Test;
+import org.mockito.Mockito;
+
+public class RuleSetCreatorTest extends TestCase
+{
+ @Test
+ public void testAddRule()
+ {
+ final RuleSetCreator creator = new RuleSetCreator();
+ creator.addRule(4, Rule.ALL, RuleOutcome.ALLOW, LegacyOperation.ACCESS);
+ creator.addRule(3, Rule.ALL, RuleOutcome.DENY, LegacyOperation.PUBLISH, ObjectType.EXCHANGE, new AclRulePredicates());
+ creator.addRule(6, Rule.ALL, RuleOutcome.ALLOW, LegacyOperation.ACCESS);
+ creator.addRule(7, Rule.ALL, RuleOutcome.DENY, LegacyOperation.PUBLISH, ObjectType.EXCHANGE, new AclRulePredicates());
+
+ RuleSet ruleSet = creator.createRuleSet(Mockito.mock(EventLoggerProvider.class));
+ assertNotNull(ruleSet);
+ assertEquals(2, ruleSet.getAllRules().size());
+ assertEquals(new Rule(Rule.ALL, new AclAction(LegacyOperation.ACCESS), RuleOutcome.ALLOW), ruleSet.getAllRules().get(1));
+ assertEquals(new Rule(Rule.ALL, new AclAction(LegacyOperation.PUBLISH, ObjectType.EXCHANGE, new AclRulePredicates()), RuleOutcome.DENY), ruleSet.getAllRules().get(0));
+ }
+
+ @Test
+ public void testIsValid()
+ {
+ final RuleSetCreator creator = new RuleSetCreator();
+ try
+ {
+ creator.addRule(3, Rule.ALL, RuleOutcome.DENY, LegacyOperation.DELETE, ObjectType.MANAGEMENT, new ObjectProperties());
+ fail("An exception is required");
+ }
+ catch (IllegalArgumentException e)
+ {
+ assertNotNull(e.getMessage());
+ }
+ }
+
+ @Test
+ public void testIsValidNumber()
+ {
+ final RuleSetCreator creator = new RuleSetCreator();
+ creator.addRule(4, Rule.ALL, RuleOutcome.ALLOW, LegacyOperation.ACCESS);
+ creator.addRule(3, Rule.ALL, RuleOutcome.DENY, LegacyOperation.PUBLISH, ObjectType.EXCHANGE, new AclRulePredicates());
+ assertTrue(creator.isValidNumber(5));
+ assertFalse(creator.isValidNumber(4));
+ }
+}
\ No newline at end of file
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org