Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
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
@@ -0,0 +1,192 @@
/*
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*/

package org.opensearch.sql.security;

import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_ACCOUNT;

import java.io.IOException;
import java.util.Base64;
import java.util.Locale;
import org.json.JSONObject;
import org.junit.Test;
import org.opensearch.client.Request;
import org.opensearch.client.RequestOptions;
import org.opensearch.client.Response;
import org.opensearch.sql.legacy.SQLIntegTestCase;
import org.opensearch.sql.legacy.TestUtils;

/**
* Regression test for SQL cursor pagination under Fine-Grained Access Control (FGAC).
*
* <p>Exercises the legacy V1 cursor path (triggered by {@code SELECT ... LIMIT n} with {@code
* fetch_size}). Before the fix, page 2 would return 403 because the continuation SearchRequest was
* created with no indices, which Security resolves to a wildcard and denies under FGAC.
*/
public class SQLCursorPermissionsIT extends SQLIntegTestCase {

private static final String ACCOUNT_USER = "account_cursor_user";
private static final String ACCOUNT_ROLE = "account_cursor_role";
private static final String STRONG_PASSWORD = "StrongPassword123!";

private boolean initialized = false;

@Override
protected void init() throws Exception {
loadIndex(Index.ACCOUNT);
createSecurityRolesAndUsers();
}

private void createSecurityRolesAndUsers() throws IOException {
if (initialized) {
return;
}
createRole(ACCOUNT_ROLE, TEST_INDEX_ACCOUNT);
createUser(ACCOUNT_USER, ACCOUNT_ROLE);
initialized = true;
}

private void createRole(String roleName, String indexPattern) throws IOException {
Request request = new Request("PUT", "/_plugins/_security/api/roles/" + roleName);
request.setJsonEntity(
String.format(
Locale.ROOT,
"""
{
"cluster_permissions": [
"cluster:admin/opensearch/ppl",
"cluster:admin/opensearch/sql"
],
"index_permissions": [{
"index_patterns": [
"%s"
],
"allowed_actions": [
"indices:data/read/search*",
"indices:admin/mappings/get",
"indices:admin/mappings/fields/get*",
"indices:monitor/settings/get",
"indices:data/read/point_in_time/create",
"indices:data/read/point_in_time/delete"
]
}]
}
""",
indexPattern));
RequestOptions.Builder opts = RequestOptions.DEFAULT.toBuilder();
opts.addHeader("Content-Type", "application/json");
request.setOptions(opts);

Response response = client().performRequest(request);
int status = response.getStatusLine().getStatusCode();
assertTrue(status == 200 || status == 201);
}

private void createUser(String username, String roleName) throws IOException {
Request userRequest = new Request("PUT", "/_plugins/_security/api/internalusers/" + username);
userRequest.setJsonEntity(
String.format(
Locale.ROOT,
"""
{
"password": "%s",
"backend_roles": [],
"attributes": {}
}
""",
STRONG_PASSWORD));
RequestOptions.Builder opts = RequestOptions.DEFAULT.toBuilder();
opts.addHeader("Content-Type", "application/json");
userRequest.setOptions(opts);

Response userResponse = client().performRequest(userRequest);
int userStatus = userResponse.getStatusLine().getStatusCode();
assertTrue(userStatus == 200 || userStatus == 201);

Request mappingRequest = new Request("PUT", "/_plugins/_security/api/rolesmapping/" + roleName);
mappingRequest.setJsonEntity(
String.format(
Locale.ROOT,
"""
{
"backend_roles": [],
"hosts": [],
"users": ["%s"]
}
""",
username));
mappingRequest.setOptions(opts);

Response mappingResponse = client().performRequest(mappingRequest);
int mappingStatus = mappingResponse.getStatusLine().getStatusCode();
assertTrue(mappingStatus == 200 || mappingStatus == 201);
}

private JSONObject executeSqlAsUser(String body, String username) throws IOException {
Request request = new Request("POST", "/_plugins/_sql");
request.setJsonEntity(body);
RequestOptions.Builder opts = RequestOptions.DEFAULT.toBuilder();
opts.addHeader("Content-Type", "application/json");
opts.addHeader(
"Authorization",
"Basic "
+ Base64.getEncoder().encodeToString((username + ":" + STRONG_PASSWORD).getBytes()));
request.setOptions(opts);

Response response = client().performRequest(request);
assertEquals(200, response.getStatusLine().getStatusCode());
return new JSONObject(TestUtils.getResponseBody(response, true));
}

@Test
public void simpleSelectUnderFgacSucceeds() throws IOException {
JSONObject result =
executeSqlAsUser(
String.format(
Locale.ROOT,
"{\"query\": \"SELECT firstname FROM %s LIMIT 1\"}",
TEST_INDEX_ACCOUNT),
ACCOUNT_USER);
assertTrue(result.has("datarows"));
}

/**
* Regression for SQL cursor pagination under FGAC. Triggers the V1 cursor path (LIMIT with
* fetch_size) and advances through multiple continuation pages. Before the fix, page 2 returned
* 403 because the continuation SearchRequest carried no indices, which Security resolves to a
* wildcard and denies.
*/
@Test
public void cursorPaginationUnderFgacSucceedsAcrossPages() throws IOException {
// LIMIT forces the V1 cursor path (V2's CanPaginateVisitor rejects LIMIT). The V1 path is
// the one that constructs the continuation SearchRequest without indices, which Security
// denies under FGAC before this fix.
JSONObject firstPage =
executeSqlAsUser(
String.format(
Locale.ROOT,
"{\"fetch_size\": 50, \"query\": \"SELECT age, balance FROM %s LIMIT 234\"}",
TEST_INDEX_ACCOUNT),
ACCOUNT_USER);
assertTrue("first page must include a cursor; body=" + firstPage, firstPage.has("cursor"));
String cursor = firstPage.getString("cursor");
assertFalse("first page cursor must not be empty", cursor.isEmpty());
assertTrue(
"expected V1 cursor (prefix 'd:'), got: "
+ cursor.substring(0, Math.min(6, cursor.length())),
cursor.startsWith("d:"));

int pages = 1;
while (!cursor.isEmpty()) {
JSONObject next =
executeSqlAsUser(
String.format(Locale.ROOT, "{\"cursor\": \"%s\"}", cursor), ACCOUNT_USER);
cursor = next.optString("cursor", "");
pages++;
}
// 234 rows / 50 per page = 5 pages
assertEquals("expected 5 V1 cursor pages under FGAC", 5, pages);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ public class DefaultCursor implements Cursor {
private static final String PIT_ID = "p";
private static final String SEARCH_REQUEST = "r";
private static final String SORT_FIELDS = "h";
private static final String INDICES = "x";
private static final ObjectMapper objectMapper = new ObjectMapper();

/**
Expand All @@ -69,6 +70,13 @@ public class DefaultCursor implements Cursor {
*/
@NonNull private String indexPattern;

/**
* Concrete index names from the original query's FROM clause. Used to scope continuation
* SearchRequests so Security FGAC authorizes against the same indices as page 1 instead of a
* wildcard.
*/
private String[] indices;

/**
* List of Schema.Column for maintaining field order and generating null values of missing fields
*/
Expand Down Expand Up @@ -137,6 +145,7 @@ public String generateCursorId() {
throw new RuntimeException("Failed to serialize sort fields to JSON string.", e);
}
json.put(SORT_FIELDS, sortFieldValue);
json.put(INDICES, new JSONArray(indices == null ? new String[0] : indices));
setSearchRequestString(json, searchSourceBuilder);

return String.format("%s:%s", type.getId(), encodeCursor(json));
Expand Down Expand Up @@ -173,10 +182,23 @@ public static DefaultCursor from(String cursorId) {
populateCursorForPit(json, cursor);
cursor.setColumns(getColumnsFromSchema(json.getJSONArray(SCHEMA_COLUMNS)));
cursor.setFieldAliasMap(fieldAliasMap(json.getJSONObject(FIELD_ALIAS_MAP)));
cursor.setIndices(getIndicesFromJson(json));

return cursor;
}

private static String[] getIndicesFromJson(JSONObject json) {
JSONArray arr = json.optJSONArray(INDICES);
if (arr == null) {
return new String[0];
}
String[] result = new String[arr.length()];
for (int i = 0; i < arr.length(); i++) {
result[i] = arr.getString(i);
}
return result;
}

private static void populateCursorForPit(JSONObject json, DefaultCursor cursor) {
cursor.setPitId(json.getString(PIT_ID));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,10 @@ private String handleDefaultCursorRequest(Client client, DefaultCursor cursor) {
SearchSourceBuilder source = cursor.getSearchSourceBuilder();
source.searchAfter(cursor.getSortFields());
source.pointInTimeBuilder(new PointInTimeBuilder(pitId));
SearchRequest searchRequest = new SearchRequest();
// Scope continuation to the original query's indices; an empty-indices SearchRequest
// resolves to a wildcard under Security FGAC and gets denied on page 2.
String[] indices = cursor.getIndices();
SearchRequest searchRequest = new SearchRequest(indices == null ? new String[0] : indices);
searchRequest.source(source);
scrollResponse = client.search(searchRequest).actionGet();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ private DefaultCursor createCursorWithPit(
cursor.setLimit(queryAction.getSelect().getRowCount());
cursor.setFetchSize(fetchSize);
cursor.setPitId(pit.getPitId());
cursor.setIndices(queryAction.getSelect().getIndexArr());
cursor.setSearchSourceBuilder(queryAction.getRequestBuilder().request().source());

if (response.getHits().getHits().length > 0) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,17 @@
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.emptyOrNullString;
import static org.hamcrest.Matchers.startsWith;
import static org.junit.Assert.assertArrayEquals;
import static org.junit.Assert.assertEquals;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.when;

import java.io.ByteArrayOutputStream;
import java.util.ArrayList;
import java.util.Base64;
import java.util.Collections;
import org.json.JSONObject;
import org.junit.Before;
import org.junit.Test;
import org.mockito.Mock;
Expand Down Expand Up @@ -88,4 +91,93 @@ public void nullCursorWhenScrollIDIsNullOrEmpty() {
cursor.setScrollId("");
assertThat(cursor.generateCursorId(), emptyOrNullString());
}

@Test
public void indicesAreSerializedIntoCursorId() {
DefaultCursor cursor = new DefaultCursor();
cursor.setRowsLeft(50);
cursor.setPitId("pit-id");
cursor.setIndexPattern("idx1|idx2");
cursor.setFetchSize(500);
cursor.setFieldAliasMap(Collections.emptyMap());
cursor.setColumns(new ArrayList<>());
cursor.setIndices(new String[] {"idx1", "idx2"});
cursor.setSearchSourceBuilder(sourceBuilder);

String cursorId = cursor.generateCursorId();
JSONObject decoded = decodePayload(cursorId);

assertEquals(2, decoded.getJSONArray("x").length());
assertEquals("idx1", decoded.getJSONArray("x").getString(0));
assertEquals("idx2", decoded.getJSONArray("x").getString(1));
}

@Test
public void nullIndicesAreSerializedAsEmptyArray() {
DefaultCursor cursor = new DefaultCursor();
cursor.setRowsLeft(50);
cursor.setPitId("pit-id");
cursor.setIndexPattern("idx1");
cursor.setFetchSize(500);
cursor.setFieldAliasMap(Collections.emptyMap());
cursor.setColumns(new ArrayList<>());
cursor.setSearchSourceBuilder(sourceBuilder);

String cursorId = cursor.generateCursorId();
JSONObject decoded = decodePayload(cursorId);

assertEquals(0, decoded.getJSONArray("x").length());
}

@Test
public void deserializeRoundTripsIndices() {
SearchSourceBuilder realSource = new SearchSourceBuilder();
DefaultCursor cursor = new DefaultCursor();
cursor.setRowsLeft(50);
cursor.setPitId("pit-id");
cursor.setIndexPattern("idx1|idx2");
cursor.setFetchSize(500);
cursor.setFieldAliasMap(Collections.emptyMap());
cursor.setColumns(new ArrayList<>());
cursor.setIndices(new String[] {"idx1", "idx2"});
cursor.setSearchSourceBuilder(realSource);

String cursorId = cursor.generateCursorId();
String payload = cursorId.substring(cursorId.indexOf(':') + 1);
DefaultCursor restored = DefaultCursor.from(payload);

assertArrayEquals(new String[] {"idx1", "idx2"}, restored.getIndices());
}

@Test
public void deserializeLegacyCursorWithoutIndicesDefaultsToEmptyArray() {
// Legacy cursor payloads written before this fix do not contain the "x" field.
// They must continue to deserialize cleanly with indices == [] so in-flight
// cursors from pre-fix nodes are not rejected after upgrade.
SearchSourceBuilder realSource = new SearchSourceBuilder();
DefaultCursor cursor = new DefaultCursor();
cursor.setRowsLeft(50);
cursor.setPitId("pit-id");
cursor.setIndexPattern("idx1");
cursor.setFetchSize(500);
cursor.setFieldAliasMap(Collections.emptyMap());
cursor.setColumns(new ArrayList<>());
cursor.setIndices(new String[] {"idx1"});
cursor.setSearchSourceBuilder(realSource);

String cursorId = cursor.generateCursorId();
String payload = cursorId.substring(cursorId.indexOf(':') + 1);
JSONObject json = new JSONObject(new String(Base64.getDecoder().decode(payload)));
json.remove("x");
String legacyPayload = Base64.getEncoder().encodeToString(json.toString().getBytes());

DefaultCursor restored = DefaultCursor.from(legacyPayload);

assertArrayEquals(new String[0], restored.getIndices());
}

private JSONObject decodePayload(String cursorId) {
String payload = cursorId.substring(cursorId.indexOf(':') + 1);
return new JSONObject(new String(Base64.getDecoder().decode(payload)));
}
}
Loading