Skip to content

Commit

Permalink
Ensure domain_name setting for AD realm is present (#61983) (#63160)
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@jkakavas
jkakavas committed Oct 2, 2020
1 parent 6b26f7d commit 3a62004
Show file tree
Hide file tree
Showing 6 changed files with 31 additions and 15 deletions.
13 changes: 10 additions & 3 deletions ...g/elasticsearch/xpack/core/security/authc/ldap/ActiveDirectorySessionFactorySettings.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
= RealmSettings.simpleString(AD_TYPE, AD_DOMAIN_NAME_SETTING_KEY, Setting.Property.NodeScope);
public static final Function<String, Setting.AffixSetting<String>> AD_DOMAIN_NAME_SETTING
= 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";
Expand Down Expand Up @@ -71,7 +78,7 @@ private 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);
Expand Down
7 changes: 1 addition & 6 deletions .../main/java/org/elasticsearch/xpack/security/authc/ldap/ActiveDirectorySessionFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
1 change: 1 addition & 0 deletions ...rity/src/test/java/org/elasticsearch/xpack/security/authc/SecurityRealmSettingsTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ protected Settings nodeSettings(int nodeOrdinal) {
.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)
Expand Down
21 changes: 17 additions & 4 deletions .../src/test/java/org/elasticsearch/xpack/security/authc/ldap/ActiveDirectoryRealmTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -490,7 +490,7 @@ private Settings settings(RealmConfig.RealmIdentifier realmIdentifier) throws Ex
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);
Expand All @@ -501,7 +501,7 @@ public void testBuildUrlFromDomainNameAndDefaultPort() throws Exception {
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();
Expand All @@ -513,7 +513,7 @@ public void testBuildUrlFromDomainNameAndCustomPort() throws Exception {
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();
Expand All @@ -522,6 +522,19 @@ public void testUrlConfiguredInSettings() throws Exception {
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();
Expand All @@ -535,7 +548,7 @@ private void assertSingleLdapServer(ActiveDirectorySessionFactory sessionFactory
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()) {
Expand Down
2 changes: 1 addition & 1 deletion ...est/java/org/elasticsearch/xpack/security/authc/ldap/AbstractActiveDirectoryTestCase.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ Settings buildAdSettings(RealmConfig.RealmIdentifier realmId, String ldapUrl, St
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)
Expand Down
2 changes: 1 addition & 1 deletion .../java/org/elasticsearch/xpack/security/authc/ldap/ActiveDirectorySessionFactoryTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,7 @@ private Settings buildAdSettings(String ldapUrl, String adDomainName, boolean ho
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)
Expand Down

0 comments on commit 3a62004

Please sign in to comment.