tolbertam commented on code in PR #2969:
URL: https://github.com/apache/cassandra/pull/2969#discussion_r1458028339


##########
src/java/org/apache/cassandra/transport/messages/StartupMessage.java:
##########
@@ -118,8 +121,34 @@ else if (compression.equals("lz4"))
             clientState.setDriverVersion(options.get(DRIVER_VERSION));
         }
 
-        if (DatabaseDescriptor.getAuthenticator().requireAuthentication())
+        IAuthenticator authenticator = DatabaseDescriptor.getAuthenticator();
+        if (authenticator.requireAuthentication())
+        {
+            // If the authenticator supports early certificate authentication, 
attempt to authenticate with certificates.
+            if (authenticator.supportsEarlyCertificateAuthentication())

Review Comment:
   If I'm understanding correctly, I think that is the way it should work with 
the current state of things:
   
   
`MutualTlsWithPasswordFallbackAuthenticator.supportsEarlyCertificateAuthentication()`
 will always return `true` 
   
   The `SaslNegotiator` returned from 
`MutualTlsWithPasswordFallbackAuthenticator.newSaslNegotiator` will return an 
implementation that evaluates to `false` for 
`requiresCertificateAuthentication()` if no certs are present on the 
connection, and `true` if certificates are present which will then cause it to 
attempt to authenticate (line 135 below).
   
   --
   
   I do like the idea of more generically naming this though;  Are there other 
kinds of authentication that you could envision of that we could do eventually 
without sending an AUTHENTICATE message?  Other things present on the 
connection that we could validate with?
   
   I think if we renamed:
   
   1. `IAuthenticator.supportsEarlyCertificateAuthentication` to 
`IAuthenticator.supportsEarlyAuthentication`
   2. `SaslNegotiator.requiresCertificateAuthentication` to 
`SaslNegotiation.requiresEarlyAuthentication`
   
   That could work?  In `AuthUtil.handleLogin` we pass `connection` and 
`state`, which should be the level of context needed to do other authentication 
(unless there is something else we can pass through that would be helpful for 
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.

To unsubscribe, e-mail: [email protected]

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


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to