From 381267231c54f8d0feecbfecad0d7f3dcd0bbc69 Mon Sep 17 00:00:00 2001 From: pgomulka Date: Thu, 24 Jun 2021 09:37:10 +0200 Subject: [PATCH 1/5] [Rest Api Compatibility] Typed TermLookups Previously removed in #46943 parsing type field in term lookup is now possible with rest compatible api. The type field is ignored relates main meta issue #51816 relates type removal meta issue #54160 --- rest-api-spec/build.gradle | 2 -- .../src/main/java/org/elasticsearch/indices/TermsLookup.java | 4 ++++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/rest-api-spec/build.gradle b/rest-api-spec/build.gradle index bdc0830c54bfd..eb57fc3d01375 100644 --- a/rest-api-spec/build.gradle +++ b/rest-api-spec/build.gradle @@ -104,8 +104,6 @@ tasks.named("yamlRestCompatTest").configure { 'mtermvectors/11_basic_with_types/Basic tests for multi termvector get', 'mtermvectors/21_deprecated_with_types/Deprecated camel case and _ parameters should fail in Term Vectors query', 'mtermvectors/30_mix_typeless_typeful/mtermvectors without types on an index that has types', - 'search/150_rewrite_on_coordinator/Ensure that we fetch the document only once', //terms_lookup - 'search/171_terms_query_with_types/Terms Query with No.of terms exceeding index.max_terms_count should FAIL', //bulk 'search/260_parameter_validation/test size=-1 is deprecated', //size=-1 change 'search/310_match_bool_prefix/multi_match multiple fields with cutoff_frequency throws exception', //cutoff_frequency 'search/340_type_query/type query', // type_query - probably should behave like match_all diff --git a/server/src/main/java/org/elasticsearch/indices/TermsLookup.java b/server/src/main/java/org/elasticsearch/indices/TermsLookup.java index 53964d11b0d6e..1e2a090f5c2d9 100644 --- a/server/src/main/java/org/elasticsearch/indices/TermsLookup.java +++ b/server/src/main/java/org/elasticsearch/indices/TermsLookup.java @@ -17,6 +17,7 @@ import org.elasticsearch.common.xcontent.ToXContentFragment; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; +import org.elasticsearch.core.RestApiVersion; import org.elasticsearch.index.mapper.MapperService; import org.elasticsearch.index.query.TermsQueryBuilder; @@ -24,6 +25,7 @@ import java.util.Objects; import static org.elasticsearch.common.xcontent.ConstructingObjectParser.constructorArg; +import static org.elasticsearch.core.RestApiVersion.equalTo; /** * Encapsulates the parameters needed to fetch terms. @@ -107,6 +109,8 @@ public TermsLookup routing(String routing) { PARSER.declareString(constructorArg(), new ParseField("id")); PARSER.declareString(constructorArg(), new ParseField("path")); PARSER.declareString(TermsLookup::routing, new ParseField("routing")); + PARSER.declareString((termLookup,type)-> {}, new ParseField("type") + .forRestApiVersion(equalTo(RestApiVersion.V_7))); } public static TermsLookup parseTermsLookup(XContentParser parser) throws IOException { From 3fd6d422338240c0dbcde04b49180920680f60be Mon Sep 17 00:00:00 2001 From: pgomulka Date: Thu, 24 Jun 2021 11:30:19 +0200 Subject: [PATCH 2/5] role validation --- .../common/xcontent/XContent.java | 4 +++ .../common/xcontent/cbor/CborXContent.java | 5 +++ .../common/xcontent/json/JsonXContent.java | 5 +++ .../common/xcontent/smile/SmileXContent.java | 7 +++++ .../common/xcontent/yaml/YamlXContent.java | 7 ++++- .../action/role/PutRoleRequestBuilder.java | 2 ++ .../authz/permission/DocumentPermissions.java | 3 +- .../authz/support/DLSRoleQueryValidator.java | 31 +++++++++++-------- .../action/TransportTermsEnumAction.java | 5 +-- .../action/role/TransportPutRoleAction.java | 12 +++---- .../authc/support/ApiKeyGenerator.java | 3 +- .../security/authz/store/FileRolesStore.java | 3 +- .../rest/action/role/RestPutRoleAction.java | 6 ++++ 13 files changed, 68 insertions(+), 25 deletions(-) diff --git a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContent.java b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContent.java index b448a613065ad..f9150d201b071 100644 --- a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContent.java +++ b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContent.java @@ -88,4 +88,8 @@ XContentParser createParserForCompatibility(NamedXContentRegistry xContentRegist DeprecationHandler deprecationHandler, byte[] data, int offset, int length, RestApiVersion restApiVersion) throws IOException; + XContentParser createParserForCompatibility(NamedXContentRegistry xContentRegistry, + DeprecationHandler deprecationHandler, String content, + RestApiVersion restApiVersion) throws IOException; + } diff --git a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/cbor/CborXContent.java b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/cbor/CborXContent.java index 0fd9e85fa48e3..8c00ab8f8b544 100644 --- a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/cbor/CborXContent.java +++ b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/cbor/CborXContent.java @@ -112,4 +112,9 @@ public XContentParser createParserForCompatibility(NamedXContentRegistry xConten return new CborXContentParser(xContentRegistry, deprecationHandler, cborFactory.createParser(data, offset, length), restApiVersion); } + @Override + public XContentParser createParserForCompatibility(NamedXContentRegistry xContentRegistry, DeprecationHandler deprecationHandler, + String content, RestApiVersion restApiVersion) throws IOException { + return new CborXContentParser(xContentRegistry, deprecationHandler, cborFactory.createParser(content), restApiVersion); + } } diff --git a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/json/JsonXContent.java b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/json/JsonXContent.java index f7f4c098c772c..96d554cae1a3a 100644 --- a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/json/JsonXContent.java +++ b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/json/JsonXContent.java @@ -112,4 +112,9 @@ public XContentParser createParserForCompatibility(NamedXContentRegistry xConten return new JsonXContentParser(xContentRegistry, deprecationHandler, jsonFactory.createParser(data, offset, length), restApiVersion); } + @Override + public XContentParser createParserForCompatibility(NamedXContentRegistry xContentRegistry, DeprecationHandler deprecationHandler, + String content, RestApiVersion restApiVersion) throws IOException { + return new JsonXContentParser(xContentRegistry, deprecationHandler, jsonFactory.createParser(content), restApiVersion); + } } diff --git a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/smile/SmileXContent.java b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/smile/SmileXContent.java index 0faa82f002b20..5fba079d90a44 100644 --- a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/smile/SmileXContent.java +++ b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/smile/SmileXContent.java @@ -113,4 +113,11 @@ public XContentParser createParserForCompatibility(NamedXContentRegistry xConten return new SmileXContentParser(xContentRegistry, deprecationHandler, smileFactory.createParser(data, offset, length), restApiVersion); } + + @Override + public XContentParser createParserForCompatibility(NamedXContentRegistry xContentRegistry, DeprecationHandler deprecationHandler, + String content, RestApiVersion restApiVersion) throws IOException { + return new SmileXContentParser(xContentRegistry, deprecationHandler, smileFactory.createParser(content), + restApiVersion); + } } diff --git a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/yaml/YamlXContent.java b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/yaml/YamlXContent.java index 92fdbfcd43967..66f7921b9e7fd 100644 --- a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/yaml/YamlXContent.java +++ b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/yaml/YamlXContent.java @@ -107,5 +107,10 @@ public XContentParser createParserForCompatibility(NamedXContentRegistry xConten restApiVersion); } - + @Override + public XContentParser createParserForCompatibility(NamedXContentRegistry xContentRegistry, DeprecationHandler deprecationHandler, + String content, RestApiVersion restApiVersion) throws IOException { + return new YamlXContentParser(xContentRegistry, deprecationHandler, yamlFactory.createParser(content), + restApiVersion); + } } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/role/PutRoleRequestBuilder.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/role/PutRoleRequestBuilder.java index 8d48f792c74f2..2b26d486aa6ad 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/role/PutRoleRequestBuilder.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/role/PutRoleRequestBuilder.java @@ -13,6 +13,7 @@ import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.xpack.core.security.authz.RoleDescriptor; +import org.elasticsearch.xpack.core.security.authz.support.DLSRoleQueryValidator; import java.io.IOException; import java.util.Map; @@ -38,6 +39,7 @@ public PutRoleRequestBuilder source(String name, BytesReference source, XContent // 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); + assert name.equals(descriptor.getName()); request.name(name); request.cluster(descriptor.getClusterPrivileges()); diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/DocumentPermissions.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/DocumentPermissions.java index 6ac06ee9b1a40..69ceff9cd403b 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/DocumentPermissions.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/DocumentPermissions.java @@ -13,6 +13,7 @@ import org.apache.lucene.search.join.ToChildBlockJoinQuery; import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.lucene.search.Queries; +import org.elasticsearch.core.RestApiVersion; import org.elasticsearch.index.query.QueryBuilder; import org.elasticsearch.index.query.QueryRewriteContext; import org.elasticsearch.index.query.SearchExecutionContext; @@ -119,7 +120,7 @@ private static void buildRoleQuery(User user, ScriptService scriptService, Shard for (BytesReference bytesReference : queries) { SearchExecutionContext context = searchExecutionContextProvider.apply(shardId); QueryBuilder queryBuilder = DLSRoleQueryValidator.evaluateAndVerifyRoleQuery(bytesReference, scriptService, - context.getXContentRegistry(), user); + context.getXContentRegistry(), user, RestApiVersion.current()); if (queryBuilder != null) { failIfQueryUsesClient(queryBuilder, context); Query roleQuery = context.toQuery(queryBuilder).query(); diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/support/DLSRoleQueryValidator.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/support/DLSRoleQueryValidator.java index 17552b6b98fb0..0779fcc457559 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/support/DLSRoleQueryValidator.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/support/DLSRoleQueryValidator.java @@ -17,6 +17,7 @@ import org.elasticsearch.common.xcontent.XContentParseException; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.XContentType; +import org.elasticsearch.core.RestApiVersion; import org.elasticsearch.index.query.AbstractQueryBuilder; import org.elasticsearch.index.query.BoolQueryBuilder; import org.elasticsearch.index.query.BoostingQueryBuilder; @@ -45,23 +46,23 @@ private DLSRoleQueryValidator() { /** * Validates the query field in the {@link RoleDescriptor.IndicesPrivileges} only if it is not a template query.
* It parses the query and builds the {@link QueryBuilder}, also checks if the query type is supported in DLS role query. - * - * @param indicesPrivileges {@link RoleDescriptor.IndicesPrivileges} + * @param indicesPrivileges {@link RoleDescriptor.IndicesPrivileges} * @param xContentRegistry {@link NamedXContentRegistry} for finding named queries + * @param restApiVersion */ public static void validateQueryField(RoleDescriptor.IndicesPrivileges[] indicesPrivileges, - NamedXContentRegistry xContentRegistry) { + NamedXContentRegistry xContentRegistry, RestApiVersion restApiVersion) { if (indicesPrivileges != null) { for (int i = 0; i < indicesPrivileges.length; i++) { BytesReference query = indicesPrivileges[i].getQuery(); try { if (query != null) { - if (isTemplateQuery(query, xContentRegistry)) { + if (isTemplateQuery(query, xContentRegistry, restApiVersion)) { // skip template query, this requires runtime information like 'User' information. continue; } - evaluateAndVerifyRoleQuery(query.utf8ToString(), xContentRegistry); + evaluateAndVerifyRoleQuery(query.utf8ToString(), xContentRegistry, restApiVersion); } } catch (ParsingException | IllegalArgumentException | IOException e) { throw new ElasticsearchParseException("failed to parse field 'query' for indices [" + @@ -72,9 +73,10 @@ public static void validateQueryField(RoleDescriptor.IndicesPrivileges[] indices } } - private static boolean isTemplateQuery(BytesReference query, NamedXContentRegistry xContentRegistry) throws IOException { - try (XContentParser parser = XContentType.JSON.xContent().createParser(xContentRegistry, - LoggingDeprecationHandler.INSTANCE, query.utf8ToString())) { + private static boolean isTemplateQuery(BytesReference query, NamedXContentRegistry xContentRegistry, RestApiVersion restApiVersion) + throws IOException { + try (XContentParser parser = XContentType.JSON.xContent().createParserForCompatibility(xContentRegistry, + LoggingDeprecationHandler.INSTANCE, query.streamInput(), restApiVersion)) { XContentParser.Token token = parser.nextToken(); if (token != XContentParser.Token.START_OBJECT) { throw new XContentParseException(parser.getTokenLocation(), "expected [" + XContentParser.Token.START_OBJECT + "] but " + @@ -103,17 +105,19 @@ private static boolean isTemplateQuery(BytesReference query, NamedXContentRegist * @param scriptService {@link ScriptService} used for evaluation of a template query * @param xContentRegistry {@link NamedXContentRegistry} for finding named queries * @param user {@link User} used when evaluation a template query + * @param restApiVersion * @return {@link QueryBuilder} if the query is valid and allowed, in case {@link RoleDescriptor.IndicesPrivileges} * * does not have a query field then it returns {@code null}. */ @Nullable public static QueryBuilder evaluateAndVerifyRoleQuery(BytesReference query, ScriptService scriptService, - NamedXContentRegistry xContentRegistry, User user) { + NamedXContentRegistry xContentRegistry, User user, + RestApiVersion restApiVersion) { if (query != null) { String templateResult = SecurityQueryTemplateEvaluator.evaluateTemplate(query.utf8ToString(), scriptService, user); try { - return evaluateAndVerifyRoleQuery(templateResult, xContentRegistry); + return evaluateAndVerifyRoleQuery(templateResult, xContentRegistry, restApiVersion); } catch (ElasticsearchParseException | ParsingException | XContentParseException | IOException e) { throw new ElasticsearchParseException("failed to parse field 'query' from the role descriptor", e); } @@ -122,10 +126,11 @@ public static QueryBuilder evaluateAndVerifyRoleQuery(BytesReference query, Scri } @Nullable - private static QueryBuilder evaluateAndVerifyRoleQuery(String query, NamedXContentRegistry xContentRegistry) throws IOException { + private static QueryBuilder evaluateAndVerifyRoleQuery(String query, NamedXContentRegistry xContentRegistry, RestApiVersion restApiVersion) + throws IOException { if (query != null) { - try (XContentParser parser = XContentFactory.xContent(query).createParser(xContentRegistry, - LoggingDeprecationHandler.INSTANCE, query)) { + try (XContentParser parser = XContentFactory.xContent(query).createParserForCompatibility(xContentRegistry, + LoggingDeprecationHandler.INSTANCE, query, restApiVersion)) { QueryBuilder queryBuilder = AbstractQueryBuilder.parseInnerQueryBuilder(parser); verifyRoleQuery(queryBuilder); return queryBuilder; diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/termsenum/action/TransportTermsEnumAction.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/termsenum/action/TransportTermsEnumAction.java index 45b116c41dd4d..1bd948585e07f 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/termsenum/action/TransportTermsEnumAction.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/termsenum/action/TransportTermsEnumAction.java @@ -36,6 +36,7 @@ import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.util.concurrent.EsThreadPoolExecutor; import org.elasticsearch.common.util.concurrent.ThreadContext; +import org.elasticsearch.core.RestApiVersion; import org.elasticsearch.core.internal.io.IOUtils; import org.elasticsearch.index.IndexService; import org.elasticsearch.index.engine.Engine; @@ -435,8 +436,8 @@ private boolean canAccess( querySource, scriptService, queryShardContext.getXContentRegistry(), - securityContext.getUser() - ); + securityContext.getUser(), + RestApiVersion.current()); QueryBuilder rewrittenQueryBuilder = Rewriteable.rewrite(queryBuilder, queryShardContext); if (rewrittenQueryBuilder instanceof MatchAllQueryBuilder) { // One of the roles assigned has "all" permissions - allow unfettered access to termsDict diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/role/TransportPutRoleAction.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/role/TransportPutRoleAction.java index fcb0ddc8f278d..4991adac1d008 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/role/TransportPutRoleAction.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/role/TransportPutRoleAction.java @@ -42,12 +42,12 @@ protected void doExecute(Task task, final PutRoleRequest request, final ActionLi return; } - try { - DLSRoleQueryValidator.validateQueryField(request.roleDescriptor().getIndicesPrivileges(), xContentRegistry); - } catch (ElasticsearchException | IllegalArgumentException e) { - listener.onFailure(e); - return; - } +// try { +// DLSRoleQueryValidator.validateQueryField(request.roleDescriptor().getIndicesPrivileges(), xContentRegistry); +// } catch (ElasticsearchException | IllegalArgumentException e) { +// listener.onFailure(e); +// return; +// } rolesStore.putRole(request, request.roleDescriptor(), listener.delegateFailure((l, created) -> { if (created) { diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/support/ApiKeyGenerator.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/support/ApiKeyGenerator.java index 71afd11c21696..089544a835ac2 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/support/ApiKeyGenerator.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/support/ApiKeyGenerator.java @@ -11,6 +11,7 @@ import org.elasticsearch.ElasticsearchSecurityException; import org.elasticsearch.action.ActionListener; import org.elasticsearch.common.xcontent.NamedXContentRegistry; +import org.elasticsearch.core.RestApiVersion; import org.elasticsearch.xpack.core.security.action.CreateApiKeyRequest; import org.elasticsearch.xpack.core.security.action.CreateApiKeyResponse; import org.elasticsearch.xpack.core.security.authc.Authentication; @@ -44,7 +45,7 @@ public void generateApiKey(Authentication authentication, CreateApiKeyRequest re ActionListener.wrap(roleDescriptors -> { for (RoleDescriptor rd : roleDescriptors) { try { - DLSRoleQueryValidator.validateQueryField(rd.getIndicesPrivileges(), xContentRegistry); + DLSRoleQueryValidator.validateQueryField(rd.getIndicesPrivileges(), xContentRegistry, RestApiVersion.current()); } catch (ElasticsearchException | IllegalArgumentException e) { listener.onFailure(e); return; diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/store/FileRolesStore.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/store/FileRolesStore.java index f36f0127f866d..51425ab44cfc1 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/store/FileRolesStore.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/store/FileRolesStore.java @@ -21,6 +21,7 @@ import org.elasticsearch.common.xcontent.NamedXContentRegistry; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.yaml.YamlXContent; +import org.elasticsearch.core.RestApiVersion; import org.elasticsearch.env.Environment; import org.elasticsearch.license.XPackLicenseState; import org.elasticsearch.license.XPackLicenseState.Feature; @@ -318,7 +319,7 @@ private static RoleDescriptor checkDescriptor(RoleDescriptor descriptor, Path pa return null; } else { try { - DLSRoleQueryValidator.validateQueryField(descriptor.getIndicesPrivileges(), xContentRegistry); + DLSRoleQueryValidator.validateQueryField(descriptor.getIndicesPrivileges(), xContentRegistry, RestApiVersion.current()); } catch (ElasticsearchException | IllegalArgumentException e) { logger.error((Supplier) () -> new ParameterizedMessage( "invalid role definition [{}] in roles file [{}]. failed to validate query field. skipping role...", roleName, diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/action/role/RestPutRoleAction.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/action/role/RestPutRoleAction.java index 70d50ea3d974a..feb687992f647 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/action/role/RestPutRoleAction.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/action/role/RestPutRoleAction.java @@ -6,6 +6,7 @@ */ package org.elasticsearch.xpack.security.rest.action.role; +import org.elasticsearch.ElasticsearchException; import org.elasticsearch.client.node.NodeClient; import org.elasticsearch.core.RestApiVersion; import org.elasticsearch.common.settings.Settings; @@ -18,6 +19,7 @@ import org.elasticsearch.rest.action.RestBuilderListener; import org.elasticsearch.xpack.core.security.action.role.PutRoleRequestBuilder; import org.elasticsearch.xpack.core.security.action.role.PutRoleResponse; +import org.elasticsearch.xpack.core.security.authz.support.DLSRoleQueryValidator; import org.elasticsearch.xpack.security.rest.action.SecurityBaseRestHandler; import java.io.IOException; @@ -55,6 +57,10 @@ public RestChannelConsumer innerPrepareRequest(RestRequest request, NodeClient c PutRoleRequestBuilder requestBuilder = new PutRoleRequestBuilder(client) .source(request.param("name"), request.requiredContent(), request.getXContentType()) .setRefreshPolicy(request.param("refresh")); + + DLSRoleQueryValidator.validateQueryField(requestBuilder.request().indices(), request.getXContentRegistry(), request.getRestApiVersion()); + + return channel -> requestBuilder.execute(new RestBuilderListener<>(channel) { @Override public RestResponse buildResponse(PutRoleResponse putRoleResponse, XContentBuilder builder) throws Exception { From e04630bd621586ed654d90cdd974b5b9a1a55435 Mon Sep 17 00:00:00 2001 From: pgomulka Date: Thu, 24 Jun 2021 12:06:02 +0200 Subject: [PATCH 3/5] precommit --- .../core/security/action/role/PutRoleRequestBuilder.java | 3 +-- .../core/security/authz/support/DLSRoleQueryValidator.java | 4 ++-- .../xpack/security/action/role/TransportPutRoleAction.java | 2 -- .../xpack/security/rest/action/role/RestPutRoleAction.java | 6 +++--- 4 files changed, 6 insertions(+), 9 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/role/PutRoleRequestBuilder.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/role/PutRoleRequestBuilder.java index 2b26d486aa6ad..90dfd16d5bb31 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/role/PutRoleRequestBuilder.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/role/PutRoleRequestBuilder.java @@ -9,11 +9,10 @@ import org.elasticsearch.action.ActionRequestBuilder; import org.elasticsearch.action.support.WriteRequestBuilder; import org.elasticsearch.client.ElasticsearchClient; -import org.elasticsearch.core.Nullable; import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.xcontent.XContentType; +import org.elasticsearch.core.Nullable; import org.elasticsearch.xpack.core.security.authz.RoleDescriptor; -import org.elasticsearch.xpack.core.security.authz.support.DLSRoleQueryValidator; import java.io.IOException; import java.util.Map; diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/support/DLSRoleQueryValidator.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/support/DLSRoleQueryValidator.java index 0779fcc457559..4cb818fd8b3ed 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/support/DLSRoleQueryValidator.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/support/DLSRoleQueryValidator.java @@ -126,8 +126,8 @@ public static QueryBuilder evaluateAndVerifyRoleQuery(BytesReference query, Scri } @Nullable - private static QueryBuilder evaluateAndVerifyRoleQuery(String query, NamedXContentRegistry xContentRegistry, RestApiVersion restApiVersion) - throws IOException { + private static QueryBuilder evaluateAndVerifyRoleQuery(String query, NamedXContentRegistry xContentRegistry, + RestApiVersion restApiVersion) throws IOException { if (query != null) { try (XContentParser parser = XContentFactory.xContent(query).createParserForCompatibility(xContentRegistry, LoggingDeprecationHandler.INSTANCE, query, restApiVersion)) { diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/role/TransportPutRoleAction.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/role/TransportPutRoleAction.java index 4991adac1d008..fcec084099065 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/role/TransportPutRoleAction.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/role/TransportPutRoleAction.java @@ -6,7 +6,6 @@ */ package org.elasticsearch.xpack.security.action.role; -import org.elasticsearch.ElasticsearchException; import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.support.ActionFilters; import org.elasticsearch.action.support.HandledTransportAction; @@ -18,7 +17,6 @@ import org.elasticsearch.xpack.core.security.action.role.PutRoleRequest; import org.elasticsearch.xpack.core.security.action.role.PutRoleResponse; import org.elasticsearch.xpack.core.security.authz.store.ReservedRolesStore; -import org.elasticsearch.xpack.core.security.authz.support.DLSRoleQueryValidator; import org.elasticsearch.xpack.security.authz.store.NativeRolesStore; public class TransportPutRoleAction extends HandledTransportAction { diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/action/role/RestPutRoleAction.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/action/role/RestPutRoleAction.java index feb687992f647..d65090368ae42 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/action/role/RestPutRoleAction.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/action/role/RestPutRoleAction.java @@ -6,11 +6,10 @@ */ package org.elasticsearch.xpack.security.rest.action.role; -import org.elasticsearch.ElasticsearchException; import org.elasticsearch.client.node.NodeClient; -import org.elasticsearch.core.RestApiVersion; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.XContentBuilder; +import org.elasticsearch.core.RestApiVersion; import org.elasticsearch.license.XPackLicenseState; import org.elasticsearch.rest.BytesRestResponse; import org.elasticsearch.rest.RestRequest; @@ -58,7 +57,8 @@ public RestChannelConsumer innerPrepareRequest(RestRequest request, NodeClient c .source(request.param("name"), request.requiredContent(), request.getXContentType()) .setRefreshPolicy(request.param("refresh")); - DLSRoleQueryValidator.validateQueryField(requestBuilder.request().indices(), request.getXContentRegistry(), request.getRestApiVersion()); + DLSRoleQueryValidator.validateQueryField(requestBuilder.request().indices(), request.getXContentRegistry(), + request.getRestApiVersion()); return channel -> requestBuilder.execute(new RestBuilderListener<>(channel) { From 8b2b49eccca0a14ca0d6f472a4826ff089365eae Mon Sep 17 00:00:00 2001 From: pgomulka Date: Thu, 24 Jun 2021 16:51:15 +0200 Subject: [PATCH 4/5] rest putrole action tests --- .../action/role/TransportPutRoleAction.java | 7 -- .../action/role/RestPutRoleActionTests.java | 99 +++++++++++++++++++ 2 files changed, 99 insertions(+), 7 deletions(-) create mode 100644 x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/rest/action/role/RestPutRoleActionTests.java diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/role/TransportPutRoleAction.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/role/TransportPutRoleAction.java index fcec084099065..affbe735a8a11 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/role/TransportPutRoleAction.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/role/TransportPutRoleAction.java @@ -40,13 +40,6 @@ protected void doExecute(Task task, final PutRoleRequest request, final ActionLi return; } -// try { -// DLSRoleQueryValidator.validateQueryField(request.roleDescriptor().getIndicesPrivileges(), xContentRegistry); -// } catch (ElasticsearchException | IllegalArgumentException e) { -// listener.onFailure(e); -// return; -// } - rolesStore.putRole(request, request.roleDescriptor(), listener.delegateFailure((l, created) -> { if (created) { logger.info("added role [{}]", request.name()); diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/rest/action/role/RestPutRoleActionTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/rest/action/role/RestPutRoleActionTests.java new file mode 100644 index 0000000000000..cb3731cba1814 --- /dev/null +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/rest/action/role/RestPutRoleActionTests.java @@ -0,0 +1,99 @@ +/* + * 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.rest.action.role; + +import junit.framework.TestCase; + +import org.elasticsearch.ElasticsearchParseException; +import org.elasticsearch.action.ActionListener; +import org.elasticsearch.action.get.GetRequest; +import org.elasticsearch.action.get.GetResponse; +import org.elasticsearch.action.support.ActionFilters; +import org.elasticsearch.common.Strings; +import org.elasticsearch.common.bytes.BytesArray; +import org.elasticsearch.common.bytes.BytesReference; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.xcontent.XContentType; +import org.elasticsearch.core.RestApiVersion; +import org.elasticsearch.license.License; +import org.elasticsearch.license.XPackLicenseState; +import org.elasticsearch.rest.RestRequest; +import org.elasticsearch.rest.action.document.RestGetAction; +import org.elasticsearch.tasks.Task; +import org.elasticsearch.test.rest.FakeRestRequest; +import org.elasticsearch.test.rest.RestActionTestCase; +import org.elasticsearch.transport.Transport; +import org.elasticsearch.transport.TransportService; +import org.elasticsearch.xpack.core.security.action.role.PutRoleRequest; +import org.elasticsearch.xpack.core.security.action.role.PutRoleResponse; +import org.elasticsearch.xpack.security.action.role.TransportPutRoleAction; +import org.elasticsearch.xpack.security.authz.store.NativeRolesStore; +import org.junit.Before; +import org.mockito.Mockito; + +import java.util.Collections; +import java.util.List; +import java.util.Map; +import java.util.concurrent.atomic.AtomicReference; + +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.instanceOf; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.notNullValue; +import static org.hamcrest.Matchers.nullValue; +import static org.mockito.Mockito.mock; + +public class RestPutRoleActionTests extends RestActionTestCase { + + final List contentTypeHeader = Collections.singletonList(randomCompatibleMediaType(RestApiVersion.V_7)); + final List jsonContentType = Collections.singletonList(XContentType.JSON.toParsedMediaType().responseContentTypeHeader()); + + @Before + public void setUpAction() { + controller().registerHandler(new RestPutRoleAction(Settings.EMPTY, new XPackLicenseState(Settings.EMPTY, ()->0))); + verifyingClient.setExecuteVerifier((actionType, request) -> { + assertThat(request, instanceOf(PutRoleRequest.class)); + return Mockito.mock(PutRoleResponse.class); + }); + } + + + public void testCreationOfRoleWithMalformedQueryJsonFails() { + + PutRoleRequest request = new PutRoleRequest(); + request.name("test"); + String[] malformedQueryJson = new String[]{"{ \"match_all\": { \"unknown_field\": \"\" } }", + "{ malformed JSON }", + "{ \"unknown\": {\"\"} }", + "{}"}; + BytesReference query = new BytesArray(randomFrom(malformedQueryJson)); + request.addIndex(new String[]{"idx1"}, new String[]{"read"}, null, null, query, randomBoolean()); + + FakeRestRequest.Builder deprecatedRequest = new FakeRestRequest.Builder(xContentRegistry()) + .withHeaders(Map.of("Content-Type", jsonContentType, "Accept", jsonContentType)) + .withPath("_security/role/rolename") + .withMethod(RestRequest.Method.POST) + .withContent(query, null); + dispatchRequest(deprecatedRequest.build()); + +// Throwable t = throwableRef.get(); +// assertThat(t, instanceOf(ElasticsearchParseException.class)); +// assertThat(t.getMessage(), containsString("failed to parse field 'query' for indices [" + +// Strings.arrayToCommaDelimitedString(new String[]{"idx1"}) + +// "] at index privilege [0] of role descriptor")); + } +// public void testValidation() { +// +// FakeRestRequest.Builder deprecatedRequest = new FakeRestRequest.Builder(xContentRegistry()).withHeaders( +// Map.of("Content-Type", contentTypeHeader, "Accept", contentTypeHeader) +// ).withPath("/some_index/some_type/some_id"); +// dispatchRequest(deprecatedRequest.withMethod(method).build()); +// assertWarnings(RestGetAction.TYPES_DEPRECATION_MESSAGE); +// } + +} From 3cfec42671d7fc287cc6935b45639ca3bab25704 Mon Sep 17 00:00:00 2001 From: pgomulka Date: Mon, 28 Jun 2021 10:54:26 +0200 Subject: [PATCH 5/5] malformed queries tests --- .../test/rest/RestActionTestCase.java | 4 + x-pack/plugin/build.gradle | 1 - .../action/role/RestPutRoleActionTests.java | 83 ++++++++----------- 3 files changed, 39 insertions(+), 49 deletions(-) diff --git a/test/framework/src/main/java/org/elasticsearch/test/rest/RestActionTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/rest/RestActionTestCase.java index a4d759242dcde..56cee804180b8 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/rest/RestActionTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/rest/RestActionTestCase.java @@ -64,6 +64,10 @@ protected RestController controller() { */ protected void dispatchRequest(RestRequest request) { FakeRestChannel channel = new FakeRestChannel(request, false, 1); + dispatchRequest(request, channel); + } + + protected void dispatchRequest(RestRequest request, FakeRestChannel channel) { ThreadContext threadContext = verifyingClient.threadPool().getThreadContext(); try(ThreadContext.StoredContext ignore = threadContext.stashContext()) { controller.dispatchRequest(request, channel, threadContext); diff --git a/x-pack/plugin/build.gradle b/x-pack/plugin/build.gradle index 109e9ab62ff1a..749da2fd39c05 100644 --- a/x-pack/plugin/build.gradle +++ b/x-pack/plugin/build.gradle @@ -149,7 +149,6 @@ tasks.named("yamlRestCompatTest").configure { 'ml/set_upgrade_mode/Test setting upgrade_mode to false when it is already false', 'ml/trained_model_cat_apis/Test cat trained models', 'roles/11_idx_arrays/Test put role api using as array of index names', - 'roles/30_prohibited_role_query/Test use prohibited query inside role query', 'rollup/delete_job/Test basic delete_job', 'rollup/delete_job/Test delete job twice', 'rollup/delete_job/Test delete running job', diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/rest/action/role/RestPutRoleActionTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/rest/action/role/RestPutRoleActionTests.java index cb3731cba1814..14785c5dd3b47 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/rest/action/role/RestPutRoleActionTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/rest/action/role/RestPutRoleActionTests.java @@ -7,46 +7,28 @@ package org.elasticsearch.xpack.security.rest.action.role; -import junit.framework.TestCase; - -import org.elasticsearch.ElasticsearchParseException; -import org.elasticsearch.action.ActionListener; -import org.elasticsearch.action.get.GetRequest; -import org.elasticsearch.action.get.GetResponse; -import org.elasticsearch.action.support.ActionFilters; -import org.elasticsearch.common.Strings; import org.elasticsearch.common.bytes.BytesArray; -import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.core.RestApiVersion; -import org.elasticsearch.license.License; import org.elasticsearch.license.XPackLicenseState; import org.elasticsearch.rest.RestRequest; -import org.elasticsearch.rest.action.document.RestGetAction; -import org.elasticsearch.tasks.Task; +import org.elasticsearch.rest.RestStatus; +import org.elasticsearch.test.rest.FakeRestChannel; import org.elasticsearch.test.rest.FakeRestRequest; import org.elasticsearch.test.rest.RestActionTestCase; -import org.elasticsearch.transport.Transport; -import org.elasticsearch.transport.TransportService; import org.elasticsearch.xpack.core.security.action.role.PutRoleRequest; import org.elasticsearch.xpack.core.security.action.role.PutRoleResponse; -import org.elasticsearch.xpack.security.action.role.TransportPutRoleAction; -import org.elasticsearch.xpack.security.authz.store.NativeRolesStore; import org.junit.Before; import org.mockito.Mockito; import java.util.Collections; import java.util.List; +import java.util.Locale; import java.util.Map; -import java.util.concurrent.atomic.AtomicReference; -import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.instanceOf; -import static org.hamcrest.Matchers.is; -import static org.hamcrest.Matchers.notNullValue; -import static org.hamcrest.Matchers.nullValue; -import static org.mockito.Mockito.mock; public class RestPutRoleActionTests extends RestActionTestCase { @@ -55,7 +37,7 @@ public class RestPutRoleActionTests extends RestActionTestCase { @Before public void setUpAction() { - controller().registerHandler(new RestPutRoleAction(Settings.EMPTY, new XPackLicenseState(Settings.EMPTY, ()->0))); + controller().registerHandler(new RestPutRoleAction(Settings.EMPTY, new XPackLicenseState(Settings.EMPTY, () -> 0))); verifyingClient.setExecuteVerifier((actionType, request) -> { assertThat(request, instanceOf(PutRoleRequest.class)); return Mockito.mock(PutRoleResponse.class); @@ -65,35 +47,40 @@ public void setUpAction() { public void testCreationOfRoleWithMalformedQueryJsonFails() { - PutRoleRequest request = new PutRoleRequest(); - request.name("test"); - String[] malformedQueryJson = new String[]{"{ \"match_all\": { \"unknown_field\": \"\" } }", + String[] malformedQueries = new String[]{"{ \"match_all\": { \"unknown_field\": \"\" } }", "{ malformed JSON }", "{ \"unknown\": {\"\"} }", "{}"}; - BytesReference query = new BytesArray(randomFrom(malformedQueryJson)); - request.addIndex(new String[]{"idx1"}, new String[]{"read"}, null, null, query, randomBoolean()); - FakeRestRequest.Builder deprecatedRequest = new FakeRestRequest.Builder(xContentRegistry()) - .withHeaders(Map.of("Content-Type", jsonContentType, "Accept", jsonContentType)) - .withPath("_security/role/rolename") - .withMethod(RestRequest.Method.POST) - .withContent(query, null); - dispatchRequest(deprecatedRequest.build()); + String queryPattern = "{" + + "\"indices\": [\n" + + " {\n" + + " \"names\": \"index\",\n" + + " \"privileges\": [\n" + + " \"all\"\n" + + " ],\n" + + " \"query\": %s" + + " }\n" + + " }\n" + + " ]" + + "}"; + for (String malformedQuery : malformedQueries) { -// Throwable t = throwableRef.get(); -// assertThat(t, instanceOf(ElasticsearchParseException.class)); -// assertThat(t.getMessage(), containsString("failed to parse field 'query' for indices [" + -// Strings.arrayToCommaDelimitedString(new String[]{"idx1"}) + -// "] at index privilege [0] of role descriptor")); - } -// public void testValidation() { -// -// FakeRestRequest.Builder deprecatedRequest = new FakeRestRequest.Builder(xContentRegistry()).withHeaders( -// Map.of("Content-Type", contentTypeHeader, "Accept", contentTypeHeader) -// ).withPath("/some_index/some_type/some_id"); -// dispatchRequest(deprecatedRequest.withMethod(method).build()); -// assertWarnings(RestGetAction.TYPES_DEPRECATION_MESSAGE); -// } + final String query = String.format(Locale.ROOT, queryPattern, malformedQuery); + FakeRestRequest.Builder deprecatedRequest = new FakeRestRequest.Builder(xContentRegistry()) + .withHeaders(Map.of("Content-Type", jsonContentType, "Accept", jsonContentType)) + .withPath("_security/role/rolename") + .withMethod(RestRequest.Method.POST) + .withContent(new BytesArray(query), null); + + RestRequest request = deprecatedRequest.build(); + FakeRestChannel channel = new FakeRestChannel(request, true, 1); + dispatchRequest(request, channel); + + assertThat(channel.errors().get(), equalTo(1)); + assertThat(channel.responses().get(), equalTo(0)); + assertThat(channel.capturedResponse().status(), equalTo(RestStatus.BAD_REQUEST)); + } + } }