m1a2st commented on code in PR #19527:
URL: https://github.com/apache/kafka/pull/19527#discussion_r2147476797


##########
clients/src/main/java/org/apache/kafka/common/security/authenticator/LoginManager.java:
##########
@@ -53,20 +56,30 @@ public class LoginManager {
     // dynamic configs (broker or client)
     private static final Map<LoginMetadata<Password>, LoginManager> 
DYNAMIC_INSTANCES = new HashMap<>();
 
-    private final Login login;
+    private final Plugin<Login> loginPlugin;
     private final LoginMetadata<?> loginMetadata;
     private final AuthenticateCallbackHandler loginCallbackHandler;
     private int refCount;
 
-    private LoginManager(JaasContext jaasContext, String saslMechanism, 
Map<String, ?> configs,
-                 LoginMetadata<?> loginMetadata) throws LoginException {
+    private LoginManager(
+        JaasContext jaasContext, 
+        String saslMechanism, 
+        Map<String, ?> configs,
+        LoginMetadata<?> loginMetadata,
+        ConnectionMode connectionMode,
+        Metrics metrics
+    ) throws LoginException {
         this.loginMetadata = loginMetadata;
-        this.login = Utils.newInstance(loginMetadata.loginClass);
+        Login login = Utils.newInstance(loginMetadata.loginClass);
+        if (connectionMode == ConnectionMode.SERVER)  
+            this.loginPlugin = Plugin.wrapInstance(login, metrics, 
SaslConfigs.SASL_LOGIN_CLASS, "mechanism", saslMechanism);

Review Comment:
   - A single `SocketServer` manages multiple `Processor` instances.  
   - Each `Processor` is associated with one `Selector`, and each `Selector` is 
linked to a `ChannelBuilder`.  
   - A `SaslChannelBuilder` is associated with exactly one `LoginManager`.  
   - An `SslChannelBuilder` is associated with exactly one `SslEngineFactory`.  
   - A `LoginManager` is associated with exactly one 
`AuthenticateCallbackHandler`.  
   - A `LoginManager` is also associated with exactly one `Login`.
   
   Since both `AuthenticateCallbackHandler` and `Login` share the same internal 
path, we propose using the `Processor` thread name as a unique identifier for 
metrics on the server side.  
   On the client side, the `client.id` can serve as the unique identifier.
   
   For `SslEngineFactory`, we can use the `SocketServer.connectionId` as its 
unique identifier.



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