Skip to content

Commit

Permalink
Reject defining workflows restriction via file and Role API (#96304)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
slobodanadamovic authored May 26, 2023
1 parent 48c6547 commit 77a80b0
Show file tree
Hide file tree
Showing 9 changed files with 232 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public PutRoleRequestBuilder(ElasticsearchClient client, PutRoleAction action) {
public PutRoleRequestBuilder source(String name, BytesReference source, XContentType xContentType) throws IOException {
// we pass false as last parameter because we want to reject the request if field permissions
// are given in 2.x syntax
RoleDescriptor descriptor = RoleDescriptor.parse(name, source, false, xContentType);
RoleDescriptor descriptor = RoleDescriptor.parse(name, source, false, xContentType, false);
assert name.equals(descriptor.getName());
request.name(name);
request.cluster(descriptor.getClusterPrivileges());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -426,23 +426,43 @@ public void writeTo(StreamOutput out) throws IOException {

public static RoleDescriptor parse(String name, BytesReference source, boolean allow2xFormat, XContentType xContentType)
throws IOException {
return parse(name, source, allow2xFormat, xContentType, true);
}

public static RoleDescriptor parse(
String name,
BytesReference source,
boolean allow2xFormat,
XContentType xContentType,
boolean allowRestriction
) throws IOException {
assert name != null;
// EMPTY is safe here because we never use namedObject
try (
InputStream stream = source.streamInput();
XContentParser parser = xContentType.xContent()
.createParser(NamedXContentRegistry.EMPTY, LoggingDeprecationHandler.INSTANCE, stream)
) {
return parse(name, parser, allow2xFormat);
return parse(name, parser, allow2xFormat, allowRestriction);
}
}

public static RoleDescriptor parse(String name, XContentParser parser, boolean allow2xFormat) throws IOException {
return parse(name, parser, allow2xFormat, TcpTransport.isUntrustedRemoteClusterEnabled());
return parse(name, parser, allow2xFormat, TcpTransport.isUntrustedRemoteClusterEnabled(), true);
}

static RoleDescriptor parse(String name, XContentParser parser, boolean allow2xFormat, boolean untrustedRemoteClusterEnabled)
public static RoleDescriptor parse(String name, XContentParser parser, boolean allow2xFormat, boolean allowRestriction)
throws IOException {
return parse(name, parser, allow2xFormat, TcpTransport.isUntrustedRemoteClusterEnabled(), allowRestriction);
}

static RoleDescriptor parse(
String name,
XContentParser parser,
boolean allow2xFormat,
boolean untrustedRemoteClusterEnabled,
boolean allowRestriction
) throws IOException {
// validate name
Validation.Error validationError = Validation.Roles.validateRoleName(name, true);
if (validationError != null) {
Expand Down Expand Up @@ -503,7 +523,7 @@ static RoleDescriptor parse(String name, XContentParser parser, boolean allow2xF
} else if (untrustedRemoteClusterEnabled
&& Fields.REMOTE_INDICES.match(currentFieldName, parser.getDeprecationHandler())) {
remoteIndicesPrivileges = parseRemoteIndices(name, parser);
} else if (Fields.RESTRICTION.match(currentFieldName, parser.getDeprecationHandler())) {
} else if (allowRestriction && Fields.RESTRICTION.match(currentFieldName, parser.getDeprecationHandler())) {
restriction = Restriction.parse(name, parser);
} else if (Fields.TYPE.match(currentFieldName, parser.getDeprecationHandler())) {
// don't need it
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -594,6 +594,51 @@ public void testSerializationWithWorkflowsRestrictionAndUnsupportedVersions() th
}
}

public void testParseRoleWithRestrictionFailsWhenAllowRestrictionIsFalse() {
final String json = """
{
"restriction": {
"workflows": ["search_application"]
}
}""";
final ElasticsearchParseException e = expectThrows(
ElasticsearchParseException.class,
() -> RoleDescriptor.parse(
"test_role_with_restriction",
XContentHelper.createParser(XContentParserConfiguration.EMPTY, new BytesArray(json), XContentType.JSON),
randomBoolean(),
randomBoolean(),
false
)
);
assertThat(
e,
TestMatchers.throwableWithMessage(
containsString("failed to parse role [test_role_with_restriction]. unexpected field [restriction]")
)
);
}

public void testParseRoleWithRestrictionWhenAllowRestrictionIsTrue() throws IOException {
final String json = """
{
"restriction": {
"workflows": ["search_application"]
}
}""";
RoleDescriptor role = RoleDescriptor.parse(
"test_role_with_restriction",
XContentHelper.createParser(XContentParserConfiguration.EMPTY, new BytesArray(json), XContentType.JSON),
randomBoolean(),
randomBoolean(),
true
);
assertThat(role.getName(), equalTo("test_role_with_restriction"));
assertThat(role.hasRestriction(), equalTo(true));
assertThat(role.hasWorkflowsRestriction(), equalTo(true));
assertThat(role.getRestriction().getWorkflows(), arrayContaining("search_application"));
}

public void testParseEmptyQuery() throws Exception {
String json = """
{
Expand Down Expand Up @@ -773,6 +818,7 @@ public void testParseRemoteIndicesPrivilegesFailsWhenUntrustedRemoteClusterEnabl
"test",
XContentHelper.createParser(XContentParserConfiguration.EMPTY, new BytesArray(json), XContentType.JSON),
false,
false,
false
)
);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

package org.elasticsearch.xpack.security.role;

import org.apache.http.client.methods.HttpPost;
import org.apache.http.client.methods.HttpPut;
import org.elasticsearch.client.Request;
import org.elasticsearch.client.Response;
import org.elasticsearch.client.ResponseException;
import org.elasticsearch.xpack.security.SecurityOnTrialLicenseRestTestCase;

import java.io.IOException;

import static org.hamcrest.Matchers.containsString;

public class RoleWithWorkflowsRestrictionRestIT extends SecurityOnTrialLicenseRestTestCase {

public void testCreateRoleWithWorkflowsRestrictionFail() {
Request createRoleRequest = new Request(HttpPut.METHOD_NAME, "/_security/role/role_with_restriction");
createRoleRequest.setJsonEntity("""
{
"cluster": ["all"],
"indices": [
{
"names": ["index-a"],
"privileges": ["all"]
}
],
"restriction":{
"workflows": ["foo", "bar"]
}
}""");

ResponseException e = expectThrows(ResponseException.class, () -> adminClient().performRequest(createRoleRequest));
assertEquals(400, e.getResponse().getStatusLine().getStatusCode());
assertThat(e.getMessage(), containsString("failed to parse role [role_with_restriction]. unexpected field [restriction]"));
}

public void testUpdateRoleWithWorkflowsRestrictionFail() throws IOException {
Request createRoleRequest = new Request(HttpPut.METHOD_NAME, "/_security/role/my_role");
createRoleRequest.setJsonEntity("""
{
"cluster": ["all"],
"indices": [
{
"names": ["index-a"],
"privileges": ["all"]
}
]
}""");
Response createRoleResponse = adminClient().performRequest(createRoleRequest);
assertOK(createRoleResponse);

Request updateRoleRequest = new Request(HttpPost.METHOD_NAME, "/_security/role/my_role");
updateRoleRequest.setJsonEntity("""
{
"cluster": ["all"],
"indices": [
{
"names": ["index-*"],
"privileges": ["all"]
}
],
"restriction":{
"workflows": ["foo", "bar"]
}
}""");

ResponseException e = expectThrows(ResponseException.class, () -> adminClient().performRequest(updateRoleRequest));
assertEquals(400, e.getResponse().getStatusLine().getStatusCode());
assertThat(e.getMessage(), containsString("failed to parse role [my_role]. unexpected field [restriction]"));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,7 @@ static RoleDescriptor parseRoleDescriptor(
if (token == XContentParser.Token.START_OBJECT) {
// we pass true as last parameter because we do not want to reject files if field permissions
// are given in 2.x syntax
RoleDescriptor descriptor = RoleDescriptor.parse(roleName, parser, true);
RoleDescriptor descriptor = RoleDescriptor.parse(roleName, parser, true, false);
return checkDescriptor(descriptor, path, logger, settings, xContentRegistry);
} else {
logger.error("invalid role definition [{}] in roles file [{}]. skipping role...", roleName, path.toAbsolutePath());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,7 @@ public void putRole(final PutRoleRequest request, final RoleDescriptor role, fin
void innerPutRole(final PutRoleRequest request, final RoleDescriptor role, final ActionListener<Boolean> listener) {
final String roleName = role.getName();
assert NativeRealmValidationUtil.validateRoleName(roleName, false) == null : "Role name was invalid or reserved: " + roleName;
assert false == role.hasRestriction() : "restriction is not supported for native roles";

securityIndex.prepareIndexIfNeededThenExecute(listener::onFailure, () -> {
final XContentBuilder xContentBuilder;
Expand Down Expand Up @@ -454,9 +455,9 @@ static RoleDescriptor transformRole(String id, BytesReference sourceBytes, Logge
assert id.startsWith(ROLE_TYPE) : "[" + id + "] does not have role prefix";
final String name = id.substring(ROLE_TYPE.length() + 1);
try {
// we pass true as last parameter because we do not want to reject permissions if the field permissions
// we pass true as allow2xFormat parameter because we do not want to reject permissions if the field permissions
// are given in 2.x syntax
RoleDescriptor roleDescriptor = RoleDescriptor.parse(name, sourceBytes, true, XContentType.JSON);
RoleDescriptor roleDescriptor = RoleDescriptor.parse(name, sourceBytes, true, XContentType.JSON, false);
final boolean dlsEnabled = Arrays.stream(roleDescriptor.getIndicesPrivileges())
.anyMatch(IndicesPrivileges::isUsingDocumentLevelSecurity);
final boolean flsEnabled = Arrays.stream(roleDescriptor.getIndicesPrivileges())
Expand Down Expand Up @@ -488,7 +489,7 @@ static RoleDescriptor transformRole(String id, BytesReference sourceBytes, Logge
return roleDescriptor;
}
} catch (Exception e) {
logger.error(() -> "error in the format of data for role [" + name + "]", e);
logger.error("error in the format of data for role [" + name + "]", e);
return null;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -617,7 +617,7 @@ public void testThatInvalidRoleDefinitions() throws Exception {
assertThat(role, notNullValue());
assertThat(role.names(), equalTo(new String[] { "valid_role" }));

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

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

assertThat(events, hasSize(1));
assertThat(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
*/
package org.elasticsearch.xpack.security.authz.store;

import org.apache.logging.log4j.Logger;
import org.elasticsearch.ElasticsearchParseException;
import org.elasticsearch.ElasticsearchSecurityException;
import org.elasticsearch.TransportVersion;
import org.elasticsearch.Version;
Expand Down Expand Up @@ -47,12 +49,14 @@
import org.elasticsearch.xpack.core.security.authz.RoleDescriptor;
import org.elasticsearch.xpack.core.security.authz.RoleDescriptor.IndicesPrivileges;
import org.elasticsearch.xpack.core.security.authz.RoleDescriptorTests;
import org.elasticsearch.xpack.core.security.authz.RoleRestrictionTests;
import org.elasticsearch.xpack.core.security.authz.privilege.ClusterPrivilegeResolver;
import org.elasticsearch.xpack.security.support.SecurityIndexManager;
import org.elasticsearch.xpack.security.support.SecuritySystemIndices;
import org.elasticsearch.xpack.security.test.SecurityTestUtils;
import org.junit.After;
import org.junit.Before;
import org.mockito.ArgumentCaptor;
import org.mockito.Mockito;

import java.io.IOException;
Expand All @@ -70,7 +74,9 @@
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.instanceOf;
import static org.hamcrest.Matchers.nullValue;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

public class NativeRolesStoreTests extends ESTestCase {
Expand Down Expand Up @@ -239,6 +245,61 @@ public void testRoleDescriptorWithFlsDlsLicensing() throws IOException {
assertThat(role, equalTo(noFlsDlsRole));
}

public void testTransformingRoleWithRestrictionFails() throws IOException {
MockLicenseState licenseState = mock(MockLicenseState.class);
when(licenseState.isAllowed(DOCUMENT_LEVEL_SECURITY_FEATURE)).thenReturn(false);
RoleDescriptor roleWithRestriction = new RoleDescriptor(
"role_with_restriction",
randomSubsetOf(ClusterPrivilegeResolver.names()).toArray(String[]::new),
new IndicesPrivileges[] {
IndicesPrivileges.builder()
.privileges("READ")
.indices(generateRandomStringArray(5, randomIntBetween(3, 9), false, false))
.grantedFields("*")
.deniedFields(generateRandomStringArray(5, randomIntBetween(3, 9), false, false))
.query(
randomBoolean()
? "{ \"term\": { \""
+ randomAlphaOfLengthBetween(3, 24)
+ "\" : \""
+ randomAlphaOfLengthBetween(3, 24)
+ "\" }"
: "{ \"match_all\": {} }"
)
.build() },
RoleDescriptorTests.randomApplicationPrivileges(),
RoleDescriptorTests.randomClusterPrivileges(),
generateRandomStringArray(5, randomIntBetween(2, 8), true, true),
RoleDescriptorTests.randomRoleDescriptorMetadata(ESTestCase.randomBoolean()),
null,
TcpTransport.isUntrustedRemoteClusterEnabled() ? RoleDescriptorTests.randomRemoteIndicesPrivileges(1, 2) : null,
RoleRestrictionTests.randomWorkflowsRestriction(1, 2)
);

XContentBuilder builder = roleWithRestriction.toXContent(
XContentBuilder.builder(XContentType.JSON.xContent()),
ToXContent.EMPTY_PARAMS
);

Logger mockedLogger = Mockito.mock(Logger.class);
BytesReference bytes = BytesReference.bytes(builder);
RoleDescriptor transformedRole = NativeRolesStore.transformRole(
RoleDescriptor.ROLE_TYPE + "-role_with_restriction",
bytes,
mockedLogger,
licenseState
);
assertThat(transformedRole, nullValue());
ArgumentCaptor<ElasticsearchParseException> exceptionCaptor = ArgumentCaptor.forClass(ElasticsearchParseException.class);
ArgumentCaptor<String> messageCaptor = ArgumentCaptor.forClass(String.class);
verify(mockedLogger).error(messageCaptor.capture(), exceptionCaptor.capture());
assertThat(messageCaptor.getValue(), containsString("error in the format of data for role [role_with_restriction]"));
assertThat(
exceptionCaptor.getValue().getMessage(),
containsString("failed to parse role [role_with_restriction]. unexpected field [restriction]")
);
}

public void testPutOfRoleWithFlsDlsUnlicensed() throws IOException {
final Client client = mock(Client.class);
final ClusterService clusterService = mockClusterServiceWithMinNodeVersion(TransportVersion.CURRENT);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,3 +45,16 @@ role5:
- names:
- 'idx1'
privileges: []

# role includes unsupported workflows restriction
role6:
cluster:
- ALL
indices:
- names: idx
privileges:
- ALL
restriction:
workflows:
- workflow1
- workflow2

0 comments on commit 77a80b0

Please sign in to comment.