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:42 UTC
[qpid-broker-j] 01/02: QPID-8548: [Broker-J] Enhance ACL file
loading and parsing
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