acsaki commented on code in PR #12179:
URL: https://github.com/apache/kafka/pull/12179#discussion_r888812184


##########
clients/src/main/java/org/apache/kafka/common/security/authenticator/SaslServerAuthenticator.java:
##########
@@ -679,10 +679,11 @@ private long 
calcCompletionTimesAndReturnSessionLifetimeMs() {
                 else if (connectionsMaxReauthMs == null)
                     retvalSessionLifetimeMs = 
zeroIfNegative(credentialExpirationMs - authenticationEndMs);
                 else
-                    retvalSessionLifetimeMs = zeroIfNegative(
-                            Math.min(credentialExpirationMs - 
authenticationEndMs, connectionsMaxReauthMs));
+                    retvalSessionLifetimeMs = 
zeroIfNegative(Math.min(credentialExpirationMs - authenticationEndMs, 
connectionsMaxReauthMs));
 
-                sessionExpirationTimeNanos = authenticationEndNanos + 1000 * 
1000 * retvalSessionLifetimeMs;
+                if (connectionsMaxReauthMs != null) {

Review Comment:
   @showuon this is what I've also found rather confusing. I agree that after 
the token expires the connection should be closed sooner or later which isn't 
going to happen when sessionExpirationTimeNanos is not set. While there is the 
Authenticator interface where comments suggest that  
#serverSessionExpirationTimeNanos should be left as null when re-authentication 
is "disabled". Does it make sense for reauth to be disabled? Or rather there 
are some clients or SASL mechanisms where we don't expect reauthentication to 
ever happen?



-- 
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: jira-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to