Skip to content

Commit

Permalink
[Backport 2.x] Fix issues with datastream backing indexes (#2243)
Browse files Browse the repository at this point in the history 
Signed-off-by: Craig Perkins <cwperx@amazon.com>
Co-authored-by: Dave Lago <davelago@amazon.com>
Co-authored-by: Sandesh Kumar <kusandes@amazon.com>
  • Loading branch information
@cwperks
3 people authored Nov 8, 2022
1 parent 887d5d5 commit 58a9f6d
Show file tree
Hide file tree
Showing 8 changed files with 530 additions and 29 deletions.
15 changes: 9 additions & 6 deletions src/main/java/org/opensearch/security/securityconf/ConfigModelV7.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@
import org.opensearch.security.user.User;

import static org.opensearch.cluster.metadata.IndexAbstraction.Type.ALIAS;
import static org.opensearch.cluster.metadata.IndexAbstraction.Type.DATA_STREAM;

public class ConfigModelV7 extends ConfigModel {

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

final WildcardMatcher matcher = WildcardMatcher.from(unresolved);
boolean includeDataStreams = true;
if (!(matcher instanceof WildcardMatcher.Exact)) {
final String[] aliasesForPermittedPattern = cs.state().getMetadata().getIndicesLookup().entrySet().stream()
.filter(e -> e.getValue().getType() == ALIAS)
final String[] aliasesAndDataStreamsForPermittedPattern = cs.state().getMetadata().getIndicesLookup().entrySet().stream()
.filter(e -> (e.getValue().getType() == ALIAS) || (e.getValue().getType() == DATA_STREAM))
.filter(e -> matcher.test(e.getKey()))
.map(e -> e.getKey())
.toArray(String[]::new);
if (aliasesForPermittedPattern.length > 0) {
final String[] resolvedAliases = resolver.concreteIndexNames(cs.state(), IndicesOptions.lenientExpandOpen(), aliasesForPermittedPattern);
resolvedIndices.addAll(Arrays.asList(resolvedAliases));
if (aliasesAndDataStreamsForPermittedPattern.length > 0) {
final String[] resolvedAliasesAndDataStreamIndices = resolver.concreteIndexNames(cs.state(),
IndicesOptions.lenientExpandOpen(), includeDataStreams, aliasesAndDataStreamsForPermittedPattern);
resolvedIndices.addAll(Arrays.asList(resolvedAliasesAndDataStreamIndices));
}
}

if (Strings.isNotBlank(unresolved)) {
final String[] resolvedIndicesFromPattern = resolver.concreteIndexNames(cs.state(), IndicesOptions.lenientExpandOpen(), unresolved);
final String[] resolvedIndicesFromPattern = resolver.concreteIndexNames(cs.state(), IndicesOptions.lenientExpandOpen(), includeDataStreams, unresolved);
resolvedIndices.addAll(Arrays.asList(resolvedIndicesFromPattern));
}

Expand Down
253 changes: 250 additions & 3 deletions src/test/java/org/opensearch/security/DataStreamIntegrationTests.java
View file

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions src/test/java/org/opensearch/security/IndexTemplateClusterPermissionsCheckTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,10 @@ public void setupRestHelper() throws Exception{
}
@Test
public void testPutIndexTemplateByNonPrivilegedUser() throws Exception {
String expectedFailureResponse = getFailureResponseReason("ds3");
String expectedFailureResponse = getFailureResponseReason("ds4");

// should fail, as user `ds3` doesn't have correct permissions
HttpResponse response = rh.executePutRequest("/_index_template/sem1234", indexTemplateBody, encodeBasicHeader("ds3", "nagilum"));
HttpResponse response = rh.executePutRequest("/_index_template/sem1234", indexTemplateBody, encodeBasicHeader("ds4", "nagilum"));
Assert.assertEquals(HttpStatus.SC_FORBIDDEN, response.getStatusCode());
Assert.assertEquals(expectedFailureResponse, response.findValueInJson("error.root_cause[0].reason"));
}
Expand Down
4 changes: 2 additions & 2 deletions src/test/java/org/opensearch/security/PitIntegrationTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -229,10 +229,10 @@ public void testDataStreamWithPits() throws Exception {
Assert.assertEquals(HttpStatus.SC_OK, resc.getStatusCode());
String pitId2 = resc.findValueInJson("pit_id");

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

// Delete all PITs should work for user with all index access
Expand Down
30 changes: 14 additions & 16 deletions src/test/java/org/opensearch/security/securityconf/impl/v7/IndexPatternTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,47 +108,47 @@ public void testAttemptResolveIndexNamesOverload() {
public void testExactNameWithNoMatches() {
doReturn("index-17").when(ip).getUnresolvedIndexPattern(user);
when(clusterService.state()).thenReturn(mock(ClusterState.class));
when(resolver.concreteIndexNames(any(), eq(IndicesOptions.lenientExpandOpen()), eq("index-17"))).thenReturn(new String[]{});
when(resolver.concreteIndexNames(any(), eq(IndicesOptions.lenientExpandOpen()), eq(true), eq("index-17"))).thenReturn(new String[]{});

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

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

verify(clusterService).state();
verify(ip).getUnresolvedIndexPattern(user);
verify(resolver).concreteIndexNames(any(), eq(IndicesOptions.lenientExpandOpen()), eq("index-17"));
verify(resolver).concreteIndexNames(any(), eq(IndicesOptions.lenientExpandOpen()), eq(true), eq("index-17"));
}

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

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

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

verify(clusterService).state();
verify(ip).getUnresolvedIndexPattern(user);
verify(resolver).concreteIndexNames(any(), eq(IndicesOptions.lenientExpandOpen()), eq("index-17"));
verify(resolver).concreteIndexNames(any(), eq(IndicesOptions.lenientExpandOpen()), eq(true), eq("index-17"));
}

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

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

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

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

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

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

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

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

verify(clusterService, times(3)).state();
verify(ip).getUnresolvedIndexPattern(user);
verify(resolver).concreteIndexNames(any(), eq(IndicesOptions.lenientExpandOpen()), eq("index-100"));
verify(resolver).concreteIndexNames(any(), eq(IndicesOptions.lenientExpandOpen()), eq("index-1*"));
verify(resolver).concreteIndexNames(any(), eq(IndicesOptions.lenientExpandOpen()), eq(true), eq("index-100"));
verify(resolver).concreteIndexNames(any(), eq(IndicesOptions.lenientExpandOpen()), eq(true), eq("index-1*"));
}

/** Verify attemptResolveIndexNames with multiple aliases */
@Test
public void testMultipleConcreteAliasedAndUnresolved() {
doReturn("index-1*").when(ip).getUnresolvedIndexPattern(user);
doReturn(createClusterState(
new IndexShorthand("index-111", Type.DATA_STREAM), // Name matches/wrong type
new IndexShorthand("index-100", Type.ALIAS), // Name and type match
new IndexShorthand("index-101", Type.ALIAS), // Name and type match
new IndexShorthand("19", Type.ALIAS) // Type matches/wrong name
)).when(clusterService).state();
when(resolver.concreteIndexNames(any(), eq(IndicesOptions.lenientExpandOpen()), eq("index-100"), eq("index-101"))).thenReturn(new String[]{"resolved-index-100", "resolved-index-101"});
when(resolver.concreteIndexNames(any(), eq(IndicesOptions.lenientExpandOpen()), eq("index-1*"))).thenReturn(new String[]{"resolved-index-17", "resolved-index-18"});
when(resolver.concreteIndexNames(any(), eq(IndicesOptions.lenientExpandOpen()), eq(true), eq("index-100"), eq("index-101"))).thenReturn(new String[]{"resolved-index-100", "resolved-index-101"});
when(resolver.concreteIndexNames(any(), eq(IndicesOptions.lenientExpandOpen()), eq(true), eq("index-1*"))).thenReturn(new String[]{"resolved-index-17", "resolved-index-18"});

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

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

verify(clusterService, times(3)).state();
verify(ip).getUnresolvedIndexPattern(user);
verify(resolver).concreteIndexNames(any(), eq(IndicesOptions.lenientExpandOpen()), eq("index-100"), eq("index-101"));
verify(resolver).concreteIndexNames(any(), eq(IndicesOptions.lenientExpandOpen()), eq("index-1*"));
verify(resolver).concreteIndexNames(any(), eq(IndicesOptions.lenientExpandOpen()), eq(true), eq("index-100"), eq("index-101"));
verify(resolver).concreteIndexNames(any(), eq(IndicesOptions.lenientExpandOpen()), eq(true), eq("index-1*"));
}

private ClusterState createClusterState(final IndexShorthand... indices) {
Expand Down
36 changes: 36 additions & 0 deletions src/test/resources/internal_users.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -346,15 +346,51 @@ ds2:
ds3:
hash: $2a$12$n5nubfWATfQjSYHiWtUyeOxMIxFInUHOAx8VMmGmxFNPGpaBmeB.m
#password is: nagilum
ds4:
hash: $2a$12$n5nubfWATfQjSYHiWtUyeOxMIxFInUHOAx8VMmGmxFNPGpaBmeB.m
#password is: nagilum
pit-1:
hash: $2a$12$n5nubfWATfQjSYHiWtUyeOxMIxFInUHOAx8VMmGmxFNPGpaBmeB.m
#password is: nagilum
pit-2:
hash: $2a$12$n5nubfWATfQjSYHiWtUyeOxMIxFInUHOAx8VMmGmxFNPGpaBmeB.m
#password is: nagilum
pit-3:
hash: $2a$12$n5nubfWATfQjSYHiWtUyeOxMIxFInUHOAx8VMmGmxFNPGpaBmeB.m
#password is: nagilum
all-pit:
hash: $2a$12$n5nubfWATfQjSYHiWtUyeOxMIxFInUHOAx8VMmGmxFNPGpaBmeB.m
#password is: nagilum
ds_admin:
hash: $2a$12$n5nubfWATfQjSYHiWtUyeOxMIxFInUHOAx8VMmGmxFNPGpaBmeB.m
#password is: nagilum
ds_dls1:
hash: $2a$12$n5nubfWATfQjSYHiWtUyeOxMIxFInUHOAx8VMmGmxFNPGpaBmeB.m
#password is: nagilum
ds_dls2:
hash: $2a$12$n5nubfWATfQjSYHiWtUyeOxMIxFInUHOAx8VMmGmxFNPGpaBmeB.m
#password is: nagilum
ds_dls3:
hash: $2a$12$n5nubfWATfQjSYHiWtUyeOxMIxFInUHOAx8VMmGmxFNPGpaBmeB.m
#password is: nagilum
ds_fls1:
hash: $2a$12$n5nubfWATfQjSYHiWtUyeOxMIxFInUHOAx8VMmGmxFNPGpaBmeB.m
#password is: nagilum
ds_fls2:
hash: $2a$12$n5nubfWATfQjSYHiWtUyeOxMIxFInUHOAx8VMmGmxFNPGpaBmeB.m
#password is: nagilum
ds_fls3:
hash: $2a$12$n5nubfWATfQjSYHiWtUyeOxMIxFInUHOAx8VMmGmxFNPGpaBmeB.m
#password is: nagilum
ds_fm1:
hash: $2a$12$n5nubfWATfQjSYHiWtUyeOxMIxFInUHOAx8VMmGmxFNPGpaBmeB.m
#password is: nagilum
ds_fm2:
hash: $2a$12$n5nubfWATfQjSYHiWtUyeOxMIxFInUHOAx8VMmGmxFNPGpaBmeB.m
#password is: nagilum
ds_fm3:
hash: $2a$12$n5nubfWATfQjSYHiWtUyeOxMIxFInUHOAx8VMmGmxFNPGpaBmeB.m
#password is: nagilum
hidden_test:
hash: $2a$12$n5nubfWATfQjSYHiWtUyeOxMIxFInUHOAx8VMmGmxFNPGpaBmeB.m
opendistro_security_roles:
Expand Down
157 changes: 157 additions & 0 deletions src/test/resources/roles.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1119,6 +1119,20 @@ data_stream_2:
- "indices:admin/get"

data_stream_3:
reserved: true
hidden: false
description: "Migrated from v6 (all types mapped)"
cluster_permissions:
- "*"
index_permissions:
- index_patterns:
- "*"
allowed_actions:
- "DATASTREAM_ALL"
- "indices:data/write/index"
- "indices:data/write/bulk*"

data_stream_4:
reserved: true
hidden: false
description: "Migrated from v6 (all types mapped)"
Expand All @@ -1129,6 +1143,135 @@ data_stream_3:
allowed_actions:
- "DATASTREAM_ALL"

data_stream_admin:
reserved: true
hidden: false
description: "Migrated from v6 (all types mapped)"
cluster_permissions:
- "*"
index_permissions:
- index_patterns:
- "my-data-stream*"
allowed_actions:
- "*"

data_stream_dls_1:
reserved: true
hidden: false
description: "Migrated from v6 (all types mapped)"
cluster_permissions: []
index_permissions:
- index_patterns:
- "my-data-stream11"
dls: "{\n \"bool\": {\n \"must\": {\n \"match\": {\n \"user.id\": \"8a4f500d\"\n }\n }\n }\n}"
allowed_actions:
- "read"

data_stream_dls_2:
reserved: true
hidden: false
description: "Migrated from v6 (all types mapped)"
cluster_permissions: []
index_permissions:
- index_patterns:
- "my-data-stream2*"
dls: "{\n \"bool\": {\n \"must\": {\n \"match\": {\n \"user.id\": \"8a4f500d\"\n }\n }\n }\n}"
allowed_actions:
- "read"

data_stream_dls_3:
reserved: true
hidden: false
description: "Migrated from v6 (all types mapped)"
cluster_permissions: []
index_permissions:
- index_patterns:
- "my-data-stream*"
dls: "{\n \"bool\": {\n \"must\": {\n \"match\": {\n \"user.id\": \"8a4f500d\"\n }\n }\n }\n}"
allowed_actions:
- "read"

data_stream_fls_1:
reserved: true
hidden: false
description: "Migrated from v6 (all types mapped)"
cluster_permissions: []
index_permissions:
- index_patterns:
- "my-data-stream11"
fls:
- "user.id"
- "message"
allowed_actions:
- "read"

data_stream_fls_2:
reserved: true
hidden: false
description: "Migrated from v6 (all types mapped)"
cluster_permissions: []
index_permissions:
- index_patterns:
- "my-data-stream2*"
fls:
- "user.id"
- "user.name"
allowed_actions:
- "read"

data_stream_fls_3:
reserved: true
hidden: false
description: "Migrated from v6 (all types mapped)"
cluster_permissions: []
index_permissions:
- index_patterns:
- "my-data-stream*"
fls:
- "~message"
allowed_actions:
- "read"

data_stream_fm_1:
reserved: true
hidden: false
description: "Migrated from v6 (all types mapped)"
cluster_permissions: []
index_permissions:
- index_patterns:
- "my-data-stream11"
masked_fields:
- "message"
allowed_actions:
- "read"

data_stream_fm_2:
reserved: true
hidden: false
description: "Migrated from v6 (all types mapped)"
cluster_permissions: []
index_permissions:
- index_patterns:
- "my-data-stream2*"
masked_fields:
- "message"
allowed_actions:
- "read"

data_stream_fm_3:
reserved: true
hidden: false
description: "Migrated from v6 (all types mapped)"
cluster_permissions: []
index_permissions:
- index_patterns:
- "my-data-stream*"
masked_fields:
- "user.name"
- "message"
allowed_actions:
- "read"

point_in_time_1:
reserved: true
hidden: false
Expand Down Expand Up @@ -1157,6 +1300,20 @@ point_in_time_2:
allowed_actions:
- "manage_point_in_time"

point_in_time_3:
reserved: true
hidden: false
description: "Migrated from v6 (all types mapped)"
index_permissions:
- index_patterns:
- "my-data-stream31"
- "pit_3"
dls: null
fls: null
masked_fields: null
allowed_actions:
- "manage_point_in_time"

point_in_time_all:
reserved: true
hidden: false
Expand Down
Loading

0 comments on commit 58a9f6d

Please sign in to comment.