Fix edge cases for parsing

Aiven-Open · Mar 6, 2023 · cbbdbfa · cbbdbfa
1 parent 985e3a1
commit cbbdbfa
Show file tree

Hide file tree

Showing 6 changed files with 48 additions and 24 deletions.
diff --git a/src/main/java/io/aiven/kafka/auth/VerdictCache.java b/src/main/java/io/aiven/kafka/auth/VerdictCache.java
@@ -19,7 +19,6 @@
 import java.util.List;
 import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
-import java.util.concurrent.CopyOnWriteArrayList;
 import java.util.function.Predicate;
 
 import org.apache.kafka.common.security.auth.KafkaPrincipal;
@@ -31,20 +30,23 @@ public class VerdictCache {
     private final Map<String, Boolean> cache = new ConcurrentHashMap<>();
 
     private VerdictCache(final List<AivenAcl> aclEntries) {
-        this.aclEntries = new CopyOnWriteArrayList<>(aclEntries);
+        this.aclEntries = aclEntries;
     }
 
     public boolean get(final KafkaPrincipal principal,
                        final String operation,
                        final String resource) {
-        final String cacheKey = resource
-            + "|" + operation
-            + "|" + principal.getName()
-            + "|" + principal.getPrincipalType();
-
-        final Predicate<AivenAcl> matcher = aclEntry ->
-            aclEntry.check(principal.getPrincipalType(), principal.getName(), operation, resource);
-        return cache.computeIfAbsent(cacheKey, key -> aclEntries.stream().anyMatch(matcher));
+        if (aclEntries != null) {
+            final String cacheKey = resource
+                + "|" + operation
+                + "|" + principal.getName()
+                + "|" + principal.getPrincipalType();
+            final Predicate<AivenAcl> matcher = aclEntry ->
+                aclEntry.check(principal.getPrincipalType(), principal.getName(), operation, resource);
+            return cache.computeIfAbsent(cacheKey, key -> aclEntries.stream().anyMatch(matcher));
+        } else {
+            return false;
+        }
     }
 
     public List<AivenAcl> aclEntries() {

diff --git a/src/main/java/io/aiven/kafka/auth/nativeacls/AclAivenToNativeConverter.java b/src/main/java/io/aiven/kafka/auth/nativeacls/AclAivenToNativeConverter.java
@@ -38,7 +38,7 @@ public static List<AclBinding> convert(final AivenAcl aivenAcl) {
         }
 
         for (final var operation : AclOperationsParser.parse(aivenAcl.operationRe.pattern())) {
-            final List<String> principals = AclPrincipalParser.parse(
+            final List<String> principals = AclPrincipalFormatter.parse(
                 aivenAcl.principalType, aivenAcl.principalRe.pattern()
             );
             for (final var principal : principals) {

diff --git a/...a/auth/nativeacls/AclPrincipalParser.java → ...uth/nativeacls/AclPrincipalFormatter.java b/...a/auth/nativeacls/AclPrincipalParser.java → ...uth/nativeacls/AclPrincipalFormatter.java
@@ -19,19 +19,19 @@
 import java.util.List;
 import java.util.stream.Collectors;
 
-class AclPrincipalParser {
+class AclPrincipalFormatter {
     // Visible for test
     static List<String> parse(final String principalType, final String principalPattern) {
         if (principalType == null || principalPattern == null) {
             return List.of();
         }
-        if (principalPattern.contains(".*")) {
-            return List.of(principalType + ":*");
-        }
         List<String> principals = RegexParser.parse(principalPattern);
         if (principals == null) {
             principals = List.of(principalPattern);
         }
+        if (principals.contains("(.*)") || principals.contains(".*")) {
+            return List.of(principalType + ":*");
+        }
         return principals.stream()
             .map(p -> principalType + ":" + p)
             .collect(Collectors.toList());

diff --git a/src/main/java/io/aiven/kafka/auth/nativeacls/RegexParser.java b/src/main/java/io/aiven/kafka/auth/nativeacls/RegexParser.java
@@ -25,7 +25,7 @@
 
 class RegexParser {
 
-    private static final Pattern PARSER_PATTERN = Pattern.compile("\\^\\(?(.*?)\\)?\\$");
+    private static final Pattern PARSER_PATTERN = Pattern.compile("(?<=^\\^)(.*?)(?=\\$$)");
 
     // Visible for test
     /**
@@ -44,7 +44,11 @@ static List<String> parse(final String pattern) {
             return null;
         }
 
-        final String group = matcher.group(1);
+        String group = matcher.group(0);
+        final int lastChar = group.length() - 1;
+        if (group.charAt(0) == '(' && group.charAt(lastChar) == ')') {
+            group = group.substring(1, lastChar);
+        }
         return Arrays.stream(group.split("\\|"))
             .filter(Predicate.not(String::isBlank))
             .collect(Collectors.toList());

diff --git a/...th/nativeacls/AclPrincipalParserTest.java → ...nativeacls/AclPrincipalFormatterTest.java b/...th/nativeacls/AclPrincipalParserTest.java → ...nativeacls/AclPrincipalFormatterTest.java
@@ -22,36 +22,42 @@
 
 import static org.assertj.core.api.Assertions.assertThat;
 
-public class AclPrincipalParserTest {
+public class AclPrincipalFormatterTest {
 
     @Test
     public final void parseSinglePrincipal() {
-        assertThat(AclPrincipalParser.parse("User", "^username$"))
+        assertThat(AclPrincipalFormatter.parse("User", "^username$"))
             .containsExactly("User:username");
     }
 
     @ParameterizedTest
-    @ValueSource(strings = {"(.*)", "^(username|(.*))$"})
+    @ValueSource(strings = {"(.*)", ".*", "^(username|(.*))$", "^(username|.*)$"})
     public final void parseWildcardPrincipal(final String value) {
-        assertThat(AclPrincipalParser.parse("User", value))
+        assertThat(AclPrincipalFormatter.parse("User", value))
             .containsExactly("User:*");
     }
 
+    @Test
+    public final void parseNotWildcard() {
+        assertThat(AclPrincipalFormatter.parse("User", "username.*"))
+            .containsExactly("User:username.*");
+    }
+
     @Test
     public final void parseMultipleUsers() {
-        assertThat(AclPrincipalParser.parse("User", "^(user1|user2)$"))
+        assertThat(AclPrincipalFormatter.parse("User", "^(user1|user2)$"))
             .containsExactly("User:user1", "User:user2");
     }
 
     @Test
     public final void parseNullPrincipal() {
-        assertThat(AclPrincipalParser.parse("User", null))
+        assertThat(AclPrincipalFormatter.parse("User", null))
             .isEmpty();
     }
 
     @Test
     public final void parseNullPrincipalType() {
-        assertThat(AclPrincipalParser.parse(null, "^username$"))
+        assertThat(AclPrincipalFormatter.parse(null, "^username$"))
             .isEmpty();
     }
 

diff --git a/src/test/java/io/aiven/kafka/auth/nativeacls/RegexParserTest.java b/src/test/java/io/aiven/kafka/auth/nativeacls/RegexParserTest.java
@@ -35,6 +35,18 @@ public final void parseRegexListSingleWithParens() {
             .containsExactly("AAA");
     }
 
+    @Test
+    public final void parseParenthesis() {
+        assertThat(RegexParser.parse("^qwe)$"))
+            .containsExactly("qwe)");
+    }
+
+    @Test
+    public final void parseNestedRegex() {
+        assertThat(RegexParser.parse("^(AAA|^(BB)$)$"))
+            .containsExactly("AAA", "^(BB)$");
+    }
+
     @Test
     public final void parseRegexListMultiple() {
         assertThat(RegexParser.parse("^(AAA|BBB|CCC|DDD)$"))