Refactors "urls" -> "url"

This lets the url be configured as a single element (the most likely usage) or as an array.  This also checks that multiple urls are either all "ldaps", or all "ldap", as it is not possible to mix them.

Original commit: elastic/x-pack-elasticsearch@b5a94b1d35

src/main/java/org/elasticsearch/shield/authc/ldap/ActiveDirectoryConnectionFactory.java

@@ -32,7 +32,6 @@ import java.util.Hashtable;
 public class ActiveDirectoryConnectionFactory extends AbstractComponent implements LdapConnectionFactory {
 
     public static final String AD_DOMAIN_NAME_SETTING = "domain_name";
-    public static final String AD_PORT = "default_port";
     public static final String AD_USER_SEARCH_BASEDN_SETTING = "user_search_dn";
     static final String MODE_NAME = "active_directory";
 
@@ -48,9 +47,15 @@ public class ActiveDirectoryConnectionFactory extends AbstractComponent implemen
             throw new ShieldException("Missing [" + AD_DOMAIN_NAME_SETTING + "] setting for active directory");
         }
         userSearchDN = componentSettings.get(AD_USER_SEARCH_BASEDN_SETTING, buildDnFromDomain(domainName));
-        int port = componentSettings.getAsInt(AD_PORT, 636);
+
-        String protocol = port == 389 ? "ldap://" : "ldaps://";
+        String[] ldapUrls = componentSettings.getAsArray(URLS_SETTING);
-        String[] ldapUrls = componentSettings.getAsArray(URLS_SETTING,  new String[] { protocol + domainName + ":" + port });
+        if (ldapUrls == null) {
+            String url = componentSettings.get(URLS_SETTING);
+            ldapUrls = url == null ? null : new String[]{url};
+        }
+        if (ldapUrls == null) {
+            ldapUrls = new String[] { "ldaps://" + domainName + ":636" };
+        }
 
         ImmutableMap.Builder<String, Serializable> builder = ImmutableMap.<String, Serializable>builder()
                 .put(Context.INITIAL_CONTEXT_FACTORY, "com.sun.jndi.ldap.LdapCtxFactory")

src/main/java/org/elasticsearch/shield/authc/ldap/LdapConnectionFactory.java

@@ -20,7 +20,7 @@ import org.elasticsearch.shield.authc.support.SecuredString;
  */
 public interface LdapConnectionFactory {
 
-    static final String URLS_SETTING = "urls"; //comma separated
+    static final String URLS_SETTING = "url"; //comma separated
 
     /**
      * Password authenticated bind

src/main/java/org/elasticsearch/shield/authc/ldap/LdapSslSocketFactory.java

@@ -5,10 +5,12 @@
  */
 package org.elasticsearch.shield.authc.ldap;
 
+import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.collect.ImmutableMap;
 import org.elasticsearch.common.logging.ESLogger;
 import org.elasticsearch.common.logging.ESLoggerFactory;
 import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.shield.ShieldSettingsException;
 import org.elasticsearch.shield.transport.ssl.SSLTrustConfig;
 
 import javax.net.SocketFactory;
@@ -28,6 +30,7 @@ import java.util.Locale;
  */
 public class LdapSslSocketFactory extends SocketFactory {
 
+    static final String JAVA_NAMING_LDAP_FACTORY_SOCKET = "java.naming.ldap.factory.socket";
     private static ESLogger logger = ESLoggerFactory.getLogger(LdapSslSocketFactory.class.getName());
     private static LdapSslSocketFactory instance;
 
@@ -51,7 +54,7 @@ public class LdapSslSocketFactory extends SocketFactory {
 
     /**
      * This is invoked by JNDI and the returned SocketFactory must be an LdapSslSocketFactory object
-     * @return
+     * @return a singleton instance of LdapSslSocketFactory set by calling the init static method.
      */
     public static SocketFactory getDefault() {
         assert instance != null;
@@ -92,19 +95,21 @@ public class LdapSslSocketFactory extends SocketFactory {
 
     /**
      * If one of the ldapUrls are SSL this will set the LdapSslSocketFactory as a socket provider on the builder
-     * @param ldapUrls
+     * @param ldapUrls array of ldap urls, either all SSL or none with SSL (no mixing)
      * @param builder set of jndi properties, that will
+     * @throws org.elasticsearch.shield.ShieldSettingsException if URLs have mixed protocols.
      */
     public static void configureJndiSSL(String[] ldapUrls, ImmutableMap.Builder<String, Serializable> builder) {
-        boolean needsSSL = false;
+        boolean secureProtocol = ldapUrls[0].toLowerCase(Locale.getDefault()).startsWith("ldaps://");
         for(String url: ldapUrls){
-            if (url.toLowerCase(Locale.getDefault()).startsWith("ldaps://")) {
+            if (secureProtocol != url.toLowerCase(Locale.getDefault()).startsWith("ldaps://")) {
-                needsSSL = true;
+                //this is because LdapSSLSocketFactory produces only SSL sockets and not clear text sockets
-                break;
+                throw new ShieldSettingsException("Configured ldap protocols are not all equal " +
+                        "(ldaps://.. and ldap://..): [" + Strings.arrayToCommaDelimitedString(ldapUrls) + "]");
             }
         }
-        if (needsSSL && instance != null) {
+        if (secureProtocol && instance != null) {
-            builder.put("java.naming.ldap.factory.socket", LdapSslSocketFactory.class.getName());
+            builder.put(JAVA_NAMING_LDAP_FACTORY_SOCKET, LdapSslSocketFactory.class.getName());
         } else {
             logger.warn("LdapSslSocketFactory not used for LDAP connections");
         }

src/main/java/org/elasticsearch/shield/authc/ldap/StandardLdapConnectionFactory.java

@@ -48,6 +48,10 @@ public class StandardLdapConnectionFactory extends AbstractComponent implements
             throw new ShieldException("Missing required ldap setting [" + USER_DN_TEMPLATES_SETTING + "]");
         }
         String[] ldapUrls = componentSettings.getAsArray(URLS_SETTING);
+        if (ldapUrls == null) {
+            String url = componentSettings.get(URLS_SETTING);
+            ldapUrls = url == null ? null : new String[]{url};
+        }
         if (ldapUrls == null) {
             throw new ShieldException("Missing required ldap setting [" + URLS_SETTING + "]");
         }

src/test/java/org/elasticsearch/shield/authc/ldap/LdapSSLSocketFactoryTests.java

@@ -0,0 +1,78 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License;
+ * you may not use this file except in compliance with the Elastic License.
+ */
+package org.elasticsearch.shield.authc.ldap;
+
+import org.elasticsearch.common.collect.ImmutableMap;
+import org.elasticsearch.common.settings.ImmutableSettings;
+import org.elasticsearch.shield.ShieldSettingsException;
+import org.elasticsearch.test.ElasticsearchTestCase;
+import org.hamcrest.Matchers;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+import java.io.File;
+import java.io.Serializable;
+import java.net.URISyntaxException;
+
+import static org.hamcrest.Matchers.equalTo;
+
+public class LdapSslSocketFactoryTests extends ElasticsearchTestCase {
+
+    @BeforeClass
+    public static void setTrustStore() throws URISyntaxException {
+        //LdapModule will set this up as a singleton normally
+        LdapSslSocketFactory.init(ImmutableSettings.builder()
+                .put("shield.authc.ldap.truststore", new File(LdapConnectionTests.class.getResource("ldaptrust.jks").toURI()))
+                .build());
+    }
+
+    @Test
+    public void testConfigure_1https(){
+        String[] urls = new String[]{"ldaps://example.com:636"};
+
+        ImmutableMap.Builder<String, Serializable> builder = ImmutableMap.<String, Serializable>builder();
+        LdapSslSocketFactory.configureJndiSSL(urls, builder);
+        ImmutableMap<String, Serializable> settings = builder.build();
+        assertThat(settings.get(LdapSslSocketFactory.JAVA_NAMING_LDAP_FACTORY_SOCKET),
+                Matchers.<Serializable>equalTo("org.elasticsearch.shield.authc.ldap.LdapSslSocketFactory"));
+    }
+
+    @Test
+    public void testConfigure_2https(){
+        String[] urls = new String[]{"ldaps://primary.example.com:636", "LDAPS://secondary.example.com:10636"};
+
+        ImmutableMap.Builder<String, Serializable> builder = ImmutableMap.<String, Serializable>builder();
+        LdapSslSocketFactory.configureJndiSSL(urls, builder);
+        ImmutableMap<String, Serializable> settings = builder.build();
+        assertThat(settings.get(LdapSslSocketFactory.JAVA_NAMING_LDAP_FACTORY_SOCKET), Matchers.<Serializable>equalTo("org.elasticsearch.shield.authc.ldap.LdapSslSocketFactory"));
+    }
+
+    @Test
+    public void testConfigure_2http(){
+        String[] urls = new String[]{"ldap://primary.example.com:392", "LDAP://secondary.example.com:10392"};
+
+        ImmutableMap.Builder<String, Serializable> builder = ImmutableMap.<String, Serializable>builder();
+        LdapSslSocketFactory.configureJndiSSL(urls, builder);
+        ImmutableMap<String, Serializable> settings = builder.build();
+        assertThat(settings.get(LdapSslSocketFactory.JAVA_NAMING_LDAP_FACTORY_SOCKET), equalTo(null));
+    }
+
+    @Test(expected = ShieldSettingsException.class)
+    public void testConfigure_1httpS_1http(){
+        String[] urls = new String[]{"LDAPS://primary.example.com:636", "ldap://secondary.example.com:392"};
+
+        ImmutableMap.Builder<String, Serializable> builder = ImmutableMap.<String, Serializable>builder();
+        LdapSslSocketFactory.configureJndiSSL(urls, builder);
+    }
+
+    @Test(expected = ShieldSettingsException.class)
+    public void testConfigure_1http_1https(){
+        String[] urls = new String[]{"ldap://primary.example.com:392", "ldaps://secondary.example.com:636"};
+
+        ImmutableMap.Builder<String, Serializable> builder = ImmutableMap.<String, Serializable>builder();
+        LdapSslSocketFactory.configureJndiSSL(urls, builder);
+    }
+}