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
2 changes: 1 addition & 1 deletion integ-test/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -502,11 +502,11 @@ testClusters.analyticsEngineSecurityIT {
plugin(getJobSchedulerPlugin())
plugin(getArrowBasePlugin())
plugin(getArrowFlightRpcPlugin())
plugin(getCompositeEnginePlugin())
plugin(getAnalyticsEnginePlugin())
plugin(getAnalyticsBackendLucenePlugin())
plugin(getAnalyticsBackendDatafusionPlugin())
plugin(getParquetDataFormatPlugin())
plugin(getCompositeEnginePlugin())
plugin ":opensearch-sql-plugin"
// Arrow Flight / streaming transport requirements
jvmArgs '--add-opens=java.base/java.nio=ALL-UNNAMED'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -393,6 +393,94 @@ public void testPPLQueryWithWildcardIndexPartialAccessDenied() throws IOExceptio
assertEquals(403, e.getResponse().getStatusLine().getStatusCode());
}

// --- Multi-index comma-separated source tests (FGAC bypass regression) ---

@Test
public void testPPLMultiIndexDeniedWhenSecondIndexUnauthorized() throws IOException {
ResponseException e =
assertThrows(
ResponseException.class,
() ->
executePPLAsUser(
"source = " + TEST_INDEX + ", " + FORBIDDEN_INDEX + " | fields name, age",
ALLOWED_USER));
assertEquals(403, e.getResponse().getStatusLine().getStatusCode());
}

@Test
public void testPPLMultiIndexDeniedWithBackticksAuthorizedFirst() throws IOException {
ResponseException e =
assertThrows(
ResponseException.class,
() ->
executePPLAsUser(
"source = `" + TEST_INDEX + "`, `" + FORBIDDEN_INDEX + "` | fields name, age",
ALLOWED_USER));
assertEquals(403, e.getResponse().getStatusLine().getStatusCode());
}

@Test
public void testPPLMultiIndexDeniedWithUnauthorizedFirst() throws IOException {
ResponseException e =
assertThrows(
ResponseException.class,
() ->
executePPLAsUser(
"source = " + FORBIDDEN_INDEX + ", " + TEST_INDEX + " | fields name, age",
ALLOWED_USER));
assertEquals(403, e.getResponse().getStatusLine().getStatusCode());
}

@Test
public void testPPLMultiIndexAllowedWhenAllAuthorized() throws IOException {
try {
JSONObject result =
executePPLAsUser(
"source = " + TEST_INDEX + ", " + TEST_INDEX_2 + " | fields name, age",
WILDCARD_USER);
assertTrue("Expected datarows in response", result.has("datarows"));
} catch (ResponseException e) {
assertNotEquals(
"Expected auth to pass (not 403) when all indices are authorized",
403,
e.getResponse().getStatusLine().getStatusCode());
}
}

// --- Edge cases: malformed comma-separated source patterns ---

@Test
public void testPPLDoubleCommaRejected() throws IOException {
ResponseException e =
assertThrows(
ResponseException.class,
() ->
executePPLAsUser(
"source = " + TEST_INDEX + ",," + FORBIDDEN_INDEX + " | fields name, age",
ALLOWED_USER));
assertEquals(400, e.getResponse().getStatusLine().getStatusCode());
}

@Test
public void testPPLLeadingCommaRejected() throws IOException {
ResponseException e =
assertThrows(
ResponseException.class,
() ->
executePPLAsUser("source = ," + TEST_INDEX + " | fields name, age", ALLOWED_USER));
assertEquals(400, e.getResponse().getStatusLine().getStatusCode());
}

@Test
public void testPPLTrailingCommaRejected() throws IOException {
ResponseException e =
assertThrows(
ResponseException.class,
() ->
executePPLAsUser("source = " + TEST_INDEX + ", | fields name, age", ALLOWED_USER));
assertEquals(400, e.getResponse().getStatusLine().getStatusCode());
}

@Test
public void testSQLQueryAllowedForAuthorizedUser() throws IOException {
try {
Expand Down Expand Up @@ -441,6 +529,51 @@ public void testSQLQueryDeniedWithSearchPermissionOnly() throws IOException {
assertEquals(403, e.getResponse().getStatusLine().getStatusCode());
}

// --- SQL multi-index syntax validation ---

@Test
public void testSQLMultiIndexCommaInFromRejected() throws IOException {
// SQL FROM "idx1,idx2" — comma inside identifier, should be rejected as syntax error
ResponseException e =
assertThrows(
ResponseException.class,
() ->
executeSQLAsUser(
"SELECT name, age FROM " + TEST_INDEX + "," + FORBIDDEN_INDEX + " LIMIT 3",
ALLOWED_USER));
assertEquals(400, e.getResponse().getStatusLine().getStatusCode());
}

@Test
public void testSQLMultiIndexCrossJoinRejected() throws IOException {
// SQL cross join syntax — should not bypass FGAC
ResponseException e =
assertThrows(
ResponseException.class,
() ->
executeSQLAsUser(
"SELECT a.name FROM " + TEST_INDEX + " a, " + FORBIDDEN_INDEX + " b LIMIT 3",
ALLOWED_USER));
assertEquals(400, e.getResponse().getStatusLine().getStatusCode());
}

@Test
public void testSQLMultiIndexJoinRejected() throws IOException {
// Explicit JOIN — should not bypass FGAC
ResponseException e =
assertThrows(
ResponseException.class,
() ->
executeSQLAsUser(
"SELECT a.name FROM "
+ TEST_INDEX
+ " a JOIN "
+ FORBIDDEN_INDEX
+ " b ON a.name = b.name LIMIT 3",
ALLOWED_USER));
assertEquals(400, e.getResponse().getStatusLine().getStatusCode());
}

/** Executes a PPL query via the production SQL plugin endpoint (/_plugins/_ppl). */
private JSONObject executePPLAsUser(String query, String username) throws IOException {
Request request = new Request("POST", "/_plugins/_ppl");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,13 +122,35 @@ public boolean isAnalyticsIndex(String query, QueryType queryType) {
try (UnifiedQueryContext context = buildParsingContext(queryType)) {
return extractIndexName(query, queryType, context)
.map(this::stripSchemaPrefix)
.map(this::isPluggableDataformatIndex)
.map(this::allIndicesArePluggableDataformat)
.orElse(false);
} catch (Exception e) {
return false;
}
}

/**
* Checks if all indices in a (possibly comma-separated) index expression are pluggable
* dataformat. For multi-index queries (source=idx1,idx2), each index is checked independently.
* Returns true only if every index is composite — mixed or all-lucene returns false.
*/
private boolean allIndicesArePluggableDataformat(String indexExpression) {
String[] indices = indexExpression.split(",");
if (indices.length == 0) {
return false;
}
for (String idx : indices) {
String trimmed = idx.trim();
if (trimmed.isEmpty()) {
continue;
}
if (!isPluggableDataformatIndex(trimmed)) {
return false;
}
}
return true;
}

private static boolean isSystemCatalog(String name) {
return SystemIndexUtils.isSystemIndex(name)
|| SystemIndexUtils.DATASOURCES_TABLE_NAME.equals(name);
Expand Down
Loading