smolnar82 commented on code in PR #995:
URL: https://github.com/apache/knox/pull/995#discussion_r1971116709


##########
gateway-server/src/main/java/org/apache/knox/gateway/services/security/impl/JettySSLService.java:
##########
@@ -236,6 +236,17 @@ public Object buildSslContextFactory(GatewayConfig config) 
throws AliasServiceEx
     return sslContextFactory;
   }
 
+  @Override
+  public void excludeTopologyFromClientAuth(SslContextFactory 
sslContextFactory, GatewayConfig config, String topologyName) {
+    if(config.getClientAuthExclude() != null && topologyName != null
+            && config.isClientAuthNeeded() && 
config.getClientAuthExclude().contains(topologyName)) {

Review Comment:
   Most of these conditions could be implemented in GatewayConfigImpl to be 
self-contained. For instance:
   `config.isClientAuthExclude(topologyName)` could return `true` if the 
desired configs are set and the topology is meant to be excluded.



##########
gateway-server/src/test/java/org/apache/knox/gateway/services/security/impl/JettySSLServiceTest.java:
##########
@@ -34,6 +34,7 @@
 import org.apache.knox.gateway.services.security.AliasService;
 import org.apache.knox.gateway.services.security.AliasServiceException;
 import org.apache.knox.gateway.services.security.KeystoreService;
+import org.easymock.EasyMock;

Review Comment:
   nit: all EasyMock methods are imported in a static way in this test class 
(see above) you may want to follow this pattern.



##########
gateway-server/src/test/java/org/apache/knox/gateway/services/security/impl/JettySSLServiceTest.java:
##########
@@ -486,4 +544,19 @@ private GatewayConfig createGatewayConfig(boolean 
isClientAuthNeeded, boolean is
     return config;
   }
 
+  private GatewayConfig createGatewayConfigForExclude(boolean 
isClientAuthNeeded, String exclude) {

Review Comment:
   nit: the `exclude` parameter could be `topologiesToExclude` (or similar) and 
the method name could be `...ForExcludeTopologiesFromClientAuth`



-- 
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: dev-unsubscr...@knox.apache.org

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

Reply via email to