diff --git a/integ-test/src/test/java/org/opensearch/sql/security/AnalyticsEngineSecurityIT.java b/integ-test/src/test/java/org/opensearch/sql/security/AnalyticsEngineSecurityIT.java index 4de5aa208b3..a9e36e9b470 100644 --- a/integ-test/src/test/java/org/opensearch/sql/security/AnalyticsEngineSecurityIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/security/AnalyticsEngineSecurityIT.java @@ -263,12 +263,6 @@ public void testSQLQueryAllowedForAuthorizedUser() throws IOException { } } - // TODO: The SQL endpoint (/_plugins/_sql) returns 500 instead of 403 for security exceptions. - // The legacy RestSqlAction error handling wraps OpenSearchSecurityException as a generic 500 - // Internal Server Error rather than propagating the 403 Forbidden status. The authorization - // IS denied (query does not execute), but the HTTP status is incorrect. These tests accept - // either 403 or 500 until the SQL plugin's error propagation is fixed. - @Test public void testSQLQueryDeniedForUnauthorizedUser() throws IOException { ResponseException e = @@ -276,11 +270,7 @@ public void testSQLQueryDeniedForUnauthorizedUser() throws IOException { ResponseException.class, () -> executeSQLAsUser("SELECT name, age FROM " + TEST_INDEX + " LIMIT 3", DENIED_USER)); - assertTrue( - "Expected 403 or 500 with security exception, got " - + e.getResponse().getStatusLine().getStatusCode(), - e.getResponse().getStatusLine().getStatusCode() == 403 - || e.getResponse().getStatusLine().getStatusCode() == 500); + assertEquals(403, e.getResponse().getStatusLine().getStatusCode()); } @Test @@ -291,11 +281,7 @@ public void testSQLQueryDeniedForForbiddenIndex() throws IOException { () -> executeSQLAsUser( "SELECT name, age FROM " + FORBIDDEN_INDEX + " LIMIT 3", ALLOWED_USER)); - assertTrue( - "Expected 403 or 500 with security exception, got " - + e.getResponse().getStatusLine().getStatusCode(), - e.getResponse().getStatusLine().getStatusCode() == 403 - || e.getResponse().getStatusLine().getStatusCode() == 500); + assertEquals(403, e.getResponse().getStatusLine().getStatusCode()); } @Test @@ -306,11 +292,7 @@ public void testSQLQueryDeniedWithSearchPermissionOnly() throws IOException { () -> executeSQLAsUser( "SELECT name, age FROM " + TEST_INDEX + " LIMIT 3", SEARCH_ONLY_USER)); - assertTrue( - "Expected 403 or 500 with security exception, got " - + e.getResponse().getStatusLine().getStatusCode(), - e.getResponse().getStatusLine().getStatusCode() == 403 - || e.getResponse().getStatusLine().getStatusCode() == 500); + assertEquals(403, e.getResponse().getStatusLine().getStatusCode()); } /** Executes a PPL query via the production SQL plugin endpoint (/_plugins/_ppl). */ diff --git a/legacy/src/main/java/org/opensearch/sql/legacy/plugin/RestSqlAction.java b/legacy/src/main/java/org/opensearch/sql/legacy/plugin/RestSqlAction.java index 2950f6c9b85..bf3ebad7d7f 100644 --- a/legacy/src/main/java/org/opensearch/sql/legacy/plugin/RestSqlAction.java +++ b/legacy/src/main/java/org/opensearch/sql/legacy/plugin/RestSqlAction.java @@ -210,7 +210,7 @@ private void handleException(RestChannel restChannel, Exception exception) { reportError(restChannel, exception, status); } - private static RestStatus getRestStatus(Exception ex) { + public static RestStatus getRestStatus(Exception ex) { int code = getRawErrorCode(ex); return RestStatus.fromCode(code); } diff --git a/plugin/src/main/java/org/opensearch/sql/plugin/SQLPlugin.java b/plugin/src/main/java/org/opensearch/sql/plugin/SQLPlugin.java index 3692f49688d..57095c96790 100644 --- a/plugin/src/main/java/org/opensearch/sql/plugin/SQLPlugin.java +++ b/plugin/src/main/java/org/opensearch/sql/plugin/SQLPlugin.java @@ -264,7 +264,7 @@ protected Object buildJsonObject(ExplainResponse resp) { @Override public void onFailure(Exception e) { channel.sendResponse( - new BytesRestResponse(RestStatus.INTERNAL_SERVER_ERROR, e.getMessage())); + new BytesRestResponse(RestSqlAction.getRestStatus(e), e.getMessage())); } }); } else { @@ -283,7 +283,7 @@ public void onResponse(TransportPPLQueryResponse response) { @Override public void onFailure(Exception e) { channel.sendResponse( - new BytesRestResponse(RestStatus.INTERNAL_SERVER_ERROR, e.getMessage())); + new BytesRestResponse(RestSqlAction.getRestStatus(e), e.getMessage())); } }); }