Ensure domain_name setting for AD realm is present (#61983) (#63159)

We would only check for a null value and not for an empty string so
that meant that we were not actually enforcing this mandatory
setting. This commits ensures we check for both and fail 
accordingly if necessary, on startup

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/ldap/ActiveDirectorySessionFactorySettings.java

@@ -5,19 +5,26 @@
  */
 package org.elasticsearch.xpack.core.security.authc.ldap;
 
+import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.settings.Setting;
 import org.elasticsearch.xpack.core.security.authc.RealmSettings;
 import org.elasticsearch.xpack.core.security.authc.ldap.support.SessionFactorySettings;
 
 import java.util.HashSet;
 import java.util.Set;
+import java.util.function.Function;
 
 import static org.elasticsearch.xpack.core.security.authc.ldap.LdapRealmSettings.AD_TYPE;
 
 public final class ActiveDirectorySessionFactorySettings {
     private static final String AD_DOMAIN_NAME_SETTING_KEY = "domain_name";
-    public static final Setting.AffixSetting<String> AD_DOMAIN_NAME_SETTING
+    public static final Function<String, Setting.AffixSetting<String>> AD_DOMAIN_NAME_SETTING
-            = RealmSettings.simpleString(AD_TYPE, AD_DOMAIN_NAME_SETTING_KEY, Setting.Property.NodeScope);
+        = RealmSettings.affixSetting(AD_DOMAIN_NAME_SETTING_KEY,
+        key -> Setting.simpleString(key, v -> {
+            if (Strings.isNullOrEmpty(v)) {
+                throw new IllegalArgumentException("missing [" + key + "] setting for active directory");
+            }
+        }, Setting.Property.NodeScope));
 
     public static final String AD_GROUP_SEARCH_BASEDN_SETTING = "group_search.base_dn";
     public static final String AD_GROUP_SEARCH_SCOPE_SETTING = "group_search.scope";
@@ -71,7 +78,7 @@ public final class ActiveDirectorySessionFactorySettings {
     public static Set<Setting.AffixSetting<?>> getSettings() {
         Set<Setting.AffixSetting<?>> settings = new HashSet<>();
         settings.addAll(SessionFactorySettings.getSettings(AD_TYPE));
-        settings.add(AD_DOMAIN_NAME_SETTING);
+        settings.add(AD_DOMAIN_NAME_SETTING.apply(AD_TYPE));
         settings.add(RealmSettings.simpleString(AD_TYPE, AD_GROUP_SEARCH_BASEDN_SETTING, Setting.Property.NodeScope));
         settings.add(RealmSettings.simpleString(AD_TYPE, AD_GROUP_SEARCH_SCOPE_SETTING, Setting.Property.NodeScope));
         settings.add(AD_USER_SEARCH_BASEDN_SETTING);

x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/authc/SecurityRealmSettingsTests.java

@@ -66,6 +66,7 @@ public class SecurityRealmSettingsTests extends SecurityIntegTestCase {
                 .put("xpack.security.authc.realms.ldap.ldap1.url", "ldap://127.0.0.1:389")
                 .put("xpack.security.authc.realms.ldap.ldap1.user_dn_templates", "cn={0},dc=example,dc=com")
                 .put("xpack.security.authc.realms.active_directory.ad1.order", 4)
+                .put("xpack.security.authc.realms.active_directory.ad1.domain_name", "domain_name")
                 .put("xpack.security.authc.realms.active_directory.ad1.url", "ldap://127.0.0.1:389")
                 .put("xpack.security.authc.realms.pki.pki1.order", 5)
                 .put("xpack.security.authc.realms.saml.saml1.order", 6)

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ldap/ActiveDirectorySessionFactory.java

@@ -75,14 +75,9 @@ class ActiveDirectorySessionFactory extends PoolingSessionFactory {
                         }
                     }
                     return config.getSetting(ActiveDirectorySessionFactorySettings.AD_USER_SEARCH_BASEDN_SETTING,
-                            () -> config.getSetting(ActiveDirectorySessionFactorySettings.AD_DOMAIN_NAME_SETTING));
+                        () -> config.getSetting(ActiveDirectorySessionFactorySettings.AD_DOMAIN_NAME_SETTING));
                 }, threadPool);
         String domainName = config.getSetting(ActiveDirectorySessionFactorySettings.AD_DOMAIN_NAME_SETTING);
-        if (domainName == null) {
-            throw new IllegalArgumentException("missing [" +
-                    RealmSettings.getFullSettingKey(config, ActiveDirectorySessionFactorySettings.AD_DOMAIN_NAME_SETTING)
-                    + "] setting for active directory");
-        }
         String domainDN = buildDnFromDomain(domainName);
         final int ldapPort = config.getSetting(ActiveDirectorySessionFactorySettings.AD_LDAP_PORT_SETTING);
         final int ldapsPort = config.getSetting(ActiveDirectorySessionFactorySettings.AD_LDAPS_PORT_SETTING);

x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ldap/ActiveDirectoryRealmTests.java

@@ -490,7 +490,7 @@ public class ActiveDirectoryRealmTests extends ESTestCase {
     public void testBuildUrlFromDomainNameAndDefaultPort() throws Exception {
         final RealmConfig.RealmIdentifier realmId = realmId("testBuildUrlFromDomainNameAndDefaultPort");
         Settings settings = Settings.builder()
-            .put(getFullSettingKey(realmId.getName(), ActiveDirectorySessionFactorySettings.AD_DOMAIN_NAME_SETTING),
+            .put(getFullSettingKey(realmId, ActiveDirectorySessionFactorySettings.AD_DOMAIN_NAME_SETTING),
                 "ad.test.elasticsearch.com")
             .build();
         RealmConfig config = setupRealm(realmId, settings);
@@ -501,7 +501,7 @@ public class ActiveDirectoryRealmTests extends ESTestCase {
     public void testBuildUrlFromDomainNameAndCustomPort() throws Exception {
         final RealmConfig.RealmIdentifier realmId = realmId("testBuildUrlFromDomainNameAndCustomPort");
         Settings settings = Settings.builder()
-            .put(getFullSettingKey(realmId.getName(), ActiveDirectorySessionFactorySettings.AD_DOMAIN_NAME_SETTING),
+            .put(getFullSettingKey(realmId, ActiveDirectorySessionFactorySettings.AD_DOMAIN_NAME_SETTING),
                 "ad.test.elasticsearch.com")
             .put(getFullSettingKey(realmId.getName(), ActiveDirectorySessionFactorySettings.AD_LDAP_PORT_SETTING), 10389)
             .build();
@@ -513,7 +513,7 @@ public class ActiveDirectoryRealmTests extends ESTestCase {
     public void testUrlConfiguredInSettings() throws Exception {
         final RealmConfig.RealmIdentifier realmId = realmId("testBuildUrlFromDomainNameAndCustomPort");
         Settings settings = Settings.builder()
-            .put(getFullSettingKey(realmId.getName(), ActiveDirectorySessionFactorySettings.AD_DOMAIN_NAME_SETTING),
+            .put(getFullSettingKey(realmId, ActiveDirectorySessionFactorySettings.AD_DOMAIN_NAME_SETTING),
                 "ad.test.elasticsearch.com")
             .put(getFullSettingKey(realmId, SessionFactorySettings.URLS_SETTING), "ldap://ad01.testing.elastic.co:20389/")
             .build();
@@ -522,6 +522,19 @@ public class ActiveDirectoryRealmTests extends ESTestCase {
         assertSingleLdapServer(sessionFactory, "ad01.testing.elastic.co", 20389);
     }
 
+    public void testMandatorySettings() throws Exception {
+        final RealmConfig.RealmIdentifier realmId = realmId("testMandatorySettingsTestRealm");
+        Settings settings = Settings.builder()
+            .put(getFullSettingKey(realmId, ActiveDirectorySessionFactorySettings.AD_DOMAIN_NAME_SETTING),
+                randomBoolean() ? null : "")
+            .build();
+        RealmConfig config = setupRealm(realmId, settings);
+        IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
+            () -> new ActiveDirectorySessionFactory(config, sslService, threadPool));
+        assertThat(e.getMessage(), containsString(getFullSettingKey(realmId,
+            ActiveDirectorySessionFactorySettings.AD_DOMAIN_NAME_SETTING)));
+    }
+
     private void assertSingleLdapServer(ActiveDirectorySessionFactory sessionFactory, String hostname, int port) {
         assertThat(sessionFactory.getServerSet(), instanceOf(FailoverServerSet.class));
         FailoverServerSet fss = (FailoverServerSet) sessionFactory.getServerSet();
@@ -535,7 +548,7 @@ public class ActiveDirectoryRealmTests extends ESTestCase {
     private Settings settings(RealmConfig.RealmIdentifier realmIdentifier, Settings extraSettings) throws Exception {
         Settings.Builder builder = Settings.builder()
                 .putList(getFullSettingKey(realmIdentifier, URLS_SETTING), ldapUrls())
-                .put(getFullSettingKey(realmIdentifier.getName(), ActiveDirectorySessionFactorySettings.AD_DOMAIN_NAME_SETTING),
+                .put(getFullSettingKey(realmIdentifier, ActiveDirectorySessionFactorySettings.AD_DOMAIN_NAME_SETTING),
                         "ad.test.elasticsearch.com")
                 .put(getFullSettingKey(realmIdentifier, DnRoleMapperSettings.USE_UNMAPPED_GROUPS_AS_ROLES_SETTING), true);
         if (inFipsJvm()) {

x-pack/qa/third-party/active-directory/src/test/java/org/elasticsearch/xpack/security/authc/ldap/AbstractActiveDirectoryTestCase.java

@@ -98,7 +98,7 @@ public abstract class AbstractActiveDirectoryTestCase extends ESTestCase {
         final String realmName = realmId.getName();
         Settings.Builder builder = Settings.builder()
             .putList(getFullSettingKey(realmId, SessionFactorySettings.URLS_SETTING), ldapUrl)
-            .put(getFullSettingKey(realmName, ActiveDirectorySessionFactorySettings.AD_DOMAIN_NAME_SETTING), adDomainName)
+            .put(getFullSettingKey(realmId, ActiveDirectorySessionFactorySettings.AD_DOMAIN_NAME_SETTING), adDomainName)
             .put(getFullSettingKey(realmName, ActiveDirectorySessionFactorySettings.AD_USER_SEARCH_BASEDN_SETTING), userSearchDN)
             .put(getFullSettingKey(realmName, ActiveDirectorySessionFactorySettings.AD_USER_SEARCH_SCOPE_SETTING), scope)
             .put(getFullSettingKey(realmName, ActiveDirectorySessionFactorySettings.AD_LDAP_PORT_SETTING), AD_LDAP_PORT)

x-pack/qa/third-party/active-directory/src/test/java/org/elasticsearch/xpack/security/authc/ldap/ActiveDirectorySessionFactoryTests.java

@@ -373,7 +373,7 @@ public class ActiveDirectorySessionFactoryTests extends AbstractActiveDirectoryT
     private Settings buildAdSettings(String ldapUrl, String adDomainName, boolean hostnameVerification, boolean useBindUser) {
         Settings.Builder builder = Settings.builder()
                 .put(getFullSettingKey(REALM_ID, SessionFactorySettings.URLS_SETTING), ldapUrl)
-                .put(getFullSettingKey(REALM_NAME, ActiveDirectorySessionFactorySettings.AD_DOMAIN_NAME_SETTING), adDomainName)
+                .put(getFullSettingKey(REALM_ID, ActiveDirectorySessionFactorySettings.AD_DOMAIN_NAME_SETTING), adDomainName)
                 .put(getFullSettingKey(REALM_NAME, ActiveDirectorySessionFactorySettings.AD_LDAP_PORT_SETTING), AD_LDAP_PORT)
                 .put(getFullSettingKey(REALM_NAME, ActiveDirectorySessionFactorySettings.AD_LDAPS_PORT_SETTING), AD_LDAPS_PORT)
                 .put(getFullSettingKey(REALM_NAME, ActiveDirectorySessionFactorySettings.AD_GC_LDAP_PORT_SETTING), AD_GC_LDAP_PORT)