Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[7.17] Fix FLS for frozen tier (#82521) #82566

Merged
merged 2 commits into from
Jan 14, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
import org.elasticsearch.core.Tuple;
import org.elasticsearch.index.mapper.FieldNamesFieldMapper;
import org.elasticsearch.index.mapper.SourceFieldMapper;
import org.elasticsearch.transport.Transports;
import org.elasticsearch.xcontent.XContentBuilder;
import org.elasticsearch.xcontent.XContentType;

Expand All @@ -44,6 +45,7 @@
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Optional;

/**
* A {@link FilterLeafReader} that exposes only a subset
Expand Down Expand Up @@ -119,7 +121,7 @@ public CacheHelper getReaderCacheHelper() {
/** An automaton that only accepts authorized fields. */
private final CharacterRunAutomaton filter;
/** {@link Terms} cache with filtered stats for the {@link FieldNamesFieldMapper} field. */
private final Terms fieldNamesFilterTerms;
private volatile Optional<Terms> fieldNamesFilterTerms;

/**
* Wrap a single segment, exposing a subset of its fields.
Expand All @@ -134,8 +136,6 @@ public CacheHelper getReaderCacheHelper() {
}
fieldInfos = new FieldInfos(filteredInfos.toArray(new FieldInfo[filteredInfos.size()]));
this.filter = filter;
final Terms fieldNameTerms = super.terms(FieldNamesFieldMapper.NAME);
this.fieldNamesFilterTerms = fieldNameTerms == null ? null : new FieldNamesTerms(fieldNameTerms);
}

/** returns true if this field is allowed. */
Expand Down Expand Up @@ -422,10 +422,22 @@ private Terms wrapTerms(Terms terms, String field) throws IOException {
if (hasField(field) == false) {
return null;
} else if (FieldNamesFieldMapper.NAME.equals(field)) {
// for the _field_names field, fields for the document
// are encoded as postings, where term is the field.
// so we hide terms for fields we filter out.
return fieldNamesFilterTerms;
// For the _field_names field, fields for the document are encoded as postings, where term is the field, so we hide terms for
// fields we filter out.
// Compute this lazily so that the DirectoryReader wrapper works together with RewriteCachingDirectoryReader (used by the
// can match phase in the frozen tier), which does not implement the terms() method.
if (fieldNamesFilterTerms == null) {
synchronized (this) {
if (fieldNamesFilterTerms == null) {
assert Transports.assertNotTransportThread("resolving filter terms");
final Terms fieldNameTerms = super.terms(FieldNamesFieldMapper.NAME);
this.fieldNamesFilterTerms = fieldNameTerms == null
? Optional.empty()
: Optional.of(new FieldNamesTerms(fieldNameTerms));
}
}
}
return fieldNamesFilterTerms.orElse(null);
} else {
return terms;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,11 @@ setup:
{
"names": ["*"],
"privileges": ["read"],
"query": {"match": {"marker": "test_1"}}
"query": {"match": {"marker": "test_1"}},
"field_security" : {
"grant" : [ "*" ],
"except" : [ "forbidden_field" ]
}
}
]
}
Expand Down Expand Up @@ -72,14 +76,37 @@ setup:

- do:
indices.create:
index: test_index
index: test_index1
body:
mappings:
properties:
location:
properties:
city:
type: "keyword"
created_at:
type: date # add date field to trigger can-match phase in searches
format: "yyyy-MM-dd"

settings:
index:
number_of_shards: 1
number_of_replicas: 0

- do:
indices.create:
index: test_index2
body:
mappings:
properties:
location:
properties:
city:
type: "keyword"
created_at:
type: date # add date field to trigger can-match phase in searches
format: "yyyy-MM-dd"

settings:
index:
number_of_shards: 1
Expand All @@ -89,10 +116,14 @@ setup:
bulk:
refresh: true
body:
- '{"index": {"_index": "test_index"}}'
- '{"marker": "test_1", "location.city": "bos"}'
- '{"index": {"_index": "test_index"}}'
- '{"marker": "test_2", "location.city": "ams"}'
- '{"index": {"_index": "test_index1"}}'
- '{"marker": "test_1", "location.city": "bos", "forbidden_field" : 1, "created_at": "2016-01-01"}'
- '{"index": {"_index": "test_index1"}}'
- '{"marker": "test_2", "location.city": "ams", "forbidden_field" : 2, "created_at": "2016-01-01"}'
- '{"index": {"_index": "test_index2"}}'
- '{"marker": "test_2", "location.city": "bos", "forbidden_field" : 1, "created_at": "2019-01-02"}'
- '{"index": {"_index": "test_index2"}}'
- '{"marker": "test_2", "location.city": "ams", "forbidden_field" : 2, "created_at": "2019-01-02"}'

- do:
snapshot.create:
Expand All @@ -102,7 +133,11 @@ setup:

- do:
indices.delete:
index: test_index
index: test_index1

- do:
indices.delete:
index: test_index2

---
teardown:
Expand Down Expand Up @@ -136,8 +171,22 @@ teardown:
wait_for_completion: true
storage: full_copy
body:
index: test_index
renamed_index: test_index
index: test_index1
renamed_index: test_index1

- match: { snapshot.snapshot: snapshot }
- match: { snapshot.shards.failed: 0 }
- match: { snapshot.shards.successful: 1 }

- do:
searchable_snapshots.mount:
repository: repository-fs
snapshot: snapshot
wait_for_completion: true
storage: full_copy
body:
index: test_index2
renamed_index: test_index2

- match: { snapshot.snapshot: snapshot }
- match: { snapshot.shards.failed: 0 }
Expand All @@ -147,16 +196,22 @@ teardown:
headers: { Authorization: "Basic ZnVsbDp4LXBhY2stdGVzdC1wYXNzd29yZA==" } # full - user
search:
rest_total_hits_as_int: true
index: test_index
index: test_index*
size: 0
from: 0
body:
query:
range:
created_at:
"gte": "2016-01-01"
"lt": "2018-02-01"
aggs:
cities:
terms:
field: location.city

- match: { _shards.total: 1 }
- match: { _shards.total: 2 }
- match: { _shards.skipped: 1 }
- match: { hits.total: 2 }
- length: { aggregations.cities.buckets: 2 }
- match: { aggregations.cities.buckets.0.key: "ams" }
Expand All @@ -168,24 +223,30 @@ teardown:
headers: { Authorization: "Basic bGltaXRlZDp4LXBhY2stdGVzdC1wYXNzd29yZA==" } # limited - user
search:
rest_total_hits_as_int: true
index: test_index
index: test_index*
size: 0
from: 0
body:
query:
range:
created_at:
"gte": "2016-01-01"
"lt": "2018-02-01"
aggs:
cities:
terms:
field: location.city

- match: { _shards.total: 1 }
- match: { _shards.total: 2 }
- match: { _shards.skipped: 1 }
- match: { hits.total: 1 }
- length: { aggregations.cities.buckets: 1 }
- match: { aggregations.cities.buckets.0.key: "bos" }
- match: { aggregations.cities.buckets.0.doc_count: 1 }

- do:
indices.delete:
index: test_index
index: test_index*

---
"Test doc level security with different users on shared_cache index":
Expand All @@ -197,8 +258,22 @@ teardown:
wait_for_completion: true
storage: shared_cache
body:
index: test_index
renamed_index: test_index
index: test_index1
renamed_index: test_index1

- match: { snapshot.snapshot: snapshot }
- match: { snapshot.shards.failed: 0 }
- match: { snapshot.shards.successful: 1 }

- do:
searchable_snapshots.mount:
repository: repository-fs
snapshot: snapshot
wait_for_completion: true
storage: shared_cache
body:
index: test_index2
renamed_index: test_index2

- match: { snapshot.snapshot: snapshot }
- match: { snapshot.shards.failed: 0 }
Expand All @@ -208,16 +283,22 @@ teardown:
headers: { Authorization: "Basic ZnVsbDp4LXBhY2stdGVzdC1wYXNzd29yZA==" } # full - user
search:
rest_total_hits_as_int: true
index: test_index
index: test_index*
size: 0
from: 0
body:
query:
range:
created_at:
"gte": "2016-01-01"
"lt": "2018-02-01"
aggs:
cities:
terms:
field: location.city

- match: { _shards.total: 1 }
- match: { _shards.total: 2 }
- match: { _shards.skipped: 1 }
- match: { hits.total: 2 }
- length: { aggregations.cities.buckets: 2 }
- match: { aggregations.cities.buckets.0.key: "ams" }
Expand All @@ -229,21 +310,27 @@ teardown:
headers: { Authorization: "Basic bGltaXRlZDp4LXBhY2stdGVzdC1wYXNzd29yZA==" } # limited - user
search:
rest_total_hits_as_int: true
index: test_index
index: test_index*
size: 0
from: 0
body:
query:
range:
created_at:
"gte": "2016-01-01"
"lt": "2018-02-01"
aggs:
cities:
terms:
field: location.city

- match: { _shards.total: 1 }
- match: { _shards.total: 2 }
- match: { _shards.skipped: 1 }
- match: { hits.total: 1 }
- length: { aggregations.cities.buckets: 1 }
- match: { aggregations.cities.buckets.0.key: "bos" }
- match: { aggregations.cities.buckets.0.doc_count: 1 }

- do:
indices.delete:
index: test_index
index: test_index*