Skip to content

Commit 77a80b0

Browse files
slobodanadamovicslobodanadamovic
authored
Reject defining workflows restriction via file and Role API (#96304)
This PR rejects file and API based role definitions that include workflows restriction. Specifying `restriction` (in file or via API) will be rejected during parsing of a `RoleDescriptor` and reported as a parsing error. Currently, only API keys are supporting roles to be restricted to a set of workflows. Relates to #96215
1 parent 48c6547 commit 77a80b0

File tree

9 files changed

+232
-12
lines changed

9 files changed

+232
-12
lines changed

‎x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/role/PutRoleRequestBuilder.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ public PutRoleRequestBuilder(ElasticsearchClient client, PutRoleAction action) {
3838
public PutRoleRequestBuilder source(String name, BytesReference source, XContentType xContentType) throws IOException {
3939
// we pass false as last parameter because we want to reject the request if field permissions
4040
// are given in 2.x syntax
41-
RoleDescriptor descriptor = RoleDescriptor.parse(name, source, false, xContentType);
41+
RoleDescriptor descriptor = RoleDescriptor.parse(name, source, false, xContentType, false);
4242
assert name.equals(descriptor.getName());
4343
request.name(name);
4444
request.cluster(descriptor.getClusterPrivileges());

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

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -426,23 +426,43 @@ public void writeTo(StreamOutput out) throws IOException {
426426

427427
public static RoleDescriptor parse(String name, BytesReference source, boolean allow2xFormat, XContentType xContentType)
428428
throws IOException {
429+
return parse(name, source, allow2xFormat, xContentType, true);
430+
}
431+
432+
public static RoleDescriptor parse(
433+
String name,
434+
BytesReference source,
435+
boolean allow2xFormat,
436+
XContentType xContentType,
437+
boolean allowRestriction
438+
) throws IOException {
429439
assert name != null;
430440
// EMPTY is safe here because we never use namedObject
431441
try (
432442
InputStream stream = source.streamInput();
433443
XContentParser parser = xContentType.xContent()
434444
.createParser(NamedXContentRegistry.EMPTY, LoggingDeprecationHandler.INSTANCE, stream)
435445
) {
436-
return parse(name, parser, allow2xFormat);
446+
return parse(name, parser, allow2xFormat, allowRestriction);
437447
}
438448
}
439449

440450
public static RoleDescriptor parse(String name, XContentParser parser, boolean allow2xFormat) throws IOException {
441-
return parse(name, parser, allow2xFormat, TcpTransport.isUntrustedRemoteClusterEnabled());
451+
return parse(name, parser, allow2xFormat, TcpTransport.isUntrustedRemoteClusterEnabled(), true);
442452
}
443453

444-
static RoleDescriptor parse(String name, XContentParser parser, boolean allow2xFormat, boolean untrustedRemoteClusterEnabled)
454+
public static RoleDescriptor parse(String name, XContentParser parser, boolean allow2xFormat, boolean allowRestriction)
445455
throws IOException {
456+
return parse(name, parser, allow2xFormat, TcpTransport.isUntrustedRemoteClusterEnabled(), allowRestriction);
457+
}
458+
459+
static RoleDescriptor parse(
460+
String name,
461+
XContentParser parser,
462+
boolean allow2xFormat,
463+
boolean untrustedRemoteClusterEnabled,
464+
boolean allowRestriction
465+
) throws IOException {
446466
// validate name
447467
Validation.Error validationError = Validation.Roles.validateRoleName(name, true);
448468
if (validationError != null) {
@@ -503,7 +523,7 @@ static RoleDescriptor parse(String name, XContentParser parser, boolean allow2xF
503523
} else if (untrustedRemoteClusterEnabled
504524
&& Fields.REMOTE_INDICES.match(currentFieldName, parser.getDeprecationHandler())) {
505525
remoteIndicesPrivileges = parseRemoteIndices(name, parser);
506-
} else if (Fields.RESTRICTION.match(currentFieldName, parser.getDeprecationHandler())) {
526+
} else if (allowRestriction && Fields.RESTRICTION.match(currentFieldName, parser.getDeprecationHandler())) {
507527
restriction = Restriction.parse(name, parser);
508528
} else if (Fields.TYPE.match(currentFieldName, parser.getDeprecationHandler())) {
509529
// don't need it

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

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -594,6 +594,51 @@ public void testSerializationWithWorkflowsRestrictionAndUnsupportedVersions() th
594594
}
595595
}
596596

597+
public void testParseRoleWithRestrictionFailsWhenAllowRestrictionIsFalse() {
598+
final String json = """
599+
{
600+
"restriction": {
601+
"workflows": ["search_application"]
602+
}
603+
}""";
604+
final ElasticsearchParseException e = expectThrows(
605+
ElasticsearchParseException.class,
606+
() -> RoleDescriptor.parse(
607+
"test_role_with_restriction",
608+
XContentHelper.createParser(XContentParserConfiguration.EMPTY, new BytesArray(json), XContentType.JSON),
609+
randomBoolean(),
610+
randomBoolean(),
611+
false
612+
)
613+
);
614+
assertThat(
615+
e,
616+
TestMatchers.throwableWithMessage(
617+
containsString("failed to parse role [test_role_with_restriction]. unexpected field [restriction]")
618+
)
619+
);
620+
}
621+
622+
public void testParseRoleWithRestrictionWhenAllowRestrictionIsTrue() throws IOException {
623+
final String json = """
624+
{
625+
"restriction": {
626+
"workflows": ["search_application"]
627+
}
628+
}""";
629+
RoleDescriptor role = RoleDescriptor.parse(
630+
"test_role_with_restriction",
631+
XContentHelper.createParser(XContentParserConfiguration.EMPTY, new BytesArray(json), XContentType.JSON),
632+
randomBoolean(),
633+
randomBoolean(),
634+
true
635+
);
636+
assertThat(role.getName(), equalTo("test_role_with_restriction"));
637+
assertThat(role.hasRestriction(), equalTo(true));
638+
assertThat(role.hasWorkflowsRestriction(), equalTo(true));
639+
assertThat(role.getRestriction().getWorkflows(), arrayContaining("search_application"));
640+
}
641+
597642
public void testParseEmptyQuery() throws Exception {
598643
String json = """
599644
{
@@ -773,6 +818,7 @@ public void testParseRemoteIndicesPrivilegesFailsWhenUntrustedRemoteClusterEnabl
773818
"test",
774819
XContentHelper.createParser(XContentParserConfiguration.EMPTY, new BytesArray(json), XContentType.JSON),
775820
false,
821+
false,
776822
false
777823
)
778824
);

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

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
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+
8+
package org.elasticsearch.xpack.security.role;
9+
10+
import org.apache.http.client.methods.HttpPost;
11+
import org.apache.http.client.methods.HttpPut;
12+
import org.elasticsearch.client.Request;
13+
import org.elasticsearch.client.Response;
14+
import org.elasticsearch.client.ResponseException;
15+
import org.elasticsearch.xpack.security.SecurityOnTrialLicenseRestTestCase;
16+
17+
import java.io.IOException;
18+
19+
import static org.hamcrest.Matchers.containsString;
20+
21+
public class RoleWithWorkflowsRestrictionRestIT extends SecurityOnTrialLicenseRestTestCase {
22+
23+
public void testCreateRoleWithWorkflowsRestrictionFail() {
24+
Request createRoleRequest = new Request(HttpPut.METHOD_NAME, "/_security/role/role_with_restriction");
25+
createRoleRequest.setJsonEntity("""
26+
{
27+
"cluster": ["all"],
28+
"indices": [
29+
{
30+
"names": ["index-a"],
31+
"privileges": ["all"]
32+
}
33+
],
34+
"restriction":{
35+
"workflows": ["foo", "bar"]
36+
}
37+
}""");
38+
39+
ResponseException e = expectThrows(ResponseException.class, () -> adminClient().performRequest(createRoleRequest));
40+
assertEquals(400, e.getResponse().getStatusLine().getStatusCode());
41+
assertThat(e.getMessage(), containsString("failed to parse role [role_with_restriction]. unexpected field [restriction]"));
42+
}
43+
44+
public void testUpdateRoleWithWorkflowsRestrictionFail() throws IOException {
45+
Request createRoleRequest = new Request(HttpPut.METHOD_NAME, "/_security/role/my_role");
46+
createRoleRequest.setJsonEntity("""
47+
{
48+
"cluster": ["all"],
49+
"indices": [
50+
{
51+
"names": ["index-a"],
52+
"privileges": ["all"]
53+
}
54+
]
55+
}""");
56+
Response createRoleResponse = adminClient().performRequest(createRoleRequest);
57+
assertOK(createRoleResponse);
58+
59+
Request updateRoleRequest = new Request(HttpPost.METHOD_NAME, "/_security/role/my_role");
60+
updateRoleRequest.setJsonEntity("""
61+
{
62+
"cluster": ["all"],
63+
"indices": [
64+
{
65+
"names": ["index-*"],
66+
"privileges": ["all"]
67+
}
68+
],
69+
"restriction":{
70+
"workflows": ["foo", "bar"]
71+
}
72+
}""");
73+
74+
ResponseException e = expectThrows(ResponseException.class, () -> adminClient().performRequest(updateRoleRequest));
75+
assertEquals(400, e.getResponse().getStatusLine().getStatusCode());
76+
assertThat(e.getMessage(), containsString("failed to parse role [my_role]. unexpected field [restriction]"));
77+
}
78+
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -309,7 +309,7 @@ static RoleDescriptor parseRoleDescriptor(
309309
if (token == XContentParser.Token.START_OBJECT) {
310310
// we pass true as last parameter because we do not want to reject files if field permissions
311311
// are given in 2.x syntax
312-
RoleDescriptor descriptor = RoleDescriptor.parse(roleName, parser, true);
312+
RoleDescriptor descriptor = RoleDescriptor.parse(roleName, parser, true, false);
313313
return checkDescriptor(descriptor, path, logger, settings, xContentRegistry);
314314
} else {
315315
logger.error("invalid role definition [{}] in roles file [{}]. skipping role...", roleName, path.toAbsolutePath());

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -242,6 +242,7 @@ public void putRole(final PutRoleRequest request, final RoleDescriptor role, fin
242242
void innerPutRole(final PutRoleRequest request, final RoleDescriptor role, final ActionListener<Boolean> listener) {
243243
final String roleName = role.getName();
244244
assert NativeRealmValidationUtil.validateRoleName(roleName, false) == null : "Role name was invalid or reserved: " + roleName;
245+
assert false == role.hasRestriction() : "restriction is not supported for native roles";
245246

246247
securityIndex.prepareIndexIfNeededThenExecute(listener::onFailure, () -> {
247248
final XContentBuilder xContentBuilder;
@@ -454,9 +455,9 @@ static RoleDescriptor transformRole(String id, BytesReference sourceBytes, Logge
454455
assert id.startsWith(ROLE_TYPE) : "[" + id + "] does not have role prefix";
455456
final String name = id.substring(ROLE_TYPE.length() + 1);
456457
try {
457-
// we pass true as last parameter because we do not want to reject permissions if the field permissions
458+
// we pass true as allow2xFormat parameter because we do not want to reject permissions if the field permissions
458459
// are given in 2.x syntax
459-
RoleDescriptor roleDescriptor = RoleDescriptor.parse(name, sourceBytes, true, XContentType.JSON);
460+
RoleDescriptor roleDescriptor = RoleDescriptor.parse(name, sourceBytes, true, XContentType.JSON, false);
460461
final boolean dlsEnabled = Arrays.stream(roleDescriptor.getIndicesPrivileges())
461462
.anyMatch(IndicesPrivileges::isUsingDocumentLevelSecurity);
462463
final boolean flsEnabled = Arrays.stream(roleDescriptor.getIndicesPrivileges())
@@ -488,7 +489,7 @@ static RoleDescriptor transformRole(String id, BytesReference sourceBytes, Logge
488489
return roleDescriptor;
489490
}
490491
} catch (Exception e) {
491-
logger.error(() -> "error in the format of data for role [" + name + "]", e);
492+
logger.error("error in the format of data for role [" + name + "]", e);
492493
return null;
493494
}
494495
}

‎x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/FileRolesStoreTests.java‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -617,7 +617,7 @@ public void testThatInvalidRoleDefinitions() throws Exception {
617617
assertThat(role, notNullValue());
618618
assertThat(role.names(), equalTo(new String[] { "valid_role" }));
619619

620-
assertThat(entries, hasSize(6));
620+
assertThat(entries, hasSize(7));
621621
assertThat(
622622
entries.get(0),
623623
startsWith("invalid role definition [fóóbár] in roles file [" + path.toAbsolutePath() + "]. invalid role name")
@@ -627,6 +627,7 @@ public void testThatInvalidRoleDefinitions() throws Exception {
627627
assertThat(entries.get(3), startsWith("failed to parse role [role3]"));
628628
assertThat(entries.get(4), startsWith("failed to parse role [role4]"));
629629
assertThat(entries.get(5), startsWith("failed to parse indices privileges for role [role5]"));
630+
assertThat(entries.get(6), startsWith("failed to parse role [role6]. unexpected field [restriction]"));
630631
}
631632

632633
public void testThatRoleNamesDoesNotResolvePermissions() throws Exception {
@@ -635,8 +636,8 @@ public void testThatRoleNamesDoesNotResolvePermissions() throws Exception {
635636
List<String> events = CapturingLogger.output(logger.getName(), Level.ERROR);
636637
events.clear();
637638
Set<String> roleNames = FileRolesStore.parseFileForRoleNames(path, logger);
638-
assertThat(roleNames.size(), is(6));
639-
assertThat(roleNames, containsInAnyOrder("valid_role", "role1", "role2", "role3", "role4", "role5"));
639+
assertThat(roleNames.size(), is(7));
640+
assertThat(roleNames, containsInAnyOrder("valid_role", "role1", "role2", "role3", "role4", "role5", "role6"));
640641

641642
assertThat(events, hasSize(1));
642643
assertThat(

‎x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/NativeRolesStoreTests.java‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66
*/
77
package org.elasticsearch.xpack.security.authz.store;
88

9+
import org.apache.logging.log4j.Logger;
10+
import org.elasticsearch.ElasticsearchParseException;
911
import org.elasticsearch.ElasticsearchSecurityException;
1012
import org.elasticsearch.TransportVersion;
1113
import org.elasticsearch.Version;
@@ -47,12 +49,14 @@
4749
import org.elasticsearch.xpack.core.security.authz.RoleDescriptor;
4850
import org.elasticsearch.xpack.core.security.authz.RoleDescriptor.IndicesPrivileges;
4951
import org.elasticsearch.xpack.core.security.authz.RoleDescriptorTests;
52+
import org.elasticsearch.xpack.core.security.authz.RoleRestrictionTests;
5053
import org.elasticsearch.xpack.core.security.authz.privilege.ClusterPrivilegeResolver;
5154
import org.elasticsearch.xpack.security.support.SecurityIndexManager;
5255
import org.elasticsearch.xpack.security.support.SecuritySystemIndices;
5356
import org.elasticsearch.xpack.security.test.SecurityTestUtils;
5457
import org.junit.After;
5558
import org.junit.Before;
59+
import org.mockito.ArgumentCaptor;
5660
import org.mockito.Mockito;
5761

5862
import java.io.IOException;
@@ -70,7 +74,9 @@
7074
import static org.hamcrest.Matchers.containsString;
7175
import static org.hamcrest.Matchers.equalTo;
7276
import static org.hamcrest.Matchers.instanceOf;
77+
import static org.hamcrest.Matchers.nullValue;
7378
import static org.mockito.Mockito.mock;
79+
import static org.mockito.Mockito.verify;
7480
import static org.mockito.Mockito.when;
7581

7682
public class NativeRolesStoreTests extends ESTestCase {
@@ -239,6 +245,61 @@ public void testRoleDescriptorWithFlsDlsLicensing() throws IOException {
239245
assertThat(role, equalTo(noFlsDlsRole));
240246
}
241247

248+
public void testTransformingRoleWithRestrictionFails() throws IOException {
249+
MockLicenseState licenseState = mock(MockLicenseState.class);
250+
when(licenseState.isAllowed(DOCUMENT_LEVEL_SECURITY_FEATURE)).thenReturn(false);
251+
RoleDescriptor roleWithRestriction = new RoleDescriptor(
252+
"role_with_restriction",
253+
randomSubsetOf(ClusterPrivilegeResolver.names()).toArray(String[]::new),
254+
new IndicesPrivileges[] {
255+
IndicesPrivileges.builder()
256+
.privileges("READ")
257+
.indices(generateRandomStringArray(5, randomIntBetween(3, 9), false, false))
258+
.grantedFields("*")
259+
.deniedFields(generateRandomStringArray(5, randomIntBetween(3, 9), false, false))
260+
.query(
261+
randomBoolean()
262+
? "{ \"term\": { \""
263+
+ randomAlphaOfLengthBetween(3, 24)
264+
+ "\" : \""
265+
+ randomAlphaOfLengthBetween(3, 24)
266+
+ "\" }"
267+
: "{ \"match_all\": {} }"
268+
)
269+
.build() },
270+
RoleDescriptorTests.randomApplicationPrivileges(),
271+
RoleDescriptorTests.randomClusterPrivileges(),
272+
generateRandomStringArray(5, randomIntBetween(2, 8), true, true),
273+
RoleDescriptorTests.randomRoleDescriptorMetadata(ESTestCase.randomBoolean()),
274+
null,
275+
TcpTransport.isUntrustedRemoteClusterEnabled() ? RoleDescriptorTests.randomRemoteIndicesPrivileges(1, 2) : null,
276+
RoleRestrictionTests.randomWorkflowsRestriction(1, 2)
277+
);
278+
279+
XContentBuilder builder = roleWithRestriction.toXContent(
280+
XContentBuilder.builder(XContentType.JSON.xContent()),
281+
ToXContent.EMPTY_PARAMS
282+
);
283+
284+
Logger mockedLogger = Mockito.mock(Logger.class);
285+
BytesReference bytes = BytesReference.bytes(builder);
286+
RoleDescriptor transformedRole = NativeRolesStore.transformRole(
287+
RoleDescriptor.ROLE_TYPE + "-role_with_restriction",
288+
bytes,
289+
mockedLogger,
290+
licenseState
291+
);
292+
assertThat(transformedRole, nullValue());
293+
ArgumentCaptor<ElasticsearchParseException> exceptionCaptor = ArgumentCaptor.forClass(ElasticsearchParseException.class);
294+
ArgumentCaptor<String> messageCaptor = ArgumentCaptor.forClass(String.class);
295+
verify(mockedLogger).error(messageCaptor.capture(), exceptionCaptor.capture());
296+
assertThat(messageCaptor.getValue(), containsString("error in the format of data for role [role_with_restriction]"));
297+
assertThat(
298+
exceptionCaptor.getValue().getMessage(),
299+
containsString("failed to parse role [role_with_restriction]. unexpected field [restriction]")
300+
);
301+
}
302+
242303
public void testPutOfRoleWithFlsDlsUnlicensed() throws IOException {
243304
final Client client = mock(Client.class);
244305
final ClusterService clusterService = mockClusterServiceWithMinNodeVersion(TransportVersion.CURRENT);

‎x-pack/plugin/security/src/test/resources/org/elasticsearch/xpack/security/authz/store/invalid_roles.yml‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,3 +45,16 @@ role5:
4545
- names:
4646
- 'idx1'
4747
privileges: []
48+
49+
# role includes unsupported workflows restriction
50+
role6:
51+
cluster:
52+
- ALL
53+
indices:
54+
- names: idx
55+
privileges:
56+
- ALL
57+
restriction:
58+
workflows:
59+
- workflow1
60+
- workflow2

0 commit comments

Comments
 (0)