From e177f79aa3a68ee043ad0a4c5fcbe22d85dfac72 Mon Sep 17 00:00:00 2001 From: Tim Vernum Date: Tue, 30 May 2017 13:40:29 +1000 Subject: [PATCH] Support wildcards in has_privileges API (elastic/x-pack-elasticsearch#1454) The has_privileges API now supports wildcards. The semantics are that the user must have a superset of the wildcard being checked. --------------------- Role | Check | Result --------------------- * | foo* | true f* | foo* | true foo* | foo* | true foo* | foo? | true foo? | foo? | true foo? | foo* | false foo | foo* | false Original commit: elastic/x-pack-elasticsearch@817550db17c40969c163388a3855636cc99a56dc --- .../user/TransportHasPrivilegesAction.java | 17 +++--- .../TransportHasPrivilegesActionTests.java | 53 +++++++++++++++---- 2 files changed, 54 insertions(+), 16 deletions(-) diff --git a/plugin/src/main/java/org/elasticsearch/xpack/security/action/user/TransportHasPrivilegesAction.java b/plugin/src/main/java/org/elasticsearch/xpack/security/action/user/TransportHasPrivilegesAction.java index 78bd0ffec31..cdb2f174552 100644 --- a/plugin/src/main/java/org/elasticsearch/xpack/security/action/user/TransportHasPrivilegesAction.java +++ b/plugin/src/main/java/org/elasticsearch/xpack/security/action/user/TransportHasPrivilegesAction.java @@ -12,7 +12,6 @@ import java.util.HashMap; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; -import java.util.function.Predicate; import org.apache.lucene.util.automaton.Automaton; import org.apache.lucene.util.automaton.Operations; @@ -81,7 +80,7 @@ public class TransportHasPrivilegesAction extends HandledTransportAction> predicateCache = new HashMap<>(); + final Map predicateCache = new HashMap<>(); final Map indices = new LinkedHashMap<>(); boolean allMatch = true; @@ -109,13 +108,15 @@ public class TransportHasPrivilegesAction extends HandledTransportAction> predicateCache) { + Map predicateCache) { final IndexPrivilege checkPrivilege = IndexPrivilege.get(Collections.singleton(checkPrivilegeName)); + final Automaton checkIndexAutomaton = Automatons.patterns(checkIndex); + List privilegeAutomatons = new ArrayList<>(); for (IndicesPermission.Group group : userRole.indices().groups()) { - final Predicate predicate = predicateCache.computeIfAbsent(group, g -> Automatons.predicate(g.indices())); - if (predicate.test(checkIndex)) { + final Automaton groupIndexAutomaton = predicateCache.computeIfAbsent(group, g -> Automatons.patterns(g.indices())); + if (testIndex(checkIndexAutomaton, groupIndexAutomaton)) { final IndexPrivilege rolePrivilege = group.privilege(); if (rolePrivilege.name().contains(checkPrivilegeName)) { return true; @@ -126,7 +127,11 @@ public class TransportHasPrivilegesAction extends HandledTransportActionlog* in the request isn't granted by logstash-* - * in the role, but logstash-2016-* is, because it's just - * treated as the name of an index. + * Wildcards in the request are treated as + * does the user have ___ privilege on every possible index that matches this pattern? + * Or, expressed differently, + * does the user have ___ privilege on a wildcard that covers (is a superset of) this pattern? */ - public void testWildcardsInRequestAreIgnored() throws Exception { + public void testWildcardHandling() throws Exception { role = Role.builder("test3") - .add(IndexPrivilege.ALL, "logstash-*") + .add(IndexPrivilege.ALL, "logstash-*", "foo?") + .add(IndexPrivilege.READ, "abc*") + .add(IndexPrivilege.WRITE, "*xyz") .build(); final HasPrivilegesRequest request = new HasPrivilegesRequest(); @@ -207,11 +209,31 @@ public class TransportHasPrivilegesActionTests extends ESTestCase { request.indexPrivileges( RoleDescriptor.IndicesPrivileges.builder() .indices("logstash-2016-*") - .privileges("write") + .privileges("write") // Yes, because (ALL,"logstash-*") + .build(), + RoleDescriptor.IndicesPrivileges.builder() + .indices("logstash-*") + .privileges("read") // Yes, because (ALL,"logstash-*") .build(), RoleDescriptor.IndicesPrivileges.builder() .indices("log*") - .privileges("read") + .privileges("manage") // No, because "log*" includes indices that "logstash-*" does not + .build(), + RoleDescriptor.IndicesPrivileges.builder() + .indices("foo*", "foo?") + .privileges("read") // Yes, "foo?", but not "foo*", because "foo*" > "foo?" + .build(), + RoleDescriptor.IndicesPrivileges.builder() + .indices("abcd*") + .privileges("read", "write") // read = Yes, because (READ, "abc*"), write = No + .build(), + RoleDescriptor.IndicesPrivileges.builder() + .indices("abc*xyz") + .privileges("read", "write", "manage") // read = Yes ( READ "abc*"), write = Yes (WRITE, "*xyz"), manage = No + .build(), + RoleDescriptor.IndicesPrivileges.builder() + .indices("a*xyz") + .privileges("read", "write", "manage") // read = No, write = Yes (WRITE, "*xyz"), manage = No .build() ); final PlainActionFuture future = new PlainActionFuture(); @@ -220,10 +242,16 @@ public class TransportHasPrivilegesActionTests extends ESTestCase { final HasPrivilegesResponse response = future.get(); assertThat(response, notNullValue()); assertThat(response.isCompleteMatch(), is(false)); - assertThat(response.getIndexPrivileges(), Matchers.iterableWithSize(2)); + assertThat(response.getIndexPrivileges(), Matchers.iterableWithSize(8)); assertThat(response.getIndexPrivileges(), containsInAnyOrder( new IndexPrivileges("logstash-2016-*", Collections.singletonMap("write", true)), - new IndexPrivileges("log*", Collections.singletonMap("read", false)) + new IndexPrivileges("logstash-*", Collections.singletonMap("read", true)), + new IndexPrivileges("log*", Collections.singletonMap("manage", false)), + new IndexPrivileges("foo?", Collections.singletonMap("read", true)), + new IndexPrivileges("foo*", Collections.singletonMap("read", false)), + new IndexPrivileges("abcd*", mapBuilder().put("read", true).put("write", false).map()), + new IndexPrivileges("abc*xyz", mapBuilder().put("read", true).put("write", true).put("manage", false).map()), + new IndexPrivileges("a*xyz", mapBuilder().put("read", false).put("write", true).put("manage", false).map()) )); } @@ -259,4 +287,9 @@ public class TransportHasPrivilegesActionTests extends ESTestCase { ) )); } + + private static MapBuilder mapBuilder() { + return MapBuilder.newMapBuilder(); + } + } \ No newline at end of file