[GitHub] nkurihar closed pull request #958: Don't create client when authentication fails on websocket proxy

[GitBox <gi...@apache.org>]

nkurihar closed pull request #958: Don't create client when authentication fails on websocket proxy
URL: https://github.com/apache/incubator-pulsar/pull/958
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/pulsar-websocket/src/main/java/org/apache/pulsar/websocket/AbstractWebSocketHandler.java b/pulsar-websocket/src/main/java/org/apache/pulsar/websocket/AbstractWebSocketHandler.java
index 800bdbe8c..5a77a006c 100644
--- a/pulsar-websocket/src/main/java/org/apache/pulsar/websocket/AbstractWebSocketHandler.java
+++ b/pulsar-websocket/src/main/java/org/apache/pulsar/websocket/AbstractWebSocketHandler.java
@@ -46,6 +46,7 @@
 
     protected final String topic;
     protected final Map<String, String> queryParams;
+    protected final boolean authResult;
 
     public AbstractWebSocketHandler(WebSocketService service, HttpServletRequest request, ServletUpgradeResponse response) {
         this.service = service;
@@ -57,10 +58,10 @@ public AbstractWebSocketHandler(WebSocketService service, HttpServletRequest req
             queryParams.put(key, values[0]);
         });
 
-        checkAuth(response);
+        authResult = checkAuth(response);
     }
 
-    private void checkAuth(ServletUpgradeResponse response) {
+    private boolean checkAuth(ServletUpgradeResponse response) {
         String authRole = "<none>";
         if (service.isAuthenticationEnabled()) {
             try {
@@ -77,7 +78,7 @@ private void checkAuth(ServletUpgradeResponse response) {
                     log.warn("[{}:{}] Failed to send error: {}", request.getRemoteAddr(), request.getRemotePort(),
                             e1.getMessage(), e1);
                 }
-                return;
+                return false;
             }
         }
 
@@ -87,7 +88,7 @@ private void checkAuth(ServletUpgradeResponse response) {
                     log.warn("[{}:{}] WebSocket Client [{}] is not authorized on topic {}", request.getRemoteAddr(),
                             request.getRemotePort(), authRole, topic);
                     response.sendError(HttpServletResponse.SC_FORBIDDEN, "Not authorized");
-                    return;
+                    return false;
                 }
             } catch (Exception e) {
                 log.warn("[{}:{}] Got an exception when authorizing WebSocket client {} on topic {} on: {}",
@@ -98,10 +99,10 @@ private void checkAuth(ServletUpgradeResponse response) {
                     log.warn("[{}:{}] Failed to send error: {}", request.getRemoteAddr(), request.getRemotePort(),
                             e1.getMessage(), e1);
                 }
-                return;
+                return false;
             }
         }
-
+        return true;
     }
 
     @Override
diff --git a/pulsar-websocket/src/main/java/org/apache/pulsar/websocket/ConsumerHandler.java b/pulsar-websocket/src/main/java/org/apache/pulsar/websocket/ConsumerHandler.java
index ff9c641fa..c0ccf51b6 100644
--- a/pulsar-websocket/src/main/java/org/apache/pulsar/websocket/ConsumerHandler.java
+++ b/pulsar-websocket/src/main/java/org/apache/pulsar/websocket/ConsumerHandler.java
@@ -86,6 +86,10 @@ public ConsumerHandler(WebSocketService service, HttpServletRequest request, Ser
         this.numBytesDelivered = new LongAdder();
         this.numMsgsAcked = new LongAdder();
 
+        if (!authResult) {
+            return;
+        }
+
         try {
             this.consumer = service.getPulsarClient().subscribe(topic, subscription, conf);
             if (!this.service.addConsumer(this)) {
diff --git a/pulsar-websocket/src/main/java/org/apache/pulsar/websocket/ProducerHandler.java b/pulsar-websocket/src/main/java/org/apache/pulsar/websocket/ProducerHandler.java
index 317bd751b..990f2cd18 100644
--- a/pulsar-websocket/src/main/java/org/apache/pulsar/websocket/ProducerHandler.java
+++ b/pulsar-websocket/src/main/java/org/apache/pulsar/websocket/ProducerHandler.java
@@ -86,6 +86,10 @@ public ProducerHandler(WebSocketService service, HttpServletRequest request, Ser
         this.numMsgsFailed = new LongAdder();
         this.publishLatencyStatsUSec = new StatsBuckets(ENTRY_LATENCY_BUCKETS_USEC);
 
+        if (!authResult) {
+            return;
+        }
+
         try {
             ProducerConfiguration conf = getProducerConfiguration();
             this.producer = service.getPulsarClient().createProducer(topic, conf);
diff --git a/pulsar-websocket/src/main/java/org/apache/pulsar/websocket/ReaderHandler.java b/pulsar-websocket/src/main/java/org/apache/pulsar/websocket/ReaderHandler.java
index 84e712be4..ddf23c571 100644
--- a/pulsar-websocket/src/main/java/org/apache/pulsar/websocket/ReaderHandler.java
+++ b/pulsar-websocket/src/main/java/org/apache/pulsar/websocket/ReaderHandler.java
@@ -81,6 +81,10 @@ public ReaderHandler(WebSocketService service, HttpServletRequest request, Servl
         this.numMsgsDelivered = new LongAdder();
         this.numBytesDelivered = new LongAdder();
 
+        if (!authResult) {
+            return;
+        }
+
         try {
             this.reader = service.getPulsarClient().createReader(topic, getMessageId(), conf);
             this.subscription = ((ReaderImpl)this.reader).getConsumer().getSubscription();


 

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