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.

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

Skip applying field masking if search request is size 0 and only contains cardinality or count aggregations #24

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,10 @@
import org.opensearch.index.query.QueryBuilder;
import org.opensearch.index.query.QueryBuilders;
import org.opensearch.search.aggregations.Aggregation;
import org.opensearch.search.aggregations.AggregationBuilders;
import org.opensearch.search.aggregations.metrics.ParsedAvg;
import org.opensearch.search.aggregations.metrics.ParsedCardinality;
import org.opensearch.search.builder.SearchSourceBuilder;
import org.opensearch.search.sort.SortOrder;
import org.opensearch.test.framework.TestSecurityConfig;
import org.opensearch.test.framework.cluster.ClusterManager;
Expand All @@ -55,6 +58,7 @@

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.allOf;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.everyItem;
import static org.hamcrest.Matchers.greaterThan;
import static org.hamcrest.Matchers.instanceOf;
Expand Down Expand Up @@ -105,6 +109,7 @@ public class FlsAndFieldMaskingTests {
static final String FIRST_INDEX_ID_SONG_2 = "INDEX_1_S2";
static final String FIRST_INDEX_ID_SONG_3 = "INDEX_1_S3";
static final String FIRST_INDEX_ID_SONG_4 = "INDEX_1_S4";
static final String FIRST_INDEX_ID_SONG_5 = "INDEX_1_S5";
static final String SECOND_INDEX_ID_SONG_1 = "INDEX_2_S1";
static final String SECOND_INDEX_ID_SONG_2 = "INDEX_2_S2";
static final String SECOND_INDEX_ID_SONG_3 = "INDEX_2_S3";
Expand Down Expand Up @@ -160,6 +165,19 @@ public class FlsAndFieldMaskingTests {
.on(SECOND_INDEX_NAME)
);

/**
* User who is allowed to see all fields on indices {@link #FIRST_INDEX_NAME} except artist
* <ul>
* <li>values of the artist fields should be masked on index {@link #FIRST_INDEX_NAME}</li>
* </ul>
*/
static final TestSecurityConfig.User MASKED_ARTIST_READER = new TestSecurityConfig.User("masked_title_lyrics_reader").roles(
new TestSecurityConfig.Role("masked_title_lyrics_reader").clusterPermissions("cluster_composite_ops_ro")
.indexPermissions("read")
.maskedFields(FIELD_ARTIST)
.on(FIRST_INDEX_NAME)
);

/**
* Function that converts field value to value masked with {@link #MASK_VALUE}
*/
Expand Down Expand Up @@ -219,7 +237,8 @@ public class FlsAndFieldMaskingTests {
MASKED_ARTIST_LYRICS_READER,
ALL_INDICES_STRING_ARTIST_READER,
ALL_INDICES_STARS_LESS_THAN_ZERO_READER,
TWINS_FIRST_ARTIST_READER
TWINS_FIRST_ARTIST_READER,
MASKED_ARTIST_READER
)
.build();

Expand Down Expand Up @@ -251,6 +270,7 @@ public class FlsAndFieldMaskingTests {
put(FIRST_INDEX_ID_SONG_2, SONGS[1]);
put(FIRST_INDEX_ID_SONG_3, SONGS[2]);
put(FIRST_INDEX_ID_SONG_4, SONGS[3]);
put(FIRST_INDEX_ID_SONG_5, SONGS[6]);
}
};

Expand Down Expand Up @@ -811,4 +831,20 @@ public void getFieldCapabilities() throws IOException {
}
}

@Test
public void getCardinalityAggregationOnMaskedField() throws IOException {
// FIELD MASKING
try (RestHighLevelClient restHighLevelClient = cluster.getRestHighLevelClient(MASKED_ARTIST_READER)) {
SearchRequest searchRequest = new SearchRequest(FIRST_INDEX_NAME);
SearchSourceBuilder sourceBuilder = new SearchSourceBuilder();
sourceBuilder.aggregation(AggregationBuilders.cardinality("unique_artists_agg").field("artist.keyword"));
sourceBuilder.size(0);
searchRequest.source(sourceBuilder);
SearchResponse response = restHighLevelClient.search(searchRequest, DEFAULT);
ParsedCardinality parsedCardinality = response.getAggregations().get("unique_artists_agg");
long cardinality = parsedCardinality.getValue();
assertThat(cardinality, equalTo(4L));
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ public class Song {
public static final String ARTIST_STRING = "String";
public static final String ARTIST_TWINS = "Twins";
public static final String TITLE_MAGNUM_OPUS = "Magnum Opus";
public static final String TITLE_ABC = "ABC";
public static final String TITLE_SONG_1_PLUS_1 = "Song 1+1";
public static final String TITLE_NEXT_SONG = "Next song";
public static final String ARTIST_NO = "No!";
Expand All @@ -41,6 +42,7 @@ public class Song {
public static final String LYRICS_4 = "Much too much";
public static final String LYRICS_5 = "Little to little";
public static final String LYRICS_6 = "confidential secret classified";
public static final String LYRICS_7 = "abcdefghijklmnopqrstuvwxyz";

public static final String GENRE_ROCK = "rock";
public static final String GENRE_JAZZ = "jazz";
Expand All @@ -56,7 +58,8 @@ public class Song {
new Song(ARTIST_TWINS, TITLE_NEXT_SONG, LYRICS_3, 3, GENRE_JAZZ),
new Song(ARTIST_NO, TITLE_POISON, LYRICS_4, 4, GENRE_ROCK),
new Song(ARTIST_YES, TITLE_AFFIRMATIVE, LYRICS_5, 5, GENRE_BLUES),
new Song(ARTIST_UNKNOWN, TITLE_CONFIDENTIAL, LYRICS_6, 6, GENRE_JAZZ) };
new Song(ARTIST_UNKNOWN, TITLE_CONFIDENTIAL, LYRICS_6, 6, GENRE_JAZZ),
new Song(ARTIST_FIRST, TITLE_ABC, LYRICS_7, 7, GENRE_ROCK), };

private final String artist;
private final String title;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -521,12 +521,30 @@ private void setFlsHeaders(EvaluatedDlsFlsConfig dlsFls, ActionRequest request)
}
}
} else {
threadContext.putHeader(
ConfigConstants.OPENDISTRO_SECURITY_MASKED_FIELD_HEADER,
Base64Helper.serializeObject((Serializable) maskedFieldsMap)
);
if (log.isDebugEnabled()) {
log.debug("attach masked fields info: {}", maskedFieldsMap);
boolean shouldIncludeFlsHeaders = true;
if (request instanceof SearchRequest) {
SearchRequest searchRequest = (SearchRequest) request;
boolean hasOnlyCountOrCardinality = true;
if (searchRequest.source().aggregations() != null) {
for (AggregationBuilder af : searchRequest.source().aggregations().getAggregatorFactories()) {
if (!(af.getType().equals("cardinality") || af.getType().equals("count"))) {
hasOnlyCountOrCardinality = false;
}
}
}
if (hasOnlyCountOrCardinality && searchRequest.source().size() == 0) {
shouldIncludeFlsHeaders = false;
}
}

if (shouldIncludeFlsHeaders) {
threadContext.putHeader(
ConfigConstants.OPENDISTRO_SECURITY_MASKED_FIELD_HEADER,
Base64Helper.serializeObject((Serializable) maskedFieldsMap)
);
if (log.isDebugEnabled()) {
log.debug("attach masked fields info: {}", maskedFieldsMap);
}
}
}
}
Expand Down
Loading