josefk31 commented on code in PR #19664:
URL: https://github.com/apache/kafka/pull/19664#discussion_r2127591131


##########
core/src/main/scala/kafka/server/KafkaApis.scala:
##########
@@ -265,33 +265,52 @@ class KafkaApis(val requestChannel: RequestChannel,
   }
 
   def handleGetReplicaLogInfo(request: RequestChannel.Request): Unit = {
-    authHelper.authorizeClusterOperation(request, CLUSTER_ACTION)
+    var partitionCount = 0
+    def processPartitions(topicLogInfo: 
GetReplicaLogInfoResponseData.TopicPartitionLogInfo,
+                          partitionIter: util.Iterator[Integer],
+                          action: Integer => 
GetReplicaLogInfoResponseData.PartitionLogInfo): Unit = {
+      while (partitionIter.hasNext && partitionCount < 
GetReplicaLogInfoRequest.MAX_PARTITIONS_PER_REQUEST) {
+        topicLogInfo.partitionLogInfo().add(action(partitionIter.next()))
+        partitionCount += 1
+      }
+    }
+
+    val isClusterAction = authorizeClusterOperation(request, CLUSTER_ACTION)
+    def isAuthorized(topicName: String): Boolean =
+      isClusterAction || authHelper.authorize(request.context, DESCRIBE, 
TOPIC, topicName)

Review Comment:
   This is an interesting comment - I did a bit more research on how `ACLs` 
work in Kafka. If I understand the comment correctly, there would need to be a 
system of hierarchy between `CLUSTER_ACTION` and `DESCRIBE` as well as a 
hierarchy between resources of `CLUSTER` and `TOPIC`.
   In other words "Principal P has CLUSTER_ACTION on CLUSTER => Principal P has 
DESCRIBE on TOPIC"
   It seems like `RESOURCES` are not hierarchical. So `TOPIC` and `CLUSTER` are 
seen as distinct entities in the ACL system. Which makes sense IG? It's like 
separating the "control-plane" and "data-plane" resources. 
   
   There is a concept of an "implied action" - so if you have `READ` operation 
then it's implied that you also have `DESCRIBE` permission but I don't think 
this applies here.
   
   That being said, brokers and controllers can be considered super-users of 
the cluster; which means that these ACLs kind of "don't matter" for them; they 
have permission to do everything anyways in the `StandardAuthorizer`. 
   
   I noticed that this is how `handleOffsetForLeaderEpoch` 
[handles](https://github.com/apache/kafka/blob/18a276edf05ded12e9b7381f6104749ef96f10c8/core/src/main/scala/kafka/server/KafkaApis.scala#L2186)
 its ACLs - it also checks `CLUSTER_ACTION` and then falls back to `DESCRIBE`. 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to