pulsar-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] rdhabalia commented on a change in pull request #2981: Allow subscribers to access subscription admin-api
Date Wed, 14 Nov 2018 23:47:19 GMT
rdhabalia commented on a change in pull request #2981: Allow subscribers to access subscription
admin-api
URL: https://github.com/apache/pulsar/pull/2981#discussion_r233665328
 
 

 ##########
 File path: pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authorization/AuthorizationProvider.java
 ##########
 @@ -71,6 +71,18 @@
     CompletableFuture<Boolean> canConsumeAsync(TopicName topicName, String role,
             AuthenticationDataSource authenticationData, String subscription);
 
+    /**
+     * Returns authorized roles that can access admin-api for given subscription
+     * 
+     * @param topicName
+     *            the fully qualified topic name associated with the topic.
+     * @param subscription
+     *            the subscription name defined by the client
+     * @return
+     */
+    CompletableFuture<Set<String>> getAuthorizedRolesOnSubscription(TopicName
topicName,
 
 Review comment:
   > If a use has consume access to some topic (on all the subscriptions) than it can "mess
up" some other consumer by attaching to the wrong subscription and stealing someone else's
messages.
   
   which is already incorrect. right? we should have a way to prevent this and with this PR
it can be possible. But by default we can't enforce Per-Sub for consumers as it can create
issue for existing topics. But may be pulsar can have a flag to enforce it in the system.
   
   > I don't see admin api permissions as different from "consume" permissions.
   
   Actually then we don't need this PR at all. No need of introducing "Per-Subscription" authorization
if "Namespace-auth(consume)" is anyway present so, "Namespace-auth" will always win and all
subscribers can access each other's cursor state. 
   So, I thought admin-api right now doesn't have any authorization present, we can make it
right atleast for admin-api by enforcing "Per-Subscription" validation. 
   
   But Let me know if you still think admin-api should not enforce "Per-subscription" then
I will make your suggested change and all api will use one `canConsume()-api`.
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

Mime
View raw message