[Analytics Engine] Add FGAC integration tests for aliases, wildcards, and index patterns

[finnegancarroll]

Description

Tests verify that the security plugin correctly resolves indices for AnalyticsQueryAction requests (which use the fallback path through IndexNameExpressionResolver since DefaultPlanExecutor does not implement TransportIndicesResolvingAction).

New test cases:

• Alias-based query access (allowed via alias, denied via concrete name)
• Wildcard index pattern in query (allowed when role matches all resolved indices)
• Wildcard query denied when user lacks access to resolved indices
• Partial access denied (user has alias-only access, wildcard expands beyond)

Related Issues

N/A

Check List

• New functionality includes testing.
• New functionality has been documented.
• New functionality has javadoc added.
• New functionality has a user manual doc added.
• New PPL command checklist all confirmed.
• API changes companion pull request created.
• Commits are signed per the DCO using --signoff or -s.
• Public documentation issue/PR created.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit 116d118)

Here are some key observations to aid the review process:

🧪 PR contains tests

🔒 No security concerns identified

✅ No TODO sections

🔀 No multiple PR themes

⚡ Recommended focus areas for review Incorrect Test Assertion Test testPPLQueryAllowedViaConcreteIndexForAliasUser assumes that granting access to an alias implicitly grants access to the underlying concrete index. This assumption may not hold in all OpenSearch security configurations. If the security plugin enforces strict alias-only access, querying via the concrete index name could fail with a 403, causing this test to fail. The test should either verify the actual security behavior or document this assumption as a prerequisite. public void testPPLQueryAllowedViaConcreteIndexForAliasUser() throws IOException { // ALIAS_USER's role has index_patterns: ["analytics_alias"]. In OpenSearch's security // model, granting access to an alias also implicitly grants access to the underlying // concrete index. This verifies the query succeeds via the concrete name. try { JSONObject result = executePPLAsUser("source = " + TEST_INDEX + " | fields name, age", ALIAS_USER); assertTrue("Expected datarows in response", result.has("datarows")); } catch (ResponseException e) { assertNotEquals( "Expected auth to pass (not 403) for alias user querying concrete index", 403, e.getResponse().getStatusLine().getStatusCode()); } }

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to 116d118

Explore these optional code suggestions:

Category	Suggestion	Impact
General	Reorder options builder for clarity The RequestOptions.Builder is created before the Request bulk object, but setOptions() is called after setJsonEntity(). Move the options builder creation and header addition to immediately before calling setOptions() to improve code readability and maintain logical flow. integ-test/src/test/java/org/opensearch/sql/security/AnalyticsEngineSecurityIT.java [85-102] -RequestOptions.Builder opts = RequestOptions.DEFAULT.toBuilder(); -opts.addHeader("Content-Type", "application/x-ndjson"); - Request bulk = new Request("POST", "/_bulk"); bulk.addParameter("refresh", "true"); bulk.setJsonEntity( String.format( Locale.ROOT, "{\"index\": {\"_index\": \"%s\"}}\n{\"name\": \"alice\", \"age\": 30}\n" + "{\"index\": {\"_index\": \"%s\"}}\n{\"name\": \"bob\", \"age\": 25}\n" + "{\"index\": {\"_index\": \"%s\"}}\n{\"name\": \"carol\", \"age\": 28}\n" + "{\"index\": {\"_index\": \"%s\"}}\n{\"name\": \"secret\", \"age\": 99}\n", TEST_INDEX, TEST_INDEX, TEST_INDEX_2, FORBIDDEN_INDEX)); +RequestOptions.Builder opts = RequestOptions.DEFAULT.toBuilder(); +opts.addHeader("Content-Type", "application/x-ndjson"); bulk.setOptions(opts); client().performRequest(bulk); Suggestion importance[1-10]: 3 __ Why: While the suggestion improves code readability by placing the RequestOptions.Builder creation closer to its usage, this is a minor stylistic improvement. The original code is functionally correct and the reordering has minimal impact on code quality or maintainability.	Low

---

Previous suggestions

Suggestions up to commit 87f9d76

Category	Suggestion	Impact
Possible issue	Remove incorrect exception handling The test logic is inverted. If authorization succeeds, the try block executes and the test passes. If it fails with 403, the catch block runs but assertNotEquals allows the test to pass when it should fail. This masks authorization failures. Remove the try-catch and let executePPLAsUser throw if auth fails. integ-test/src/test/java/org/opensearch/sql/security/AnalyticsEngineSecurityIT.java [264-274] -try { - JSONObject result = - executePPLAsUser("source = " + TEST_INDEX + " | fields name, age", EXACT_PERM_USER); - assertTrue("Expected datarows in response", result.has("datarows")); -} catch (ResponseException e) { - assertNotEquals( - "Expected auth to pass (not 403) for user with exact analytics/query permission", - 403, - e.getResponse().getStatusLine().getStatusCode()); -} +JSONObject result = + executePPLAsUser("source = " + TEST_INDEX + " | fields name, age", EXACT_PERM_USER); +assertTrue("Expected datarows in response", result.has("datarows")); Suggestion importance[1-10]: 9 __ Why: The try-catch logic is fundamentally flawed. When authorization fails with 403, assertNotEquals passes the test instead of failing it, masking critical security failures. This is a serious bug that undermines the test's purpose.	High
	Fix flawed authorization test logic The try-catch pattern incorrectly handles authorization failures. When auth fails with 403, the catch block's assertNotEquals passes the test instead of failing it. This defeats the purpose of verifying successful authorization. Remove the try-catch wrapper. integ-test/src/test/java/org/opensearch/sql/security/AnalyticsEngineSecurityIT.java [313-323] -try { - JSONObject result = - executePPLAsUser("source = " + TEST_ALIAS + " | fields name, age", ALIAS_USER); - assertTrue("Expected datarows in response", result.has("datarows")); -} catch (ResponseException e) { - assertNotEquals( - "Expected auth to pass (not 403) for alias-permitted user", - 403, - e.getResponse().getStatusLine().getStatusCode()); -} +JSONObject result = + executePPLAsUser("source = " + TEST_ALIAS + " | fields name, age", ALIAS_USER); +assertTrue("Expected datarows in response", result.has("datarows")); Suggestion importance[1-10]: 9 __ Why: The exception handling pattern incorrectly allows the test to pass when authorization fails with 403. This defeats the test's purpose of verifying successful alias-based authorization and represents a critical testing flaw.	High
	Correct authorization verification pattern The exception handling pattern is incorrect. If the query fails with 403, the catch block allows the test to pass via assertNotEquals, which contradicts the test's intent to verify successful authorization. Remove the try-catch to properly fail on authorization errors. integ-test/src/test/java/org/opensearch/sql/security/AnalyticsEngineSecurityIT.java [354-364] -try { - JSONObject result = - executePPLAsUser("source = analytics_security* | fields name, age", WILDCARD_USER); - assertTrue("Expected datarows in response", result.has("datarows")); -} catch (ResponseException e) { - assertNotEquals( - "Expected auth to pass (not 403) for wildcard query with matching permissions", - 403, - e.getResponse().getStatusLine().getStatusCode()); -} +JSONObject result = + executePPLAsUser("source = analytics_security* | fields name, age", WILDCARD_USER); +assertTrue("Expected datarows in response", result.has("datarows")); Suggestion importance[1-10]: 9 __ Why: The try-catch pattern allows the test to pass when authorization fails with 403, contradicting the test's intent to verify successful wildcard query authorization. This is a significant bug in the test logic.	High

Suggestions up to commit 180e13d

Category	Suggestion	Impact
General	Validate alias creation response The alias creation request lacks error handling and doesn't verify the alias was successfully created. If the alias creation fails silently, subsequent tests relying on TEST_ALIAS will fail with misleading error messages. Add response validation to ensure the alias operation succeeded. integ-test/src/test/java/org/opensearch/sql/security/AnalyticsEngineSecurityIT.java [103-112] Request aliasReq = new Request("POST", "/_aliases"); aliasReq.setJsonEntity( String.format( Locale.ROOT, """ {"actions": [{"add": {"index": "%s", "alias": "%s"}}]} """, TEST_INDEX, TEST_ALIAS)); -client().performRequest(aliasReq); +Response aliasResp = client().performRequest(aliasReq); +assertEquals(200, aliasResp.getStatusLine().getStatusCode()); Suggestion importance[1-10]: 7 __ Why: Adding response validation for alias creation improves test reliability by catching setup failures early. However, this is a test improvement rather than a critical bug fix, as test failures would still be detected (albeit with less clear error messages).	Medium

[github-actions]

Persistent review updated to latest commit 87f9d76

[github-actions]

Persistent review updated to latest commit 116d118