From 7bc1c2371071483f79aee26499f4c92a0c0f560e Mon Sep 17 00:00:00 2001 From: pfurio Date: Wed, 29 May 2024 14:41:43 +0200 Subject: [PATCH 1/2] catalog: study admins can update projects, #TASK-6322 --- .../CatalogAuthorizationManager.java | 30 ++++++++++++++++- .../catalog/managers/CatalogManagerTest.java | 33 +++++++++++++++++++ 2 files changed, 62 insertions(+), 1 deletion(-) diff --git a/opencga-catalog/src/main/java/org/opencb/opencga/catalog/auth/authorization/CatalogAuthorizationManager.java b/opencga-catalog/src/main/java/org/opencb/opencga/catalog/auth/authorization/CatalogAuthorizationManager.java index b58c117d94f..7022be4a0e7 100644 --- a/opencga-catalog/src/main/java/org/opencb/opencga/catalog/auth/authorization/CatalogAuthorizationManager.java +++ b/opencga-catalog/src/main/java/org/opencb/opencga/catalog/auth/authorization/CatalogAuthorizationManager.java @@ -19,6 +19,7 @@ import org.apache.commons.collections4.CollectionUtils; import org.apache.commons.lang3.StringUtils; import org.opencb.commons.datastore.core.Query; +import org.opencb.commons.datastore.core.QueryOptions; import org.opencb.opencga.catalog.db.DBAdaptorFactory; import org.opencb.opencga.catalog.db.api.*; import org.opencb.opencga.catalog.exceptions.CatalogAuthorizationException; @@ -79,7 +80,34 @@ public void checkCanEditProject(String organizationId, long projectId, String us if (isAtLeastOrganizationOwnerOrAdmin(organizationId, userId)) { return; } - throw new CatalogAuthorizationException("Permission denied: Only the organization owner or administrators can update the project."); + try { + checkUserIsStudyAdminInAllStudiesOfProject(organizationId, projectId, userId); + } catch (CatalogException e) { + throw new CatalogAuthorizationException("Permission denied: Only the organization owner, administrators, or users who are " + + "study administrators in all studies within the project can update the project.", e); + } + } + + private void checkUserIsStudyAdminInAllStudiesOfProject(String organizationId, long projectUid, String userId) throws CatalogException { + Query query = new Query(StudyDBAdaptor.QueryParams.PROJECT_UID.key(), projectUid); + QueryOptions options = new QueryOptions(QueryOptions.INCLUDE, + Arrays.asList(StudyDBAdaptor.QueryParams.GROUPS.key(), StudyDBAdaptor.QueryParams.FQN.key())); + OpenCGAResult studyResult = dbAdaptorFactory.getCatalogStudyDBAdaptor(organizationId).get(query, options); + List nonAdminStudyIds = new ArrayList<>(); + for (Study study : studyResult.getResults()) { + for (Group group : study.getGroups()) { + if (group.getId().equals(ADMINS_GROUP)) { + if (!group.getUserIds().contains(userId)) { + nonAdminStudyIds.add(study.getFqn()); + } + break; + } + } + } + if (!nonAdminStudyIds.isEmpty()) { + throw new CatalogAuthorizationException("Permission denied: User " + userId + " is not an administrator of the following" + + " studies: " + String.join(", ", nonAdminStudyIds)); + } } @Override diff --git a/opencga-catalog/src/test/java/org/opencb/opencga/catalog/managers/CatalogManagerTest.java b/opencga-catalog/src/test/java/org/opencb/opencga/catalog/managers/CatalogManagerTest.java index 21d0322207b..53f9b20f203 100644 --- a/opencga-catalog/src/test/java/org/opencb/opencga/catalog/managers/CatalogManagerTest.java +++ b/opencga-catalog/src/test/java/org/opencb/opencga/catalog/managers/CatalogManagerTest.java @@ -656,6 +656,39 @@ public void testModifyProject() throws CatalogException { catalogManager.getProjectManager().update(project1, options, null, ownerToken); } + @Test + public void updateProjectPermissionTest() throws CatalogException { + ObjectMap params = new ObjectMap() + .append(ProjectDBAdaptor.QueryParams.DESCRIPTION.key(), "my new description"); + Project project = catalogManager.getProjectManager().update(project1, params, INCLUDE_RESULT, ownerToken).first(); + assertEquals("my new description", project.getDescription()); + + params.put(ProjectDBAdaptor.QueryParams.DESCRIPTION.key(), "my new description 2"); + project = catalogManager.getProjectManager().update(project1, params, INCLUDE_RESULT, orgAdminToken2).first(); + assertEquals("my new description 2", project.getDescription()); + + params.put(ProjectDBAdaptor.QueryParams.DESCRIPTION.key(), "my new description 3"); + project = catalogManager.getProjectManager().update(project1, params, INCLUDE_RESULT, studyAdminToken1).first(); + assertEquals("my new description 3", project.getDescription()); + + // Make normalUser1 admin of first study + catalogManager.getStudyManager().updateGroup(studyFqn, ParamConstants.ADMINS_GROUP, ParamUtils.BasicUpdateAction.ADD, + new GroupUpdateParams(Collections.singletonList(normalUserId1)), ownerToken); + // And remove normalUser1 from the admins group of the second study (just in case) + catalogManager.getStudyManager().updateGroup(studyFqn2, ParamConstants.ADMINS_GROUP, ParamUtils.BasicUpdateAction.REMOVE, + new GroupUpdateParams(Collections.singletonList(normalUserId1)), ownerToken); + + CatalogAuthorizationException catalogAuthorizationException = assertThrows(CatalogAuthorizationException.class, + () -> catalogManager.getProjectManager().update(project1, params, INCLUDE_RESULT, normalToken1)); + assertFalse(catalogAuthorizationException.getCause().getMessage().contains(studyFqn)); + assertTrue(catalogAuthorizationException.getCause().getMessage().contains(studyFqn2)); + + catalogAuthorizationException = assertThrows(CatalogAuthorizationException.class, + () -> catalogManager.getProjectManager().update(project1, params, INCLUDE_RESULT, normalToken2)); + assertTrue(catalogAuthorizationException.getCause().getMessage().contains(studyFqn)); + assertTrue(catalogAuthorizationException.getCause().getMessage().contains(studyFqn2)); + } + @Test public void updatePrivateParamsFromProjectTest() throws CatalogException { catalogManager.getProjectManager().setDatastoreVariant(projectFqn1, new DataStore(), opencgaToken); From 8b88c603f190676bfb9d82c2b78c5d415e22405e Mon Sep 17 00:00:00 2001 From: pfurio Date: Tue, 18 Jun 2024 10:50:38 +0200 Subject: [PATCH 2/2] catalog: add more cases to the test, #TASK-6322 --- .../catalog/managers/CatalogManagerTest.java | 43 +++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/opencga-catalog/src/test/java/org/opencb/opencga/catalog/managers/CatalogManagerTest.java b/opencga-catalog/src/test/java/org/opencb/opencga/catalog/managers/CatalogManagerTest.java index 53f9b20f203..80db404fd60 100644 --- a/opencga-catalog/src/test/java/org/opencb/opencga/catalog/managers/CatalogManagerTest.java +++ b/opencga-catalog/src/test/java/org/opencb/opencga/catalog/managers/CatalogManagerTest.java @@ -687,6 +687,49 @@ public void updateProjectPermissionTest() throws CatalogException { () -> catalogManager.getProjectManager().update(project1, params, INCLUDE_RESULT, normalToken2)); assertTrue(catalogAuthorizationException.getCause().getMessage().contains(studyFqn)); assertTrue(catalogAuthorizationException.getCause().getMessage().contains(studyFqn2)); + + // Remove orgAdminUser1 from the administrators group of the organization + Map actionMap = new HashMap<>(); + actionMap.put(OrganizationDBAdaptor.QueryParams.ADMINS.key(), ParamUtils.BasicUpdateAction.REMOVE); + QueryOptions options = new QueryOptions(Constants.ACTIONS, actionMap); + catalogManager.getOrganizationManager().update(organizationId, new OrganizationUpdateParams() + .setAdmins(Collections.singletonList(orgAdminUserId1)), options, ownerToken); + + catalogAuthorizationException = assertThrows(CatalogAuthorizationException.class, + () -> catalogManager.getProjectManager().update(project1, params, INCLUDE_RESULT, orgAdminToken1)); + assertTrue(catalogAuthorizationException.getCause().getMessage().contains(studyFqn)); + assertTrue(catalogAuthorizationException.getCause().getMessage().contains(studyFqn2)); + + // Create a third study + catalogManager.getStudyManager().create(project1, new Study().setId("study_3"), null, ownerToken); + catalogAuthorizationException = assertThrows(CatalogAuthorizationException.class, + () -> catalogManager.getProjectManager().update(project1, params, INCLUDE_RESULT, orgAdminToken1)); + assertTrue(catalogAuthorizationException.getCause().getMessage().contains(studyFqn)); + assertTrue(catalogAuthorizationException.getCause().getMessage().contains(studyFqn2)); + assertTrue(catalogAuthorizationException.getCause().getMessage().contains("study_3")); + + // Add orgAdminUser1 to the administrators group of the third study + catalogManager.getStudyManager().updateGroup("study_3", ParamConstants.ADMINS_GROUP, ParamUtils.BasicUpdateAction.ADD, + new GroupUpdateParams(Collections.singletonList(orgAdminUserId1)), ownerToken); + catalogAuthorizationException = assertThrows(CatalogAuthorizationException.class, + () -> catalogManager.getProjectManager().update(project1, params, INCLUDE_RESULT, orgAdminToken1)); + assertTrue(catalogAuthorizationException.getCause().getMessage().contains(studyFqn)); + assertTrue(catalogAuthorizationException.getCause().getMessage().contains(studyFqn2)); + assertFalse(catalogAuthorizationException.getCause().getMessage().contains("study_3")); + + // Add orgAdminUser1 to the administrators group of the second study + catalogManager.getStudyManager().updateGroup(studyFqn2, ParamConstants.ADMINS_GROUP, ParamUtils.BasicUpdateAction.ADD, + new GroupUpdateParams(Collections.singletonList(orgAdminUserId1)), ownerToken); + catalogAuthorizationException = assertThrows(CatalogAuthorizationException.class, + () -> catalogManager.getProjectManager().update(project1, params, INCLUDE_RESULT, orgAdminToken1)); + assertTrue(catalogAuthorizationException.getCause().getMessage().contains(studyFqn)); + assertFalse(catalogAuthorizationException.getCause().getMessage().contains(studyFqn2)); + assertFalse(catalogAuthorizationException.getCause().getMessage().contains("study_3")); + + // Add orgAdminUser1 to the administrators group of the remaining study + catalogManager.getStudyManager().updateGroup(studyFqn, ParamConstants.ADMINS_GROUP, ParamUtils.BasicUpdateAction.ADD, + new GroupUpdateParams(Collections.singletonList(orgAdminUserId1)), ownerToken); + catalogManager.getProjectManager().update(project1, params, INCLUDE_RESULT, orgAdminToken1); } @Test