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.l...@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-unsubscr...@qpid.apache.org For additional commands, e-mail: commits-h...@qpid.apache.org