Skip to content

Commit 1f392a7

Browse files
tvernumtvernum
authored
Support roles with application privileges against wildcard applications (#40398)
This commit introduces 2 changes to application privileges: - The validation rules now accept a wildcard in the "suffix" of an application name. Wildcards were always accepted in the application name, but the "valid filename" check for the suffix incorrectly prevented the use of wildcards there. - A role may now be defined against a wildcard application (e.g. kibana-*) and this will be correctly treated as granting the named privileges against all named applications. This does not allow wildcard application names in the body of a "has-privileges" check, but the "has-privileges" check can test concrete application names against roles with wildcards.
1 parent 7b6e714 commit 1f392a7

File tree

9 files changed

+302
-38
lines changed

9 files changed

+302
-38
lines changed

‎x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/ApplicationPermission.java‎

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -104,15 +104,18 @@ public ResourcePrivilegesMap checkResourcePrivileges(final String applicationNam
104104
for (String checkResource : checkForResources) {
105105
for (String checkPrivilegeName : checkForPrivilegeNames) {
106106
final Set<String> nameSet = Collections.singleton(checkPrivilegeName);
107-
final ApplicationPrivilege checkPrivilege = ApplicationPrivilege.get(applicationName, nameSet, storedPrivileges);
108-
assert checkPrivilege.getApplication().equals(applicationName) : "Privilege " + checkPrivilege + " should have application "
109-
+ applicationName;
110-
assert checkPrivilege.name().equals(nameSet) : "Privilege " + checkPrivilege + " should have name " + nameSet;
111-
112-
if (grants(checkPrivilege, checkResource)) {
113-
resourcePrivilegesMapBuilder.addResourcePrivilege(checkResource, checkPrivilegeName, Boolean.TRUE);
114-
} else {
115-
resourcePrivilegesMapBuilder.addResourcePrivilege(checkResource, checkPrivilegeName, Boolean.FALSE);
107+
final Set<ApplicationPrivilege> checkPrivileges = ApplicationPrivilege.get(applicationName, nameSet, storedPrivileges);
108+
logger.trace("Resolved privileges [{}] for [{},{}]", checkPrivileges, applicationName, nameSet);
109+
for (ApplicationPrivilege checkPrivilege : checkPrivileges) {
110+
assert Automatons.predicate(applicationName).test(checkPrivilege.getApplication()) : "Privilege " + checkPrivilege +
111+
" should have application " + applicationName;
112+
assert checkPrivilege.name().equals(nameSet) : "Privilege " + checkPrivilege + " should have name " + nameSet;
113+
114+
if (grants(checkPrivilege, checkResource)) {
115+
resourcePrivilegesMapBuilder.addResourcePrivilege(checkResource, checkPrivilegeName, Boolean.TRUE);
116+
} else {
117+
resourcePrivilegesMapBuilder.addResourcePrivilege(checkResource, checkPrivilegeName, Boolean.FALSE);
118+
}
116119
}
117120
}
118121
}

‎x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ApplicationPrivilege.java‎

Lines changed: 32 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
package org.elasticsearch.xpack.core.security.authz.privilege;
77

88
import org.elasticsearch.common.Strings;
9+
import org.elasticsearch.xpack.core.security.support.Automatons;
910

1011
import java.util.Arrays;
1112
import java.util.Collection;
@@ -15,6 +16,7 @@
1516
import java.util.Objects;
1617
import java.util.Set;
1718
import java.util.function.Function;
19+
import java.util.function.Predicate;
1820
import java.util.regex.Pattern;
1921
import java.util.stream.Collectors;
2022

@@ -101,7 +103,7 @@ private static void validateApplicationName(String application, boolean allowWil
101103
if (allowWildcard == false) {
102104
throw new IllegalArgumentException("Application names may not contain '*' (found '" + application + "')");
103105
}
104-
if(application.equals("*")) {
106+
if (application.equals("*")) {
105107
// this is allowed and short-circuiting here makes the later validation simpler
106108
return;
107109
}
@@ -128,7 +130,10 @@ private static void validateApplicationName(String application, boolean allowWil
128130
}
129131

130132
if (parts.length > 1) {
131-
final String suffix = parts[1];
133+
String suffix = parts[1];
134+
if (allowWildcard && suffix.endsWith("*")) {
135+
suffix = suffix.substring(0, suffix.length() - 1);
136+
}
132137
if (Strings.validFileName(suffix) == false) {
133138
throw new IllegalArgumentException("An application name suffix may not contain any of the characters '" +
134139
Strings.collectionToDelimitedString(Strings.INVALID_FILENAME_CHARS, "") + "' (found '" + suffix + "')");
@@ -165,20 +170,38 @@ public static void validatePrivilegeOrActionName(String name) {
165170
}
166171

167172
/**
168-
* Finds or creates an application privileges with the provided names.
173+
* Finds or creates a collection of application privileges with the provided names.
174+
* If application is a wildcard, it will be expanded to all matching application names in {@code stored}
169175
* Each element in {@code name} may be the name of a stored privilege (to be resolved from {@code stored}, or a bespoke action pattern.
170176
*/
171-
public static ApplicationPrivilege get(String application, Set<String> name, Collection<ApplicationPrivilegeDescriptor> stored) {
177+
public static Set<ApplicationPrivilege> get(String application, Set<String> name, Collection<ApplicationPrivilegeDescriptor> stored) {
172178
if (name.isEmpty()) {
173-
return NONE.apply(application);
179+
return Collections.singleton(NONE.apply(application));
180+
} else if (application.contains("*")) {
181+
Predicate<String> predicate = Automatons.predicate(application);
182+
final Set<ApplicationPrivilege> result = stored.stream()
183+
.map(ApplicationPrivilegeDescriptor::getApplication)
184+
.filter(predicate)
185+
.distinct()
186+
.map(appName -> resolve(appName, name, stored))
187+
.collect(Collectors.toSet());
188+
if (result.isEmpty()) {
189+
return Collections.singleton(resolve(application, name, Collections.emptyMap()));
190+
} else {
191+
return result;
192+
}
174193
} else {
175-
Map<String, ApplicationPrivilegeDescriptor> lookup = stored.stream()
176-
.filter(apd -> apd.getApplication().equals(application))
177-
.collect(Collectors.toMap(ApplicationPrivilegeDescriptor::getName, Function.identity()));
178-
return resolve(application, name, lookup);
194+
return Collections.singleton(resolve(application, name, stored));
179195
}
180196
}
181197

198+
private static ApplicationPrivilege resolve(String application, Set<String> name, Collection<ApplicationPrivilegeDescriptor> stored) {
199+
final Map<String, ApplicationPrivilegeDescriptor> lookup = stored.stream()
200+
.filter(apd -> apd.getApplication().equals(application))
201+
.collect(Collectors.toMap(ApplicationPrivilegeDescriptor::getName, Function.identity()));
202+
return resolve(application, name, lookup);
203+
}
204+
182205
private static ApplicationPrivilege resolve(String application, Set<String> names, Map<String, ApplicationPrivilegeDescriptor> lookup) {
183206
final int size = names.size();
184207
if (size == 0) {

‎x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ApplicationPrivilegeDescriptor.java‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@
2323
import java.util.Objects;
2424
import java.util.Set;
2525

26+
import static org.elasticsearch.common.Strings.collectionToCommaDelimitedString;
27+
2628
/**
2729
* An {@code ApplicationPrivilegeDescriptor} is a representation of a <em>stored</em> {@link ApplicationPrivilege}.
2830
* A user (via a role) can be granted an application privilege by name (e.g. ("myapp", "read").
@@ -104,6 +106,11 @@ public XContentBuilder toXContent(XContentBuilder builder, boolean includeTypeFi
104106
return builder.endObject();
105107
}
106108

109+
@Override
110+
public String toString() {
111+
return getClass().getSimpleName() + "{[" + application + "],[" + name + "],[" + collectionToCommaDelimitedString(actions) + "]}";
112+
}
113+
107114
/**
108115
* Construct a new {@link ApplicationPrivilegeDescriptor} from XContent.
109116
*

‎x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/ApplicationPermissionTests.java‎

Lines changed: 28 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,15 +13,16 @@
1313

1414
import java.util.ArrayList;
1515
import java.util.Arrays;
16+
import java.util.Collection;
1617
import java.util.Collections;
1718
import java.util.List;
1819
import java.util.Set;
1920
import java.util.stream.Collectors;
2021
import java.util.stream.Stream;
2122

22-
import static java.util.Collections.singletonList;
2323
import static org.hamcrest.Matchers.containsInAnyOrder;
2424
import static org.hamcrest.Matchers.equalTo;
25+
import static org.hamcrest.Matchers.hasSize;
2526

2627
public class ApplicationPermissionTests extends ESTestCase {
2728

@@ -34,6 +35,7 @@ public class ApplicationPermissionTests extends ESTestCase {
3435
private ApplicationPrivilege app1Delete = storePrivilege("app1", "delete", "write/delete");
3536
private ApplicationPrivilege app1Create = storePrivilege("app1", "create", "write/create");
3637
private ApplicationPrivilege app2Read = storePrivilege("app2", "read", "read/*");
38+
private ApplicationPrivilege otherAppRead = storePrivilege("other-app", "read", "read/*");
3739

3840
private ApplicationPrivilege storePrivilege(String app, String name, String... patterns) {
3941
store.add(new ApplicationPrivilegeDescriptor(app, name, Sets.newHashSet(patterns), Collections.emptyMap()));
@@ -104,6 +106,16 @@ public void testDoesNotMatchAcrossApplications() {
104106
assertThat(buildPermission(app1All, "*").grants(app2Read, "123"), equalTo(false));
105107
}
106108

109+
public void testMatchingWithWildcardApplicationNames() {
110+
final Set<ApplicationPrivilege> readAllApp = ApplicationPrivilege.get("app*", Collections.singleton("read"), store);
111+
assertThat(buildPermission(readAllApp, "*").grants(app1Read, "123"), equalTo(true));
112+
assertThat(buildPermission(readAllApp, "foo/*").grants(app2Read, "foo/bar"), equalTo(true));
113+
114+
assertThat(buildPermission(readAllApp, "*").grants(app1Write, "123"), equalTo(false));
115+
assertThat(buildPermission(readAllApp, "foo/*").grants(app2Read, "bar/baz"), equalTo(false));
116+
assertThat(buildPermission(readAllApp, "*").grants(otherAppRead, "abc"), equalTo(false));
117+
}
118+
107119
public void testMergedPermissionChecking() {
108120
final ApplicationPrivilege app1ReadWrite = compositePrivilege("app1", app1Read, app1Write);
109121
final ApplicationPermission hasPermission = buildPermission(app1ReadWrite, "allow/*");
@@ -138,16 +150,27 @@ public void testInspectPermissionContents() {
138150
}
139151

140152
private ApplicationPrivilege actionPrivilege(String appName, String... actions) {
141-
return ApplicationPrivilege.get(appName, Sets.newHashSet(actions), Collections.emptyList());
153+
final Set<ApplicationPrivilege> privileges = ApplicationPrivilege.get(appName, Sets.newHashSet(actions), Collections.emptyList());
154+
assertThat(privileges, hasSize(1));
155+
return privileges.iterator().next();
142156
}
143157

144158
private ApplicationPrivilege compositePrivilege(String application, ApplicationPrivilege... children) {
145159
Set<String> names = Stream.of(children).map(ApplicationPrivilege::name).flatMap(Set::stream).collect(Collectors.toSet());
146-
return ApplicationPrivilege.get(application, names, store);
160+
final Set<ApplicationPrivilege> privileges = ApplicationPrivilege.get(application, names, store);
161+
assertThat(privileges, hasSize(1));
162+
return privileges.iterator().next();
147163
}
148164

149-
150165
private ApplicationPermission buildPermission(ApplicationPrivilege privilege, String... resources) {
151-
return new ApplicationPermission(singletonList(new Tuple<>(privilege, Sets.newHashSet(resources))));
166+
return buildPermission(Collections.singleton(privilege), resources);
167+
}
168+
169+
private ApplicationPermission buildPermission(Collection<ApplicationPrivilege> privileges, String... resources) {
170+
final Set<String> resourceSet = Sets.newHashSet(resources);
171+
final List<Tuple<ApplicationPrivilege, Set<String>>> privilegesAndResources = privileges.stream()
172+
.map(p -> new Tuple<>(p, resourceSet))
173+
.collect(Collectors.toList());
174+
return new ApplicationPermission(privilegesAndResources);
152175
}
153176
}

‎x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/privilege/ApplicationPrivilegeTests.java‎

Lines changed: 25 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import org.elasticsearch.common.util.set.Sets;
1111
import org.elasticsearch.test.ESTestCase;
1212
import org.elasticsearch.test.EqualsHashCodeTestUtils;
13+
import org.hamcrest.Matchers;
1314
import org.junit.Assert;
1415

1516
import java.util.Arrays;
@@ -22,9 +23,11 @@
2223

2324
import static org.elasticsearch.common.Strings.collectionToCommaDelimitedString;
2425
import static org.hamcrest.Matchers.arrayContainingInAnyOrder;
26+
import static org.hamcrest.Matchers.contains;
2527
import static org.hamcrest.Matchers.containsInAnyOrder;
2628
import static org.hamcrest.Matchers.containsString;
2729
import static org.hamcrest.Matchers.equalTo;
30+
import static org.hamcrest.Matchers.iterableWithSize;
2831

2932
public class ApplicationPrivilegeTests extends ESTestCase {
3033

@@ -59,6 +62,12 @@ public void testValidationOfApplicationName() {
5962
assertNoException(app, () -> ApplicationPrivilege.validateApplicationName(app));
6063
assertNoException(app, () -> ApplicationPrivilege.validateApplicationNameOrWildcard(app));
6164
}
65+
66+
// wildcards in the suffix
67+
for (String app : Arrays.asList("app1-*", "app1-foo*", "app1-.*", "app1-.foo.*", appNameWithSpecialChars + "*")) {
68+
assertValidationFailure(app, "application name", () -> ApplicationPrivilege.validateApplicationName(app));
69+
assertNoException(app, () -> ApplicationPrivilege.validateApplicationNameOrWildcard(app));
70+
}
6271
}
6372

6473
public void testValidationOfPrivilegeName() {
@@ -101,16 +110,23 @@ public void testNonePrivilege() {
101110
}
102111

103112
public void testGetPrivilegeByName() {
104-
final ApplicationPrivilegeDescriptor descriptor = descriptor("my-app", "read", "data:read/*", "action:login");
113+
final ApplicationPrivilegeDescriptor myRead = descriptor("my-app", "read", "data:read/*", "action:login");
105114
final ApplicationPrivilegeDescriptor myWrite = descriptor("my-app", "write", "data:write/*", "action:login");
106115
final ApplicationPrivilegeDescriptor myAdmin = descriptor("my-app", "admin", "data:read/*", "action:*");
107116
final ApplicationPrivilegeDescriptor yourRead = descriptor("your-app", "read", "data:read/*", "action:login");
108-
final Set<ApplicationPrivilegeDescriptor> stored = Sets.newHashSet(descriptor, myWrite, myAdmin, yourRead);
117+
final Set<ApplicationPrivilegeDescriptor> stored = Sets.newHashSet(myRead, myWrite, myAdmin, yourRead);
118+
119+
final Set<ApplicationPrivilege> myAppRead = ApplicationPrivilege.get("my-app", Collections.singleton("read"), stored);
120+
assertThat(myAppRead, iterableWithSize(1));
121+
assertPrivilegeEquals(myAppRead.iterator().next(), myRead);
109122

110-
assertEqual(ApplicationPrivilege.get("my-app", Collections.singleton("read"), stored), descriptor);
111-
assertEqual(ApplicationPrivilege.get("my-app", Collections.singleton("write"), stored), myWrite);
123+
final Set<ApplicationPrivilege> myAppWrite = ApplicationPrivilege.get("my-app", Collections.singleton("write"), stored);
124+
assertThat(myAppWrite, iterableWithSize(1));
125+
assertPrivilegeEquals(myAppWrite.iterator().next(), myWrite);
112126

113-
final ApplicationPrivilege readWrite = ApplicationPrivilege.get("my-app", Sets.newHashSet("read", "write"), stored);
127+
final Set<ApplicationPrivilege> myReadWrite = ApplicationPrivilege.get("my-app", Sets.newHashSet("read", "write"), stored);
128+
assertThat(myReadWrite, Matchers.hasSize(1));
129+
final ApplicationPrivilege readWrite = myReadWrite.iterator().next();
114130
assertThat(readWrite.getApplication(), equalTo("my-app"));
115131
assertThat(readWrite.name(), containsInAnyOrder("read", "write"));
116132
assertThat(readWrite.getPatterns(), arrayContainingInAnyOrder("data:read/*", "data:write/*", "action:login"));
@@ -124,10 +140,10 @@ public void testGetPrivilegeByName() {
124140
}
125141
}
126142

127-
private void assertEqual(ApplicationPrivilege myReadPriv, ApplicationPrivilegeDescriptor myRead) {
128-
assertThat(myReadPriv.getApplication(), equalTo(myRead.getApplication()));
129-
assertThat(getPrivilegeName(myReadPriv), equalTo(myRead.getName()));
130-
assertThat(Sets.newHashSet(myReadPriv.getPatterns()), equalTo(myRead.getActions()));
143+
private void assertPrivilegeEquals(ApplicationPrivilege privilege, ApplicationPrivilegeDescriptor descriptor) {
144+
assertThat(privilege.getApplication(), equalTo(descriptor.getApplication()));
145+
assertThat(privilege.name(), contains(descriptor.getName()));
146+
assertThat(Sets.newHashSet(privilege.getPatterns()), equalTo(descriptor.getActions()));
131147
}
132148

133149
private ApplicationPrivilegeDescriptor descriptor(String application, String name, String... actions) {

‎x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/store/CompositeRolesStore.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -402,8 +402,8 @@ public static void buildRoleFromDescriptors(Collection<RoleDescriptor> roleDescr
402402
.flatMap(Collection::stream)
403403
.collect(Collectors.toSet());
404404
privilegeStore.getPrivileges(applicationNames, applicationPrivilegeNames, ActionListener.wrap(appPrivileges -> {
405-
applicationPrivilegesMap.forEach((key, names) ->
406-
builder.addApplicationPrivilege(ApplicationPrivilege.get(key.v1(), names, appPrivileges), key.v2()));
405+
applicationPrivilegesMap.forEach((key, names) -> ApplicationPrivilege.get(key.v1(), names, appPrivileges)
406+
.forEach(priv -> builder.addApplicationPrivilege(priv, key.v2())));
407407
listener.onResponse(builder.build());
408408
}, listener::onFailure));
409409
}

0 commit comments

Comments
 (0)