Skip to content

Commit 8d7259d

Browse files
cwperkscwperks
authored
Fix issue in HashingStoredFieldVisitor with stored fields (#4826)
Signed-off-by: Craig Perkins <cwperx@amazon.com>
1 parent 1b207a8 commit 8d7259d

2 files changed

Lines changed: 126 additions & 2 deletions

File tree

‎src/integrationTest/java/org/opensearch/security/StoredFieldsTests.java‎

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,110 @@
1+
/*
2+
* Copyright OpenSearch Contributors
3+
* SPDX-License-Identifier: Apache-2.0
4+
*
5+
* The OpenSearch Contributors require contributions made to
6+
* this file be licensed under the Apache-2.0 license or a
7+
* compatible open source license.
8+
*
9+
*/
10+
package org.opensearch.security;
11+
12+
import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope;
13+
import org.apache.http.HttpStatus;
14+
import org.junit.Assert;
15+
import org.junit.BeforeClass;
16+
import org.junit.ClassRule;
17+
import org.junit.Test;
18+
import org.junit.runner.RunWith;
19+
20+
import org.opensearch.action.admin.indices.create.CreateIndexResponse;
21+
import org.opensearch.client.Client;
22+
import org.opensearch.test.framework.TestSecurityConfig;
23+
import org.opensearch.test.framework.cluster.ClusterManager;
24+
import org.opensearch.test.framework.cluster.LocalCluster;
25+
import org.opensearch.test.framework.cluster.TestRestClient;
26+
27+
import static org.opensearch.action.support.WriteRequest.RefreshPolicy.IMMEDIATE;
28+
import static org.opensearch.test.framework.TestSecurityConfig.AuthcDomain.AUTHC_HTTPBASIC_INTERNAL;
29+
30+
@RunWith(com.carrotsearch.randomizedtesting.RandomizedRunner.class)
31+
@ThreadLeakScope(ThreadLeakScope.Scope.NONE)
32+
public class StoredFieldsTests {
33+
static final TestSecurityConfig.User TEST_USER_MASKED_FIELDS = new TestSecurityConfig.User("test_user_masked_fields").roles(
34+
new TestSecurityConfig.Role("role_masked_fields").clusterPermissions("cluster_composite_ops_ro")
35+
.indexPermissions("read")
36+
.maskedFields("restricted")
37+
.on("test_index")
38+
);
39+
40+
static final TestSecurityConfig.User TEST_USER_FLS = new TestSecurityConfig.User("test_user_fls").roles(
41+
new TestSecurityConfig.Role("role_fls").clusterPermissions("cluster_composite_ops_ro")
42+
.indexPermissions("read")
43+
.fls("~restricted")
44+
.on("test_index")
45+
);
46+
47+
@ClassRule
48+
public static final LocalCluster cluster = new LocalCluster.Builder().clusterManager(ClusterManager.THREE_CLUSTER_MANAGERS)
49+
.authc(AUTHC_HTTPBASIC_INTERNAL)
50+
.users(TEST_USER_MASKED_FIELDS, TEST_USER_FLS)
51+
.build();
52+
53+
@BeforeClass
54+
public static void createTestData() {
55+
try (Client client = cluster.getInternalNodeClient()) {
56+
CreateIndexResponse r = client.admin()
57+
.indices()
58+
.prepareCreate("test_index")
59+
.setMapping("raw", "type=keyword,store=true", "restricted", "type=keyword,store=true")
60+
.get();
61+
62+
client.prepareIndex("test_index").setRefreshPolicy(IMMEDIATE).setSource("raw", "hello", "restricted", "boo!").get();
63+
}
64+
}
65+
66+
@Test
67+
public void testStoredWithWithApplicableMaskedFieldRestrictions() {
68+
try (TestRestClient client = cluster.getRestClient(TEST_USER_MASKED_FIELDS)) {
69+
TestRestClient.HttpResponse normalSearchResponse = client.get("test_index/_search");
70+
Assert.assertFalse(normalSearchResponse.getBody().contains("boo!"));
71+
72+
TestRestClient.HttpResponse fieldSearchResponse = client.postJson("test_index/_search", """
73+
{
74+
"stored_fields": [
75+
"raw",
76+
"restricted"
77+
]
78+
}
79+
""");
80+
fieldSearchResponse.assertStatusCode(HttpStatus.SC_OK);
81+
Assert.assertTrue(fieldSearchResponse.getBody().contains("raw"));
82+
Assert.assertTrue(fieldSearchResponse.getBody().contains("hello"));
83+
Assert.assertTrue(fieldSearchResponse.getBody().contains("restricted"));
84+
Assert.assertFalse(fieldSearchResponse.getBody().contains("boo!"));
85+
}
86+
}
87+
88+
@Test
89+
public void testStoredWithWithApplicableFlsRestrictions() {
90+
try (TestRestClient client = cluster.getRestClient(TEST_USER_FLS)) {
91+
TestRestClient.HttpResponse normalSearchResponse = client.get("test_index/_search");
92+
Assert.assertFalse(normalSearchResponse.getBody().contains("boo!"));
93+
94+
TestRestClient.HttpResponse fieldSearchResponse = client.postJson("test_index/_search", """
95+
{
96+
"stored_fields": [
97+
"raw",
98+
"restricted"
99+
]
100+
}
101+
""");
102+
fieldSearchResponse.assertStatusCode(HttpStatus.SC_OK);
103+
Assert.assertTrue(fieldSearchResponse.getBody().contains("raw"));
104+
Assert.assertTrue(fieldSearchResponse.getBody().contains("hello"));
105+
Assert.assertFalse(fieldSearchResponse.getBody().contains("restricted"));
106+
Assert.assertFalse(fieldSearchResponse.getBody().contains("boo!"));
107+
}
108+
}
109+
110+
}

‎src/main/java/org/opensearch/security/configuration/DlsFlsFilterLeafReader.java‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -663,11 +663,20 @@ public void binaryField(final FieldInfo fieldInfo, final byte[] value) throws IO
663663
}
664664

665665
delegate.binaryField(fieldInfo, Utils.jsonMapToByteArray(filteredSource));
666-
} else {
666+
} else if (shouldInclude(fieldInfo.name)) {
667667
delegate.binaryField(fieldInfo, value);
668668
}
669669
}
670670

671+
private boolean shouldInclude(String field) {
672+
if (excludesSet != null && !excludesSet.isEmpty()) {
673+
return !excludesSet.contains(field);
674+
} else if (includesSet != null && !includesSet.isEmpty()) {
675+
return includesSet.contains(field);
676+
}
677+
return true;
678+
}
679+
671680
@Override
672681
public Status needsField(final FieldInfo fieldInfo) throws IOException {
673682
return isFls(fieldInfo.name) ? delegate.needsField(fieldInfo) : Status.NO;
@@ -733,7 +742,12 @@ public void binaryField(final FieldInfo fieldInfo, final byte[] value) throws IO
733742
final XContentBuilder xBuilder = XContentBuilder.builder(bytesRefTuple.v1().xContent()).map(filteredSource);
734743
delegate.binaryField(fieldInfo, BytesReference.toBytes(BytesReference.bytes(xBuilder)));
735744
} else {
736-
delegate.binaryField(fieldInfo, value);
745+
final MaskedField mf = maskedFieldsMap.getMaskedField(fieldInfo.name).orElse(null);
746+
if (mf != null) {
747+
delegate.binaryField(fieldInfo, mf.mask(value));
748+
} else {
749+
delegate.binaryField(fieldInfo, value);
750+
}
737751
}
738752
}
739753

0 commit comments

Comments
 (0)