Skip to content

Commit 44a3c97

Browse files
authored
[8.2] Fix resolution of wildcard application privileges (#87359)
Previously, a role with application privileges of { "application": "*", "privileges": ["*"] "resources": ["*"] } would be resolved to mean every _defined_ privilege in every _defined_ application (subject to condition 1 described below). This implementation was based on the assumption that every action that would ever be checked by _has_privileges would be explicitly defined as an application privilege (via PUT /_security/privilege) That assumption would have been fine if not for 2 discrepancies 1. The resolved privileges were then filtered by privilege name and wildcards were not respected. So the role shown above would actually filter down to nothing. However if the user had another role with named privileges (not wildcards) then the filtering would include that privilege _if it was defined_. 2. The logic to construct a runtime ApplicationPrivilege from the resolved ApplicationPrivilegeDescriptor instances had special case logic to handle 0 matching descriptors. This was needed so that superuser always had every privilege, even if the descriptors had not yet been defined, or where unavailable for some reason. However because this logic only applied if there were no matching descriptors the behaviour was inconsistent. For the most part it would look like wildcard application privileges functioned correctly even when no application privileges were defined, but this behaviour could not be relied on if the user had additional roles that references defined application privileges (per point 1). This change solves point 2, by always including the wildcard permission even if there are matching descriptors. It does nothing to solve point 1, and it is likely that we need additional commits to cleanup the logic there. Backport of: #87293
1 parent 0ef67a3 commit 44a3c97

File tree

5 files changed

+331
-12
lines changed

5 files changed

+331
-12
lines changed

docs/changelog/87293.yaml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
pr: 87293
2+
summary: Fix resolution of wildcard application privileges
3+
area: Authorization
4+
type: bug
5+
issues: []

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

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

99
import org.elasticsearch.common.Strings;
10+
import org.elasticsearch.common.util.set.Sets;
1011
import org.elasticsearch.xpack.core.security.support.Automatons;
1112

1213
import java.util.Arrays;
@@ -192,18 +193,15 @@ public static Set<ApplicationPrivilege> get(String application, Set<String> name
192193
if (name.isEmpty()) {
193194
return Collections.singleton(NONE.apply(application));
194195
} else if (application.contains("*")) {
195-
Predicate<String> predicate = Automatons.predicate(application);
196-
final Set<ApplicationPrivilege> result = stored.stream()
196+
final Set<ApplicationPrivilege> result = Sets.newHashSet(resolve(application, name, Collections.emptyMap()));
197+
final Predicate<String> predicate = Automatons.predicate(application);
198+
stored.stream()
197199
.map(ApplicationPrivilegeDescriptor::getApplication)
198200
.filter(predicate)
199201
.distinct()
200202
.map(appName -> resolve(appName, name, stored))
201-
.collect(Collectors.toSet());
202-
if (result.isEmpty()) {
203-
return Collections.singleton(resolve(application, name, Collections.emptyMap()));
204-
} else {
205-
return result;
206-
}
203+
.forEach(result::add);
204+
return result;
207205
} else {
208206
return Collections.singleton(resolve(application, name, stored));
209207
}

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

Lines changed: 47 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@
1212
import org.elasticsearch.common.util.set.Sets;
1313
import org.elasticsearch.test.ESTestCase;
1414
import org.elasticsearch.test.EqualsHashCodeTestUtils;
15+
import org.hamcrest.CustomTypeSafeMatcher;
16+
import org.hamcrest.Matcher;
1517
import org.hamcrest.Matchers;
1618
import org.junit.Assert;
1719

@@ -25,10 +27,10 @@
2527

2628
import static org.elasticsearch.common.Strings.collectionToCommaDelimitedString;
2729
import static org.hamcrest.Matchers.arrayContainingInAnyOrder;
28-
import static org.hamcrest.Matchers.contains;
2930
import static org.hamcrest.Matchers.containsInAnyOrder;
3031
import static org.hamcrest.Matchers.containsString;
3132
import static org.hamcrest.Matchers.equalTo;
33+
import static org.hamcrest.Matchers.hasItem;
3234
import static org.hamcrest.Matchers.iterableWithSize;
3335

3436
public class ApplicationPrivilegeTests extends ESTestCase {
@@ -145,10 +147,51 @@ public void testGetPrivilegeByName() {
145147
}
146148
}
147149

150+
public void testGetPrivilegeByWildcard() {
151+
final ApplicationPrivilegeDescriptor apmRead = descriptor("apm", "read", "action:read/*");
152+
final ApplicationPrivilegeDescriptor apmWrite = descriptor("apm", "write", "action:write/*");
153+
final ApplicationPrivilegeDescriptor kibanaRead = descriptor("kibana", "read", "data:read/*", "action:read:*");
154+
final ApplicationPrivilegeDescriptor kibanaWrite = descriptor("kibana", "write", "data:write/*", "action:w*");
155+
final Set<ApplicationPrivilegeDescriptor> stored = Sets.newHashSet(apmRead, apmWrite, kibanaRead, kibanaWrite);
156+
157+
{
158+
final Set<ApplicationPrivilege> everyThing = ApplicationPrivilege.get("*", Set.of("*"), stored);
159+
assertThat(everyThing, hasItem(privilegeEquals("*", "*", Set.of("*"))));
160+
assertThat(everyThing, hasItem(privilegeEquals("apm", "*", Set.of("*"))));
161+
assertThat(everyThing, hasItem(privilegeEquals("kibana", "*", Set.of("*"))));
162+
assertThat(everyThing, iterableWithSize(3));
163+
}
164+
{
165+
final Set<ApplicationPrivilege> allKibana = ApplicationPrivilege.get("kibana", Set.of("*"), stored);
166+
assertThat(allKibana, hasItem(privilegeEquals("kibana", "*", Set.of("*"))));
167+
assertThat(allKibana, iterableWithSize(1));
168+
}
169+
{
170+
final Set<ApplicationPrivilege> allRead = ApplicationPrivilege.get("*", Set.of("read"), stored);
171+
assertThat(allRead, hasItem(privilegeEquals(kibanaRead)));
172+
assertThat(allRead, hasItem(privilegeEquals(apmRead)));
173+
assertThat(allRead, hasItem(privilegeEquals("*", "read", Set.of())));
174+
assertThat(allRead, iterableWithSize(3));
175+
}
176+
}
177+
148178
private void assertPrivilegeEquals(ApplicationPrivilege privilege, ApplicationPrivilegeDescriptor descriptor) {
149-
assertThat(privilege.getApplication(), equalTo(descriptor.getApplication()));
150-
assertThat(privilege.name(), contains(descriptor.getName()));
151-
assertThat(Sets.newHashSet(privilege.getPatterns()), equalTo(descriptor.getActions()));
179+
assertThat(privilege, privilegeEquals(descriptor));
180+
}
181+
182+
private Matcher<ApplicationPrivilege> privilegeEquals(ApplicationPrivilegeDescriptor descriptor) {
183+
return privilegeEquals(descriptor.getApplication(), descriptor.getName(), descriptor.getActions());
184+
}
185+
186+
private Matcher<ApplicationPrivilege> privilegeEquals(String application, String name, Set<String> actions) {
187+
return new CustomTypeSafeMatcher<>("equals(" + application + ";" + name + ";" + actions + ")") {
188+
@Override
189+
protected boolean matchesSafely(ApplicationPrivilege item) {
190+
return item.getApplication().equals(application)
191+
&& item.name().equals(Set.of(name))
192+
&& Set.of(item.getPatterns()).equals(actions);
193+
}
194+
};
152195
}
153196

154197
private ApplicationPrivilegeDescriptor descriptor(String application, String name, String... actions) {
Lines changed: 259 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,259 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License
4+
* 2.0; you may not use this file except in compliance with the Elastic License
5+
* 2.0.
6+
*/
7+
package org.elasticsearch.xpack.security;
8+
9+
import org.elasticsearch.client.Request;
10+
import org.elasticsearch.client.RequestOptions;
11+
import org.elasticsearch.common.settings.SecureString;
12+
import org.elasticsearch.core.CheckedConsumer;
13+
import org.elasticsearch.test.TestSecurityClient;
14+
import org.elasticsearch.test.XContentTestUtils;
15+
import org.elasticsearch.xcontent.ObjectPath;
16+
import org.elasticsearch.xcontent.XContentType;
17+
import org.elasticsearch.xpack.core.security.authc.support.UsernamePasswordToken;
18+
import org.elasticsearch.xpack.core.security.authz.RoleDescriptor;
19+
import org.elasticsearch.xpack.core.security.authz.permission.ResourcePrivileges;
20+
import org.elasticsearch.xpack.core.security.authz.privilege.ConfigurableClusterPrivilege;
21+
import org.elasticsearch.xpack.core.security.user.User;
22+
import org.hamcrest.Matchers;
23+
import org.junit.Before;
24+
25+
import java.io.IOException;
26+
import java.util.HashSet;
27+
import java.util.List;
28+
import java.util.Locale;
29+
import java.util.Map;
30+
import java.util.Set;
31+
import java.util.stream.Collectors;
32+
import java.util.stream.Stream;
33+
34+
import static org.hamcrest.Matchers.aMapWithSize;
35+
import static org.hamcrest.Matchers.equalTo;
36+
37+
public class HasApplicationPrivilegesIT extends SecurityInBasicRestTestCase {
38+
39+
private TestSecurityClient securityClient;
40+
41+
@Before
42+
public void setupClient() {
43+
securityClient = new TestSecurityClient(adminClient());
44+
}
45+
46+
public void testUserWithWildcardPrivileges() throws Exception {
47+
var mainApplication = randomApplicationName();
48+
49+
// Privilege names that are defined application privileges, possible on the "main" app, possibly for another app.
50+
final Set<String> definedPrivilegeNames = new HashSet<>();
51+
52+
// All applications for which application privileges have been defined
53+
final Set<String> allApplications = new HashSet<>();
54+
allApplications.add(mainApplication);
55+
CheckedConsumer<String, IOException> createApplicationPrivilege = (app) -> {
56+
final String privilegeName;
57+
// If this is the first privilege for this app, then maybe (randomly) reuse a privilege name that was defined for another app
58+
if (allApplications.contains(app) == false && definedPrivilegeNames.size() > 0 && randomBoolean()) {
59+
privilegeName = randomFrom(definedPrivilegeNames);
60+
} else {
61+
privilegeName = randomValueOtherThanMany(definedPrivilegeNames::contains, this::randomPrivilegeName);
62+
}
63+
64+
createApplicationPrivilege(app, privilegeName, randomArray(1, 4, String[]::new, () -> randomActionName()));
65+
allApplications.add(app);
66+
definedPrivilegeNames.add(privilegeName);
67+
};
68+
69+
// Create 0 or more application privileges for this application
70+
for (int i = randomIntBetween(0, 2); i > 0; i--) {
71+
createApplicationPrivilege.accept(mainApplication);
72+
}
73+
74+
// Create 0 or more application privileges for other applications
75+
for (int i = randomIntBetween(0, 3); i > 0; i--) {
76+
createApplicationPrivilege.accept(randomValueOtherThan(mainApplication, this::randomApplicationName));
77+
}
78+
79+
// Define a role with all privileges (by wildcard) for this application
80+
var roleName = randomAlphaOfLengthBetween(6, 10);
81+
var singleAppOnly = randomBoolean();
82+
createRole(roleName, singleAppOnly ? mainApplication : "*", new String[] { "*" }, new String[] { "*" });
83+
84+
final Set<String> allRoles = new HashSet<>();
85+
allRoles.add(roleName);
86+
87+
// Create 0 or more additional roles with privileges for one of the applications
88+
for (int i = randomIntBetween(0, 3); i > 0; i--) {
89+
var extraRoleName = randomValueOtherThanMany(allRoles::contains, () -> randomAlphaOfLengthBetween(8, 16));
90+
final String privilegeName = definedPrivilegeNames.size() > 0 && randomBoolean()
91+
? randomFrom(definedPrivilegeNames) // This may or may not correspond to the application we pick. Both are valid tests
92+
: randomPrivilegeName();
93+
createRole(
94+
extraRoleName,
95+
randomFrom(allApplications),
96+
new String[] { privilegeName },
97+
new String[] { "data/" + randomAlphaOfLength(6) }
98+
);
99+
allRoles.add(extraRoleName);
100+
}
101+
102+
// Create a user with all (might be 1 or more) of the roles
103+
var username = randomAlphaOfLengthBetween(8, 12);
104+
var password = new SecureString(randomAlphaOfLength(12).toCharArray());
105+
createUser(username, password, allRoles);
106+
107+
// Assert that has_privileges returns true for any arbitrary privilege or action in that application
108+
var reqOptions = RequestOptions.DEFAULT.toBuilder()
109+
.addHeader("Authorization", UsernamePasswordToken.basicAuthHeaderValue(username, password))
110+
.build();
111+
112+
final String testPrivilege;
113+
if (randomBoolean() && definedPrivilegeNames.size() > 0) {
114+
testPrivilege = randomFrom(definedPrivilegeNames);
115+
} else if (randomBoolean()) {
116+
testPrivilege = randomPrivilegeName();
117+
} else {
118+
testPrivilege = randomActionName();
119+
}
120+
var testResource = randomAlphaOfLengthBetween(4, 12);
121+
122+
{
123+
final List<ResourcePrivileges> shouldHavePrivileges = hasPrivilege(
124+
reqOptions,
125+
mainApplication,
126+
new String[] { testPrivilege },
127+
new String[] { testResource }
128+
);
129+
130+
assertSinglePrivilege(shouldHavePrivileges, testResource, testPrivilege, true);
131+
}
132+
133+
if (singleAppOnly) {
134+
List<ResourcePrivileges> shouldNotHavePrivileges = hasPrivilege(
135+
reqOptions,
136+
randomValueOtherThanMany(allApplications::contains, this::randomApplicationName),
137+
new String[] { testPrivilege },
138+
new String[] { testResource }
139+
);
140+
assertSinglePrivilege(shouldNotHavePrivileges, testResource, testPrivilege, false);
141+
142+
if (allApplications.size() > 1) { // there is an app other than the main app
143+
shouldNotHavePrivileges = hasPrivilege(
144+
reqOptions,
145+
randomValueOtherThan(mainApplication, () -> randomFrom(allApplications)),
146+
new String[] { testPrivilege },
147+
new String[] { testResource }
148+
);
149+
assertSinglePrivilege(shouldNotHavePrivileges, testResource, testPrivilege, false);
150+
}
151+
}
152+
}
153+
154+
private void assertSinglePrivilege(
155+
List<ResourcePrivileges> hasPrivilegesResult,
156+
String expectedResource,
157+
String expectedPrivilegeName,
158+
boolean shoudHavePrivilege
159+
) {
160+
assertThat(hasPrivilegesResult, Matchers.hasSize(1));
161+
assertThat(hasPrivilegesResult.get(0).getResource(), equalTo(expectedResource));
162+
assertThat(hasPrivilegesResult.get(0).getPrivileges(), Matchers.hasEntry(expectedPrivilegeName, shoudHavePrivilege));
163+
assertThat(hasPrivilegesResult.get(0).getPrivileges(), aMapWithSize(1));
164+
}
165+
166+
private List<ResourcePrivileges> hasPrivilege(RequestOptions requestOptions, String appName, String[] privileges, String[] resources)
167+
throws IOException {
168+
logger.info("Checking privileges: App=[{}] Privileges=[{}] Resources=[{}]", appName, privileges, resources);
169+
Request req = new Request("POST", "/_security/user/_has_privileges");
170+
req.setOptions(requestOptions);
171+
Map<String, Object> body = Map.ofEntries(
172+
Map.entry(
173+
"application",
174+
List.of(
175+
Map.ofEntries(
176+
Map.entry("application", appName),
177+
Map.entry("privileges", List.of(privileges)),
178+
Map.entry("resources", List.of(resources))
179+
)
180+
)
181+
)
182+
);
183+
req.setJsonEntity(XContentTestUtils.convertToXContent(body, XContentType.JSON).utf8ToString());
184+
final Map<String, Object> response = responseAsMap(client().performRequest(req));
185+
logger.info("Has privileges: [{}]", response);
186+
final Map<String, Object> privilegesByResource = ObjectPath.eval("application." + appName, response);
187+
return Stream.of(resources).map(res -> {
188+
Map<String, Boolean> priv = ObjectPath.eval(res, privilegesByResource);
189+
return ResourcePrivileges.builder(res).addPrivileges(priv).build();
190+
}).collect(Collectors.toList());
191+
}
192+
193+
private void createUser(String username, SecureString password, Set<String> roles) throws IOException {
194+
logger.info("Create User [{}] with roles [{}]", username, roles);
195+
securityClient.putUser(new User(username, roles.toArray(String[]::new)), password);
196+
}
197+
198+
private void createRole(String roleName, String applicationName, String[] privileges, String[] resources) throws IOException {
199+
logger.info(
200+
"Create role [{}] with privileges App=[{}] Privileges=[{}] Resources=[{}]",
201+
roleName,
202+
applicationName,
203+
privileges,
204+
resources
205+
);
206+
securityClient.putRole(
207+
new RoleDescriptor(
208+
roleName,
209+
new String[0], // cluster
210+
new RoleDescriptor.IndicesPrivileges[0],
211+
new RoleDescriptor.ApplicationResourcePrivileges[] {
212+
RoleDescriptor.ApplicationResourcePrivileges.builder()
213+
.application(applicationName)
214+
.privileges(privileges)
215+
.resources(resources)
216+
.build() },
217+
new ConfigurableClusterPrivilege[0],
218+
new String[0],// run-as
219+
Map.of(), // metadata
220+
Map.of() // transient metadata
221+
)
222+
);
223+
}
224+
225+
private void createApplicationPrivilege(String applicationName, String privilegeName, String[] actions) {
226+
logger.info("Create app privilege App=[{}] Privilege=[{}] Actions=[{}]", applicationName, privilegeName, actions);
227+
try {
228+
securityClient.putApplicationPrivilege(applicationName, privilegeName, actions);
229+
} catch (IOException e) {
230+
throw new AssertionError(
231+
"Failed to create application privilege app=["
232+
+ applicationName
233+
+ "], privilege=["
234+
+ privilegeName
235+
+ "], actions=["
236+
+ String.join(",", actions)
237+
+ "]",
238+
e
239+
);
240+
}
241+
}
242+
243+
private String randomApplicationName() {
244+
return randomAlphaOfLength(1).toLowerCase(Locale.ROOT) + randomAlphaOfLengthBetween(3, 7);
245+
}
246+
247+
private String randomPrivilegeName() {
248+
if (randomBoolean()) {
249+
return randomAlphaOfLength(1).toLowerCase(Locale.ROOT) + randomAlphaOfLengthBetween(3, 7);
250+
} else {
251+
return randomAlphaOfLengthBetween(2, 4).toLowerCase(Locale.ROOT) + randomFrom(".", "_", "-") + randomAlphaOfLengthBetween(2, 6);
252+
}
253+
}
254+
255+
private String randomActionName() {
256+
return randomAlphaOfLengthBetween(3, 5) + ":" + randomAlphaOfLengthBetween(3, 5);
257+
}
258+
259+
}

0 commit comments

Comments
 (0)