Adds input validation

Signed-off-by: Darshit Chanpura <[email protected]>
DarshitChanpura · Dec 13, 2024 · 428e11e · 428e11e
1 parent 3ce3d92
commit 428e11e
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 1 deletion.
diff --git 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
@@ -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()) {