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.

Sign up for GitHub

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

[Backport 2.4] Fix issues with datastream backing indexes #2242

Merged
merged 3 commits into from
Nov 8, 2022
Merged

[Backport 2.4] Fix issues with datastream backing indexes #2242

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
77744a8
Merge pull request from GHSA-wmx7-x4jp-9jgg
Nov 7, 2022
0373028
Datastream test fixes
cwperks Nov 7, 2022
8785b25
Add pit-3 mapping
cwperks Nov 8, 2022
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
15 changes: 9 additions & 6 deletions src/main/java/org/opensearch/security/securityconf/ConfigModelV7.java
View file
Open in desktop
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
Open in desktop

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions src/test/java/org/opensearch/security/IndexTemplateClusterPermissionsCheckTest.java
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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