From c4578f772b4e12b05bfe047a026a5076d111e8dc Mon Sep 17 00:00:00 2001
From: juanf <juanf>
Date: Fri, 3 Feb 2017 15:01:42 +0000
Subject: [PATCH] SSDM-4686 : v3 canDelete roles bug - translating
 AuthorizationFailure into UnauthorizedObject exception

SVN: 37681
---
 .../entity/AbstractDeleteEntityExecutor.java  | 14 ++++--
 .../asapi/v3/DeleteDataSetTest.java           |  7 ++-
 .../asapi/v3/DeleteExperimentTest.java        | 11 ++---
 .../asapi/v3/DeleteProjectTest.java           |  8 ++--
 .../systemtest/asapi/v3/DeleteSampleTest.java | 11 ++---
 .../asapi/v3/DeleteVocabularyTermTest.java    | 47 +++++++++++++------
 6 files changed, 60 insertions(+), 38 deletions(-)

diff --git a/openbis/source/java/ch/ethz/sis/openbis/generic/server/asapi/v3/executor/entity/AbstractDeleteEntityExecutor.java b/openbis/source/java/ch/ethz/sis/openbis/generic/server/asapi/v3/executor/entity/AbstractDeleteEntityExecutor.java
index 9726d45f613..c86c4d3e4b6 100644
--- a/openbis/source/java/ch/ethz/sis/openbis/generic/server/asapi/v3/executor/entity/AbstractDeleteEntityExecutor.java
+++ b/openbis/source/java/ch/ethz/sis/openbis/generic/server/asapi/v3/executor/entity/AbstractDeleteEntityExecutor.java
@@ -25,8 +25,11 @@ import javax.annotation.Resource;
 
 import org.springframework.beans.factory.annotation.Autowired;
 
+import ch.ethz.sis.openbis.generic.asapi.v3.dto.common.id.IObjectId;
 import ch.ethz.sis.openbis.generic.asapi.v3.dto.deletion.AbstractObjectDeletionOptions;
+import ch.ethz.sis.openbis.generic.asapi.v3.exceptions.UnauthorizedObjectAccessException;
 import ch.ethz.sis.openbis.generic.server.asapi.v3.executor.IOperationContext;
+import ch.systemsx.cisd.common.exceptions.AuthorizationFailureException;
 import ch.systemsx.cisd.openbis.generic.server.ComponentNames;
 import ch.systemsx.cisd.openbis.generic.server.business.bo.ICommonBusinessObjectFactory;
 import ch.systemsx.cisd.openbis.generic.server.dataaccess.IDAOFactory;
@@ -79,7 +82,14 @@ public abstract class AbstractDeleteEntityExecutor<DELETION_ID, ENTITY_ID, ENTIT
             ENTITY_ID entityId = entry.getKey();
             ENTITY_PE entity = entry.getValue();
 
-            checkAccess(context, entityId, entity);
+            try
+            {
+                checkAccess(context, entityId, entity);
+            } catch (AuthorizationFailureException ex)
+            {
+                throw new UnauthorizedObjectAccessException((IObjectId) entityId);
+            }
+
             updateModificationDateAndModifier(context, entity);
         }
 
@@ -98,8 +108,6 @@ public abstract class AbstractDeleteEntityExecutor<DELETION_ID, ENTITY_ID, ENTIT
 
     protected abstract Map<ENTITY_ID, ENTITY_PE> map(IOperationContext context, List<? extends ENTITY_ID> entityIds);
 
-    // protected abstract void checkAccess(IOperationContext context);
-
     protected abstract void checkAccess(IOperationContext context, ENTITY_ID entityId, ENTITY_PE entity);
 
     protected abstract void updateModificationDateAndModifier(IOperationContext context, ENTITY_PE entity);
diff --git a/openbis/sourceTest/java/ch/ethz/sis/openbis/systemtest/asapi/v3/DeleteDataSetTest.java b/openbis/sourceTest/java/ch/ethz/sis/openbis/systemtest/asapi/v3/DeleteDataSetTest.java
index d1ca2f904e6..ffcb34fb3fd 100644
--- a/openbis/sourceTest/java/ch/ethz/sis/openbis/systemtest/asapi/v3/DeleteDataSetTest.java
+++ b/openbis/sourceTest/java/ch/ethz/sis/openbis/systemtest/asapi/v3/DeleteDataSetTest.java
@@ -19,14 +19,13 @@ package ch.ethz.sis.openbis.systemtest.asapi.v3;
 import java.util.ArrayList;
 import java.util.Collections;
 
-import junit.framework.Assert;
-
 import org.testng.annotations.Test;
 
 import ch.ethz.sis.openbis.generic.asapi.v3.dto.dataset.delete.DataSetDeletionOptions;
 import ch.ethz.sis.openbis.generic.asapi.v3.dto.dataset.id.DataSetPermId;
 import ch.ethz.sis.openbis.generic.asapi.v3.dto.deletion.id.IDeletionId;
 import ch.systemsx.cisd.common.action.IDelegatedAction;
+import junit.framework.Assert;
 
 /**
  * @author pkupczyk
@@ -124,7 +123,7 @@ public class DeleteDataSetTest extends AbstractDeletionTest
     {
         final DataSetPermId permId = new DataSetPermId("20120619092259000-22");
 
-        assertAuthorizationFailureException(new IDelegatedAction()
+        assertUnauthorizedObjectAccessException(new IDelegatedAction()
             {
                 @Override
                 public void execute()
@@ -136,7 +135,7 @@ public class DeleteDataSetTest extends AbstractDeletionTest
 
                     v3api.deleteDataSets(sessionToken, Collections.singletonList(permId), options);
                 }
-            });
+            }, permId);
     }
     // waiting for better times
     // @Test
diff --git a/openbis/sourceTest/java/ch/ethz/sis/openbis/systemtest/asapi/v3/DeleteExperimentTest.java b/openbis/sourceTest/java/ch/ethz/sis/openbis/systemtest/asapi/v3/DeleteExperimentTest.java
index b3e577a04fa..2853224f1ec 100644
--- a/openbis/sourceTest/java/ch/ethz/sis/openbis/systemtest/asapi/v3/DeleteExperimentTest.java
+++ b/openbis/sourceTest/java/ch/ethz/sis/openbis/systemtest/asapi/v3/DeleteExperimentTest.java
@@ -23,8 +23,6 @@ import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
 
-import junit.framework.Assert;
-
 import org.springframework.test.context.transaction.TestTransaction;
 import org.testng.annotations.Test;
 
@@ -37,6 +35,7 @@ import ch.ethz.sis.openbis.generic.asapi.v3.dto.sample.id.SamplePermId;
 import ch.ethz.sis.openbis.systemtest.asapi.v3.index.RemoveFromIndexState;
 import ch.systemsx.cisd.common.action.IDelegatedAction;
 import ch.systemsx.cisd.openbis.generic.shared.dto.ExperimentPE;
+import junit.framework.Assert;
 
 /**
  * @author pkupczyk
@@ -150,7 +149,7 @@ public class DeleteExperimentTest extends AbstractDeletionTest
     {
         final ExperimentPermId permId = createCisdExperiment();
 
-        assertAuthorizationFailureException(new IDelegatedAction()
+        assertUnauthorizedObjectAccessException(new IDelegatedAction()
             {
                 @Override
                 public void execute()
@@ -162,7 +161,7 @@ public class DeleteExperimentTest extends AbstractDeletionTest
 
                     v3api.deleteExperiments(sessionToken, Collections.singletonList(permId), options);
                 }
-            });
+            }, permId);
     }
 
     @Test
@@ -170,7 +169,7 @@ public class DeleteExperimentTest extends AbstractDeletionTest
     {
         final ExperimentPermId permId = new ExperimentPermId("200902091255058-1037");
 
-        assertAuthorizationFailureException(new IDelegatedAction()
+        assertUnauthorizedObjectAccessException(new IDelegatedAction()
             {
                 @Override
                 public void execute()
@@ -182,7 +181,7 @@ public class DeleteExperimentTest extends AbstractDeletionTest
 
                     v3api.deleteExperiments(sessionToken, Collections.singletonList(permId), options);
                 }
-            });
+            }, permId);
     }
 
 }
diff --git a/openbis/sourceTest/java/ch/ethz/sis/openbis/systemtest/asapi/v3/DeleteProjectTest.java b/openbis/sourceTest/java/ch/ethz/sis/openbis/systemtest/asapi/v3/DeleteProjectTest.java
index 3f277ae41a3..7c4361b4c74 100644
--- a/openbis/sourceTest/java/ch/ethz/sis/openbis/systemtest/asapi/v3/DeleteProjectTest.java
+++ b/openbis/sourceTest/java/ch/ethz/sis/openbis/systemtest/asapi/v3/DeleteProjectTest.java
@@ -88,7 +88,7 @@ public class DeleteProjectTest extends AbstractDeletionTest
     {
         final ProjectPermId permId = createCisdProject();
 
-        assertAuthorizationFailureException(new IDelegatedAction()
+        assertUnauthorizedObjectAccessException(new IDelegatedAction()
             {
                 @Override
                 public void execute()
@@ -100,7 +100,7 @@ public class DeleteProjectTest extends AbstractDeletionTest
 
                     v3api.deleteProjects(sessionToken, Collections.singletonList(permId), options);
                 }
-            });
+            }, permId);
     }
 
     @Test
@@ -108,7 +108,7 @@ public class DeleteProjectTest extends AbstractDeletionTest
     {
         final ProjectPermId permId = new ProjectPermId("20120814110011738-105");
 
-        assertAuthorizationFailureException(new IDelegatedAction()
+        assertUnauthorizedObjectAccessException(new IDelegatedAction()
             {
                 @Override
                 public void execute()
@@ -120,6 +120,6 @@ public class DeleteProjectTest extends AbstractDeletionTest
 
                     v3api.deleteProjects(sessionToken, Collections.singletonList(permId), options);
                 }
-            });
+            }, permId);
     }
 }
diff --git a/openbis/sourceTest/java/ch/ethz/sis/openbis/systemtest/asapi/v3/DeleteSampleTest.java b/openbis/sourceTest/java/ch/ethz/sis/openbis/systemtest/asapi/v3/DeleteSampleTest.java
index e1b2fe7f538..427077d5b12 100644
--- a/openbis/sourceTest/java/ch/ethz/sis/openbis/systemtest/asapi/v3/DeleteSampleTest.java
+++ b/openbis/sourceTest/java/ch/ethz/sis/openbis/systemtest/asapi/v3/DeleteSampleTest.java
@@ -23,8 +23,6 @@ import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
 
-import junit.framework.Assert;
-
 import org.springframework.test.context.transaction.TestTransaction;
 import org.testng.annotations.Test;
 
@@ -38,6 +36,7 @@ import ch.ethz.sis.openbis.generic.asapi.v3.dto.sample.id.SamplePermId;
 import ch.ethz.sis.openbis.systemtest.asapi.v3.index.RemoveFromIndexState;
 import ch.systemsx.cisd.common.action.IDelegatedAction;
 import ch.systemsx.cisd.openbis.generic.shared.dto.SamplePE;
+import junit.framework.Assert;
 
 /**
  * @author pkupczyk
@@ -62,7 +61,7 @@ public class DeleteSampleTest extends AbstractDeletionTest
     {
         final SamplePermId permId = new SamplePermId("200902091250077-1060");
 
-        assertAuthorizationFailureException(new IDelegatedAction()
+        assertUnauthorizedObjectAccessException(new IDelegatedAction()
             {
                 @Override
                 public void execute()
@@ -74,7 +73,7 @@ public class DeleteSampleTest extends AbstractDeletionTest
 
                     v3api.deleteSamples(sessionToken, Collections.singletonList(permId), options);
                 }
-            });
+            }, permId);
     }
 
     @Test
@@ -184,7 +183,7 @@ public class DeleteSampleTest extends AbstractDeletionTest
     {
         final SamplePermId permId = createCisdSample(null);
 
-        assertAuthorizationFailureException(new IDelegatedAction()
+        assertUnauthorizedObjectAccessException(new IDelegatedAction()
             {
                 @Override
                 public void execute()
@@ -196,7 +195,7 @@ public class DeleteSampleTest extends AbstractDeletionTest
 
                     v3api.deleteSamples(sessionToken, Collections.singletonList(permId), options);
                 }
-            });
+            }, permId);
     }
 
 }
diff --git a/openbis/sourceTest/java/ch/ethz/sis/openbis/systemtest/asapi/v3/DeleteVocabularyTermTest.java b/openbis/sourceTest/java/ch/ethz/sis/openbis/systemtest/asapi/v3/DeleteVocabularyTermTest.java
index 12175b640ff..cd8be363d8e 100644
--- a/openbis/sourceTest/java/ch/ethz/sis/openbis/systemtest/asapi/v3/DeleteVocabularyTermTest.java
+++ b/openbis/sourceTest/java/ch/ethz/sis/openbis/systemtest/asapi/v3/DeleteVocabularyTermTest.java
@@ -35,6 +35,7 @@ import ch.ethz.sis.openbis.generic.asapi.v3.dto.vocabulary.id.IVocabularyTermId;
 import ch.ethz.sis.openbis.generic.asapi.v3.dto.vocabulary.id.VocabularyPermId;
 import ch.ethz.sis.openbis.generic.asapi.v3.dto.vocabulary.id.VocabularyTermPermId;
 import ch.ethz.sis.openbis.systemtest.asapi.v3.index.ReindexingState;
+import ch.systemsx.cisd.common.action.IDelegatedAction;
 import ch.systemsx.cisd.common.exceptions.UserFailureException;
 
 /**
@@ -44,15 +45,23 @@ import ch.systemsx.cisd.common.exceptions.UserFailureException;
 public class DeleteVocabularyTermTest extends AbstractVocabularyTermTest
 {
 
-    @Test(expectedExceptions = UserFailureException.class, expectedExceptionsMessageRegExp = ".*None of method roles '\\[SPACE_POWER_USER, SPACE_ADMIN, INSTANCE_ADMIN, SPACE_ETL_SERVER, INSTANCE_ETL_SERVER\\]' could be found in roles of user 'observer'.*")
+    @Test
     public void testDeleteTermUnauthorized()
     {
-        String sessionToken = v3api.login(TEST_GROUP_OBSERVER, PASSWORD);
-
-        VocabularyTermDeletionOptions options = new VocabularyTermDeletionOptions();
-        options.setReason("Just for testing");
-
-        v3api.deleteVocabularyTerms(sessionToken, Arrays.asList(new VocabularyTermPermId("HUMAN", "ORGANISM")), options);
+        final VocabularyTermPermId permId = new VocabularyTermPermId("HUMAN", "ORGANISM");
+        assertUnauthorizedObjectAccessException(new IDelegatedAction()
+            {
+                @Override
+                public void execute()
+                {
+                    String sessionToken = v3api.login(TEST_GROUP_OBSERVER, PASSWORD);
+
+                    VocabularyTermDeletionOptions options = new VocabularyTermDeletionOptions();
+                    options.setReason("Just for testing");
+
+                    v3api.deleteVocabularyTerms(sessionToken, Arrays.asList(permId), options);
+                }
+            }, permId);
     }
 
     @Test
@@ -179,19 +188,27 @@ public class DeleteVocabularyTermTest extends AbstractVocabularyTermTest
         v3api.deleteVocabularyTerms(sessionToken, Arrays.asList(termIdFly), options);
     }
 
-    @Test(expectedExceptions = UserFailureException.class, expectedExceptionsMessageRegExp = ".*None of method roles '\\[SPACE_POWER_USER, SPACE_ADMIN, INSTANCE_ADMIN, SPACE_ETL_SERVER, INSTANCE_ETL_SERVER\\]' could be found in roles of user 'observer'.*")
+    @Test
     public void testReplaceTermUnauthorized()
     {
-        String sessionToken = v3api.login(TEST_GROUP_OBSERVER, PASSWORD);
+        final VocabularyTermPermId termIdReplaced = new VocabularyTermPermId("HUMAN", "ORGANISM");
+        assertUnauthorizedObjectAccessException(new IDelegatedAction()
+            {
+                @Override
+                public void execute()
+                {
+                    String sessionToken = v3api.login(TEST_GROUP_OBSERVER, PASSWORD);
 
-        VocabularyTermPermId termIdReplaced = new VocabularyTermPermId("HUMAN", "ORGANISM");
-        VocabularyTermPermId termIdReplacement = new VocabularyTermPermId("FLY", "ORGANISM");
+                    VocabularyTermPermId termIdReplacement = new VocabularyTermPermId("FLY", "ORGANISM");
 
-        VocabularyTermDeletionOptions options = new VocabularyTermDeletionOptions();
-        options.setReason("Just for testing");
-        options.replace(termIdReplaced, termIdReplacement);
+                    VocabularyTermDeletionOptions options = new VocabularyTermDeletionOptions();
+                    options.setReason("Just for testing");
+                    options.replace(termIdReplaced, termIdReplacement);
+
+                    v3api.deleteVocabularyTerms(sessionToken, Arrays.asList(termIdReplaced), options);
+                }
 
-        v3api.deleteVocabularyTerms(sessionToken, Arrays.asList(termIdReplaced), options);
+            }, termIdReplaced);
     }
 
     @Test(expectedExceptions = UserFailureException.class, expectedExceptionsMessageRegExp = ".*The following terms where not chosen to be deleted but had replacements specified: \\[VocabularyTermPE\\{code=HUMAN,label=<null>\\}\\].*")
-- 
GitLab