diff --git a/openbis/source/java/ch/ethz/sis/openbis/generic/server/asapi/v3/executor/deletion/ConfirmDeletionExecutor.java b/openbis/source/java/ch/ethz/sis/openbis/generic/server/asapi/v3/executor/deletion/ConfirmDeletionExecutor.java index 4872cd6104521a01819e0a83c7d4f627e8e8687f..71705ac166c5cf7d34ca1fc894525000bd1eda8c 100644 --- a/openbis/source/java/ch/ethz/sis/openbis/generic/server/asapi/v3/executor/deletion/ConfirmDeletionExecutor.java +++ b/openbis/source/java/ch/ethz/sis/openbis/generic/server/asapi/v3/executor/deletion/ConfirmDeletionExecutor.java @@ -32,6 +32,7 @@ import ch.ethz.sis.openbis.generic.asapi.v3.dto.deletion.id.IDeletionId; import ch.ethz.sis.openbis.generic.asapi.v3.exceptions.ObjectNotFoundException; 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.common.exceptions.UserFailureException; import ch.systemsx.cisd.openbis.generic.server.ComponentNames; import ch.systemsx.cisd.openbis.generic.server.authorization.AuthorizationDataProvider; @@ -78,8 +79,6 @@ public class ConfirmDeletionExecutor implements IConfirmDeletionExecutor throw new UserFailureException("Deletion ids cannot be null."); } - authorizationExecutor.canConfirm(context); - // We do not want to fail with nulls but rather ignore them. Ignoring the nulls allows us to // pass the result of the deleteXXX method directly to confirmDeletions without any checks // (the deleteXXX methods return null when an object to be deleted does not exist, e.g. it had been already deleted) @@ -94,6 +93,14 @@ public class ConfirmDeletionExecutor implements IConfirmDeletionExecutor } } + try + { + authorizationExecutor.canConfirm(context, deletionIdsWithoutNulls); + } catch (AuthorizationFailureException ex) + { + throw new UnauthorizedObjectAccessException(deletionIdsWithoutNulls); + } + IDeletionDAO deletionDAO = daoFactory.getDeletionDAO(); Map<IDeletionId, DeletionPE> deletionMap = mapDeletionByIdExecutor.map(context, deletionIdsWithoutNulls); diff --git a/openbis/source/java/ch/ethz/sis/openbis/generic/server/asapi/v3/executor/deletion/DeletionAuthorizationExecutor.java b/openbis/source/java/ch/ethz/sis/openbis/generic/server/asapi/v3/executor/deletion/DeletionAuthorizationExecutor.java index 8d361c735c932dde040eebf1adcac339af87013b..3651cd3bb85a8da702d9d5d6edfad53e01f9f59c 100644 --- a/openbis/source/java/ch/ethz/sis/openbis/generic/server/asapi/v3/executor/deletion/DeletionAuthorizationExecutor.java +++ b/openbis/source/java/ch/ethz/sis/openbis/generic/server/asapi/v3/executor/deletion/DeletionAuthorizationExecutor.java @@ -16,15 +16,21 @@ package ch.ethz.sis.openbis.generic.server.asapi.v3.executor.deletion; +import java.util.List; + import org.springframework.stereotype.Component; +import ch.ethz.sis.openbis.generic.asapi.v3.dto.deletion.id.IDeletionId; import ch.ethz.sis.openbis.generic.server.asapi.v3.executor.IOperationContext; +import ch.systemsx.cisd.openbis.generic.server.authorization.annotation.AuthorizationGuard; import ch.systemsx.cisd.openbis.generic.server.authorization.annotation.Capability; import ch.systemsx.cisd.openbis.generic.server.authorization.annotation.RolesAllowed; +import ch.systemsx.cisd.openbis.generic.server.authorization.predicate.V3DeletionIdPredicate; +import ch.systemsx.cisd.openbis.generic.server.authorization.predicate.V3RevertDeletionPredicate; import ch.systemsx.cisd.openbis.generic.shared.DatabaseCreateOrDeleteModification; import ch.systemsx.cisd.openbis.generic.shared.DatabaseUpdateModification; -import ch.systemsx.cisd.openbis.generic.shared.basic.dto.RoleWithHierarchy; import ch.systemsx.cisd.openbis.generic.shared.basic.dto.DatabaseModificationKind.ObjectKind; +import ch.systemsx.cisd.openbis.generic.shared.basic.dto.RoleWithHierarchy; /** * @author pkupczyk @@ -37,7 +43,7 @@ public class DeletionAuthorizationExecutor implements IDeletionAuthorizationExec @DatabaseCreateOrDeleteModification(value = { ObjectKind.DELETION, ObjectKind.EXPERIMENT, ObjectKind.SAMPLE, ObjectKind.DATA_SET }) @RolesAllowed({ RoleWithHierarchy.SPACE_ADMIN, RoleWithHierarchy.SPACE_ETL_SERVER }) @Capability("CONFIRM_DELETION") - public void canConfirm(IOperationContext context) + public void canConfirm(IOperationContext context, @AuthorizationGuard(guardClass = V3DeletionIdPredicate.class) List<? extends IDeletionId> ids) { } @@ -46,7 +52,8 @@ public class DeletionAuthorizationExecutor implements IDeletionAuthorizationExec @DatabaseUpdateModification(value = { ObjectKind.EXPERIMENT, ObjectKind.SAMPLE, ObjectKind.DATA_SET }) @RolesAllowed({ RoleWithHierarchy.SPACE_USER, RoleWithHierarchy.SPACE_ETL_SERVER }) @Capability("REVERT_DELETION") - public void canRevert(IOperationContext context) + public void canRevert(IOperationContext context, + @AuthorizationGuard(guardClass = V3RevertDeletionPredicate.class) List<? extends IDeletionId> ids) { } diff --git a/openbis/source/java/ch/ethz/sis/openbis/generic/server/asapi/v3/executor/deletion/IDeletionAuthorizationExecutor.java b/openbis/source/java/ch/ethz/sis/openbis/generic/server/asapi/v3/executor/deletion/IDeletionAuthorizationExecutor.java index ae12b8f401bc2d5bb796d8179fc3f6c57963df37..24d394b90930092ed8a273b2b3359498855ba47a 100644 --- a/openbis/source/java/ch/ethz/sis/openbis/generic/server/asapi/v3/executor/deletion/IDeletionAuthorizationExecutor.java +++ b/openbis/source/java/ch/ethz/sis/openbis/generic/server/asapi/v3/executor/deletion/IDeletionAuthorizationExecutor.java @@ -16,6 +16,9 @@ package ch.ethz.sis.openbis.generic.server.asapi.v3.executor.deletion; +import java.util.List; + +import ch.ethz.sis.openbis.generic.asapi.v3.dto.deletion.id.IDeletionId; import ch.ethz.sis.openbis.generic.server.asapi.v3.executor.IOperationContext; import ch.ethz.sis.openbis.generic.server.asapi.v3.executor.common.IObjectAuthorizationExecutor; @@ -25,9 +28,9 @@ import ch.ethz.sis.openbis.generic.server.asapi.v3.executor.common.IObjectAuthor public interface IDeletionAuthorizationExecutor extends IObjectAuthorizationExecutor { - void canConfirm(IOperationContext context); + void canConfirm(IOperationContext context, List<? extends IDeletionId> ids); - void canRevert(IOperationContext context); + void canRevert(IOperationContext context, List<? extends IDeletionId> ids); void canGet(IOperationContext context); diff --git a/openbis/source/java/ch/ethz/sis/openbis/generic/server/asapi/v3/executor/deletion/RevertDeletionExecutor.java b/openbis/source/java/ch/ethz/sis/openbis/generic/server/asapi/v3/executor/deletion/RevertDeletionExecutor.java index 26c76e6ba796d550d6bbc55254684677234aa588..0ec7d22affb864de0c925653da73755222c12d9b 100644 --- a/openbis/source/java/ch/ethz/sis/openbis/generic/server/asapi/v3/executor/deletion/RevertDeletionExecutor.java +++ b/openbis/source/java/ch/ethz/sis/openbis/generic/server/asapi/v3/executor/deletion/RevertDeletionExecutor.java @@ -34,6 +34,7 @@ import ch.ethz.sis.openbis.generic.asapi.v3.dto.deletion.id.IDeletionId; import ch.ethz.sis.openbis.generic.asapi.v3.exceptions.ObjectNotFoundException; 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.authorization.AuthorizationDataProvider; import ch.systemsx.cisd.openbis.generic.server.authorization.validator.DeletionValidator; @@ -76,8 +77,6 @@ public class RevertDeletionExecutor implements IRevertDeletionExecutor @Override public void revert(IOperationContext context, List<? extends IDeletionId> deletionIds) { - authorizationExecutor.canRevert(context); - if (context == null) { throw new IllegalArgumentException("Context cannot be null"); @@ -128,6 +127,14 @@ public class RevertDeletionExecutor implements IRevertDeletionExecutor Map<IDeletionId, DeletionPE> map = mapDeletionByIdExecutor.map(context, deletionIds); List<Long> deletionTechIds = new LinkedList<Long>(); + try + { + authorizationExecutor.canRevert(context, deletionIds); + } catch (AuthorizationFailureException ex) + { + throw new UnauthorizedObjectAccessException(deletionIds); + } + for (IDeletionId deletionId : deletionIds) { DeletionPE deletion = map.get(deletionId); diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/authorization/predicate/V3DeletionIdPredicate.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/authorization/predicate/V3DeletionIdPredicate.java new file mode 100644 index 0000000000000000000000000000000000000000..bc3d7f1253a92389a33a7f9b3c7029d732fe8416 --- /dev/null +++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/authorization/predicate/V3DeletionIdPredicate.java @@ -0,0 +1,44 @@ +package ch.systemsx.cisd.openbis.generic.server.authorization.predicate; + +import java.util.List; + +import ch.ethz.sis.openbis.generic.asapi.v3.dto.deletion.id.IDeletionId; +import ch.systemsx.cisd.common.exceptions.Status; +import ch.systemsx.cisd.openbis.generic.server.authorization.IAuthorizationDataProvider; +import ch.systemsx.cisd.openbis.generic.server.authorization.RoleWithIdentifier; +import ch.systemsx.cisd.openbis.generic.server.authorization.annotation.ShouldFlattenCollections; +import ch.systemsx.cisd.openbis.generic.server.authorization.predicate.v3tov1.DeletionIdTranslator; +import ch.systemsx.cisd.openbis.generic.shared.basic.TechId; +import ch.systemsx.cisd.openbis.generic.shared.dto.PersonPE;; + +@ShouldFlattenCollections(value = false) +public class V3DeletionIdPredicate extends AbstractPredicate<List<IDeletionId>> +{ + + protected final DeletionTechIdCollectionPredicate deletionTechIdCollectionPredicate; + + public V3DeletionIdPredicate() + { + this.deletionTechIdCollectionPredicate = new DeletionTechIdCollectionPredicate(); + } + + @Override + public final void init(IAuthorizationDataProvider provider) + { + deletionTechIdCollectionPredicate.init(provider); + } + + @Override + public String getCandidateDescription() + { + return "v3 deletion id object"; + } + + @Override + protected Status doEvaluation(PersonPE person, List<RoleWithIdentifier> allowedRoles, List<IDeletionId> values) + { + assert deletionTechIdCollectionPredicate.initialized : "Predicate has not been initialized"; + List<TechId> valuesAsTechIds = DeletionIdTranslator.translate(values); + return deletionTechIdCollectionPredicate.doEvaluation(person, allowedRoles, valuesAsTechIds); + } +} diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/authorization/predicate/V3RevertDeletionPredicate.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/authorization/predicate/V3RevertDeletionPredicate.java new file mode 100644 index 0000000000000000000000000000000000000000..bb6ed14ca56fc6c461fe45afbaf35f8263720dfb --- /dev/null +++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/authorization/predicate/V3RevertDeletionPredicate.java @@ -0,0 +1,42 @@ +package ch.systemsx.cisd.openbis.generic.server.authorization.predicate; + +import java.util.List; + +import ch.ethz.sis.openbis.generic.asapi.v3.dto.deletion.id.IDeletionId; +import ch.systemsx.cisd.common.exceptions.Status; +import ch.systemsx.cisd.openbis.generic.server.authorization.IAuthorizationDataProvider; +import ch.systemsx.cisd.openbis.generic.server.authorization.RoleWithIdentifier; +import ch.systemsx.cisd.openbis.generic.server.authorization.annotation.ShouldFlattenCollections; +import ch.systemsx.cisd.openbis.generic.server.authorization.predicate.v3tov1.DeletionIdTranslator; +import ch.systemsx.cisd.openbis.generic.shared.basic.TechId; +import ch.systemsx.cisd.openbis.generic.shared.dto.PersonPE; + +@ShouldFlattenCollections(value = false) +public class V3RevertDeletionPredicate extends AbstractPredicate<List<IDeletionId>> +{ + private RevertDeletionPredicate revertDeletionPredicate; + + public V3RevertDeletionPredicate() + { + this.revertDeletionPredicate = new RevertDeletionPredicate(); + } + + @Override + public final void init(IAuthorizationDataProvider provider) + { + revertDeletionPredicate.init(provider); + } + + @Override + public String getCandidateDescription() + { + return "v3 deletion id object"; + } + + @Override + protected Status doEvaluation(PersonPE person, List<RoleWithIdentifier> allowedRoles, List<IDeletionId> values) + { + List<TechId> valuesAsTechIds = DeletionIdTranslator.translate(values); + return revertDeletionPredicate.doEvaluation(person, allowedRoles, valuesAsTechIds); + } +} diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/authorization/predicate/v3tov1/DeletionIdTranslator.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/authorization/predicate/v3tov1/DeletionIdTranslator.java new file mode 100644 index 0000000000000000000000000000000000000000..37bcf7fe82134b910f3ea080368c6e4ce88c69db --- /dev/null +++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/authorization/predicate/v3tov1/DeletionIdTranslator.java @@ -0,0 +1,31 @@ +package ch.systemsx.cisd.openbis.generic.server.authorization.predicate.v3tov1; + +import java.util.ArrayList; +import java.util.List; + +import ch.ethz.sis.openbis.generic.asapi.v3.dto.deletion.id.DeletionTechId; +import ch.ethz.sis.openbis.generic.asapi.v3.dto.deletion.id.IDeletionId; +import ch.systemsx.cisd.openbis.generic.shared.basic.TechId; + +public class DeletionIdTranslator +{ + + public static TechId translate(IDeletionId v3deletionId) + { + if (v3deletionId instanceof DeletionTechId) + { + return new TechId(((DeletionTechId) v3deletionId).getTechId()); + } + return null; + } + + public static List<TechId> translate(List<IDeletionId> values) + { + List<TechId> valuesAsTechIds = new ArrayList<TechId>(); + for (IDeletionId value : values) + { + valuesAsTechIds.add(translate(value)); + } + return valuesAsTechIds; + } +} diff --git a/openbis/sourceTest/java/ch/ethz/sis/openbis/systemtest/asapi/v3/AbstractTest.java b/openbis/sourceTest/java/ch/ethz/sis/openbis/systemtest/asapi/v3/AbstractTest.java index 0eceb6b8a7e6d05ec469255ea3a76ea27fef23f4..9e2810d177229c8a391fa106b8967f35e13ea584 100644 --- a/openbis/sourceTest/java/ch/ethz/sis/openbis/systemtest/asapi/v3/AbstractTest.java +++ b/openbis/sourceTest/java/ch/ethz/sis/openbis/systemtest/asapi/v3/AbstractTest.java @@ -135,9 +135,8 @@ public class AbstractTest extends SystemTestCase protected static final String TEST_GROUP_ADMIN = "admin"; protected static final String TEST_NO_HOME_SPACE = "homeless"; - + protected static final String PASSWORD = "password"; - private BufferedAppender logRecorder; @@ -685,7 +684,17 @@ public class AbstractTest extends SystemTestCase { assertNotNull(e.getCause()); assertEquals(e.getCause().getClass(), UnauthorizedObjectAccessException.class); - assertEquals(((UnauthorizedObjectAccessException) e.getCause()).getObjectId(), expectedObjectId); + + List<? extends IObjectId> objectIds = ((UnauthorizedObjectAccessException) e.getCause()).getObjectIds(); + if (objectIds != null) + { + assertEquals(true, objectIds.contains(expectedObjectId)); + } else + { + IObjectId objectId = ((UnauthorizedObjectAccessException) e.getCause()).getObjectId(); + assertEquals(objectId, expectedObjectId); + } + assertExceptionContext(e, expectedContextPattern); } } diff --git a/openbis/sourceTest/java/ch/ethz/sis/openbis/systemtest/asapi/v3/ConfirmDeletionTest.java b/openbis/sourceTest/java/ch/ethz/sis/openbis/systemtest/asapi/v3/ConfirmDeletionTest.java index 7a6b45682b9479519b3979c4356c204be01f6f46..fb18a9ea74d9840324fc0cac46dff3f565e07206 100644 --- a/openbis/sourceTest/java/ch/ethz/sis/openbis/systemtest/asapi/v3/ConfirmDeletionTest.java +++ b/openbis/sourceTest/java/ch/ethz/sis/openbis/systemtest/asapi/v3/ConfirmDeletionTest.java @@ -185,7 +185,7 @@ public class ConfirmDeletionTest extends AbstractDeletionTest deletionOptions.setReason("It is just a test"); final IDeletionId deletionId = v3api.deleteExperiments(sessionToken, Collections.singletonList(experimentId), deletionOptions); - assertAuthorizationFailureException(new IDelegatedAction() + assertUnauthorizedObjectAccessException(new IDelegatedAction() { @Override public void execute() @@ -193,7 +193,7 @@ public class ConfirmDeletionTest extends AbstractDeletionTest String sessionToken2 = v3api.login(TEST_OBSERVER_CISD, PASSWORD); v3api.confirmDeletions(sessionToken2, Collections.singletonList(deletionId)); } - }); + }, deletionId); } @Test @@ -207,7 +207,7 @@ public class ConfirmDeletionTest extends AbstractDeletionTest deletionOptions.setReason("It is just a test"); final IDeletionId deletionId = v3api.deleteExperiments(sessionToken, Collections.singletonList(experimentId), deletionOptions); - assertAuthorizationFailureException(new IDelegatedAction() + assertUnauthorizedObjectAccessException(new IDelegatedAction() { @Override public void execute() @@ -215,7 +215,7 @@ public class ConfirmDeletionTest extends AbstractDeletionTest String sessionToken2 = v3api.login(TEST_NO_HOME_SPACE, PASSWORD); v3api.confirmDeletions(sessionToken2, Collections.singletonList(deletionId)); } - }); + }, deletionId); } } diff --git a/openbis/sourceTest/java/ch/ethz/sis/openbis/systemtest/asapi/v3/RevertDeletionTest.java b/openbis/sourceTest/java/ch/ethz/sis/openbis/systemtest/asapi/v3/RevertDeletionTest.java index e5181ed716145f4a993a7762a2c96e185d65feff..f0e3cb4bce146a3e556ec5ba823420cc638805ac 100644 --- a/openbis/sourceTest/java/ch/ethz/sis/openbis/systemtest/asapi/v3/RevertDeletionTest.java +++ b/openbis/sourceTest/java/ch/ethz/sis/openbis/systemtest/asapi/v3/RevertDeletionTest.java @@ -142,7 +142,7 @@ public class RevertDeletionTest extends AbstractDeletionTest deletionOptions.setReason("It is just a test"); final IDeletionId deletionId = v3api.deleteExperiments(sessionToken, Collections.singletonList(experimentId), deletionOptions); - assertAuthorizationFailureException(new IDelegatedAction() + assertUnauthorizedObjectAccessException(new IDelegatedAction() { @Override public void execute() @@ -150,16 +150,16 @@ public class RevertDeletionTest extends AbstractDeletionTest String sessionToken2 = v3api.login(TEST_OBSERVER_CISD, PASSWORD); v3api.revertDeletions(sessionToken2, Collections.singletonList(deletionId)); } - }); + }, deletionId); } - + @Test public void testRevertDeletionWithSamePowerUserWhoDeleted() { String sessionToken = v3api.login(TEST_POWER_USER_CISD, PASSWORD); - + ExperimentPermId experimentId = createCisdExperiment(); - + ExperimentDeletionOptions deletionOptions = new ExperimentDeletionOptions(); deletionOptions.setReason("It is just a test"); final IDeletionId deletionId = v3api.deleteExperiments(sessionToken, Collections.singletonList(experimentId), deletionOptions); @@ -167,18 +167,18 @@ public class RevertDeletionTest extends AbstractDeletionTest assertExperimentDoesNotExist(experimentId); v3api.revertDeletions(sessionToken, Collections.singletonList(deletionId)); - + assertDeletionDoesNotExist(deletionId); assertExperimentExists(experimentId); } - + @Test public void testRevertDeletionWithDifferentPowerUserWhoDeleted() { String sessionToken = v3api.login(TEST_USER, PASSWORD); - + ExperimentPermId experimentId = createCisdExperiment(); - + ExperimentDeletionOptions deletionOptions = new ExperimentDeletionOptions(); deletionOptions.setReason("It is just a test"); final IDeletionId deletionId = v3api.deleteExperiments(sessionToken, Collections.singletonList(experimentId), deletionOptions);