Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add exclude_roles configuration parameter to LDAP authorization backend #20

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Open

Add exclude_roles configuration parameter to LDAP authorization backend #20

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
87ef963
filter ldap roles
MaciejMierzwa Dec 6, 2023
20fd635
extract method
MaciejMierzwa Dec 7, 2023
71eff37
spotless
MaciejMierzwa Dec 7, 2023
eaabbc9
extract method
MaciejMierzwa Dec 8, 2023
dcd2479
Merge branch 'main' into hide-ldap-roles
cwperks Feb 5, 2024
3588fed
Make exclude_roles a list to match configuration for skip_users
cwperks Feb 5, 2024
8bbd8cc
Add test with wildcard
cwperks Feb 5, 2024
6b90a51
Update message
cwperks Feb 6, 2024
779889b
Use corresponding backend
cwperks Feb 7, 2024
41e5286
Remove redundant test
cwperks Feb 7, 2024
6a32580
Change to debug
cwperks Feb 7, 2024
c19fe0c
Handle null pattern in WildcardMatcher
cwperks Feb 8, 2024
8141cd5
Add test for list containing null values
cwperks Feb 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 12 additions & 6 deletions src/main/java/com/amazon/dlic/auth/ldap/backend/LDAPAuthorizationBackend.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ public class LDAPAuthorizationBackend implements AuthorizationBackend {
protected static final Logger log = LogManager.getLogger(LDAPAuthorizationBackend.class);
private final Settings settings;
private final WildcardMatcher skipUsersMatcher;
private final WildcardMatcher excludeRolesMatcher;
private final WildcardMatcher nestedRoleMatcher;
private final Path configPath;
private final List<Map.Entry<String, Settings>> roleBaseSettings;
Expand All @@ -112,6 +113,7 @@ public class LDAPAuthorizationBackend implements AuthorizationBackend {
public LDAPAuthorizationBackend(final Settings settings, final Path configPath) {
this.settings = settings;
this.skipUsersMatcher = WildcardMatcher.from(settings.getAsList(ConfigConstants.LDAP_AUTHZ_SKIP_USERS));
this.excludeRolesMatcher = WildcardMatcher.from(settings.getAsList(ConfigConstants.LDAP_AUTHZ_EXCLUDE_ROLES));
this.nestedRoleMatcher = settings.getAsBoolean(ConfigConstants.LDAP_AUTHZ_RESOLVE_NESTED_ROLES, false)
? WildcardMatcher.from(settings.getAsList(ConfigConstants.LDAP_AUTHZ_NESTEDROLEFILTER))
: null;
Expand Down Expand Up @@ -962,10 +964,12 @@ public void fillRoles(final User user, final AuthCredentials optionalAuthCreds)
for (final LdapName roleLdapName : nestedReturn) {
final String role = getRoleFromEntry(connection, roleLdapName, roleName);

if (!Strings.isNullOrEmpty(role)) {
user.addRole(role);
if (excludeRolesMatcher.test(role)) {
if (isDebugEnabled) {
log.debug("Role was excluded or empty, attribute: '{}' for entry: {}", roleName, roleLdapName);
}
} else {
log.warn("No or empty attribute '{}' for entry {}", roleName, roleLdapName);
user.addRole(role);
}
}

Expand All @@ -974,10 +978,12 @@ public void fillRoles(final User user, final AuthCredentials optionalAuthCreds)
for (final LdapName roleLdapName : ldapRoles) {
final String role = getRoleFromEntry(connection, roleLdapName, roleName);

if (!Strings.isNullOrEmpty(role)) {
user.addRole(role);
if (excludeRolesMatcher.test(role)) {
if (isDebugEnabled) {
log.debug("Role was excluded or empty, attribute: '{}' for entry: {}", roleName, roleLdapName);
}
} else {
log.warn("No or empty attribute '{}' for entry {}", roleName, roleLdapName);
user.addRole(role);
}
}

Expand Down
1 change: 1 addition & 0 deletions src/main/java/com/amazon/dlic/auth/ldap/util/ConfigConstants.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ public final class ConfigConstants {
public static final String LDAP_AUTHZ_USERROLEATTRIBUTE = "userroleattribute";// multi-value
public static final String LDAP_AUTHZ_USERROLENAME = "userrolename";// multi-value
public static final String LDAP_AUTHZ_SKIP_USERS = "skip_users";
public static final String LDAP_AUTHZ_EXCLUDE_ROLES = "exclude_roles";
public static final String LDAP_AUTHZ_ROLESEARCH_ENABLED = "rolesearch_enabled";
public static final String LDAP_AUTHZ_NESTEDROLEFILTER = "nested_role_filter";
public static final String LDAP_AUTHZ_MAX_NESTED_DEPTH = "max_nested_depth";
Expand Down
1 change: 0 additions & 1 deletion src/main/java/com/amazon/dlic/auth/ldap/util/LdapHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,6 @@ private static Object escapeForwardSlash(Object input) {
} else {
return input;
}

}

}
24 changes: 16 additions & 8 deletions src/main/java/com/amazon/dlic/auth/ldap2/LDAPAuthorizationBackend2.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ public class LDAPAuthorizationBackend2 implements AuthorizationBackend, Destroya
protected static final Logger log = LogManager.getLogger(LDAPAuthorizationBackend2.class);
private final Settings settings;
private final WildcardMatcher skipUsersMatcher;
private final WildcardMatcher excludeRolesMatcher;
private final WildcardMatcher nestedRoleMatcher;
private final List<Map.Entry<String, Settings>> roleBaseSettings;
private ConnectionPool connectionPool;
Expand All @@ -80,6 +81,7 @@ public class LDAPAuthorizationBackend2 implements AuthorizationBackend, Destroya
public LDAPAuthorizationBackend2(final Settings settings, final Path configPath) throws SSLConfigException {
this.settings = settings;
this.skipUsersMatcher = WildcardMatcher.from(settings.getAsList(ConfigConstants.LDAP_AUTHZ_SKIP_USERS));
this.excludeRolesMatcher = WildcardMatcher.from(settings.getAsList(ConfigConstants.LDAP_AUTHZ_EXCLUDE_ROLES));
this.nestedRoleMatcher = settings.getAsBoolean(ConfigConstants.LDAP_AUTHZ_RESOLVE_NESTED_ROLES, false)
? WildcardMatcher.from(settings.getAsList(ConfigConstants.LDAP_AUTHZ_NESTEDROLEFILTER))
: null;
Expand Down Expand Up @@ -329,8 +331,10 @@ private void fillRoles0(final User user, final AuthCredentials optionalAuthCreds
for (final Iterator<LdapEntry> iterator = rolesResult.iterator(); iterator.hasNext();) {
LdapEntry searchResultEntry = iterator.next();
LdapName ldapName = new LdapName(searchResultEntry.getDn());
ldapRoles.add(ldapName);
resultRoleSearchBaseKeys.put(ldapName, roleSearchSettingsEntry);
if (!excludeRolesMatcher.test(searchResultEntry.getDn())) {
ldapRoles.add(ldapName);
resultRoleSearchBaseKeys.put(ldapName, roleSearchSettingsEntry);
}
}
}
}
Expand Down Expand Up @@ -376,10 +380,12 @@ private void fillRoles0(final User user, final AuthCredentials optionalAuthCreds
for (final LdapName roleLdapName : nestedReturn) {
final String role = getRoleFromEntry(connection, roleLdapName, roleName);

if (!Strings.isNullOrEmpty(role)) {
user.addRole(role);
if (excludeRolesMatcher.test(role)) {
if (isDebugEnabled) {
log.debug("Role was excluded or empty attribute '{}' for entry {}", roleName, roleLdapName);
}
} else {
log.warn("No or empty attribute '{}' for entry {}", roleName, roleLdapName);
user.addRole(role);
}
}

Expand All @@ -388,10 +394,12 @@ private void fillRoles0(final User user, final AuthCredentials optionalAuthCreds
for (final LdapName roleLdapName : ldapRoles) {
final String role = getRoleFromEntry(connection, roleLdapName, roleName);

if (!Strings.isNullOrEmpty(role)) {
user.addRole(role);
if (excludeRolesMatcher.test(role)) {
if (isDebugEnabled) {
log.debug("Role was excluded or empty attribute '{}' for entry {}", roleName, roleLdapName);
}
} else {
log.warn("No or empty attribute '{}' for entry {}", roleName, roleLdapName);
user.addRole(role);
}
}

Expand Down
8 changes: 6 additions & 2 deletions src/main/java/org/opensearch/security/support/WildcardMatcher.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,9 @@ public String toString() {
};

public static WildcardMatcher from(String pattern, boolean caseSensitive) {
if (pattern.equals("*")) {
if (pattern == null) {
return NONE;
} else if (pattern.equals("*")) {
return ANY;
} else if (pattern.startsWith("/") && pattern.endsWith("/")) {
return new RegexMatcher(pattern, caseSensitive);
Expand All @@ -168,7 +170,9 @@ public static WildcardMatcher from(String pattern) {
// This may in future use more optimized techniques to combine multiple WildcardMatchers in a single automaton
public static <T> WildcardMatcher from(Stream<T> stream, boolean caseSensitive) {
Collection<WildcardMatcher> matchers = stream.map(t -> {
if (t instanceof String) {
if (t == null) {
return NONE;
} else if (t instanceof String) {
return WildcardMatcher.from(((String) t), caseSensitive);
} else if (t instanceof WildcardMatcher) {
return ((WildcardMatcher) t);
Expand Down
86 changes: 86 additions & 0 deletions src/test/java/com/amazon/dlic/auth/ldap/LdapBackendTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import java.nio.charset.StandardCharsets;
import java.nio.file.Paths;
import java.util.Arrays;
import java.util.List;

import org.hamcrest.MatcherAssert;
import org.junit.AfterClass;
Expand All @@ -39,6 +40,7 @@
import org.ldaptive.ReturnAttributes;

import static org.hamcrest.Matchers.hasItem;
import static org.hamcrest.Matchers.not;

public class LdapBackendTest {

Expand Down Expand Up @@ -579,6 +581,90 @@ public void testLdapAuthorizationNested() throws Exception {
MatcherAssert.assertThat(user.getRoles(), hasItem("nested1"));
}

@Test
public void testLdapNestedRoleFiltering() {

final Settings settings = Settings.builder()
.putList(ConfigConstants.LDAP_HOSTS, "localhost:" + ldapPort)
.put(ConfigConstants.LDAP_AUTHC_USERSEARCH, "(uid={0})")
.put(ConfigConstants.LDAP_AUTHC_USERBASE, "ou=people,o=TEST")
.put(ConfigConstants.LDAP_AUTHZ_ROLEBASE, "ou=groups,o=TEST")
.put(ConfigConstants.LDAP_AUTHZ_ROLENAME, "cn")
.put(ConfigConstants.LDAP_AUTHZ_RESOLVE_NESTED_ROLES, true)
.put(ConfigConstants.LDAP_AUTHZ_ROLESEARCH, "(uniqueMember={0})")
.putList(ConfigConstants.LDAP_AUTHZ_EXCLUDE_ROLES, List.of("nested1", "nested2"))
.build();

final User user = new User("spock");

new LDAPAuthorizationBackend(settings, null).fillRoles(user, null);

Assert.assertNotNull(user);
Assert.assertEquals("spock", user.getName());
Assert.assertEquals(2, user.getRoles().size());
// filtered out
MatcherAssert.assertThat(user.getRoles(), not(hasItem("nested1")));
MatcherAssert.assertThat(user.getRoles(), not(hasItem("nested2")));
MatcherAssert.assertThat(user.getRoles(), hasItem("role2"));
MatcherAssert.assertThat(user.getRoles(), hasItem("ceo"));
}

@Test
public void testLdapNestedRoleFilteringWithExcludedRolesWildcard() {

final Settings settings = Settings.builder()
.putList(ConfigConstants.LDAP_HOSTS, "localhost:" + ldapPort)
.put(ConfigConstants.LDAP_AUTHC_USERSEARCH, "(uid={0})")
.put(ConfigConstants.LDAP_AUTHC_USERBASE, "ou=people,o=TEST")
.put(ConfigConstants.LDAP_AUTHZ_ROLEBASE, "ou=groups,o=TEST")
.put(ConfigConstants.LDAP_AUTHZ_ROLENAME, "cn")
.put(ConfigConstants.LDAP_AUTHZ_RESOLVE_NESTED_ROLES, true)
.put(ConfigConstants.LDAP_AUTHZ_ROLESEARCH, "(uniqueMember={0})")
.putList(ConfigConstants.LDAP_AUTHZ_EXCLUDE_ROLES, List.of("nested*"))
.build();

final User user = new User("spock");

new LDAPAuthorizationBackend(settings, null).fillRoles(user, null);

Assert.assertNotNull(user);
Assert.assertEquals("spock", user.getName());
Assert.assertEquals(2, user.getRoles().size());
// filtered out
MatcherAssert.assertThat(user.getRoles(), not(hasItem("nested1")));
MatcherAssert.assertThat(user.getRoles(), not(hasItem("nested2")));
MatcherAssert.assertThat(user.getRoles(), hasItem("role2"));
MatcherAssert.assertThat(user.getRoles(), hasItem("ceo"));
}

@Test
public void testLdapdRoleFiltering() {

final Settings settings = Settings.builder()
.putList(ConfigConstants.LDAP_HOSTS, "localhost:" + ldapPort)
.put(ConfigConstants.LDAP_AUTHC_USERSEARCH, "(uid={0})")
.put(ConfigConstants.LDAP_AUTHC_USERBASE, "ou=people,o=TEST")
.put(ConfigConstants.LDAP_AUTHZ_ROLEBASE, "ou=groups,o=TEST")
.put(ConfigConstants.LDAP_AUTHZ_ROLENAME, "cn")
.put(ConfigConstants.LDAP_AUTHZ_RESOLVE_NESTED_ROLES, true)
.put(ConfigConstants.LDAP_AUTHZ_ROLESEARCH, "(uniqueMember={0})")
.putList(ConfigConstants.LDAP_AUTHZ_EXCLUDE_ROLES, List.of("ceo", "role1", "role2"))
.build();

final User user = new User("spock");

new LDAPAuthorizationBackend(settings, null).fillRoles(user, null);

Assert.assertNotNull(user);
Assert.assertEquals("spock", user.getName());
Assert.assertEquals(2, user.getRoles().size());
MatcherAssert.assertThat(user.getRoles(), hasItem("nested1"));
MatcherAssert.assertThat(user.getRoles(), hasItem("nested2"));
// filtered out
MatcherAssert.assertThat(user.getRoles(), not(hasItem("role2")));
MatcherAssert.assertThat(user.getRoles(), not(hasItem("ceo")));
}

@Test
public void testLdapAuthorizationNestedFilter() throws Exception {

Expand Down
12 changes: 7 additions & 5 deletions src/test/java/com/amazon/dlic/auth/ldap2/LdapBackendTestNewStyleConfig2.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import java.nio.file.Paths;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.TreeSet;

import org.apache.commons.lang3.exception.ExceptionUtils;
Expand Down Expand Up @@ -564,16 +565,18 @@ public void testLdapAuthorizationNested() throws Exception {
.put(ConfigConstants.LDAP_AUTHZ_ROLENAME, "cn")
.put(ConfigConstants.LDAP_AUTHZ_RESOLVE_NESTED_ROLES, true)
.put("roles.g1.search", "(uniqueMember={0})")
.putList(ConfigConstants.LDAP_AUTHZ_EXCLUDE_ROLES, List.of("nested2"))
.build();

final User user = new User("spock");

new LDAPAuthorizationBackend(settings, null).fillRoles(user, null);
new LDAPAuthorizationBackend2(settings, null).fillRoles(user, null);

Assert.assertNotNull(user);
Assert.assertEquals("spock", user.getName());
Assert.assertEquals(4, user.getRoles().size());
Assert.assertEquals("nested1", new ArrayList<>(new TreeSet<>(user.getRoles())).get(1));
Assert.assertEquals(3, user.getRoles().size());
Assert.assertTrue(user.getRoles().contains("nested1"));
Assert.assertFalse(user.getRoles().contains("nested2"));
}

@Test
Expand Down Expand Up @@ -759,7 +762,7 @@ public void testLdapAuthorizationNestedAttrFilter() throws Exception {
}

@Test
public void testLdapAuthorizationNestedAttrFilterAll() throws Exception {
public void testLdapAuthorizationNestedAttrFilterAll() {

final Settings settings = createBaseSettings().putList(ConfigConstants.LDAP_HOSTS, "localhost:" + ldapPort)
.put("users.u1.search", "(uid={0})")
Expand All @@ -780,7 +783,6 @@ public void testLdapAuthorizationNestedAttrFilterAll() throws Exception {
Assert.assertNotNull(user);
Assert.assertEquals("spock", user.getName());
Assert.assertEquals(4, user.getRoles().size());

}

@Test
Expand Down
2 changes: 2 additions & 0 deletions src/test/java/org/opensearch/security/UtilTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,8 @@ public void testWildcardMatcherClasses() {
assertTrue(wc("/\\S+/").test("abc"));
assertTrue(wc("abc").test("abc"));
assertFalse(wc("ABC").test("abc"));
assertFalse(wc(null).test("abc"));
assertTrue(WildcardMatcher.from(null, "abc").test("abc"));
}

@Test
Expand Down
Loading