honeymoose/OpenSearch
1
0
Fork 0
mirror of https://github.com/honeymoose/OpenSearch.git synced 2025-02-17 10:25:15 +00:00
Code Issues Packages Projects Releases Wiki Activity

Do not add null listeners to the DnRoleMapper (elastic/x-pack-elasticsearch#636)

Browse Source 
The DnRoleMapper was changed when moving to asynchronous authentication, which introduced a regression that leads to a
NPE being thrown on file refresh. The cause is the addition of a null listener from the LdapRealm upon construction.

This change removes the listener constructor argument as it was always null or a empty runnable and adds a check in the
addListener method to ensure the listener is not null. Finally, the DnRoleMapperTests had a bug where the temporary
filenames did not have the correct suffix and tests were not actually testing what was intended.

relates elastic/x-pack-elasticsearch#608

Original commit: elastic/x-pack-elasticsearch@f47f258590
This commit is contained in:
Jay Modi 2017-02-23 11:16:14 -07:00 committed by GitHub
parent 4a001706e7
commit 405230c308
7 changed files with 61 additions and 26 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
plugin/src/main/java/org/elasticsearch/xpack/security/authc/ldap/LdapRealm.java
View File

@ -45,7 +45,7 @@ public final class LdapRealm extends CachingUsernamePasswordRealm {
public LdapRealm(String type, RealmConfig config, ResourceWatcherService watcherService, SSLService sslService,
ThreadPool threadPool) throws LDAPException {
this(type, config, sessionFactory(config, sslService, type), new DnRoleMapper(type, config, watcherService, null), threadPool);
this(type, config, sessionFactory(config, sslService, type), new DnRoleMapper(type, config, watcherService), threadPool);
}
// pkg private for testing

2
plugin/src/main/java/org/elasticsearch/xpack/security/authc/pki/PkiRealm.java
View File

@ -62,7 +62,7 @@ public class PkiRealm extends Realm {
public PkiRealm(RealmConfig config, ResourceWatcherService watcherService, SSLService sslService) {
this(config, new DnRoleMapper(TYPE, config, watcherService, null), sslService);
this(config, new DnRoleMapper(TYPE, config, watcherService), sslService);
}
// pkg private for testing

9
plugin/src/main/java/org/elasticsearch/xpack/security/authc/support/DnRoleMapper.java
View File

@ -29,6 +29,7 @@ import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.concurrent.CopyOnWriteArrayList;
import java.util.function.Function;
@ -56,15 +57,13 @@ public class DnRoleMapper {
private final String realmType;
private final Path file;
private final boolean useUnmappedGroupsAsRoles;
private final CopyOnWriteArrayList<Runnable> listeners = new CopyOnWriteArrayList<>();
private volatile Map<DN, Set<String>> dnRoles;
private CopyOnWriteArrayList<Runnable> listeners;
public DnRoleMapper(String realmType, RealmConfig config, ResourceWatcherService watcherService, Runnable listener) {
public DnRoleMapper(String realmType, RealmConfig config, ResourceWatcherService watcherService) {
this.realmType = realmType;
this.config = config;
this.logger = config.logger(getClass());
this.listeners = new CopyOnWriteArrayList<>(Collections.singleton(listener));
useUnmappedGroupsAsRoles = USE_UNMAPPED_GROUPS_AS_ROLES_SETTING.get(config.settings());
file = resolveFile(config.settings(), config.env());
@ -79,7 +78,7 @@ public class DnRoleMapper {
}
public synchronized void addListener(Runnable listener) {
listeners.add(listener);
listeners.add(Objects.requireNonNull(listener, "listener cannot be null"));
}
public static Path resolveFile(Settings settings, Environment env) {

16
plugin/src/test/java/org/elasticsearch/xpack/security/authc/ldap/ActiveDirectoryRealmTests.java
View File

@ -132,7 +132,7 @@ public class ActiveDirectoryRealmTests extends ESTestCase {
Settings settings = settings();
RealmConfig config = new RealmConfig("testAuthenticateUserPrincipleName", settings, globalSettings);
ActiveDirectorySessionFactory sessionFactory = new ActiveDirectorySessionFactory(config, sslService);
DnRoleMapper roleMapper = new DnRoleMapper(LdapRealm.AD_TYPE, config, resourceWatcherService, () -> {});
DnRoleMapper roleMapper = new DnRoleMapper(LdapRealm.AD_TYPE, config, resourceWatcherService);
LdapRealm realm = new LdapRealm(LdapRealm.AD_TYPE, config, sessionFactory, roleMapper, threadPool);
PlainActionFuture<User> future = new PlainActionFuture<>();
@ -146,7 +146,7 @@ public class ActiveDirectoryRealmTests extends ESTestCase {
Settings settings = settings();
RealmConfig config = new RealmConfig("testAuthenticateSAMAccountName", settings, globalSettings);
ActiveDirectorySessionFactory sessionFactory = new ActiveDirectorySessionFactory(config, sslService);
DnRoleMapper roleMapper = new DnRoleMapper(LdapRealm.AD_TYPE, config, resourceWatcherService, () -> {});
DnRoleMapper roleMapper = new DnRoleMapper(LdapRealm.AD_TYPE, config, resourceWatcherService);
LdapRealm realm = new LdapRealm(LdapRealm.AD_TYPE, config, sessionFactory, roleMapper, threadPool);
// Thor does not have a UPN of form CN=Thor@ad.test.elasticsearch.com
@ -170,7 +170,7 @@ public class ActiveDirectoryRealmTests extends ESTestCase {
Settings settings = settings();
RealmConfig config = new RealmConfig("testAuthenticateCachesSuccesfulAuthentications", settings, globalSettings);
ActiveDirectorySessionFactory sessionFactory = spy(new ActiveDirectorySessionFactory(config, sslService));
DnRoleMapper roleMapper = new DnRoleMapper(LdapRealm.AD_TYPE, config, resourceWatcherService, () -> {});
DnRoleMapper roleMapper = new DnRoleMapper(LdapRealm.AD_TYPE, config, resourceWatcherService);
LdapRealm realm = new LdapRealm(LdapRealm.AD_TYPE, config, sessionFactory, roleMapper, threadPool);
int count = randomIntBetween(2, 10);
@ -188,7 +188,7 @@ public class ActiveDirectoryRealmTests extends ESTestCase {
Settings settings = settings(Settings.builder().put(CachingUsernamePasswordRealm.CACHE_TTL_SETTING.getKey(), -1).build());
RealmConfig config = new RealmConfig("testAuthenticateCachingCanBeDisabled", settings, globalSettings);
ActiveDirectorySessionFactory sessionFactory = spy(new ActiveDirectorySessionFactory(config, sslService));
DnRoleMapper roleMapper = new DnRoleMapper(LdapRealm.AD_TYPE, config, resourceWatcherService, () -> {});
DnRoleMapper roleMapper = new DnRoleMapper(LdapRealm.AD_TYPE, config, resourceWatcherService);
LdapRealm realm = new LdapRealm(LdapRealm.AD_TYPE, config, sessionFactory, roleMapper, threadPool);
int count = randomIntBetween(2, 10);
@ -206,7 +206,7 @@ public class ActiveDirectoryRealmTests extends ESTestCase {
Settings settings = settings();
RealmConfig config = new RealmConfig("testAuthenticateCachingClearsCacheOnRoleMapperRefresh", settings, globalSettings);
ActiveDirectorySessionFactory sessionFactory = spy(new ActiveDirectorySessionFactory(config, sslService));
DnRoleMapper roleMapper = new DnRoleMapper(LdapRealm.AD_TYPE, config, resourceWatcherService, () -> {});
DnRoleMapper roleMapper = new DnRoleMapper(LdapRealm.AD_TYPE, config, resourceWatcherService);
LdapRealm realm = new LdapRealm(LdapRealm.AD_TYPE, config, sessionFactory, roleMapper, threadPool);
int count = randomIntBetween(2, 10);
@ -237,7 +237,7 @@ public class ActiveDirectoryRealmTests extends ESTestCase {
.build());
RealmConfig config = new RealmConfig("testRealmMapsGroupsToRoles", settings, globalSettings);
ActiveDirectorySessionFactory sessionFactory = new ActiveDirectorySessionFactory(config, sslService);
DnRoleMapper roleMapper = new DnRoleMapper(LdapRealm.AD_TYPE, config, resourceWatcherService, () -> {});
DnRoleMapper roleMapper = new DnRoleMapper(LdapRealm.AD_TYPE, config, resourceWatcherService);
LdapRealm realm = new LdapRealm(LdapRealm.AD_TYPE, config, sessionFactory, roleMapper, threadPool);
PlainActionFuture<User> future = new PlainActionFuture<>();
@ -253,7 +253,7 @@ public class ActiveDirectoryRealmTests extends ESTestCase {
.build());
RealmConfig config = new RealmConfig("testRealmMapsGroupsToRoles", settings, globalSettings);
ActiveDirectorySessionFactory sessionFactory = new ActiveDirectorySessionFactory(config, sslService);
DnRoleMapper roleMapper = new DnRoleMapper(LdapRealm.AD_TYPE, config, resourceWatcherService, () -> {});
DnRoleMapper roleMapper = new DnRoleMapper(LdapRealm.AD_TYPE, config, resourceWatcherService);
LdapRealm realm = new LdapRealm(LdapRealm.AD_TYPE, config, sessionFactory, roleMapper, threadPool);
PlainActionFuture<User> future = new PlainActionFuture<>();
@ -271,7 +271,7 @@ public class ActiveDirectoryRealmTests extends ESTestCase {
.build());
RealmConfig config = new RealmConfig("testRealmUsageStats", settings, globalSettings);
ActiveDirectorySessionFactory sessionFactory = new ActiveDirectorySessionFactory(config, sslService);
DnRoleMapper roleMapper = new DnRoleMapper(LdapRealm.AD_TYPE, config, resourceWatcherService, () -> {});
DnRoleMapper roleMapper = new DnRoleMapper(LdapRealm.AD_TYPE, config, resourceWatcherService);
LdapRealm realm = new LdapRealm(LdapRealm.AD_TYPE, config, sessionFactory, roleMapper, threadPool);
Map<String, Object> stats = realm.usageStats();

4
plugin/src/test/java/org/elasticsearch/xpack/security/authc/ldap/LdapRealmTests.java
View File

@ -267,7 +267,7 @@ public class LdapRealmTests extends LdapTestCase {
LdapSessionFactory ldapFactory = new LdapSessionFactory(config, sslService);
LdapRealm ldap = new LdapRealm(LdapRealm.LDAP_TYPE, config, ldapFactory,
new DnRoleMapper(LdapRealm.LDAP_TYPE, config, resourceWatcherService, null), threadPool);
new DnRoleMapper(LdapRealm.LDAP_TYPE, config, resourceWatcherService), threadPool);
PlainActionFuture<User> future = new PlainActionFuture<>();
ldap.authenticate(new UsernamePasswordToken("Horatio Hornblower", SecuredStringTests.build(PASSWORD)), future);
@ -322,7 +322,7 @@ public class LdapRealmTests extends LdapTestCase {
LdapSessionFactory ldapFactory = new LdapSessionFactory(config, sslService);
LdapRealm realm = new LdapRealm(LdapRealm.LDAP_TYPE, config, ldapFactory,
new DnRoleMapper(LdapRealm.LDAP_TYPE, config, resourceWatcherService, null), threadPool);
new DnRoleMapper(LdapRealm.LDAP_TYPE, config, resourceWatcherService), threadPool);
Map<String, Object> stats = realm.usageStats();
assertThat(stats, is(notNullValue()));

2
plugin/src/test/java/org/elasticsearch/xpack/security/authc/ldap/support/LdapTestCase.java
View File

@ -126,7 +126,7 @@ public abstract class LdapTestCase extends ESTestCase {
Settings global = Settings.builder().put("path.home", createTempDir()).build();
RealmConfig config = new RealmConfig("ldap1", settings, global);
return new DnRoleMapper(LdapRealm.LDAP_TYPE, config, resourceWatcherService, () -> {});
return new DnRoleMapper(LdapRealm.LDAP_TYPE, config, resourceWatcherService);
}
protected LdapSession session(SessionFactory factory, String username, SecuredString password) {

52
plugin/src/test/java/org/elasticsearch/xpack/security/authc/support/DnRoleMapperTests.java
View File

@ -68,7 +68,7 @@ public class DnRoleMapperTests extends ESTestCase {
@Before
public void init() throws IOException {
settings = Settings.builder()
.put("resource.reload.interval.high", "2s")
.put("resource.reload.interval.high", "100ms")
.put("path.home", createTempDir())
.build();
env = new Environment(settings);
@ -84,7 +84,7 @@ public class DnRoleMapperTests extends ESTestCase {
}
public void testMapper_ConfiguredWithUnreadableFile() throws Exception {
Path file = createTempFile();
Path file = createTempFile("", ".yml");
// writing in utf_16 should cause a parsing error as we try to read the file in utf_8
Files.write(file, Collections.singletonList("aldlfkjldjdflkjd"), StandardCharsets.UTF_16);
@ -155,6 +155,42 @@ public class DnRoleMapperTests extends ESTestCase {
assertThat(mapper.mappingsCount(), is(0));
}
public void testMapperAutoReloadWithoutListener() throws Exception {
Path roleMappingFile = getDataPath("role_mapping.yml");
Path file = env.configFile().resolve("test_role_mapping.yml");
Files.copy(roleMappingFile, file, StandardCopyOption.REPLACE_EXISTING);
ResourceWatcherService watcherService = new ResourceWatcherService(settings, threadPool);
DnRoleMapper mapper = createMapper(file, watcherService);
Set<String> roles = mapper.resolveRoles("", Collections.singletonList("cn=shield,ou=marvel,o=superheros"));
assertThat(roles, notNullValue());
assertThat(roles.size(), is(1));
assertThat(roles, contains("security"));
watcherService.start();
try (BufferedWriter writer = Files.newBufferedWriter(file, StandardCharsets.UTF_8, StandardOpenOption.APPEND)) {
writer.newLine();
writer.append("fantastic_four:\n")
.append(" - \"cn=fantastic_four,ou=marvel,o=superheros\"");
}
assertBusy(() -> {
Set<String> resolvedRoles = mapper.resolveRoles("", Collections.singletonList("cn=fantastic_four,ou=marvel,o=superheros"));
assertThat(resolvedRoles, notNullValue());
assertThat(resolvedRoles.size(), is(1));
assertThat(resolvedRoles, contains("fantastic_four"));
}, 2L, TimeUnit.SECONDS);
}
public void testAddNullListener() throws Exception {
Path file = env.configFile().resolve("test_role_mapping.yml");
ResourceWatcherService watcherService = new ResourceWatcherService(settings, threadPool);
DnRoleMapper mapper = createMapper(file, watcherService);
NullPointerException e = expectThrows(NullPointerException.class, () -> mapper.addListener(null));
assertEquals("listener cannot be null", e.getMessage());
}
public void testParseFile() throws Exception {
Path file = getDataPath("role_mapping.yml");
Logger logger = CapturingLogger.newCapturingLogger(Level.INFO);
@ -205,7 +241,7 @@ public class DnRoleMapperTests extends ESTestCase {
}
public void testParseFile_WhenCannotReadFile() throws Exception {
Path file = createTempFile();
Path file = createTempFile("", ".yml");
// writing in utf_16 should cause a parsing error as we try to read the file in utf_8
Files.write(file, Collections.singletonList("aldlfkjldjdflkjd"), StandardCharsets.UTF_16);
Logger logger = CapturingLogger.newCapturingLogger(Level.INFO);
@ -218,7 +254,7 @@ public class DnRoleMapperTests extends ESTestCase {
}
public void testParseFileLenient_WhenCannotReadFile() throws Exception {
Path file = createTempFile();
Path file = createTempFile("", ".yml");
// writing in utf_16 should cause a parsing error as we try to read the file in utf_8
Files.write(file, Collections.singletonList("aldlfkjldjdflkjd"), StandardCharsets.UTF_16);
Logger logger = CapturingLogger.newCapturingLogger(Level.INFO);
@ -237,7 +273,7 @@ public class DnRoleMapperTests extends ESTestCase {
.build();
RealmConfig config = new RealmConfig("ldap1", ldapSettings, settings);
DnRoleMapper mapper = new DnRoleMapper(LdapRealm.LDAP_TYPE, config, new ResourceWatcherService(settings, threadPool), null);
DnRoleMapper mapper = new DnRoleMapper(LdapRealm.LDAP_TYPE, config, new ResourceWatcherService(settings, threadPool));
Set<String> roles = mapper.resolveRoles("", Arrays.asList(STARK_GROUP_DNS));
@ -251,7 +287,7 @@ public class DnRoleMapperTests extends ESTestCase {
.build();
RealmConfig config = new RealmConfig("ldap1", ldapSettings, settings);
DnRoleMapper mapper = new DnRoleMapper(LdapRealm.LDAP_TYPE, config, new ResourceWatcherService(settings, threadPool), null);
DnRoleMapper mapper = new DnRoleMapper(LdapRealm.LDAP_TYPE, config, new ResourceWatcherService(settings, threadPool));
Set<String> roles = mapper.resolveRoles("", Arrays.asList(STARK_GROUP_DNS));
assertThat(roles, hasItems("genius", "billionaire", "playboy", "philanthropist", "shield", "avengers"));
@ -265,7 +301,7 @@ public class DnRoleMapperTests extends ESTestCase {
.build();
RealmConfig config = new RealmConfig("ldap-userdn-role", ldapSettings, settings);
DnRoleMapper mapper = new DnRoleMapper(LdapRealm.LDAP_TYPE, config, new ResourceWatcherService(settings, threadPool), null);
DnRoleMapper mapper = new DnRoleMapper(LdapRealm.LDAP_TYPE, config, new ResourceWatcherService(settings, threadPool));
Set<String> roles = mapper.resolveRoles("cn=Horatio Hornblower,ou=people,o=sevenSeas", Collections.<String>emptyList());
assertThat(roles, hasItem("avenger"));
@ -276,6 +312,6 @@ public class DnRoleMapperTests extends ESTestCase {
.put("files.role_mapping", file.toAbsolutePath())
.build();
RealmConfig config = new RealmConfig("ad-group-mapper-test", realmSettings, settings, env);
return new DnRoleMapper(randomBoolean() ? LdapRealm.AD_TYPE : LdapRealm.LDAP_TYPE, config, watcherService, () -> {});
return new DnRoleMapper(randomBoolean() ? LdapRealm.AD_TYPE : LdapRealm.LDAP_TYPE, config, watcherService);
}
}