From cbbdbfa78f3230a5448ffa92ccf8ce98de3fba9c Mon Sep 17 00:00:00 2001 From: Giuseppe Lillo Date: Thu, 16 Feb 2023 12:55:47 +0100 Subject: [PATCH] Fix edge cases for parsing --- .../io/aiven/kafka/auth/VerdictCache.java | 22 ++++++++++--------- .../nativeacls/AclAivenToNativeConverter.java | 2 +- ...Parser.java => AclPrincipalFormatter.java} | 8 +++---- .../kafka/auth/nativeacls/RegexParser.java | 8 +++++-- ...st.java => AclPrincipalFormatterTest.java} | 20 +++++++++++------ .../auth/nativeacls/RegexParserTest.java | 12 ++++++++++ 6 files changed, 48 insertions(+), 24 deletions(-) rename src/main/java/io/aiven/kafka/auth/nativeacls/{AclPrincipalParser.java => AclPrincipalFormatter.java} (92%) rename src/test/java/io/aiven/kafka/auth/nativeacls/{AclPrincipalParserTest.java => AclPrincipalFormatterTest.java} (68%) diff --git a/src/main/java/io/aiven/kafka/auth/VerdictCache.java b/src/main/java/io/aiven/kafka/auth/VerdictCache.java index 75b576b..2d4d956 100644 --- 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 cache = new ConcurrentHashMap<>(); private VerdictCache(final List 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 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 matcher = aclEntry -> + aclEntry.check(principal.getPrincipalType(), principal.getName(), operation, resource); + return cache.computeIfAbsent(cacheKey, key -> aclEntries.stream().anyMatch(matcher)); + } else { + return false; + } } public List 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 index ed354b1..602461a 100644 --- 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 convert(final AivenAcl aivenAcl) { } for (final var operation : AclOperationsParser.parse(aivenAcl.operationRe.pattern())) { - final List principals = AclPrincipalParser.parse( + final List principals = AclPrincipalFormatter.parse( aivenAcl.principalType, aivenAcl.principalRe.pattern() ); for (final var principal : principals) { diff --git a/src/main/java/io/aiven/kafka/auth/nativeacls/AclPrincipalParser.java b/src/main/java/io/aiven/kafka/auth/nativeacls/AclPrincipalFormatter.java similarity index 92% rename from src/main/java/io/aiven/kafka/auth/nativeacls/AclPrincipalParser.java rename to src/main/java/io/aiven/kafka/auth/nativeacls/AclPrincipalFormatter.java index 6080617..232f765 100644 --- a/src/main/java/io/aiven/kafka/auth/nativeacls/AclPrincipalParser.java +++ b/src/main/java/io/aiven/kafka/auth/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 parse(final String principalType, final String principalPattern) { if (principalType == null || principalPattern == null) { return List.of(); } - if (principalPattern.contains(".*")) { - return List.of(principalType + ":*"); - } List 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 index 5e2a9a1..f0b4452 100644 --- 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 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/src/test/java/io/aiven/kafka/auth/nativeacls/AclPrincipalParserTest.java b/src/test/java/io/aiven/kafka/auth/nativeacls/AclPrincipalFormatterTest.java similarity index 68% rename from src/test/java/io/aiven/kafka/auth/nativeacls/AclPrincipalParserTest.java rename to src/test/java/io/aiven/kafka/auth/nativeacls/AclPrincipalFormatterTest.java index acfcc3d..d940fd2 100644 --- a/src/test/java/io/aiven/kafka/auth/nativeacls/AclPrincipalParserTest.java +++ b/src/test/java/io/aiven/kafka/auth/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 index 298ec7b..d33f523 100644 --- 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)$"))