diff --git a/integ-test/src/test/java/org/opensearch/sql/security/SQLCursorPermissionsIT.java b/integ-test/src/test/java/org/opensearch/sql/security/SQLCursorPermissionsIT.java new file mode 100644 index 00000000000..cacec3b2623 --- /dev/null +++ b/integ-test/src/test/java/org/opensearch/sql/security/SQLCursorPermissionsIT.java @@ -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). + * + *

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); + } +} diff --git a/legacy/src/main/java/org/opensearch/sql/legacy/cursor/DefaultCursor.java b/legacy/src/main/java/org/opensearch/sql/legacy/cursor/DefaultCursor.java index 166266ca79a..859e2bae31c 100644 --- a/legacy/src/main/java/org/opensearch/sql/legacy/cursor/DefaultCursor.java +++ b/legacy/src/main/java/org/opensearch/sql/legacy/cursor/DefaultCursor.java @@ -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(); /** @@ -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 */ @@ -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)); @@ -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)); diff --git a/legacy/src/main/java/org/opensearch/sql/legacy/executor/cursor/CursorResultExecutor.java b/legacy/src/main/java/org/opensearch/sql/legacy/executor/cursor/CursorResultExecutor.java index 8adffea526e..462237f1f09 100644 --- a/legacy/src/main/java/org/opensearch/sql/legacy/executor/cursor/CursorResultExecutor.java +++ b/legacy/src/main/java/org/opensearch/sql/legacy/executor/cursor/CursorResultExecutor.java @@ -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(); diff --git a/legacy/src/main/java/org/opensearch/sql/legacy/executor/format/PrettyFormatRestExecutor.java b/legacy/src/main/java/org/opensearch/sql/legacy/executor/format/PrettyFormatRestExecutor.java index 8ef4b1396a0..c74d20a239a 100644 --- a/legacy/src/main/java/org/opensearch/sql/legacy/executor/format/PrettyFormatRestExecutor.java +++ b/legacy/src/main/java/org/opensearch/sql/legacy/executor/format/PrettyFormatRestExecutor.java @@ -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) { diff --git a/legacy/src/test/java/org/opensearch/sql/legacy/unittest/cursor/DefaultCursorTest.java b/legacy/src/test/java/org/opensearch/sql/legacy/unittest/cursor/DefaultCursorTest.java index 5a5840b4469..216c057a1e7 100644 --- a/legacy/src/test/java/org/opensearch/sql/legacy/unittest/cursor/DefaultCursorTest.java +++ b/legacy/src/test/java/org/opensearch/sql/legacy/unittest/cursor/DefaultCursorTest.java @@ -8,6 +8,7 @@ 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; @@ -15,7 +16,9 @@ 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; @@ -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))); + } }