Improve array validator to also check for blank string in addition to null

CHANGELOG.md

@@ -13,6 +13,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 
 ### Bug Fixes
 - Create a WildcardMatcher.NONE when creating a WildcardMatcher with an empty string ([#5694](https://github.com/opensearch-project/security/pull/5694))
+- Improve array validator to also check for blank string in addition to null ([#5714](https://github.com/opensearch-project/security/pull/5714))
 
 ### Refactoring
 

src/integrationTest/java/org/opensearch/security/api/AbstractConfigEntityApiIntegrationTest.java

@@ -199,7 +199,11 @@ void creationOfReadOnlyEntityForbidden(final String entityName, final TestRestCl
     }
 
     void assertNullValuesInArray(final TestRestClient.HttpResponse response) throws Exception {
-        assertThat(response.getBody(), response.getTextFromJsonBody("/reason"), equalTo("`null` is not allowed as json array element"));
+        assertThat(
+            response.getBody(),
+            response.getTextFromJsonBody("/reason"),
+            equalTo("`null` or blank values are not allowed as json array elements")
+        );
     }
 
     void assertInvalidKeys(final TestRestClient.HttpResponse response, final String expectedInvalidKeys) {

src/integrationTest/java/org/opensearch/security/api/InternalUsersRestApiIntegrationTest.java

@@ -254,6 +254,12 @@ void invalidJson(final TestRestClient client, final String predefinedUserName) t
                 (builder, params) -> builder.startObject().field("backend_roles", configJsonArray(generateArrayValues(true))).endObject()
             )
         );
+        assertNullValuesInArray(
+            client.putJson(
+                apiPath(randomAsciiAlphanumOfLength(10)),
+                (builder, params) -> builder.startObject().field("backend_roles", configJsonArray("a", "")).endObject()
+            )
+        );
         // patch
         badRequest(() -> client.patch(apiPath(), patch(addOp(randomAsciiAlphanumOfLength(10), EMPTY_BODY))));
         badRequest(

src/integrationTest/java/org/opensearch/security/api/RolesMappingRestApiIntegrationTest.java

@@ -270,6 +270,7 @@ void verifyCrudOperations(Boolean hidden, Boolean reserved, TestRestClient clien
         ok(() -> client.patch(apiPath(roleName), patch(addOp("users", configJsonArray("g", "h")))));
         ok(() -> client.patch(apiPath(roleName), patch(addOp("and_backend_roles", configJsonArray("i", "j")))));
         ok(() -> client.patch(apiPath(roleName), patch(addOp("and_backend_roles", configJsonArray("i", "j")))), "No updates required");
+        badRequest(() -> client.patch(apiPath(roleName), patch(replaceOp("backend_roles", configJsonArray("c", "")))));
 
         ok(() -> client.patch(apiPath(), patch(removeOp(roleName))));
         notFound(() -> client.get(apiPath(roleName)));

src/main/java/org/opensearch/security/dlic/rest/validation/RequestContentValidator.java

@@ -238,7 +238,7 @@ private ValidationResult<JsonNode> nullValuesInArrayValidator(final JsonNode jso
         for (final Map.Entry<String, DataType> allowedKey : validationContext.allowedKeys().entrySet()) {
             JsonNode value = jsonContent.get(allowedKey.getKey());
             if (value != null) {
-                if (hasNullArrayElement(value)) {
+                if (hasNullOrBlankArrayElement(value)) {
                     this.validationError = ValidationError.NULL_ARRAY_ELEMENT;
                     return ValidationResult.error(RestStatus.BAD_REQUEST, this);
                 }
@@ -247,14 +247,14 @@ private ValidationResult<JsonNode> nullValuesInArrayValidator(final JsonNode jso
         return ValidationResult.success(jsonContent);
     }
 
-    private boolean hasNullArrayElement(final JsonNode node) {
+    private boolean hasNullOrBlankArrayElement(final JsonNode node) {
         for (final JsonNode element : node) {
-            if (element.isNull()) {
+            if (element.isNull() || (element.isTextual() && Strings.isNullOrEmpty(element.asText().trim()))) {
                 if (node.isArray()) {
                     return true;
                 }
             } else if (element.isContainerNode()) {
-                if (hasNullArrayElement(element)) {
+                if (hasNullOrBlankArrayElement(element)) {
                     return true;
                 }
             }
@@ -380,7 +380,7 @@ public enum ValidationError {
         PAYLOAD_NOT_ALLOWED("Request body not allowed for this action."),
         PAYLOAD_MANDATORY("Request body required for this action."),
         SECURITY_NOT_INITIALIZED("Security index not initialized"),
-        NULL_ARRAY_ELEMENT("`null` is not allowed as json array element");
+        NULL_ARRAY_ELEMENT("`null` or blank values are not allowed as json array elements");
 
         private final String message;
 

src/test/java/org/opensearch/security/dlic/rest/api/AuditApiActionTest.java

@@ -686,18 +686,38 @@ public void testPatchRequest() throws Exception {
     }
 
     private String getTestPayload() {
-        return "{"
-            + "\"enabled\":true,"
-            + "\"audit\":{"
-            + "\"enable_rest\":true,\"disabled_rest_categories\":[\"AUTHENTICATED\"],"
-            + "\"enable_transport\":true,\"disabled_transport_categories\":[\"SSL_EXCEPTION\"],"
-            + "\"resolve_bulk_requests\":true,\"log_request_body\":true,\"resolve_indices\":true,\"exclude_sensitive_headers\":true,"
-            + "\"ignore_users\":[\"test-user-1\"],\"ignore_requests\":[\"test-request\"], \"ignore_headers\":[\"\"], \"ignore_url_params\":[]},"
-            + "\"compliance\":{"
-            + "\"enabled\":true,"
-            + "\"internal_config\":true,\"external_config\":true,"
-            + "\"read_metadata_only\":true,\"read_watched_fields\":{\"test-read-watch-field\":[]},\"read_ignore_users\":[\"test-user-2\"],"
-            + "\"write_metadata_only\":true,\"write_log_diffs\":true,\"write_watched_indices\":[\"test-write-watch-index\"],\"write_ignore_users\":[\"test-user-3\"]}"
-            + "}";
+        return """
+            {
+              "enabled": true,
+              "audit": {
+                "enable_rest": true,
+                "disabled_rest_categories": ["AUTHENTICATED"],
+                "enable_transport": true,
+                "disabled_transport_categories": ["SSL_EXCEPTION"],
+                "resolve_bulk_requests": true,
+                "log_request_body": true,
+                "resolve_indices": true,
+                "exclude_sensitive_headers": true,
+                "ignore_users": ["test-user-1"],
+                "ignore_requests": ["test-request"],
+                "ignore_headers": [],
+                "ignore_url_params": []
+              },
+              "compliance": {
+                "enabled": true,
+                "internal_config": true,
+                "external_config": true,
+                "read_metadata_only": true,
+                "read_watched_fields": {
+                  "test-read-watch-field": []
+                },
+                "read_ignore_users": ["test-user-2"],
+                "write_metadata_only": true,
+                "write_log_diffs": true,
+                "write_watched_indices": ["test-write-watch-index"],
+                "write_ignore_users": ["test-user-3"]
+              }
+            }
+            """;
     }
 }