Skip to content

Commit 7ad5f19

Browse files
gmjehovichgmjehovich
committed
Prevent invalid privileges in manage roles privilege
1 parent 7532ad5 commit 7ad5f19

File tree

3 files changed

+106
-32
lines changed

3 files changed

+106
-32
lines changed

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -562,6 +562,11 @@ public static ManageRolesPrivilege parse(XContentParser parser) throws IOExcepti
562562
}
563563
for (String privilege : indexPrivilege.privileges) {
564564
IndexPrivilege namedPrivilege = IndexPrivilege.getNamedOrNull(privilege);
565+
566+
// Use resolveBySelectorAccess to determine whether the passed privilege is valid.
567+
// IllegalArgumentException is thrown here when an invalid permission is encountered.
568+
IndexPrivilege.resolveBySelectorAccess(Set.of(privilege));
569+
565570
if (namedPrivilege != null && namedPrivilege.getSelectorPredicate() == IndexComponentSelectorPredicate.FAILURES) {
566571
throw new IllegalArgumentException(
567572
"Failure store related privileges are not supported as targets of manage roles but found [" + privilege + "]"

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

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
import java.util.List;
3939
import java.util.Objects;
4040

41+
import static org.hamcrest.Matchers.containsString;
4142
import static org.hamcrest.Matchers.nullValue;
4243
import static org.hamcrest.core.Is.is;
4344
import static org.hamcrest.core.IsEqual.equalTo;
@@ -257,6 +258,35 @@ public void testPutRoleRequestContainsNonIndexPrivileges() {
257258
assertThat(permissionCheck(permission, "cluster:admin/xpack/security/role/put", putRoleRequest), is(false));
258259
}
259260

261+
public void testParseInvalidPrivilege() throws Exception {
262+
final XContent xContent = XContentType.JSON.xContent();
263+
264+
final String invalidJsonString = """
265+
{
266+
"manage": {
267+
"indices": [
268+
{
269+
"names": ["test-*"],
270+
"privileges": ["foobar"]
271+
}
272+
]
273+
}
274+
}
275+
""";
276+
277+
try (XContentParser parser = xContent.createParser(XContentParserConfiguration.EMPTY, invalidJsonString.getBytes())) {
278+
assertThat(parser.nextToken(), equalTo(XContentParser.Token.START_OBJECT));
279+
assertThat(parser.nextToken(), equalTo(XContentParser.Token.FIELD_NAME));
280+
281+
IllegalArgumentException exception = expectThrows(
282+
IllegalArgumentException.class,
283+
() -> ManageRolesPrivilege.parse(parser)
284+
);
285+
286+
assertThat(exception.getMessage(), containsString("unknown index privilege [foobar]"));
287+
}
288+
}
289+
260290
private static boolean permissionCheck(ClusterPermission permission, String action, ActionRequest request) {
261291
final Authentication authentication = AuthenticationTestHelper.builder().build();
262292
assertThat(request.validate(), nullValue());

‎x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/role/BulkPutRoleRestIT.java‎

Lines changed: 71 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,9 @@ public void testPutManyValidRoles() throws Exception {
3232
"test1",
3333
new RoleDescriptor(
3434
"test1",
35-
new String[] { "all" },
36-
new RoleDescriptor.IndicesPrivileges[] {
37-
RoleDescriptor.IndicesPrivileges.builder().indices("*").privileges("all").build() },
35+
new String[]{"all"},
36+
new RoleDescriptor.IndicesPrivileges[]{
37+
RoleDescriptor.IndicesPrivileges.builder().indices("*").privileges("all").build()},
3838
null,
3939
null,
4040
null,
@@ -50,9 +50,9 @@ public void testPutManyValidRoles() throws Exception {
5050
"test2",
5151
new RoleDescriptor(
5252
"test2",
53-
new String[] { "all" },
54-
new RoleDescriptor.IndicesPrivileges[] {
55-
RoleDescriptor.IndicesPrivileges.builder().indices("*").privileges("read").build() },
53+
new String[]{"all"},
54+
new RoleDescriptor.IndicesPrivileges[]{
55+
RoleDescriptor.IndicesPrivileges.builder().indices("*").privileges("read").build()},
5656
null,
5757
null,
5858
null,
@@ -68,9 +68,9 @@ public void testPutManyValidRoles() throws Exception {
6868
"test3",
6969
new RoleDescriptor(
7070
"test3",
71-
new String[] { "all" },
72-
new RoleDescriptor.IndicesPrivileges[] {
73-
RoleDescriptor.IndicesPrivileges.builder().indices("*").privileges("write").build() },
71+
new String[]{"all"},
72+
new RoleDescriptor.IndicesPrivileges[]{
73+
RoleDescriptor.IndicesPrivileges.builder().indices("*").privileges("write").build()},
7474
null,
7575
null,
7676
null,
@@ -110,9 +110,9 @@ public void testPutMixedValidInvalidRoles() throws Exception {
110110
"test1",
111111
new RoleDescriptor(
112112
"test1",
113-
new String[] { "all" },
114-
new RoleDescriptor.IndicesPrivileges[] {
115-
RoleDescriptor.IndicesPrivileges.builder().indices("*").privileges("all").build() },
113+
new String[]{"all"},
114+
new RoleDescriptor.IndicesPrivileges[]{
115+
RoleDescriptor.IndicesPrivileges.builder().indices("*").privileges("all").build()},
116116
null,
117117
null,
118118
null,
@@ -129,9 +129,9 @@ public void testPutMixedValidInvalidRoles() throws Exception {
129129
"test3",
130130
new RoleDescriptor(
131131
"test3",
132-
new String[] { "all" },
133-
new RoleDescriptor.IndicesPrivileges[] {
134-
RoleDescriptor.IndicesPrivileges.builder().indices("*").privileges("write").build() },
132+
new String[]{"all"},
133+
new RoleDescriptor.IndicesPrivileges[]{
134+
RoleDescriptor.IndicesPrivileges.builder().indices("*").privileges("write").build()},
135135
null,
136136
null,
137137
null,
@@ -195,9 +195,9 @@ public void testBulkUpdates() throws Exception {
195195
"test1",
196196
new RoleDescriptor(
197197
"test1",
198-
new String[] { "all" },
199-
new RoleDescriptor.IndicesPrivileges[] {
200-
RoleDescriptor.IndicesPrivileges.builder().indices("*").privileges("all").build() },
198+
new String[]{"all"},
199+
new RoleDescriptor.IndicesPrivileges[]{
200+
RoleDescriptor.IndicesPrivileges.builder().indices("*").privileges("all").build()},
201201
null,
202202
null,
203203
null,
@@ -213,9 +213,9 @@ public void testBulkUpdates() throws Exception {
213213
"test2",
214214
new RoleDescriptor(
215215
"test2",
216-
new String[] { "all" },
217-
new RoleDescriptor.IndicesPrivileges[] {
218-
RoleDescriptor.IndicesPrivileges.builder().indices("*").privileges("read").build() },
216+
new String[]{"all"},
217+
new RoleDescriptor.IndicesPrivileges[]{
218+
RoleDescriptor.IndicesPrivileges.builder().indices("*").privileges("read").build()},
219219
null,
220220
null,
221221
null,
@@ -231,19 +231,19 @@ public void testBulkUpdates() throws Exception {
231231
"test3",
232232
new RoleDescriptor(
233233
"test3",
234-
new String[] { "all" },
235-
new RoleDescriptor.IndicesPrivileges[] {
236-
RoleDescriptor.IndicesPrivileges.builder().indices("*").privileges("write").build() },
234+
new String[]{"all"},
235+
new RoleDescriptor.IndicesPrivileges[]{
236+
RoleDescriptor.IndicesPrivileges.builder().indices("*").privileges("write").build()},
237237
null,
238238
null,
239239
null,
240240
null,
241241
null,
242-
new RoleDescriptor.RemoteIndicesPrivileges[] {
242+
new RoleDescriptor.RemoteIndicesPrivileges[]{
243243
RoleDescriptor.RemoteIndicesPrivileges.builder("my_cluster*", "other_cluster")
244244
.indices("logs-*")
245245
.privileges("read")
246-
.build() },
246+
.build()},
247247
null,
248248
null,
249249
null
@@ -278,9 +278,9 @@ public void testBulkUpdates() throws Exception {
278278
"test2",
279279
new RoleDescriptor(
280280
"test2",
281-
new String[] { "all" },
282-
new RoleDescriptor.IndicesPrivileges[] {
283-
RoleDescriptor.IndicesPrivileges.builder().indices("*").privileges("all").build() },
281+
new String[]{"all"},
282+
new RoleDescriptor.IndicesPrivileges[]{
283+
RoleDescriptor.IndicesPrivileges.builder().indices("*").privileges("all").build()},
284284
null,
285285
null,
286286
null,
@@ -296,9 +296,9 @@ public void testBulkUpdates() throws Exception {
296296
"test3",
297297
new RoleDescriptor(
298298
"test3",
299-
new String[] { "all" },
300-
new RoleDescriptor.IndicesPrivileges[] {
301-
RoleDescriptor.IndicesPrivileges.builder().indices("*").privileges("all").build() },
299+
new String[]{"all"},
300+
new RoleDescriptor.IndicesPrivileges[]{
301+
RoleDescriptor.IndicesPrivileges.builder().indices("*").privileges("all").build()},
302302
null,
303303
null,
304304
null,
@@ -312,4 +312,43 @@ public void testBulkUpdates() throws Exception {
312312
);
313313
}
314314
}
315+
316+
public void testPutRoleWithInvalidManageRolesPrivilege() throws Exception {
317+
final String badRoleName = "bad-role";
318+
319+
final ResponseException exception = expectThrows(
320+
ResponseException.class,
321+
() -> upsertRoles(String.format("""
322+
{
323+
"roles": {
324+
"%s": {
325+
"global": {
326+
"role": {
327+
"manage": {
328+
"indices": [
329+
{
330+
"names": ["allowed-index-prefix-*"],
331+
"privileges": ["foobar"]
332+
}
333+
]
334+
}
335+
}
336+
}
337+
}
338+
}
339+
}""", badRoleName))
340+
);
341+
342+
assertThat(exception.getMessage(), containsString("unknown index privilege [foobar]"));
343+
assertRoleDoesNotExist(badRoleName);
344+
}
345+
346+
private void assertRoleDoesNotExist(final String roleName) throws Exception {
347+
final ResponseException roleNotFound = expectThrows(
348+
ResponseException.class,
349+
() -> adminClient().performRequest(new Request("GET", "/_security/role/" + roleName))
350+
);
351+
assertEquals(404, roleNotFound.getResponse().getStatusLine().getStatusCode());
352+
}
315353
}
354+

0 commit comments

Comments
 (0)