ivankelly commented on a change in pull request #1002: Making Pulsar Proxy more 
secure
URL: https://github.com/apache/incubator-pulsar/pull/1002#discussion_r197818903
 
 

 ##########
 File path: 
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/ServerCnx.java
 ##########
 @@ -187,73 +187,128 @@ public void exceptionCaught(ChannelHandlerContext ctx, 
Throwable cause) throws E
     @Override
     protected void handleLookup(CommandLookupTopic lookup) {
         final long requestId = lookup.getRequestId();
-        final String topic = lookup.getTopic();
+        final String topicName = lookup.getTopic();
         if (log.isDebugEnabled()) {
-            log.debug("[{}] Received Lookup from {} for {}", topic, 
remoteAddress, requestId);
+            log.debug("[{}] Received Lookup from {} for {}", topicName, 
remoteAddress, requestId);
         }
+
         final Semaphore lookupSemaphore = service.getLookupRequestSemaphore();
         if (lookupSemaphore.tryAcquire()) {
-            lookupDestinationAsync(getBrokerService().pulsar(), 
DestinationName.get(topic), lookup.getAuthoritative(),
-                    getRole(), lookup.getRequestId()).handle((lookupResponse, 
ex) -> {
-                        if (ex == null) {
-                            ctx.writeAndFlush(lookupResponse);
-                        } else {
-                            // it should never happen
-                            log.warn("[{}] lookup failed with error {}, {}", 
remoteAddress, topic, ex.getMessage(), ex);
-                            ctx.writeAndFlush(
-                                    
newLookupErrorResponse(ServerError.ServiceNotReady, ex.getMessage(), 
requestId));
-                        }
-                        lookupSemaphore.release();
-                        return null;
-                    });
+            final String originalPrincipal = lookup.hasOriginalPrincipal() ? 
lookup.getOriginalPrincipal()
+                    : this.originalPrincipal;
+            CompletableFuture<Boolean> isProxyAuthorizedFuture;
+            if (service.isAuthorizationEnabled() && originalPrincipal != null) 
{
+                isProxyAuthorizedFuture = service.getAuthorizationManager()
+                        .canLookupAsync(DestinationName.get(topicName), 
authRole);
 
 Review comment:
   @jai1 @merlimat is authRole correct here? Surely if we are coming from the 
proxy and originalPrincipal is set, we should be checking if the original 
principle which can access the resource?

----------------------------------------------------------------
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:
[email protected]


With regards,
Apache Git Services

Reply via email to