From 4a7302e77422b0f9248469bddfae56f25edcd582 Mon Sep 17 00:00:00 2001 From: Florent MILLOT <75525996+flomillot@users.noreply.github.com> Date: Thu, 27 Aug 2026 10:35:35 +0200 Subject: [PATCH 1/3] Add an endpoint filtering the elements a user can access The existing /elements/authorized answers all-or-nothing: a single forbidden element denies the whole request. Resolving the permissions of several independent elements therefore took one call each. The new /elements/permission returns which of the given elements the user may access, leaving out the forbidden and the unknown ones. The user groups are resolved once for the whole batch instead of once per element. Signed-off-by: Florent MILLOT <75525996+flomillot@users.noreply.github.com> --- .../directory/server/DirectoryController.java | 11 ++++ .../server/services/PermissionService.java | 35 ++++++++++++- .../server/PermissionServiceTest.java | 51 +++++++++++++++++++ 3 files changed, 95 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/gridsuite/directory/server/DirectoryController.java b/src/main/java/org/gridsuite/directory/server/DirectoryController.java index c148bf2f..1c2d4994 100644 --- a/src/main/java/org/gridsuite/directory/server/DirectoryController.java +++ b/src/main/java/org/gridsuite/directory/server/DirectoryController.java @@ -236,6 +236,17 @@ public ResponseEntity areElementsAccessible(@RequestParam("ids") List> getAccessibleElements(@RequestParam("ids") List elementUuids, + @RequestParam(value = "accessType") PermissionType permissionType, + @RequestHeader("userId") String userId) { + return ResponseEntity.ok().body(permissionService.filterAccessibleElements(userId, elementUuids, permissionType)); + } + @GetMapping(value = "/directories/{directoryUuid}/permissions", produces = MediaType.APPLICATION_JSON_VALUE) @Operation(summary = "Get permissions for the directory") @ApiResponses(value = { diff --git a/src/main/java/org/gridsuite/directory/server/services/PermissionService.java b/src/main/java/org/gridsuite/directory/server/services/PermissionService.java index d63c212c..f32f5cfd 100644 --- a/src/main/java/org/gridsuite/directory/server/services/PermissionService.java +++ b/src/main/java/org/gridsuite/directory/server/services/PermissionService.java @@ -17,6 +17,7 @@ import org.gridsuite.directory.server.repository.PermissionRepository; import org.springframework.stereotype.Service; import java.util.*; +import java.util.function.Supplier; import java.util.stream.Collectors; import static org.gridsuite.directory.server.DirectoryService.DIRECTORY; import static org.gridsuite.directory.server.dto.PermissionType.MANAGE; @@ -70,6 +71,29 @@ public void checkDirectoriesPermission(String userId, List elementUuids, U } } + /** + * Tells which of the given elements the user may access. + * + * @param userId User ID checking permissions for + * @param elementUuids List of element UUIDs to check permissions on + * @param permissionType Type of permission to check (READ, WRITE, MANAGE) + * @return the uuids of the accessible elements, in no particular order + */ + public List filterAccessibleElements(String userId, List elementUuids, PermissionType permissionType) { + boolean isExploreAdmin = roleService.isUserExploreAdmin(); + //Resolved once for the whole batch: hasElementPermission would otherwise query user-admin-server for + //every single element. + List userGroupIds = isExploreAdmin ? List.of() : getUserGroupIds(userId); + return directoryElementRepository.findAllByIdIn(elementUuids).stream() + //If it's a directory we check its own permission else we check the permission on its parent directory + .filter(element -> isExploreAdmin || hasElementPermission(userId, + element.getType().equals(DIRECTORY) ? element.getId() : element.getParentId(), + permissionType, + () -> userGroupIds)) + .map(DirectoryElementEntity::getId) + .toList(); + } + public boolean hasReadPermissions(String userId, List elementUuids) { return roleService.isUserExploreAdmin() || directoryElementRepository.findAllByIdIn(elementUuids).stream().allMatch(element -> //If it's a directory we check its own write permission else we check the permission on the element parent directory @@ -204,6 +228,10 @@ private boolean checkPermission(String userId, List elementUuids, Permissi } private boolean hasElementPermission(String userId, UUID uuid, PermissionType permissionType) { + return hasElementPermission(userId, uuid, permissionType, () -> getUserGroupIds(userId)); + } + + private boolean hasElementPermission(String userId, UUID uuid, PermissionType permissionType, Supplier> userGroupIds) { //Check global permission first boolean globalPermission = checkPermission(permissionRepository.findById(new PermissionId(uuid, ALL_USERS, "")), permissionType); if (globalPermission) { @@ -217,14 +245,17 @@ private boolean hasElementPermission(String userId, UUID uuid, PermissionType pe } //Finally check group permission - return userAdminService.getUserGroups(userId) + return userGroupIds.get() .stream() - .map(UserGroupDTO::id) .anyMatch(groupId -> checkPermission(permissionRepository.findById(new PermissionId(uuid, "", groupId.toString())), permissionType) ); } + private List getUserGroupIds(String userId) { + return userAdminService.getUserGroups(userId).stream().map(UserGroupDTO::id).toList(); + } + private boolean checkPermission(Optional permissionEntity, PermissionType permissionType) { return permissionEntity .map(p -> switch (permissionType) { diff --git a/src/test/java/org/gridsuite/directory/server/PermissionServiceTest.java b/src/test/java/org/gridsuite/directory/server/PermissionServiceTest.java index aa50434b..24c3ea76 100644 --- a/src/test/java/org/gridsuite/directory/server/PermissionServiceTest.java +++ b/src/test/java/org/gridsuite/directory/server/PermissionServiceTest.java @@ -47,6 +47,7 @@ import java.io.UnsupportedEncodingException; import java.util.*; import java.util.stream.Collectors; +import static org.assertj.core.api.Assertions.assertThat; import static org.gridsuite.directory.server.DirectoryService.DIRECTORY; import static org.gridsuite.directory.server.dto.PermissionType.READ; import static org.gridsuite.directory.server.dto.PermissionType.WRITE; @@ -504,6 +505,56 @@ private void grantGroupPermission(UUID directoryUuid, UUID groupId, PermissionTy permissionRepository.save(permission); } + @Test + void testFilterAccessibleElements() throws Exception { + UUID openDir = insertRootDirectory(ADMIN_USER, "openDir"); + UUID restrictedDir = insertRootDirectory(ADMIN_USER, "restrictedDir"); + + UUID openElement = insertSubElement(openDir, toElementAttributes(null, "openElement", TYPE_01, ADMIN_USER)); + UUID restrictedElement = insertSubElement(restrictedDir, toElementAttributes(null, "restrictedElement", TYPE_01, ADMIN_USER)); + UUID unknownElement = UUID.randomUUID(); + + // Only GROUP_TWO may write into restrictedDir, while USER_ONE belongs to GROUP_ONE + updateDirectoryPermissions(ADMIN_USER, restrictedDir, List.of( + new PermissionDTO(true, List.of(), READ), + new PermissionDTO(false, List.of(GROUP_TWO_ID), WRITE) + )).andExpect(status().isOk()); + + // USER_ONE can't write in restrictedDir + assertThat(getAccessibleElements(USER_ONE, List.of(openElement, restrictedElement), WRITE)) + .containsExactlyInAnyOrder(openElement); + + // USER_TWO belongs to GROUP_TWO, so it may write into both + assertThat(getAccessibleElements(USER_TWO, List.of(openElement, restrictedElement), WRITE)) + .containsExactlyInAnyOrder(openElement, restrictedElement); + + // READ is left open to everyone on both directories + assertThat(getAccessibleElements(USER_ONE, List.of(openElement, restrictedElement), READ)) + .containsExactlyInAnyOrder(openElement, restrictedElement); + + // An unknown element is never accessible, not even to an explore admin + assertThat(getAccessibleElements(USER_ONE, List.of(unknownElement), WRITE)).isEmpty(); + assertThat(getAccessibleElements(ADMIN_USER, List.of(openElement, restrictedElement, unknownElement), WRITE)) + .containsExactlyInAnyOrder(openElement, restrictedElement); + } + + /** + * Helper method asking which of the given elements the user may access + */ + private List getAccessibleElements(String userId, List elementUuids, PermissionType permissionType) throws Exception { + String ids = elementUuids.stream().map(UUID::toString).collect(Collectors.joining(",")); + + MvcResult result = mockMvc.perform(get("/v1/elements/permission") + .param("ids", ids) + .param("accessType", permissionType.name()) + .header(USER_ID_HEADER, userId) + .header(USER_ROLES_HEADER, userId.equals(ADMIN_USER) ? ADMIN_ROLE : USER_ROLE)) + .andExpect(status().isOk()) + .andReturn(); + + return objectMapper.readValue(result.getResponse().getContentAsString(), new TypeReference<>() { }); + } + @Test void testRecursiveChecks() throws Exception { // Setup test users and directories From 82be06e6f08310ebf5a003edec638ab74d30f0bc Mon Sep 17 00:00:00 2001 From: Florent MILLOT <75525996+flomillot@users.noreply.github.com> Date: Wed, 9 Sep 2026 12:12:12 +0200 Subject: [PATCH 2/3] Rename the batch permission endpoint to /elements/accessible The endpoint returns elements, not a permission, which its name now reflects. Its operation and response descriptions are aligned with the explore-server endpoint fronting it, which described the same thing in other words. Signed-off-by: Florent MILLOT <75525996+flomillot@users.noreply.github.com> --- .../gridsuite/directory/server/DirectoryController.java | 7 ++++--- .../gridsuite/directory/server/PermissionServiceTest.java | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/gridsuite/directory/server/DirectoryController.java b/src/main/java/org/gridsuite/directory/server/DirectoryController.java index 0ee30bd9..08e299b9 100644 --- a/src/main/java/org/gridsuite/directory/server/DirectoryController.java +++ b/src/main/java/org/gridsuite/directory/server/DirectoryController.java @@ -236,10 +236,11 @@ public ResponseEntity areElementsAccessible(@RequestParam("ids") List> getAccessibleElements(@RequestParam("ids") List elementUuids, @RequestParam(value = "accessType") PermissionType permissionType, diff --git a/src/test/java/org/gridsuite/directory/server/PermissionServiceTest.java b/src/test/java/org/gridsuite/directory/server/PermissionServiceTest.java index 24c3ea76..fc6915b5 100644 --- a/src/test/java/org/gridsuite/directory/server/PermissionServiceTest.java +++ b/src/test/java/org/gridsuite/directory/server/PermissionServiceTest.java @@ -544,7 +544,7 @@ void testFilterAccessibleElements() throws Exception { private List getAccessibleElements(String userId, List elementUuids, PermissionType permissionType) throws Exception { String ids = elementUuids.stream().map(UUID::toString).collect(Collectors.joining(",")); - MvcResult result = mockMvc.perform(get("/v1/elements/permission") + MvcResult result = mockMvc.perform(get("/v1/elements/accessible") .param("ids", ids) .param("accessType", permissionType.name()) .header(USER_ID_HEADER, userId) From 06e5fcd207defbc4d4d9affaba9d1fcc1be83c71 Mon Sep 17 00:00:00 2001 From: Florent MILLOT <75525996+flomillot@users.noreply.github.com> Date: Wed, 9 Sep 2026 12:20:55 +0200 Subject: [PATCH 3/3] Rename the group ids supplier parameter userGroupIds already names a resolved List a few lines above - the very list the lambda passed here captures - while this parameter is the Supplier deferring that resolution. Signed-off-by: Florent MILLOT <75525996+flomillot@users.noreply.github.com> --- .../directory/server/services/PermissionService.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/gridsuite/directory/server/services/PermissionService.java b/src/main/java/org/gridsuite/directory/server/services/PermissionService.java index f32f5cfd..361432f6 100644 --- a/src/main/java/org/gridsuite/directory/server/services/PermissionService.java +++ b/src/main/java/org/gridsuite/directory/server/services/PermissionService.java @@ -231,7 +231,7 @@ private boolean hasElementPermission(String userId, UUID uuid, PermissionType pe return hasElementPermission(userId, uuid, permissionType, () -> getUserGroupIds(userId)); } - private boolean hasElementPermission(String userId, UUID uuid, PermissionType permissionType, Supplier> userGroupIds) { + private boolean hasElementPermission(String userId, UUID uuid, PermissionType permissionType, Supplier> userGroupIdsSupplier) { //Check global permission first boolean globalPermission = checkPermission(permissionRepository.findById(new PermissionId(uuid, ALL_USERS, "")), permissionType); if (globalPermission) { @@ -245,7 +245,7 @@ private boolean hasElementPermission(String userId, UUID uuid, PermissionType pe } //Finally check group permission - return userGroupIds.get() + return userGroupIdsSupplier.get() .stream() .anyMatch(groupId -> checkPermission(permissionRepository.findById(new PermissionId(uuid, "", groupId.toString())), permissionType)