nodece commented on code in PR #18130:
URL: https://github.com/apache/pulsar/pull/18130#discussion_r1081463873


##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/ServerCnx.java:
##########
@@ -707,73 +709,96 @@ private void completeConnect(int clientProtoVersion, 
String clientVersion, boole
     }
 
     // According to auth result, send newConnected or newAuthChallenge command.
-    private State doAuthentication(AuthData clientData,
-                                   int clientProtocolVersion,
-                                   String clientVersion) throws Exception {
-
+    private CompletableFuture<Void> doAuthenticationAsync(AuthData clientData, 
int clientProtocolVersion,
+                                                          String 
clientVersion) {
         // The original auth state can only be set on subsequent auth attempts 
(and only
         // in presence of a proxy and if the proxy is forwarding the 
credentials).
         // In this case, the re-validation needs to be done against the 
original client
-        // credentials.
-        boolean useOriginalAuthState = (originalAuthState != null);
-        AuthenticationState authState =  useOriginalAuthState ? 
originalAuthState : this.authState;
-        String authRole = useOriginalAuthState ? originalPrincipal : 
this.authRole;
-        AuthData brokerData = authState.authenticate(clientData);
-
-        if (log.isDebugEnabled()) {
-            log.debug("Authenticate using original auth state : {}, role = 
{}", useOriginalAuthState, authRole);
-        }
+        // credentials, but we only can new an authentication state, because 
some authentication data(TLS, SASL)
+        // based on outside service.

Review Comment:
   > This seems like a gap in the protocol where multi-staged authentication is 
not able to be properly processed through the proxy. However, if its a gap, 
that means it is something that could be improved without breaking anything.
   
   When using the multi-staged authentication, we need to set 
`authenticateOriginalAuthData=false` in the broker, and then it works fine, the 
broker just checks the proxy's authentication data.
   
   > One question is whether it would help to have a distinct stages where we 
first authenticate the proxy and then authenticate its original client (when 
configured to do so). 
   
   The protocol needs to be changed, distinguishing between client and proxy.
   
   
   The authentication logic here is a bit confusing, but for the current design 
we need to give the correct configuration in our documentation.



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

To unsubscribe, e-mail: [email protected]

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

Reply via email to