diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/SecurityIT.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/SecurityIT.java index abf65d19df3..8122ff17648 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/SecurityIT.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/SecurityIT.java @@ -190,7 +190,7 @@ public class SecurityIT extends ESRestHighLevelClientTestCase { .name(roleName) .clusterPrivileges(randomSubsetOf(randomInt(3), Role.ClusterPrivilegeName.ALL_ARRAY)) .indicesPrivileges( - randomArray(3, IndicesPrivileges[]::new, () -> IndicesPrivilegesTests.createNewRandom(randomAlphaOfLength(3)))) + randomArray(3, IndicesPrivileges[]::new, () -> IndicesPrivilegesTests.createNewRandom("{\"match_all\": {}}"))) .applicationResourcePrivileges(randomArray(3, ApplicationResourcePrivileges[]::new, () -> ApplicationResourcePrivilegesTests.createNewRandom(randomAlphaOfLength(3).toLowerCase(Locale.ROOT)))) .runAsPrivilege(randomArray(3, String[]::new, () -> randomAlphaOfLength(3))); 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 08d754b4e53..c5b946ad595 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 @@ -12,29 +12,18 @@ import org.apache.lucene.search.join.BitSetProducer; import org.apache.lucene.search.join.ToChildBlockJoinQuery; import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.lucene.search.Queries; -import org.elasticsearch.common.xcontent.LoggingDeprecationHandler; -import org.elasticsearch.common.xcontent.XContentFactory; -import org.elasticsearch.common.xcontent.XContentParser; -import org.elasticsearch.index.query.BoolQueryBuilder; -import org.elasticsearch.index.query.BoostingQueryBuilder; -import org.elasticsearch.index.query.ConstantScoreQueryBuilder; -import org.elasticsearch.index.query.GeoShapeQueryBuilder; import org.elasticsearch.index.query.QueryBuilder; import org.elasticsearch.index.query.QueryRewriteContext; import org.elasticsearch.index.query.QueryShardContext; import org.elasticsearch.index.query.Rewriteable; -import org.elasticsearch.index.query.TermsQueryBuilder; -import org.elasticsearch.index.query.functionscore.FunctionScoreQueryBuilder; import org.elasticsearch.index.search.NestedHelper; import org.elasticsearch.index.shard.ShardId; import org.elasticsearch.script.ScriptService; -import org.elasticsearch.xpack.core.security.authz.support.SecurityQueryTemplateEvaluator; +import org.elasticsearch.xpack.core.security.authz.support.DLSRoleQueryValidator; import org.elasticsearch.xpack.core.security.user.User; import java.io.IOException; -import java.util.ArrayList; import java.util.Collections; -import java.util.List; import java.util.Set; import java.util.function.Function; @@ -127,11 +116,9 @@ public final class DocumentPermissions { BooleanQuery.Builder filter) throws IOException { for (BytesReference bytesReference : queries) { QueryShardContext queryShardContext = queryShardContextProvider.apply(shardId); - String templateResult = SecurityQueryTemplateEvaluator.evaluateTemplate(bytesReference.utf8ToString(), scriptService, user); - try (XContentParser parser = XContentFactory.xContent(templateResult).createParser(queryShardContext.getXContentRegistry(), - LoggingDeprecationHandler.INSTANCE, templateResult)) { - QueryBuilder queryBuilder = queryShardContext.parseInnerQueryBuilder(parser); - verifyRoleQuery(queryBuilder); + QueryBuilder queryBuilder = DLSRoleQueryValidator.evaluateAndVerifyRoleQuery(bytesReference, scriptService, + queryShardContext.getXContentRegistry(), user); + if (queryBuilder != null) { failIfQueryUsesClient(queryBuilder, queryShardContext); Query roleQuery = queryShardContext.toQuery(queryBuilder).query(); filter.add(roleQuery, SHOULD); @@ -153,50 +140,6 @@ public final class DocumentPermissions { filter.setMinimumNumberShouldMatch(1); } - /** - * Checks whether the role query contains queries we know can't be used as DLS role query. - */ - static void verifyRoleQuery(QueryBuilder queryBuilder) throws IOException { - if (queryBuilder instanceof TermsQueryBuilder) { - TermsQueryBuilder termsQueryBuilder = (TermsQueryBuilder) queryBuilder; - if (termsQueryBuilder.termsLookup() != null) { - throw new IllegalArgumentException("terms query with terms lookup isn't supported as part of a role query"); - } - } else if (queryBuilder instanceof GeoShapeQueryBuilder) { - GeoShapeQueryBuilder geoShapeQueryBuilder = (GeoShapeQueryBuilder) queryBuilder; - if (geoShapeQueryBuilder.shape() == null) { - throw new IllegalArgumentException("geoshape query referring to indexed shapes isn't support as part of a role query"); - } - } else if (queryBuilder.getName().equals("percolate")) { - // actually only if percolate query is referring to an existing document then this is problematic, - // a normal percolate query does work. However we can't check that here as this query builder is inside - // another module. So we don't allow the entire percolate query. I don't think users would ever use - // a percolate query as role query, so this restriction shouldn't prohibit anyone from using dls. - throw new IllegalArgumentException("percolate query isn't support as part of a role query"); - } else if (queryBuilder.getName().equals("has_child")) { - throw new IllegalArgumentException("has_child query isn't support as part of a role query"); - } else if (queryBuilder.getName().equals("has_parent")) { - throw new IllegalArgumentException("has_parent query isn't support as part of a role query"); - } else if (queryBuilder instanceof BoolQueryBuilder) { - BoolQueryBuilder boolQueryBuilder = (BoolQueryBuilder) queryBuilder; - List clauses = new ArrayList<>(); - clauses.addAll(boolQueryBuilder.filter()); - clauses.addAll(boolQueryBuilder.must()); - clauses.addAll(boolQueryBuilder.mustNot()); - clauses.addAll(boolQueryBuilder.should()); - for (QueryBuilder clause : clauses) { - verifyRoleQuery(clause); - } - } else if (queryBuilder instanceof ConstantScoreQueryBuilder) { - verifyRoleQuery(((ConstantScoreQueryBuilder) queryBuilder).innerQuery()); - } else if (queryBuilder instanceof FunctionScoreQueryBuilder) { - verifyRoleQuery(((FunctionScoreQueryBuilder) queryBuilder).query()); - } else if (queryBuilder instanceof BoostingQueryBuilder) { - verifyRoleQuery(((BoostingQueryBuilder) queryBuilder).negativeQuery()); - verifyRoleQuery(((BoostingQueryBuilder) queryBuilder).positiveQuery()); - } - } - /** * Fall back validation that verifies that queries during rewrite don't use * the client to make remote calls. In the case of DLS this can cause a dead 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 new file mode 100644 index 00000000000..1e4df7e8a4a --- /dev/null +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/support/DLSRoleQueryValidator.java @@ -0,0 +1,182 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ +package org.elasticsearch.xpack.core.security.authz.support; + +import org.elasticsearch.ElasticsearchParseException; +import org.elasticsearch.common.Nullable; +import org.elasticsearch.common.ParsingException; +import org.elasticsearch.common.Strings; +import org.elasticsearch.common.bytes.BytesReference; +import org.elasticsearch.common.xcontent.LoggingDeprecationHandler; +import org.elasticsearch.common.xcontent.NamedXContentRegistry; +import org.elasticsearch.common.xcontent.XContentFactory; +import org.elasticsearch.common.xcontent.XContentParseException; +import org.elasticsearch.common.xcontent.XContentParser; +import org.elasticsearch.common.xcontent.XContentType; +import org.elasticsearch.index.query.AbstractQueryBuilder; +import org.elasticsearch.index.query.BoolQueryBuilder; +import org.elasticsearch.index.query.BoostingQueryBuilder; +import org.elasticsearch.index.query.ConstantScoreQueryBuilder; +import org.elasticsearch.index.query.GeoShapeQueryBuilder; +import org.elasticsearch.index.query.QueryBuilder; +import org.elasticsearch.index.query.TermsQueryBuilder; +import org.elasticsearch.index.query.functionscore.FunctionScoreQueryBuilder; +import org.elasticsearch.script.ScriptService; +import org.elasticsearch.xpack.core.security.authz.RoleDescriptor; +import org.elasticsearch.xpack.core.security.user.User; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.List; + +/** + * This class helps in evaluating the query field if it is template, + * validating the query and checking if the query type is allowed to be used in DLS role query. + */ +public final class DLSRoleQueryValidator { + + 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 xContentRegistry {@link NamedXContentRegistry} for finding named queries + */ + public static void validateQueryField(RoleDescriptor.IndicesPrivileges[] indicesPrivileges, + NamedXContentRegistry xContentRegistry) { + if (indicesPrivileges != null) { + for (int i = 0; i < indicesPrivileges.length; i++) { + BytesReference query = indicesPrivileges[i].getQuery(); + try { + if (query != null) { + if (isTemplateQuery(query, xContentRegistry)) { + // skip template query, this requires runtime information like 'User' information. + continue; + } + + evaluateAndVerifyRoleQuery(query.utf8ToString(), xContentRegistry); + } + } catch (ParsingException | IllegalArgumentException | IOException e) { + throw new ElasticsearchParseException("failed to parse field 'query' for indices [" + + Strings.arrayToCommaDelimitedString(indicesPrivileges[i].getIndices()) + + "] at index privilege [" + i + "] of role descriptor", e); + } + } + } + } + + private static boolean isTemplateQuery(BytesReference query, NamedXContentRegistry xContentRegistry) throws IOException { + try (XContentParser parser = XContentType.JSON.xContent().createParser(xContentRegistry, + LoggingDeprecationHandler.INSTANCE, query.utf8ToString())) { + XContentParser.Token token = parser.nextToken(); + if (token != XContentParser.Token.START_OBJECT) { + throw new XContentParseException(parser.getTokenLocation(), "expected [" + XContentParser.Token.START_OBJECT + "] but " + + "found [" + token + "] instead"); + } + token = parser.nextToken(); + if (token != XContentParser.Token.FIELD_NAME) { + throw new XContentParseException(parser.getTokenLocation(), "expected [" + XContentParser.Token.FIELD_NAME + "] with " + + "value a query name or 'template' but found [" + token + "] instead"); + } + String fieldName = parser.currentName(); + if ("template".equals(fieldName)) { + return true; + } + } + + return false; + } + + /** + * Evaluates the query if it is a template and then validates the query by parsing + * and building the {@link QueryBuilder}. It also checks if the query type is + * supported in DLS role query. + * + * @param query {@link BytesReference} query field from the role + * @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 + * @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) { + if (query != null) { + String templateResult = SecurityQueryTemplateEvaluator.evaluateTemplate(query.utf8ToString(), scriptService, + user); + try { + return evaluateAndVerifyRoleQuery(templateResult, xContentRegistry); + } catch (ElasticsearchParseException | ParsingException | XContentParseException | IOException e) { + throw new ElasticsearchParseException("failed to parse field 'query' from the role descriptor", e); + } + } + return null; + } + + @Nullable + private static QueryBuilder evaluateAndVerifyRoleQuery(String query, NamedXContentRegistry xContentRegistry) throws IOException { + if (query != null) { + try (XContentParser parser = XContentFactory.xContent(query).createParser(xContentRegistry, + LoggingDeprecationHandler.INSTANCE, query)) { + QueryBuilder queryBuilder = AbstractQueryBuilder.parseInnerQueryBuilder(parser); + verifyRoleQuery(queryBuilder); + return queryBuilder; + } + } + return null; + } + + /** + * Checks whether the role query contains queries we know can't be used as DLS role query. + * + * @param queryBuilder {@link QueryBuilder} for given query + */ + // pkg protected for testing + static void verifyRoleQuery(QueryBuilder queryBuilder) { + if (queryBuilder instanceof TermsQueryBuilder) { + TermsQueryBuilder termsQueryBuilder = (TermsQueryBuilder) queryBuilder; + if (termsQueryBuilder.termsLookup() != null) { + throw new IllegalArgumentException("terms query with terms lookup isn't supported as part of a role query"); + } + } else if (queryBuilder instanceof GeoShapeQueryBuilder) { + GeoShapeQueryBuilder geoShapeQueryBuilder = (GeoShapeQueryBuilder) queryBuilder; + if (geoShapeQueryBuilder.shape() == null) { + throw new IllegalArgumentException("geoshape query referring to indexed shapes isn't supported as part of a role query"); + } + } else if (queryBuilder.getName().equals("percolate")) { + // actually only if percolate query is referring to an existing document then this is problematic, + // a normal percolate query does work. However we can't check that here as this query builder is inside + // another module. So we don't allow the entire percolate query. I don't think users would ever use + // a percolate query as role query, so this restriction shouldn't prohibit anyone from using dls. + throw new IllegalArgumentException("percolate query isn't supported as part of a role query"); + } else if (queryBuilder.getName().equals("has_child")) { + throw new IllegalArgumentException("has_child query isn't supported as part of a role query"); + } else if (queryBuilder.getName().equals("has_parent")) { + throw new IllegalArgumentException("has_parent query isn't supported as part of a role query"); + } else if (queryBuilder instanceof BoolQueryBuilder) { + BoolQueryBuilder boolQueryBuilder = (BoolQueryBuilder) queryBuilder; + List clauses = new ArrayList<>(); + clauses.addAll(boolQueryBuilder.filter()); + clauses.addAll(boolQueryBuilder.must()); + clauses.addAll(boolQueryBuilder.mustNot()); + clauses.addAll(boolQueryBuilder.should()); + for (QueryBuilder clause : clauses) { + verifyRoleQuery(clause); + } + } else if (queryBuilder instanceof ConstantScoreQueryBuilder) { + verifyRoleQuery(((ConstantScoreQueryBuilder) queryBuilder).innerQuery()); + } else if (queryBuilder instanceof FunctionScoreQueryBuilder) { + verifyRoleQuery(((FunctionScoreQueryBuilder) queryBuilder).query()); + } else if (queryBuilder instanceof BoostingQueryBuilder) { + verifyRoleQuery(((BoostingQueryBuilder) queryBuilder).negativeQuery()); + verifyRoleQuery(((BoostingQueryBuilder) queryBuilder).positiveQuery()); + } + } +} diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/support/SecurityQueryTemplateEvaluator.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/support/SecurityQueryTemplateEvaluator.java index 73a1d7fcde5..0fac1016342 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/support/SecurityQueryTemplateEvaluator.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/support/SecurityQueryTemplateEvaluator.java @@ -44,10 +44,8 @@ public final class SecurityQueryTemplateEvaluator { * @return resultant query string after compiling and executing the script. * If the source does not contain template then it will return the query * source without any modifications. - * @throws IOException thrown when there is any error parsing the query - * string. */ - public static String evaluateTemplate(final String querySource, final ScriptService scriptService, final User user) throws IOException { + public static String evaluateTemplate(final String querySource, final ScriptService scriptService, final User user) { // EMPTY is safe here because we never use namedObject try (XContentParser parser = XContentFactory.xContent(querySource).createParser(NamedXContentRegistry.EMPTY, LoggingDeprecationHandler.INSTANCE, querySource)) { @@ -76,6 +74,8 @@ public final class SecurityQueryTemplateEvaluator { } else { return querySource; } + } catch (IOException ioe) { + throw new ElasticsearchParseException("failed to parse query", ioe); } } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/accesscontrol/SecurityIndexReaderWrapperIntegrationTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/accesscontrol/SecurityIndexReaderWrapperIntegrationTests.java index ca49e4ae4a3..ca2b38318a0 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/accesscontrol/SecurityIndexReaderWrapperIntegrationTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/accesscontrol/SecurityIndexReaderWrapperIntegrationTests.java @@ -28,12 +28,10 @@ import org.elasticsearch.common.lucene.index.ElasticsearchDirectoryReader; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.util.BigArrays; import org.elasticsearch.common.util.concurrent.ThreadContext; -import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.index.IndexSettings; import org.elasticsearch.index.mapper.MapperService; import org.elasticsearch.index.query.ParsedQuery; import org.elasticsearch.index.query.QueryShardContext; -import org.elasticsearch.index.query.TermQueryBuilder; import org.elasticsearch.index.query.TermsQueryBuilder; import org.elasticsearch.index.shard.ShardId; import org.elasticsearch.license.XPackLicenseState; @@ -55,9 +53,7 @@ import static java.util.Collections.singleton; import static java.util.Collections.singletonMap; import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.equalTo; -import static org.mockito.Matchers.any; import static org.mockito.Matchers.anyString; -import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.when; @@ -76,9 +72,6 @@ public class SecurityIndexReaderWrapperIntegrationTests extends AbstractBuilderT final Authentication authentication = mock(Authentication.class); when(authentication.getUser()).thenReturn(mock(User.class)); threadContext.putTransient(AuthenticationField.AUTHENTICATION_KEY, authentication); - IndicesAccessControl.IndexAccessControl indexAccessControl = new IndicesAccessControl.IndexAccessControl(true, new - FieldPermissions(), - DocumentPermissions.filteredBy(singleton(new BytesArray("{\"match_all\" : {}}")))); IndexSettings indexSettings = IndexSettingsModule.newIndexSettings(shardId.getIndex(), Settings.EMPTY); Client client = mock(Client.class); when(client.settings()).thenReturn(Settings.EMPTY); @@ -90,14 +83,6 @@ public class SecurityIndexReaderWrapperIntegrationTests extends AbstractBuilderT DocumentSubsetBitsetCache bitsetCache = new DocumentSubsetBitsetCache(Settings.EMPTY); XPackLicenseState licenseState = mock(XPackLicenseState.class); when(licenseState.isDocumentAndFieldLevelSecurityAllowed()).thenReturn(true); - SecurityIndexReaderWrapper wrapper = new SecurityIndexReaderWrapper(s -> queryShardContext, - bitsetCache, threadContext, licenseState, scriptService) { - - @Override - protected IndicesAccessControl getIndicesAccessControl() { - return new IndicesAccessControl(true, singletonMap("_index", indexAccessControl)); - } - }; Directory directory = newDirectory(); IndexWriter iw = new IndexWriter( @@ -144,17 +129,32 @@ public class SecurityIndexReaderWrapperIntegrationTests extends AbstractBuilderT DirectoryReader directoryReader = ElasticsearchDirectoryReader.wrap(DirectoryReader.open(directory), shardId); for (int i = 0; i < numValues; i++) { + String termQuery = "{\"term\": {\"field\": \""+ values[i] + "\"} }"; + IndicesAccessControl.IndexAccessControl indexAccessControl = new IndicesAccessControl.IndexAccessControl(true, new + FieldPermissions(), + DocumentPermissions.filteredBy(singleton(new BytesArray(termQuery)))); + SecurityIndexReaderWrapper wrapper = new SecurityIndexReaderWrapper(s -> queryShardContext, + bitsetCache, threadContext, licenseState, scriptService) { + + @Override + protected IndicesAccessControl getIndicesAccessControl() { + return new IndicesAccessControl(true, singletonMap("_index", indexAccessControl)); + } + }; + ParsedQuery parsedQuery = new ParsedQuery(new TermQuery(new Term("field", values[i]))); - doReturn(new TermQueryBuilder("field", values[i])).when(queryShardContext).parseInnerQueryBuilder(any(XContentParser.class)); when(queryShardContext.toQuery(new TermsQueryBuilder("field", values[i]))).thenReturn(parsedQuery); + DirectoryReader wrappedDirectoryReader = wrapper.apply(directoryReader); IndexSearcher indexSearcher = new ContextIndexSearcher(wrappedDirectoryReader, IndexSearcher.getDefaultSimilarity(), IndexSearcher.getDefaultQueryCache(), IndexSearcher.getDefaultQueryCachingPolicy()); int expectedHitCount = valuesHitCount[i]; logger.info("Going to verify hit count with query [{}] with expected total hits [{}]", parsedQuery.query(), expectedHitCount); + TotalHitCountCollector countCollector = new TotalHitCountCollector(); indexSearcher.search(new MatchAllDocsQuery(), countCollector); + assertThat(countCollector.getTotalHits(), equalTo(expectedHitCount)); assertThat(wrappedDirectoryReader.numDocs(), equalTo(expectedHitCount)); } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/DocumentPermissionsTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/DocumentPermissionsTests.java index f8d1334df7e..37b6dd70e5d 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/DocumentPermissionsTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/DocumentPermissionsTests.java @@ -6,23 +6,14 @@ package org.elasticsearch.xpack.core.security.authz.permission; -import org.apache.lucene.search.join.ScoreMode; import org.elasticsearch.client.Client; import org.elasticsearch.common.bytes.BytesArray; import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.index.query.BoolQueryBuilder; -import org.elasticsearch.index.query.BoostingQueryBuilder; -import org.elasticsearch.index.query.ConstantScoreQueryBuilder; -import org.elasticsearch.index.query.GeoShapeQueryBuilder; -import org.elasticsearch.index.query.MatchAllQueryBuilder; import org.elasticsearch.index.query.QueryBuilder; import org.elasticsearch.index.query.QueryRewriteContext; import org.elasticsearch.index.query.TermsQueryBuilder; -import org.elasticsearch.index.query.functionscore.FunctionScoreQueryBuilder; import org.elasticsearch.indices.TermsLookup; -import org.elasticsearch.join.query.HasChildQueryBuilder; -import org.elasticsearch.join.query.HasParentQueryBuilder; import org.elasticsearch.test.ESTestCase; import java.io.IOException; @@ -68,44 +59,6 @@ public class DocumentPermissionsTests extends ESTestCase { assertThat(ae.getMessage(), containsString("nested scoping for document permissions is not permitted")); } - public void testVerifyRoleQuery() throws Exception { - QueryBuilder queryBuilder1 = new TermsQueryBuilder("field", "val1", "val2"); - DocumentPermissions.verifyRoleQuery(queryBuilder1); - - QueryBuilder queryBuilder2 = new TermsQueryBuilder("field", new TermsLookup("_index", "_type", "_id", "_path")); - Exception e = expectThrows(IllegalArgumentException.class, () -> DocumentPermissions.verifyRoleQuery(queryBuilder2)); - assertThat(e.getMessage(), equalTo("terms query with terms lookup isn't supported as part of a role query")); - - QueryBuilder queryBuilder3 = new GeoShapeQueryBuilder("field", "_id", "_type"); - e = expectThrows(IllegalArgumentException.class, () -> DocumentPermissions.verifyRoleQuery(queryBuilder3)); - assertThat(e.getMessage(), equalTo("geoshape query referring to indexed shapes isn't support as part of a role query")); - - QueryBuilder queryBuilder4 = new HasChildQueryBuilder("_type", new MatchAllQueryBuilder(), ScoreMode.None); - e = expectThrows(IllegalArgumentException.class, () -> DocumentPermissions.verifyRoleQuery(queryBuilder4)); - assertThat(e.getMessage(), equalTo("has_child query isn't support as part of a role query")); - - QueryBuilder queryBuilder5 = new HasParentQueryBuilder("_type", new MatchAllQueryBuilder(), false); - e = expectThrows(IllegalArgumentException.class, () -> DocumentPermissions.verifyRoleQuery(queryBuilder5)); - assertThat(e.getMessage(), equalTo("has_parent query isn't support as part of a role query")); - - QueryBuilder queryBuilder6 = new BoolQueryBuilder().must(new GeoShapeQueryBuilder("field", "_id", "_type")); - e = expectThrows(IllegalArgumentException.class, () -> DocumentPermissions.verifyRoleQuery(queryBuilder6)); - assertThat(e.getMessage(), equalTo("geoshape query referring to indexed shapes isn't support as part of a role query")); - - QueryBuilder queryBuilder7 = new ConstantScoreQueryBuilder(new GeoShapeQueryBuilder("field", "_id", "_type")); - e = expectThrows(IllegalArgumentException.class, () -> DocumentPermissions.verifyRoleQuery(queryBuilder7)); - assertThat(e.getMessage(), equalTo("geoshape query referring to indexed shapes isn't support as part of a role query")); - - QueryBuilder queryBuilder8 = new FunctionScoreQueryBuilder(new GeoShapeQueryBuilder("field", "_id", "_type")); - e = expectThrows(IllegalArgumentException.class, () -> DocumentPermissions.verifyRoleQuery(queryBuilder8)); - assertThat(e.getMessage(), equalTo("geoshape query referring to indexed shapes isn't support as part of a role query")); - - QueryBuilder queryBuilder9 = new BoostingQueryBuilder(new GeoShapeQueryBuilder("field", "_id", "_type"), - new MatchAllQueryBuilder()); - e = expectThrows(IllegalArgumentException.class, () -> DocumentPermissions.verifyRoleQuery(queryBuilder9)); - assertThat(e.getMessage(), equalTo("geoshape query referring to indexed shapes isn't support as part of a role query")); - } - public void testFailIfQueryUsesClient() throws Exception { Client client = mock(Client.class); when(client.settings()).thenReturn(Settings.EMPTY); diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/support/DLSRoleQueryValidatorTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/support/DLSRoleQueryValidatorTests.java new file mode 100644 index 00000000000..b441c40340d --- /dev/null +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/support/DLSRoleQueryValidatorTests.java @@ -0,0 +1,63 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ +package org.elasticsearch.xpack.core.security.authz.support; + +import org.apache.lucene.search.join.ScoreMode; +import org.elasticsearch.index.query.BoolQueryBuilder; +import org.elasticsearch.index.query.BoostingQueryBuilder; +import org.elasticsearch.index.query.ConstantScoreQueryBuilder; +import org.elasticsearch.index.query.GeoShapeQueryBuilder; +import org.elasticsearch.index.query.MatchAllQueryBuilder; +import org.elasticsearch.index.query.QueryBuilder; +import org.elasticsearch.index.query.TermsQueryBuilder; +import org.elasticsearch.index.query.functionscore.FunctionScoreQueryBuilder; +import org.elasticsearch.indices.TermsLookup; +import org.elasticsearch.join.query.HasChildQueryBuilder; +import org.elasticsearch.join.query.HasParentQueryBuilder; +import org.elasticsearch.test.ESTestCase; + +import static org.hamcrest.Matchers.equalTo; + +public class DLSRoleQueryValidatorTests extends ESTestCase { + + public void testVerifyRoleQuery() throws Exception { + QueryBuilder queryBuilder1 = new TermsQueryBuilder("field", "val1", "val2"); + DLSRoleQueryValidator.verifyRoleQuery(queryBuilder1); + + QueryBuilder queryBuilder2 = new TermsQueryBuilder("field", new TermsLookup("_index", "_id", "_path")); + Exception e = expectThrows(IllegalArgumentException.class, () -> DLSRoleQueryValidator.verifyRoleQuery(queryBuilder2)); + assertThat(e.getMessage(), equalTo("terms query with terms lookup isn't supported as part of a role query")); + + QueryBuilder queryBuilder3 = new GeoShapeQueryBuilder("field", "_id", "_type"); + e = expectThrows(IllegalArgumentException.class, () -> DLSRoleQueryValidator.verifyRoleQuery(queryBuilder3)); + assertThat(e.getMessage(), equalTo("geoshape query referring to indexed shapes isn't supported as part of a role query")); + + QueryBuilder queryBuilder4 = new HasChildQueryBuilder("_type", new MatchAllQueryBuilder(), ScoreMode.None); + e = expectThrows(IllegalArgumentException.class, () -> DLSRoleQueryValidator.verifyRoleQuery(queryBuilder4)); + assertThat(e.getMessage(), equalTo("has_child query isn't supported as part of a role query")); + + QueryBuilder queryBuilder5 = new HasParentQueryBuilder("_type", new MatchAllQueryBuilder(), false); + e = expectThrows(IllegalArgumentException.class, () -> DLSRoleQueryValidator.verifyRoleQuery(queryBuilder5)); + assertThat(e.getMessage(), equalTo("has_parent query isn't supported as part of a role query")); + + QueryBuilder queryBuilder6 = new BoolQueryBuilder().must(new GeoShapeQueryBuilder("field", "_id", "_type")); + e = expectThrows(IllegalArgumentException.class, () -> DLSRoleQueryValidator.verifyRoleQuery(queryBuilder6)); + assertThat(e.getMessage(), equalTo("geoshape query referring to indexed shapes isn't supported as part of a role query")); + + QueryBuilder queryBuilder7 = new ConstantScoreQueryBuilder(new GeoShapeQueryBuilder("field", "_id", "_type")); + e = expectThrows(IllegalArgumentException.class, () -> DLSRoleQueryValidator.verifyRoleQuery(queryBuilder7)); + assertThat(e.getMessage(), equalTo("geoshape query referring to indexed shapes isn't supported as part of a role query")); + + QueryBuilder queryBuilder8 = new FunctionScoreQueryBuilder(new GeoShapeQueryBuilder("field", "_id", "_type")); + e = expectThrows(IllegalArgumentException.class, () -> DLSRoleQueryValidator.verifyRoleQuery(queryBuilder8)); + assertThat(e.getMessage(), equalTo("geoshape query referring to indexed shapes isn't supported as part of a role query")); + + QueryBuilder queryBuilder9 = new BoostingQueryBuilder(new GeoShapeQueryBuilder("field", "_id", "_type"), + new MatchAllQueryBuilder()); + e = expectThrows(IllegalArgumentException.class, () -> DLSRoleQueryValidator.verifyRoleQuery(queryBuilder9)); + assertThat(e.getMessage(), equalTo("geoshape query referring to indexed shapes isn't supported as part of a role query")); + } +} diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java index 46419739f6e..7bd57c1b02a 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java @@ -388,7 +388,7 @@ public class Security extends Plugin implements ActionPlugin, IngestPlugin, Netw NamedXContentRegistry xContentRegistry, Environment environment, NodeEnvironment nodeEnvironment, NamedWriteableRegistry namedWriteableRegistry) { try { - return createComponents(client, threadPool, clusterService, resourceWatcherService, scriptService); + return createComponents(client, threadPool, clusterService, resourceWatcherService, scriptService, xContentRegistry); } catch (final Exception e) { throw new IllegalStateException("security initialization failed", e); } @@ -396,7 +396,8 @@ public class Security extends Plugin implements ActionPlugin, IngestPlugin, Netw // pkg private for testing - tests want to pass in their set of extensions hence we are not using the extension service directly Collection createComponents(Client client, ThreadPool threadPool, ClusterService clusterService, - ResourceWatcherService resourceWatcherService, ScriptService scriptService) throws Exception { + ResourceWatcherService resourceWatcherService, ScriptService scriptService, + NamedXContentRegistry xContentRegistry) throws Exception { if (enabled == false) { return Collections.emptyList(); } @@ -451,7 +452,8 @@ public class Security extends Plugin implements ActionPlugin, IngestPlugin, Netw dlsBitsetCache.set(new DocumentSubsetBitsetCache(settings)); final FieldPermissionsCache fieldPermissionsCache = new FieldPermissionsCache(settings); - final FileRolesStore fileRolesStore = new FileRolesStore(settings, env, resourceWatcherService, getLicenseState()); + final FileRolesStore fileRolesStore = new FileRolesStore(settings, env, resourceWatcherService, getLicenseState(), + xContentRegistry); final NativeRolesStore nativeRolesStore = new NativeRolesStore(settings, client, getLicenseState(), securityIndex.get()); final ReservedRolesStore reservedRolesStore = new ReservedRolesStore(); List, ActionListener>> rolesProviders = new ArrayList<>(); diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/TransportCreateApiKeyAction.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/TransportCreateApiKeyAction.java index 09612c5e01f..72a92516e59 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/TransportCreateApiKeyAction.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/TransportCreateApiKeyAction.java @@ -6,11 +6,13 @@ package org.elasticsearch.xpack.security.action; +import org.elasticsearch.ElasticsearchException; import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.support.ActionFilters; import org.elasticsearch.action.support.HandledTransportAction; import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.io.stream.Writeable; +import org.elasticsearch.common.xcontent.NamedXContentRegistry; import org.elasticsearch.tasks.Task; import org.elasticsearch.transport.TransportService; import org.elasticsearch.xpack.core.security.SecurityContext; @@ -18,6 +20,8 @@ import org.elasticsearch.xpack.core.security.action.CreateApiKeyAction; import org.elasticsearch.xpack.core.security.action.CreateApiKeyRequest; import org.elasticsearch.xpack.core.security.action.CreateApiKeyResponse; import org.elasticsearch.xpack.core.security.authc.Authentication; +import org.elasticsearch.xpack.core.security.authz.RoleDescriptor; +import org.elasticsearch.xpack.core.security.authz.support.DLSRoleQueryValidator; import org.elasticsearch.xpack.security.authc.ApiKeyService; import org.elasticsearch.xpack.security.authz.store.CompositeRolesStore; @@ -32,14 +36,16 @@ public final class TransportCreateApiKeyAction extends HandledTransportAction) CreateApiKeyRequest::new); this.apiKeyService = apiKeyService; this.securityContext = context; this.rolesStore = rolesStore; + this.xContentRegistry = xContentRegistry; } @Override @@ -49,7 +55,17 @@ public final class TransportCreateApiKeyAction extends HandledTransportAction(Arrays.asList(authentication.getUser().roles())), - ActionListener.wrap(roleDescriptors -> apiKeyService.createApiKey(authentication, request, roleDescriptors, listener), + ActionListener.wrap(roleDescriptors -> { + for (RoleDescriptor rd : roleDescriptors) { + try { + DLSRoleQueryValidator.validateQueryField(rd.getIndicesPrivileges(), xContentRegistry); + } catch (ElasticsearchException | IllegalArgumentException e) { + listener.onFailure(e); + return; + } + } + apiKeyService.createApiKey(authentication, request, roleDescriptors, listener); + }, listener::onFailure)); } } 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 c0a91bcdb02..300c8c835ff 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 @@ -5,26 +5,32 @@ */ 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; import org.elasticsearch.common.inject.Inject; +import org.elasticsearch.common.xcontent.NamedXContentRegistry; import org.elasticsearch.tasks.Task; import org.elasticsearch.transport.TransportService; import org.elasticsearch.xpack.core.security.action.role.PutRoleAction; 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 { private final NativeRolesStore rolesStore; + private final NamedXContentRegistry xContentRegistry; @Inject - public TransportPutRoleAction(ActionFilters actionFilters, NativeRolesStore rolesStore, TransportService transportService) { + public TransportPutRoleAction(ActionFilters actionFilters, NativeRolesStore rolesStore, TransportService transportService, + NamedXContentRegistry xContentRegistry) { super(PutRoleAction.NAME, transportService, actionFilters, PutRoleRequest::new); this.rolesStore = rolesStore; + this.xContentRegistry = xContentRegistry; } @Override @@ -35,6 +41,13 @@ public class TransportPutRoleAction extends HandledTransportAction() { @Override public void onResponse(Boolean created) { diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/user/TransportHasPrivilegesAction.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/user/TransportHasPrivilegesAction.java index ae400172bf1..7401463bf57 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/user/TransportHasPrivilegesAction.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/user/TransportHasPrivilegesAction.java @@ -8,10 +8,13 @@ package org.elasticsearch.xpack.security.action.user; import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.support.ActionFilters; import org.elasticsearch.action.support.HandledTransportAction; +import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.inject.Inject; +import org.elasticsearch.common.xcontent.NamedXContentRegistry; import org.elasticsearch.tasks.Task; import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.transport.TransportService; +import org.elasticsearch.xpack.core.security.SecurityContext; import org.elasticsearch.xpack.core.security.action.user.HasPrivilegesAction; import org.elasticsearch.xpack.core.security.action.user.HasPrivilegesRequest; import org.elasticsearch.xpack.core.security.action.user.HasPrivilegesResponse; @@ -36,15 +39,20 @@ public class TransportHasPrivilegesAction extends HandledTransportAction authorizationService.checkPrivileges(authentication, request, applicationPrivilegeDescriptors, listener), listener::onFailure)); diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/esnative/ESNativeRealmMigrateTool.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/esnative/ESNativeRealmMigrateTool.java index 0fbe54d7c10..dc77385bc1c 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/esnative/ESNativeRealmMigrateTool.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/esnative/ESNativeRealmMigrateTool.java @@ -323,7 +323,7 @@ public class ESNativeRealmMigrateTool extends LoggingAwareMultiCommand { } terminal.println("importing roles from [" + rolesFile + "]..."); Logger logger = getTerminalLogger(terminal); - Map roles = FileRolesStore.parseRoleDescriptors(rolesFile, logger, true, Settings.EMPTY); + Map roles = FileRolesStore.parseRoleDescriptors(rolesFile, logger, true, Settings.EMPTY, null); Set existingRoles; try { existingRoles = getRolesThatExist(terminal, env.settings(), env, options); 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 a2be72dc6d6..5a2c5456534 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 @@ -9,6 +9,7 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.message.ParameterizedMessage; import org.apache.logging.log4j.util.Supplier; +import org.elasticsearch.ElasticsearchException; import org.elasticsearch.ElasticsearchParseException; import org.elasticsearch.action.ActionListener; import org.elasticsearch.common.Nullable; @@ -29,6 +30,7 @@ import org.elasticsearch.xpack.core.security.authz.RoleDescriptor; import org.elasticsearch.xpack.core.security.authz.RoleDescriptor.IndicesPrivileges; import org.elasticsearch.xpack.core.security.authz.store.ReservedRolesStore; import org.elasticsearch.xpack.core.security.authz.store.RoleRetrievalResult; +import org.elasticsearch.xpack.core.security.authz.support.DLSRoleQueryValidator; import org.elasticsearch.xpack.core.security.support.NoOpLogger; import org.elasticsearch.xpack.core.security.support.Validation; @@ -61,27 +63,30 @@ public class FileRolesStore implements BiConsumer, ActionListener>> listeners = new ArrayList<>(); private volatile Map permissions; - public FileRolesStore(Settings settings, Environment env, ResourceWatcherService watcherService, XPackLicenseState licenseState) + public FileRolesStore(Settings settings, Environment env, ResourceWatcherService watcherService, XPackLicenseState licenseState, + NamedXContentRegistry xContentRegistry) throws IOException { - this(settings, env, watcherService, null, licenseState); + this(settings, env, watcherService, null, licenseState, xContentRegistry); } FileRolesStore(Settings settings, Environment env, ResourceWatcherService watcherService, Consumer> listener, - XPackLicenseState licenseState) throws IOException { + XPackLicenseState licenseState, NamedXContentRegistry xContentRegistry) throws IOException { this.settings = settings; this.file = resolveFile(env); if (listener != null) { listeners.add(listener); } this.licenseState = licenseState; + this.xContentRegistry = xContentRegistry; FileWatcher watcher = new FileWatcher(file.getParent()); watcher.addListener(new FileListener()); watcherService.add(watcher, ResourceWatcherService.Frequency.HIGH); - permissions = parseFile(file, logger, settings, licenseState); + permissions = parseFile(file, logger, settings, licenseState, xContentRegistry); } @@ -150,15 +155,17 @@ public class FileRolesStore implements BiConsumer, ActionListener parseFileForRoleNames(Path path, Logger logger) { - return parseRoleDescriptors(path, logger, false, Settings.EMPTY).keySet(); + // EMPTY is safe here because we never use namedObject as we are just parsing role names + return parseRoleDescriptors(path, logger, false, Settings.EMPTY, NamedXContentRegistry.EMPTY).keySet(); } - public static Map parseFile(Path path, Logger logger, Settings settings, XPackLicenseState licenseState) { - return parseFile(path, logger, true, settings, licenseState); + public static Map parseFile(Path path, Logger logger, Settings settings, XPackLicenseState licenseState, + NamedXContentRegistry xContentRegistry) { + return parseFile(path, logger, true, settings, licenseState, xContentRegistry); } - public static Map parseFile(Path path, Logger logger, boolean resolvePermission, - Settings settings, XPackLicenseState licenseState) { + public static Map parseFile(Path path, Logger logger, boolean resolvePermission, Settings settings, + XPackLicenseState licenseState, NamedXContentRegistry xContentRegistry) { if (logger == null) { logger = NoOpLogger.INSTANCE; } @@ -170,7 +177,7 @@ public class FileRolesStore implements BiConsumer, ActionListener roleSegments = roleSegments(path); final boolean flsDlsLicensed = licenseState.isDocumentAndFieldLevelSecurityAllowed(); for (String segment : roleSegments) { - RoleDescriptor descriptor = parseRoleDescriptor(segment, path, logger, resolvePermission, settings); + RoleDescriptor descriptor = parseRoleDescriptor(segment, path, logger, resolvePermission, settings, xContentRegistry); if (descriptor != null) { if (ReservedRolesStore.isReserved(descriptor.getName())) { logger.warn("role [{}] is reserved. the relevant role definition in the mapping file will be ignored", @@ -202,7 +209,8 @@ public class FileRolesStore implements BiConsumer, ActionListener parseRoleDescriptors(Path path, Logger logger, boolean resolvePermission, Settings settings) { + public static Map parseRoleDescriptors(Path path, Logger logger, boolean resolvePermission, Settings settings, + NamedXContentRegistry xContentRegistry) { if (logger == null) { logger = NoOpLogger.INSTANCE; } @@ -213,7 +221,7 @@ public class FileRolesStore implements BiConsumer, ActionListener roleSegments = roleSegments(path); for (String segment : roleSegments) { - RoleDescriptor rd = parseRoleDescriptor(segment, path, logger, resolvePermission, settings); + RoleDescriptor rd = parseRoleDescriptor(segment, path, logger, resolvePermission, settings, xContentRegistry); if (rd != null) { roles.put(rd.getName(), rd); } @@ -231,12 +239,12 @@ public class FileRolesStore implements BiConsumer, ActionListener, ActionListener, ActionListener) () -> new ParameterizedMessage( + "invalid role definition [{}] in roles file [{}]. failed to validate query field. skipping role...", roleName, + path.toAbsolutePath()), e); + return null; + } } } return descriptor; @@ -350,7 +367,7 @@ public class FileRolesStore implements BiConsumer, ActionListener previousPermissions = permissions; try { - permissions = parseFile(file, logger, settings, licenseState); + permissions = parseFile(file, logger, settings, licenseState, xContentRegistry); logger.info("updated roles (roles file [{}] {})", file.toAbsolutePath(), Files.exists(file) ? "changed" : "removed"); } catch (Exception e) { diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/SecurityTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/SecurityTests.java index 92130ca5f4e..4a1cf946676 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/SecurityTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/SecurityTests.java @@ -128,7 +128,8 @@ public class SecurityTests extends ESTestCase { Client client = mock(Client.class); when(client.threadPool()).thenReturn(threadPool); when(client.settings()).thenReturn(settings); - return security.createComponents(client, threadPool, clusterService, mock(ResourceWatcherService.class), mock(ScriptService.class)); + return security.createComponents(client, threadPool, clusterService, mock(ResourceWatcherService.class), mock(ScriptService.class), + xContentRegistry()); } private static T findComponent(Class type, Collection components) { diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/role/PutRoleBuilderTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/role/PutRoleBuilderTests.java index ba305e15ed7..33fe3259b3c 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/role/PutRoleBuilderTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/role/PutRoleBuilderTests.java @@ -32,4 +32,4 @@ public class PutRoleBuilderTests extends ESTestCase { "[role1], use [\"field_security\": {\"grant\":[...],\"except\":[...]}] instead")); } } -} \ No newline at end of file +} diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/role/TransportPutRoleActionTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/role/TransportPutRoleActionTests.java index 3cbb7782688..f7f3647f798 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/role/TransportPutRoleActionTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/role/TransportPutRoleActionTests.java @@ -5,10 +5,21 @@ */ package org.elasticsearch.xpack.security.action.role; +import org.elasticsearch.ElasticsearchParseException; import org.elasticsearch.ElasticsearchSecurityException; import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.support.ActionFilters; +import org.elasticsearch.common.ParseField; +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.NamedXContentRegistry; +import org.elasticsearch.index.query.MatchAllQueryBuilder; +import org.elasticsearch.index.query.QueryBuilder; +import org.elasticsearch.index.query.TermQueryBuilder; +import org.elasticsearch.join.query.HasChildQueryBuilder; +import org.elasticsearch.join.query.HasParentQueryBuilder; import org.elasticsearch.tasks.Task; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.transport.Transport; @@ -22,6 +33,7 @@ import org.mockito.invocation.InvocationOnMock; import org.mockito.stubbing.Answer; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.concurrent.atomic.AtomicReference; @@ -41,12 +53,26 @@ import static org.mockito.Mockito.verifyZeroInteractions; public class TransportPutRoleActionTests extends ESTestCase { + @Override + protected NamedXContentRegistry xContentRegistry() { + return new NamedXContentRegistry(Arrays.asList( + new NamedXContentRegistry.Entry(QueryBuilder.class, new ParseField(MatchAllQueryBuilder.NAME), + (p, c) -> MatchAllQueryBuilder.fromXContent(p)), + new NamedXContentRegistry.Entry(QueryBuilder.class, new ParseField(HasChildQueryBuilder.NAME), + (p, c) -> HasChildQueryBuilder.fromXContent(p)), + new NamedXContentRegistry.Entry(QueryBuilder.class, new ParseField(HasParentQueryBuilder.NAME), + (p, c) -> HasParentQueryBuilder.fromXContent(p)), + new NamedXContentRegistry.Entry(QueryBuilder.class, new ParseField(TermQueryBuilder.NAME), + (p, c) -> TermQueryBuilder.fromXContent(p)))); + } + public void testReservedRole() { final String roleName = randomFrom(new ArrayList<>(ReservedRolesStore.names())); NativeRolesStore rolesStore = mock(NativeRolesStore.class); TransportService transportService = new TransportService(Settings.EMPTY, mock(Transport.class), null, TransportService.NOOP_TRANSPORT_INTERCEPTOR, x -> null, null, Collections.emptySet()); - TransportPutRoleAction action = new TransportPutRoleAction(mock(ActionFilters.class), rolesStore, transportService); + TransportPutRoleAction action = new TransportPutRoleAction(mock(ActionFilters.class), rolesStore, transportService, + xContentRegistry()); PutRoleRequest request = new PutRoleRequest(); request.name(roleName); @@ -76,7 +102,8 @@ public class TransportPutRoleActionTests extends ESTestCase { NativeRolesStore rolesStore = mock(NativeRolesStore.class); TransportService transportService = new TransportService(Settings.EMPTY, mock(Transport.class), null, TransportService.NOOP_TRANSPORT_INTERCEPTOR, x -> null, null, Collections.emptySet()); - TransportPutRoleAction action = new TransportPutRoleAction(mock(ActionFilters.class), rolesStore, transportService); + TransportPutRoleAction action = new TransportPutRoleAction(mock(ActionFilters.class), rolesStore, transportService, + xContentRegistry()); final boolean created = randomBoolean(); PutRoleRequest request = new PutRoleRequest(); @@ -119,7 +146,8 @@ public class TransportPutRoleActionTests extends ESTestCase { NativeRolesStore rolesStore = mock(NativeRolesStore.class); TransportService transportService = new TransportService(Settings.EMPTY, mock(Transport.class), null, TransportService.NOOP_TRANSPORT_INTERCEPTOR, x -> null, null, Collections.emptySet()); - TransportPutRoleAction action = new TransportPutRoleAction(mock(ActionFilters.class), rolesStore, transportService); + TransportPutRoleAction action = new TransportPutRoleAction(mock(ActionFilters.class), rolesStore, transportService, + xContentRegistry()); PutRoleRequest request = new PutRoleRequest(); request.name(roleName); @@ -154,4 +182,78 @@ public class TransportPutRoleActionTests extends ESTestCase { assertThat(throwableRef.get(), is(sameInstance(e))); verify(rolesStore, times(1)).putRole(eq(request), any(RoleDescriptor.class), any(ActionListener.class)); } + + public void testCreationOfRoleWithMalformedQueryJsonFails() { + NativeRolesStore rolesStore = mock(NativeRolesStore.class); + TransportService transportService = new TransportService(Settings.EMPTY, mock(Transport.class), null, + TransportService.NOOP_TRANSPORT_INTERCEPTOR, x -> null, null, Collections.emptySet()); + TransportPutRoleAction action = new TransportPutRoleAction(mock(ActionFilters.class), rolesStore, transportService, + xContentRegistry()); + 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()); + + final AtomicReference throwableRef = new AtomicReference<>(); + final AtomicReference responseRef = new AtomicReference<>(); + action.doExecute(mock(Task.class), request, new ActionListener() { + @Override + public void onResponse(PutRoleResponse response) { + responseRef.set(response); + } + + @Override + public void onFailure(Exception e) { + throwableRef.set(e); + } + }); + + assertThat(responseRef.get(), is(nullValue())); + assertThat(throwableRef.get(), is(notNullValue())); + 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 testCreationOfRoleWithUnsupportedQueryFails() throws Exception { + NativeRolesStore rolesStore = mock(NativeRolesStore.class); + TransportService transportService = new TransportService(Settings.EMPTY, mock(Transport.class), null, + TransportService.NOOP_TRANSPORT_INTERCEPTOR, x -> null, null, Collections.emptySet()); + TransportPutRoleAction action = new TransportPutRoleAction(mock(ActionFilters.class), rolesStore, transportService, + xContentRegistry()); + PutRoleRequest request = new PutRoleRequest(); + request.name("test"); + String hasChildQuery = "{ \"has_child\": { \"type\": \"child\", \"query\": { \"match_all\": {} } } }"; + String hasParentQuery = "{ \"has_parent\": { \"parent_type\": \"parent\", \"query\": { \"match_all\": {} } } }"; + BytesReference query = new BytesArray(randomFrom(hasChildQuery, hasParentQuery)); + request.addIndex(new String[]{"idx1"}, new String[]{"read"}, null, null, query, randomBoolean()); + + final AtomicReference throwableRef = new AtomicReference<>(); + final AtomicReference responseRef = new AtomicReference<>(); + action.doExecute(mock(Task.class), request, new ActionListener() { + @Override + public void onResponse(PutRoleResponse response) { + responseRef.set(response); + } + + @Override + public void onFailure(Exception e) { + throwableRef.set(e); + } + }); + + assertThat(responseRef.get(), is(nullValue())); + assertThat(throwableRef.get(), is(notNullValue())); + 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")); + } } diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/user/HasPrivilegesRequestBuilderTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/user/HasPrivilegesRequestBuilderTests.java index 0b9de2da332..612437a4b99 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/user/HasPrivilegesRequestBuilderTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/user/HasPrivilegesRequestBuilderTests.java @@ -116,4 +116,4 @@ public class HasPrivilegesRequestBuilderTests extends ESTestCase { ); assertThat(parseException.getMessage(), containsString("[cluster,index,applications] are missing")); } -} \ No newline at end of file +} diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/user/TransportHasPrivilegesActionTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/user/TransportHasPrivilegesActionTests.java new file mode 100644 index 00000000000..7cd2ed7a2f7 --- /dev/null +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/user/TransportHasPrivilegesActionTests.java @@ -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; + * you may not use this file except in compliance with the Elastic License. + */ +package org.elasticsearch.xpack.security.action.user; + +import org.elasticsearch.action.support.ActionFilters; +import org.elasticsearch.action.support.PlainActionFuture; +import org.elasticsearch.common.bytes.BytesArray; +import org.elasticsearch.common.util.concurrent.ThreadContext; +import org.elasticsearch.tasks.Task; +import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.threadpool.TestThreadPool; +import org.elasticsearch.threadpool.ThreadPool; +import org.elasticsearch.transport.TransportService; +import org.elasticsearch.xpack.core.security.SecurityContext; +import org.elasticsearch.xpack.core.security.action.user.HasPrivilegesRequest; +import org.elasticsearch.xpack.core.security.action.user.HasPrivilegesResponse; +import org.elasticsearch.xpack.core.security.authc.Authentication; +import org.elasticsearch.xpack.core.security.authc.AuthenticationField; +import org.elasticsearch.xpack.core.security.authz.RoleDescriptor; +import org.elasticsearch.xpack.core.security.user.User; +import org.elasticsearch.xpack.security.authz.AuthorizationService; +import org.elasticsearch.xpack.security.authz.store.NativePrivilegeStore; +import org.junit.After; +import org.junit.Before; + +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.notNullValue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +public class TransportHasPrivilegesActionTests extends ESTestCase { + private ThreadPool threadPool; + + @Before + public void createThreadPool() { + threadPool = new TestThreadPool("has privileges action tests"); + } + + @After + public void stopThreadPool() { + terminate(threadPool); + } + + public void testHasPrivilegesRequestDoesNotAllowDLSRoleQueryBasedIndicesPrivileges() { + final ThreadContext threadContext = threadPool.getThreadContext(); + final SecurityContext context = mock(SecurityContext.class); + final User user = new User("user-1", "superuser"); + final Authentication authentication = new Authentication(user, + new Authentication.RealmRef("native", "default_native", "node1"), null); + when(context.getAuthentication()).thenReturn(authentication); + threadContext.putTransient(AuthenticationField.AUTHENTICATION_KEY, authentication); + final TransportHasPrivilegesAction transportHasPrivilegesAction = new TransportHasPrivilegesAction(threadPool, + mock(TransportService.class), mock(ActionFilters.class), mock(AuthorizationService.class), mock(NativePrivilegeStore.class), + context, xContentRegistry()); + + final HasPrivilegesRequest request = new HasPrivilegesRequest(); + final RoleDescriptor.IndicesPrivileges[] indicesPrivileges = new RoleDescriptor.IndicesPrivileges[randomIntBetween(1, 5)]; + for (int i = 0; i < indicesPrivileges.length; i++) { + indicesPrivileges[i] = RoleDescriptor.IndicesPrivileges.builder() + .privileges(randomFrom("read", "write")) + .indices(randomAlphaOfLengthBetween(2, 8)) + .query(new BytesArray(randomAlphaOfLength(5))) + .build(); + } + request.indexPrivileges(indicesPrivileges); + request.username("user-1"); + + final PlainActionFuture listener = new PlainActionFuture<>(); + transportHasPrivilegesAction.doExecute(mock(Task.class), request, listener); + + final IllegalArgumentException ile = expectThrows(IllegalArgumentException.class, () -> listener.actionGet()); + assertThat(ile, notNullValue()); + assertThat(ile.getMessage(), containsString("users may only check the index privileges without any DLS role query")); + } +} diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/esnative/ESNativeMigrateToolTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/esnative/ESNativeMigrateToolTests.java index a73fc93f32e..2e5e87d3077 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/esnative/ESNativeMigrateToolTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/esnative/ESNativeMigrateToolTests.java @@ -131,7 +131,7 @@ public class ESNativeMigrateToolTests extends NativeRealmIntegTestCase { .cluster("all", "none") .runAs("root", "nobody") .addIndices(new String[] { "index" }, new String[] { "read" }, new String[] { "body", "title" }, null, - new BytesArray("{\"query\": {\"match_all\": {}}}"), randomBoolean()) + new BytesArray("{\"match_all\": {}}"), randomBoolean()) .get(); addedRoles.add(rname); } diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/esnative/NativeRealmIntegTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/esnative/NativeRealmIntegTests.java index 89a82c5e51b..3bc220d6a68 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/esnative/NativeRealmIntegTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/esnative/NativeRealmIntegTests.java @@ -198,7 +198,7 @@ public class NativeRealmIntegTests extends NativeRealmIntegTestCase { .cluster("all", "none") .runAs("root", "nobody") .addIndices(new String[]{"index"}, new String[]{"read"}, new String[]{"body", "title"}, null, - new BytesArray("{\"query\": {\"match_all\": {}}}"), randomBoolean()) + new BytesArray("{\"match_all\": {}}"), randomBoolean()) .metadata(metadata) .get(); logger.error("--> waiting for .security index"); @@ -215,13 +215,13 @@ public class NativeRealmIntegTests extends NativeRealmIntegTestCase { .cluster("all", "none") .runAs("root", "nobody") .addIndices(new String[]{"index"}, new String[]{"read"}, new String[]{"body", "title"}, null, - new BytesArray("{\"query\": {\"match_all\": {}}}"), randomBoolean()) + new BytesArray("{\"match_all\": {}}"), randomBoolean()) .get(); c.preparePutRole("test_role3") .cluster("all", "none") .runAs("root", "nobody") .addIndices(new String[]{"index"}, new String[]{"read"}, new String[]{"body", "title"}, null, - new BytesArray("{\"query\": {\"match_all\": {}}}"), randomBoolean()) + new BytesArray("{\"match_all\": {}}"), randomBoolean()) .get(); logger.info("--> retrieving all roles"); diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/RoleDescriptorTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/RoleDescriptorTests.java index 37788d82403..765d5e83a76 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/RoleDescriptorTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/RoleDescriptorTests.java @@ -61,7 +61,7 @@ public class RoleDescriptorTests extends ESTestCase { .indices("i1", "i2") .privileges("read") .grantedFields("body", "title") - .query("{\"query\": {\"match_all\": {}}}") + .query("{\"match_all\": {}}") .build() }; final RoleDescriptor.ApplicationResourcePrivileges[] applicationPrivileges = { @@ -82,7 +82,7 @@ public class RoleDescriptorTests extends ESTestCase { assertThat(descriptor.toString(), is("Role[name=test, cluster=[all,none]" + ", global=[{APPLICATION:manage:applications=app01,app02}]" + ", indicesPrivileges=[IndicesPrivileges[indices=[i1,i2], allowRestrictedIndices=[false], privileges=[read]" + - ", field_security=[grant=[body,title], except=null], query={\"query\": {\"match_all\": {}}}],]" + + ", field_security=[grant=[body,title], except=null], query={\"match_all\": {}}],]" + ", applicationPrivileges=[ApplicationResourcePrivileges[application=my_app, privileges=[read,write], resources=[*]],]" + ", runAs=[sudo], metadata=[{}]]")); } @@ -94,7 +94,7 @@ public class RoleDescriptorTests extends ESTestCase { .privileges("read") .grantedFields("body", "title") .allowRestrictedIndices(randomBoolean()) - .query("{\"query\": {\"match_all\": {}}}") + .query("{\"match_all\": {}}") .build() }; final RoleDescriptor.ApplicationResourcePrivileges[] applicationPrivileges = { @@ -136,7 +136,7 @@ public class RoleDescriptorTests extends ESTestCase { "\"p2\"]}, {\"names\": \"idx2\", \"allow_restricted_indices\": true, \"privileges\": [\"p3\"], \"field_security\": " + "{\"grant\": [\"f1\", \"f2\"]}}, {\"names\": " + "\"idx2\", \"allow_restricted_indices\": false," + - "\"privileges\": [\"p3\"], \"field_security\": {\"grant\": [\"f1\", \"f2\"]}, \"query\": \"{\\\"match_all\\\": {}}\"}]}"; + "\"privileges\": [\"p3\"], \"field_security\": {\"grant\": [\"f1\", \"f2\"]}, \"query\": {\"match_all\": {}} }]}"; rd = RoleDescriptor.parse("test", new BytesArray(q), false, XContentType.JSON); assertEquals("test", rd.getName()); assertArrayEquals(new String[] { "a", "b" }, rd.getClusterPrivileges()); @@ -261,6 +261,18 @@ public class RoleDescriptorTests extends ESTestCase { assertNull(rd.getIndicesPrivileges()[0].getQuery()); } + public void testParseNullQuery() throws Exception { + String json = "{\"cluster\":[\"a\", \"b\"], \"run_as\": [\"m\", \"n\"], \"index\": [{\"names\": [\"idx1\",\"idx2\"], " + + "\"privileges\": [\"p1\", \"p2\"], \"query\": null}]}"; + RoleDescriptor rd = RoleDescriptor.parse("test", new BytesArray(json), false, XContentType.JSON); + assertEquals("test", rd.getName()); + assertArrayEquals(new String[] { "a", "b" }, rd.getClusterPrivileges()); + assertEquals(1, rd.getIndicesPrivileges().length); + assertArrayEquals(new String[] { "idx1", "idx2" }, rd.getIndicesPrivileges()[0].getIndices()); + assertArrayEquals(new String[] { "m", "n" }, rd.getRunAs()); + assertNull(rd.getIndicesPrivileges()[0].getQuery()); + } + public void testParseEmptyQueryUsingDeprecatedIndicesField() throws Exception { String json = "{\"cluster\":[\"a\", \"b\"], \"run_as\": [\"m\", \"n\"], \"indices\": [{\"names\": [\"idx1\",\"idx2\"], " + "\"privileges\": [\"p1\", \"p2\"], \"query\": \"\"}]}"; @@ -283,4 +295,5 @@ public class RoleDescriptorTests extends ESTestCase { assertEquals(1, parsed.getTransientMetadata().size()); assertEquals(true, parsed.getTransientMetadata().get("enabled")); } + } diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/FileRolesStoreTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/FileRolesStoreTests.java index 3a2c3089100..99ae113e15f 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/FileRolesStoreTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/FileRolesStoreTests.java @@ -9,9 +9,13 @@ import org.apache.logging.log4j.Level; import org.apache.logging.log4j.Logger; import org.apache.lucene.util.automaton.MinimizationOperations; import org.apache.lucene.util.automaton.Operations; +import org.elasticsearch.common.ParseField; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.xcontent.NamedXContentRegistry; import org.elasticsearch.env.Environment; import org.elasticsearch.env.TestEnvironment; +import org.elasticsearch.index.query.MatchAllQueryBuilder; +import org.elasticsearch.index.query.QueryBuilder; import org.elasticsearch.license.XPackLicenseState; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.threadpool.TestThreadPool; @@ -45,6 +49,7 @@ import java.util.Set; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; +import static java.util.Collections.singletonList; import static org.hamcrest.Matchers.arrayContaining; import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.containsString; @@ -61,11 +66,17 @@ import static org.mockito.Mockito.when; public class FileRolesStoreTests extends ESTestCase { + @Override + protected NamedXContentRegistry xContentRegistry() { + return new NamedXContentRegistry(singletonList(new NamedXContentRegistry.Entry(QueryBuilder.class, + new ParseField(MatchAllQueryBuilder.NAME), (p, c) -> MatchAllQueryBuilder.fromXContent(p)))); + } + public void testParseFile() throws Exception { Path path = getDataPath("roles.yml"); Map roles = FileRolesStore.parseFile(path, logger, Settings.builder() .put(XPackSettings.DLS_FLS_ENABLED.getKey(), true) - .build(), new XPackLicenseState(Settings.EMPTY)); + .build(), new XPackLicenseState(Settings.EMPTY), xContentRegistry()); assertThat(roles, notNullValue()); assertThat(roles.size(), is(9)); @@ -235,6 +246,8 @@ public class FileRolesStoreTests extends ESTestCase { assertTrue(group.getFieldPermissions().grantsAccessTo("boo")); assertTrue(group.getFieldPermissions().hasFieldLevelSecurity()); assertThat(group.getQuery(), notNullValue()); + + assertThat(roles.get("role_query_invalid"), nullValue()); } public void testParseFileWithFLSAndDLSDisabled() throws Exception { @@ -244,14 +257,15 @@ public class FileRolesStoreTests extends ESTestCase { events.clear(); Map roles = FileRolesStore.parseFile(path, logger, Settings.builder() .put(XPackSettings.DLS_FLS_ENABLED.getKey(), false) - .build(), new XPackLicenseState(Settings.EMPTY)); + .build(), new XPackLicenseState(Settings.EMPTY), xContentRegistry()); assertThat(roles, notNullValue()); assertThat(roles.size(), is(6)); assertThat(roles.get("role_fields"), nullValue()); assertThat(roles.get("role_query"), nullValue()); assertThat(roles.get("role_query_fields"), nullValue()); + assertThat(roles.get("role_query_invalid"), nullValue()); - assertThat(events, hasSize(3)); + assertThat(events, hasSize(4)); assertThat( events.get(0), startsWith("invalid role definition [role_fields] in roles file [" + path.toAbsolutePath() + @@ -262,6 +276,9 @@ public class FileRolesStoreTests extends ESTestCase { assertThat(events.get(2), startsWith("invalid role definition [role_query_fields] in roles file [" + path.toAbsolutePath() + "]. document and field level security is not enabled.")); + assertThat(events.get(3), + startsWith("invalid role definition [role_query_invalid] in roles file [" + path.toAbsolutePath() + + "]. document and field level security is not enabled.")); } public void testParseFileWithFLSAndDLSUnlicensed() throws Exception { @@ -271,7 +288,7 @@ public class FileRolesStoreTests extends ESTestCase { events.clear(); XPackLicenseState licenseState = mock(XPackLicenseState.class); when(licenseState.isDocumentAndFieldLevelSecurityAllowed()).thenReturn(false); - Map roles = FileRolesStore.parseFile(path, logger, Settings.EMPTY, licenseState); + Map roles = FileRolesStore.parseFile(path, logger, Settings.EMPTY, licenseState, xContentRegistry()); assertThat(roles, notNullValue()); assertThat(roles.size(), is(9)); assertNotNull(roles.get("role_fields")); @@ -295,7 +312,8 @@ public class FileRolesStoreTests extends ESTestCase { public void testDefaultRolesFile() throws Exception { // TODO we should add the config dir to the resources so we don't copy this stuff around... Path path = getDataPath("default_roles.yml"); - Map roles = FileRolesStore.parseFile(path, logger, Settings.EMPTY, new XPackLicenseState(Settings.EMPTY)); + Map roles = FileRolesStore.parseFile(path, logger, Settings.EMPTY, new XPackLicenseState(Settings.EMPTY), + xContentRegistry()); assertThat(roles, notNullValue()); assertThat(roles.size(), is(0)); } @@ -325,7 +343,7 @@ public class FileRolesStoreTests extends ESTestCase { FileRolesStore store = new FileRolesStore(settings, env, watcherService, roleSet -> { modifiedRoles.addAll(roleSet); latch.countDown(); - }, new XPackLicenseState(Settings.EMPTY)); + }, new XPackLicenseState(Settings.EMPTY), xContentRegistry()); Set descriptors = store.roleDescriptors(Collections.singleton("role1")); assertThat(descriptors, notNullValue()); @@ -368,7 +386,7 @@ public class FileRolesStoreTests extends ESTestCase { store = new FileRolesStore(settings, env, watcherService, roleSet -> { truncatedFileRolesModified.addAll(roleSet); truncateLatch.countDown(); - }, new XPackLicenseState(Settings.EMPTY)); + }, new XPackLicenseState(Settings.EMPTY), xContentRegistry()); final Set allRolesPreTruncate = store.getAllRoleNames(); try (BufferedWriter writer = Files.newBufferedWriter(tmp, StandardCharsets.UTF_8, StandardOpenOption.TRUNCATE_EXISTING)) { @@ -391,7 +409,7 @@ public class FileRolesStoreTests extends ESTestCase { store = new FileRolesStore(settings, env, watcherService, roleSet -> { modifiedFileRolesModified.addAll(roleSet); modifyLatch.countDown(); - }, new XPackLicenseState(Settings.EMPTY)); + }, new XPackLicenseState(Settings.EMPTY), xContentRegistry()); try (BufferedWriter writer = Files.newBufferedWriter(tmp, StandardCharsets.UTF_8, StandardOpenOption.TRUNCATE_EXISTING)) { writer.append("role5:").append(System.lineSeparator()); @@ -416,7 +434,8 @@ public class FileRolesStoreTests extends ESTestCase { public void testThatEmptyFileDoesNotResultInLoop() throws Exception { Path file = createTempFile(); Files.write(file, Collections.singletonList("#"), StandardCharsets.UTF_8); - Map roles = FileRolesStore.parseFile(file, logger, Settings.EMPTY, new XPackLicenseState(Settings.EMPTY)); + Map roles = FileRolesStore.parseFile(file, logger, Settings.EMPTY, new XPackLicenseState(Settings.EMPTY), + xContentRegistry()); assertThat(roles.keySet(), is(empty())); } @@ -425,7 +444,8 @@ public class FileRolesStoreTests extends ESTestCase { Logger logger = CapturingLogger.newCapturingLogger(Level.ERROR, null); List entries = CapturingLogger.output(logger.getName(), Level.ERROR); entries.clear(); - Map roles = FileRolesStore.parseFile(path, logger, Settings.EMPTY, new XPackLicenseState(Settings.EMPTY)); + Map roles = FileRolesStore.parseFile(path, logger, Settings.EMPTY, new XPackLicenseState(Settings.EMPTY), + xContentRegistry()); assertThat(roles.size(), is(1)); assertThat(roles, hasKey("valid_role")); RoleDescriptor descriptor = roles.get("valid_role"); @@ -467,7 +487,8 @@ public class FileRolesStoreTests extends ESTestCase { List events = CapturingLogger.output(logger.getName(), Level.ERROR); events.clear(); Path path = getDataPath("reserved_roles.yml"); - Map roles = FileRolesStore.parseFile(path, logger, Settings.EMPTY, new XPackLicenseState(Settings.EMPTY)); + Map roles = FileRolesStore.parseFile(path, logger, Settings.EMPTY, new XPackLicenseState(Settings.EMPTY), + xContentRegistry()); assertThat(roles, notNullValue()); assertThat(roles.size(), is(1)); @@ -498,7 +519,8 @@ public class FileRolesStoreTests extends ESTestCase { .put(XPackSettings.DLS_FLS_ENABLED.getKey(), flsDlsEnabled) .build(); Environment env = TestEnvironment.newEnvironment(settings); - FileRolesStore store = new FileRolesStore(settings, env, mock(ResourceWatcherService.class), new XPackLicenseState(Settings.EMPTY)); + FileRolesStore store = new FileRolesStore(settings, env, mock(ResourceWatcherService.class), new XPackLicenseState(Settings.EMPTY), + xContentRegistry()); Map usageStats = store.usageStats(); @@ -512,9 +534,10 @@ public class FileRolesStoreTests extends ESTestCase { Path path = getDataPath("roles2xformat.yml"); byte[] bytes = Files.readAllBytes(path); String roleString = new String(bytes, Charset.defaultCharset()); - RoleDescriptor role = FileRolesStore.parseRoleDescriptor(roleString, path, logger, true, Settings.EMPTY); + RoleDescriptor role = FileRolesStore.parseRoleDescriptor(roleString, path, logger, true, Settings.EMPTY, xContentRegistry()); RoleDescriptor.IndicesPrivileges indicesPrivileges = role.getIndicesPrivileges()[0]; assertThat(indicesPrivileges.getGrantedFields(), arrayContaining("foo", "boo")); assertNull(indicesPrivileges.getDeniedFields()); } + } diff --git a/x-pack/plugin/security/src/test/resources/org/elasticsearch/xpack/security/authz/store/roles.yml b/x-pack/plugin/security/src/test/resources/org/elasticsearch/xpack/security/authz/store/roles.yml index 99459c5f5ec..c1c8bc4b1d7 100644 --- a/x-pack/plugin/security/src/test/resources/org/elasticsearch/xpack/security/authz/store/roles.yml +++ b/x-pack/plugin/security/src/test/resources/org/elasticsearch/xpack/security/authz/store/roles.yml @@ -65,8 +65,16 @@ role_query_fields: privileges: - READ query: - match_all: + match_all: {} field_security: grant: - foo - - boo \ No newline at end of file + - boo + +role_query_invalid: + indices: + - names: + - 'query_idx' + privileges: + - READ + query: '{ "unknown": {} }' \ No newline at end of file diff --git a/x-pack/plugin/security/src/test/resources/org/elasticsearch/xpack/security/authz/store/roles2xformat.yml b/x-pack/plugin/security/src/test/resources/org/elasticsearch/xpack/security/authz/store/roles2xformat.yml index ebfdce617a0..d0eb7ba4922 100644 --- a/x-pack/plugin/security/src/test/resources/org/elasticsearch/xpack/security/authz/store/roles2xformat.yml +++ b/x-pack/plugin/security/src/test/resources/org/elasticsearch/xpack/security/authz/store/roles2xformat.yml @@ -5,7 +5,7 @@ role1: privileges: - READ query: - match_all: + match_all: {} fields: - foo - boo \ No newline at end of file diff --git a/x-pack/plugin/src/test/resources/rest-api-spec/test/roles/30_prohibited_role_query.yml b/x-pack/plugin/src/test/resources/rest-api-spec/test/roles/30_prohibited_role_query.yml index abddcdc6dda..d3b32456324 100644 --- a/x-pack/plugin/src/test/resources/rest-api-spec/test/roles/30_prohibited_role_query.yml +++ b/x-pack/plugin/src/test/resources/rest-api-spec/test/roles/30_prohibited_role_query.yml @@ -7,7 +7,11 @@ setup: cluster.health: wait_for_status: yellow +--- +"Test use prohibited query inside role query": + - do: + catch: /terms query with terms lookup isn't supported as part of a role query/ security.put_role: name: "role" body: > @@ -24,49 +28,3 @@ setup: ] } - - do: - security.put_user: - username: "joe" - body: > - { - "password": "x-pack-test-password", - "roles" : [ "role" ] - } - ---- -teardown: - - do: - security.delete_user: - username: "joe" - ignore: 404 - - do: - security.delete_role: - name: "role" - ignore: 404 - - ---- -"Test use prohibited query inside role query": - - - do: - headers: - Authorization: "Basic am9lOngtcGFjay10ZXN0LXBhc3N3b3Jk" - index: - index: index - type: type - id: 1 - body: > - { - "foo": "bar" - } - - - - do: - catch: /terms query with terms lookup isn't supported as part of a role query/ - headers: - Authorization: "Basic am9lOngtcGFjay10ZXN0LXBhc3N3b3Jk" - search: - rest_total_hits_as_int: true - index: index - body: { "query" : { "match_all" : {} } } -