search

Aiven-Open / auth-for-apache-kafka

Aiven Authentication and Authorization Plugins for Apache Kafka®
Apache License 2.0
2 stars 3 forks source link

Implement ACL permission types #135

Closed giuseppelillo closed 1 year ago

giuseppelillo commented 1 year ago

Introduce the concept of ACL permission types: an ACL can represent an allow rule or a deny rule. Deny ACLs behave like blocklist and they have precedence during the evaluation of a request. Allow ACLs behave like allowlist and are checked only if there's no Deny ACL that denies the request.

giuseppelillo commented 1 year ago

@giuseppelillo Although I appreciate the proposed hierarchy, I think it may be enough to add permission type as an enum [1] to the AivenAcl, and default to ALLOW to keep compatibility with current implementation.

This may remove some mapping as part of loading the Acls from JSON, as there would be no need to deal with Abstract classes.

[1] https://github.com/apache/kafka/blob/trunk/clients/src/main/java/org/apache/kafka/common/acl/AclPermissionType.java

I removed the hierarchy of Acl types. It was originally done to have consistency at compile time so that you can only use DenyAcls to deny requests and AllowAcls to authorize requests, while without this you have two lists of AivenAcl which could potentially be mixed. Anyway it's not a huge gain and it did add more complexity to the codebase so I'm fine with removing it. However I still left the AclPermissionType class because I did not wanted to couple too much with the Kafka Acl permission type, as they have also other values (ANY and UNKNOWN) which are not relevant to our use case.

ivanyu commented 1 year ago

@giuseppelillo please squash, let's merge!