-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add acl resource literal and prefixed matching #215
Conversation
17fdce3
to
6826a5d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems there are conflicts with hidden flag. I'll look a bit more. Now spotted also some minor issues
src/main/java/io/aiven/kafka/auth/utils/ResourceLiteralWildcardMatcher.java
Outdated
Show resolved
Hide resolved
3f2deb1
to
0924b2d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some minor things found, but this new exception being thrown with in some cases from validate: I'm not sure what will happen with it. What do you think, is it safe and why?
src/main/java/io/aiven/kafka/auth/json/reader/AclJsonReader.java
Outdated
Show resolved
Hide resolved
0924b2d
to
75360ed
Compare
75360ed
to
d2938ed
Compare
For this feature instead of adding a field
resourcePatternType
that distinguishes between literal, prefixed and regex (the one used in Aiven ACLs) matching, I added the fieldsresourceLiteral
andresourcePrefixed
. I did this way because we already had resource matching splito intoresourceRe
andresourceRePattern
(the later used for backreference regex), and it felt natural (and more backward compatible) adding the new matching in the same way.In the authorization method, to select between the proper resource matcher, it is used the one who is not null (as was done before my PR). This is a bit fragile by itself, for this reason I added the method
validate()
to verify that exactly one of the four resource matchers is not null. The validation is called by the constructor and by the decode class AclJsonReader.java.The literal prefix needed a bit of work for the wildcard match because the wildcard rule is defined like
Topic:*
orGroup:*
, etc and the prefix must match the one of the action that we are authorizing, so for example:Topic:*
Topic:some-topic
true
Topic:*
Group:abcde
false
Group:*
Group:abcde
true
I didn't want to introduce some big refactor so I added the class ResourceLiteralWildcardMatcher.java to perform this operation, trying to keep it fast as possible.