dajac commented on code in PR #18989:
URL: https://github.com/apache/kafka/pull/18989#discussion_r1966692671
##########
core/src/main/scala/kafka/server/KafkaApis.scala:
##########
@@ -2592,6 +2607,32 @@ class KafkaApis(val requestChannel: RequestChannel,
response.groups.addAll(results)
}
+ // Clients are not allowed to see topics that are not authorized for
Describe.
+ val topicsToCheck = new mutable.HashSet[String]
+ response.groups.forEach(_.members.forEach { member =>
+ List(member.assignment, member.targetAssignment).foreach {
assignment =>
+ assignment.topicPartitions.asScala.foreach { tp =>
+ topicsToCheck += tp.topicName
+ }
+ }
+ })
+ val authorizedTopics =
authHelper.filterByAuthorized(request.context, DESCRIBE, TOPIC,
Review Comment:
That’s right. The thing is that we cannot apply it on there because the
broker is not aware of the topic partitions. It just sees bytes by design.
We could consider parsing the bytes to check the partitions too but as it
has been like this since the beginning of the classic protocol. We would need a
KIP to discuss it for sure. Personally, I would not change it.
Regarding your second question, the Consumer is not impacted by this as both
implementations require topic describe but in different ways. Only the admin
client will require extra permissions if not given yet.
@dongnuo123 Could you add a sentence about it in the doc? We could mention
this in the ops.html, consumer group section.
--
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]
