Skip to content

Commit

Permalink
Update indices resolution to be clearer (#1999)
Browse files Browse the repository at this point in the history 
(cherry picked from commit 42b936e)
  • Loading branch information
@peternied @github-actions
peternied authored and github-actions[bot] committed Aug 8, 2022
1 parent f0cfbbc commit 8a04d18
Show file tree
Hide file tree
Showing 6 changed files with 491 additions and 22 deletions.
51 changes: 29 additions & 22 deletions src/main/java/org/opensearch/security/securityconf/ConfigModelV7.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
import com.google.common.collect.SetMultimap;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.apache.logging.log4j.util.Strings;

import org.opensearch.ExceptionsHelper;
import org.opensearch.action.support.IndicesOptions;
Expand Down Expand Up @@ -379,8 +380,7 @@ public EvaluatedDlsFlsConfig getDlsFls(User user, boolean dfmEmptyOverwritesAll,

for (SecurityRole role : roles) {
for (IndexPattern ip : role.getIpatterns()) {
Set<String> concreteIndices;
concreteIndices = ip.getResolvedIndexPattern(user, resolver, cs, false);
final Set<String> concreteIndices = ip.concreteIndexNames(user, resolver, cs);
String dls = ip.getDlsQuery(user);

if (dls != null && dls.length() > 0) {
Expand Down Expand Up @@ -561,7 +561,7 @@ private Set<String> getAllResolvedPermittedIndices(Resolved resolved, User user,
// }
if (patternMatch) {
//resolved but can contain patterns for nonexistent indices
final WildcardMatcher permitted = WildcardMatcher.from(p.getResolvedIndexPattern(user, resolver, cs, true)); //maybe they do not exist
final WildcardMatcher permitted = WildcardMatcher.from(p.attemptResolveIndexNames(user, resolver, cs)); //maybe they do not exist
final Set<String> res = new HashSet<>();
if (!resolved.isLocalAll() && !resolved.getAllIndices().contains("*") && !resolved.getAllIndices().contains("_all")) {
//resolved but can contain patterns for nonexistent indices
Expand Down Expand Up @@ -753,35 +753,42 @@ public String getUnresolvedIndexPattern(User user) {
return replaceProperties(indexPattern, user);
}

public Set<String> getResolvedIndexPattern(User user, IndexNameExpressionResolver resolver, ClusterService cs, boolean appendUnresolved) {
String unresolved = getUnresolvedIndexPattern(user);
WildcardMatcher matcher = WildcardMatcher.from(unresolved);
String[] resolved = null;
/** Finds the indices accessible to the user and resolves them to concrete names */
public Set<String> concreteIndexNames(final User user, final IndexNameExpressionResolver resolver, final ClusterService cs) {
return getResolvedIndexPattern(user, resolver, cs, false);
}

/** Finds the indices accessible to the user and attempts to resolve them to names, also includes any unresolved names */
public Set<String> attemptResolveIndexNames(final User user, final IndexNameExpressionResolver resolver, final ClusterService cs) {
return getResolvedIndexPattern(user, resolver, cs, true);
}

public Set<String> getResolvedIndexPattern(final User user, final IndexNameExpressionResolver resolver, final ClusterService cs, final boolean appendUnresolved) {
final String unresolved = getUnresolvedIndexPattern(user);
final ImmutableSet.Builder<String> resolvedIndices = new ImmutableSet.Builder<>();

final WildcardMatcher matcher = WildcardMatcher.from(unresolved);
if (!(matcher instanceof WildcardMatcher.Exact)) {
final String[] aliasesForPermittedPattern = cs.state().getMetadata().getIndicesLookup().entrySet().stream()
.filter(e -> e.getValue().getType() == ALIAS)
.filter(e -> matcher.test(e.getKey()))
.map(e -> e.getKey())
.toArray(String[]::new);

if (aliasesForPermittedPattern.length > 0) {
resolved = resolver.concreteIndexNames(cs.state(), IndicesOptions.lenientExpandOpen(), aliasesForPermittedPattern);
final String[] resolvedAliases = resolver.concreteIndexNames(cs.state(), IndicesOptions.lenientExpandOpen(), aliasesForPermittedPattern);
resolvedIndices.addAll(Arrays.asList(resolvedAliases));
}
}

if (resolved == null && !unresolved.isEmpty()) {
resolved = resolver.concreteIndexNames(cs.state(), IndicesOptions.lenientExpandOpen(), unresolved);
if (Strings.isNotBlank(unresolved)) {
final String[] resolvedIndicesFromPattern = resolver.concreteIndexNames(cs.state(), IndicesOptions.lenientExpandOpen(), unresolved);
resolvedIndices.addAll(Arrays.asList(resolvedIndicesFromPattern));
}
if (resolved == null || resolved.length == 0) {
return ImmutableSet.of(unresolved);
} else {
ImmutableSet.Builder<String> builder = ImmutableSet.<String>builder()
.addAll(Arrays.asList(resolved));
if (appendUnresolved) {
builder.add(unresolved);
}
return builder.build();

if (appendUnresolved || resolvedIndices.build().isEmpty()) {
resolvedIndices.add(unresolved);
}
return resolvedIndices.build();
}

public String getDlsQuery(User user) {
Expand Down Expand Up @@ -996,12 +1003,12 @@ private static boolean impliesTypePerm(Set<IndexPattern> ipatterns, Resolved res
indexMatcherAndPermissions = ipatterns
.stream()
.filter(indexPattern -> "*".equals(indexPattern.getUnresolvedIndexPattern(user)))
.map(p -> new IndexMatcherAndPermissions(p.getResolvedIndexPattern(user, resolver, cs, true), p.perms))
.map(p -> new IndexMatcherAndPermissions(p.attemptResolveIndexNames(user, resolver, cs), p.perms))
.toArray(IndexMatcherAndPermissions[]::new);
} else {
indexMatcherAndPermissions = ipatterns
.stream()
.map(p -> new IndexMatcherAndPermissions(p.getResolvedIndexPattern(user, resolver, cs, true), p.perms))
.map(p -> new IndexMatcherAndPermissions(p.attemptResolveIndexNames(user, resolver, cs), p.perms))
.toArray(IndexMatcherAndPermissions[]::new);
}
return resolvedRequestedIndices
Expand Down
160 changes: 160 additions & 0 deletions src/test/java/org/opensearch/security/dlic/dlsfls/FlsIndexingTests.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,160 @@
/*
* SPDX-License-Identifier: Apache-2.0
*
* The OpenSearch Contributors require contributions made to
* this file be licensed under the Apache-2.0 license or a
* compatible open source license.
*
* Modifications Copyright OpenSearch Contributors. See
* GitHub history for details.
*/

package org.opensearch.security.dlic.dlsfls;

import org.apache.http.Header;
import org.apache.http.HttpStatus;
import org.junit.Test;

import org.opensearch.action.index.IndexRequest;
import org.opensearch.action.support.WriteRequest.RefreshPolicy;
import org.opensearch.client.Client;
import org.opensearch.common.xcontent.XContentType;
import org.opensearch.security.test.DynamicSecurityConfig;
import org.opensearch.security.test.helper.rest.RestHelper.HttpResponse;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.core.IsEqual.equalTo;
import static org.hamcrest.core.IsNot.not;
import static org.hamcrest.core.StringContains.containsString;

public class FlsIndexingTests extends AbstractDlsFlsTest {

protected void populateData(final Client tc) {
// Create several documents in different indices with shared field names,
// different roles will have different levels of FLS restrictions
tc.index(new IndexRequest("yellow-pages").id("1").setRefreshPolicy(RefreshPolicy.IMMEDIATE)
.source("{\"phone-all\":1001,\"phone-some\":1002,\"phone-one\":1003}", XContentType.JSON)).actionGet();
tc.index(new IndexRequest("green-pages").id("2").setRefreshPolicy(RefreshPolicy.IMMEDIATE)
.source("{\"phone-all\":2001,\"phone-some\":2002,\"phone-one\":2003}", XContentType.JSON)).actionGet();
tc.index(new IndexRequest("blue-book").id("3").setRefreshPolicy(RefreshPolicy.IMMEDIATE)
.source("{\"phone-all\":3001,\"phone-some\":3002,\"phone-one\":3003}", XContentType.JSON)).actionGet();

// Seperate index used to test aliasing
tc.index(new IndexRequest(".hidden").id("1").setRefreshPolicy(RefreshPolicy.IMMEDIATE)
.source("{}", XContentType.JSON)).actionGet();
}

private Header asPhoneOneUser = encodeBasicHeader("user_aaa", "password");
private Header asPhoneSomeUser = encodeBasicHeader("user_bbb", "password");
private Header asPhoneAllUser = encodeBasicHeader("user_ccc", "password");

private final String searchQuery = "/*/_search?filter_path=hits.hits&pretty";

@Test
public void testSingleIndexFlsApplied() throws Exception {
setup(new DynamicSecurityConfig()
.setSecurityRoles("roles_fls_indexing.yml")
.setSecurityRolesMapping("roles_mapping_fls_indexing.yml"));

final HttpResponse phoneOneFilteredResponse = rh.executeGetRequest(searchQuery, asPhoneOneUser);
assertThat(phoneOneFilteredResponse.getStatusCode(), equalTo(HttpStatus.SC_OK));
assertThat(phoneOneFilteredResponse.getBody(), not(containsString("1003")));
assertThat(phoneOneFilteredResponse.getBody(), containsString("1002"));
assertThat(phoneOneFilteredResponse.getBody(), containsString("1001"));

assertThat(phoneOneFilteredResponse.getBody(), containsString("2003"));
assertThat(phoneOneFilteredResponse.getBody(), containsString("2002"));
assertThat(phoneOneFilteredResponse.getBody(), containsString("2001"));

assertThat(phoneOneFilteredResponse.getBody(), containsString("3003"));
assertThat(phoneOneFilteredResponse.getBody(), containsString("3002"));
assertThat(phoneOneFilteredResponse.getBody(), containsString("3001"));
}

@Test
public void testSingleIndexFlsAppliedForLimitedResults() throws Exception {
setup(new DynamicSecurityConfig()
.setSecurityRoles("roles_fls_indexing.yml")
.setSecurityRolesMapping("roles_mapping_fls_indexing.yml"));

final HttpResponse phoneOneFilteredResponse = rh.executeGetRequest("/yellow-pages/_search?filter_path=hits.hits&pretty", asPhoneOneUser);
assertThat(phoneOneFilteredResponse.getStatusCode(), equalTo(HttpStatus.SC_OK));
assertThat(phoneOneFilteredResponse.getBody(), not(containsString("1003")));
assertThat(phoneOneFilteredResponse.getBody(), containsString("1002"));
assertThat(phoneOneFilteredResponse.getBody(), containsString("1001"));

assertThat(phoneOneFilteredResponse.getBody(), not(containsString("2003")));
assertThat(phoneOneFilteredResponse.getBody(), not(containsString("2002")));
assertThat(phoneOneFilteredResponse.getBody(), not(containsString("2001")));

assertThat(phoneOneFilteredResponse.getBody(), not(containsString("3003")));
assertThat(phoneOneFilteredResponse.getBody(), not(containsString("3002")));
assertThat(phoneOneFilteredResponse.getBody(), not(containsString("3001")));
}

@Test
public void testSeveralIndexFlsApplied() throws Exception {
setup(new DynamicSecurityConfig()
.setSecurityRoles("roles_fls_indexing.yml")
.setSecurityRolesMapping("roles_mapping_fls_indexing.yml"));

final HttpResponse phoneSomeFilteredResponse = rh.executeGetRequest(searchQuery, asPhoneSomeUser);
assertThat(phoneSomeFilteredResponse.getStatusCode(), equalTo(HttpStatus.SC_OK));
assertThat(phoneSomeFilteredResponse.getBody(), containsString("1003"));
assertThat(phoneSomeFilteredResponse.getBody(), not(containsString("1002")));
assertThat(phoneSomeFilteredResponse.getBody(), containsString("1001"));

assertThat(phoneSomeFilteredResponse.getBody(), containsString("2003"));
assertThat(phoneSomeFilteredResponse.getBody(), not(containsString("2002")));
assertThat(phoneSomeFilteredResponse.getBody(), containsString("2001"));

assertThat(phoneSomeFilteredResponse.getBody(), containsString("3003"));
assertThat(phoneSomeFilteredResponse.getBody(), containsString("3002"));
assertThat(phoneSomeFilteredResponse.getBody(), containsString("3001"));
}

@Test
public void testAllIndexFlsApplied() throws Exception {
setup(new DynamicSecurityConfig()
.setSecurityRoles("roles_fls_indexing.yml")
.setSecurityRolesMapping("roles_mapping_fls_indexing.yml"));

final HttpResponse phoneAllFilteredResponse = rh.executeGetRequest(searchQuery, asPhoneAllUser);
assertThat(phoneAllFilteredResponse.getStatusCode(), equalTo(HttpStatus.SC_OK));
assertThat(phoneAllFilteredResponse.getBody(), containsString("1003"));
assertThat(phoneAllFilteredResponse.getBody(), containsString("1002"));
assertThat(phoneAllFilteredResponse.getBody(), not(containsString("1001")));

assertThat(phoneAllFilteredResponse.getBody(), containsString("2003"));
assertThat(phoneAllFilteredResponse.getBody(), containsString("2002"));
assertThat(phoneAllFilteredResponse.getBody(), not(containsString("2001")));

assertThat(phoneAllFilteredResponse.getBody(), containsString("3003"));
assertThat(phoneAllFilteredResponse.getBody(), containsString("3002"));
assertThat(phoneAllFilteredResponse.getBody(), not(containsString("3001")));
}

@Test
public void testAllIndexFlsAppliedWithAlias() throws Exception {
setup(new DynamicSecurityConfig()
.setSecurityRoles("roles_fls_indexing.yml")
.setSecurityRolesMapping("roles_mapping_fls_indexing.yml"));

final HttpResponse createAlias = rh.executePostRequest("_aliases", "{\"actions\":[{\"add\":{\"index\":\".hidden\",\"alias\":\"ducky\"}}]}", asPhoneAllUser);
assertThat(createAlias.getStatusCode(), equalTo(HttpStatus.SC_OK));

final HttpResponse phoneAllFilteredResponse = rh.executeGetRequest(searchQuery, asPhoneAllUser);
assertThat(phoneAllFilteredResponse.getStatusCode(), equalTo(HttpStatus.SC_OK));
assertThat(phoneAllFilteredResponse.getBody(), containsString("1003"));
assertThat(phoneAllFilteredResponse.getBody(), containsString("1002"));
assertThat(phoneAllFilteredResponse.getBody(), not(containsString("1001")));

assertThat(phoneAllFilteredResponse.getBody(), containsString("2003"));
assertThat(phoneAllFilteredResponse.getBody(), containsString("2002"));
assertThat(phoneAllFilteredResponse.getBody(), not(containsString("2001")));

assertThat(phoneAllFilteredResponse.getBody(), containsString("3003"));
assertThat(phoneAllFilteredResponse.getBody(), containsString("3002"));
assertThat(phoneAllFilteredResponse.getBody(), not(containsString("3001")));
}
}
Loading

0 comments on commit 8a04d18

Please sign in to comment.