rhbz1001405 - refactor code

- customised trust manager no need to be conditional as it's only used by resteasy
- remove unused test class
- remove metavar from option

zanata-client-commands/src/main/java/org/zanata/client/commands/ConfigurableOptions.java

@@ -61,8 +61,8 @@ public interface ConfigurableOptions extends BasicOptions
    /**
     * Disable SSL certificate verification when connecting to Zanata host by https.
     */
-   boolean isSSLCertDisabled();
+   boolean isDisableSSLCert();
 
-   @Option(name = "--disable-ssl-cert", metaVar = "BOOLEAN", usage = "Whether verification of SSL certificates should be disabled")
+   @Option(name = "--disable-ssl-cert", usage = "Whether verification of SSL certificates should be disabled")
    public void setDisableSSLCert(boolean disableSSLCert);
 }

zanata-client-commands/src/main/java/org/zanata/client/commands/ConfigurableOptionsImpl.java

@@ -132,13 +132,13 @@ public void setLogHttp(boolean logHttp)
 
 
    @Override
-   public boolean isSSLCertDisabled()
+   public boolean isDisableSSLCert()
    {
       return disableSSLCert;
    }
 
    @Override
-   @Option(name = "--disable-ssl-cert", metaVar = "BOOLEAN", usage = "Whether verification of SSL certificates should be disabled")
+   @Option(name = "--disable-ssl-cert", usage = "Whether verification of SSL certificates should be disabled")
    public void setDisableSSLCert(boolean disableSSLCert)
    {
       this.disableSSLCert = disableSSLCert;

zanata-client-commands/src/main/java/org/zanata/client/commands/OptionsUtil.java

@@ -147,12 +147,23 @@ public static ZanataProxyFactory createRequestFactory(ConfigurableOptions opts)
       try
       {
          if (opts.getUrl() == null)
+         {
             throw new ConfigException("Server URL must be specified");
+         }
          if (opts.getUsername() == null)
+         {
             throw new ConfigException("Username must be specified");
+         }
          if (opts.getKey() == null)
+         {
             throw new ConfigException("API key must be specified");
-         return new ZanataProxyFactory(opts.getUrl().toURI(), opts.getUsername(), opts.getKey(), VersionUtility.getAPIVersionInfo(), opts.getLogHttp(), opts.isSSLCertDisabled());
+         }
+         if (opts.isDisableSSLCert())
+         {
+            log.warn("Skipping SSL certificate verification for {}", opts.getUrl());
+            log.warn("You should consider adding the certificate instead of skipping it.");
+         }
+         return new ZanataProxyFactory(opts.getUrl().toURI(), opts.getUsername(), opts.getKey(), VersionUtility.getAPIVersionInfo(), opts.getLogHttp(), opts.isDisableSSLCert());
       }
       catch (URISyntaxException e)
       {

zanata-maven-plugin/src/main/java/org/zanata/maven/ConfigurableMojo.java

@@ -318,7 +318,7 @@ public void setLogHttp(boolean logHttp)
    }
 
    @Override
-   public boolean isSSLCertDisabled()
+   public boolean isDisableSSLCert()
    {
       return disableSSLCert;
    }

zanata-rest-client/src/main/java/org/zanata/rest/client/ZanataProxyFactory.java

@@ -4,15 +4,11 @@
 import java.net.URI;
 import java.net.URISyntaxException;
 import java.net.URL;
-import java.security.KeyStore;
-import java.security.KeyStoreException;
-import java.security.NoSuchAlgorithmException;
 import java.security.SecureRandom;
 import java.security.cert.CertificateException;
 import java.security.cert.X509Certificate;
 import javax.net.ssl.SSLContext;
 import javax.net.ssl.TrustManager;
-import javax.net.ssl.TrustManagerFactory;
 import javax.net.ssl.X509TrustManager;
 import javax.ws.rs.core.Response;
 
@@ -35,9 +31,6 @@
 import org.zanata.rest.service.AsynchronousProcessResource;
 import org.zanata.rest.service.CopyTransResource;
 import org.zanata.rest.service.StatisticsResource;
-import com.google.common.base.Predicates;
-import com.google.common.collect.Iterables;
-import com.google.common.collect.Lists;
 
 public class ZanataProxyFactory implements ITranslationResourcesFactory
 {
@@ -60,27 +53,13 @@ public class ZanataProxyFactory implements ITranslationResourcesFactory
     * @param base base url
     * @param username username
     * @param apiKey api key
-    * @param apiVersionInfo client API version
+    * @param clientApiVersion client API version
     * @param logHttp whether to log http output
     * @param sslCertDisabled whether to disable SSL certificate verification
     */
-   public ZanataProxyFactory(URI base, String username, String apiKey, VersionInfo apiVersionInfo, boolean logHttp, boolean sslCertDisabled)
+   public ZanataProxyFactory(URI base, String username, String apiKey, VersionInfo clientApiVersion, boolean logHttp, boolean sslCertDisabled)
    {
-      this(base, username, apiKey, null, apiVersionInfo, logHttp, sslCertDisabled);
-   }
-
-   /**
-    * This is used by test to substitute a mocked ClientExecutor. 
-    * When used in production, if sslCertDisabled is true, we will always create a customised ClientExecutor that will disable
-    * SSL certificate verification.
-    *
-    * @param executor client executor
-    * @param sslCertDisabled whether to disable SSL certificate verification
-    */
-   protected ZanataProxyFactory(URI base, String username, String apiKey, ClientExecutor executor, VersionInfo clientApiVersion,
-                             boolean logHttp, boolean sslCertDisabled)
-   {
-      ClientExecutor clientExecutor = executor;
+      ClientExecutor clientExecutor = null;
       if (sslCertDisabled)
       {
          clientExecutor = createClientExecutor();
@@ -117,12 +96,12 @@ else if (serverVersion.contains(RestConstant.SNAPSHOT_VERSION) && !serverTimesta
       }
    }
 
-   private ApacheHttpClient4Executor createClientExecutor()
+   protected ClientExecutor createClientExecutor()
    {
       try
       {
          // Create a trust manager that does not validate certificate chains against our server
-         TrustManager[] trustOurCerts = new TrustManager[] {new SkippableX509TrustManager()};
+         TrustManager[] trustOurCerts = new TrustManager[] {new AcceptAllX509TrustManager()};
 
          SSLContext sslContext = SSLContext.getInstance("TLS");
          sslContext.init(null, trustOurCerts, new SecureRandom());
@@ -425,46 +404,19 @@ public int compareToServerVersion( String version )
    }
 
 
-   private class SkippableX509TrustManager implements X509TrustManager
+   private static class AcceptAllX509TrustManager implements X509TrustManager
    {
-      private final X509TrustManager defaultTrustManager;
-
-      public SkippableX509TrustManager() throws NoSuchAlgorithmException, KeyStoreException
-      {
-         TrustManagerFactory trustManagerFactory = TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm());
-         trustManagerFactory.init((KeyStore) null);
-         // default trust manager
-         defaultTrustManager = (X509TrustManager) Iterables.find(
-               Lists.newArrayList(trustManagerFactory.getTrustManagers()), Predicates.instanceOf(X509TrustManager.class));
-      }
-
       public X509Certificate[] getAcceptedIssuers()
       {
-         return defaultTrustManager.getAcceptedIssuers();
+         return null;
       }
 
       public void checkClientTrusted(X509Certificate[] certs, String authType) throws CertificateException
       {
-         defaultTrustManager.checkClientTrusted(certs, authType);
       }
 
       public void checkServerTrusted(X509Certificate[] certs, String authType) throws CertificateException
       {
-         for (X509Certificate cert : certs)
-         {
-            String host = getBaseUrl().getHost();
-            String commonNameInCert = String.format("CN=%s,", host);
-            if (cert.getSubjectDN().getName().contains(commonNameInCert))
-            {
-               log.warn("Skipped SSL certificate verification for {}", host);
-               log.warn("You should consider adding the certificate instead of skipping it.");
-               //we should link a wiki or something to allow users to follow
-            }
-            else
-            {
-               defaultTrustManager.checkServerTrusted(certs, authType);
-            }
-         }
       }
    }
 }