Merge pull request #2458 from opencb/TASK-6322

TASK-6322 - Study admins should be able to update Project
opencb · Jun 26, 2024 · faa3137 · faa3137
2 parents faf5d28 + afa356b
commit faa3137
Show file tree

Hide file tree

Showing 2 changed files with 105 additions and 1 deletion.
diff --git a/.../main/java/org/opencb/opencga/catalog/auth/authorization/CatalogAuthorizationManager.java b/.../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<Study> studyResult = dbAdaptorFactory.getCatalogStudyDBAdaptor(organizationId).get(query, options);
+        List<String> 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
@@ -665,6 +665,82 @@ 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));
+
+        // Remove orgAdminUser1 from the administrators group of the organization
+        Map<String, Object> 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
     public void updatePrivateParamsFromProjectTest() throws CatalogException {
         catalogManager.getProjectManager().setDatastoreVariant(projectFqn1, new DataStore(), opencgaToken);