eolivelli commented on a change in pull request #2355:
URL: https://github.com/apache/bookkeeper/pull/2355#discussion_r435021238



##########
File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/BookieRequestProcessor.java
##########
@@ -564,10 +564,19 @@ public void operationComplete(Future<Channel> future) 
throws Exception {
                     AuthHandler.ServerSideHandler authHandler = c.pipeline()
                             .get(AuthHandler.ServerSideHandler.class);
                     authHandler.authProvider.onProtocolUpgrade();
-                    if (future.isSuccess()) {
+
+                    /*
+                     * Success of the future doesn't guarantee success in 
authentication
+                     * future.isSuccess() only checks if the result field is 
not null
+                     */
+                    if (future.isSuccess() && authHandler.authenticated) {

Review comment:
       can we add a "isAuthenticated" method to authHandler ?
   this way we want access directly the field




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

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to