Skip to content

Commit

Permalink
Log deprecation message on legacy ldap pool settings (#2099)
Browse files Browse the repository at this point in the history
* Log deprecation message on legacy ldap pool settings

Signed-off-by: Peter Nied <[email protected]>
Signed-off-by: Peter Nied <[email protected]>
Co-authored-by: Martin Kemp <[email protected]>
  • Loading branch information
peternied and Martin Kemp authored Oct 6, 2022
1 parent ca4917c commit e5ab646
Show file tree
Hide file tree
Showing 6 changed files with 114 additions and 12 deletions.
8 changes: 4 additions & 4 deletions src/main/java/com/amazon/dlic/auth/ldap/LdapUser.java
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,12 @@ public class LdapUser extends User {
private final String originalUsername;

public LdapUser(final String name, String originalUsername, final LdapEntry userEntry,
final AuthCredentials credentials, int customAttrMaxValueLen, WildcardMatcher whitelistedCustomLdapAttrMatcher) {
final AuthCredentials credentials, int customAttrMaxValueLen, WildcardMatcher allowlistedCustomLdapAttrMatcher) {
super(name, null, credentials);
this.originalUsername = originalUsername;
this.userEntry = userEntry;
Map<String, String> attributes = getCustomAttributesMap();
attributes.putAll(extractLdapAttributes(originalUsername, userEntry, customAttrMaxValueLen, whitelistedCustomLdapAttrMatcher));
attributes.putAll(extractLdapAttributes(originalUsername, userEntry, customAttrMaxValueLen, allowlistedCustomLdapAttrMatcher));
}

/**
Expand All @@ -57,7 +57,7 @@ public String getOriginalUsername() {
}

public static Map<String, String> extractLdapAttributes(String originalUsername, final LdapEntry userEntry,
int customAttrMaxValueLen, WildcardMatcher whitelistedCustomLdapAttrMatcher) {
int customAttrMaxValueLen, WildcardMatcher allowlistedCustomLdapAttrMatcher) {
Map<String, String> attributes = new HashMap<>();
attributes.put("ldap.original.username", originalUsername);
attributes.put("ldap.dn", userEntry.getDn());
Expand All @@ -69,7 +69,7 @@ public static Map<String, String> extractLdapAttributes(String originalUsername,
// only consider attributes which are not binary and where its value is not
// longer than customAttrMaxValueLen characters
if (val != null && val.length() > 0 && val.length() <= customAttrMaxValueLen) {
if (whitelistedCustomLdapAttrMatcher.test(attr.getName())) {
if (allowlistedCustomLdapAttrMatcher.test(attr.getName())) {
attributes.put("attr.ldap." + attr.getName(), val);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@
import org.opensearch.security.user.AuthCredentials;
import org.opensearch.security.user.User;

import static org.opensearch.security.setting.DeprecatedSettings.checkForDeprecatedSetting;

public class LDAPAuthenticationBackend implements AuthenticationBackend {

static final int ZERO_PLACEHOLDER = 0;
Expand All @@ -56,7 +58,7 @@ public class LDAPAuthenticationBackend implements AuthenticationBackend {
private final Path configPath;
private final List<Map.Entry<String, Settings>> userBaseSettings;
private final int customAttrMaxValueLen;
private final WildcardMatcher whitelistedCustomLdapAttrMatcher;
private final WildcardMatcher allowlistedCustomLdapAttrMatcher;

private final String[] returnAttributes;

Expand All @@ -67,8 +69,10 @@ public LDAPAuthenticationBackend(final Settings settings, final Path configPath)
this.returnAttributes = settings.getAsList(ConfigConstants.LDAP_RETURN_ATTRIBUTES, Arrays.asList(ReturnAttributes.ALL.value())).toArray(new String[0]);

customAttrMaxValueLen = settings.getAsInt(ConfigConstants.LDAP_CUSTOM_ATTR_MAXVAL_LEN, 36);
whitelistedCustomLdapAttrMatcher = WildcardMatcher.from(settings.getAsList(ConfigConstants.LDAP_CUSTOM_ATTR_WHITELIST,
checkForDeprecatedSetting(settings, ConfigConstants.LDAP_CUSTOM_ATTR_WHITELIST, ConfigConstants.LDAP_CUSTOM_ATTR_ALLOWLIST);
final List<String> customAttrAllowList = settings.getAsList(ConfigConstants.LDAP_CUSTOM_ATTR_ALLOWLIST, settings.getAsList(ConfigConstants.LDAP_CUSTOM_ATTR_WHITELIST,
Collections.singletonList("*")));
allowlistedCustomLdapAttrMatcher = WildcardMatcher.from(customAttrAllowList);
}

@Override
Expand Down Expand Up @@ -127,9 +131,9 @@ public User authenticate(final AuthCredentials credentials) throws OpenSearchSec

// by default all ldap attributes which are not binary and with a max value
// length of 36 are included in the user object
// if the whitelist contains at least one value then all attributes will be
// additional check if whitelisted (whitelist can contain wildcard and regex)
return new LdapUser(username, user, entry, credentials, customAttrMaxValueLen, whitelistedCustomLdapAttrMatcher);
// if the allowlist contains at least one value then all attributes will be
// additional check if allowlisted (allowlist can contain wildcard and regex)
return new LdapUser(username, user, entry, credentials, customAttrMaxValueLen, allowlistedCustomLdapAttrMatcher);

} catch (final Exception e) {
if (log.isDebugEnabled()) {
Expand Down Expand Up @@ -164,7 +168,7 @@ public boolean exists(final User user) {
boolean exists = userEntry != null;

if(exists) {
user.addAttributes(LdapUser.extractLdapAttributes(userName, userEntry, customAttrMaxValueLen, whitelistedCustomLdapAttrMatcher));
user.addAttributes(LdapUser.extractLdapAttributes(userName, userEntry, customAttrMaxValueLen, allowlistedCustomLdapAttrMatcher));
}

return exists;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ public final class ConfigConstants {
// custom attributes
public static final String LDAP_CUSTOM_ATTR_MAXVAL_LEN = "custom_attr_maxval_len";
public static final String LDAP_CUSTOM_ATTR_WHITELIST = "custom_attr_whitelist";
public static final String LDAP_CUSTOM_ATTR_ALLOWLIST = "custom_attr_allowlist";
public static final String LDAP_RETURN_ATTRIBUTES = "custom_return_attributes";

public static final String LDAP_CONNECTION_STRATEGY = "connection_strategy";
Expand All @@ -83,6 +84,9 @@ public final class ConfigConstants {

public static final String LDAP_POOL_TYPE = "pool.type";

public static final String LDAP_LEGACY_POOL_PRUNING_PERIOD = "pruning.period";
public static final String LDAP_LEGACY_POOL_IDLE_TIME = "pruning.idleTime";

public static final String LDAP_POOL_PRUNING_PERIOD = "pool.pruning_period";
public static final String LDAP_POOL_IDLE_TIME = "pool.idle_time";

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,8 @@

import org.opensearch.common.settings.Settings;

import static org.opensearch.security.setting.DeprecatedSettings.checkForDeprecatedSetting;

public class LDAPConnectionFactoryFactory {

private static final Logger log = LogManager.getLogger(LDAPConnectionFactoryFactory.class);
Expand Down Expand Up @@ -127,10 +129,13 @@ public ConnectionPool createConnectionPool() {
}

result.setValidator(getConnectionValidator());

checkForDeprecatedSetting(settings, ConfigConstants.LDAP_LEGACY_POOL_PRUNING_PERIOD, ConfigConstants.LDAP_POOL_PRUNING_PERIOD);
checkForDeprecatedSetting(settings, ConfigConstants.LDAP_LEGACY_POOL_IDLE_TIME, ConfigConstants.LDAP_POOL_IDLE_TIME);

result.setPruneStrategy(new IdlePruneStrategy(
Duration.ofMinutes(this.settings.getAsLong(ConfigConstants.LDAP_POOL_PRUNING_PERIOD, this.settings.getAsLong("pruning.period", 5l))),
Duration.ofMinutes(this.settings.getAsLong(ConfigConstants.LDAP_POOL_IDLE_TIME, this.settings.getAsLong("pruning.idleTime", 10l))))
Duration.ofMinutes(this.settings.getAsLong(ConfigConstants.LDAP_POOL_PRUNING_PERIOD, this.settings.getAsLong(ConfigConstants.LDAP_LEGACY_POOL_PRUNING_PERIOD, 5l))),
Duration.ofMinutes(this.settings.getAsLong(ConfigConstants.LDAP_POOL_IDLE_TIME, this.settings.getAsLong(ConfigConstants.LDAP_LEGACY_POOL_IDLE_TIME, 10l))))
);

result.initialize();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/*
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*/

package org.opensearch.security.setting;

import org.opensearch.common.logging.DeprecationLogger;
import org.opensearch.common.settings.Settings;

/**
* Functionality around settings that have been deprecated
*/
public class DeprecatedSettings {

static DeprecationLogger DEPRECATION_LOGGER = DeprecationLogger.getLogger(DeprecatedSettings.class);

/**
* Checks for an deprecated key found in a setting, logs that it should be replaced with the another key
*/
public static void checkForDeprecatedSetting(final Settings settings, final String legacySettingKey, final String validSettingKey) {
if (settings.hasValue(legacySettingKey)) {
DEPRECATION_LOGGER.deprecate(legacySettingKey, "Found deprecated setting '{}', please replace with '{}'", legacySettingKey, validSettingKey);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
/*
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*/

package org.opensearch.security.setting;

import org.junit.After;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.Mock;
import org.mockito.junit.MockitoJUnitRunner;

import org.opensearch.common.logging.DeprecationLogger;
import org.opensearch.common.settings.Settings;

import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyNoInteractions;
import static org.mockito.Mockito.verifyNoMoreInteractions;
import static org.opensearch.security.setting.DeprecatedSettings.checkForDeprecatedSetting;

@RunWith(MockitoJUnitRunner.class)
public class DeprecatedSettingsTest {

@Mock
private DeprecationLogger logger;

private DeprecationLogger original;

@Before
public void before() {
original = DeprecatedSettings.DEPRECATION_LOGGER;
DeprecatedSettings.DEPRECATION_LOGGER = logger;
}

@After
public void after() {
DeprecatedSettings.DEPRECATION_LOGGER = original;
verifyNoMoreInteractions(logger);
}

@Test
public void testCheckForDeprecatedSettingNoLegacy() {
final Settings settings = Settings.builder().put("properKey", "value").build();

checkForDeprecatedSetting(settings, "legacyKey", "properKey");

verifyNoInteractions(logger);
}

@Test
public void testCheckForDeprecatedSettingFoundLegacy() {
final Settings settings = Settings.builder().put("legacyKey", "value").build();

checkForDeprecatedSetting(settings, "legacyKey", "properKey");

verify(logger).deprecate(eq("legacyKey"), anyString(), any());
}
}

0 comments on commit e5ab646

Please sign in to comment.