pulsar-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] merlimat commented on a change in pull request #2981: Allow subscribers to access subscription admin-api
Date Thu, 15 Nov 2018 00:02:07 GMT
merlimat commented on a change in pull request #2981: Allow subscribers to access subscription
admin-api
URL: https://github.com/apache/pulsar/pull/2981#discussion_r233668565
 
 

 ##########
 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:
   > which is already incorrect. right? we should have a way to prevent this and with this
PR it can be possible. 
   
   I think having per-subscription ACLs should be enough, right? 
   
   > 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.
   
   Even for existing topic, we can just add the per-subscription ACLs, no? 
   
   > 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.
   
   But you just need to grant one permission, either at the namespace level or at the subscription
level if you want to have a user to be restricted. 
   
   I think this change is good. Otherwise having 2 different ways to enforce authorization
for consume vs admin makes things very confusing.
   
   Let's stick with 3 levels of authorization (namespace, topic, subscription). Whoever has
"consume" access on any of these, will be able to consume and perform admin related operations
on that topic/subscription. 

----------------------------------------------------------------
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