michaeljmarshall commented on pull request #11872: URL: https://github.com/apache/pulsar/pull/11872#issuecomment-910828509
@gaoran10 and @eolivelli - thank you for your feedback. In writing the tests, I discovered that my initial comments were overstated. There is an authorization check in the `preValidation` method. The check comes when the `getPartitionedTopicMetadataAsync` is called from the `preValidation` method. In `getPartitionedTopicMetadataAsync`, we run the following check: https://github.com/apache/pulsar/blob/d57c1d29ffeee7a6f17a77e5d198892c6d1cdd27/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/AdminResource.java#L445 This check however does not use the `allowTopicPolicyOperationAsync` method from the `AuthorizationProvider` interface. It instead only verifies that a given role can `lookup` a given topic, and if a role can do that, then they can modify that topic's policies. It looks like the `allowTopicPolicyOperationAsync` is a recently added method. Now that we have this dedicated method, I think we should use it. Further, note that all of the endpoints I modified are annotated with the following: ```java @ApiResponse(code = 403, message = "Don't have admin permission") ``` This implies to me that the original design was to only allow modification of the topic policy endpoints by tenant admins, not just roles with produce/consume permission. Please let me know what you think. Thanks! -- 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]
