pulsar-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] [pulsar] merlimat commented on a change in pull request #6074: PIP-55: Refresh Authentication Credentials
Date Fri, 24 Jan 2020 22:46:02 GMT
merlimat commented on a change in pull request #6074: PIP-55: Refresh Authentication Credentials
URL: https://github.com/apache/pulsar/pull/6074#discussion_r370875273
 
 

 ##########
 File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/service/ServerCnx.java
 ##########
 @@ -497,8 +523,60 @@ private void doAuthentication(AuthData clientData,
             log.debug("[{}] Authentication in progress client by method {}.",
                 remoteAddress, authMethod);
         }
-        state = State.Connecting;
-        return;
+        return State.Connecting;
+    }
+
+    public void refreshAuthenticationCredentials() {
+        if (getState() != State.Connected || !isActive) {
+            // Connection is either still being established or already closed.
+            return;
+        }
+
+        AuthenticationState authState = this.originalAuthState != null ? originalAuthState
: this.authState;
+        if (authState != null && !authState.isExpired()) {
+            // Credentials are still valid. Nothing to do at this point
+            return;
+        }
+
+        if (originalPrincipal != null && originalAuthState == null) {
+            log.info(
+                    "[{}] Cannot revalidate user credential when using proxy and not forwarding
the credentials. Closing connection",
+                    remoteAddress);
+            return;
+        }
+
+        ctx.executor().execute(SafeRun.safeRun(() -> {
+            log.info("[{}] Refreshing authentication credentials", remoteAddress);
+
+            if (!supportsAuthenticationRefresh()) {
+                log.warn("[{}] Closing connection because client doesn't support auth credentials
refresh", remoteAddress);
+                ctx.close();
 
 Review comment:
   If we don't close the connection for older clients, it becomes a security loophole.
   
   Also,  keep in mind that this is only apply if the credentials are expiring. In that case,
a client will typically be reading the credentials each time are needed (eg: from a file or
through a supplier function). 
   If that was not the case, it would have trouble even without this change. (eg: when a broker
gets restarted it will not be able to reconnect anymore).
   
   After the connection gets closed, an older client will be anyway able to immediately reconnect
and refresh the authentication.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

Mime
View raw message