Skip to content

Commit

Permalink
Always grant access to allowlist
Browse files Browse the repository at this point in the history
  • Loading branch information
n1v0lg committed Dec 9, 2024
1 parent 6383588 commit dd06125
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ public static Automaton buildPermittedFieldsAutomaton(final String[] grantedFiel
if (grantedFields == null || Arrays.stream(grantedFields).anyMatch(Regex::isMatchAllPattern)) {
grantedFieldsAutomaton = Automatons.MATCH_ALL;
} else {
grantedFieldsAutomaton = Operations.union(Automatons.patterns(grantedFields), METADATA_FIELDS_ALLOWLIST_AUTOMATON);
grantedFieldsAutomaton = Automatons.patterns(grantedFields);
}

Automaton deniedFieldsAutomaton;
Expand All @@ -214,6 +214,11 @@ public static Automaton buildPermittedFieldsAutomaton(final String[] grantedFiel
deniedFieldsAutomaton = Automatons.patterns(deniedFields);
}

// short-circuit if we know that all fields are allowed
if (grantedFieldsAutomaton == Automatons.MATCH_ALL && deniedFieldsAutomaton == Automatons.EMPTY) {
return Automatons.MATCH_ALL;
}

grantedFieldsAutomaton = Operations.removeDeadStates(
Operations.determinize(grantedFieldsAutomaton, Operations.DEFAULT_DETERMINIZE_WORK_LIMIT)
);
Expand All @@ -222,8 +227,8 @@ public static Automaton buildPermittedFieldsAutomaton(final String[] grantedFiel
);

if (Automatons.subsetOf(deniedFieldsAutomaton, grantedFieldsAutomaton) == false) {
// TODO optimize this: we don't want to break existing roles completely, so we need to account for denied fields
// that cover the "_*" pattern, since this was previously supported
// We should not break existing roles completely, so we need to account for denied fields that cover the "_*" pattern,
// since this was previously supported
final Automaton legacyMetadataFieldsAutomaton = Operations.concatenate(Automata.makeChar('_'), Automata.makeAnyString());
final Automaton grantedFieldsWithLegacyMetadataFieldsAutomaton = Operations.removeDeadStates(
Operations.determinize(
Expand All @@ -243,8 +248,16 @@ public static Automaton buildPermittedFieldsAutomaton(final String[] grantedFiel
// log something?
}

grantedFieldsAutomaton = Automatons.minusAndDeterminize(grantedFieldsAutomaton, deniedFieldsAutomaton);
return grantedFieldsAutomaton;
return Operations.removeDeadStates(
Operations.determinize(
Operations.union(
Automatons.minusAndDeterminize(grantedFieldsAutomaton, deniedFieldsAutomaton),
// add in allowlisted metadata fields after removing denied fields since we always allow them
METADATA_FIELDS_ALLOWLIST_AUTOMATON
),
Operations.DEFAULT_DETERMINIZE_WORK_LIMIT
)
);
}

/**
Expand Down Expand Up @@ -332,5 +345,4 @@ public int hashCode() {
public long ramBytesUsed() {
return ramBytesUsed;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,6 @@ public void testAllBuiltinMetadataFieldsEitherAllowlistedOrExcluded() {
final Set<String> categorizedFields = Sets.union(excludedFields, FieldPermissions.METADATA_FIELDS_ALLOWLIST);

final Set<String> builtinMetadataFields = IndicesModule.getBuiltInMetadataFields();
// ensure all built-in metadata fields are either allowlisted or excluded, usings sets.difference
final Set<String> uncategorizedFields = Sets.difference(builtinMetadataFields, categorizedFields);
assertThat(
"Several metadata fields are neither allowlisted nor explicitly excluded from allowlist "
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ protected String configRoles() {
privileges: [ ALL ]
field_security:
grant: [ field1, _field1, _field2, id ]
except: [ _field2 ]
except: [ _field2, _id, _seq_no ]
""";
}

Expand Down Expand Up @@ -466,9 +466,21 @@ public void testUnderscoredFieldsFiltered() {
{
FieldCapabilitiesRequest fieldCapabilitiesRequest = new FieldCapabilitiesRequest().fields("*").indices("test");
FieldCapabilitiesResponse response = client().filterWithHeader(
Collections.singletonMap(BASIC_AUTH_HEADER, basicAuthHeaderValue("user5", USERS_PASSWD))
Collections.singletonMap(BASIC_AUTH_HEADER, basicAuthHeaderValue("user6", USERS_PASSWD))
).fieldCaps(fieldCapabilitiesRequest).actionGet();
assertExpectedFieldsIgnoringAllowlistedMetadataFields(response, "field1");

GetFieldMappingsResponse getFieldMappingsResponse = client().filterWithHeader(
Collections.singletonMap(BASIC_AUTH_HEADER, basicAuthHeaderValue("user6", USERS_PASSWD))
).admin().indices().prepareGetFieldMappings("test").setFields("*").get();
Map<String, Map<String, GetFieldMappingsResponse.FieldMappingMetadata>> mappings = getFieldMappingsResponse.mappings();
assertEquals(1, mappings.size());
assertExpectedFieldsIgnoringAllowlistedMetadataFields(mappings.get("test"), "field1");

GetIndexResponse getIndexResponse = client().filterWithHeader(
Collections.singletonMap(BASIC_AUTH_HEADER, basicAuthHeaderValue("user6", USERS_PASSWD))
).admin().indices().prepareGetIndex().setIndices("test").get();
assertExpectedFields(getIndexResponse.getMappings(), "field1");
}

{
Expand All @@ -477,6 +489,18 @@ public void testUnderscoredFieldsFiltered() {
Collections.singletonMap(BASIC_AUTH_HEADER, basicAuthHeaderValue("user7", USERS_PASSWD))
).fieldCaps(fieldCapabilitiesRequest).actionGet();
assertExpectedFieldsIgnoringAllowlistedMetadataFields(response, "field1", "_field1");

GetFieldMappingsResponse getFieldMappingsResponse = client().filterWithHeader(
Collections.singletonMap(BASIC_AUTH_HEADER, basicAuthHeaderValue("user7", USERS_PASSWD))
).admin().indices().prepareGetFieldMappings("test").setFields("*").get();
Map<String, Map<String, GetFieldMappingsResponse.FieldMappingMetadata>> mappings = getFieldMappingsResponse.mappings();
assertEquals(1, mappings.size());
assertExpectedFieldsIgnoringAllowlistedMetadataFields(mappings.get("test"), "field1", "_field1");

GetIndexResponse getIndexResponse = client().filterWithHeader(
Collections.singletonMap(BASIC_AUTH_HEADER, basicAuthHeaderValue("user7", USERS_PASSWD))
).admin().indices().prepareGetIndex().setIndices("test").get();
assertExpectedFields(getIndexResponse.getMappings(), "field1", "_field1");
}
}

Expand Down

0 comments on commit dd06125

Please sign in to comment.