Skip to content

Commit bca4612

Browse files
cwperksSandesh KumarDave Lago
authored
[Backport 2.4] Fix issues with datastream backing indexes (#2242)
* Resolving backing indices of data streams when resolving for aliases * Fixing resolution of indices for non-wild card scenarios / exact matches * Adding tests for DLS/FLS/Field-Masking on Data Streams * Datastream test fixes * Add pit-3 mapping Co-authored-by: Sandesh Kumar <[email protected]> Signed-off-by: Craig Perkins <[email protected]> Co-authored-by: Dave Lago <[email protected]> Co-authored-by: Sandesh Kumar <[email protected]>
1 parent 982f281 commit bca4612

File tree

8 files changed

+530
-29
lines changed

8 files changed

+530
-29
lines changed

‎src/main/java/org/opensearch/security/securityconf/ConfigModelV7.java‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@
6868
import org.opensearch.security.user.User;
6969

7070
import static org.opensearch.cluster.metadata.IndexAbstraction.Type.ALIAS;
71+
import static org.opensearch.cluster.metadata.IndexAbstraction.Type.DATA_STREAM;
7172

7273
public class ConfigModelV7 extends ConfigModel {
7374

@@ -768,20 +769,22 @@ public Set<String> getResolvedIndexPattern(final User user, final IndexNameExpre
768769
final ImmutableSet.Builder<String> resolvedIndices = new ImmutableSet.Builder<>();
769770

770771
final WildcardMatcher matcher = WildcardMatcher.from(unresolved);
772+
boolean includeDataStreams = true;
771773
if (!(matcher instanceof WildcardMatcher.Exact)) {
772-
final String[] aliasesForPermittedPattern = cs.state().getMetadata().getIndicesLookup().entrySet().stream()
773-
.filter(e -> e.getValue().getType() == ALIAS)
774+
final String[] aliasesAndDataStreamsForPermittedPattern = cs.state().getMetadata().getIndicesLookup().entrySet().stream()
775+
.filter(e -> (e.getValue().getType() == ALIAS) || (e.getValue().getType() == DATA_STREAM))
774776
.filter(e -> matcher.test(e.getKey()))
775777
.map(e -> e.getKey())
776778
.toArray(String[]::new);
777-
if (aliasesForPermittedPattern.length > 0) {
778-
final String[] resolvedAliases = resolver.concreteIndexNames(cs.state(), IndicesOptions.lenientExpandOpen(), aliasesForPermittedPattern);
779-
resolvedIndices.addAll(Arrays.asList(resolvedAliases));
779+
if (aliasesAndDataStreamsForPermittedPattern.length > 0) {
780+
final String[] resolvedAliasesAndDataStreamIndices = resolver.concreteIndexNames(cs.state(),
781+
IndicesOptions.lenientExpandOpen(), includeDataStreams, aliasesAndDataStreamsForPermittedPattern);
782+
resolvedIndices.addAll(Arrays.asList(resolvedAliasesAndDataStreamIndices));
780783
}
781784
}
782785

783786
if (Strings.isNotBlank(unresolved)) {
784-
final String[] resolvedIndicesFromPattern = resolver.concreteIndexNames(cs.state(), IndicesOptions.lenientExpandOpen(), unresolved);
787+
final String[] resolvedIndicesFromPattern = resolver.concreteIndexNames(cs.state(), IndicesOptions.lenientExpandOpen(), includeDataStreams, unresolved);
785788
resolvedIndices.addAll(Arrays.asList(resolvedIndicesFromPattern));
786789
}
787790

‎src/test/java/org/opensearch/security/DataStreamIntegrationTests.java‎

Lines changed: 250 additions & 3 deletions
Large diffs are not rendered by default.

‎src/test/java/org/opensearch/security/IndexTemplateClusterPermissionsCheckTest.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,10 +36,10 @@ public void setupRestHelper() throws Exception{
3636
}
3737
@Test
3838
public void testPutIndexTemplateByNonPrivilegedUser() throws Exception {
39-
String expectedFailureResponse = getFailureResponseReason("ds3");
39+
String expectedFailureResponse = getFailureResponseReason("ds4");
4040

4141
// should fail, as user `ds3` doesn't have correct permissions
42-
HttpResponse response = rh.executePutRequest("/_index_template/sem1234", indexTemplateBody, encodeBasicHeader("ds3", "nagilum"));
42+
HttpResponse response = rh.executePutRequest("/_index_template/sem1234", indexTemplateBody, encodeBasicHeader("ds4", "nagilum"));
4343
Assert.assertEquals(HttpStatus.SC_FORBIDDEN, response.getStatusCode());
4444
Assert.assertEquals(expectedFailureResponse, response.findValueInJson("error.root_cause[0].reason"));
4545
}

‎src/test/java/org/opensearch/security/PitIntegrationTests.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -229,10 +229,10 @@ public void testDataStreamWithPits() throws Exception {
229229
Assert.assertEquals(HttpStatus.SC_OK, resc.getStatusCode());
230230
String pitId2 = resc.findValueInJson("pit_id");
231231

232-
// since pit-2 doesn't have permission to backing data stream indices, throw security error
232+
// since pit-3 doesn't have permission to backing data stream indices, throw security error
233233
resc = rh.executeGetRequest("/_cat/pit_segments",
234234
"{\"pit_id\":\"" + pitId2 +"\"}",
235-
encodeBasicHeader("pit-2", "nagilum"));
235+
encodeBasicHeader("pit-3", "nagilum"));
236236
Assert.assertEquals(HttpStatus.SC_FORBIDDEN, resc.getStatusCode());
237237

238238
// Delete all PITs should work for user with all index access

‎src/test/java/org/opensearch/security/securityconf/impl/v7/IndexPatternTests.java‎

Lines changed: 14 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -108,47 +108,47 @@ public void testAttemptResolveIndexNamesOverload() {
108108
public void testExactNameWithNoMatches() {
109109
doReturn("index-17").when(ip).getUnresolvedIndexPattern(user);
110110
when(clusterService.state()).thenReturn(mock(ClusterState.class));
111-
when(resolver.concreteIndexNames(any(), eq(IndicesOptions.lenientExpandOpen()), eq("index-17"))).thenReturn(new String[]{});
111+
when(resolver.concreteIndexNames(any(), eq(IndicesOptions.lenientExpandOpen()), eq(true), eq("index-17"))).thenReturn(new String[]{});
112112

113113
final Set<String> results = ip.concreteIndexNames(user, resolver, clusterService);
114114

115115
assertThat(results, contains("index-17"));
116116

117117
verify(clusterService).state();
118118
verify(ip).getUnresolvedIndexPattern(user);
119-
verify(resolver).concreteIndexNames(any(), eq(IndicesOptions.lenientExpandOpen()), eq("index-17"));
119+
verify(resolver).concreteIndexNames(any(), eq(IndicesOptions.lenientExpandOpen()), eq(true), eq("index-17"));
120120
}
121121

122122
/** Verify concreteIndexNames on exact name matches */
123123
@Test
124124
public void testExactName() {
125125
doReturn("index-17").when(ip).getUnresolvedIndexPattern(user);
126126
when(clusterService.state()).thenReturn(mock(ClusterState.class));
127-
when(resolver.concreteIndexNames(any(), eq(IndicesOptions.lenientExpandOpen()), eq("index-17"))).thenReturn(new String[]{"resolved-index-17"});
127+
when(resolver.concreteIndexNames(any(), eq(IndicesOptions.lenientExpandOpen()), eq(true), eq("index-17"))).thenReturn(new String[]{"resolved-index-17"});
128128

129129
final Set<String> results = ip.concreteIndexNames(user, resolver, clusterService);
130130

131131
assertThat(results, contains("resolved-index-17"));
132132

133133
verify(clusterService).state();
134134
verify(ip).getUnresolvedIndexPattern(user);
135-
verify(resolver).concreteIndexNames(any(), eq(IndicesOptions.lenientExpandOpen()), eq("index-17"));
135+
verify(resolver).concreteIndexNames(any(), eq(IndicesOptions.lenientExpandOpen()), eq(true), eq("index-17"));
136136
}
137137

138138
/** Verify concreteIndexNames on multiple matches */
139139
@Test
140140
public void testMultipleConcreteIndices() {
141141
doReturn("index-1*").when(ip).getUnresolvedIndexPattern(user);
142142
doReturn(createClusterState()).when(clusterService).state();
143-
when(resolver.concreteIndexNames(any(), eq(IndicesOptions.lenientExpandOpen()), eq("index-1*"))).thenReturn(new String[]{"resolved-index-17", "resolved-index-18"});
143+
when(resolver.concreteIndexNames(any(), eq(IndicesOptions.lenientExpandOpen()), eq(true), eq("index-1*"))).thenReturn(new String[]{"resolved-index-17", "resolved-index-18"});
144144

145145
final Set<String> results = ip.concreteIndexNames(user, resolver, clusterService);
146146

147147
assertThat(results, contains("resolved-index-17", "resolved-index-18"));
148148

149149
verify(clusterService, times(2)).state();
150150
verify(ip).getUnresolvedIndexPattern(user);
151-
verify(resolver).concreteIndexNames(any(), eq(IndicesOptions.lenientExpandOpen()), eq("index-1*"));
151+
verify(resolver).concreteIndexNames(any(), eq(IndicesOptions.lenientExpandOpen()), eq(true), eq("index-1*"));
152152
}
153153

154154
/** Verify concreteIndexNames when there is an alias */
@@ -157,44 +157,42 @@ public void testMultipleConcreteIndicesWithOneAlias() {
157157
doReturn("index-1*").when(ip).getUnresolvedIndexPattern(user);
158158

159159
doReturn(createClusterState(
160-
new IndexShorthand("index-111", Type.DATA_STREAM), // Name matches/wrong type
161160
new IndexShorthand("index-100", Type.ALIAS), // Name and type match
162161
new IndexShorthand("19", Type.ALIAS) // Type matches/wrong name
163162
)).when(clusterService).state();
164-
when(resolver.concreteIndexNames(any(), eq(IndicesOptions.lenientExpandOpen()), eq("index-100"))).thenReturn(new String[]{"resolved-index-100"});
165-
when(resolver.concreteIndexNames(any(), eq(IndicesOptions.lenientExpandOpen()), eq("index-1*"))).thenReturn(new String[]{"resolved-index-17", "resolved-index-18"});
163+
when(resolver.concreteIndexNames(any(), eq(IndicesOptions.lenientExpandOpen()), eq(true), eq("index-100"))).thenReturn(new String[]{"resolved-index-100"});
164+
when(resolver.concreteIndexNames(any(), eq(IndicesOptions.lenientExpandOpen()), eq(true), eq("index-1*"))).thenReturn(new String[]{"resolved-index-17", "resolved-index-18"});
166165

167166
final Set<String> results = ip.concreteIndexNames(user, resolver, clusterService);
168167

169168
assertThat(results, contains("resolved-index-100", "resolved-index-17", "resolved-index-18"));
170169

171170
verify(clusterService, times(3)).state();
172171
verify(ip).getUnresolvedIndexPattern(user);
173-
verify(resolver).concreteIndexNames(any(), eq(IndicesOptions.lenientExpandOpen()), eq("index-100"));
174-
verify(resolver).concreteIndexNames(any(), eq(IndicesOptions.lenientExpandOpen()), eq("index-1*"));
172+
verify(resolver).concreteIndexNames(any(), eq(IndicesOptions.lenientExpandOpen()), eq(true), eq("index-100"));
173+
verify(resolver).concreteIndexNames(any(), eq(IndicesOptions.lenientExpandOpen()), eq(true), eq("index-1*"));
175174
}
176175

177176
/** Verify attemptResolveIndexNames with multiple aliases */
178177
@Test
179178
public void testMultipleConcreteAliasedAndUnresolved() {
180179
doReturn("index-1*").when(ip).getUnresolvedIndexPattern(user);
181180
doReturn(createClusterState(
182-
new IndexShorthand("index-111", Type.DATA_STREAM), // Name matches/wrong type
183181
new IndexShorthand("index-100", Type.ALIAS), // Name and type match
184182
new IndexShorthand("index-101", Type.ALIAS), // Name and type match
185183
new IndexShorthand("19", Type.ALIAS) // Type matches/wrong name
186184
)).when(clusterService).state();
187-
when(resolver.concreteIndexNames(any(), eq(IndicesOptions.lenientExpandOpen()), eq("index-100"), eq("index-101"))).thenReturn(new String[]{"resolved-index-100", "resolved-index-101"});
188-
when(resolver.concreteIndexNames(any(), eq(IndicesOptions.lenientExpandOpen()), eq("index-1*"))).thenReturn(new String[]{"resolved-index-17", "resolved-index-18"});
185+
when(resolver.concreteIndexNames(any(), eq(IndicesOptions.lenientExpandOpen()), eq(true), eq("index-100"), eq("index-101"))).thenReturn(new String[]{"resolved-index-100", "resolved-index-101"});
186+
when(resolver.concreteIndexNames(any(), eq(IndicesOptions.lenientExpandOpen()), eq(true), eq("index-1*"))).thenReturn(new String[]{"resolved-index-17", "resolved-index-18"});
189187

190188
final Set<String> results = ip.attemptResolveIndexNames(user, resolver, clusterService);
191189

192190
assertThat(results, contains("resolved-index-100", "resolved-index-101", "resolved-index-17", "resolved-index-18", "index-1*"));
193191

194192
verify(clusterService, times(3)).state();
195193
verify(ip).getUnresolvedIndexPattern(user);
196-
verify(resolver).concreteIndexNames(any(), eq(IndicesOptions.lenientExpandOpen()), eq("index-100"), eq("index-101"));
197-
verify(resolver).concreteIndexNames(any(), eq(IndicesOptions.lenientExpandOpen()), eq("index-1*"));
194+
verify(resolver).concreteIndexNames(any(), eq(IndicesOptions.lenientExpandOpen()), eq(true), eq("index-100"), eq("index-101"));
195+
verify(resolver).concreteIndexNames(any(), eq(IndicesOptions.lenientExpandOpen()), eq(true), eq("index-1*"));
198196
}
199197

200198
private ClusterState createClusterState(final IndexShorthand... indices) {

‎src/test/resources/internal_users.yml‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -346,15 +346,51 @@ ds2:
346346
ds3:
347347
hash: $2a$12$n5nubfWATfQjSYHiWtUyeOxMIxFInUHOAx8VMmGmxFNPGpaBmeB.m
348348
#password is: nagilum
349+
ds4:
350+
hash: $2a$12$n5nubfWATfQjSYHiWtUyeOxMIxFInUHOAx8VMmGmxFNPGpaBmeB.m
351+
#password is: nagilum
349352
pit-1:
350353
hash: $2a$12$n5nubfWATfQjSYHiWtUyeOxMIxFInUHOAx8VMmGmxFNPGpaBmeB.m
351354
#password is: nagilum
352355
pit-2:
353356
hash: $2a$12$n5nubfWATfQjSYHiWtUyeOxMIxFInUHOAx8VMmGmxFNPGpaBmeB.m
354357
#password is: nagilum
358+
pit-3:
359+
hash: $2a$12$n5nubfWATfQjSYHiWtUyeOxMIxFInUHOAx8VMmGmxFNPGpaBmeB.m
360+
#password is: nagilum
355361
all-pit:
356362
hash: $2a$12$n5nubfWATfQjSYHiWtUyeOxMIxFInUHOAx8VMmGmxFNPGpaBmeB.m
357363
#password is: nagilum
364+
ds_admin:
365+
hash: $2a$12$n5nubfWATfQjSYHiWtUyeOxMIxFInUHOAx8VMmGmxFNPGpaBmeB.m
366+
#password is: nagilum
367+
ds_dls1:
368+
hash: $2a$12$n5nubfWATfQjSYHiWtUyeOxMIxFInUHOAx8VMmGmxFNPGpaBmeB.m
369+
#password is: nagilum
370+
ds_dls2:
371+
hash: $2a$12$n5nubfWATfQjSYHiWtUyeOxMIxFInUHOAx8VMmGmxFNPGpaBmeB.m
372+
#password is: nagilum
373+
ds_dls3:
374+
hash: $2a$12$n5nubfWATfQjSYHiWtUyeOxMIxFInUHOAx8VMmGmxFNPGpaBmeB.m
375+
#password is: nagilum
376+
ds_fls1:
377+
hash: $2a$12$n5nubfWATfQjSYHiWtUyeOxMIxFInUHOAx8VMmGmxFNPGpaBmeB.m
378+
#password is: nagilum
379+
ds_fls2:
380+
hash: $2a$12$n5nubfWATfQjSYHiWtUyeOxMIxFInUHOAx8VMmGmxFNPGpaBmeB.m
381+
#password is: nagilum
382+
ds_fls3:
383+
hash: $2a$12$n5nubfWATfQjSYHiWtUyeOxMIxFInUHOAx8VMmGmxFNPGpaBmeB.m
384+
#password is: nagilum
385+
ds_fm1:
386+
hash: $2a$12$n5nubfWATfQjSYHiWtUyeOxMIxFInUHOAx8VMmGmxFNPGpaBmeB.m
387+
#password is: nagilum
388+
ds_fm2:
389+
hash: $2a$12$n5nubfWATfQjSYHiWtUyeOxMIxFInUHOAx8VMmGmxFNPGpaBmeB.m
390+
#password is: nagilum
391+
ds_fm3:
392+
hash: $2a$12$n5nubfWATfQjSYHiWtUyeOxMIxFInUHOAx8VMmGmxFNPGpaBmeB.m
393+
#password is: nagilum
358394
hidden_test:
359395
hash: $2a$12$n5nubfWATfQjSYHiWtUyeOxMIxFInUHOAx8VMmGmxFNPGpaBmeB.m
360396
opendistro_security_roles:

‎src/test/resources/roles.yml‎

Lines changed: 157 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1119,6 +1119,20 @@ data_stream_2:
11191119
- "indices:admin/get"
11201120

11211121
data_stream_3:
1122+
reserved: true
1123+
hidden: false
1124+
description: "Migrated from v6 (all types mapped)"
1125+
cluster_permissions:
1126+
- "*"
1127+
index_permissions:
1128+
- index_patterns:
1129+
- "*"
1130+
allowed_actions:
1131+
- "DATASTREAM_ALL"
1132+
- "indices:data/write/index"
1133+
- "indices:data/write/bulk*"
1134+
1135+
data_stream_4:
11221136
reserved: true
11231137
hidden: false
11241138
description: "Migrated from v6 (all types mapped)"
@@ -1129,6 +1143,135 @@ data_stream_3:
11291143
allowed_actions:
11301144
- "DATASTREAM_ALL"
11311145

1146+
data_stream_admin:
1147+
reserved: true
1148+
hidden: false
1149+
description: "Migrated from v6 (all types mapped)"
1150+
cluster_permissions:
1151+
- "*"
1152+
index_permissions:
1153+
- index_patterns:
1154+
- "my-data-stream*"
1155+
allowed_actions:
1156+
- "*"
1157+
1158+
data_stream_dls_1:
1159+
reserved: true
1160+
hidden: false
1161+
description: "Migrated from v6 (all types mapped)"
1162+
cluster_permissions: []
1163+
index_permissions:
1164+
- index_patterns:
1165+
- "my-data-stream11"
1166+
dls: "{\n \"bool\": {\n \"must\": {\n \"match\": {\n \"user.id\": \"8a4f500d\"\n }\n }\n }\n}"
1167+
allowed_actions:
1168+
- "read"
1169+
1170+
data_stream_dls_2:
1171+
reserved: true
1172+
hidden: false
1173+
description: "Migrated from v6 (all types mapped)"
1174+
cluster_permissions: []
1175+
index_permissions:
1176+
- index_patterns:
1177+
- "my-data-stream2*"
1178+
dls: "{\n \"bool\": {\n \"must\": {\n \"match\": {\n \"user.id\": \"8a4f500d\"\n }\n }\n }\n}"
1179+
allowed_actions:
1180+
- "read"
1181+
1182+
data_stream_dls_3:
1183+
reserved: true
1184+
hidden: false
1185+
description: "Migrated from v6 (all types mapped)"
1186+
cluster_permissions: []
1187+
index_permissions:
1188+
- index_patterns:
1189+
- "my-data-stream*"
1190+
dls: "{\n \"bool\": {\n \"must\": {\n \"match\": {\n \"user.id\": \"8a4f500d\"\n }\n }\n }\n}"
1191+
allowed_actions:
1192+
- "read"
1193+
1194+
data_stream_fls_1:
1195+
reserved: true
1196+
hidden: false
1197+
description: "Migrated from v6 (all types mapped)"
1198+
cluster_permissions: []
1199+
index_permissions:
1200+
- index_patterns:
1201+
- "my-data-stream11"
1202+
fls:
1203+
- "user.id"
1204+
- "message"
1205+
allowed_actions:
1206+
- "read"
1207+
1208+
data_stream_fls_2:
1209+
reserved: true
1210+
hidden: false
1211+
description: "Migrated from v6 (all types mapped)"
1212+
cluster_permissions: []
1213+
index_permissions:
1214+
- index_patterns:
1215+
- "my-data-stream2*"
1216+
fls:
1217+
- "user.id"
1218+
- "user.name"
1219+
allowed_actions:
1220+
- "read"
1221+
1222+
data_stream_fls_3:
1223+
reserved: true
1224+
hidden: false
1225+
description: "Migrated from v6 (all types mapped)"
1226+
cluster_permissions: []
1227+
index_permissions:
1228+
- index_patterns:
1229+
- "my-data-stream*"
1230+
fls:
1231+
- "~message"
1232+
allowed_actions:
1233+
- "read"
1234+
1235+
data_stream_fm_1:
1236+
reserved: true
1237+
hidden: false
1238+
description: "Migrated from v6 (all types mapped)"
1239+
cluster_permissions: []
1240+
index_permissions:
1241+
- index_patterns:
1242+
- "my-data-stream11"
1243+
masked_fields:
1244+
- "message"
1245+
allowed_actions:
1246+
- "read"
1247+
1248+
data_stream_fm_2:
1249+
reserved: true
1250+
hidden: false
1251+
description: "Migrated from v6 (all types mapped)"
1252+
cluster_permissions: []
1253+
index_permissions:
1254+
- index_patterns:
1255+
- "my-data-stream2*"
1256+
masked_fields:
1257+
- "message"
1258+
allowed_actions:
1259+
- "read"
1260+
1261+
data_stream_fm_3:
1262+
reserved: true
1263+
hidden: false
1264+
description: "Migrated from v6 (all types mapped)"
1265+
cluster_permissions: []
1266+
index_permissions:
1267+
- index_patterns:
1268+
- "my-data-stream*"
1269+
masked_fields:
1270+
- "user.name"
1271+
- "message"
1272+
allowed_actions:
1273+
- "read"
1274+
11321275
point_in_time_1:
11331276
reserved: true
11341277
hidden: false
@@ -1157,6 +1300,20 @@ point_in_time_2:
11571300
allowed_actions:
11581301
- "manage_point_in_time"
11591302

1303+
point_in_time_3:
1304+
reserved: true
1305+
hidden: false
1306+
description: "Migrated from v6 (all types mapped)"
1307+
index_permissions:
1308+
- index_patterns:
1309+
- "my-data-stream31"
1310+
- "pit_3"
1311+
dls: null
1312+
fls: null
1313+
masked_fields: null
1314+
allowed_actions:
1315+
- "manage_point_in_time"
1316+
11601317
point_in_time_all:
11611318
reserved: true
11621319
hidden: false

0 commit comments

Comments
 (0)