diff --git a/SPECIFICATION.md b/SPECIFICATION.md index eae4e08..6869625 100644 --- a/SPECIFICATION.md +++ b/SPECIFICATION.md @@ -49,14 +49,17 @@ and-expression = "&" (access-token / paren-expression) [and-expression or-expression = "|" (access-token / paren-expression) [or-expression] access-token = 1*( ALPHA / DIGIT / "_" / "-" / "." / ":" / slash ) -access-token =/ DQUOTE 1*(utf8-subset / escaped) DQUOTE +access-token =/ DQUOTE 1*(unicode-subset / escaped) DQUOTE -utf8-subset = %x20-21 / %x23-5B / %x5D-7E / unicode-beyond-ascii ; utf8 minus '"' and '\' -unicode-beyond-ascii = %x0080-D7FF / %xE000-10FFFF +unicode-subset = %x00-21 / %x23-5B / %x5D-7F / unicode-beyond-ascii ; unicode minus '"' and '\' escaped = "\" DQUOTE / "\\" slash = "/" ``` +Authorizations must be Unicode characters. Not all Unicode characters are human readable or even visible +(see [Unicode control characters][6]), implementations should provide a way to limit valid authorizations to +a subset of unicode characters (like human-readable characters). + ### Examples of Proper Expressions * `BLUE` @@ -73,8 +76,8 @@ slash = "/" ## Serialization -An AccessExpression is a UTF-8 string. It can be serialized using a byte array as long as it -can be deserialized back into the same UTF-8 string. +An access expression or authorization must be a Unicode string. Serialization of an access expression or authorization +should use UTF-8. ## Evaluation @@ -140,3 +143,4 @@ within the Authorizations object, the token is unquoted, and the `\` character i [3]: https://en.wikipedia.org/wiki/Boolean_algebra [4]: https://en.wikipedia.org/wiki/Logical_conjunction [5]: https://en.wikipedia.org/wiki/Logical_disjunction +[6]: https://en.wikipedia.org/wiki/Unicode_control_characters diff --git a/antlr4-example/src/main/java/org/apache/accumulo/access/antlr4/AccessExpressionAntlrEvaluator.java b/antlr4-example/src/main/java/org/apache/accumulo/access/antlr4/AccessExpressionAntlrEvaluator.java index 0d1ba19..d9bd39e 100644 --- a/antlr4-example/src/main/java/org/apache/accumulo/access/antlr4/AccessExpressionAntlrEvaluator.java +++ b/antlr4-example/src/main/java/org/apache/accumulo/access/antlr4/AccessExpressionAntlrEvaluator.java @@ -25,9 +25,8 @@ import org.antlr.v4.runtime.tree.ParseTree; import org.antlr.v4.runtime.tree.TerminalNode; -import org.apache.accumulo.access.AccessExpression; +import org.apache.accumulo.access.Access; import org.apache.accumulo.access.Authorizations; -import org.apache.accumulo.access.InvalidAccessExpressionException; import org.apache.accumulo.access.grammars.AccessExpressionParser.Access_expressionContext; import org.apache.accumulo.access.grammars.AccessExpressionParser.Access_tokenContext; import org.apache.accumulo.access.grammars.AccessExpressionParser.And_expressionContext; @@ -37,6 +36,8 @@ public class AccessExpressionAntlrEvaluator { + public static final Access ACCESS = Access.builder().build(); + private class Entity { private Set authorizations; @@ -60,7 +61,7 @@ public AccessExpressionAntlrEvaluator(List authSets) { e.authorizations = new HashSet<>(entityAuths.size() * 2); a.asSet().stream().forEach(auth -> { e.authorizations.add(auth); - String quoted = AccessExpression.quote(auth); + String quoted = ACCESS.quote(auth); if (!quoted.startsWith("\"")) { quoted = '"' + quoted + '"'; } @@ -69,14 +70,6 @@ public AccessExpressionAntlrEvaluator(List authSets) { } } - public boolean canAccess(byte[] accessExpression) throws InvalidAccessExpressionException { - return canAccess(AccessExpression.of(accessExpression)); - } - - public boolean canAccess(AccessExpression accessExpression) { - return canAccess(accessExpression.getExpression()); - } - public boolean canAccess(String accessExpression) { if ("".equals(accessExpression)) { return true; diff --git a/antlr4-example/src/test/java/org/apache/accumulo/access/grammar/antlr/AccessExpressionAntlrBenchmark.java b/antlr4-example/src/test/java/org/apache/accumulo/access/grammar/antlr/AccessExpressionAntlrBenchmark.java index e32593a..acf4905 100644 --- a/antlr4-example/src/test/java/org/apache/accumulo/access/grammar/antlr/AccessExpressionAntlrBenchmark.java +++ b/antlr4-example/src/test/java/org/apache/accumulo/access/grammar/antlr/AccessExpressionAntlrBenchmark.java @@ -19,6 +19,7 @@ package org.apache.accumulo.access.grammar.antlr; import static java.nio.charset.StandardCharsets.UTF_8; +import static org.apache.accumulo.access.grammar.antlr.Antlr4Tests.ACCESS; import java.io.IOException; import java.net.URISyntaxException; @@ -29,7 +30,6 @@ import java.util.stream.Collectors; import java.util.stream.Stream; -import org.apache.accumulo.access.Authorizations; import org.apache.accumulo.access.antlr.TestDataLoader; import org.apache.accumulo.access.antlr4.AccessExpressionAntlrEvaluator; import org.apache.accumulo.access.antlr4.AccessExpressionAntlrParser; @@ -64,7 +64,7 @@ public static class EvaluatorTests { List parsedExpressions; - List expressions; + List expressions; } @State(Scope.Benchmark) @@ -89,7 +89,7 @@ public void loadData() throws IOException, URISyntaxException { et.expressions = new ArrayList<>(); et.evaluator = new AccessExpressionAntlrEvaluator(Stream.of(testDataSet.auths) - .map(a -> Authorizations.of(Set.of(a))).collect(Collectors.toList())); + .map(a -> ACCESS.newAuthorizations(Set.of(a))).collect(Collectors.toList())); for (var tests : testDataSet.tests) { if (tests.expectedResult != TestDataLoader.ExpectedResult.ERROR) { @@ -97,7 +97,7 @@ public void loadData() throws IOException, URISyntaxException { allTestExpressionsStr.add(exp); byte[] byteExp = exp.getBytes(UTF_8); allTestExpressions.add(byteExp); - et.expressions.add(byteExp); + et.expressions.add(exp); et.parsedExpressions.add(AccessExpressionAntlrParser.parseAccessExpression(exp)); } } @@ -160,7 +160,7 @@ public void measureEvaluation(BenchmarkState state, Blackhole blackhole) { @Benchmark public void measureEvaluationAndParsing(BenchmarkState state, Blackhole blackhole) { for (EvaluatorTests evaluatorTests : state.getEvaluatorTests()) { - for (byte[] expression : evaluatorTests.expressions) { + for (String expression : evaluatorTests.expressions) { blackhole.consume(evaluatorTests.evaluator.canAccess(expression)); } } diff --git a/antlr4-example/src/test/java/org/apache/accumulo/access/grammar/antlr/Antlr4Tests.java b/antlr4-example/src/test/java/org/apache/accumulo/access/grammar/antlr/Antlr4Tests.java index 3ed6a9e..6d74196 100644 --- a/antlr4-example/src/test/java/org/apache/accumulo/access/grammar/antlr/Antlr4Tests.java +++ b/antlr4-example/src/test/java/org/apache/accumulo/access/grammar/antlr/Antlr4Tests.java @@ -18,7 +18,6 @@ */ package org.apache.accumulo.access.grammar.antlr; -import static java.nio.charset.StandardCharsets.UTF_8; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNull; @@ -40,7 +39,7 @@ import org.antlr.v4.runtime.RecognitionException; import org.antlr.v4.runtime.Recognizer; import org.apache.accumulo.access.AccessEvaluator; -import org.apache.accumulo.access.AccessExpression; +import org.apache.accumulo.access.Access; import org.apache.accumulo.access.Authorizations; import org.apache.accumulo.access.InvalidAccessExpressionException; import org.apache.accumulo.access.antlr.TestDataLoader; @@ -55,6 +54,8 @@ public class Antlr4Tests { + public static final Access ACCESS = Access.builder().build(); + private void testParse(String input) throws Exception { CodePointCharStream expression = CharStreams.fromString(input); final AtomicLong errors = new AtomicLong(0); @@ -108,10 +109,10 @@ public void testCompareWithAccessExpressionImplParsing() throws Exception { ExpectedResult result = test.expectedResult; for (String cv : test.expressions) { if (result == ExpectedResult.ERROR) { - assertThrows(InvalidAccessExpressionException.class, () -> AccessExpression.of(cv)); + assertThrows(InvalidAccessExpressionException.class, () -> ACCESS.newExpression(cv)); assertThrows(AssertionError.class, () -> testParse(cv)); } else { - AccessExpression.of(cv); + ACCESS.validateExpression(cv); testParse(cv); } } @@ -122,7 +123,7 @@ public void testCompareWithAccessExpressionImplParsing() throws Exception { @Test public void testSimpleEvaluation() throws Exception { String accessExpression = "(one&two)|(foo&bar)"; - Authorizations auths = Authorizations.of(Set.of("four", "three", "one", "two")); + Authorizations auths = ACCESS.newAuthorizations(Set.of("four", "three", "one", "two")); AccessExpressionAntlrEvaluator eval = new AccessExpressionAntlrEvaluator(List.of(auths)); assertTrue(eval.canAccess(accessExpression)); } @@ -130,7 +131,7 @@ public void testSimpleEvaluation() throws Exception { @Test public void testSimpleEvaluationFailure() throws Exception { String accessExpression = "(A&B&C)"; - Authorizations auths = Authorizations.of(Set.of("A", "C")); + Authorizations auths = ACCESS.newAuthorizations(Set.of("A", "C")); AccessExpressionAntlrEvaluator eval = new AccessExpressionAntlrEvaluator(List.of(auths)); assertFalse(eval.canAccess(accessExpression)); } @@ -143,8 +144,8 @@ public void testCompareAntlrEvaluationAgainstAccessEvaluatorImpl() throws Except for (TestDataSet testSet : testData) { List authSets = Stream.of(testSet.auths) - .map(a -> Authorizations.of(Set.of(a))).collect(Collectors.toList()); - AccessEvaluator evaluator = AccessEvaluator.of(authSets); + .map(a -> ACCESS.newAuthorizations(Set.of(a))).collect(Collectors.toList()); + AccessEvaluator evaluator = ACCESS.newEvaluator(authSets); AccessExpressionAntlrEvaluator antlr = new AccessExpressionAntlrEvaluator(authSets); for (TestExpressions test : testSet.tests) { @@ -152,58 +153,20 @@ public void testCompareAntlrEvaluationAgainstAccessEvaluatorImpl() throws Except switch (test.expectedResult) { case ACCESSIBLE: assertTrue(evaluator.canAccess(expression), expression); - assertTrue(evaluator.canAccess(expression.getBytes(UTF_8)), expression); - assertTrue(evaluator.canAccess(AccessExpression.of(expression)), expression); - assertTrue(evaluator.canAccess(AccessExpression.of(expression.getBytes(UTF_8))), - expression); - assertEquals(expression, - AccessExpression.of(expression.getBytes(UTF_8)).getExpression()); - assertEquals(expression, AccessExpression.of(expression).getExpression()); - assertTrue(antlr.canAccess(expression), expression); - assertTrue(antlr.canAccess(expression.getBytes(UTF_8)), expression); - assertTrue(antlr.canAccess(AccessExpression.of(expression)), expression); - assertTrue(antlr.canAccess(AccessExpression.of(expression.getBytes(UTF_8))), - expression); break; case INACCESSIBLE: assertFalse(evaluator.canAccess(expression), expression); - assertFalse(evaluator.canAccess(expression.getBytes(UTF_8)), expression); - assertFalse(evaluator.canAccess(AccessExpression.of(expression)), expression); - assertFalse(evaluator.canAccess(AccessExpression.of(expression.getBytes(UTF_8))), - expression); - assertEquals(expression, - AccessExpression.of(expression.getBytes(UTF_8)).getExpression()); - assertEquals(expression, AccessExpression.of(expression).getExpression()); - assertFalse(antlr.canAccess(expression), expression); - assertFalse(antlr.canAccess(expression.getBytes(UTF_8)), expression); - assertFalse(antlr.canAccess(AccessExpression.of(expression)), expression); - assertFalse(antlr.canAccess(AccessExpression.of(expression.getBytes(UTF_8))), - expression); break; case ERROR: assertThrows(InvalidAccessExpressionException.class, () -> evaluator.canAccess(expression), expression); - assertThrows(InvalidAccessExpressionException.class, - () -> evaluator.canAccess(expression.getBytes(UTF_8)), expression); - assertThrows(InvalidAccessExpressionException.class, - () -> evaluator.canAccess(AccessExpression.of(expression)), expression); - assertThrows(InvalidAccessExpressionException.class, - () -> evaluator.canAccess(AccessExpression.of(expression.getBytes(UTF_8))), - expression); assertThrows(InvalidAccessExpressionException.class, () -> antlr.canAccess(expression), expression); - assertThrows(InvalidAccessExpressionException.class, - () -> antlr.canAccess(expression.getBytes(UTF_8)), expression); - assertThrows(InvalidAccessExpressionException.class, - () -> antlr.canAccess(AccessExpression.of(expression)), expression); - assertThrows(InvalidAccessExpressionException.class, - () -> antlr.canAccess(AccessExpression.of(expression.getBytes(UTF_8))), - expression); break; default: throw new IllegalArgumentException(); diff --git a/core/src/main/java/org/apache/accumulo/access/Access.java b/core/src/main/java/org/apache/accumulo/access/Access.java new file mode 100644 index 0000000..4f35f23 --- /dev/null +++ b/core/src/main/java/org/apache/accumulo/access/Access.java @@ -0,0 +1,219 @@ +/* + * 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 + * + * https://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.accumulo.access; + +import java.util.Collection; +import java.util.Set; +import java.util.function.Consumer; + +import org.apache.accumulo.access.impl.BuilderImpl; + +/** + * The entry point into Accumulo Access to create access expressions, expression evaluators, and + * authorization sets. + * + * @see #builder() + * @since 1.0 + */ +public interface Access { + + interface Builder { + /** + * Provide a validator to accumulo access to narrow the set of valid authorizations for your + * specific use case. If one is not provided then {@link AuthorizationValidator#DEFAULT} will be + * used. + * + *

+ * The provided validator is called very frequently within accumulo access and implementations + * that are slow will slow down accumulo access. + */ + Builder authorizationValidator(AuthorizationValidator validator); + + Access build(); + } + + /** + * Used to create an instance of AccumuloAccess. For efficiency, the recommend way to use this is + * to create a single instance and somehow make it available to an entire project for use. In + * addition to being efficient this ensures the entire project is using the same configuration. + */ + static Builder builder() { + return new BuilderImpl(); + } + + /** + * Validates an access expression and returns an immutable AccessExpression object. If passing + * access expressions as arguments in code, consider using this type instead of a String. The + * advantage of passing this type over a String is that its known to be a valid expression. Also, + * this type is much more informative than a String type. Conceptually this method calls + * {@link #validateExpression(String)} and if that passes creates an immutable object that wraps + * the expression. + * + * @throws InvalidAccessExpressionException if the given expression is not valid + * @throws InvalidAuthorizationException when the expression contains an authorization that is not + * valid + * @throws NullPointerException when the argument is null + */ + AccessExpression newExpression(String expression) + throws InvalidAccessExpressionException, InvalidAuthorizationException; + + /** + * Quickly validates that an access expression is properly formed. + * + * @param expression a potential access expression that + * @throws InvalidAccessExpressionException if the given expression is not valid + * @throws InvalidAuthorizationException if the expression contains an invalid authorization + * @throws NullPointerException when the argument is null + */ + void validateExpression(String expression) + throws InvalidAccessExpressionException, InvalidAuthorizationException; + + /** + * Validates an access expression and returns an immutable object with a parse tree. Creating the + * parse tree is expensive relative to calling {@link #newExpression(String)} or + * {@link #validateExpression(String)}, so only use this method when the parse tree is always + * needed. If the code may only use the parse tree sometimes, then it may be best to call + * {@link #newExpression(String)} to create the access expression and then call + * {@link AccessExpression#parse()} when needed. + * + * @throws NullPointerException when the argument is null + * @throws InvalidAuthorizationException when the expression contains an authorization that is not + * valid + * @throws InvalidAccessExpressionException if the given expression is not valid + */ + ParsedAccessExpression newParsedExpression(String expression) + throws InvalidAccessExpressionException, InvalidAuthorizationException; + + /** + * Creates an Authorizations object from the set of input authorization strings. + * + * @param authorizations set of authorization strings + * @throws InvalidAuthorizationException when the expression contains an authorization that is not + * valid + * @return Authorizations object + */ + Authorizations newAuthorizations(Set authorizations) throws InvalidAuthorizationException; + + /** + * Validates an access expression and finds all authorizations in it passing them to the + * authorizationConsumer. For example, for the expression {@code (A&B)|(A&C)|(A&D)}, this method + * would pass {@code A,B,A,C,A,D} to the consumer one at a time. The function will conceptually + * call {@link #unquote(String)} prior to passing an authorization to authorizationConsumer. + * + *

+ * What this method does could also be accomplished by creating a parse tree using + * {@link Access#newParsedExpression(String)} and then recursively walking the parse tree. The + * implementation of this method does not create a parse tree and is much faster. If a parse tree + * is already available, then it would likely be faster to use it rather than call this method. + *

+ * + * @throws InvalidAccessExpressionException when the expression is not valid. + * @throws InvalidAuthorizationException when the expression contains an authorization that is not + * valid + * @throws NullPointerException when any argument is null + */ + void findAuthorizations(String expression, Consumer authorizationConsumer) + throws InvalidAccessExpressionException, InvalidAuthorizationException; + + /** + * Authorizations occurring in an access expression can only contain the characters listed in the + * specification unless + * quoted (surrounded by quotation marks). Use this method to quote authorizations that occur in + * an access expression. This method will only quote if it is needed. + * + * @throws NullPointerException when the argument is null + */ + String quote(String authorization) throws InvalidAuthorizationException; + + /** + * Reverses what {@link #quote(String)} does, so will unquote and unescape an authorization if + * needed. If the authorization is not quoted then it is returned as-is. + * + * @throws NullPointerException when the argument is null + */ + String unquote(String authorization) throws InvalidAuthorizationException; + + /** + * Creates an AccessEvaluator from an Authorizations object + * + * @param authorizations auths to use in the AccessEvaluator + * @return AccessEvaluator object + */ + AccessEvaluator newEvaluator(Authorizations authorizations); + + /** + * Creates an AccessEvaluator from an Authorizer + * + * @param authorizer authorizer to use in the AccessEvaluator + * @return AccessEvaluator object + */ + AccessEvaluator newEvaluator(AccessEvaluator.Authorizer authorizer); + + /** + * Creates an AccessEvaluator from multiple sets of authorizations. Each expression will be + * evaluated independently against each set of authorizations and will only be deemed accessible + * if accessible for all. For example the following code would print false, true, and then false. + * + * 
+   *     {@code
+   * AccumuloAccess accumuloAccess = ...;
+   * Collection authSets =
+   *     List.of(Authorizations.of("A", "B"), Authorizations.of("C", "D"));
+   * var evaluator = accumuloAccess.newEvaluator(authSets);
+   *
+   * System.out.println(evaluator.canAccess("A"));
+   * System.out.println(evaluator.canAccess("A|D"));
+   * System.out.println(evaluator.canAccess("A&D"));
+   *
+   * }
+   *
+ * + *

+ * The following table shows how each expression in the example above will evaluate for each + * authorization set. In order to return true for {@code canAccess()} the expression must evaluate + * to true for each authorization set. + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + *
Evaluations
[A,B][C,D]
ATrueFalse
A|DTrueTrue
A&DFalseFalse
+ */ + AccessEvaluator newEvaluator(Collection authorizationSets); +} diff --git a/core/src/main/java/org/apache/accumulo/access/AccessEvaluator.java b/core/src/main/java/org/apache/accumulo/access/AccessEvaluator.java index ea0fb2c..faa3472 100644 --- a/core/src/main/java/org/apache/accumulo/access/AccessEvaluator.java +++ b/core/src/main/java/org/apache/accumulo/access/AccessEvaluator.java @@ -18,8 +18,6 @@ */ package org.apache.accumulo.access; -import java.util.Collection; - import org.apache.accumulo.access.impl.AccessEvaluatorImpl; import org.apache.accumulo.access.impl.MultiAccessEvaluatorImpl; @@ -65,14 +63,6 @@ public sealed interface AccessEvaluator permits AccessEvaluatorImpl, MultiAccess */ boolean canAccess(String accessExpression) throws InvalidAccessExpressionException; - /** - * @param accessExpression for this parameter a valid access expression is expected. - * @return true if the expression is visible using the authorizations supplied at creation, false - * otherwise - * @throws InvalidAccessExpressionException when the expression is not valid - */ - boolean canAccess(byte[] accessExpression) throws InvalidAccessExpressionException; - /** * @param accessExpression previously validated access expression * @return true if the expression is visible using the authorizations supplied at creation, false @@ -80,81 +70,6 @@ public sealed interface AccessEvaluator permits AccessEvaluatorImpl, MultiAccess */ boolean canAccess(AccessExpression accessExpression); - /** - * Creates an AccessEvaluator from an Authorizations object - * - * @param authorizations auths to use in the AccessEvaluator - * @return AccessEvaluator object - */ - static AccessEvaluator of(Authorizations authorizations) { - return new AccessEvaluatorImpl(authorizations); - } - - /** - * Creates an AccessEvaluator from an Authorizer object - * - * @param authorizer authorizer to use in the AccessEvaluator - * @return AccessEvaluator object - */ - static AccessEvaluator of(Authorizer authorizer) { - return new AccessEvaluatorImpl(authorizer); - } - - /** - * Allows providing multiple sets of authorizations. Each expression will be evaluated - * independently against each set of authorizations and will only be deemed accessible if - * accessible for all. For example the following code would print false, true, and then false. - * - * 

-   *     {@code
-   * Collection authSets =
-   *     List.of(Authorizations.of("A", "B"), Authorizations.of("C", "D"));
-   * var evaluator = AccessEvaluator.of(authSets);
-   *
-   * System.out.println(evaluator.canAccess("A"));
-   * System.out.println(evaluator.canAccess("A|D"));
-   * System.out.println(evaluator.canAccess("A&D"));
-   *
-   * }
-   *
- * - *

- * The following table shows how each expression in the example above will evaluate for each - * authorization set. In order to return true for {@code canAccess()} the expression must evaluate - * to true for each authorization set. - * - * - * - * - * - * - * - * - * - * - * - * - * - * - * - * - * - * - * - * - * - * - * - * - *
Evaluations
[A,B][C,D]
ATrueFalse
A|DTrueTrue
A&DFalseFalse
- * - * - * - */ - static AccessEvaluator of(Collection authorizationSets) { - return MultiAccessEvaluatorImpl.of(authorizationSets); - } - /** * An interface that is used to check if an authorization seen in an access expression is * authorized. diff --git a/core/src/main/java/org/apache/accumulo/access/AccessExpression.java b/core/src/main/java/org/apache/accumulo/access/AccessExpression.java index cf32670..7c7de37 100644 --- a/core/src/main/java/org/apache/accumulo/access/AccessExpression.java +++ b/core/src/main/java/org/apache/accumulo/access/AccessExpression.java @@ -18,87 +18,13 @@ */ package org.apache.accumulo.access; -import static java.nio.charset.StandardCharsets.UTF_8; - import java.io.Serializable; -import java.util.Arrays; -import java.util.function.Consumer; -import java.util.function.Predicate; -import org.apache.accumulo.access.impl.AccessEvaluatorImpl; import org.apache.accumulo.access.impl.AccessExpressionImpl; -import org.apache.accumulo.access.impl.BytesWrapper; -import org.apache.accumulo.access.impl.ParsedAccessExpressionImpl; -import org.apache.accumulo.access.impl.ParserEvaluator; -import org.apache.accumulo.access.impl.Tokenizer; /** - * This class offers the ability to operate on access expressions. - * - *

- * Below is an example of how to use this API. - * - * 

- * {@code
- * // The following authorization does not need quoting
- * // so the return value is the same as the input.
- * var auth1 = AccessExpression.quote("CAT");
- *
- * // The following two authorizations need quoting and the return values will be quoted.
- * var auth2 = AccessExpression.quote("πŸ¦•");
- * var auth3 = AccessExpression.quote("πŸ¦–");
- *
- * // Create an AccessExpression using auth1, auth2, and auth3
- * var exp = "(" + auth1 + "&" + auth3 + ")|(" + auth1 + "&" + auth2 + ")";
- *
- * // Validate the expression w/o creating an object
- * AccessExpression.validate(exp);
- * System.out.println(exp);
- *
- * // Validate the expression and create an immutable AccessExpression object.  This object can be passed around in code and other code knows it valid and does not need to revalidate.
- * AccessExpression accessExpression = AccessExpression.of(exp);
- * System.out.println(accessExpression);
- *
- * // Print the authorization in the expression
- * AccessExpression.findAuthorizations(exp, System.out::println);
- *
- * // Create an AccessExpression with a parse tree.  Creating this is more expensive than calling AccessExpression.of(), so it should only be used if the parse tree is needed.
- * ParsedAccessExpression parsed = AccessExpression.parse(exp);
- * System.out.println("type:"+parsed.getType()+" child[0]:"+parsed.getChildren().get(0)+" child[1]:"+  child[1]:"+parsed.getChildren().get(1));
- *
- * }
- *
+ * An immutable wrapper for a validated access expression. * - * The above example will print the following. - * - * 
- * {@code
- * (CAT&"πŸ¦–")|(CAT&"πŸ¦•")
- * (CAT&"πŸ¦–")|(CAT&"πŸ¦•")
- * CAT
- * πŸ¦–
- * CAT
- * πŸ¦•
- * type:OR child[0]:CAT&"πŸ¦–" child[1]:CAT&"πŸ¦•"
- * }
- *
- * - * The following code will throw an {@link InvalidAccessExpressionException} because the expression - * is not valid. - * - * 
- * {@code
- * AccessExpression.validate("A&B|C");
- * }
- *
- * - *

- * Instances of this class are thread-safe. - * - *

- * Note: The underlying implementation uses UTF-8 when converting between bytes and Strings. - * - * @see Accumulo Access Documentation * @since 1.0.0 */ public sealed abstract class AccessExpression implements Serializable @@ -115,10 +41,10 @@ protected AccessExpression() {} /** * Parses the access expression if it was never parsed before. If this access expression was - * created using {@link #parse(String)} or {@link #parse(byte[])} then it will have a parse from + * created using {@link Access#newParsedExpression(String)} then it will have a parse from * inception and this method will return itself. If the access expression was created using - * {@link #of(String)} or {@link #of(byte[])} then this method will create a parse tree the first - * time its called and remember it, returning the remembered parse tree on subsequent calls. + * {@link Access#newExpression(String)} then this method will create a parse tree the first time + * its called and remember it, returning the remembered parse tree on subsequent calls. */ public abstract ParsedAccessExpression parse(); @@ -140,180 +66,4 @@ public int hashCode() { public String toString() { return getExpression(); } - - /** - * Validates an access expression and returns an immutable AccessExpression object. If passing - * access expressions as arguments in code, consider using this type instead of a String. The - * advantage of passing this type over a String is that its known to be a valid expression. Also - * this type is much more informative than a String type. Conceptually this method calls - * {@link #validate(String)} and if that passes creates an immutable object that wraps the - * expression. - * - * @throws InvalidAccessExpressionException if the given expression is not valid - * @throws NullPointerException when the argument is null - */ - public static AccessExpression of(String expression) throws InvalidAccessExpressionException { - return new AccessExpressionImpl(expression); - } - - /** - * @see #of(String) - */ - public static AccessExpression of(byte[] expression) throws InvalidAccessExpressionException { - return new AccessExpressionImpl(expression); - } - - /** - * @return an empty AccessExpression that is immutable. - */ - public static AccessExpression of() { - return AccessExpressionImpl.EMPTY; - } - - /** - * @see #parse(String) - */ - public static ParsedAccessExpression parse(byte[] expression) - throws InvalidAccessExpressionException { - if (expression.length == 0) { - return ParsedAccessExpressionImpl.EMPTY; - } - - return ParsedAccessExpressionImpl.parseExpression(Arrays.copyOf(expression, expression.length)); - } - - /** - * Validates an access expression and returns an immutable object with a parse tree. Creating the - * parse tree is expensive relative to calling {@link #of(String)} or {@link #validate(String)}, - * so only use this method when the parse tree is always needed. If the code may only use the - * parse tree sometimes, then it may be best to call {@link #of(String)} to create the access - * expression and then call {@link AccessExpression#parse()} when needed. - * - * @throws NullPointerException when the argument is null - * @throws InvalidAccessExpressionException if the given expression is not valid - */ - public static ParsedAccessExpression parse(String expression) - throws InvalidAccessExpressionException { - if (expression.isEmpty()) { - return ParsedAccessExpressionImpl.EMPTY; - } - // Calling expression.getBytes(UTF8) will create a byte array that only this code has access to, - // so not need to copy that byte array. That is why parse(byte[]) is not called here because - // that would copy the array again. - return ParsedAccessExpressionImpl.parseExpression(expression.getBytes(UTF_8)); - } - - /** - * Quickly validates that an access expression is properly formed. - * - * @param expression a potential access expression that is expected to be encoded using UTF-8 - * @throws InvalidAccessExpressionException if the given expression is not valid - * @throws NullPointerException when the argument is null - */ - public static void validate(byte[] expression) throws InvalidAccessExpressionException { - if (expression.length > 0) { - Predicate atp = authToken -> true; - ParserEvaluator.parseAccessExpression(expression, atp, atp); - } // else empty expression is valid, avoid object allocation - } - - /** - * @see #validate(byte[]) - */ - public static void validate(String expression) throws InvalidAccessExpressionException { - if (!expression.isEmpty()) { - validate(expression.getBytes(UTF_8)); - } // else empty expression is valid, avoid object allocation - } - - /** - * Validates and access expression and finds all authorizations in it passing them to the - * authorizationConsumer. For example, for the expression {@code (A&B)|(A&C)|(A&D)}, this method - * would pass {@code A,B,A,C,A,D} to the consumer one at a time. The function will conceptually - * call {@link #unquote(String)} prior to passing an authorization to authorizationConsumer. - * - *

- * What this method does could also be accomplished by creating a parse tree using - * {@link AccessExpression#parse(String)} and then recursively walking the parse tree. The - * implementation of this method does not create a parse tree and is much faster. If a parse tree - * is already available, then it would likely be faster to use it rather than call this method. - *

- * - * @throws InvalidAccessExpressionException when the expression is not valid. - * @throws NullPointerException when any argument is null - */ - public static void findAuthorizations(String expression, Consumer authorizationConsumer) - throws InvalidAccessExpressionException { - findAuthorizations(expression.getBytes(UTF_8), authorizationConsumer); - } - - /** - * @see #findAuthorizations(String, Consumer) - */ - public static void findAuthorizations(byte[] expression, Consumer authorizationConsumer) - throws InvalidAccessExpressionException { - ParserEvaluator.findAuthorizations(expression, authorizationConsumer); - } - - /** - * Authorizations occurring in an access expression can only contain the characters listed in the - * specification unless - * quoted (surrounded by quotation marks). Use this method to quote authorizations that occur in - * an access expression. This method will only quote if it is needed. - * - * @throws NullPointerException when the argument is null - */ - public static byte[] quote(byte[] term) { - if (term.length == 0) { - throw new IllegalArgumentException("Empty strings are not legal authorizations."); - } - - boolean needsQuote = false; - - for (byte b : term) { - if (!Tokenizer.isValidAuthChar(b)) { - needsQuote = true; - break; - } - } - - if (!needsQuote) { - return term; - } - - return AccessEvaluatorImpl.escape(term, true); - } - - /** - * Authorizations occurring in an access expression can only contain the characters listed in the - * specification unless - * quoted (surrounded by quotation marks). Use this method to quote authorizations that occur in - * an access expression. This method will only quote if it is needed. - * - * @throws NullPointerException when the argument is null - */ - public static String quote(String term) { - return new String(quote(term.getBytes(UTF_8)), UTF_8); - } - - /** - * Reverses what {@link #quote(String)} does, so will unquote and unescape an authorization if - * needed. If the authorization is not quoted then it is returned as-is. - * - * @throws NullPointerException when the argument is null - */ - public static String unquote(String term) { - if (term.equals("\"\"") || term.isEmpty()) { - throw new IllegalArgumentException("Empty strings are not legal authorizations."); - } - - if (term.charAt(0) == '"' && term.charAt(term.length() - 1) == '"') { - term = term.substring(1, term.length() - 1); - return AccessEvaluatorImpl.unescape(new BytesWrapper(term.getBytes(UTF_8))); - } else { - return term; - } - } } diff --git a/core/src/main/java/org/apache/accumulo/access/AuthorizationValidator.java b/core/src/main/java/org/apache/accumulo/access/AuthorizationValidator.java new file mode 100644 index 0000000..7baa907 --- /dev/null +++ b/core/src/main/java/org/apache/accumulo/access/AuthorizationValidator.java @@ -0,0 +1,115 @@ +/* + * 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 + * + * https://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.accumulo.access; + +import java.nio.charset.CharsetDecoder; +import java.util.function.BiPredicate; + +/** + * Implementations validate authorizations for Accumulo Access. Creating implementations that are + * stricter for a given domain can help avoid expressions that contain unexpected and unused + * authorizations. + * + *

+ * When an authorization is quoted and/or escaped in access expression that is undone before is + * passed to this predicate. Conceptually it is like {@link Access#unquote(String)} is called prior + * to being passed to this predicate. If the authorization was quoted that information is passed + * along as it may be useful for optimizations. + * + *

+ * A CharSequence is passed to this predicate for efficiency. It allows having a view into the + * larger expression at parse time without any memory allocations. It is not safe to keep a + * reference to the passed in char sequence as it is only stable while the predicate is called. If a + * reference needs to be kept for some side effect, then call {@code toString()} to allocate a copy. + * Avoiding calls to {@code toString()} will result in faster parsing. + *

+ * + * @since 1.0.0 + */ +public interface AuthorizationValidator + extends BiPredicate { + + /** + * @since 1.0.0 + */ + enum AuthorizationCharacters { + /** + * This authorization could potentially contain any java character. + */ + ANY, + /** + * Authorization only contains the characters + * + * 
{@code [0-9a-zA-Z_-.:/] }
+ */ + BASIC + } + + /** + * This is the default validator for Accumulo Access. It does the following check of characters in + * an authorization. + * + * 
+   *     {@code
+   *     AuthorizationValidator DEFAULT = (auth, authChars) -> {
+   *       if (authChars == AuthorizationCharacters.BASIC) {
+   *         // The authorization is already known to only contain a small set of ASCII chars and no
+   *         // further validation is needed.
+   *         return true;
+   *       }
+   *
+   *       // Unsure what characters are present, so must validate them all.
+   *       for (int i = 0; i < auth.length(); i++) {
+   *         var c = auth.charAt(i);
+   *         if (!Character.isDefined(c) || Character.isISOControl(c) || c == '\uFFFD') {
+   *           return false;
+   *         }
+   *       }
+   *     }
+   *     }
+   *
+ * + *

+ * The character U+FFFD is the Unicode replacement character and standard java libraries will + * insert this into strings when it has problem decoding UTF-8. Therefore, seeing this character + * likely means a java string was derived from corrupt or invalid UTF-8 data. This is why its + * considered invalid in an authorization by default. + * + * @see Character#isDefined(char) + * @see Character#isISOControl(char) + * @see CharsetDecoder#replacement() + * @since 1.0.0 + */ + AuthorizationValidator DEFAULT = (auth, authChars) -> { + if (authChars == AuthorizationCharacters.BASIC) { + // The authorization is already known to only contain a small set of ASCII chars and no + // further validation is needed. + return true; + } + + // Unsure what characters are present, so must validate them all. + for (int i = 0; i < auth.length(); i++) { + var c = auth.charAt(i); + if (!Character.isDefined(c) || Character.isISOControl(c) || c == '\uFFFD') { + return false; + } + } + return true; + }; +} diff --git a/core/src/main/java/org/apache/accumulo/access/Authorizations.java b/core/src/main/java/org/apache/accumulo/access/Authorizations.java index 407b2eb..f3945d7 100644 --- a/core/src/main/java/org/apache/accumulo/access/Authorizations.java +++ b/core/src/main/java/org/apache/accumulo/access/Authorizations.java @@ -19,7 +19,6 @@ package org.apache.accumulo.access; import java.io.Serializable; -import java.util.Iterator; import java.util.Set; /** @@ -28,75 +27,8 @@ *

* Instances of this class are thread-safe. * - *

- * Note: The underlying implementation uses UTF-8 when converting between bytes and Strings. - * * @since 1.0.0 */ -public final class Authorizations implements Iterable, Serializable { - - private static final long serialVersionUID = 1L; - - private static final Authorizations EMPTY = new Authorizations(Set.of()); - - private final Set authorizations; - - private Authorizations(Set authorizations) { - this.authorizations = Set.copyOf(authorizations); - } - - /** - * Returns the set of authorization strings in this Authorization object - * - * @return immutable set of authorization strings - */ - public Set asSet() { - return authorizations; - } - - @Override - public boolean equals(Object o) { - if (o instanceof Authorizations) { - var oa = (Authorizations) o; - return authorizations.equals(oa.authorizations); - } - - return false; - } - - @Override - public int hashCode() { - return authorizations.hashCode(); - } - - @Override - public String toString() { - return authorizations.toString(); - } - - /** - * @return a pre-allocated empty Authorizations object - */ - public static Authorizations of() { - return EMPTY; - } - - /** - * Creates an Authorizations object from the set of input authorization strings. - * - * @param authorizations set of authorization strings - * @return Authorizations object - */ - public static Authorizations of(Set authorizations) { - if (authorizations.isEmpty()) { - return EMPTY; - } else { - return new Authorizations(authorizations); - } - } - - @Override - public Iterator iterator() { - return authorizations.iterator(); - } +public interface Authorizations extends Iterable, Serializable { + Set asSet(); } diff --git a/core/src/main/java/org/apache/accumulo/access/InvalidAccessExpressionException.java b/core/src/main/java/org/apache/accumulo/access/InvalidAccessExpressionException.java index 0119682..324ad95 100644 --- a/core/src/main/java/org/apache/accumulo/access/InvalidAccessExpressionException.java +++ b/core/src/main/java/org/apache/accumulo/access/InvalidAccessExpressionException.java @@ -21,11 +21,10 @@ import java.util.regex.PatternSyntaxException; /** - * An exception that is thrown when an access expression is not correct. + * An exception that is thrown when an access expression is not valid. * * @since 1.0.0 */ -// TODO rename to illegal... public final class InvalidAccessExpressionException extends IllegalArgumentException { private static final long serialVersionUID = 1L; diff --git a/core/src/main/java/org/apache/accumulo/access/InvalidAuthorizationException.java b/core/src/main/java/org/apache/accumulo/access/InvalidAuthorizationException.java new file mode 100644 index 0000000..bd1ecca --- /dev/null +++ b/core/src/main/java/org/apache/accumulo/access/InvalidAuthorizationException.java @@ -0,0 +1,28 @@ +/* + * 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 + * + * https://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.accumulo.access; + +/** + * @since 1.0.0 + */ +public class InvalidAuthorizationException extends RuntimeException { + public InvalidAuthorizationException(String auth) { + super("authorization : '" + auth + "'"); + } +} diff --git a/core/src/main/java/org/apache/accumulo/access/ParsedAccessExpression.java b/core/src/main/java/org/apache/accumulo/access/ParsedAccessExpression.java index 5928b1b..498df67 100644 --- a/core/src/main/java/org/apache/accumulo/access/ParsedAccessExpression.java +++ b/core/src/main/java/org/apache/accumulo/access/ParsedAccessExpression.java @@ -25,8 +25,8 @@ /** * Instances of this class are immutable and wrap a verified access expression and a parse tree for * the access expression. To create an instance of this class call - * {@link AccessExpression#parse(String)}. The Accumulo Access project has examples that show how to - * use the parse tree. + * {@link Access#newParsedExpression(String)}. The Accumulo Access project has examples that show + * how to use the parse tree. * * @since 1.0.0 */ @@ -52,7 +52,7 @@ public enum ExpressionType { /** * Indicates an access expression is a single authorization. For this type * {@link #getExpression()} will return the authorization in quoted and escaped form. Depending - * on the use case {@link #unquote(String)} may need to be called. + * on the use case {@link Access#unquote(String)} may need to be called. */ AUTHORIZATION, /** diff --git a/core/src/main/java/org/apache/accumulo/access/impl/AccessEvaluatorImpl.java b/core/src/main/java/org/apache/accumulo/access/impl/AccessEvaluatorImpl.java index d5189c2..6f34a69 100644 --- a/core/src/main/java/org/apache/accumulo/access/impl/AccessEvaluatorImpl.java +++ b/core/src/main/java/org/apache/accumulo/access/impl/AccessEvaluatorImpl.java @@ -18,11 +18,10 @@ */ package org.apache.accumulo.access.impl; -import static java.nio.charset.StandardCharsets.UTF_8; -import static org.apache.accumulo.access.impl.ByteUtils.BACKSLASH; -import static org.apache.accumulo.access.impl.ByteUtils.QUOTE; -import static org.apache.accumulo.access.impl.ByteUtils.isQuoteOrSlash; -import static org.apache.accumulo.access.impl.ByteUtils.isQuoteSymbol; +import static org.apache.accumulo.access.impl.CharUtils.BACKSLASH; +import static org.apache.accumulo.access.impl.CharUtils.QUOTE; +import static org.apache.accumulo.access.impl.CharUtils.isQuoteOrSlash; +import static org.apache.accumulo.access.impl.CharUtils.isQuoteSymbol; import java.util.HashSet; import java.util.Set; @@ -30,42 +29,54 @@ import org.apache.accumulo.access.AccessEvaluator; import org.apache.accumulo.access.AccessExpression; +import org.apache.accumulo.access.AuthorizationValidator; import org.apache.accumulo.access.Authorizations; import org.apache.accumulo.access.InvalidAccessExpressionException; public final class AccessEvaluatorImpl implements AccessEvaluator { - private final Predicate authorizedPredicate; + private final Predicate authorizedPredicate; + private final AuthorizationValidator authorizationValidator; /** * Create an AccessEvaluatorImpl using an Authorizer object */ - public AccessEvaluatorImpl(Authorizer authorizationChecker) { - this.authorizedPredicate = auth -> authorizationChecker.isAuthorized(unescape(auth)); + public AccessEvaluatorImpl(Authorizer authorizationChecker, + AuthorizationValidator authorizationValidator) { + this.authorizedPredicate = auth -> authorizationChecker.isAuthorized(auth.toString()); + this.authorizationValidator = authorizationValidator; } /** * Create an AccessEvaluatorImpl using a collection of authorizations */ - public AccessEvaluatorImpl(Authorizations authorizations) { + public AccessEvaluatorImpl(Authorizations authorizations, + AuthorizationValidator authorizationValidator) { var authsSet = authorizations.asSet(); - final Set authBytes = new HashSet<>(authsSet.size()); + final Set wrappedAuths = new HashSet<>(authsSet.size()); for (String authorization : authsSet) { - var auth = authorization.getBytes(UTF_8); - if (auth.length == 0) { + if (authorization.isEmpty()) { throw new IllegalArgumentException("Empty authorization"); } - authBytes.add(new BytesWrapper(AccessEvaluatorImpl.escape(auth, false))); + + wrappedAuths.add(new CharsWrapper(authorization.toCharArray())); } - authorizedPredicate = authBytes::contains; + this.authorizedPredicate = auth -> { + if (auth instanceof CharsWrapper) { + return wrappedAuths.contains(auth); + } else { + return wrappedAuths.contains(new CharsWrapper(auth.toString().toCharArray())); + } + }; + this.authorizationValidator = authorizationValidator; } - public static String unescape(BytesWrapper auth) { + public static CharSequence unescape(CharSequence auth) { int escapeCharCount = 0; for (int i = 0; i < auth.length(); i++) { - byte b = auth.byteAt(i); - if (isQuoteOrSlash(b)) { + char c = auth.charAt(i); + if (isQuoteOrSlash(c)) { escapeCharCount++; } } @@ -75,28 +86,28 @@ public static String unescape(BytesWrapper auth) { throw new IllegalArgumentException("Illegal escape sequence in auth : " + auth); } - byte[] unescapedCopy = new byte[auth.length() - escapeCharCount / 2]; + char[] unescapedCopy = new char[auth.length() - escapeCharCount / 2]; int pos = 0; for (int i = 0; i < auth.length(); i++) { - byte b = auth.byteAt(i); - if (b == BACKSLASH) { + char c = auth.charAt(i); + if (c == BACKSLASH) { i++; - b = auth.byteAt(i); - if (!isQuoteOrSlash(b)) { + c = auth.charAt(i); + if (!isQuoteOrSlash(c)) { throw new IllegalArgumentException("Illegal escape sequence in auth : " + auth); } - } else if (isQuoteSymbol(b)) { + } else if (isQuoteSymbol(c)) { // should only see quote after a slash throw new IllegalArgumentException( "Illegal character after slash in auth String : " + auth); } - unescapedCopy[pos++] = b; + unescapedCopy[pos++] = c; } - return new String(unescapedCopy, UTF_8); + return new String(unescapedCopy); } else { - return auth.toString(); + return auth; } } @@ -107,23 +118,24 @@ public static String unescape(BytesWrapper auth) { * @param shouldQuote true to wrap escaped authorization in quotes * @return escaped authorization string */ - public static byte[] escape(byte[] auth, boolean shouldQuote) { + public static CharSequence escape(CharSequence auth, boolean shouldQuote) { int escapeCount = 0; - for (byte value : auth) { - if (isQuoteOrSlash(value)) { + for (int i = 0; i < auth.length(); i++) { + if (isQuoteOrSlash(auth.charAt(i))) { escapeCount++; } } if (escapeCount > 0 || shouldQuote) { - byte[] escapedAuth = new byte[auth.length + escapeCount + (shouldQuote ? 2 : 0)]; + char[] escapedAuth = new char[auth.length() + escapeCount + (shouldQuote ? 2 : 0)]; int index = shouldQuote ? 1 : 0; - for (byte b : auth) { - if (isQuoteOrSlash(b)) { + for (int i = 0; i < auth.length(); i++) { + char c = auth.charAt(i); + if (isQuoteOrSlash(c)) { escapedAuth[index++] = BACKSLASH; } - escapedAuth[index++] = b; + escapedAuth[index++] = c; } if (shouldQuote) { @@ -131,7 +143,7 @@ public static byte[] escape(byte[] auth, boolean shouldQuote) { escapedAuth[escapedAuth.length - 1] = QUOTE; } - auth = escapedAuth; + auth = new String(escapedAuth); } return auth; } @@ -143,20 +155,21 @@ public boolean canAccess(AccessExpression expression) { @Override public boolean canAccess(String expression) throws InvalidAccessExpressionException { - return evaluate(expression.getBytes(UTF_8)); - } - - @Override - public boolean canAccess(byte[] expression) throws InvalidAccessExpressionException { return evaluate(expression); } - boolean evaluate(byte[] accessExpression) throws InvalidAccessExpressionException { - var bytesWrapper = ParserEvaluator.lookupWrappers.get(); - Predicate atp = authToken -> { - bytesWrapper.set(authToken.data, authToken.start, authToken.len); - return authorizedPredicate.test(bytesWrapper); + boolean evaluate(String accessExpression) throws InvalidAccessExpressionException { + var charsWrapper = ParserEvaluator.lookupWrappers.get(); + Predicate atp = authToken -> authorizedPredicate + .test(ParserEvaluator.validateAuth(authorizationValidator, authToken, charsWrapper)); + + // This is used once the expression is known to always be true or false. For this case only need + // to validate authorizations, do not need to look them up in a set. + Predicate shortCircuit = authToken -> { + ParserEvaluator.validateAuth(authorizationValidator, authToken, charsWrapper); + return true; }; - return ParserEvaluator.parseAccessExpression(accessExpression, atp, authToken -> true); + + return ParserEvaluator.parseAccessExpression(accessExpression, atp, shortCircuit); } } diff --git a/core/src/main/java/org/apache/accumulo/access/impl/AccessExpressionImpl.java b/core/src/main/java/org/apache/accumulo/access/impl/AccessExpressionImpl.java index 9f60e33..da611b6 100644 --- a/core/src/main/java/org/apache/accumulo/access/impl/AccessExpressionImpl.java +++ b/core/src/main/java/org/apache/accumulo/access/impl/AccessExpressionImpl.java @@ -18,8 +18,6 @@ */ package org.apache.accumulo.access.impl; -import static java.nio.charset.StandardCharsets.UTF_8; - import java.util.concurrent.atomic.AtomicReference; import org.apache.accumulo.access.AccessExpression; @@ -34,15 +32,9 @@ public final class AccessExpressionImpl extends AccessExpression { private final AtomicReference parseTreeRef = new AtomicReference<>(); public AccessExpressionImpl(String expression) { - validate(expression); this.expression = expression; } - public AccessExpressionImpl(byte[] expression) { - validate(expression); - this.expression = new String(expression, UTF_8); - } - @Override public String getExpression() { return expression; @@ -52,12 +44,48 @@ public String getExpression() { public ParsedAccessExpression parse() { ParsedAccessExpression parseTree = parseTreeRef.get(); if (parseTree == null) { + // This expression authorizations were already validated, so can pass a lambda that always + // returns true parseTreeRef.compareAndSet(null, - ParsedAccessExpressionImpl.parseExpression(expression.getBytes(UTF_8))); + ParsedAccessExpressionImpl.parseExpression(expression, (auth, quoting) -> true)); // must get() again in case another thread won w/ the compare and set, this ensures this // method always returns the exact same object parseTree = parseTreeRef.get(); } return parseTree; } + + public static CharSequence quote(CharSequence term) { + if (term.isEmpty()) { + throw new IllegalArgumentException("Empty strings are not legal authorizations."); + } + + boolean needsQuote = false; + + for (int i = 0; i < term.length(); i++) { + if (!Tokenizer.isValidAuthChar(term.charAt(i))) { + needsQuote = true; + break; + } + } + + if (!needsQuote) { + return term; + } + + return AccessEvaluatorImpl.escape(term, true); + } + + public static CharSequence unquote(CharSequence term) { + if (term.equals("\"\"") || term.isEmpty()) { + throw new IllegalArgumentException("Empty strings are not legal authorizations."); + } + + if (term.charAt(0) == '"' && term.charAt(term.length() - 1) == '"') { + term = term.subSequence(1, term.length() - 1); + return AccessEvaluatorImpl.unescape(term); + } else { + return term; + } + } } diff --git a/core/src/main/java/org/apache/accumulo/access/impl/AccessImpl.java b/core/src/main/java/org/apache/accumulo/access/impl/AccessImpl.java new file mode 100644 index 0000000..d8b1d57 --- /dev/null +++ b/core/src/main/java/org/apache/accumulo/access/impl/AccessImpl.java @@ -0,0 +1,117 @@ +/* + * 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 + * + * https://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.accumulo.access.impl; + +import static org.apache.accumulo.access.AuthorizationValidator.AuthorizationCharacters.ANY; + +import java.util.Collection; +import java.util.Objects; +import java.util.Set; +import java.util.function.Consumer; + +import org.apache.accumulo.access.Access; +import org.apache.accumulo.access.AccessEvaluator; +import org.apache.accumulo.access.AccessExpression; +import org.apache.accumulo.access.AuthorizationValidator; +import org.apache.accumulo.access.Authorizations; +import org.apache.accumulo.access.InvalidAccessExpressionException; +import org.apache.accumulo.access.InvalidAuthorizationException; +import org.apache.accumulo.access.ParsedAccessExpression; + +public class AccessImpl implements Access { + + private final AuthorizationValidator authValidator; + + private void validateAuthorization(CharSequence auth, + AuthorizationValidator.AuthorizationCharacters quoting) { + if (auth.isEmpty()) { + throw new IllegalArgumentException("Empty string is not a valid authorization"); + } + if (!authValidator.test(auth, quoting)) { + throw new InvalidAuthorizationException(auth.toString()); + } + } + + public AccessImpl(AuthorizationValidator authValidator) { + this.authValidator = Objects.requireNonNull(authValidator); + } + + @Override + public AccessExpression newExpression(String expression) { + if (expression.isEmpty()) { + return AccessExpressionImpl.EMPTY; + } + validateExpression(expression); + return new AccessExpressionImpl(expression); + } + + @Override + public ParsedAccessExpression newParsedExpression(String expression) { + return ParsedAccessExpressionImpl.parseExpression(expression, authValidator); + } + + @Override + public Authorizations newAuthorizations(Set authorizations) { + if (authorizations.isEmpty()) { + return AuthorizationsImpl.EMPTY; + } else { + authorizations.forEach(auth -> validateAuthorization(auth, ANY)); + return new AuthorizationsImpl(authorizations); + } + } + + @Override + public void findAuthorizations(String expression, Consumer authorizationConsumer) + throws InvalidAccessExpressionException { + ParserEvaluator.findAuthorizations(expression, authorizationConsumer, authValidator); + } + + @Override + public String quote(String authorization) { + validateAuthorization(authorization, ANY); + return AccessExpressionImpl.quote(authorization).toString(); + } + + @Override + public String unquote(String authorization) { + var unquoted = AccessExpressionImpl.unquote(authorization); + validateAuthorization(unquoted, ANY); + return unquoted.toString(); + } + + @Override + public void validateExpression(String expression) throws InvalidAccessExpressionException { + ParserEvaluator.validate(expression, authValidator); + } + + @Override + public AccessEvaluator newEvaluator(Authorizations authorizations) { + return new AccessEvaluatorImpl(authorizations, authValidator); + } + + @Override + public AccessEvaluator newEvaluator(AccessEvaluator.Authorizer authorizer) { + return new AccessEvaluatorImpl(authorizer, authValidator); + } + + @Override + public AccessEvaluator newEvaluator(Collection authorizationSets) { + return new MultiAccessEvaluatorImpl(authorizationSets, authValidator); + } +} diff --git a/core/src/main/java/org/apache/accumulo/access/impl/AuthorizationsImpl.java b/core/src/main/java/org/apache/accumulo/access/impl/AuthorizationsImpl.java new file mode 100644 index 0000000..6ad1d46 --- /dev/null +++ b/core/src/main/java/org/apache/accumulo/access/impl/AuthorizationsImpl.java @@ -0,0 +1,52 @@ +/* + * 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 + * + * https://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.accumulo.access.impl; + +import java.io.Serial; +import java.util.Iterator; +import java.util.Set; + +import org.apache.accumulo.access.Authorizations; + +public record AuthorizationsImpl(Set authorizations) implements Authorizations { + + @Serial + private static final long serialVersionUID = 1L; + + static final Authorizations EMPTY = new AuthorizationsImpl(Set.of()); + + public AuthorizationsImpl(Set authorizations) { + this.authorizations = Set.copyOf(authorizations); + } + + /** + * Returns the set of authorization strings in this Authorization object + * + * @return immutable set of authorization strings + */ + @Override + public Set asSet() { + return authorizations; + } + + @Override + public Iterator iterator() { + return authorizations.iterator(); + } +} diff --git a/core/src/main/java/org/apache/accumulo/access/impl/BuilderImpl.java b/core/src/main/java/org/apache/accumulo/access/impl/BuilderImpl.java new file mode 100644 index 0000000..7e8fc88 --- /dev/null +++ b/core/src/main/java/org/apache/accumulo/access/impl/BuilderImpl.java @@ -0,0 +1,40 @@ +/* + * 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 + * + * https://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.accumulo.access.impl; + +import java.util.Objects; + +import org.apache.accumulo.access.Access; +import org.apache.accumulo.access.AuthorizationValidator; + +public class BuilderImpl implements Access.Builder { + + private AuthorizationValidator validator; + + @Override + public Access.Builder authorizationValidator(AuthorizationValidator validator) { + this.validator = Objects.requireNonNull(validator); + return this; + } + + @Override + public Access build() { + return new AccessImpl(validator == null ? AuthorizationValidator.DEFAULT : validator); + } +} diff --git a/core/src/main/java/org/apache/accumulo/access/impl/ByteUtils.java b/core/src/main/java/org/apache/accumulo/access/impl/CharUtils.java similarity index 72% rename from core/src/main/java/org/apache/accumulo/access/impl/ByteUtils.java rename to core/src/main/java/org/apache/accumulo/access/impl/CharUtils.java index 8f3c209..696fbb1 100644 --- a/core/src/main/java/org/apache/accumulo/access/impl/ByteUtils.java +++ b/core/src/main/java/org/apache/accumulo/access/impl/CharUtils.java @@ -22,37 +22,37 @@ * This class exists to avoid repeat conversions from byte to char as well as to provide helper * methods for comparing them. */ -final class ByteUtils { - static final byte QUOTE = (byte) '"'; - static final byte BACKSLASH = (byte) '\\'; - static final byte AND_OPERATOR = (byte) '&'; - static final byte OR_OPERATOR = (byte) '|'; +final class CharUtils { + static final char QUOTE = '"'; + static final char BACKSLASH = '\\'; + static final char AND_OPERATOR = '&'; + static final char OR_OPERATOR = '|'; - private ByteUtils() { + private CharUtils() { // private constructor to prevent instantiation } - static boolean isQuoteSymbol(byte b) { + static boolean isQuoteSymbol(char b) { return b == QUOTE; } - static boolean isBackslashSymbol(byte b) { + static boolean isBackslashSymbol(char b) { return b == BACKSLASH; } - static boolean isQuoteOrSlash(byte b) { + static boolean isQuoteOrSlash(char b) { return isQuoteSymbol(b) || isBackslashSymbol(b); } - static boolean isAndOperator(byte b) { + static boolean isAndOperator(char b) { return b == AND_OPERATOR; } - static boolean isOrOperator(byte b) { + static boolean isOrOperator(char b) { return b == OR_OPERATOR; } - static boolean isAndOrOperator(byte b) { + static boolean isAndOrOperator(char b) { return isAndOperator(b) || isOrOperator(b); } } diff --git a/core/src/main/java/org/apache/accumulo/access/impl/BytesWrapper.java b/core/src/main/java/org/apache/accumulo/access/impl/CharsWrapper.java similarity index 51% rename from core/src/main/java/org/apache/accumulo/access/impl/BytesWrapper.java rename to core/src/main/java/org/apache/accumulo/access/impl/CharsWrapper.java index e034f70..4085aff 100644 --- a/core/src/main/java/org/apache/accumulo/access/impl/BytesWrapper.java +++ b/core/src/main/java/org/apache/accumulo/access/impl/CharsWrapper.java @@ -18,46 +18,59 @@ */ package org.apache.accumulo.access.impl; -import static java.nio.charset.StandardCharsets.UTF_8; -import static java.util.Objects.checkFromIndexSize; -import static java.util.Objects.checkIndex; - import java.util.Arrays; +import java.util.Objects; -public final class BytesWrapper implements Comparable { - - private byte[] data; +public final class CharsWrapper implements CharSequence { + private char[] wrapped; private int offset; - private int length; - - /** - * Creates a new sequence. The given byte array is used directly as the backing array, so later - * changes made to the array reflect into the new sequence. - * - * @param data byte data - */ - public BytesWrapper(byte[] data) { - set(data, 0, data.length); + private int len; + + CharsWrapper(char[] wrapped) { + this.wrapped = wrapped; + this.offset = 0; + this.len = this.wrapped.length; } - public byte byteAt(int i) { - return data[offset + checkIndex(i, length)]; + private CharsWrapper(char[] wrapped, int offset, int len) { + this.wrapped = wrapped; + this.offset = offset; + this.len = len; } + @Override public int length() { - return length; + return len; } @Override - public int compareTo(BytesWrapper obs) { - return Arrays.compare(data, offset, offset + length(), obs.data, obs.offset, - obs.offset + obs.length()); + public char charAt(int index) { + Objects.checkIndex(index, len); + return wrapped[offset + index]; + } + + @Override + public CharSequence subSequence(int start, int end) { + Objects.checkFromToIndex(start, end, len); + return new CharsWrapper(wrapped, start + offset, end - start); + } + + @Override + public int hashCode() { + int hash = 1; + + int end = offset + length(); + for (int i = offset; i < end; i++) { + hash = (31 * hash) + wrapped[i]; + } + + return hash; } @Override public boolean equals(Object o) { - if (o instanceof BytesWrapper) { - BytesWrapper obs = (BytesWrapper) o; + if (o instanceof CharsWrapper) { + CharsWrapper obs = (CharsWrapper) o; if (this == o) { return true; @@ -67,39 +80,21 @@ public boolean equals(Object o) { return false; } - return compareTo(obs) == 0; + return Arrays.equals(wrapped, offset, offset + len, obs.wrapped, obs.offset, + obs.offset + obs.len); } return false; - - } - - @Override - public int hashCode() { - int hash = 1; - - int end = offset + length(); - for (int i = offset; i < end; i++) { - hash = (31 * hash) + data[i]; - } - - return hash; } @Override public String toString() { - return new String(data, offset, length, UTF_8); + return new String(wrapped, offset, len); } - /* - * Wraps the given byte[] and captures the current offset and length. This method does *not* make - * a copy of the input buffer - */ - void set(byte[] data, int offset, int length) { - checkFromIndexSize(offset, length, data.length); - this.data = data; - this.offset = offset; - this.length = length; + public void set(char[] data, int start, int len) { + this.wrapped = data; + this.offset = start; + this.len = len; } - } diff --git a/core/src/main/java/org/apache/accumulo/access/impl/MultiAccessEvaluatorImpl.java b/core/src/main/java/org/apache/accumulo/access/impl/MultiAccessEvaluatorImpl.java index 8c268f8..e2a905c 100644 --- a/core/src/main/java/org/apache/accumulo/access/impl/MultiAccessEvaluatorImpl.java +++ b/core/src/main/java/org/apache/accumulo/access/impl/MultiAccessEvaluatorImpl.java @@ -18,39 +18,30 @@ */ package org.apache.accumulo.access.impl; -import static java.nio.charset.StandardCharsets.UTF_8; - import java.util.ArrayList; import java.util.Collection; import java.util.List; import org.apache.accumulo.access.AccessEvaluator; import org.apache.accumulo.access.AccessExpression; +import org.apache.accumulo.access.AuthorizationValidator; import org.apache.accumulo.access.Authorizations; import org.apache.accumulo.access.InvalidAccessExpressionException; public final class MultiAccessEvaluatorImpl implements AccessEvaluator { - public static AccessEvaluator of(Collection authorizationSets) { - return new MultiAccessEvaluatorImpl(authorizationSets); - } - private final List evaluators; - private MultiAccessEvaluatorImpl(Collection authorizationSets) { + MultiAccessEvaluatorImpl(Collection authorizationSets, + AuthorizationValidator validator) { evaluators = new ArrayList<>(authorizationSets.size()); for (Authorizations authorizations : authorizationSets) { - evaluators.add(new AccessEvaluatorImpl(authorizations)); + evaluators.add(new AccessEvaluatorImpl(authorizations, validator)); } } @Override public boolean canAccess(String accessExpression) throws InvalidAccessExpressionException { - return canAccess(accessExpression.getBytes(UTF_8)); - } - - @Override - public boolean canAccess(byte[] accessExpression) throws InvalidAccessExpressionException { for (AccessEvaluator evaluator : evaluators) { if (!evaluator.canAccess(accessExpression)) { return false; diff --git a/core/src/main/java/org/apache/accumulo/access/impl/ParsedAccessExpressionImpl.java b/core/src/main/java/org/apache/accumulo/access/impl/ParsedAccessExpressionImpl.java index 99f6464..4f97f32 100644 --- a/core/src/main/java/org/apache/accumulo/access/impl/ParsedAccessExpressionImpl.java +++ b/core/src/main/java/org/apache/accumulo/access/impl/ParsedAccessExpressionImpl.java @@ -18,36 +18,36 @@ */ package org.apache.accumulo.access.impl; -import static java.nio.charset.StandardCharsets.UTF_8; import static org.apache.accumulo.access.ParsedAccessExpression.ExpressionType.AND; import static org.apache.accumulo.access.ParsedAccessExpression.ExpressionType.AUTHORIZATION; import static org.apache.accumulo.access.ParsedAccessExpression.ExpressionType.OR; -import static org.apache.accumulo.access.impl.ByteUtils.AND_OPERATOR; -import static org.apache.accumulo.access.impl.ByteUtils.OR_OPERATOR; -import static org.apache.accumulo.access.impl.ByteUtils.isAndOrOperator; +import static org.apache.accumulo.access.impl.CharUtils.AND_OPERATOR; +import static org.apache.accumulo.access.impl.CharUtils.OR_OPERATOR; +import static org.apache.accumulo.access.impl.CharUtils.isAndOrOperator; import java.util.ArrayList; import java.util.List; import java.util.concurrent.atomic.AtomicReference; +import org.apache.accumulo.access.AuthorizationValidator; +import org.apache.accumulo.access.InvalidAuthorizationException; import org.apache.accumulo.access.ParsedAccessExpression; public final class ParsedAccessExpressionImpl extends ParsedAccessExpression { private static final long serialVersionUID = 1L; - private final byte[] expression; + private final String wholeExpression; + private final AtomicReference expression = new AtomicReference<>(null); private final int offset; private final int length; private final ExpressionType type; private final List children; - private final AtomicReference stringExpression = new AtomicReference<>(null); - public static final ParsedAccessExpression EMPTY = new ParsedAccessExpressionImpl(); - private ParsedAccessExpressionImpl(byte operator, byte[] expression, int offset, int length, + private ParsedAccessExpressionImpl(char operator, String wholeExpression, int offset, int length, List children) { if (children.isEmpty()) { throw new IllegalArgumentException("Must have children with an operator"); @@ -61,15 +61,15 @@ private ParsedAccessExpressionImpl(byte operator, byte[] expression, int offset, this.type = OR; } - this.expression = expression; + this.wholeExpression = wholeExpression; this.offset = offset; this.length = length; this.children = List.copyOf(children); } - private ParsedAccessExpressionImpl(byte[] expression, int offset, int length) { + private ParsedAccessExpressionImpl(String wholeExpression, int offset, int length) { this.type = AUTHORIZATION; - this.expression = expression; + this.wholeExpression = wholeExpression; this.offset = offset; this.length = length; this.children = List.of(); @@ -77,21 +77,21 @@ private ParsedAccessExpressionImpl(byte[] expression, int offset, int length) { ParsedAccessExpressionImpl() { this.type = ExpressionType.EMPTY; + this.wholeExpression = ""; this.offset = 0; this.length = 0; - this.expression = new byte[0]; this.children = List.of(); } @Override public String getExpression() { - String strExp = stringExpression.get(); + String strExp = expression.get(); if (strExp != null) { return strExp; } - strExp = new String(expression, offset, length, UTF_8); - stringExpression.compareAndSet(null, strExp); - return stringExpression.get(); + strExp = wholeExpression.substring(offset, length + offset); + expression.compareAndSet(null, strExp); + return expression.get(); } @Override @@ -109,27 +109,30 @@ public List getChildren() { return children; } - public static ParsedAccessExpression parseExpression(byte[] expression) { - if (expression.length == 0) { + public static ParsedAccessExpression parseExpression(String expression, + AuthorizationValidator authorizationValidator) { + if (expression.isEmpty()) { return ParsedAccessExpressionImpl.EMPTY; } - Tokenizer tokenizer = new Tokenizer(expression); - var parsed = ParsedAccessExpressionImpl.parseExpression(tokenizer, false); + Tokenizer tokenizer = ParserEvaluator.getPerThreadTokenizer(expression); + var parsed = + ParsedAccessExpressionImpl.parseExpression(tokenizer, expression, authorizationValidator); if (tokenizer.hasNext()) { // not all input was read, so not a valid expression - tokenizer.error("Unexpected character '" + (char) tokenizer.peek() + "'"); + tokenizer.error("Unexpected character '" + tokenizer.peek() + "'"); } return parsed; } private static ParsedAccessExpressionImpl parseExpression(Tokenizer tokenizer, - boolean wrappedWithParens) { + String wholeExpression, AuthorizationValidator authorizationValidator) { int beginOffset = tokenizer.curentOffset(); - ParsedAccessExpressionImpl node = parseParenExpressionOrAuthorization(tokenizer); + ParsedAccessExpressionImpl node = + parseParenExpressionOrAuthorization(tokenizer, wholeExpression, authorizationValidator); if (tokenizer.hasNext()) { var operator = tokenizer.peek(); @@ -138,7 +141,8 @@ private static ParsedAccessExpressionImpl parseExpression(Tokenizer tokenizer, nodes.add(node); do { tokenizer.advance(); - ParsedAccessExpression next = parseParenExpressionOrAuthorization(tokenizer); + ParsedAccessExpression next = parseParenExpressionOrAuthorization(tokenizer, + wholeExpression, authorizationValidator); nodes.add(next); } while (tokenizer.hasNext() && tokenizer.peek() == operator); @@ -149,7 +153,7 @@ private static ParsedAccessExpressionImpl parseExpression(Tokenizer tokenizer, int endOffset = tokenizer.curentOffset(); - node = new ParsedAccessExpressionImpl(operator, tokenizer.expression(), beginOffset, + node = new ParsedAccessExpressionImpl(operator, wholeExpression, beginOffset, endOffset - beginOffset, nodes); } } @@ -157,8 +161,8 @@ private static ParsedAccessExpressionImpl parseExpression(Tokenizer tokenizer, return node; } - private static ParsedAccessExpressionImpl - parseParenExpressionOrAuthorization(Tokenizer tokenizer) { + private static ParsedAccessExpressionImpl parseParenExpressionOrAuthorization(Tokenizer tokenizer, + String wholeExpression, AuthorizationValidator authorizationValidator) { if (!tokenizer.hasNext()) { tokenizer .error("Expected a '(' character or an authorization token instead saw end of input"); @@ -166,12 +170,31 @@ private static ParsedAccessExpressionImpl parseExpression(Tokenizer tokenizer, if (tokenizer.peek() == ParserEvaluator.OPEN_PAREN) { tokenizer.advance(); - var node = parseExpression(tokenizer, true); + var node = parseExpression(tokenizer, wholeExpression, authorizationValidator); tokenizer.next(ParserEvaluator.CLOSE_PAREN); return node; } else { var auth = tokenizer.nextAuthorization(true); - return new ParsedAccessExpressionImpl(auth.data, auth.start, auth.len); + CharSequence unquotedAuth; + AuthorizationValidator.AuthorizationCharacters quoting; + var wrapper = ParserEvaluator.lookupWrappers.get(); + if (CharUtils.isQuoteSymbol(auth.data[auth.start])) { + wrapper.set(auth.data, auth.start + 1, auth.len - 2); + if (auth.hasEscapes) { + unquotedAuth = AccessEvaluatorImpl.unescape(wrapper); + } else { + unquotedAuth = wrapper; + } + quoting = AuthorizationValidator.AuthorizationCharacters.ANY; + } else { + wrapper.set(auth.data, auth.start, auth.len); + unquotedAuth = wrapper; + quoting = AuthorizationValidator.AuthorizationCharacters.BASIC; + } + if (!authorizationValidator.test(unquotedAuth, quoting)) { + throw new InvalidAuthorizationException(unquotedAuth.toString()); + } + return new ParsedAccessExpressionImpl(wholeExpression, auth.start, auth.len); } } } diff --git a/core/src/main/java/org/apache/accumulo/access/impl/ParserEvaluator.java b/core/src/main/java/org/apache/accumulo/access/impl/ParserEvaluator.java index 2df624c..557d89a 100644 --- a/core/src/main/java/org/apache/accumulo/access/impl/ParserEvaluator.java +++ b/core/src/main/java/org/apache/accumulo/access/impl/ParserEvaluator.java @@ -18,44 +18,90 @@ */ package org.apache.accumulo.access.impl; -import static org.apache.accumulo.access.impl.ByteUtils.isAndOrOperator; +import static org.apache.accumulo.access.impl.CharUtils.isAndOrOperator; import java.util.function.Consumer; import java.util.function.Predicate; +import org.apache.accumulo.access.AuthorizationValidator; import org.apache.accumulo.access.InvalidAccessExpressionException; +import org.apache.accumulo.access.InvalidAuthorizationException; /** * Code for parsing and evaluating an access expression at the same time. */ public final class ParserEvaluator { - static final byte OPEN_PAREN = (byte) '('; - static final byte CLOSE_PAREN = (byte) ')'; + static final char OPEN_PAREN = '('; + static final char CLOSE_PAREN = ')'; - private static final byte[] EMPTY = new byte[0]; + static final ThreadLocal lookupWrappers = + ThreadLocal.withInitial(() -> new CharsWrapper(new char[0])); + static final ThreadLocal tokenizers = + ThreadLocal.withInitial(() -> new Tokenizer(new char[0])); + static final ThreadLocal expressionArrays = ThreadLocal.withInitial(() -> new char[128]); - static final ThreadLocal lookupWrappers = - ThreadLocal.withInitial(() -> new BytesWrapper(EMPTY)); - private static final ThreadLocal tokenizers = - ThreadLocal.withInitial(() -> new Tokenizer(EMPTY)); + static Tokenizer getPerThreadTokenizer(String expression) { + var tokenizer = tokenizers.get(); + var array = expressionArrays.get(); + if (array.length < expression.length()) { + int newLen = array.length; + while (newLen < expression.length()) { + newLen = Math.multiplyExact(newLen, 2); + } + array = new char[newLen]; + expressionArrays.set(array); + } + expression.getChars(0, expression.length(), array, 0); + tokenizer.reset(array, expression.length()); + + return tokenizer; + } + + static CharSequence validateAuth(AuthorizationValidator authValidator, + Tokenizer.AuthorizationToken authToken, CharsWrapper charsWrapper) { + charsWrapper.set(authToken.data, authToken.start, authToken.len); + CharSequence authorizations; + if (authToken.hasEscapes) { + authorizations = AccessEvaluatorImpl.unescape(charsWrapper); + } else { + authorizations = charsWrapper; + } + if (!authValidator.test(authorizations, authToken.quoting)) { + throw new InvalidAuthorizationException(authorizations.toString()); + } + return authorizations; + } - public static void findAuthorizations(byte[] expression, Consumer authorizationConsumer) + public static void validate(String expression, AuthorizationValidator authValidator) throws InvalidAccessExpressionException { - var bytesWrapper = ParserEvaluator.lookupWrappers.get(); + if (expression.isEmpty()) { + return; + } + + var charsWrapper = ParserEvaluator.lookupWrappers.get(); + Predicate vp = authToken -> { + validateAuth(authValidator, authToken, charsWrapper); + return true; + }; + + ParserEvaluator.parseAccessExpression(expression, vp, vp); + } + + public static void findAuthorizations(String expression, Consumer authorizationConsumer, + AuthorizationValidator authValidator) throws InvalidAccessExpressionException { + var charsWrapper = ParserEvaluator.lookupWrappers.get(); Predicate atp = authToken -> { - bytesWrapper.set(authToken.data, authToken.start, authToken.len); - authorizationConsumer.accept(AccessEvaluatorImpl.unescape(bytesWrapper)); + authorizationConsumer.accept(validateAuth(authValidator, authToken, charsWrapper).toString()); return true; }; ParserEvaluator.parseAccessExpression(expression, atp, atp); } - public static boolean parseAccessExpression(byte[] expression, + public static boolean parseAccessExpression(String expression, Predicate authorizedPredicate, Predicate shortCircuitPredicate) { - var tokenizer = tokenizers.get(); - tokenizer.reset(expression); + var tokenizer = getPerThreadTokenizer(expression); return parseAccessExpression(tokenizer, authorizedPredicate, shortCircuitPredicate); } @@ -71,7 +117,7 @@ private static boolean parseAccessExpression(Tokenizer tokenizer, if (tokenizer.hasNext()) { // not all input was read, so not a valid expression - tokenizer.error("Unexpected character '" + (char) tokenizer.peek() + "'"); + tokenizer.error("Unexpected character '" + tokenizer.peek() + "'"); } return node; @@ -86,13 +132,13 @@ private static boolean parseExpression(Tokenizer tokenizer, if (tokenizer.hasNext()) { var operator = tokenizer.peek(); - if (operator == ByteUtils.AND_OPERATOR) { + if (operator == CharUtils.AND_OPERATOR) { result = parseAndExpression(result, tokenizer, authorizedPredicate, shortCircuitPredicate); if (tokenizer.hasNext() && isAndOrOperator(tokenizer.peek())) { // A case of mixed operators, lets give a clear error message tokenizer.error("Cannot mix '|' and '&'"); } - } else if (operator == ByteUtils.OR_OPERATOR) { + } else if (operator == CharUtils.OR_OPERATOR) { result = parseOrExpression(result, tokenizer, authorizedPredicate, shortCircuitPredicate); if (tokenizer.hasNext() && isAndOrOperator(tokenizer.peek())) { // A case of mixed operators, lets give a clear error message @@ -117,7 +163,7 @@ private static boolean parseAndExpression(boolean result, Tokenizer tokenizer, var nextResult = parseParenExpressionOrAuthorization(tokenizer, authorizedPredicate, shortCircuitPredicate); result &= nextResult; - } while (tokenizer.hasNext() && tokenizer.peek() == ByteUtils.AND_OPERATOR); + } while (tokenizer.hasNext() && tokenizer.peek() == CharUtils.AND_OPERATOR); return result; } @@ -134,7 +180,7 @@ private static boolean parseOrExpression(boolean result, Tokenizer tokenizer, var nextResult = parseParenExpressionOrAuthorization(tokenizer, authorizedPredicate, shortCircuitPredicate); result |= nextResult; - } while (tokenizer.hasNext() && tokenizer.peek() == ByteUtils.OR_OPERATOR); + } while (tokenizer.hasNext() && tokenizer.peek() == CharUtils.OR_OPERATOR); return result; } diff --git a/core/src/main/java/org/apache/accumulo/access/impl/Tokenizer.java b/core/src/main/java/org/apache/accumulo/access/impl/Tokenizer.java index da5b6ab..6b46f35 100644 --- a/core/src/main/java/org/apache/accumulo/access/impl/Tokenizer.java +++ b/core/src/main/java/org/apache/accumulo/access/impl/Tokenizer.java @@ -18,13 +18,13 @@ */ package org.apache.accumulo.access.impl; -import static java.nio.charset.StandardCharsets.UTF_8; -import static org.apache.accumulo.access.impl.ByteUtils.isBackslashSymbol; -import static org.apache.accumulo.access.impl.ByteUtils.isQuoteOrSlash; -import static org.apache.accumulo.access.impl.ByteUtils.isQuoteSymbol; +import static org.apache.accumulo.access.impl.CharUtils.isBackslashSymbol; +import static org.apache.accumulo.access.impl.CharUtils.isQuoteOrSlash; +import static org.apache.accumulo.access.impl.CharUtils.isQuoteSymbol; import java.util.stream.IntStream; +import org.apache.accumulo.access.AuthorizationValidator; import org.apache.accumulo.access.InvalidAccessExpressionException; /** @@ -47,47 +47,51 @@ public final class Tokenizer { "_-:./".chars().forEach(c -> validAuthChars[c] = true); } - public static boolean isValidAuthChar(byte b) { - return validAuthChars[0xff & b]; + public static boolean isValidAuthChar(char b) { + return validAuthChars[0xff & b] && b < 256; } - private byte[] expression; + private char[] expression; + private int len; private int index; private final AuthorizationToken authorizationToken = new AuthorizationToken(); public static class AuthorizationToken { - public byte[] data; + public char[] data; public int start; public int len; + public boolean hasEscapes; + public AuthorizationValidator.AuthorizationCharacters quoting; + } - Tokenizer(byte[] expression) { - this.expression = expression; - authorizationToken.data = expression; + Tokenizer(char[] expression) { + reset(expression, expression.length); } - void reset(byte[] expression) { + void reset(char[] expression, int len) { this.expression = expression; - authorizationToken.data = expression; this.index = 0; + this.len = len; + this.authorizationToken.data = expression; } boolean hasNext() { - return index < expression.length; + return index < len; } public void advance() { index++; } - public void next(byte expected) { + public void next(char expected) { if (!hasNext()) { - error("Expected '" + (char) expected + "' instead saw end of input"); + error("Expected '" + expected + "' instead saw end of input"); } if (expression[index] != expected) { - error("Expected '" + (char) expected + "' instead saw '" + (char) (expression[index]) + "'"); + error("Expected '" + expected + "' instead saw '" + (expression[index]) + "'"); } index++; } @@ -97,17 +101,13 @@ public void error(String msg) { } public void error(String msg, int idx) { - throw new InvalidAccessExpressionException(msg, new String(expression, UTF_8), idx); + throw new InvalidAccessExpressionException(msg, new String(expression, 0, len), idx); } - byte peek() { + char peek() { return expression[index]; } - byte[] expression() { - return expression; - } - public int curentOffset() { return index; } @@ -116,17 +116,19 @@ AuthorizationToken nextAuthorization(boolean includeQuotes) { if (isQuoteSymbol(expression[index])) { int start = ++index; - while (index < expression.length && !isQuoteSymbol(expression[index])) { + boolean hasEscapes = false; + while (index < len && !isQuoteSymbol(expression[index])) { if (isBackslashSymbol(expression[index])) { index++; - if (index == expression.length || !isQuoteOrSlash(expression[index])) { + if (index == len || !isQuoteOrSlash(expression[index])) { error("Invalid escaping within quotes", index - 1); } + hasEscapes = true; } index++; } - if (index == expression.length) { + if (index == len) { error("Unclosed quote", start - 1); } @@ -136,6 +138,8 @@ AuthorizationToken nextAuthorization(boolean includeQuotes) { authorizationToken.start = start; authorizationToken.len = index - start; + authorizationToken.hasEscapes = hasEscapes; + authorizationToken.quoting = AuthorizationValidator.AuthorizationCharacters.ANY; if (includeQuotes) { authorizationToken.start--; @@ -148,15 +152,16 @@ AuthorizationToken nextAuthorization(boolean includeQuotes) { } else if (isValidAuthChar(expression[index])) { int start = index; - while (index < expression.length && isValidAuthChar(expression[index])) { + do { index++; - } + } while (index < len && isValidAuthChar(expression[index])); authorizationToken.start = start; authorizationToken.len = index - start; + authorizationToken.hasEscapes = false; + authorizationToken.quoting = AuthorizationValidator.AuthorizationCharacters.BASIC; return authorizationToken; } else { - error( - "Expected a '(' character or an authorization token instead saw '" + (char) peek() + "'"); + error("Expected a '(' character or an authorization token instead saw '" + peek() + "'"); return null; } } diff --git a/core/src/main/resources/org/apache/accumulo/access/specification/AccessExpression.abnf b/core/src/main/resources/org/apache/accumulo/access/specification/AccessExpression.abnf index 30f45b3..f2aae57 100644 --- a/core/src/main/resources/org/apache/accumulo/access/specification/AccessExpression.abnf +++ b/core/src/main/resources/org/apache/accumulo/access/specification/AccessExpression.abnf @@ -28,9 +28,8 @@ and-expression = "&" (access-token / paren-expression) [and-expression or-expression = "|" (access-token / paren-expression) [or-expression] access-token = 1*( ALPHA / DIGIT / "_" / "-" / "." / ":" / slash ) -access-token =/ DQUOTE 1*(utf8-subset / escaped) DQUOTE +access-token =/ DQUOTE 1*(unicode-subset / escaped) DQUOTE -utf8-subset = %x20-21 / %x23-5B / %x5D-7E / unicode-beyond-ascii ; utf8 minus '"' and '\' -unicode-beyond-ascii = %x0080-D7FF / %xE000-10FFFF +unicode-subset = %x00-21 / %x23-5B / %x5D-7F / unicode-beyond-ascii ; unicode minus '"' and '\' escaped = "\" DQUOTE / "\\" slash = "/" diff --git a/core/src/test/java/org/apache/accumulo/access/tests/AccessEvaluatorTest.java b/core/src/test/java/org/apache/accumulo/access/tests/AccessEvaluatorTest.java index 35a0d5c..b72b2a0 100644 --- a/core/src/test/java/org/apache/accumulo/access/tests/AccessEvaluatorTest.java +++ b/core/src/test/java/org/apache/accumulo/access/tests/AccessEvaluatorTest.java @@ -19,8 +19,6 @@ package org.apache.accumulo.access.tests; import static java.nio.charset.StandardCharsets.UTF_8; -import static org.apache.accumulo.access.AccessExpression.quote; -import static org.apache.accumulo.access.AccessExpression.unquote; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -29,17 +27,17 @@ import java.io.IOException; import java.lang.reflect.Type; import java.util.ArrayList; +import java.util.HashSet; import java.util.List; import java.util.Set; import java.util.stream.Collectors; import java.util.stream.Stream; +import org.apache.accumulo.access.Access; import org.apache.accumulo.access.AccessEvaluator; -import org.apache.accumulo.access.AccessExpression; -import org.apache.accumulo.access.Authorizations; +import org.apache.accumulo.access.AuthorizationValidator; import org.apache.accumulo.access.InvalidAccessExpressionException; import org.apache.accumulo.access.impl.AccessEvaluatorImpl; -import org.apache.accumulo.access.impl.BytesWrapper; import org.junit.jupiter.api.Test; import com.google.gson.Gson; @@ -85,28 +83,30 @@ public void runTestCases() throws IOException { assertFalse(testData.isEmpty()); + var access = Access.builder().build(); + for (var testSet : testData) { System.out.println("runTestCases for " + testSet.description); AccessEvaluator evaluator; assertTrue(testSet.auths.length >= 1); if (testSet.auths.length == 1) { - evaluator = AccessEvaluator.of(Authorizations.of(Set.of(testSet.auths[0]))); - runTestCases(testSet, evaluator); + evaluator = access.newEvaluator(access.newAuthorizations(Set.of(testSet.auths[0]))); + runTestCases(access, testSet, evaluator); Set auths = Stream.of(testSet.auths[0]).collect(Collectors.toSet()); - evaluator = AccessEvaluator.of(auths::contains); - runTestCases(testSet, evaluator); + evaluator = access.newEvaluator(auths::contains); + runTestCases(access, testSet, evaluator); } else { - var authSets = Stream.of(testSet.auths).map(a -> Authorizations.of(Set.of(a))) + var authSets = Stream.of(testSet.auths).map(a -> access.newAuthorizations(Set.of(a))) .collect(Collectors.toList()); - evaluator = AccessEvaluator.of(authSets); - runTestCases(testSet, evaluator); + evaluator = access.newEvaluator(authSets); + runTestCases(access, testSet, evaluator); } } - } - private static void runTestCases(TestDataSet testSet, AccessEvaluator evaluator) { + private static void runTestCases(Access accumuloAccess, TestDataSet testSet, + AccessEvaluator evaluator) { assertFalse(testSet.tests.isEmpty()); @@ -120,52 +120,42 @@ private static void runTestCases(TestDataSet testSet, AccessEvaluator evaluator) // exception if (tests.expectedResult == ExpectedResult.ACCESSIBLE || tests.expectedResult == ExpectedResult.INACCESSIBLE) { - AccessExpression.validate(expression); - AccessExpression.validate(expression.getBytes(UTF_8)); - assertEquals(expression, AccessExpression.of(expression).getExpression()); - assertEquals(expression, AccessExpression.of(expression.getBytes(UTF_8)).getExpression()); - // parsing an expression will strip uneeded outer parens - assertTrue(expression.contains(AccessExpression.parse(expression).getExpression())); - assertTrue(expression - .contains(AccessExpression.parse(expression.getBytes(UTF_8)).getExpression())); - AccessExpression.findAuthorizations(expression, auth -> {}); - AccessExpression.findAuthorizations(expression.getBytes(UTF_8), auth -> {}); + accumuloAccess.validateExpression(expression); + assertEquals(expression, accumuloAccess.newExpression(expression).getExpression()); + // parsing an expression will strip unneeded outer parens + assertTrue( + expression.contains(accumuloAccess.newParsedExpression(expression).getExpression())); + accumuloAccess.findAuthorizations(expression, auth -> {}); } switch (tests.expectedResult) { case ACCESSIBLE: assertTrue(evaluator.canAccess(expression), expression); - assertTrue(evaluator.canAccess(expression.getBytes(UTF_8)), expression); - assertTrue(evaluator.canAccess(AccessExpression.of(expression)), expression); - assertTrue(evaluator.canAccess(AccessExpression.parse(expression)), expression); - assertTrue(evaluator.canAccess(AccessExpression.parse(expression).getExpression()), + assertTrue(evaluator.canAccess(accumuloAccess.newExpression(expression)), expression); + assertTrue(evaluator.canAccess(accumuloAccess.newParsedExpression(expression)), + expression); + assertTrue( + evaluator.canAccess(accumuloAccess.newParsedExpression(expression).getExpression()), expression); break; case INACCESSIBLE: assertFalse(evaluator.canAccess(expression), expression); - assertFalse(evaluator.canAccess(expression.getBytes(UTF_8)), expression); - assertFalse(evaluator.canAccess(AccessExpression.of(expression)), expression); - assertFalse(evaluator.canAccess(AccessExpression.parse(expression)), expression); - assertFalse(evaluator.canAccess(AccessExpression.parse(expression).getExpression()), + assertFalse(evaluator.canAccess(accumuloAccess.newExpression(expression)), expression); + assertFalse(evaluator.canAccess(accumuloAccess.newParsedExpression(expression)), + expression); + assertFalse( + evaluator.canAccess(accumuloAccess.newParsedExpression(expression).getExpression()), expression); break; case ERROR: assertThrows(InvalidAccessExpressionException.class, () -> evaluator.canAccess(expression), expression); assertThrows(InvalidAccessExpressionException.class, - () -> evaluator.canAccess(expression.getBytes(UTF_8)), expression); - assertThrows(InvalidAccessExpressionException.class, - () -> AccessExpression.validate(expression), expression); - assertThrows(InvalidAccessExpressionException.class, - () -> AccessExpression.validate(expression.getBytes(UTF_8)), expression); - assertThrows(InvalidAccessExpressionException.class, - () -> AccessExpression.of(expression), expression); - assertThrows(InvalidAccessExpressionException.class, - () -> AccessExpression.of(expression.getBytes(UTF_8)), expression); + () -> accumuloAccess.validateExpression(expression), expression); assertThrows(InvalidAccessExpressionException.class, - () -> AccessExpression.parse(expression), expression); + () -> accumuloAccess.newExpression(expression), expression); assertThrows(InvalidAccessExpressionException.class, - () -> AccessExpression.parse(expression.getBytes(UTF_8)), expression); + () -> accumuloAccess.newParsedExpression(expression), expression); break; default: throw new IllegalArgumentException(); @@ -176,48 +166,49 @@ private static void runTestCases(TestDataSet testSet, AccessEvaluator evaluator) @Test public void testEmptyAuthorizations() { - assertThrows(IllegalArgumentException.class, - () -> AccessEvaluator.of(Authorizations.of(Set.of("")))); - assertThrows(IllegalArgumentException.class, - () -> AccessEvaluator.of(Authorizations.of(Set.of("", "A")))); - assertThrows(IllegalArgumentException.class, - () -> AccessEvaluator.of(Authorizations.of(Set.of("A", "")))); - assertThrows(IllegalArgumentException.class, - () -> AccessEvaluator.of(Authorizations.of(Set.of("")))); + var access = Access.builder().build(); + assertThrows(IllegalArgumentException.class, () -> access.newAuthorizations(Set.of(""))); + assertThrows(IllegalArgumentException.class, () -> access.newAuthorizations(Set.of("", "A"))); + assertThrows(IllegalArgumentException.class, () -> access.newAuthorizations(Set.of("A", ""))); + assertThrows(IllegalArgumentException.class, () -> access.newAuthorizations(Set.of(""))); } @Test public void testSpecialChars() { + var access = Access.builder().build(); // special chars do not need quoting for (String qt : List.of("A_", "_", "A_C", "_C")) { - assertEquals(qt, quote(qt)); + assertEquals(qt, access.quote(qt)); for (char c : new char[] {'/', ':', '-', '.'}) { String qt2 = qt.replace('_', c); - assertEquals(qt2, quote(qt2)); + assertEquals(qt2, access.quote(qt2)); } } - assertEquals("a_b:c/d.e", quote("a_b:c/d.e")); + assertEquals("a_b:c/d.e", access.quote("a_b:c/d.e")); } @Test public void testQuote() { - assertEquals("\"A#C\"", quote("A#C")); - assertEquals("A#C", unquote(quote("A#C"))); - assertEquals("\"A\\\"C\"", quote("A\"C")); - assertEquals("A\"C", unquote(quote("A\"C"))); - assertEquals("\"A\\\"\\\\C\"", quote("A\"\\C")); - assertEquals("A\"\\C", unquote(quote("A\"\\C"))); - assertEquals("ACS", quote("ACS")); - assertEquals("ACS", unquote(quote("ACS"))); - assertEquals("\"九\"", quote("九")); - assertEquals("九", unquote(quote("九"))); - assertEquals("\"五十\"", quote("五十")); - assertEquals("五十", unquote(quote("五十"))); + var access = Access.builder().build(); + assertEquals("\"A#C\"", access.quote("A#C")); + assertEquals("A#C", access.unquote(access.quote("A#C"))); + assertEquals("\"A\\\"C\"", access.quote("A\"C")); + assertEquals("A\"C", access.unquote(access.quote("A\"C"))); + assertEquals("\"A\\\"\\\\C\"", access.quote("A\"\\C")); + assertEquals("A\"\\C", access.unquote(access.quote("A\"\\C"))); + assertEquals("ACS", access.quote("ACS")); + assertEquals("ACS", access.unquote(access.quote("ACS"))); + assertEquals("\"九\"", access.quote("九")); + assertEquals("九", access.unquote(access.quote("九"))); + assertEquals("\"五十\"", access.quote("五十")); + assertEquals("五十", access.unquote(access.quote("五十"))); + assertThrows(IllegalArgumentException.class, () -> access.quote("")); + assertThrows(IllegalArgumentException.class, () -> access.unquote("")); } private static String unescape(String s) { - return AccessEvaluatorImpl.unescape(new BytesWrapper(s.getBytes(UTF_8))); + return AccessEvaluatorImpl.unescape(s).toString(); } @Test @@ -235,5 +226,46 @@ public void testUnescape() { .forEach(seq -> assertThrows(IllegalArgumentException.class, () -> unescape(seq), message)); } + @Test + public void testNullAuthValidator() { + assertThrows(NullPointerException.class, + () -> Access.builder().authorizationValidator(null).build()); + } + + @Test + public void testAuthValidation() { + // This test ensures that unquoted and unescaped auths are passed to the auth validator. + HashSet seenAuths = new HashSet<>(); + AuthorizationValidator authorizationValidator = (auth, authChars) -> { + seenAuths.add(auth.toString()); + return AuthorizationValidator.DEFAULT.test(auth, authChars); + }; + var access = Access.builder().authorizationValidator(authorizationValidator).build(); + var qa1 = access.quote("A"); + var qa2 = access.quote("B/C"); + var qa3 = access.quote("D\\E"); + + assertEquals(Set.of("A", "B/C", "D\\E"), seenAuths); + seenAuths.clear(); + + assertEquals("A", access.unquote(qa1)); + assertEquals("B/C", access.unquote(qa2)); + assertEquals("D\\E", access.unquote(qa3)); + + assertEquals(Set.of("A", "B/C", "D\\E"), seenAuths); + seenAuths.clear(); + + var eval = access.newEvaluator(access.newAuthorizations(Set.of("A"))); + assertFalse(eval.canAccess(qa1 + "&" + qa2 + "&" + qa3)); + assertEquals(Set.of("A", "B/C", "D\\E"), seenAuths); + seenAuths.clear(); + + eval = access.newEvaluator(a -> a.equals("A")); + assertFalse(eval.canAccess(qa1 + "&" + qa2 + "&" + qa3)); + assertEquals(Set.of("A", "B/C", "D\\E"), seenAuths); + seenAuths.clear(); + + } + // TODO need to copy all test from Accumulo } diff --git a/core/src/test/java/org/apache/accumulo/access/tests/AccessExpressionBenchmark.java b/core/src/test/java/org/apache/accumulo/access/tests/AccessExpressionBenchmark.java index b9706b2..a5eb3b4 100644 --- a/core/src/test/java/org/apache/accumulo/access/tests/AccessExpressionBenchmark.java +++ b/core/src/test/java/org/apache/accumulo/access/tests/AccessExpressionBenchmark.java @@ -28,10 +28,8 @@ import java.util.stream.Collectors; import java.util.stream.Stream; +import org.apache.accumulo.access.Access; import org.apache.accumulo.access.AccessEvaluator; -import org.apache.accumulo.access.AccessExpression; -import org.apache.accumulo.access.Authorizations; -import org.apache.accumulo.access.ParsedAccessExpression; import org.apache.accumulo.core.security.ColumnVisibility; import org.apache.accumulo.core.security.VisibilityEvaluator; import org.apache.accumulo.core.security.VisibilityParseException; @@ -74,12 +72,14 @@ public static class VisibilityEvaluatorTests { public static class EvaluatorTests { AccessEvaluator evaluator; - List expressions; + List expressions; } @State(Scope.Benchmark) public static class BenchmarkState { + private Access access; + private ArrayList allTestExpressions; private ArrayList allTestExpressionsStr; @@ -90,6 +90,7 @@ public static class BenchmarkState { @Setup public void loadData() throws IOException { + access = Access.builder().build(); List testData = AccessEvaluatorTest.readTestData(); allTestExpressions = new ArrayList<>(); allTestExpressionsStr = new ArrayList<>(); @@ -120,11 +121,12 @@ public void loadData() throws IOException { et.expressions = new ArrayList<>(); if (testDataSet.auths.length == 1) { - et.evaluator = AccessEvaluator.of(Authorizations.of(Set.of(testDataSet.auths[0]))); + et.evaluator = + access.newEvaluator(access.newAuthorizations(Set.of(testDataSet.auths[0]))); } else { - var authSets = Stream.of(testDataSet.auths).map(a -> Authorizations.of(Set.of(a))) + var authSets = Stream.of(testDataSet.auths).map(a -> access.newAuthorizations(Set.of(a))) .collect(Collectors.toList()); - et.evaluator = AccessEvaluator.of(authSets); + et.evaluator = access.newEvaluator(authSets); } for (var tests : testDataSet.tests) { @@ -133,7 +135,7 @@ public void loadData() throws IOException { allTestExpressionsStr.add(exp); byte[] byteExp = exp.getBytes(UTF_8); allTestExpressions.add(byteExp); - et.expressions.add(byteExp); + et.expressions.add(exp); vet.expressions.add(byteExp); vet.columnVisibilities.add(new ColumnVisibility(byteExp)); } @@ -168,8 +170,9 @@ List getVisibilityEvaluatorTests() { */ @Benchmark public void measureBytesValidation(BenchmarkState state, Blackhole blackhole) { + var accumuloAccess = state.access; for (byte[] accessExpression : state.getBytesExpressions()) { - AccessExpression.validate(accessExpression); + accumuloAccess.validateExpression(new String(accessExpression, UTF_8)); } } @@ -178,8 +181,9 @@ public void measureBytesValidation(BenchmarkState state, Blackhole blackhole) { */ @Benchmark public void measureStringValidation(BenchmarkState state, Blackhole blackhole) { + var accumuloAccess = state.access; for (String accessExpression : state.getStringExpressions()) { - AccessExpression.validate(accessExpression); + accumuloAccess.validateExpression(accessExpression); } } @@ -189,8 +193,9 @@ public void measureStringValidation(BenchmarkState state, Blackhole blackhole) { */ @Benchmark public void measureCreateParseTree(BenchmarkState state, Blackhole blackhole) { + var accumuloAccess = state.access; for (String accessExpression : state.getStringExpressions()) { - blackhole.consume(ParsedAccessExpression.parse(accessExpression)); + blackhole.consume(accumuloAccess.newParsedExpression(accessExpression)); } } @@ -200,7 +205,7 @@ public void measureCreateParseTree(BenchmarkState state, Blackhole blackhole) { @Benchmark public void measureParseAndEvaluation(BenchmarkState state, Blackhole blackhole) { for (EvaluatorTests evaluatorTests : state.getEvaluatorTests()) { - for (byte[] expression : evaluatorTests.expressions) { + for (String expression : evaluatorTests.expressions) { blackhole.consume(evaluatorTests.evaluator.canAccess(expression)); } } @@ -250,10 +255,13 @@ public static void main(String[] args) throws RunnerException, IOException { System.out.println("Number of Expressions: " + numExpressions); - Options opt = new OptionsBuilder().include(AccessExpressionBenchmark.class.getSimpleName()) - .mode(Mode.Throughput).operationsPerInvocation(numExpressions) - .timeUnit(TimeUnit.MICROSECONDS).warmupTime(TimeValue.seconds(5)).warmupIterations(3) - .measurementIterations(4).forks(3).build(); + var include = System.getenv().getOrDefault("ACCESS_BENCHMARK", + AccessExpressionBenchmark.class.getSimpleName()); + + Options opt = new OptionsBuilder().include(include).mode(Mode.Throughput) + .operationsPerInvocation(numExpressions).timeUnit(TimeUnit.MICROSECONDS) + .warmupTime(TimeValue.seconds(5)).warmupIterations(3).measurementIterations(4).forks(3) + .build(); new Runner(opt).run(); } } diff --git a/core/src/test/java/org/apache/accumulo/access/tests/AccessExpressionTest.java b/core/src/test/java/org/apache/accumulo/access/tests/AccessExpressionTest.java index c7f4c40..55abca3 100644 --- a/core/src/test/java/org/apache/accumulo/access/tests/AccessExpressionTest.java +++ b/core/src/test/java/org/apache/accumulo/access/tests/AccessExpressionTest.java @@ -31,13 +31,14 @@ import java.io.InputStreamReader; import java.net.URISyntaxException; import java.util.ArrayList; -import java.util.Arrays; import java.util.HashSet; import java.util.List; import java.util.function.Predicate; import java.util.stream.Collectors; +import org.apache.accumulo.access.Access; import org.apache.accumulo.access.AccessExpression; +import org.apache.accumulo.access.AuthorizationValidator; import org.apache.accumulo.access.InvalidAccessExpressionException; import org.apache.accumulo.access.ParsedAccessExpression; import org.junit.jupiter.api.Test; @@ -47,6 +48,12 @@ public class AccessExpressionTest { @Test public void testGetAuthorizations() { + HashSet seenAuths = new HashSet<>(); + AuthorizationValidator authorizationValidator = (auth, authChars) -> { + seenAuths.add(auth.toString()); + return AuthorizationValidator.DEFAULT.test(auth, authChars); + }; + var access = Access.builder().authorizationValidator(authorizationValidator).build(); // Test data pairs where the first entry of each pair is an expression to normalize and second // is the expected authorization in the expression var testData = new ArrayList>(); @@ -66,24 +73,22 @@ public void testGetAuthorizations() { var expression = testCase.get(0); var expected = testCase.get(1); HashSet found = new HashSet<>(); - AccessExpression.findAuthorizations(expression, found::add); + access.findAuthorizations(expression, found::add); var actual = found.stream().sorted().collect(Collectors.joining(",")); assertEquals(expected, actual); found.clear(); - AccessExpression.findAuthorizations(expression.getBytes(UTF_8), found::add); - actual = found.stream().sorted().collect(Collectors.joining(",")); + actual = seenAuths.stream().sorted().collect(Collectors.joining(",")); assertEquals(expected, actual); + seenAuths.clear(); } } void checkError(String expression, String expected, int index) { - checkError(() -> AccessExpression.validate(expression), expected, index); - checkError(() -> AccessExpression.validate(expression.getBytes(UTF_8)), expected, index); - checkError(() -> AccessExpression.of(expression), expected, index); - checkError(() -> AccessExpression.of(expression.getBytes(UTF_8)), expected, index); - checkError(() -> AccessExpression.parse(expression), expected, index); - checkError(() -> AccessExpression.parse(expression.getBytes(UTF_8)), expected, index); + var access = Access.builder().build(); + checkError(() -> access.validateExpression(expression), expected, index); + checkError(() -> access.newExpression(expression), expected, index); + checkError(() -> access.newParsedExpression(expression), expected, index); } void checkError(Executable executable, String expected, int index) { @@ -129,10 +134,11 @@ public void testErrorMessages() { @Test public void testEqualsHashcode() { - var ae1 = AccessExpression.of("A&B"); - var ae2 = AccessExpression.of("A&C"); - var ae3 = AccessExpression.of("A&B"); - var ae4 = AccessExpression.parse("A&B"); + var access = Access.builder().build(); + var ae1 = access.newExpression("A&B"); + var ae2 = access.newExpression("A&C"); + var ae3 = access.newExpression("A&B"); + var ae4 = access.newParsedExpression("A&B"); assertEquals(ae1, ae3); assertEquals(ae1, ae4); @@ -183,26 +189,23 @@ public void testSpecificationDocumentation() throws IOException, URISyntaxExcept @Test public void testEmpty() { + var access = Access.builder().build(); // do not expect empty expression to fail validation - AccessExpression.validate(new byte[0]); - AccessExpression.validate(""); - assertEquals("", AccessExpression.of(new byte[0]).getExpression()); - assertEquals("", AccessExpression.of("").getExpression()); - - for (var parsed : List.of(AccessExpression.parse(new byte[0]), AccessExpression.parse(""))) { - assertEquals("", parsed.getExpression()); - assertTrue(parsed.getChildren().isEmpty()); - assertEquals(ParsedAccessExpression.ExpressionType.EMPTY, parsed.getType()); - } + access.validateExpression(""); + assertEquals("", access.newExpression("").getExpression()); + + var parsed = access.newParsedExpression(""); + assertEquals("", parsed.getExpression()); + assertTrue(parsed.getChildren().isEmpty()); + assertEquals(ParsedAccessExpression.ExpressionType.EMPTY, parsed.getType()); } @Test public void testImmutable() { - byte[] exp = "A&B&(C|D)".getBytes(UTF_8); - var exp1 = AccessExpression.of(exp); - var exp2 = AccessExpression.parse(exp); - Arrays.fill(exp, (byte) 0); - assertEquals("A&B&(C|D)", exp1.getExpression()); + var access = Access.builder().build(); + var exp = "A&B&(C|D)"; + var exp2 = access.newParsedExpression(exp); + assertEquals("A&B&(C|D)", exp2.getExpression()); assertEquals("A", exp2.getChildren().get(0).getExpression()); @@ -219,22 +222,13 @@ public void testImmutable() { @Test public void testNull() { - assertThrows(NullPointerException.class, () -> AccessExpression.parse((byte[]) null)); - assertThrows(NullPointerException.class, () -> AccessExpression.parse((String) null)); - assertThrows(NullPointerException.class, () -> AccessExpression.validate((byte[]) null)); - assertThrows(NullPointerException.class, () -> AccessExpression.validate((String) null)); - assertThrows(NullPointerException.class, () -> AccessExpression.of((byte[]) null)); - assertThrows(NullPointerException.class, () -> AccessExpression.of((String) null)); - assertThrows(NullPointerException.class, - () -> AccessExpression.findAuthorizations((byte[]) null, auth -> {})); - assertThrows(NullPointerException.class, - () -> AccessExpression.findAuthorizations((String) null, auth -> {})); - assertThrows(NullPointerException.class, - () -> AccessExpression.findAuthorizations("A&B".getBytes(UTF_8), null)); - assertThrows(NullPointerException.class, - () -> AccessExpression.findAuthorizations("A&B", null)); - assertThrows(NullPointerException.class, () -> AccessExpression.quote((byte[]) null)); - assertThrows(NullPointerException.class, () -> AccessExpression.quote((String) null)); - assertThrows(NullPointerException.class, () -> AccessExpression.unquote(null)); + var access = Access.builder().build(); + assertThrows(NullPointerException.class, () -> access.newParsedExpression(null)); + assertThrows(NullPointerException.class, () -> access.validateExpression(null)); + assertThrows(NullPointerException.class, () -> access.newExpression(null)); + assertThrows(NullPointerException.class, () -> access.findAuthorizations(null, auth -> {})); + assertThrows(NullPointerException.class, () -> access.findAuthorizations("A&B", null)); + assertThrows(NullPointerException.class, () -> access.quote(null)); + assertThrows(NullPointerException.class, () -> access.unquote(null)); } } diff --git a/core/src/test/java/org/apache/accumulo/access/tests/AuthorizationTest.java b/core/src/test/java/org/apache/accumulo/access/tests/AuthorizationTest.java index 53d8164..b869465 100644 --- a/core/src/test/java/org/apache/accumulo/access/tests/AuthorizationTest.java +++ b/core/src/test/java/org/apache/accumulo/access/tests/AuthorizationTest.java @@ -18,26 +18,36 @@ */ package org.apache.accumulo.access.tests; +import static java.nio.charset.StandardCharsets.UTF_8; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import java.util.List; import java.util.Set; +import org.apache.accumulo.access.Access; import org.apache.accumulo.access.Authorizations; +import org.apache.accumulo.access.InvalidAccessExpressionException; +import org.apache.accumulo.access.InvalidAuthorizationException; +import org.apache.accumulo.access.impl.Tokenizer; import org.junit.jupiter.api.Test; public class AuthorizationTest { @Test public void testEquality() { - Authorizations auths1 = Authorizations.of(Set.of("A", "B", "C")); - Authorizations auths2 = Authorizations.of(Set.of("A", "B", "C")); + var access = Access.builder().build(); + Authorizations auths1 = access.newAuthorizations(Set.of("A", "B", "C")); + Authorizations auths2 = access.newAuthorizations(Set.of("A", "B", "C")); assertEquals(auths1, auths2); assertEquals(auths1.hashCode(), auths2.hashCode()); - Authorizations auths3 = Authorizations.of(Set.of("D", "E", "F")); + Authorizations auths3 = access.newAuthorizations(Set.of("D", "E", "F")); assertNotEquals(auths1, auths3); assertNotEquals(auths1.hashCode(), auths3.hashCode()); @@ -45,11 +55,241 @@ public void testEquality() { @Test public void testEmpty() { + var access = Access.builder().build(); // check if new object is allocated - assertSame(Authorizations.of(), Authorizations.of()); - // check if optimization is working - assertSame(Authorizations.of(), Authorizations.of(Set.of())); - assertEquals(Set.of(), Authorizations.of().asSet()); - assertSame(Set.of(), Authorizations.of().asSet()); + assertSame(access.newAuthorizations(Set.of()), access.newAuthorizations(Set.of())); + assertEquals(Set.of(), access.newAuthorizations(Set.of()).asSet()); + assertSame(Set.of(), access.newAuthorizations(Set.of()).asSet()); + } + + @Test + public void testAuthorizationValidation() { + + // create an instance of accumulo access that expects all auths to start with a lower case + // letter followed by one or more lower case letters or digits. + var accumuloAccess = Access.builder().authorizationValidator((auth, quoting) -> { + if (auth.length() < 2) { + return false; + } + + char c = auth.charAt(0); + if (!Character.isLowerCase(c) || !Character.isLetter(c)) { + return false; + } + + for (int i = 1; i < auth.length(); i++) { + c = auth.charAt(i); + boolean valid = Character.isDigit(c) || (Character.isLetter(c) && Character.isLowerCase(c)); + if (!valid) { + return false; + } + } + + return true; + }).build(); + + runTest("ac", "a9", "dc", "9a", accumuloAccess); + } + + @Test + public void testNonUnicode() { + // test with a character that is not unicode + char c = '\u0378'; + assertFalse(Character.isDefined(c)); + assertFalse(Character.isISOControl(c)); + var badAuth = new String(new char[] {'a', c}); + + var access = Access.builder().build(); + runTest("ac", "a9", "dc", badAuth, access); + } + + @Test + public void testControlCharacters() { + char c = '\u000c'; + assertTrue(Character.isDefined(c)); + assertTrue(Character.isISOControl(c)); + + var access = Access.builder().build(); + var badAuth = new String(new char[] {'a', c}); + runTest("ac", "a9", "dc", badAuth, access); + } + + @Test + public void testReplacementCharacter() { + char c = '\uFFFD'; + assertEquals(c + "", UTF_8.newDecoder().replacement()); + + var access = Access.builder().build(); + var badAuth = new String(new char[] {'a', c}); + runTest("ac", "a9", "dc", badAuth, access); + } + + @Test + public void testAuthorizationCharacters() { + for (char c = 0; c < Character.MAX_VALUE; c++) { + boolean valid = (c >= 'a' && c <= 'z') | (c >= 'A' && c <= 'Z') | (c >= '0' && c <= '9') + | c == '_' | c == '-' | c == ':' | c == '.' | c == '/'; + // This code had a bug where it was only considering the lower 8 bits of the 16 bits in the + // char and that caused an obscure and easy to miss problem. So this test was written to cover + // all 64k chars. + assertEquals(valid, Tokenizer.isValidAuthChar(c)); + } + } + + @Test + public void testUnescaped() { + // This test ensures that auth passed to the authorization validator are unescaped, even if they + // are escaped in the expression + var accumuloAccess = Access.builder().authorizationValidator((auth, quoting) -> { + for (int i = 0; i < auth.length(); i++) { + assertNotEquals('\\', auth.charAt(i)); + } + return true; + }).build(); + + var quoted = accumuloAccess.quote("ABC\"D"); + assertEquals('"' + "ABC\\\"D" + '"', quoted); + assertEquals("ABC\"D", accumuloAccess.unquote(quoted)); + + var auths1 = accumuloAccess.newAuthorizations(Set.of("ABC\"D", "DEF")); + var auths2 = accumuloAccess.newAuthorizations(Set.of("ABC\"D", "XYZ")); + var evaluator = accumuloAccess.newEvaluator(auths1); + + assertTrue(evaluator.canAccess(quoted + "|XYZ")); + assertTrue(evaluator.canAccess("(XYZ&RST)|" + quoted)); + + assertTrue(evaluator.canAccess(accumuloAccess.newExpression("(XYZ&RST)|" + quoted))); + assertTrue(evaluator.canAccess(accumuloAccess.newParsedExpression("(XYZ&RST)|" + quoted))); + assertTrue(evaluator.canAccess(accumuloAccess.newExpression("(XYZ&RST)|" + quoted).parse())); + + var multiEvaluator = accumuloAccess.newEvaluator(List.of(auths1, auths2)); + assertTrue(multiEvaluator.canAccess(quoted + "|XYZ")); + assertFalse(multiEvaluator.canAccess(quoted + "&DEF")); + assertTrue(evaluator.canAccess(quoted + "&DEF")); + + } + + @Test + public void testMultiCharCodepoint() { + // Some unicode code points span two UTF-16 chars, this test ensures its possible to handle + // those. + + String doubleChar = ""; + // find any valid code point that takes two chars + for (int i = 1 << 16; i < Integer.MAX_VALUE; i++) { + if (Character.isDefined(i)) { + var dc = Character.toChars(i); + if (dc.length == 2) { + doubleChar = new String(dc); + break; + } + } + } + + assertEquals(2, doubleChar.length()); + + // create an auth that uses doubleChar + var auth1 = "a" + doubleChar; + var auth2 = "abc"; + + var access = Access.builder().build(); + + var exp1 = access.quote(auth1) + "&" + auth2; + var exp2 = access.quote(auth1) + "|" + auth2; + + assertEquals('"' + auth1 + '"' + "&" + auth2, exp1); + assertEquals('"' + auth1 + '"' + "|" + auth2, exp2); + + var auths1 = access.newAuthorizations(Set.of(auth1, auth2)); + var evaluator1 = access.newEvaluator(auths1); + assertTrue(evaluator1.canAccess(exp1)); + assertTrue(evaluator1.canAccess(exp2)); + + var auths2 = access.newAuthorizations(Set.of(auth1)); + var evaluator2 = access.newEvaluator(auths2); + assertFalse(evaluator2.canAccess(exp1)); + assertTrue(evaluator2.canAccess(exp2)); + + var auths3 = access.newAuthorizations(Set.of(auth2)); + var evaluator3 = access.newEvaluator(auths3); + assertFalse(evaluator3.canAccess(exp1)); + assertTrue(evaluator3.canAccess(exp2)); + } + + private static void runTest(String goodAuth1, String goodAuth2, String goodAuth3, String badAuth, + Access accumuloAccess) { + List auths = List.of(goodAuth1, goodAuth2, badAuth, goodAuth3); + + var auths1 = accumuloAccess.newAuthorizations(Set.of(goodAuth1, goodAuth2)); + var auths2 = accumuloAccess.newAuthorizations(Set.of(goodAuth3, goodAuth2)); + + var evaluator = accumuloAccess.newEvaluator(auths1); + var multiEvaluator = accumuloAccess.newEvaluator(List.of(auths1, auths2)); + var evaluator2 = accumuloAccess.newEvaluator(auth -> auth.equals(goodAuth3)); + + for (int i = 0; i < 4; i++) { + var a1 = auths.get(i); + var a2 = auths.get((i + 1) % 4); + var a3 = auths.get((i + 2) % 4); + var a4 = auths.get((i + 3) % 4); + + // create the same expression with the invalid auth in different places + var exp = + '"' + a1 + '"' + "|(" + '"' + a2 + '"' + "&" + '"' + a3 + '"' + ")|" + '"' + a4 + '"'; + var exception = assertThrows(InvalidAuthorizationException.class, + () -> accumuloAccess.validateExpression(exp)); + assertTrue(exception.getMessage().contains(badAuth)); + + exception = assertThrows(InvalidAuthorizationException.class, + () -> accumuloAccess.newExpression(exp)); + assertTrue(exception.getMessage().contains(badAuth)); + + exception = assertThrows(InvalidAuthorizationException.class, + () -> accumuloAccess.newParsedExpression(exp)); + assertTrue(exception.getMessage().contains(badAuth)); + + exception = assertThrows(InvalidAuthorizationException.class, () -> evaluator.canAccess(exp)); + assertTrue(exception.getMessage().contains(badAuth)); + + exception = + assertThrows(InvalidAuthorizationException.class, () -> multiEvaluator.canAccess(exp)); + assertTrue(exception.getMessage().contains(badAuth)); + + exception = + assertThrows(InvalidAuthorizationException.class, () -> evaluator2.canAccess(exp)); + assertTrue(exception.getMessage().contains(badAuth)); + + exception = assertThrows(InvalidAuthorizationException.class, + () -> accumuloAccess.findAuthorizations(exp, a -> {})); + assertTrue(exception.getMessage().contains(badAuth)); + + exception = assertThrows(InvalidAuthorizationException.class, + () -> accumuloAccess.newAuthorizations(Set.of(a1, a2, a3, a4))); + assertTrue(exception.getMessage().contains(badAuth)); + + if (badAuth.chars().anyMatch(c -> !Tokenizer.isValidAuthChar((char) c))) { + // If the expression is created w/o quoting then the bad auth should just be seen as an + // invalid character in the expression and it should not even consult the authorization + // validation code. This should result in a different exception. + var exp2 = a1 + "|(" + a2 + "&" + a3 + ")|" + a4; + var exception2 = + assertThrows(InvalidAccessExpressionException.class, () -> evaluator.canAccess(exp2)); + assertTrue(exception2.getMessage().contains(badAuth)); + } else { + // The bad auth does not need quoting, but should still see an invalid auth exception + var exp2 = a1 + "|(" + a2 + "&" + a3 + ")|" + a4; + var exception2 = + assertThrows(InvalidAuthorizationException.class, () -> evaluator.canAccess(exp2)); + assertTrue(exception2.getMessage().contains(badAuth)); + } + } + + var exception = + assertThrows(InvalidAuthorizationException.class, () -> accumuloAccess.quote(badAuth)); + assertTrue(exception.getMessage().contains(badAuth)); + + exception = assertThrows(InvalidAuthorizationException.class, + () -> accumuloAccess.unquote('"' + badAuth + '"')); + assertTrue(exception.getMessage().contains(badAuth)); } } diff --git a/core/src/test/java/org/apache/accumulo/access/tests/ParsedAccessExpressionTest.java b/core/src/test/java/org/apache/accumulo/access/tests/ParsedAccessExpressionTest.java index 3ef9c55..0acbdce 100644 --- a/core/src/test/java/org/apache/accumulo/access/tests/ParsedAccessExpressionTest.java +++ b/core/src/test/java/org/apache/accumulo/access/tests/ParsedAccessExpressionTest.java @@ -18,7 +18,6 @@ */ package org.apache.accumulo.access.tests; -import static java.nio.charset.StandardCharsets.UTF_8; import static org.apache.accumulo.access.ParsedAccessExpression.ExpressionType.AND; import static org.apache.accumulo.access.ParsedAccessExpression.ExpressionType.AUTHORIZATION; import static org.apache.accumulo.access.ParsedAccessExpression.ExpressionType.EMPTY; @@ -28,26 +27,44 @@ import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertThrows; +import java.util.HashSet; import java.util.List; +import java.util.Set; -import org.apache.accumulo.access.AccessExpression; +import org.apache.accumulo.access.Access; +import org.apache.accumulo.access.AuthorizationValidator; import org.apache.accumulo.access.ParsedAccessExpression; import org.junit.jupiter.api.Test; public class ParsedAccessExpressionTest { @Test public void testParsing() { - String expression = "(BLUE&(RED|PINK|YELLOW))|((YELLOW|\"GREEN/GREY\")&(RED|BLUE))|BLACK"; - for (var parsed : List.of(AccessExpression.parse(expression), - AccessExpression.parse(expression.getBytes(UTF_8)), AccessExpression.of(expression).parse(), - AccessExpression.of(expression.getBytes(UTF_8)).parse())) { + HashSet seenAuths = new HashSet<>(); + AuthorizationValidator authorizationValidator = (auth, authChars) -> { + seenAuths.add(auth.toString()); + return AuthorizationValidator.DEFAULT.test(auth, authChars); + }; + var access = Access.builder().authorizationValidator(authorizationValidator).build(); + String expression = + "(BLUE&(RED|PINK|YELLOW))|((YELLOW|\"GREEN/GREY\")&(RED|BLUE))|\"BLACK\\\\RED\""; + + var pae1 = access.newParsedExpression(expression); + // check that unescaped and unquoted auths are passed in to the auth validator + assertEquals(Set.of("BLUE", "RED", "PINK", "YELLOW", "GREEN/GREY", "BLACK\\RED"), seenAuths); + seenAuths.clear(); + var pae2 = access.newExpression(expression).parse(); + // check that unescaped and unquoted auths are passed in to the auth validator + assertEquals(Set.of("BLUE", "RED", "PINK", "YELLOW", "GREEN/GREY", "BLACK\\RED"), seenAuths); + + for (var parsed : List.of(pae1, pae2)) { // verify root node - verify("(BLUE&(RED|PINK|YELLOW))|((YELLOW|\"GREEN/GREY\")&(RED|BLUE))|BLACK", OR, 3, parsed); + verify("(BLUE&(RED|PINK|YELLOW))|((YELLOW|\"GREEN/GREY\")&(RED|BLUE))|\"BLACK\\\\RED\"", OR, + 3, parsed); // verify all nodes at level 1 in the tree verify("BLUE&(RED|PINK|YELLOW)", AND, 2, parsed, 0); verify("(YELLOW|\"GREEN/GREY\")&(RED|BLUE)", AND, 2, parsed, 1); - verify("BLACK", AUTHORIZATION, 0, parsed, 2); + verify("\"BLACK\\\\RED\"", AUTHORIZATION, 0, parsed, 2); // verify all nodes at level 2 in the tree verify("BLUE", AUTHORIZATION, 0, parsed, 0, 0); @@ -68,16 +85,15 @@ public void testParsing() { @Test public void testEmpty() { - var parsed = AccessExpression.parse(""); - verify("", EMPTY, 0, parsed); - parsed = AccessExpression.parse(new byte[0]); + var access = Access.builder().build(); + var parsed = access.newParsedExpression(""); verify("", EMPTY, 0, parsed); } @Test public void testParseTwice() { - for (var expression : List.of(AccessExpression.of("A&B"), - AccessExpression.of("A&B".getBytes(UTF_8)))) { + var access = Access.builder().build(); + for (var expression : List.of(access.newExpression("A&B"))) { var parsed = expression.parse(); assertNotSame(expression, parsed); assertEquals(expression.getExpression(), parsed.getExpression()); diff --git a/examples/src/main/java/org/apache/accumulo/access/examples/AccessExample.java b/examples/src/main/java/org/apache/accumulo/access/examples/AccessExample.java index d60bcfb..b9fdd6d 100644 --- a/examples/src/main/java/org/apache/accumulo/access/examples/AccessExample.java +++ b/examples/src/main/java/org/apache/accumulo/access/examples/AccessExample.java @@ -25,8 +25,9 @@ import java.util.Set; import java.util.TreeMap; +import org.apache.accumulo.access.Access; import org.apache.accumulo.access.AccessEvaluator; -import org.apache.accumulo.access.Authorizations; +import org.apache.accumulo.access.AuthorizationValidator; public class AccessExample { @@ -52,8 +53,11 @@ public void run(PrintStream out, String... authorizations) { out.printf("Showing accessible records using authorizations: %s%n", Arrays.toString(authorizations)); + var access = Access.builder().authorizationValidator(AuthorizationValidator.DEFAULT).build(); + // Create an access evaluator using the provided authorizations - AccessEvaluator evaluator = AccessEvaluator.of(Authorizations.of(Set.of(authorizations))); + AccessEvaluator evaluator = + access.newEvaluator(access.newAuthorizations(Set.of(authorizations))); // Print each record whose access expression permits viewing using the provided authorizations getData().forEach((record, accessExpression) -> { diff --git a/examples/src/main/java/org/apache/accumulo/access/examples/ParseExamples.java b/examples/src/main/java/org/apache/accumulo/access/examples/ParseExamples.java index d9cf134..4aff924 100644 --- a/examples/src/main/java/org/apache/accumulo/access/examples/ParseExamples.java +++ b/examples/src/main/java/org/apache/accumulo/access/examples/ParseExamples.java @@ -18,15 +18,13 @@ */ package org.apache.accumulo.access.examples; -import static org.apache.accumulo.access.AccessExpression.quote; -import static org.apache.accumulo.access.AccessExpression.unquote; import static org.apache.accumulo.access.ParsedAccessExpression.ExpressionType.AND; import static org.apache.accumulo.access.ParsedAccessExpression.ExpressionType.AUTHORIZATION; import java.util.Map; import java.util.TreeSet; -import org.apache.accumulo.access.AccessExpression; +import org.apache.accumulo.access.Access; import org.apache.accumulo.access.ParsedAccessExpression; import org.apache.accumulo.access.ParsedAccessExpression.ExpressionType; @@ -36,6 +34,8 @@ */ public class ParseExamples { + public static final Access ACCESS = Access.builder().build(); + private ParseExamples() {} /** @@ -47,10 +47,10 @@ public static void replaceAuthorizations(ParsedAccessExpression parsed, // If the term is quoted in the expression, the quotes will be preserved. Calling unquote() // will only unescape and unquote if the string is quoted, otherwise it returns the string as // is. - String auth = unquote(parsed.getExpression()); + String auth = ACCESS.unquote(parsed.getExpression()); // Must quote any authorization that needs it. Calling quote() will only quote and escape if // needed, otherwise it returns the string as is. - expressionBuilder.append(quote(replacements.getOrDefault(auth, auth))); + expressionBuilder.append(ACCESS.quote(replacements.getOrDefault(auth, auth))); } else { String operator = parsed.getType() == AND ? "&" : "|"; String sep = ""; @@ -103,7 +103,7 @@ public int compareTo(NormalizedExpression o) { if (cmp == 0) { if (type == AUTHORIZATION) { // sort based on the unquoted and unescaped form of the authorization - cmp = unquote(expression).compareTo(unquote(o.expression)); + cmp = ACCESS.unquote(expression).compareTo(ACCESS.unquote(o.expression)); } else { cmp = expression.compareTo(o.expression); } @@ -170,8 +170,8 @@ public static NormalizedExpression normalize(ParsedAccessExpression parsed) { if (parsed.getType() == AUTHORIZATION) { // If the authorization is quoted and it does not need to be quoted then the following two // lines will remove the unnecessary quoting. - String unquoted = AccessExpression.unquote(parsed.getExpression()); - String quoted = AccessExpression.quote(unquoted); + String unquoted = ACCESS.unquote(parsed.getExpression()); + String quoted = ACCESS.quote(unquoted); return new NormalizedExpression(quoted, parsed.getType()); } else { // The tree set does the work of sorting and deduplicating sub expressions. @@ -217,7 +217,7 @@ public static void walk(String indent, ParsedAccessExpression parsed) { public static void main(String[] args) { - var parsed = AccessExpression.parse("((RED&GREEN)|(PINK&BLUE))"); + var parsed = ACCESS.newParsedExpression("((RED&GREEN)|(PINK&BLUE))"); System.out.printf("Operating on %s%n", parsed); diff --git a/examples/src/test/java/org/apache/accumulo/access/examples/test/ParseExamplesTest.java b/examples/src/test/java/org/apache/accumulo/access/examples/test/ParseExamplesTest.java index 6800a01..c34d045 100644 --- a/examples/src/test/java/org/apache/accumulo/access/examples/test/ParseExamplesTest.java +++ b/examples/src/test/java/org/apache/accumulo/access/examples/test/ParseExamplesTest.java @@ -18,7 +18,7 @@ */ package org.apache.accumulo.access.examples.test; -import static java.nio.charset.StandardCharsets.UTF_8; +import static org.apache.accumulo.access.examples.ParseExamples.ACCESS; import static org.apache.accumulo.access.examples.ParseExamples.replaceAuthorizations; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -26,7 +26,6 @@ import java.util.List; import java.util.Map; -import org.apache.accumulo.access.AccessExpression; import org.apache.accumulo.access.examples.ParseExamples; import org.junit.jupiter.api.Test; @@ -90,11 +89,7 @@ public void testNormalize() { var expression = testCase.get(0); var expected = testCase.get(1); - var actual = ParseExamples.normalize(AccessExpression.parse(expression)).expression; - assertEquals(expected, actual); - - actual = - ParseExamples.normalize(AccessExpression.parse(expression.getBytes(UTF_8))).expression; + var actual = ParseExamples.normalize(ACCESS.newParsedExpression(expression)).expression; assertEquals(expected, actual); } } @@ -102,13 +97,13 @@ public void testNormalize() { @Test public void testReplace() { // Test replacement code w/ quoting and escaping. - var parsed = AccessExpression.parse("((RED&\"ESC\\\\\")|(PINK&BLUE))"); + var parsed = ACCESS.newParsedExpression("((RED&\"ESC\\\\\")|(PINK&BLUE))"); StringBuilder expressionBuilder = new StringBuilder(); replaceAuthorizations(parsed, expressionBuilder, Map.of("ESC\\", "NEEDS+QUOTE")); assertEquals("(RED&\"NEEDS+QUOTE\")|(PINK&BLUE)", expressionBuilder.toString()); // Test replacing multiple - parsed = AccessExpression.parse("((RED&(GREEN|YELLOW))|(PINK&BLUE))"); + parsed = ACCESS.newParsedExpression("((RED&(GREEN|YELLOW))|(PINK&BLUE))"); expressionBuilder = new StringBuilder(); replaceAuthorizations(parsed, expressionBuilder, Map.of("RED", "ROUGE", "GREEN", "AQUA")); assertEquals("(ROUGE&(AQUA|YELLOW))|(PINK&BLUE)", expressionBuilder.toString()); diff --git a/src/build/ci/spotbugs-exclude.xml b/src/build/ci/spotbugs-exclude.xml index c7e7650..797c675 100644 --- a/src/build/ci/spotbugs-exclude.xml +++ b/src/build/ci/spotbugs-exclude.xml @@ -23,4 +23,9 @@ + + + + +