From c08a99273ccf3cd88d26f3421ba24c56599277c8 Mon Sep 17 00:00:00 2001 From: Darshit Chanpura Date: Mon, 9 Dec 2024 21:45:06 -0500 Subject: [PATCH] Adds super-admin bypass Signed-off-by: Darshit Chanpura --- .../resources/ResourceAccessHandler.java | 16 +++++++++++++-- .../ResourceSharingIndexHandler.java | 20 +++++++++++++------ 2 files changed, 28 insertions(+), 8 deletions(-) diff --git a/src/main/java/org/opensearch/security/resources/ResourceAccessHandler.java b/src/main/java/org/opensearch/security/resources/ResourceAccessHandler.java index d060ce6f38..721692c85a 100644 --- a/src/main/java/org/opensearch/security/resources/ResourceAccessHandler.java +++ b/src/main/java/org/opensearch/security/resources/ResourceAccessHandler.java @@ -82,8 +82,14 @@ public Set listAccessibleResourcesInPlugin(String pluginIndex) { public boolean hasPermission(String resourceId, String systemIndexName, String scope) { final User user = threadContext.getPersistent(ConfigConstants.OPENDISTRO_SECURITY_USER); + LOGGER.info("Checking if {} has {} permission to resource {}", user.getName(), scope, resourceId); + // check if user is admin, if yes the user has permission + if (adminDNs.isAdmin(user)) { + return true; + } + Set userRoles = user.getSecurityRoles(); Set userBackendRoles = user.getRoles(); @@ -110,7 +116,10 @@ public ResourceSharing shareWith(String resourceId, String systemIndexName, Shar final User user = threadContext.getPersistent(ConfigConstants.OPENDISTRO_SECURITY_USER); LOGGER.info("Sharing resource {} created by {} with {}", resourceId, user.getName(), shareWith.toString()); - return this.resourceSharingIndexHandler.updateResourceSharingInfo(resourceId, systemIndexName, user.getName(), shareWith); + // check if user is admin, if yes the user has permission + boolean isAdmin = adminDNs.isAdmin(user); + + return this.resourceSharingIndexHandler.updateResourceSharingInfo(resourceId, systemIndexName, user.getName(), shareWith, isAdmin); } public ResourceSharing revokeAccess( @@ -122,7 +131,10 @@ public ResourceSharing revokeAccess( final User user = threadContext.getPersistent(ConfigConstants.OPENDISTRO_SECURITY_USER); LOGGER.info("User {} revoking access to resource {} for {} for scopes {} ", user.getName(), resourceId, revokeAccess, scopes); - return this.resourceSharingIndexHandler.revokeAccess(resourceId, systemIndexName, revokeAccess, scopes, user.getName()); + // check if user is admin, if yes the user has permission + boolean isAdmin = adminDNs.isAdmin(user); + + return this.resourceSharingIndexHandler.revokeAccess(resourceId, systemIndexName, revokeAccess, scopes, user.getName(), isAdmin); } public boolean deleteResourceSharingRecord(String resourceId, String systemIndexName) { diff --git a/src/main/java/org/opensearch/security/resources/ResourceSharingIndexHandler.java b/src/main/java/org/opensearch/security/resources/ResourceSharingIndexHandler.java index 6f07f608c9..a4ef90e492 100644 --- a/src/main/java/org/opensearch/security/resources/ResourceSharingIndexHandler.java +++ b/src/main/java/org/opensearch/security/resources/ResourceSharingIndexHandler.java @@ -145,7 +145,6 @@ public void createResourceSharingIndexIfAbsent(Callable callable) { * @return ResourceSharing Returns resourceSharing object if the operation was successful, null otherwise * @throws IOException if there are issues with index operations or JSON processing */ - public ResourceSharing indexResourceSharing(String resourceId, String resourceIndex, CreatedBy createdBy, ShareWith shareWith) throws IOException { try { @@ -665,7 +664,7 @@ private void executeSearchRequest(Set resourceIds, Scroll scroll, Search /** * Updates the sharing configuration for an existing resource in the resource sharing index. - * NOTE: This method only grants new access. To remove access use {@link #revokeAccess(String, String, Map, Set, String)} + * NOTE: This method only grants new access. To remove access use {@link #revokeAccess(String, String, Map, Set, String, boolean)} * This method modifies the sharing permissions for a specific resource identified by its * resource ID and source index. * @@ -680,10 +679,17 @@ private void executeSearchRequest(Set resourceIds, Scroll scroll, Search * "backend_roles": ["backend_role1"] * } * } + * @param isAdmin Boolean indicating whether the user requesting to share is an admin or not * @return ResourceSharing Returns resourceSharing object if the update was successful, null otherwise * @throws RuntimeException if there's an error during the update operation */ - public ResourceSharing updateResourceSharingInfo(String resourceId, String sourceIdx, String requestUserName, ShareWith shareWith) { + public ResourceSharing updateResourceSharingInfo( + String resourceId, + String sourceIdx, + String requestUserName, + ShareWith shareWith, + boolean isAdmin + ) { XContentBuilder builder; Map shareWithMap; try { @@ -701,7 +707,7 @@ public ResourceSharing updateResourceSharingInfo(String resourceId, String sourc // Check if the user requesting to share is the owner of the resource // TODO Add a way for users who are not creators to be able to share the resource ResourceSharing currentSharingInfo = fetchDocumentById(sourceIdx, resourceId); - if (currentSharingInfo != null && !currentSharingInfo.getCreatedBy().getUser().equals(requestUserName)) { + if (!isAdmin && currentSharingInfo != null && !currentSharingInfo.getCreatedBy().getUser().equals(requestUserName)) { LOGGER.error("User {} is not authorized to share resource {}", requestUserName, resourceId); return null; } @@ -876,6 +882,7 @@ private boolean updateByQueryResourceSharing(String sourceIdx, String resourceId * values to be removed from the sharing configuration * @param scopes A list of scopes to revoke access from. If null or empty, access is revoked from all scopes * @param requestUserName The user trying to revoke the accesses + * @param isAdmin Boolean indicating whether the user is an admin or not * @return The updated ResourceSharing object after revoking access, or null if the document doesn't exist * @throws IllegalArgumentException if resourceId, sourceIdx is null/empty, or if revokeAccess is null/empty * @throws RuntimeException if the update operation fails or encounters an error @@ -898,7 +905,8 @@ public ResourceSharing revokeAccess( String sourceIdx, Map> revokeAccess, Set scopes, - String requestUserName + String requestUserName, + boolean isAdmin ) { if (StringUtils.isBlank(resourceId) || StringUtils.isBlank(sourceIdx) || revokeAccess == null || revokeAccess.isEmpty()) { throw new IllegalArgumentException("resourceId, sourceIdx, and revokeAccess must not be null or empty"); @@ -906,7 +914,7 @@ public ResourceSharing revokeAccess( // TODO Check if access can be revoked by non-creator ResourceSharing currentSharingInfo = fetchDocumentById(sourceIdx, resourceId); - if (currentSharingInfo != null && !currentSharingInfo.getCreatedBy().getUser().equals(requestUserName)) { + if (!isAdmin && currentSharingInfo != null && !currentSharingInfo.getCreatedBy().getUser().equals(requestUserName)) { LOGGER.error("User {} is not authorized to revoke access to resource {}", requestUserName, resourceId); return null; }