diff --git a/plugin/src/main/java/org/elasticsearch/xpack/security/authc/ldap/LdapRealm.java b/plugin/src/main/java/org/elasticsearch/xpack/security/authc/ldap/LdapRealm.java index 154058abb51..48e2015d8ab 100644 --- a/plugin/src/main/java/org/elasticsearch/xpack/security/authc/ldap/LdapRealm.java +++ b/plugin/src/main/java/org/elasticsearch/xpack/security/authc/ldap/LdapRealm.java @@ -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 diff --git a/plugin/src/main/java/org/elasticsearch/xpack/security/authc/pki/PkiRealm.java b/plugin/src/main/java/org/elasticsearch/xpack/security/authc/pki/PkiRealm.java index 1cbc2d23c77..02aec511f00 100644 --- a/plugin/src/main/java/org/elasticsearch/xpack/security/authc/pki/PkiRealm.java +++ b/plugin/src/main/java/org/elasticsearch/xpack/security/authc/pki/PkiRealm.java @@ -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 diff --git a/plugin/src/main/java/org/elasticsearch/xpack/security/authc/support/DnRoleMapper.java b/plugin/src/main/java/org/elasticsearch/xpack/security/authc/support/DnRoleMapper.java index 1c072b03227..f99ca534221 100644 --- a/plugin/src/main/java/org/elasticsearch/xpack/security/authc/support/DnRoleMapper.java +++ b/plugin/src/main/java/org/elasticsearch/xpack/security/authc/support/DnRoleMapper.java @@ -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 listeners = new CopyOnWriteArrayList<>(); private volatile Map> dnRoles; - private CopyOnWriteArrayList 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) { diff --git a/plugin/src/test/java/org/elasticsearch/xpack/security/authc/ldap/ActiveDirectoryRealmTests.java b/plugin/src/test/java/org/elasticsearch/xpack/security/authc/ldap/ActiveDirectoryRealmTests.java index 9ef7bb2f4a3..8e2622ea453 100644 --- a/plugin/src/test/java/org/elasticsearch/xpack/security/authc/ldap/ActiveDirectoryRealmTests.java +++ b/plugin/src/test/java/org/elasticsearch/xpack/security/authc/ldap/ActiveDirectoryRealmTests.java @@ -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 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 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 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 stats = realm.usageStats(); diff --git a/plugin/src/test/java/org/elasticsearch/xpack/security/authc/ldap/LdapRealmTests.java b/plugin/src/test/java/org/elasticsearch/xpack/security/authc/ldap/LdapRealmTests.java index 43a5f466f65..9bbd85f7234 100644 --- a/plugin/src/test/java/org/elasticsearch/xpack/security/authc/ldap/LdapRealmTests.java +++ b/plugin/src/test/java/org/elasticsearch/xpack/security/authc/ldap/LdapRealmTests.java @@ -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 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 stats = realm.usageStats(); assertThat(stats, is(notNullValue())); diff --git a/plugin/src/test/java/org/elasticsearch/xpack/security/authc/ldap/support/LdapTestCase.java b/plugin/src/test/java/org/elasticsearch/xpack/security/authc/ldap/support/LdapTestCase.java index 066e3d1c3fb..e19d491808c 100644 --- a/plugin/src/test/java/org/elasticsearch/xpack/security/authc/ldap/support/LdapTestCase.java +++ b/plugin/src/test/java/org/elasticsearch/xpack/security/authc/ldap/support/LdapTestCase.java @@ -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) { diff --git a/plugin/src/test/java/org/elasticsearch/xpack/security/authc/support/DnRoleMapperTests.java b/plugin/src/test/java/org/elasticsearch/xpack/security/authc/support/DnRoleMapperTests.java index 85b23170256..e6f6471deb6 100644 --- a/plugin/src/test/java/org/elasticsearch/xpack/security/authc/support/DnRoleMapperTests.java +++ b/plugin/src/test/java/org/elasticsearch/xpack/security/authc/support/DnRoleMapperTests.java @@ -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 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 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 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 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 roles = mapper.resolveRoles("cn=Horatio Hornblower,ou=people,o=sevenSeas", Collections.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); } }