Skip to content

Replace Authorizer with Predicate<String> #64

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
DomGarguilo wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Replace Authorizer with Predicate<String> #64

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e5c63a5
Replace Authorizer with Predicate<String>
DomGarguilo Feb 20, 2024
790c20e
Merge remote-tracking branch 'upstream/main' into replaceAuthorizer
DomGarguilo Feb 20, 2024
e16d3e1
Fix javadoc wording
DomGarguilo Feb 20, 2024
4e80d25
incorportate feedback
DomGarguilo Feb 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 11 additions & 12 deletions src/main/java/org/apache/accumulo/access/AccessEvaluator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
package org.apache.accumulo.access;

import java.util.Collection;
import java.util.List;
import java.util.function.Predicate;

/**
* This class is used to decide if an entity with a given set of authorizations can access
Expand Down Expand Up @@ -88,12 +90,18 @@ static AccessEvaluator of(Authorizations authorizations) {
}

/**
* Creates an AccessEvaluator from an Authorizer object
* Creates an AccessEvaluator from a Predicate&lt;String&gt;. The Predicate is used to test
* whether an authorization seen in an access expression is authorized. The {@code test()} method
* of the Predicate will be called with a single authorization as its argument. The Predicate
* should return true if the exact String matches an authorization that should be granted access,
* and false otherwise.
*
* @param authorizer authorizer to use in the AccessEvaluator
* @param authorizer Predicate&lt;String&gt; to use in the AccessEvaluator. This Predicate should
* return true for authorizations that should be granted access, and false for those that
* should not.
* @return AccessEvaluator object
*/
static AccessEvaluator of(Authorizer authorizer) {
static AccessEvaluator of(Predicate<String> authorizer) {
return new AccessEvaluatorImpl(authorizer);
}

Expand Down Expand Up @@ -159,13 +167,4 @@ static AccessEvaluator of(String... authorizations) {
return new AccessEvaluatorImpl(AccessEvaluatorImpl.convert(authorizations));
}

/**
* An interface that is used to check if an authorization seen in an access expression is
Copy link
Contributor

@keith-turner keith-turner Feb 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This javadoc should be moved (with rewording) to the method that now takes a predicate. Need to explain what the purpose of the predicate is, which this javadoc kinda does.

Copy link
Member Author

@DomGarguilo DomGarguilo Feb 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added to the javadoc in 4e80d25 to try to address this.

* authorized.
*
* @since 1.0.0
*/
interface Authorizer {
boolean isAuthorized(String auth);
}
}
6 changes: 3 additions & 3 deletions src/main/java/org/apache/accumulo/access/AccessEvaluatorImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,10 +74,10 @@ static Collection<List<byte[]>> convert(Authorizations authorizations) {
}

/**
* Create an AccessEvaluatorImpl using an Authorizer object
* Create an AccessEvaluatorImpl using a Predicate&lt;String&gt;
*/
AccessEvaluatorImpl(Authorizer authorizationChecker) {
this.authorizedPredicates = List.of(auth -> authorizationChecker.isAuthorized(unescape(auth)));
AccessEvaluatorImpl(Predicate<String> authorizationChecker) {
this.authorizedPredicates = List.of(auth -> authorizationChecker.test(unescape(auth)));
}

/**
Expand Down
5 changes: 2 additions & 3 deletions src/test/java/org/apache/accumulo/access/AccessEvaluatorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,9 @@
import java.io.IOException;
import java.lang.reflect.Type;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Objects;
import java.util.Set;
import java.util.stream.Collectors;
import java.util.stream.Stream;

Expand Down Expand Up @@ -87,8 +87,7 @@ public void runTestCases() throws IOException {
evaluator = AccessEvaluator.of(testSet.auths[0]);
runTestCases(testSet, evaluator);

Set<String> auths = Stream.of(testSet.auths[0]).collect(Collectors.toSet());
evaluator = AccessEvaluator.of(auths::contains);
evaluator = AccessEvaluator.of(Arrays.asList(testSet.auths[0])::contains);
runTestCases(testSet, evaluator);
} else {
var authSets =
Expand Down