Skip to content

Commit

Permalink
Allow custom return attributes (opensearch-project#2093) (opensearch-…
Browse files Browse the repository at this point in the history
…project#2110)

* Allow custom return attributes

Signed-off-by: Martin Kemp <martin_leon.kemp@mercedes-benz.com>
(cherry picked from commit b9b7e1f)

Co-authored-by: Martin Kemp <martinkempsa@gmail.com>
  • Loading branch information
opensearch-trigger-bot[bot] and Martin-Kemp authored Sep 27, 2022
1 parent 2a60b5b commit 2303402
Show file tree
Hide file tree
Showing 11 changed files with 121 additions and 40 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import org.ldaptive.Connection;
import org.ldaptive.ConnectionConfig;
import org.ldaptive.LdapEntry;
import org.ldaptive.ReturnAttributes;
import org.ldaptive.SearchFilter;
import org.ldaptive.SearchScope;

Expand Down Expand Up @@ -57,10 +58,13 @@ public class LDAPAuthenticationBackend implements AuthenticationBackend {
private final int customAttrMaxValueLen;
private final WildcardMatcher whitelistedCustomLdapAttrMatcher;

private final String[] returnAttributes;

public LDAPAuthenticationBackend(final Settings settings, final Path configPath) {
this.settings = settings;
this.configPath = configPath;
this.userBaseSettings = getUserBaseSettings(settings);
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,
Expand All @@ -82,7 +86,7 @@ public User authenticate(final AuthCredentials credentials) throws OpenSearchSec
try {
ldapConnection = LDAPAuthorizationBackend.getConnection(settings, configPath);

entry = exists(user, ldapConnection, settings, userBaseSettings);
entry = exists(user, ldapConnection, settings, userBaseSettings, this.returnAttributes);

// fake a user that no exists
// makes guessing if a user exists or not harder when looking on the
Expand Down Expand Up @@ -156,7 +160,7 @@ public boolean exists(final User user) {

try {
ldapConnection = LDAPAuthorizationBackend.getConnection(settings, configPath);
LdapEntry userEntry = exists(userName, ldapConnection, settings, userBaseSettings);
LdapEntry userEntry = exists(userName, ldapConnection, settings, userBaseSettings, this.returnAttributes);
boolean exists = userEntry != null;

if(exists) {
Expand Down Expand Up @@ -197,20 +201,19 @@ static List<Map.Entry<String, Settings>> getUserBaseSettings(Settings settings)
}

static LdapEntry exists(final String user, Connection ldapConnection, Settings settings,
List<Map.Entry<String, Settings>> userBaseSettings) throws Exception {

List<Map.Entry<String, Settings>> userBaseSettings, String[] returnAttributes) throws Exception {
if (settings.getAsBoolean(ConfigConstants.LDAP_FAKE_LOGIN_ENABLED, false)
|| settings.getAsBoolean(ConfigConstants.LDAP_SEARCH_ALL_BASES, false)
|| settings.hasValue(ConfigConstants.LDAP_AUTHC_USERBASE)) {
return existsSearchingAllBases(user, ldapConnection, userBaseSettings);
return existsSearchingAllBases(user, ldapConnection, userBaseSettings, returnAttributes);
} else {
return existsSearchingUntilFirstHit(user, ldapConnection, userBaseSettings);
return existsSearchingUntilFirstHit(user, ldapConnection, userBaseSettings, returnAttributes);
}

}

private static LdapEntry existsSearchingUntilFirstHit(final String user, Connection ldapConnection,
List<Map.Entry<String, Settings>> userBaseSettings) throws Exception {
List<Map.Entry<String, Settings>> userBaseSettings, final String[] returnAttributes) throws Exception {
final String username = user;

final boolean isDebugEnabled = log.isDebugEnabled();
Expand All @@ -224,7 +227,8 @@ private static LdapEntry existsSearchingUntilFirstHit(final String user, Connect
List<LdapEntry> result = LdapHelper.search(ldapConnection,
baseSettings.get(ConfigConstants.LDAP_AUTHCZ_BASE, DEFAULT_USERBASE),
f,
SearchScope.SUBTREE);
SearchScope.SUBTREE,
returnAttributes);

if (isDebugEnabled) {
log.debug("Results for LDAP search for {} in base {} is {}", user, entry.getKey(), result);
Expand All @@ -239,7 +243,7 @@ private static LdapEntry existsSearchingUntilFirstHit(final String user, Connect
}

private static LdapEntry existsSearchingAllBases(final String user, Connection ldapConnection,
List<Map.Entry<String, Settings>> userBaseSettings) throws Exception {
List<Map.Entry<String, Settings>> userBaseSettings, final String[] returnAttributes) throws Exception {
final String username = user;
Set<LdapEntry> result = new HashSet<>();

Expand All @@ -254,7 +258,8 @@ private static LdapEntry existsSearchingAllBases(final String user, Connection l
List<LdapEntry> foundEntries = LdapHelper.search(ldapConnection,
baseSettings.get(ConfigConstants.LDAP_AUTHCZ_BASE, DEFAULT_USERBASE),
f,
SearchScope.SUBTREE);
SearchScope.SUBTREE,
returnAttributes);

if (isDebugEnabled) {
log.debug("Results for LDAP search for " + user + " in base " + entry.getKey() + ":\n" + result);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
import org.ldaptive.LdapEntry;
import org.ldaptive.LdapException;
import org.ldaptive.Response;
import org.ldaptive.ReturnAttributes;
import org.ldaptive.SearchFilter;
import org.ldaptive.SearchScope;
import org.ldaptive.control.RequestControl;
Expand Down Expand Up @@ -103,6 +104,8 @@ public class LDAPAuthorizationBackend implements AuthorizationBackend {
private final List<Map.Entry<String, Settings>> roleBaseSettings;
private final List<Map.Entry<String, Settings>> userBaseSettings;

private final String[] returnAttributes;

public LDAPAuthorizationBackend(final Settings settings, final Path configPath) {
this.settings = settings;
this.skipUsersMatcher = WildcardMatcher.from(settings.getAsList(ConfigConstants.LDAP_AUTHZ_SKIP_USERS));
Expand All @@ -111,6 +114,7 @@ public LDAPAuthorizationBackend(final Settings settings, final Path configPath)
this.configPath = configPath;
this.roleBaseSettings = getRoleSearchSettings(settings);
this.userBaseSettings = LDAPAuthenticationBackend.getUserBaseSettings(settings);
this.returnAttributes = settings.getAsList(ConfigConstants.LDAP_RETURN_ATTRIBUTES, Arrays.asList(ReturnAttributes.ALL.value())).toArray(new String[0]);
}

@SuppressWarnings("removal")
Expand Down Expand Up @@ -724,7 +728,7 @@ public void fillRoles(final User user, final AuthCredentials optionalAuthCreds)
log.debug("DBGTRACE (4): authenticatedUser="+authenticatedUser+" -> "+Arrays.toString(authenticatedUser.getBytes(StandardCharsets.UTF_8)));
}

entry = LdapHelper.lookup(connection, authenticatedUser);
entry = LdapHelper.lookup(connection, authenticatedUser, this.returnAttributes);

if (entry == null) {
throw new OpenSearchSecurityException("No user '" + authenticatedUser + "' found");
Expand All @@ -735,7 +739,7 @@ public void fillRoles(final User user, final AuthCredentials optionalAuthCreds)
if (isDebugEnabled)
log.debug("DBGTRACE (5): authenticatedUser="+user.getName()+" -> "+Arrays.toString(user.getName().getBytes(StandardCharsets.UTF_8)));

entry = LDAPAuthenticationBackend.exists(user.getName(), connection, settings, userBaseSettings);
entry = LDAPAuthenticationBackend.exists(user.getName(), connection, settings, userBaseSettings, this.returnAttributes);

if (isTraceEnabled) {
log.trace("{} is not a valid DN and was resolved to {}", authenticatedUser, entry);
Expand Down Expand Up @@ -848,7 +852,7 @@ public void fillRoles(final User user, final AuthCredentials optionalAuthCreds)
List<LdapEntry> rolesResult = LdapHelper.search(connection,
roleSearchSettings.get(ConfigConstants.LDAP_AUTHCZ_BASE, DEFAULT_ROLEBASE),
f,
SearchScope.SUBTREE);
SearchScope.SUBTREE, this.returnAttributes);

if (isTraceEnabled) {
log.trace("Results for LDAP group search for {} in base {}:\n{}", escapedDn, roleSearchSettingsEntry.getKey(), rolesResult);
Expand Down Expand Up @@ -966,7 +970,7 @@ protected Set<LdapName> resolveNestedRoles(final LdapName roleDn, final Connecti
final Set<LdapName> result = new HashSet<>(20);
final HashMultimap<LdapName, Map.Entry<String, Settings>> resultRoleSearchBaseKeys = HashMultimap.create();

final LdapEntry e0 = LdapHelper.lookup(ldapConnection, roleDn.toString());
final LdapEntry e0 = LdapHelper.lookup(ldapConnection, roleDn.toString(), this.returnAttributes);

if (e0.getAttribute(userRoleName) != null) {
final Collection<String> userRoles = e0.getAttribute(userRoleName).getStringValues();
Expand Down Expand Up @@ -1018,7 +1022,8 @@ protected Set<LdapName> resolveNestedRoles(final LdapName roleDn, final Connecti
List<LdapEntry> foundEntries = LdapHelper.search(ldapConnection,
roleSearchSettings.get(ConfigConstants.LDAP_AUTHCZ_BASE, DEFAULT_ROLEBASE),
f,
SearchScope.SUBTREE);
SearchScope.SUBTREE,
this.returnAttributes);

if (isTraceEnabled) {
log.trace("Results for LDAP group search for {} in base {}:\n{}", escapedDn, roleSearchBaseSettingsEntry.getKey(), foundEntries);
Expand Down Expand Up @@ -1096,7 +1101,7 @@ private String getRoleFromEntry(final Connection ldapConnection, final LdapName
}

try {
final LdapEntry roleEntry = LdapHelper.lookup(ldapConnection, ldapName.toString());
final LdapEntry roleEntry = LdapHelper.lookup(ldapConnection, ldapName.toString(), this.returnAttributes);

if(roleEntry != null) {
final LdapAttribute roleAttribute = roleEntry.getAttribute(role);
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_RETURN_ATTRIBUTES = "custom_return_attributes";

public static final String LDAP_CONNECTION_STRATEGY = "connection_strategy";

Expand Down
9 changes: 4 additions & 5 deletions src/main/java/com/amazon/dlic/auth/ldap/util/LdapHelper.java
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
import org.ldaptive.LdapEntry;
import org.ldaptive.LdapException;
import org.ldaptive.Response;
import org.ldaptive.ReturnAttributes;
import org.ldaptive.SearchFilter;
import org.ldaptive.SearchOperation;
import org.ldaptive.SearchRequest;
Expand All @@ -41,7 +40,7 @@ public class LdapHelper {
private static SearchFilter ALL = new SearchFilter("(objectClass=*)");
@SuppressWarnings("removal")
public static List<LdapEntry> search(final Connection conn, final String unescapedDn, SearchFilter filter,
final SearchScope searchScope) throws LdapException {
final SearchScope searchScope, final String[] returnAttributes) throws LdapException {

final SecurityManager sm = System.getSecurityManager();

Expand All @@ -59,7 +58,7 @@ public List<LdapEntry> run() throws Exception {
request.setReferralHandler(new SearchReferralHandler());
request.setSearchScope(searchScope);
request.setDerefAliases(DerefAliases.ALWAYS);
request.setReturnAttributes(ReturnAttributes.ALL.value());
request.setReturnAttributes(returnAttributes);
final SearchOperation search = new SearchOperation(conn);
// referrals will be followed to build the response
final Response<SearchResult> r = search.execute(request);
Expand All @@ -81,9 +80,9 @@ public List<LdapEntry> run() throws Exception {
}
}

public static LdapEntry lookup(final Connection conn, final String unescapedDn) throws LdapException {
public static LdapEntry lookup(final Connection conn, final String unescapedDn, final String[] returnAttributes) throws LdapException {

final List<LdapEntry> entries = search(conn, unescapedDn, ALL, SearchScope.OBJECT);
final List<LdapEntry> entries = search(conn, unescapedDn, ALL, SearchScope.OBJECT, returnAttributes);

if (entries.size() == 1) {
return entries.get(0);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import org.ldaptive.LdapEntry;
import org.ldaptive.LdapException;
import org.ldaptive.Response;
import org.ldaptive.ReturnAttributes;
import org.ldaptive.pool.ConnectionPool;

import com.amazon.dlic.auth.ldap.LdapUser;
Expand Down Expand Up @@ -58,6 +59,7 @@ public class LDAPAuthenticationBackend2 implements AuthenticationBackend, Destro
private LDAPUserSearcher userSearcher;
private final int customAttrMaxValueLen;
private final WildcardMatcher whitelistedCustomLdapAttrMatcher;
private final String[] returnAttributes;

public LDAPAuthenticationBackend2(final Settings settings, final Path configPath) throws SSLConfigException {
this.settings = settings;
Expand All @@ -75,6 +77,7 @@ public LDAPAuthenticationBackend2(final Settings settings, final Path configPath
}

this.userSearcher = new LDAPUserSearcher(settings);
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,
Collections.singletonList("*")));
Expand Down Expand Up @@ -119,7 +122,7 @@ private User authenticate0(final AuthCredentials credentials) throws OpenSearchS
ldapConnection = connectionFactory.getConnection();
ldapConnection.open();

LdapEntry entry = userSearcher.exists(ldapConnection, user);
LdapEntry entry = userSearcher.exists(ldapConnection, user, this.returnAttributes);

// fake a user that no exists
// makes guessing if a user exists or not harder when looking on the
Expand Down Expand Up @@ -211,7 +214,7 @@ private boolean exists0(final User user) {
try {
ldapConnection = this.connectionFactory.getConnection();
ldapConnection.open();
LdapEntry userEntry = this.userSearcher.exists(ldapConnection, userName);
LdapEntry userEntry = this.userSearcher.exists(ldapConnection, userName, this.returnAttributes);

boolean exists = userEntry != null;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import java.security.AccessController;
import java.security.PrivilegedActionException;
import java.security.PrivilegedExceptionAction;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
Expand All @@ -35,6 +36,7 @@
import org.ldaptive.LdapAttribute;
import org.ldaptive.LdapEntry;
import org.ldaptive.LdapException;
import org.ldaptive.ReturnAttributes;
import org.ldaptive.SearchFilter;
import org.ldaptive.SearchScope;
import org.ldaptive.pool.ConnectionPool;
Expand Down Expand Up @@ -73,6 +75,7 @@ public class LDAPAuthorizationBackend2 implements AuthorizationBackend, Destroya
private ConnectionPool connectionPool;
private ConnectionFactory connectionFactory;
private LDAPUserSearcher userSearcher;
private final String[] returnAttributes;

public LDAPAuthorizationBackend2(final Settings settings, final Path configPath) throws SSLConfigException {
this.settings = settings;
Expand All @@ -87,6 +90,7 @@ public LDAPAuthorizationBackend2(final Settings settings, final Path configPath)
this.connectionPool = ldapConnectionFactoryFactory.createConnectionPool();
this.connectionFactory = ldapConnectionFactoryFactory.createConnectionFactory(this.connectionPool);
this.userSearcher = new LDAPUserSearcher(settings);
this.returnAttributes = settings.getAsList(ConfigConstants.LDAP_RETURN_ATTRIBUTES, Arrays.asList(ReturnAttributes.ALL.value())).toArray(new String[0]);
}

private static List<Map.Entry<String, Settings>> getRoleSearchSettings(Settings settings) {
Expand Down Expand Up @@ -203,14 +207,14 @@ private void fillRoles0(final User user, final AuthCredentials optionalAuthCreds
log.trace("{} is a valid DN", authenticatedUser);
}

entry = LdapHelper.lookup(connection, authenticatedUser);
entry = LdapHelper.lookup(connection, authenticatedUser, this.returnAttributes);

if (entry == null) {
throw new OpenSearchSecurityException("No user '" + authenticatedUser + "' found");
}

} else {
entry = this.userSearcher.exists(connection, user.getName());
entry = this.userSearcher.exists(connection, user.getName(), this.returnAttributes);

if (isTraceEnabled) {
log.trace("{} is not a valid DN and was resolved to {}", authenticatedUser, entry);
Expand Down Expand Up @@ -309,7 +313,8 @@ private void fillRoles0(final User user, final AuthCredentials optionalAuthCreds
List<LdapEntry> rolesResult = LdapHelper.search(connection,
roleSearchSettings.get(ConfigConstants.LDAP_AUTHCZ_BASE, DEFAULT_ROLEBASE),
f,
SearchScope.SUBTREE);
SearchScope.SUBTREE,
this.returnAttributes);

if (isTraceEnabled) {
log.trace("Results for LDAP group search for {} in base {}:\n{}", escapedDn, roleSearchSettingsEntry.getKey(), rolesResult);
Expand Down Expand Up @@ -425,7 +430,7 @@ protected Set<LdapName> resolveNestedRoles(final LdapName roleDn, final Connecti
final Set<LdapName> result = new HashSet<>(20);
final HashMultimap<LdapName, Map.Entry<String, Settings>> resultRoleSearchBaseKeys = HashMultimap.create();

final LdapEntry e0 = LdapHelper.lookup(ldapConnection, roleDn.toString());
final LdapEntry e0 = LdapHelper.lookup(ldapConnection, roleDn.toString(), this.returnAttributes);
final boolean isDebugEnabled = log.isDebugEnabled();

if (e0.getAttribute(userRoleName) != null) {
Expand Down Expand Up @@ -466,7 +471,8 @@ protected Set<LdapName> resolveNestedRoles(final LdapName roleDn, final Connecti
List<LdapEntry> foundEntries = LdapHelper.search(ldapConnection,
roleSearchSettings.get(ConfigConstants.LDAP_AUTHCZ_BASE, DEFAULT_ROLEBASE),
f,
SearchScope.SUBTREE);
SearchScope.SUBTREE,
this.returnAttributes);

if (isTraceEnabled) {
log.trace("Results for LDAP group search for {} in base {}:\n{}", escapedDn, roleSearchBaseSettingsEntry.getKey(), foundEntries);
Expand Down Expand Up @@ -544,7 +550,7 @@ private String getRoleFromEntry(final Connection ldapConnection, final LdapName
}

try {
final LdapEntry roleEntry = LdapHelper.lookup(ldapConnection, ldapName.toString());
final LdapEntry roleEntry = LdapHelper.lookup(ldapConnection, ldapName.toString(), this.returnAttributes);

if(roleEntry != null) {
final LdapAttribute roleAttribute = roleEntry.getAttribute(role);
Expand Down
Loading

0 comments on commit 2303402

Please sign in to comment.