Skip to content
Open
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
6 changes: 6 additions & 0 deletions docs/changelog/91720.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
pr: 91720
summary: Mark empty `_terms_enum` results due to DLS as incomplete
area: Search
type: enhancement
issues:
- 88321
Original file line number Diff line number Diff line change
Expand Up @@ -715,7 +715,20 @@ private void asyncNodeOperation(NodeTermsEnumRequest request, Task task, ActionL
ThreadContext threadContext = transportService.getThreadPool().getThreadContext();
final XPackLicenseState frozenLicenseState = licenseState.copyCurrentLicenseState();
for (ShardId shardId : request.shardIds().toArray(new ShardId[0])) {
if (canAccess(shardId, request, frozenLicenseState, threadContext) == false || canMatchShard(shardId, request) == false) {
if (canAccess(shardId, request, frozenLicenseState, threadContext) == false) {
listener.onResponse(
new NodeTermsEnumResponse(
request.nodeId(),
Collections.emptyList(),
"cannot execute [_terms_enum] request on index ["
+ shardId.getIndexName()
+ "] due to "
+ "DLS/FLS security restrictions.",
false
)
);
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing return after calling listener causes duplicate responses

High Severity

When canAccess returns false, listener.onResponse() is called but the method continues execution without returning. The loop proceeds to check additional shards and eventually calls listener.onResponse() again (at line 737 if no shards remain, or via dataNodeOperation). This violates the ActionListener contract that it be invoked exactly once, likely causing response-already-sent errors or undefined behavior.

Fix in Cursor Fix in Web

if (canMatchShard(shardId, request) == false) {
// Permission denied or can't match, remove shardID from request
request.remove(shardId);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -549,18 +549,25 @@ teardown:
- length: { terms: 1 }

- do:
headers: { Authorization: "Basic ZGxzX3NvbWVfdXNlcjp4LXBhY2stdGVzdC1wYXNzd29yZA==" } # dls_some_user sees selected docs
headers: { Authorization: "Basic ZGxzX3NvbWVfdXNlcjp4LXBhY2stdGVzdC1wYXNzd29yZA==" } # dls_some_user doesn't see all docs
terms_enum:
index: test_security
body: {"field": "foo", "string":"b"}
- length: {terms: 0}
index: test_security
body: { "field": "foo", "string": "b" }

- length: { terms: 0 }
- match: { complete: false }
- match: { _shards.failed: 1 }
- match: { _shards.failures.0.reason.type: "broadcast_shard_operation_failed_exception" }
- match: { _shards.failures.0.reason.reason: "cannot execute [_terms_enum] request on index [test_security] due to DLS/FLS security restrictions." }

- do:
headers: { Authorization: "Basic ZmxzX3VzZXI6eC1wYWNrLXRlc3QtcGFzc3dvcmQ=" } # fls_user can't see field
terms_enum:
index: test_security
body: {"field": "foo", "string":"b"}
- length: {terms: 0}
index: test_security
body: { "field": "foo", "string": "b" }
- length: { terms: 0 }
- match: { complete: true }
- match: { _shards.failed: 0 }

---
"Test security with API keys":
Expand Down Expand Up @@ -612,14 +619,17 @@ teardown:
}
}
- match: { name: "dls_all_user_bad_key" }
- set: { encoded: login_creds}
- set: { encoded: login_creds }
- do:
headers:
Authorization: ApiKey ${login_creds} # dls_all_user bad API key sees selected docs
terms_enum:
index: test_security
body: { "field": "foo", "string": "b" }
- length: { terms: 0 }
- match: { complete: false }
- match: { _shards.failed: 1 }
- match: { _shards.failures.0.reason.type: "broadcast_shard_operation_failed_exception" }

- do:
headers: { Authorization: "Basic ZGxzX3NvbWVfdXNlcjp4LXBhY2stdGVzdC1wYXNzd29yZA==" } # dls_some_user
Expand All @@ -641,11 +651,14 @@ teardown:
}
}
- match: { name: "dls_some_user_key" }
- set: { encoded: login_creds}
- set: { encoded: login_creds }
- do:
headers:
Authorization: ApiKey ${login_creds} # dls_some_user's API key sees selected user regardless of the key's role descriptor
terms_enum:
index: test_security
body: { "field": "foo", "string": "b" }
- length: { terms: 0 }
- match: { complete: false }
- match: { _shards.failed: 1 }
- match: { _shards.failures.0.reason.type: "broadcast_shard_operation_failed_exception" }