From b887fad51ad587c464c03ab1763193a13ff82bea Mon Sep 17 00:00:00 2001 From: Alexander Reelsen Date: Tue, 26 Aug 2014 10:49:48 +0200 Subject: [PATCH] Config file parsing: Improved parsing for edge cases Created some tests to improve parsing of files, most importantly fixed an endless loop in YAML parsing and made the non-yaml parsers a bit more error resistant. Closes elastic/elasticsearch#40 Original commit: elastic/x-pack-elasticsearch@8873138d0c86fc92ff1be39002fc486912318594 --- .../authc/esusers/FileUserPasswdStore.java | 4 +- .../authc/esusers/FileUserRolesStore.java | 22 ++- .../shield/authz/store/FileRolesStore.java | 6 +- .../n2n/IPFilteringN2NAuthenticator.java | 5 +- .../esusers/FileUserPasswdStoreTests.java | 16 +++ .../esusers/FileUserRolesStoreTests.java | 34 +++++ .../authz/store/FileRolesStoreTests.java | 16 +++ .../n2n/IPFilteringN2NAuthenticatorTests.java | 125 +++++++++--------- 8 files changed, 160 insertions(+), 68 deletions(-) diff --git a/src/main/java/org/elasticsearch/shield/authc/esusers/FileUserPasswdStore.java b/src/main/java/org/elasticsearch/shield/authc/esusers/FileUserPasswdStore.java index 81d04845518..1790cc36820 100644 --- a/src/main/java/org/elasticsearch/shield/authc/esusers/FileUserPasswdStore.java +++ b/src/main/java/org/elasticsearch/shield/authc/esusers/FileUserPasswdStore.java @@ -100,7 +100,9 @@ public class FileUserPasswdStore extends AbstractComponent implements UserPasswd lineNr++; int i = line.indexOf(":"); if (i <= 0 || i == line.length() - 1) { - logger.error("Invalid entry in users file [" + path.toAbsolutePath() + "], line [" + lineNr + "]. Skipping..."); + if (logger != null) { + logger.error("Invalid entry in users file [" + path.toAbsolutePath() + "], line [" + lineNr + "]. Skipping..."); + } continue; } String username = line.substring(0, i).trim(); diff --git a/src/main/java/org/elasticsearch/shield/authc/esusers/FileUserRolesStore.java b/src/main/java/org/elasticsearch/shield/authc/esusers/FileUserRolesStore.java index 5df5e55acb7..e3ee21f5bfa 100644 --- a/src/main/java/org/elasticsearch/shield/authc/esusers/FileUserRolesStore.java +++ b/src/main/java/org/elasticsearch/shield/authc/esusers/FileUserRolesStore.java @@ -93,12 +93,32 @@ public class FileUserRolesStore extends AbstractComponent implements UserRolesSt lineNr++; int i = line.indexOf(":"); if (i <= 0 || i == line.length() - 1) { - logger.error("Invalid entry in users file [" + path.toAbsolutePath() + "], line [" + lineNr + "]. Skipping..."); + if (logger != null) { + logger.error("Invalid entry in users file [" + path.toAbsolutePath() + "], line [" + lineNr + "]. Skipping..."); + } continue; } String username = line.substring(0, i).trim(); + if (Strings.isEmpty(username)) { + if (logger != null) { + logger.error("Invalid username entry in users file [" + path.toAbsolutePath() + "], line [" + lineNr + "]. Skipping..."); + } + continue; + } String rolesStr = line.substring(i + 1).trim(); + if (Strings.isEmpty(rolesStr)) { + if (logger != null) { + logger.error("Invalid roles entry in users file [" + path.toAbsolutePath() + "], line [" + lineNr + "]. Skipping..."); + } + continue; + } String[] roles = ROLES_DELIM.split(rolesStr); + if (roles.length == 0) { + if (logger != null) { + logger.error("Invalid roles entry in users file [" + path.toAbsolutePath() + "], line [" + lineNr + "]. Skipping..."); + } + continue; + } usersRoles.put(username, roles); } diff --git a/src/main/java/org/elasticsearch/shield/authz/store/FileRolesStore.java b/src/main/java/org/elasticsearch/shield/authz/store/FileRolesStore.java index 2a71aaa6d2b..82f47f04c47 100644 --- a/src/main/java/org/elasticsearch/shield/authz/store/FileRolesStore.java +++ b/src/main/java/org/elasticsearch/shield/authz/store/FileRolesStore.java @@ -11,6 +11,8 @@ import org.elasticsearch.common.collect.ImmutableMap; import org.elasticsearch.common.collect.ImmutableSet; import org.elasticsearch.common.component.AbstractComponent; import org.elasticsearch.common.inject.Inject; +import org.elasticsearch.common.jackson.dataformat.yaml.snakeyaml.error.YAMLException; +import org.elasticsearch.common.jackson.dataformat.yaml.snakeyaml.scanner.ScannerException; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentFactory; @@ -86,7 +88,7 @@ public class FileRolesStore extends AbstractComponent implements RolesStore { XContentParser parser = YamlXContent.yamlXContent.createParser(input); XContentParser.Token token; String currentFieldName = null; - while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) { + while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT && token != null) { if (token == XContentParser.Token.FIELD_NAME) { currentFieldName = parser.currentName(); } else if (token == XContentParser.Token.START_OBJECT && currentFieldName != null) { @@ -160,7 +162,7 @@ public class FileRolesStore extends AbstractComponent implements RolesStore { return roles.build(); - } catch (IOException ioe) { + } catch (YAMLException|IOException ioe) { throw new ElasticsearchException("Failed to read roles file [" + path.toAbsolutePath() + "]", ioe); } } diff --git a/src/main/java/org/elasticsearch/shield/n2n/IPFilteringN2NAuthenticator.java b/src/main/java/org/elasticsearch/shield/n2n/IPFilteringN2NAuthenticator.java index 9abf953aa3c..1a6c3db477a 100644 --- a/src/main/java/org/elasticsearch/shield/n2n/IPFilteringN2NAuthenticator.java +++ b/src/main/java/org/elasticsearch/shield/n2n/IPFilteringN2NAuthenticator.java @@ -11,6 +11,7 @@ import org.elasticsearch.common.Nullable; import org.elasticsearch.common.Strings; import org.elasticsearch.common.component.AbstractComponent; import org.elasticsearch.common.inject.Inject; +import org.elasticsearch.common.jackson.dataformat.yaml.snakeyaml.error.YAMLException; import org.elasticsearch.common.net.InetAddresses; import org.elasticsearch.common.netty.handler.ipfilter.IpFilterRule; import org.elasticsearch.common.netty.handler.ipfilter.IpSubnetFilterRule; @@ -103,8 +104,8 @@ public class IPFilteringN2NAuthenticator extends AbstractComponent implements N2 } } - } catch (IOException ioe) { - throw new ElasticsearchException("Failed to read & parse host access file [" + path.toAbsolutePath() + "]", ioe); + } catch (YAMLException|IOException ioe) { + throw new ElasticsearchParseException("Failed to read & parse host access file [" + path.toAbsolutePath() + "]", ioe); } if (rules.size() == 0) { diff --git a/src/test/java/org/elasticsearch/shield/authc/esusers/FileUserPasswdStoreTests.java b/src/test/java/org/elasticsearch/shield/authc/esusers/FileUserPasswdStoreTests.java index 2bede7dde85..1ee00c504ec 100644 --- a/src/test/java/org/elasticsearch/shield/authc/esusers/FileUserPasswdStoreTests.java +++ b/src/test/java/org/elasticsearch/shield/authc/esusers/FileUserPasswdStoreTests.java @@ -10,12 +10,17 @@ import org.elasticsearch.common.settings.ImmutableSettings; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.env.Environment; import org.elasticsearch.shield.authc.support.Hasher; +import org.elasticsearch.shield.authz.Permission; +import org.elasticsearch.shield.authz.store.FileRolesStore; import org.elasticsearch.test.ElasticsearchTestCase; import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.watcher.ResourceWatcherService; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.TemporaryFolder; import java.io.BufferedWriter; +import java.io.File; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; @@ -31,6 +36,9 @@ import static org.hamcrest.Matchers.*; */ public class FileUserPasswdStoreTests extends ElasticsearchTestCase { + @Rule + public TemporaryFolder tempFolder = new TemporaryFolder(); + @Test public void testParseFile() throws Exception { Path path = Paths.get(getClass().getResource("users").toURI()); @@ -99,6 +107,14 @@ public class FileUserPasswdStoreTests extends ElasticsearchTestCase { threadPool.shutdownNow(); } } + } + @Test + public void testThatInvalidLineDoesNotResultInLoggerNPE() throws Exception { + File file = tempFolder.newFile(); + com.google.common.io.Files.write("NotValidUsername=Password\nuser:pass".getBytes(org.elasticsearch.common.base.Charsets.UTF_8), file); + Map users = FileUserPasswdStore.parseFile(file.toPath(), null); + assertThat(users, notNullValue()); + assertThat(users.keySet(), hasSize(1)); } } diff --git a/src/test/java/org/elasticsearch/shield/authc/esusers/FileUserRolesStoreTests.java b/src/test/java/org/elasticsearch/shield/authc/esusers/FileUserRolesStoreTests.java index aa9e191a4b3..8aa69e2136b 100644 --- a/src/test/java/org/elasticsearch/shield/authc/esusers/FileUserRolesStoreTests.java +++ b/src/test/java/org/elasticsearch/shield/authc/esusers/FileUserRolesStoreTests.java @@ -9,16 +9,22 @@ import com.carrotsearch.ant.tasks.junit4.dependencies.com.google.common.base.Cha import org.elasticsearch.common.settings.ImmutableSettings; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.env.Environment; +import org.elasticsearch.shield.authz.Permission; +import org.elasticsearch.shield.authz.store.FileRolesStore; import org.elasticsearch.test.ElasticsearchTestCase; import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.watcher.ResourceWatcherService; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.TemporaryFolder; import java.io.BufferedWriter; +import java.io.File; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; import java.nio.file.StandardOpenOption; +import java.util.Locale; import java.util.Map; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; @@ -30,6 +36,9 @@ import static org.hamcrest.Matchers.*; */ public class FileUserRolesStoreTests extends ElasticsearchTestCase { + @Rule + public TemporaryFolder tempFolder = new TemporaryFolder(); + @Test public void testParseFile() throws Exception { Path path = Paths.get(getClass().getResource("users_roles").toURI()); @@ -102,6 +111,31 @@ public class FileUserRolesStoreTests extends ElasticsearchTestCase { threadPool.shutdownNow(); } } + } + @Test + public void testThatEmptyFileIsParsed() throws Exception { + assertInvalidInputIsSilentlyIngored(""); + assertInvalidInputIsSilentlyIngored("#"); + } + + @Test + public void testThatEmptyUserNameDoesNotThrowException() throws Exception { + assertInvalidInputIsSilentlyIngored(":role1,role2"); + assertInvalidInputIsSilentlyIngored(" :role1,role2"); + } + + @Test + public void testThatEmptyRoleDoesNotThrowException() throws Exception { + assertInvalidInputIsSilentlyIngored("user:"); + assertInvalidInputIsSilentlyIngored("user: "); + assertInvalidInputIsSilentlyIngored("user: , "); + } + + private void assertInvalidInputIsSilentlyIngored(String input) throws Exception { + File file = tempFolder.newFile(); + com.google.common.io.Files.write(input.getBytes(Charsets.UTF_8), file); + Map usersRoles = FileUserRolesStore.parseFile(file.toPath(), null); + assertThat(String.format(Locale.ROOT, "Expected userRoles to be empty, but was %s", usersRoles.keySet()), usersRoles.keySet(), hasSize(0)); } } diff --git a/src/test/java/org/elasticsearch/shield/authz/store/FileRolesStoreTests.java b/src/test/java/org/elasticsearch/shield/authz/store/FileRolesStoreTests.java index 742ed80d1e5..09c7c7bc92f 100644 --- a/src/test/java/org/elasticsearch/shield/authz/store/FileRolesStoreTests.java +++ b/src/test/java/org/elasticsearch/shield/authz/store/FileRolesStoreTests.java @@ -5,6 +5,7 @@ */ package org.elasticsearch.shield.authz.store; +import org.elasticsearch.ElasticsearchException; import org.elasticsearch.common.base.Charsets; import org.elasticsearch.common.settings.ImmutableSettings; import org.elasticsearch.common.settings.Settings; @@ -19,6 +20,7 @@ import org.junit.Test; import org.junit.rules.TemporaryFolder; import java.io.BufferedWriter; +import java.io.File; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; @@ -147,6 +149,20 @@ public class FileRolesStoreTests extends ElasticsearchTestCase { threadPool.shutdownNow(); } } + } + @Test + public void testThatEmptyFileDoesNotResultInLoop() throws Exception { + File file = tempFolder.newFile(); + com.google.common.io.Files.write("#".getBytes(Charsets.UTF_8), file); + Map roles = FileRolesStore.parseFile(file.toPath()); + assertThat(roles.keySet(), is(empty())); + } + + @Test(expected = ElasticsearchException.class) + public void testThatInvalidYAMLThrowsElasticsearchException() throws Exception { + File file = tempFolder.newFile(); + com.google.common.io.Files.write("user: cluster: ALL indices: '.*': ALL".getBytes(Charsets.UTF_8), file); + FileRolesStore.parseFile(file.toPath()); } } diff --git a/src/test/java/org/elasticsearch/shield/n2n/IPFilteringN2NAuthenticatorTests.java b/src/test/java/org/elasticsearch/shield/n2n/IPFilteringN2NAuthenticatorTests.java index 4a4c8bfb654..bf6086e505a 100644 --- a/src/test/java/org/elasticsearch/shield/n2n/IPFilteringN2NAuthenticatorTests.java +++ b/src/test/java/org/elasticsearch/shield/n2n/IPFilteringN2NAuthenticatorTests.java @@ -6,6 +6,7 @@ package org.elasticsearch.shield.n2n; import com.google.common.io.Files; +import org.elasticsearch.ElasticsearchParseException; import org.elasticsearch.common.base.Charsets; import org.elasticsearch.common.net.InetAddresses; import org.elasticsearch.common.settings.Settings; @@ -14,13 +15,16 @@ import org.elasticsearch.env.Environment; import org.elasticsearch.test.ElasticsearchTestCase; import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.watcher.ResourceWatcherService; +import org.junit.After; import org.junit.Before; import org.junit.Rule; import org.junit.Test; import org.junit.rules.TemporaryFolder; import java.io.File; +import java.io.IOException; import java.security.Principal; +import java.util.Locale; import static org.elasticsearch.common.settings.ImmutableSettings.settingsBuilder; import static org.hamcrest.Matchers.is; @@ -42,122 +46,119 @@ public class IPFilteringN2NAuthenticatorTests extends ElasticsearchTestCase { private ResourceWatcherService resourceWatcherService; private File configFile; + private Settings settings; + private IPFilteringN2NAuthenticator ipFilteringN2NAuthenticator; @Before public void init() throws Exception { configFile = temporaryFolder.newFile(); - Settings settings = settingsBuilder().put("watcher.interval.medium", TimeValue.timeValueMillis(200)).build(); - resourceWatcherService = new ResourceWatcherService(settings, new ThreadPool("resourceWatcher")).start(); + Settings resourceWatcherServiceSettings = settingsBuilder().put("watcher.interval.medium", TimeValue.timeValueMillis(200)).build(); + resourceWatcherService = new ResourceWatcherService(resourceWatcherServiceSettings, new ThreadPool("resourceWatcher")).start(); + settings = settingsBuilder().put("shield.n2n.file", configFile.getPath()).build(); + } + + @After + public void shutdown() { + resourceWatcherService.stop(); } @Test public void testThatIpV4AddressesCanBeProcessed() throws Exception { - String testData = "allow: 127.0.0.1\ndeny: 10.0.0.0/8"; - Files.write(testData.getBytes(Charsets.UTF_8), configFile); + writeConfigFile("allow: 127.0.0.1\ndeny: 10.0.0.0/8"); - Settings settings = settingsBuilder().put("shield.n2n.file", configFile.getPath()).build(); - IPFilteringN2NAuthenticator ipFilteringN2NAuthenticator = new IPFilteringN2NAuthenticator(settings, new Environment(), resourceWatcherService); - - assertThat(ipFilteringN2NAuthenticator.authenticate(NULL_PRINCIPAL, InetAddresses.forString("127.0.0.1"), 1024), is(true)); - assertThat(ipFilteringN2NAuthenticator.authenticate(NULL_PRINCIPAL, InetAddresses.forString("10.2.3.4"), 1024), is(false)); + assertAddressIsAllowed(ipFilteringN2NAuthenticator, "127.0.0.1"); + assertAddressIsDenied(ipFilteringN2NAuthenticator, "10.2.3.4"); } @Test public void testThatIpV6AddressesCanBeProcessed() throws Exception { - String testData = "allow: 2001:0db8:1234::/48\ndeny: 1234:0db8:85a3:0000:0000:8a2e:0370:7334"; - Files.write(testData.getBytes(Charsets.UTF_8), configFile); + writeConfigFile("allow: 2001:0db8:1234::/48\ndeny: 1234:0db8:85a3:0000:0000:8a2e:0370:7334"); - Settings settings = settingsBuilder().put("shield.n2n.file", configFile.getPath()).build(); - IPFilteringN2NAuthenticator ipFilteringN2NAuthenticator = new IPFilteringN2NAuthenticator(settings, new Environment(), resourceWatcherService); - - assertThat(ipFilteringN2NAuthenticator.authenticate(NULL_PRINCIPAL, InetAddresses.forString("2001:0db8:1234:0000:0000:8a2e:0370:7334"), 1024), is(true)); - assertThat(ipFilteringN2NAuthenticator.authenticate(NULL_PRINCIPAL, InetAddresses.forString("1234:0db8:85a3:0000:0000:8a2e:0370:7334"), 1024), is(false)); + assertAddressIsAllowed(ipFilteringN2NAuthenticator, "2001:0db8:1234:0000:0000:8a2e:0370:7334"); + assertAddressIsDenied(ipFilteringN2NAuthenticator, "1234:0db8:85a3:0000:0000:8a2e:0370:7334"); } @Test public void testThatHostnamesCanBeProcessed() throws Exception { - String testData = "allow: localhost\ndeny: '*.google.com'"; - Files.write(testData.getBytes(Charsets.UTF_8), configFile); + writeConfigFile("allow: localhost\ndeny: '*.google.com'"); - Settings settings = settingsBuilder().put("shield.n2n.file", configFile.getPath()).build(); - IPFilteringN2NAuthenticator ipFilteringN2NAuthenticator = new IPFilteringN2NAuthenticator(settings, new Environment(), resourceWatcherService); - - assertThat(ipFilteringN2NAuthenticator.authenticate(NULL_PRINCIPAL, InetAddresses.forString("127.0.0.1"), 1024), is(true)); - assertThat(ipFilteringN2NAuthenticator.authenticate(NULL_PRINCIPAL, InetAddresses.forString("173.194.70.100"), 1024), is(false)); + assertAddressIsAllowed(ipFilteringN2NAuthenticator, "127.0.0.1"); + assertAddressIsDenied(ipFilteringN2NAuthenticator, "173.194.70.100"); } @Test public void testThatFileDeletionResultsInAllowingAll() throws Exception { - String testData = "allow: 127.0.0.1"; - Files.write(testData.getBytes(Charsets.UTF_8), configFile); + writeConfigFile("allow: 127.0.0.1"); - Settings settings = settingsBuilder().put("shield.n2n.file", configFile.getPath()).build(); - IPFilteringN2NAuthenticator ipFilteringN2NAuthenticator = new IPFilteringN2NAuthenticator(settings, new Environment(), resourceWatcherService); - - assertThat(ipFilteringN2NAuthenticator.authenticate(NULL_PRINCIPAL, InetAddresses.forString("127.0.0.1"), 1024), is(true)); + assertAddressIsAllowed(ipFilteringN2NAuthenticator, "127.0.0.1"); configFile.delete(); assertThat(configFile.exists(), is(false)); sleep(250); - assertThat(ipFilteringN2NAuthenticator.authenticate(NULL_PRINCIPAL, InetAddresses.forString("127.0.0.1"), 1024), is(false)); + assertAddressIsDenied(ipFilteringN2NAuthenticator, "127.0.0.1"); } @Test public void testThatAnAllowAllAuthenticatorWorks() throws Exception { - String testData = "allow: all"; - Files.write(testData.getBytes(Charsets.UTF_8), configFile); + writeConfigFile("allow: all"); - Settings settings = settingsBuilder().put("shield.n2n.file", configFile.getPath()).build(); - IPFilteringN2NAuthenticator ipFilteringN2NAuthenticator = new IPFilteringN2NAuthenticator(settings, new Environment(), resourceWatcherService); - - assertThat(ipFilteringN2NAuthenticator.authenticate(NULL_PRINCIPAL, InetAddresses.forString("127.0.0.1"), 1024), is(true)); - assertThat(ipFilteringN2NAuthenticator.authenticate(NULL_PRINCIPAL, InetAddresses.forString("173.194.70.100"), 1024), is(true)); + assertAddressIsAllowed(ipFilteringN2NAuthenticator, "127.0.0.1"); + assertAddressIsAllowed(ipFilteringN2NAuthenticator, "173.194.70.100"); } @Test public void testThatCommaSeparatedValuesWork() throws Exception { - String testData = "allow: 192.168.23.0/24, localhost\ndeny: all"; - Files.write(testData.getBytes(Charsets.UTF_8), configFile); + writeConfigFile("allow: 192.168.23.0/24, localhost\ndeny: all"); - Settings settings = settingsBuilder().put("shield.n2n.file", configFile.getPath()).build(); - IPFilteringN2NAuthenticator ipFilteringN2NAuthenticator = new IPFilteringN2NAuthenticator(settings, new Environment(), resourceWatcherService); - - assertThat(ipFilteringN2NAuthenticator.authenticate(NULL_PRINCIPAL, InetAddresses.forString("192.168.23.1"), 1024), is(true)); - assertThat(ipFilteringN2NAuthenticator.authenticate(NULL_PRINCIPAL, InetAddresses.forString("127.0.0.1"), 1024), is(true)); - assertThat(ipFilteringN2NAuthenticator.authenticate(NULL_PRINCIPAL, InetAddresses.forString("10.1.2.3"), 1024), is(false)); + assertAddressIsAllowed(ipFilteringN2NAuthenticator, "192.168.23.1"); + assertAddressIsAllowed(ipFilteringN2NAuthenticator, "127.0.0.1"); + assertAddressIsDenied(ipFilteringN2NAuthenticator, "10.1.2.3"); } @Test public void testThatOrderIsImportant() throws Exception { - String testData = "deny: localhost\nallow: localhost"; - Files.write(testData.getBytes(Charsets.UTF_8), configFile); + writeConfigFile("deny: localhost\nallow: localhost"); - Settings settings = settingsBuilder().put("shield.n2n.file", configFile.getPath()).build(); - IPFilteringN2NAuthenticator ipFilteringN2NAuthenticator = new IPFilteringN2NAuthenticator(settings, new Environment(), resourceWatcherService); - - assertThat(ipFilteringN2NAuthenticator.authenticate(NULL_PRINCIPAL, InetAddresses.forString("127.0.0.1"), 1024), is(false)); + assertAddressIsDenied(ipFilteringN2NAuthenticator, "127.0.0.1"); } @Test public void testThatOrderIsImportantViceVersa() throws Exception { - String testData = "allow: localhost\ndeny: localhost"; - Files.write(testData.getBytes(Charsets.UTF_8), configFile); + writeConfigFile("allow: localhost\ndeny: localhost"); - Settings settings = settingsBuilder().put("shield.n2n.file", configFile.getPath()).build(); - IPFilteringN2NAuthenticator ipFilteringN2NAuthenticator = new IPFilteringN2NAuthenticator(settings, new Environment(), resourceWatcherService); - - assertThat(ipFilteringN2NAuthenticator.authenticate(NULL_PRINCIPAL, InetAddresses.forString("127.0.0.1"), 1024), is(true)); + assertAddressIsAllowed(ipFilteringN2NAuthenticator, "127.0.0.1"); } @Test public void testThatEmptyFileDoesNotLeadIntoLoop() throws Exception { - String testData = "# \n\n"; - Files.write(testData.getBytes(Charsets.UTF_8), configFile); + writeConfigFile("# \n\n"); - Settings settings = settingsBuilder().put("shield.n2n.file", configFile.getPath()).build(); - IPFilteringN2NAuthenticator ipFilteringN2NAuthenticator = new IPFilteringN2NAuthenticator(settings, new Environment(), resourceWatcherService); + assertAddressIsDenied(ipFilteringN2NAuthenticator, "127.0.0.1"); + } - assertThat(ipFilteringN2NAuthenticator.authenticate(NULL_PRINCIPAL, InetAddresses.forString("127.0.0.1"), 1024), is(false)); + @Test(expected = ElasticsearchParseException.class) + public void testThatInvalidFileThrowsCorrectException() throws Exception { + writeConfigFile("deny: all allow: all \n\n"); + IPFilteringN2NAuthenticator.parseFile(configFile.toPath()); + } + + private void writeConfigFile(String data) throws IOException { + Files.write(data.getBytes(Charsets.UTF_8), configFile); + ipFilteringN2NAuthenticator = new IPFilteringN2NAuthenticator(settings, new Environment(), resourceWatcherService); + } + + private void assertAddressIsAllowed(IPFilteringN2NAuthenticator ipFilteringN2NAuthenticator, String ... inetAddresses) { + for (String inetAddress : inetAddresses) { + String message = String.format(Locale.ROOT, "Expected address %s to be allowed", inetAddress); + assertThat(message, ipFilteringN2NAuthenticator.authenticate(NULL_PRINCIPAL, InetAddresses.forString(inetAddress), 1024), is(true)); + } + } + + private void assertAddressIsDenied(IPFilteringN2NAuthenticator ipFilteringN2NAuthenticator, String ... inetAddresses) { + for (String inetAddress : inetAddresses) { + String message = String.format(Locale.ROOT, "Expected address %s to be denied", inetAddress); + assertThat(message, ipFilteringN2NAuthenticator.authenticate(NULL_PRINCIPAL, InetAddresses.forString(inetAddress), 1024), is(false)); + } } }