From 428e11e204492d91c91161ee5b6aa7838abb28f3 Mon Sep 17 00:00:00 2001
From: Darshit Chanpura <dchanp@amazon.com>
Date: Fri, 13 Dec 2024 01:43:22 -0500
Subject: [PATCH] Adds input validation

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>
---
 .../resources/ResourceAccessHandler.java      | 32 +++++++++++++++++++
 .../ResourceSharingIndexHandler.java          |  1 -
 2 files changed, 32 insertions(+), 1 deletion(-)

diff --git a/src/main/java/org/opensearch/security/resources/ResourceAccessHandler.java b/src/main/java/org/opensearch/security/resources/ResourceAccessHandler.java
index 6837f88cbf..41b999c009 100644
--- a/src/main/java/org/opensearch/security/resources/ResourceAccessHandler.java
+++ b/src/main/java/org/opensearch/security/resources/ResourceAccessHandler.java
@@ -59,6 +59,9 @@ public ResourceAccessHandler(
      * @return A set of accessible resource IDs.
      */
     public <T> Set<T> getAccessibleResourcesForCurrentUser(String resourceIndex, Class<T> clazz) {
+        if (areArgumentsInvalid(resourceIndex, clazz)) {
+            return Collections.emptySet();
+        }
         final User user = threadContext.getPersistent(ConfigConstants.OPENDISTRO_SECURITY_USER);
         if (user == null) {
             LOGGER.info("Unable to fetch user details ");
@@ -100,6 +103,9 @@ public <T> Set<T> getAccessibleResourcesForCurrentUser(String resourceIndex, Cla
      * @return True if the user has the specified permission, false otherwise.
      */
     public boolean hasPermission(String resourceId, String resourceIndex, String scope) {
+        if (areArgumentsInvalid(resourceId, resourceIndex, scope)) {
+            return false;
+        }
         final User user = threadContext.getPersistent(ConfigConstants.OPENDISTRO_SECURITY_USER);
 
         LOGGER.info("Checking if {} has {} permission to resource {}", user.getName(), scope, resourceId);
@@ -139,6 +145,9 @@ public boolean hasPermission(String resourceId, String resourceIndex, String sco
      * @return The updated ResourceSharing document.
      */
     public ResourceSharing shareWith(String resourceId, String resourceIndex, ShareWith shareWith) {
+        if (areArgumentsInvalid(resourceId, resourceIndex, shareWith)) {
+            return null;
+        }
         final User user = threadContext.getPersistent(ConfigConstants.OPENDISTRO_SECURITY_USER);
         LOGGER.info("Sharing resource {} created by {} with {}", resourceId, user.getName(), shareWith.toString());
 
@@ -162,6 +171,9 @@ public ResourceSharing revokeAccess(
         Map<EntityType, Set<String>> revokeAccess,
         Set<String> scopes
     ) {
+        if (areArgumentsInvalid(resourceId, resourceIndex, revokeAccess, scopes)) {
+            return null;
+        }
         final User user = threadContext.getPersistent(ConfigConstants.OPENDISTRO_SECURITY_USER);
         LOGGER.info("User {} revoking access to resource {} for {} for scopes {} ", user.getName(), resourceId, revokeAccess, scopes);
 
@@ -178,6 +190,9 @@ public ResourceSharing revokeAccess(
      * @return True if the record was successfully deleted, false otherwise.
      */
     public boolean deleteResourceSharingRecord(String resourceId, String resourceIndex) {
+        if (areArgumentsInvalid(resourceId, resourceIndex)) {
+            return false;
+        }
         final User user = threadContext.getPersistent(ConfigConstants.OPENDISTRO_SECURITY_USER);
         LOGGER.info("Deleting resource sharing record for resource {} in {} created by {}", resourceId, resourceIndex, user.getName());
 
@@ -198,6 +213,7 @@ public boolean deleteResourceSharingRecord(String resourceId, String resourceInd
      * @return True if all records were successfully deleted, false otherwise.
      */
     public boolean deleteAllResourceSharingRecordsForCurrentUser() {
+
         final User user = threadContext.getPersistent(ConfigConstants.OPENDISTRO_SECURITY_USER);
         LOGGER.info("Deleting all resource sharing records for resource {}", user.getName());
 
@@ -308,4 +324,20 @@ private boolean checkSharing(ResourceSharing document, EntityType entityType, St
             .orElse(false); // Return false if no matching scope is found
     }
 
+    private boolean areArgumentsInvalid(Object... args) {
+        if (args == null) {
+            return true;
+        }
+        for (Object arg : args) {
+            if (arg == null) {
+                return true;
+            }
+            // Additional check for String type arguments
+            if (arg instanceof String && ((String) arg).trim().isEmpty()) {
+                return true;
+            }
+        }
+        return false;
+    }
+
 }
diff --git a/src/main/java/org/opensearch/security/resources/ResourceSharingIndexHandler.java b/src/main/java/org/opensearch/security/resources/ResourceSharingIndexHandler.java
index c0f6ea2bd0..839af57f9c 100644
--- a/src/main/java/org/opensearch/security/resources/ResourceSharingIndexHandler.java
+++ b/src/main/java/org/opensearch/security/resources/ResourceSharingIndexHandler.java
@@ -1151,7 +1151,6 @@ public boolean deleteAllRecordsForUser(String name) {
      * @return A set of deserialized documents.
      */
     private <T> Set<T> getResourcesFromIds(Set<String> resourceIds, String resourceIndex, Class<T> clazz) {
-
         Set<T> result = new HashSet<>();
         // stashing Context to avoid permission issues in-case resourceIndex is a system index
         try (ThreadContext.StoredContext ctx = this.threadPool.getThreadContext().stashContext()) {