Skip to content

Commit

Permalink
Refactor and share use of deprecationn logger
Browse files Browse the repository at this point in the history
Signed-off-by: Peter Nied <[email protected]>
  • Loading branch information
peternied committed Sep 28, 2022
1 parent 3356ade commit db3b7f5
Show file tree
Hide file tree
Showing 7 changed files with 112 additions and 23 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@
import org.opensearch.security.auth.HTTPAuthenticator;
import org.opensearch.security.user.AuthCredentials;

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

public abstract class AbstractHTTPJwtAuthenticator implements HTTPAuthenticator {
private final static Logger log = LogManager.getLogger(AbstractHTTPJwtAuthenticator.class);

Expand Down
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 @@ -43,6 +43,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 @@ -55,16 +57,18 @@ 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;

public LDAPAuthenticationBackend(final Settings settings, final Path configPath) {
this.settings = settings;
this.configPath = configPath;
this.userBaseSettings = getUserBaseSettings(settings);

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 @@ -123,9 +127,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 @@ -160,7 +164,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_CONNECTION_STRATEGY = "connection_strategy";

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,13 +60,13 @@
import com.amazon.dlic.util.SettingsBasedSSLConfigurator;
import com.amazon.dlic.util.SettingsBasedSSLConfigurator.SSLConfigException;

import org.opensearch.common.logging.DeprecationLogger;
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);
private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(LDAPConnectionFactoryFactory.class);


private final Settings settings;
Expand Down Expand Up @@ -131,8 +131,8 @@ public ConnectionPool createConnectionPool() {

result.setValidator(getConnectionValidator());

LDAPConnectionFactoryFactory.checkForDeprecatedSetting(settings, ConfigConstants.LDAP_LEGACY_POOL_PRUNING_PERIOD, ConfigConstants.LDAP_POOL_PRUNING_PERIOD);
LDAPConnectionFactoryFactory.checkForDeprecatedSetting(settings, ConfigConstants.LDAP_LEGACY_POOL_IDLE_TIME, ConfigConstants.LDAP_POOL_IDLE_TIME);
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(ConfigConstants.LDAP_LEGACY_POOL_PRUNING_PERIOD, 5l))),
Expand All @@ -144,15 +144,6 @@ public ConnectionPool createConnectionPool() {
return result;
}

/**
* Checks for an deprecated key found in a setting, logs that it should be replaced with the another key
*/
private static void checkForDeprecatedSetting(final Settings settings, final String legacySettingKey, final String validSettingKey) {
if (settings.hasValue(legacySettingKey)) {
deprecationLogger.deprecate(legacySettingKey, "Found deprecated setting '{}', please replace with '{}'", legacySettingKey, validSettingKey);
}
}

private ConnectionConfig getConnectionConfig() {
ConnectionConfig result = new ConnectionConfig(getLdapUrlString());

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
/*
* SPDX-License-Identifier: Apache-2.0
*
* The OpenSearch Contributors require contributions made to
* this file be licensed under the Apache-2.0 license or a
* compatible open source license.
*
* Modifications Copyright OpenSearch Contributors. See
* GitHub history for details.
*/
package org.opensearch.security.setting;

import com.google.common.annotations.VisibleForTesting;

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,58 @@
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 db3b7f5

Please sign in to comment.