diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/FieldPermissions.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/FieldPermissions.java index bc665e301f095..8f2088f55ade6 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/FieldPermissions.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/FieldPermissions.java @@ -18,7 +18,7 @@ import org.elasticsearch.common.Strings; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.regex.Regex; -import org.elasticsearch.core.Nullable; +import org.elasticsearch.common.util.CollectionUtils; import org.elasticsearch.xpack.core.security.authz.accesscontrol.FieldSubsetReader; import org.elasticsearch.xpack.core.security.authz.permission.FieldPermissionsDefinition.FieldGrantExcludeGroup; import org.elasticsearch.xpack.core.security.authz.support.SecurityQueryTemplateEvaluator.DlsQueryEvaluationContext; @@ -61,9 +61,8 @@ public final class FieldPermissions implements Accountable, CacheKey { BASE_HASHSET_ENTRY_SIZE = mapEntryShallowSize + 2 * RamUsageEstimator.NUM_BYTES_OBJECT_REF; } - private final FieldPermissionsDefinition fieldPermissionsDefinition; - @Nullable - private final FieldPermissionsDefinition limitedByFieldPermissionsDefinition; + private final List fieldPermissionsDefinitions; + // an automaton that represents a union of one more sets of permitted and denied fields private final CharacterRunAutomaton permittedFieldsAutomaton; private final boolean permittedFieldsAutomatonIsTotal; @@ -72,7 +71,7 @@ public final class FieldPermissions implements Accountable, CacheKey { private final long ramBytesUsed; /** Constructor that does not enable field-level security: all fields are accepted. */ - public FieldPermissions() { + private FieldPermissions() { this(new FieldPermissionsDefinition(null, null), Automatons.MATCH_ALL); } @@ -85,33 +84,33 @@ public FieldPermissions(FieldPermissionsDefinition fieldPermissionsDefinition) { /** Constructor that enables field-level security based on include/exclude rules. Exclude rules * have precedence over include rules. */ FieldPermissions(FieldPermissionsDefinition fieldPermissionsDefinition, Automaton permittedFieldsAutomaton) { - this(fieldPermissionsDefinition, null, permittedFieldsAutomaton); + this( + List.of(Objects.requireNonNull(fieldPermissionsDefinition, "field permission definition cannot be null")), + permittedFieldsAutomaton + ); } /** Constructor that enables field-level security based on include/exclude rules. Exclude rules * have precedence over include rules. */ - private FieldPermissions( - FieldPermissionsDefinition fieldPermissionsDefinition, - @Nullable FieldPermissionsDefinition limitedByFieldPermissionsDefinition, - Automaton permittedFieldsAutomaton - ) { + private FieldPermissions(List fieldPermissionsDefinitions, Automaton permittedFieldsAutomaton) { if (permittedFieldsAutomaton.isDeterministic() == false && permittedFieldsAutomaton.getNumStates() > 1) { // we only accept deterministic automata so that the CharacterRunAutomaton constructor // directly wraps the provided automaton throw new IllegalArgumentException("Only accepts deterministic automata"); } - this.fieldPermissionsDefinition = Objects.requireNonNull(fieldPermissionsDefinition, "field permission definition cannot be null"); - this.limitedByFieldPermissionsDefinition = limitedByFieldPermissionsDefinition; + this.fieldPermissionsDefinitions = Objects.requireNonNull( + fieldPermissionsDefinitions, + "field permission definitions cannot be null" + ); this.originalAutomaton = permittedFieldsAutomaton; this.permittedFieldsAutomaton = new CharacterRunAutomaton(permittedFieldsAutomaton); // we cache the result of isTotal since this might be a costly operation this.permittedFieldsAutomatonIsTotal = Operations.isTotal(permittedFieldsAutomaton); long ramBytesUsed = BASE_FIELD_PERM_DEF_BYTES; - ramBytesUsed += ramBytesUsedForFieldPermissionsDefinition(this.fieldPermissionsDefinition); - if (this.limitedByFieldPermissionsDefinition != null) { - ramBytesUsed += ramBytesUsedForFieldPermissionsDefinition(this.limitedByFieldPermissionsDefinition); - } + ramBytesUsed += this.fieldPermissionsDefinitions.stream() + .mapToLong(FieldPermissions::ramBytesUsedForFieldPermissionsDefinition) + .sum(); ramBytesUsed += permittedFieldsAutomaton.ramBytesUsed(); ramBytesUsed += runAutomatonRamBytesUsed(permittedFieldsAutomaton); this.ramBytesUsed = ramBytesUsed; @@ -199,12 +198,16 @@ public static Automaton buildPermittedFieldsAutomaton(final String[] grantedFiel */ public FieldPermissions limitFieldPermissions(FieldPermissions limitedBy) { if (hasFieldLevelSecurity() && limitedBy != null && limitedBy.hasFieldLevelSecurity()) { + // TODO: cache the automaton computation with FieldPermissionsCache Automaton _permittedFieldsAutomaton = Automatons.intersectAndMinimize(getIncludeAutomaton(), limitedBy.getIncludeAutomaton()); - return new FieldPermissions(fieldPermissionsDefinition, limitedBy.fieldPermissionsDefinition, _permittedFieldsAutomaton); + return new FieldPermissions( + CollectionUtils.concatLists(fieldPermissionsDefinitions, limitedBy.fieldPermissionsDefinitions), + _permittedFieldsAutomaton + ); } else if (limitedBy != null && limitedBy.hasFieldLevelSecurity()) { - return new FieldPermissions(limitedBy.getFieldPermissionsDefinition(), limitedBy.getIncludeAutomaton()); + return new FieldPermissions(limitedBy.fieldPermissionsDefinitions, limitedBy.getIncludeAutomaton()); } else if (hasFieldLevelSecurity()) { - return new FieldPermissions(this.getFieldPermissionsDefinition(), getIncludeAutomaton()); + return new FieldPermissions(fieldPermissionsDefinitions, getIncludeAutomaton()); } return FieldPermissions.DEFAULT; } @@ -217,23 +220,13 @@ public boolean grantsAccessTo(String fieldName) { return permittedFieldsAutomatonIsTotal || permittedFieldsAutomaton.run(fieldName); } - public FieldPermissionsDefinition getFieldPermissionsDefinition() { - return fieldPermissionsDefinition; - } - - public FieldPermissionsDefinition getLimitedByFieldPermissionsDefinition() { - return limitedByFieldPermissionsDefinition; + public List getFieldPermissionsDefinitions() { + return fieldPermissionsDefinitions; } @Override public void buildCacheKey(StreamOutput out, DlsQueryEvaluationContext context) throws IOException { - fieldPermissionsDefinition.buildCacheKey(out, context); - if (limitedByFieldPermissionsDefinition != null) { - out.writeBoolean(true); - limitedByFieldPermissionsDefinition.buildCacheKey(out, context); - } else { - out.writeBoolean(false); - } + out.writeCollection(fieldPermissionsDefinitions, (o, fpd) -> fpd.buildCacheKey(o, context)); } /** Return whether field-level security is enabled, ie. whether any field might be filtered out. */ @@ -259,13 +252,12 @@ public boolean equals(Object o) { if (o == null || getClass() != o.getClass()) return false; FieldPermissions that = (FieldPermissions) o; return permittedFieldsAutomatonIsTotal == that.permittedFieldsAutomatonIsTotal - && fieldPermissionsDefinition.equals(that.fieldPermissionsDefinition) - && Objects.equals(limitedByFieldPermissionsDefinition, that.limitedByFieldPermissionsDefinition); + && fieldPermissionsDefinitions.equals(that.fieldPermissionsDefinitions); } @Override public int hashCode() { - return Objects.hash(fieldPermissionsDefinition, limitedByFieldPermissionsDefinition, permittedFieldsAutomatonIsTotal); + return Objects.hash(fieldPermissionsDefinitions, permittedFieldsAutomatonIsTotal); } @Override diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/FieldPermissionsCache.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/FieldPermissionsCache.java index 2845120c5246a..51bc57bc934bd 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/FieldPermissionsCache.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/FieldPermissionsCache.java @@ -72,26 +72,25 @@ public FieldPermissions getFieldPermissions(FieldPermissionsDefinition fieldPerm } /** - * Returns a field permissions object that corresponds to the merging of the given field permissions and caches the instance if one was - * not found in the cache. + * Returns a field permissions object that corresponds to the union of the given field permissions. + * Union means a field is granted if it is granted by any of the FieldPermissions from the given + * collection. + * The returned instance is cached if one was not found in the cache. */ - FieldPermissions getFieldPermissions(Collection fieldPermissionsCollection) { + FieldPermissions union(Collection fieldPermissionsCollection) { Optional allowAllFieldPermissions = fieldPermissionsCollection.stream() .filter(((Predicate) (FieldPermissions::hasFieldLevelSecurity)).negate()) .findFirst(); return allowAllFieldPermissions.orElseGet(() -> { final Set fieldGrantExcludeGroups = new HashSet<>(); for (FieldPermissions fieldPermissions : fieldPermissionsCollection) { - final FieldPermissionsDefinition definition = fieldPermissions.getFieldPermissionsDefinition(); - final FieldPermissionsDefinition limitedByDefinition = fieldPermissions.getLimitedByFieldPermissionsDefinition(); - if (definition == null) { - throw new IllegalArgumentException("Expected field permission definition, but found null"); - } else if (limitedByDefinition != null) { + final List fieldPermissionsDefinitions = fieldPermissions.getFieldPermissionsDefinitions(); + if (fieldPermissionsDefinitions.size() != 1) { throw new IllegalArgumentException( - "Expected no limited-by field permission definition, but found [" + limitedByDefinition + "]" + "Expected a single field permission definition, but found [" + fieldPermissionsDefinitions + "]" ); } - fieldGrantExcludeGroups.addAll(definition.getFieldGrantExcludeGroups()); + fieldGrantExcludeGroups.addAll(fieldPermissionsDefinitions.get(0).getFieldGrantExcludeGroups()); } final FieldPermissionsDefinition combined = new FieldPermissionsDefinition(fieldGrantExcludeGroups); try { diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/IndicesPermission.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/IndicesPermission.java index 3782159af8697..70a418e1e1c2f 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/IndicesPermission.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/IndicesPermission.java @@ -494,7 +494,7 @@ && containsPrivilegeThatGrantsMappingUpdatesForBwc(group))) { if (indexFieldPermissions != null && indexFieldPermissions.isEmpty() == false) { fieldPermissions = indexFieldPermissions.size() == 1 ? indexFieldPermissions.iterator().next() - : fieldPermissionsCache.getFieldPermissions(indexFieldPermissions); + : fieldPermissionsCache.union(indexFieldPermissions); } else { fieldPermissions = FieldPermissions.DEFAULT; } @@ -609,6 +609,7 @@ public static class Group { private final String[] indices; private final StringMatcher indexNameMatcher; private final Supplier indexNameAutomaton; + // TODO: Use FieldPermissionsDefinition instead of FieldPermissions. The former is a better counterpart to query private final FieldPermissions fieldPermissions; private final Set query; // by default certain restricted indices are exempted when granting privileges, as they should generally be hidden for ordinary 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 1796431afdd2c..6342f573a838c 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 @@ -159,7 +159,7 @@ public void testDLS() throws Exception { for (int i = 0; i < numValues; i++) { String termQuery = "{\"term\": {\"field\": \"" + values[i] + "\"} }"; IndicesAccessControl.IndexAccessControl indexAccessControl = new IndicesAccessControl.IndexAccessControl( - new FieldPermissions(), + FieldPermissions.DEFAULT, DocumentPermissions.filteredBy(singleton(new BytesArray(termQuery))) ); SecurityIndexReaderWrapper wrapper = new SecurityIndexReaderWrapper( @@ -224,7 +224,7 @@ public void testDLSWithLimitedPermissions() throws Exception { queries.add(new BytesArray("{\"terms\" : { \"f2\" : [\"fv22\"] } }")); queries.add(new BytesArray("{\"terms\" : { \"f2\" : [\"fv32\"] } }")); IndicesAccessControl.IndexAccessControl indexAccessControl = new IndicesAccessControl.IndexAccessControl( - new FieldPermissions(), + FieldPermissions.DEFAULT, DocumentPermissions.filteredBy(queries) ); queries = singleton(new BytesArray("{\"terms\" : { \"f1\" : [\"fv11\", \"fv21\", \"fv31\"] } }")); @@ -232,7 +232,7 @@ public void testDLSWithLimitedPermissions() throws Exception { queries = singleton(new BytesArray("{\"terms\" : { \"f1\" : [\"fv11\", \"fv31\"] } }")); } IndicesAccessControl.IndexAccessControl limitedIndexAccessControl = new IndicesAccessControl.IndexAccessControl( - new FieldPermissions(), + FieldPermissions.DEFAULT, DocumentPermissions.filteredBy(queries) ); IndexSettings indexSettings = IndexSettingsModule.newIndexSettings( diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/FieldPermissionsCacheTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/FieldPermissionsCacheTests.java index 4a3ecab2b3d0c..d5429ff62b2c3 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/FieldPermissionsCacheTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/FieldPermissionsCacheTests.java @@ -44,9 +44,7 @@ public void testMergeFieldPermissions() { FieldPermissions fieldPermissions2 = randomBoolean() ? fieldPermissionsCache.getFieldPermissions(allowed2, denied2) : new FieldPermissions(fieldPermissionDef(allowed2, denied2)); - FieldPermissions mergedFieldPermissions = fieldPermissionsCache.getFieldPermissions( - Arrays.asList(fieldPermissions1, fieldPermissions2) - ); + FieldPermissions mergedFieldPermissions = fieldPermissionsCache.union(Arrays.asList(fieldPermissions1, fieldPermissions2)); assertTrue(mergedFieldPermissions.grantsAccessTo(allowedPrefix1 + "b")); assertTrue(mergedFieldPermissions.grantsAccessTo(allowedPrefix2 + "b")); assertFalse(mergedFieldPermissions.grantsAccessTo(denied1[0])); @@ -62,7 +60,7 @@ public void testMergeFieldPermissions() { fieldPermissions2 = randomBoolean() ? fieldPermissionsCache.getFieldPermissions(allowed2, denied2) : new FieldPermissions(fieldPermissionDef(allowed2, denied2)); - mergedFieldPermissions = fieldPermissionsCache.getFieldPermissions(Arrays.asList(fieldPermissions1, fieldPermissions2)); + mergedFieldPermissions = fieldPermissionsCache.union(Arrays.asList(fieldPermissions1, fieldPermissions2)); assertFalse(mergedFieldPermissions.hasFieldLevelSecurity()); allowed1 = new String[] {}; @@ -75,7 +73,7 @@ public void testMergeFieldPermissions() { fieldPermissions2 = randomBoolean() ? fieldPermissionsCache.getFieldPermissions(allowed2, denied2) : new FieldPermissions(fieldPermissionDef(allowed2, denied2)); - mergedFieldPermissions = fieldPermissionsCache.getFieldPermissions(Arrays.asList(fieldPermissions1, fieldPermissions2)); + mergedFieldPermissions = fieldPermissionsCache.union(Arrays.asList(fieldPermissions1, fieldPermissions2)); for (String field : allowed2) { assertTrue(mergedFieldPermissions.grantsAccessTo(field)); } @@ -93,7 +91,7 @@ public void testMergeFieldPermissions() { fieldPermissions2 = randomBoolean() ? fieldPermissionsCache.getFieldPermissions(allowed2, denied2) : new FieldPermissions(fieldPermissionDef(allowed2, denied2)); - mergedFieldPermissions = fieldPermissionsCache.getFieldPermissions(Arrays.asList(fieldPermissions1, fieldPermissions2)); + mergedFieldPermissions = fieldPermissionsCache.union(Arrays.asList(fieldPermissions1, fieldPermissions2)); assertTrue(mergedFieldPermissions.grantsAccessTo("a")); assertTrue(mergedFieldPermissions.grantsAccessTo("b")); @@ -107,7 +105,7 @@ public void testMergeFieldPermissions() { fieldPermissions2 = randomBoolean() ? fieldPermissionsCache.getFieldPermissions(allowed2, denied2) : new FieldPermissions(fieldPermissionDef(allowed2, denied2)); - mergedFieldPermissions = fieldPermissionsCache.getFieldPermissions(Arrays.asList(fieldPermissions1, fieldPermissions2)); + mergedFieldPermissions = fieldPermissionsCache.union(Arrays.asList(fieldPermissions1, fieldPermissions2)); assertTrue(mergedFieldPermissions.grantsAccessTo("a")); assertTrue(mergedFieldPermissions.grantsAccessTo("b")); assertFalse(mergedFieldPermissions.grantsAccessTo("aa")); @@ -126,7 +124,7 @@ public void testNonFlsAndFlsMerging() { FieldPermissionsCache cache = new FieldPermissionsCache(Settings.EMPTY); for (int i = 0; i < scaledRandomIntBetween(1, 12); i++) { Collections.shuffle(permissionsList, random()); - FieldPermissions result = cache.getFieldPermissions(permissionsList); + FieldPermissions result = cache.union(permissionsList); assertFalse(result.hasFieldLevelSecurity()); } } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/FieldPermissionsTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/FieldPermissionsTests.java index 70709525f9951..6608216fad960 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/FieldPermissionsTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/FieldPermissionsTests.java @@ -16,8 +16,11 @@ import java.io.IOException; import java.util.Arrays; +import java.util.List; import java.util.Set; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.notNullValue; @@ -25,6 +28,16 @@ public class FieldPermissionsTests extends ESTestCase { + public void test() { + FieldPermissions fieldPermissions = FieldPermissions.DEFAULT; + fieldPermissions = fieldPermissions.limitFieldPermissions( + new FieldPermissions(fieldPermissionDef(new String[] { "f1", "f2" }, new String[] { "" })) + ); + + assertThat(fieldPermissions.grantsAccessTo("f1"), is(true)); + assertThat(fieldPermissions.grantsAccessTo("f2"), is(true)); + } + public void testFieldPermissionsIntersection() { final FieldPermissions fieldPermissions = FieldPermissions.DEFAULT; @@ -36,13 +49,13 @@ public void testFieldPermissionsIntersection() { ); { - FieldPermissions result = fieldPermissions.limitFieldPermissions(randomFrom(new FieldPermissions(), null)); + FieldPermissions result = fieldPermissions.limitFieldPermissions(randomFrom(FieldPermissions.DEFAULT, null)); assertThat(result, is(notNullValue())); assertThat(result, IsSame.sameInstance(FieldPermissions.DEFAULT)); } { - FieldPermissions result = fieldPermissions1.limitFieldPermissions(new FieldPermissions()); + FieldPermissions result = fieldPermissions1.limitFieldPermissions(FieldPermissions.DEFAULT); assertThat(result, is(notNullValue())); assertThat(result, not(same(fieldPermissions))); assertThat(result, not(same(fieldPermissions1))); @@ -82,9 +95,47 @@ public void testFieldPermissionsIntersection() { } } + public void testMultipleLimiting() { + // Basic test for a number of permission definitions + FieldPermissions fieldPermissions = FieldPermissions.DEFAULT; + final int nSets = randomIntBetween(2, 8); + final FieldPermissionsDefinition fieldPermissionsDefinition = fieldPermissionDef( + new String[] { "f1", "f2", "f3*" }, + new String[] { "f3" } + ); + for (int i = 0; i < nSets; i++) { + fieldPermissions = fieldPermissions.limitFieldPermissions(new FieldPermissions(fieldPermissionsDefinition)); + } + final List fieldPermissionsDefinitions = fieldPermissions.getFieldPermissionsDefinitions(); + assertNonNullFieldPermissionDefinitions(fieldPermissionsDefinitions, nSets); + fieldPermissionsDefinitions.forEach(fpd -> assertThat(fpd, equalTo(fieldPermissionsDefinition))); + assertThat(fieldPermissions.grantsAccessTo(randomFrom("f1", "f2", "f31")), is(true)); + assertThat(fieldPermissions.grantsAccessTo("f3"), is(false)); + + // More realistic intersection + fieldPermissions = FieldPermissions.DEFAULT; + fieldPermissions = fieldPermissions.limitFieldPermissions( + new FieldPermissions(fieldPermissionDef(new String[] { "f1", "f2", "f3*", "f4*" }, new String[] { "f3" })) + ); + fieldPermissions = fieldPermissions.limitFieldPermissions( + new FieldPermissions(fieldPermissionDef(new String[] { "f2", "f3*", "f4*", "f5*" }, new String[] { "f4" })) + ); + fieldPermissions = fieldPermissions.limitFieldPermissions( + new FieldPermissions(fieldPermissionDef(new String[] { "f3*", "f4*", "f5*", "f6" }, new String[] { "f5" })) + ); + assertNonNullFieldPermissionDefinitions(fieldPermissions.getFieldPermissionsDefinitions(), 3); + + assertThat(fieldPermissions.grantsAccessTo(randomFrom("f1", "f2", "f5", "f6") + randomAlphaOfLengthBetween(0, 10)), is(false)); + assertThat(fieldPermissions.grantsAccessTo("f3"), is(false)); + assertThat(fieldPermissions.grantsAccessTo("f4"), is(false)); + + assertThat(fieldPermissions.grantsAccessTo("f3" + randomAlphaOfLengthBetween(1, 10)), is(true)); + assertThat(fieldPermissions.grantsAccessTo("f4" + randomAlphaOfLengthBetween(1, 10)), is(true)); + } + public void testMustHaveNonNullFieldPermissionsDefinition() { - final FieldPermissions fieldPermissions0 = new FieldPermissions(); - assertThat(fieldPermissions0.getFieldPermissionsDefinition(), notNullValue()); + final FieldPermissions fieldPermissions0 = FieldPermissions.DEFAULT; + assertNonNullFieldPermissionDefinitions(fieldPermissions0.getFieldPermissionsDefinitions()); expectThrows(NullPointerException.class, () -> new FieldPermissions(null)); expectThrows(NullPointerException.class, () -> new FieldPermissions(null, Automatons.MATCH_ALL)); @@ -92,13 +143,15 @@ public void testMustHaveNonNullFieldPermissionsDefinition() { FieldPermissions.DEFAULT, new FieldPermissions(fieldPermissionDef(new String[] { "f1", "f2", "f3*" }, new String[] { "f3" })) ); - assertThat(fieldPermissions03.limitFieldPermissions(null).getFieldPermissionsDefinition(), notNullValue()); - assertThat(fieldPermissions03.limitFieldPermissions(FieldPermissions.DEFAULT).getFieldPermissionsDefinition(), notNullValue()); - assertThat( + assertNonNullFieldPermissionDefinitions(fieldPermissions03.limitFieldPermissions(null).getFieldPermissionsDefinitions()); + assertNonNullFieldPermissionDefinitions( + fieldPermissions03.limitFieldPermissions(FieldPermissions.DEFAULT).getFieldPermissionsDefinitions() + ); + assertNonNullFieldPermissionDefinitions( fieldPermissions03.limitFieldPermissions( new FieldPermissions(fieldPermissionDef(new String[] { "f1", "f3*", "f4" }, new String[] { "f3" })) - ).getFieldPermissionsDefinition(), - notNullValue() + ).getFieldPermissionsDefinitions(), + fieldPermissions03.hasFieldLevelSecurity() ? 2 : 1 ); } @@ -154,7 +207,7 @@ public void testWriteCacheKeyWillDistinguishBetweenDefinitionAndLimitedByDefinit // Just limited by final BytesStreamOutput out3 = new BytesStreamOutput(); - final FieldPermissions fieldPermissions3 = new FieldPermissions().limitFieldPermissions( + final FieldPermissions fieldPermissions3 = FieldPermissions.DEFAULT.limitFieldPermissions( new FieldPermissions( new FieldPermissionsDefinition( Set.of( @@ -180,4 +233,13 @@ private static FieldPermissionsDefinition fieldPermissionDef(String[] granted, S return new FieldPermissionsDefinition(granted, denied); } + private void assertNonNullFieldPermissionDefinitions(List fieldPermissionsDefinitions) { + assertNonNullFieldPermissionDefinitions(fieldPermissionsDefinitions, 1); + } + + private void assertNonNullFieldPermissionDefinitions(List fieldPermissionsDefinitions, int expectedSize) { + assertThat(fieldPermissionsDefinitions, notNullValue()); + assertThat(fieldPermissionsDefinitions, hasSize(expectedSize)); + fieldPermissionsDefinitions.forEach(fieldPermissionsDefinition -> assertThat(fieldPermissionsDefinition, notNullValue())); + } } diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/RBACEngine.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/RBACEngine.java index 54d174ea6bf10..0a43b01019b4b 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/RBACEngine.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/RBACEngine.java @@ -724,9 +724,11 @@ static GetUserPrivilegesResponse buildUserPrivilegesResponseObject(Role userRole final Set queries = group.getQuery() == null ? Collections.emptySet() : group.getQuery(); final Set fieldSecurity; if (group.getFieldPermissions().hasFieldLevelSecurity()) { - final FieldPermissionsDefinition definition = group.getFieldPermissions().getFieldPermissionsDefinition(); - assert group.getFieldPermissions().getLimitedByFieldPermissionsDefinition() == null + final List fieldPermissionsDefinitions = group.getFieldPermissions() + .getFieldPermissionsDefinitions(); + assert fieldPermissionsDefinitions.size() == 1 : "limited-by field must not exist since we do not support reporting user privileges for limited roles"; + final FieldPermissionsDefinition definition = fieldPermissionsDefinitions.get(0); fieldSecurity = definition.getFieldGrantExcludeGroups(); } else { fieldSecurity = Collections.emptySet(); diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/accesscontrol/IndicesAccessControlTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/accesscontrol/IndicesAccessControlTests.java index 05a10260375f5..b2f7d56554668 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/accesscontrol/IndicesAccessControlTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/accesscontrol/IndicesAccessControlTests.java @@ -83,7 +83,7 @@ public void testLimitedIndicesAccessControl() { indicesAccessControl = new IndicesAccessControl( true, - Collections.singletonMap("_index", new IndexAccessControl(new FieldPermissions(), DocumentPermissions.allowAll())) + Collections.singletonMap("_index", new IndexAccessControl(FieldPermissions.DEFAULT, DocumentPermissions.allowAll())) ); limitedByIndicesAccessControl = new IndicesAccessControl(true, Collections.emptyMap()); result = indicesAccessControl.limitIndicesAccessControl(limitedByIndicesAccessControl); @@ -95,11 +95,11 @@ public void testLimitedIndicesAccessControl() { indicesAccessControl = new IndicesAccessControl( true, - Collections.singletonMap("_index", new IndexAccessControl(new FieldPermissions(), DocumentPermissions.allowAll())) + Collections.singletonMap("_index", new IndexAccessControl(FieldPermissions.DEFAULT, DocumentPermissions.allowAll())) ); limitedByIndicesAccessControl = new IndicesAccessControl( true, - Collections.singletonMap("_index", new IndexAccessControl(new FieldPermissions(), DocumentPermissions.allowAll())) + Collections.singletonMap("_index", new IndexAccessControl(FieldPermissions.DEFAULT, DocumentPermissions.allowAll())) ); result = indicesAccessControl.limitIndicesAccessControl(limitedByIndicesAccessControl); assertThat(result, is(notNullValue())); @@ -120,7 +120,7 @@ public void testLimitedIndicesAccessControl() { true, Map.ofEntries( Map.entry("_index", new IndexAccessControl(fieldPermissions1, DocumentPermissions.allowAll())), - Map.entry("another-index", new IndexAccessControl(new FieldPermissions(), DocumentPermissions.allowAll())) + Map.entry("another-index", new IndexAccessControl(FieldPermissions.DEFAULT, DocumentPermissions.allowAll())) ) ); limitedByIndicesAccessControl = new IndicesAccessControl( @@ -159,15 +159,15 @@ public void testLimitedIndicesAccessControl() { indicesAccessControl = new IndicesAccessControl( true, Map.ofEntries( - Map.entry("_index", new IndexAccessControl(new FieldPermissions(), DocumentPermissions.allowAll())), - Map.entry("another-index", new IndexAccessControl(new FieldPermissions(), documentPermissions2)) + Map.entry("_index", new IndexAccessControl(FieldPermissions.DEFAULT, DocumentPermissions.allowAll())), + Map.entry("another-index", new IndexAccessControl(FieldPermissions.DEFAULT, documentPermissions2)) ) ); limitedByIndicesAccessControl = new IndicesAccessControl( true, Map.ofEntries( - Map.entry("_index", new IndexAccessControl(new FieldPermissions(), documentPermissions1)), - Map.entry("another-index", new IndexAccessControl(new FieldPermissions(), DocumentPermissions.allowAll())) + Map.entry("_index", new IndexAccessControl(FieldPermissions.DEFAULT, documentPermissions1)), + Map.entry("another-index", new IndexAccessControl(FieldPermissions.DEFAULT, DocumentPermissions.allowAll())) ) ); result = indicesAccessControl.limitIndicesAccessControl(limitedByIndicesAccessControl); diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/accesscontrol/IndicesPermissionTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/accesscontrol/IndicesPermissionTests.java index ea0e786bee6f0..13f94bb104b9a 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/accesscontrol/IndicesPermissionTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/accesscontrol/IndicesPermissionTests.java @@ -95,7 +95,7 @@ public void testAuthorize() { // no field level security: role = Role.builder(RESTRICTED_INDICES, "_role") - .add(new FieldPermissions(), query, IndexPrivilege.ALL, randomBoolean(), "_index") + .add(FieldPermissions.DEFAULT, query, IndexPrivilege.ALL, randomBoolean(), "_index") .build(); permissions = role.authorize(SearchAction.NAME, Sets.newHashSet("_index"), lookup, fieldPermissionsCache); assertThat(permissions.getIndexPermissions("_index"), notNullValue()); @@ -258,7 +258,7 @@ public void testCorePermissionAuthorize() { FieldPermissionsCache fieldPermissionsCache = new FieldPermissionsCache(Settings.EMPTY); IndicesPermission core = new IndicesPermission.Builder(RESTRICTED_INDICES).addGroup( IndexPrivilege.ALL, - new FieldPermissions(), + FieldPermissions.DEFAULT, null, randomBoolean(), "a1" @@ -285,7 +285,7 @@ public void testCorePermissionAuthorize() { // test with two indices core = new IndicesPermission.Builder(RESTRICTED_INDICES).addGroup( IndexPrivilege.ALL, - new FieldPermissions(), + FieldPermissions.DEFAULT, null, randomBoolean(), "a1" @@ -337,7 +337,7 @@ public void testErrorMessageIfIndexPatternIsTooComplex() { ElasticsearchSecurityException.class, () -> new IndicesPermission.Group( IndexPrivilege.ALL, - new FieldPermissions(), + FieldPermissions.DEFAULT, null, randomBoolean(), RESTRICTED_INDICES, @@ -368,7 +368,7 @@ public void testSecurityIndicesPermissions() { // allow_restricted_indices: false IndicesPermission indicesPermission = new IndicesPermission.Builder(RESTRICTED_INDICES).addGroup( IndexPrivilege.ALL, - new FieldPermissions(), + FieldPermissions.DEFAULT, null, false, "*" @@ -388,7 +388,7 @@ public void testSecurityIndicesPermissions() { // allow_restricted_indices: true indicesPermission = new IndicesPermission.Builder(RESTRICTED_INDICES).addGroup( IndexPrivilege.ALL, - new FieldPermissions(), + FieldPermissions.DEFAULT, null, true, "*" @@ -419,7 +419,7 @@ public void testAsyncSearchIndicesPermissions() { // allow_restricted_indices: false IndicesPermission indicesPermission = new IndicesPermission.Builder(RESTRICTED_INDICES).addGroup( IndexPrivilege.ALL, - new FieldPermissions(), + FieldPermissions.DEFAULT, null, false, "*" @@ -437,7 +437,7 @@ public void testAsyncSearchIndicesPermissions() { // allow_restricted_indices: true indicesPermission = new IndicesPermission.Builder(RESTRICTED_INDICES).addGroup( IndexPrivilege.ALL, - new FieldPermissions(), + FieldPermissions.DEFAULT, null, true, "*" @@ -470,7 +470,7 @@ public void testAuthorizationForBackingIndices() { SortedMap lookup = metadata.getIndicesLookup(); IndicesPermission indicesPermission = new IndicesPermission.Builder(RESTRICTED_INDICES).addGroup( IndexPrivilege.READ, - new FieldPermissions(), + FieldPermissions.DEFAULT, null, false, dataStreamName @@ -490,7 +490,7 @@ public void testAuthorizationForBackingIndices() { indicesPermission = new IndicesPermission.Builder(RESTRICTED_INDICES).addGroup( IndexPrivilege.CREATE_DOC, - new FieldPermissions(), + FieldPermissions.DEFAULT, null, false, dataStreamName @@ -535,7 +535,7 @@ public void testAuthorizationForMappingUpdates() { FieldPermissionsCache fieldPermissionsCache = new FieldPermissionsCache(Settings.EMPTY); IndicesPermission core = new IndicesPermission.Builder(RESTRICTED_INDICES).addGroup( IndexPrivilege.INDEX, - new FieldPermissions(), + FieldPermissions.DEFAULT, null, randomBoolean(), "test*" diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/accesscontrol/OptOutQueryCacheTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/accesscontrol/OptOutQueryCacheTests.java index a3828b8f957f5..102fa69ab6a82 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/accesscontrol/OptOutQueryCacheTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/accesscontrol/OptOutQueryCacheTests.java @@ -174,7 +174,7 @@ public void testOptOutQueryCacheIndexDoesNotHaveFieldLevelSecurity() { final IndicesQueryCache indicesQueryCache = mock(IndicesQueryCache.class); final ThreadContext threadContext = new ThreadContext(Settings.EMPTY); final IndicesAccessControl.IndexAccessControl indexAccessControl = mock(IndicesAccessControl.IndexAccessControl.class); - when(indexAccessControl.getFieldPermissions()).thenReturn(new FieldPermissions()); + when(indexAccessControl.getFieldPermissions()).thenReturn(FieldPermissions.DEFAULT); final IndicesAccessControl indicesAccessControl = mock(IndicesAccessControl.class); when(indicesAccessControl.getIndexPermissions("index")).thenReturn(indexAccessControl); when(indicesAccessControl.isGranted()).thenReturn(true); diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/interceptor/IndicesAliasesRequestInterceptorTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/interceptor/IndicesAliasesRequestInterceptorTests.java index 2c64cdbd53795..1c94f6e9018de 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/interceptor/IndicesAliasesRequestInterceptorTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/interceptor/IndicesAliasesRequestInterceptorTests.java @@ -68,7 +68,7 @@ public void testInterceptorThrowsWhenFLSDLSEnabled() { if (useFls) { fieldPermissions = new FieldPermissions(new FieldPermissionsDefinition(new String[] { "foo" }, null)); } else { - fieldPermissions = new FieldPermissions(); + fieldPermissions = FieldPermissions.DEFAULT; } final boolean useDls = (useFls == false) || randomBoolean(); final Set queries; diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/interceptor/ResizeRequestInterceptorTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/interceptor/ResizeRequestInterceptorTests.java index 8cfb23709b580..4f40487fec46a 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/interceptor/ResizeRequestInterceptorTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/interceptor/ResizeRequestInterceptorTests.java @@ -69,7 +69,7 @@ public void testResizeRequestInterceptorThrowsWhenFLSDLSEnabled() { if (useFls) { fieldPermissions = new FieldPermissions(new FieldPermissionsDefinition(new String[] { "foo" }, null)); } else { - fieldPermissions = new FieldPermissions(); + fieldPermissions = FieldPermissions.DEFAULT; } final boolean useDls = (useFls == false) || randomBoolean(); final Set queries;