[GitHub] [james-project] ottoka commented on a change in pull request #750: JAMES-3672 : Support TLS authentication via client certificate

[GitBox <gi...@apache.org>]

ottoka commented on a change in pull request #750:
URL: https://github.com/apache/james-project/pull/750#discussion_r751082468



##########
File path: protocols/netty/src/main/java/org/apache/james/protocols/netty/AbstractSSLAwareChannelPipelineFactory.java
##########
@@ -64,6 +66,12 @@ public ChannelPipeline getPipeline() throws Exception {
             if (enabledCipherSuites != null && enabledCipherSuites.length > 0) {
                 engine.setEnabledCipherSuites(enabledCipherSuites);
             }
+            if (Boolean.TRUE.equals(clientAuth)) {
+                engine.setNeedClientAuth(true);
+            }
+            if (Boolean.FALSE.equals(clientAuth)) {
+                engine.setWantClientAuth(true);
+            }

Review comment:
       Actually this is a tri-state logic: null=none=no client auth, false=want=optional client auth, true=need=mandatory client auth. So clientAuth can be null here, and in that case we must not call either of the engine.setXXXClientAuth(true).
   
   Admittedly this is a somewhat ugly API in the JDK, there is no good way around it. I can rename clientAuth to clientAuthRequired to maybe make things a bit clearer.




-- 
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: notifications-unsubscribe@james.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org