From 40bcae5f6523de55c5ea5ead085c7ca5f6b977bc Mon Sep 17 00:00:00 2001 From: Tommi Vainikainen Date: Wed, 13 Nov 2024 10:58:19 +0200 Subject: [PATCH] Support implicit describe operations This implements also implied allow describe + allow describe configs feature as defined by org.apache.kafka.common.acl.AclOperation class. This mode works only if operations are defined in new mode as list of strings instead of regular expression. Some of the existing tests have been changed format to ensure backward compatibility by having tests using both formats. To support all current Kafka operations, Kafka dependency is bumped to 3.6. --- README.md | 11 +- build.gradle | 2 +- .../kafka/auth/AivenAclAuthorizerV2.java | 3 +- .../io/aiven/kafka/auth/VerdictCache.java | 3 +- .../kafka/auth/json/AclOperationType.java | 40 ++++++ .../io/aiven/kafka/auth/json/AivenAcl.java | 94 +++++++++++++- .../auth/json/reader/AbstractJsonReader.java | 17 +++ .../nativeacls/AclAivenToNativeConverter.java | 8 +- .../kafka/auth/AivenAclAuthorizerV2Test.java | 117 ++++++++++++++++++ .../aiven/kafka/auth/json/AivenAclTest.java | 44 +++---- .../auth/json/reader/AclJsonReaderTest.java | 20 ++- src/test/resources/acl_operations.json | 70 +++++++++++ src/test/resources/acl_wrong_operations.json | 8 ++ src/test/resources/acls_deny.json | 2 +- src/test/resources/acls_full.json | 12 +- src/test/resources/acls_host_match.json | 4 +- src/test/resources/acls_no_type.json | 2 +- .../acls_resource_literal_match.json | 4 +- .../resources/acls_resource_prefix_match.json | 2 +- 19 files changed, 412 insertions(+), 51 deletions(-) create mode 100644 src/main/java/io/aiven/kafka/auth/json/AclOperationType.java create mode 100644 src/test/resources/acl_operations.json create mode 100644 src/test/resources/acl_wrong_operations.json diff --git a/README.md b/README.md index 48ed00c..e624db5 100644 --- a/README.md +++ b/README.md @@ -7,7 +7,7 @@ Config file is watched for modifications and reloaded as necessary. ### AivenAclEntry -Class implementing a single ACL entry verification. Principal, operation and +Class implementing a single ACL entry verification. Principal and resource are expressed as regular expressions. Alternatively to straight regular expression for resource, AivenAclEntry can @@ -18,6 +18,12 @@ avoid encoding separate rules for each project. Literal and prefixed matchers work as defined in the Apache Kafka documentation. Only one resource matcher can be specified per acl. +Operations can be expressed as a list of operation names, or in deprecated mode +as regular expression in `operation` field. If both are defined, `operations` +takes precedence. For operations listed with operation names, also implicit Decribe +is supported if Read, Write, Alter, or Delete is allowed, and implicit +DescribeConfigs if AlterConfigs is allowed. + Permission type allows to define the verification result in case of an ACL match. By default, the permission type is `ALLOW`. @@ -27,7 +33,7 @@ A specific ACL entry can be hidden from public listing by setting hidden flag. [ { - "operation": "^(.*)$", + "operations": ["All"], "principal": ( "^CN=(?[a-z0-9-]+),OU=(?n[0-9]+)," "O=00000000-0000-a000-1000-(500000000005|a00000000001|b00000000001|d00000000001),ST=vm$" @@ -38,6 +44,7 @@ A specific ACL entry can be hidden from public listing by setting hidden flag. "hidden": true }, { + "operations": ["Describe", "DescribeConfigs", "Read", "Write"], "operation": "^(Describe|DescribeConfigs|Read|Write)$", "principal": "^CN=(?[a-z0-9-]+),OU=(?n[0-9]+),O=(?[a-f0-9-]+),ST=vm$", "principal_type": "Prune", diff --git a/build.gradle b/build.gradle index d60e7b8..0fedc9d 100644 --- a/build.gradle +++ b/build.gradle @@ -74,7 +74,7 @@ jmh { } ext { - kafkaVersion = "2.8.0" + kafkaVersion = "3.6.0" mockitoVersion = "5.14.2" } diff --git a/src/main/java/io/aiven/kafka/auth/AivenAclAuthorizerV2.java b/src/main/java/io/aiven/kafka/auth/AivenAclAuthorizerV2.java index 7a50433..b74cfa6 100644 --- a/src/main/java/io/aiven/kafka/auth/AivenAclAuthorizerV2.java +++ b/src/main/java/io/aiven/kafka/auth/AivenAclAuthorizerV2.java @@ -59,7 +59,6 @@ import io.aiven.kafka.auth.json.AivenAcl; import io.aiven.kafka.auth.json.reader.AclJsonReader; 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; @@ -195,7 +194,7 @@ public final List authorize(final AuthorizableRequestContex final String host = requestContext.clientAddress().getHostAddress(); final boolean verdict = cacheReference.get().get(principal, host, - LegacyOperationNameFormatter.format(operation), + operation, resourceToCheck); final var authResult = verdict ? AuthorizationResult.ALLOWED : AuthorizationResult.DENIED; diff --git a/src/main/java/io/aiven/kafka/auth/VerdictCache.java b/src/main/java/io/aiven/kafka/auth/VerdictCache.java index 3009ffa..b4057c0 100644 --- a/src/main/java/io/aiven/kafka/auth/VerdictCache.java +++ b/src/main/java/io/aiven/kafka/auth/VerdictCache.java @@ -26,6 +26,7 @@ import java.util.stream.Collectors; import java.util.stream.Stream; +import org.apache.kafka.common.acl.AclOperation; import org.apache.kafka.common.security.auth.KafkaPrincipal; import io.aiven.kafka.auth.json.AclPermissionType; @@ -44,7 +45,7 @@ private VerdictCache(@Nonnull final List denyAclEntries, @Nonnull fina public boolean get( final KafkaPrincipal principal, final String host, - final String operation, + final AclOperation operation, final String resource ) { final String principalType = principal.getPrincipalType(); diff --git a/src/main/java/io/aiven/kafka/auth/json/AclOperationType.java b/src/main/java/io/aiven/kafka/auth/json/AclOperationType.java new file mode 100644 index 0000000..346c967 --- /dev/null +++ b/src/main/java/io/aiven/kafka/auth/json/AclOperationType.java @@ -0,0 +1,40 @@ +/* + * Copyright 2024 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.json; + +public enum AclOperationType { + Unknown(org.apache.kafka.common.acl.AclOperation.UNKNOWN), + All(org.apache.kafka.common.acl.AclOperation.ALL), + Read(org.apache.kafka.common.acl.AclOperation.READ), + Write(org.apache.kafka.common.acl.AclOperation.WRITE), + Create1(org.apache.kafka.common.acl.AclOperation.CREATE), + Delete(org.apache.kafka.common.acl.AclOperation.DELETE), + Alter(org.apache.kafka.common.acl.AclOperation.ALTER), + Describe(org.apache.kafka.common.acl.AclOperation.DESCRIBE), + ClusterAction(org.apache.kafka.common.acl.AclOperation.CLUSTER_ACTION), + DescribeConfigs(org.apache.kafka.common.acl.AclOperation.DESCRIBE_CONFIGS), + AlterConfigs(org.apache.kafka.common.acl.AclOperation.ALTER_CONFIGS), + IdempotentWrite(org.apache.kafka.common.acl.AclOperation.IDEMPOTENT_WRITE), + CreateTokens(org.apache.kafka.common.acl.AclOperation.CREATE_TOKENS), + DescribeTokens(org.apache.kafka.common.acl.AclOperation.DESCRIBE_TOKENS); + + public final org.apache.kafka.common.acl.AclOperation nativeType; + + AclOperationType(final org.apache.kafka.common.acl.AclOperation nativeType) { + this.nativeType = nativeType; + } +} 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 49d9435..cf3db8e 100644 --- a/src/main/java/io/aiven/kafka/auth/json/AivenAcl.java +++ b/src/main/java/io/aiven/kafka/auth/json/AivenAcl.java @@ -16,10 +16,17 @@ package io.aiven.kafka.auth.json; +import java.util.Collections; +import java.util.EnumSet; +import java.util.List; import java.util.Objects; +import java.util.Set; import java.util.regex.Matcher; import java.util.regex.Pattern; +import org.apache.kafka.common.acl.AclOperation; + +import io.aiven.kafka.auth.nameformatters.LegacyOperationNameFormatter; import io.aiven.kafka.auth.utils.ResourceLiteralWildcardMatcher; import com.google.gson.annotations.SerializedName; @@ -37,6 +44,8 @@ public class AivenAcl { @SerializedName("operation") public final Pattern operationRe; + public final List operations; + @SerializedName("resource") public final Pattern resourceRe; @@ -73,6 +82,32 @@ public AivenAcl( this.principalRe = Pattern.compile(principal); this.hostMatcher = host; this.operationRe = Pattern.compile(operation); + this.operations = null; + this.resourceRe = Objects.nonNull(resource) ? Pattern.compile(resource) : null; + this.resourceRePattern = resourcePattern; + this.resourceLiteral = resourceLiteral; + this.resourcePrefix = resourcePrefix; + this.permissionType = Objects.requireNonNullElse(permissionType, AclPermissionType.ALLOW); + this.hidden = hidden; + } + + public AivenAcl( + final String principalType, + final String principal, + final String host, + final List operations, + final String resource, + final String resourcePattern, + final String resourceLiteral, + final String resourcePrefix, + final AclPermissionType permissionType, + final boolean hidden + ) { + this.principalType = principalType; + this.principalRe = Pattern.compile(principal); + this.hostMatcher = host; + this.operationRe = null; + this.operations = operations; this.resourceRe = Objects.nonNull(resource) ? Pattern.compile(resource) : null; this.resourceRePattern = resourcePattern; this.resourceLiteral = resourceLiteral; @@ -101,12 +136,63 @@ public String getHostMatcher() { public Boolean match(final String principalType, final String principal, final String host, - final String operation, + final AclOperation operation, final String resource) { if (this.principalType == null || this.principalType.equals(principalType)) { final Matcher mp = this.principalRe.matcher(principal); - final Matcher mo = this.operationRe.matcher(operation); - return mp.find() && mo.find() && this.hostMatch(host) && this.resourceMatch(resource, mp); + return mp.find() && this.matchOperation(operation) && this.hostMatch(host) + && this.resourceMatch(resource, mp); + } + return false; + } + + /* Some operations implicitly allow describe operation, but with allow rules only */ + private static final Set IMPLIES_DESCRIBE = Collections.unmodifiableSet( + EnumSet.of(AclOperation.DESCRIBE, AclOperation.READ, AclOperation.WRITE, + AclOperation.DELETE, AclOperation.ALTER)); + + private static final Set IMPLIES_DESCRIBE_CONFIGS = Collections.unmodifiableSet( + EnumSet.of(AclOperation.DESCRIBE_CONFIGS, AclOperation.ALTER_CONFIGS)); + + + private boolean matchOperation(final AclOperation operation) { + if (this.operations != null) { + for (final var mo : this.operations) { + if (matchSingleOperation(operation, mo)) { + return true; + } + } + return false; + } else { + final Matcher mo = this.operationRe.matcher(LegacyOperationNameFormatter.format(operation)); + return mo.find(); + } + } + + private boolean matchSingleOperation(final AclOperation operation, final AclOperationType mo) { + if (mo.nativeType == AclOperation.ALL) { + return true; + } else if (getPermissionType() == AclPermissionType.ALLOW) { + switch (operation) { + case DESCRIBE: + if (IMPLIES_DESCRIBE.contains(mo.nativeType)) { + return true; + } + break; + case DESCRIBE_CONFIGS: + if (IMPLIES_DESCRIBE_CONFIGS.contains(mo.nativeType)) { + return true; + } + break; + default: + if (operation == mo.nativeType) { + return true; + } + } + } else { + if (operation == mo.nativeType) { + return true; + } } return false; } @@ -177,7 +263,7 @@ private boolean comparePattern(final Pattern p1, final Pattern p2) { @Override public int hashCode() { return Objects.hash( - principalType, principalRe, hostMatcher, operationRe, resourceRe, + principalType, principalRe, hostMatcher, operationRe, operations, resourceRe, resourceRePattern, resourceLiteral, resourcePrefix, getPermissionType() ); } diff --git a/src/main/java/io/aiven/kafka/auth/json/reader/AbstractJsonReader.java b/src/main/java/io/aiven/kafka/auth/json/reader/AbstractJsonReader.java index 2a949b5..8141608 100644 --- a/src/main/java/io/aiven/kafka/auth/json/reader/AbstractJsonReader.java +++ b/src/main/java/io/aiven/kafka/auth/json/reader/AbstractJsonReader.java @@ -21,6 +21,7 @@ import java.util.regex.Pattern; import java.util.regex.PatternSyntaxException; +import io.aiven.kafka.auth.json.AclOperationType; import io.aiven.kafka.auth.json.AclPermissionType; import com.google.gson.GsonBuilder; @@ -37,6 +38,7 @@ public abstract class AbstractJsonReader implements JsonReader { protected final GsonBuilder gsonBuilder = new GsonBuilder() .registerTypeAdapter(Pattern.class, new RegexpJsonDeserializer()) + .registerTypeAdapter(AclOperationType.class, new AclOperationTypeDeserializer()) .registerTypeAdapter(AclPermissionType.class, new AclPermissionTypeDeserializer()); protected AbstractJsonReader(final Path configFile) { @@ -72,4 +74,19 @@ public AclPermissionType deserialize(final JsonElement jsonElement, } } + protected static class AclOperationTypeDeserializer implements JsonDeserializer { + @Override + public AclOperationType deserialize(final JsonElement jsonElement, + final Type type, + final JsonDeserializationContext ctx) throws JsonParseException { + try { + if (jsonElement.isJsonNull()) { + return AclOperationType.Unknown; + } + return AclOperationType.valueOf(jsonElement.getAsString()); + } catch (final IllegalArgumentException e) { + throw new JsonParseException("Cannot deserialize operation type", e); + } + } + } } 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 8d5fe98..71473d4 100644 --- a/src/main/java/io/aiven/kafka/auth/nativeacls/AclAivenToNativeConverter.java +++ b/src/main/java/io/aiven/kafka/auth/nativeacls/AclAivenToNativeConverter.java @@ -19,9 +19,11 @@ import java.util.ArrayList; import java.util.List; import java.util.Objects; +import java.util.stream.Collectors; import org.apache.kafka.common.acl.AccessControlEntry; import org.apache.kafka.common.acl.AclBinding; +import org.apache.kafka.common.acl.AclOperation; import io.aiven.kafka.auth.json.AivenAcl; @@ -36,7 +38,11 @@ public static List convert(final AivenAcl aivenAcl) { return result; } - for (final var operation : AclOperationsParser.parse(aivenAcl.operationRe.pattern())) { + final Iterable operations = aivenAcl.operations != null + ? aivenAcl.operations.stream().map(o -> o.nativeType).collect(Collectors.toList()) + : AclOperationsParser.parse(aivenAcl.operationRe.pattern()); + + for (final var operation : operations) { final List principals = AclPrincipalFormatter.parse( aivenAcl.principalType, aivenAcl.principalRe.pattern() ); diff --git a/src/test/java/io/aiven/kafka/auth/AivenAclAuthorizerV2Test.java b/src/test/java/io/aiven/kafka/auth/AivenAclAuthorizerV2Test.java index 5929f8d..41f5bb4 100644 --- a/src/test/java/io/aiven/kafka/auth/AivenAclAuthorizerV2Test.java +++ b/src/test/java/io/aiven/kafka/auth/AivenAclAuthorizerV2Test.java @@ -459,11 +459,128 @@ public void testResourcePrefixMatcher() throws IOException { checkSingleAction(requestCtx("User", "user2"), action(READ_OPERATION, topic("groupB.topic")), false); } + @Test + public void testAclOperations() throws IOException { + Files.copy(this.getClass().getResourceAsStream("/acl_operations.json"), configFilePath); + startAuthorizer(); + + assertThat(auth.acls(AclBindingFilter.ANY)) + .containsExactly( + new AclBinding( + new ResourcePattern(ResourceType.TOPIC, "implicit_alterconfigs2", PatternType.LITERAL), + new AccessControlEntry("User:implicit4", "*", AclOperation.ALTER_CONFIGS, AclPermissionType.DENY)), + new AclBinding( + new ResourcePattern(ResourceType.TOPIC, "topic1", PatternType.LITERAL), + new AccessControlEntry("User:re_1", "*", AclOperation.READ, AclPermissionType.ALLOW)), + new AclBinding( + new ResourcePattern(ResourceType.TOPIC, "topic1", PatternType.LITERAL), + new AccessControlEntry("User:enum_1", "*", AclOperation.READ, AclPermissionType.ALLOW)), + new AclBinding( + new ResourcePattern(ResourceType.TOPIC, "implicit_describe_not_in_re", PatternType.LITERAL), + new AccessControlEntry("User:implicit", "*", AclOperation.READ, AclPermissionType.ALLOW)), + new AclBinding( + new ResourcePattern(ResourceType.TOPIC, "implicit_describe_not_in_re", PatternType.LITERAL), + new AccessControlEntry("User:implicit", "*", AclOperation.WRITE, AclPermissionType.ALLOW)), + new AclBinding( + new ResourcePattern(ResourceType.TOPIC, "implicit_describe_not_in_re", PatternType.LITERAL), + new AccessControlEntry("User:implicit", "*", AclOperation.ALTER, AclPermissionType.ALLOW)), + new AclBinding( + new ResourcePattern(ResourceType.TOPIC, "implicit_describe_not_in_re", PatternType.LITERAL), + new AccessControlEntry("User:implicit", "*", AclOperation.DELETE, AclPermissionType.ALLOW)), + new AclBinding( + new ResourcePattern(ResourceType.GROUP, "implicit_describe_read", PatternType.LITERAL), + new AccessControlEntry("User:implicit1", "*", AclOperation.READ, AclPermissionType.ALLOW)), + new AclBinding( + new ResourcePattern(ResourceType.GROUP, "implicit_describe_delete", PatternType.LITERAL), + new AccessControlEntry("User:implicit1", "*", AclOperation.DELETE, AclPermissionType.ALLOW)), + new AclBinding( + new ResourcePattern(ResourceType.TOPIC, "implicit_describe_alter", PatternType.LITERAL), + new AccessControlEntry("User:implicit2", "*", AclOperation.ALTER, AclPermissionType.ALLOW)), + new AclBinding( + new ResourcePattern(ResourceType.TOPIC, "implicit_describe_write", PatternType.LITERAL), + new AccessControlEntry("User:implicit2", "*", AclOperation.WRITE, AclPermissionType.ALLOW)), + new AclBinding( + new ResourcePattern(ResourceType.TOPIC, "implicit_describe_alterconfigs", PatternType.LITERAL), + new AccessControlEntry("User:implicit3", "*", AclOperation.ALTER_CONFIGS, AclPermissionType.ALLOW)), + new AclBinding( + new ResourcePattern(ResourceType.TOPIC, "implicit_alterconfigs2", PatternType.LITERAL), + new AccessControlEntry("User:implicit4", "*", AclOperation.ALL, AclPermissionType.ALLOW)), + new AclBinding( + new ResourcePattern(ResourceType.TOPIC, "foobar", PatternType.LITERAL), + new AccessControlEntry("User:all", "*", AclOperation.ALL, AclPermissionType.ALLOW)) + ); + + // Check that implicit describe is not added in regex mode + checkSingleAction(requestCtx("User", "implicit"), + action(AclOperation.READ, topic("implicit_describe_not_in_re")), true); + checkSingleAction(requestCtx("User", "implicit"), + action(AclOperation.WRITE, topic("implicit_describe_not_in_re")), true); + checkSingleAction(requestCtx("User", "implicit"), + action(AclOperation.ALTER, topic("implicit_describe_not_in_re")), true); + checkSingleAction(requestCtx("User", "implicit"), + action(AclOperation.DELETE, topic("implicit_describe_not_in_re")), true); + checkSingleAction(requestCtx("User", "implicit"), + action(AclOperation.DESCRIBE, topic("implicit_describe_not_in_re")), false); + + // check that implicit describe works in enum mode + checkSingleAction(requestCtx("User", "implicit1"), + action(AclOperation.READ, group("implicit_describe_read")), true); + checkSingleAction(requestCtx("User", "implicit1"), + action(AclOperation.DELETE, group("implicit_describe_read")), false); + checkSingleAction(requestCtx("User", "implicit1"), + action(AclOperation.DESCRIBE, group("implicit_describe_read")), true); + + checkSingleAction(requestCtx("User", "implicit1"), + action(AclOperation.READ, group("implicit_describe_delete")), false); + checkSingleAction(requestCtx("User", "implicit1"), + action(AclOperation.DELETE, group("implicit_describe_delete")), true); + checkSingleAction(requestCtx("User", "implicit1"), + action(AclOperation.DESCRIBE, group("implicit_describe_delete")), true); + + checkSingleAction(requestCtx("User", "implicit2"), + action(AclOperation.ALTER, topic("implicit_describe_alter")), true); + checkSingleAction(requestCtx("User", "implicit2"), + action(AclOperation.WRITE, topic("implicit_describe_alter")), false); + checkSingleAction(requestCtx("User", "implicit2"), + action(AclOperation.DESCRIBE, topic("implicit_describe_alter")), true); + + checkSingleAction(requestCtx("User", "implicit2"), + action(AclOperation.ALTER, topic("implicit_describe_write")), false); + checkSingleAction(requestCtx("User", "implicit2"), + action(AclOperation.WRITE, topic("implicit_describe_write")), true); + checkSingleAction(requestCtx("User", "implicit2"), + action(AclOperation.DESCRIBE, topic("implicit_describe_write")), true); + + checkSingleAction(requestCtx("User", "implicit3"), + action(AclOperation.ALTER_CONFIGS, topic("implicit_describe_alterconfigs")), true); + checkSingleAction(requestCtx("User", "implicit3"), + action(AclOperation.DESCRIBE_CONFIGS, topic("implicit_describe_alterconfigs")), true); + + // Check that All operation works and denying AlterConfigs does not implicitly deny DescribeConfigs + checkSingleAction(requestCtx("User", "implicit4"), + action(AclOperation.ALTER_CONFIGS, topic("implicit_alterconfigs2")), false); + checkSingleAction(requestCtx("User", "implicit4"), + action(AclOperation.DESCRIBE_CONFIGS, topic("implicit_alterconfigs2")), true); + + // Check that All operation works + checkSingleAction(requestCtx("User", "all"), + action(AclOperation.READ, topic("foobar")), true); + checkSingleAction(requestCtx("User", "all"), + action(AclOperation.WRITE, topic("foobar")), true); + checkSingleAction(requestCtx("User", "all"), + action(AclOperation.ALTER_CONFIGS, topic("foobar")), true); + checkSingleAction(requestCtx("User", "all"), + action(AclOperation.DESCRIBE_CONFIGS, topic("foobar")), true); + } public ResourcePattern topic(final String name) { return new ResourcePattern(ResourceType.TOPIC, name, PatternType.LITERAL); } + public ResourcePattern group(final String name) { + return new ResourcePattern(ResourceType.GROUP, name, PatternType.LITERAL); + } + private void startAuthorizer() { final AuthorizerServerInfo serverInfo = mock(AuthorizerServerInfo.class); when(serverInfo.endpoints()).thenReturn(List.of()); diff --git a/src/test/java/io/aiven/kafka/auth/json/AivenAclTest.java b/src/test/java/io/aiven/kafka/auth/json/AivenAclTest.java index 23ce12e..3520a65 100644 --- a/src/test/java/io/aiven/kafka/auth/json/AivenAclTest.java +++ b/src/test/java/io/aiven/kafka/auth/json/AivenAclTest.java @@ -16,6 +16,8 @@ package io.aiven.kafka.auth.json; +import org.apache.kafka.common.acl.AclOperation; + import org.junit.jupiter.api.Test; import static org.junit.jupiter.api.Assertions.assertFalse; @@ -38,11 +40,11 @@ public void testAivenAclEntry() { false // hidden ); - assertTrue(entry.match("User", "CN=p_pass_s", "*", "Read", "Topic:p_pass_s")); - assertFalse(entry.match("User", "CN=fail", "*", "Read", "Topic:p_pass_s")); - assertFalse(entry.match("User", "CN=p_pass_s", "*", "Write", "Topic:p_pass_s")); - assertFalse(entry.match("User", "CN=p_pass_s", "*", "Read", "Topic:fail")); - assertFalse(entry.match("NonUser", "CN=p_pass_s", "*", "Read", "Topic:p_pass_s")); + assertTrue(entry.match("User", "CN=p_pass_s", "*", AclOperation.READ, "Topic:p_pass_s")); + assertFalse(entry.match("User", "CN=fail", "*", AclOperation.READ, "Topic:p_pass_s")); + assertFalse(entry.match("User", "CN=p_pass_s", "*", AclOperation.WRITE, "Topic:p_pass_s")); + assertFalse(entry.match("User", "CN=p_pass_s", "*", AclOperation.READ, "Topic:fail")); + assertFalse(entry.match("NonUser", "CN=p_pass_s", "*", AclOperation.READ, "Topic:p_pass_s")); // Test with principal undefined entry = new AivenAcl( @@ -58,10 +60,10 @@ public void testAivenAclEntry() { false // hidden ); - assertTrue(entry.match("User", "CN=p_pass_s", "*", "Read", "Topic:p_pass_s")); - assertTrue(entry.match("NonUser", "CN=p_pass_s", "*", "Read", "Topic:p_pass_s")); - assertFalse(entry.match("User", "CN=fail", "*", "Read", "Topic:p_pass_s")); - assertFalse(entry.match("User", "CN=p_pass_s", "*", "Read", "Topic:fail")); + assertTrue(entry.match("User", "CN=p_pass_s", "*", AclOperation.READ, "Topic:p_pass_s")); + assertTrue(entry.match("NonUser", "CN=p_pass_s", "*", AclOperation.READ, "Topic:p_pass_s")); + assertFalse(entry.match("User", "CN=fail", "*", AclOperation.READ, "Topic:p_pass_s")); + assertFalse(entry.match("User", "CN=p_pass_s", "*", AclOperation.READ, "Topic:fail")); // Test resources defined by pattern entry = new AivenAcl( @@ -77,10 +79,10 @@ public void testAivenAclEntry() { false // hidden ); - assertTrue(entry.match("User", "CN=p_user1_s", "*", "Read", "Topic:p_user1_s")); - assertTrue(entry.match("User", "CN=p_user2_s", "*", "Read", "Topic:p_user2_s")); - assertFalse(entry.match("User", "CN=p_user1_s", "*", "Read", "Topic:p_user2_s")); - assertFalse(entry.match("User", "CN=p_user2_s", "*", "Read", "Topic:p_user1_s")); + assertTrue(entry.match("User", "CN=p_user1_s", "*", AclOperation.READ, "Topic:p_user1_s")); + assertTrue(entry.match("User", "CN=p_user2_s", "*", AclOperation.READ, "Topic:p_user2_s")); + assertFalse(entry.match("User", "CN=p_user1_s", "*", AclOperation.READ, "Topic:p_user2_s")); + assertFalse(entry.match("User", "CN=p_user2_s", "*", AclOperation.READ, "Topic:p_user1_s")); // Test resources defined by literal match entry = new AivenAcl( @@ -96,8 +98,8 @@ public void testAivenAclEntry() { false // hidden ); - assertTrue(entry.match("User", "CN=p_user1_s", "*", "Read", "Topic:^(][")); - assertFalse(entry.match("User", "CN=p_user1_s", "*", "Read", "Topic:wrong_topic")); + assertTrue(entry.match("User", "CN=p_user1_s", "*", AclOperation.READ, "Topic:^(][")); + assertFalse(entry.match("User", "CN=p_user1_s", "*", AclOperation.READ, "Topic:wrong_topic")); // Test resources defined by prefix match @@ -113,11 +115,11 @@ public void testAivenAclEntry() { null, // permission type false // hidden ); - assertTrue(entry.match("User", "CN=p_user1_s", "*", "Read", "Topic:organizationA.topic1")); - assertTrue(entry.match("User", "CN=p_user1_s", "*", "Read", "Topic:organizationA.topic2")); - assertTrue(entry.match("User", "CN=p_user1_s", "*", "Read", "Topic:organizationA.")); - assertFalse(entry.match("User", "CN=p_user1_s", "*", "Read", "Topic:organizationB.topic1")); - assertFalse(entry.match("User", "CN=p_user1_s", "*", "Read", "Topic:organizationA")); - assertFalse(entry.match("User", "CN=p_user1_s", "*", "Read", "Topic:AAAorganizationA.")); + assertTrue(entry.match("User", "CN=p_user1_s", "*", AclOperation.READ, "Topic:organizationA.topic1")); + assertTrue(entry.match("User", "CN=p_user1_s", "*", AclOperation.READ, "Topic:organizationA.topic2")); + assertTrue(entry.match("User", "CN=p_user1_s", "*", AclOperation.READ, "Topic:organizationA.")); + assertFalse(entry.match("User", "CN=p_user1_s", "*", AclOperation.READ, "Topic:organizationB.topic1")); + assertFalse(entry.match("User", "CN=p_user1_s", "*", AclOperation.READ, "Topic:organizationA")); + assertFalse(entry.match("User", "CN=p_user1_s", "*", AclOperation.READ, "Topic:AAAorganizationA.")); } } diff --git a/src/test/java/io/aiven/kafka/auth/json/reader/AclJsonReaderTest.java b/src/test/java/io/aiven/kafka/auth/json/reader/AclJsonReaderTest.java index 8df554d..4e15f81 100644 --- a/src/test/java/io/aiven/kafka/auth/json/reader/AclJsonReaderTest.java +++ b/src/test/java/io/aiven/kafka/auth/json/reader/AclJsonReaderTest.java @@ -17,7 +17,9 @@ package io.aiven.kafka.auth.json.reader; import java.io.File; +import java.util.List; +import io.aiven.kafka.auth.json.AclOperationType; import io.aiven.kafka.auth.json.AclPermissionType; import io.aiven.kafka.auth.json.AivenAcl; @@ -38,29 +40,29 @@ public final void parseAcls() { assertThat(acls).containsExactly( new AivenAcl("User", "^pass-3$", "*", "^Read$", "^Topic:denied$", null, null, null, AclPermissionType.DENY, false), - new AivenAcl("User", "^pass-0$", "*", "^Read$", + new AivenAcl("User", "^pass-0$", "*", List.of(AclOperationType.Read), "^Topic:(.*)$", null, null, null, AclPermissionType.ALLOW, false), new AivenAcl("User", "^pass-1$", "*", "^Read$", "^Topic:(.*)$", null, null, null, AclPermissionType.ALLOW, false), - new AivenAcl("User", "^pass-2$", "*", "^Read$", + new AivenAcl("User", "^pass-2$", "*", List.of(AclOperationType.Read), "^Topic:(.*)$", null, null, null, AclPermissionType.ALLOW, false), new AivenAcl("User", "^pass-3$", "*", "^Read$", "^Topic:(.*)$", null, null, null, AclPermissionType.ALLOW, false), - new AivenAcl("User", "^pass-4$", "*", "^Read$", + new AivenAcl("User", "^pass-4$", "*", List.of(AclOperationType.Read), "^Topic:(.*)$", null, null, null, AclPermissionType.ALLOW, false), new AivenAcl("User", "^pass-5$", "*", "^Read$", "^Topic:(.*)$", null, null, null, AclPermissionType.ALLOW, false), - new AivenAcl("User", "^pass-6$", "*", "^Read$", + new AivenAcl("User", "^pass-6$", "*", List.of(AclOperationType.Read), "^Topic:(.*)$", null, null, null, AclPermissionType.ALLOW, false), new AivenAcl("User", "^pass-7$", "*", "^Read$", "^Topic:(.*)$", null, null, null, AclPermissionType.ALLOW, false), new AivenAcl("User", "^pass-8$", "*", "^Read$", "^Topic:(.*)$", null, null, null, AclPermissionType.ALLOW, false), - new AivenAcl("User", "^pass-9$", "*", "^Read$", + new AivenAcl("User", "^pass-9$", "*", List.of(AclOperationType.Read), "^Topic:(.*)$", null, null, null, AclPermissionType.ALLOW, false), new AivenAcl("User", "^pass-10$", "*", "^Read$", "^Topic:(.*)$", null, null, null, AclPermissionType.ALLOW, false), - new AivenAcl("User", "^pass-11$", "*", "^Read$", + new AivenAcl("User", "^pass-11$", "*", List.of(AclOperationType.Read), "^Topic:(.*)$", null, null, null, AclPermissionType.ALLOW, false), new AivenAcl("User", "^pass-12$", "*", "^Read$", "^Topic:(.*)$", null, null, null, AclPermissionType.ALLOW, false), @@ -114,4 +116,10 @@ public final void parseWrong() { assertThrows(JsonParseException.class, jsonReader::read); } + @Test + public final void parseWrongOperations() { + final var path = new File(this.getClass().getResource("/acl_wrong_operations.json").getPath()).toPath(); + final var jsonReader = new AclJsonReader(path); + assertThrows(JsonParseException.class, jsonReader::read); + } } diff --git a/src/test/resources/acl_operations.json b/src/test/resources/acl_operations.json new file mode 100644 index 0000000..8e1bbc8 --- /dev/null +++ b/src/test/resources/acl_operations.json @@ -0,0 +1,70 @@ +[ + { + "principal_type": "User", + "principal": "^re_1$", + "operation": "^Read$", + "resource": "^Topic:topic1$" + }, + { + "principal_type": "User", + "principal": "^enum_1$", + "operations": ["Read"], + "resource": "^Topic:topic1$" + }, + { + "principal_type": "User", + "principal": "^implicit$", + "operation": "^(Read|Write|Alter|Delete)$", + "resource": "^Topic:implicit_describe_not_in_re$" + }, + { + "principal_type": "User", + "principal": "^implicit1$", + "operations": ["Read"], + "resource": "^Group:implicit_describe_read$" + }, + { + "principal_type": "User", + "principal": "^implicit1$", + "operations": ["Delete"], + "resource": "^Group:implicit_describe_delete$" + }, + { + "principal_type": "User", + "principal": "^implicit2$", + "operations": ["Alter"], + "resource": "^Topic:implicit_describe_alter$" + }, + { + "principal_type": "User", + "principal": "^implicit2$", + "operations": ["Write"], + "resource": "^Topic:implicit_describe_write$" + }, + { + "principal_type": "User", + "principal": "^implicit3$", + "operations": ["AlterConfigs"], + "resource": "^Topic:implicit_describe_alterconfigs$" + }, + { + "principal_type": "User", + "principal": "^implicit4$", + "operations": ["All"], + "resource": "^Topic:implicit_alterconfigs2$", + "permission_type": "ALLOW" + }, + { + "principal_type": "User", + "principal": "^implicit4$", + "operations": ["AlterConfigs"], + "resource": "^Topic:implicit_alterconfigs2$", + "permission_type": "DENY" + }, + { + "principal_type": "User", + "principal": "^all$", + "operations": ["All"], + "resource": "^Topic:foobar$" + } +] \ No newline at end of file diff --git a/src/test/resources/acl_wrong_operations.json b/src/test/resources/acl_wrong_operations.json new file mode 100644 index 0000000..dfa3b1f --- /dev/null +++ b/src/test/resources/acl_wrong_operations.json @@ -0,0 +1,8 @@ +[ + { + "principal_type": "User", + "principal": "^p$", + "operations": ["Read", "Ring"], + "resource": "^(.*)$" + } + ] \ No newline at end of file diff --git a/src/test/resources/acls_deny.json b/src/test/resources/acls_deny.json index 4246c8d..f25341c 100644 --- a/src/test/resources/acls_deny.json +++ b/src/test/resources/acls_deny.json @@ -2,7 +2,7 @@ { "principal_type": "User", "principal": "^(.*)$", - "operation": "^Read$", + "operations": ["Read"], "resource": "^Topic:(.*)$", "permission_type": "ALLOW" }, diff --git a/src/test/resources/acls_full.json b/src/test/resources/acls_full.json index 43023b5..d0a612a 100644 --- a/src/test/resources/acls_full.json +++ b/src/test/resources/acls_full.json @@ -9,7 +9,7 @@ { "principal_type": "User", "principal": "^pass-0$", - "operation": "^Read$", + "operations": ["Read"], "resource": "^Topic:(.*)$" }, { @@ -21,7 +21,7 @@ { "principal_type": "User", "principal": "^pass-2$", - "operation": "^Read$", + "operations": ["Read"], "resource": "^Topic:(.*)$" }, { @@ -33,7 +33,7 @@ { "principal_type": "User", "principal": "^pass-4$", - "operation": "^Read$", + "operations": ["Read"], "resource": "^Topic:(.*)$" }, { @@ -45,7 +45,7 @@ { "principal_type": "User", "principal": "^pass-6$", - "operation": "^Read$", + "operations": ["Read"], "resource": "^Topic:(.*)$" }, { @@ -63,7 +63,7 @@ { "principal_type": "User", "principal": "^pass-9$", - "operation": "^Read$", + "operations": ["Read"], "resource": "^Topic:(.*)$" }, { @@ -75,7 +75,7 @@ { "principal_type": "User", "principal": "^pass-11$", - "operation": "^Read$", + "operations": ["Read"], "resource": "^Topic:(.*)$" }, { diff --git a/src/test/resources/acls_host_match.json b/src/test/resources/acls_host_match.json index a5e9a29..d244199 100644 --- a/src/test/resources/acls_host_match.json +++ b/src/test/resources/acls_host_match.json @@ -3,14 +3,14 @@ "principal_type": "User", "principal": "^(.*)$", "host": "*", - "operation": "^(.*)$", + "operations": ["All"], "resource": "^Topic:topic-1$" }, { "principal_type": "User", "principal": "^(.*)$", "host": "192.168.123.45", - "operation": "^(.*)$", + "operations": ["All"], "resource": "^Topic:topic-2$" }, { diff --git a/src/test/resources/acls_no_type.json b/src/test/resources/acls_no_type.json index f1702d7..94c1ed3 100644 --- a/src/test/resources/acls_no_type.json +++ b/src/test/resources/acls_no_type.json @@ -1,7 +1,7 @@ [ { "principal": "^pass$", - "operation": "^Read$", + "operations": ["Read"], "resource": "^Topic:(.*)$" } ] \ No newline at end of file diff --git a/src/test/resources/acls_resource_literal_match.json b/src/test/resources/acls_resource_literal_match.json index 04d669c..2a1b110 100644 --- a/src/test/resources/acls_resource_literal_match.json +++ b/src/test/resources/acls_resource_literal_match.json @@ -8,14 +8,14 @@ { "principal_type": "User", "principal": "^(user2)$", - "operation": "^(.*)$", + "operations": ["All"], "resource_literal": "Topic:topic-2", "permission_type": "DENY" }, { "principal_type": "User", "principal": "^(user3)$", - "operation": "^(.*)$", + "operations": ["All"], "resource_literal": "Topic:*" } ] diff --git a/src/test/resources/acls_resource_prefix_match.json b/src/test/resources/acls_resource_prefix_match.json index 23b235b..a95a974 100644 --- a/src/test/resources/acls_resource_prefix_match.json +++ b/src/test/resources/acls_resource_prefix_match.json @@ -8,7 +8,7 @@ { "principal_type": "User", "principal": "^(user2)$", - "operation": "^(.*)$", + "operations": ["All"], "resource_prefix": "Topic:groupB.", "permission_type": "DENY" }