From 52fd19412e9c67f06f0d1b8b6082bf4c8012e13d Mon Sep 17 00:00:00 2001 From: Ivan Yurchenko Date: Mon, 22 Aug 2022 17:05:26 +0300 Subject: [PATCH 1/3] Return ACLs from authorizer This commit makes the best effort to convert the current regex-based ACLs to Kafka native ACLs when the authorized is asked to list them. Filtering is not yet supported. --- checkstyle/checkstyle-suppressions.xml | 3 + .../kafka/auth/AivenAclAuthorizerConfig.java | 11 ++ .../kafka/auth/AivenAclAuthorizerV2.java | 10 +- .../io/aiven/kafka/auth/VerdictCache.java | 7 +- .../io/aiven/kafka/auth/json/AivenAcl.java | 10 +- .../OperationNameFormatter.java | 67 ++++++++++ .../ResourceTypeNameFormatter.java | 49 +++++++ .../nativeacls/AclAivenToNativeConverter.java | 55 ++++++++ .../auth/nativeacls/AclOperationsParser.java | 57 +++++++++ .../kafka/auth/nativeacls/RegexParser.java | 60 +++++++++ .../nativeacls/ResourcePatternParser.java | 109 ++++++++++++++++ .../auth/nativeacls/ResourceTypeParser.java | 53 ++++++++ .../auth/AivenAclAuthorizerConfigTest.java | 3 + .../kafka/auth/AivenAclAuthorizerV2Test.java | 65 ++++++++-- .../AclAivenToNativeConverterTest.java | 99 +++++++++++++++ .../nativeacls/AclOperationsParserTest.java | 112 ++++++++++++++++ .../auth/nativeacls/RegexParserTest.java | 55 ++++++++ .../nativeacls/ResourcePatternParserTest.java | 120 ++++++++++++++++++ .../nativeacls/ResourceTypeParserTest.java | 96 ++++++++++++++ .../resources/test_acls_for_acls_method.json | 20 +++ 20 files changed, 1045 insertions(+), 16 deletions(-) create mode 100644 src/main/java/io/aiven/kafka/auth/nameformatters/OperationNameFormatter.java create mode 100644 src/main/java/io/aiven/kafka/auth/nameformatters/ResourceTypeNameFormatter.java create mode 100644 src/main/java/io/aiven/kafka/auth/nativeacls/AclAivenToNativeConverter.java create mode 100644 src/main/java/io/aiven/kafka/auth/nativeacls/AclOperationsParser.java create mode 100644 src/main/java/io/aiven/kafka/auth/nativeacls/RegexParser.java create mode 100644 src/main/java/io/aiven/kafka/auth/nativeacls/ResourcePatternParser.java create mode 100644 src/main/java/io/aiven/kafka/auth/nativeacls/ResourceTypeParser.java create mode 100644 src/test/java/io/aiven/kafka/auth/nativeacls/AclAivenToNativeConverterTest.java create mode 100644 src/test/java/io/aiven/kafka/auth/nativeacls/AclOperationsParserTest.java create mode 100644 src/test/java/io/aiven/kafka/auth/nativeacls/RegexParserTest.java create mode 100644 src/test/java/io/aiven/kafka/auth/nativeacls/ResourcePatternParserTest.java create mode 100644 src/test/java/io/aiven/kafka/auth/nativeacls/ResourceTypeParserTest.java create mode 100644 src/test/resources/test_acls_for_acls_method.json diff --git a/checkstyle/checkstyle-suppressions.xml b/checkstyle/checkstyle-suppressions.xml index 47b07bb..69103f8 100644 --- a/checkstyle/checkstyle-suppressions.xml +++ b/checkstyle/checkstyle-suppressions.xml @@ -17,4 +17,7 @@ + + + diff --git a/src/main/java/io/aiven/kafka/auth/AivenAclAuthorizerConfig.java b/src/main/java/io/aiven/kafka/auth/AivenAclAuthorizerConfig.java index a4076d1..6082274 100644 --- a/src/main/java/io/aiven/kafka/auth/AivenAclAuthorizerConfig.java +++ b/src/main/java/io/aiven/kafka/auth/AivenAclAuthorizerConfig.java @@ -30,6 +30,7 @@ public final class AivenAclAuthorizerConfig extends AbstractConfig { private static final String AUDITOR_CLASS_NAME_CONF = "aiven.acl.authorizer.auditor.class.name"; private static final String LOG_DENIALS_CONF = "aiven.acl.authorizer.log.denials"; private static final String CONFIG_REFRESH_CONF = "aiven.acl.authorizer.config.refresh.interval"; + private static final String LIST_ACLS_ENABLED_CONF = "aiven.acl.authorizer.list.acls.enabled"; public AivenAclAuthorizerConfig(final Map originals) { super(configDef(), originals); @@ -61,6 +62,12 @@ public static ConfigDef configDef() { 10_000, ConfigDef.Importance.LOW, "The interval between ACL reloads" + ).define( + LIST_ACLS_ENABLED_CONF, + ConfigDef.Type.BOOLEAN, + true, + ConfigDef.Importance.LOW, + "Whether to allow listing ACLs" ); } @@ -79,4 +86,8 @@ public boolean logDenials() { public int configRefreshInterval() { return getInt(CONFIG_REFRESH_CONF); } + + public boolean listAclsEnabled() { + return getBoolean(LIST_ACLS_ENABLED_CONF); + } } diff --git a/src/main/java/io/aiven/kafka/auth/AivenAclAuthorizerV2.java b/src/main/java/io/aiven/kafka/auth/AivenAclAuthorizerV2.java index 16b3c53..334d13c 100644 --- a/src/main/java/io/aiven/kafka/auth/AivenAclAuthorizerV2.java +++ b/src/main/java/io/aiven/kafka/auth/AivenAclAuthorizerV2.java @@ -57,6 +57,7 @@ import io.aiven.kafka.auth.json.reader.JsonReaderException; import io.aiven.kafka.auth.nameformatters.LegacyOperationNameFormatter; import io.aiven.kafka.auth.nameformatters.LegacyResourceTypeNameFormatter; +import io.aiven.kafka.auth.nativeacls.AclAivenToNativeConverter; import kafka.network.RequestChannel.Session; import org.slf4j.Logger; @@ -242,7 +243,12 @@ public final List> deleteAcls( @Override public final Iterable acls(final AclBindingFilter filter) { - LOGGER.error("`acls` is not implemented"); - return List.of(); + if (this.config.listAclsEnabled()) { + // Filtering is not supported yet. + return AclAivenToNativeConverter.convert(this.cacheReference.get().aclEntries()); + } else { + LOGGER.warn("Listing ACLs is disabled"); + return List.of(); + } } } diff --git a/src/main/java/io/aiven/kafka/auth/VerdictCache.java b/src/main/java/io/aiven/kafka/auth/VerdictCache.java index fccba09..aecf205 100644 --- a/src/main/java/io/aiven/kafka/auth/VerdictCache.java +++ b/src/main/java/io/aiven/kafka/auth/VerdictCache.java @@ -19,6 +19,7 @@ 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; @@ -30,7 +31,7 @@ public class VerdictCache { private final Map cache = new ConcurrentHashMap<>(); private VerdictCache(final List aclEntries) { - this.aclEntries = aclEntries; + this.aclEntries = new CopyOnWriteArrayList<>(aclEntries); } public boolean get(final KafkaPrincipal principal, @@ -50,6 +51,10 @@ public boolean get(final KafkaPrincipal principal, } } + public List aclEntries() { + return aclEntries; + } + public static VerdictCache create(final List aclEntries) { return new VerdictCache(aclEntries); } diff --git a/src/main/java/io/aiven/kafka/auth/json/AivenAcl.java b/src/main/java/io/aiven/kafka/auth/json/AivenAcl.java index f636216..a71143f 100644 --- a/src/main/java/io/aiven/kafka/auth/json/AivenAcl.java +++ b/src/main/java/io/aiven/kafka/auth/json/AivenAcl.java @@ -24,19 +24,19 @@ public class AivenAcl { @SerializedName("principal_type") - private final String principalType; + public final String principalType; @SerializedName("principal") - private final Pattern principalRe; + public final Pattern principalRe; @SerializedName("operation") - private final Pattern operationRe; + public final Pattern operationRe; @SerializedName("resource") - private final Pattern resourceRe; + public final Pattern resourceRe; @SerializedName("resource_pattern") - private final String resourceRePattern; + public final String resourceRePattern; public AivenAcl(final String principalType, final String principal, diff --git a/src/main/java/io/aiven/kafka/auth/nameformatters/OperationNameFormatter.java b/src/main/java/io/aiven/kafka/auth/nameformatters/OperationNameFormatter.java new file mode 100644 index 0000000..7cc4fe6 --- /dev/null +++ b/src/main/java/io/aiven/kafka/auth/nameformatters/OperationNameFormatter.java @@ -0,0 +1,67 @@ +/* + * Copyright 2022 Aiven Oy https://aiven.io + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.aiven.kafka.auth.nameformatters; + +import org.apache.kafka.common.acl.AclOperation; + +public class OperationNameFormatter { + public static AclOperation format(final String operation) { + switch (operation) { + case "Unknown": + return AclOperation.UNKNOWN; + + case "Any": + return AclOperation.ANY; + + case "All": + return AclOperation.ALL; + + case "Read": + return AclOperation.READ; + + case "Write": + return AclOperation.WRITE; + + case "Create": + return AclOperation.CREATE; + + case "Delete": + return AclOperation.DELETE; + + case "Alter": + return AclOperation.ALTER; + + case "Describe": + return AclOperation.DESCRIBE; + + case "ClusterAction": + return AclOperation.CLUSTER_ACTION; + + case "DescribeConfigs": + return AclOperation.DESCRIBE_CONFIGS; + + case "AlterConfigs": + return AclOperation.ALTER_CONFIGS; + + case "IdempotentWrite": + return AclOperation.IDEMPOTENT_WRITE; + + default: + return AclOperation.UNKNOWN; + } + } +} diff --git a/src/main/java/io/aiven/kafka/auth/nameformatters/ResourceTypeNameFormatter.java b/src/main/java/io/aiven/kafka/auth/nameformatters/ResourceTypeNameFormatter.java new file mode 100644 index 0000000..e8c6d2b --- /dev/null +++ b/src/main/java/io/aiven/kafka/auth/nameformatters/ResourceTypeNameFormatter.java @@ -0,0 +1,49 @@ +/* + * Copyright 2022 Aiven Oy https://aiven.io + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.aiven.kafka.auth.nameformatters; + +import org.apache.kafka.common.resource.ResourceType; + +public class ResourceTypeNameFormatter { + public static ResourceType format(final String resourceType) { + switch (resourceType) { + case "Unknown": + return ResourceType.UNKNOWN; + + case "Any": + return ResourceType.ANY; + + case "Topic": + return ResourceType.TOPIC; + + case "Group": + return ResourceType.GROUP; + + case "Cluster": + return ResourceType.CLUSTER; + + case "TransactionalId": + return ResourceType.TRANSACTIONAL_ID; + + case "DelegationToken": + return ResourceType.DELEGATION_TOKEN; + + default: + return ResourceType.UNKNOWN; + } + } +} diff --git a/src/main/java/io/aiven/kafka/auth/nativeacls/AclAivenToNativeConverter.java b/src/main/java/io/aiven/kafka/auth/nativeacls/AclAivenToNativeConverter.java new file mode 100644 index 0000000..d6ba1a6 --- /dev/null +++ b/src/main/java/io/aiven/kafka/auth/nativeacls/AclAivenToNativeConverter.java @@ -0,0 +1,55 @@ +/* + * Copyright 2022 Aiven Oy https://aiven.io + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.aiven.kafka.auth.nativeacls; + +import java.util.ArrayList; +import java.util.List; +import java.util.Objects; + +import org.apache.kafka.common.acl.AccessControlEntry; +import org.apache.kafka.common.acl.AclBinding; +import org.apache.kafka.common.acl.AclPermissionType; + +import io.aiven.kafka.auth.json.AivenAcl; + +public class AclAivenToNativeConverter { + public static Iterable convert(final List aivenAcls) { + final List result = new ArrayList<>(); + + for (final var aclEntry : aivenAcls) { + if (!Objects.equals(aclEntry.principalType, "User")) { + continue; + } + + for (final var operation : AclOperationsParser.parse(aclEntry.operationRe.pattern())) { + List principals = RegexParser.parse(aclEntry.principalRe.pattern()); + if (principals == null) { + principals = List.of(aclEntry.principalRe.pattern()); + } + for (final var principal : principals) { + final var accessControlEntry = new AccessControlEntry( + principal, "*", operation, AclPermissionType.ALLOW); + for (final var resourcePattern : ResourcePatternParser.parse(aclEntry.resourceRe.pattern())) { + result.add(new AclBinding(resourcePattern, accessControlEntry)); + } + } + } + } + + return result; + } +} diff --git a/src/main/java/io/aiven/kafka/auth/nativeacls/AclOperationsParser.java b/src/main/java/io/aiven/kafka/auth/nativeacls/AclOperationsParser.java new file mode 100644 index 0000000..b0c3703 --- /dev/null +++ b/src/main/java/io/aiven/kafka/auth/nativeacls/AclOperationsParser.java @@ -0,0 +1,57 @@ +/* + * Copyright 2022 Aiven Oy https://aiven.io + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.aiven.kafka.auth.nativeacls; + +import java.util.List; +import java.util.stream.Collectors; + +import org.apache.kafka.common.acl.AclOperation; + +import io.aiven.kafka.auth.nameformatters.OperationNameFormatter; + +class AclOperationsParser { + // Visible for test + static Iterable parse(final String operationPattern) { + if (operationPattern == null) { + return List.of(); + } + + if (operationPattern.equals("^.*$") || operationPattern.equals("^(.*)$")) { + return List.of( + AclOperation.READ, + AclOperation.WRITE, + AclOperation.CREATE, + AclOperation.DELETE, + AclOperation.ALTER, + AclOperation.DESCRIBE, + AclOperation.CLUSTER_ACTION, + AclOperation.DESCRIBE_CONFIGS, + AclOperation.ALTER_CONFIGS, + AclOperation.IDEMPOTENT_WRITE + ); + } + + final List parsedOperationList = RegexParser.parse(operationPattern); + if (parsedOperationList == null) { + return List.of(); + } + return parsedOperationList.stream() + .map(OperationNameFormatter::format) + .filter(o -> o != AclOperation.UNKNOWN) + .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 new file mode 100644 index 0000000..2fbb3ed --- /dev/null +++ b/src/main/java/io/aiven/kafka/auth/nativeacls/RegexParser.java @@ -0,0 +1,60 @@ +/* + * Copyright 2022 Aiven Oy https://aiven.io + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.aiven.kafka.auth.nativeacls; + +import java.util.Arrays; +import java.util.List; +import java.util.stream.Collectors; + +class RegexParser { + + // Visible for test + /** + * Parses a regex pattern into a list. + *
+ * For example, ^(AAA|BBB|CCC)$ results in List("AAA", "BBB", "CCC"), + * ^AAA$ results in List("AAA"). Unparsable strings results in {@code null}. + */ + static List parse(String pattern) { + if (pattern == null) { + return null; + } + + // Remove regex pattern prefix and postfix. + if (!pattern.startsWith("^")) { + return null; + } + pattern = pattern.substring(1); + if (pattern.startsWith("(")) { + pattern = pattern.substring(1); + } + + if (!pattern.endsWith("$")) { + return null; + } + pattern = pattern.substring(0, pattern.length() - 1); + if (pattern.endsWith(")")) { + pattern = pattern.substring(0, pattern.length() - 1); + } + + if (pattern.equals("^(.*)$")) { + return null; + } + + return Arrays.stream(pattern.split("\\|")).collect(Collectors.toList()); + } +} diff --git a/src/main/java/io/aiven/kafka/auth/nativeacls/ResourcePatternParser.java b/src/main/java/io/aiven/kafka/auth/nativeacls/ResourcePatternParser.java new file mode 100644 index 0000000..083c2b2 --- /dev/null +++ b/src/main/java/io/aiven/kafka/auth/nativeacls/ResourcePatternParser.java @@ -0,0 +1,109 @@ +/* + * Copyright 2022 Aiven Oy https://aiven.io + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.aiven.kafka.auth.nativeacls; + +import java.util.ArrayList; +import java.util.List; + +import org.apache.kafka.common.resource.PatternType; +import org.apache.kafka.common.resource.ResourcePattern; +import org.apache.kafka.common.resource.ResourceType; + +class ResourcePatternParser { + // Visible for test + static Iterable parse(final String resourcePattern) { + if (resourcePattern == null) { + return List.of(); + } + + if (resourcePattern.equals("^.*$") || resourcePattern.equals("^(.*)$")) { + final var resourcePatternNormalized = resourcePattern.equals("^(.*)$") ? "^.*$" : resourcePattern; + return List.of( + new ResourcePattern(ResourceType.TOPIC, resourcePatternNormalized, PatternType.LITERAL), + new ResourcePattern(ResourceType.GROUP, resourcePatternNormalized, PatternType.LITERAL), + new ResourcePattern(ResourceType.CLUSTER, resourcePatternNormalized, PatternType.LITERAL), + new ResourcePattern(ResourceType.TRANSACTIONAL_ID, resourcePatternNormalized, PatternType.LITERAL), + new ResourcePattern(ResourceType.DELEGATION_TOKEN, resourcePatternNormalized, PatternType.LITERAL) + ); + } + + final String[] parts = resourcePattern.split(":"); + if (parts.length != 2) { + return List.of(); + } + + // Normalize and parse the left part. + final List resourceTypes = parseResourceTypes(parts[0]); + if (resourceTypes == null) { + return List.of(); + } + + final List resources = parseResources(parts[1]); + if (resources == null) { + return List.of(); + } + + final List result = new ArrayList<>(resourceTypes.size() * resources.size()); + for (final ResourceType resourceType : resourceTypes) { + for (final String resource : resources) { + result.add( + new ResourcePattern(resourceType, resource, PatternType.LITERAL) + ); + } + } + return result; + } + + private static List parseResourceTypes(String leftPart) { + if (!leftPart.startsWith("^")) { + return null; + } + if (leftPart.startsWith("^(")) { + leftPart = leftPart.substring(2); + } else { + leftPart = leftPart.substring(1); + } + if (leftPart.endsWith(")")) { + leftPart = leftPart.substring(0, leftPart.length() - 1); + } + leftPart = leftPart.trim(); + if (leftPart.isEmpty()) { + return null; + } + return ResourceTypeParser.parse("^(" + leftPart + ")$"); + } + + private static List parseResources(String rightPart) { + if (!rightPart.endsWith("$")) { + return null; + } + if (rightPart.endsWith(")$")) { + rightPart = rightPart.substring(0, rightPart.length() - 2); + } else { + rightPart = rightPart.substring(0, rightPart.length() - 1); + } + if (rightPart.startsWith("(")) { + rightPart = rightPart.substring(1); + } + rightPart = rightPart.trim(); + if (rightPart.isEmpty()) { + return null; + } + + return RegexParser.parse("^(" + rightPart + ")$"); + } +} diff --git a/src/main/java/io/aiven/kafka/auth/nativeacls/ResourceTypeParser.java b/src/main/java/io/aiven/kafka/auth/nativeacls/ResourceTypeParser.java new file mode 100644 index 0000000..0ae22dd --- /dev/null +++ b/src/main/java/io/aiven/kafka/auth/nativeacls/ResourceTypeParser.java @@ -0,0 +1,53 @@ +/* + * Copyright 2022 Aiven Oy https://aiven.io + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.aiven.kafka.auth.nativeacls; + +import java.util.List; +import java.util.stream.Collectors; + +import org.apache.kafka.common.resource.ResourceType; + +import io.aiven.kafka.auth.nameformatters.ResourceTypeNameFormatter; + +class ResourceTypeParser { + // Visible for test + static List parse(final String resourceTypePattern) { + if (resourceTypePattern == null) { + return List.of(); + } + + if (resourceTypePattern.equals("^.*$") || resourceTypePattern.equals("^(.*)$")) { + return List.of( + ResourceType.TOPIC, + ResourceType.GROUP, + ResourceType.CLUSTER, + ResourceType.TRANSACTIONAL_ID, + ResourceType.DELEGATION_TOKEN + ); + } + + final List parsedResourceTypeList = RegexParser.parse(resourceTypePattern); + if (parsedResourceTypeList == null) { + return List.of(); + } + + return parsedResourceTypeList.stream() + .map(ResourceTypeNameFormatter::format) + .filter(rt -> rt != ResourceType.UNKNOWN) + .collect(Collectors.toList()); + } +} diff --git a/src/test/java/io/aiven/kafka/auth/AivenAclAuthorizerConfigTest.java b/src/test/java/io/aiven/kafka/auth/AivenAclAuthorizerConfigTest.java index cda6a7c..60ed6ef 100644 --- a/src/test/java/io/aiven/kafka/auth/AivenAclAuthorizerConfigTest.java +++ b/src/test/java/io/aiven/kafka/auth/AivenAclAuthorizerConfigTest.java @@ -45,6 +45,7 @@ void correctMinimalConfig() { assertEquals("/test", config.getConfigFile().getAbsolutePath()); assertEquals(NoAuditor.class, config.getAuditor().getClass()); assertTrue(config.logDenials()); + assertTrue(config.listAclsEnabled()); } @Test @@ -55,12 +56,14 @@ void correctFullConfig() { userActivityProps.put("aiven.acl.authorizer.auditor.aggregation.period", "123"); userActivityProps.put("aiven.acl.authorizer.log.denials", "false"); userActivityProps.put("aiven.acl.authorizer.config.refresh.interval", "10"); + userActivityProps.put("aiven.acl.authorizer.list.acls.enabled", "false"); var config = new AivenAclAuthorizerConfig(userActivityProps); assertEquals("/test", config.getConfigFile().getAbsolutePath()); assertEquals(UserActivityAuditor.class, config.getAuditor().getClass()); assertFalse(config.logDenials()); assertEquals(10, config.configRefreshInterval()); + assertFalse(config.listAclsEnabled()); final Map userActivityOpsProps = new HashMap<>(); userActivityOpsProps.put("aiven.acl.authorizer.configuration", "/test"); diff --git a/src/test/java/io/aiven/kafka/auth/AivenAclAuthorizerV2Test.java b/src/test/java/io/aiven/kafka/auth/AivenAclAuthorizerV2Test.java index 6fe36bb..7788b99 100644 --- a/src/test/java/io/aiven/kafka/auth/AivenAclAuthorizerV2Test.java +++ b/src/test/java/io/aiven/kafka/auth/AivenAclAuthorizerV2Test.java @@ -20,12 +20,16 @@ import java.net.InetAddress; import java.nio.file.Files; import java.nio.file.Path; +import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.concurrent.TimeUnit; import org.apache.kafka.common.Endpoint; +import org.apache.kafka.common.acl.AccessControlEntry; +import org.apache.kafka.common.acl.AclBinding; import org.apache.kafka.common.acl.AclOperation; +import org.apache.kafka.common.acl.AclPermissionType; import org.apache.kafka.common.network.ClientInformation; import org.apache.kafka.common.network.ListenerName; import org.apache.kafka.common.protocol.ApiKeys; @@ -33,6 +37,7 @@ import org.apache.kafka.common.requests.RequestHeader; import org.apache.kafka.common.resource.PatternType; import org.apache.kafka.common.resource.ResourcePattern; +import org.apache.kafka.common.resource.ResourceType; import org.apache.kafka.common.security.auth.KafkaPrincipal; import org.apache.kafka.common.security.auth.SecurityProtocol; import org.apache.kafka.server.authorizer.Action; @@ -52,14 +57,14 @@ public class AivenAclAuthorizerV2Test { static final ResourcePattern TOPIC_RESOURCE = new ResourcePattern( - org.apache.kafka.common.resource.ResourceType.TOPIC, - "Target", - PatternType.LITERAL + org.apache.kafka.common.resource.ResourceType.TOPIC, + "Target", + PatternType.LITERAL ); static final ResourcePattern GROUP_RESOURCE = new ResourcePattern( - org.apache.kafka.common.resource.ResourceType.GROUP, - "Target", - PatternType.LITERAL + org.apache.kafka.common.resource.ResourceType.GROUP, + "Target", + PatternType.LITERAL ); static final AclOperation READ_OPERATION = AclOperation.READ; static final AclOperation CREATE_OPERATION = AclOperation.CREATE; @@ -109,8 +114,8 @@ public class AivenAclAuthorizerV2Test { void setUp() { configFilePath = tmpDir.resolve("acl.json"); configs = Map.of( - "aiven.acl.authorizer.configuration", configFilePath.toString(), - "aiven.acl.authorizer.config.refresh.interval", "10"); + "aiven.acl.authorizer.configuration", configFilePath.toString(), + "aiven.acl.authorizer.config.refresh.interval", "10"); auth.configure(configs); } @@ -119,6 +124,50 @@ void tearDown() { auth.close(); } + @Test + public void testAclsMethodWhenListingEnabled() throws IOException { + Files.copy(this.getClass().getResourceAsStream("/test_acls_for_acls_method.json"), configFilePath); + startAuthorizer(); + + assertThat(auth.acls(null)) + .containsExactly( + new AclBinding( + new ResourcePattern(ResourceType.TOPIC, "xxx", PatternType.LITERAL), + new AccessControlEntry("test\\-user", "*", AclOperation.ALTER, AclPermissionType.ALLOW)), + new AclBinding( + new ResourcePattern(ResourceType.TOPIC, "xxx", PatternType.LITERAL), + new AccessControlEntry("test\\-user", "*", AclOperation.ALTER_CONFIGS, AclPermissionType.ALLOW)), + new AclBinding( + new ResourcePattern(ResourceType.TOPIC, "xxx", PatternType.LITERAL), + new AccessControlEntry("test\\-user", "*", AclOperation.DELETE, AclPermissionType.ALLOW)), + new AclBinding( + new ResourcePattern(ResourceType.TOPIC, "xxx", PatternType.LITERAL), + new AccessControlEntry("test\\-user", "*", AclOperation.READ, AclPermissionType.ALLOW)), + new AclBinding( + new ResourcePattern(ResourceType.TOPIC, "xxx", PatternType.LITERAL), + new AccessControlEntry("test\\-user", "*", AclOperation.WRITE, AclPermissionType.ALLOW)), + new AclBinding( + new ResourcePattern(ResourceType.TOPIC, ".*", PatternType.LITERAL), + new AccessControlEntry("test\\-user", "*", AclOperation.DESCRIBE, AclPermissionType.ALLOW)), + new AclBinding( + new ResourcePattern(ResourceType.TOPIC, ".*", PatternType.LITERAL), + new AccessControlEntry("test\\-user", "*", AclOperation.DESCRIBE_CONFIGS, AclPermissionType.ALLOW)) + ); + } + + @Test + public void testAclsMethodWhenListingDisabled() throws IOException { + final var configsUpdated = new HashMap<>(configs); + configsUpdated.put("aiven.acl.authorizer.list.acls.enabled", "false"); + auth.configure(configsUpdated); + + Files.copy(this.getClass().getResourceAsStream("/test_acls_for_acls_method.json"), configFilePath); + startAuthorizer(); + + assertThat(auth.acls(null)) + .isEmpty(); + } + @Test public void testAivenAclAuthorizer() throws IOException, InterruptedException { Files.write(configFilePath, ACL_JSON.getBytes()); diff --git a/src/test/java/io/aiven/kafka/auth/nativeacls/AclAivenToNativeConverterTest.java b/src/test/java/io/aiven/kafka/auth/nativeacls/AclAivenToNativeConverterTest.java new file mode 100644 index 0000000..ba363c6 --- /dev/null +++ b/src/test/java/io/aiven/kafka/auth/nativeacls/AclAivenToNativeConverterTest.java @@ -0,0 +1,99 @@ +/* + * Copyright 2022 Aiven Oy https://aiven.io + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.aiven.kafka.auth.nativeacls; + +import java.util.ArrayList; +import java.util.List; + +import org.apache.kafka.common.acl.AccessControlEntry; +import org.apache.kafka.common.acl.AclBinding; +import org.apache.kafka.common.acl.AclOperation; +import org.apache.kafka.common.acl.AclPermissionType; +import org.apache.kafka.common.resource.PatternType; +import org.apache.kafka.common.resource.ResourcePattern; +import org.apache.kafka.common.resource.ResourceType; + +import io.aiven.kafka.auth.json.AivenAcl; + +import org.junit.jupiter.api.Test; + +import static org.assertj.core.api.Assertions.assertThat; + +public class AclAivenToNativeConverterTest { + @Test + public final void testConvertSimple() { + final var result = AclAivenToNativeConverter.convert( + List.of(new AivenAcl( + "User", + "^(test\\-user)$", + "^(Alter|AlterConfigs|Delete|Read|Write)$", + "^Topic:(xxx)$", + null + )) + ); + final ResourcePattern resourcePattern = new ResourcePattern(ResourceType.TOPIC, "xxx", PatternType.LITERAL); + assertThat(result).containsExactly( + new AclBinding( + resourcePattern, + new AccessControlEntry("test\\-user", "*", AclOperation.ALTER, AclPermissionType.ALLOW)), + new AclBinding( + resourcePattern, + new AccessControlEntry("test\\-user", "*", AclOperation.ALTER_CONFIGS, AclPermissionType.ALLOW)), + new AclBinding( + resourcePattern, + new AccessControlEntry("test\\-user", "*", AclOperation.DELETE, AclPermissionType.ALLOW)), + new AclBinding( + resourcePattern, + new AccessControlEntry("test\\-user", "*", AclOperation.READ, AclPermissionType.ALLOW)), + new AclBinding( + resourcePattern, + new AccessControlEntry("test\\-user", "*", AclOperation.WRITE, AclPermissionType.ALLOW)) + ); + } + + @Test + public final void testSuperadmin() { + final var result = AclAivenToNativeConverter.convert( + List.of(new AivenAcl( + "User", + "^(admin)$", + "^(.*)$", + "^(.*)$", + null + )) + ); + + final List expected = new ArrayList<>(); + final List expectedResourceTypes = List.of( + ResourceType.TOPIC, ResourceType.GROUP, ResourceType.CLUSTER, + ResourceType.TRANSACTIONAL_ID, ResourceType.DELEGATION_TOKEN); + final List expectedAclOperations = List.of( + AclOperation.READ, AclOperation.WRITE, AclOperation.CREATE, + AclOperation.DELETE, AclOperation.ALTER, AclOperation.DESCRIBE, + AclOperation.CLUSTER_ACTION, AclOperation.DESCRIBE_CONFIGS, + AclOperation.ALTER_CONFIGS, AclOperation.IDEMPOTENT_WRITE); + for (final var resourceType : expectedResourceTypes) { + for (final var aclOperation : expectedAclOperations) { + expected.add(new AclBinding( + new ResourcePattern(resourceType, "^.*$", PatternType.LITERAL), + new AccessControlEntry("admin", "*", aclOperation, AclPermissionType.ALLOW)) + ); + } + } + assertThat(result).containsAll(expected); + } +} diff --git a/src/test/java/io/aiven/kafka/auth/nativeacls/AclOperationsParserTest.java b/src/test/java/io/aiven/kafka/auth/nativeacls/AclOperationsParserTest.java new file mode 100644 index 0000000..a999702 --- /dev/null +++ b/src/test/java/io/aiven/kafka/auth/nativeacls/AclOperationsParserTest.java @@ -0,0 +1,112 @@ +/* + * Copyright 2022 Aiven Oy https://aiven.io + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.aiven.kafka.auth.nativeacls; + +import org.apache.kafka.common.acl.AclOperation; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +import static org.assertj.core.api.Assertions.assertThat; + +public class AclOperationsParserTest { + @Test + public final void parseAclOperationsSingle() { + assertThat(AclOperationsParser.parse("^Create$")) + .containsExactly(AclOperation.CREATE); + } + + @Test + public final void parseAclOperationsSingleWithParens() { + assertThat(AclOperationsParser.parse("^(Create)$")) + .containsExactly(AclOperation.CREATE); + } + + @Test + public final void parseAclOperationsMultiple() { + // List all possible here, apart from Unknown. + final String operationPattern = "^(Any|All|Read|Write|Create|Delete|Alter|Describe|" + + "ClusterAction|DescribeConfigs|AlterConfigs|IdempotentWrite)$"; + assertThat(AclOperationsParser.parse(operationPattern)) + .containsExactly( + AclOperation.ANY, + AclOperation.ALL, + AclOperation.READ, + AclOperation.WRITE, + AclOperation.CREATE, + AclOperation.DELETE, + AclOperation.ALTER, + AclOperation.DESCRIBE, + AclOperation.CLUSTER_ACTION, + AclOperation.DESCRIBE_CONFIGS, + AclOperation.ALTER_CONFIGS, + AclOperation.IDEMPOTENT_WRITE + ); + } + + @ParameterizedTest + @ValueSource(strings = {"^(.*)$", "^.*$"}) + public final void parseAclOperationsGlobalWildcard(final String value) { + assertThat(AclOperationsParser.parse(value)) + .containsExactly( + AclOperation.READ, + AclOperation.WRITE, + AclOperation.CREATE, + AclOperation.DELETE, + AclOperation.ALTER, + AclOperation.DESCRIBE, + AclOperation.CLUSTER_ACTION, + AclOperation.DESCRIBE_CONFIGS, + AclOperation.ALTER_CONFIGS, + AclOperation.IDEMPOTENT_WRITE + ); + } + + @Test + public final void parseAclOperationsSingleUnknown() { + assertThat(AclOperationsParser.parse("^(Some)$")) + .isEmpty(); + } + + @Test + public final void parseAclOperationsNull() { + assertThat(AclOperationsParser.parse(null)) + .isEmpty(); + } + + @Test + public final void parseAclOperationsMultipleWithUnknown() { + assertThat(AclOperationsParser.parse("^(Create|Describe|AlterConfigs|Some)$")) + .containsExactly( + AclOperation.CREATE, + AclOperation.DESCRIBE, + AclOperation.ALTER_CONFIGS); + } + + @ParameterizedTest + @ValueSource(strings = { + "^(Create|Delete)", + "(Create|Delete)$", + "^(Create|Delete", + "Create|Delete)$" + }) + public final void parseAclOperationsInvalid(final String pattern) { + assertThat(AclOperationsParser.parse(pattern)) + .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 new file mode 100644 index 0000000..298ec7b --- /dev/null +++ b/src/test/java/io/aiven/kafka/auth/nativeacls/RegexParserTest.java @@ -0,0 +1,55 @@ +/* + * Copyright 2022 Aiven Oy https://aiven.io + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.aiven.kafka.auth.nativeacls; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +import static org.assertj.core.api.Assertions.assertThat; + +public class RegexParserTest { + @Test + public final void parseRegexListSingle() { + assertThat(RegexParser.parse("^AAA$")) + .containsExactly("AAA"); + } + + @Test + public final void parseRegexListSingleWithParens() { + assertThat(RegexParser.parse("^(AAA)$")) + .containsExactly("AAA"); + } + + @Test + public final void parseRegexListMultiple() { + assertThat(RegexParser.parse("^(AAA|BBB|CCC|DDD)$")) + .containsExactly("AAA", "BBB", "CCC", "DDD"); + } + + @ParameterizedTest + @ValueSource(strings = { + "^(AAA|BBB|CCC|DDD)", + "(AAA|BBB|CCC|DDD)$", + "^(AAA|BBB|CCC|DDD", + "AAA|BBB|CCC|DDD)$" + }) + public final void parseRegexListInvalid(final String pattern) { + assertThat(RegexParser.parse(pattern)) + .isNull(); + } +} diff --git a/src/test/java/io/aiven/kafka/auth/nativeacls/ResourcePatternParserTest.java b/src/test/java/io/aiven/kafka/auth/nativeacls/ResourcePatternParserTest.java new file mode 100644 index 0000000..657c190 --- /dev/null +++ b/src/test/java/io/aiven/kafka/auth/nativeacls/ResourcePatternParserTest.java @@ -0,0 +1,120 @@ +/* + * Copyright 2022 Aiven Oy https://aiven.io + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.aiven.kafka.auth.nativeacls; + +import org.apache.kafka.common.resource.PatternType; +import org.apache.kafka.common.resource.ResourcePattern; +import org.apache.kafka.common.resource.ResourceType; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +import static org.assertj.core.api.Assertions.assertThat; + +public class ResourcePatternParserTest { + @ParameterizedTest + @ValueSource(strings = {"^(.*)$", "^.*$"}) + public final void parseResourcePatternGlobalWildcard(final String value) { + assertThat(ResourcePatternParser.parse(value)) + .containsExactly( + new ResourcePattern(ResourceType.TOPIC, "^.*$", PatternType.LITERAL), + new ResourcePattern(ResourceType.GROUP, "^.*$", PatternType.LITERAL), + new ResourcePattern(ResourceType.CLUSTER, "^.*$", PatternType.LITERAL), + new ResourcePattern(ResourceType.TRANSACTIONAL_ID, "^.*$", PatternType.LITERAL), + new ResourcePattern(ResourceType.DELEGATION_TOKEN, "^.*$", PatternType.LITERAL) + ); + } + + @ParameterizedTest + @ValueSource(strings = {"^Cluster:(.*)$", "^Cluster:.*$"}) + public final void parseResourcePatternSingleResourceTypeWildcard(final String value) { + assertThat(ResourcePatternParser.parse(value)) + .containsExactly(new ResourcePattern(ResourceType.CLUSTER, ".*", PatternType.LITERAL)); + } + + @ParameterizedTest + @ValueSource(strings = {"^TransactionalId:(xxx)$", "^TransactionalId:xxx$"}) + public final void parseResourcePatternSingleResourceTypeSingleResource(final String value) { + assertThat(ResourcePatternParser.parse(value)) + .containsExactly(new ResourcePattern(ResourceType.TRANSACTIONAL_ID, "xxx", PatternType.LITERAL)); + } + + @Test + public final void parseResourcePatternSingleResourceTypeMultipleResource() { + assertThat(ResourcePatternParser.parse("^Topic:(xxx|yyy|bac.*xyz)$")) + .containsExactly( + new ResourcePattern(ResourceType.TOPIC, "xxx", PatternType.LITERAL), + new ResourcePattern(ResourceType.TOPIC, "yyy", PatternType.LITERAL), + new ResourcePattern(ResourceType.TOPIC, "bac.*xyz", PatternType.LITERAL) + ); + } + + @Test + public final void parseResourcePatternMultipleResourceTypeWildcard() { + assertThat(ResourcePatternParser.parse("^(Cluster|Topic):(.*)$")) + .containsExactly( + new ResourcePattern(ResourceType.CLUSTER, ".*", PatternType.LITERAL), + new ResourcePattern(ResourceType.TOPIC, ".*", PatternType.LITERAL) + ); + } + + @ParameterizedTest + @ValueSource(strings = {"^(Cluster|Topic):(xxx)$", "^(Cluster|Topic):xxx$"}) + public final void parseResourcePatternMultipleResourceTypeSingleResource(final String value) { + assertThat(ResourcePatternParser.parse(value)) + .containsExactly( + new ResourcePattern(ResourceType.CLUSTER, "xxx", PatternType.LITERAL), + new ResourcePattern(ResourceType.TOPIC, "xxx", PatternType.LITERAL) + ); + } + + @Test + public final void parseResourcePatternMultipleResourceTypeMultipleResource() { + assertThat(ResourcePatternParser.parse("^(Cluster|Topic):(xxx|yyy|bac.*xyz)$")) + .containsExactly( + new ResourcePattern(ResourceType.CLUSTER, "xxx", PatternType.LITERAL), + new ResourcePattern(ResourceType.CLUSTER, "yyy", PatternType.LITERAL), + new ResourcePattern(ResourceType.CLUSTER, "bac.*xyz", PatternType.LITERAL), + new ResourcePattern(ResourceType.TOPIC, "xxx", PatternType.LITERAL), + new ResourcePattern(ResourceType.TOPIC, "yyy", PatternType.LITERAL), + new ResourcePattern(ResourceType.TOPIC, "bac.*xyz", PatternType.LITERAL) + ); + } + + + @ParameterizedTest + @ValueSource(strings = { + "^(AAA|BBB|CCC|DDD)", + "(AAA|BBB|CCC|DDD)$", + "^(AAA|BBB|CCC|DDD", + "AAA|BBB|CCC|DDD)$", + "^Cluster$", + "^Cluster:$", + "^:xxx$", + }) + public final void parseResourcePatternInvalid(final String pattern) { + assertThat(ResourcePatternParser.parse(pattern)) + .isEmpty(); + } + + @Test + public final void parseResourcePatternNull() { + assertThat(ResourcePatternParser.parse(null)) + .isEmpty(); + } +} diff --git a/src/test/java/io/aiven/kafka/auth/nativeacls/ResourceTypeParserTest.java b/src/test/java/io/aiven/kafka/auth/nativeacls/ResourceTypeParserTest.java new file mode 100644 index 0000000..4ecfe44 --- /dev/null +++ b/src/test/java/io/aiven/kafka/auth/nativeacls/ResourceTypeParserTest.java @@ -0,0 +1,96 @@ +/* + * Copyright 2022 Aiven Oy https://aiven.io + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.aiven.kafka.auth.nativeacls; + +import org.apache.kafka.common.resource.ResourceType; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +import static org.assertj.core.api.Assertions.assertThat; + +public class ResourceTypeParserTest { + @Test + public final void parseResourceTypeSingle() { + assertThat(ResourceTypeParser.parse("^Topic$")) + .containsExactly(ResourceType.TOPIC); + } + + @Test + public final void parseResourceTypeSingleWithParens() { + assertThat(ResourceTypeParser.parse("^(Topic)$")) + .containsExactly(ResourceType.TOPIC); + } + + @Test + public final void parseResourceTypeMultiple() { + // List all possible here, apart from Unknown. + assertThat(ResourceTypeParser.parse("^(Any|Topic|Group|Cluster|TransactionalId|DelegationToken)$")) + .containsExactly( + ResourceType.ANY, + ResourceType.TOPIC, + ResourceType.GROUP, + ResourceType.CLUSTER, + ResourceType.TRANSACTIONAL_ID, + ResourceType.DELEGATION_TOKEN + ); + } + + @ParameterizedTest + @ValueSource(strings = {"^(.*)$", "^.*$"}) + public final void parseResourceTypeGlobalWildcard(final String value) { + assertThat(ResourceTypeParser.parse(value)) + .containsExactly( + ResourceType.TOPIC, + ResourceType.GROUP, + ResourceType.CLUSTER, + ResourceType.TRANSACTIONAL_ID, + ResourceType.DELEGATION_TOKEN + ); + } + + @Test + public final void parseResourceTypeUnknown() { + assertThat(ResourceTypeParser.parse("^(Some)$")) + .isEmpty(); + } + + @Test + public final void parseResourceTypeNull() { + assertThat(ResourceTypeParser.parse(null)) + .isEmpty(); + } + + @Test + public final void parseResourceTypeMultipleWithUnknown() { + assertThat(ResourceTypeParser.parse("^(Topic|Cluster|Some)$")) + .containsExactly(ResourceType.TOPIC, ResourceType.CLUSTER); + } + + @ParameterizedTest + @ValueSource(strings = { + "^(Topic|Cluster)", + "(Topic|Cluster)$", + "^(Topic|Cluster", + "Topic|Cluster)$" + }) + public final void parseResourceTypeInvalid(final String pattern) { + assertThat(ResourceTypeParser.parse(pattern)) + .isEmpty(); + } +} diff --git a/src/test/resources/test_acls_for_acls_method.json b/src/test/resources/test_acls_for_acls_method.json new file mode 100644 index 0000000..c557260 --- /dev/null +++ b/src/test/resources/test_acls_for_acls_method.json @@ -0,0 +1,20 @@ +[ + { + "operation": "^(Alter|AlterConfigs|Delete|Read|Write)$", + "principal": "^(test\\-user)$", + "principal_type": "User", + "resource": "^Topic:(xxx)$" + }, + { + "operation": "^(Describe|DescribeConfigs)$", + "principal": "^(test\\-user)$", + "principal_type": "User", + "resource": "^Topic:(.*)$" + }, + { + "operation": "^(.*)$", + "principal": "^(.*)$", + "principal_type": "Service", + "resource": "^(.*)$" + } +] From 985e3a121b2729944ac19f7407cb779f974c064d Mon Sep 17 00:00:00 2001 From: Giuseppe Lillo Date: Mon, 9 Jan 2023 15:18:53 +0100 Subject: [PATCH 2/3] Add filtering and prefixed resource type --- .../kafka/auth/AivenAclAuthorizerV2.java | 10 +- .../io/aiven/kafka/auth/VerdictCache.java | 22 ++-- .../io/aiven/kafka/auth/json/AivenAcl.java | 2 +- .../ResourceTypeNameFormatter.java | 3 - .../nativeacls/AclAivenToNativeConverter.java | 32 ++--- .../auth/nativeacls/AclOperationsParser.java | 7 ++ .../auth/nativeacls/AclPrincipalParser.java | 40 ++++++ .../kafka/auth/nativeacls/RegexParser.java | 32 ++--- .../nativeacls/ResourcePatternParser.java | 93 ++++++-------- .../auth/nativeacls/ResourceTypeParser.java | 11 +- .../kafka/auth/AivenAclAuthorizerV2Test.java | 107 ++++++++++++++-- .../AclAivenToNativeConverterTest.java | 115 ++++++++++++++++-- .../nativeacls/AclPrincipalParserTest.java | 58 +++++++++ .../nativeacls/ResourcePatternParserTest.java | 32 +++-- .../nativeacls/ResourceTypeParserTest.java | 43 +------ .../resources/test_acls_for_acls_method.json | 18 +++ 16 files changed, 435 insertions(+), 190 deletions(-) create mode 100644 src/main/java/io/aiven/kafka/auth/nativeacls/AclPrincipalParser.java create mode 100644 src/test/java/io/aiven/kafka/auth/nativeacls/AclPrincipalParserTest.java diff --git a/src/main/java/io/aiven/kafka/auth/AivenAclAuthorizerV2.java b/src/main/java/io/aiven/kafka/auth/AivenAclAuthorizerV2.java index 334d13c..63b0188 100644 --- a/src/main/java/io/aiven/kafka/auth/AivenAclAuthorizerV2.java +++ b/src/main/java/io/aiven/kafka/auth/AivenAclAuthorizerV2.java @@ -229,7 +229,7 @@ private void logAuthVerdict(final boolean verdict, public final List> createAcls( final AuthorizableRequestContext requestContext, final List aclBindings) { - LOGGER.error("`createAcls` is not implemented"); + LOGGER.warn("`createAcls` is not implemented"); return List.of(); } @@ -237,15 +237,17 @@ public final List> createAcls( public final List> deleteAcls( final AuthorizableRequestContext requestContext, final List aclBindingFilters) { - LOGGER.error("`deleteAcls` is not implemented"); + LOGGER.warn("`deleteAcls` is not implemented"); return List.of(); } @Override public final Iterable acls(final AclBindingFilter filter) { if (this.config.listAclsEnabled()) { - // Filtering is not supported yet. - return AclAivenToNativeConverter.convert(this.cacheReference.get().aclEntries()); + return this.cacheReference.get().aclEntries().stream() + .flatMap(acl -> AclAivenToNativeConverter.convert(acl).stream()) + .filter(filter::matches) + .collect(Collectors.toList()); } else { LOGGER.warn("Listing ACLs is disabled"); return List.of(); diff --git a/src/main/java/io/aiven/kafka/auth/VerdictCache.java b/src/main/java/io/aiven/kafka/auth/VerdictCache.java index aecf205..75b576b 100644 --- a/src/main/java/io/aiven/kafka/auth/VerdictCache.java +++ b/src/main/java/io/aiven/kafka/auth/VerdictCache.java @@ -37,22 +37,18 @@ private VerdictCache(final List aclEntries) { public boolean get(final KafkaPrincipal principal, final String operation, final String resource) { - 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; - } + 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)); } public List aclEntries() { - return aclEntries; + return List.copyOf(aclEntries); } public static VerdictCache create(final List aclEntries) { diff --git a/src/main/java/io/aiven/kafka/auth/json/AivenAcl.java b/src/main/java/io/aiven/kafka/auth/json/AivenAcl.java index a71143f..0b0af88 100644 --- a/src/main/java/io/aiven/kafka/auth/json/AivenAcl.java +++ b/src/main/java/io/aiven/kafka/auth/json/AivenAcl.java @@ -22,7 +22,7 @@ import com.google.gson.annotations.SerializedName; -public class AivenAcl { +public class AivenAcl { @SerializedName("principal_type") public final String principalType; diff --git a/src/main/java/io/aiven/kafka/auth/nameformatters/ResourceTypeNameFormatter.java b/src/main/java/io/aiven/kafka/auth/nameformatters/ResourceTypeNameFormatter.java index e8c6d2b..42971e4 100644 --- a/src/main/java/io/aiven/kafka/auth/nameformatters/ResourceTypeNameFormatter.java +++ b/src/main/java/io/aiven/kafka/auth/nameformatters/ResourceTypeNameFormatter.java @@ -21,9 +21,6 @@ public class ResourceTypeNameFormatter { public static ResourceType format(final String resourceType) { switch (resourceType) { - case "Unknown": - return ResourceType.UNKNOWN; - case "Any": return ResourceType.ANY; 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 d6ba1a6..ed354b1 100644 --- a/src/main/java/io/aiven/kafka/auth/nativeacls/AclAivenToNativeConverter.java +++ b/src/main/java/io/aiven/kafka/auth/nativeacls/AclAivenToNativeConverter.java @@ -27,25 +27,25 @@ import io.aiven.kafka.auth.json.AivenAcl; public class AclAivenToNativeConverter { - public static Iterable convert(final List aivenAcls) { + public static List convert(final AivenAcl aivenAcl) { final List result = new ArrayList<>(); + if (aivenAcl.resourceRe == null) { + return result; + } - for (final var aclEntry : aivenAcls) { - if (!Objects.equals(aclEntry.principalType, "User")) { - continue; - } + if (!Objects.equals(aivenAcl.principalType, "User")) { + return result; + } - for (final var operation : AclOperationsParser.parse(aclEntry.operationRe.pattern())) { - List principals = RegexParser.parse(aclEntry.principalRe.pattern()); - if (principals == null) { - principals = List.of(aclEntry.principalRe.pattern()); - } - for (final var principal : principals) { - final var accessControlEntry = new AccessControlEntry( - principal, "*", operation, AclPermissionType.ALLOW); - for (final var resourcePattern : ResourcePatternParser.parse(aclEntry.resourceRe.pattern())) { - result.add(new AclBinding(resourcePattern, accessControlEntry)); - } + for (final var operation : AclOperationsParser.parse(aivenAcl.operationRe.pattern())) { + final List principals = AclPrincipalParser.parse( + aivenAcl.principalType, aivenAcl.principalRe.pattern() + ); + for (final var principal : principals) { + final var accessControlEntry = new AccessControlEntry( + principal, "*", operation, AclPermissionType.ALLOW); + for (final var resourcePattern : ResourcePatternParser.parse(aivenAcl.resourceRe.pattern())) { + result.add(new AclBinding(resourcePattern, accessControlEntry)); } } } diff --git a/src/main/java/io/aiven/kafka/auth/nativeacls/AclOperationsParser.java b/src/main/java/io/aiven/kafka/auth/nativeacls/AclOperationsParser.java index b0c3703..776ca05 100644 --- a/src/main/java/io/aiven/kafka/auth/nativeacls/AclOperationsParser.java +++ b/src/main/java/io/aiven/kafka/auth/nativeacls/AclOperationsParser.java @@ -23,7 +23,13 @@ import io.aiven.kafka.auth.nameformatters.OperationNameFormatter; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + class AclOperationsParser { + + private static final Logger LOGGER = LoggerFactory.getLogger(AclOperationsParser.class); + // Visible for test static Iterable parse(final String operationPattern) { if (operationPattern == null) { @@ -47,6 +53,7 @@ static Iterable parse(final String operationPattern) { final List parsedOperationList = RegexParser.parse(operationPattern); if (parsedOperationList == null) { + LOGGER.debug("Nothing parsed from operation {}", operationPattern); return List.of(); } return parsedOperationList.stream() diff --git a/src/main/java/io/aiven/kafka/auth/nativeacls/AclPrincipalParser.java b/src/main/java/io/aiven/kafka/auth/nativeacls/AclPrincipalParser.java new file mode 100644 index 0000000..6080617 --- /dev/null +++ b/src/main/java/io/aiven/kafka/auth/nativeacls/AclPrincipalParser.java @@ -0,0 +1,40 @@ +/* + * Copyright 2023 Aiven Oy https://aiven.io + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.aiven.kafka.auth.nativeacls; + +import java.util.List; +import java.util.stream.Collectors; + +class AclPrincipalParser { + // 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); + } + 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 2fbb3ed..5e2a9a1 100644 --- a/src/main/java/io/aiven/kafka/auth/nativeacls/RegexParser.java +++ b/src/main/java/io/aiven/kafka/auth/nativeacls/RegexParser.java @@ -18,10 +18,15 @@ import java.util.Arrays; import java.util.List; +import java.util.function.Predicate; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import java.util.stream.Collectors; class RegexParser { + private static final Pattern PARSER_PATTERN = Pattern.compile("\\^\\(?(.*?)\\)?\\$"); + // Visible for test /** * Parses a regex pattern into a list. @@ -29,32 +34,19 @@ class RegexParser { * For example, ^(AAA|BBB|CCC)$ results in List("AAA", "BBB", "CCC"), * ^AAA$ results in List("AAA"). Unparsable strings results in {@code null}. */ - static List parse(String pattern) { + static List parse(final String pattern) { if (pattern == null) { return null; } - // Remove regex pattern prefix and postfix. - if (!pattern.startsWith("^")) { - return null; - } - pattern = pattern.substring(1); - if (pattern.startsWith("(")) { - pattern = pattern.substring(1); - } - - if (!pattern.endsWith("$")) { - return null; - } - pattern = pattern.substring(0, pattern.length() - 1); - if (pattern.endsWith(")")) { - pattern = pattern.substring(0, pattern.length() - 1); - } - - if (pattern.equals("^(.*)$")) { + final Matcher matcher = PARSER_PATTERN.matcher(pattern); + if (!matcher.find() || matcher.groupCount() != 1) { return null; } - return Arrays.stream(pattern.split("\\|")).collect(Collectors.toList()); + final String group = matcher.group(1); + return Arrays.stream(group.split("\\|")) + .filter(Predicate.not(String::isBlank)) + .collect(Collectors.toList()); } } diff --git a/src/main/java/io/aiven/kafka/auth/nativeacls/ResourcePatternParser.java b/src/main/java/io/aiven/kafka/auth/nativeacls/ResourcePatternParser.java index 083c2b2..9a6e4d8 100644 --- a/src/main/java/io/aiven/kafka/auth/nativeacls/ResourcePatternParser.java +++ b/src/main/java/io/aiven/kafka/auth/nativeacls/ResourcePatternParser.java @@ -17,93 +17,78 @@ package io.aiven.kafka.auth.nativeacls; import java.util.ArrayList; +import java.util.Arrays; import java.util.List; +import java.util.function.Predicate; +import java.util.regex.Matcher; +import java.util.regex.Pattern; +import java.util.stream.Collectors; import org.apache.kafka.common.resource.PatternType; import org.apache.kafka.common.resource.ResourcePattern; import org.apache.kafka.common.resource.ResourceType; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + class ResourcePatternParser { + private static final Pattern PARSER_PATTERN = Pattern.compile("\\^\\(?(.*?)\\)?:\\(?(.*?)\\)?\\$"); + private static final Logger LOGGER = LoggerFactory.getLogger(ResourcePatternParser.class); + // Visible for test + static Iterable parse(final String resourcePattern) { if (resourcePattern == null) { return List.of(); } if (resourcePattern.equals("^.*$") || resourcePattern.equals("^(.*)$")) { - final var resourcePatternNormalized = resourcePattern.equals("^(.*)$") ? "^.*$" : resourcePattern; return List.of( - new ResourcePattern(ResourceType.TOPIC, resourcePatternNormalized, PatternType.LITERAL), - new ResourcePattern(ResourceType.GROUP, resourcePatternNormalized, PatternType.LITERAL), - new ResourcePattern(ResourceType.CLUSTER, resourcePatternNormalized, PatternType.LITERAL), - new ResourcePattern(ResourceType.TRANSACTIONAL_ID, resourcePatternNormalized, PatternType.LITERAL), - new ResourcePattern(ResourceType.DELEGATION_TOKEN, resourcePatternNormalized, PatternType.LITERAL) + new ResourcePattern(ResourceType.TOPIC, "*", PatternType.LITERAL), + new ResourcePattern(ResourceType.GROUP, "*", PatternType.LITERAL), + new ResourcePattern(ResourceType.CLUSTER, "*", PatternType.LITERAL), + new ResourcePattern(ResourceType.TRANSACTIONAL_ID, "*", PatternType.LITERAL), + new ResourcePattern(ResourceType.DELEGATION_TOKEN, "*", PatternType.LITERAL) ); } - final String[] parts = resourcePattern.split(":"); - if (parts.length != 2) { + final Matcher matcher = PARSER_PATTERN.matcher(resourcePattern); + if (!matcher.find() || matcher.groupCount() != 2) { + LOGGER.debug("Nothing parsed from resource pattern {}", resourcePattern); return List.of(); } - - // Normalize and parse the left part. - final List resourceTypes = parseResourceTypes(parts[0]); - if (resourceTypes == null) { + final String resourceTypesGroup = matcher.group(1); + final String resourcesGroup = matcher.group(2); + if (resourceTypesGroup.isBlank() || resourcesGroup.isBlank()) { + LOGGER.debug("Parsed empty resource type or resource for {}", resourcePattern); return List.of(); } + final List resourceTypes = Arrays.stream(resourceTypesGroup.split("\\|")) + .flatMap(type -> ResourceTypeParser.parse(type).stream()) + .collect(Collectors.toList()); - final List resources = parseResources(parts[1]); - if (resources == null) { - return List.of(); - } + final List resources = Arrays.stream(resourcesGroup.split("\\|")) + .filter(Predicate.not(String::isBlank)) + .collect(Collectors.toList()); final List result = new ArrayList<>(resourceTypes.size() * resources.size()); for (final ResourceType resourceType : resourceTypes) { for (final String resource : resources) { - result.add( - new ResourcePattern(resourceType, resource, PatternType.LITERAL) - ); + result.add(createResourcePattern(resourceType, resource)); } } return result; } - private static List parseResourceTypes(String leftPart) { - if (!leftPart.startsWith("^")) { - return null; - } - if (leftPart.startsWith("^(")) { - leftPart = leftPart.substring(2); - } else { - leftPart = leftPart.substring(1); - } - if (leftPart.endsWith(")")) { - leftPart = leftPart.substring(0, leftPart.length() - 1); - } - leftPart = leftPart.trim(); - if (leftPart.isEmpty()) { - return null; - } - return ResourceTypeParser.parse("^(" + leftPart + ")$"); - } - - private static List parseResources(String rightPart) { - if (!rightPart.endsWith("$")) { - return null; - } - if (rightPart.endsWith(")$")) { - rightPart = rightPart.substring(0, rightPart.length() - 2); + private static ResourcePattern createResourcePattern(final ResourceType resourceType, final String resource) { + if (resource.equals(".*") || resource.equals("(.*)")) { + return new ResourcePattern(resourceType, "*", PatternType.LITERAL); + } else if (resource.endsWith("(.*)")) { + return new ResourcePattern( + resourceType, resource.substring(0, resource.length() - 4), PatternType.PREFIXED + ); } else { - rightPart = rightPart.substring(0, rightPart.length() - 1); - } - if (rightPart.startsWith("(")) { - rightPart = rightPart.substring(1); + return new ResourcePattern(resourceType, resource, PatternType.LITERAL); } - rightPart = rightPart.trim(); - if (rightPart.isEmpty()) { - return null; - } - - return RegexParser.parse("^(" + rightPart + ")$"); } } diff --git a/src/main/java/io/aiven/kafka/auth/nativeacls/ResourceTypeParser.java b/src/main/java/io/aiven/kafka/auth/nativeacls/ResourceTypeParser.java index 0ae22dd..46837d9 100644 --- a/src/main/java/io/aiven/kafka/auth/nativeacls/ResourceTypeParser.java +++ b/src/main/java/io/aiven/kafka/auth/nativeacls/ResourceTypeParser.java @@ -17,7 +17,6 @@ package io.aiven.kafka.auth.nativeacls; import java.util.List; -import java.util.stream.Collectors; import org.apache.kafka.common.resource.ResourceType; @@ -40,14 +39,6 @@ static List parse(final String resourceTypePattern) { ); } - final List parsedResourceTypeList = RegexParser.parse(resourceTypePattern); - if (parsedResourceTypeList == null) { - return List.of(); - } - - return parsedResourceTypeList.stream() - .map(ResourceTypeNameFormatter::format) - .filter(rt -> rt != ResourceType.UNKNOWN) - .collect(Collectors.toList()); + return List.of(ResourceTypeNameFormatter.format(resourceTypePattern)); } } diff --git a/src/test/java/io/aiven/kafka/auth/AivenAclAuthorizerV2Test.java b/src/test/java/io/aiven/kafka/auth/AivenAclAuthorizerV2Test.java index 7788b99..bbb88ac 100644 --- a/src/test/java/io/aiven/kafka/auth/AivenAclAuthorizerV2Test.java +++ b/src/test/java/io/aiven/kafka/auth/AivenAclAuthorizerV2Test.java @@ -27,7 +27,9 @@ import org.apache.kafka.common.Endpoint; import org.apache.kafka.common.acl.AccessControlEntry; +import org.apache.kafka.common.acl.AccessControlEntryFilter; import org.apache.kafka.common.acl.AclBinding; +import org.apache.kafka.common.acl.AclBindingFilter; import org.apache.kafka.common.acl.AclOperation; import org.apache.kafka.common.acl.AclPermissionType; import org.apache.kafka.common.network.ClientInformation; @@ -37,6 +39,7 @@ import org.apache.kafka.common.requests.RequestHeader; import org.apache.kafka.common.resource.PatternType; import org.apache.kafka.common.resource.ResourcePattern; +import org.apache.kafka.common.resource.ResourcePatternFilter; import org.apache.kafka.common.resource.ResourceType; import org.apache.kafka.common.security.auth.KafkaPrincipal; import org.apache.kafka.common.security.auth.SecurityProtocol; @@ -73,6 +76,9 @@ public class AivenAclAuthorizerV2Test { + "\"operation\":\"^Read$\",\"resource\":\"^Topic:(.*)$\"}]"; static final String ACL_JSON_NOTYPE = "[{\"principal\":\"^pass$\",\"operation\":\"^Read$\",\"resource\":\"^Topic:(.*)$\"}]"; + static final String ACL_TOPIC_PREFIX_JSON = + "[{\"principal_type\":\"User\",\"principal\":\"^pass$\"," + + "\"operation\":\"^Read$\",\"resource\":\"^Topic:(prefix-(.*))$\"}]"; static final String ACL_JSON_LONG = "[" + "{\"principal_type\":\"User\",\"principal\":\"^pass-0$\"," + "\"operation\":\"^Read$\",\"resource\":\"^Topic:(.*)$\"}," @@ -129,30 +135,102 @@ public void testAclsMethodWhenListingEnabled() throws IOException { Files.copy(this.getClass().getResourceAsStream("/test_acls_for_acls_method.json"), configFilePath); startAuthorizer(); - assertThat(auth.acls(null)) + assertThat(auth.acls(AclBindingFilter.ANY)) .containsExactly( new AclBinding( new ResourcePattern(ResourceType.TOPIC, "xxx", PatternType.LITERAL), - new AccessControlEntry("test\\-user", "*", AclOperation.ALTER, AclPermissionType.ALLOW)), + new AccessControlEntry("User:test\\-user", "*", AclOperation.ALTER, AclPermissionType.ALLOW)), new AclBinding( new ResourcePattern(ResourceType.TOPIC, "xxx", PatternType.LITERAL), - new AccessControlEntry("test\\-user", "*", AclOperation.ALTER_CONFIGS, AclPermissionType.ALLOW)), + new AccessControlEntry("User:test\\-user", "*", AclOperation.ALTER_CONFIGS, AclPermissionType.ALLOW) + ), new AclBinding( new ResourcePattern(ResourceType.TOPIC, "xxx", PatternType.LITERAL), - new AccessControlEntry("test\\-user", "*", AclOperation.DELETE, AclPermissionType.ALLOW)), + new AccessControlEntry("User:test\\-user", "*", AclOperation.DELETE, AclPermissionType.ALLOW)), new AclBinding( new ResourcePattern(ResourceType.TOPIC, "xxx", PatternType.LITERAL), - new AccessControlEntry("test\\-user", "*", AclOperation.READ, AclPermissionType.ALLOW)), + new AccessControlEntry("User:test\\-user", "*", AclOperation.READ, AclPermissionType.ALLOW)), new AclBinding( new ResourcePattern(ResourceType.TOPIC, "xxx", PatternType.LITERAL), - new AccessControlEntry("test\\-user", "*", AclOperation.WRITE, AclPermissionType.ALLOW)), + new AccessControlEntry("User:test\\-user", "*", AclOperation.WRITE, AclPermissionType.ALLOW)), + new AclBinding( + new ResourcePattern(ResourceType.TOPIC, "*", PatternType.LITERAL), + new AccessControlEntry("User:test\\-user", "*", AclOperation.DESCRIBE, AclPermissionType.ALLOW)), + new AclBinding( + new ResourcePattern(ResourceType.TOPIC, "*", PatternType.LITERAL), + new AccessControlEntry( + "User:test\\-user", "*", AclOperation.DESCRIBE_CONFIGS, AclPermissionType.ALLOW + )), new AclBinding( - new ResourcePattern(ResourceType.TOPIC, ".*", PatternType.LITERAL), - new AccessControlEntry("test\\-user", "*", AclOperation.DESCRIBE, AclPermissionType.ALLOW)), + new ResourcePattern(ResourceType.TOPIC, "prefix\\.", PatternType.PREFIXED), + new AccessControlEntry("User:test\\-user", "*", AclOperation.READ, AclPermissionType.ALLOW)), new AclBinding( - new ResourcePattern(ResourceType.TOPIC, ".*", PatternType.LITERAL), - new AccessControlEntry("test\\-user", "*", AclOperation.DESCRIBE_CONFIGS, AclPermissionType.ALLOW)) + new ResourcePattern(ResourceType.TOPIC, "*", PatternType.LITERAL), + new AccessControlEntry("User:test\\-admin", "*", AclOperation.CREATE, AclPermissionType.ALLOW)), + new AclBinding( + new ResourcePattern(ResourceType.TOPIC, "*", PatternType.LITERAL), + new AccessControlEntry("User:test\\-admin", "*", AclOperation.DELETE, AclPermissionType.ALLOW)) ); + + + assertThat(auth.acls(new AclBindingFilter( + new ResourcePatternFilter(ResourceType.TOPIC, "xxx", PatternType.MATCH), + new AccessControlEntryFilter("User:test\\-user", "*", AclOperation.ALTER_CONFIGS, AclPermissionType.ALLOW) + ))).containsExactly( + new AclBinding( + new ResourcePattern(ResourceType.TOPIC, "xxx", PatternType.LITERAL), + new AccessControlEntry("User:test\\-user", "*", AclOperation.ALTER_CONFIGS, AclPermissionType.ALLOW)) + ); + + assertThat(auth.acls(new AclBindingFilter( + new ResourcePatternFilter(ResourceType.TOPIC, "prefix\\.example", PatternType.MATCH), + AccessControlEntryFilter.ANY + ))).containsExactly( + new AclBinding( + new ResourcePattern(ResourceType.TOPIC, "*", PatternType.LITERAL), + new AccessControlEntry("User:test\\-user", "*", AclOperation.DESCRIBE, AclPermissionType.ALLOW)), + new AclBinding( + new ResourcePattern(ResourceType.TOPIC, "*", PatternType.LITERAL), + new AccessControlEntry( + "User:test\\-user", "*", AclOperation.DESCRIBE_CONFIGS, AclPermissionType.ALLOW + )), + new AclBinding( + new ResourcePattern(ResourceType.TOPIC, "prefix\\.", PatternType.PREFIXED), + new AccessControlEntry("User:test\\-user", "*", AclOperation.READ, AclPermissionType.ALLOW)), + new AclBinding( + new ResourcePattern(ResourceType.TOPIC, "*", PatternType.LITERAL), + new AccessControlEntry("User:test\\-admin", "*", AclOperation.CREATE, AclPermissionType.ALLOW)), + new AclBinding( + new ResourcePattern(ResourceType.TOPIC, "*", PatternType.LITERAL), + new AccessControlEntry("User:test\\-admin", "*", AclOperation.DELETE, AclPermissionType.ALLOW)) + ); + + assertThat(auth.acls(new AclBindingFilter( + new ResourcePatternFilter(ResourceType.TOPIC, "xxx", PatternType.MATCH), + new AccessControlEntryFilter("User:test\\-user", "*", AclOperation.ANY, AclPermissionType.ALLOW) + ))).containsExactly( + new AclBinding( + new ResourcePattern(ResourceType.TOPIC, "xxx", PatternType.LITERAL), + new AccessControlEntry("User:test\\-user", "*", AclOperation.ALTER, AclPermissionType.ALLOW)), + new AclBinding( + new ResourcePattern(ResourceType.TOPIC, "xxx", PatternType.LITERAL), + new AccessControlEntry("User:test\\-user", "*", AclOperation.ALTER_CONFIGS, AclPermissionType.ALLOW)), + new AclBinding( + new ResourcePattern(ResourceType.TOPIC, "xxx", PatternType.LITERAL), + new AccessControlEntry("User:test\\-user", "*", AclOperation.DELETE, AclPermissionType.ALLOW)), + new AclBinding( + new ResourcePattern(ResourceType.TOPIC, "xxx", PatternType.LITERAL), + new AccessControlEntry("User:test\\-user", "*", AclOperation.READ, AclPermissionType.ALLOW)), + new AclBinding( + new ResourcePattern(ResourceType.TOPIC, "xxx", PatternType.LITERAL), + new AccessControlEntry("User:test\\-user", "*", AclOperation.WRITE, AclPermissionType.ALLOW)), + new AclBinding( + new ResourcePattern(ResourceType.TOPIC, "*", PatternType.LITERAL), + new AccessControlEntry("User:test\\-user", "*", AclOperation.DESCRIBE, AclPermissionType.ALLOW)), + new AclBinding( + new ResourcePattern(ResourceType.TOPIC, "*", PatternType.LITERAL), + new AccessControlEntry("User:test\\-user", "*", AclOperation.DESCRIBE_CONFIGS, AclPermissionType.ALLOW)) + ); } @Test @@ -200,6 +278,15 @@ public void testAivenAclAuthorizer() throws IOException, InterruptedException { checkSingleAction(requestCtx("User", "pass"), action(READ_OPERATION, TOPIC_RESOURCE), true); checkSingleAction(requestCtx("NonUser", "pass"), action(READ_OPERATION, TOPIC_RESOURCE), true); + Files.write(configFilePath, ACL_TOPIC_PREFIX_JSON.getBytes()); + Thread.sleep(100); + + checkSingleAction(requestCtx("User", "pass"), action(READ_OPERATION, new ResourcePattern( + org.apache.kafka.common.resource.ResourceType.TOPIC, + "prefix-topic", + PatternType.LITERAL + )), true); + Files.write(configFilePath, ACL_JSON_LONG.getBytes()); Thread.sleep(100); diff --git a/src/test/java/io/aiven/kafka/auth/nativeacls/AclAivenToNativeConverterTest.java b/src/test/java/io/aiven/kafka/auth/nativeacls/AclAivenToNativeConverterTest.java index ba363c6..eafec9c 100644 --- a/src/test/java/io/aiven/kafka/auth/nativeacls/AclAivenToNativeConverterTest.java +++ b/src/test/java/io/aiven/kafka/auth/nativeacls/AclAivenToNativeConverterTest.java @@ -37,44 +37,102 @@ public class AclAivenToNativeConverterTest { @Test public final void testConvertSimple() { final var result = AclAivenToNativeConverter.convert( - List.of(new AivenAcl( + new AivenAcl( "User", "^(test\\-user)$", "^(Alter|AlterConfigs|Delete|Read|Write)$", "^Topic:(xxx)$", null - )) + ) ); final ResourcePattern resourcePattern = new ResourcePattern(ResourceType.TOPIC, "xxx", PatternType.LITERAL); assertThat(result).containsExactly( new AclBinding( resourcePattern, - new AccessControlEntry("test\\-user", "*", AclOperation.ALTER, AclPermissionType.ALLOW)), + new AccessControlEntry("User:test\\-user", "*", AclOperation.ALTER, AclPermissionType.ALLOW)), new AclBinding( resourcePattern, - new AccessControlEntry("test\\-user", "*", AclOperation.ALTER_CONFIGS, AclPermissionType.ALLOW)), + new AccessControlEntry("User:test\\-user", "*", AclOperation.ALTER_CONFIGS, AclPermissionType.ALLOW)), new AclBinding( resourcePattern, - new AccessControlEntry("test\\-user", "*", AclOperation.DELETE, AclPermissionType.ALLOW)), + new AccessControlEntry("User:test\\-user", "*", AclOperation.DELETE, AclPermissionType.ALLOW)), new AclBinding( resourcePattern, - new AccessControlEntry("test\\-user", "*", AclOperation.READ, AclPermissionType.ALLOW)), + new AccessControlEntry("User:test\\-user", "*", AclOperation.READ, AclPermissionType.ALLOW)), new AclBinding( resourcePattern, - new AccessControlEntry("test\\-user", "*", AclOperation.WRITE, AclPermissionType.ALLOW)) + new AccessControlEntry("User:test\\-user", "*", AclOperation.WRITE, AclPermissionType.ALLOW)) + ); + } + + @Test + public final void testConvertPrefix() { + final var result = AclAivenToNativeConverter.convert( + new AivenAcl( + "User", + "^(test\\-user)$", + "^Read$", + "^Topic:(topic\\.(.*))$", + null + ) + ); + assertThat(result).containsExactly( + new AclBinding( + new ResourcePattern(ResourceType.TOPIC, "topic\\.", PatternType.PREFIXED), + new AccessControlEntry("User:test\\-user", "*", AclOperation.READ, AclPermissionType.ALLOW) + ) + ); + } + + @Test + public final void testConvertMultiplePrefixes() { + final var result = AclAivenToNativeConverter.convert( + new AivenAcl( + "User", + "^(test\\-user)$", + "^(Delete|Read|Write)$", + "^Topic:(topic\\.(.*)|prefix\\-(.*))$", + null + ) + ); + assertThat(result).containsExactly( + new AclBinding( + new ResourcePattern(ResourceType.TOPIC, "topic\\.", PatternType.PREFIXED), + new AccessControlEntry("User:test\\-user", "*", AclOperation.DELETE, AclPermissionType.ALLOW) + ), + new AclBinding( + new ResourcePattern(ResourceType.TOPIC, "prefix\\-", PatternType.PREFIXED), + new AccessControlEntry("User:test\\-user", "*", AclOperation.DELETE, AclPermissionType.ALLOW) + ), + new AclBinding( + new ResourcePattern(ResourceType.TOPIC, "topic\\.", PatternType.PREFIXED), + new AccessControlEntry("User:test\\-user", "*", AclOperation.READ, AclPermissionType.ALLOW) + ), + new AclBinding( + new ResourcePattern(ResourceType.TOPIC, "prefix\\-", PatternType.PREFIXED), + new AccessControlEntry("User:test\\-user", "*", AclOperation.READ, AclPermissionType.ALLOW) + ), + new AclBinding( + new ResourcePattern(ResourceType.TOPIC, "topic\\.", PatternType.PREFIXED), + new AccessControlEntry("User:test\\-user", "*", AclOperation.WRITE, AclPermissionType.ALLOW) + ), + new AclBinding( + new ResourcePattern(ResourceType.TOPIC, "prefix\\-", PatternType.PREFIXED), + new AccessControlEntry("User:test\\-user", "*", AclOperation.WRITE, AclPermissionType.ALLOW) + ) ); } @Test public final void testSuperadmin() { final var result = AclAivenToNativeConverter.convert( - List.of(new AivenAcl( + new AivenAcl( "User", "^(admin)$", "^(.*)$", "^(.*)$", null - )) + ) ); final List expected = new ArrayList<>(); @@ -89,11 +147,46 @@ public final void testSuperadmin() { for (final var resourceType : expectedResourceTypes) { for (final var aclOperation : expectedAclOperations) { expected.add(new AclBinding( - new ResourcePattern(resourceType, "^.*$", PatternType.LITERAL), - new AccessControlEntry("admin", "*", aclOperation, AclPermissionType.ALLOW)) + new ResourcePattern(resourceType, "*", PatternType.LITERAL), + new AccessControlEntry("User:admin", "*", aclOperation, AclPermissionType.ALLOW)) ); } } assertThat(result).containsAll(expected); } + + @Test + public final void testAllUsers() { + final var result = AclAivenToNativeConverter.convert( + new AivenAcl( + "User", + "^(.*)$", + "^Read$", + "^Topic:(xxx)$", + null + ) + ); + + assertThat(result).containsExactly( + new AclBinding( + new ResourcePattern(ResourceType.TOPIC, "xxx", PatternType.LITERAL), + new AccessControlEntry("User:*", "*", AclOperation.READ, AclPermissionType.ALLOW) + ) + ); + } + + @Test + public final void testNoUserPrincipalType() { + final var result = AclAivenToNativeConverter.convert( + new AivenAcl( + "Group", + "^example$", + "^Read$", + "^Topic:(xxx)$", + null + ) + ); + + assertThat(result).isEmpty(); + } } diff --git a/src/test/java/io/aiven/kafka/auth/nativeacls/AclPrincipalParserTest.java b/src/test/java/io/aiven/kafka/auth/nativeacls/AclPrincipalParserTest.java new file mode 100644 index 0000000..acfcc3d --- /dev/null +++ b/src/test/java/io/aiven/kafka/auth/nativeacls/AclPrincipalParserTest.java @@ -0,0 +1,58 @@ +/* + * Copyright 2023 Aiven Oy https://aiven.io + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.aiven.kafka.auth.nativeacls; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +import static org.assertj.core.api.Assertions.assertThat; + +public class AclPrincipalParserTest { + + @Test + public final void parseSinglePrincipal() { + assertThat(AclPrincipalParser.parse("User", "^username$")) + .containsExactly("User:username"); + } + + @ParameterizedTest + @ValueSource(strings = {"(.*)", "^(username|(.*))$"}) + public final void parseWildcardPrincipal(final String value) { + assertThat(AclPrincipalParser.parse("User", value)) + .containsExactly("User:*"); + } + + @Test + public final void parseMultipleUsers() { + assertThat(AclPrincipalParser.parse("User", "^(user1|user2)$")) + .containsExactly("User:user1", "User:user2"); + } + + @Test + public final void parseNullPrincipal() { + assertThat(AclPrincipalParser.parse("User", null)) + .isEmpty(); + } + + @Test + public final void parseNullPrincipalType() { + assertThat(AclPrincipalParser.parse(null, "^username$")) + .isEmpty(); + } + +} diff --git a/src/test/java/io/aiven/kafka/auth/nativeacls/ResourcePatternParserTest.java b/src/test/java/io/aiven/kafka/auth/nativeacls/ResourcePatternParserTest.java index 657c190..061955b 100644 --- a/src/test/java/io/aiven/kafka/auth/nativeacls/ResourcePatternParserTest.java +++ b/src/test/java/io/aiven/kafka/auth/nativeacls/ResourcePatternParserTest.java @@ -32,11 +32,11 @@ public class ResourcePatternParserTest { public final void parseResourcePatternGlobalWildcard(final String value) { assertThat(ResourcePatternParser.parse(value)) .containsExactly( - new ResourcePattern(ResourceType.TOPIC, "^.*$", PatternType.LITERAL), - new ResourcePattern(ResourceType.GROUP, "^.*$", PatternType.LITERAL), - new ResourcePattern(ResourceType.CLUSTER, "^.*$", PatternType.LITERAL), - new ResourcePattern(ResourceType.TRANSACTIONAL_ID, "^.*$", PatternType.LITERAL), - new ResourcePattern(ResourceType.DELEGATION_TOKEN, "^.*$", PatternType.LITERAL) + new ResourcePattern(ResourceType.TOPIC, "*", PatternType.LITERAL), + new ResourcePattern(ResourceType.GROUP, "*", PatternType.LITERAL), + new ResourcePattern(ResourceType.CLUSTER, "*", PatternType.LITERAL), + new ResourcePattern(ResourceType.TRANSACTIONAL_ID, "*", PatternType.LITERAL), + new ResourcePattern(ResourceType.DELEGATION_TOKEN, "*", PatternType.LITERAL) ); } @@ -44,7 +44,7 @@ public final void parseResourcePatternGlobalWildcard(final String value) { @ValueSource(strings = {"^Cluster:(.*)$", "^Cluster:.*$"}) public final void parseResourcePatternSingleResourceTypeWildcard(final String value) { assertThat(ResourcePatternParser.parse(value)) - .containsExactly(new ResourcePattern(ResourceType.CLUSTER, ".*", PatternType.LITERAL)); + .containsExactly(new ResourcePattern(ResourceType.CLUSTER, "*", PatternType.LITERAL)); } @ParameterizedTest @@ -68,8 +68,8 @@ public final void parseResourcePatternSingleResourceTypeMultipleResource() { public final void parseResourcePatternMultipleResourceTypeWildcard() { assertThat(ResourcePatternParser.parse("^(Cluster|Topic):(.*)$")) .containsExactly( - new ResourcePattern(ResourceType.CLUSTER, ".*", PatternType.LITERAL), - new ResourcePattern(ResourceType.TOPIC, ".*", PatternType.LITERAL) + new ResourcePattern(ResourceType.CLUSTER, "*", PatternType.LITERAL), + new ResourcePattern(ResourceType.TOPIC, "*", PatternType.LITERAL) ); } @@ -96,6 +96,22 @@ public final void parseResourcePatternMultipleResourceTypeMultipleResource() { ); } + @Test + public final void parsePrefixResource() { + assertThat(ResourcePatternParser.parse("^Topic:(topic.(.*))$")) + .containsExactly( + new ResourcePattern(ResourceType.TOPIC, "topic.", PatternType.PREFIXED) + ); + } + + @Test + public final void parseMultiplePrefixResource() { + assertThat(ResourcePatternParser.parse("^Topic:(topic1.(.*)|topic2.(.*))$")) + .containsExactly( + new ResourcePattern(ResourceType.TOPIC, "topic1.", PatternType.PREFIXED), + new ResourcePattern(ResourceType.TOPIC, "topic2.", PatternType.PREFIXED) + ); + } @ParameterizedTest @ValueSource(strings = { diff --git a/src/test/java/io/aiven/kafka/auth/nativeacls/ResourceTypeParserTest.java b/src/test/java/io/aiven/kafka/auth/nativeacls/ResourceTypeParserTest.java index 4ecfe44..89fa07c 100644 --- a/src/test/java/io/aiven/kafka/auth/nativeacls/ResourceTypeParserTest.java +++ b/src/test/java/io/aiven/kafka/auth/nativeacls/ResourceTypeParserTest.java @@ -27,30 +27,10 @@ public class ResourceTypeParserTest { @Test public final void parseResourceTypeSingle() { - assertThat(ResourceTypeParser.parse("^Topic$")) + assertThat(ResourceTypeParser.parse("Topic")) .containsExactly(ResourceType.TOPIC); } - @Test - public final void parseResourceTypeSingleWithParens() { - assertThat(ResourceTypeParser.parse("^(Topic)$")) - .containsExactly(ResourceType.TOPIC); - } - - @Test - public final void parseResourceTypeMultiple() { - // List all possible here, apart from Unknown. - assertThat(ResourceTypeParser.parse("^(Any|Topic|Group|Cluster|TransactionalId|DelegationToken)$")) - .containsExactly( - ResourceType.ANY, - ResourceType.TOPIC, - ResourceType.GROUP, - ResourceType.CLUSTER, - ResourceType.TRANSACTIONAL_ID, - ResourceType.DELEGATION_TOKEN - ); - } - @ParameterizedTest @ValueSource(strings = {"^(.*)$", "^.*$"}) public final void parseResourceTypeGlobalWildcard(final String value) { @@ -66,8 +46,8 @@ public final void parseResourceTypeGlobalWildcard(final String value) { @Test public final void parseResourceTypeUnknown() { - assertThat(ResourceTypeParser.parse("^(Some)$")) - .isEmpty(); + assertThat(ResourceTypeParser.parse("Some")) + .containsExactly(ResourceType.UNKNOWN); } @Test @@ -76,21 +56,4 @@ public final void parseResourceTypeNull() { .isEmpty(); } - @Test - public final void parseResourceTypeMultipleWithUnknown() { - assertThat(ResourceTypeParser.parse("^(Topic|Cluster|Some)$")) - .containsExactly(ResourceType.TOPIC, ResourceType.CLUSTER); - } - - @ParameterizedTest - @ValueSource(strings = { - "^(Topic|Cluster)", - "(Topic|Cluster)$", - "^(Topic|Cluster", - "Topic|Cluster)$" - }) - public final void parseResourceTypeInvalid(final String pattern) { - assertThat(ResourceTypeParser.parse(pattern)) - .isEmpty(); - } } diff --git a/src/test/resources/test_acls_for_acls_method.json b/src/test/resources/test_acls_for_acls_method.json index c557260..0381bce 100644 --- a/src/test/resources/test_acls_for_acls_method.json +++ b/src/test/resources/test_acls_for_acls_method.json @@ -11,6 +11,24 @@ "principal_type": "User", "resource": "^Topic:(.*)$" }, + { + "operation": "^Read$", + "principal": "^(test\\-user)$", + "principal_type": "User", + "resource": "^Topic:(prefix\\.(.*))$" + }, + { + "operation": "^Create$", + "principal": "^(test\\-admin)$", + "principal_type": "User", + "resource": "^Topic:(.*)$" + }, + { + "operation": "^Delete$", + "principal": "^(test\\-admin)$", + "principal_type": "User", + "resource": "^Topic:(.*)$" + }, { "operation": "^(.*)$", "principal": "^(.*)$", From cbbdbfa78f3230a5448ffa92ccf8ce98de3fba9c Mon Sep 17 00:00:00 2001 From: Giuseppe Lillo Date: Thu, 16 Feb 2023 12:55:47 +0100 Subject: [PATCH 3/3] 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)$"))