Hardens input validation for resource sharing APIs

CHANGELOG.md

@@ -26,6 +26,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 - Add support for X509 v3 extensions (SAN) for authentication  ([#5701](https://github.com/opensearch-project/security/pull/5701))
 - [Resource Sharing] Requires default_owner for resource/migrate API ([#5789](https://github.com/opensearch-project/security/pull/5789))
 - Add --timeout (-to) as an option to securityadmin.sh ([#5787](https://github.com/opensearch-project/security/pull/5787))
+- Hardens input validation for resource sharing APIs ([#5831](https://github.com/opensearch-project/security/pull/5831)
 
 ### Bug Fixes
 - Create a WildcardMatcher.NONE when creating a WildcardMatcher with an empty string ([#5694](https://github.com/opensearch-project/security/pull/5694))

sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/securityapis/AccessibleResourcesApiTests.java

@@ -74,13 +74,14 @@ public void clearIndices() {
     }
 
     @Test
-    @SuppressWarnings("unchecked")
     public void testListAccessibleResources_gibberishParams() {
         try (TestRestClient client = cluster.getRestClient(USER_ADMIN)) {
             TestRestClient.HttpResponse response = client.get(SECURITY_LIST_ENDPOINT + "?resource_type=" + "some-type");
-            response.assertStatusCode(HttpStatus.SC_OK);
-            List<Object> types = (List<Object>) response.bodyAsMap().get("resources");
-            assertThat(types.size(), equalTo(0));
+            response.assertStatusCode(HttpStatus.SC_BAD_REQUEST);
+            assertThat(
+                response.getBody(),
+                containsString("Invalid resource type: some-type. Must be one of: [sample-resource, sample-resource-group]")
+            );
         }
     }
 

sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/securityapis/MigrateApiTests.java

@@ -401,7 +401,185 @@ public void testMigrateAPIWithSuperAdmin_invalidDefaultAccessLevel() {
                 migrateResponse,
                 RestMatchers.isBadRequest(
                     "/message",
-                    "Invalid access level blah for resource sharing for resource type [" + RESOURCE_TYPE + "]"
+                    "Invalid access level blah for resource type ["
+                        + RESOURCE_TYPE
+                        + "]. Allowed: sample_read_write, sample_read_only, sample_full_access"
+                )
+            );
+        }
+    }
+
+    @Test
+    public void testMigrateAPI_inputValidation_invalidValues() {
+        // Ensure there is at least one resource so migration can proceed to validation
+        createSampleResource();
+
+        try (TestRestClient client = cluster.getRestClient(cluster.getAdminCertificate())) {
+
+            // ------------------------------
+            // 1) Invalid username_path (whitespace)
+            // ------------------------------
+            String invalidUserPathPayload = """
+                {
+                  "source_index": "%s",
+                  "username_path": " /user",
+                  "backend_roles_path": "/user/backend_roles",
+                  "default_owner": "some_user",
+                  "default_access_level": {
+                    "%s": "sample_read_only"
+                  }
+                }
+                """.formatted(RESOURCE_INDEX_NAME, RESOURCE_TYPE);
+
+            TestRestClient.HttpResponse response = client.postJson(RESOURCE_SHARING_MIGRATION_ENDPOINT, invalidUserPathPayload);
+
+            assertThat(response, RestMatchers.isBadRequest("/message", "username_path must not contain whitespace"));
+
+            // ------------------------------
+            // 2) Invalid backend_roles_path (whitespace)
+            // ------------------------------
+            String invalidBackendPathPayload = """
+                {
+                  "source_index": "%s",
+                  "username_path": "created_by.user",
+                  "backend_roles_path": "created_by. backend_roles",
+                  "default_owner": "some_user",
+                  "default_access_level": {
+                    "%s": "sample_read_only"
+                  }
+                }
+                """.formatted(RESOURCE_INDEX_NAME, RESOURCE_TYPE);
+
+            response = client.postJson(RESOURCE_SHARING_MIGRATION_ENDPOINT, invalidBackendPathPayload);
+
+            assertThat(response, RestMatchers.isBadRequest("/message", "backend_roles_path must not contain whitespace"));
+
+            // ------------------------------
+            // 3) Invalid default_owner (bad characters)
+            // ------------------------------
+            String invalidDefaultOwnerPayload = """
+                {
+                  "source_index": "%s",
+                  "username_path": "created_by.user",
+                  "backend_roles_path": "created_by.backend_roles",
+                  "default_owner": "owner name",
+                  "default_access_level": {
+                    "%s": "sample_read_only"
+                  }
+                }
+                """.formatted(RESOURCE_INDEX_NAME, RESOURCE_TYPE);
+
+            response = client.postJson(RESOURCE_SHARING_MIGRATION_ENDPOINT, invalidDefaultOwnerPayload);
+
+            assertThat(
+                response,
+                RestMatchers.isBadRequest("/message", "default_owner contains invalid characters; allowed: A-Z a-z 0-9 _ - :")
+            );
+
+            // ------------------------------
+            // 4) default_access_level is NOT an object
+            // ------------------------------
+            String defaultAccessNotObjectPayload = """
+                {
+                  "source_index": "%s",
+                  "username_path": "created_by.user",
+                  "backend_roles_path": "created_by.backend_roles",
+                  "default_owner": "some_user",
+                  "default_access_level": "sample_read_only"
+                }
+                """.formatted(RESOURCE_INDEX_NAME);
+
+            response = client.postJson(RESOURCE_SHARING_MIGRATION_ENDPOINT, defaultAccessNotObjectPayload);
+
+            assertThat(response, RestMatchers.isBadRequest("/reason", "Wrong datatype"));
+
+            // ------------------------------
+            // 5) default_access_level is an empty object {}
+            // ------------------------------
+            String defaultAccessEmptyObjectPayload = """
+                {
+                  "source_index": "%s",
+                  "username_path": "created_by.user",
+                  "backend_roles_path": "created_by.backend_roles",
+                  "default_owner": "some_user",
+                  "default_access_level": { }
+                }
+                """.formatted(RESOURCE_INDEX_NAME);
+
+            response = client.postJson(RESOURCE_SHARING_MIGRATION_ENDPOINT, defaultAccessEmptyObjectPayload);
+
+            assertThat(response, RestMatchers.isBadRequest("/message", "default_access_level cannot be empty"));
+
+            // ------------------------------
+            // 6) default_access_level has empty value for a type
+            // ------------------------------
+            String defaultAccessEmptyValuePayload = """
+                {
+                  "source_index": "%s",
+                  "username_path": "created_by.user",
+                  "backend_roles_path": "created_by.backend_roles",
+                  "default_owner": "some_user",
+                  "default_access_level": {
+                    "%s": ""
+                  }
+                }
+                """.formatted(RESOURCE_INDEX_NAME, RESOURCE_TYPE);
+
+            response = client.postJson(RESOURCE_SHARING_MIGRATION_ENDPOINT, defaultAccessEmptyValuePayload);
+
+            assertThat(
+                response,
+                RestMatchers.isBadRequest("/message", "default_access_level for type [" + RESOURCE_TYPE + "] must be a non-empty string")
+            );
+
+            // ------------------------------
+            // 7) Invalid source_index (not in protected types)
+            // ------------------------------
+            String invalidSourceIndexPayload = """
+                {
+                  "source_index": "some-other-index",
+                  "username_path": "created_by.user",
+                  "backend_roles_path": "created_by.backend_roles",
+                  "default_owner": "some_user",
+                  "default_access_level": {
+                    "%s": "sample_read_only"
+                  }
+                }
+                """.formatted(RESOURCE_TYPE);
+
+            response = client.postJson(RESOURCE_SHARING_MIGRATION_ENDPOINT, invalidSourceIndexPayload);
+
+            assertThat(
+                response,
+                RestMatchers.isBadRequest(
+                    "/message",
+                    "Invalid resource index [some-other-index]. Allowed indices: [" + RESOURCE_INDEX_NAME + "]"
+                )
+            );
+
+            // ------------------------------
+            // 8) Invalid access level value for valid type
+            // (this exercises validateAccessLevel + last try/catch)
+            // ------------------------------
+            String invalidAccessLevelPayload = """
+                {
+                  "source_index": "%s",
+                  "username_path": "created_by.user",
+                  "backend_roles_path": "created_by.backend_roles",
+                  "default_owner": "some_user",
+                  "default_access_level": {
+                    "%s": "blah"
+                  }
+                }
+                """.formatted(RESOURCE_INDEX_NAME, RESOURCE_TYPE);
+
+            response = client.postJson(RESOURCE_SHARING_MIGRATION_ENDPOINT, invalidAccessLevelPayload);
+
+            assertThat(
+                response,
+                RestMatchers.isBadRequest(
+                    "/message",
+                    "Invalid access level blah for resource type [sample-resource]. Allowed: sample_read_write, sample_read_only, sample_full_access"
                 )
             );
         }

sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/securityapis/ShareApiTests.java

@@ -95,7 +95,7 @@ public void testGibberishPayload() {
                     SECURITY_SHARE_ENDPOINT + "?resource_id=" + "some-id" + "&resource_type=" + RESOURCE_TYPE
                 );
                 response.assertStatusCode(HttpStatus.SC_FORBIDDEN); // since resource-index exists but resource-id doesn't, but user
-                                                                    // shouldn't know that
+                // shouldn't know that
 
                 response = client.get(SECURITY_SHARE_ENDPOINT + "?resource_id=" + adminResId + "&resource_type=" + "some-type");
                 response.assertStatusCode(HttpStatus.SC_BAD_REQUEST); // since type doesn't exist, so does the corresponding index
@@ -108,7 +108,7 @@ public void testGibberishPayload() {
                     putSharingInfoPayload("some-id", RESOURCE_TYPE, SAMPLE_READ_ONLY, Recipient.USERS, NO_ACCESS_USER.getName())
                 );
                 response.assertStatusCode(HttpStatus.SC_FORBIDDEN); // since resource-index exists but resource-id doesn't, but user
-                                                                    // shouldn't know that
+                // shouldn't know that
 
                 response = client.putJson(
                     SECURITY_SHARE_ENDPOINT,
@@ -342,6 +342,102 @@ public void testPostSharingInfo() {
                 response.assertStatusCode(HttpStatus.SC_FORBIDDEN);
             }
         }
-    }
 
+        @Test
+        public void testShareApi_putAndPatch_invalidPayloadValidation() {
+            // Use admin so we actually hit payload parsing & validation, not auth failures
+            try (TestRestClient client = cluster.getRestClient(USER_ADMIN)) {
+
+                // 1) PUT with invalid resource_id (invalid chars)
+                String putInvalidResourceIdPayload = """
+                    {
+                      "resource_id": "invalid id",   // space not allowed
+                      "resource_type": "%s",
+                      "share_with": {
+                        "%s": {
+                          "users": ["%s"]
+                        }
+                      }
+                    }
+                    """.formatted(RESOURCE_TYPE, SAMPLE_READ_ONLY, NO_ACCESS_USER.getName());
+
+                TestRestClient.HttpResponse response = client.putJson(SECURITY_SHARE_ENDPOINT, putInvalidResourceIdPayload);
+                response.assertStatusCode(HttpStatus.SC_BAD_REQUEST);
+                assertThat(response.getBody(), containsString("resource_id"));
+                assertThat(response.getBody(), containsString("contains invalid characters"));
+
+                // 2) PUT with invalid principal value (users entry contains space)
+                String putInvalidUserPayload = """
+                    {
+                      "resource_id": "%s",
+                      "resource_type": "%s",
+                      "share_with": {
+                        "%s": {
+                          "users": ["invalid user"]
+                        }
+                      }
+                    }
+                    """.formatted(adminResId, RESOURCE_TYPE, SAMPLE_READ_ONLY);
+
+                response = client.putJson(SECURITY_SHARE_ENDPOINT, putInvalidUserPayload);
+                response.assertStatusCode(HttpStatus.SC_BAD_REQUEST);
+                assertThat(response.getBody(), containsString("users"));
+                assertThat(response.getBody(), containsString("contains invalid characters"));
+
+                // 3) PUT with invalid access level key
+                String putInvalidAccessLevelPayload = """
+                    {
+                      "resource_id": "%s",
+                      "resource_type": "%s",
+                      "share_with": {
+                        "blah": {
+                          "users": ["%s"]
+                        }
+                      }
+                    }
+                    """.formatted(adminResId, RESOURCE_TYPE, NO_ACCESS_USER.getName());
+
+                response = client.putJson(SECURITY_SHARE_ENDPOINT, putInvalidAccessLevelPayload);
+                response.assertStatusCode(HttpStatus.SC_BAD_REQUEST);
+                assertThat(response.getBody(), containsString("access_level must be one of:"));
+                assertThat(response.getBody(), not(containsString("blah")));
+
+                // 4) PATCH with invalid principal value (same validation path as PUT, via Recipients)
+                String patchInvalidUserPayload = """
+                    {
+                      "resource_id": "%s",
+                      "resource_type": "%s",
+                      "add": {
+                        "%s": {
+                          "users": ["invalid user"]
+                        }
+                      }
+                    }
+                    """.formatted(adminResId, RESOURCE_TYPE, SAMPLE_READ_ONLY);
+
+                response = client.patch(SECURITY_SHARE_ENDPOINT, patchInvalidUserPayload);
+                response.assertStatusCode(HttpStatus.SC_BAD_REQUEST);
+                assertThat(response.getBody(), containsString("users"));
+                assertThat(response.getBody(), containsString("contains invalid characters"));
+
+                // 5) PATCH with invalid access level key
+                String patchInvalidAccessLevelPayload = """
+                    {
+                      "resource_id": "%s",
+                      "resource_type": "%s",
+                      "add": {
+                        "blah": {
+                          "users": ["%s"]
+                        }
+                      }
+                    }
+                    """.formatted(adminResId, RESOURCE_TYPE, NO_ACCESS_USER.getName());
+
+                response = client.patch(SECURITY_SHARE_ENDPOINT, patchInvalidAccessLevelPayload);
+                response.assertStatusCode(HttpStatus.SC_BAD_REQUEST);
+                assertThat(response.getBody(), containsString("access_level must be one of:"));
+                assertThat(response.getBody(), not(containsString("blah")));
+            }
+        }
+    }
 }

src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java

@@ -708,7 +708,14 @@ public List<RestHandler> getRestHandlers(
                     new ShareRestAction(resourcePluginInfo, resourceSharingEnabledSetting, resourceSharingProtectedResourceTypesSetting)
                 );
                 handlers.add(new ResourceTypesRestAction(resourcePluginInfo, resourceSharingEnabledSetting));
-                handlers.add(new AccessibleResourcesRestAction(resourceAccessHandler, resourcePluginInfo, resourceSharingEnabledSetting));
+                handlers.add(
+                    new AccessibleResourcesRestAction(
+                        resourceAccessHandler,
+                        resourcePluginInfo,
+                        resourceSharingEnabledSetting,
+                        resourceSharingProtectedResourceTypesSetting
+                    )
+                );
 
             }
             log.debug("Added {} rest handler(s)", handlers.size());