From 521ebe467246179281e7aa64d6fcdfcc3bb1c006 Mon Sep 17 00:00:00 2001 From: uboness Date: Thu, 23 Oct 2014 15:52:03 +0200 Subject: [PATCH] Change the way patterns are resolved in roles.yml Now, there are two types of supported patters: - wildcards (default) - simple wildcard match where `*` indicates zero or more characters and `?` indicates a single character (`\` can be used as an escape charachter) - regular expressions - can be "enabled" by wrapping the pattern in `/` (e.g. `/foo.*/`). The regex syntax is based on lucene's regex syntax (not Java's Pattern). Closes elastic/elasticsearch#253 Original commit: elastic/x-pack-elasticsearch@edd912122d8194260d996cf8fa4fac79b48d2f6f --- .../elasticsearch/shield/authz/Privilege.java | 32 ++++---- .../shield/support/Automatons.java | 68 +++++++++++++++- .../MultipleIndicesPermissionsTests.java | 4 +- .../shield/authz/PermissionTests.java | 6 +- .../IndicesResolverIntegrationTests.java | 4 +- .../authz/store/FileRolesStoreTests.java | 4 +- .../shield/support/AutomatonsTests.java | 80 +++++++++++++++++++ .../shield/test/ShieldIntegrationTest.java | 2 +- .../elasticsearch/test/ShieldRestTests.java | 2 +- .../shield/authz/store/roles.yml | 2 +- .../org/elasticsearch/shield/plugin/roles.yml | 2 +- 11 files changed, 173 insertions(+), 33 deletions(-) create mode 100644 src/test/java/org/elasticsearch/shield/support/AutomatonsTests.java diff --git a/src/main/java/org/elasticsearch/shield/authz/Privilege.java b/src/main/java/org/elasticsearch/shield/authz/Privilege.java index eb08de12769..821a93fb333 100644 --- a/src/main/java/org/elasticsearch/shield/authz/Privilege.java +++ b/src/main/java/org/elasticsearch/shield/authz/Privilege.java @@ -33,7 +33,7 @@ import static org.elasticsearch.shield.support.Automatons.patterns; */ public abstract class Privilege

> { - static final String SUB_ACTION_SUFFIX_PATTERN = ".*"; + static final String SUB_ACTION_SUFFIX_PATTERN = "*"; public static final System SYSTEM = new System(); @@ -76,7 +76,7 @@ public abstract class Privilege

> { public static class System extends Privilege { protected static final Predicate PREDICATE = new AutomatonPredicate(patterns( - "internal:.*" + "internal:*" )); private System() { @@ -97,18 +97,18 @@ public abstract class Privilege

> { public static class Index extends AutomatonPrivilege { public static final Index NONE = new Index(Name.NONE, Automata.makeEmpty()); - public static final Index ALL = new Index(Name.ALL, "indices:.*"); - public static final Index MANAGE = new Index("manage", "indices:monitor/.*", "indices:admin/.*"); + public static final Index ALL = new Index(Name.ALL, "indices:*"); + public static final Index MANAGE = new Index("manage", "indices:monitor/*", "indices:admin/*"); public static final Index CREATE_INDEX = new Index("create_index", "indices:admin/create"); - public static final Index MONITOR = new Index("monitor", "indices:monitor/.*"); - public static final Index DATA_ACCESS = new Index("data_access", "indices:data/.*"); - public static final Index CRUD = new Index("crud", "indices:data/write/.*", "indices:data/read/.*"); - public static final Index READ = new Index("read", "indices:data/read/.*"); - public static final Index SEARCH = new Index("search", SearchAction.NAME + ".*", GetAction.NAME + ".*"); - public static final Index GET = new Index("get", GetAction.NAME + ".*"); - public static final Index INDEX = new Index("index", "indices:data/write/index.*", "indices:data/write/update"); - public static final Index DELETE = new Index("delete", "indices:data/write/delete.*"); - public static final Index WRITE = new Index("write", "indices:data/write/.*"); + public static final Index MONITOR = new Index("monitor", "indices:monitor/*"); + public static final Index DATA_ACCESS = new Index("data_access", "indices:data/*"); + public static final Index CRUD = new Index("crud", "indices:data/write/*", "indices:data/read/*"); + public static final Index READ = new Index("read", "indices:data/read/*"); + public static final Index SEARCH = new Index("search", SearchAction.NAME + "*", GetAction.NAME + "*"); + public static final Index GET = new Index("get", GetAction.NAME + "*"); + public static final Index INDEX = new Index("index", "indices:data/write/index*", "indices:data/write/update"); + public static final Index DELETE = new Index("delete", "indices:data/write/delete*"); + public static final Index WRITE = new Index("write", "indices:data/write/*"); public static final Index BENCHMARK = new Index("benchmark", "indices:data/benchmark"); private static final Index[] values = new Index[] { @@ -196,8 +196,8 @@ public abstract class Privilege

> { public static class Cluster extends AutomatonPrivilege { public static final Cluster NONE = new Cluster(Name.NONE, Automata.makeEmpty()); - public static final Cluster ALL = new Cluster(Name.ALL, "cluster:.*", "indices:admin/template/.*"); - public static final Cluster MONITOR = new Cluster("monitor", "cluster:monitor/.*"); + public static final Cluster ALL = new Cluster(Name.ALL, "cluster:*", "indices:admin/template/*"); + public static final Cluster MONITOR = new Cluster("monitor", "cluster:monitor/*"); private static final Cluster[] values = new Cluster[] { NONE, ALL, MONITOR }; @@ -272,7 +272,7 @@ public abstract class Privilege

> { } static String actionToPattern(String text) { - return text.replace(":", "\\:") + SUB_ACTION_SUFFIX_PATTERN; + return text + SUB_ACTION_SUFFIX_PATTERN; } @SuppressWarnings("unchecked") diff --git a/src/main/java/org/elasticsearch/shield/support/Automatons.java b/src/main/java/org/elasticsearch/shield/support/Automatons.java index 0486672ebc9..c42b7c3ca0b 100644 --- a/src/main/java/org/elasticsearch/shield/support/Automatons.java +++ b/src/main/java/org/elasticsearch/shield/support/Automatons.java @@ -7,9 +7,12 @@ package org.elasticsearch.shield.support; import org.apache.lucene.util.automaton.Automata; import org.apache.lucene.util.automaton.Automaton; +import org.apache.lucene.util.automaton.Operations; import org.apache.lucene.util.automaton.RegExp; +import java.util.ArrayList; import java.util.Collection; +import java.util.List; import static org.apache.lucene.util.automaton.MinimizationOperations.minimize; import static org.apache.lucene.util.automaton.Operations.*; @@ -19,20 +22,30 @@ import static org.apache.lucene.util.automaton.Operations.*; */ public final class Automatons { + static final char WILDCARD_STRING = '*'; // String equality with support for wildcards + static final char WILDCARD_CHAR = '?'; // Char equality with support for wildcards + static final char WILDCARD_ESCAPE = '\\'; // Escape character + private Automatons() { } + /** + * Builds and returns an automaton that will represent the union of all the given patterns. + */ public static Automaton patterns(String... patterns) { if (patterns.length == 0) { return Automata.makeEmpty(); } - Automaton automaton = new RegExp(patterns[0]).toAutomaton(); + Automaton automaton = pattern(patterns[0]); for (String pattern : patterns) { - automaton = union(automaton, new RegExp(pattern).toAutomaton()); + automaton = union(automaton, pattern(pattern)); } return determinize(minimize(automaton)); } + /** + * Builds and returns an automaton that will represent the union of all the given patterns. + */ public static Automaton patterns(Collection patterns) { if (patterns.isEmpty()) { return Automata.makeEmpty(); @@ -40,14 +53,61 @@ public final class Automatons { Automaton automaton = null; for (String pattern : patterns) { if (automaton == null) { - automaton = new RegExp(pattern).toAutomaton(); + automaton = pattern(pattern); } else { - automaton = union(automaton, new RegExp(pattern).toAutomaton()); + automaton = union(automaton, pattern(pattern)); } } return determinize(minimize(automaton)); } + /** + * Builds and returns an automaton that represents the given pattern. + */ + static Automaton pattern(String pattern) { + if (pattern.startsWith("/")) { // it's a lucene regexp + if (pattern.length() == 1 || !pattern.endsWith("/")) { + throw new IllegalArgumentException("Invalid pattern [" + pattern + "]. Patterns starting with '/' " + + "indicate regular expression pattern and therefore must also end with '/'." + + " Other patterns (those that do not start with '/') will be treated as simple wildcard patterns"); + } + String regex = pattern.substring(1, pattern.length() - 1); + return new RegExp(regex).toAutomaton(); + } + return wildcard(pattern); + } + + /** + * Builds and returns an automaton that represents the given pattern. + */ + static Automaton wildcard(String text) { + List automata = new ArrayList<>(); + for (int i = 0; i < text.length();) { + final int c = text.codePointAt(i); + int length = Character.charCount(c); + switch(c) { + case WILDCARD_STRING: + automata.add(Automata.makeAnyString()); + break; + case WILDCARD_CHAR: + automata.add(Automata.makeAnyChar()); + break; + case WILDCARD_ESCAPE: + // add the next codepoint instead, if it exists + if (i + length < text.length()) { + final int nextChar = text.codePointAt(i + length); + length += Character.charCount(nextChar); + automata.add(Automata.makeChar(nextChar)); + break; + } // else fallthru, lenient parsing with a trailing \ + default: + automata.add(Automata.makeChar(c)); + } + i += length; + } + return Operations.concatenate(automata); + } + public static Automaton unionAndDeterminize(Automaton a1, Automaton a2) { return determinize(union(a1, a2)); } diff --git a/src/test/java/org/elasticsearch/shield/MultipleIndicesPermissionsTests.java b/src/test/java/org/elasticsearch/shield/MultipleIndicesPermissionsTests.java index 3cc2e2bae81..f27a24de418 100644 --- a/src/test/java/org/elasticsearch/shield/MultipleIndicesPermissionsTests.java +++ b/src/test/java/org/elasticsearch/shield/MultipleIndicesPermissionsTests.java @@ -27,8 +27,8 @@ public class MultipleIndicesPermissionsTests extends ShieldIntegrationTest { public static final String ROLES = "user:\n" + " cluster: all\n" + " indices:\n" + - " '.*': manage\n" + - " '.*': write\n" + + " '*': manage\n" + + " '/.*/': write\n" + " 'test': read\n" + " 'test1': read\n"; diff --git a/src/test/java/org/elasticsearch/shield/authz/PermissionTests.java b/src/test/java/org/elasticsearch/shield/authz/PermissionTests.java index acf6c481a07..a422bae8088 100644 --- a/src/test/java/org/elasticsearch/shield/authz/PermissionTests.java +++ b/src/test/java/org/elasticsearch/shield/authz/PermissionTests.java @@ -25,9 +25,9 @@ public class PermissionTests extends ElasticsearchTestCase { @Before public void init() { Permission.Global.Builder builder = Permission.Global.builder(mock(AuthorizationService.class)); - builder.add(union(SEARCH, MONITOR), "test_.*", "foo.*"); - builder.add(union(READ), "baz_.*foo", "fool.*bar"); - builder.add(union(MONITOR), "bar.*"); + builder.add(union(SEARCH, MONITOR), "test_*", "/foo.*/"); + builder.add(union(READ), "baz_*foo", "/fool.*bar/"); + builder.add(union(MONITOR), "/bar.*/"); permission = builder.build(); } diff --git a/src/test/java/org/elasticsearch/shield/authz/indicesresolver/IndicesResolverIntegrationTests.java b/src/test/java/org/elasticsearch/shield/authz/indicesresolver/IndicesResolverIntegrationTests.java index 0c34a1db0ab..55e005e9ea7 100644 --- a/src/test/java/org/elasticsearch/shield/authz/indicesresolver/IndicesResolverIntegrationTests.java +++ b/src/test/java/org/elasticsearch/shield/authz/indicesresolver/IndicesResolverIntegrationTests.java @@ -29,8 +29,8 @@ public class IndicesResolverIntegrationTests extends ShieldIntegrationTest { return DEFAULT_ROLE + ":\n" + " cluster: ALL\n" + " indices:\n" + - " '.*': manage,write\n" + - " 'test.*': read\n"; + " '*': manage,write\n" + + " '/test.*/': read\n"; } @Test 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 f6449d58329..920b4585e35 100644 --- a/src/test/java/org/elasticsearch/shield/authz/store/FileRolesStoreTests.java +++ b/src/test/java/org/elasticsearch/shield/authz/store/FileRolesStoreTests.java @@ -89,7 +89,7 @@ public class FileRolesStoreTests extends ElasticsearchTestCase { group = permission.indices().groups()[0]; assertThat(group.indices(), notNullValue()); assertThat(group.indices().length, is(1)); - assertThat(group.indices()[0], equalTo(".*_.*")); + assertThat(group.indices()[0], equalTo("/.*_.*/")); assertThat(group.privilege(), notNullValue()); assertThat(group.privilege().isAlias(Privilege.Index.union(Privilege.Index.READ, Privilege.Index.WRITE)), is(true)); } @@ -164,7 +164,7 @@ public class FileRolesStoreTests extends ElasticsearchTestCase { @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); + com.google.common.io.Files.write("user: cluster: ALL indices: '*': ALL".getBytes(Charsets.UTF_8), file); FileRolesStore.parseFile(file.toPath(), logger, mock(AuthorizationService.class)); } } diff --git a/src/test/java/org/elasticsearch/shield/support/AutomatonsTests.java b/src/test/java/org/elasticsearch/shield/support/AutomatonsTests.java new file mode 100644 index 00000000000..5857863d911 --- /dev/null +++ b/src/test/java/org/elasticsearch/shield/support/AutomatonsTests.java @@ -0,0 +1,80 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ +package org.elasticsearch.shield.support; + +import org.apache.lucene.util.automaton.Automaton; +import org.apache.lucene.util.automaton.CharacterRunAutomaton; +import org.elasticsearch.test.ElasticsearchTestCase; +import org.junit.Test; + +import static org.elasticsearch.shield.support.Automatons.*; +import static org.hamcrest.Matchers.*; + +/** + * + */ +public class AutomatonsTests extends ElasticsearchTestCase { + + @Test + public void testPatterns_UnionOfMultiplePatterns() throws Exception { + assertMatch(patterns("/fo.*/", "ba*"), "foo"); + assertMatch(patterns("/fo.*/", "ba*"), "bar"); + assertMismatch(patterns("/fo.*/", "ba*"), "zipfoo"); + } + + @Test + public void testPattern_Single() throws Exception { + assertMatch(pattern("/.*st/"), "test"); + assertMatch(pattern("/t.*st/"), "test"); + assertMatch(pattern("/tes*./"), "test"); + assertMatch(pattern("/test/"), "test"); + assertMismatch(pattern("/.*st/"), "tet"); + assertMatch(pattern("*st"), "test"); + assertMatch(pattern("t*t"), "test"); + assertMatch(pattern("t?st"), "test"); + assertMismatch(pattern("t?t"), "test"); + assertMatch(pattern("tes*"), "test"); + assertMatch(pattern("test"), "test"); + assertMismatch(pattern("*st"), "tet"); + assertInvalidPattern("/test"); + assertInvalidPattern("/te*"); + assertInvalidPattern("/te.*"); + assertMismatch(pattern(".*st"), "test"); + assertMatch(pattern("*st\\"), "test\\"); + assertMatch(pattern("tes.*/"), "tes.t/"); + assertMatch(pattern("\\/test"), "/test"); + } + + @Test + public void testWildcard() throws Exception { + assertMatch(wildcard("*st"), "test"); + assertMatch(wildcard("t*st"), "test"); + assertMatch(wildcard("tes*"), "test"); + assertMatch(wildcard("test"), "test"); + assertMismatch(wildcard("*st"), "tet"); + assertMismatch(wildcard("t\\*st"), "test"); + assertMatch(wildcard("t\\*st"), "t*st"); + } + + private void assertMatch(Automaton automaton, String text) { + CharacterRunAutomaton runAutomaton = new CharacterRunAutomaton(automaton); + assertThat(runAutomaton.run(text), is(true)); + } + + private void assertMismatch(Automaton automaton, String text) { + CharacterRunAutomaton runAutomaton = new CharacterRunAutomaton(automaton); + assertThat(runAutomaton.run(text), is(false)); + } + + private void assertInvalidPattern(String text) { + try { + pattern(text); + fail("expected an error on invalid pattern [" + text + "]"); + } catch (IllegalArgumentException iae) { + // expected + } + } +} diff --git a/src/test/java/org/elasticsearch/shield/test/ShieldIntegrationTest.java b/src/test/java/org/elasticsearch/shield/test/ShieldIntegrationTest.java index 1dfa2a01b9c..d2825fa077f 100644 --- a/src/test/java/org/elasticsearch/shield/test/ShieldIntegrationTest.java +++ b/src/test/java/org/elasticsearch/shield/test/ShieldIntegrationTest.java @@ -53,7 +53,7 @@ public abstract class ShieldIntegrationTest extends ElasticsearchIntegrationTest public static final String CONFIG_ROLE_ALLOW_ALL = DEFAULT_ROLE + ":\n" + " cluster: ALL\n" + " indices:\n" + - " '.*': ALL\n"; + " '*': ALL\n"; @ClassRule public static TemporaryFolder tmpFolder = new TemporaryFolder(); diff --git a/src/test/java/org/elasticsearch/test/ShieldRestTests.java b/src/test/java/org/elasticsearch/test/ShieldRestTests.java index 3a83e723908..bee7579c5d4 100644 --- a/src/test/java/org/elasticsearch/test/ShieldRestTests.java +++ b/src/test/java/org/elasticsearch/test/ShieldRestTests.java @@ -56,7 +56,7 @@ public class ShieldRestTests extends ElasticsearchRestTests { public static final String CONFIG_ROLE_ALLOW_ALL = DEFAULT_ROLE + ":\n" + " cluster: ALL\n" + " indices:\n" + - " '.*': ALL\n"; + " '*': ALL\n"; static { diff --git a/src/test/resources/org/elasticsearch/shield/authz/store/roles.yml b/src/test/resources/org/elasticsearch/shield/authz/store/roles.yml index 2ab39b7a510..0c9b98823d4 100644 --- a/src/test/resources/org/elasticsearch/shield/authz/store/roles.yml +++ b/src/test/resources/org/elasticsearch/shield/authz/store/roles.yml @@ -9,5 +9,5 @@ role2: role3: indices: - '.*_.*': READ, WRITE + '/.*_.*/': READ, WRITE diff --git a/src/test/resources/org/elasticsearch/shield/plugin/roles.yml b/src/test/resources/org/elasticsearch/shield/plugin/roles.yml index 6099c708ed9..b4feb8357bd 100644 --- a/src/test/resources/org/elasticsearch/shield/plugin/roles.yml +++ b/src/test/resources/org/elasticsearch/shield/plugin/roles.yml @@ -1,4 +1,4 @@ user: cluster: ALL indices: - '.*': ALL \ No newline at end of file + '*': ALL \ No newline at end of file